Let S3 copy annotations on single-part S3CrtClient CopyObject

.changelog/bugfix-s3crt-small-copy-annotations.json

@@ -0,0 +1,6 @@
+{
+  "type": "bugfix",
+  "category": "aws-cpp-sdk-s3-crt",
+  "contributor": "sbiscigl",
+  "description": "S3CrtClient::CopyObject no longer reads and rewrites annotations itself for objects under 1 GiB, because S3's single-part CopyObject already copies them. Small copies with AnnotationDirective::COPY no longer make extra annotation calls or need annotation permissions."
+}

generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp

@@ -880,10 +880,13 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
     httpRequest->SetHeaderValue("x-amz-copy-source-if-match", sourceETag);
   }
 
+  // Below the multipart threshold S3's single-part CopyObject honors the metadata, tagging and
+  // annotation directives natively, so skip copying them here.
+  if (head.GetContentLength() < (1LL * 1024 * 1024 * 1024)) {
+    return annotations;
+  }
+
   if (copyAnnotations) {
-    // Neither the multipart copy nor S3's single-part CopyObject carries annotations, so always
-    // read them from the source here (regardless of size) and return them to be written to the
-    // destination after the copy completes (see the wrapped completion handler).
     Aws::String continuationToken;
     do {
       Model::ListObjectAnnotationsRequest listRequest;
@@ -922,12 +925,6 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
     } while (!continuationToken.empty());
   }
 
-  // Below the multipart threshold S3's single-part CopyObject honors the metadata/tagging directives
-  // natively, so skip header injection (annotations are handled above regardless of size).
-  if (head.GetContentLength() < (1LL * 1024 * 1024 * 1024)) {
-    return annotations;
-  }
-
   if (copyMetadata) {
     if (!head.GetContentType().empty()) {
       httpRequest->SetHeaderValue("content-type", head.GetContentType());

tests/aws-cpp-sdk-s3-crt-integration-tests/BucketAndObjectOperationTest.cpp

@@ -8,6 +8,7 @@
 #include <aws/core/auth/AWSCredentialsProviderChain.h>
 #include <aws/core/client/ClientConfiguration.h>
 #include <aws/core/client/CoreErrors.h>
+#include <aws/core/client/DefaultRetryStrategy.h>
 #include <aws/core/client/RetryStrategy.h>
 #include <aws/core/http/HttpClientFactory.h>
 #include <aws/core/http/HttpClient.h>
@@ -47,6 +48,11 @@
 #include <aws/s3-crt/model/PutObjectTaggingRequest.h>
 #include <aws/s3-crt/model/MetadataDirective.h>
 #include <aws/s3-crt/model/TaggingDirective.h>
+#include <aws/s3-crt/model/AnnotationDirective.h>
+#include <aws/s3-crt/model/PutObjectAnnotationRequest.h>
+#include <aws/s3-crt/model/ListObjectAnnotationsRequest.h>
+#include <aws/s3-crt/model/GetObjectAnnotationRequest.h>
+#include <aws/s3-crt/model/UploadPartCopyRequest.h>
 #include <aws/s3-crt/model/PutBucketVersioningRequest.h>
 #include <aws/s3-crt/model/VersioningConfiguration.h>
 #include <aws/s3-crt/model/BucketVersioningStatus.h>
@@ -55,8 +61,11 @@
 #include <aws/testing/ProxyConfig.h>
 #include <aws/testing/platform/PlatformTesting.h>
 #include <aws/testing/TestingEnvironment.h>
+#include <atomic>
 #include <fstream>
+#include <functional>
 #include <future>
+#include <iterator>
 
 #ifdef _WIN32
 #pragma warning(disable: 4127)
@@ -95,6 +104,7 @@
     static std::string BASE_CHECKSUMS_BUCKET_NAME = "checksums-crt";
     static std::string BASE_CRT_CREDENTIALS_TEST_BUCKET_NAME = "crt-credentials-test";
     static std::string BASE_VERSIONED_COPY_BUCKET_NAME = "versionedcopytest";
+    static std::string BASE_ANNOTATIONS_COPY_BUCKET_NAME = "annotationcopy";
     static const char* ALLOCATION_TAG = "BucketAndObjectOperationTest";
     static const char* TEST_OBJ_KEY = "TestObjectKey";
     static const char* TEST_NOT_MODIFIED_OBJ_KEY = "TestNotModifiedObjectKey";
@@ -136,7 +146,8 @@
               std::ref(BASE_EVENT_STREAM_ERRORS_IN_EVENT_TEST_BUCKET_NAME),
               std::ref(BASE_CHECKSUMS_BUCKET_NAME),
               std::ref(BASE_CRT_CREDENTIALS_TEST_BUCKET_NAME),
-              std::ref(BASE_VERSIONED_COPY_BUCKET_NAME)
+              std::ref(BASE_VERSIONED_COPY_BUCKET_NAME),
+              std::ref(BASE_ANNOTATIONS_COPY_BUCKET_NAME)
             };
 
         for (auto& testBucketName : TEST_BUCKETS)
@@ -153,15 +164,33 @@
         long CalculateDelayBeforeNextRetry(const AWSError<CoreErrors>&, long) const override { return 0; }
     };
 
+    class AnnotationRequestCountingRetryStrategy: public Aws::Client::DefaultRetryStrategy
+    {
+    public:
+        void RequestBookkeeping(const HttpResponseOutcome& outcome) override { Count(outcome); }
+        void RequestBookkeeping(const HttpResponseOutcome& outcome, const AWSError<CoreErrors>&) override { Count(outcome); }
+        size_t GetAnnotationRequestCount() const { return m_annotationRequestCount; }
+
+    private:
+        void Count(const HttpResponseOutcome& outcome)
+        {
+            if (outcome.IsSuccess() &&
+                outcome.GetResult()->GetOriginatingRequest().GetUri().GetQueryString().find("annotation") != Aws::String::npos)
+            {
+                ++m_annotationRequestCount;
+            }
+        }
+
+        std::atomic<size_t> m_annotationRequestCount{0};
+    };
+
     class BucketAndObjectOperationTest : public ::testing::Test
     {
     public:
         static std::shared_ptr<S3CrtClient> Client;
         static std::shared_ptr<S3CrtClient> oregonClient;
         static std::shared_ptr<HttpClient> m_HttpClient;
 
-    protected:
-
         static void SetUpTestCase()
         {
             EnsureUniqueBucketNames();
@@ -185,6 +214,7 @@
             DeleteBucket(CalculateBucketName(BASE_EVENT_STREAM_ERRORS_IN_EVENT_TEST_BUCKET_NAME.c_str()));
             DeleteBucket(CalculateBucketName(BASE_CHECKSUMS_BUCKET_NAME.c_str()));
             DeleteBucket(CalculateBucketName(BASE_CRT_CREDENTIALS_TEST_BUCKET_NAME.c_str()));
+            DeleteBucket(CalculateBucketName(BASE_ANNOTATIONS_COPY_BUCKET_NAME.c_str()));
 
             Client = nullptr;
             oregonClient = nullptr;
@@ -1152,6 +1182,122 @@
         Client->DeleteBucket(deleteBucketRequest);
     }
 
+    void CopyObjectWithAnnotations(S3CrtClient& copyClient,
+                                   const std::function<void(const Aws::String& bucketName, const Aws::String& sourceKey)>& createSource)
+    {
+        const auto& client = BucketAndObjectOperationTest::Client;
+        const Aws::String fullBucketName = BucketAndObjectOperationTest::CalculateBucketName(BASE_ANNOTATIONS_COPY_BUCKET_NAME.c_str());
+        SCOPED_TRACE(Aws::String("FullBucketName ") + fullBucketName);
+        CreateBucketRequest createBucketRequest;
+        createBucketRequest.SetBucket(fullBucketName);
+        createBucketRequest.SetACL(BucketCannedACL::private_);
+        CreateBucketOutcome createBucketOutcome = client->CreateBucket(createBucketRequest);
+        AWS_ASSERT_SUCCESS(createBucketOutcome);
+        ASSERT_TRUE(BucketAndObjectOperationTest::WaitForBucketToPropagate(fullBucketName));
+        BucketAndObjectOperationTest::TagTestBucket(fullBucketName, client);
+
+        const Aws::String testName = ::testing::UnitTest::GetInstance()->current_test_info()->name();
+        const Aws::String sourceKey = testName + "-source";
+        const Aws::String destKey = testName + "-destination";
+        ASSERT_NO_FATAL_FAILURE(createSource(fullBucketName, sourceKey));
+
+        const Aws::Map<Aws::String, Aws::String> annotations{{"classification", "internal"}, {"owner", "sdk-team"}};
+        for (const auto& annotation : annotations)
+        {
+            PutObjectAnnotationRequest putAnnotationRequest;
+            putAnnotationRequest.WithBucket(fullBucketName).WithKey(sourceKey).WithAnnotationName(annotation.first);
+            putAnnotationRequest.SetBody(Aws::MakeShared<Aws::StringStream>(ALLOCATION_TAG, annotation.second));
+            auto putAnnotationOutcome = client->PutObjectAnnotation(putAnnotationRequest);
+            AWS_ASSERT_SUCCESS(putAnnotationOutcome);
+        }
+
+        CopyObjectRequest copyRequest;
+        copyRequest.WithBucket(fullBucketName)
+            .WithKey(destKey)
+            .WithCopySource(fullBucketName + "/" + sourceKey)
+            .WithAnnotationDirective(AnnotationDirective::COPY);
+        auto copyOutcome = copyClient.CopyObject(copyRequest);
+        AWS_ASSERT_SUCCESS(copyOutcome);
+
+        ListObjectAnnotationsRequest listAnnotationsRequest;
+        listAnnotationsRequest.WithBucket(fullBucketName).WithKey(destKey);
+        auto listAnnotationsOutcome = client->ListObjectAnnotations(listAnnotationsRequest);
+        AWS_ASSERT_SUCCESS(listAnnotationsOutcome);
+        Aws::Map<Aws::String, Aws::String> copiedAnnotations;
+        for (const auto& entry : listAnnotationsOutcome.GetResult().GetAnnotations())
+        {
+            GetObjectAnnotationRequest getAnnotationRequest;
+            getAnnotationRequest.WithBucket(fullBucketName).WithKey(destKey).WithAnnotationName(entry.GetAnnotationName());
+            auto getAnnotationOutcome = client->GetObjectAnnotation(getAnnotationRequest);
+            AWS_ASSERT_SUCCESS(getAnnotationOutcome);
+            auto& payload = getAnnotationOutcome.GetResult().GetAnnotationPayload();
+            copiedAnnotations.emplace(entry.GetAnnotationName(), Aws::String(std::istreambuf_iterator<char>(payload), {}));
+        }
+        ASSERT_EQ(annotations, copiedAnnotations);
+    }
+
+    TEST_F(BucketAndObjectOperationTest, TestSinglePartCopyObjectLeavesAnnotationsToS3)
+    {
+        auto retryStrategy = Aws::MakeShared<AnnotationRequestCountingRetryStrategy>(ALLOCATION_TAG);
+        Aws::S3Crt::ClientConfiguration countingClientConfig;
+        countingClientConfig.region = Aws::Region::US_EAST_1;
+        countingClientConfig.retryStrategy = retryStrategy;
+        S3CrtClient countingClient(countingClientConfig);
+
+        ASSERT_NO_FATAL_FAILURE(CopyObjectWithAnnotations(countingClient, [](const Aws::String& bucketName, const Aws::String& sourceKey)
+        {
+            PutObjectRequest putObjectRequest;
+            putObjectRequest.WithBucket(bucketName).WithKey(sourceKey);
+            putObjectRequest.SetBody(Aws::MakeShared<Aws::StringStream>(ALLOCATION_TAG, "single part annotations copy test payload"));
+            auto putObjectOutcome = Client->PutObject(putObjectRequest);
+            AWS_ASSERT_SUCCESS(putObjectOutcome);
+        }));
+        ASSERT_EQ(0u, retryStrategy->GetAnnotationRequestCount());
+    }
+
+    TEST_F(BucketAndObjectOperationTest, TestMultipartCopyObjectCopiesAnnotations)
+    {
+        ASSERT_NO_FATAL_FAILURE(CopyObjectWithAnnotations(*Client, [](const Aws::String& bucketName, const Aws::String& sourceKey)
+        {
+            const Aws::String partSourceKey = sourceKey + "-part";
+            PutObjectRequest putPartSourceRequest;
+            putPartSourceRequest.WithBucket(bucketName).WithKey(partSourceKey);
+            putPartSourceRequest.SetBody(Aws::MakeShared<Aws::StringStream>(ALLOCATION_TAG, Aws::String(8 * 1024 * 1024, 'x')));
+            auto putPartSourceOutcome = Client->PutObject(putPartSourceRequest);
+            AWS_ASSERT_SUCCESS(putPartSourceOutcome);
+
+            CreateMultipartUploadRequest createMultipartUploadRequest;
+            createMultipartUploadRequest.WithBucket(bucketName).WithKey(sourceKey);
+            auto createMultipartUploadOutcome = Client->CreateMultipartUpload(createMultipartUploadRequest);
+            AWS_ASSERT_SUCCESS(createMultipartUploadOutcome);
+            const Aws::String uploadId = createMultipartUploadOutcome.GetResult().GetUploadId();
+
+            CompletedMultipartUpload completedMultipartUpload;
+            for (int partNumber = 1; partNumber <= 128; ++partNumber)
+            {
+                UploadPartCopyRequest uploadPartCopyRequest;
+                uploadPartCopyRequest.WithBucket(bucketName)
+                    .WithKey(sourceKey)
+                    .WithUploadId(uploadId)
+                    .WithPartNumber(partNumber)
+                    .WithCopySource(bucketName + "/" + partSourceKey);
+                auto uploadPartCopyOutcome = Client->UploadPartCopy(uploadPartCopyRequest);
+                AWS_ASSERT_SUCCESS(uploadPartCopyOutcome);
+                completedMultipartUpload.AddParts(CompletedPart()
+                    .WithPartNumber(partNumber)
+                    .WithETag(uploadPartCopyOutcome.GetResult().GetCopyPartResult().GetETag()));
+            }
+
+            CompleteMultipartUploadRequest completeMultipartUploadRequest;
+            completeMultipartUploadRequest.WithBucket(bucketName)
+                .WithKey(sourceKey)
+                .WithUploadId(uploadId)
+                .WithMultipartUpload(completedMultipartUpload);
+            auto completeMultipartUploadOutcome = Client->CompleteMultipartUpload(completeMultipartUploadRequest);
+            AWS_ASSERT_SUCCESS(completeMultipartUploadOutcome);
+        }));
+    }
+
     TEST_F(BucketAndObjectOperationTest, TestObjectOperationWithEventStream)
     {
         GTEST_SKIP() << "Select objects is not supported on new AWS accounts";

tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/S3CrtSpecificOperations.vm

@@ -374,11 +374,15 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
     httpRequest->SetHeaderValue("x-amz-copy-source-if-match", sourceETag);
   }
 
+  // Below the multipart threshold S3's single-part CopyObject honors the metadata, tagging and
+  // annotation directives natively, so skip copying them here.
+  if (head.GetContentLength() < (1LL * 1024 * 1024 * 1024))
+  {
+    return annotations;
+  }
+
   if (copyAnnotations)
   {
-    // Neither the multipart copy nor S3's single-part CopyObject carries annotations, so always
-    // read them from the source here (regardless of size) and return them to be written to the
-    // destination after the copy completes (see the wrapped completion handler).
     Aws::String continuationToken;
     do
     {
@@ -424,13 +428,6 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
     } while (!continuationToken.empty());
   }
 
-  // Below the multipart threshold S3's single-part CopyObject honors the metadata/tagging directives
-  // natively, so skip header injection (annotations are handled above regardless of size).
-  if (head.GetContentLength() < (1LL * 1024 * 1024 * 1024))
-  {
-    return annotations;
-  }
-
   if (copyMetadata)
   {
     if (!head.GetContentType().empty()) { httpRequest->SetHeaderValue("content-type", head.GetContentType()); }

tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/SmithyS3CrtSpecificOperations.vm

@@ -374,11 +374,15 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
     httpRequest->SetHeaderValue("x-amz-copy-source-if-match", sourceETag);
   }
 
+  // Below the multipart threshold S3's single-part CopyObject honors the metadata, tagging and
+  // annotation directives natively, so skip copying them here.
+  if (head.GetContentLength() < (1LL * 1024 * 1024 * 1024))
+  {
+    return annotations;
+  }
+
   if (copyAnnotations)
   {
-    // Neither the multipart copy nor S3's single-part CopyObject carries annotations, so always
-    // read them from the source here (regardless of size) and return them to be written to the
-    // destination after the copy completes (see the wrapped completion handler).
     Aws::String continuationToken;
     do
     {
@@ -424,13 +428,6 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
     } while (!continuationToken.empty());
   }
 
-  // Below the multipart threshold S3's single-part CopyObject honors the metadata/tagging directives
-  // natively, so skip header injection (annotations are handled above regardless of size).
-  if (head.GetContentLength() < (1LL * 1024 * 1024 * 1024))
-  {
-    return annotations;
-  }
-
   if (copyMetadata)
   {
     if (!head.GetContentType().empty()) { httpRequest->SetHeaderValue("content-type", head.GetContentType()); }