code review

KnpLabs · Feb 8, 2018 · 9f3eb09 · 9f3eb09
1 parent c195b58
commit 9f3eb09
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 26 deletions.
diff --git a/spec/Gaufrette/Adapter/OpenStackSpec.php b/spec/Gaufrette/Adapter/OpenStackSpec.php
@@ -144,7 +144,7 @@ function it_throws_storage_failure_while_checking_if_file_exists(Container $cont
         $this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringexists('test');
     }
 
-    function it_list_objects(Container $container)
+    function it_lists_objects(Container $container)
     {
         $client = new Client();
         $api = new Api();
@@ -174,7 +174,7 @@ function it_throws_storage_failure_while_listing_objects(Container $container)
         $this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringkeys();
     }
 
-    function it_list_objects_with_prefix(Container $container)
+    function it_lists_objects_with_prefix(Container $container)
     {
         $client = new Client();
         $api = new Api();
@@ -335,7 +335,7 @@ function it_throws_storage_failure_while_fetching_metadata(Container $container,
         $this->shouldThrow('Gaufrette\Exception\StorageFailure')->duringgetMetadata('test');
     }
 
-    function it_set_metadata(Container $container, StorageObject $object)
+    function it_sets_metadata(Container $container, StorageObject $object)
     {
         $container->getObject('test')->willReturn($object);
         $object->resetMetadata([

diff --git a/spec/Gaufrette/FilesystemSpec.php b/spec/Gaufrette/FilesystemSpec.php
@@ -5,7 +5,6 @@
 use Gaufrette\Adapter;
 use Gaufrette\Exception\StorageFailure;
 use Gaufrette\File;
-use Gaufrette\Exception\StorageFailure;
 use PhpSpec\ObjectBehavior;
 use Prophecy\Argument;
 

diff --git a/src/Gaufrette/Adapter/OpenStack.php b/src/Gaufrette/Adapter/OpenStack.php
@@ -21,7 +21,7 @@
  * @see http://docs.os.php-opencloud.com/en/latest/services/object-store/v1/objects.html
  * @see http://refdocs.os.php-opencloud.com/OpenStack/OpenStack.html
  */
-class OpenStack implements Adapter,
+final class OpenStack implements Adapter,
                            ChecksumCalculator,
                            ListKeysAware,
                            MetadataSupporter,
@@ -31,22 +31,22 @@ class OpenStack implements Adapter,
     /**
      * @var Service
      */
-    protected $objectStore;
+    private $objectStore;
 
     /**
      * @var string
      */
-    protected $containerName;
+    private $containerName;
 
     /**
      * @var bool
      */
-    protected $createContainer;
+    private $createContainer;
 
     /**
      * @var Container
      */
-    protected $container;
+    private $container;
 
     /**
      * @param Service $objectStore
@@ -63,11 +63,11 @@ public function __construct(Service $objectStore, $containerName, $createContain
     /**
      * Returns an initialized container.
      *
-     * @throws \RuntimeException
+     * @throws StorageFailure
      *
      * @return Container
      */
-    protected function getContainer()
+    private function getContainer()
     {
         if ($this->container) {
             return $this->container;
@@ -79,20 +79,33 @@ protected function getContainer()
             }
 
             if (!$this->createContainer) {
-                throw new \RuntimeException(sprintf('Container "%s" does not exist.', $this->containerName));
+                throw new StorageFailure(sprintf('Container "%s" does not exist.', $this->containerName));
             }
 
-            try {
-                return $this->container = $this->objectStore->createContainer(['name' => $this->containerName]);
-            } catch (BadResponseError $e) {
-                throw new \RuntimeException(
-                    sprintf('Container "%s" could not be created (HTTP %d response received)', $this->containerName, $e->getResponse()->getStatusCode())
-                );
-            }
+            return $this->container = $this->createContainer();
         } catch (BadResponseError $e) {
-            // non 404 status error received
-            throw new \RuntimeException(
-                sprintf('HTTP %d response received when checking the existence of the container "%s"', $e->getResponse()->getStatusCode(), $this->containerName)
+            throw new StorageFailure(
+                sprintf('HTTP %d response received when checking the existence of the container "%s"', $e->getResponse()->getStatusCode(), $this->containerName),
+                $e->getCode(),
+                $e
+            );
+        }
+    }
+
+    /**
+     * @throws StorageFailure
+     *
+     * @return Container
+     */
+    private function createContainer()
+    {
+        try {
+            return $this->objectStore->createContainer(['name' => $this->containerName]);
+        } catch (BadResponseError $e) {
+            throw new StorageFailure(
+                sprintf('Container "%s" could not be created (HTTP %d response received)', $this->containerName, $e->getResponse()->getStatusCode()),
+                $e->getCode(),
+                $e
             );
         }
     }
@@ -218,9 +231,17 @@ public function rename($sourceKey, $targetKey)
             );
         }
 
-        $this->write($targetKey, $this->read($sourceKey));
+        try {
+            $this->write($targetKey, $this->read($sourceKey));
 
-        $this->delete($sourceKey);
+            $this->delete($sourceKey);
+        } catch (StorageFailure $e) {
+            throw StorageFailure::unexpectedFailure(
+                'rename',
+                ['sourceKey' => $sourceKey, 'targetKey' => $targetKey],
+                $e
+            );
+        }
     }
 
     /**
@@ -325,7 +346,7 @@ public function size($key)
      *
      * @return StorageObject
      */
-    protected function getObject($key)
+    private function getObject($key)
     {
         return $this->getContainer()->getObject($key);
     }
@@ -341,7 +362,7 @@ protected function getObject($key)
      *
      * @return StorageObject
      */
-    protected function retrieveObject($key)
+    private function retrieveObject($key)
     {
         $object = $this->getObject($key);
         $object->retrieve();