Skip to content

Commit

Permalink
Let CopyObject overwrite store headers
Browse files Browse the repository at this point in the history
The headers we store as "storeHeaders" are part of the
"System-defined object metadata" which can be the only metadata
overwritten when copying objects.

Fixes #2005
  • Loading branch information
afranken committed Sep 27, 2024
1 parent 7b46711 commit a7d6b5d
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,43 @@ internal class CopyObjectV2IT : S3TestBase() {
assertThat(it.response().storageClass()).isEqualTo(StorageClass.STANDARD_IA)
}
}

@Test
@S3VerifiedTodo
fun testCopyObject_overwriteStoreHeader(testInfo: TestInfo) {
val sourceKey = UPLOAD_FILE_NAME
val uploadFile = File(UPLOAD_FILE_NAME)
val bucketName = givenBucketV2(testInfo)

s3ClientV2.putObject(
PutObjectRequest.builder()
.bucket(bucketName)
.key(sourceKey)
.contentDisposition("")
.build(),
RequestBody.fromFile(uploadFile)
)

val destinationBucketName = givenRandomBucketV2()
val destinationKey = "copyOf/$sourceKey"

s3ClientV2.copyObject(CopyObjectRequest
.builder()
.sourceBucket(bucketName)
.sourceKey(sourceKey)
.destinationBucket(destinationBucketName)
.destinationKey(destinationKey)
.metadataDirective(MetadataDirective.REPLACE)
.contentDisposition("attachment")
.build())

s3ClientV2.getObject(GetObjectRequest
.builder()
.bucket(destinationBucketName)
.key(destinationKey)
.build()
).use {
assertThat(it.response().contentDisposition()).isEqualTo("attachment")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -696,17 +696,20 @@ public ResponseEntity<CopyObjectResult> copyObject(@PathVariable String bucketNa
var s3ObjectMetadata = objectService.verifyObjectExists(copySource.bucket(), copySource.key());
objectService.verifyObjectMatchingForCopy(match, noneMatch, s3ObjectMetadata);

Map<String, String> metadata = Collections.emptyMap();
Map<String, String> userMetadata = Collections.emptyMap();
Map<String, String> storeHeaders = Collections.emptyMap();
if (MetadataDirective.REPLACE == metadataDirective) {
metadata = userMetadataFrom(httpHeaders);
userMetadata = userMetadataFrom(httpHeaders);
storeHeaders = storeHeadersFrom(httpHeaders);
}

var copyObjectResult = objectService.copyS3Object(copySource.bucket(),
copySource.key(),
bucketName,
key.key(),
encryptionHeadersFrom(httpHeaders),
metadata,
storeHeaders,
userMetadata,
storageClass);

//return version id / copy source version id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public CopyObjectResult copyS3Object(String sourceBucketName,
String destinationBucketName,
String destinationKey,
Map<String, String> encryptionHeaders,
Map<String, String> storeHeaders,
Map<String, String> userMetadata,
StorageClass storageClass) {
var sourceBucketMetadata = bucketStore.getBucketMetadata(sourceBucketName);
Expand All @@ -93,8 +94,9 @@ public CopyObjectResult copyS3Object(String sourceBucketName,
if (sourceKey.equals(destinationKey) && sourceBucketName.equals(destinationBucketName)) {
return objectStore.pretendToCopyS3Object(sourceBucketMetadata,
sourceId,
userMetadata,
encryptionHeaders,
storeHeaders,
userMetadata,
storageClass);
}

Expand All @@ -103,7 +105,7 @@ public CopyObjectResult copyS3Object(String sourceBucketName,
try {
return objectStore.copyS3Object(sourceBucketMetadata, sourceId,
destinationBucketMetadata, destinationId, destinationKey,
encryptionHeaders, userMetadata, storageClass);
encryptionHeaders, storeHeaders, userMetadata, storageClass);
} catch (Exception e) {
//something went wrong with writing the destination file, clean up ID from BucketStore.
bucketStore.removeFromBucket(destinationKey, destinationBucketName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ public CopyObjectResult copyS3Object(BucketMetadata sourceBucket,
UUID destinationId,
String destinationKey,
Map<String, String> encryptionHeaders,
Map<String, String> storeHeaders,
Map<String, String> userMetadata,
StorageClass storageClass) {
var sourceObject = getS3ObjectMetadata(sourceBucket, sourceId);
Expand All @@ -316,7 +317,8 @@ public CopyObjectResult copyS3Object(BucketMetadata sourceBucket,
destinationId,
destinationKey,
sourceObject.contentType(),
sourceObject.storeHeaders(),
storeHeaders == null || storeHeaders.isEmpty()
? sourceObject.storeHeaders() : storeHeaders,
sourceObject.dataPath(),
userMetadata == null || userMetadata.isEmpty()
? sourceObject.userMetadata() : userMetadata,
Expand All @@ -340,15 +342,16 @@ public CopyObjectResult copyS3Object(BucketMetadata sourceBucket,
*/
public CopyObjectResult pretendToCopyS3Object(BucketMetadata sourceBucket,
UUID sourceId,
Map<String, String> userMetadata,
Map<String, String> encryptionHeaders,
Map<String, String> storeHeaders,
Map<String, String> userMetadata,
StorageClass storageClass) {
var sourceObject = getS3ObjectMetadata(sourceBucket, sourceId);
if (sourceObject == null) {
return null;
}

verifyPretendCopy(sourceObject, userMetadata, encryptionHeaders, storageClass);
verifyPretendCopy(sourceObject, userMetadata, encryptionHeaders, storeHeaders, storageClass);

writeMetafile(sourceBucket, new S3ObjectMetadata(
sourceObject.id(),
Expand All @@ -365,7 +368,8 @@ public CopyObjectResult pretendToCopyS3Object(BucketMetadata sourceBucket,
sourceObject.legalHold(),
sourceObject.retention(),
sourceObject.owner(),
sourceObject.storeHeaders(),
storeHeaders == null || storeHeaders.isEmpty()
? sourceObject.storeHeaders() : storeHeaders,
encryptionHeaders == null || encryptionHeaders.isEmpty()
? sourceObject.encryptionHeaders() : encryptionHeaders,
sourceObject.checksumAlgorithm(),
Expand All @@ -378,11 +382,16 @@ public CopyObjectResult pretendToCopyS3Object(BucketMetadata sourceBucket,
private void verifyPretendCopy(S3ObjectMetadata sourceObject,
Map<String, String> userMetadata,
Map<String, String> encryptionHeaders,
Map<String, String> storeHeaders,
StorageClass storageClass) {
var userDataUnChanged = userMetadata == null || userMetadata.isEmpty();
var encryptionHeadersUnChanged = encryptionHeaders == null || encryptionHeaders.isEmpty();
var storeHeadersUnChanged = storeHeaders == null || storeHeaders.isEmpty();
var storageClassUnChanged = storageClass == null || storageClass == sourceObject.storageClass();
if (userDataUnChanged && storageClassUnChanged && encryptionHeadersUnChanged) {
if (userDataUnChanged
&& storageClassUnChanged
&& encryptionHeadersUnChanged
&& storeHeadersUnChanged) {
throw INVALID_COPY_REQUEST_SAME_KEY;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

/**
* Represents an object in S3, used to serialize and deserialize all metadata locally.
* <a href="https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingMetadata.html">See API</a>
*/
public record S3ObjectMetadata(
UUID id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ internal class ObjectStoreTest : StoreTestBase() {
objectStore.copyS3Object(
metadataFrom(sourceBucketName), sourceId,
metadataFrom(destinationBucketName),
destinationId, destinationObjectName, emptyMap(), NO_USER_METADATA, StorageClass.STANDARD_IA
destinationId, destinationObjectName, emptyMap(), emptyMap(), NO_USER_METADATA, StorageClass.STANDARD_IA
)

objectStore.getS3ObjectMetadata(metadataFrom(destinationBucketName), destinationId).also {
Expand Down Expand Up @@ -288,6 +288,7 @@ internal class ObjectStoreTest : StoreTestBase() {
destinationId,
destinationObjectName,
encryptionHeaders(),
emptyMap(),
NO_USER_METADATA,
StorageClass.STANDARD_IA
)
Expand Down

0 comments on commit a7d6b5d

Please sign in to comment.