Skip to content

Commit 35b31f9

Browse files
committed
Test cases fixes for rename
1 parent 0bfe007 commit 35b31f9

File tree

7 files changed

+129
-107
lines changed

7 files changed

+129
-107
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -993,8 +993,6 @@ public Void call() throws Exception {
993993
delete(fs.getPath(), fs.isDirectory());
994994
if (fs.isDirectory()) {
995995
statIncrement(DIRECTORIES_DELETED);
996-
} else {
997-
statIncrement(FILES_DELETED);
998996
}
999997
return null;
1000998
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ public enum AzureServiceErrorCode {
5656
OTHER_SERVER_THROTTLING("ServerBusy", HttpURLConnection.HTTP_UNAVAILABLE,
5757
"The server is currently unable to receive requests. Please retry your request."),
5858
INVALID_QUERY_PARAMETER_VALUE("InvalidQueryParameterValue", HttpURLConnection.HTTP_BAD_REQUEST, null),
59-
INVALID_RENAME_DESTINATION("InvalidRenameDestinationPath", HttpURLConnection.HTTP_BAD_REQUEST, null),
6059
AUTHORIZATION_PERMISSION_MISS_MATCH("AuthorizationPermissionMismatch", HttpURLConnection.HTTP_FORBIDDEN, null),
6160
ACCOUNT_REQUIRES_HTTPS("AccountRequiresHttps", HttpURLConnection.HTTP_BAD_REQUEST, null),
6261
MD5_MISMATCH("Md5Mismatch", HttpURLConnection.HTTP_BAD_REQUEST,

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,6 +1161,8 @@ public AbfsRestOperation deletePath(final String path,
11611161
} else {
11621162
return idempotencyOp;
11631163
}
1164+
} finally {
1165+
incrementAbfsDeleteFile();
11641166
}
11651167

11661168
return op;

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

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131

3232
import org.apache.hadoop.classification.VisibleForTesting;
3333
import org.apache.hadoop.fs.Path;
34-
import org.apache.hadoop.fs.PathIOException;
3534
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
3635
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
3736
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TimeoutException;
@@ -258,36 +257,14 @@ private boolean containsColon(Path p) {
258257
private boolean preCheck(final Path src, final Path dst,
259258
final PathInformation pathInformation)
260259
throws AzureBlobFileSystemException {
261-
validateDestinationPath(src, dst);
260+
validateDestinationIsNotSubDir(src, dst);
262261
validateSourcePath(pathInformation);
263262
validateDestinationPathNotExist(src, dst, pathInformation);
264263
validateDestinationParentExist(src, dst, pathInformation);
265264

266265
return true;
267266
}
268267

269-
/**
270-
* Validate if the format of the destination path is correct and if the destination
271-
* path is not a sub-directory of the source path.
272-
*
273-
* @param src source path
274-
* @param dst destination path
275-
*
276-
* @throws AbfsRestOperationException if the destination path is invalid
277-
*/
278-
private void validateDestinationPath(final Path src, final Path dst)
279-
throws AbfsRestOperationException {
280-
if (containsColon(dst)) {
281-
throw new AbfsRestOperationException(
282-
HttpURLConnection.HTTP_BAD_REQUEST,
283-
AzureServiceErrorCode.INVALID_RENAME_DESTINATION.getErrorCode(), null,
284-
new PathIOException(dst.toUri().getPath(),
285-
"Destination path contains colon"));
286-
}
287-
288-
validateDestinationIsNotSubDir(src, dst);
289-
}
290-
291268
/**
292269
* Validate if the destination path is not a sub-directory of the source path.
293270
*

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.hadoop.fs.azurebfs.AbfsConfiguration;
3636
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
3737
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
38+
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.FileSystemOperationUnhandledException;
3839
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
3940
import org.apache.hadoop.fs.azurebfs.contracts.services.BlobListResultSchema;
4041
import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultEntrySchema;
@@ -119,7 +120,14 @@ private boolean takeAction(List<Path> paths)
119120
LOG.debug("Thread interrupted while taking action on path: {}",
120121
path.toUri().getPath());
121122
} catch (ExecutionException e) {
122-
executionException = (AzureBlobFileSystemException) e.getCause();
123+
LOG.debug("Execution exception while taking action on path: {}",
124+
path.toUri().getPath());
125+
if (e.getCause() instanceof AzureBlobFileSystemException) {
126+
executionException = (AzureBlobFileSystemException) e.getCause();
127+
} else {
128+
executionException =
129+
new FileSystemOperationUnhandledException(executionException);
130+
}
123131
}
124132
}
125133
if (executionException != null) {
@@ -261,7 +269,7 @@ protected String listAndEnqueue(final ListBlobQueue listBlobQueue,
261269
protected void addPaths(final List<Path> paths,
262270
final ListResultSchema retrievedSchema) {
263271
for (ListResultEntrySchema entry : retrievedSchema.paths()) {
264-
Path entryPath = new Path(ROOT_PATH, entry.name());
272+
Path entryPath = new Path(ROOT_PATH + entry.name());
265273
if (!entryPath.equals(this.path)) {
266274
paths.add(entryPath);
267275
}

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

Lines changed: 116 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -311,12 +311,67 @@ public void testRenameNotFoundBlobToEmptyRoot() throws Exception {
311311
*
312312
* @throws Exception if an error occurs during test execution
313313
*/
314-
@Test(expected = IOException.class)
315-
public void testRenameBlobToDstWithColonInPath() throws Exception {
314+
@Test
315+
public void testRenameBlobToDstWithColonInSourcePath() throws Exception {
316316
AzureBlobFileSystem fs = getFileSystem();
317317
assumeBlobServiceType();
318+
fs.create(new Path("/src:/file"));
319+
Assertions.assertThat(
320+
fs.rename(new Path("/src:"),
321+
new Path("/dst"))
322+
).isTrue();
323+
}
324+
325+
/**
326+
* Tests renaming a source path to a destination path that contains a colon in the path.
327+
* This verifies that the rename operation handles paths with special characters like a colon.
328+
*
329+
* The test creates a source directory and renames it to a destination path that includes a colon,
330+
* ensuring that the operation succeeds without errors.
331+
*
332+
* @throws Exception if an error occurs during test execution
333+
*/
334+
@Test
335+
public void testRenameWithColonInDestinationPath() throws Exception {
336+
AzureBlobFileSystem fs = getFileSystem();
318337
fs.create(new Path("/src"));
319-
fs.rename(new Path("/src"), new Path("/dst:file"));
338+
Assertions.assertThat(
339+
fs.rename(new Path("/src"),
340+
new Path("/dst:"))
341+
).isTrue();
342+
}
343+
344+
@Test
345+
public void testRenameWithColonInSourcePath() throws Exception {
346+
AzureBlobFileSystem fs = getFileSystem();
347+
String sourceDirectory = "/src:";
348+
String destinationDirectory = "/dst";
349+
String fileName = "file";
350+
fs.create(new Path(sourceDirectory, fileName));
351+
fs.create(new Path(sourceDirectory + "/Test:", fileName));
352+
// Rename from source to destination
353+
Assertions.assertThat(
354+
fs.rename(new Path(sourceDirectory),
355+
new Path(destinationDirectory))
356+
).isTrue();
357+
Assertions.assertThat(
358+
fs.exists(new Path(sourceDirectory, fileName)))
359+
.isFalse();
360+
Assertions.assertThat(
361+
fs.exists(new Path(destinationDirectory, fileName)))
362+
.isTrue();
363+
364+
// Rename from destination to source
365+
Assertions.assertThat(
366+
fs.rename(new Path(destinationDirectory),
367+
new Path(sourceDirectory))
368+
).isTrue();
369+
Assertions.assertThat(
370+
fs.exists(new Path(sourceDirectory, fileName)))
371+
.isTrue();
372+
Assertions.assertThat(
373+
fs.exists(new Path(destinationDirectory, fileName)))
374+
.isFalse();
320375
}
321376

322377
/**
@@ -1883,64 +1938,64 @@ public void testRenameDeleteFailureInBetween() throws Exception {
18831938
}
18841939
}
18851940

1886-
// /**
1887-
// * Tests renaming a file or directory when the destination path contains
1888-
// * a colon (":"). The test ensures that:
1889-
// * - The source directory exists before the rename.
1890-
// * - The file is successfully renamed to the destination path.
1891-
// * - The old source directory no longer exists after the rename.
1892-
// * - The new destination directory exists after the rename.
1893-
// *
1894-
// * @throws Exception if an error occurs during file system operations
1895-
// */
1896-
// @Test
1897-
// public void testRenameWhenDestinationPathContainsColon() throws Exception {
1898-
// AzureBlobFileSystem fs = getFileSystem();
1899-
// fs.setWorkingDirectory(new Path(ROOT_PATH));
1900-
// String fileName = "file";
1901-
// Path src = new Path("/test1/");
1902-
// Path dst = new Path("/test1:/");
1903-
//
1904-
// // Create the file
1905-
// fs.create(new Path(src, fileName));
1906-
//
1907-
// // Perform the rename operation and validate the results
1908-
// performRenameAndValidate(fs, src, dst, fileName);
1909-
// }
1910-
1911-
// /**
1912-
// * Performs the rename operation and validates the existence of the directories and files.
1913-
// *
1914-
// * @param fs the AzureBlobFileSystem instance
1915-
// * @param src the source path to be renamed
1916-
// * @param dst the destination path for the rename
1917-
// * @param fileName the name of the file to be renamed
1918-
// */
1919-
// private void performRenameAndValidate(AzureBlobFileSystem fs, Path src, Path dst, String fileName)
1920-
// throws IOException {
1921-
// // Assert the source directory exists
1922-
// Assertions.assertThat(fs.exists(src))
1923-
// .describedAs("Old directory should exist before rename")
1924-
// .isTrue();
1925-
//
1926-
// // Perform rename
1927-
// fs.rename(src, dst);
1928-
//
1929-
// // Assert the destination directory and file exist after rename
1930-
// Assertions.assertThat(fs.exists(new Path(dst, fileName)))
1931-
// .describedAs("Rename should be successful")
1932-
// .isTrue();
1933-
//
1934-
// // Assert the source directory no longer exists
1935-
// Assertions.assertThat(fs.exists(src))
1936-
// .describedAs("Old directory should not exist")
1937-
// .isFalse();
1938-
//
1939-
// // Assert the new destination directory exists
1940-
// Assertions.assertThat(fs.exists(dst))
1941-
// .describedAs("New directory should exist")
1942-
// .isTrue();
1943-
// }
1941+
/**
1942+
* Tests renaming a file or directory when the destination path contains
1943+
* a colon (":"). The test ensures that:
1944+
* - The source directory exists before the rename.
1945+
* - The file is successfully renamed to the destination path.
1946+
* - The old source directory no longer exists after the rename.
1947+
* - The new destination directory exists after the rename.
1948+
*
1949+
* @throws Exception if an error occurs during file system operations
1950+
*/
1951+
@Test
1952+
public void testRenameWhenDestinationPathContainsColon() throws Exception {
1953+
AzureBlobFileSystem fs = getFileSystem();
1954+
fs.setWorkingDirectory(new Path(ROOT_PATH));
1955+
String fileName = "file";
1956+
Path src = new Path("/test1/");
1957+
Path dst = new Path("/test1:/");
1958+
1959+
// Create the file
1960+
fs.create(new Path(src, fileName));
1961+
1962+
// Perform the rename operation and validate the results
1963+
performRenameAndValidate(fs, src, dst, fileName);
1964+
}
1965+
1966+
/**
1967+
* Performs the rename operation and validates the existence of the directories and files.
1968+
*
1969+
* @param fs the AzureBlobFileSystem instance
1970+
* @param src the source path to be renamed
1971+
* @param dst the destination path for the rename
1972+
* @param fileName the name of the file to be renamed
1973+
*/
1974+
private void performRenameAndValidate(AzureBlobFileSystem fs, Path src, Path dst, String fileName)
1975+
throws IOException {
1976+
// Assert the source directory exists
1977+
Assertions.assertThat(fs.exists(src))
1978+
.describedAs("Old directory should exist before rename")
1979+
.isTrue();
1980+
1981+
// Perform rename
1982+
fs.rename(src, dst);
1983+
1984+
// Assert the destination directory and file exist after rename
1985+
Assertions.assertThat(fs.exists(new Path(dst, fileName)))
1986+
.describedAs("Rename should be successful")
1987+
.isTrue();
1988+
1989+
// Assert the source directory no longer exists
1990+
Assertions.assertThat(fs.exists(src))
1991+
.describedAs("Old directory should not exist")
1992+
.isFalse();
1993+
1994+
// Assert the new destination directory exists
1995+
Assertions.assertThat(fs.exists(dst))
1996+
.describedAs("New directory should exist")
1997+
.isTrue();
1998+
}
19441999

19452000
/**
19462001
* Tests the behavior of the atomic rename key for the root folder

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,21 +24,15 @@
2424
import org.junit.runner.RunWith;
2525
import org.junit.runners.Parameterized;
2626

27-
import org.apache.hadoop.fs.azurebfs.constants.AbfsServiceType;
28-
import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
2927
import org.apache.hadoop.fs.FileStatus;
3028
import org.apache.hadoop.fs.Path;
31-
import org.apache.hadoop.fs.PathIOException;
3229

33-
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
34-
import static org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.COLON;
3530
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsDirectory;
3631
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertIsFile;
3732
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertMkdirs;
3833
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathDoesNotExist;
3934
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertPathExists;
4035
import static org.apache.hadoop.fs.contract.ContractTestUtils.assertRenameOutcome;
41-
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
4236

4337
/**
4438
* Parameterized test of rename operations of unicode paths.
@@ -90,17 +84,6 @@ public void testRenameFileUsingUnicode() throws Exception {
9084
assertIsFile(fs, filePath);
9185

9286
Path folderPath2 = new Path(destDir);
93-
if (getAbfsServiceType() == AbfsServiceType.BLOB
94-
&& destDir.contains(COLON)) {
95-
AbfsRestOperationException ex = intercept(
96-
AbfsRestOperationException.class, () -> {
97-
fs.rename(folderPath1, folderPath2);
98-
return null;
99-
});
100-
assertTrue(ex.getCause() instanceof PathIOException);
101-
assertEquals(HTTP_BAD_REQUEST, ex.getStatusCode());
102-
return;
103-
}
10487
assertRenameOutcome(fs, folderPath1, folderPath2, true);
10588
assertPathDoesNotExist(fs, "renamed", folderPath1);
10689
assertIsDirectory(fs, folderPath2);

0 commit comments

Comments
 (0)