From 6c221c28d63605b227518a0df2841b932ee87dac Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Mon, 6 Jan 2020 22:47:29 +0800 Subject: [PATCH] refactor updater --- app/Http/Controllers/UpdateController.php | 124 ++++++++---------- app/Http/View/Composers/FootComposer.php | 1 - app/Services/PackageManager.php | 29 +--- resources/assets/src/scripts/route.ts | 5 +- resources/assets/src/views/admin/Update.ts | 22 ++++ resources/assets/src/views/admin/Update.vue | 102 -------------- .../assets/tests/views/admin/Update.test.ts | 60 ++------- resources/views/admin/update.twig | 62 ++++----- .../ControllersTest/UpdateControllerTest.php | 28 ++-- tests/ServicesTest/PackageManagerTest.php | 31 ----- 10 files changed, 139 insertions(+), 325 deletions(-) create mode 100644 resources/assets/src/views/admin/Update.ts delete mode 100644 resources/assets/src/views/admin/Update.vue diff --git a/app/Http/Controllers/UpdateController.php b/app/Http/Controllers/UpdateController.php index e48b2536..e6d1ac50 100644 --- a/app/Http/Controllers/UpdateController.php +++ b/app/Http/Controllers/UpdateController.php @@ -3,66 +3,58 @@ namespace App\Http\Controllers; use App\Services\PackageManager; +use Composer\CaBundle\CaBundle; use Composer\Semver\Comparator; use Exception; +use GuzzleHttp\Client; +use GuzzleHttp\Exception\RequestException; use Illuminate\Contracts\Console\Kernel as Artisan; use Illuminate\Filesystem\Filesystem; -use Illuminate\Http\Request; use Illuminate\Support\Arr; use Symfony\Component\Finder\SplFileInfo; class UpdateController extends Controller { - protected $currentVersion; - protected $updateSource; - protected $guzzle; - protected $error; - protected $info = []; + const SPEC = 2; - public function __construct(\GuzzleHttp\Client $guzzle) + public function showUpdatePage(Client $client) { - $this->updateSource = config('app.update_source'); - $this->currentVersion = config('app.version'); - $this->guzzle = $guzzle; + $info = $this->getUpdateInfo($client); + $canUpdate = $this->canUpdate(Arr::get($info, 'info')); + + return view('admin.update', [ + 'info' => [ + 'latest' => Arr::get($info, 'info.latest'), + 'current' => config('app.version'), + ], + 'error' => Arr::get($info, 'error', $canUpdate['reason']), + 'can_update' => $canUpdate['can'], + ]); } - public function showUpdatePage() - { - $info = [ - 'latest' => Arr::get($this->getUpdateInfo(), 'latest'), - 'current' => $this->currentVersion, - ]; - $error = $this->error; - $extra = ['canUpdate' => $this->canUpdate()]; - - return view('admin.update', compact('info', 'error', 'extra')); - } - - public function download(Request $request, PackageManager $package, Filesystem $filesystem) - { - if (!$this->canUpdate()) { - return json([]); + public function download( + PackageManager $package, + 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); } - $path = storage_path('packages/bs_'.$this->info['latest'].'.zip'); - switch ($request->get('action')) { - case 'download': - try { - $package->download($this->info['url'], $path)->extract(base_path()); + $info = $info['info']; + $path = storage_path('packages/bs_'.$info['latest'].'.zip'); + try { + $package->download($info['url'], $path)->extract(base_path()); - // Delete options cache. This allows us to update the version info which is recorded as an option. - $filesystem->delete(storage_path('options.php')); + // Delete options cache. This allows us to update the version. + $filesystem->delete(storage_path('options.php')); - return json(trans('admin.update.complete'), 0); - } catch (Exception $e) { - report($e); + return json(trans('admin.update.complete'), 0); + } catch (Exception $e) { + report($e); - return json($e->getMessage(), 1); - } - case 'progress': - return $package->progress(); - default: - return json(trans('general.illegal-parameters'), 1); + return json($e->getMessage(), 1); } } @@ -87,43 +79,37 @@ class UpdateController extends Controller return view('setup.updates.success'); } - protected function getUpdateInfo() + protected function getUpdateInfo(Client $client) { - $acceptableSpec = 2; - if (app()->runningUnitTests() || !$this->info) { - try { - $json = $this->guzzle->request( - 'GET', - $this->updateSource, - ['verify' => \Composer\CaBundle\CaBundle::getSystemCaRootBundlePath()] - )->getBody(); - $info = json_decode($json, true); - if (Arr::get($info, 'spec') == $acceptableSpec) { - $this->info = $info; - } else { - $this->error = trans('admin.update.errors.spec'); - } - } catch (Exception $e) { - $this->error = $e->getMessage(); + try { + $response = $client->request('GET', config('app.update_source'), [ + 'verify' => CaBundle::getSystemCaRootBundlePath(), + ]); + $info = json_decode($response->getBody(), true); + if (Arr::get($info, 'spec') === self::SPEC) { + return ['ok' => true, 'info' => $info]; + } else { + return ['ok' => false, 'error' => trans('admin.update.errors.spec')]; } + } catch (RequestException $e) { + return ['ok' => false, 'error' => $e->getMessage()]; } - - return $this->info; } - protected function canUpdate() + protected function canUpdate($info = []) { - $this->getUpdateInfo(); - - $php = Arr::get($this->info, 'php'); + $php = Arr::get($info, 'php'); preg_match('/(\d+\.\d+\.\d+)/', PHP_VERSION, $matches); $version = $matches[1]; if (Comparator::lessThan($version, $php)) { - $this->error = trans('admin.update.errors.php', ['version' => $php]); - - return false; + return [ + 'can' => false, + 'reason' => trans('admin.update.errors.php', ['version' => $php]), + ]; } - return Comparator::greaterThan(Arr::get($this->info, 'latest'), $this->currentVersion); + $can = Comparator::greaterThan(Arr::get($info, 'latest'), config('app.version')); + + return ['can' => $can, 'reason' => '']; } } diff --git a/app/Http/View/Composers/FootComposer.php b/app/Http/View/Composers/FootComposer.php index 5ec4ec77..6690f24a 100644 --- a/app/Http/View/Composers/FootComposer.php +++ b/app/Http/View/Composers/FootComposer.php @@ -2,7 +2,6 @@ namespace App\Http\View\Composers; -use App\Models\User; use App\Services\Translations\JavaScript; use App\Services\Webpack; use Illuminate\Contracts\Events\Dispatcher; diff --git a/app/Services/PackageManager.php b/app/Services/PackageManager.php index 227c5bf1..ecae22cc 100644 --- a/app/Services/PackageManager.php +++ b/app/Services/PackageManager.php @@ -4,17 +4,17 @@ declare(strict_types=1); namespace App\Services; -use Cache; use Exception; +use GuzzleHttp\Client; use Illuminate\Filesystem\Filesystem; use ZipArchive; class PackageManager { - protected $guzzle; protected $path; - protected $cacheKey; - protected $onProgress; + + /** @var Client */ + protected $guzzle; /** @var Filesystem */ protected $filesystem; @@ -23,35 +23,28 @@ class PackageManager protected $zipper; public function __construct( - \GuzzleHttp\Client $guzzle, + 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])); - }; } public function download(string $url, string $path, $shasum = null): self { $this->path = $path; - $this->cacheKey = "download_$url"; - Cache::forget($this->cacheKey); try { $this->guzzle->request('GET', $url, [ 'sink' => $path, - 'progress' => $this->onProgress, 'verify' => \Composer\CaBundle\CaBundle::getSystemCaRootBundlePath(), ]); } catch (Exception $e) { throw new Exception(trans('admin.download.errors.download', ['error' => $e->getMessage()])); } - Cache::forget($this->cacheKey); - if (is_string($shasum) && sha1_file($path) != strtolower($shasum)) { + if (is_string($shasum) && sha1_file($path) !== strtolower($shasum)) { $this->filesystem->delete($path); throw new Exception(trans('admin.download.errors.shasum')); } @@ -71,14 +64,4 @@ class PackageManager throw new Exception(trans('admin.download.errors.unzip')); } } - - public function progress(): float - { - $progress = unserialize(Cache::get($this->cacheKey)); - if ($progress['total'] == 0) { - return 0; - } else { - return $progress['done'] / $progress['total']; - } - } } diff --git a/resources/assets/src/scripts/route.ts b/resources/assets/src/scripts/route.ts index 4001b7ba..2994ed15 100644 --- a/resources/assets/src/scripts/route.ts +++ b/resources/assets/src/scripts/route.ts @@ -88,8 +88,9 @@ export default [ }, { path: 'admin/update', - component: () => import('../views/admin/Update.vue'), - el: '#update-button', + module: [ + () => import('../views/admin/Update'), + ], }, { path: 'auth/login', diff --git a/resources/assets/src/views/admin/Update.ts b/resources/assets/src/views/admin/Update.ts new file mode 100644 index 00000000..b8566586 --- /dev/null +++ b/resources/assets/src/views/admin/Update.ts @@ -0,0 +1,22 @@ +import { post, ResponseBody } from '../../scripts/net' +import { showModal } from '../../scripts/notify' +import { trans } from '../../scripts/i18n' + +export default async function handler(event: MouseEvent) { + const button = event.target as HTMLButtonElement + button.disabled = true + + const text = button.textContent + button.innerHTML = ` ${trans('admin.downloading')}` + + const { code, message }: ResponseBody = await post('/admin/update/download') + button.textContent = text + button.disabled = false + await showModal({ mode: 'alert', text: message }) + if (code === 0) { + location.href = '/' + } +} + +const button = document.querySelector('#update') +button?.addEventListener('click', handler) diff --git a/resources/assets/src/views/admin/Update.vue b/resources/assets/src/views/admin/Update.vue deleted file mode 100644 index 6d04e9c4..00000000 --- a/resources/assets/src/views/admin/Update.vue +++ /dev/null @@ -1,102 +0,0 @@ - - - diff --git a/resources/assets/tests/views/admin/Update.test.ts b/resources/assets/tests/views/admin/Update.test.ts index 8651bb7a..3a83bfa4 100644 --- a/resources/assets/tests/views/admin/Update.test.ts +++ b/resources/assets/tests/views/admin/Update.test.ts @@ -1,58 +1,24 @@ -import Vue from 'vue' -import { mount } from '@vue/test-utils' import { flushPromises } from '../../utils' import { showModal } from '@/scripts/notify' -import Update from '@/views/admin/Update.vue' +import { post } from '@/scripts/net' +import handler from '@/views/admin/Update' jest.mock('@/scripts/notify') +jest.mock('@/scripts/net') -afterEach(() => { - window.blessing.extra = { canUpdate: true } -}) -test('button should be disabled if update is unavailable', () => { - window.blessing.extra = { canUpdate: false } - const wrapper = mount(Update) - expect(wrapper.find('button').attributes('disabled')).toBe('disabled') -}) +test('click button', async () => { + post.mockResolvedValueOnce({ code: 1, message: 'failed' }) + .mockResolvedValue({ code: 0, message: 'ok' }) -test('perform update', async () => { - window.$ = jest.fn(() => ({ - modal() {}, - })) - Vue.prototype.$http.post - .mockResolvedValueOnce({ code: 1, message: 'fail' }) - .mockResolvedValue({}) - const wrapper = mount(Update) - const button = wrapper.find('button') + const button = document.createElement('button') + button.addEventListener('click', handler) - button.trigger('click') + const event = new MouseEvent('click') + button.dispatchEvent(event) await flushPromises() - expect(showModal).toBeCalledWith({ - mode: 'alert', - text: 'fail', - type: 'danger', - okButtonType: 'outline-light', - }) + expect(showModal).toBeCalledWith({ mode: 'alert', text: 'failed' }) - button.trigger('click') - jest.runOnlyPendingTimers() + button.dispatchEvent(event) await flushPromises() - expect(Vue.prototype.$http.get).toBeCalledWith( - '/admin/update/download', - { action: 'progress' }, - ) - expect(Vue.prototype.$http.post).toBeCalledWith( - '/admin/update/download', - { action: 'download' }, - ) -}) - -test('polling for querying download progress', async () => { - const wrapper = mount }>(Update) - wrapper.setData({ updating: true }) - await wrapper.vm.polling() - expect(Vue.prototype.$http.get).toBeCalledWith( - '/admin/update/download', - { action: 'progress' }, - ) + expect(showModal).toBeCalledWith({ mode: 'alert', text: 'ok' }) }) diff --git a/resources/views/admin/update.twig b/resources/views/admin/update.twig index 6b461cbe..de62205d 100644 --- a/resources/views/admin/update.twig +++ b/resources/views/admin/update.twig @@ -10,28 +10,28 @@

{{ trans('admin.update.info.title') }}

- {% if extra.canUpdate %} -
- {{ trans('admin.update.info.available') }} -
- - - - - - - - - - - -
{{ trans('admin.update.info.versions.latest') }} - v{{ info.latest }} -
{{ trans('admin.update.info.versions.current') }} - v{{ info.current }} -
+ {% if can_update %} +
+ {{ trans('admin.update.info.available') }} +
+ + + + + + + + + + + +
{{ trans('admin.update.info.versions.latest') }} + v{{ info.latest }} +
{{ trans('admin.update.info.versions.current') }} + v{{ info.current }} +
{% else %} - {% if error is not empty %} + {% if error %}
{{ trans('admin.update.errors.connection', {error: error}) }}
@@ -54,7 +54,15 @@ {% endif %}
- + {% if can_update %} + + {% else %} + + {% endif %}
- {% for text in trans('admin.update.cautions.text')|split('\n') %} -

{{ text }}

- {% endfor %} + {{ trans('admin.update.cautions.text')|nl2br }} {{ trans('admin.update.cautions.link') }} @@ -84,9 +90,3 @@
{% endblock %} - -{% block before_foot %} - -{% endblock %} diff --git a/tests/HttpTest/ControllersTest/UpdateControllerTest.php b/tests/HttpTest/ControllersTest/UpdateControllerTest.php index 2190c1ca..fea90590 100644 --- a/tests/HttpTest/ControllersTest/UpdateControllerTest.php +++ b/tests/HttpTest/ControllersTest/UpdateControllerTest.php @@ -36,8 +36,6 @@ class UpdateControllerTest extends TestCase // Missing `spec` field $this->appendToGuzzleQueue([ new Response(200, [], $this->mockFakeUpdateInfo('8.9.3', ['spec' => 0])), - // Weird. Don't remove the following line, or the tests will fail. - new Response(200, [], $this->mockFakeUpdateInfo('8.9.3', ['php' => '100.0.0'])), ]); $this->get('/admin/update')->assertSee(trans('admin.update.errors.spec')); @@ -59,8 +57,14 @@ class UpdateControllerTest extends TestCase $this->setupGuzzleClientMock(); // Already up-to-date + $this->appendToGuzzleQueue([ + new Response(200, [], $this->mockFakeUpdateInfo('1.2.3')), + ]); $this->getJson('/admin/update/download') - ->assertDontSee(trans('general.illegal-parameters')); + ->assertJson([ + 'code' => 1, + 'message' => trans('admin.update.info.up-to-date'), + ]); // Download $this->appendToGuzzleQueue([ @@ -70,31 +74,17 @@ class UpdateControllerTest extends TestCase $this->mock(PackageManager::class, function ($mock) { $mock->shouldReceive('download')->andThrow(new \Exception('ddd')); }); - $this->getJson('/admin/update/download?action=download') - ->assertJson(['code' => 1]); + $this->getJson('/admin/update/download')->assertJson(['code' => 1]); $this->mock(PackageManager::class, function ($mock) { $mock->shouldReceive('download')->andReturnSelf(); $mock->shouldReceive('extract')->andReturn(true); - $mock->shouldReceive('progress'); }); $this->mock(\Illuminate\Filesystem\Filesystem::class, function ($mock) { $mock->shouldReceive('delete')->with(storage_path('options.php'))->once(); $mock->shouldReceive('exists')->with(storage_path('install.lock'))->andReturn(true); }); - $this->getJson('/admin/update/download?action=download') + $this->getJson('/admin/update/download') ->assertJson(['code' => 0, 'message' => trans('admin.update.complete')]); - - // Get download progress - $this->getJson('/admin/update/download?action=progress') - ->assertSee('0'); - - // Invalid action - $this->appendToGuzzleQueue(200, [], $this->mockFakeUpdateInfo('8.9.3')); - $this->getJson('/admin/update/download?action=no') - ->assertJson([ - 'code' => 1, - 'message' => trans('general.illegal-parameters'), - ]); } public function testUpdate() diff --git a/tests/ServicesTest/PackageManagerTest.php b/tests/ServicesTest/PackageManagerTest.php index c22450c8..270a349d 100644 --- a/tests/ServicesTest/PackageManagerTest.php +++ b/tests/ServicesTest/PackageManagerTest.php @@ -3,7 +3,6 @@ namespace Tests; use App\Services\PackageManager; -use Cache; use Exception; use GuzzleHttp\Client; use GuzzleHttp\Exception\RequestException; @@ -12,7 +11,6 @@ use GuzzleHttp\HandlerStack; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use Illuminate\Filesystem\Filesystem; -use ReflectionClass; use ZipArchive; class PackageManagerTest extends TestCase @@ -74,33 +72,4 @@ class PackageManagerTest extends TestCase $this->expectException(Exception::class); $package->extract('dest'); } - - public function testProgress() - { - $package = resolve(PackageManager::class); - $reflect = new ReflectionClass($package); - $property = $reflect->getProperty('cacheKey'); - $property->setAccessible(true); - $property->setValue($package, 'key'); - - Cache::put('key', serialize(['total' => 0, 'done' => 0])); - $this->assertEquals(0, $package->progress()); - - Cache::put('key', serialize(['total' => 2, 'done' => 1])); - $this->assertEquals(0.5, $package->progress()); - } - - public function testOnProgress() - { - $package = resolve(PackageManager::class); - $reflect = new ReflectionClass($package); - $property = $reflect->getProperty('cacheKey'); - $property->setAccessible(true); - $property->setValue($package, 'key'); - $closure = $reflect->getProperty('onProgress'); - $closure->setAccessible(true); - - Cache::shouldReceive('put')->with('key', serialize(['total' => 5, 'done' => 4])); - call_user_func($closure->getValue($package), 5, 4); - } }