From 4580d3b12e620c2185f9096dac18f1fe60c90949 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Thu, 4 Jan 2024 04:32:59 -0800 Subject: [PATCH 01/13] Added Support for Paginated Delete --- .../hadoop/fs/azurebfs/AbfsConfiguration.java | 13 +- .../azurebfs/constants/AbfsHttpConstants.java | 1 + .../azurebfs/constants/ConfigurationKeys.java | 5 + .../constants/FileSystemConfigurations.java | 1 + .../azurebfs/constants/HttpQueryParams.java | 1 + .../fs/azurebfs/services/AbfsClient.java | 16 +- .../fs/azurebfs/services/AbfsClientUtils.java | 13 + .../fs/azurebfs/services/ITestAbfsClient.java | 2 +- .../services/ITestAbfsPaginatedDelete.java | 279 ++++++++++++++++++ 9 files changed, 326 insertions(+), 5 deletions(-) create mode 100644 hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index e1bc8aa57b263..4a822aa1badae 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -338,6 +338,10 @@ public class AbfsConfiguration{ FS_AZURE_ABFS_RENAME_RESILIENCE, DefaultValue = DEFAULT_ENABLE_ABFS_RENAME_RESILIENCE) private boolean renameResilience; + @BooleanConfigurationValidatorAnnotation(ConfigurationKey = + FS_AZURE_ENABLE_PAGINATED_DELETE, DefaultValue = DEFAULT_ENABLE_PAGINATED_DELETE) + private boolean isPaginatedDeleteEnabled; + private String clientProvidedEncryptionKey; private String clientProvidedEncryptionKeySHA; @@ -1191,7 +1195,12 @@ public boolean getRenameResilience() { return renameResilience; } - void setRenameResilience(boolean actualResilience) { - renameResilience = actualResilience; + public boolean isPaginatedDeleteEnabled() { + return isPaginatedDeleteEnabled; + } + + @VisibleForTesting + public void setIsPaginatedDeleteEnabled(boolean isPaginatedDeleteEnabled) { + this.isPaginatedDeleteEnabled = isPaginatedDeleteEnabled; } } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java index 91f6bddcc1d46..00784bdc770c0 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java @@ -121,6 +121,7 @@ public final class AbfsHttpConstants { public static final char CHAR_PLUS = '+'; public static final String DECEMBER_2019_API_VERSION = "2019-12-12"; public static final String APRIL_2021_API_VERSION = "2021-04-10"; + public static final String AUGUST_2023_API_VERSION = "2023-08-03"; /** * Value that differentiates categories of the http_status. diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/ConfigurationKeys.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/ConfigurationKeys.java index 91f9eff532888..ae76938efb5a9 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/ConfigurationKeys.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/ConfigurationKeys.java @@ -247,6 +247,11 @@ public final class ConfigurationKeys { /** Add extra resilience to rename failures, at the expense of performance. */ public static final String FS_AZURE_ABFS_RENAME_RESILIENCE = "fs.azure.enable.rename.resilience"; + /** + * Specify whether paginated behavior is to be expected or not in delete path. + */ + public static final String FS_AZURE_ENABLE_PAGINATED_DELETE = "fs.azure.enable.paginated.delete"; + public static String accountProperty(String property, String account) { return property + "." + account; } diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java index 00fc4a6a3db77..798867750b000 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java @@ -119,6 +119,7 @@ public final class FileSystemConfigurations { public static final int STREAM_ID_LEN = 12; public static final boolean DEFAULT_ENABLE_ABFS_LIST_ITERATOR = true; public static final boolean DEFAULT_ENABLE_ABFS_RENAME_RESILIENCE = true; + public static final boolean DEFAULT_ENABLE_PAGINATED_DELETE = false; /** * Limit of queued block upload operations before writes diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/HttpQueryParams.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/HttpQueryParams.java index e9bb95cad21cd..f7e37dcb6d50d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/HttpQueryParams.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/HttpQueryParams.java @@ -40,6 +40,7 @@ public final class HttpQueryParams { public static final String QUERY_PARAM_CLOSE = "close"; public static final String QUERY_PARAM_UPN = "upn"; public static final String QUERY_PARAM_BLOBTYPE = "blobtype"; + public static final String QUERY_PARAM_PAGINATED = "paginated"; //query params for SAS public static final String QUERY_PARAM_SAOID = "saoid"; 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 8eeb548f500b4..f11692f9adfb0 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 @@ -1053,12 +1053,24 @@ public AbfsRestOperation read(final String path, return op; } - public AbfsRestOperation deletePath(final String path, final boolean recursive, final String continuation, + public AbfsRestOperation deletePath(final String path, final boolean recursive, + final String continuation, TracingContext tracingContext) throws AzureBlobFileSystemException { final List requestHeaders = createDefaultHeaders(); - final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); + + if (abfsConfiguration.isPaginatedDeleteEnabled() && recursive) { + // Change the x-ms-version to "2023-08-03" if its less than that. + if (xMsVersion.compareTo(AUGUST_2023_API_VERSION) < 0) { + requestHeaders.removeIf(header -> header.getName().equalsIgnoreCase(X_MS_VERSION)); + requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, AUGUST_2023_API_VERSION)); + } + + // Add paginated query parameter + abfsUriQueryBuilder.addQuery(QUERY_PARAM_PAGINATED, TRUE); + } + abfsUriQueryBuilder.addQuery(QUERY_PARAM_RECURSIVE, String.valueOf(recursive)); abfsUriQueryBuilder.addQuery(QUERY_PARAM_CONTINUATION, continuation); String operation = recursive ? SASTokenProvider.DELETE_RECURSIVE_OPERATION : SASTokenProvider.DELETE_OPERATION; diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java index e7dbf208c9b06..3dbd7bd4aad71 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java @@ -18,8 +18,12 @@ package org.apache.hadoop.fs.azurebfs.services; +import java.util.List; + import org.apache.hadoop.fs.azurebfs.extensions.EncryptionContextProvider; +import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_VERSION; + public final class AbfsClientUtils { private AbfsClientUtils() { @@ -31,4 +35,13 @@ public static void setIsNamespaceEnabled(final AbfsClient abfsClient, final Bool public static void setEncryptionContextProvider(final AbfsClient abfsClient, final EncryptionContextProvider provider) { abfsClient.setEncryptionContextProvider(provider); } + + public static String getHeaderValue(List reqHeaders, String headerName) { + for (AbfsHttpHeader header : reqHeaders) { + if (header.getName().equals(headerName)) { + return header.getValue(); + } + } + return ""; + } } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java index 4f87e02000249..10da0400640a5 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java @@ -410,7 +410,7 @@ public static AbfsClient getMockAbfsClient(AbfsClient baseAbfsClientInstance, return client; } - private static AbfsClient setAbfsClientField( + static AbfsClient setAbfsClientField( final AbfsClient client, final String fieldName, Object fieldObject) throws Exception { diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java new file mode 100644 index 0000000000000..b22dd69831c3c --- /dev/null +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -0,0 +1,279 @@ +/** + * 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.hadoop.fs.azurebfs.services; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; +import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest; +import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem; +import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; +import org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider; +import org.apache.hadoop.fs.azurebfs.utils.AclTestHelpers; +import org.apache.hadoop.fs.azurebfs.utils.TracingContext; +import org.apache.hadoop.fs.permission.AclEntry; +import org.apache.hadoop.fs.permission.AclEntryScope; +import org.apache.hadoop.fs.permission.AclEntryType; +import org.apache.hadoop.fs.permission.FsAction; +import org.apache.hadoop.util.Lists; + +import org.assertj.core.api.Assertions; +import org.junit.Assume; +import org.junit.Test; + +import java.io.IOException; +import java.util.List; +import java.util.UUID; + +import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; +import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.AUGUST_2023_API_VERSION; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DECEMBER_2019_API_VERSION; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION; +import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME; +import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_VERSION; +import static org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams.QUERY_PARAM_PAGINATED; +import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID; +import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET; +import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID; +import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CLIENT_ID; +import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CLIENT_SECRET; +import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT; +import static org.apache.hadoop.fs.azurebfs.services.AbfsClientUtils.getHeaderValue; +import static org.apache.hadoop.test.LambdaTestUtils.intercept; + +public class ITestAbfsPaginatedDelete extends AbstractAbfsIntegrationTest { + + private AzureBlobFileSystem superUserFs; + private AzureBlobFileSystem firstTestUserFs; + private String firstTestUserGuid; + + private boolean isHnsEnabled; + public ITestAbfsPaginatedDelete() throws Exception { + } + + @Override + public void setup() throws Exception { + isHnsEnabled = this.getConfiguration().getBoolean(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false); + loadConfiguredFileSystem(); + super.setup(); + this.superUserFs = getFileSystem(); + this.firstTestUserGuid = getConfiguration() + .get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); + + if(isHnsEnabled) { + // setting up ACL permissions for test user + setFirstTestUserFsAuth(); + setDefaultAclOnRoot(this.firstTestUserGuid); + } + } + + /** + * Test to check that recursive deletePath works with paginated enabled and + * disabled for both empty and non-empty directory. + * When enabled appropriate xMsVersion should be used. + * @throws Exception + */ + @Test + public void testRecursiveDeleteWithPagination() throws Exception { + testRecursiveDeleteWithPaginationInternal(false, true, DECEMBER_2019_API_VERSION); + testRecursiveDeleteWithPaginationInternal(false, true, AUGUST_2023_API_VERSION); + testRecursiveDeleteWithPaginationInternal(false, false, DECEMBER_2019_API_VERSION); + testRecursiveDeleteWithPaginationInternal(false, false, AUGUST_2023_API_VERSION); + testRecursiveDeleteWithPaginationInternal(true, true, DECEMBER_2019_API_VERSION); + testRecursiveDeleteWithPaginationInternal(true, false, AUGUST_2023_API_VERSION); + } + + /** + * Test to check that non-recursive delete works with both paginated enabled + * and disabled only for empty directories. + * Pagination should not be set when recursive is false. + * @throws Exception + */ + @Test + public void testNonRecursiveDeleteWithPagination() throws Exception { + testNonRecursiveDeleteWithPaginationInternal(true); + testNonRecursiveDeleteWithPaginationInternal(false); + } + + /** + * Test to check that with pagination enabled, invalid CT will fail + * @throws Exception + */ + @Test + public void testRecursiveDeleteWithInvalidCT() throws Exception { + testRecursiveDeleteWithInvalidCTInternal(true); + testRecursiveDeleteWithInvalidCTInternal(false); + } + + public void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolean isPaginatedDeleteEnabled, + String xMsVersion) throws Exception { + final AzureBlobFileSystem fs = isHnsEnabled ? this.firstTestUserFs : getFileSystem(); + TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); + Path testPath; + if (isEmptyDir) { + testPath = new Path("/emptyPath"); + fs.mkdirs(testPath); + } else { + testPath = createSmallDir(); + } + + // Set the paginated enabled value and xMsVersion at client level. + AbfsClient client = ITestAbfsClient.setAbfsClientField( + fs.getAbfsStore().getClient(), "xMsVersion", xMsVersion); + client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); + + AbfsRestOperation op = client.deletePath(testPath.toString(), true, null, testTracingContext); + + // Getting the xMsVersion that was used to make the request + String xMsVersionUsed = getHeaderValue(op.getRequestHeaders(), X_MS_VERSION); + String urlUsed = op.getUrl().toString(); + + // Assert that appropriate xMsVersion and query param was used to make request + if (isPaginatedDeleteEnabled && xMsVersion.compareTo(AUGUST_2023_API_VERSION) < 0) { + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(AUGUST_2023_API_VERSION); + Assertions.assertThat(urlUsed) + .describedAs("Url must have paginated = true as query param") + .contains(QUERY_PARAM_PAGINATED); + + } else if (isPaginatedDeleteEnabled && xMsVersion.compareTo(AUGUST_2023_API_VERSION) >= 0) { + Assertions.assertThat(urlUsed) + .describedAs("Url must have paginated = true as query param") + .contains(QUERY_PARAM_PAGINATED); + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(xMsVersion); + } else { + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(xMsVersion); + Assertions.assertThat(urlUsed) + .describedAs("Url must not have paginated = true as query param") + .doesNotContain(QUERY_PARAM_PAGINATED); + } + + // Assert that deletion was successful in every scenario. + AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> + client.getPathStatus(testPath.toString(), false, testTracingContext, null)); + Assertions.assertThat(e.getStatusCode()) + .describedAs("Path should have been deleted").isEqualTo(HTTP_NOT_FOUND); + } + + public void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDeleteEnabled) throws Exception{ + final AzureBlobFileSystem fs = isHnsEnabled ? this.firstTestUserFs : getFileSystem(); + TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); + Path testPath = new Path("/emptyPath"); + fs.mkdirs(testPath); + + // Set the paginated enabled value and xMsVersion at client level. + AbfsClient client = fs.getAbfsStore().getClient(); + client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); + AbfsRestOperation op = client.deletePath(testPath.toString(), false, null, testTracingContext); + + // Getting the url that was used to make the request + String urlUsed = op.getUrl().toString(); + + // Assert that appropriate query param was used to make request + Assertions.assertThat(urlUsed) + .describedAs("Url must not have paginated as query param") + .doesNotContain(QUERY_PARAM_PAGINATED); + + // Assert that deletion was successful in every scenario. + AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> + client.getPathStatus(testPath.toString(), false, testTracingContext, null)); + Assertions.assertThat(e.getStatusCode()) + .describedAs("Path should have been deleted").isEqualTo(HTTP_NOT_FOUND); + } + + public void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled) throws Exception { + final AzureBlobFileSystem fs = isHnsEnabled ? this.firstTestUserFs : getFileSystem(); + Path smallDirPath = createSmallDir(); + String randomCT = "randomContinuationToken1234"; + TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); + + AbfsClient client = fs.getAbfsStore().getClient(); + client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(true); + + AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> + client.deletePath(smallDirPath.toString(), true, randomCT, testTracingContext)); + Assertions.assertThat(e.getStatusCode()) + .describedAs("Request Should fail with 400").isEqualTo(HTTP_BAD_REQUEST); + } + + private void setFirstTestUserFsAuth() throws IOException { + if (this.firstTestUserFs != null) { + return; + } + checkIfConfigIsSet(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT + + "." + getAccountName()); + Configuration conf = getRawConfiguration(); + setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_ID, FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID); + setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_SECRET, + FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET); + conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name()); + conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME + "." + + getAccountName(), ClientCredsTokenProvider.class.getName()); + conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, + false); + this.firstTestUserFs = (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration()); + } + + private void setTestFsConf(final String fsConfKey, + final String testFsConfKey) { + final String confKeyWithAccountName = fsConfKey + "." + getAccountName(); + final String confValue = getConfiguration() + .getString(testFsConfKey, ""); + getRawConfiguration().set(confKeyWithAccountName, confValue); + } + + private void setDefaultAclOnRoot(String uid) + throws IOException { + List aclSpec = Lists.newArrayList(AclTestHelpers + .aclEntry(AclEntryScope.ACCESS, AclEntryType.USER, uid, FsAction.ALL), + AclTestHelpers.aclEntry(AclEntryScope.DEFAULT, AclEntryType.USER, uid, FsAction.ALL)); + this.superUserFs.modifyAclEntries(new Path("/"), aclSpec); + } + + private Path createSmallDir() throws IOException { + String rootPath = "/smallDir" + StringUtils.right( + UUID.randomUUID().toString(), 10); + String firstFilePath = rootPath + "/placeholderFile"; + this.superUserFs.create(new Path(firstFilePath)); + + for (int i = 1; i <= 2; i++) { + String dirPath = "/dirLevel1-" + i + "/dirLevel2-" + i; + String filePath = rootPath + dirPath + "/file-" + i; + this.superUserFs.create(new Path(filePath)); + } + return new Path(rootPath); + } + + private void checkIfConfigIsSet(String configKey){ + AbfsConfiguration conf = getConfiguration(); + String value = conf.get(configKey); + Assume.assumeTrue(configKey + " config is mandatory for the test to run", + value != null && value.trim().length() > 1); + } +} \ No newline at end of file From e5605ac3b7beff404cf6147413c3cecac8925b54 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Thu, 4 Jan 2024 06:43:19 -0800 Subject: [PATCH 02/13] PR Checks --- .../fs/azurebfs/services/AbfsClientUtils.java | 2 -- .../services/ITestAbfsPaginatedDelete.java | 22 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java index 3dbd7bd4aad71..b1ac30d33805c 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientUtils.java @@ -22,8 +22,6 @@ import org.apache.hadoop.fs.azurebfs.extensions.EncryptionContextProvider; -import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_VERSION; - public final class AbfsClientUtils { private AbfsClientUtils() { diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index b22dd69831c3c..a51baf3bd178d 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -132,7 +132,8 @@ public void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolea TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); Path testPath; if (isEmptyDir) { - testPath = new Path("/emptyPath"); + testPath = new Path("/emptyPath" + StringUtils.right( + UUID.randomUUID().toString(), 10)); fs.mkdirs(testPath); } else { testPath = createSmallDir(); @@ -151,13 +152,12 @@ public void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolea // Assert that appropriate xMsVersion and query param was used to make request if (isPaginatedDeleteEnabled && xMsVersion.compareTo(AUGUST_2023_API_VERSION) < 0) { - Assertions.assertThat(xMsVersionUsed) - .describedAs("Request was made with wrong x-ms-version") - .isEqualTo(AUGUST_2023_API_VERSION); Assertions.assertThat(urlUsed) .describedAs("Url must have paginated = true as query param") .contains(QUERY_PARAM_PAGINATED); - + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(AUGUST_2023_API_VERSION); } else if (isPaginatedDeleteEnabled && xMsVersion.compareTo(AUGUST_2023_API_VERSION) >= 0) { Assertions.assertThat(urlUsed) .describedAs("Url must have paginated = true as query param") @@ -166,12 +166,12 @@ public void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolea .describedAs("Request was made with wrong x-ms-version") .isEqualTo(xMsVersion); } else { - Assertions.assertThat(xMsVersionUsed) - .describedAs("Request was made with wrong x-ms-version") - .isEqualTo(xMsVersion); Assertions.assertThat(urlUsed) .describedAs("Url must not have paginated = true as query param") .doesNotContain(QUERY_PARAM_PAGINATED); + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(xMsVersion); } // Assert that deletion was successful in every scenario. @@ -195,7 +195,7 @@ public void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDele // Getting the url that was used to make the request String urlUsed = op.getUrl().toString(); - // Assert that appropriate query param was used to make request + // Assert that paginated query param was not set to make request Assertions.assertThat(urlUsed) .describedAs("Url must not have paginated as query param") .doesNotContain(QUERY_PARAM_PAGINATED); @@ -214,7 +214,7 @@ public void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled) TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); AbfsClient client = fs.getAbfsStore().getClient(); - client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(true); + client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedEnabled); AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> client.deletePath(smallDirPath.toString(), true, randomCT, testTracingContext)); @@ -276,4 +276,4 @@ private void checkIfConfigIsSet(String configKey){ Assume.assumeTrue(configKey + " config is mandatory for the test to run", value != null && value.trim().length() > 1); } -} \ No newline at end of file +} From 1b0e9a30f9d9016671c0b33dbc61c09c0530b006 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Wed, 24 Jan 2024 02:44:52 -0800 Subject: [PATCH 03/13] Defining API_Versions as Enums --- .../azurebfs/constants/AbfsHttpConstants.java | 31 +++++- .../fs/azurebfs/services/AbfsClient.java | 72 ++++++------ .../fs/azurebfs/services/ITestAbfsClient.java | 5 +- .../services/ITestAbfsPaginatedDelete.java | 103 ++++++++++-------- 4 files changed, 128 insertions(+), 83 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java index d719b049a1d78..745a3b2ebdbe6 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java @@ -120,9 +120,34 @@ public final class AbfsHttpConstants { public static final char CHAR_EQUALS = '='; public static final char CHAR_STAR = '*'; public static final char CHAR_PLUS = '+'; - public static final String DECEMBER_2019_API_VERSION = "2019-12-12"; - public static final String APRIL_2021_API_VERSION = "2021-04-10"; - public static final String AUGUST_2023_API_VERSION = "2023-08-03"; + + /** + * Specifies the version of the REST protocol used for processing the request. + * Versions should be added in enum list in ascending chronological order. + * Latest one should be added last in the list. + * When upgrading the version for whole driver, update the getCurrentVersion; + */ + public enum API_VERSION { + + DEC_12_2019("2019-12-12"), + APR_10_2021("2021-04-10"), + AUG_03_2023("2023-08-03"); + + private final String xMsApiVersion; + + API_VERSION(String xMsApiVersion) { + this.xMsApiVersion = xMsApiVersion; + } + + @Override + public String toString() { + return xMsApiVersion; + } + + public static final API_VERSION getCurrentVersion() { + return DEC_12_2019; + } + } /** * Value that differentiates categories of the http_status. 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 f11692f9adfb0..01cace89f53b9 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 @@ -92,7 +92,7 @@ public class AbfsClient implements Closeable { private final URL baseUrl; private final SharedKeyCredentials sharedKeyCredentials; - private String xMsVersion = DECEMBER_2019_API_VERSION; + private API_VERSION xMsVersion = API_VERSION.getCurrentVersion(); private final ExponentialRetryPolicy retryPolicy; private final String filesystem; private final AbfsConfiguration abfsConfiguration; @@ -139,7 +139,7 @@ private AbfsClient(final URL baseUrl, if (encryptionContextProvider != null) { this.encryptionContextProvider = encryptionContextProvider; - xMsVersion = APRIL_2021_API_VERSION; // will be default once server change deployed + xMsVersion = API_VERSION.APR_10_2021; // will be default once server change deployed encryptionType = EncryptionType.ENCRYPTION_CONTEXT; } else if (abfsConfiguration.getEncodedClientProvidedEncryptionKey() != null) { clientProvidedEncryptionKey = @@ -233,9 +233,9 @@ AbfsThrottlingIntercept getIntercept() { return intercept; } - List createDefaultHeaders() { + List createDefaultHeaders(API_VERSION xMsVersion) { final List requestHeaders = new ArrayList(); - requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion)); + requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion.toString())); requestHeaders.add(new AbfsHttpHeader(ACCEPT, APPLICATION_JSON + COMMA + SINGLE_WHITE_SPACE + APPLICATION_OCTET_STREAM)); requestHeaders.add(new AbfsHttpHeader(ACCEPT_CHARSET, @@ -305,7 +305,7 @@ AbfsUriQueryBuilder createDefaultUriQueryBuilder() { public AbfsRestOperation createFilesystem(TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = new AbfsUriQueryBuilder(); abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM); @@ -320,7 +320,7 @@ public AbfsRestOperation createFilesystem(TracingContext tracingContext) public AbfsRestOperation setFilesystemProperties(final String properties, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); // JDK7 does not support PATCH, so to work around the issue we will use // PUT and specify the real method in the X-Http-Method-Override header. requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE, @@ -345,7 +345,7 @@ public AbfsRestOperation setFilesystemProperties(final String properties, public AbfsRestOperation listPath(final String relativePath, final boolean recursive, final int listMaxResults, final String continuation, TracingContext tracingContext) throws IOException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM); @@ -367,7 +367,7 @@ public AbfsRestOperation listPath(final String relativePath, final boolean recur } public AbfsRestOperation getFilesystemProperties(TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM); @@ -383,7 +383,7 @@ public AbfsRestOperation getFilesystemProperties(TracingContext tracingContext) } public AbfsRestOperation deleteFilesystem(TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM); @@ -434,7 +434,7 @@ public AbfsRestOperation createPath(final String path, final ContextEncryptionAdapter contextEncryptionAdapter, final TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); if (isFile) { addEncryptionKeyRequestHeaders(path, requestHeaders, true, contextEncryptionAdapter, tracingContext); @@ -495,7 +495,7 @@ public AbfsRestOperation createPath(final String path, } public AbfsRestOperation acquireLease(final String path, int duration, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, ACQUIRE_LEASE_ACTION)); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_DURATION, Integer.toString(duration))); @@ -515,7 +515,7 @@ public AbfsRestOperation acquireLease(final String path, int duration, TracingCo public AbfsRestOperation renewLease(final String path, final String leaseId, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, RENEW_LEASE_ACTION)); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, leaseId)); @@ -534,7 +534,7 @@ public AbfsRestOperation renewLease(final String path, final String leaseId, public AbfsRestOperation releaseLease(final String path, final String leaseId, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, RELEASE_LEASE_ACTION)); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, leaseId)); @@ -553,7 +553,7 @@ public AbfsRestOperation releaseLease(final String path, public AbfsRestOperation breakLease(final String path, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, BREAK_LEASE_ACTION)); requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_BREAK_PERIOD, DEFAULT_LEASE_BREAK_PERIOD)); @@ -601,7 +601,7 @@ public AbfsClientRenameResult renamePath( boolean isMetadataIncompleteState, boolean isNamespaceEnabled) throws IOException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final boolean hasEtag = !isEmpty(sourceEtag); @@ -797,7 +797,7 @@ public AbfsRestOperation append(final String path, final byte[] buffer, AppendRequestParameters reqParams, final String cachedSasToken, ContextEncryptionAdapter contextEncryptionAdapter, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); addEncryptionKeyRequestHeaders(path, requestHeaders, false, contextEncryptionAdapter, tracingContext); if (reqParams.isExpectHeaderEnabled()) { @@ -929,7 +929,7 @@ public AbfsRestOperation flush(final String path, final long position, final String cachedSasToken, final String leaseId, ContextEncryptionAdapter contextEncryptionAdapter, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); addEncryptionKeyRequestHeaders(path, requestHeaders, false, contextEncryptionAdapter, tracingContext); // JDK7 does not support PATCH, so to workaround the issue we will use @@ -962,7 +962,7 @@ public AbfsRestOperation flush(final String path, final long position, public AbfsRestOperation setPathProperties(final String path, final String properties, final TracingContext tracingContext, final ContextEncryptionAdapter contextEncryptionAdapter) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); addEncryptionKeyRequestHeaders(path, requestHeaders, false, contextEncryptionAdapter, tracingContext); // JDK7 does not support PATCH, so to workaround the issue we will use @@ -990,7 +990,7 @@ public AbfsRestOperation getPathStatus(final String path, final boolean includeProperties, final TracingContext tracingContext, final ContextEncryptionAdapter contextEncryptionAdapter) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); String operation = SASTokenProvider.GET_PROPERTIES_OPERATION; @@ -1027,7 +1027,7 @@ public AbfsRestOperation read(final String path, String cachedSasToken, ContextEncryptionAdapter contextEncryptionAdapter, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); addEncryptionKeyRequestHeaders(path, requestHeaders, false, contextEncryptionAdapter, tracingContext); requestHeaders.add(new AbfsHttpHeader(RANGE, @@ -1057,16 +1057,14 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, final String continuation, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders + = (isPaginatedDeleteEnabled(tracingContext, recursive) + && xMsVersion.compareTo(API_VERSION.AUG_03_2023) < 0) + ? createDefaultHeaders(API_VERSION.AUG_03_2023) + : createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); - if (abfsConfiguration.isPaginatedDeleteEnabled() && recursive) { - // Change the x-ms-version to "2023-08-03" if its less than that. - if (xMsVersion.compareTo(AUGUST_2023_API_VERSION) < 0) { - requestHeaders.removeIf(header -> header.getName().equalsIgnoreCase(X_MS_VERSION)); - requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, AUGUST_2023_API_VERSION)); - } - + if (isPaginatedDeleteEnabled(tracingContext, recursive)) { // Add paginated query parameter abfsUriQueryBuilder.addQuery(QUERY_PARAM_PAGINATED, TRUE); } @@ -1142,7 +1140,7 @@ public AbfsRestOperation deleteIdempotencyCheckOp(final AbfsRestOperation op) { public AbfsRestOperation setOwner(final String path, final String owner, final String group, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); // JDK7 does not support PATCH, so to workaround the issue we will use // PUT and specify the real method in the X-Http-Method-Override header. requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE, @@ -1172,7 +1170,7 @@ public AbfsRestOperation setOwner(final String path, final String owner, final S public AbfsRestOperation setPermission(final String path, final String permission, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); // JDK7 does not support PATCH, so to workaround the issue we will use // PUT and specify the real method in the X-Http-Method-Override header. requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE, @@ -1202,7 +1200,7 @@ public AbfsRestOperation setAcl(final String path, final String aclSpecString, public AbfsRestOperation setAcl(final String path, final String aclSpecString, final String eTag, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); // JDK7 does not support PATCH, so to workaround the issue we will use // PUT and specify the real method in the X-Http-Method-Override header. requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE, @@ -1235,7 +1233,7 @@ public AbfsRestOperation getAclStatus(final String path, TracingContext tracingC public AbfsRestOperation getAclStatus(final String path, final boolean useUPN, TracingContext tracingContext) throws AzureBlobFileSystemException { - final List requestHeaders = createDefaultHeaders(); + final List requestHeaders = createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); abfsUriQueryBuilder.addQuery(HttpQueryParams.QUERY_PARAM_ACTION, AbfsHttpConstants.GET_ACCESS_CONTROL); @@ -1273,7 +1271,7 @@ public AbfsRestOperation checkAccess(String path, String rwx, TracingContext tra AbfsRestOperationType.CheckAccess, AbfsHttpConstants.HTTP_METHOD_HEAD, url, - createDefaultHeaders()); + createDefaultHeaders(xMsVersion)); op.execute(tracingContext); return op; } @@ -1413,6 +1411,12 @@ private synchronized Boolean getIsNamespaceEnabled(TracingContext tracingContext return isNamespaceEnabled; } + private Boolean isPaginatedDeleteEnabled(TracingContext tracingContext, + boolean isRecursiveDelete) throws AzureBlobFileSystemException { + return abfsConfiguration.isPaginatedDeleteEnabled() + && getIsNamespaceEnabled(tracingContext) && isRecursiveDelete; + } + public AuthType getAuthType() { return authType; } @@ -1513,7 +1517,7 @@ protected AbfsCounters getAbfsCounters() { return abfsCounters; } - public String getxMsVersion() { + public API_VERSION getxMsVersion() { return xMsVersion; } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java index d7d47099acef8..6d96ba9694cd7 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java @@ -47,6 +47,7 @@ import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.API_VERSION.getCurrentVersion; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APPEND_ACTION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EXPECT_100_JDK_ERROR; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PATCH; @@ -372,7 +373,7 @@ public static AbfsClient getMockAbfsClient(AbfsClient baseAbfsClientInstance, when(client.createRequestUrl(any(), any())).thenCallRealMethod(); when(client.getAccessToken()).thenCallRealMethod(); when(client.getSharedKeyCredentials()).thenCallRealMethod(); - when(client.createDefaultHeaders()).thenCallRealMethod(); + when(client.createDefaultHeaders(getCurrentVersion())).thenCallRealMethod(); when(client.getAbfsConfiguration()).thenReturn(abfsConfig); when(client.getIntercept()).thenReturn( AbfsThrottlingInterceptFactory.getInstance( @@ -446,7 +447,7 @@ public static URL getTestUrl(AbfsClient client, String path) throws * @return List of AbfsHttpHeaders */ public static List getTestRequestHeaders(AbfsClient client) { - return client.createDefaultHeaders(); + return client.createDefaultHeaders(getCurrentVersion()); } /** diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index a51baf3bd178d..b3de949a93023 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -25,6 +25,7 @@ import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest; import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem; +import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException; import org.apache.hadoop.fs.azurebfs.oauth2.ClientCredsTokenProvider; import org.apache.hadoop.fs.azurebfs.utils.AclTestHelpers; @@ -38,6 +39,7 @@ import org.assertj.core.api.Assertions; import org.junit.Assume; import org.junit.Test; +import org.mockito.Mockito; import java.io.IOException; import java.util.List; @@ -45,8 +47,6 @@ import static java.net.HttpURLConnection.HTTP_BAD_REQUEST; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.AUGUST_2023_API_VERSION; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.DECEMBER_2019_API_VERSION; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT; import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION; @@ -96,12 +96,18 @@ public void setup() throws Exception { */ @Test public void testRecursiveDeleteWithPagination() throws Exception { - testRecursiveDeleteWithPaginationInternal(false, true, DECEMBER_2019_API_VERSION); - testRecursiveDeleteWithPaginationInternal(false, true, AUGUST_2023_API_VERSION); - testRecursiveDeleteWithPaginationInternal(false, false, DECEMBER_2019_API_VERSION); - testRecursiveDeleteWithPaginationInternal(false, false, AUGUST_2023_API_VERSION); - testRecursiveDeleteWithPaginationInternal(true, true, DECEMBER_2019_API_VERSION); - testRecursiveDeleteWithPaginationInternal(true, false, AUGUST_2023_API_VERSION); + testRecursiveDeleteWithPaginationInternal(false, true, + AbfsHttpConstants.API_VERSION.DEC_12_2019); + testRecursiveDeleteWithPaginationInternal(false, true, + AbfsHttpConstants.API_VERSION.AUG_03_2023); + testRecursiveDeleteWithPaginationInternal(false, false, + AbfsHttpConstants.API_VERSION.DEC_12_2019); + testRecursiveDeleteWithPaginationInternal(false, false, + AbfsHttpConstants.API_VERSION.AUG_03_2023); + testRecursiveDeleteWithPaginationInternal(true, true, + AbfsHttpConstants.API_VERSION.DEC_12_2019); + testRecursiveDeleteWithPaginationInternal(true, false, + AbfsHttpConstants.API_VERSION.AUG_03_2023); } /** @@ -126,10 +132,11 @@ public void testRecursiveDeleteWithInvalidCT() throws Exception { testRecursiveDeleteWithInvalidCTInternal(false); } - public void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolean isPaginatedDeleteEnabled, - String xMsVersion) throws Exception { - final AzureBlobFileSystem fs = isHnsEnabled ? this.firstTestUserFs : getFileSystem(); - TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); + private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, + boolean isPaginatedDeleteEnabled, + AbfsHttpConstants.API_VERSION xMsVersion) throws Exception { + final AzureBlobFileSystem fs = getUserFileSystem(); + TracingContext testTracingContext = getTestTracingContext(fs, true); Path testPath; if (isEmptyDir) { testPath = new Path("/emptyPath" + StringUtils.right( @@ -139,58 +146,60 @@ public void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolea testPath = createSmallDir(); } - // Set the paginated enabled value and xMsVersion at client level. - AbfsClient client = ITestAbfsClient.setAbfsClientField( - fs.getAbfsStore().getClient(), "xMsVersion", xMsVersion); - client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); + // Set the paginated enabled value and xMsVersion at spiedClient level. + AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient()); + ITestAbfsClient.setAbfsClientField(spiedClient, "xMsVersion", xMsVersion); + spiedClient.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); - AbfsRestOperation op = client.deletePath(testPath.toString(), true, null, testTracingContext); + AbfsRestOperation op = spiedClient.deletePath( + testPath.toString(), true, null, testTracingContext); // Getting the xMsVersion that was used to make the request String xMsVersionUsed = getHeaderValue(op.getRequestHeaders(), X_MS_VERSION); String urlUsed = op.getUrl().toString(); // Assert that appropriate xMsVersion and query param was used to make request - if (isPaginatedDeleteEnabled && xMsVersion.compareTo(AUGUST_2023_API_VERSION) < 0) { - Assertions.assertThat(urlUsed) - .describedAs("Url must have paginated = true as query param") - .contains(QUERY_PARAM_PAGINATED); - Assertions.assertThat(xMsVersionUsed) - .describedAs("Request was made with wrong x-ms-version") - .isEqualTo(AUGUST_2023_API_VERSION); - } else if (isPaginatedDeleteEnabled && xMsVersion.compareTo(AUGUST_2023_API_VERSION) >= 0) { + if (isPaginatedDeleteEnabled) { Assertions.assertThat(urlUsed) .describedAs("Url must have paginated = true as query param") .contains(QUERY_PARAM_PAGINATED); - Assertions.assertThat(xMsVersionUsed) - .describedAs("Request was made with wrong x-ms-version") - .isEqualTo(xMsVersion); + if (xMsVersion.compareTo(AbfsHttpConstants.API_VERSION.AUG_03_2023) < 0) { + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(AbfsHttpConstants.API_VERSION.AUG_03_2023.toString()); + } else if (xMsVersion.compareTo(AbfsHttpConstants.API_VERSION.AUG_03_2023) >= 0) { + Assertions.assertThat(xMsVersionUsed) + .describedAs("Request was made with wrong x-ms-version") + .isEqualTo(xMsVersion.toString()); + } } else { Assertions.assertThat(urlUsed) .describedAs("Url must not have paginated = true as query param") .doesNotContain(QUERY_PARAM_PAGINATED); Assertions.assertThat(xMsVersionUsed) .describedAs("Request was made with wrong x-ms-version") - .isEqualTo(xMsVersion); + .isEqualTo(xMsVersion.toString()); } // Assert that deletion was successful in every scenario. AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> - client.getPathStatus(testPath.toString(), false, testTracingContext, null)); + spiedClient.getPathStatus(testPath.toString(), false, testTracingContext, null)); Assertions.assertThat(e.getStatusCode()) .describedAs("Path should have been deleted").isEqualTo(HTTP_NOT_FOUND); } - public void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDeleteEnabled) throws Exception{ - final AzureBlobFileSystem fs = isHnsEnabled ? this.firstTestUserFs : getFileSystem(); - TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); + private void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDeleteEnabled) throws Exception{ + final AzureBlobFileSystem fs = getUserFileSystem(); + TracingContext testTracingContext = getTestTracingContext(fs, true); Path testPath = new Path("/emptyPath"); fs.mkdirs(testPath); - // Set the paginated enabled value and xMsVersion at client level. - AbfsClient client = fs.getAbfsStore().getClient(); - client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); - AbfsRestOperation op = client.deletePath(testPath.toString(), false, null, testTracingContext); + // Set the paginated enabled value and xMsVersion at spiedClient level. + AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient()); + spiedClient.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); + + AbfsRestOperation op = spiedClient.deletePath( + testPath.toString(), false, null, testTracingContext); // Getting the url that was used to make the request String urlUsed = op.getUrl().toString(); @@ -202,24 +211,30 @@ public void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDele // Assert that deletion was successful in every scenario. AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> - client.getPathStatus(testPath.toString(), false, testTracingContext, null)); + spiedClient.getPathStatus(testPath.toString(), false, testTracingContext, null)); Assertions.assertThat(e.getStatusCode()) .describedAs("Path should have been deleted").isEqualTo(HTTP_NOT_FOUND); } - public void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled) throws Exception { - final AzureBlobFileSystem fs = isHnsEnabled ? this.firstTestUserFs : getFileSystem(); + private void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled) throws Exception { + final AzureBlobFileSystem fs = getUserFileSystem(); Path smallDirPath = createSmallDir(); String randomCT = "randomContinuationToken1234"; TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); - AbfsClient client = fs.getAbfsStore().getClient(); - client.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedEnabled); + AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient()); + spiedClient.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedEnabled); AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> - client.deletePath(smallDirPath.toString(), true, randomCT, testTracingContext)); + spiedClient.deletePath(smallDirPath.toString(), true, randomCT, testTracingContext)); Assertions.assertThat(e.getStatusCode()) - .describedAs("Request Should fail with 400").isEqualTo(HTTP_BAD_REQUEST); + .describedAs("Request Should fail with Bad Request").isEqualTo(HTTP_BAD_REQUEST); + } + + private AzureBlobFileSystem getUserFileSystem() { + // For HNS account only Server will trigger Pagination for ACL checks + // And for ACL Checks file system user should not be superUser. + return this.isHnsEnabled ? this.firstTestUserFs : this.superUserFs; } private void setFirstTestUserFsAuth() throws IOException { From 5567944f0831adc0071845780f453b5b6fb3a183 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Wed, 24 Jan 2024 21:45:40 -0800 Subject: [PATCH 04/13] Addressed Comments --- .../fs/azurebfs/services/AbfsClient.java | 6 +- .../services/ITestAbfsPaginatedDelete.java | 59 ++++++++----------- 2 files changed, 29 insertions(+), 36 deletions(-) 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 01cace89f53b9..0c7b2316aef9f 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 @@ -1411,9 +1411,13 @@ private synchronized Boolean getIsNamespaceEnabled(TracingContext tracingContext return isNamespaceEnabled; } + protected Boolean getIsPaginatedDeleteEnabled() { + return abfsConfiguration.isPaginatedDeleteEnabled(); + } + private Boolean isPaginatedDeleteEnabled(TracingContext tracingContext, boolean isRecursiveDelete) throws AzureBlobFileSystemException { - return abfsConfiguration.isPaginatedDeleteEnabled() + return getIsPaginatedDeleteEnabled() && getIsNamespaceEnabled(tracingContext) && isRecursiveDelete; } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index b3de949a93023..30e87ae368ea1 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -66,7 +66,6 @@ public class ITestAbfsPaginatedDelete extends AbstractAbfsIntegrationTest { private AzureBlobFileSystem superUserFs; private AzureBlobFileSystem firstTestUserFs; - private String firstTestUserGuid; private boolean isHnsEnabled; public ITestAbfsPaginatedDelete() throws Exception { @@ -78,13 +77,16 @@ public void setup() throws Exception { loadConfiguredFileSystem(); super.setup(); this.superUserFs = getFileSystem(); - this.firstTestUserGuid = getConfiguration() - .get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); + + // Test User Credentials. + String firstTestUserGuid = getConfiguration().get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); + String clientId = getConfiguration().getString(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID, ""); + String clientSecret = getConfiguration().getString(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET, ""); if(isHnsEnabled) { // setting up ACL permissions for test user - setFirstTestUserFsAuth(); - setDefaultAclOnRoot(this.firstTestUserGuid); + setFirstTestUserFsAuth(clientId, clientSecret); + setDefaultAclOnRoot(firstTestUserGuid); } } @@ -149,7 +151,7 @@ private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, // Set the paginated enabled value and xMsVersion at spiedClient level. AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient()); ITestAbfsClient.setAbfsClientField(spiedClient, "xMsVersion", xMsVersion); - spiedClient.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); + Mockito.doReturn(isPaginatedDeleteEnabled).when(spiedClient).getIsPaginatedDeleteEnabled(); AbfsRestOperation op = spiedClient.deletePath( testPath.toString(), true, null, testTracingContext); @@ -196,7 +198,7 @@ private void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDel // Set the paginated enabled value and xMsVersion at spiedClient level. AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient()); - spiedClient.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedDeleteEnabled); + Mockito.doReturn(isPaginatedDeleteEnabled).when(spiedClient).getIsPaginatedDeleteEnabled(); AbfsRestOperation op = spiedClient.deletePath( testPath.toString(), false, null, testTracingContext); @@ -223,7 +225,7 @@ private void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true); AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient()); - spiedClient.getAbfsConfiguration().setIsPaginatedDeleteEnabled(isPaginatedEnabled); + Mockito.doReturn(isPaginatedEnabled).when(spiedClient).getIsPaginatedDeleteEnabled(); AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> spiedClient.deletePath(smallDirPath.toString(), true, randomCT, testTracingContext)); @@ -237,36 +239,30 @@ private AzureBlobFileSystem getUserFileSystem() { return this.isHnsEnabled ? this.firstTestUserFs : this.superUserFs; } - private void setFirstTestUserFsAuth() throws IOException { + private void setFirstTestUserFsAuth(String clientId, String clientSecret) throws IOException { if (this.firstTestUserFs != null) { return; } - checkIfConfigIsSet(FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT - + "." + getAccountName()); + + // Check if OAuth Client Endpoint is provided. + String configKey = FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT; + String value = getConfiguration().get(configKey); + Assume.assumeTrue(configKey + " config is mandatory for the test to run", + value != null && value.trim().length() > 1); + Configuration conf = getRawConfiguration(); - setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_ID, FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID); - setTestFsConf(FS_AZURE_BLOB_FS_CLIENT_SECRET, - FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET); + conf.set(FS_AZURE_BLOB_FS_CLIENT_ID, clientId); + conf.set(FS_AZURE_BLOB_FS_CLIENT_SECRET, clientSecret); conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name()); - conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME + "." - + getAccountName(), ClientCredsTokenProvider.class.getName()); - conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, - false); + conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME, ClientCredsTokenProvider.class.getName()); + conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,false); this.firstTestUserFs = (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration()); } - private void setTestFsConf(final String fsConfKey, - final String testFsConfKey) { - final String confKeyWithAccountName = fsConfKey + "." + getAccountName(); - final String confValue = getConfiguration() - .getString(testFsConfKey, ""); - getRawConfiguration().set(confKeyWithAccountName, confValue); - } - private void setDefaultAclOnRoot(String uid) throws IOException { - List aclSpec = Lists.newArrayList(AclTestHelpers - .aclEntry(AclEntryScope.ACCESS, AclEntryType.USER, uid, FsAction.ALL), + List aclSpec = Lists.newArrayList(AclTestHelpers.aclEntry( + AclEntryScope.ACCESS, AclEntryType.USER, uid, FsAction.ALL), AclTestHelpers.aclEntry(AclEntryScope.DEFAULT, AclEntryType.USER, uid, FsAction.ALL)); this.superUserFs.modifyAclEntries(new Path("/"), aclSpec); } @@ -284,11 +280,4 @@ private Path createSmallDir() throws IOException { } return new Path(rootPath); } - - private void checkIfConfigIsSet(String configKey){ - AbfsConfiguration conf = getConfiguration(); - String value = conf.get(configKey); - Assume.assumeTrue(configKey + " config is mandatory for the test to run", - value != null && value.trim().length() > 1); - } } From 42010aa466746fe4b6d2ebd7db0d1e6167ca7764 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Wed, 24 Jan 2024 21:54:42 -0800 Subject: [PATCH 05/13] Removing Unused Imports --- .../org/apache/hadoop/fs/azurebfs/services/AbfsClient.java | 6 +++--- .../fs/azurebfs/services/ITestAbfsPaginatedDelete.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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 0c7b2316aef9f..7cc4d2cf07ef2 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 @@ -1058,13 +1058,13 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, TracingContext tracingContext) throws AzureBlobFileSystemException { final List requestHeaders - = (isPaginatedDeleteEnabled(tracingContext, recursive) + = (isPaginatedDelete(tracingContext, recursive) && xMsVersion.compareTo(API_VERSION.AUG_03_2023) < 0) ? createDefaultHeaders(API_VERSION.AUG_03_2023) : createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); - if (isPaginatedDeleteEnabled(tracingContext, recursive)) { + if (isPaginatedDelete(tracingContext, recursive)) { // Add paginated query parameter abfsUriQueryBuilder.addQuery(QUERY_PARAM_PAGINATED, TRUE); } @@ -1415,7 +1415,7 @@ protected Boolean getIsPaginatedDeleteEnabled() { return abfsConfiguration.isPaginatedDeleteEnabled(); } - private Boolean isPaginatedDeleteEnabled(TracingContext tracingContext, + private Boolean isPaginatedDelete(TracingContext tracingContext, boolean isRecursiveDelete) throws AzureBlobFileSystemException { return getIsPaginatedDeleteEnabled() && getIsNamespaceEnabled(tracingContext) && isRecursiveDelete; diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index 30e87ae368ea1..e2a904fb04681 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -22,7 +22,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.fs.azurebfs.AbfsConfiguration; import org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest; import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem; import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants; @@ -250,6 +249,7 @@ private void setFirstTestUserFsAuth(String clientId, String clientSecret) throws Assume.assumeTrue(configKey + " config is mandatory for the test to run", value != null && value.trim().length() > 1); + // Set the required configuration Configuration conf = getRawConfiguration(); conf.set(FS_AZURE_BLOB_FS_CLIENT_ID, clientId); conf.set(FS_AZURE_BLOB_FS_CLIENT_SECRET, clientSecret); From db957389a25f246b4fff0c6b399d56453f338232 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Wed, 24 Jan 2024 22:30:00 -0800 Subject: [PATCH 06/13] Checkstyle Fixes --- .../azurebfs/constants/AbfsHttpConstants.java | 6 ++--- .../fs/azurebfs/services/AbfsClient.java | 12 +++++----- .../fs/azurebfs/services/ITestAbfsClient.java | 2 +- .../services/ITestAbfsPaginatedDelete.java | 22 +++++++++---------- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java index 745a3b2ebdbe6..adbffbc9b0f39 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java @@ -127,7 +127,7 @@ public final class AbfsHttpConstants { * Latest one should be added last in the list. * When upgrading the version for whole driver, update the getCurrentVersion; */ - public enum API_VERSION { + public enum ApiVersion { DEC_12_2019("2019-12-12"), APR_10_2021("2021-04-10"), @@ -135,7 +135,7 @@ public enum API_VERSION { private final String xMsApiVersion; - API_VERSION(String xMsApiVersion) { + ApiVersion(String xMsApiVersion) { this.xMsApiVersion = xMsApiVersion; } @@ -144,7 +144,7 @@ public String toString() { return xMsApiVersion; } - public static final API_VERSION getCurrentVersion() { + public static ApiVersion getCurrentVersion() { return DEC_12_2019; } } 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 7cc4d2cf07ef2..af35ea69144cf 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 @@ -92,7 +92,7 @@ public class AbfsClient implements Closeable { private final URL baseUrl; private final SharedKeyCredentials sharedKeyCredentials; - private API_VERSION xMsVersion = API_VERSION.getCurrentVersion(); + private ApiVersion xMsVersion = ApiVersion.getCurrentVersion(); private final ExponentialRetryPolicy retryPolicy; private final String filesystem; private final AbfsConfiguration abfsConfiguration; @@ -139,7 +139,7 @@ private AbfsClient(final URL baseUrl, if (encryptionContextProvider != null) { this.encryptionContextProvider = encryptionContextProvider; - xMsVersion = API_VERSION.APR_10_2021; // will be default once server change deployed + xMsVersion = ApiVersion.APR_10_2021; // will be default once server change deployed encryptionType = EncryptionType.ENCRYPTION_CONTEXT; } else if (abfsConfiguration.getEncodedClientProvidedEncryptionKey() != null) { clientProvidedEncryptionKey = @@ -233,7 +233,7 @@ AbfsThrottlingIntercept getIntercept() { return intercept; } - List createDefaultHeaders(API_VERSION xMsVersion) { + List createDefaultHeaders(ApiVersion xMsVersion) { final List requestHeaders = new ArrayList(); requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion.toString())); requestHeaders.add(new AbfsHttpHeader(ACCEPT, APPLICATION_JSON @@ -1059,8 +1059,8 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive, throws AzureBlobFileSystemException { final List requestHeaders = (isPaginatedDelete(tracingContext, recursive) - && xMsVersion.compareTo(API_VERSION.AUG_03_2023) < 0) - ? createDefaultHeaders(API_VERSION.AUG_03_2023) + && xMsVersion.compareTo(ApiVersion.AUG_03_2023) < 0) + ? createDefaultHeaders(ApiVersion.AUG_03_2023) : createDefaultHeaders(xMsVersion); final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder(); @@ -1521,7 +1521,7 @@ protected AbfsCounters getAbfsCounters() { return abfsCounters; } - public API_VERSION getxMsVersion() { + public ApiVersion getxMsVersion() { return xMsVersion; } diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java index 6d96ba9694cd7..07548d658c576 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java @@ -47,7 +47,7 @@ import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; -import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.API_VERSION.getCurrentVersion; +import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ApiVersion.getCurrentVersion; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APPEND_ACTION; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EXPECT_100_JDK_ERROR; import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PATCH; diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index e2a904fb04681..e5bed79825d95 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -82,7 +82,7 @@ public void setup() throws Exception { String clientId = getConfiguration().getString(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID, ""); String clientSecret = getConfiguration().getString(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET, ""); - if(isHnsEnabled) { + if (isHnsEnabled) { // setting up ACL permissions for test user setFirstTestUserFsAuth(clientId, clientSecret); setDefaultAclOnRoot(firstTestUserGuid); @@ -98,17 +98,17 @@ public void setup() throws Exception { @Test public void testRecursiveDeleteWithPagination() throws Exception { testRecursiveDeleteWithPaginationInternal(false, true, - AbfsHttpConstants.API_VERSION.DEC_12_2019); + AbfsHttpConstants.ApiVersion.DEC_12_2019); testRecursiveDeleteWithPaginationInternal(false, true, - AbfsHttpConstants.API_VERSION.AUG_03_2023); + AbfsHttpConstants.ApiVersion.AUG_03_2023); testRecursiveDeleteWithPaginationInternal(false, false, - AbfsHttpConstants.API_VERSION.DEC_12_2019); + AbfsHttpConstants.ApiVersion.DEC_12_2019); testRecursiveDeleteWithPaginationInternal(false, false, - AbfsHttpConstants.API_VERSION.AUG_03_2023); + AbfsHttpConstants.ApiVersion.AUG_03_2023); testRecursiveDeleteWithPaginationInternal(true, true, - AbfsHttpConstants.API_VERSION.DEC_12_2019); + AbfsHttpConstants.ApiVersion.DEC_12_2019); testRecursiveDeleteWithPaginationInternal(true, false, - AbfsHttpConstants.API_VERSION.AUG_03_2023); + AbfsHttpConstants.ApiVersion.AUG_03_2023); } /** @@ -135,7 +135,7 @@ public void testRecursiveDeleteWithInvalidCT() throws Exception { private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, boolean isPaginatedDeleteEnabled, - AbfsHttpConstants.API_VERSION xMsVersion) throws Exception { + AbfsHttpConstants.ApiVersion xMsVersion) throws Exception { final AzureBlobFileSystem fs = getUserFileSystem(); TracingContext testTracingContext = getTestTracingContext(fs, true); Path testPath; @@ -164,11 +164,11 @@ private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir, Assertions.assertThat(urlUsed) .describedAs("Url must have paginated = true as query param") .contains(QUERY_PARAM_PAGINATED); - if (xMsVersion.compareTo(AbfsHttpConstants.API_VERSION.AUG_03_2023) < 0) { + if (xMsVersion.compareTo(AbfsHttpConstants.ApiVersion.AUG_03_2023) < 0) { Assertions.assertThat(xMsVersionUsed) .describedAs("Request was made with wrong x-ms-version") - .isEqualTo(AbfsHttpConstants.API_VERSION.AUG_03_2023.toString()); - } else if (xMsVersion.compareTo(AbfsHttpConstants.API_VERSION.AUG_03_2023) >= 0) { + .isEqualTo(AbfsHttpConstants.ApiVersion.AUG_03_2023.toString()); + } else if (xMsVersion.compareTo(AbfsHttpConstants.ApiVersion.AUG_03_2023) >= 0) { Assertions.assertThat(xMsVersionUsed) .describedAs("Request was made with wrong x-ms-version") .isEqualTo(xMsVersion.toString()); From 197f3bcbbec9d21c62f063dcee893d7cb1c547ce Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Wed, 24 Jan 2024 22:40:23 -0800 Subject: [PATCH 07/13] Addressed Comments --- .../org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java index 5c458db1a0fba..be864e5aac10d 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java @@ -1212,9 +1212,4 @@ public boolean getRenameResilience() { public boolean isPaginatedDeleteEnabled() { return isPaginatedDeleteEnabled; } - - @VisibleForTesting - public void setIsPaginatedDeleteEnabled(boolean isPaginatedDeleteEnabled) { - this.isPaginatedDeleteEnabled = isPaginatedDeleteEnabled; - } } From 1ab45fa6343269d01eef5a64682eb10b22586ee3 Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Thu, 25 Jan 2024 01:40:05 -0800 Subject: [PATCH 08/13] Yetus Rerun --- .../services/ITestAbfsPaginatedDelete.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index e5bed79825d95..1cdcf061ba4da 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -72,15 +72,19 @@ public ITestAbfsPaginatedDelete() throws Exception { @Override public void setup() throws Exception { - isHnsEnabled = this.getConfiguration().getBoolean(FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false); + isHnsEnabled = this.getConfiguration().getBoolean( + FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false); loadConfiguredFileSystem(); super.setup(); this.superUserFs = getFileSystem(); // Test User Credentials. - String firstTestUserGuid = getConfiguration().get(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); - String clientId = getConfiguration().getString(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID, ""); - String clientSecret = getConfiguration().getString(FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET, ""); + String firstTestUserGuid = getConfiguration().get( + FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID); + String clientId = getConfiguration().getString( + FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID, ""); + String clientSecret = getConfiguration().getString( + FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET, ""); if (isHnsEnabled) { // setting up ACL permissions for test user @@ -227,7 +231,8 @@ private void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled Mockito.doReturn(isPaginatedEnabled).when(spiedClient).getIsPaginatedDeleteEnabled(); AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () -> - spiedClient.deletePath(smallDirPath.toString(), true, randomCT, testTracingContext)); + spiedClient.deletePath( + smallDirPath.toString(), true, randomCT, testTracingContext)); Assertions.assertThat(e.getStatusCode()) .describedAs("Request Should fail with Bad Request").isEqualTo(HTTP_BAD_REQUEST); } @@ -254,7 +259,8 @@ private void setFirstTestUserFsAuth(String clientId, String clientSecret) throws conf.set(FS_AZURE_BLOB_FS_CLIENT_ID, clientId); conf.set(FS_AZURE_BLOB_FS_CLIENT_SECRET, clientSecret); conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name()); - conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME, ClientCredsTokenProvider.class.getName()); + conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME, + ClientCredsTokenProvider.class.getName()); conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,false); this.firstTestUserFs = (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration()); } From 2b502e4f127af68c0b381f14101b8af0ffb9b72b Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Fri, 1 Mar 2024 00:17:53 -0800 Subject: [PATCH 09/13] Checkstyle Fix --- .../hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java index 1cdcf061ba4da..21879527bf523 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java @@ -261,7 +261,7 @@ private void setFirstTestUserFsAuth(String clientId, String clientSecret) throws conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name()); conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME, ClientCredsTokenProvider.class.getName()); - conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,false); + conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, false); this.firstTestUserFs = (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration()); } From d7d9d065dd66ab82a5685f36531b1c91e84265dd Mon Sep 17 00:00:00 2001 From: Anuj Modi Date: Tue, 2 Apr 2024 02:04:21 -0700 Subject: [PATCH 10/13] Refactoring Tests Code --- .../fs/azurebfs/AzureBlobFileSystemStore.java | 4 +- .../azurebfs/constants/AbfsHttpConstants.java | 3 + .../fs/azurebfs/services/AbfsClient.java | 69 +++---- .../azurebfs/AbstractAbfsIntegrationTest.java | 9 +- .../azurebfs/ITestAbfsCustomEncryption.java | 3 +- .../ITestAzureBlobFileSystemDelete.java | 8 +- .../fs/azurebfs/services/ITestAbfsClient.java | 5 +- .../services/ITestAbfsPaginatedDelete.java | 175 +++++++++++------- 8 files changed, 162 insertions(+), 114 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java index 8ece527e56a8d..484b1ed9e10b7 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java @@ -1077,8 +1077,8 @@ public void delete(final Path path, final boolean recursive, do { try (AbfsPerfInfo perfInfo = startTracking("delete", "deletePath")) { - AbfsRestOperation op = client - .deletePath(relativePath, recursive, continuation, tracingContext); + AbfsRestOperation op = client.deletePath(relativePath, recursive, + continuation, tracingContext, getIsNamespaceEnabled(tracingContext)); perfInfo.registerResult(op.getResult()); continuation = op.getResult().getResponseHeader(HttpHeaderConfigurations.X_MS_CONTINUATION); perfInfo.registerSuccess(true); diff --git a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java index af10e22fb058e..4f5ee5f9683fc 100644 --- a/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java +++ b/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/AbfsHttpConstants.java @@ -150,6 +150,9 @@ public static ApiVersion getCurrentVersion() { } } + @Deprecated + public static final String DECEMBER_2019_API_VERSION = ApiVersion.DEC_12_2019.toString(); + /** * Value that differentiates categories of the http_status. *
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 9d87c1dfc85c2..3c58a94542e5c 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
@@ -122,7 +122,6 @@ public class AbfsClient implements Closeable {
   private final ListeningScheduledExecutorService executorService;
   private Boolean isNamespaceEnabled;
 
-
   private boolean renameResilience;
 
   /**
@@ -259,13 +258,18 @@ AbfsThrottlingIntercept getIntercept() {
     return intercept;
   }
 
-  List createDefaultHeaders(ApiVersion xMsVersion) {
+  @VisibleForTesting
+  protected List createDefaultHeaders() {
+    return createDefaultHeaders(this.xMsVersion);
+  }
+
+  private List createDefaultHeaders(ApiVersion xMsVersion) {
     final List requestHeaders = new ArrayList();
     requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion.toString()));
     requestHeaders.add(new AbfsHttpHeader(ACCEPT, APPLICATION_JSON
-            + COMMA + SINGLE_WHITE_SPACE + APPLICATION_OCTET_STREAM));
+        + COMMA + SINGLE_WHITE_SPACE + APPLICATION_OCTET_STREAM));
     requestHeaders.add(new AbfsHttpHeader(ACCEPT_CHARSET,
-            UTF_8));
+        UTF_8));
     requestHeaders.add(new AbfsHttpHeader(CONTENT_TYPE, EMPTY_STRING));
     requestHeaders.add(new AbfsHttpHeader(USER_AGENT, userAgent));
     return requestHeaders;
@@ -331,7 +335,7 @@ AbfsUriQueryBuilder createDefaultUriQueryBuilder() {
 
   public AbfsRestOperation createFilesystem(TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final AbfsUriQueryBuilder abfsUriQueryBuilder = new AbfsUriQueryBuilder();
     abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM);
@@ -346,7 +350,7 @@ public AbfsRestOperation createFilesystem(TracingContext tracingContext)
 
   public AbfsRestOperation setFilesystemProperties(final String properties,
       TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     // JDK7 does not support PATCH, so to work around the issue we will use
     // PUT and specify the real method in the X-Http-Method-Override header.
     requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE,
@@ -371,7 +375,7 @@ public AbfsRestOperation setFilesystemProperties(final String properties,
   public AbfsRestOperation listPath(final String relativePath, final boolean recursive, final int listMaxResults,
                                     final String continuation, TracingContext tracingContext)
       throws IOException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
     abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM);
@@ -393,7 +397,7 @@ public AbfsRestOperation listPath(final String relativePath, final boolean recur
   }
 
   public AbfsRestOperation getFilesystemProperties(TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
     abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM);
@@ -409,7 +413,7 @@ public AbfsRestOperation getFilesystemProperties(TracingContext tracingContext)
   }
 
   public AbfsRestOperation deleteFilesystem(TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
     abfsUriQueryBuilder.addQuery(QUERY_PARAM_RESOURCE, FILESYSTEM);
@@ -460,7 +464,7 @@ public AbfsRestOperation createPath(final String path,
       final ContextEncryptionAdapter contextEncryptionAdapter,
       final TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     if (isFile) {
       addEncryptionKeyRequestHeaders(path, requestHeaders, true,
           contextEncryptionAdapter, tracingContext);
@@ -521,7 +525,7 @@ public AbfsRestOperation createPath(final String path,
   }
 
   public AbfsRestOperation acquireLease(final String path, int duration, TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, ACQUIRE_LEASE_ACTION));
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_DURATION, Integer.toString(duration)));
@@ -541,7 +545,7 @@ public AbfsRestOperation acquireLease(final String path, int duration, TracingCo
 
   public AbfsRestOperation renewLease(final String path, final String leaseId,
       TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, RENEW_LEASE_ACTION));
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, leaseId));
@@ -560,7 +564,7 @@ public AbfsRestOperation renewLease(final String path, final String leaseId,
 
   public AbfsRestOperation releaseLease(final String path,
       final String leaseId, TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, RELEASE_LEASE_ACTION));
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ID, leaseId));
@@ -579,7 +583,7 @@ public AbfsRestOperation releaseLease(final String path,
 
   public AbfsRestOperation breakLease(final String path,
       TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_ACTION, BREAK_LEASE_ACTION));
     requestHeaders.add(new AbfsHttpHeader(X_MS_LEASE_BREAK_PERIOD, DEFAULT_LEASE_BREAK_PERIOD));
@@ -627,7 +631,7 @@ public AbfsClientRenameResult renamePath(
           boolean isMetadataIncompleteState,
           boolean isNamespaceEnabled)
       throws IOException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final boolean hasEtag = !isEmpty(sourceEtag);
 
@@ -823,7 +827,7 @@ public AbfsRestOperation append(final String path, final byte[] buffer,
       AppendRequestParameters reqParams, final String cachedSasToken,
       ContextEncryptionAdapter contextEncryptionAdapter, TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     addEncryptionKeyRequestHeaders(path, requestHeaders, false,
         contextEncryptionAdapter, tracingContext);
     if (reqParams.isExpectHeaderEnabled()) {
@@ -982,7 +986,7 @@ public AbfsRestOperation flush(final String path, final long position,
       final String cachedSasToken, final String leaseId,
       ContextEncryptionAdapter contextEncryptionAdapter, TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     addEncryptionKeyRequestHeaders(path, requestHeaders, false,
         contextEncryptionAdapter, tracingContext);
     // JDK7 does not support PATCH, so to workaround the issue we will use
@@ -1015,7 +1019,7 @@ public AbfsRestOperation flush(final String path, final long position,
   public AbfsRestOperation setPathProperties(final String path, final String properties,
                                              final TracingContext tracingContext, final ContextEncryptionAdapter contextEncryptionAdapter)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     addEncryptionKeyRequestHeaders(path, requestHeaders, false,
         contextEncryptionAdapter, tracingContext);
     // JDK7 does not support PATCH, so to workaround the issue we will use
@@ -1043,7 +1047,7 @@ public AbfsRestOperation getPathStatus(final String path,
       final boolean includeProperties, final TracingContext tracingContext,
       final ContextEncryptionAdapter contextEncryptionAdapter)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
     String operation = SASTokenProvider.GET_PROPERTIES_OPERATION;
@@ -1080,7 +1084,7 @@ public AbfsRestOperation read(final String path,
       String cachedSasToken,
       ContextEncryptionAdapter contextEncryptionAdapter,
       TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     addEncryptionKeyRequestHeaders(path, requestHeaders, false,
         contextEncryptionAdapter, tracingContext);
     AbfsHttpHeader rangeHeader = new AbfsHttpHeader(RANGE,
@@ -1119,16 +1123,17 @@ public AbfsRestOperation read(final String path,
 
   public AbfsRestOperation deletePath(final String path, final boolean recursive,
                                       final String continuation,
-                                      TracingContext tracingContext)
+                                      TracingContext tracingContext,
+                                      final boolean isNamespaceEnabled)
           throws AzureBlobFileSystemException {
     final List requestHeaders
-        = (isPaginatedDelete(tracingContext, recursive)
+        = (isPaginatedDelete(recursive, isNamespaceEnabled)
         && xMsVersion.compareTo(ApiVersion.AUG_03_2023) < 0)
         ? createDefaultHeaders(ApiVersion.AUG_03_2023)
-        : createDefaultHeaders(xMsVersion);
+        : createDefaultHeaders();
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
 
-    if (isPaginatedDelete(tracingContext, recursive)) {
+    if (isPaginatedDelete(recursive, isNamespaceEnabled)) {
       // Add paginated query parameter
       abfsUriQueryBuilder.addQuery(QUERY_PARAM_PAGINATED, TRUE);
     }
@@ -1204,7 +1209,7 @@ public AbfsRestOperation deleteIdempotencyCheckOp(final AbfsRestOperation op) {
   public AbfsRestOperation setOwner(final String path, final String owner, final String group,
                                     TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     // JDK7 does not support PATCH, so to workaround the issue we will use
     // PUT and specify the real method in the X-Http-Method-Override header.
     requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE,
@@ -1234,7 +1239,7 @@ public AbfsRestOperation setOwner(final String path, final String owner, final S
   public AbfsRestOperation setPermission(final String path, final String permission,
                                          TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     // JDK7 does not support PATCH, so to workaround the issue we will use
     // PUT and specify the real method in the X-Http-Method-Override header.
     requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE,
@@ -1264,7 +1269,7 @@ public AbfsRestOperation setAcl(final String path, final String aclSpecString,
   public AbfsRestOperation setAcl(final String path, final String aclSpecString, final String eTag,
                                   TracingContext tracingContext)
       throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
     // JDK7 does not support PATCH, so to workaround the issue we will use
     // PUT and specify the real method in the X-Http-Method-Override header.
     requestHeaders.add(new AbfsHttpHeader(X_HTTP_METHOD_OVERRIDE,
@@ -1297,7 +1302,7 @@ public AbfsRestOperation getAclStatus(final String path, TracingContext tracingC
 
   public AbfsRestOperation getAclStatus(final String path, final boolean useUPN,
                                         TracingContext tracingContext) throws AzureBlobFileSystemException {
-    final List requestHeaders = createDefaultHeaders(xMsVersion);
+    final List requestHeaders = createDefaultHeaders();
 
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
     abfsUriQueryBuilder.addQuery(HttpQueryParams.QUERY_PARAM_ACTION, AbfsHttpConstants.GET_ACCESS_CONTROL);
@@ -1335,7 +1340,7 @@ public AbfsRestOperation checkAccess(String path, String rwx, TracingContext tra
         AbfsRestOperationType.CheckAccess,
         AbfsHttpConstants.HTTP_METHOD_HEAD,
         url,
-        createDefaultHeaders(xMsVersion));
+        createDefaultHeaders());
     op.execute(tracingContext);
     return op;
   }
@@ -1479,10 +1484,8 @@ protected Boolean getIsPaginatedDeleteEnabled() {
     return abfsConfiguration.isPaginatedDeleteEnabled();
   }
 
-  private Boolean isPaginatedDelete(TracingContext tracingContext,
-      boolean isRecursiveDelete) throws AzureBlobFileSystemException {
-    return getIsPaginatedDeleteEnabled()
-        && getIsNamespaceEnabled(tracingContext) && isRecursiveDelete;
+  private Boolean isPaginatedDelete(boolean isRecursiveDelete, boolean isNamespaceEnabled) {
+    return getIsPaginatedDeleteEnabled() && isNamespaceEnabled && isRecursiveDelete;
   }
 
   public AuthType getAuthType() {
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
index 16f2025f21aa2..45bf28df2ba2b 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
@@ -26,6 +26,7 @@
 import java.util.concurrent.Callable;
 
 import org.junit.After;
+import org.junit.Assume;
 import org.junit.Before;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,7 +41,6 @@
 import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
 import org.apache.hadoop.fs.azurebfs.security.AbfsDelegationTokenManager;
 import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
-import org.apache.hadoop.fs.azurebfs.services.AbfsClientUtils;
 import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
 import org.apache.hadoop.fs.azurebfs.services.AuthType;
 import org.apache.hadoop.fs.azurebfs.services.ITestAbfsClient;
@@ -215,7 +215,6 @@ public void setup() throws Exception {
       wasb = new NativeAzureFileSystem(azureNativeFileSystemStore);
       wasb.initialize(wasbUri, rawConfig);
     }
-    AbfsClientUtils.setIsNamespaceEnabled(abfs.getAbfsClient(), true);
   }
 
   @After
@@ -532,4 +531,10 @@ protected long assertAbfsStatistics(AbfsStatistic statistic,
         (long) metricMap.get(statistic.getStatName()));
     return expectedValue;
   }
+
+  protected void assumeValidTestConfigPresent(final Configuration conf, final String key) {
+    String configuredValue = conf.get(key);
+    Assume.assumeTrue(String.format("Missing Required Test Config: %s.", key),
+        configuredValue != null && !configuredValue.isEmpty());
+  }
 }
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java
index 9bd023572c263..33b05be59d5a8 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsCustomEncryption.java
@@ -323,8 +323,9 @@ private AbfsRestOperation callOperation(AzureBlobFileSystem fs,
         return client.renamePath(path, new Path(path + "_2").toString(),
           null, tc, null, false, fs.getIsNamespaceEnabled(tc)).getOp();
       case DELETE:
+        TracingContext testTC = getTestTracingContext(fs, false);
         return client.deletePath(path, false, null,
-          getTestTracingContext(fs, false));
+            testTC, fs.getIsNamespaceEnabled(testTC));
       case GET_ATTR:
         return client.getPathStatus(path, true,
             getTestTracingContext(fs, false),
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java
index 57f5702f74fab..fd5d312176321 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelete.java
@@ -242,7 +242,8 @@ public void testDeleteIdempotencyTriggerHttp404() throws Exception {
         "/NonExistingPath",
         false,
         null,
-        getTestTracingContext(fs, true)));
+        getTestTracingContext(fs, true),
+        fs.getIsNamespaceEnabled(getTestTracingContext(fs, true))));
 
     // mock idempotency check to mimic retried case
     AbfsClient mockClient = ITestAbfsClient.getMockAbfsClient(
@@ -269,14 +270,15 @@ public void testDeleteIdempotencyTriggerHttp404() throws Exception {
     doReturn(idempotencyRetOp).when(mockClient).deleteIdempotencyCheckOp(any());
     TracingContext tracingContext = getTestTracingContext(fs, false);
     doReturn(tracingContext).when(idempotencyRetOp).createNewTracingContext(any());
-    when(mockClient.deletePath("/NonExistingPath", false, null, tracingContext))
+    when(mockClient.deletePath("/NonExistingPath", false, null,
+        tracingContext, fs.getIsNamespaceEnabled(tracingContext)))
         .thenCallRealMethod();
 
     Assertions.assertThat(mockClient.deletePath(
         "/NonExistingPath",
         false,
         null,
-        tracingContext)
+        tracingContext, fs.getIsNamespaceEnabled(tracingContext))
         .getResult()
         .getStatusCode())
         .describedAs("Idempotency check reports successful "
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java
index a6d5bc713723b..c16bbf7c536f7 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java
@@ -47,7 +47,6 @@
 import org.apache.hadoop.security.ssl.DelegatingSSLSocketFactory;
 
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
-import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ApiVersion.getCurrentVersion;
 import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.APPEND_ACTION;
 import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.EXPECT_100_JDK_ERROR;
 import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.HTTP_METHOD_PATCH;
@@ -375,7 +374,7 @@ public static AbfsClient getMockAbfsClient(AbfsClient baseAbfsClientInstance,
     when(client.createRequestUrl(any(), any())).thenCallRealMethod();
     when(client.getAccessToken()).thenCallRealMethod();
     when(client.getSharedKeyCredentials()).thenCallRealMethod();
-    when(client.createDefaultHeaders(getCurrentVersion())).thenCallRealMethod();
+    when(client.createDefaultHeaders()).thenCallRealMethod();
     when(client.getAbfsConfiguration()).thenReturn(abfsConfig);
     when(client.getIntercept()).thenReturn(
         AbfsThrottlingInterceptFactory.getInstance(
@@ -449,7 +448,7 @@ public static URL getTestUrl(AbfsClient client, String path) throws
    * @return List of AbfsHttpHeaders
    */
   public static List getTestRequestHeaders(AbfsClient client) {
-    return client.createDefaultHeaders(getCurrentVersion());
+    return client.createDefaultHeaders();
   }
 
   /**
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
index 21879527bf523..418e500d5acc7 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
@@ -18,6 +18,14 @@
 
 package org.apache.hadoop.fs.azurebfs.services;
 
+import java.io.IOException;
+import java.util.List;
+import java.util.UUID;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+import org.mockito.Mockito;
+
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
@@ -35,21 +43,13 @@
 import org.apache.hadoop.fs.permission.FsAction;
 import org.apache.hadoop.util.Lists;
 
-import org.assertj.core.api.Assertions;
-import org.junit.Assume;
-import org.junit.Test;
-import org.mockito.Mockito;
-
-import java.io.IOException;
-import java.util.List;
-import java.util.UUID;
-
 import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION;
 import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME;
 import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT;
-import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION;
 import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME;
+import static org.apache.hadoop.fs.azurebfs.constants.FileSystemUriSchemes.ABFS_SECURE_SCHEME;
 import static org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations.X_MS_VERSION;
 import static org.apache.hadoop.fs.azurebfs.constants.HttpQueryParams.QUERY_PARAM_PAGINATED;
 import static org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys.FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID;
@@ -61,23 +61,47 @@
 import static org.apache.hadoop.fs.azurebfs.services.AbfsClientUtils.getHeaderValue;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
 
+/**
+ * Tests to verify server side pagination feature is supported from driver.
+ */
 public class ITestAbfsPaginatedDelete extends AbstractAbfsIntegrationTest {
 
+  /**
+   * File system using super-user OAuth, used to create the directory.
+   */
   private AzureBlobFileSystem superUserFs;
-  private AzureBlobFileSystem firstTestUserFs;
 
+  /**
+   * File system using NoRBAC user OAuth, used to delete the directory.
+   * This user will have default ACL permissions set on  root path including delete.
+   * Since this is not a super-user, azure servers will trigger recursive ACL
+   * checks on root path when delete is called using this user OAuth token.
+   */
+  private AzureBlobFileSystem testUserFs;
+
+  /**
+   * Service supports Pagination only for HNS Accounts.
+   */
   private boolean isHnsEnabled;
+
   public ITestAbfsPaginatedDelete() throws Exception {
   }
 
   @Override
   public void setup() throws Exception {
-    isHnsEnabled = this.getConfiguration().getBoolean(
-        FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false);
-    loadConfiguredFileSystem();
     super.setup();
     this.superUserFs = getFileSystem();
 
+    assumeValidTestConfigPresent(this.getRawConfiguration(),
+        FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT);
+    isHnsEnabled = this.getConfiguration().getBoolean(
+        FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false);
+
+    assumeTestUserCredentialsConfigured();
+    this.testUserFs = isHnsEnabled ? createTestUserFs() : null;
+  }
+
+  private AzureBlobFileSystem createTestUserFs() throws IOException {
     // Test User Credentials.
     String firstTestUserGuid = getConfiguration().get(
         FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID);
@@ -86,11 +110,23 @@ public void setup() throws Exception {
     String clientSecret = getConfiguration().getString(
         FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET, "");
 
-    if (isHnsEnabled) {
-      // setting up ACL permissions for test user
-      setFirstTestUserFsAuth(clientId, clientSecret);
-      setDefaultAclOnRoot(firstTestUserGuid);
-    }
+    Configuration testUserConf = new Configuration(getRawConfiguration());
+    setTestUserConf(testUserConf, FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name());
+    setTestUserConf(testUserConf, FS_AZURE_BLOB_FS_CLIENT_ID, clientId);
+    setTestUserConf(testUserConf, FS_AZURE_BLOB_FS_CLIENT_SECRET, clientSecret);
+    setTestUserConf(testUserConf, FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME,
+        ClientCredsTokenProvider.class.getName());
+
+    testUserConf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, false);
+    testUserConf.setBoolean(String.format("fs.%s.impl.disable.cache", ABFS_SECURE_SCHEME), true);
+
+    setDefaultAclOnRoot(firstTestUserGuid);
+    return (AzureBlobFileSystem) FileSystem.newInstance(testUserConf);
+  }
+
+  private void setTestUserConf (Configuration conf, String key, String value) {
+    conf.set(key, value);
+    conf.set(key + "." + getAccountName(), value);
   }
 
   /**
@@ -138,15 +174,16 @@ public void testRecursiveDeleteWithInvalidCT() throws Exception {
   }
 
   private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir,
-      boolean isPaginatedDeleteEnabled,
-      AbfsHttpConstants.ApiVersion xMsVersion) throws Exception {
+      boolean isPaginatedDeleteEnabled, AbfsHttpConstants.ApiVersion xMsVersion)
+      throws Exception {
     final AzureBlobFileSystem fs = getUserFileSystem();
-    TracingContext testTracingContext = getTestTracingContext(fs, true);
+    TracingContext testTC = getTestTracingContext(fs, true);
+
     Path testPath;
     if (isEmptyDir) {
       testPath = new Path("/emptyPath" + StringUtils.right(
           UUID.randomUUID().toString(), 10));
-      fs.mkdirs(testPath);
+      superUserFs.mkdirs(testPath);
     } else {
       testPath = createSmallDir();
     }
@@ -157,14 +194,14 @@ private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir,
     Mockito.doReturn(isPaginatedDeleteEnabled).when(spiedClient).getIsPaginatedDeleteEnabled();
 
     AbfsRestOperation op = spiedClient.deletePath(
-        testPath.toString(), true, null, testTracingContext);
+        testPath.toString(), true, null, testTC, isHnsEnabled);
 
     // Getting the xMsVersion that was used to make the request
     String xMsVersionUsed = getHeaderValue(op.getRequestHeaders(), X_MS_VERSION);
     String urlUsed = op.getUrl().toString();
 
     // Assert that appropriate xMsVersion and query param was used to make request
-    if (isPaginatedDeleteEnabled) {
+    if (isPaginatedDeleteEnabled && isHnsEnabled) {
       Assertions.assertThat(urlUsed)
           .describedAs("Url must have paginated = true as query param")
           .contains(QUERY_PARAM_PAGINATED);
@@ -188,23 +225,23 @@ private void testRecursiveDeleteWithPaginationInternal(boolean isEmptyDir,
 
     // Assert that deletion was successful in every scenario.
     AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
-        spiedClient.getPathStatus(testPath.toString(), false, testTracingContext, null));
-    Assertions.assertThat(e.getStatusCode())
-        .describedAs("Path should have been deleted").isEqualTo(HTTP_NOT_FOUND);
+        spiedClient.getPathStatus(testPath.toString(), false, testTC, null));
+    assertStatusCode(e, HTTP_NOT_FOUND);
   }
 
   private void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDeleteEnabled) throws Exception{
     final AzureBlobFileSystem fs = getUserFileSystem();
-    TracingContext testTracingContext = getTestTracingContext(fs, true);
+    TracingContext testTC = getTestTracingContext(fs, true);
+
     Path testPath = new Path("/emptyPath");
-    fs.mkdirs(testPath);
+    superUserFs.mkdirs(testPath);
 
-    // Set the paginated enabled value and xMsVersion at spiedClient level.
+    // Set the paginated enabled value at spiedClient level.
     AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient());
     Mockito.doReturn(isPaginatedDeleteEnabled).when(spiedClient).getIsPaginatedDeleteEnabled();
 
     AbfsRestOperation op = spiedClient.deletePath(
-        testPath.toString(), false, null, testTracingContext);
+        testPath.toString(), false, null, testTC, isHnsEnabled);
 
     // Getting the url that was used to make the request
     String urlUsed = op.getUrl().toString();
@@ -216,60 +253,36 @@ private void testNonRecursiveDeleteWithPaginationInternal(boolean isPaginatedDel
 
     // Assert that deletion was successful in every scenario.
     AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
-        spiedClient.getPathStatus(testPath.toString(), false, testTracingContext, null));
-    Assertions.assertThat(e.getStatusCode())
-        .describedAs("Path should have been deleted").isEqualTo(HTTP_NOT_FOUND);
+        spiedClient.getPathStatus(testPath.toString(), false, testTC, null));
+    assertStatusCode(e, HTTP_NOT_FOUND);
   }
 
   private void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled) throws Exception {
     final AzureBlobFileSystem fs = getUserFileSystem();
-    Path smallDirPath = createSmallDir();
+
+    Path testPath = createSmallDir();
     String randomCT = "randomContinuationToken1234";
-    TracingContext testTracingContext = getTestTracingContext(this.firstTestUserFs, true);
+    TracingContext testTC = getTestTracingContext(this.testUserFs, true);
 
     AbfsClient spiedClient = Mockito.spy(fs.getAbfsStore().getClient());
     Mockito.doReturn(isPaginatedEnabled).when(spiedClient).getIsPaginatedDeleteEnabled();
 
     AbfsRestOperationException e = intercept(AbfsRestOperationException.class, () ->
-        spiedClient.deletePath(
-            smallDirPath.toString(), true, randomCT, testTracingContext));
-    Assertions.assertThat(e.getStatusCode())
-        .describedAs("Request Should fail with Bad Request").isEqualTo(HTTP_BAD_REQUEST);
-  }
-
-  private AzureBlobFileSystem getUserFileSystem() {
-    // For HNS account only Server will trigger Pagination for ACL checks
-    // And for ACL Checks file system user should not be superUser.
-    return this.isHnsEnabled ? this.firstTestUserFs : this.superUserFs;
-  }
-
-  private void setFirstTestUserFsAuth(String clientId, String clientSecret) throws IOException {
-    if (this.firstTestUserFs != null) {
-      return;
-    }
-
-    // Check if OAuth Client Endpoint is provided.
-    String configKey = FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT;
-    String value = getConfiguration().get(configKey);
-    Assume.assumeTrue(configKey + " config is mandatory for the test to run",
-        value != null && value.trim().length() > 1);
-
-    // Set the required configuration
-    Configuration conf = getRawConfiguration();
-    conf.set(FS_AZURE_BLOB_FS_CLIENT_ID, clientId);
-    conf.set(FS_AZURE_BLOB_FS_CLIENT_SECRET, clientSecret);
-    conf.set(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.OAuth.name());
-    conf.set(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME,
-        ClientCredsTokenProvider.class.getName());
-    conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, false);
-    this.firstTestUserFs = (AzureBlobFileSystem) FileSystem.newInstance(getRawConfiguration());
+        spiedClient.deletePath(testPath.toString(), true, randomCT, testTC, isHnsEnabled));
+    assertStatusCode(e, HTTP_BAD_REQUEST);
   }
 
+  /**
+   * Provide test user default ACL permissions on root.
+   * @param uid
+   * @throws IOException
+   */
   private void setDefaultAclOnRoot(String uid)
       throws IOException {
-    List aclSpec =  Lists.newArrayList(AclTestHelpers.aclEntry(
+    List aclSpec = Lists.newArrayList(AclTestHelpers.aclEntry(
         AclEntryScope.ACCESS, AclEntryType.USER, uid, FsAction.ALL),
         AclTestHelpers.aclEntry(AclEntryScope.DEFAULT, AclEntryType.USER, uid, FsAction.ALL));
+    // Use SuperUser Privilege to set ACL on root for test user.
     this.superUserFs.modifyAclEntries(new Path("/"), aclSpec);
   }
 
@@ -286,4 +299,26 @@ private Path createSmallDir() throws IOException {
     }
     return new Path(rootPath);
   }
+
+  private AzureBlobFileSystem getUserFileSystem() {
+    return this.isHnsEnabled ? this.testUserFs : this.superUserFs;
+  }
+
+  private void assertStatusCode(final AbfsRestOperationException e, final int statusCode) {
+    Assertions.assertThat(e.getStatusCode())
+        .describedAs("Request Should fail with Bad Request instead of %s",
+            e.getMessage())
+        .isEqualTo(statusCode);
+  }
+
+  private void assumeTestUserCredentialsConfigured() {
+    assumeValidTestConfigPresent(getRawConfiguration(),
+        FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT);
+    assumeValidTestConfigPresent(getRawConfiguration(),
+        FS_AZURE_BLOB_FS_CHECKACCESS_TEST_USER_GUID);
+    assumeValidTestConfigPresent(getRawConfiguration(),
+        FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_ID);
+    assumeValidTestConfigPresent(getRawConfiguration(),
+        FS_AZURE_BLOB_FS_CHECKACCESS_TEST_CLIENT_SECRET);
+  }
 }

From f500632b9b690fc139fb724da63bccbca64f5d2f Mon Sep 17 00:00:00 2001
From: Anuj Modi 
Date: Tue, 2 Apr 2024 02:54:31 -0700
Subject: [PATCH 11/13] Reverting Unrequired Codechange

---
 .../apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java  | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
index 45bf28df2ba2b..7f37ae335e718 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
@@ -41,6 +41,7 @@
 import org.apache.hadoop.fs.azurebfs.oauth2.AccessTokenProvider;
 import org.apache.hadoop.fs.azurebfs.security.AbfsDelegationTokenManager;
 import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
+import org.apache.hadoop.fs.azurebfs.services.AbfsClientUtils;
 import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
 import org.apache.hadoop.fs.azurebfs.services.AuthType;
 import org.apache.hadoop.fs.azurebfs.services.ITestAbfsClient;
@@ -215,6 +216,7 @@ public void setup() throws Exception {
       wasb = new NativeAzureFileSystem(azureNativeFileSystemStore);
       wasb.initialize(wasbUri, rawConfig);
     }
+    AbfsClientUtils.setIsNamespaceEnabled(abfs.getAbfsClient(), true);
   }
 
   @After

From a31d3ae0722b103c16f10e6861850245ee64fd40 Mon Sep 17 00:00:00 2001
From: Anuj Modi 
Date: Tue, 2 Apr 2024 03:29:00 -0700
Subject: [PATCH 12/13] Addressing Comments

---
 .../fs/azurebfs/services/AbfsClient.java      | 21 ++++++++++++++++---
 .../azurebfs/AbstractAbfsIntegrationTest.java |  1 +
 .../services/ITestAbfsPaginatedDelete.java    | 13 ++++++++++--
 3 files changed, 30 insertions(+), 5 deletions(-)

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 3c58a94542e5c..ee99f992801e3 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
@@ -258,11 +258,20 @@ AbfsThrottlingIntercept getIntercept() {
     return intercept;
   }
 
+  /**
+   * Create request headers for Rest Operation using the current API version.
+   * @return default request headers
+   */
   @VisibleForTesting
   protected List createDefaultHeaders() {
     return createDefaultHeaders(this.xMsVersion);
   }
 
+  /**
+   * Create request headers for Rest Operation using the specified API version.
+   * @param xMsVersion
+   * @return default request headers
+   */
   private List createDefaultHeaders(ApiVersion xMsVersion) {
     final List requestHeaders = new ArrayList();
     requestHeaders.add(new AbfsHttpHeader(X_MS_VERSION, xMsVersion.toString()));
@@ -1126,9 +1135,15 @@ public AbfsRestOperation deletePath(final String path, final boolean recursive,
                                       TracingContext tracingContext,
                                       final boolean isNamespaceEnabled)
           throws AzureBlobFileSystemException {
-    final List requestHeaders
-        = (isPaginatedDelete(recursive, isNamespaceEnabled)
-        && xMsVersion.compareTo(ApiVersion.AUG_03_2023) < 0)
+    /*
+     * If Pagination is enabled and current API version is old,
+     * use the minimum required version for pagination.
+     * If Pagination is enabled and current API version is later than minimum required
+     * version for pagination, use current version only as azure service is backward compatible.
+     * If pagination is disabled, use the current API version only.
+     */
+    final List requestHeaders = (isPaginatedDelete(recursive,
+        isNamespaceEnabled) && xMsVersion.compareTo(ApiVersion.AUG_03_2023) < 0)
         ? createDefaultHeaders(ApiVersion.AUG_03_2023)
         : createDefaultHeaders();
     final AbfsUriQueryBuilder abfsUriQueryBuilder = createDefaultUriQueryBuilder();
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
index 7f37ae335e718..49defd46e3288 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
@@ -216,6 +216,7 @@ public void setup() throws Exception {
       wasb = new NativeAzureFileSystem(azureNativeFileSystemStore);
       wasb.initialize(wasbUri, rawConfig);
     }
+    // Todo: To be fixed in HADOOP-19137
     AbfsClientUtils.setIsNamespaceEnabled(abfs.getAbfsClient(), true);
   }
 
diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
index 418e500d5acc7..54946b443a06c 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
@@ -87,6 +87,10 @@ public class ITestAbfsPaginatedDelete extends AbstractAbfsIntegrationTest {
   public ITestAbfsPaginatedDelete() throws Exception {
   }
 
+  /**
+   * Create file system instances for both super-user and test user.
+   * @throws Exception
+   */
   @Override
   public void setup() throws Exception {
     super.setup();
@@ -273,7 +277,7 @@ private void testRecursiveDeleteWithInvalidCTInternal(boolean isPaginatedEnabled
   }
 
   /**
-   * Provide test user default ACL permissions on root.
+   * Provide test user with default ACL permissions on root.
    * @param uid
    * @throws IOException
    */
@@ -300,6 +304,11 @@ private Path createSmallDir() throws IOException {
     return new Path(rootPath);
   }
 
+  /**
+   * Select the filesystem to be used for delete API.
+   * For HNS Disabled accounts, test User FS won't have permissions as ACL is not supported
+   * @return
+   */
   private AzureBlobFileSystem getUserFileSystem() {
     return this.isHnsEnabled ? this.testUserFs : this.superUserFs;
   }
@@ -307,7 +316,7 @@ private AzureBlobFileSystem getUserFileSystem() {
   private void assertStatusCode(final AbfsRestOperationException e, final int statusCode) {
     Assertions.assertThat(e.getStatusCode())
         .describedAs("Request Should fail with Bad Request instead of %s",
-            e.getMessage())
+            e.toString())
         .isEqualTo(statusCode);
   }
 

From 47f6623fc6c1a0fa8947a231a078b25a1c2befb8 Mon Sep 17 00:00:00 2001
From: Anuj Modi 
Date: Wed, 3 Apr 2024 05:23:34 -0700
Subject: [PATCH 13/13] Checkstyle Checks

---
 .../hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
index 54946b443a06c..5dd92f430e059 100644
--- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
+++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsPaginatedDelete.java
@@ -128,7 +128,7 @@ private AzureBlobFileSystem createTestUserFs() throws IOException {
     return (AzureBlobFileSystem) FileSystem.newInstance(testUserConf);
   }
 
-  private void setTestUserConf (Configuration conf, String key, String value) {
+  private void setTestUserConf(Configuration conf, String key, String value) {
     conf.set(key, value);
     conf.set(key + "." + getAccountName(), value);
   }