Skip to content

Commit f5324b0

Browse files
afrankenafranken
committed
Proper ListObjectVersions support, fixes
Tested the ITs against S3, found a few unexpected behaviours, fixed S3Mock to reflect actual S3 behaviour. Fixes #64
1 parent 1527a1a commit f5324b0

File tree

7 files changed

+133
-44
lines changed

7 files changed

+133
-44
lines changed

‎integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/ListObjectVersionsV2IT.kt‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ internal class ListObjectVersionsV2IT : S3TestBase() {
2929
private val s3ClientV2: S3Client = createS3ClientV2()
3030

3131
@Test
32-
@S3VerifiedSuccess(year = 2024)
33-
fun testListObjectVersions(testInfo: TestInfo) {
32+
@S3VerifiedSuccess(year = 2025)
33+
fun listObjectVersions(testInfo: TestInfo) {
3434
val uploadFile = File(UPLOAD_FILE_NAME)
3535
val bucketName = givenBucketV2(testInfo)
3636
s3ClientV2.putBucketVersioning {
@@ -68,7 +68,39 @@ internal class ListObjectVersionsV2IT : S3TestBase() {
6868

6969
@Test
7070
@S3VerifiedTodo
71-
fun testListObjectVersions_deleteMarker(testInfo: TestInfo) {
71+
fun listObjectVersions_noVersioning(testInfo: TestInfo) {
72+
val uploadFile = File(UPLOAD_FILE_NAME)
73+
val bucketName = givenBucketV2(testInfo)
74+
75+
s3ClientV2.putObject(
76+
{
77+
it.bucket(bucketName)
78+
it.key("$UPLOAD_FILE_NAME-1")
79+
},
80+
RequestBody.fromFile(uploadFile)
81+
)
82+
83+
s3ClientV2.putObject(
84+
{
85+
it.bucket(bucketName)
86+
it.key("$UPLOAD_FILE_NAME-2")
87+
},
88+
RequestBody.fromFile(uploadFile)
89+
)
90+
91+
s3ClientV2.listObjectVersions {
92+
it.bucket(bucketName)
93+
}.also {
94+
assertThat(it.versions())
95+
.hasSize(2)
96+
.extracting("versionId")
97+
.containsExactlyInAnyOrder("null", "null")
98+
}
99+
}
100+
101+
@Test
102+
@S3VerifiedSuccess(year = 2025)
103+
fun listObjectVersions_deleteMarker(testInfo: TestInfo) {
72104
val uploadFile = File(UPLOAD_FILE_NAME)
73105
val bucketName = givenBucketV2(testInfo)
74106
s3ClientV2.putBucketVersioning {

‎integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ import software.amazon.awssdk.services.s3.model.ObjectLockEnabled
6363
import software.amazon.awssdk.services.s3.model.ObjectLockLegalHoldStatus
6464
import software.amazon.awssdk.services.s3.model.PutObjectResponse
6565
import software.amazon.awssdk.services.s3.model.S3Exception
66-
import software.amazon.awssdk.services.s3.model.S3Object
6766
import software.amazon.awssdk.services.s3.model.S3Response
6867
import software.amazon.awssdk.services.s3.model.StorageClass
6968
import software.amazon.awssdk.services.s3.model.UploadPartResponse
@@ -88,7 +87,6 @@ import java.time.Instant
8887
import java.util.UUID
8988
import java.util.concurrent.Executors
9089
import java.util.concurrent.ThreadFactory
91-
import java.util.function.Consumer
9290
import java.util.stream.Stream
9391
import javax.net.ssl.SSLContext
9492
import javax.net.ssl.SSLEngine
@@ -401,26 +399,47 @@ internal abstract class S3TestBase {
401399
}
402400

403401
private fun deleteObjectsInBucket(bucket: Bucket, objectLockEnabled: Boolean) {
404-
_s3ClientV2.listObjectsV2 {
402+
_s3ClientV2.listObjectVersions {
405403
it.bucket(bucket.name())
406404
it.encodingType(EncodingType.URL)
407-
}.contents().forEach(
408-
Consumer { s3Object: S3Object ->
405+
}.also {
406+
it.versions().forEach { objectVersion ->
407+
if (objectLockEnabled) {
408+
//must remove potential legal hold, otherwise object can't be deleted
409+
_s3ClientV2.putObjectLegalHold {
410+
it.bucket(bucket.name())
411+
it.key(objectVersion.key())
412+
it.versionId(objectVersion.versionId())
413+
it.legalHold {
414+
it.status(ObjectLockLegalHoldStatus.OFF)
415+
}
416+
}
417+
}
418+
_s3ClientV2.deleteObject {
419+
it.bucket(bucket.name())
420+
it.key(objectVersion.key())
421+
it.versionId(objectVersion.versionId())
422+
}
423+
}
424+
it.deleteMarkers().forEach { marker ->
409425
if (objectLockEnabled) {
410426
//must remove potential legal hold, otherwise object can't be deleted
411427
_s3ClientV2.putObjectLegalHold {
412428
it.bucket(bucket.name())
413-
it.key(s3Object.key())
429+
it.key(marker.key())
430+
it.versionId(marker.versionId())
414431
it.legalHold {
415432
it.status(ObjectLockLegalHoldStatus.OFF)
416433
}
417434
}
418435
}
419436
_s3ClientV2.deleteObject {
420437
it.bucket(bucket.name())
421-
it.key(s3Object.key())
438+
it.key(marker.key())
439+
it.versionId(marker.versionId())
422440
}
423-
})
441+
}
442+
}
424443
}
425444

426445
private fun isObjectLockEnabled(bucket: Bucket): Boolean {

‎integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/VersionsV2IT.kt‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,37 @@ internal class VersionsV2IT : S3TestBase() {
3535
private val s3ClientV2: S3Client = createS3ClientV2()
3636

3737
@Test
38-
@S3VerifiedTodo
38+
@S3VerifiedSuccess(year = 2025)
39+
fun testListObjectVersions_nonVersionEnabledBucket(testInfo: TestInfo) {
40+
val bucketName = givenBucketV2(testInfo)
41+
givenObjectV2(bucketName, UPLOAD_FILE_NAME)
42+
val listObjectVersions = s3ClientV2.listObjectVersions { it.bucket(bucketName) }
43+
assertThat(listObjectVersions.hasVersions()).isTrue
44+
assertThat(listObjectVersions.versions()[0].key()).isEqualTo(UPLOAD_FILE_NAME)
45+
assertThat(listObjectVersions.versions()[0].versionId()).isEqualTo("null")
46+
}
47+
48+
@Test
49+
@S3VerifiedSuccess(year = 2025)
50+
fun testListObjectVersions_versionEnabledBucket(testInfo: TestInfo) {
51+
val bucketName = givenBucketV2(testInfo)
52+
s3ClientV2.putBucketVersioning {
53+
it.bucket(bucketName)
54+
it.versioningConfiguration {
55+
it.status(BucketVersioningStatus.ENABLED)
56+
}
57+
}
58+
59+
val versionId = givenObjectV2(bucketName, UPLOAD_FILE_NAME).versionId()
60+
61+
val listObjectVersions = s3ClientV2.listObjectVersions { it.bucket(bucketName) }
62+
assertThat(listObjectVersions.hasVersions()).isTrue
63+
assertThat(listObjectVersions.versions()[0].key()).isEqualTo(UPLOAD_FILE_NAME)
64+
assertThat(listObjectVersions.versions()[0].versionId()).isEqualTo(versionId)
65+
}
66+
67+
@Test
68+
@S3VerifiedSuccess(year = 2025)
3969
fun testPutGetObject_withVersion(testInfo: TestInfo) {
4070
val uploadFile = File(UPLOAD_FILE_NAME)
4171
val expectedChecksum = DigestUtil.checksumFor(uploadFile.toPath(), Algorithm.SHA1)
@@ -76,7 +106,7 @@ internal class VersionsV2IT : S3TestBase() {
76106
}
77107

78108
@Test
79-
@S3VerifiedTodo
109+
@S3VerifiedSuccess(year = 2025)
80110
fun testPutGetObject_withMultipleVersions(testInfo: TestInfo) {
81111
val uploadFile = File(UPLOAD_FILE_NAME)
82112
val bucketName = givenBucketV2(testInfo)
@@ -127,7 +157,7 @@ internal class VersionsV2IT : S3TestBase() {
127157
}
128158

129159
@Test
130-
@S3VerifiedTodo
160+
@S3VerifiedSuccess(year = 2025)
131161
fun testPutGetDeleteObject_withVersion(testInfo: TestInfo) {
132162
val uploadFile = File(UPLOAD_FILE_NAME)
133163
val bucketName = givenBucketV2(testInfo)
@@ -157,8 +187,6 @@ internal class VersionsV2IT : S3TestBase() {
157187
it.bucket(bucketName)
158188
it.key(UPLOAD_FILE_NAME)
159189
it.versionId(versionId2)
160-
}.also {
161-
assertThat(it.deleteMarker()).isEqualTo(false)
162190
}
163191

164192
s3ClientV2.getObject {
@@ -170,7 +198,7 @@ internal class VersionsV2IT : S3TestBase() {
170198
}
171199

172200
@Test
173-
@S3VerifiedTodo
201+
@S3VerifiedSuccess(year = 2025)
174202
fun testPutGetDeleteObject_withDeleteMarker(testInfo: TestInfo) {
175203
val uploadFile = File(UPLOAD_FILE_NAME)
176204
val bucketName = givenBucketV2(testInfo)

‎server/src/main/java/com/adobe/testing/s3mock/BucketController.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -516,11 +516,7 @@ public ResponseEntity<ListVersionsResult> listObjectVersions(
516516
@RequestParam(name = VERSION_ID_MARKER, required = false) String versionIdMarker,
517517
@RequestParam(name = ENCODING_TYPE, required = false) String encodingType,
518518
@RequestParam(name = MAX_KEYS, defaultValue = "1000", required = false) Integer maxKeys) {
519-
BucketMetadata bucketMetadata = bucketService.verifyBucketExists(bucketName);
520-
if (!bucketMetadata.isVersioningEnabled()) {
521-
//TODO: find correct exception.
522-
throw NOT_FOUND_BUCKET_VERSIONING_CONFIGURATION;
523-
}
519+
bucketService.verifyBucketExists(bucketName);
524520
bucketService.verifyMaxKeys(maxKeys);
525521
bucketService.verifyEncodingType(encodingType);
526522
var listVersionsResult =

‎server/src/main/java/com/adobe/testing/s3mock/dto/ObjectVersion.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,15 @@ public static ObjectVersion from(S3ObjectMetadata s3ObjectMetadata, boolean isLa
6161
s3ObjectMetadata.versionId());
6262
}
6363

64-
public static ObjectVersion from(S3Object s3Object, boolean isLatest, String versionId) {
64+
public static ObjectVersion from(S3Object s3Object) {
6565
return new ObjectVersion(s3Object.key(),
6666
s3Object.lastModified(),
6767
s3Object.etag(),
6868
s3Object.size(),
6969
s3Object.storageClass(),
7070
s3Object.owner(),
7171
s3Object.checksumAlgorithm(),
72-
isLatest,
73-
versionId);
72+
false,
73+
"null");
7474
}
7575
}

‎server/src/main/java/com/adobe/testing/s3mock/service/BucketService.java‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -250,23 +250,28 @@ public ListVersionsResult listVersions(String bucketName,
250250
break;
251251
}
252252
var id = bucket.getID(object.key());
253-
var s3ObjectVersions = objectStore.getS3ObjectVersions(bucket, id);
254-
for (var s3ObjectVersion : s3ObjectVersions.versions()) {
255-
var s3ObjectMetadata = objectStore.getS3ObjectMetadata(bucket, id, s3ObjectVersion);
256-
if (!s3ObjectMetadata.deleteMarker()) {
257-
if (objectVersions.size() > maxKeys) {
258-
nextVersionIdMarker = s3ObjectVersion;
259-
break;
253+
254+
if (bucket.isVersioningEnabled()) {
255+
var s3ObjectVersions = objectStore.getS3ObjectVersions(bucket, id);
256+
for (var s3ObjectVersion : s3ObjectVersions.versions()) {
257+
var s3ObjectMetadata = objectStore.getS3ObjectMetadata(bucket, id, s3ObjectVersion);
258+
if (!s3ObjectMetadata.deleteMarker()) {
259+
if (objectVersions.size() > maxKeys) {
260+
nextVersionIdMarker = s3ObjectVersion;
261+
break;
262+
}
263+
objectVersions.add(
264+
ObjectVersion.from(s3ObjectMetadata,
265+
Objects.equals(s3ObjectVersions.getLatestVersion(), s3ObjectVersion))
266+
);
267+
} else {
268+
deleteMarkers.add(
269+
DeleteMarkerEntry.from(s3ObjectMetadata,
270+
Objects.equals(s3ObjectVersions.getLatestVersion(), s3ObjectVersion)));
260271
}
261-
objectVersions.add(
262-
ObjectVersion.from(s3ObjectMetadata,
263-
Objects.equals(s3ObjectVersions.getLatestVersion(), s3ObjectVersion))
264-
);
265-
} else {
266-
deleteMarkers.add(
267-
DeleteMarkerEntry.from(s3ObjectMetadata,
268-
Objects.equals(s3ObjectVersions.getLatestVersion(), s3ObjectVersion)));
269272
}
273+
} else {
274+
objectVersions.add(ObjectVersion.from(object));
270275
}
271276
}
272277

‎server/src/main/java/com/adobe/testing/s3mock/store/ObjectStore.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ public class ObjectStore extends StoreBase {
5757
private static final String VERSIONED_META_FILE = "%s-objectMetadata.json";
5858
private static final String VERSIONED_DATA_FILE = "%s-binaryData";
5959
private static final String VERSIONS_FILE = "versions.json";
60+
//if a bucket isn't version enabled, some APIs return "null" as the versionId for objects.
61+
//clients may also pass in "null" as a version, expecting the behaviour for non-versioned objects.
62+
private static final String NULL_VERSION = "null";
6063

6164
/**
6265
* This map stores one lock object per S3Object ID.
@@ -514,7 +517,7 @@ private void verifyPretendCopy(S3ObjectMetadata sourceObject,
514517
public boolean deleteObject(BucketMetadata bucket, UUID id, String versionId) {
515518
var s3ObjectMetadata = getS3ObjectMetadata(bucket, id, versionId);
516519
if (s3ObjectMetadata != null) {
517-
if (bucket.isVersioningEnabled()) {
520+
if (bucket.isVersioningEnabled() && !"null".equals(versionId)) {
518521
if (versionId != null) {
519522
return doDeleteVersion(bucket, id, versionId);
520523
} else {
@@ -531,8 +534,14 @@ private boolean doDeleteVersion(BucketMetadata bucket, UUID id, String versionId
531534
synchronized (lockStore.get(id)) {
532535
try {
533536
var existingVersions = getS3ObjectVersions(bucket, id);
534-
existingVersions.deleteVersion(versionId);
535-
writeVersionsfile(bucket, id, existingVersions);
537+
if (existingVersions.versions().size() <= 1) {
538+
//this is the last version of an object, delete object completely.
539+
return doDeleteObject(bucket, id);
540+
} else {
541+
//there is at least one version of an object left, delete only the version.
542+
existingVersions.deleteVersion(versionId);
543+
writeVersionsfile(bucket, id, existingVersions);
544+
}
536545
} catch (Exception e) {
537546
throw new IllegalStateException("Could not delete object-version " + id, e);
538547
}
@@ -624,7 +633,7 @@ private Path getObjectFolderPath(BucketMetadata bucket, UUID id) {
624633
}
625634

626635
private Path getMetaFilePath(BucketMetadata bucket, UUID id, String versionId) {
627-
if (versionId != null) {
636+
if (versionId != null && !NULL_VERSION.equals(versionId)) {
628637
return getObjectFolderPath(bucket, id).resolve(format(VERSIONED_META_FILE, versionId));
629638
}
630639
return getObjectFolderPath(bucket, id).resolve(META_FILE);

0 commit comments

Comments
 (0)