PathConvertingFileSystem converts FileStatus objects in-place (#638)

* PathConvertingFileSystem renames paths without creating new FileStatus objects * Add generated changelog entries * Reflect updated FileStatus conversion in test Co-authored-by: svc-changelog <[email protected]>
palantir · Dec 4, 2022 · b5112a2 · b5112a2
1 parent d0853ab
commit b5112a2
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 19 deletions.
diff --git a/changelog/@unreleased/pr-638.v2.yml b/changelog/@unreleased/pr-638.v2.yml
@@ -0,0 +1,6 @@
+type: improvement
+improvement:
+  description: PathConvertingFileSystem renames paths without creating new FileStatus
+    objects.
+  links:
+  - https://github.com/palantir/hadoop-crypto/pull/638
diff --git a/hadoop-crypto/src/main/java/com/palantir/crypto2/hadoop/PathConvertingFileSystem.java b/hadoop-crypto/src/main/java/com/palantir/crypto2/hadoop/PathConvertingFileSystem.java
@@ -154,19 +154,8 @@ private Path from(Path path) {
         return fromFunc.apply(path);
     }
 
-    private FileStatus toReturnFileStatus(FileStatus status) throws IOException {
-        // same as FileStatus copy constructor
-        return new FileStatus(
-                status.getLen(),
-                status.isDirectory(),
-                status.getReplication(),
-                status.getBlockSize(),
-                status.getModificationTime(),
-                status.getAccessTime(),
-                status.getPermission(),
-                status.getOwner(),
-                status.getGroup(),
-                status.isSymlink() ? status.getSymlink() : null, // getSymlink throws if file is not a symlink
-                from(status.getPath()));
+    private FileStatus toReturnFileStatus(FileStatus status) {
+        status.setPath(from(status.getPath()));
+        return status;
     }
 }
diff --git a/hadoop-crypto/src/test/java/com/palantir/crypto2/hadoop/PathConvertingFileSystemTest.java b/hadoop-crypto/src/test/java/com/palantir/crypto2/hadoop/PathConvertingFileSystemTest.java
@@ -120,18 +120,24 @@ public void makeQualified() {
 
     @Test
     public void listStatus() throws Exception {
-        when(delegate.listStatus(DELEGATE_PATH)).thenReturn(new FileStatus[] {fileStatus(DELEGATE_PATH)});
+        FileStatus delegateFileStatus = fileStatus(DELEGATE_PATH);
+        when(delegate.listStatus(DELEGATE_PATH)).thenReturn(new FileStatus[] {delegateFileStatus});
         FileStatus[] fileStatuses = convertingFs.listStatus(PATH);
 
-        assertThat(fileStatuses).containsExactly(fileStatus(RETURN_PATH));
+        assertThat(fileStatuses)
+                .satisfiesExactly(status ->
+                        // The returned status is the same object returned by the delegate, the path converted in-place
+                        assertThat(status).isEqualTo(fileStatus(RETURN_PATH)).isSameAs(delegateFileStatus));
     }
 
     @Test
     public void getFileStatus() throws Exception {
-        when(delegate.getFileStatus(DELEGATE_PATH)).thenReturn(fileStatus(DELEGATE_PATH));
-        FileStatus fileStatus = convertingFs.getFileStatus(PATH);
+        FileStatus delegateFileStatus = fileStatus(DELEGATE_PATH);
+        when(delegate.getFileStatus(DELEGATE_PATH)).thenReturn(delegateFileStatus);
+        FileStatus convertedFileStatus = convertingFs.getFileStatus(PATH);
 
-        assertThat(fileStatus).isEqualTo(fileStatus(RETURN_PATH));
+        // The returned status is the same object returned by the delegate, the path converted in-place
+        assertThat(convertedFileStatus).isEqualTo(fileStatus(RETURN_PATH)).isSameAs(delegateFileStatus);
     }
 
     @Test