Skip to content

Commit

Permalink
do not create container for AzureBlobStorage
Browse files Browse the repository at this point in the history
Related to #618.

Container creation is out of Gaufrette scope. The container should be
created by the deleveloper on its own.
Thus, the multi container mode has been removed, as it was creating
containers on the fly.
  • Loading branch information
nicolasmure committed Jul 1, 2019
1 parent a93e739 commit 06e3ffb
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 473 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
32 changes: 7 additions & 25 deletions spec/Gaufrette/Adapter/AzureBlobStorageSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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('<Code>SomeErrorCode</Code>');
$azureException->getErrorText()->willReturn(Argument::type('string'));

$this->shouldThrow(StorageFailure::class)->duringCreateContainer('containerName');
}
}
182 changes: 3 additions & 179 deletions src/Gaufrette/Adapter/AzureBlobStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down Expand Up @@ -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', [
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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];
}

/**
Expand Down
12 changes: 10 additions & 2 deletions tests/Gaufrette/Functional/Adapter/AzureBlobStorageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
}

Expand All @@ -41,6 +49,6 @@ public function tearDown()
return;
}

$this->adapter->deleteContainer($this->container);
$this->blobProxy->deleteContainer($this->container);
}
}
Loading

0 comments on commit 06e3ffb

Please sign in to comment.