From 881e9e8c24dbfbc6ad79575344a9b44add0a93d5 Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Tue, 24 Oct 2023 07:57:46 +0300 Subject: [PATCH 1/8] UHF-9113: Add empty drupal/flysystem_azure so composer package can be removed --- modules/flysystem_azure/flysystem_azure.info.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 modules/flysystem_azure/flysystem_azure.info.yml diff --git a/modules/flysystem_azure/flysystem_azure.info.yml b/modules/flysystem_azure/flysystem_azure.info.yml new file mode 100644 index 0000000..ea7f87e --- /dev/null +++ b/modules/flysystem_azure/flysystem_azure.info.yml @@ -0,0 +1,7 @@ +name: Flysystem Azure +description: 'Provides drupal/flysystem_azure so composer package can be removed.' +type: module +core_version_requirement: ^9 || ^10 +package: Flysystem +dependencies: + - flysystem:flysystem From 3b27fc9c74655ee9df6b3fca1dd045b585b69aba Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Mon, 23 Oct 2023 16:30:21 +0300 Subject: [PATCH 2/8] UHF-9113: Resolve dependency problems with drupal 10 --- composer.json | 10 +- src/AzureBlobStorageAdapter.php | 321 ++++++++++++++++++ .../Adapter/AzureBlobStorageAdapter.php | 29 ++ src/Flysystem/Azure.php | 82 ++++- 4 files changed, 429 insertions(+), 13 deletions(-) create mode 100644 src/AzureBlobStorageAdapter.php create mode 100644 src/Flysystem/Adapter/AzureBlobStorageAdapter.php diff --git a/composer.json b/composer.json index ab75554..eb154fd 100644 --- a/composer.json +++ b/composer.json @@ -5,17 +5,11 @@ "license": "GPL-2.0-or-later", "minimum-stability": "dev", "require": { - "drupal/flysystem_azure": "1.0.x-dev" + "drupal/flysystem": "^2.1@RC", + "microsoft/azure-storage-blob": "^1.1" }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0", "drupal/coder": "^8.3" - }, - "extra": { - "patches": { - "drupal/flysystem_azure": { - "D10 patch": "https://www.drupal.org/files/issues/2022-06-15/flysystem_azure.1.0.x-dev.rector.patch" - } - } } } diff --git a/src/AzureBlobStorageAdapter.php b/src/AzureBlobStorageAdapter.php new file mode 100644 index 0000000..e19fdb5 --- /dev/null +++ b/src/AzureBlobStorageAdapter.php @@ -0,0 +1,321 @@ +client = $client; + $this->container = $container; + $this->setPathPrefix($prefix); + } + + public function write($path, $contents, Config $config) + { + return $this->upload($path, $contents, $config) + compact('contents'); + } + + public function writeStream($path, $resource, Config $config) + { + return $this->upload($path, $resource, $config); + } + + protected function upload($path, $contents, Config $config) + { + $destination = $this->applyPathPrefix($path); + + $options = $this->getOptionsFromConfig($config); + + if (empty($options->getContentType())) { + $options->setContentType(Util::guessMimeType($path, $contents)); + } + + /** + * We manually create the stream to prevent it from closing the resource + * in its destructor. + */ + $stream = GuzzleUtil::streamFor($contents); + $response = $this->client->createBlockBlob( + $this->container, + $destination, + $contents, + $options + ); + + $stream->detach(); + + return [ + 'path' => $path, + 'timestamp' => (int) $response->getLastModified()->getTimestamp(), + 'dirname' => Util::dirname($path), + 'type' => 'file', + ]; + } + + public function update($path, $contents, Config $config) + { + return $this->upload($path, $contents, $config) + compact('contents'); + } + + public function updateStream($path, $resource, Config $config) + { + return $this->upload($path, $resource, $config); + } + + public function rename($path, $newpath) + { + return $this->copy($path, $newpath) && $this->delete($path); + } + + public function copy($path, $newpath) + { + $source = $this->applyPathPrefix($path); + $destination = $this->applyPathPrefix($newpath); + $this->client->copyBlob($this->container, $destination, $this->container, $source); + + return true; + } + + public function delete($path) + { + try { + $this->client->deleteBlob($this->container, $this->applyPathPrefix($path)); + } catch (ServiceException $exception) { + if ($exception->getCode() !== 404) { + throw $exception; + } + } + + return true; + } + + public function deleteDir($dirname) + { + $prefix = $this->applyPathPrefix($dirname); + $options = new ListBlobsOptions(); + $options->setPrefix($prefix . '/'); + $listResults = $this->client->listBlobs($this->container, $options); + foreach ($listResults->getBlobs() as $blob) { + $this->client->deleteBlob($this->container, $blob->getName()); + } + + return true; + } + + public function createDir($dirname, Config $config) + { + return ['path' => $dirname, 'type' => 'dir']; + } + + public function has($path) + { + return $this->getMetadata($path); + } + + public function read($path) + { + $response = $this->readStream($path); + + if ( ! isset($response['stream']) || ! is_resource($response['stream'])) { + return $response; + } + + $response['contents'] = stream_get_contents($response['stream']); + unset($response['stream']); + + return $response; + } + + public function readStream($path) + { + $location = $this->applyPathPrefix($path); + + try { + $response = $this->client->getBlob( + $this->container, + $location + ); + + return $this->normalizeBlobProperties( + $path, + $response->getProperties() + ) + ['stream' => $response->getContentStream()]; + } catch (ServiceException $exception) { + if ($exception->getCode() !== 404) { + throw $exception; + } + + return false; + } + } + + public function listContents($directory = '', $recursive = false) + { + $result = []; + $location = $this->applyPathPrefix($directory); + + if (strlen($location) > 0) { + $location = rtrim($location, '/') . '/'; + } + + $options = new ListBlobsOptions(); + $options->setPrefix($location); + $options->setMaxResults($this->maxResultsForContentsListing); + + if ( ! $recursive) { + $options->setDelimiter('/'); + } + + list_contents: + $response = $this->client->listBlobs($this->container, $options); + $continuationToken = $response->getContinuationToken(); + foreach ($response->getBlobs() as $blob) { + $name = $blob->getName(); + + if ($location === '' || strpos($name, $location) === 0) { + $result[] = $this->normalizeBlobProperties($name, $blob->getProperties()); + } + } + + if ( ! $recursive) { + $result = array_merge($result, array_map([$this, 'normalizeBlobPrefix'], $response->getBlobPrefixes())); + } + + if ($continuationToken instanceof ContinuationToken) { + $options->setContinuationToken($continuationToken); + goto list_contents; + } + + return Util::emulateDirectories($result); + } + + public function getMetadata($path) + { + $path = $this->applyPathPrefix($path); + + try { + return $this->normalizeBlobProperties( + $path, + $this->client->getBlobProperties($this->container, $path)->getProperties() + ); + } catch (ServiceException $exception) { + if ($exception->getCode() !== 404) { + throw $exception; + } + + return false; + } + } + + public function getSize($path) + { + return $this->getMetadata($path); + } + + public function getMimetype($path) + { + return $this->getMetadata($path); + } + + public function getTimestamp($path) + { + return $this->getMetadata($path); + } + + protected function getOptionsFromConfig(Config $config) + { + $options = $config->get('blobOptions', new CreateBlockBlobOptions()); + foreach (static::$metaOptions as $option) { + if ( ! $config->has($option)) { + continue; + } + call_user_func([$options, "set$option"], $config->get($option)); + } + if ($mimetype = $config->get('mimetype')) { + $options->setContentType($mimetype); + } + + return $options; + } + + protected function normalizeBlobProperties($path, BlobProperties $properties) + { + $path = $this->removePathPrefix($path); + + if (substr($path, -1) === '/') { + return ['type' => 'dir', 'path' => rtrim($path, '/')]; + } + + return [ + 'path' => $path, + 'timestamp' => (int) $properties->getLastModified()->format('U'), + 'dirname' => Util::dirname($path), + 'mimetype' => $properties->getContentType(), + 'size' => $properties->getContentLength(), + 'type' => 'file', + ]; + } + + /** + * @param int $numberOfResults + */ + public function setMaxResultsForContentsListing($numberOfResults) + { + $this->maxResultsForContentsListing = $numberOfResults; + } + + protected function normalizeBlobPrefix(BlobPrefix $blobPrefix) + { + return ['type' => 'dir', 'path' => $this->removePathPrefix(rtrim($blobPrefix->getName(), '/'))]; + } +} diff --git a/src/Flysystem/Adapter/AzureBlobStorageAdapter.php b/src/Flysystem/Adapter/AzureBlobStorageAdapter.php new file mode 100644 index 0000000..bacde2a --- /dev/null +++ b/src/Flysystem/Adapter/AzureBlobStorageAdapter.php @@ -0,0 +1,29 @@ + 'dir', + 'path' => $path, + ]; + } + return $metadata; + } + +} diff --git a/src/Flysystem/Azure.php b/src/Flysystem/Azure.php index d793c59..1fc9feb 100644 --- a/src/Flysystem/Azure.php +++ b/src/Flysystem/Azure.php @@ -5,15 +5,18 @@ namespace Drupal\helfi_azure_fs\Flysystem; use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\Plugin\ContainerFactoryPluginInterface; +use Drupal\flysystem\Plugin\FlysystemPluginInterface; use Drupal\Core\File\FileUrlGeneratorInterface; -use Drupal\flysystem_azure\Flysystem\Adapter\AzureBlobStorageAdapter; -use Drupal\flysystem_azure\Flysystem\Azure as AzureBase; +use Drupal\helfi_azure_fs\Flysystem\Adapter\AzureBlobStorageAdapter; +use Drupal\flysystem\Plugin\FlysystemUrlTrait; use MicrosoftAzure\Storage\Blob\BlobRestProxy; use MicrosoftAzure\Storage\Blob\Internal\BlobResources; use MicrosoftAzure\Storage\Common\Internal\Authentication\SharedAccessSignatureAuthScheme; use MicrosoftAzure\Storage\Common\Internal\Authentication\SharedKeyAuthScheme; use MicrosoftAzure\Storage\Common\Internal\Middlewares\CommonRequestMiddleware; use MicrosoftAzure\Storage\Common\Internal\Resources; +use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; /** @@ -23,7 +26,39 @@ * * @Adapter(id = "helfi_azure") */ -final class Azure extends AzureBase { +final class Azure implements FlysystemPluginInterface, ContainerFactoryPluginInterface { + + use FlysystemUrlTrait { + getExternalUrl as getDownloadUrl; + } + + /** + * Is public. + * + * @var bool + */ + protected bool $isPublic = TRUE; + + /** + * Plugin configuration. + * + * @var array + */ + protected array $configuration; + + /** + * The Client proxy. + * + * @var \MicrosoftAzure\Storage\Blob\BlobRestProxy + */ + protected BlobRestProxy $client; + + /** + * The logger. + * + * @var \Psr\Log\LoggerInterface + */ + protected LoggerInterface $logger; /** * The file url generator service. @@ -39,11 +74,22 @@ final class Azure extends AzureBase { */ private array $externalUrls = []; + /** + * Constructs an Azure object. + * + * @param array $configuration + * Plugin configuration array. + */ + public function __construct(array $configuration) { + $this->configuration = $configuration; + } + /** * {@inheritdoc} */ - public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : self { - $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition); + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition): self { + $instance = new static($configuration); + $instance->logger = $container->get('logger.factory')->get('flysystem_azure'); $instance->fileUrlGenerator = $container->get('file_url_generator'); return $instance; } @@ -139,4 +185,30 @@ public function getExternalUrl($uri): string { $this->calculateUrlPrefix(), UrlHelper::encodePath($target)); } + /** + * {@inheritdoc} + */ + public function ensure($force = FALSE): array { + try { + $this->getAdapter()->listContents(); + } + catch (\Exception $e) { + $this->logger->error($e->getMessage()); + } + + return []; + } + + /** + * Calculates the URL prefix. + * + * @return string + * The URL prefix in the form + * protocol://[name].blob.[endpointSuffix]/[container]. + */ + protected function calculateUrlPrefix(): string { + return $this->configuration['protocol'] . '://' . $this->configuration['name'] . '.blob.' . + $this->configuration['endpointSuffix'] . '/' . $this->configuration['container']; + } + } From 31ab472a75bb87a987446e05516d0af75ad4204f Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Tue, 24 Oct 2023 07:57:52 +0300 Subject: [PATCH 3/8] UHF-9113: Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 166f2d9..e06ba72 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ $schemes = [ ]; $config['helfi_azure_fs.settings']['use_blob_storage'] = TRUE; $settings['flysystem'] = $schemes; +$settings['is_azure'] = TRUE; ``` The correct values can be found by running `printenv | grep AZURE_BLOB_STORAGE` inside a OpenShift Drupal pod. From 5293097acd3356beabbec4fdbe399b5d8a1a347c Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Tue, 24 Oct 2023 09:12:38 +0300 Subject: [PATCH 4/8] UHF-9113: Fix phpcs errors --- src/AzureBlobStorageAdapter.php | 8 +++++--- src/Flysystem/Azure.php | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/AzureBlobStorageAdapter.php b/src/AzureBlobStorageAdapter.php index e19fdb5..0b655dd 100644 --- a/src/AzureBlobStorageAdapter.php +++ b/src/AzureBlobStorageAdapter.php @@ -2,15 +2,18 @@ /** * @file - * AzureBlobStorageAdapter.php from league/flysystem-azure-blob-storage:1.0.0 - * with small changes to make the dependencies play nicely with Drupal 10. + * AzureBlobStorageAdapter.php from league/flysystem-azure-blob-storage:1.0.0. + * Modified to make this play nicely with Drupal 10 dependencies. * * @license MIT * @license https://github.com/thephpleague/flysystem-azure-blob-storage/blob/1.0.0/LICENSE */ +// phpcs:ignoreFile + namespace Drupal\helfi_azure_fs; +use GuzzleHttp\Psr7\Utils as GuzzleUtil; use League\Flysystem\Adapter\AbstractAdapter; use League\Flysystem\Adapter\Polyfill\NotSupportingVisibilityTrait; use League\Flysystem\Config; @@ -22,7 +25,6 @@ use MicrosoftAzure\Storage\Blob\Models\ListBlobsOptions; use MicrosoftAzure\Storage\Common\Exceptions\ServiceException; use MicrosoftAzure\Storage\Common\Models\ContinuationToken; -use GuzzleHttp\Psr7\Utils as GuzzleUtil; use function array_merge; use function compact; diff --git a/src/Flysystem/Azure.php b/src/Flysystem/Azure.php index 1fc9feb..17d6571 100644 --- a/src/Flysystem/Azure.php +++ b/src/Flysystem/Azure.php @@ -5,11 +5,11 @@ namespace Drupal\helfi_azure_fs\Flysystem; use Drupal\Component\Utility\UrlHelper; +use Drupal\Core\File\FileUrlGeneratorInterface; use Drupal\Core\Plugin\ContainerFactoryPluginInterface; use Drupal\flysystem\Plugin\FlysystemPluginInterface; -use Drupal\Core\File\FileUrlGeneratorInterface; -use Drupal\helfi_azure_fs\Flysystem\Adapter\AzureBlobStorageAdapter; use Drupal\flysystem\Plugin\FlysystemUrlTrait; +use Drupal\helfi_azure_fs\Flysystem\Adapter\AzureBlobStorageAdapter; use MicrosoftAzure\Storage\Blob\BlobRestProxy; use MicrosoftAzure\Storage\Blob\Internal\BlobResources; use MicrosoftAzure\Storage\Common\Internal\Authentication\SharedAccessSignatureAuthScheme; From 562e9d946bac9e968efdc6d88ac528a656dfd6be Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Fri, 27 Oct 2023 09:44:20 +0300 Subject: [PATCH 5/8] UHF-9113: use constructor property promotion --- src/Flysystem/Azure.php | 41 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/src/Flysystem/Azure.php b/src/Flysystem/Azure.php index 17d6571..7d119da 100644 --- a/src/Flysystem/Azure.php +++ b/src/Flysystem/Azure.php @@ -39,13 +39,6 @@ final class Azure implements FlysystemPluginInterface, ContainerFactoryPluginInt */ protected bool $isPublic = TRUE; - /** - * Plugin configuration. - * - * @var array - */ - protected array $configuration; - /** * The Client proxy. * @@ -53,20 +46,6 @@ final class Azure implements FlysystemPluginInterface, ContainerFactoryPluginInt */ protected BlobRestProxy $client; - /** - * The logger. - * - * @var \Psr\Log\LoggerInterface - */ - protected LoggerInterface $logger; - - /** - * The file url generator service. - * - * @var \Drupal\Core\File\FileUrlGeneratorInterface - */ - private FileUrlGeneratorInterface $fileUrlGenerator; - /** * List of urls already requested, indexed by uri. * @@ -79,19 +58,27 @@ final class Azure implements FlysystemPluginInterface, ContainerFactoryPluginInt * * @param array $configuration * Plugin configuration array. + * @param \Psr\Log\LoggerInterface $logger + * The logger. + * @param \Drupal\Core\File\FileUrlGeneratorInterface $fileUrlGenerator + * The file url generator service. */ - public function __construct(array $configuration) { - $this->configuration = $configuration; + public function __construct( + private array $configuration, + private LoggerInterface $logger, + private FileUrlGeneratorInterface $fileUrlGenerator + ) { } /** * {@inheritdoc} */ public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition): self { - $instance = new static($configuration); - $instance->logger = $container->get('logger.factory')->get('flysystem_azure'); - $instance->fileUrlGenerator = $container->get('file_url_generator'); - return $instance; + return new static( + $configuration, + $container->get('logger.factory')->get('flysystem_azure'), + $container->get('file_url_generator') + ); } /** From 8709bfeee995636e05111e4d8536545fc9cc3d87 Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Fri, 27 Oct 2023 09:51:11 +0300 Subject: [PATCH 6/8] UHF-9113: Update readme.md --- README.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e06ba72..4fa931f 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ ![CI](https://github.com/City-of-Helsinki/drupal-module-helfi-azure-fs/workflows/CI/badge.svg) [![codecov](https://codecov.io/gh/City-of-Helsinki/drupal-module-helfi-azure-fs/branch/main/graph/badge.svg?token=46YWS8J8NN)](https://codecov.io/gh/City-of-Helsinki/drupal-module-helfi-azure-fs) -Provides various fixes to deal with Azure's NFS mount and an integration to Azure's Blob storage service using [flysystem](https://www.drupal.org/project/flysystem) and [flysystem_azure](https://www.drupal.org/project/flysystem_azure) modules. +Provides various fixes to deal with Azure's NFS mount and an integration to Azure's Blob storage service using [flysystem](https://www.drupal.org/project/flysystem). Azure's NFS file mount does not support certain file operations (such as chmod), causing any request that performs them to give a 5xx error, like when trying to generate an image style. @@ -18,18 +18,17 @@ Enable the module. ### Using Azure Blob storage to host all files (optional) -- Enable `flysystem_azure` module: `drush en flysystem_azure` - Populate required environment variables: ``` AZURE_BLOB_STORAGE_CONTAINER: The container name -AZURE_BLOB_STORAGE_KEY: The blob storage secret AZURE_BLOB_STORAGE_NAME: The blob storage name +BLOBSTORAGE_ACCOUNT_KEY: The blob storage secret ``` or if you're using SAS token authentication: ``` -AZURE_BLOB_STORAGE_SAS_TOKEN: The SAS token +BLOBSTORAGE_SAS_TOKEN: The SAS token AZURE_BLOB_STORAGE_NAME: The blob storage name ``` @@ -56,7 +55,7 @@ $settings['flysystem'] = $schemes; $settings['is_azure'] = TRUE; ``` -The correct values can be found by running `printenv | grep AZURE_BLOB_STORAGE` inside a OpenShift Drupal pod. +The correct values can be found by running `printenv | grep BLOB` inside a OpenShift Drupal pod. ## Contact From 21433cd1d436efbcb7d7bf527643736a3602cd72 Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Fri, 27 Oct 2023 10:40:36 +0300 Subject: [PATCH 7/8] UHF-9113: Remove unused variable --- src/Flysystem/Azure.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/Flysystem/Azure.php b/src/Flysystem/Azure.php index 7d119da..bf23fda 100644 --- a/src/Flysystem/Azure.php +++ b/src/Flysystem/Azure.php @@ -32,13 +32,6 @@ final class Azure implements FlysystemPluginInterface, ContainerFactoryPluginInt getExternalUrl as getDownloadUrl; } - /** - * Is public. - * - * @var bool - */ - protected bool $isPublic = TRUE; - /** * The Client proxy. * From b9286ad3c58c07eb472081616bfaab493b024374 Mon Sep 17 00:00:00 2001 From: Santeri Hurnanen Date: Fri, 27 Oct 2023 13:43:03 +0300 Subject: [PATCH 8/8] UHF-9113: Fix tests --- tests/src/Unit/AzureTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/src/Unit/AzureTest.php b/tests/src/Unit/AzureTest.php index 9520b83..476b6d8 100644 --- a/tests/src/Unit/AzureTest.php +++ b/tests/src/Unit/AzureTest.php @@ -76,7 +76,9 @@ public function testGetExternalUrl() : void { * @dataProvider connectionStringData */ public function testGetClient(array $configuration, array $expected) : void { - $azure = new Azure($configuration); + $fileUrlGenerator = $this->prophesize(FileUrlGeneratorInterface::class); + $logger = $this->prophesize(LoggerInterface::class); + $azure = new Azure($configuration, $logger->reveal(), $fileUrlGenerator->reveal()); $client = $azure->getClient(); $this->assertEquals($expected['primaryUri'], (string) $client->getPsrPrimaryUri()); $this->assertEquals($expected['secondaryUri'], (string) $client->getPsrSecondaryUri());