Issues

ZF-10628: Zend_Cloud_StorageService_Adapter_S3 ignores metadata adapter options passed into constructor

Description

I think it makes sense to have default metadata for {{Zend_Cloud_StorageService_Adapter_S3}} which is then used in the {{storeItem()}} call, if no metadata is passed to {{storeItem()}} explicitly.

That way, the {{storeItem()}} call can remain storage service adapter agnostic.

-Looking at http://framework.zend.com/manual/en/… it seems that the 'metadata' option, when passed as part of the options in the constructor, is supposed to affect storeItem(). At the moment this is ignored, however. So setting metadata needs to be done on the storeItem() call explicitly, which breaks the abstraction that Zend_Cloud otherwise provides.-

Comments

I can't figure out how to attach the patch as a file.


--- Zend/Cloud/StorageService/Adapter/S3.php    (revision 23287)
+++ Zend/Cloud/StorageService/Adapter/S3.php    (working copy)
@@ -54,6 +54,7 @@
     protected $_s3;
     protected $_defaultBucketName = null;
     protected $_defaultBucketAsDomain = false;
+    protected $_defaultMetadata = null;
 
     /**
      * Constructor
@@ -93,7 +94,11 @@
         if (isset($options[self::BUCKET_AS_DOMAIN])) {
             $this->_defaultBucketAsDomain = $options[self::BUCKET_AS_DOMAIN];
         }
+
+        if (isset($options[self::METADATA])) {
+            $this->_defaultMetadata = $options[self::METADATA];
+        }
     }
 
     /**
      * Get an item from the storage service.
@@ -138,7 +143,7 @@
             return $this->_s3->putObject(
                 $fullPath, 
                 $data, 
-                empty($options[self::METADATA]) ? null : $options[self::METADATA]
+                empty($options[self::METADATA]) ? $this->_defaultMetadata : $options[self::METADATA]
             );
         } catch (Zend_Service_Amazon_S3_Exception  $e) { 
             throw new Zend_Cloud_StorageService_Exception('Error on store: '.$e->getMessage(), $e->getCode(), $e);

I believe that this issue not is bug and yes an improvement. Because in documentation is mentioned your use only in storeItem(), in others options are mentioned your use in constructor.

Indeed. I misinterpreted the "Used in" column in the documentation. Improvement it is.

Any chance I can get you to write a test for this change? :)