Fix createDirectoryMarkerIfNotExists (#7510)

treeverse · Feb 25, 2024 · 9e2452a · 9e2452a
1 parent df64464
commit 9e2452a
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 5 deletions.
diff --git a/clients/hadoopfs/src/main/java/io/lakefs/LakeFSFileSystem.java b/clients/hadoopfs/src/main/java/io/lakefs/LakeFSFileSystem.java
@@ -615,9 +615,17 @@ private void createDirectoryMarkerIfNotExists(Path f) throws IOException {
         }
         try {
             ObjectsApi objects = lfsClient.getObjectsApi();
-            objects.uploadObject(objectLocation.getRepository(), objectLocation.getRef(), objectLocation.getPath())
-                .ifNoneMatch("*").content(emptyFile)
-                .execute();
+            ObjectStatsList osl = objects.listObjects(objectLocation.getRepository(), objectLocation.getRef())
+                    .userMetadata(false).presign(false).amount(5).prefix(objectLocation.getPath())
+                    .execute();
+            List<ObjectStats> l = osl.getResults();
+            if (l.isEmpty()) {
+                // No object with any name that starts with objectLocation - create a directory marker in place.
+                objects.uploadObject(objectLocation.getRepository(), objectLocation.getRef(), objectLocation.getPath())
+                        .ifNoneMatch("*").content(emptyFile)
+                        .execute();
+            }
+
         } catch (ApiException e) {
             if (e.getCode() == HttpStatus.SC_PRECONDITION_FAILED) {
                 LOG.trace("createDirectoryMarkerIfNotExists: Ignore {} response, marker exists");

diff --git a/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java b/clients/hadoopfs/src/test/java/io/lakefs/LakeFSFileSystemServerTest.java
@@ -219,7 +219,6 @@ public void testDelete_FileNotExists() throws IOException {
     @Test
     public void testDelete_EmptyDirectoryExists() throws IOException {
         ObjectLocation dirObjLoc = new ObjectLocation("lakefs", "repo", "main", "delete/me");
-        String key = objectLocToS3ObjKey(dirObjLoc);
 
         mockStatObjectNotFound(dirObjLoc.getRepository(), dirObjLoc.getRef(), dirObjLoc.getPath());
         ObjectStats srcStats = makeObjectStats(dirObjLoc.getPath() + Constants.SEPARATOR);
@@ -230,6 +229,8 @@ public void testDelete_EmptyDirectoryExists() throws IOException {
                     ImmutablePagination.builder().prefix("delete/me/").build(),
                     srcStats);
 
+        // Mock listing in createDirectoryMarkerIfNotExists to return listing
+        mockListing("repo", "main", ImmutablePagination.builder().prefix("delete/").build());
         mockDirectoryMarker(dirObjLoc.getParent());
         mockStatObject(dirObjLoc.getRepository(), dirObjLoc.getRef(), dirObjLoc.getPath(), srcStats);
         mockDeleteObject("repo", "main", "delete/me/");
@@ -299,6 +300,10 @@ public void testDelete_DirectoryWithFileRecursive() throws IOException {
         // recursive will always end successfully
         Path path = new Path("lakefs://repo/main/delete/sample");
 
+        // Mock listing in createDirectoryMarkerIfNotExists to return empty path
+        mockListing("repo", "main",
+                ImmutablePagination.builder().prefix("delete/").build());
+
         mockDirectoryMarker(ObjectLocation.pathToObjectLocation(null, path.getParent()));
         // Must create a parent directory marker: it wasn't deleted, and now
         // perhaps is empty.
@@ -330,6 +335,9 @@ protected void caseDeleteDirectoryRecursive(int bulkSize, int numObjects) throws
             }
             mockDeleteObjects("repo", "main", pl);
         }
+        // Mock listing in createDirectoryMarkerIfNotExists to return empty path
+        mockListing("repo", "main",
+                ImmutablePagination.builder().prefix("delete/").build());
         // Mock parent directory marker creation at end of fs.delete to show
         // the directory marker exists.
         mockUploadObject("repo", "main", "delete/");
@@ -500,7 +508,7 @@ public void testRename_existingDirToExistingFileName() throws IOException {
      * file -> existing-directory-name: rename(src.txt, existing-dstdir) -> existing-dstdir/src.txt
      */
     @Test
-    public void testRename_existingFileToExistingDirName() throws ApiException, IOException {
+    public void testRename_existingFileToExistingDirName() throws IOException {
         Path src = new Path("lakefs://repo/main/existing-dir1/existing.src");
         ObjectStats srcStats = makeObjectStats("existing-dir1/existing.src");
         mockStatObject("repo", "main", "existing-dir1/existing.src", srcStats);
@@ -528,6 +536,10 @@ public void testRename_existingFileToExistingDirName() throws ApiException, IOEx
         mockGetBranch("repo", "main");
         mockDeleteObject("repo", "main", "existing-dir1/existing.src");
 
+        // Mock listing in createDirectoryMarkerIfNotExists to return empty path
+        mockListing("repo", "main",
+                ImmutablePagination.builder().prefix("existing-dir1/").build());
+
         // Need a directory marker at the source because it's now empty!
         mockUploadObject("repo", "main", "existing-dir1/");