From ea14b9ada9ab0e64d47426bc30d65c7a82a2bfc3 Mon Sep 17 00:00:00 2001 From: Arkadiusz Kondas Date: Fri, 17 Jul 2020 10:27:07 +0200 Subject: [PATCH] Move all proxy logic to Proxy class (#223) --- src/Command/ProxySyncReleasesCommand.php | 2 +- src/Controller/Admin/ProxyController.php | 2 +- src/Message/Proxy/RemoveDist.php | 9 ++++- .../Proxy/RemoveDistHandler.php | 10 ++--- src/Service/Dist/Storage.php | 8 ---- src/Service/Dist/Storage/FileStorage.php | 39 ------------------- src/Service/Dist/Storage/InMemoryStorage.php | 11 ------ src/Service/Proxy.php | 11 +++++- src/Service/Proxy/Metadata.php | 6 +-- .../Proxy/RemoveDistHandlerTest.php | 6 +-- .../Service/Dist/Storage/FileStorageTest.php | 31 +++++++++------ tests/Unit/Service/Proxy/MetadataTest.php | 18 +++++++++ tests/Unit/Service/ProxyTest.php | 22 +++++++++++ 13 files changed, 90 insertions(+), 85 deletions(-) create mode 100644 tests/Unit/Service/Proxy/MetadataTest.php diff --git a/src/Command/ProxySyncReleasesCommand.php b/src/Command/ProxySyncReleasesCommand.php index b5b17b6a..f0772707 100644 --- a/src/Command/ProxySyncReleasesCommand.php +++ b/src/Command/ProxySyncReleasesCommand.php @@ -80,7 +80,7 @@ private function syncPackages(\SimpleXMLElement $feed): void list($name, $version) = explode(' ', (string) $item->guid); if (isset($syncedPackages[$name])) { $this->lock->refresh(); - $proxy->downloadByVersion($name, $version); + $proxy->download($name, $version); } } } diff --git a/src/Controller/Admin/ProxyController.php b/src/Controller/Admin/ProxyController.php index b6ec71d8..4cbce81b 100644 --- a/src/Controller/Admin/ProxyController.php +++ b/src/Controller/Admin/ProxyController.php @@ -60,7 +60,7 @@ public function stats(Request $request): Response */ public function remove(string $proxy, string $packageName): Response { - $this->dispatchMessage(new RemoveDist($packageName)); + $this->dispatchMessage(new RemoveDist($proxy, $packageName)); $this->addFlash('success', sprintf('Dist files for package %s will be removed.', $packageName)); diff --git a/src/Message/Proxy/RemoveDist.php b/src/Message/Proxy/RemoveDist.php index c62fafa6..fdefc5b6 100644 --- a/src/Message/Proxy/RemoveDist.php +++ b/src/Message/Proxy/RemoveDist.php @@ -6,13 +6,20 @@ final class RemoveDist { + private string $proxy; private string $packageName; - public function __construct(string $packageName) + public function __construct(string $proxy, string $packageName) { + $this->proxy = $proxy; $this->packageName = $packageName; } + public function proxy(): string + { + return $this->proxy; + } + public function packageName(): string { return $this->packageName; diff --git a/src/MessageHandler/Proxy/RemoveDistHandler.php b/src/MessageHandler/Proxy/RemoveDistHandler.php index ffa29e08..cad9b705 100644 --- a/src/MessageHandler/Proxy/RemoveDistHandler.php +++ b/src/MessageHandler/Proxy/RemoveDistHandler.php @@ -5,20 +5,20 @@ namespace Buddy\Repman\MessageHandler\Proxy; use Buddy\Repman\Message\Proxy\RemoveDist; -use Buddy\Repman\Service\Dist\Storage; +use Buddy\Repman\Service\Proxy\ProxyRegister; use Symfony\Component\Messenger\Handler\MessageHandlerInterface; final class RemoveDistHandler implements MessageHandlerInterface { - private Storage $distStorage; + private ProxyRegister $register; - public function __construct(Storage $distStorage) + public function __construct(ProxyRegister $register) { - $this->distStorage = $distStorage; + $this->register = $register; } public function __invoke(RemoveDist $message): void { - $this->distStorage->remove($message->packageName()); + $this->register->getByHost($message->proxy())->removeDist($message->packageName()); } } diff --git a/src/Service/Dist/Storage.php b/src/Service/Dist/Storage.php index 9f262d29..869cc6d4 100644 --- a/src/Service/Dist/Storage.php +++ b/src/Service/Dist/Storage.php @@ -5,7 +5,6 @@ namespace Buddy\Repman\Service\Dist; use Buddy\Repman\Service\Dist; -use Munus\Collection\GenericList; interface Storage { @@ -19,11 +18,4 @@ public function download(string $url, Dist $dist, array $headers = []): void; public function filename(Dist $dist): string; public function size(Dist $dist): int; - - /** - * @return GenericList - */ - public function packages(string $repo): GenericList; - - public function remove(string $packageName): void; } diff --git a/src/Service/Dist/Storage/FileStorage.php b/src/Service/Dist/Storage/FileStorage.php index e53d4c2b..12552ce2 100644 --- a/src/Service/Dist/Storage/FileStorage.php +++ b/src/Service/Dist/Storage/FileStorage.php @@ -8,9 +8,6 @@ use Buddy\Repman\Service\Dist; use Buddy\Repman\Service\Dist\Storage; use Buddy\Repman\Service\Downloader; -use Munus\Collection\GenericList; -use Symfony\Component\Finder\Finder; -use Symfony\Component\Finder\SplFileInfo; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; final class FileStorage implements Storage @@ -71,42 +68,6 @@ public function size(Dist $dist): int return $size === false ? 0 : $size; } - /** - * @return GenericList - */ - public function packages(string $repo): GenericList - { - $dir = $this->distsDir.'/'.$repo.'/dist'; - if (!is_dir($dir)) { - return GenericList::empty(); - } - - $files = Finder::create()->directories()->sortByName()->depth(1)->ignoreVCS(true)->in($dir); - - return GenericList::ofAll(array_map( - fn (SplFileInfo $fileInfo) => $fileInfo->getRelativePathname(), - iterator_to_array($files->getIterator() - ))); - } - - public function remove(string $packageName): void - { - $dirs = []; - foreach (Finder::create()->directories()->path($packageName)->ignoreVCS(true)->in($this->distsDir) as $dir) { - /* @var SplFileInfo $dir */ - $dirs[] = $dir->getPathname(); - foreach (Finder::create()->files()->in($dir->getPathname()) as $file) { - /* @var SplFileInfo $file */ - @unlink($file->getPathname()); - } - } - - // can't remove dir in Finder loop, RecursiveDirectoryIterator throws error - foreach ($dirs as $dir) { - @rmdir($dir); - } - } - private function ensureDirExist(string $filename): void { $dirname = dirname($filename); diff --git a/src/Service/Dist/Storage/InMemoryStorage.php b/src/Service/Dist/Storage/InMemoryStorage.php index 8a088863..59fecf3d 100644 --- a/src/Service/Dist/Storage/InMemoryStorage.php +++ b/src/Service/Dist/Storage/InMemoryStorage.php @@ -6,7 +6,6 @@ use Buddy\Repman\Service\Dist; use Buddy\Repman\Service\Dist\Storage; -use Munus\Collection\GenericList; /** * @codeCoverageIgnore @@ -47,14 +46,4 @@ public function size(Dist $dist): int { return 0; } - - public function packages(string $repo): GenericList - { - return GenericList::ofAll($this->dists); - } - - public function remove(string $packageName): void - { - // TODO: Implement remove() method. - } } diff --git a/src/Service/Proxy.php b/src/Service/Proxy.php index ab45beda..73dcff9b 100644 --- a/src/Service/Proxy.php +++ b/src/Service/Proxy.php @@ -86,7 +86,7 @@ public function syncedPackages(): GenericList return $packages; } - public function downloadByVersion(string $package, string $version): void + public function download(string $package, string $version): void { $lastDist = null; @@ -101,6 +101,15 @@ public function downloadByVersion(string $package, string $version): void } } + public function removeDist(string $package): void + { + if (mb_strlen($package) === 0) { + throw new \InvalidArgumentException('Empty package name'); + } + + $this->filesystem->deleteDir(sprintf('%s/dist/%s', $this->name, $package)); + } + /** * @return mixed[] */ diff --git a/src/Service/Proxy/Metadata.php b/src/Service/Proxy/Metadata.php index c077d9a3..e344d041 100644 --- a/src/Service/Proxy/Metadata.php +++ b/src/Service/Proxy/Metadata.php @@ -22,11 +22,11 @@ public function __construct(int $timestamp, $stream) $this->stream = $stream; } - public static function fromString(string $string): self + public static function fromString(string $string, string $streamName = 'php://memory'): self { - $stream = fopen('php://memory', 'r+'); + $stream = @fopen($streamName, 'r+'); if ($stream === false) { - throw new \RuntimeException('Failed to open in-memory stream'); + throw new \RuntimeException(sprintf('Failed to open %s stream', $streamName)); } fwrite($stream, $string); rewind($stream); diff --git a/tests/Integration/MessageHandler/Proxy/RemoveDistHandlerTest.php b/tests/Integration/MessageHandler/Proxy/RemoveDistHandlerTest.php index ccff3b03..fa4473ac 100644 --- a/tests/Integration/MessageHandler/Proxy/RemoveDistHandlerTest.php +++ b/tests/Integration/MessageHandler/Proxy/RemoveDistHandlerTest.php @@ -5,7 +5,7 @@ namespace Buddy\Repman\Tests\Integration\MessageHandler\Proxy; use Buddy\Repman\Message\Proxy\RemoveDist; -use Buddy\Repman\Service\Dist\Storage; +use Buddy\Repman\Service\Proxy\ProxyRegister; use Buddy\Repman\Tests\Integration\IntegrationTestCase; use Munus\Collection\GenericList; use Symfony\Component\Messenger\MessageBusInterface; @@ -15,11 +15,11 @@ final class RemoveDistHandlerTest extends IntegrationTestCase public function testRemoveDistByPackageName(): void { $this->container()->get(MessageBusInterface::class)->dispatch( - new RemoveDist('some-vendor/some-name') + new RemoveDist('packagist.org', 'some-vendor/some-name') ); self::assertTrue(GenericList::ofAll(['buddy-works/repman'])->equals( - $this->container()->get(Storage::class)->packages('packagist.org') + $this->container()->get(ProxyRegister::class)->getByHost('packagist.org')->syncedPackages() )); } } diff --git a/tests/Unit/Service/Dist/Storage/FileStorageTest.php b/tests/Unit/Service/Dist/Storage/FileStorageTest.php index 6876853f..a93e6757 100644 --- a/tests/Unit/Service/Dist/Storage/FileStorageTest.php +++ b/tests/Unit/Service/Dist/Storage/FileStorageTest.php @@ -8,9 +8,9 @@ use Buddy\Repman\Service\Dist\Storage\FileStorage; use Buddy\Repman\Service\Downloader; use Buddy\Repman\Tests\Doubles\FakeDownloader; -use Munus\Collection\GenericList; use PHPUnit\Framework\TestCase; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; final class FileStorageTest extends TestCase { @@ -86,23 +86,30 @@ public function testHasPackage(): void ))); } - public function testDistPackageRemove(): void + public function testSize(): void { $this->createTempFile($packagePath = $this->basePath.'/packagist.org/dist/buddy-works/repman/0.1.2.0_f0c896.zip'); - $this->createTempFile($otherPath = $this->basePath.'/packagist.org/dist/buddy-works/other-package/0.1.2.0_f0c896.zip'); - self::assertFileExists($packagePath); - self::assertFileExists($otherPath); - - $this->storage->remove('buddy-works/repman'); - - self::assertFileNotExists($packagePath); - self::assertFileExists($otherPath); + self::assertEquals(7, $this->storage->size(new Dist( + 'packagist.org', + 'buddy-works/repman', + '0.1.2.0', + 'f0c896', + 'zip' + ))); } - public function testReturnEmptyPackagesListWhenDirNotExist(): void + public function testThrowHttpNotFoundExceptionWhenFileNotFound(): void { - self::assertTrue(GenericList::empty()->equals($this->storage->packages('not-exist'))); + $this->expectException(NotFoundHttpException::class); + + $this->storage->download('https://some.domain/packagist.org/dist/not-found', new Dist( + 'packagist.org', + 'buddy-works/repman', + '0.1.2.0', + 'f0c896', + 'zip' + )); } private function createTempFile(string $path): void diff --git a/tests/Unit/Service/Proxy/MetadataTest.php b/tests/Unit/Service/Proxy/MetadataTest.php new file mode 100644 index 00000000..d86c9d91 --- /dev/null +++ b/tests/Unit/Service/Proxy/MetadataTest.php @@ -0,0 +1,18 @@ +expectException(\RuntimeException::class); + + Metadata::fromString('string', 'php://invalid'); + } +} diff --git a/tests/Unit/Service/ProxyTest.php b/tests/Unit/Service/ProxyTest.php index aa6a1f5f..e0ef6017 100644 --- a/tests/Unit/Service/ProxyTest.php +++ b/tests/Unit/Service/ProxyTest.php @@ -42,4 +42,26 @@ public function testDownloadDistWhenNotExists(): void fclose($distribution->get()); unlink($distPath); } + + public function testDistRemove(): void + { + $distDir = __DIR__.'/../../Resources/packagist.org/dist/'; + mkdir($distDir.'vendor/package', 0777, true); + file_put_contents($distDir.'vendor/package/some.zip', 'package-data'); + + $this->proxy->removeDist('vendor/package'); + + self::assertFileNotExists($distDir.'vendor/package/some.zip'); + self::assertDirectoryNotExists($distDir.'vendor/package'); + + // test if remove package that not exist does not cause error + $this->proxy->removeDist('vendor/package'); + } + + public function testPreventRemoveDist(): void + { + $this->expectException(\InvalidArgumentException::class); + + $this->proxy->removeDist(''); + } }