HADOOP-19089: [ABFS] Reverting Back Support of setXAttr() and getXAttr() on root path (#6592)

This reverts most of
HADOOP-18869: [ABFS] Fix behavior of a File System APIs on root path (#6003).

Calling getXAttr("/") or setXAttr("/") on an abfs container will fail with

`Operation failed: "The request URI is invalid.", HTTP 400 Bad Request`

 
This change is to ensure:
* Consistency across ADLS clients
* Consistency across authentication mechanisms.

Contributed by Anuj Modi

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java

@@ -952,7 +952,7 @@ public class AzureBlobFileSystem extends FileSystem
   }
 
   /**
-   * Set the value of an attribute for a path.
+   * Set the value of an attribute for a non-root path.
    *
    * @param path The path on which to set the attribute
    * @param name The attribute to set
@@ -979,32 +979,22 @@ public class AzureBlobFileSystem extends FileSystem
       TracingContext tracingContext = new TracingContext(clientCorrelationId,
           fileSystemId, FSOperationType.SET_ATTR, true, tracingHeaderFormat,
           listener);
-      Hashtable<String, String> properties;
+      Hashtable<String, String> properties = abfsStore
+          .getPathStatus(qualifiedPath, tracingContext);
       String xAttrName = ensureValidAttributeName(name);
-
-      if (path.isRoot()) {
-        properties = abfsStore.getFilesystemProperties(tracingContext);
-      } else {
-        properties = abfsStore.getPathStatus(qualifiedPath, tracingContext);
-      }
-
       boolean xAttrExists = properties.containsKey(xAttrName);
       XAttrSetFlag.validate(name, xAttrExists, flag);
 
       String xAttrValue = abfsStore.decodeAttribute(value);
       properties.put(xAttrName, xAttrValue);
-      if (path.isRoot()) {
-        abfsStore.setFilesystemProperties(properties, tracingContext);
-      } else {
-        abfsStore.setPathProperties(qualifiedPath, properties, tracingContext);
-      }
+      abfsStore.setPathProperties(qualifiedPath, properties, tracingContext);
     } catch (AzureBlobFileSystemException ex) {
       checkException(path, ex);
     }
   }
 
   /**
-   * Get the value of an attribute for a path.
+   * Get the value of an attribute for a non-root path.
    *
    * @param path The path on which to get the attribute
    * @param name The attribute to get
@@ -1029,15 +1019,9 @@ public class AzureBlobFileSystem extends FileSystem
       TracingContext tracingContext = new TracingContext(clientCorrelationId,
           fileSystemId, FSOperationType.GET_ATTR, true, tracingHeaderFormat,
           listener);
-      Hashtable<String, String> properties;
+      Hashtable<String, String> properties = abfsStore
+          .getPathStatus(qualifiedPath, tracingContext);
       String xAttrName = ensureValidAttributeName(name);
-
-      if (path.isRoot()) {
-        properties = abfsStore.getFilesystemProperties(tracingContext);
-      } else {
-        properties = abfsStore.getPathStatus(qualifiedPath, tracingContext);
-      }
-
       if (properties.containsKey(xAttrName)) {
         String xAttrValue = properties.get(xAttrName);
         value = abfsStore.encodeAttribute(xAttrValue);

hadoop-tools/hadoop-azure/src/site/markdown/abfs.md

@@ -1229,6 +1229,11 @@ The fix is to mimic the ownership to the local OS user, by adding the below prop
 
 Once the above properties are configured, `hdfs dfs -ls abfs://container1@abfswales1.dfs.core.windows.net/` shows the ADLS Gen2 files/directories are now owned by 'user1'.
 
+## <a name="KnownIssues"></a> Known Issues
+
+Following failures are known and expected to fail as of now.
+1. AzureBlobFileSystem.setXAttr() and AzureBlobFileSystem.getXAttr() will fail when attempted on root ("/") path with `Operation failed: "The request URI is invalid.", HTTP 400 Bad Request`
+
 ## <a name="testing"></a> Testing ABFS
 
 See the relevant section in [Testing Azure](testing_azure.html).

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemAttributes.java

@@ -27,8 +27,11 @@ import org.junit.Test;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.XAttrSetFlag;
 import org.apache.hadoop.fs.azurebfs.constants.FSOperationType;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
 import org.apache.hadoop.fs.azurebfs.utils.TracingHeaderValidator;
 
+import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
+import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ROOT_PATH;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
 
 /**
@@ -45,8 +48,7 @@ public class ITestAzureBlobFileSystemAttributes extends AbstractAbfsIntegrationT
   @Test
   public void testSetGetXAttr() throws Exception {
     AzureBlobFileSystem fs = getFileSystem();
-    AbfsConfiguration conf = fs.getAbfsStore().getAbfsConfiguration();
-    final Path testPath = path("setGetXAttr");
+    final Path testPath = path(getMethodName());
     fs.create(testPath);
     testGetSetXAttrHelper(fs, testPath);
   }
@@ -56,7 +58,7 @@ public class ITestAzureBlobFileSystemAttributes extends AbstractAbfsIntegrationT
     AzureBlobFileSystem fs = getFileSystem();
     byte[] attributeValue = fs.getAbfsStore().encodeAttribute("one");
     String attributeName = "user.someAttribute";
-    Path testFile = path("createReplaceXAttr");
+    Path testFile = path(getMethodName());
 
     // after creating a file, it must be possible to create a new xAttr
     touch(testFile);
@@ -75,7 +77,7 @@ public class ITestAzureBlobFileSystemAttributes extends AbstractAbfsIntegrationT
     byte[] attributeValue1 = fs.getAbfsStore().encodeAttribute("one");
     byte[] attributeValue2 = fs.getAbfsStore().encodeAttribute("two");
     String attributeName = "user.someAttribute";
-    Path testFile = path("replaceXAttr");
+    Path testFile = path(getMethodName());
 
     // after creating a file, it must not be possible to replace an xAttr
     intercept(IOException.class, () -> {
@@ -91,13 +93,6 @@ public class ITestAzureBlobFileSystemAttributes extends AbstractAbfsIntegrationT
         .containsExactly(attributeValue2);
   }
 
-  @Test
-  public void testGetSetXAttrOnRoot() throws Exception {
-    AzureBlobFileSystem fs = getFileSystem();
-    final Path testPath = new Path("/");
-    testGetSetXAttrHelper(fs, testPath);
-  }
-
   private void testGetSetXAttrHelper(final AzureBlobFileSystem fs,
       final Path testPath) throws Exception {
 
@@ -154,4 +149,28 @@ public class ITestAzureBlobFileSystemAttributes extends AbstractAbfsIntegrationT
         .describedAs("Retrieved Attribute Does not Matches in Decoded Form")
         .isEqualTo(decodedAttributeValue2);
   }
+
+  @Test
+  public void testGetSetXAttrOnRoot() throws Exception {
+    AzureBlobFileSystem fs = getFileSystem();
+    String attributeName = "user.attribute1";
+    byte[] attributeValue = fs.getAbfsStore().encodeAttribute("hi");
+    final Path testPath = new Path(ROOT_PATH);
+
+    AbfsRestOperationException ex = intercept(AbfsRestOperationException.class, () -> {
+      fs.getXAttr(testPath, attributeName);
+    });
+
+    Assertions.assertThat(ex.getStatusCode())
+        .describedAs("GetXAttr() on root should fail with Bad Request")
+        .isEqualTo(HTTP_BAD_REQUEST);
+
+    ex = intercept(AbfsRestOperationException.class, () -> {
+      fs.setXAttr(testPath, attributeName, attributeValue, CREATE_FLAG);
+    });
+
+    Assertions.assertThat(ex.getStatusCode())
+        .describedAs("SetXAttr() on root should fail with Bad Request")
+        .isEqualTo(HTTP_BAD_REQUEST);
+  }
 }