fix(listing): skip free versions when looking for a latest delete marker

Signed-off-by: Denis Babchuk <babchukdenis@icloud.com>

cmd/erasure-server-pool-free-version_test.go

@@ -254,3 +254,55 @@ func TestFreeVersionScannerLatestVersion(t *testing.T) {
 		t.Errorf("scanner queued %v for expiry, want %v", expired, want)
 	}
 }
+
+// ListObjectsV2 lists obj-01: its latest version is v2, not a delete marker.
+// obj-02 has no versions left.
+func TestFreeVersionListObjects(t *testing.T) {
+	f := newFreeVersionTest(t, "")
+	lo, err := f.z.ListObjectsV2(t.Context(), f.bucket, "", "", "", 100, false, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var got []string
+	for _, oi := range lo.Objects {
+		got = append(got, oi.Name)
+	}
+	if want := []string{"obj-01"}; !slices.Equal(got, want) {
+		t.Errorf("ListObjectsV2: %v, want %v", got, want)
+	}
+}
+
+// The listing checks of the latest version skip the free version: obj-01 is
+// shown, obj-02 has nothing to show. Both the encoded metadata and the
+// decoded form, which the listing caches, are checked.
+func TestFreeVersionLatestDeleteMarker(t *testing.T) {
+	f := newFreeVersionTest(t, "")
+	for _, tc := range []struct {
+		object string
+		hidden bool
+	}{{"obj-01", false}, {"obj-02", true}} {
+		entry := metaCacheEntry{name: tc.object, metadata: f.xlMeta(0, tc.object)}
+		meta, _, err := isIndexedMetaV2(entry.metadata)
+		if err != nil || meta == nil {
+			t.Fatalf("%s: metadata is not indexed: %v", tc.object, err)
+		}
+		if got := meta.IsLatestDeleteMarker(); got != tc.hidden {
+			t.Errorf("%s: IsLatestDeleteMarker %v, want %v", tc.object, got, tc.hidden)
+		}
+		if got := meta.AllHidden(true); got != tc.hidden {
+			t.Errorf("%s: AllHidden(true) %v, want %v", tc.object, got, tc.hidden)
+		}
+		if got := meta.AllHidden(false); got != tc.hidden {
+			t.Errorf("%s: AllHidden(false) %v, want %v", tc.object, got, tc.hidden)
+		}
+		if got := entry.isLatestDeletemarker(); got != tc.hidden {
+			t.Errorf("%s: isLatestDeletemarker %v, want %v", tc.object, got, tc.hidden)
+		}
+		if _, err := entry.xlmeta(); err != nil {
+			t.Fatal(err)
+		}
+		if got := entry.isLatestDeletemarker(); got != tc.hidden {
+			t.Errorf("%s: isLatestDeletemarker of the decoded metadata %v, want %v", tc.object, got, tc.hidden)
+		}
+	}
+}

cmd/metacache-entries.go

@@ -177,14 +177,12 @@ func (e metaCacheEntry) isInDir(dir, separator string) bool {
 }
 
 // isLatestDeletemarker returns whether the latest version is a delete marker.
+// Free versions are not versions of the object and are skipped.
 // If metadata is NOT versioned false will always be returned.
 // If v2 and UNABLE to load metadata true will be returned.
 func (e *metaCacheEntry) isLatestDeletemarker() bool {
 	if e.cached != nil {
-		if len(e.cached.versions) == 0 {
+		return e.cached.isLatestDeleteMarker()
-			return true
-		}
-		return e.cached.versions[0].header.Type == DeleteType
 	}
 	if !isXL2V1Format(e.metadata) {
 		return false
@@ -196,10 +194,10 @@ func (e *metaCacheEntry) isLatestDeletemarker() bool {
 	}
 	// Fall back...
 	xlMeta, err := e.xlmeta()
-	if err != nil || len(xlMeta.versions) == 0 {
+	if err != nil {
 		return true
 	}
-	return xlMeta.versions[0].header.Type == DeleteType
+	return xlMeta.isLatestDeleteMarker()
 }
 
 // isAllFreeVersions returns if all objects are free versions.

cmd/xl-storage-format-v2.go

@@ -1138,6 +1138,17 @@ func (x *xlMetaV2) latestModtime() time.Time {
 	return time.Unix(0, x.versions[0].header.ModTime)
 }
 
+// isLatestDeleteMarker returns true if the latest version that is not a free
+// version is a delete marker, or if there is none.
+func (x *xlMetaV2) isLatestDeleteMarker() bool {
+	for _, ver := range x.versions {
+		if !ver.header.FreeVersion() {
+			return ver.header.Type == DeleteType
+		}
+	}
+	return true
+}
+
 func (x *xlMetaV2) addVersion(ver xlMetaV2Version) error {
 	modTime := ver.getModTime().UnixNano()
 	if !ver.Valid() {
@@ -2289,6 +2300,7 @@ func (x xlMetaBuf) ListVersions(volume, path string, allParts bool) ([]FileInfo,
 }
 
 // IsLatestDeleteMarker returns true if latest version is a deletemarker or there are no versions.
+// Free versions are not versions of the object and are skipped.
 // If any error occurs false is returned.
 func (x xlMetaBuf) IsLatestDeleteMarker() bool {
 	vers, headerV, _, buf, err := decodeXLHeaders(x)
@@ -2298,13 +2310,17 @@ func (x xlMetaBuf) IsLatestDeleteMarker() bool {
 	if vers == 0 {
 		return true
 	}
-	isDeleteMarker := false
+	isDeleteMarker := true // only free versions
 
 	_ = decodeVersions(buf, vers, func(idx int, hdr, _ []byte) error {
 		var xl xlMetaV2VersionHeader
 		if _, err := xl.unmarshalV(headerV, hdr); err != nil {
+			isDeleteMarker = false
 			return errDoneForNow
 		}
+		if xl.FreeVersion() {
+			return nil
+		}
 		isDeleteMarker = xl.Type == DeleteType
 		return errDoneForNow
 	})
@@ -2312,7 +2328,7 @@ func (x xlMetaBuf) IsLatestDeleteMarker() bool {
 }
 
 // AllHidden returns true are no versions that would show up in a listing (ie all free markers)
-// Optionally also return early if top is a delete marker.
+// Optionally also return early if the top version that is not free is a delete marker.
 func (x xlMetaBuf) AllHidden(topDeleteMarker bool) bool {
 	vers, headerV, _, buf, err := decodeXLHeaders(x)
 	if err != nil {
@@ -2328,16 +2344,12 @@ func (x xlMetaBuf) AllHidden(topDeleteMarker bool) bool {
 		if _, err := xl.unmarshalV(headerV, hdr); err != nil {
 			return errDoneForNow
 		}
-		if topDeleteMarker && idx == 0 && xl.Type == DeleteType {
+		if xl.FreeVersion() {
-			hidden = true
+			// Check next version
-			return errDoneForNow
+			return nil
 		}
-		if !xl.FreeVersion() {
+		hidden = topDeleteMarker && xl.Type == DeleteType
-			hidden = false
+		return errDoneForNow
-			return errDoneForNow
-		}
-		// Check next version
-		return nil
 	})
 	return hidden
 }