janhoy commented on code in PR #3750: URL: https://github.com/apache/solr/pull/3750#discussion_r2558918492
########## solr/modules/azure-blob-repository/src/test/org/apache/solr/azureblob/AbstractAzureBlobClientTest.java: ########## @@ -0,0 +1,249 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.azureblob; + +import com.azure.core.http.HttpClient; +import com.azure.core.http.okhttp.OkHttpAsyncHttpClientBuilder; +import com.azure.storage.blob.BlobServiceClient; +import com.azure.storage.blob.BlobServiceClientBuilder; +import com.carrotsearch.randomizedtesting.ThreadFilter; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters; +import java.io.IOException; +import java.io.OutputStream; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import okhttp3.OkHttpClient; +import org.apache.solr.SolrTestCaseJ4; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.rules.TemporaryFolder; + +import static org.junit.Assume.assumeTrue; + +/** + * Abstract class for tests with Azure Blob Storage emulator. + * + * <p>Uses ThreadLeakFilters to ignore OkHttp's global singleton threads (TaskRunner and Okio + * Watchdog) which are JVM-wide and cannot be shut down per-client. This is a standard pattern in + * Solr tests for HTTP clients with global thread pools. + */ +@ThreadLeakFilters( + filters = { + AbstractAzureBlobClientTest.OkHttpThreadLeakFilter.class, + }) +public class AbstractAzureBlobClientTest extends SolrTestCaseJ4 { + + protected String containerName; + + @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + + AzureBlobStorageClient client; + private static String connectionString; + // Shared OkHttpClient across all tests to reuse the global thread pools + private static OkHttpClient sharedOkHttpClient; + protected org.apache.solr.client.solrj.cloud.SocketProxy proxy; + + @BeforeClass + public static void setUpClass() { + // Create a single shared OkHttpClient for all tests + // This reuses OkHttp's global thread pools efficiently + sharedOkHttpClient = new OkHttpClient.Builder().build(); + } + + @Before + public void setUpClient() throws Exception { + // Check if Azurite is running before attempting to connect + assumeAzuriteIsRunning(); + + setAzureTestCredentials(); + + // Use Azurite connection string for local testing + connectionString = + "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://localhost:10000/devstoreaccount1;";; + + // Put a proxy in front of Azurite to simulate connection loss like S3 tests + proxy = new org.apache.solr.client.solrj.cloud.SocketProxy(); + proxy.open(new java.net.URI(getBlobServiceUrl())); + + // Reuse the shared OkHttpClient + HttpClient httpClient = new OkHttpAsyncHttpClientBuilder(sharedOkHttpClient).build(); + + // Route Blob endpoint through the proxy by adjusting the connection string + String proxiedConn = connectionString.replace(":10000", ":" + proxy.getListenPort()); + BlobServiceClient blobServiceClient = + new BlobServiceClientBuilder() + .connectionString(proxiedConn) + .httpClient(httpClient) + .buildClient(); + + containerName = "test-" + java.util.UUID.randomUUID(); + client = new AzureBlobStorageClient(blobServiceClient, containerName); + } + + /** + * Set up Azure test credentials to avoid using real Azure credentials during testing. Similar to + * how S3 tests use ProfileFileSystemSetting to avoid polluting the test environment. + */ + public static void setAzureTestCredentials() { + // Set test Azure credentials to avoid using real credentials + System.setProperty("AZURE_CLIENT_ID", "test-client-id"); + System.setProperty("AZURE_TENANT_ID", "test-tenant-id"); + System.setProperty("AZURE_CLIENT_SECRET", "test-client-secret"); + + // Set Azurite-specific environment variables + System.setProperty( + "AZURE_STORAGE_CONNECTION_STRING", + "DefaultEndpointsProtocol=http;AccountName=devstoreaccount1;AccountKey=Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==;BlobEndpoint=http://localhost:10000/devstoreaccount1;";); + } + + @After + public void tearDownClient() { + if (client != null) { + try { + client.deleteContainerForTests(); + } catch (Throwable ignored) { + } + client.close(); + } + if (proxy != null) { + proxy.close(); + proxy = null; + } + // Note: We don't clean up sharedOkHttpClient here as it's shared across all tests + // It will be cleaned up in afterAll() + } + + /** Simulate a connection loss on the proxy similar to S3 tests. */ + void initiateBlobConnectionLoss() throws AzureBlobException { + if (proxy != null) { + proxy.halfClose(); + } + } + + @AfterClass + public static void afterAll() { + // Clean up the shared OkHttpClient + if (sharedOkHttpClient != null) { + // Shutdown dispatcher's executor service + sharedOkHttpClient.dispatcher().executorService().shutdown(); + // Cancel all queued calls + sharedOkHttpClient.dispatcher().cancelAll(); + // Evict all connections + sharedOkHttpClient.connectionPool().evictAll(); + // Close cache if present + try { + if (sharedOkHttpClient.cache() != null) { + sharedOkHttpClient.cache().close(); + } + } catch (Throwable ignored) { + } + // Wait for threads to terminate + try { + sharedOkHttpClient + .dispatcher() + .executorService() + .awaitTermination(2, java.util.concurrent.TimeUnit.SECONDS); + } catch (Throwable ignored) { + } + sharedOkHttpClient = null; + } + + // Shutdown Reactor schedulers to clean up background threads + try { + reactor.core.scheduler.Schedulers.shutdownNow(); + // Give threads time to terminate + Thread.sleep(100); + } catch (Throwable ignored) { + } + } + + /** + * Helper method to push a string to Azure Blob Storage. + * + * @param path Destination path in blob storage. + * @param content Arbitrary content for the test. + */ + void pushContent(String path, String content) throws AzureBlobException { + pushContent(path, content.getBytes(StandardCharsets.UTF_8)); + } + + void pushContent(String path, byte[] content) throws AzureBlobException { + try (OutputStream output = client.pushStream(path)) { + output.write(content); + } catch (IOException e) { + throw new AzureBlobException("Failed to write content", e); + } + } + + /** Get the connection string for tests that need direct access to the blob service. */ + static String getConnectionString() { + return connectionString; + } + + /** Get the blob service URL for tests that need direct access. */ + String getBlobServiceUrl() { + return "http://localhost:10000";; + } + + /** + * Check if Azurite is running on localhost:10000. If not, skip the test. + * + * <p>Azurite must be started manually for these tests to run. This allows tests to pass in CI + * environments where Azurite may not be available. + */ + private void assumeAzuriteIsRunning() { + boolean azuriteRunning = false; + try (Socket socket = new Socket("localhost", 10000)) { Review Comment: We never hard code ports in tests, will cause weird port conflicts on CI hardware, tests running in parallel etc. Are you able to obtain a free port for this instead? Have not read all the test code, and I see some port replacements further down related to proxying.. Also I cannot find where this Azurite program is started in this test, it seems as it is a requirement of the test, in other locations it is described as an npm tool, but I cannot see initialization here? Please enlighten. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
