From 078fcf47ec80f740869b30ea1de824397c1bf37c Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Tue, 14 Jan 2020 10:58:20 +0800 Subject: [PATCH] refactor downloader --- app/Http/Controllers/MarketController.php | 12 ++- app/Http/Controllers/UpdateController.php | 20 ++--- app/Services/PackageManager.php | 67 ----------------- app/Services/Unzip.php | 37 +++++++++ .../ControllersTest/MarketControllerTest.php | 28 +++---- .../ControllersTest/UpdateControllerTest.php | 12 ++- tests/ServicesTest/PackageManagerTest.php | 75 ------------------- tests/ServicesTest/UnzipTest.php | 38 ++++++++++ 8 files changed, 110 insertions(+), 179 deletions(-) delete mode 100644 app/Services/PackageManager.php create mode 100644 app/Services/Unzip.php delete mode 100644 tests/ServicesTest/PackageManagerTest.php create mode 100644 tests/ServicesTest/UnzipTest.php diff --git a/app/Http/Controllers/MarketController.php b/app/Http/Controllers/MarketController.php index d22b388c..0453e40b 100644 --- a/app/Http/Controllers/MarketController.php +++ b/app/Http/Controllers/MarketController.php @@ -2,9 +2,10 @@ namespace App\Http\Controllers; -use App\Services\PackageManager; use App\Services\Plugin; use App\Services\PluginManager; +use App\Services\Unzip; +use Composer\CaBundle\CaBundle; use Composer\Semver\Comparator; use Exception; use Illuminate\Http\Request; @@ -58,7 +59,7 @@ class MarketController extends Controller return $plugins; } - public function download(Request $request, PluginManager $manager, PackageManager $package) + public function download(Request $request, PluginManager $manager, Unzip $unzip) { $name = $request->get('name'); $metadata = $this->getPluginMetadata($name); @@ -77,12 +78,15 @@ class MarketController extends Controller } $url = $metadata['dist']['url']; - $filename = Arr::last(explode('/', $url)); $pluginsDir = $manager->getPluginsDirs()->first(); $path = storage_path("packages/$name".'_'.$metadata['version'].'.zip'); try { - $package->download($url, $path, $metadata['dist']['shasum'])->extract($pluginsDir); + $this->guzzle->get($url, [ + 'sink' => $path, + 'verify' => CaBundle::getSystemCaRootBundlePath(), + ]); + $unzip->extract($path, $pluginsDir); } catch (Exception $e) { return json($e->getMessage(), 1); } diff --git a/app/Http/Controllers/UpdateController.php b/app/Http/Controllers/UpdateController.php index 955700c1..18db7296 100644 --- a/app/Http/Controllers/UpdateController.php +++ b/app/Http/Controllers/UpdateController.php @@ -2,7 +2,7 @@ namespace App\Http\Controllers; -use App\Services\PackageManager; +use App\Services\Unzip; use Cache; use Composer\CaBundle\CaBundle; use Composer\Semver\Comparator; @@ -33,20 +33,21 @@ class UpdateController extends Controller ]); } - public function download( - PackageManager $package, - Filesystem $filesystem, - Client $client - ) { + public function download(Unzip $unzip, Filesystem $filesystem, Client $client) + { $info = $this->getUpdateInfo($client); if (!$info['ok'] || !$this->canUpdate($info['info'])['can']) { return json(trans('admin.update.info.up-to-date'), 1); } $info = $info['info']; - $path = storage_path('packages/bs_'.$info['latest'].'.zip'); + $path = tempnam(sys_get_temp_dir(), 'bs'); try { - $package->download($info['url'], $path)->extract(base_path()); + $client->get($info['url'], [ + 'sink' => $path, + 'verify' => CaBundle::getSystemCaRootBundlePath(), + ]); + $unzip->extract($path, base_path()); // Delete options cache. This allows us to update the version. $filesystem->delete(storage_path('options.php')); @@ -55,7 +56,7 @@ class UpdateController extends Controller } catch (Exception $e) { report($e); - return json($e->getMessage(), 1); + return json(trans('admin.download.errors.download', ['error' => $e->getMessage()]), 1); } } @@ -88,6 +89,7 @@ class UpdateController extends Controller 'verify' => CaBundle::getSystemCaRootBundlePath(), ]); $info = json_decode($response->getBody(), true); + if (Arr::get($info, 'spec') === self::SPEC) { return ['ok' => true, 'info' => $info]; } else { diff --git a/app/Services/PackageManager.php b/app/Services/PackageManager.php deleted file mode 100644 index ecae22cc..00000000 --- a/app/Services/PackageManager.php +++ /dev/null @@ -1,67 +0,0 @@ -guzzle = $guzzle; - $this->filesystem = $filesystem; - $this->zipper = $zipper; - } - - public function download(string $url, string $path, $shasum = null): self - { - $this->path = $path; - try { - $this->guzzle->request('GET', $url, [ - 'sink' => $path, - 'verify' => \Composer\CaBundle\CaBundle::getSystemCaRootBundlePath(), - ]); - } catch (Exception $e) { - throw new Exception(trans('admin.download.errors.download', ['error' => $e->getMessage()])); - } - - if (is_string($shasum) && sha1_file($path) !== strtolower($shasum)) { - $this->filesystem->delete($path); - throw new Exception(trans('admin.download.errors.shasum')); - } - - return $this; - } - - public function extract(string $destination): void - { - $zip = $this->zipper; - $resource = $zip->open($this->path); - - if ($resource === true && $zip->extractTo($destination)) { - $zip->close(); - $this->filesystem->delete($this->path); - } else { - throw new Exception(trans('admin.download.errors.unzip')); - } - } -} diff --git a/app/Services/Unzip.php b/app/Services/Unzip.php new file mode 100644 index 00000000..fd7a6921 --- /dev/null +++ b/app/Services/Unzip.php @@ -0,0 +1,37 @@ +filesystem = $filesystem; + $this->zipper = $zipper; + } + + public function extract(string $file, string $destination): void + { + $zip = $this->zipper; + $resource = $zip->open($file); + + if ($resource === true && $zip->extractTo($destination)) { + $zip->close(); + $this->filesystem->delete($file); + } else { + throw new Exception(trans('admin.download.errors.unzip')); + } + } +} diff --git a/tests/HttpTest/ControllersTest/MarketControllerTest.php b/tests/HttpTest/ControllersTest/MarketControllerTest.php index d1e49f2d..d84585d9 100644 --- a/tests/HttpTest/ControllersTest/MarketControllerTest.php +++ b/tests/HttpTest/ControllersTest/MarketControllerTest.php @@ -2,9 +2,9 @@ namespace Tests; -use App\Services\PackageManager; use App\Services\Plugin; use App\Services\PluginManager; +use App\Services\Unzip; use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; @@ -64,25 +64,19 @@ class MarketControllerTest extends TestCase 'dist' => ['url' => 'http://nowhere.test/', 'shasum' => 'deadbeef'], ], ]]); - $this->appendToGuzzleQueue([new Response(200, [], $fakeRegistry)]); - $this->mock(PackageManager::class, function ($mock) { - $mock->shouldReceive('download') - ->withArgs(['http://nowhere.test/', storage_path('packages/fake_0.0.0.zip'), 'deadbeef']) - ->once() - ->andThrow(new \Exception()); - }); + $this->appendToGuzzleQueue([ + new Response(200, [], $fakeRegistry), + new Response(404), + ]); $this->postJson('/admin/plugins/market/download', ['name' => 'fake']) ->assertJson(['code' => 1]); - $this->appendToGuzzleQueue([new Response(200, [], $fakeRegistry)]); - $this->mock(PackageManager::class, function ($mock) { - $mock->shouldReceive('download') - ->withArgs(['http://nowhere.test/', storage_path('packages/fake_0.0.0.zip'), 'deadbeef']) - ->once() - ->andReturnSelf(); - $mock->shouldReceive('extract') - ->with(base_path('plugins')) - ->once(); + $this->appendToGuzzleQueue([ + new Response(200, [], $fakeRegistry), + new Response(200), + ]); + $this->mock(Unzip::class, function ($mock) { + $mock->shouldReceive('extract')->once(); }); $this->postJson('/admin/plugins/market/download', ['name' => 'fake']) ->assertJson([ diff --git a/tests/HttpTest/ControllersTest/UpdateControllerTest.php b/tests/HttpTest/ControllersTest/UpdateControllerTest.php index d9bda627..27c7861f 100644 --- a/tests/HttpTest/ControllersTest/UpdateControllerTest.php +++ b/tests/HttpTest/ControllersTest/UpdateControllerTest.php @@ -2,7 +2,7 @@ namespace Tests; -use App\Services\PackageManager; +use App\Services\Unzip; use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; @@ -69,15 +69,13 @@ class UpdateControllerTest extends TestCase // Download $this->appendToGuzzleQueue([ new Response(200, [], $this->mockFakeUpdateInfo('8.9.3')), + new Response(404), new Response(200, [], $this->mockFakeUpdateInfo('8.9.3')), + new Response(200), ]); - $this->mock(PackageManager::class, function ($mock) { - $mock->shouldReceive('download')->andThrow(new \Exception('ddd')); - }); $this->postJson('/admin/update/download')->assertJson(['code' => 1]); - $this->mock(PackageManager::class, function ($mock) { - $mock->shouldReceive('download')->andReturnSelf(); - $mock->shouldReceive('extract')->andReturn(true); + $this->mock(Unzip::class, function ($mock) { + $mock->shouldReceive('extract')->once()->andReturn(); }); $this->mock(\Illuminate\Filesystem\Filesystem::class, function ($mock) { $mock->shouldReceive('delete')->with(storage_path('options.php'))->once(); diff --git a/tests/ServicesTest/PackageManagerTest.php b/tests/ServicesTest/PackageManagerTest.php deleted file mode 100644 index 270a349d..00000000 --- a/tests/ServicesTest/PackageManagerTest.php +++ /dev/null @@ -1,75 +0,0 @@ - $handler]); - $this->instance(Client::class, $client); - - $package = resolve(PackageManager::class); - $this->assertInstanceOf( - PackageManager::class, - $package->download('url', storage_path('packages/temp')) - ); - - $this->expectExceptionMessage(trans('admin.download.errors.download', ['error' => 'error'])); - $package->download('url', storage_path('packages/temp')); - } - - public function testShasumCheck() - { - $mock = new MockHandler([new Response(200, [], 'contents')]); - $handler = HandlerStack::create($mock); - $client = new Client(['handler' => $handler]); - $this->instance(Client::class, $client); - - $package = resolve(PackageManager::class); - $this->expectExceptionMessage(trans('admin.download.errors.shasum')); - $package->download('url', storage_path('packages/temp'), 'deadbeef'); - } - - public function testExtract() - { - $this->mock(ZipArchive::class, function ($mock) { - $mock->shouldReceive('open') - ->twice() - ->andReturn(true, false); - - $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->extract('dest'); - } -} diff --git a/tests/ServicesTest/UnzipTest.php b/tests/ServicesTest/UnzipTest.php new file mode 100644 index 00000000..19bc9baa --- /dev/null +++ b/tests/ServicesTest/UnzipTest.php @@ -0,0 +1,38 @@ +mock(ZipArchive::class, function ($mock) { + $mock->shouldReceive('open') + ->twice() + ->andReturn(true, false); + + $mock->shouldReceive('extractTo') + ->with('dest') + ->once() + ->andReturn(true); + + $mock->shouldReceive('close')->once(); + }); + $this->mock(Filesystem::class, function ($mock) { + $mock->shouldReceive('delete')->once(); + }); + /** @var Unzip */ + $unzip = resolve(Unzip::class); + + // The call below is expected success. + $unzip->extract('f.zip', 'dest'); + + $this->expectException(Exception::class); + $unzip->extract('f.zip', 'dest'); + } +}