Merge pull request #13829 from owncloud/appmanager-list

Better caching for enabled apps

4 files changed, 79 insertions, 60 deletions

diff --git a/tests/lib/app.php b/tests/lib/app.php
index 0c0eb28b3ba..86a407c1a95 100644
--- a/tests/lib/app.php
+++ b/tests/lib/app.php
@@ -1,4 +1,5 @@
 <?php
+
 /**
  * Copyright (c) 2012 Bernhard Posselt <dev@bernhard-posselt.com>
  * Copyright (c) 2014 Vincent Petry <pvince81@owncloud.com>
@@ -6,11 +7,8 @@
  * later.
  * See the COPYING-README file.
  */
-
 class Test_App extends \Test\TestCase {
 
-	private $oldAppConfigService;
-
 	const TEST_USER1 = 'user1';
 	const TEST_USER2 = 'user2';
 	const TEST_USER3 = 'user3';
@@ -398,10 +396,9 @@ class Test_App extends \Test\TestCase {
 					'appforgroup12' => '["group2","group1"]',
 				)
 			)
-		);
+			);
 
 		$apps = \OC_App::getEnabledApps(true, $forceAll);
-		$this->assertEquals($expectedApps, $apps);
 
 		$this->restoreAppConfig();
 		\OC_User::setUserId(null);
@@ -412,6 +409,8 @@ class Test_App extends \Test\TestCase {
 
 		$group1->delete();
 		$group2->delete();
+
+		$this->assertEquals($expectedApps, $apps);
 	}
 
 	/**
@@ -432,7 +431,7 @@ class Test_App extends \Test\TestCase {
 					'app2' => 'no',
 				)
 			)
-		);
+			);
 
 		$apps = \OC_App::getEnabledApps(true);
 		$this->assertEquals(array('files', 'app3'), $apps);
@@ -447,30 +446,6 @@ class Test_App extends \Test\TestCase {
 		$user1->delete();
 	}
 
-	/**
-	 * Tests that the apps list is re-requested (not cached) when
-	 * no user is set.
-	 */
-	public function testEnabledAppsNoCache() {
-		$this->setupAppConfigMock()->expects($this->exactly(2))
-			->method('getValues')
-			->will($this->returnValue(
-				array(
-					'app3' => 'yes',
-					'app2' => 'no',
-				)
-			)
-		);
-
-		$apps = \OC_App::getEnabledApps(true);
-		$this->assertEquals(array('files', 'app3'), $apps);
-
-		// mock should be called again here
-		$apps = \OC_App::getEnabledApps(false);
-		$this->assertEquals(array('files', 'app3'), $apps);
-
-		$this->restoreAppConfig();
-	}
 
 	private function setupAppConfigMock() {
 		$appConfig = $this->getMock(
@@ -487,22 +462,27 @@ class Test_App extends \Test\TestCase {
 
 	/**
 	 * Register an app config mock for testing purposes.
+	 *
 	 * @param $appConfig app config mock
 	 */
 	private function registerAppConfig($appConfig) {
-		$this->oldAppConfigService = \OC::$server->query('AppConfig');
 		\OC::$server->registerService('AppConfig', function ($c) use ($appConfig) {
 			return $appConfig;
 		});
+		\OC::$server->registerService('AppManager', function (\OC\Server $c) use ($appConfig) {
+			return new \OC\App\AppManager($c->getUserSession(), $appConfig, $c->getGroupManager());
+		});
 	}
 
 	/**
 	 * Restore the original app config service.
 	 */
 	private function restoreAppConfig() {
-		$oldService = $this->oldAppConfigService;
-		\OC::$server->registerService('AppConfig', function ($c) use ($oldService){
-			return $oldService;
+		\OC::$server->registerService('AppConfig', function ($c) {
+			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());
 		});
 
 		// 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 4c0555b501f..cb41f737469 100644
--- a/tests/lib/app/manager.php
+++ b/tests/lib/app/manager.php
@@ -192,4 +192,36 @@ class Manager extends \PHPUnit_Framework_TestCase {
 		$appConfig->setValue('test', 'enabled', '["foo"]');
 		$this->assertTrue($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());
+	}
+
+	public function testGetAppsForUser() {
+		$userSession = $this->getMock('\OCP\IUserSession');
+		$groupManager = $this->getMock('\OCP\IGroupManager');
+
+		$user = new User('user1', null);
+
+		$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));
+	}
 }
diff --git a/tests/lib/appconfig.php b/tests/lib/appconfig.php
index 188721ff92d..ead5b859277 100644
--- a/tests/lib/appconfig.php
+++ b/tests/lib/appconfig.php
@@ -11,6 +11,12 @@ class Test_Appconfig extends \Test\TestCase {
 	public static function setUpBeforeClass() {
 		parent::setUpBeforeClass();
 
+		$query = \OC_DB::prepare('DELETE FROM `*PREFIX*appconfig` WHERE `appid` = ?');
+		$query->execute(array('testapp'));
+		$query->execute(array('someapp'));
+		$query->execute(array('123456'));
+		$query->execute(array('anotherapp'));
+
 		$query = \OC_DB::prepare('INSERT INTO `*PREFIX*appconfig` VALUES (?, ?, ?)');
 
 		$query->execute(array('testapp', 'enabled', 'true'));
diff --git a/tests/lib/util.php b/tests/lib/util.php
index 25c9e31beaf..6870b218076 100644
--- a/tests/lib/util.php
+++ b/tests/lib/util.php
@@ -1,11 +1,11 @@
 <?php
+
 /**
  * Copyright (c) 2012 Lukas Reschke <lukas@statuscode.ch>
  * This file is licensed under the Affero General Public License version 3 or
  * later.
  * See the COPYING-README file.
  */
-
 class Test_Util extends \Test\TestCase {
 	public function testGetVersion() {
 		$version = \OC_Util::getVersion();
@@ -105,7 +105,7 @@ class Test_Util extends \Test\TestCase {
 		$this->assertEquals('This is a good string without HTML.', $result);
 	}
 
-	function testEncodePath(){
+	function testEncodePath() {
 		$component = '/§#@test%&^ä/-child';
 		$result = OC_Util::encodePath($component);
 		$this->assertEquals("/%C2%A7%23%40test%25%26%5E%C3%A4/-child", $result);
@@ -210,14 +210,12 @@ class Test_Util extends \Test\TestCase {
 	/**
 	 * @dataProvider baseNameProvider
 	 */
-	public function testBaseName($expected, $file)
-	{
+	public function testBaseName($expected, $file) {
 		$base = \OC_Util::basename($file);
 		$this->assertEquals($expected, $base);
 	}
 
-	public function baseNameProvider()
-	{
+	public function baseNameProvider() {
 		return array(
 			array('public_html', '/home/user/public_html/'),
 			array('public_html', '/home/user/public_html'),
@@ -288,11 +286,11 @@ class Test_Util extends \Test\TestCase {
 
 		\OC_User::createUser($uid, "passwd");
 
-		foreach($groups as $group) {
+		foreach ($groups as $group) {
 			\OC_Group::createGroup($group);
 		}
 
-		foreach($membership as $group) {
+		foreach ($membership as $group) {
 			\OC_Group::addToGroup($uid, $group);
 		}
 
@@ -308,7 +306,7 @@ class Test_Util extends \Test\TestCase {
 		\OC_User::deleteUser($uid);
 		\OC_User::setUserId('');
 
-		foreach($groups as $group) {
+		foreach ($groups as $group) {
 			\OC_Group::deleteGroup($group);
 		}
 
@@ -317,7 +315,7 @@ class Test_Util extends \Test\TestCase {
 
 	}
 
-	public function dataProviderForTestIsSharingDisabledForUser()    {
+	public function dataProviderForTestIsSharingDisabledForUser() {
 		return array(
 			// existing groups, groups the user belong to, groups excluded from sharing, expected result
 			array(array('g1', 'g2', 'g3'), array(), array('g1'), false),
@@ -327,8 +325,8 @@ class Test_Util extends \Test\TestCase {
 			array(array('g1', 'g2', 'g3'), array('g1', 'g2'), array('g1'), false),
 			array(array('g1', 'g2', 'g3'), array('g1', 'g2'), array('g1', 'g2'), true),
 			array(array('g1', 'g2', 'g3'), array('g1', 'g2'), array('g1', 'g2', 'g3'), true),
-        );
-    }
+		);
+	}
 
 	/**
 	 * Test default apps
@@ -341,15 +339,21 @@ class Test_Util extends \Test\TestCase {
 		$oldWebRoot = \OC::$WEBROOT;
 		\OC::$WEBROOT = '';
 
-		Dummy_OC_App::setEnabledApps($enabledApps);
+		$appManager = $this->getMock('\OCP\App\IAppManager');
+		$appManager->expects($this->any())
+			->method('isEnabledForUser')
+			->will($this->returnCallback(function($appId) use ($enabledApps){
+				return in_array($appId, $enabledApps);
+		}));
+		Dummy_OC_Util::$appManager = $appManager;
+
 		// need to set a user id to make sure enabled apps are read from cache
 		\OC_User::setUserId($this->getUniqueID());
 		\OCP\Config::setSystemValue('defaultapp', $defaultAppConfig);
-		$this->assertEquals('http://localhost/' . $expectedPath, \OC_Util::getDefaultPageUrl());
+		$this->assertEquals('http://localhost/' . $expectedPath, Dummy_OC_Util::getDefaultPageUrl());
 
 		// restore old state
 		\OC::$WEBROOT = $oldWebRoot;
-		Dummy_OC_App::restore();
 		\OCP\Config::setSystemValue('defaultapp', $oldDefaultApps);
 		\OC_User::setUserId(null);
 	}
@@ -405,18 +409,15 @@ class Test_Util extends \Test\TestCase {
 }
 
 /**
- * Dummy OC Apps class to make it possible to override
- * enabled apps
+ * Dummy OC Util class to make it possible to override the app manager
  */
-class Dummy_OC_App extends OC_App {
-	private static $enabledAppsCacheBackup;
-
-	public static function setEnabledApps($enabledApps) {
-		self::$enabledAppsCacheBackup = self::$enabledAppsCache;
-		self::$enabledAppsCache = $enabledApps;
-	}
+class Dummy_OC_Util extends OC_Util {
+	/**
+	 * @var \OCP\App\IAppManager
+	 */
+	public static $appManager;
 
-	public static function restore() {
-		self::$enabledAppsCache = self::$enabledAppsCacheBackup;
+	protected static function getAppManager() {
+		return self::$appManager;
 	}
 }