1
0
Fork 0

Make FileDownloader always download file first in vendor-dir/composer/$tmp instead of next to install path to avoid issues with custom installers not being loaded when downloading on first install, and use cleanup method properly

pull/8729/head
Jordi Boggiano 2020-03-28 20:38:50 +01:00
parent 918768fc54
commit 1b1d59ee6c
No known key found for this signature in database
GPG Key ID: 7BBD42C429EC80BC
6 changed files with 70 additions and 32 deletions

View File

@ -91,32 +91,15 @@ abstract class ArchiveDownloader extends FileDownloader
}
$this->filesystem->removeDirectory($temporaryDir);
if ($this->filesystem->isDirEmpty($this->config->get('vendor-dir').'/composer/')) {
$this->filesystem->removeDirectory($this->config->get('vendor-dir').'/composer/');
}
if ($this->filesystem->isDirEmpty($this->config->get('vendor-dir'))) {
$this->filesystem->removeDirectory($this->config->get('vendor-dir'));
}
} catch (\Exception $e) {
// clean up
$this->filesystem->removeDirectory($path);
$this->filesystem->removeDirectory($temporaryDir);
if (file_exists($fileName)) {
$this->filesystem->unlink($fileName);
}
throw $e;
}
}
/**
* {@inheritdoc}
*/
protected function getFileName(PackageInterface $package, $path)
{
return rtrim($path.'_'.md5($path.spl_object_hash($package)).'.'.pathinfo(parse_url($package->getDistUrl(), PHP_URL_PATH), PATHINFO_EXTENSION), '.');
}
/**
* Extract file to directory
*

View File

@ -101,8 +101,9 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface
);
}
$this->filesystem->ensureDirectoryExists($path);
$fileName = $this->getFileName($package, $path);
$this->filesystem->ensureDirectoryExists($path);
$this->filesystem->ensureDirectoryExists(dirname($fileName));
$io = $this->io;
$cache = $this->cache;
@ -234,6 +235,19 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface
*/
public function cleanup($type, PackageInterface $package, $path, PackageInterface $prevPackage = null)
{
$fileName = $this->getFileName($package, $path);
if (file_exists($fileName)) {
$this->filesystem->unlink($fileName);
}
if ($this->filesystem->isDirEmpty($this->config->get('vendor-dir').'/composer/')) {
$this->filesystem->removeDirectory($this->config->get('vendor-dir').'/composer/');
}
if ($this->filesystem->isDirEmpty($this->config->get('vendor-dir'))) {
$this->filesystem->removeDirectory($this->config->get('vendor-dir'));
}
if ($this->filesystem->isDirEmpty($path)) {
$this->filesystem->removeDirectory($path);
}
}
/**
@ -302,7 +316,7 @@ class FileDownloader implements DownloaderInterface, ChangeReportInterface
*/
protected function getFileName(PackageInterface $package, $path)
{
return $path.'_'.pathinfo(parse_url($package->getDistUrl(), PHP_URL_PATH), PATHINFO_BASENAME);
return rtrim($this->config->get('vendor-dir').'/composer/'.md5($package.spl_object_hash($package)).'.'.pathinfo(parse_url($package->getDistUrl(), PHP_URL_PATH), PATHINFO_EXTENSION), '.');
}
/**

View File

@ -16,6 +16,8 @@ use Composer\Test\TestCase;
class ArchiveDownloaderTest extends TestCase
{
protected $config;
public function testGetFileName()
{
$packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock();
@ -28,8 +30,13 @@ class ArchiveDownloaderTest extends TestCase
$method = new \ReflectionMethod($downloader, 'getFileName');
$method->setAccessible(true);
$this->config->expects($this->any())
->method('get')
->with('vendor-dir')
->will($this->returnValue('/vendor'));
$first = $method->invoke($downloader, $packageMock, '/path');
$this->assertRegExp('#/path_[a-z0-9]+\.js#', $first);
$this->assertRegExp('#/vendor/composer/[a-z0-9]+\.js#', $first);
$this->assertSame($first, $method->invoke($downloader, $packageMock, '/path'));
}
@ -158,8 +165,8 @@ class ArchiveDownloaderTest extends TestCase
'Composer\Downloader\ArchiveDownloader',
array(
$io = $this->getMockBuilder('Composer\IO\IOInterface')->getMock(),
$config = $this->getMockBuilder('Composer\Config')->getMock(),
new \Composer\Util\HttpDownloader($io, $config),
$this->config = $this->getMockBuilder('Composer\Config')->getMock(),
new \Composer\Util\HttpDownloader($io, $this->config),
)
);
}

View File

@ -21,11 +21,12 @@ use Composer\Util\Loop;
class FileDownloaderTest extends TestCase
{
private $httpDownloader;
private $config;
protected function getDownloader($io = null, $config = null, $eventDispatcher = null, $cache = null, $httpDownloader = null, $filesystem = null)
{
$io = $io ?: $this->getMockBuilder('Composer\IO\IOInterface')->getMock();
$config = $config ?: $this->getMockBuilder('Composer\Config')->getMock();
$this->config = $config ?: $this->getMockBuilder('Composer\Config')->getMock();
$httpDownloader = $httpDownloader ?: $this->getMockBuilder('Composer\Util\HttpDownloader')->disableOriginalConstructor()->getMock();
$httpDownloader
->expects($this->any())
@ -33,7 +34,7 @@ class FileDownloaderTest extends TestCase
->will($this->returnValue(\React\Promise\resolve(new Response(array('url' => 'http://example.org/'), 200, array(), 'file~'))));
$this->httpDownloader = $httpDownloader;
return new FileDownloader($io, $config, $httpDownloader, $eventDispatcher, $cache, $filesystem);
return new FileDownloader($io, $this->config, $httpDownloader, $eventDispatcher, $cache, $filesystem);
}
/**
@ -54,11 +55,11 @@ class FileDownloaderTest extends TestCase
public function testDownloadToExistingFile()
{
$packageMock = $this->getMockBuilder('Composer\Package\PackageInterface')->getMock();
$packageMock->expects($this->once())
$packageMock->expects($this->any())
->method('getDistUrl')
->will($this->returnValue('url'))
;
$packageMock->expects($this->once())
$packageMock->expects($this->any())
->method('getDistUrls')
->will($this->returnValue(array('url')))
;
@ -93,7 +94,12 @@ class FileDownloaderTest extends TestCase
$method = new \ReflectionMethod($downloader, 'getFileName');
$method->setAccessible(true);
$this->assertEquals('/path_script.js', $method->invoke($downloader, $packageMock, '/path'));
$this->config->expects($this->once())
->method('get')
->with('vendor-dir')
->will($this->returnValue('/vendor'));
$this->assertRegExp('#/vendor/composer/[a-z0-9]+\.js#', $method->invoke($downloader, $packageMock, '/path'));
}
public function testDownloadButFileIsUnsaved()
@ -126,6 +132,12 @@ class FileDownloaderTest extends TestCase
;
$downloader = $this->getDownloader($ioMock);
$this->config->expects($this->once())
->method('get')
->with('vendor-dir')
->will($this->returnValue($path.'/vendor'));
try {
$promise = $downloader->download($packageMock, $path);
$loop = new Loop($this->httpDownloader);
@ -199,8 +211,18 @@ class FileDownloaderTest extends TestCase
$path = $this->getUniqueTmpDirectory();
$downloader = $this->getDownloader(null, null, null, null, null, $filesystem);
// make sure the file expected to be downloaded is on disk already
touch($path.'_script.js');
$this->config->expects($this->any())
->method('get')
->with('vendor-dir')
->will($this->returnValue($path.'/vendor'));
$method = new \ReflectionMethod($downloader, 'getFileName');
$method->setAccessible(true);
$dlFile = $method->invoke($downloader, $packageMock, $path);
mkdir(dirname($dlFile), 0777, true);
touch($dlFile);
try {
$promise = $downloader->download($packageMock, $path);
@ -255,13 +277,25 @@ class FileDownloaderTest extends TestCase
->with($this->stringContains('Downgrading'));
$path = $this->getUniqueTmpDirectory();
touch($path.'_script.js');
$filesystem = $this->getMockBuilder('Composer\Util\Filesystem')->getMock();
$filesystem->expects($this->once())
->method('removeDirectory')
->will($this->returnValue(true));
$downloader = $this->getDownloader($ioMock, null, null, null, null, $filesystem);
// make sure the file expected to be downloaded is on disk already
$this->config->expects($this->any())
->method('get')
->with('vendor-dir')
->will($this->returnValue($path.'/vendor'));
$method = new \ReflectionMethod($downloader, 'getFileName');
$method->setAccessible(true);
$dlFile = $method->invoke($downloader, $newPackage, $path);
mkdir(dirname($dlFile), 0777, true);
touch($dlFile);
$promise = $downloader->download($newPackage, $path, $oldPackage);
$loop = new Loop($this->httpDownloader);
$loop->wait(array($promise));

View File

@ -70,10 +70,10 @@ class XzDownloaderTest extends TestCase
$downloader = new XzDownloader($io, $config, $httpDownloader = new HttpDownloader($io, $this->getMockBuilder('Composer\Config')->getMock()), null, null, null);
try {
$promise = $downloader->download($packageMock, $this->testDir);
$promise = $downloader->download($packageMock, $this->testDir.'/install-path');
$loop = new Loop($httpDownloader);
$loop->wait(array($promise));
$downloader->install($packageMock, $this->testDir);
$downloader->install($packageMock, $this->testDir.'/install-path');
$this->fail('Download of invalid tarball should throw an exception');
} catch (\RuntimeException $e) {

View File

@ -69,7 +69,7 @@ class ZipDownloaderTest extends TestCase
$this->markTestSkipped('zip extension missing');
}
$this->config->expects($this->at(0))
$this->config->expects($this->any())
->method('get')
->with('vendor-dir')
->will($this->returnValue($this->testDir));