Skip to content

Commit e63092d

Browse files
fix tests
1 parent 44be31e commit e63092d

File tree

2 files changed

+38
-44
lines changed

2 files changed

+38
-44
lines changed

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

Lines changed: 36 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.io.UnsupportedEncodingException;
2929
import java.net.HttpURLConnection;
3030
import java.net.URL;
31+
import java.net.URLDecoder;
3132
import java.net.URLEncoder;
3233
import java.nio.charset.CharacterCodingException;
3334
import java.nio.charset.Charset;
@@ -457,7 +458,7 @@ public void createNonRecursivePreCheck(Path parentPath,
457458

458459
/**
459460
* Get Rest Operation for API
460-
* <a href="https://learn.microsoft.com/en-us/rest/api/storageservices/put-blob">Put Blob</a>.
461+
* <a href="../../../../site/markdown/blobEndpoint.md#put-blob">Put Blob</a>.
461462
* Creates a file or directory (marker file) at the specified path.
462463
*
463464
* @param path the path of the directory to be created.
@@ -487,7 +488,7 @@ public AbfsRestOperation createPath(final String path,
487488
contextEncryptionAdapter, tracingContext);
488489
} else {
489490
// Create a directory with the specified parameters
490-
op = createDirectory(path, overwrite, permissions, isAppendBlob, eTag,
491+
op = createDirectory(path, permissions, isAppendBlob, eTag,
491492
contextEncryptionAdapter, tracingContext);
492493
}
493494
return op;
@@ -634,7 +635,6 @@ private boolean checkEmptyDirectoryPathExists(final String path,
634635
* Creates a directory at the specified path.
635636
*
636637
* @param path the path of the directory to be created.
637-
* @param overwrite whether to overwrite the existing directory.
638638
* @param permissions the permissions to be set for the directory.
639639
* @param isAppendBlob whether the directory is an append blob.
640640
* @param eTag the eTag of the directory.
@@ -644,7 +644,6 @@ private boolean checkEmptyDirectoryPathExists(final String path,
644644
* @throws AzureBlobFileSystemException if the rest operation fails.
645645
*/
646646
private AbfsRestOperation createDirectory(final String path,
647-
final boolean overwrite,
648647
final AzureBlobFileSystemStore.Permissions permissions,
649648
final boolean isAppendBlob,
650649
final String eTag,
@@ -664,8 +663,8 @@ HTTP_METHOD_PUT, createRequestUrl(path, EMPTY_STRING),
664663
LOG.error("Path exists as file {} : {}", path, ex.getMessage());
665664
throw ex;
666665
}
667-
createParentMarkersIfNeeded(path, overwrite, permissions, isAppendBlob,
668-
eTag, contextEncryptionAdapter, tracingContext);
666+
createParentMarkersIfNeeded(path, permissions, isAppendBlob, eTag,
667+
contextEncryptionAdapter, tracingContext);
669668
}
670669
return createPathRestOp(path, false, true, permissions,
671670
isAppendBlob, eTag, contextEncryptionAdapter, tracingContext);
@@ -675,24 +674,22 @@ HTTP_METHOD_PUT, createRequestUrl(path, EMPTY_STRING),
675674
* Creates markers for the parent path if needed.
676675
*
677676
* @param path the path for which to create parent markers.
678-
* @param overwrite whether to overwrite if the path already exists.
679677
* @param permissions the permissions to set on the path.
680678
* @param isAppendBlob whether the path is an append blob.
681679
* @param eTag the eTag of the path.
682680
* @param contextEncryptionAdapter the context encryption adapter.
683681
* @param tracingContext the tracing context.
684682
* @throws AzureBlobFileSystemException if the creation of markers fails.
685683
*/
686-
private void createParentMarkersIfNeeded(String path,
687-
boolean overwrite,
684+
public void createParentMarkersIfNeeded(String path,
688685
AzureBlobFileSystemStore.Permissions permissions,
689686
boolean isAppendBlob,
690687
String eTag,
691688
ContextEncryptionAdapter contextEncryptionAdapter,
692689
TracingContext tracingContext) throws AzureBlobFileSystemException {
693690
Path parentPath = new Path(path).getParent();
694691
if (parentPath != null && !parentPath.isRoot()) {
695-
createMarkers(parentPath, overwrite, permissions, isAppendBlob, eTag,
692+
createMarkers(parentPath, permissions, isAppendBlob, eTag,
696693
contextEncryptionAdapter, tracingContext);
697694
}
698695
}
@@ -701,7 +698,6 @@ private void createParentMarkersIfNeeded(String path,
701698
* Validates the path and creates markers if necessary when the namespace is disabled.
702699
*
703700
* @param path the path to validate and create markers for.
704-
* @param overwrite whether to overwrite if the path already exists.
705701
* @param permissions the permissions to set on the path.
706702
* @param isAppendBlob whether the path is an append blob.
707703
* @param eTag the eTag of the path.
@@ -710,8 +706,7 @@ private void createParentMarkersIfNeeded(String path,
710706
* @throws AbfsRestOperationException if a conflict is detected.
711707
* @throws AzureBlobFileSystemException if the creation of markers fails.
712708
*/
713-
public void validatePathAndCreateMarkersIfNeeded(final String path,
714-
final boolean overwrite,
709+
public void checkDirectoryAndCreateMarkersIfNeeded(final String path,
715710
final AzureBlobFileSystemStore.Permissions permissions,
716711
final boolean isAppendBlob,
717712
final String eTag,
@@ -724,7 +719,7 @@ public void validatePathAndCreateMarkersIfNeeded(final String path,
724719
PATH_EXISTS,
725720
null);
726721
}
727-
createParentMarkersIfNeeded(path, overwrite, permissions, isAppendBlob,
722+
createParentMarkersIfNeeded(path, permissions, isAppendBlob,
728723
eTag, contextEncryptionAdapter, tracingContext);
729724
}
730725
}
@@ -762,38 +757,37 @@ private AbfsRestOperation createFile(final String path,
762757
PATH_EXISTS,
763758
null);
764759
}
765-
createParentMarkersIfNeeded(path, overwrite, permissions, isAppendBlob,
766-
eTag, contextEncryptionAdapter, tracingContext);
760+
createParentMarkersIfNeeded(path, permissions, isAppendBlob, eTag,
761+
contextEncryptionAdapter, tracingContext);
767762
}
768763
return createPathRestOp(path, true, overwrite, permissions,
769-
isAppendBlob, eTag, contextEncryptionAdapter, tracingContext);
764+
isAppendBlob, eTag, contextEncryptionAdapter, tracingContext);
770765
}
771766

772767
/**
773-
* Creates marker blobs for the parent directories of the specified path.
768+
* Creates marker blobs for the parent directories of the specified path.
774769
*
775770
* @param path The path for which parent directories need to be created.
776-
* @param overwrite A flag indicating whether existing directories should be overwritten.
777771
* @param permissions The permissions to be set for the created directories.
778772
* @param isAppendBlob A flag indicating whether the created blob should be of type APPEND_BLOB.
779773
* @param eTag The eTag to be matched for conditional requests.
780774
* @param contextEncryptionAdapter The encryption adapter for context encryption.
781775
* @param tracingContext The tracing context for the operation.
776+
*
782777
* @throws AzureBlobFileSystemException If the creation of any parent directory fails.
783778
*/
784779
private void createMarkers(final Path path,
785-
final boolean overwrite,
786780
final AzureBlobFileSystemStore.Permissions permissions,
787781
final boolean isAppendBlob,
788782
final String eTag,
789783
final ContextEncryptionAdapter contextEncryptionAdapter,
790784
final TracingContext tracingContext) throws AzureBlobFileSystemException {
791785
ArrayList<Path> keysToCreateAsFolder = new ArrayList<>();
792-
checkParentChainForFile(path, tracingContext,
786+
findParentPathsForMarkerCreation(path, tracingContext,
793787
keysToCreateAsFolder);
794788
for (Path pathToCreate : keysToCreateAsFolder) {
795789
try {
796-
createPathRestOp(pathToCreate.toUri().getPath(), false, overwrite,
790+
createPathRestOp(pathToCreate.toUri().getPath(), false, false,
797791
permissions,
798792
isAppendBlob, eTag, contextEncryptionAdapter, tracingContext);
799793
} catch (AbfsRestOperationException e) {
@@ -808,7 +802,7 @@ private void createMarkers(final Path path,
808802
* @param path path to check the hierarchy for.
809803
* @param tracingContext the tracingcontext.
810804
*/
811-
private void checkParentChainForFile(Path path, TracingContext tracingContext,
805+
private void findParentPathsForMarkerCreation(Path path, TracingContext tracingContext,
812806
List<Path> keysToCreateAsFolder) throws AzureBlobFileSystemException {
813807
AbfsHttpOperation opResult = null;
814808
Path current = path;
@@ -824,14 +818,15 @@ private void checkParentChainForFile(Path path, TracingContext tracingContext,
824818
throw ex;
825819
}
826820
}
827-
boolean isDirectory = opResult != null && checkIsDir(opResult);
828-
if (opResult != null && !isDirectory) {
821+
boolean isExplicitDirectory = opResult != null && checkIsDir(opResult);
822+
if (opResult != null && !isExplicitDirectory) {
823+
// exists as a file.
829824
throw new AbfsRestOperationException(HTTP_CONFLICT,
830825
AzureServiceErrorCode.PATH_CONFLICT.getErrorCode(),
831826
PATH_EXISTS,
832827
null);
833828
}
834-
if (isDirectory) {
829+
if (isExplicitDirectory) {
835830
return;
836831
}
837832
keysToCreateAsFolder.add(current);
@@ -860,7 +855,7 @@ public AbfsRestOperation conditionalCreateOverwriteFile(String relativePath,
860855
ContextEncryptionAdapter contextEncryptionAdapter,
861856
TracingContext tracingContext) throws IOException {
862857
try {
863-
validatePathAndCreateMarkersIfNeeded(relativePath, false, permissions,
858+
checkDirectoryAndCreateMarkersIfNeeded(relativePath, permissions,
864859
isAppendBlob, null, contextEncryptionAdapter, tracingContext);
865860
} catch (AzureBlobFileSystemException ex) {
866861
LOG.error("Path exists as directory {} : {}", relativePath, ex.getMessage());
@@ -889,8 +884,8 @@ public AbfsRestOperation conditionalCreateOverwriteFile(String relativePath,
889884
}
890885

891886
// If present as an explicit empty directory, we should throw conflict exception.
892-
boolean isDir = checkIsDir(op.getResult());
893-
if (isDir) {
887+
boolean isExplicitDir = checkIsDir(op.getResult());
888+
if (isExplicitDir) {
894889
throw new AbfsRestOperationException(HTTP_CONFLICT,
895890
AzureServiceErrorCode.PATH_CONFLICT.getErrorCode(),
896891
PATH_EXISTS,
@@ -1081,7 +1076,7 @@ destination, sourceEtag, isAtomicRenameKey(source), tracingContext
10811076
final AbfsRestOperation successOp = getAbfsRestOperation(
10821077
AbfsRestOperationType.RenamePath, HTTP_METHOD_PUT,
10831078
url, requestHeaders);
1084-
successOp.hardSetResult(HttpURLConnection.HTTP_OK);
1079+
successOp.hardSetResult(HTTP_OK);
10851080
return new AbfsClientRenameResult(successOp, true, false);
10861081
} else {
10871082
throw new AbfsRestOperationException(HTTP_INTERNAL_ERROR,
@@ -1575,7 +1570,7 @@ public AbfsRestOperation deletePath(final String path,
15751570
final AbfsRestOperation successOp = getAbfsRestOperation(
15761571
AbfsRestOperationType.DeletePath, HTTP_METHOD_DELETE,
15771572
url, requestHeaders);
1578-
successOp.hardSetResult(HttpURLConnection.HTTP_OK);
1573+
successOp.hardSetResult(HTTP_OK);
15791574
return successOp;
15801575
} else {
15811576
throw new AbfsRestOperationException(HTTP_INTERNAL_ERROR,
@@ -1805,8 +1800,8 @@ public boolean checkIsDir(AbfsHttpOperation result) {
18051800
@Override
18061801
public boolean checkUserError(int responseStatusCode) {
18071802
return (responseStatusCode >= HttpURLConnection.HTTP_BAD_REQUEST
1808-
&& responseStatusCode < HttpURLConnection.HTTP_INTERNAL_ERROR
1809-
&& responseStatusCode != HttpURLConnection.HTTP_CONFLICT);
1803+
&& responseStatusCode < HTTP_INTERNAL_ERROR
1804+
&& responseStatusCode != HTTP_CONFLICT);
18101805
}
18111806

18121807
/**
@@ -2019,7 +2014,7 @@ public void takeGetPathStatusAtomicRenameKeyAction(final Path path,
20192014
return;
20202015
}
20212016
} catch (AbfsRestOperationException ex) {
2022-
if (ex.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) {
2017+
if (ex.getStatusCode() == HTTP_NOT_FOUND) {
20232018
return;
20242019
}
20252020
throw ex;
@@ -2029,7 +2024,7 @@ public void takeGetPathStatusAtomicRenameKeyAction(final Path path,
20292024
try {
20302025
RenameAtomicity renameAtomicity = getRedoRenameAtomicity(
20312026
pendingJsonPath, Integer.parseInt(pendingJsonFileStatus.getResult()
2032-
.getResponseHeader(HttpHeaderConfigurations.CONTENT_LENGTH)),
2027+
.getResponseHeader(CONTENT_LENGTH)),
20332028
tracingContext);
20342029
renameAtomicity.redo();
20352030
renameSrcHasChanged = false;
@@ -2041,8 +2036,8 @@ public void takeGetPathStatusAtomicRenameKeyAction(final Path path,
20412036
* to a HTTP_CONFLICT. In this case, no more operation needs to be taken, and
20422037
* the calling getPathStatus can return this source path as result.
20432038
*/
2044-
if (ex.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND
2045-
|| ex.getStatusCode() == HttpURLConnection.HTTP_CONFLICT) {
2039+
if (ex.getStatusCode() == HTTP_NOT_FOUND
2040+
|| ex.getStatusCode() == HTTP_CONFLICT) {
20462041
renameSrcHasChanged = true;
20472042
} else {
20482043
throw ex;
@@ -2092,8 +2087,8 @@ private boolean takeListPathAtomicRenameKeyAction(final Path path,
20922087
* since this is a renamePendingJson file and would be deleted by the redo operation,
20932088
* the calling listPath should not return this json path as result.
20942089
*/
2095-
if (ex.getStatusCode() != HttpURLConnection.HTTP_NOT_FOUND
2096-
&& ex.getStatusCode() != HttpURLConnection.HTTP_CONFLICT) {
2090+
if (ex.getStatusCode() != HTTP_NOT_FOUND
2091+
&& ex.getStatusCode() != HTTP_CONFLICT) {
20972092
throw ex;
20982093
}
20992094
}
@@ -2216,7 +2211,7 @@ private BlobListResultSchema getListResultSchemaFromPathStatus(String relativePa
22162211
entrySchema.setContentLength(Long.parseLong(pathStatus.getResult().getResponseHeader(CONTENT_LENGTH)));
22172212
entrySchema.setLastModifiedTime(
22182213
pathStatus.getResult().getResponseHeader(LAST_MODIFIED));
2219-
entrySchema.setETag(AzureBlobFileSystemStore.extractEtagHeader(pathStatus.getResult()));
2214+
entrySchema.setETag(extractEtagHeader(pathStatus.getResult()));
22202215

22212216
// If listing is done on explicit directory, do not include directory in the listing.
22222217
if (!entrySchema.isDirectory()) {
@@ -2234,7 +2229,7 @@ private static String encodeMetadataAttribute(String value)
22342229
private static String decodeMetadataAttribute(String encoded)
22352230
throws UnsupportedEncodingException {
22362231
return encoded == null ? null
2237-
: java.net.URLDecoder.decode(encoded, StandardCharsets.UTF_8.name());
2232+
: URLDecoder.decode(encoded, StandardCharsets.UTF_8.name());
22382233
}
22392234

22402235
private boolean isNonEmptyListing(String path,

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,10 +645,9 @@ public void testNegativeScenariosForCreateOverwriteDisabled()
645645
any(TracingContext.class));
646646

647647
if (mockClient instanceof AbfsBlobClient) {
648-
// Mock for validatePathAndCreateMarkers to do nothing
648+
// Mock for checkDirectoryAndCreateMarkersIfNeeded to do nothing
649649
doNothing().when((AbfsBlobClient) mockClient)
650-
.validatePathAndCreateMarkersIfNeeded(any(String.class),
651-
any(Boolean.class),
650+
.checkDirectoryAndCreateMarkersIfNeeded(any(String.class),
652651
any(AzureBlobFileSystemStore.Permissions.class),
653652
any(Boolean.class),
654653
any(String.class), any(ContextEncryptionAdapter.class),

0 commit comments

Comments
 (0)