(gobblin) branch master updated: Remove setpermission step for pre-existing folders (#4017)

[wlo]

This is an automated email from the ASF dual-hosted git repository.

wlo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/gobblin.git


The following commit(s) were added to refs/heads/master by this push:
     new 4543e02b8d Remove setpermission step for pre-existing folders (#4017)
4543e02b8d is described below

commit 4543e02b8d8a28cbf0c289f303b0be55b0a647ad
Author: William Lo <lo.willia...@gmail.com>
AuthorDate: Tue Aug 6 19:49:18 2024 -0400

    Remove setpermission step for pre-existing folders (#4017)
    
    * Remove setpermission step for pre-existing folders
---
 .../gobblin/data/management/copy/ManifestBasedDataset.java     | 10 ++++++++++
 .../management/dataset/ManifestBasedDatasetFinderTest.java     |  6 ++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git 
a/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java
 
b/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java
index 060d4acef8..834010ff4c 100644
--- 
a/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java
+++ 
b/gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java
@@ -20,10 +20,12 @@ package org.apache.gobblin.data.management.copy;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
 
@@ -155,6 +157,14 @@ public class ManifestBasedDataset implements 
IterableCopyableDataset {
           toDelete.add(targetFs.getFileStatus(fileToCopy));
         }
       }
+      // Only set permission for newly created folders on target
+      // To change permissions for existing dirs, expectation is to add the 
folder to the manifest
+      Set<String> parentFolders = new 
HashSet<>(flattenedAncestorPermissions.keySet());
+      for (String folder : parentFolders) {
+        if (targetFs.exists(new Path(folder))) {
+          flattenedAncestorPermissions.remove(folder);
+        }
+      }
       // We need both precommit step to create the directories copying to, and 
a postcommit step to ensure that the execute bit needed for recursive rename is 
reset
       CommitStep createDirectoryWithPermissionsCommitStep = new 
CreateDirectoryWithPermissionsCommitStep(targetFs, ancestorOwnerAndPermissions, 
this.properties);
       CommitStep setPermissionCommitStep = new 
SetPermissionCommitStep(targetFs, flattenedAncestorPermissions, 
this.properties);
diff --git 
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/dataset/ManifestBasedDatasetFinderTest.java
 
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/dataset/ManifestBasedDatasetFinderTest.java
index a5b87ef37c..33100d7b8d 100644
--- 
a/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/dataset/ManifestBasedDatasetFinderTest.java
+++ 
b/gobblin-data-management/src/test/java/org/apache/gobblin/data/management/dataset/ManifestBasedDatasetFinderTest.java
@@ -130,6 +130,7 @@ public class ManifestBasedDatasetFinderTest {
       setSourceAndDestFsMocks(sourceFs, destFs, manifestPath, manifestReadFs);
       Mockito.when(destFs.exists(new 
Path("/tmp/dataset/test1.txt"))).thenReturn(true);
       Mockito.when(destFs.exists(new 
Path("/tmp/dataset/test2.txt"))).thenReturn(false);
+      Mockito.when(destFs.exists(new Path("/tmp"))).thenReturn(true);
       
Mockito.when(destFs.getFileStatus(any(Path.class))).thenReturn(localFs.getFileStatus(new
 Path(tmpDir.toString())));
 
       List<AclEntry> aclEntrySource = 
AclEntry.parseAclSpec("user::rwx,group::rwx,other::rwx", true);
@@ -154,9 +155,10 @@ public class ManifestBasedDatasetFinderTest {
       CommitStep setPermissionStep = ((PostPublishStep) 
fileSet.getFiles().get(3)).getStep();
       Assert.assertTrue(setPermissionStep instanceof SetPermissionCommitStep);
       Map<String, OwnerAndPermission> ownerAndPermissionMap = 
((SetPermissionCommitStep) setPermissionStep).getPathAndPermissions();
-      Assert.assertEquals(ownerAndPermissionMap.size(), 2);
+      // Ignore /tmp as it already exists on destination
+      Assert.assertEquals(ownerAndPermissionMap.size(), 1);
       Assert.assertTrue(ownerAndPermissionMap.containsKey("/tmp/dataset"));
-      Assert.assertTrue(ownerAndPermissionMap.containsKey("/tmp"));
+
 
     }
   }