From e5eab1faa467a981c220d766b6c8c83293e74a51 Mon Sep 17 00:00:00 2001 From: Adam Date: Fri, 12 Jul 2024 12:18:26 -0400 Subject: [PATCH] Split out the static cache into its own loader (#118) --- src/Loader.php | 18 +---------- src/StaticCache.php | 43 ++++++++++++++++++++++++++ src/TufValidatedComposerRepository.php | 3 +- tests/ComposerCommandsTest.php | 4 +-- tests/LoaderTest.php | 34 +------------------- tests/StaticCacheTest.php | 39 +++++++++++++++++++++++ 6 files changed, 88 insertions(+), 53 deletions(-) create mode 100644 src/StaticCache.php create mode 100644 tests/StaticCacheTest.php diff --git a/src/Loader.php b/src/Loader.php index 914f644..14243ee 100644 --- a/src/Loader.php +++ b/src/Loader.php @@ -5,7 +5,6 @@ use Composer\Downloader\MaxFileSizeExceededException; use Composer\Downloader\TransportException; use Composer\InstalledVersions; -use Composer\IO\IOInterface; use Composer\Util\HttpDownloader; use GuzzleHttp\Promise\Create; use GuzzleHttp\Promise\PromiseInterface; @@ -19,15 +18,9 @@ */ class Loader implements LoaderInterface { - /** - * @var \Psr\Http\Message\StreamInterface[] - */ - private array $cache = []; - public function __construct( private HttpDownloader $downloader, private ComposerFileStorage $storage, - private IOInterface $io, private string $baseUrl = '' ) {} @@ -37,15 +30,6 @@ public function __construct( public function load(string $locator, int $maxBytes): PromiseInterface { $url = $this->baseUrl . $locator; - if (array_key_exists($url, $this->cache)) { - $this->io->debug("[TUF] Loading $url from static cache."); - - $cachedStream = $this->cache[$url]; - // The underlying stream should always be seekable. - assert($cachedStream->isSeekable()); - $cachedStream->rewind(); - return Create::promiseFor($cachedStream); - } $options = [ // Add 1 to $maxBytes to work around a bug in Composer. @@ -90,7 +74,7 @@ public function load(string $locator, int $maxBytes): PromiseInterface fwrite($content, $response->getBody()); } - $stream = $this->cache[$url] = Utils::streamFor($content); + $stream = Utils::streamFor($content); $stream->rewind(); return Create::promiseFor($stream); } diff --git a/src/StaticCache.php b/src/StaticCache.php new file mode 100644 index 0000000..5c6b4d8 --- /dev/null +++ b/src/StaticCache.php @@ -0,0 +1,43 @@ +cache)) { + $this->io->debug("[TUF] Loading '$locator' from static cache."); + + $cachedStream = $this->cache[$locator]; + // The underlying stream should always be seekable. + assert($cachedStream->isSeekable()); + $cachedStream->rewind(); + return Create::promiseFor($cachedStream); + } + return $this->decorated->load($locator, $maxBytes) + ->then(function (StreamInterface $stream) use ($locator) { + $this->cache[$locator] = $stream; + return $stream; + }); + } +} diff --git a/src/TufValidatedComposerRepository.php b/src/TufValidatedComposerRepository.php index 09e162d..e16bebb 100644 --- a/src/TufValidatedComposerRepository.php +++ b/src/TufValidatedComposerRepository.php @@ -66,7 +66,8 @@ public function __construct(array $repoConfig, IOInterface $io, Config $config, // @todo: Write a custom implementation of FileStorage that stores repo keys to user's global composer cache? $storage = $this->initializeStorage($url, $config); - $loader = new Loader($httpDownloader, $storage, $io, $metadataUrl); + $loader = new Loader($httpDownloader, $storage, $metadataUrl); + $loader = new StaticCache($loader, $io); $loader = new SizeCheckingLoader($loader); $this->updater = new ComposerCompatibleUpdater($loader, $storage); diff --git a/tests/ComposerCommandsTest.php b/tests/ComposerCommandsTest.php index 2522a55..2907e91 100644 --- a/tests/ComposerCommandsTest.php +++ b/tests/ComposerCommandsTest.php @@ -75,8 +75,8 @@ public function testRequireAndRemove(): void // Even though we are searching delegated roles for multiple targets, we should see the TUF metadata // loaded from the static cache. - $this->assertStringContainsString('[TUF] Loading http://localhost:8080/metadata/1.package_metadata.json from static cache.', $debug); - $this->assertStringContainsString('[TUF] Loading http://localhost:8080/metadata/1.package.json from static cache.', $debug); + $this->assertStringContainsString("[TUF] Loading '1.package_metadata.json' from static cache.", $debug); + $this->assertStringContainsString("[TUF] Loading '1.package.json' from static cache.", $debug); // The metadata should actually be *downloaded* no more than twice -- once while the // dependency tree is being solved, and again when the solved dependencies are actually // downloaded (which is done by Composer effectively re-invoking itself, resulting in diff --git a/tests/LoaderTest.php b/tests/LoaderTest.php index d7f42f7..c249173 100644 --- a/tests/LoaderTest.php +++ b/tests/LoaderTest.php @@ -5,7 +5,6 @@ use Composer\Config; use Composer\Downloader\MaxFileSizeExceededException; use Composer\Downloader\TransportException; -use Composer\IO\IOInterface; use Composer\Util\Http\Response; use Composer\Util\HttpDownloader; use DMS\PHPUnitExtensions\ArraySubset\Constraint\ArraySubset; @@ -27,7 +26,6 @@ public function testLoader(): void return new Loader( $downloader, $this->createMock(ComposerFileStorage::class), - $this->createMock(IOInterface::class), '/metadata/' ); }; @@ -111,43 +109,13 @@ public function testNotModifiedResponse(): void ->with($url, $this->mockOptions(1025, $modifiedTime)) ->willReturn($response); - $loader = new Loader($downloader, $storage, $this->createMock(IOInterface::class)); + $loader = new Loader($downloader, $storage); // Since the response has no actual body data, the fact that we get the contents // of the file we wrote here is proof that it was ultimately read from persistent // storage by the loader. $this->assertSame('Some test data.', $loader->load('2.test.json', 1024)->wait()->getContents()); } - public function testStaticCache(): void - { - $response = $this->createMock(Response::class); - $response->expects($this->any()) - ->method('getStatusCode') - ->willReturn(200); - $response->expects($this->any()) - ->method('getBody') - ->willReturn('Truly, this is amazing stuff.'); - - $downloader = $this->createMock(HttpDownloader::class); - $downloader->expects($this->once()) - ->method('get') - ->with('foo.txt', $this->mockOptions(1025)) - ->willReturn($response); - - $loader = new Loader($downloader, $this->createMock(ComposerFileStorage::class), $this->createMock(IOInterface::class)); - $stream = $loader->load('foo.txt', 1024)->wait(); - - // We should be at the beginning of the stream. - $this->assertSame(0, $stream->tell()); - // Skip to the end of the stream, so we can confirm that it is rewound - // when loaded from the static cache. - $stream->seek(0, SEEK_END); - $this->assertGreaterThan(0, $stream->tell()); - - $this->assertSame($stream, $loader->load('foo.txt', 1024)->wait()); - $this->assertSame(0, $stream->tell()); - } - private function mockOptions(int $expectedSize, ?\DateTimeInterface $modifiedTime = null): object { $options = ['max_file_size' => $expectedSize]; diff --git a/tests/StaticCacheTest.php b/tests/StaticCacheTest.php new file mode 100644 index 0000000..6a8593d --- /dev/null +++ b/tests/StaticCacheTest.php @@ -0,0 +1,39 @@ +createMock(LoaderInterface::class); + + $mockedStream = Utils::streamFor('Across the universe'); + $decorated->expects($this->once()) + ->method('load') + ->with('foo.txt', 60) + ->willReturn(Create::promiseFor($mockedStream)); + + $loader = new StaticCache($decorated, $this->createMock(IOInterface::class)); + $stream = $loader->load('foo.txt', 60)->wait(); + + // We should be at the beginning of the stream. + $this->assertSame(0, $stream->tell()); + // Skip to the end of the stream, so we can confirm that it is rewound when loaded from the static cache. + $stream->seek(0, SEEK_END); + $this->assertGreaterThan(0, $stream->tell()); + + $this->assertSame($stream, $loader->load('foo.txt', 60)->wait()); + $this->assertSame(0, $stream->tell()); + } +}