From 54894b0c57080a9364308f287c7d478840ec65f1 Mon Sep 17 00:00:00 2001 From: Anatoliy Melnikau Date: Thu, 22 Dec 2022 17:14:22 +0300 Subject: [PATCH] Refactor code --- src/GoogleStorageAdapter.php | 114 ++++++++------- tests/GoogleStorageAdapterTests.php | 207 ++++++---------------------- 2 files changed, 101 insertions(+), 220 deletions(-) diff --git a/src/GoogleStorageAdapter.php b/src/GoogleStorageAdapter.php index d2f8330..4a9a4ac 100755 --- a/src/GoogleStorageAdapter.php +++ b/src/GoogleStorageAdapter.php @@ -16,9 +16,6 @@ class GoogleStorageAdapter implements FilesystemAdapter { - /** - * @const STORAGE_API_URI_DEFAULT - */ public const STORAGE_API_URI_DEFAULT = 'https://storage.googleapis.com'; protected StorageClient $storageClient; @@ -27,8 +24,12 @@ class GoogleStorageAdapter implements FilesystemAdapter protected string $pathSeparator = '/'; protected string $storageApiUri; - public function __construct(StorageClient $storageClient, Bucket $bucket, string $pathPrefix = null, string $storageApiUri = null) - { + public function __construct( + StorageClient $storageClient, + Bucket $bucket, + string $pathPrefix = null, + string $storageApiUri = null + ) { $this->storageClient = $storageClient; $this->bucket = $bucket; @@ -164,9 +165,9 @@ public function write(string $path, string $contents, Config $config): void /** * {@inheritdoc} */ - public function writeStream(string $path, $resource, Config $config): void + public function writeStream(string $path, $contents, Config $config): void { - $this->upload($path, $resource, $config); + $this->upload($path, $contents, $config); } /** @@ -193,14 +194,10 @@ public function updateStream(string $path, $resource, Config $config): array protected function getOptionsFromConfig(Config $config): array { $options = []; - - if ($visibility = $config->get('visibility')) { - $options['predefinedAcl'] = $this->getPredefinedAclForVisibility($visibility); - } else { - // if a file is created without an acl, it isn't accessible via the console - // we therefore default to private - $options['predefinedAcl'] = $this->getPredefinedAclForVisibility(Visibility::PRIVATE); - } + // if a file is created without an acl, it isn't accessible via the console + // we therefore default to private + $visibility = $config->get('visibility') ?: Visibility::PRIVATE; + $options['predefinedAcl'] = $this->getPredefinedAclForVisibility($visibility); if ($metadata = $config->get('metadata')) { $options['metadata'] = $metadata; @@ -262,7 +259,7 @@ protected function normaliseObject(StorageObject $object): array 'dirname' => $this->dirname($name), 'path' => $name, 'timestamp' => strtotime($info['updated']), - 'mimetype' => isset($info['contentType']) ? $info['contentType'] : '', + 'mimetype' => $info['contentType'] ?? '', 'size' => $info['size'], ]; } @@ -274,7 +271,7 @@ public function move(string $source, string $destination, Config $config): void { try { $this->copy($source, $destination, $config); - } catch (UnableToCopyFile $exception){ + } catch (UnableToCopyFile $exception) { $this->delete($source); } } @@ -282,19 +279,19 @@ public function move(string $source, string $destination, Config $config): void /** * {@inheritdoc} */ - public function copy(string $path, string $newpath, Config $config): void + public function copy(string $source, string $destination, Config $config): void { - $newpath = $this->applyPathPrefix($newpath); + $destination = $this->applyPathPrefix($destination); // we want the new file to have the same visibility as the original file - $visibility = $this->getRawVisibility($path); + $visibility = $this->getRawVisibility($source); $options = [ - 'name' => $newpath, + 'name' => $destination, 'predefinedAcl' => $this->getPredefinedAclForVisibility($visibility), ]; - if (!$this->getObject($path)->copy($this->bucket, $options)->exists()) { - throw UnableToCopyFile::fromLocationTo($path, $newpath); + if (!$this->getObject($source)->copy($this->bucket, $options)->exists()) { + throw UnableToCopyFile::fromLocationTo($source, $destination); } } @@ -317,14 +314,14 @@ public function deleteDir(string $dirname): void $this->deleteDirectory($dirname); } - public function deleteDirectory(string $dirname): void + public function deleteDirectory(string $path): void { - $dirname = $this->normaliseDirName($dirname); - $objects = $this->listContents($dirname, true); + $path = $this->normalizeDirPostfix($path); + $objects = $this->listContents($path, true); // We first delete the file, so that we can delete // the empty folder at the end. - uasort($objects, function ($a, $b) { + uasort($objects, static function (array $a, array $b): int { return $b['type'] === 'file' ? 1 : -1; }); @@ -333,10 +330,10 @@ public function deleteDirectory(string $dirname): void foreach ($objects as $object) { // normalise directories path if ($object['type'] === 'dir') { - $object['path'] = $this->normaliseDirName($object['path']); + $object['path'] = $this->normalizeDirPostfix($object['path']); } - if (strpos($object['path'], $dirname) !== false) { + if (strpos($object['path'], $path) !== false) { $filtered_objects[] = $object; } } @@ -364,28 +361,15 @@ public function createDir(string $dirname, Config $config): array { @trigger_error(sprintf('Method "%s:createDir()" id deprecated. Use "%1$s:createDirectory()"', __CLASS__), \E_USER_DEPRECATED); - return $this->upload($this->normaliseDirName($dirname), '', $config); + return $this->upload($this->normalizeDirPostfix($dirname), '', $config); } /** * {@inheritdoc} */ - public function createDirectory(string $dirname, Config $config): void - { - $this->upload($this->normaliseDirName($dirname), '', $config); - } - - /** - * Returns a normalised directory name from the given path. - */ - protected function normaliseDirName(string $dirname): string - { - return rtrim($dirname, '/') . '/'; - } - - protected function normalizeDirname(string $dirname): string + public function createDirectory(string $path, Config $config): void { - return $dirname === '.' ? '' : $dirname; + $this->upload($this->normalizeDirPostfix($path), '', $config); } /** @@ -434,11 +418,11 @@ public function readStream(string $path) /** * {@inheritdoc} */ - public function listContents(string $directory = '', bool $recursive = false): iterable + public function listContents(string $path = '', bool $deep = false): iterable { - $directory = $this->applyPathPrefix($directory); + $path = $this->applyPathPrefix($path); - $objects = $this->bucket->objects(['prefix' => $directory]); + $objects = $this->bucket->objects(['prefix' => $path]); $normalised = []; foreach ($objects as $object) { @@ -612,7 +596,7 @@ public function getTemporaryUrl(string $path, $expiration, array $options = []): $signedUrl = $object->signedUrl($expiration, $options); if ($this->getStorageApiUri() !== self::STORAGE_API_URI_DEFAULT) { - list($url, $params) = explode('?', $signedUrl, 2); + [, $params] = explode('?', $signedUrl, 2); $signedUrl = $this->getUrl($path) . '?' . $params; } @@ -638,12 +622,12 @@ protected function basename(string $path): string // coverage is not reported. // Handle relative paths with drive letters. c:file.txt. - while (preg_match('#^[a-zA-Z]{1}:[^\\\/]#', $basename)) { + while (preg_match('#^[a-zA-Z]:[^\\\/]#', $basename)) { $basename = substr($basename, 2); } // Remove colon for standalone drive letter names. - if (preg_match('#^[a-zA-Z]{1}:$#', $basename)) { + if (preg_match('#^[a-zA-Z]:$#', $basename)) { $basename = rtrim($basename, ':'); } @@ -656,7 +640,7 @@ protected function basename(string $path): string */ protected function dirname(string $path): string { - return $this->normalizeDirname(dirname($path)); + return $this->normalizeDotName(dirname($path)); } /** @@ -674,7 +658,7 @@ protected function emulateDirectories(array $listing): array $directories = array_diff(array_unique($directories), array_unique($listedDirectories)); foreach ($directories as $directory) { - $listing[] = $this->pathinfo($directory) + ['type' => 'dir']; + $listing[] = $this->getPathInfo($directory) + ['type' => 'dir']; } return $listing; @@ -689,13 +673,13 @@ protected function emulateObjectDirectories(array $object, array $directories, a $listedDirectories[] = $object['path']; } - if ( ! isset($object['dirname']) || trim($object['dirname']) === '') { + if (!isset($object['dirname']) || trim($object['dirname']) === '') { return [$directories, $listedDirectories]; } $parent = $object['dirname']; - while (isset($parent) && trim($parent) !== '' && ! \in_array($parent, $directories, true)) { + while ($parent && trim($parent) !== '' && !\in_array($parent, $directories, true)) { $directories[] = $parent; $parent = $this->dirname($parent); } @@ -713,6 +697,7 @@ protected function getRawVisibility(string $path): string { try { $acl = $this->getObject($path)->acl()->get(['entity' => 'allUsers']); + return $acl['role'] === Acl::ROLE_READER ? Visibility::PUBLIC : Visibility::PRIVATE; } catch (NotFoundException $e) { // object may not have an acl entry, so handle that gracefully @@ -726,6 +711,7 @@ protected function getRawVisibility(string $path): string protected function getObject(string $path): StorageObject { $path = $this->applyPathPrefix($path); + return $this->bucket->object($path); } @@ -737,18 +723,30 @@ protected function getPredefinedAclForVisibility(string $visibility): string /** * The method grabbed from class \League\Flysystem\Util of league/flysystem:dev-1.0.x. */ - protected function pathinfo(string $path): array + protected function getPathInfo(string $path): array { $pathinfo = compact('path'); if ('' !== $dirname = dirname($path)) { - $pathinfo['dirname'] = $this->normalizeDirname($dirname); + $pathinfo['dirname'] = $this->normalizeDotName($dirname); } $pathinfo['basename'] = $this->basename($path); - $pathinfo += pathinfo($pathinfo['basename']); return $pathinfo + ['dirname' => '']; } + + /** + * Returns a normalised directory name from the given path. + */ + protected function normalizeDirPostfix(string $dirname): string + { + return rtrim($dirname, '/') . '/'; + } + + protected function normalizeDotName(string $dirname): string + { + return $dirname === '.' ? '' : $dirname; + } } diff --git a/tests/GoogleStorageAdapterTests.php b/tests/GoogleStorageAdapterTests.php index d76dc16..c726c5c 100755 --- a/tests/GoogleStorageAdapterTests.php +++ b/tests/GoogleStorageAdapterTests.php @@ -32,8 +32,15 @@ public function testGetBucket(): void $this->assertSame($bucket, $adapter->getBucket()); } - public function testWrite(): void - { + /** + * @dataProvider getDataForTestWriteContent + */ + public function testWriteContent( + array $expected, + string $contents, + string $predefinedAcl, + ?string $visibility + ): void { $bucket = $this->createMock(Bucket::class); $storageObject = $this->createMock(StorageObject::class); @@ -54,10 +61,10 @@ public function testWrite(): void ->expects($this->once()) ->method('upload') ->with( - 'This is the file contents.', + $contents, [ 'name' => 'prefix/file1.txt', - 'predefinedAcl' => 'projectPrivate', + 'predefinedAcl' => $predefinedAcl, ], ) ->willReturn($storageObject); @@ -72,122 +79,13 @@ public function testWrite(): void $adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix'); - $adapter->write('file1.txt', 'This is the file contents.', new Config()); - - $expected = [ - 'type' => 'file', - 'dirname' => '', - 'path' => 'file1.txt', - 'timestamp' => 1474901082, - 'mimetype' => 'text/plain', - 'size' => 5, - ]; - $this->assertEquals($expected, $adapter->getMetadata('file1.txt')); - } - - public function testWriteWithPrivateVisibility(): void - { - $bucket = $this->createMock(Bucket::class); - - $storageObject = $this->createMock(StorageObject::class); - $storageObject - ->expects($this->exactly(2)) - ->method('name') - ->willReturn('prefix/file1.txt'); - $storageObject - ->expects($this->exactly(2)) - ->method('info') - ->willReturn([ - 'updated' => '2016-09-26T14:44:42+00:00', - 'contentType' => 'text/plain', - 'size' => 5, - ]); - - $bucket - ->expects($this->once()) - ->method('upload') - ->with( - 'This is the file contents.', - [ - 'name' => 'prefix/file1.txt', - 'predefinedAcl' => 'projectPrivate', - ] - ) - ->willReturn($storageObject); - - $bucket - ->expects($this->once()) - ->method('object') - ->with('prefix/file1.txt', []) - ->willReturn($storageObject); - - $storageClient = $this->createMock(StorageClient::class); - - $adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix'); - - $adapter->write('file1.txt', 'This is the file contents.', new Config(['visibility' => Visibility::PRIVATE])); - - $expected = [ - 'type' => 'file', - 'dirname' => '', - 'path' => 'file1.txt', - 'timestamp' => 1474901082, - 'mimetype' => 'text/plain', - 'size' => 5, - ]; - $this->assertEquals($expected, $adapter->getMetadata('file1.txt')); - } - - public function testWriteWithPublicVisibility(): void - { - $bucket = $this->createMock(Bucket::class); - - $storageObject = $this->createMock(StorageObject::class); - $storageObject - ->expects($this->exactly(2)) - ->method('name') - ->willReturn('prefix/file1.txt'); - $storageObject - ->expects($this->exactly(2)) - ->method('info') - ->willReturn([ - 'updated' => '2016-09-26T14:44:42+00:00', - 'contentType' => 'text/plain', - 'size' => 5, - ]); - - $bucket - ->expects($this->once()) - ->method('upload') - ->with( - 'This is the file contents.', - [ - 'name' => 'prefix/file1.txt', - 'predefinedAcl' => 'publicRead', - ], - ) - ->willReturn($storageObject); + $configOptions = []; + if ($visibility) { + $configOptions['visibility'] = $visibility; + } - $bucket - ->expects($this->once()) - ->method('object') - ->with('prefix/file1.txt', []) - ->willReturn($storageObject); - - $storageClient = $this->createMock(StorageClient::class); - - $adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix'); + $adapter->write('file1.txt', 'This is the file contents.', new Config($configOptions)); - $adapter->write('file1.txt', 'This is the file contents.', new Config(['visibility' => Visibility::PUBLIC])); - - $expected = [ - 'type' => 'file', - 'dirname' => '', - 'path' => 'file1.txt', - 'timestamp' => 1474901082, - 'mimetype' => 'text/plain', - 'size' => 5, - ]; $this->assertEquals($expected, $adapter->getMetadata('file1.txt')); } @@ -465,7 +363,10 @@ public function testDelete(): void $adapter->delete('file.txt'); } - public function testDeleteDir(): void + /** + * @dataProvider getDataForTestDeleteDirectory + */ + public function testDeleteDirectory(string $path): void { $storageClient = $this->createMock(StorageClient::class); $bucket = $this->createMock(Bucket::class); @@ -497,56 +398,15 @@ public function testDeleteDir(): void ->expects($this->once()) ->method('objects') ->with([ - 'prefix' => 'prefix/dir_name/' + 'prefix' => 'prefix/dir_name/', ]) ->willReturn([$storageObject]); $adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix'); - $adapter->deleteDirectory('dir_name'); + $adapter->deleteDirectory($path); } - public function testDeleteDirWithTrailingSlash(): void - { - $storageClient = $this->createMock(StorageClient::class); - $bucket = $this->createMock(Bucket::class); - - $storageObject = $this->createMock(StorageObject::class); - $storageObject - ->expects($this->exactly(3)) - ->method('delete'); - - $storageObject - ->expects($this->once()) - ->method('name') - ->willReturn('prefix/dir_name/directory1/file1.txt', []); - $storageObject - ->expects($this->once()) - ->method('info') - ->willReturn([ - 'updated' => '2016-09-26T14:44:42+00:00', - 'contentType' => 'text/plain', - 'size' => 5, - ]); - - $bucket - ->expects($this->exactly(3)) - ->method('object') - ->withConsecutive(['prefix/dir_name/directory1/file1.txt', []], ['prefix/dir_name/directory1/', []], ['prefix/dir_name/', []]) - ->willReturnOnConsecutiveCalls($storageObject, $storageObject, $storageObject); - - $bucket - ->expects($this->once()) - ->method('objects') - ->with([ - 'prefix' => 'prefix/dir_name/' - ]) - ->willReturn([$storageObject]); - - $adapter = new GoogleStorageAdapter($storageClient, $bucket, 'prefix'); - - $adapter->deleteDirectory('dir_name//'); - } public function testSetVisibilityPrivate(): void { $bucket = $this->createMock(Bucket::class); @@ -1112,4 +972,27 @@ public function testGetUrl(): void // no bucket name on custom domain $this->assertEquals('http://my-domain.com/another-prefix/dir/file.txt', $adapter->getUrl('dir/file.txt')); } + + public function getDataForTestDeleteDirectory(): iterable + { + yield ['dir_name']; + yield ['dir_name//']; + } + + public function getDataForTestWriteContent(): iterable + { + $contents = 'This is the file contents.'; + $expected = [ + 'type' => 'file', + 'dirname' => '', + 'path' => 'file1.txt', + 'timestamp' => 1474901082, + 'mimetype' => 'text/plain', + 'size' => 5, + ]; + + yield [$expected, $contents, 'projectPrivate', null]; + yield [$expected, $contents, 'projectPrivate', Visibility::PRIVATE]; + yield [$expected, $contents, 'publicRead', Visibility::PUBLIC]; + } }