From 57e1ce6cdb4309603c4f8f891253f16efa54b40a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A4nz=20Friederes?= Date: Wed, 16 Sep 2020 10:54:14 +0200 Subject: [PATCH 1/4] Change the file download cache key with the processed URL, implement custom cache key --- src/Composer/Downloader/FileDownloader.php | 18 ++++++++------ src/Composer/Plugin/PreFileDownloadEvent.php | 25 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index 9fa498af8..403a4ac08 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -113,6 +113,10 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $urls[$index] = array( 'base' => $url, 'processed' => $processedUrl, + // we use the complete download url here to avoid conflicting entries + // from different packages, which would potentially allow a given package + // in a third party repo to pre-populate the cache for the same package in + // packagist for example. 'cacheKey' => $this->getCacheKey($package, $processedUrl) ); } @@ -136,6 +140,11 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface if ($eventDispatcher) { $preFileDownloadEvent = new PreFileDownloadEvent(PluginEvents::PRE_FILE_DOWNLOAD, $httpDownloader, $url['processed'], 'package', $package); $eventDispatcher->dispatch($preFileDownloadEvent->getName(), $preFileDownloadEvent); + if ($preFileDownloadEvent->getCustomCacheKey() !== null) { + $url['cacheKey'] = $this->getCacheKey($package, $preFileDownloadEvent->getCustomCacheKey()); + } else if ($preFileDownloadEvent->getProcessedUrl() !== $url['processed']) { + $url['cacheKey'] = $this->getCacheKey($package, $preFileDownloadEvent->getProcessedUrl()); + } $url['processed'] = $preFileDownloadEvent->getProcessedUrl(); } @@ -398,14 +407,9 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface return $url; } - private function getCacheKey(PackageInterface $package, $processedUrl) + private function getCacheKey(PackageInterface $package, $key) { - // we use the complete download url here to avoid conflicting entries - // from different packages, which would potentially allow a given package - // in a third party repo to pre-populate the cache for the same package in - // packagist for example. - $cacheKey = sha1($processedUrl); - + $cacheKey = sha1($key); return $package->getName().'/'.$cacheKey.'.'.$package->getDistType(); } diff --git a/src/Composer/Plugin/PreFileDownloadEvent.php b/src/Composer/Plugin/PreFileDownloadEvent.php index f7b523b94..318558c20 100644 --- a/src/Composer/Plugin/PreFileDownloadEvent.php +++ b/src/Composer/Plugin/PreFileDownloadEvent.php @@ -32,6 +32,11 @@ class PreFileDownloadEvent extends Event */ private $processedUrl; + /** + * @var string + */ + private $customCacheKey; + /** * @var string */ @@ -88,6 +93,26 @@ class PreFileDownloadEvent extends Event $this->processedUrl = $processedUrl; } + /** + * Retrieves a custom package cache key for this download. + * + * @return string + */ + public function getCustomCacheKey() + { + return $this->customCacheKey; + } + + /** + * Sets a custom package cache key for this download. + * + * @param string $customCacheKey New cache key + */ + public function setCustomCacheKey($customCacheKey) + { + $this->customCacheKey = $customCacheKey; + } + /** * Returns the type of this download (package, metadata). * From a1a3e29f520749718a617527e00a4270bb4af7a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A4nz=20Friederes?= Date: Wed, 16 Sep 2020 11:39:34 +0200 Subject: [PATCH 2/4] Fix using inside anonymous function --- src/Composer/Downloader/FileDownloader.php | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index 403a4ac08..3729b1f95 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -106,6 +106,11 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface throw new \InvalidArgumentException('The given package is missing url information'); } + $generateCacheKey = function (PackageInterface $package, $key) { + $cacheKey = sha1($key); + return $package->getName().'/'.$cacheKey.'.'.$package->getDistType(); + }; + $retries = 3; $urls = $package->getDistUrls(); foreach ($urls as $index => $url) { @@ -117,7 +122,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface // from different packages, which would potentially allow a given package // in a third party repo to pre-populate the cache for the same package in // packagist for example. - 'cacheKey' => $this->getCacheKey($package, $processedUrl) + 'cacheKey' => $generateCacheKey($package, $processedUrl) ); } @@ -134,16 +139,16 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $accept = null; $reject = null; - $download = function () use ($io, $output, $httpDownloader, $cache, $eventDispatcher, $package, $fileName, &$urls, &$accept, &$reject) { + $download = function () use ($io, $output, $httpDownloader, $cache, $generateCacheKey, $eventDispatcher, $package, $fileName, &$urls, &$accept, &$reject) { $url = reset($urls); if ($eventDispatcher) { $preFileDownloadEvent = new PreFileDownloadEvent(PluginEvents::PRE_FILE_DOWNLOAD, $httpDownloader, $url['processed'], 'package', $package); $eventDispatcher->dispatch($preFileDownloadEvent->getName(), $preFileDownloadEvent); if ($preFileDownloadEvent->getCustomCacheKey() !== null) { - $url['cacheKey'] = $this->getCacheKey($package, $preFileDownloadEvent->getCustomCacheKey()); + $url['cacheKey'] = $generateCacheKey($package, $preFileDownloadEvent->getCustomCacheKey()); } else if ($preFileDownloadEvent->getProcessedUrl() !== $url['processed']) { - $url['cacheKey'] = $this->getCacheKey($package, $preFileDownloadEvent->getProcessedUrl()); + $url['cacheKey'] = $generateCacheKey($package, $preFileDownloadEvent->getProcessedUrl()); } $url['processed'] = $preFileDownloadEvent->getProcessedUrl(); } @@ -407,12 +412,6 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface return $url; } - private function getCacheKey(PackageInterface $package, $key) - { - $cacheKey = sha1($key); - return $package->getName().'/'.$cacheKey.'.'.$package->getDistType(); - } - /** * {@inheritDoc} * @throws \RuntimeException From 7a49cda9f6d22389b75969baff814bb1ad265998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A4nz=20Friederes?= Date: Wed, 16 Sep 2020 20:13:20 +0200 Subject: [PATCH 3/4] Fix PHPDoc types on new PreFileDownloadEvent property --- src/Composer/Plugin/PreFileDownloadEvent.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Composer/Plugin/PreFileDownloadEvent.php b/src/Composer/Plugin/PreFileDownloadEvent.php index 318558c20..0f83b3178 100644 --- a/src/Composer/Plugin/PreFileDownloadEvent.php +++ b/src/Composer/Plugin/PreFileDownloadEvent.php @@ -33,7 +33,7 @@ class PreFileDownloadEvent extends Event private $processedUrl; /** - * @var string + * @var string|null */ private $customCacheKey; @@ -96,7 +96,7 @@ class PreFileDownloadEvent extends Event /** * Retrieves a custom package cache key for this download. * - * @return string + * @return string|null */ public function getCustomCacheKey() { @@ -106,7 +106,7 @@ class PreFileDownloadEvent extends Event /** * Sets a custom package cache key for this download. * - * @param string $customCacheKey New cache key + * @param string|null $customCacheKey New cache key */ public function setCustomCacheKey($customCacheKey) { From 369c2ff552685b6e65fea7fa2cb96cc54d9c879c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A4nz=20Friederes?= Date: Mon, 5 Oct 2020 20:52:21 +0200 Subject: [PATCH 4/4] Implement PR code style feedback --- src/Composer/Downloader/FileDownloader.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index 3729b1f95..7d44142d7 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -106,8 +106,9 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface throw new \InvalidArgumentException('The given package is missing url information'); } - $generateCacheKey = function (PackageInterface $package, $key) { + $cacheKeyGenerator = function (PackageInterface $package, $key) { $cacheKey = sha1($key); + return $package->getName().'/'.$cacheKey.'.'.$package->getDistType(); }; @@ -122,7 +123,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface // from different packages, which would potentially allow a given package // in a third party repo to pre-populate the cache for the same package in // packagist for example. - 'cacheKey' => $generateCacheKey($package, $processedUrl) + 'cacheKey' => $cacheKeyGenerator($package, $processedUrl) ); } @@ -139,16 +140,16 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface $accept = null; $reject = null; - $download = function () use ($io, $output, $httpDownloader, $cache, $generateCacheKey, $eventDispatcher, $package, $fileName, &$urls, &$accept, &$reject) { + $download = function () use ($io, $output, $httpDownloader, $cache, $cacheKeyGenerator, $eventDispatcher, $package, $fileName, &$urls, &$accept, &$reject) { $url = reset($urls); if ($eventDispatcher) { $preFileDownloadEvent = new PreFileDownloadEvent(PluginEvents::PRE_FILE_DOWNLOAD, $httpDownloader, $url['processed'], 'package', $package); $eventDispatcher->dispatch($preFileDownloadEvent->getName(), $preFileDownloadEvent); if ($preFileDownloadEvent->getCustomCacheKey() !== null) { - $url['cacheKey'] = $generateCacheKey($package, $preFileDownloadEvent->getCustomCacheKey()); + $url['cacheKey'] = $cacheKeyGenerator($package, $preFileDownloadEvent->getCustomCacheKey()); } else if ($preFileDownloadEvent->getProcessedUrl() !== $url['processed']) { - $url['cacheKey'] = $generateCacheKey($package, $preFileDownloadEvent->getProcessedUrl()); + $url['cacheKey'] = $cacheKeyGenerator($package, $preFileDownloadEvent->getProcessedUrl()); } $url['processed'] = $preFileDownloadEvent->getProcessedUrl(); }