Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the static cache persist for the entire life of the PHP process, to reduce HTTP requests for unmodified metadata #120

Merged
merged 6 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/StaticCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,36 +8,46 @@
use Psr\Http\Message\StreamInterface;
use Tuf\Loader\LoaderInterface;

/**
* Caches all downloaded TUF metadata streams in memory.
*
* Certain Composer commands will completely reset Composer in the middle of the
* process -- for example, the `require` command does it before actually updating
* the installed packages, which will blow away a non-static (instance) cache.
* Making $cache static makes it persist for the lifetime of the PHP process, no
* matter how many times Composer resets itself.
*
* Because this is effectively the single static cache for *every* TUF-protected
* repository, it's internally divided into bins, keyed by the base URL from which
* the TUF metadata is downloaded.
*/
class StaticCache implements LoaderInterface
{
/**
* @var \Psr\Http\Message\StreamInterface[]
*/
private array $cache = [];
private static array $cache = [];

public function __construct(
private readonly LoaderInterface $decorated,
private readonly IOInterface $io,
private readonly string $bin,
) {}

/**
* {@inheritDoc}
*/
public function load(string $locator, int $maxBytes): PromiseInterface
{
if (array_key_exists($locator, $this->cache)) {
$cachedStream = static::$cache[$this->bin][$locator] ?? null;
if ($cachedStream) {
$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;
return static::$cache[$this->bin][$locator] = $stream;
});
}
}
2 changes: 1 addition & 1 deletion src/TufValidatedComposerRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,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 StaticCache($loader, $io);
$loader = new StaticCache($loader, $io, $metadataUrl);
$loader = new SizeCheckingLoader($loader);
$this->updater = new ComposerCompatibleUpdater($loader, $storage);

Expand Down
4 changes: 3 additions & 1 deletion tests/ComposerCommandsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,11 @@ public function testRequireAndRemove(): void
$this->assertStringNotContainsStringIgnoringCase("[TUF] Target 'drupal/core-recommended/10.3.0.0' validated.", $debug);

// Even though we are searching delegated roles for multiple targets, we should see the TUF metadata
// loaded from the static cache.
// loaded from the static cache...
$this->assertStringContainsString("[TUF] Loading '1.package_metadata.json' from static cache.", $debug);
$this->assertStringContainsString("[TUF] Loading '1.package.json' from static cache.", $debug);
// ...which should preclude any "not modified" responses.
$this->assertStringNotContainsString('[304] http://localhost:8080/', $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
Expand Down
2 changes: 1 addition & 1 deletion tests/FunctionalTestBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ protected function setUp(): void

protected function startServer(): void
{
$this->server = new Process([PHP_BINARY, '-S', 'localhost:8080'], static::SERVER_ROOT);
$this->server = new Process([PHP_BINARY, '-S', 'localhost:8080', 'router.php'], static::SERVER_ROOT);
$this->server->start();
$serverStarted = $this->server->waitUntil(function ($outputType, $output): bool {
return str_contains($output, 'Development Server (http://localhost:8080) started');
Expand Down
2 changes: 1 addition & 1 deletion tests/StaticCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function testStaticCache(): void
->with('foo.txt', 60)
->willReturn(Create::promiseFor($mockedStream));

$loader = new StaticCache($decorated, $this->createMock(IOInterface::class));
$loader = new StaticCache($decorated, $this->createMock(IOInterface::class), 'picard');
$stream = $loader->load('foo.txt', 60)->wait();

// We should be at the beginning of the stream.
Expand Down
24 changes: 24 additions & 0 deletions tests/server_root/router.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

$file = __DIR__ . $_SERVER['SCRIPT_NAME'];
Copy link
Collaborator Author

@phenaproxima phenaproxima Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was added to support the functional testing. PHP's built-in server doesn't natively handle If-Modified-Since headers, so I had to implement that here.


if (!file_exists($file)) {
header('HTTP/1.1 404 Not Found');
exit;
}

if (array_key_exists('HTTP_IF_MODIFIED_SINCE', $_SERVER)) {
$modifiedSince = new \DateTimeImmutable($_SERVER['HTTP_IF_MODIFIED_SINCE']);
$modifiedAt = @filemtime($file);
} else {
$modifiedSince = false;
$modifiedAt = false;
}

if ($modifiedSince && $modifiedAt && $modifiedAt <= $modifiedSince->getTimestamp()) {
header('HTTP/1.1 304 Not Modified');
exit;
}

header('HTTP/1.1 200 Found');
echo file_get_contents($file);
Loading