diff --git a/app/Services/PackageManager.php b/app/Services/PackageManager.php index bb4c73a7..497637de 100644 --- a/app/Services/PackageManager.php +++ b/app/Services/PackageManager.php @@ -6,6 +6,8 @@ namespace App\Services; use Cache; use Exception; +use ZipArchive; +use Illuminate\Filesystem\Filesystem; class PackageManager { @@ -14,9 +16,20 @@ class PackageManager protected $cacheKey; protected $onProgress; - public function __construct(\GuzzleHttp\Client $guzzle) - { + /** @var Filesystem */ + protected $filesystem; + + /** @var ZipArchive */ + protected $zipper; + + public function __construct( + \GuzzleHttp\Client $guzzle, + Filesystem $filesystem, + ZipArchive $zipper + ) { $this->guzzle = $guzzle; + $this->filesystem = $filesystem; + $this->zipper = $zipper; $this->onProgress = function ($total, $done) { Cache::put($this->cacheKey, serialize(['total' => $total, 'done' => $done])); }; @@ -39,7 +52,7 @@ class PackageManager Cache::forget($this->cacheKey); if (is_string($shasum) && sha1_file($path) != strtolower($shasum)) { - @unlink($path); + $this->filesystem->delete($path); throw new Exception(trans('admin.download.errors.shasum')); } @@ -48,12 +61,12 @@ class PackageManager public function extract(string $destination): void { - $zip = new \ZipArchive(); + $zip = $this->zipper; $resource = $zip->open($this->path); if ($resource === true && $zip->extractTo($destination)) { $zip->close(); - @unlink($this->path); + $this->filesystem->delete($this->path); } else { throw new Exception(trans('admin.download.errors.unzip')); } diff --git a/tests/Concerns/FakePackageManager.php b/tests/Concerns/FakePackageManager.php deleted file mode 100644 index 02c3f5fa..00000000 --- a/tests/Concerns/FakePackageManager.php +++ /dev/null @@ -1,37 +0,0 @@ -guzzle = $guzzle; - $this->throw = $throw; - } - - public function download(string $url, string $path, $shasum = null): PackageManager - { - if ($this->throw) { - throw new \Exception(''); - } else { - return $this; - } - } - - public function extract(string $destination): void - { - // - } - - public function progress(): float - { - return 0.0; - } -} diff --git a/tests/ServicesTest/PackageManagerTest.php b/tests/ServicesTest/PackageManagerTest.php index 671f5687..6fc4ace4 100644 --- a/tests/ServicesTest/PackageManagerTest.php +++ b/tests/ServicesTest/PackageManagerTest.php @@ -12,6 +12,7 @@ use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use App\Services\PackageManager; use GuzzleHttp\Handler\MockHandler; +use Illuminate\Filesystem\Filesystem; use GuzzleHttp\Exception\RequestException; class PackageManagerTest extends TestCase @@ -25,8 +26,9 @@ class PackageManagerTest extends TestCase ]); $handler = HandlerStack::create($mock); $client = new Client(['handler' => $handler]); + $this->instance(Client::class, $client); - $package = new PackageManager($client); + $package = resolve(PackageManager::class); $this->assertInstanceOf( PackageManager::class, $package->download('url', storage_path('packages/temp')) @@ -41,26 +43,33 @@ class PackageManagerTest extends TestCase public function testExtract() { - $mock = new MockHandler([new Response(200, [], 'contents')]); - $handler = HandlerStack::create($mock); - $client = new Client(['handler' => $handler]); - $package = new PackageManager($client); - $path = storage_path('packages/temp.zip'); + $this->mock(ZipArchive::class, function ($mock) { + $mock->shouldReceive('open') + ->twice() + ->andReturn(true, false); - $package->download('url', $path); - $zip = new ZipArchive(); - $this->assertTrue($zip->open($path, ZipArchive::OVERWRITE)); - $this->assertTrue($zip->addEmptyDir('zip-test')); - $zip->close(); - $package->extract(storage_path('testing')); + $mock->shouldReceive('extractTo') + ->with('dest') + ->once() + ->andReturn(true); + + $mock->shouldReceive('close')->once(); + }); + $this->mock(Filesystem::class, function ($mock) { + $mock->shouldReceive('delete')->once(); + }); + $package = resolve(PackageManager::class); + + // The call below is expected success. + $package->extract('dest'); $this->expectException(Exception::class); - $package->download('url', $path)->extract(storage_path('testing')); + $package->extract('dest'); } public function testProgress() { - $package = new PackageManager(new Client()); + $package = resolve(PackageManager::class); $reflect = new ReflectionClass($package); $property = $reflect->getProperty('cacheKey'); $property->setAccessible(true); @@ -75,7 +84,7 @@ class PackageManagerTest extends TestCase public function testOnProgress() { - $package = new PackageManager(new Client()); + $package = resolve(PackageManager::class); $reflect = new ReflectionClass($package); $property = $reflect->getProperty('cacheKey'); $property->setAccessible(true); diff --git a/tests/UpdateControllerTest.php b/tests/UpdateControllerTest.php index 711fd195..7ca93004 100644 --- a/tests/UpdateControllerTest.php +++ b/tests/UpdateControllerTest.php @@ -79,10 +79,16 @@ class UpdateControllerTest extends TestCase new Response(200, [], $this->mockFakeUpdateInfo('8.9.3')), new Response(200, [], $this->mockFakeUpdateInfo('8.9.3')), ]); - app()->instance(PackageManager::class, new Concerns\FakePackageManager(null, true)); + $this->mock(PackageManager::class, function ($mock) { + $mock->shouldReceive('download')->andThrow(new \Exception('ddd')); + }); $this->getJson('/admin/update/download?action=download') ->assertJson(['code' => 1]); - app()->bind(PackageManager::class, Concerns\FakePackageManager::class); + $this->mock(PackageManager::class, function ($mock) { + $mock->shouldReceive('download')->andReturnSelf(); + $mock->shouldReceive('extract')->andReturn(true); + $mock->shouldReceive('progress'); + }); $this->getJson('/admin/update/download?action=download') ->assertJson(['code' => 0, 'message' => trans('admin.update.complete')]);