diff --git a/CHANGELOG.md b/CHANGELOG.md index 171688f1d..084870a6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,18 @@ which is the latest supported version of the SDK for OpenStack instead of https://github.com/rackspace/php-opencloud (#533). - Google Cloud Storage Adapter (#557) -## Removed +## Removed (introduces BC breaks) - The [OpenCloud adapter](https://github.com/KnpLabs/Gaufrette/blob/v0.5.0/src/Gaufrette/Adapter/OpenCloud.php) has been removed. - The [ObjectStoreFactory](https://github.com/KnpLabs/Gaufrette/blob/v0.5.0/src/Gaufrette/Adapter/OpenStackCloudFiles/ObjectStoreFactory.php) has been removed. +## Changes (introduces BC breaks) + +- Gaufrette is no longer responsible for bucket / container creation. This +should be done prior to any adapter usage (#618). + Thank you @nicolasmure and @PanzerLlama for your contributions ! v0.8.3 diff --git a/UPGRADE.md b/UPGRADE.md index 11d716416..24e8d2535 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,6 +1,17 @@ 1.0 === +**Gaufrette\Adapter\AzureblobStorage:** +As container management is out of Gaufrette scope (see #618), this adapter has +the following BC breaks : +* The `createContainer` public method has been removed. +* The `deleteContainer` public method has been removed. +* The `getCreateContainerOptions` public method has been removed. +* The `setCreateContainerOptions` public method has been removed. +* Drop support for [multi continer mode](https://github.com/KnpLabs/Gaufrette/blob/b488cf8f595c3c7a35005f72b60692e14c69398c/doc/adapters/azure-blob-storage.md#multi-container-mode). +* The constructor's `create` parameter has been removed. +* The constructor's `containerName` parameter is now mandatory (string). + **Gaufrette\Adapter\OpenStackCloudFiles\ObjectStoreFactory:** * This factory has been removed diff --git a/doc/adapters/azure-blob-storage.md b/doc/adapters/azure-blob-storage.md index 38a3df98e..8e636440a 100644 --- a/doc/adapters/azure-blob-storage.md +++ b/doc/adapters/azure-blob-storage.md @@ -20,36 +20,16 @@ You should be able to find your **endpoint**, **account name** and **account key Thanks to the blob proxy factory, the adapter lazy loads the connection to the endpoint, so it will not create any connection until it's really needed (eg. when a read or write operation is issued). -## Multi-container mode - -If you specify a container name, adapter will use only that container for all blobs. - -If you omit specifying a container, it will use a so-called multi-container mode in which container name is determined -directly from key. This allows for more flexibility if you're using dedicated storage accounts per asset type -(ie. one for images, one for videos) as you get to group assets logically, use container-level privileges, etc. +Note that your container should be created by your own needs in order to use +this adapter. ## Example ```php -write('my/stuff.txt', 'This is my stuff'); - -// multi-container mode -$adapter = new Gaufrette\Adapter\AzureBlobStorage($factory); -// make auto-created containers public by default -$containerOptions = new MicrosoftAzure\Storage\Blob\Models\CreateContainerOptions; -$containerOptions->setPublicAccess(true); -$adapter->setCreateContainerOptions($containerOptions); -$filesystem = new Gaufrette\Filesystem($adapter); -// container=my (auto-created), path=stuff.txt -$filesystem->write('my/stuff.txt', 'This is my stuff'); - ``` diff --git a/spec/Gaufrette/Adapter/AzureBlobStorageSpec.php b/spec/Gaufrette/Adapter/AzureBlobStorageSpec.php index 80e0603a7..5bb5ab5f5 100644 --- a/spec/Gaufrette/Adapter/AzureBlobStorageSpec.php +++ b/spec/Gaufrette/Adapter/AzureBlobStorageSpec.php @@ -31,6 +31,13 @@ function it_should_be_initializable() $this->shouldHaveType('Gaufrette\Adapter\MetadataSupporter'); } + function it_should_require_a_container_name(BlobProxyFactoryInterface $blobFactory) + { + $this->beConstructedWith($blobFactory); + + $this->shouldThrow(\ArgumentCountError::class)->duringInstantiation(); + } + function it_reads_file(BlobProxyFactoryInterface $blobFactory, IBlob $blob, GetBlobResult $blobContent) { $blobFactory->create()->willReturn($blob); @@ -302,29 +309,4 @@ function it_throws_storage_failure_when_it_fails_to_get_keys( $this->shouldThrow(StorageFailure::class)->duringKeys(); } - - function it_creates_container(BlobProxyFactoryInterface $blobFactory, IBlob $blob) - { - $blobFactory->create()->willReturn($blob); - - $blob->createContainer('containerName', null)->shouldBeCalled(); - - $this->createContainer('containerName'); - } - - function it_throws_storage_failure_when_it_fails_to_create_container( - BlobProxyFactoryInterface $blobFactory, - IBlob $blob, - ServiceException $azureException, - ResponseInterface $response - ) { - $blobFactory->create()->willReturn($blob); - - $blob->createContainer('containerName', null)->willThrow($azureException->getWrappedObject()); - $azureException->getResponse()->willReturn($response); - $response->getBody()->willReturn('SomeErrorCode'); - $azureException->getErrorText()->willReturn(Argument::type('string')); - - $this->shouldThrow(StorageFailure::class)->duringCreateContainer('containerName'); - } } diff --git a/src/Gaufrette/Adapter/AzureBlobStorage.php b/src/Gaufrette/Adapter/AzureBlobStorage.php index 4506c463a..bff9aefc4 100644 --- a/src/Gaufrette/Adapter/AzureBlobStorage.php +++ b/src/Gaufrette/Adapter/AzureBlobStorage.php @@ -9,7 +9,6 @@ use Gaufrette\Adapter\AzureBlobStorage\BlobProxyFactoryInterface; use MicrosoftAzure\Storage\Blob\Models\Blob; use MicrosoftAzure\Storage\Blob\Models\CreateBlockBlobOptions; -use MicrosoftAzure\Storage\Blob\Models\CreateContainerOptions; use MicrosoftAzure\Storage\Common\Exceptions\ServiceException; /** @@ -46,104 +45,18 @@ class AzureBlobStorage implements Adapter, MetadataSupporter, SizeCalculator, Ch */ protected $blobProxy; - /** - * @var bool - */ - protected $multiContainerMode = false; - - /** - * @var CreateContainerOptions - */ - protected $createContainerOptions; - /** * @param AzureBlobStorage\BlobProxyFactoryInterface $blobProxyFactory - * @param string|null $containerName - * @param bool $create + * @param string $containerName * @param bool $detectContentType * * @throws \RuntimeException */ - public function __construct(BlobProxyFactoryInterface $blobProxyFactory, $containerName = null, $create = false, $detectContentType = true) + public function __construct(BlobProxyFactoryInterface $blobProxyFactory, string $containerName, bool $detectContentType = true) { $this->blobProxyFactory = $blobProxyFactory; $this->containerName = $containerName; $this->detectContentType = $detectContentType; - - if (null === $containerName) { - $this->multiContainerMode = true; - } elseif ($create) { - $this->createContainer($containerName); - } - } - - /** - * @return CreateContainerOptions - */ - public function getCreateContainerOptions() - { - return $this->createContainerOptions; - } - - /** - * @param CreateContainerOptions $options - */ - public function setCreateContainerOptions(CreateContainerOptions $options) - { - $this->createContainerOptions = $options; - } - - /** - * Creates a new container. - * - * @param string $containerName - * @param \MicrosoftAzure\Storage\Blob\Models\CreateContainerOptions $options - * - * @throws StorageFailure if cannot create the container - */ - public function createContainer($containerName, CreateContainerOptions $options = null) - { - $this->init(); - - if (null === $options) { - $options = $this->getCreateContainerOptions(); - } - - try { - $this->blobProxy->createContainer($containerName, $options); - } catch (ServiceException $e) { - $errorCode = $this->getErrorCodeFromServiceException($e); - - // We don't care if the container was created between check and creation attempt - // it might be due to a parallel execution creating it. - if ($errorCode !== self::ERROR_CONTAINER_ALREADY_EXISTS) { - throw StorageFailure::unexpectedFailure('createContainer', [ - 'containerName' => $containerName, - 'options' => $options, - ], $e); - } - } - } - - /** - * Deletes a container. - * - * @param string $containerName - * @param DeleteContainerOptions $options - * - * @throws StorageFailure if cannot delete the container - */ - public function deleteContainer($containerName, DeleteContainerOptions $options = null) - { - $this->init(); - - try { - $this->blobProxy->deleteContainer($containerName, $options); - } catch (ServiceException $e) { - throw StorageFailure::unexpectedFailure('deleteContainer', [ - 'containerName' => $containerName, - ], $e); - } } /** @@ -187,10 +100,6 @@ public function write($key, $content) } try { - if ($this->multiContainerMode) { - $this->createContainer($containerName); - } - $this->blobProxy->createBlockBlob($containerName, $key, $content, $options); } catch (ServiceException $e) { throw StorageFailure::unexpectedFailure('write', [ @@ -231,76 +140,10 @@ public function keys() { $this->init(); - if ($this->multiContainerMode) { - return $this->keysForMultiContainerMode(); - } - - return $this->keysForSingleContainerMode(); - } - - /** - * List objects stored when the adapter is used in multi container mode. - * - * @return array - * - * @throws \RuntimeException - */ - private function keysForMultiContainerMode() - { - try { - $containersList = $this->blobProxy->listContainers()->getContainers(); - $lists = []; - - foreach ($containersList as $container) { - $lists[] = $this->fetchContainerKeysAndIgnore404($container->getName()); - } - - return !empty($lists) ? array_merge(...$lists) : []; - } catch (ServiceException $e) { - throw StorageFailure::unexpectedFailure('keys', [ - 'multiContainerMode' => $this->multiContainerMode, - 'containerName' => $this->containerName, - ], $e); - } - } - - /** - * List the keys in a container and ignore any 404 error. - * - * This prevent race conditions happening when a container is deleted after calling listContainers() and - * before the container keys are fetched. - * - * @param string $containerName - * - * @return array - */ - private function fetchContainerKeysAndIgnore404($containerName) - { - try { - return $this->fetchBlobs($containerName, $containerName); - } catch (ServiceException $e) { - if ($e->getResponse()->getStatusCode() === 404) { - return []; - } - - throw $e; - } - } - - /** - * List objects stored when the adapter is not used in multi container mode. - * - * @return array - * - * @throws \RuntimeException - */ - private function keysForSingleContainerMode() - { try { return $this->fetchBlobs($this->containerName); } catch (ServiceException $e) { throw StorageFailure::unexpectedFailure('keys', [ - 'multiContainerMode' => $this->multiContainerMode, 'containerName' => $this->containerName, ], $e); } @@ -413,10 +256,6 @@ public function rename($sourceKey, $targetKey) list($targetContainerName, $targetKey) = $this->tokenizeKey($targetKey); try { - if ($this->multiContainerMode) { - $this->createContainer($targetContainerName); - } - $this->blobProxy->copyBlob($targetContainerName, $targetKey, $sourceContainerName, $sourceKey); $this->blobProxy->deleteBlob($sourceContainerName, $sourceKey); } catch (ServiceException $e) { @@ -535,22 +374,7 @@ private function guessContentType($content) */ private function tokenizeKey($key) { - $containerName = $this->containerName; - if (false === $this->multiContainerMode) { - return [$containerName, $key]; - } - - if (false === ($index = strpos($key, '/'))) { - throw new InvalidKey(sprintf( - 'Failed to establish container name from key "%s", container name is required in multi-container mode', - $key - )); - } - - $containerName = substr($key, 0, $index); - $key = substr($key, $index + 1); - - return [$containerName, $key]; + return [$this->containerName, $key]; } /** diff --git a/tests/Gaufrette/Functional/Adapter/AzureBlobStorageTest.php b/tests/Gaufrette/Functional/Adapter/AzureBlobStorageTest.php index e53e669e1..d00b7b3d9 100644 --- a/tests/Gaufrette/Functional/Adapter/AzureBlobStorageTest.php +++ b/tests/Gaufrette/Functional/Adapter/AzureBlobStorageTest.php @@ -18,6 +18,9 @@ class AzureBlobStorageTest extends FunctionalTestCase /** @var AzureBlobStorage */ private $adapter; + /** @var \MicrosoftAzure\Storage\Blob\Internal\IBlob */ + private $blobProxy; + public function setUp() { $account = getenv('AZURE_ACCOUNT'); @@ -30,8 +33,13 @@ public function setUp() $connection = sprintf('BlobEndpoint=http://%1$s.blob.core.windows.net/;AccountName=%1$s;AccountKey=%2$s', $account, $key); + $blobProxyFactory = new BlobProxyFactory($connection); + $this->blobProxy = $blobProxyFactory->create(); + $this->container = uniqid($containerName); - $this->adapter = new AzureBlobStorage(new BlobProxyFactory($connection), $this->container, true); + $this->blobProxy->createContainer($this->container); + + $this->adapter = new AzureBlobStorage($blobProxyFactory, $this->container); $this->filesystem = new Filesystem($this->adapter); } @@ -41,6 +49,21 @@ public function tearDown() return; } - $this->adapter->deleteContainer($this->container); + $this->blobProxy->deleteContainer($this->container); + } + + /** + * @test + * @group functional + * @expectedException \Gaufrette\Exception\StorageFailure + */ + public function shouldThrowWhenUsingAnUnexistingContainer() + { + $this->blobProxy->deleteContainer($this->container); + + $this->filesystem->write('foo', 'Some content'); + + // will not delete the container again, see self::tearDown function + $this->adapter = null; } } diff --git a/tests/Gaufrette/Functional/Adapter/AzureMultiContainerBlobStorageTest.php b/tests/Gaufrette/Functional/Adapter/AzureMultiContainerBlobStorageTest.php deleted file mode 100644 index 328996594..000000000 --- a/tests/Gaufrette/Functional/Adapter/AzureMultiContainerBlobStorageTest.php +++ /dev/null @@ -1,266 +0,0 @@ -markTestSkipped('Either AZURE_ACCOUNT and/or AZURE_KEY env variables are not defined.'); - } - - $connection = sprintf('BlobEndpoint=http://%1$s.blob.core.windows.net/;AccountName=%1$s;AccountKey=%2$s', $account, $key); - - $this->adapter = new AzureBlobStorage(new BlobProxyFactory($connection)); - $this->filesystem = new Filesystem($this->adapter); - } - - /** - * @test - * @group functional - */ - public function shouldWriteAndRead() - { - $path1 = $this->createUniqueContainerName('container') . '/foo'; - $path2 = $this->createUniqueContainerName('test') . '/subdir/foo'; - - $this->filesystem->write($path1, 'Some content'); - $this->filesystem->write($path2, 'Some content1', true); - - $this->assertEquals('Some content', $this->filesystem->read($path1)); - $this->assertEquals('Some content1', $this->filesystem->read($path2)); - } - - /** - * @test - * @group functional - */ - public function shouldUpdateFileContent() - { - $path = $this->createUniqueContainerName('container') . '/foo'; - - $this->filesystem->write($path, 'Some content'); - $this->filesystem->write($path, 'Some content updated', true); - - $this->assertEquals('Some content updated', $this->filesystem->read($path)); - } - - /** - * @test - * @group functional - */ - public function shouldCheckIfFileExists() - { - $path1 = $this->createUniqueContainerName('container') . '/foo'; - $path2 = $this->createUniqueContainerName('test') . '/somefile'; - - $this->assertFalse($this->filesystem->has($path1)); - - $this->filesystem->write($path1, 'Some content'); - - $this->assertTrue($this->filesystem->has($path1)); - $this->assertFalse($this->filesystem->has($path2)); - } - - /** - * @test - * @group functional - */ - public function shouldGetMtime() - { - $path = $this->createUniqueContainerName('container') . '/foo'; - - $this->filesystem->write($path, 'Some content'); - - $this->assertGreaterThan(0, $this->filesystem->mtime($path)); - } - - /** - * @test - * @group functional - */ - public function shouldGetSize() - { - $path = $this->createUniqueContainerName('container') . '/foo'; - - $this->filesystem->write($path, 'Some content'); - - $this->assertEquals(12, $this->filesystem->size($path)); - } - - /** - * @test - * @group functional - */ - public function shouldGetMd5Hash() - { - $path = $this->createUniqueContainerName('container') . '/foo'; - - $content = 'Some content'; - $this->filesystem->write($path, $content); - - $this->assertEquals(\md5($content), $this->filesystem->checksum($path)); - } - - /** - * @test - * @group functional - * @expectedException \RuntimeException - * @expectedMessage Could not get mtime for the "foo" key - */ - public function shouldFailWhenTryMtimeForKeyWhichDoesNotExist() - { - $this->assertFalse($this->filesystem->mtime('container5/foo')); - } - - /** - * @test - * @group functional - */ - public function shouldRenameFile() - { - $somedir = $this->createUniqueContainerName('somedir'); - $path1 = $this->createUniqueContainerName('container') . '/foo'; - $path2 = $this->createUniqueContainerName('container-new') . '/boo'; - $path3 = $somedir . '/sub/boo'; - - $this->filesystem->write($path1, 'Some content'); - $this->filesystem->rename($path1, $path2); - - $this->assertFalse($this->filesystem->has($path1)); - $this->assertEquals('Some content', $this->filesystem->read($path2)); - - $this->filesystem->write($path1, 'Some content'); - $this->filesystem->rename($path1, $path3); - - $this->assertFalse($this->filesystem->has($somedir . '/sub/foo')); - $this->assertEquals('Some content', $this->filesystem->read($path3)); - } - - /** - * @test - * @group functional - */ - public function shouldDeleteFile() - { - $path = $this->createUniqueContainerName('container') . '/foo'; - - $this->filesystem->write($path, 'Some content'); - - $this->assertTrue($this->filesystem->has($path)); - - $this->filesystem->delete($path); - - $this->assertFalse($this->filesystem->has($path)); - } - - /** - * @test - * @group functional - */ - public function shouldFetchKeys() - { - $path1 = $this->createUniqueContainerName('container-1') . '/foo'; - $path2 = $this->createUniqueContainerName('container-2') . '/bar'; - $path3 = $this->createUniqueContainerName('container-3') . '/baz'; - - $this->filesystem->write($path1, 'Some content'); - $this->filesystem->write($path2, 'Some content'); - $this->filesystem->write($path3, 'Some content'); - - $actualKeys = $this->filesystem->keys(); - foreach ([$path1, $path2, $path3] as $key) { - $this->assertContains($key, $actualKeys); - } - } - - /** - * @test - * @group functional - */ - public function shouldWorkWithHiddenFiles() - { - $path = $this->createUniqueContainerName('container') . '/.foo'; - - $this->filesystem->write($path, 'hidden'); - $this->assertTrue($this->filesystem->has($path)); - $this->assertContains($path, $this->filesystem->keys()); - $this->filesystem->delete($path); - $this->assertFalse($this->filesystem->has($path)); - } - - /** - * @test - * @group functional - */ - public function shouldKeepFileObjectInRegister() - { - $path = $this->createUniqueContainerName('container') . '/somefile'; - - $FileObjectA = $this->filesystem->createFile($path); - $FileObjectB = $this->filesystem->createFile($path); - - $this->assertSame($FileObjectB, $FileObjectA); - } - - /** - * @test - * @group functional - */ - public function shouldWriteToSameFile() - { - $path = $this->createUniqueContainerName('container') . '/somefile'; - - $FileObjectA = $this->filesystem->createFile($path); - $FileObjectA->setContent('ABC'); - - $FileObjectB = $this->filesystem->createFile($path); - $FileObjectB->setContent('DEF'); - - $this->assertEquals('DEF', $FileObjectA->getContent()); - } - - private function createUniqueContainerName($prefix) - { - $this->containers[] = $container = uniqid($prefix); - - return $container; - } - - public function tearDown() - { - foreach ($this->containers as $container) { - try { - $this->adapter->deleteContainer($container); - } catch (StorageFailure $e) { - if (null !== ($previous = $e->getPrevious()) - && $previous instanceof ServiceException - && $previous->getResponse()->getStatusCode() === 404 - ) { - continue; - } - - throw $e; - } - } - } -}