fix: fix the rename bug.

Signed-off-by: Yang Yu <[email protected]>

Author: yuyang733

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>com.qcloud.cos</groupId>
     <artifactId>hadoop-cos</artifactId>
-    <version>${hadoop.version}-8.3.22</version>
+    <version>8.3.22</version>
     <packaging>jar</packaging>
 
     <name>Apache Hadoop Tencent Cloud COS Support</name>

src/main/java/org/apache/hadoop/fs/CosNFileSystem.java

@@ -306,7 +306,7 @@ public FSDataOutputStream append(Path f, int bufferSize,
         Progressable progress) throws IOException {
         if (this.isPosixBucket) {
             throw new UnsupportedOperationException(
-                "The posix bucket does not support append operation through S3 gateway");
+                    "The posix bucket does not support append operation through S3 gateway");
         }
 
         // 如果配置中开启客户端加密，则不支持
@@ -317,7 +317,7 @@ public FSDataOutputStream append(Path f, int bufferSize,
 
         // 如果原文件使用了客户端加密，则不支持
         try {
-            if(this.getXAttr(f, CSE_ALGORITHM_USER_METADATA) != null) {
+            if (this.getXAttr(f, CSE_ALGORITHM_USER_METADATA) != null) {
                 throw new UnsupportedOperationException("Not supported currently because the file is encrypted by client side");
             }
         } catch (IOException e) {
@@ -406,7 +406,7 @@ public boolean truncate(Path f, long newLength) throws IOException {
 
         // 如果原文件使用了客户端加密，则不支持
         try {
-            if(this.getXAttr(f, CSE_ALGORITHM_USER_METADATA) != null) {
+            if (this.getXAttr(f, CSE_ALGORITHM_USER_METADATA) != null) {
                 throw new UnsupportedOperationException("Not supported currently because the file is encrypted by client side");
             }
         } catch (IOException e) {
@@ -1026,105 +1026,86 @@ public boolean rename(Path src, Path dst) throws IOException {
             return false;
         }
 
-        // the preconditions for the rename operation.
-        // reference: https://hadoop.apache.org/docs/r3.3.0/hadoop-project-dist/hadoop-common/filesystem/filesystem.html#rename
-        Pair<CosNFileStatus, CosNFileStatus> renameFileStatusPair = renameInitiate(src, dst);
-
-        // the postconditions for the rename operation.
-        // reference: https://hadoop.apache.org/docs/r3.3.0/hadoop-project-dist/hadoop-common/filesystem/filesystem.html#rename
-        if (src.equals(dst)) {
-            if (renameFileStatusPair.getFirst() != null) {
-                if (renameFileStatusPair.getFirst().isDirectory()) {
-                    //Renaming a directory onto itself is no-op; return value is not specified.
-                    //In POSIX the result is False; in HDFS the result is True.
-                    return true;
-                }
-                if (renameFileStatusPair.getFirst().isFile()) {
-                    // Renaming a file to itself is a no-op; the result is True.
-                    return true;
-                }
-                // For symlink types, the Hadoop file system specification does not provide clear instructions,
-                // I tested the symlink in the POSIX file system, and the same behavior is also true.
-                return true;
-            }
-            return false;
-        }
-
-        if (!isPosixBucket) {
-            return internalCopyAndDelete(
-                    src, renameFileStatusPair.getFirst(),
-                    dst, renameFileStatusPair.getSecond());
-        } else {
-            return internalRename(src, dst);
-        }
-    }
-
-    private Pair<CosNFileStatus, CosNFileStatus> renameInitiate(Path srcPath, Path dstPath)
-            throws PathIOException, IOException {
-        // Preconditions
-        Preconditions.checkNotNull(srcPath);
-        Preconditions.checkNotNull(dstPath);
-        Preconditions.checkArgument(srcPath.isAbsolute());
-        Preconditions.checkArgument(dstPath.isAbsolute());
-
-        Pair<CosNFileStatus, CosNFileStatus> renameFileStatusPair = new Pair<>();
-
         // Hadoop FileSystem Specification: if not exists(FS, src) : raise FileNotFoundException
         CosNFileStatus srcFileStatus = null;
         try {
-            srcFileStatus = (CosNFileStatus) this.getFileStatus(srcPath);
+            srcFileStatus = (CosNFileStatus) this.getFileStatus(src);
         } catch (FileNotFoundException e) {
-            LOG.error("The source path [{}] is not exist.", srcPath);
+            LOG.error("The source path [{}] is not exist.", src);
             throw e;
         }
-        renameFileStatusPair.setFirst(srcFileStatus);
 
         // Hadoop FileSystem Specification: if isDescendant(FS, src, dest) : raise IOException
-        Path dstParentPath = dstPath.getParent();
-        while (null != dstParentPath && !srcPath.equals(dstParentPath)) {
+        Path dstParentPath = dst.getParent();
+        while (null != dstParentPath && !src.equals(dstParentPath)) {
             dstParentPath = dstParentPath.getParent();
         }
         if (null != dstParentPath) {
-            LOG.error("It is not allowed to rename a parent directory:{} to its subdirectory:{}.", srcPath, dstPath);
-            PathIOException pathIOException = new PathIOException(srcPath.toString(),
+            LOG.error("It is not allowed to rename a parent directory:{} to its subdirectory:{}.", src, dst);
+            PathIOException pathIOException = new PathIOException(src.toString(),
                     "It is not allowed to rename a parent directory to its subdirectory");
             pathIOException.setOperation("rename");
-            pathIOException.setTargetPath(dstPath.toString());
+            pathIOException.setTargetPath(dst.toString());
             throw pathIOException;
         }
 
         // Hadoop FileSystem Specification: isRoot(FS, dest) or exists(FS, parent(dest))
         CosNFileStatus dstFileStatus = null;
         try {
-            dstFileStatus = (CosNFileStatus) this.getFileStatus(dstPath);
+            dstFileStatus = (CosNFileStatus) this.getFileStatus(dst);
             if (dstFileStatus.isFile()) {
-                throw new FileAlreadyExistsException(dstPath.toString());
+                throw new FileAlreadyExistsException(dst.toString());
             } else {
                 // The destination path is an existing directory,
-                Path tempDstPath = new Path(dstPath, srcPath.getName());
+                dst = new Path(dst, src.getName());
                 try {
-                    FileStatus fileStatus = this.getFileStatus(tempDstPath, FileStatusProbeEnum.LIST_ONLY);
-                    if (fileStatus != null) {
-                        throw new FileAlreadyExistsException(dstPath.toString());
+                    dstFileStatus = (CosNFileStatus) this.getFileStatus(dst, FileStatusProbeEnum.LIST_ONLY);
+                    if (dstFileStatus != null) {
+                        throw new FileAlreadyExistsException(dst.toString());
                     }
                 } catch (FileNotFoundException ignore) {
                     // OK, expects Not Found.
                 }
             }
-            renameFileStatusPair.setSecond(dstFileStatus);
         } catch (FileNotFoundException e) {
             // Hadoop FileSystem Specification: if isFile(FS, parent(dest)) : raise IOException
-            Path tempDstParentPath = dstPath.getParent();
+            Path tempDstParentPath = dst.getParent();
             FileStatus dstParentStatus = this.getFileStatus(tempDstParentPath);
             if (!dstParentStatus.isDirectory()) {
                 PathIOException pathIOException = new PathIOException(tempDstParentPath.toString(),
                         String.format("Can not rename into a file [%s]", tempDstParentPath));
-                pathIOException.setTargetPath(dstPath.toString());
+                pathIOException.setTargetPath(dst.toString());
                 throw pathIOException;
             }
         }
 
-        return renameFileStatusPair;
+        // the postconditions for the rename operation.
+        // reference: https://hadoop.apache.org/docs/r3.3.0/hadoop-project-dist/hadoop-common/filesystem/filesystem.html#rename
+        if (src.equals(dst)) {
+            if (srcFileStatus.isDirectory()) {
+                //Renaming a directory onto itself is no-op; return value is not specified.
+                //In POSIX the result is False; in HDFS the result is True.
+                return true;
+            }
+            if (srcFileStatus.isFile()) {
+                // Renaming a file to itself is a no-op; the result is True.
+                return true;
+            }
+            if (srcFileStatus.isSymlink()) {
+                // For symlink types, the Hadoop file system specification does not provide clear instructions,
+                // I tested the symlink in the POSIX file system, and the same behavior is also true.
+                return true;
+            }
+            return false;
+        }
+
+        if (!isPosixBucket) {
+            return internalCopyAndDelete(
+                    src, srcFileStatus,
+                    dst, dstFileStatus);
+        } else {
+            return internalRename(src, dst);
+        }
     }
 
     private boolean internalCopyAndDelete(Path srcPath, CosNFileStatus srcFileStatus,

src/test/java/org/apache/hadoop/fs/ITestCosNFileSystemRename.java

@@ -32,7 +32,13 @@ public void testRenameWithPreExistingDestination() throws Throwable {
 		createBaseFileWithData(10, src);
 		Path dest = new Path(testPath, "dest");
 		createBaseFileWithData(10, dest);
-		assertRenameOutcome(fs, src, dest, false);
+		boolean exceptionThrown = false;
+		try {
+			assertRenameOutcome(fs, src, dest, false);
+		} catch (FileAlreadyExistsException e) {
+			exceptionThrown = true;
+		}
+		assertTrue("Expected FileAlreadyExistsException to be thrown", exceptionThrown);
 	}
 
 	@Test