From df61d43529418aace241b99be106ff9a35188dac Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 24 Nov 2017 10:41:51 +0100 Subject: [PATCH] Make isUpdateAvailable non-static Signed-off-by: Morris Jobke --- .../lib/Notification/BackgroundJob.php | 9 +++++-- .../tests/Notification/BackgroundJobTest.php | 8 +++++- lib/private/Installer.php | 25 +++++++++---------- lib/private/Updater.php | 2 +- settings/Controller/AppSettingsController.php | 19 ++++++++------ .../Controller/AppSettingsControllerTest.php | 10 +++++++- tests/lib/InstallerTest.php | 3 ++- 7 files changed, 50 insertions(+), 26 deletions(-) diff --git a/apps/updatenotification/lib/Notification/BackgroundJob.php b/apps/updatenotification/lib/Notification/BackgroundJob.php index 08baa35664..3c0cac60cd 100644 --- a/apps/updatenotification/lib/Notification/BackgroundJob.php +++ b/apps/updatenotification/lib/Notification/BackgroundJob.php @@ -53,6 +53,9 @@ class BackgroundJob extends TimedJob { /** @var IClientService */ protected $client; + /** @var Installer */ + protected $installer; + /** @var string[] */ protected $users; @@ -64,8 +67,9 @@ class BackgroundJob extends TimedJob { * @param IGroupManager $groupManager * @param IAppManager $appManager * @param IClientService $client + * @param Installer $installer */ - public function __construct(IConfig $config, IManager $notificationManager, IGroupManager $groupManager, IAppManager $appManager, IClientService $client) { + public function __construct(IConfig $config, IManager $notificationManager, IGroupManager $groupManager, IAppManager $appManager, IClientService $client, Installer $installer) { // Run once a day $this->setInterval(60 * 60 * 24); @@ -74,6 +78,7 @@ class BackgroundJob extends TimedJob { $this->groupManager = $groupManager; $this->appManager = $appManager; $this->client = $client; + $this->installer = $installer; } protected function run($argument) { @@ -257,6 +262,6 @@ class BackgroundJob extends TimedJob { * @return string|false */ protected function isUpdateAvailable($app) { - return Installer::isUpdateAvailable($app, \OC::$server->getAppFetcher()); + return $this->installer->isUpdateAvailable($app); } } diff --git a/apps/updatenotification/tests/Notification/BackgroundJobTest.php b/apps/updatenotification/tests/Notification/BackgroundJobTest.php index 92a8a687f5..0355b10a09 100644 --- a/apps/updatenotification/tests/Notification/BackgroundJobTest.php +++ b/apps/updatenotification/tests/Notification/BackgroundJobTest.php @@ -23,6 +23,7 @@ namespace OCA\UpdateNotification\Tests\Notification; +use OC\Installer; use OCA\UpdateNotification\Notification\BackgroundJob; use OCP\App\IAppManager; use OCP\Http\Client\IClientService; @@ -47,6 +48,8 @@ class BackgroundJobTest extends TestCase { protected $appManager; /** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */ protected $client; + /** @var Installer|\PHPUnit_Framework_MockObject_MockObject */ + protected $installer; public function setUp() { parent::setUp(); @@ -56,6 +59,7 @@ class BackgroundJobTest extends TestCase { $this->groupManager = $this->createMock(IGroupManager::class); $this->appManager = $this->createMock(IAppManager::class); $this->client = $this->createMock(IClientService::class); + $this->installer = $this->createMock(Installer::class); } /** @@ -69,7 +73,8 @@ class BackgroundJobTest extends TestCase { $this->notificationManager, $this->groupManager, $this->appManager, - $this->client + $this->client, + $this->installer ); } { return $this->getMockBuilder(BackgroundJob::class) @@ -79,6 +84,7 @@ class BackgroundJobTest extends TestCase { $this->groupManager, $this->appManager, $this->client, + $this->installer, ]) ->setMethods($methods) ->getMock(); diff --git a/lib/private/Installer.php b/lib/private/Installer.php index cc6725e917..70d6c10b33 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -67,6 +67,10 @@ class Installer { private $logger; /** @var IConfig */ private $config; + /** @var array - for caching the result of app fetcher */ + private $apps = null; + /** @var bool|null - for caching the result of the ready status */ + private $isInstanceReadyForUpdates = null; /** * @param AppFetcher $appFetcher @@ -187,7 +191,7 @@ class Installer { * @return bool */ public function updateAppstoreApp($appId) { - if(self::isUpdateAvailable($appId, $this->appFetcher)) { + if($this->isUpdateAvailable($appId)) { try { $this->downloadApp($appId); } catch (\Exception $e) { @@ -375,29 +379,24 @@ class Installer { * Check if an update for the app is available * * @param string $appId - * @param AppFetcher $appFetcher * @return string|false false or the version number of the update */ - public static function isUpdateAvailable($appId, - AppFetcher $appFetcher) { - static $isInstanceReadyForUpdates = null; - - if ($isInstanceReadyForUpdates === null) { + public function isUpdateAvailable($appId) { + if ($this->isInstanceReadyForUpdates === null) { $installPath = OC_App::getInstallPath(); if ($installPath === false || $installPath === null) { - $isInstanceReadyForUpdates = false; + $this->isInstanceReadyForUpdates = false; } else { - $isInstanceReadyForUpdates = true; + $this->isInstanceReadyForUpdates = true; } } - if ($isInstanceReadyForUpdates === false) { + if ($this->isInstanceReadyForUpdates === false) { return false; } - static $apps = null; - if ($apps === null) { - $apps = $appFetcher->get(); + if ($this->apps === null) { + $apps = $this->appFetcher->get(); } foreach($apps as $app) { diff --git a/lib/private/Updater.php b/lib/private/Updater.php index 5c8d5a2cd4..996163daac 100644 --- a/lib/private/Updater.php +++ b/lib/private/Updater.php @@ -468,7 +468,7 @@ class Updater extends BasicEmitter { foreach($disabledApps as $app) { try { $this->emit('\OC\Updater', 'checkAppStoreAppBefore', [$app]); - if (Installer::isUpdateAvailable($app, \OC::$server->getAppFetcher())) { + if ($this->installer->isUpdateAvailable($app)) { $this->emit('\OC\Updater', 'upgradeAppStoreApp', [$app]); $this->installer->updateAppstoreApp($app); } diff --git a/settings/Controller/AppSettingsController.php b/settings/Controller/AppSettingsController.php index 26858eabcf..f2a92b52f6 100644 --- a/settings/Controller/AppSettingsController.php +++ b/settings/Controller/AppSettingsController.php @@ -37,6 +37,7 @@ use OC\App\AppStore\Fetcher\CategoryFetcher; use OC\App\AppStore\Version\VersionParser; use OC\App\DependencyAnalyzer; use OC\App\Platform; +use OC\Installer; use OCP\App\IAppManager; use \OCP\AppFramework\Controller; use OCP\AppFramework\Http\ContentSecurityPolicy; @@ -74,6 +75,8 @@ class AppSettingsController extends Controller { private $l10nFactory; /** @var BundleFetcher */ private $bundleFetcher; + /** @var Installer */ + private $installer; /** * @param string $appName @@ -86,6 +89,7 @@ class AppSettingsController extends Controller { * @param AppFetcher $appFetcher * @param IFactory $l10nFactory * @param BundleFetcher $bundleFetcher + * @param Installer $installer */ public function __construct($appName, IRequest $request, @@ -96,7 +100,8 @@ class AppSettingsController extends Controller { CategoryFetcher $categoryFetcher, AppFetcher $appFetcher, IFactory $l10nFactory, - BundleFetcher $bundleFetcher) { + BundleFetcher $bundleFetcher, + Installer $installer) { parent::__construct($appName, $request); $this->l10n = $l10n; $this->config = $config; @@ -106,6 +111,7 @@ class AppSettingsController extends Controller { $this->appFetcher = $appFetcher; $this->l10nFactory = $l10nFactory; $this->bundleFetcher = $bundleFetcher; + $this->installer = $installer; } /** @@ -270,8 +276,7 @@ class AppSettingsController extends Controller { ]; - $appFetcher = \OC::$server->getAppFetcher(); - $newVersion = \OC\Installer::isUpdateAvailable($app['id'], $appFetcher); + $newVersion = $this->installer->isUpdateAvailable($app['id']); if($newVersion && $this->appManager->isInstalled($app['id'])) { $formattedApps[count($formattedApps)-1]['update'] = $newVersion; } @@ -284,7 +289,7 @@ class AppSettingsController extends Controller { $appClass = new \OC_App(); $apps = $appClass->listAllApps(); foreach($apps as $key => $app) { - $newVersion = \OC\Installer::isUpdateAvailable($app['id'], $this->appFetcher); + $newVersion = $this->installer->isUpdateAvailable($app['id']); if($newVersion !== false) { $apps[$key]['update'] = $newVersion; } else { @@ -317,7 +322,7 @@ class AppSettingsController extends Controller { $apps = $appClass->listAllApps(); foreach($apps as $key => $app) { - $newVersion = \OC\Installer::isUpdateAvailable($app['id'], $this->appFetcher); + $newVersion = $this->installer->isUpdateAvailable($app['id']); $apps[$key]['update'] = $newVersion; } @@ -342,7 +347,7 @@ class AppSettingsController extends Controller { }); foreach($apps as $key => $app) { - $newVersion = \OC\Installer::isUpdateAvailable($app['id'], $this->appFetcher); + $newVersion = $this->installer->isUpdateAvailable($app['id']); $apps[$key]['update'] = $newVersion; } @@ -363,7 +368,7 @@ class AppSettingsController extends Controller { }); $apps = array_map(function ($app) { - $newVersion = \OC\Installer::isUpdateAvailable($app['id'], $this->appFetcher); + $newVersion = $this->installer->isUpdateAvailable($app['id']); if ($newVersion !== false) { $app['update'] = $newVersion; } diff --git a/tests/Settings/Controller/AppSettingsControllerTest.php b/tests/Settings/Controller/AppSettingsControllerTest.php index e264d0dfbf..6631873d8a 100644 --- a/tests/Settings/Controller/AppSettingsControllerTest.php +++ b/tests/Settings/Controller/AppSettingsControllerTest.php @@ -25,6 +25,7 @@ namespace Tests\Settings\Controller; use OC\App\AppStore\Bundles\BundleFetcher; use OC\App\AppStore\Fetcher\AppFetcher; use OC\App\AppStore\Fetcher\CategoryFetcher; +use OC\Installer; use OC\Settings\Controller\AppSettingsController; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\JSONResponse; @@ -63,6 +64,8 @@ class AppSettingsControllerTest extends TestCase { private $l10nFactory; /** @var BundleFetcher|\PHPUnit_Framework_MockObject_MockObject */ private $bundleFetcher; + /** @var Installer|\PHPUnit_Framework_MockObject_MockObject */ + private $installer; public function setUp() { parent::setUp(); @@ -79,6 +82,7 @@ class AppSettingsControllerTest extends TestCase { $this->appFetcher = $this->createMock(AppFetcher::class); $this->l10nFactory = $this->createMock(IFactory::class); $this->bundleFetcher = $this->createMock(BundleFetcher::class); + $this->installer = $this->createMock(Installer::class); $this->appSettingsController = new AppSettingsController( 'settings', @@ -90,11 +94,15 @@ class AppSettingsControllerTest extends TestCase { $this->categoryFetcher, $this->appFetcher, $this->l10nFactory, - $this->bundleFetcher + $this->bundleFetcher, + $this->installer ); } public function testListCategories() { + $this->installer->expects($this->any()) + ->method('isUpdateAvailable') + ->willReturn(false); $expected = new JSONResponse([ [ 'id' => 2, diff --git a/tests/lib/InstallerTest.php b/tests/lib/InstallerTest.php index 68f3676faa..897bc47210 100644 --- a/tests/lib/InstallerTest.php +++ b/tests/lib/InstallerTest.php @@ -154,7 +154,8 @@ class InstallerTest extends TestCase { ->method('get') ->willReturn($appArray); - $this->assertSame($updateAvailable, Installer::isUpdateAvailable('files', $this->appFetcher)); + $installer = $this->getInstaller(); + $this->assertSame($updateAvailable, $installer->isUpdateAvailable('files')); } /**