Implement static caching to improve performance (#89)

php-tuf · Oct 31, 2023 · 9eab630 · 9eab630
1 parent 8c29f58
commit 9eab630
Show file tree

Hide file tree

Showing 8 changed files with 118 additions and 61 deletions.
diff --git a/src/Loader.php b/src/Loader.php
@@ -4,6 +4,7 @@
 
 use Composer\Downloader\MaxFileSizeExceededException;
 use Composer\Downloader\TransportException;
+use Composer\IO\IOInterface;
 use Composer\Util\HttpDownloader;
 use GuzzleHttp\Psr7\Utils;
 use Psr\Http\Message\StreamInterface;
@@ -16,16 +17,33 @@
  */
 class Loader implements LoaderInterface
 {
-    public function __construct(private HttpDownloader $downloader, private ComposerFileStorage $storage, private string $baseUrl = '')
-    {
-    }
+    /**
+     * @var \Psr\Http\Message\StreamInterface[]
+     */
+    private array $cache = [];
+
+    public function __construct(
+        private HttpDownloader $downloader,
+        private ComposerFileStorage $storage,
+        private IOInterface $io,
+        private string $baseUrl = ''
+    ) {}
 
     /**
      * {@inheritDoc}
      */
     public function load(string $locator, int $maxBytes): StreamInterface
     {
         $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, since it's a string we read into memory.
+            assert($cachedStream->isSeekable());
+            $cachedStream->rewind();
+            return $cachedStream;
+        }
 
         $options = [
             // Add 1 to $maxBytes to work around a bug in Composer.
@@ -52,7 +70,7 @@ public function load(string $locator, int $maxBytes): StreamInterface
             } else {
                 $content = $response->getBody();
             }
-            return Utils::streamFor($content);
+            return $this->cache[$url] = Utils::streamFor($content);
         } catch (TransportException $e) {
             if ($e->getStatusCode() === 404) {
                 throw new RepoFileNotFound("$locator not found");

diff --git a/src/TufValidatedComposerRepository.php b/src/TufValidatedComposerRepository.php
@@ -66,7 +66,7 @@ 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, $metadataUrl);
+            $loader = new Loader($httpDownloader, $storage, $io, $metadataUrl);
             $loader = new SizeCheckingLoader($loader);
             $this->updater = new ComposerCompatibleUpdater($loader, $storage);
 

diff --git a/tests/ComposerCommandsTest.php b/tests/ComposerCommandsTest.php
@@ -15,12 +15,7 @@
  */
 class ComposerCommandsTest extends TestCase
 {
-    /**
-     * The path to the test project.
-     *
-     * @var string
-     */
-    private static $projectDir;
+    private const CLIENT_DIR = __DIR__ . '/client';
 
     /**
      * The built-in PHP server process.
@@ -46,8 +41,9 @@ public static function setUpBeforeClass(): void
         });
         static::assertTrue($serverStarted);
 
-        // Ensure that static::composer() runs in the correct directory.
-        static::$projectDir = __DIR__ . '/client';
+        // Create a backup of composer.json that we can restore at the end of the test.
+        // @see ::tearDownAfterClass()
+        copy(self::CLIENT_DIR . '/composer.json', self::CLIENT_DIR . '/composer.json.orig');
 
         // Create a Composer repository with all the installed vendor
         // dependencies, so that the test project doesn't need to interact
@@ -74,7 +70,7 @@ public static function setUpBeforeClass(): void
                 ];
             }
         }
-        $destination = static::$projectDir . '/vendor.json';
+        $destination = self::CLIENT_DIR . '/vendor.json';
         file_put_contents($destination, json_encode($vendor, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
         static::composer('config', 'repo.vendor', 'composer', 'file://' . $destination);
 
@@ -96,10 +92,14 @@ public static function tearDownAfterClass(): void
 
         // Delete files and directories created during the test.
         $file_system = new Filesystem();
-        foreach (['vendor', 'composer.lock', 'vendor.json'] as $file) {
-            $file_system->remove(static::$projectDir . '/' . $file);
+        foreach (['vendor', 'composer.json', 'composer.lock', 'vendor.json'] as $file) {
+            $file_system->remove(self::CLIENT_DIR . '/' . $file);
         }
 
+        // Create a backup of composer.json that we can restore at the end of the test.
+        // @see ::tearDownAfterClass()
+        rename(self::CLIENT_DIR . '/composer.json.orig', self::CLIENT_DIR . '/composer.json');
+
         parent::tearDownAfterClass();
     }
 
@@ -119,8 +119,9 @@ private static function composer(string ...$command): Process
         // Always run in very, very verbose mode.
         $command[] = '-vvv';
 
-        $process = new Process($command, static::$projectDir);
-        $process->mustRun();
+        $process = (new Process($command))
+            ->setWorkingDirectory(self::CLIENT_DIR)
+            ->mustRun();
         static::assertSame(0, $process->getExitCode());
         // There should not be any deprecation warnings.
         static::assertStringNotContainsStringIgnoringCase('deprecated', $process->getOutput());
@@ -130,46 +131,28 @@ private static function composer(string ...$command): Process
     }
 
     /**
-     * Asserts that a package is installed in the test project.
-     *
-     * @param string $package
-     *   The name of the package (e.g., 'drupal/core').
-     */
-    private function assertPackageInstalled(string $package): void
-    {
-        $this->assertFileExists(static::$projectDir . "/vendor/$package");
-    }
-
-    /**
-     * Asserts that a package is not installed in the test project.
-     *
-     * @param string $package
-     *   The name of the package (e.g., 'drupal/core').
-     */
-    private function assertPackageNotInstalled(string $package): void
-    {
-        $this->assertFileDoesNotExist(static::$projectDir . "/vendor/$package");
-    }
-
-    /**
-     * Tests requiring and removing a TUF-protected package.
+     * Tests requiring and removing a TUF-protected package with a dependency.
      */
     public function testRequireAndRemove(): void
     {
-        $package = 'drupal/token';
-        $this->assertPackageNotInstalled($package);
+        $vendorDir = self::CLIENT_DIR . '/vendor';
+
+        $this->assertDirectoryDoesNotExist("$vendorDir/drupal/token");
+        $this->assertDirectoryDoesNotExist("$vendorDir/drupal/pathauto");
 
         // Run Composer in very, very verbose mode so that we can capture and assert the
         // debugging messages generated by the plugin, which will be logged to STDERR.
-        $debug = $this->composer('require', $package, '--with-all-dependencies', '-vvv')
+        $debug = $this->composer('require', 'drupal/pathauto', '--with-all-dependencies', '-vvv')
             ->getErrorOutput();
         $this->assertStringContainsString('TUF integration enabled.', $debug);
         $this->assertStringContainsString('[TUF] Root metadata for http://localhost:8080/targets loaded from ', $debug);
         $this->assertStringContainsString('[TUF] Packages from http://localhost:8080/targets are verified by TUF.', $debug);
         $this->assertStringContainsString('[TUF] Metadata source: http://localhost:8080/metadata/', $debug);
         $this->assertStringContainsString("[TUF] Target 'packages.json' limited to 120 bytes.", $debug);
         $this->assertStringContainsString("[TUF] Target 'packages.json' validated.", $debug);
-        $this->assertStringContainsString("[TUF] Target 'files/packages/8/p2/drupal/token.json' limited to 1379 bytes.", $debug);
+        $this->assertStringContainsString("[TUF] Target 'files/packages/8/p2/drupal/pathauto.json' limited to 1610 bytes.", $debug);
+        $this->assertStringContainsString("[TUF] Target 'files/packages/8/p2/drupal/pathauto.json' validated.", $debug);
+        $this->assertStringContainsString("[TUF] Target 'files/packages/8/p2/drupal/token.json' limited to 1330 bytes.", $debug);
         $this->assertStringContainsString("[TUF] Target 'files/packages/8/p2/drupal/token.json' validated.", $debug);
         // token~dev.json doesn't exist, so the plugin will limit it to a hard-coded maximum
         // size, and there should not be a message saying that it was validated.
@@ -179,22 +162,48 @@ public function testRequireAndRemove(): void
         // information will be stored in the transport options saved to the lock file.
         $this->assertStringContainsString("[TUF] Target 'drupal/token/1.9.0.0' validated.", $debug);
 
-        $this->assertPackageInstalled($package);
+        // 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);
+        // The metadata should actually be *downloaded* twice -- once while the dependency tree is
+        // being solved by Composer, and again when the solved dependencies are actually downloaded
+        // (which is done by Composer effectively re-invoking itself, which results in the static
+        // cache being reset).
+        // @see \Composer\Command\RequireCommand::doUpdate()
+        $this->assertStringContainsStringCount('Downloading http://localhost:8080/metadata/1.package_metadata.json', $debug, 2);
+        $this->assertStringContainsStringCount('Downloading http://localhost:8080/metadata/1.package.json', $debug, 2);
+
+        $this->assertDirectoryExists("$vendorDir/drupal/token");
+        $this->assertDirectoryExists("$vendorDir/drupal/pathauto");
 
         // Load the locked package to ensure that the TUF information was saved.
         // @see \Tuf\ComposerIntegration\TufValidatedComposerRepository::configurePackageTransportOptions()
-        $lock = static::$projectDir . '/composer.lock';
-        $this->assertFileIsReadable($lock);
-        $lock = new JsonFile($lock);
+        $lock = new JsonFile(self::CLIENT_DIR . '/composer.lock');
+        $this->assertTrue($lock->exists());
         $lock = new FilesystemRepository($lock);
-        $locked_package = $lock->findPackage($package, '*');
-        $this->assertInstanceOf(PackageInterface::class, $locked_package);
-        $transport_options = $locked_package->getTransportOptions();
-        $this->assertSame('http://localhost:8080/targets', $transport_options['tuf']['repository']);
-        $this->assertSame('drupal/token/1.9.0.0', $transport_options['tuf']['target']);
-        $this->assertNotEmpty($transport_options['max_file_size']);
-
-        $this->composer('remove', $package);
-        $this->assertPackageNotInstalled($package);
+
+        $transportOptions = $lock->findPackage('drupal/token', '*')
+            ?->getTransportOptions();
+        $this->assertIsArray($transportOptions);
+        $this->assertSame('http://localhost:8080/targets', $transportOptions['tuf']['repository']);
+        $this->assertSame('drupal/token/1.9.0.0', $transportOptions['tuf']['target']);
+        $this->assertNotEmpty($transportOptions['max_file_size']);
+
+        $transportOptions = $lock->findPackage('drupal/pathauto', '*')
+            ?->getTransportOptions();
+        $this->assertIsArray($transportOptions);
+        $this->assertSame('http://localhost:8080/targets', $transportOptions['tuf']['repository']);
+        $this->assertSame('drupal/pathauto/1.12.0.0', $transportOptions['tuf']['target']);
+        $this->assertNotEmpty($transportOptions['max_file_size']);
+
+        $this->composer('remove', 'drupal/pathauto', 'drupal/token');
+        $this->assertDirectoryDoesNotExist("$vendorDir/drupal/token");
+        $this->assertDirectoryDoesNotExist("$vendorDir/drupal/pathauto");
+    }
+
+    private function assertStringContainsStringCount(string $needle, string $haystack, int $count): void
+    {
+        $this->assertSame($count, substr_count($haystack, $needle), "Failed asserting that '$needle' appears $count time(s) in '$haystack'.");
     }
 }
diff --git a/tests/LoaderTest.php b/tests/LoaderTest.php
@@ -5,6 +5,7 @@
 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 PHPUnit\Framework\TestCase;
@@ -25,8 +26,9 @@ class LoaderTest extends TestCase
     public function testLoader(): void
     {
         $downloader = $this->prophesize(HttpDownloader::class);
+        $io = $this->prophesize(IOInterface::class);
         $storage = $this->prophesize(ComposerFileStorage::class);
-        $loader = new Loader($downloader->reveal(), $storage->reveal(), '/metadata/');
+        $loader = new Loader($downloader->reveal(), $storage->reveal(), $io->reveal(), '/metadata/');
 
         $url = '/metadata/root.json';
         $downloader->get($url, ['max_file_size' => 129])
@@ -103,10 +105,35 @@ public function testNotModifiedResponse(): void
             ->willReturn($response->reveal())
             ->shouldBeCalled();
 
-        $loader = new Loader($downloader->reveal(), $storage);
+        $loader = new Loader($downloader->reveal(), $storage, $this->prophesize(IOInterface::class)->reveal());
         // 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)->getContents());
     }
+
+    public function testStaticCache(): void
+    {
+        $response = $this->prophesize(Response::class);
+        $response->getStatusCode()->willReturn(200);
+        $response->getBody()->willReturn('Truly, this is amazing stuff.');
+
+        $downloader = $this->prophesize(HttpDownloader::class);
+        $downloader->get('foo.txt', ['max_file_size' => 1025])
+            ->willReturn($response->reveal())
+            ->shouldBeCalledOnce();
+
+        $loader = new Loader($downloader->reveal(), $this->prophesize(ComposerFileStorage::class)->reveal(), $this->prophesize(IOInterface::class)->reveal());
+        $stream = $loader->load('foo.txt', 1024);
+
+        // 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));
+        $this->assertSame(0, $stream->tell());
+    }
 }
diff --git a/tests/server/generate.py b/tests/server/generate.py
@@ -56,7 +56,9 @@ def generate_fixture():
     # Add more targets here as needed.
     repository.targets.add_targets(['packages.json'])
     repository.targets('package_metadata').add_target('files/packages/8/p2/drupal/token.json')
+    repository.targets('package_metadata').add_target('files/packages/8/p2/drupal/pathauto.json')
     repository.targets('package').add_target('drupal/token/1.9.0.0')
+    repository.targets('package').add_target('drupal/pathauto/1.12.0.0')
 
     # Write and publish the repository.
     repository.mark_dirty(['snapshot', 'targets', 'timestamp', 'package_metadata', 'package'])

diff --git a/tests/server/targets/drupal/pathauto/1.12.0.0 b/tests/server/targets/drupal/pathauto/1.12.0.0
diff --git a/tests/server/targets/files/packages/8/p2/drupal/pathauto.json b/tests/server/targets/files/packages/8/p2/drupal/pathauto.json
@@ -0,0 +1 @@
+{"packages":{"drupal/pathauto":[{"keywords":"Actively maintained","homepage":"https://www.drupal.org/project/pathauto","version":"1.12.0","version_normalized":"1.12.0.0","license":"GPL-2.0-or-later","authors":[{"name":"Berdir","homepage":"https://www.drupal.org/user/214652"},{"name":"Dave Reid","homepage":"https://www.drupal.org/user/53892"},{"name":"Freso","homepage":"https://www.drupal.org/user/27504"},{"name":"greggles","homepage":"https://www.drupal.org/user/36762"}],"support":{"source":"https://cgit.drupalcode.org/pathauto","issues":"https://www.drupal.org/project/issues/pathauto","documentation":"https://www.drupal.org/docs/8/modules/pathauto"},"source":{"type":"git","url":"https://git.drupalcode.org/project/pathauto.git","reference":"8.x-1.12"},"dist":{"type":"zip","url":"https://ftp.drupal.org/files/projects/pathauto-8.x-1.12.zip","reference":"8.x-1.12","shasum":"b7b6432e315e38e59a7c6cc117134326c580de4c"},"type":"drupal-module","uid":"pathauto-3392484","name":"drupal/pathauto","extra":{"drupal":{"version":"8.x-1.12","datestamp":"1696776683","security-coverage":{"status":"covered","message":"Covered by Drupal's security advisory policy"}},"drush":{"services":{"drush.services.yml":"^9 || ^10"}}},"description":"Provides a mechanism for modules to automatically generate aliases for the content they manage.","require":{"drupal/token":"*"},"suggest":{"drupal/redirect":"When installed Pathauto will provide a new \"Update Action\" in case your URLs change. This is the recommended update action and is considered the best practice for SEO and usability."}}]},"minified":"composer/2.0"}
diff --git a/tests/server/targets/files/packages/8/p2/drupal/token.json b/tests/server/targets/files/packages/8/p2/drupal/token.json
@@ -1 +1 @@
-{"packages":{"drupal\/token":[{"keywords":"Actively maintained,Rules","homepage":"https:\/\/www.drupal.org\/project\/token","version":"1.9.0","version_normalized":"1.9.0.0","license":"GPL-2.0-or-later","authors":[{"name":"Berdir","homepage":"https:\/\/www.drupal.org\/user\/214652"},{"name":"Dave Reid","homepage":"https:\/\/www.drupal.org\/user\/53892"},{"name":"eaton","homepage":"https:\/\/www.drupal.org\/user\/16496"},{"name":"fago","homepage":"https:\/\/www.drupal.org\/user\/16747"},{"name":"greggles","homepage":"https:\/\/www.drupal.org\/user\/36762"},{"name":"mikeryan","homepage":"https:\/\/www.drupal.org\/user\/4420"}],"support":{"source":"https:\/\/git.drupalcode.org\/project\/token"},"source":{"type":"git","url":"https:\/\/git.drupalcode.org\/project\/token.git","reference":"8.x-1.9"},"dist":{"type":"zip","url":"https:\/\/ftp.drupal.org\/files\/projects\/token-8.x-1.9.zip","reference":"8.x-1.9","shasum":"a5d234382a1a0e4ba61d4c7a2fa10671ca656be4"},"type":"drupal-module","uid":"token-3189015","name":"drupal\/token","extra":{"drupal":{"version":"8.x-1.9","datestamp":"1608284866","security-coverage":{"status":"covered","message":"Covered by Drupal\u0027s security advisory policy"}},"drush":{"services":{"drush.services.yml":"^9 || ^10"}}},"description":"Provides a user interface for the Token API, some missing core tokens."}]},"minified":"composer\/2.0"}
+{"packages":{"drupal/token":[{"keywords":"Actively maintained,Rules","homepage":"https://www.drupal.org/project/token","version":"1.9.0","version_normalized":"1.9.0.0","license":"GPL-2.0-or-later","authors":[{"name":"Berdir","homepage":"https://www.drupal.org/user/214652"},{"name":"Dave Reid","homepage":"https://www.drupal.org/user/53892"},{"name":"eaton","homepage":"https://www.drupal.org/user/16496"},{"name":"fago","homepage":"https://www.drupal.org/user/16747"},{"name":"greggles","homepage":"https://www.drupal.org/user/36762"},{"name":"mikeryan","homepage":"https://www.drupal.org/user/4420"}],"support":{"source":"https://git.drupalcode.org/project/token"},"source":{"type":"git","url":"https://git.drupalcode.org/project/token.git","reference":"8.x-1.9"},"dist":{"type":"zip","url":"https://ftp.drupal.org/files/projects/token-8.x-1.9.zip","reference":"8.x-1.9","shasum":"a5d234382a1a0e4ba61d4c7a2fa10671ca656be4"},"type":"drupal-module","uid":"token-3189015","name":"drupal/token","extra":{"drupal":{"version":"8.x-1.9","datestamp":"1608284866","security-coverage":{"status":"covered","message":"Covered by Drupal's security advisory policy"}},"drush":{"services":{"drush.services.yml":"^9 || ^10"}}},"description":"Provides a user interface for the Token API, some missing core tokens."}]},"minified":"composer/2.0"}