Skip to content

Commit

Permalink
Merge pull request #2064 from adobe/2005-let-copy-object-overwrite-st…
Browse files Browse the repository at this point in the history
…ore-headers

Let CopyObject overwrite store headers
  • Loading branch information
afranken authored Sep 27, 2024
2 parents 240b31e + b16f9fe commit 4fb5ef9
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 11 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ Version 3.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Jav
* Version updates (build dependencies)
* TBD

## 3.10.2
3.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Java integration.

* Features and fixes
* Let CopyObject overwrite store headers (fixes #2005)
* Version updates (build dependencies)
* Bump org.apache.maven.plugins:maven-deploy-plugin from 3.1.2 to 3.1.3
* Bump org.apache.maven.plugins:maven-dependency-plugin from 3.7.1 to 3.8.0
* Bump org.apache.maven.plugins:maven-javadoc-plugin from 3.8.0 to 3.10.0
* Bump com.puppycrawl.tools:checkstyle from 10.17.0 to 10.18.1
* Bump actions/checkout from 4.1.7 to 4.2.0
* Bump github/codeql-action from 3.26.7 to 3.26.9
* Bump actions/setup-java from 4.3.0 to 4.4.0

## 3.10.1
3.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Java integration.

Expand Down
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
1 change: 1 addition & 0 deletions server/src/main/resources/application-trace.properties
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@

logging.level.root=trace
logging.level.org.springframework.web=trace
spring.mvc.log-request-details=true
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 4fb5ef9

Please sign in to comment.