Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public void endElement(final String uri,
if (parentNode.equals(AbfsHttpConstants.XML_TAG_METADATA)) {
currentBlobEntry.addMetadata(currentNode, value);
// For Marker blobs hdi_isFolder will be present as metadata
if (AbfsHttpConstants.XML_TAG_HDI_ISFOLDER.equals(currentNode)) {
if (AbfsHttpConstants.XML_TAG_HDI_ISFOLDER.equalsIgnoreCase(currentNode)) {
currentBlobEntry.setIsDirectory(Boolean.valueOf(value));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,22 @@ public Map<String, List<String>> getResponseHeaders() {
return headers;
}

/**{@inheritDoc}*/
@Override
public String getResponseHeaderIgnoreCase(final String headerName) {
Map<String, List<String>> responseHeaders = getResponseHeaders();
if (responseHeaders == null || responseHeaders.isEmpty()) {
return null;
}
// Search for the header value case-insensitively
return responseHeaders.entrySet().stream()
.filter(entry -> entry.getKey() != null
&& entry.getKey().equalsIgnoreCase(headerName))
.flatMap(entry -> entry.getValue().stream())
.findFirst()
.orElse(null); // Return null if no match is found
}

/**{@inheritDoc}*/
@Override
protected InputStream getContentInputStream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,8 @@ public AbfsRestOperation setPathProperties(final String path,
this.createPathRestOp(path, false, false, false, null,
contextEncryptionAdapter, tracingContext);
// Make sure hdi_isFolder is added to the list of properties to be set.
boolean hdiIsFolderExists = properties.containsKey(XML_TAG_HDI_ISFOLDER);
boolean hdiIsFolderExists = properties.keySet()
.stream().anyMatch(XML_TAG_HDI_ISFOLDER::equalsIgnoreCase);
if (!hdiIsFolderExists) {
properties.put(XML_TAG_HDI_ISFOLDER, TRUE);
}
Expand Down Expand Up @@ -1548,31 +1549,10 @@ public AbfsRestOperation deleteBlobPath(final Path blobPath,
*/
@Override
public boolean checkIsDir(AbfsHttpOperation result) {
String dirHeaderName = getHeaderNameIgnoreCase(result, X_MS_META_HDI_ISFOLDER);
// If the header is not found, return false (not a directory)
if (dirHeaderName == null) {
return false;
}

String resourceType = result.getResponseHeader(dirHeaderName);
String resourceType = result.getResponseHeaderIgnoreCase(X_MS_META_HDI_ISFOLDER);
return resourceType != null && resourceType.equals(TRUE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes since we already checked for null in the getHeaderNameIgnoreCase method, this repeated check may not be needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it was an existing line of code, I didn’t change it. However, it does make sense to remove this null check.

}

/**
* Get the header name case-insensitively.
* @param result executed rest operation containing response from server.
* @param header The header to be checked.
* @return Header if found, null otherwise.
*/
private String getHeaderNameIgnoreCase(AbfsHttpOperation result, String header) {
Map<String, List<String>> responseHeaders = result.getResponseHeaders();
// Search for the key case-insensitively in the headers
return responseHeaders.keySet().stream()
.filter(key -> key != null && key.equalsIgnoreCase(header))
.findFirst()
.orElse(null); // Return null if no match is found
}

/**
* Returns true if the status code lies in the range of user error.
* In the case of HTTP_CONFLICT for PutBlockList we fall back to DFS and hence
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.net.URL;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -236,6 +235,14 @@ public final InputStream getListResultStream() {

public abstract Map<String, List<String>> getResponseHeaders();

/**
* Get response header value for the given headerKey ignoring case.
*
* @param httpHeader header key.
* @return header value.
*/
public abstract String getResponseHeaderIgnoreCase(String httpHeader);

// Returns a trace message for the request
@Override
public String toString() {
Expand Down Expand Up @@ -744,14 +751,20 @@ public String getTracingContextSuffix() {

@Override
public String getResponseHeader(final String httpHeader) {
return "";
return EMPTY_STRING;
}

@Override
public Map<String, List<String>> getResponseHeaders() {
return new HashMap<>();
}

/**{@inheritDoc}*/
@Override
public String getResponseHeaderIgnoreCase(final String headerName) {
return EMPTY_STRING;
}

@Override
public void sendPayload(final byte[] buffer,
final int offset,
Expand All @@ -777,16 +790,25 @@ public AbfsHttpOperationWithFixedResultForGetFileStatus(final URL url,
@Override
public String getResponseHeader(final String httpHeader) {
// Directories on FNS-Blob are identified by a special metadata header.
if (httpHeader.equalsIgnoreCase(X_MS_META_HDI_ISFOLDER)) {
if (httpHeader.equals(X_MS_META_HDI_ISFOLDER)) {
return TRUE;
}
return EMPTY_STRING;
}

@Override
public Map<String, List<String>> getResponseHeaders() {
return Collections.singletonMap(X_MS_META_HDI_ISFOLDER,
Collections.singletonList(TRUE));
return new HashMap<>();
}

/**{@inheritDoc}*/
@Override
public String getResponseHeaderIgnoreCase(final String httpHeader) {
// Directories on FNS-Blob are identified by a special metadata header.
if (httpHeader.equalsIgnoreCase(X_MS_META_HDI_ISFOLDER)) {
return TRUE;
}
return EMPTY_STRING;
}

@Override
Expand Down Expand Up @@ -937,14 +959,20 @@ public String getTracingContextSuffix() {

@Override
public String getResponseHeader(final String httpHeader) {
return "";
return EMPTY_STRING;
}

@Override
public Map<String, List<String>> getResponseHeaders() {
return new HashMap<>();
}

/**{@inheritDoc}*/
@Override
public String getResponseHeaderIgnoreCase(final String headerName) {
return EMPTY_STRING;
}

@Override
public void sendPayload(final byte[] buffer,
final int offset,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,22 @@ public Map<String, List<String>> getResponseHeaders() {
return connection.getHeaderFields();
}

/**{@inheritDoc}*/
@Override
public String getResponseHeaderIgnoreCase(final String headerName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both the children are using the same method implementation, same can be added in parent itself if it won't change in future for other children if added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three more implementations of AbfsHttpOperation where the implementation is different. Thats the reason I have implemented it separately for all.

Map<String, List<String>> responseHeaders = getResponseHeaders();
if (responseHeaders == null || responseHeaders.isEmpty()) {
return null;
}
// Search for the header value case-insensitively
return responseHeaders.entrySet().stream()
.filter(entry -> entry.getKey() != null
&& entry.getKey().equalsIgnoreCase(headerName))
.flatMap(entry -> entry.getValue().stream())
.findFirst()
.orElse(null); // Return null if no match is found
}

/**{@inheritDoc}*/
public void sendPayload(byte[] buffer, int offset, int length)
throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.azurebfs.services.AbfsBlobClient;
import org.apache.hadoop.fs.azurebfs.services.AbfsHttpOperation;
import org.apache.hadoop.fs.permission.FsPermission;

import static org.apache.hadoop.fs.CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY;
import static org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemListStatus.mockAbfsRestOperation;
import static org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemListStatus.mockIngressClientHandler;
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists;
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -284,4 +287,61 @@ private void verifyFileNotFound(FileNotFoundException ex, String key) {
Assertions.assertThat(ex).isNotNull();
Assertions.assertThat(ex.getMessage()).contains(key);
}

/**
* Test directory status with different HDI folder configuration,
* verifying the correct header and directory state.
*/
private void testIsDirectory(boolean expected, String... configName) throws Exception {
try (AzureBlobFileSystem fs = Mockito.spy(getFileSystem())) {
assumeBlobServiceType();
AbfsBlobClient abfsBlobClient = mockIngressClientHandler(fs);
// Mock the operation to modify the headers
mockAbfsRestOperation(abfsBlobClient, configName);

// Create the path and invoke mkdirs method
Path path = new Path("/testPath");
fs.mkdirs(path);

// Assert that the response header has the updated value
FileStatus fileStatus = fs.getFileStatus(path);

AbfsHttpOperation op = abfsBlobClient.getPathStatus(
path.toUri().getPath(),
true, getTestTracingContext(fs, true),
null).getResult();

Assertions.assertThat(abfsBlobClient.checkIsDir(op))
.describedAs("Directory should be marked as " + expected)
.isEqualTo(expected);

// Verify the header and directory state
Assertions.assertThat(fileStatus.isDirectory())
.describedAs("Expected directory state: " + expected)
.isEqualTo(expected);

fs.delete(path, true);
}
}

/**
* Test to verify the directory status with different HDI folder configurations.
* Verifying the correct header and directory state.
*/
@Test
public void testIsDirectoryWithDifferentCases() throws Exception {
testIsDirectory(true, "HDI_ISFOLDER");

testIsDirectory(true, "Hdi_ISFOLDER");

testIsDirectory(true, "Hdi_isfolder");

testIsDirectory(true, "hdi_isfolder");

testIsDirectory(false, "Hdi_isfolder1");

testIsDirectory(true, "HDI_ISFOLDER", "Hdi_ISFOLDER", "Hdi_isfolder");

testIsDirectory(true, "HDI_ISFOLDER", "Hdi_ISFOLDER1", "Test");
}
}
Loading