bugfix: align mpcopy with copyobject metadata directive (#3293)

Author: stobrien89

.changes/nextrelease/fix-mpcopy-metadata.json

@@ -0,0 +1,7 @@
+[
+  {
+    "type": "bugfix",
+    "category": "S3",
+    "description": "Updates `MultipartCopy` to fully align with `CopyObject` metadata directive behavior. When `$config['metadata_directive']` is set to `COPY` (default), source object metadata takes precedence over any matching values provided in `$config['params']`."
+  }
+]

src/S3/MultipartCopy.php

@@ -13,6 +13,11 @@ class MultipartCopy extends AbstractUploadManager
         getInitiateParams as private traitGetInitiateParams;
     }
 
+    private const VALID_METADATA_DIRECTIVES = [
+        'COPY' => true,
+        'REPLACE' => true,
+    ];
+
     /**
      * Metadata fields that can be copied from the source object
      * to the destination during a multipart copy.
@@ -70,10 +75,10 @@ class MultipartCopy extends AbstractUploadManager
      *   source object metadata to the destination. Set to 'COPY' to
      *   automatically forward metadata fields (Metadata, CacheControl,
      *   ContentDisposition, ContentEncoding, ContentLanguage, ContentType,
-     *   Expires) from the source object. Set to 'REPLACE' to suppress automatic
-     *   metadata copying (you must then provide your own metadata via the
-     *   'params' option). User-provided values in 'params' always take
-     *   precedence over source metadata.
+     *   Expires) from the source object. When set to 'COPY', source metadata
+     *   takes precedence and any matching fields provided in 'params' are
+     *   ignored. Set to 'REPLACE' to suppress automatic metadata copying and
+     *   use your own values via the 'params' option.
      * - source_metadata: (Aws\ResultInterface) The result of a HeadObject call
      *   on the copy source. If not provided, the SDK makes a HeadObject request
      *   to obtain the source object's size and metadata. Providing this avoids
@@ -211,7 +216,7 @@ protected function getInitiateParams()
 
         $directive = strtoupper($this->config['metadata_directive'] ?? 'COPY');
 
-        if (!in_array($directive, ['COPY', 'REPLACE'], true)) {
+        if (!isset(self::VALID_METADATA_DIRECTIVES[$directive])) {
             throw new \InvalidArgumentException(
                 "Invalid metadata_directive value '$directive'."
                 . " Must be 'COPY' or 'REPLACE'."
@@ -221,7 +226,7 @@ protected function getInitiateParams()
         if ($directive === 'COPY') {
             $sourceMetadata = $this->getSourceMetadata();
             foreach (self::$copyMetadataFields as $field) {
-                if (!isset($params[$field]) && !empty($sourceMetadata[$field])) {
+                if (!empty($sourceMetadata[$field])) {
                     $params[$field] = $sourceMetadata[$field];
                 }
             }

src/S3/ObjectCopier.php

@@ -17,8 +17,12 @@
  *
  * Makes a HeadObject call on the source to determine object size. Objects
  * below the multipart threshold (default 5 GB) are copied with a single
- * CopyObject call using MetadataDirective: COPY. Objects above the threshold
- * use MultipartCopy, which also preserves source metadata by default.
+ * CopyObject call using MetadataDirective: COPY.
+ * 
+ * Objects above the threshold use MultipartCopy, which preserves source metadata by default.
+ * When the multipart path is used with the default metadata_directive ('COPY'),
+ * metadata fields in 'params' (e.g. Metadata, ContentType, CacheControl)
+ * are overridden by the source object's values.
  */
 class ObjectCopier implements PromisorInterface
 {

tests/S3/MultipartCopyTest.php

@@ -310,7 +310,7 @@ public function testMetadataDirectiveReplaceSuppressesSourceMetadata()
         $this->assertArrayNotHasKey('ContentDisposition', $initiateParams);
     }
 
-    public function testUserParamsOverrideSourceMetadata()
+    public function testSourceMetadataOverridesUserParamsWhenCopy()
     {
         $client = $this->getTestClient('s3');
         $url = 'http://foo.s3.amazonaws.com/bar';
@@ -344,10 +344,9 @@ public function testUserParamsOverrideSourceMetadata()
         ]);
         $uploader->upload();
 
-        // User-provided params should take precedence
-        $this->assertSame('text/plain', $initiateParams['ContentType']);
-        $this->assertSame(['user-key' => 'user-value'], $initiateParams['Metadata']);
-        // Source metadata that was NOT overridden should still be copied
+        // Source metadata takes precedence over user-provided params when directive is COPY
+        $this->assertSame('application/pdf', $initiateParams['ContentType']);
+        $this->assertSame(['source-key' => 'source-value'], $initiateParams['Metadata']);
         $this->assertSame('max-age=3600', $initiateParams['CacheControl']);
     }