smiroslav commented on code in PR #1280: URL: https://github.com/apache/jackrabbit-oak/pull/1280#discussion_r1467648277
########## oak-segment-azure/src/test/java/org/apache/jackrabbit/oak/segment/azure/tool/ToolUtilsTest.java: ########## @@ -18,59 +18,95 @@ */ package org.apache.jackrabbit.oak.segment.azure.tool; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.Appender; +import ch.qos.logback.core.read.ListAppender; import com.microsoft.azure.storage.StorageCredentials; import com.microsoft.azure.storage.StorageCredentialsAccountAndKey; import com.microsoft.azure.storage.StorageCredentialsSharedAccessSignature; + +import java.net.URISyntaxException; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.stream.Collectors; +import com.microsoft.azure.storage.StorageException; +import com.microsoft.azure.storage.blob.CloudBlobDirectory; import org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzuriteDockerRule; import org.apache.jackrabbit.oak.segment.azure.AzureUtilities; import org.apache.jackrabbit.oak.segment.azure.util.Environment; +import org.jetbrains.annotations.NotNull; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; +import org.slf4j.LoggerFactory; +import static org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_ACCOUNT_NAME; +import static org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_CLIENT_ID; +import static org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_CLIENT_SECRET; +import static org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_SECRET_KEY; +import static org.apache.jackrabbit.oak.segment.azure.AzureUtilities.AZURE_TENANT_ID; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeNotNull; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mockStatic; public class ToolUtilsTest { - private static final String CONTAINER_URL = "https://myaccount.blob.core.windows.net/oak-test"; - private static final String REPO_DIR = "repository"; - private static final String SEGMENT_STORE_PATH = CONTAINER_URL + '/' + REPO_DIR; + private static final Environment ENVIRONMENT = new Environment(); + + private static final String CONTAINER_URL_FORMAT = "https://%s.blob.core.windows.net/%s"; + private static final String SEGMENT_STORE_PATH_FORMAT = CONTAINER_URL_FORMAT + "/%s"; + + private static final String DEFAULT_ACCOUNT_NAME = "myaccount"; + private static final String DEFAULT_CONTAINER_NAME = "oak"; + private static final String DEFAULT_REPO_DIR = "repository"; + private static final String DEFAULT_CONTAINER_URL = String.format(CONTAINER_URL_FORMAT, DEFAULT_ACCOUNT_NAME, DEFAULT_CONTAINER_NAME); + private static final String DEFAULT_SEGMENT_STORE_PATH = String.format(SEGMENT_STORE_PATH_FORMAT, DEFAULT_ACCOUNT_NAME, DEFAULT_CONTAINER_NAME, DEFAULT_REPO_DIR); + public static final String AZURE_SECRET_KEY_WARNING = "AZURE_CLIENT_ID, AZURE_CLIENT_SECRET and AZURE_TENANT_ID environment variables empty or missing. Switching to authentication with AZURE_SECRET_KEY."; private final TestEnvironment environment = new TestEnvironment(); @Test public void createCloudBlobDirectoryWithAccessKey() { - environment.setVariable("AZURE_SECRET_KEY", AzuriteDockerRule.ACCOUNT_KEY); + environment.setVariable(AZURE_SECRET_KEY, AzuriteDockerRule.ACCOUNT_KEY); + + final ListAppender<ILoggingEvent> logAppender = subscribeAppender(); StorageCredentialsAccountAndKey credentials = expectCredentials( StorageCredentialsAccountAndKey.class, - () -> ToolUtils.createCloudBlobDirectory(SEGMENT_STORE_PATH, environment) + () -> ToolUtils.createCloudBlobDirectory(DEFAULT_SEGMENT_STORE_PATH, environment), + DEFAULT_CONTAINER_URL ); - - assertEquals("myaccount", credentials.getAccountName()); + + assertTrue(checkLogContainsMessage(AZURE_SECRET_KEY_WARNING, logAppender.list.stream().map(ILoggingEvent::getFormattedMessage).collect(Collectors.toList()))); Review Comment: I am not in favor of this type of check. Instead, I would prefer to examine the concrete class returned (subtype of StorageCredentials) or use another method for verification. -- 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org