From d01cfde98229c6d1bd14feefd889205027920d14 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 1 Apr 2015 15:37:22 +0200 Subject: [PATCH 1/3] Correctly purge the cache when an app is disabled via cli --- lib/private/app/appmanager.php | 18 +++++++++++++++++- lib/private/server.php | 10 ++++++---- settings/ajax/disableapp.php | 4 ---- settings/ajax/enableapp.php | 3 --- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/private/app/appmanager.php b/lib/private/app/appmanager.php index 1cfa0bce25..c05b2a3b03 100644 --- a/lib/private/app/appmanager.php +++ b/lib/private/app/appmanager.php @@ -24,6 +24,7 @@ namespace OC\App; use OCP\App\IAppManager; use OCP\IAppConfig; +use OCP\ICacheFactory; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserSession; @@ -44,6 +45,9 @@ class AppManager implements IAppManager { */ private $groupManager; + /** @var \OCP\ICacheFactory */ + private $memCacheFactory; + /** * @var string[] $appId => $enabled */ @@ -54,10 +58,11 @@ class AppManager implements IAppManager { * @param \OCP\IAppConfig $appConfig * @param \OCP\IGroupManager $groupManager */ - public function __construct(IUserSession $userSession, IAppConfig $appConfig, IGroupManager $groupManager) { + public function __construct(IUserSession $userSession, IAppConfig $appConfig, IGroupManager $groupManager, ICacheFactory $memCacheFactory) { $this->userSession = $userSession; $this->appConfig = $appConfig; $this->groupManager = $groupManager; + $this->memCacheFactory = $memCacheFactory; } /** @@ -157,6 +162,7 @@ class AppManager implements IAppManager { public function enableApp($appId) { $this->installedAppsCache[$appId] = 'yes'; $this->appConfig->setValue($appId, 'enabled', 'yes'); + $this->clearAppsCache(); } /** @@ -172,6 +178,7 @@ class AppManager implements IAppManager { }, $groups); $this->installedAppsCache[$appId] = json_encode($groupIds); $this->appConfig->setValue($appId, 'enabled', json_encode($groupIds)); + $this->clearAppsCache(); } /** @@ -186,5 +193,14 @@ class AppManager implements IAppManager { } unset($this->installedAppsCache[$appId]); $this->appConfig->setValue($appId, 'enabled', 'no'); + $this->clearAppsCache(); + } + + /** + * Clear the cached list of apps when enabling/disabling an app + */ + protected function clearAppsCache() { + $settingsMemCache = $this->memCacheFactory->create('settings'); + $settingsMemCache->clear('listApps'); } } diff --git a/lib/private/server.php b/lib/private/server.php index 592f8d9a04..8c5169f229 100644 --- a/lib/private/server.php +++ b/lib/private/server.php @@ -309,10 +309,12 @@ class Server extends SimpleContainer implements IServerContainer { return new TempManager(get_temp_dir(), $c->getLogger()); }); $this->registerService('AppManager', function(Server $c) { - $userSession = $c->getUserSession(); - $appConfig = $c->getAppConfig(); - $groupManager = $c->getGroupManager(); - return new \OC\App\AppManager($userSession, $appConfig, $groupManager); + return new \OC\App\AppManager( + $c->getUserSession(), + $c->getAppConfig(), + $c->getGroupManager(), + $c->getMemCacheFactory() + ); }); $this->registerService('DateTimeZone', function(Server $c) { return new DateTimeZone( diff --git a/settings/ajax/disableapp.php b/settings/ajax/disableapp.php index cc02203b4a..f99969d91a 100644 --- a/settings/ajax/disableapp.php +++ b/settings/ajax/disableapp.php @@ -31,9 +31,5 @@ if (!array_key_exists('appid', $_POST)) { $appId = (string)$_POST['appid']; $appId = OC_App::cleanAppId($appId); -// FIXME: Clear the cache - move that into some sane helper method -\OC::$server->getMemCacheFactory()->create('settings')->remove('listApps-0'); -\OC::$server->getMemCacheFactory()->create('settings')->remove('listApps-1'); - OC_App::disable($appId); OC_JSON::success(); diff --git a/settings/ajax/enableapp.php b/settings/ajax/enableapp.php index aa688c4964..b63bce76d9 100644 --- a/settings/ajax/enableapp.php +++ b/settings/ajax/enableapp.php @@ -30,9 +30,6 @@ $groups = isset($_POST['groups']) ? (array)$_POST['groups'] : null; try { OC_App::enable(OC_App::cleanAppId((string)$_POST['appid']), $groups); - // FIXME: Clear the cache - move that into some sane helper method - \OC::$server->getMemCacheFactory()->create('settings')->remove('listApps-0'); - \OC::$server->getMemCacheFactory()->create('settings')->remove('listApps-1'); OC_JSON::success(); } catch (Exception $e) { OC_Log::write('core', $e->getMessage(), OC_Log::ERROR); From aa6573cf542737070a7404f86c98b22349508644 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 1 Apr 2015 17:19:44 +0200 Subject: [PATCH 2/3] Adjust tests and check whether clear() is called --- tests/lib/app.php | 4 +- tests/lib/app/manager.php | 180 +++++++++++++++++--------------------- 2 files changed, 82 insertions(+), 102 deletions(-) diff --git a/tests/lib/app.php b/tests/lib/app.php index 86a407c1a9..3f380f74fd 100644 --- a/tests/lib/app.php +++ b/tests/lib/app.php @@ -470,7 +470,7 @@ class Test_App extends \Test\TestCase { return $appConfig; }); \OC::$server->registerService('AppManager', function (\OC\Server $c) use ($appConfig) { - return new \OC\App\AppManager($c->getUserSession(), $appConfig, $c->getGroupManager()); + return new \OC\App\AppManager($c->getUserSession(), $appConfig, $c->getGroupManager(), $c->getMemCacheFactory()); }); } @@ -482,7 +482,7 @@ class Test_App extends \Test\TestCase { return new \OC\AppConfig(\OC_DB::getConnection()); }); \OC::$server->registerService('AppManager', function (\OC\Server $c) { - return new \OC\App\AppManager($c->getUserSession(), $c->getAppConfig(), $c->getGroupManager()); + return new \OC\App\AppManager($c->getUserSession(), $c->getAppConfig(), $c->getGroupManager(), $c->getMemCacheFactory()); }); // Remove the cache of the mocked apps list with a forceRefresh diff --git a/tests/lib/app/manager.php b/tests/lib/app/manager.php index cb41f73746..6cf7eb3bb6 100644 --- a/tests/lib/app/manager.php +++ b/tests/lib/app/manager.php @@ -54,22 +54,55 @@ class Manager extends \PHPUnit_Framework_TestCase { return $config; } + /** @var \OCP\IUserSession */ + protected $userSession; + + /** @var \OCP\IGroupManager */ + protected $groupManager; + + /** @var \OCP\IAppConfig */ + protected $appConfig; + + /** @var \OCP\ICache */ + protected $cache; + + /** @var \OCP\ICacheFactory */ + protected $cacheFactory; + + /** @var \OCP\App\IAppManager */ + protected $manager; + + protected function setUp() { + parent::setUp(); + + $this->userSession = $this->getMock('\OCP\IUserSession'); + $this->groupManager = $this->getMock('\OCP\IGroupManager'); + $this->appConfig = $this->getAppConfig(); + $this->cacheFactory = $this->getMock('\OCP\ICacheFactory'); + $this->cache = $this->getMock('\OCP\ICache'); + $this->cacheFactory->expects($this->any()) + ->method('create') + ->with('settings') + ->willReturn($this->cache); + $this->manager = new \OC\App\AppManager($this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory); + } + + protected function expectClearCache() { + $this->cache->expects($this->once()) + ->method('clear') + ->with('listApps'); + } + public function testEnableApp() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $manager->enableApp('test'); - $this->assertEquals('yes', $appConfig->getValue('test', 'enabled', 'no')); + $this->expectClearCache(); + $this->manager->enableApp('test'); + $this->assertEquals('yes', $this->appConfig->getValue('test', 'enabled', 'no')); } public function testDisableApp() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $manager->disableApp('test'); - $this->assertEquals('no', $appConfig->getValue('test', 'enabled', 'no')); + $this->expectClearCache(); + $this->manager->disableApp('test'); + $this->assertEquals('no', $this->appConfig->getValue('test', 'enabled', 'no')); } public function testEnableAppForGroups() { @@ -77,151 +110,98 @@ class Manager extends \PHPUnit_Framework_TestCase { new Group('group1', array(), null), new Group('group2', array(), null) ); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $userSession = $this->getMock('\OCP\IUserSession'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $manager->enableAppForGroups('test', $groups); - $this->assertEquals('["group1","group2"]', $appConfig->getValue('test', 'enabled', 'no')); + $this->expectClearCache(); + $this->manager->enableAppForGroups('test', $groups); + $this->assertEquals('["group1","group2"]', $this->appConfig->getValue('test', 'enabled', 'no')); } public function testIsInstalledEnabled() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', 'yes'); - $this->assertTrue($manager->isInstalled('test')); + $this->appConfig->setValue('test', 'enabled', 'yes'); + $this->assertTrue($this->manager->isInstalled('test')); } public function testIsInstalledDisabled() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', 'no'); - $this->assertFalse($manager->isInstalled('test')); + $this->appConfig->setValue('test', 'enabled', 'no'); + $this->assertFalse($this->manager->isInstalled('test')); } public function testIsInstalledEnabledForGroups() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', '["foo"]'); - $this->assertTrue($manager->isInstalled('test')); + $this->appConfig->setValue('test', 'enabled', '["foo"]'); + $this->assertTrue($this->manager->isInstalled('test')); } public function testIsEnabledForUserEnabled() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', 'yes'); + $this->appConfig->setValue('test', 'enabled', 'yes'); $user = new User('user1', null); - $this->assertTrue($manager->isEnabledForUser('test', $user)); + $this->assertTrue($this->manager->isEnabledForUser('test', $user)); } public function testIsEnabledForUserDisabled() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', 'no'); + $this->appConfig->setValue('test', 'enabled', 'no'); $user = new User('user1', null); - $this->assertFalse($manager->isEnabledForUser('test', $user)); + $this->assertFalse($this->manager->isEnabledForUser('test', $user)); } public function testIsEnabledForUserEnabledForGroup() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); $user = new User('user1', null); - - $groupManager->expects($this->once()) + $this->groupManager->expects($this->once()) ->method('getUserGroupIds') ->with($user) ->will($this->returnValue(array('foo', 'bar'))); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', '["foo"]'); - $this->assertTrue($manager->isEnabledForUser('test', $user)); + $this->appConfig->setValue('test', 'enabled', '["foo"]'); + $this->assertTrue($this->manager->isEnabledForUser('test', $user)); } public function testIsEnabledForUserDisabledForGroup() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); $user = new User('user1', null); - - $groupManager->expects($this->once()) + $this->groupManager->expects($this->once()) ->method('getUserGroupIds') ->with($user) ->will($this->returnValue(array('bar'))); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', '["foo"]'); - $this->assertFalse($manager->isEnabledForUser('test', $user)); + $this->appConfig->setValue('test', 'enabled', '["foo"]'); + $this->assertFalse($this->manager->isEnabledForUser('test', $user)); } public function testIsEnabledForUserLoggedOut() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', '["foo"]'); - $this->assertFalse($manager->IsEnabledForUser('test')); + $this->appConfig->setValue('test', 'enabled', '["foo"]'); + $this->assertFalse($this->manager->IsEnabledForUser('test')); } public function testIsEnabledForUserLoggedIn() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); $user = new User('user1', null); - $userSession->expects($this->once()) + $this->userSession->expects($this->once()) ->method('getUser') ->will($this->returnValue($user)); - $groupManager->expects($this->once()) + $this->groupManager->expects($this->once()) ->method('getUserGroupIds') ->with($user) ->will($this->returnValue(array('foo', 'bar'))); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test', 'enabled', '["foo"]'); - $this->assertTrue($manager->isEnabledForUser('test')); + $this->appConfig->setValue('test', 'enabled', '["foo"]'); + $this->assertTrue($this->manager->isEnabledForUser('test')); } public function testGetInstalledApps() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test1', 'enabled', 'yes'); - $appConfig->setValue('test2', 'enabled', 'no'); - $appConfig->setValue('test3', 'enabled', '["foo"]'); - $this->assertEquals(['test1', 'test3'], $manager->getInstalledApps()); + $this->appConfig->setValue('test1', 'enabled', 'yes'); + $this->appConfig->setValue('test2', 'enabled', 'no'); + $this->appConfig->setValue('test3', 'enabled', '["foo"]'); + $this->assertEquals(['test1', 'test3'], $this->manager->getInstalledApps()); } public function testGetAppsForUser() { - $userSession = $this->getMock('\OCP\IUserSession'); - $groupManager = $this->getMock('\OCP\IGroupManager'); - $user = new User('user1', null); - - $groupManager->expects($this->any()) + $this->groupManager->expects($this->any()) ->method('getUserGroupIds') ->with($user) ->will($this->returnValue(array('foo', 'bar'))); - $appConfig = $this->getAppConfig(); - $manager = new \OC\App\AppManager($userSession, $appConfig, $groupManager); - $appConfig->setValue('test1', 'enabled', 'yes'); - $appConfig->setValue('test2', 'enabled', 'no'); - $appConfig->setValue('test3', 'enabled', '["foo"]'); - $appConfig->setValue('test4', 'enabled', '["asd"]'); - $this->assertEquals(['test1', 'test3'], $manager->getEnabledAppsForUser($user)); + $this->appConfig->setValue('test1', 'enabled', 'yes'); + $this->appConfig->setValue('test2', 'enabled', 'no'); + $this->appConfig->setValue('test3', 'enabled', '["foo"]'); + $this->appConfig->setValue('test4', 'enabled', '["asd"]'); + $this->assertEquals(['test1', 'test3'], $this->manager->getEnabledAppsForUser($user)); } } From 696c750cfe4df66d3535f8696636113fd3cbd584 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 2 Apr 2015 08:28:42 +0200 Subject: [PATCH 3/3] Fix docs and line breaks --- lib/private/app/appmanager.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/app/appmanager.php b/lib/private/app/appmanager.php index c05b2a3b03..2a147d4de6 100644 --- a/lib/private/app/appmanager.php +++ b/lib/private/app/appmanager.php @@ -57,8 +57,12 @@ class AppManager implements IAppManager { * @param \OCP\IUserSession $userSession * @param \OCP\IAppConfig $appConfig * @param \OCP\IGroupManager $groupManager + * @param \OCP\ICacheFactory $memCacheFactory */ - public function __construct(IUserSession $userSession, IAppConfig $appConfig, IGroupManager $groupManager, ICacheFactory $memCacheFactory) { + public function __construct(IUserSession $userSession, + IAppConfig $appConfig, + IGroupManager $groupManager, + ICacheFactory $memCacheFactory) { $this->userSession = $userSession; $this->appConfig = $appConfig; $this->groupManager = $groupManager;