resolving comments

apache · Jan 24, 2025 · e339293 · e339293
1 parent 401052c
commit e339293
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 10 deletions.
diff --git a/...src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java b/...src/main/java/org/apache/hadoop/fs/azurebfs/contracts/services/AzureServiceErrorCode.java
@@ -58,8 +58,9 @@ public enum AzureServiceErrorCode {
   ACCOUNT_REQUIRES_HTTPS("AccountRequiresHttps", HttpURLConnection.HTTP_BAD_REQUEST, null),
   MD5_MISMATCH("Md5Mismatch", HttpURLConnection.HTTP_BAD_REQUEST,
           "The MD5 value specified in the request did not match with the MD5 value calculated by the server."),
-  UNKNOWN(null, -1, null),
-  UNAUTHORIZED_BLOB_OVERWRITE("UnauthorizedBlobOverwrite", HttpURLConnection.HTTP_FORBIDDEN, "This request is not authorized to perform blob overwrites.");
+  UNAUTHORIZED_BLOB_OVERWRITE("UnauthorizedBlobOverwrite", HttpURLConnection.HTTP_FORBIDDEN,
+          "This request is not authorized to perform blob overwrites."),
+  UNKNOWN(null, -1, null);
 
   private final String errorCode;
   private final int httpStatusCode;

diff --git a/...ools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java b/...ools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsDfsClient.java
@@ -129,6 +129,7 @@
 import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.RENAME_DESTINATION_PARENT_PATH_NOT_FOUND;
 import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.SOURCE_PATH_NOT_FOUND;
 import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.UNAUTHORIZED_BLOB_OVERWRITE;
+import static org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_FILE_ALREADY_EXISTS;
 
 /**
  * AbfsClient interacting with the DFS Endpoint.
@@ -615,9 +616,12 @@ public AbfsClientRenameResult renamePath(
         throw e;
       }
 
+      // ref: HADOOP-19393. Write permission checks can occur before validating
+      // rename operation's validity. If there is an existing destination path, it may be rejected
+      // with an authorization error. Catching and throwing FileAlreadyExistsException instead.
       if(op.getResult().getStorageErrorCode()
           .equals(UNAUTHORIZED_BLOB_OVERWRITE.getErrorCode())){
-        throw new FileAlreadyExistsException("File already exists.");
+        throw new FileAlreadyExistsException(ERR_FILE_ALREADY_EXISTS);
       }
 
       // ref: HADOOP-18242. Rename failure occurring due to a rare case of

diff --git a/...p-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsErrors.java b/...p-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsErrors.java
@@ -28,6 +28,7 @@
 @InterfaceAudience.Public
 @InterfaceStability.Evolving
 public final class AbfsErrors {
+  public static final String ERR_FILE_ALREADY_EXISTS = "FIle already exists.";
   public static final String ERR_WRITE_WITHOUT_LEASE = "Attempted to write to file without lease";
   public static final String ERR_LEASE_EXPIRED = "A lease ID was specified, but the lease for the"
       + " resource has expired";

diff --git a/...re/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelegationSAS.java b/...re/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemDelegationSAS.java
@@ -53,6 +53,7 @@
 
 import static org.apache.hadoop.fs.azurebfs.constants.ConfigurationKeys.FS_AZURE_SAS_TOKEN_PROVIDER_TYPE;
 import static org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode.AUTHORIZATION_PERMISSION_MISS_MATCH;
+import static org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_FILE_ALREADY_EXISTS;
 import static org.apache.hadoop.fs.azurebfs.utils.AclTestHelpers.aclEntry;
 import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
 import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists;
@@ -223,13 +224,7 @@ public void checkExceptionForRenameOverwrites() throws Exception {
     touch(src);
     touch(dest);
 
-    try {
-      fs.rename(src, dest);
-      Assertions.fail("Exception expected on rename overwrites.");
-    } catch (FileAlreadyExistsException e) {
-      Assertions.assertThat(e)
-          .isInstanceOf(FileAlreadyExistsException.class);
-    }
+    intercept(FileAlreadyExistsException.class, ERR_FILE_ALREADY_EXISTS, () -> fs.rename(src, dest));
   }
 
   @Test