From 0cbaae8d373e56ef3914349b03c6171e5bfcf38c Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Tue, 11 Mar 2025 01:20:13 -0700 Subject: [PATCH 1/3] case insensitive check on hdi_isfolder --- .../fs/azurebfs/services/AbfsBlobClient.java | 23 +++- .../fs/azurebfs/services/AbfsClient.java | 3 +- .../azurebfs/services/AbfsHttpOperation.java | 6 +- .../ITestAzureBlobFileSystemMkDir.java | 117 ++++++++++++++++++ 4 files changed, 145 insertions(+), 4 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java index f184ef5f7a9b7..529b0c67f3b87 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java @@ -1528,10 +1528,31 @@ public AbfsRestOperation deleteBlobPath(final Path blobPath, */ @Override public boolean checkIsDir(AbfsHttpOperation result) { - String resourceType = result.getResponseHeader(X_MS_META_HDI_ISFOLDER); + String dirHeaderName = getHeaderNameIgnoreCase(result, X_MS_META_HDI_ISFOLDER); + // If the header is not found, return false (not a directory) + if (dirHeaderName == null) { + return false; + } + + String resourceType = result.getResponseHeader(dirHeaderName); return resourceType != null && resourceType.equals(TRUE); } + /** + * Get the header name case-insensitively. + * @param result executed rest operation containing response from server. + * @param header The header to be checked. + * @return Header if found, null otherwise. + */ + private String getHeaderNameIgnoreCase(AbfsHttpOperation result, String header) { + Map> responseHeaders = result.getResponseHeaders(); + // Search for the key case-insensitively in the headers + return responseHeaders.keySet().stream() + .filter(key -> key != null && key.equalsIgnoreCase(header)) + .findFirst() + .orElse(null); // Return null if no match is found + } + /** * Returns true if the status code lies in the range of user error. * In the case of HTTP_CONFLICT for PutBlockList we fall back to DFS and hence diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java index 58366941331c7..cb662643bea56 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java @@ -1601,7 +1601,8 @@ AbfsRestOperation getAbfsRestOperation(final AbfsRestOperationType operationType * @param requestHeaders The list of HTTP headers for the request. * @return An AbfsRestOperation instance. */ - AbfsRestOperation getAbfsRestOperation(final AbfsRestOperationType operationType, + @VisibleForTesting + public AbfsRestOperation getAbfsRestOperation(final AbfsRestOperationType operationType, final String httpMethod, final URL url, final List requestHeaders) { diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java index 017a4f4180750..0e9e4ea30eedb 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java @@ -24,6 +24,7 @@ import java.net.URL; import java.time.Duration; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -776,7 +777,7 @@ public AbfsHttpOperationWithFixedResultForGetFileStatus(final URL url, @Override public String getResponseHeader(final String httpHeader) { // Directories on FNS-Blob are identified by a special metadata header. - if (httpHeader.equals(X_MS_META_HDI_ISFOLDER)) { + if (httpHeader.equalsIgnoreCase(X_MS_META_HDI_ISFOLDER)) { return TRUE; } return EMPTY_STRING; @@ -784,7 +785,8 @@ public String getResponseHeader(final String httpHeader) { @Override public Map> getResponseHeaders() { - return new HashMap<>(); + return Collections.singletonMap(X_MS_META_HDI_ISFOLDER, + Collections.singletonList(TRUE)); } @Override diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java index e54b98e0b7a6e..123f832923767 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java @@ -18,8 +18,11 @@ package org.apache.hadoop.fs.azurebfs; +import java.net.URL; +import java.util.List; import java.util.UUID; +import org.assertj.core.api.Assertions; import org.junit.Assume; import org.junit.Test; import org.mockito.Mockito; @@ -28,14 +31,25 @@ import org.apache.hadoop.fs.FileAlreadyExistsException; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient; import org.apache.hadoop.fs.azurebfs.services.AbfsClient; +import org.apache.hadoop.fs.azurebfs.services.AbfsClientHandler; +import org.apache.hadoop.fs.azurebfs.services.AbfsHttpHeader; +import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation; +import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperationType; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CONNECTIONS_MADE; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PUT; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.TRUE; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ENABLE_MKDIR_OVERWRITE; import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_FS_AZURE_ENABLE_MKDIR_OVERWRITE; +import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_METADATA_PREFIX; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs; import static org.apache.hadoop.test.LambdaTestUtils.intercept; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; /** * Test mkdir operation. @@ -167,4 +181,107 @@ public void testMkdirWithExistingFilename() throws Exception { intercept(FileAlreadyExistsException.class, () -> fs.mkdirs(new Path("/testFilePath"))); intercept(FileAlreadyExistsException.class, () -> fs.mkdirs(new Path("/testFilePath/newDir"))); } + + /** + * Test mkdirs with HDI folder configuration, + * verifying the correct header and directory state. + */ + @Test + public void testMkdirsWithDifferentCaseHDIConfig() throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) { + assumeBlobServiceType(); + AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs); + String configName = X_MS_METADATA_PREFIX + "Hdi_isfolder"; + // Mock the operation to modify the headers + mockAbfsRestOperation(abfsBlobClient, configName); + + // Create the path and invoke mkdirs method + Path path = new Path("/testPath"); + fs.mkdirs(path); + + // Assert that the response header has the updated value + AbfsHttpOperation op = abfsBlobClient.getPathStatus(path.toUri().getPath(), + true, getTestTracingContext(fs, true), + null).getResult(); + + // Verify the header and directory state + Assertions.assertThat(op.getResponseHeader(configName)) + .describedAs("Header should be set to true") + .isEqualTo(TRUE); + Assertions.assertThat(abfsBlobClient.checkIsDir(op)) + .describedAs("Directory should be marked as true") + .isTrue(); + } + } + + /** + * Test mkdirs with wrong HDI folder configuration, + * verifying the correct header and directory state. + */ + @Test + public void testMkdirsWithWrongHDIConfig() throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) { + assumeBlobServiceType(); + AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs); + String configName = X_MS_METADATA_PREFIX + "Hdi_isfolder1"; + + // Mock the operation to modify the headers + mockAbfsRestOperation(abfsBlobClient, configName); + + // Create the path and invoke mkdirs method + Path path = new Path("/testPath"); + fs.mkdirs(path); + + // Assert the header and directory state + AbfsHttpOperation op = abfsBlobClient.getPathStatus(path.toUri().getPath(), + true, getTestTracingContext(fs, true), + null).getResult(); + + // Verify the header and directory state + Assertions.assertThat(op.getResponseHeader(configName)) + .describedAs("Header should be set to TRUE") + .isEqualTo(TRUE); + Assertions.assertThat(abfsBlobClient.checkIsDir(op)) + .describedAs("No Directory config set, should be marked as false") + .isFalse(); + } + } + + /** + * Helper method to mock the AbfsRestOperation and modify the request headers. + * + * @param abfsBlobClient the mocked AbfsBlobClient + * @param newHeader the header to add in place of the old one + */ + private void mockAbfsRestOperation(AbfsBlobClient abfsBlobClient, String newHeader) { + Mockito.doAnswer(invocation -> { + List requestHeaders = invocation.getArgument(3); + + // Remove the actual HDI config header and add the new one + requestHeaders.removeIf(header -> + HttpHeaderConfigurations.X_MS_META_HDI_ISFOLDER.equals(header.getName())); + requestHeaders.add(new AbfsHttpHeader(newHeader, TRUE)); + + // Call the real method + return invocation.callRealMethod(); + }).when(abfsBlobClient).getAbfsRestOperation(eq(AbfsRestOperationType.PutBlob), + eq(HTTP_METHOD_PUT), any(URL.class), anyList()); + } + + /** + * Helper method to mock the AbfsBlobClient and set up the client handler. + * + * @param fs the AzureBlobFileSystem instance + * @return the mocked AbfsBlobClient + */ + private AbfsBlobClient mockIngressClientHandler(AzureBlobFileSystem fs) { + AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore()); + AbfsClientHandler clientHandler = Mockito.spy(store.getClientHandler()); + AbfsBlobClient abfsBlobClient = (AbfsBlobClient) Mockito.spy( + clientHandler.getClient()); + fs.getAbfsStore().setClient(abfsBlobClient); + fs.getAbfsStore().setClientHandler(clientHandler); + Mockito.doReturn(abfsBlobClient).when(clientHandler).getIngressClient(); + return abfsBlobClient; + } } From da8d546c0f5e25c71a12901380adc6739537a4f3 Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Wed, 2 Apr 2025 03:28:04 -0700 Subject: [PATCH 2/3] Added Test cases --- .../contracts/services/BlobListXmlParser.java | 2 +- .../services/AbfsAHCHttpOperation.java | 16 +++ .../fs/azurebfs/services/AbfsBlobClient.java | 26 +--- .../azurebfs/services/AbfsHttpOperation.java | 40 +++++- .../services/AbfsJdkHttpOperation.java | 16 +++ .../ITestAzureBlobFileSystemFileStatus.java | 60 +++++++++ .../ITestAzureBlobFileSystemListStatus.java | 114 +++++++++++++++++ .../ITestAzureBlobFileSystemMkDir.java | 117 ------------------ 8 files changed, 244 insertions(+), 147 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/BlobListXmlParser.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/BlobListXmlParser.java index 9cfbf23789884..592074863121b 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/BlobListXmlParser.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/BlobListXmlParser.java @@ -207,7 +207,7 @@ public void endElement(final String uri, if (parentNode.equals(AbfsHttpConstants.XML_TAG_METADATA)) { currentBlobEntry.addMetadata(currentNode, value); // For Marker blobs hdi_isFolder will be present as metadata - if (AbfsHttpConstants.XML_TAG_HDI_ISFOLDER.equals(currentNode)) { + if (AbfsHttpConstants.XML_TAG_HDI_ISFOLDER.equalsIgnoreCase(currentNode)) { currentBlobEntry.setIsDirectory(Boolean.valueOf(value)); } } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAHCHttpOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAHCHttpOperation.java index dd585d32fb7cf..2569c6a4bd3d0 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAHCHttpOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsAHCHttpOperation.java @@ -312,6 +312,22 @@ public Map> getResponseHeaders() { return headers; } + /**{@inheritDoc}*/ + @Override + public String getResponseHeaderIgnoreCase(final String headerName) { + Map> responseHeaders = getResponseHeaders(); + if (responseHeaders == null || responseHeaders.isEmpty()) { + return null; + } + // Search for the header value case-insensitively + return responseHeaders.entrySet().stream() + .filter(entry -> entry.getKey() != null + && entry.getKey().equalsIgnoreCase(headerName)) + .flatMap(entry -> entry.getValue().stream()) + .findFirst() + .orElse(null); // Return null if no match is found + } + /**{@inheritDoc}*/ @Override protected InputStream getContentInputStream() diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java index 54efb7600168d..32eba86774a8c 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java @@ -1137,7 +1137,8 @@ public AbfsRestOperation setPathProperties(final String path, this.createPathRestOp(path, false, false, false, null, contextEncryptionAdapter, tracingContext); // Make sure hdi_isFolder is added to the list of properties to be set. - boolean hdiIsFolderExists = properties.containsKey(XML_TAG_HDI_ISFOLDER); + boolean hdiIsFolderExists = properties.keySet() + .stream().anyMatch(XML_TAG_HDI_ISFOLDER::equalsIgnoreCase); if (!hdiIsFolderExists) { properties.put(XML_TAG_HDI_ISFOLDER, TRUE); } @@ -1548,31 +1549,10 @@ public AbfsRestOperation deleteBlobPath(final Path blobPath, */ @Override public boolean checkIsDir(AbfsHttpOperation result) { - String dirHeaderName = getHeaderNameIgnoreCase(result, X_MS_META_HDI_ISFOLDER); - // If the header is not found, return false (not a directory) - if (dirHeaderName == null) { - return false; - } - - String resourceType = result.getResponseHeader(dirHeaderName); + String resourceType = result.getResponseHeaderIgnoreCase(X_MS_META_HDI_ISFOLDER); return resourceType != null && resourceType.equals(TRUE); } - /** - * Get the header name case-insensitively. - * @param result executed rest operation containing response from server. - * @param header The header to be checked. - * @return Header if found, null otherwise. - */ - private String getHeaderNameIgnoreCase(AbfsHttpOperation result, String header) { - Map> responseHeaders = result.getResponseHeaders(); - // Search for the key case-insensitively in the headers - return responseHeaders.keySet().stream() - .filter(key -> key != null && key.equalsIgnoreCase(header)) - .findFirst() - .orElse(null); // Return null if no match is found - } - /** * Returns true if the status code lies in the range of user error. * In the case of HTTP_CONFLICT for PutBlockList we fall back to DFS and hence diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java index da5371a40f414..89c97b68baa69 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsHttpOperation.java @@ -24,7 +24,6 @@ import java.net.URL; import java.time.Duration; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -236,6 +235,14 @@ public final InputStream getListResultStream() { public abstract Map> getResponseHeaders(); + /** + * Get response header value for the given headerKey ignoring case. + * + * @param httpHeader header key. + * @return header value. + */ + public abstract String getResponseHeaderIgnoreCase(String httpHeader); + // Returns a trace message for the request @Override public String toString() { @@ -744,7 +751,7 @@ public String getTracingContextSuffix() { @Override public String getResponseHeader(final String httpHeader) { - return ""; + return EMPTY_STRING; } @Override @@ -752,6 +759,12 @@ public Map> getResponseHeaders() { return new HashMap<>(); } + /**{@inheritDoc}*/ + @Override + public String getResponseHeaderIgnoreCase(final String headerName) { + return EMPTY_STRING; + } + @Override public void sendPayload(final byte[] buffer, final int offset, @@ -777,7 +790,7 @@ public AbfsHttpOperationWithFixedResultForGetFileStatus(final URL url, @Override public String getResponseHeader(final String httpHeader) { // Directories on FNS-Blob are identified by a special metadata header. - if (httpHeader.equalsIgnoreCase(X_MS_META_HDI_ISFOLDER)) { + if (httpHeader.equals(X_MS_META_HDI_ISFOLDER)) { return TRUE; } return EMPTY_STRING; @@ -785,8 +798,17 @@ public String getResponseHeader(final String httpHeader) { @Override public Map> getResponseHeaders() { - return Collections.singletonMap(X_MS_META_HDI_ISFOLDER, - Collections.singletonList(TRUE)); + return new HashMap<>(); + } + + /**{@inheritDoc}*/ + @Override + public String getResponseHeaderIgnoreCase(final String httpHeader) { + // Directories on FNS-Blob are identified by a special metadata header. + if (httpHeader.equalsIgnoreCase(X_MS_META_HDI_ISFOLDER)) { + return TRUE; + } + return EMPTY_STRING; } @Override @@ -937,7 +959,7 @@ public String getTracingContextSuffix() { @Override public String getResponseHeader(final String httpHeader) { - return ""; + return EMPTY_STRING; } @Override @@ -945,6 +967,12 @@ public Map> getResponseHeaders() { return new HashMap<>(); } + /**{@inheritDoc}*/ + @Override + public String getResponseHeaderIgnoreCase(final String headerName) { + return EMPTY_STRING; + } + @Override public void sendPayload(final byte[] buffer, final int offset, diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsJdkHttpOperation.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsJdkHttpOperation.java index 20fe215e03954..c233fae9e3763 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsJdkHttpOperation.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsJdkHttpOperation.java @@ -104,6 +104,22 @@ public Map> getResponseHeaders() { return connection.getHeaderFields(); } + /**{@inheritDoc}*/ + @Override + public String getResponseHeaderIgnoreCase(final String headerName) { + Map> responseHeaders = getResponseHeaders(); + if (responseHeaders == null || responseHeaders.isEmpty()) { + return null; + } + // Search for the header value case-insensitively + return responseHeaders.entrySet().stream() + .filter(entry -> entry.getKey() != null + && entry.getKey().equalsIgnoreCase(headerName)) + .flatMap(entry -> entry.getValue().stream()) + .findFirst() + .orElse(null); // Return null if no match is found + } + /**{@inheritDoc}*/ public void sendPayload(byte[] buffer, int offset, int length) throws IOException { diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemFileStatus.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemFileStatus.java index f226e5b61fba7..2d7298f1e1bd4 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemFileStatus.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemFileStatus.java @@ -29,9 +29,12 @@ import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient; +import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation; import org.apache.hadoop.fs.permission.FsPermission; import static org.apache.hadoop.fs.CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY; +import static org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemListStatus.mockAbfsRestOperation; +import static org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemListStatus.mockIngressClientHandler; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists; import static org.apache.hadoop.test.LambdaTestUtils.intercept; import static org.mockito.ArgumentMatchers.any; @@ -284,4 +287,61 @@ private void verifyFileNotFound(FileNotFoundException ex, String key) { Assertions.assertThat(ex).isNotNull(); Assertions.assertThat(ex.getMessage()).contains(key); } + + /** + * Test directory status with different HDI folder configuration, + * verifying the correct header and directory state. + */ + private void testIsDirectory(boolean expected, String... configName) throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) { + assumeBlobServiceType(); + AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs); + // Mock the operation to modify the headers + mockAbfsRestOperation(abfsBlobClient, configName); + + // Create the path and invoke mkdirs method + Path path = new Path("/testPath"); + fs.mkdirs(path); + + // Assert that the response header has the updated value + FileStatus fileStatus = fs.getFileStatus(path); + + AbfsHttpOperation op = abfsBlobClient.getPathStatus( + path.toUri().getPath(), + true, getTestTracingContext(fs, true), + null).getResult(); + + Assertions.assertThat(abfsBlobClient.checkIsDir(op)) + .describedAs("Directory should be marked as " + expected) + .isEqualTo(expected); + + // Verify the header and directory state + Assertions.assertThat(fileStatus.isDirectory()) + .describedAs("Expected directory state: " + expected) + .isEqualTo(expected); + + fs.delete(path, true); + } + } + + /** + * Test to verify the directory status with different HDI folder configurations. + * Verifying the correct header and directory state. + */ + @Test + public void testIsDirectoryWithDifferentCases() throws Exception { + testIsDirectory(true, "HDI_ISFOLDER"); + + testIsDirectory(true, "Hdi_ISFOLDER"); + + testIsDirectory(true, "Hdi_isfolder"); + + testIsDirectory(true, "hdi_isfolder"); + + testIsDirectory(false, "Hdi_isfolder1"); + + testIsDirectory(true, "HDI_ISFOLDER", "Hdi_ISFOLDER", "Hdi_isfolder"); + + testIsDirectory(true, "HDI_ISFOLDER", "Hdi_ISFOLDER1", "Test"); + } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java index 1c4ef6d421272..cc658dd5d93e3 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java @@ -21,6 +21,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.net.SocketTimeoutException; +import java.net.URL; import java.util.ArrayList; import java.util.List; import java.util.concurrent.Callable; @@ -40,7 +41,13 @@ import org.apache.hadoop.fs.LocatedFileStatus; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.azurebfs.constants.FSOperationType; +import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; +import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient; +import org.apache.hadoop.fs.azurebfs.services.AbfsClientHandler; +import org.apache.hadoop.fs.azurebfs.services.AbfsHttpHeader; +import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation; +import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperationType; import org.apache.hadoop.fs.azurebfs.services.ListResponseData; import org.apache.hadoop.fs.azurebfs.services.AbfsClient; import org.apache.hadoop.fs.azurebfs.services.AbfsClientTestUtil; @@ -51,8 +58,11 @@ import org.apache.hadoop.fs.contract.ContractTestUtils; import static java.net.HttpURLConnection.HTTP_OK; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PUT; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ROOT_PATH; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.TRUE; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_LIST_MAX_RESULTS; +import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_METADATA_PREFIX; import static org.apache.hadoop.fs.azurebfs.services.RenameAtomicity.SUFFIX; import static org.apache.hadoop.fs.azurebfs.services.RetryReasonConstants.CONNECTION_TIMEOUT_JDK_MESSAGE; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs; @@ -62,6 +72,8 @@ import static org.apache.hadoop.test.LambdaTestUtils.intercept; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; @@ -524,4 +536,106 @@ private void assertDirectoryFileStatus(final FileStatus fileStatus, Assertions.assertThat(fileStatus.getLen()) .describedAs("Content Length Not as Expected").isEqualTo(0); } + + /** + * Helper method to mock the AbfsRestOperation and modify the request headers. + * + * @param abfsBlobClient the mocked AbfsBlobClient + * @param newHeader the header to add in place of the old one + */ + public static void mockAbfsRestOperation(AbfsBlobClient abfsBlobClient, String ...newHeader) { + Mockito.doAnswer(invocation -> { + List requestHeaders = invocation.getArgument(3); + + // Remove the actual HDI config header and add the new one + requestHeaders.removeIf(header -> + HttpHeaderConfigurations.X_MS_META_HDI_ISFOLDER.equals(header.getName())); + for (String header : newHeader) { + requestHeaders.add(new AbfsHttpHeader(X_MS_METADATA_PREFIX + header, TRUE)); + } + + // Call the real method + return invocation.callRealMethod(); + }).when(abfsBlobClient).getAbfsRestOperation(eq(AbfsRestOperationType.PutBlob), + eq(HTTP_METHOD_PUT), any(URL.class), anyList()); + } + + /** + * Helper method to mock the AbfsBlobClient and set up the client handler. + * + * @param fs the AzureBlobFileSystem instance + * @return the mocked AbfsBlobClient + */ + public static AbfsBlobClient mockIngressClientHandler(AzureBlobFileSystem fs) { + AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore()); + AbfsClientHandler clientHandler = Mockito.spy(store.getClientHandler()); + AbfsBlobClient abfsBlobClient = (AbfsBlobClient) Mockito.spy( + clientHandler.getClient()); + fs.getAbfsStore().setClient(abfsBlobClient); + fs.getAbfsStore().setClientHandler(clientHandler); + Mockito.doReturn(abfsBlobClient).when(clientHandler).getIngressClient(); + return abfsBlobClient; + } + + /** + * Test directory status with different HDI folder configuration, + * verifying the correct header and directory state. + */ + private void testIsDirectory(boolean expected, String... configName) throws Exception { + try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) { + assumeBlobServiceType(); + AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs); + // Mock the operation to modify the headers + mockAbfsRestOperation(abfsBlobClient, configName); + + // Create the path and invoke mkdirs method + Path path = new Path("/testPath"); + fs.mkdirs(path); + + // Assert that the response header has the updated value + FileStatus[] fileStatus = fs.listStatus(path.getParent()); + + AbfsHttpOperation op = abfsBlobClient.getPathStatus( + path.toUri().getPath(), + true, getTestTracingContext(fs, true), + null).getResult(); + + Assertions.assertThat(abfsBlobClient.checkIsDir(op)) + .describedAs("Directory should be marked as " + expected) + .isEqualTo(expected); + + // Verify the header and directory state + Assertions.assertThat(fileStatus.length) + .describedAs("Expected directory state: " + expected) + .isEqualTo(1); + + // Verify the header and directory state + Assertions.assertThat(fileStatus[0].isDirectory()) + .describedAs("Expected directory state: " + expected) + .isEqualTo(expected); + + fs.delete(path, true); + } + } + + /** + * Test to verify the directory status with different HDI folder configurations. + * Verifying the correct header and directory state. + */ + @Test + public void testIsDirectoryWithDifferentCases() throws Exception { + testIsDirectory(true, "HDI_ISFOLDER"); + + testIsDirectory(true, "Hdi_ISFOLDER"); + + testIsDirectory(true, "Hdi_isfolder"); + + testIsDirectory(true, "hdi_isfolder"); + + testIsDirectory(false, "Hdi_isfolder1"); + + testIsDirectory(true, "HDI_ISFOLDER", "Hdi_ISFOLDER", "Hdi_isfolder"); + + testIsDirectory(true, "HDI_ISFOLDER", "Hdi_ISFOLDER1", "Test"); + } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java index 123f832923767..e54b98e0b7a6e 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemMkDir.java @@ -18,11 +18,8 @@ package org.apache.hadoop.fs.azurebfs; -import java.net.URL; -import java.util.List; import java.util.UUID; -import org.assertj.core.api.Assertions; import org.junit.Assume; import org.junit.Test; import org.mockito.Mockito; @@ -31,25 +28,14 @@ import org.apache.hadoop.fs.FileAlreadyExistsException; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations; import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient; import org.apache.hadoop.fs.azurebfs.services.AbfsClient; -import org.apache.hadoop.fs.azurebfs.services.AbfsClientHandler; -import org.apache.hadoop.fs.azurebfs.services.AbfsHttpHeader; -import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation; -import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperationType; import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.CONNECTIONS_MADE; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PUT; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.TRUE; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ENABLE_MKDIR_OVERWRITE; import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_FS_AZURE_ENABLE_MKDIR_OVERWRITE; -import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_METADATA_PREFIX; import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs; import static org.apache.hadoop.test.LambdaTestUtils.intercept; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.eq; /** * Test mkdir operation. @@ -181,107 +167,4 @@ public void testMkdirWithExistingFilename() throws Exception { intercept(FileAlreadyExistsException.class, () -> fs.mkdirs(new Path("/testFilePath"))); intercept(FileAlreadyExistsException.class, () -> fs.mkdirs(new Path("/testFilePath/newDir"))); } - - /** - * Test mkdirs with HDI folder configuration, - * verifying the correct header and directory state. - */ - @Test - public void testMkdirsWithDifferentCaseHDIConfig() throws Exception { - try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) { - assumeBlobServiceType(); - AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs); - String configName = X_MS_METADATA_PREFIX + "Hdi_isfolder"; - // Mock the operation to modify the headers - mockAbfsRestOperation(abfsBlobClient, configName); - - // Create the path and invoke mkdirs method - Path path = new Path("/testPath"); - fs.mkdirs(path); - - // Assert that the response header has the updated value - AbfsHttpOperation op = abfsBlobClient.getPathStatus(path.toUri().getPath(), - true, getTestTracingContext(fs, true), - null).getResult(); - - // Verify the header and directory state - Assertions.assertThat(op.getResponseHeader(configName)) - .describedAs("Header should be set to true") - .isEqualTo(TRUE); - Assertions.assertThat(abfsBlobClient.checkIsDir(op)) - .describedAs("Directory should be marked as true") - .isTrue(); - } - } - - /** - * Test mkdirs with wrong HDI folder configuration, - * verifying the correct header and directory state. - */ - @Test - public void testMkdirsWithWrongHDIConfig() throws Exception { - try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) { - assumeBlobServiceType(); - AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs); - String configName = X_MS_METADATA_PREFIX + "Hdi_isfolder1"; - - // Mock the operation to modify the headers - mockAbfsRestOperation(abfsBlobClient, configName); - - // Create the path and invoke mkdirs method - Path path = new Path("/testPath"); - fs.mkdirs(path); - - // Assert the header and directory state - AbfsHttpOperation op = abfsBlobClient.getPathStatus(path.toUri().getPath(), - true, getTestTracingContext(fs, true), - null).getResult(); - - // Verify the header and directory state - Assertions.assertThat(op.getResponseHeader(configName)) - .describedAs("Header should be set to TRUE") - .isEqualTo(TRUE); - Assertions.assertThat(abfsBlobClient.checkIsDir(op)) - .describedAs("No Directory config set, should be marked as false") - .isFalse(); - } - } - - /** - * Helper method to mock the AbfsRestOperation and modify the request headers. - * - * @param abfsBlobClient the mocked AbfsBlobClient - * @param newHeader the header to add in place of the old one - */ - private void mockAbfsRestOperation(AbfsBlobClient abfsBlobClient, String newHeader) { - Mockito.doAnswer(invocation -> { - List requestHeaders = invocation.getArgument(3); - - // Remove the actual HDI config header and add the new one - requestHeaders.removeIf(header -> - HttpHeaderConfigurations.X_MS_META_HDI_ISFOLDER.equals(header.getName())); - requestHeaders.add(new AbfsHttpHeader(newHeader, TRUE)); - - // Call the real method - return invocation.callRealMethod(); - }).when(abfsBlobClient).getAbfsRestOperation(eq(AbfsRestOperationType.PutBlob), - eq(HTTP_METHOD_PUT), any(URL.class), anyList()); - } - - /** - * Helper method to mock the AbfsBlobClient and set up the client handler. - * - * @param fs the AzureBlobFileSystem instance - * @return the mocked AbfsBlobClient - */ - private AbfsBlobClient mockIngressClientHandler(AzureBlobFileSystem fs) { - AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore()); - AbfsClientHandler clientHandler = Mockito.spy(store.getClientHandler()); - AbfsBlobClient abfsBlobClient = (AbfsBlobClient) Mockito.spy( - clientHandler.getClient()); - fs.getAbfsStore().setClient(abfsBlobClient); - fs.getAbfsStore().setClientHandler(clientHandler); - Mockito.doReturn(abfsBlobClient).when(clientHandler).getIngressClient(); - return abfsBlobClient; - } } From ecfe2820791be8f5f48660dee962f47e3a4c3aa2 Mon Sep 17 00:00:00 2001 From: bhattmanish98 Date: Wed, 2 Apr 2025 08:16:45 -0700 Subject: [PATCH 3/3] Checkstyle fix --- .../hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java index cc658dd5d93e3..7f7d96bd4773b 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemListStatus.java @@ -543,7 +543,7 @@ private void assertDirectoryFileStatus(final FileStatus fileStatus, * @param abfsBlobClient the mocked AbfsBlobClient * @param newHeader the header to add in place of the old one */ - public static void mockAbfsRestOperation(AbfsBlobClient abfsBlobClient, String ...newHeader) { + public static void mockAbfsRestOperation(AbfsBlobClient abfsBlobClient, String... newHeader) { Mockito.doAnswer(invocation -> { List requestHeaders = invocation.getArgument(3);