Skip to content

Commit 6c6df40

Browse files
authored
HADOOP-18869: [ABFS] Fix behavior of a File System APIs on root path (#6003)
Contributed by Anmol Asrani
1 parent 9c621fc commit 6c6df40

File tree

6 files changed

+137
-40
lines changed

6 files changed

+137
-40
lines changed

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

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@
110110
import org.apache.hadoop.util.LambdaUtils;
111111
import org.apache.hadoop.util.Progressable;
112112

113+
import static java.net.HttpURLConnection.HTTP_CONFLICT;
113114
import static org.apache.hadoop.fs.CommonConfigurationKeys.IOSTATISTICS_LOGGING_LEVEL;
114115
import static org.apache.hadoop.fs.CommonConfigurationKeys.IOSTATISTICS_LOGGING_LEVEL_DEFAULT;
115116
import static org.apache.hadoop.fs.Options.OpenFileOptions.FS_OPTION_OPENFILE_STANDARD_OPTIONS;
@@ -120,6 +121,7 @@
120121
import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.BLOCK_UPLOAD_ACTIVE_BLOCKS_DEFAULT;
121122
import static org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DATA_BLOCKS_BUFFER_DEFAULT;
122123
import static org.apache.hadoop.fs.azurebfs.constants.InternalConstants.CAPABILITY_SAFE_READAHEAD;
124+
import static org.apache.hadoop.fs.azurebfs.services.AbfsErrors.ERR_CREATE_ON_ROOT;
123125
import static org.apache.hadoop.fs.impl.PathCapabilitiesSupport.validatePathCapabilityArgs;
124126
import static org.apache.hadoop.fs.statistics.IOStatisticsLogging.logIOStatisticsAtLevel;
125127
import static org.apache.hadoop.util.functional.RemoteIterators.filteringRemoteIterator;
@@ -324,6 +326,12 @@ public FSDataOutputStream create(final Path f,
324326

325327
statIncrement(CALL_CREATE);
326328
trailingPeriodCheck(f);
329+
if (f.isRoot()) {
330+
throw new AbfsRestOperationException(HTTP_CONFLICT,
331+
AzureServiceErrorCode.PATH_CONFLICT.getErrorCode(),
332+
ERR_CREATE_ON_ROOT,
333+
null);
334+
}
327335

328336
Path qualifiedPath = makeQualified(f);
329337

@@ -348,6 +356,12 @@ public FSDataOutputStream createNonRecursive(final Path f, final FsPermission pe
348356
final Progressable progress) throws IOException {
349357

350358
statIncrement(CALL_CREATE_NON_RECURSIVE);
359+
if (f.isRoot()) {
360+
throw new AbfsRestOperationException(HTTP_CONFLICT,
361+
AzureServiceErrorCode.PATH_CONFLICT.getErrorCode(),
362+
ERR_CREATE_ON_ROOT,
363+
null);
364+
}
351365
final Path parent = f.getParent();
352366
TracingContext tracingContext = new TracingContext(clientCorrelationId,
353367
fileSystemId, FSOperationType.CREATE_NON_RECURSIVE, tracingHeaderFormat,
@@ -965,15 +979,25 @@ public void setXAttr(final Path path,
965979
TracingContext tracingContext = new TracingContext(clientCorrelationId,
966980
fileSystemId, FSOperationType.SET_ATTR, true, tracingHeaderFormat,
967981
listener);
968-
Hashtable<String, String> properties = abfsStore
969-
.getPathStatus(qualifiedPath, tracingContext);
982+
Hashtable<String, String> properties;
970983
String xAttrName = ensureValidAttributeName(name);
984+
985+
if (path.isRoot()) {
986+
properties = abfsStore.getFilesystemProperties(tracingContext);
987+
} else {
988+
properties = abfsStore.getPathStatus(qualifiedPath, tracingContext);
989+
}
990+
971991
boolean xAttrExists = properties.containsKey(xAttrName);
972992
XAttrSetFlag.validate(name, xAttrExists, flag);
973993

974994
String xAttrValue = abfsStore.decodeAttribute(value);
975995
properties.put(xAttrName, xAttrValue);
976-
abfsStore.setPathProperties(qualifiedPath, properties, tracingContext);
996+
if (path.isRoot()) {
997+
abfsStore.setFilesystemProperties(properties, tracingContext);
998+
} else {
999+
abfsStore.setPathProperties(qualifiedPath, properties, tracingContext);
1000+
}
9771001
} catch (AzureBlobFileSystemException ex) {
9781002
checkException(path, ex);
9791003
}
@@ -1005,9 +1029,15 @@ public byte[] getXAttr(final Path path, final String name)
10051029
TracingContext tracingContext = new TracingContext(clientCorrelationId,
10061030
fileSystemId, FSOperationType.GET_ATTR, true, tracingHeaderFormat,
10071031
listener);
1008-
Hashtable<String, String> properties = abfsStore
1009-
.getPathStatus(qualifiedPath, tracingContext);
1032+
Hashtable<String, String> properties;
10101033
String xAttrName = ensureValidAttributeName(name);
1034+
1035+
if (path.isRoot()) {
1036+
properties = abfsStore.getFilesystemProperties(tracingContext);
1037+
} else {
1038+
properties = abfsStore.getPathStatus(qualifiedPath, tracingContext);
1039+
}
1040+
10111041
if (properties.containsKey(xAttrName)) {
10121042
String xAttrValue = properties.get(xAttrName);
10131043
value = abfsStore.encodeAttribute(xAttrValue);
@@ -1488,7 +1518,7 @@ public static void checkException(final Path path,
14881518
case HttpURLConnection.HTTP_NOT_FOUND:
14891519
throw (IOException) new FileNotFoundException(message)
14901520
.initCause(exception);
1491-
case HttpURLConnection.HTTP_CONFLICT:
1521+
case HTTP_CONFLICT:
14921522
throw (IOException) new FileAlreadyExistsException(message)
14931523
.initCause(exception);
14941524
case HttpURLConnection.HTTP_FORBIDDEN:

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ public void setPathProperties(final Path path,
494494
final Hashtable<String, String> properties, TracingContext tracingContext)
495495
throws AzureBlobFileSystemException {
496496
try (AbfsPerfInfo perfInfo = startTracking("setPathProperties", "setPathProperties")){
497-
LOG.debug("setFilesystemProperties for filesystem: {} path: {} with properties: {}",
497+
LOG.debug("setPathProperties for filesystem: {} path: {} with properties: {}",
498498
client.getFileSystem(),
499499
path,
500500
properties);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,6 @@ public final class AbfsErrors {
4848
+ "operation";
4949
public static final String ERR_NO_LEASE_THREADS = "Lease desired but no lease threads "
5050
+ "configured, set " + FS_AZURE_LEASE_THREADS;
51-
51+
public static final String ERR_CREATE_ON_ROOT = "Cannot create file over root path";
5252
private AbfsErrors() {}
5353
}

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

Lines changed: 74 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import java.io.IOException;
2222
import java.util.EnumSet;
2323

24-
import org.junit.Assume;
24+
import org.assertj.core.api.Assertions;
2525
import org.junit.Test;
2626

2727
import org.apache.hadoop.fs.Path;
@@ -46,45 +46,24 @@ public ITestAzureBlobFileSystemAttributes() throws Exception {
4646
public void testSetGetXAttr() throws Exception {
4747
AzureBlobFileSystem fs = getFileSystem();
4848
AbfsConfiguration conf = fs.getAbfsStore().getAbfsConfiguration();
49-
Assume.assumeTrue(getIsNamespaceEnabled(fs));
50-
51-
byte[] attributeValue1 = fs.getAbfsStore().encodeAttribute("hi");
52-
byte[] attributeValue2 = fs.getAbfsStore().encodeAttribute("你好");
53-
String attributeName1 = "user.asciiAttribute";
54-
String attributeName2 = "user.unicodeAttribute";
55-
Path testFile = path("setGetXAttr");
56-
57-
// after creating a file, the xAttr should not be present
58-
touch(testFile);
59-
assertNull(fs.getXAttr(testFile, attributeName1));
60-
61-
// after setting the xAttr on the file, the value should be retrievable
62-
fs.registerListener(
63-
new TracingHeaderValidator(conf.getClientCorrelationId(),
64-
fs.getFileSystemId(), FSOperationType.SET_ATTR, true, 0));
65-
fs.setXAttr(testFile, attributeName1, attributeValue1);
66-
fs.setListenerOperation(FSOperationType.GET_ATTR);
67-
assertArrayEquals(attributeValue1, fs.getXAttr(testFile, attributeName1));
68-
fs.registerListener(null);
69-
70-
// after setting a second xAttr on the file, the first xAttr values should not be overwritten
71-
fs.setXAttr(testFile, attributeName2, attributeValue2);
72-
assertArrayEquals(attributeValue1, fs.getXAttr(testFile, attributeName1));
73-
assertArrayEquals(attributeValue2, fs.getXAttr(testFile, attributeName2));
49+
final Path testPath = path("setGetXAttr");
50+
fs.create(testPath);
51+
testGetSetXAttrHelper(fs, testPath);
7452
}
7553

7654
@Test
7755
public void testSetGetXAttrCreateReplace() throws Exception {
7856
AzureBlobFileSystem fs = getFileSystem();
79-
Assume.assumeTrue(getIsNamespaceEnabled(fs));
8057
byte[] attributeValue = fs.getAbfsStore().encodeAttribute("one");
8158
String attributeName = "user.someAttribute";
8259
Path testFile = path("createReplaceXAttr");
8360

8461
// after creating a file, it must be possible to create a new xAttr
8562
touch(testFile);
8663
fs.setXAttr(testFile, attributeName, attributeValue, CREATE_FLAG);
87-
assertArrayEquals(attributeValue, fs.getXAttr(testFile, attributeName));
64+
Assertions.assertThat(fs.getXAttr(testFile, attributeName))
65+
.describedAs("Retrieved Attribute Value is Not as Expected")
66+
.containsExactly(attributeValue);
8867

8968
// however after the xAttr is created, creating it again must fail
9069
intercept(IOException.class, () -> fs.setXAttr(testFile, attributeName, attributeValue, CREATE_FLAG));
@@ -93,7 +72,6 @@ public void testSetGetXAttrCreateReplace() throws Exception {
9372
@Test
9473
public void testSetGetXAttrReplace() throws Exception {
9574
AzureBlobFileSystem fs = getFileSystem();
96-
Assume.assumeTrue(getIsNamespaceEnabled(fs));
9775
byte[] attributeValue1 = fs.getAbfsStore().encodeAttribute("one");
9876
byte[] attributeValue2 = fs.getAbfsStore().encodeAttribute("two");
9977
String attributeName = "user.someAttribute";
@@ -108,6 +86,72 @@ public void testSetGetXAttrReplace() throws Exception {
10886
// however after the xAttr is created, replacing it must succeed
10987
fs.setXAttr(testFile, attributeName, attributeValue1, CREATE_FLAG);
11088
fs.setXAttr(testFile, attributeName, attributeValue2, REPLACE_FLAG);
111-
assertArrayEquals(attributeValue2, fs.getXAttr(testFile, attributeName));
89+
Assertions.assertThat(fs.getXAttr(testFile, attributeName))
90+
.describedAs("Retrieved Attribute Value is Not as Expected")
91+
.containsExactly(attributeValue2);
92+
}
93+
94+
@Test
95+
public void testGetSetXAttrOnRoot() throws Exception {
96+
AzureBlobFileSystem fs = getFileSystem();
97+
final Path testPath = new Path("/");
98+
testGetSetXAttrHelper(fs, testPath);
99+
}
100+
101+
private void testGetSetXAttrHelper(final AzureBlobFileSystem fs,
102+
final Path testPath) throws Exception {
103+
104+
String attributeName1 = "user.attribute1";
105+
String attributeName2 = "user.attribute2";
106+
String decodedAttributeValue1 = "hi";
107+
String decodedAttributeValue2 = "hello";
108+
byte[] attributeValue1 = fs.getAbfsStore().encodeAttribute(decodedAttributeValue1);
109+
byte[] attributeValue2 = fs.getAbfsStore().encodeAttribute(decodedAttributeValue2);
110+
111+
// Attribute not present initially
112+
Assertions.assertThat(fs.getXAttr(testPath, attributeName1))
113+
.describedAs("Cannot get attribute before setting it")
114+
.isNull();
115+
Assertions.assertThat(fs.getXAttr(testPath, attributeName2))
116+
.describedAs("Cannot get attribute before setting it")
117+
.isNull();
118+
119+
// Set the Attributes
120+
fs.registerListener(
121+
new TracingHeaderValidator(fs.getAbfsStore().getAbfsConfiguration()
122+
.getClientCorrelationId(),
123+
fs.getFileSystemId(), FSOperationType.SET_ATTR, true, 0));
124+
fs.setXAttr(testPath, attributeName1, attributeValue1);
125+
126+
// Check if the attribute is retrievable
127+
fs.setListenerOperation(FSOperationType.GET_ATTR);
128+
byte[] rv = fs.getXAttr(testPath, attributeName1);
129+
Assertions.assertThat(rv)
130+
.describedAs("Retrieved Attribute Does not Matches in Encoded Form")
131+
.containsExactly(attributeValue1);
132+
Assertions.assertThat(fs.getAbfsStore().decodeAttribute(rv))
133+
.describedAs("Retrieved Attribute Does not Matches in Decoded Form")
134+
.isEqualTo(decodedAttributeValue1);
135+
fs.registerListener(null);
136+
137+
// Set the second Attribute
138+
fs.setXAttr(testPath, attributeName2, attributeValue2);
139+
140+
// Check all the attributes present and previous Attribute not overridden
141+
rv = fs.getXAttr(testPath, attributeName1);
142+
Assertions.assertThat(rv)
143+
.describedAs("Retrieved Attribute Does not Matches in Encoded Form")
144+
.containsExactly(attributeValue1);
145+
Assertions.assertThat(fs.getAbfsStore().decodeAttribute(rv))
146+
.describedAs("Retrieved Attribute Does not Matches in Decoded Form")
147+
.isEqualTo(decodedAttributeValue1);
148+
149+
rv = fs.getXAttr(testPath, attributeName2);
150+
Assertions.assertThat(rv)
151+
.describedAs("Retrieved Attribute Does not Matches in Encoded Form")
152+
.containsExactly(attributeValue2);
153+
Assertions.assertThat(fs.getAbfsStore().decodeAttribute(rv))
154+
.describedAs("Retrieved Attribute Does not Matches in Decoded Form")
155+
.isEqualTo(decodedAttributeValue2);
112156
}
113157
}

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.EnumSet;
2626
import java.util.UUID;
2727

28+
import org.assertj.core.api.Assertions;
2829
import org.junit.Test;
2930

3031
import org.apache.hadoop.conf.Configuration;
@@ -33,6 +34,7 @@
3334
import org.apache.hadoop.fs.FileSystem;
3435
import org.apache.hadoop.fs.FSDataOutputStream;
3536
import org.apache.hadoop.fs.Path;
37+
import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
3638
import org.apache.hadoop.fs.permission.FsAction;
3739
import org.apache.hadoop.fs.permission.FsPermission;
3840
import org.apache.hadoop.test.GenericTestUtils;
@@ -146,6 +148,26 @@ public void testCreateNonRecursive2() throws Exception {
146148
assertIsFile(fs, testFile);
147149
}
148150

151+
@Test
152+
public void testCreateOnRoot() throws Exception {
153+
final AzureBlobFileSystem fs = getFileSystem();
154+
Path testFile = path(AbfsHttpConstants.ROOT_PATH);
155+
AbfsRestOperationException ex = intercept(AbfsRestOperationException.class, () ->
156+
fs.create(testFile, true));
157+
if (ex.getStatusCode() != HTTP_CONFLICT) {
158+
// Request should fail with 409.
159+
throw ex;
160+
}
161+
162+
ex = intercept(AbfsRestOperationException.class, () ->
163+
fs.createNonRecursive(testFile, FsPermission.getDefault(),
164+
false, 1024, (short) 1, 1024, null));
165+
if (ex.getStatusCode() != HTTP_CONFLICT) {
166+
// Request should fail with 409.
167+
throw ex;
168+
}
169+
}
170+
149171
/**
150172
* Attempts to use to the ABFS stream after it is closed.
151173
*/
@@ -190,7 +212,8 @@ public void testTryWithResources() throws Throwable {
190212
// the exception raised in close() must be in the caught exception's
191213
// suppressed list
192214
Throwable[] suppressed = fnfe.getSuppressed();
193-
assertEquals("suppressed count", 1, suppressed.length);
215+
Assertions.assertThat(suppressed.length)
216+
.describedAs("suppressed count should be 1").isEqualTo(1);
194217
Throwable inner = suppressed[0];
195218
if (!(inner instanceof IOException)) {
196219
throw inner;

hadoop-tools/hadoop-azure/src/test/resources/abfs.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
<configuration xmlns:xi="http://www.w3.org/2001/XInclude">
2020
<property>
2121
<name>fs.contract.test.root-tests-enabled</name>
22-
<value>false</value>
22+
<value>true</value>
2323
</property>
2424

2525
<property>

0 commit comments

Comments
 (0)