Merge pull request #13829 from owncloud/appmanager-list

Better caching for enabled apps
This commit is contained in:
Vincent Petry 2015-02-23 16:03:32 +01:00
commit 4290e1990e
11 changed files with 265 additions and 174 deletions

View File

@ -94,13 +94,13 @@ abstract class TestCase extends \Test\TestCase {
protected function setUp() {
parent::setUp();
$this->assertFalse(\OC_App::isEnabled('files_encryption'));
//login as user1
self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
$this->data = 'foobar';
$this->view = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files');
$this->assertFalse(\OC_App::isEnabled('files_encryption'));
}
protected function tearDown() {

View File

@ -62,6 +62,7 @@ class OC_App {
/**
* clean the appId
*
* @param string|boolean $app AppId that needs to be cleaned
* @return string
*/
@ -71,6 +72,7 @@ class OC_App {
/**
* loads all apps
*
* @param array $types
* @return bool
*
@ -216,7 +218,7 @@ class OC_App {
* @param bool $forceRefresh whether to refresh the cache
* @param bool $all whether to return apps for all users, not only the
* currently logged in one
* @return array
* @return string[]
*/
public static function getEnabledApps($forceRefresh = false, $all = false) {
if (!OC_Config::getValue('installed', false)) {
@ -224,52 +226,29 @@ class OC_App {
}
// in incognito mode or when logged out, $user will be false,
// which is also the case during an upgrade
$appManager = \OC::$server->getAppManager();
if ($all) {
$user = null;
if (!$all) {
$user = \OC_User::getUser();
}
if (is_string($user) && !$forceRefresh && !empty(self::$enabledAppsCache)) {
return self::$enabledAppsCache;
}
$apps = array();
$appConfig = \OC::$server->getAppConfig();
$appStatus = $appConfig->getValues(false, 'enabled');
foreach ($appStatus as $app => $enabled) {
if ($app === 'files') {
continue;
}
if ($enabled === 'yes') {
$apps[] = $app;
} else if ($enabled !== 'no') {
$groups = json_decode($enabled);
if (is_array($groups)) {
if (is_string($user)) {
foreach ($groups as $group) {
if (\OC_Group::inGroup($user, $group)) {
$apps[] = $app;
break;
}
}
} else {
// global, consider app as enabled
$apps[] = $app;
}
}
$user = \OC::$server->getUserSession()->getUser();
}
if (is_null($user)) {
$apps = $appManager->getInstalledApps();
} else {
$apps = $appManager->getEnabledAppsForUser($user);
}
$apps = array_filter($apps, function ($app) {
return $app !== 'files';//we add this manually
});
sort($apps);
array_unshift($apps, 'files');
// Only cache the app list, when the user is logged in.
// Otherwise we cache the list with disabled apps, although
// the apps are enabled for the user after he logged in.
if ($user) {
self::$enabledAppsCache = $apps;
}
return $apps;
}
/**
* checks whether or not an app is enabled
*
* @param string $app app
* @return bool
*
@ -279,12 +258,12 @@ class OC_App {
if ('files' == $app) {
return true;
}
$enabledApps = self::getEnabledApps();
return in_array($app, $enabledApps);
return \OC::$server->getAppManager()->isEnabledForUser($app);
}
/**
* enables an app
*
* @param mixed $app app
* @param array $groups (optional) when set, only these groups will have access to the app
* @throws \Exception
@ -298,10 +277,11 @@ class OC_App {
$app = self::installApp($app);
}
$appManager = \OC::$server->getAppManager();
if (!is_null($groups)) {
OC_Appconfig::setValue($app, 'enabled', json_encode($groups));
$appManager->enableAppForGroups($app, $groups);
} else {
OC_Appconfig::setValue($app, 'enabled', 'yes');
$appManager->enableApp($app);
}
}
@ -335,6 +315,7 @@ class OC_App {
/**
* This function set an app as disabled in appconfig.
*
* @param string $app app
* @throws Exception
*/
@ -345,11 +326,13 @@ class OC_App {
self::$enabledAppsCache = array(); // flush
// check if app is a shipped app or not. if not delete
\OC_Hook::emit('OC_App', 'pre_disable', array('app' => $app));
OC_Appconfig::setValue($app, 'enabled', 'no' );
$appManager = \OC::$server->getAppManager();
$appManager->disableApp($app);
}
/**
* adds an entry to the navigation
*
* @param array $data array containing the data
* @return bool
*
@ -372,6 +355,7 @@ class OC_App {
/**
* marks a navigation entry as active
*
* @param string $id id of the entry
* @return bool
*
@ -386,6 +370,7 @@ class OC_App {
/**
* Get the navigation entries for the $app
*
* @param string $app app
* @return array an array of the $data added with addNavigationEntry
*
@ -402,6 +387,7 @@ class OC_App {
/**
* gets the active Menu entry
*
* @return string id or empty string
*
* This function returns the id of the active navigation entry (set by
@ -413,6 +399,7 @@ class OC_App {
/**
* Returns the Settings Navigation
*
* @return string
*
* This function returns an array containing all settings pages added. The
@ -517,6 +504,7 @@ class OC_App {
/**
* search for an app in all app-directories
*
* @param $appId
* @return mixed (bool|string)
*/
@ -617,6 +605,7 @@ class OC_App {
/**
* get app's version based on it's path
*
* @param string $path
* @return string
*/
@ -664,6 +653,7 @@ class OC_App {
/**
* Returns the navigation
*
* @return array
*
* This function returns an array containing all entries added. The
@ -755,6 +745,7 @@ class OC_App {
/**
* get a list of all apps in the apps folder
*
* @return array an array of app names (string IDs)
* @todo: change the name of this method to getInstalledApps, which is more accurate
*/
@ -788,6 +779,7 @@ class OC_App {
/**
* Lists all apps, this is used in apps.php
*
* @return array
*/
public static function listAllApps($onlyLocal = false) {
@ -861,7 +853,8 @@ class OC_App {
foreach ($remoteApps AS $key => $remote) {
if ($app['name'] === $remote['name'] ||
(isset($app['ocsid']) &&
$app['ocsid'] === $remote['id'])) {
$app['ocsid'] === $remote['id'])
) {
unset($remoteApps[$key]);
}
}
@ -904,6 +897,7 @@ class OC_App {
/**
* get a list of all apps on apps.owncloud.com
*
* @return array|false multi-dimensional array of apps.
* Keys: id, name, type, typename, personid, license, detailpage, preview, changed, description
*/

View File

@ -24,6 +24,7 @@ namespace OC\App;
use OCP\App\IAppManager;
use OCP\IAppConfig;
use OCP\IGroupManager;
use OCP\IUser;
use OCP\IUserSession;
class AppManager implements IAppManager {
@ -61,7 +62,7 @@ class AppManager implements IAppManager {
/**
* @return string[] $appId => $enabled
*/
private function getInstalledApps() {
private function getInstalledAppsValues() {
if (!$this->installedAppsCache) {
$values = $this->appConfig->getValues(false, 'enabled');
$this->installedAppsCache = array_filter($values, function ($value) {
@ -72,6 +73,29 @@ class AppManager implements IAppManager {
return $this->installedAppsCache;
}
/**
* List all installed apps
*
* @return string[]
*/
public function getInstalledApps() {
return array_keys($this->getInstalledAppsValues());
}
/**
* List all apps enabled for a user
*
* @param \OCP\IUser $user
* @return string[]
*/
public function getEnabledAppsForUser(IUser $user) {
$apps = $this->getInstalledAppsValues();
$appsForUser = array_filter($apps, function ($enabled) use ($user) {
return $this->checkAppForUser($enabled, $user);
});
return array_keys($appsForUser);
}
/**
* Check if an app is enabled for user
*
@ -83,9 +107,20 @@ class AppManager implements IAppManager {
if (is_null($user)) {
$user = $this->userSession->getUser();
}
$installedApps = $this->getInstalledApps();
$installedApps = $this->getInstalledAppsValues();
if (isset($installedApps[$appId])) {
$enabled = $installedApps[$appId];
return $this->checkAppForUser($installedApps[$appId], $user);
} else {
return false;
}
}
/**
* @param string $enabled
* @param IUser $user
* @return bool
*/
private function checkAppForUser($enabled, $user) {
if ($enabled === 'yes') {
return true;
} elseif (is_null($user)) {
@ -100,9 +135,6 @@ class AppManager implements IAppManager {
}
return false;
}
} else {
return false;
}
}
/**
@ -112,7 +144,7 @@ class AppManager implements IAppManager {
* @return bool
*/
public function isInstalled($appId) {
$installedApps = $this->getInstalledApps();
$installedApps = $this->getInstalledAppsValues();
return isset($installedApps[$appId]);
}
@ -122,6 +154,7 @@ class AppManager implements IAppManager {
* @param string $appId
*/
public function enableApp($appId) {
$this->installedAppsCache[$appId] = 'yes';
$this->appConfig->setValue($appId, 'enabled', 'yes');
}
@ -136,6 +169,7 @@ class AppManager implements IAppManager {
/** @var \OCP\IGroup $group */
return $group->getGID();
}, $groups);
$this->installedAppsCache[$appId] = json_encode($groupIds);
$this->appConfig->setValue($appId, 'enabled', json_encode($groupIds));
}
@ -149,6 +183,7 @@ class AppManager implements IAppManager {
if ($appId === 'files') {
throw new \Exception("files can't be disabled.");
}
unset($this->installedAppsCache[$appId]);
$this->appConfig->setValue($appId, 'enabled', 'no');
}
}

View File

@ -236,16 +236,7 @@ class Manager extends PublicEmitter implements IGroupManager {
* @return array with group ids
*/
public function getUserGroupIds($user) {
$groupIds = array();
$userId = $user->getUID();
if (isset($this->cachedUserGroups[$userId])) {
return array_keys($this->cachedUserGroups[$userId]);
} else {
foreach ($this->backends as $backend) {
$groupIds = array_merge($groupIds, $backend->getUserGroups($userId));
}
}
return $groupIds;
return array_keys($this->getUserGroups($user));
}
/**

View File

@ -66,6 +66,7 @@ class OC_User {
/**
* registers backend
*
* @param string $backend name of the backend
* @deprecated Add classes by calling OC_User::useBackend() with a class instance instead
* @return bool
@ -79,6 +80,7 @@ class OC_User {
/**
* gets available backends
*
* @deprecated
* @return array an array of backends
*
@ -90,6 +92,7 @@ class OC_User {
/**
* gets used backends
*
* @deprecated
* @return array an array of backends
*
@ -101,6 +104,7 @@ class OC_User {
/**
* Adds the backend to the list of used backends
*
* @param string|OC_User_Interface $backend default: database The backend to use for user management
* @return bool
*
@ -173,6 +177,7 @@ class OC_User {
/**
* Create a new user
*
* @param string $uid The username of the user to create
* @param string $password The password of the new user
* @throws Exception
@ -190,6 +195,7 @@ class OC_User {
/**
* delete a user
*
* @param string $uid The username of the user to delete
* @return bool
*
@ -207,6 +213,7 @@ class OC_User {
/**
* Try to login a user
*
* @param string $loginname The login name of the user to log in
* @param string $password The password of the user
* @return boolean|null
@ -288,11 +295,18 @@ class OC_User {
* Sets user id for session and triggers emit
*/
public static function setUserId($uid) {
$userSession = \OC::$server->getUserSession();
$userManager = \OC::$server->getUserManager();
if ($user = $userManager->get($uid)) {
$userSession->setUser($user);
} else {
\OC::$server->getSession()->set('user_id', $uid);
}
}
/**
* Sets user display name for session
*
* @param string $uid
* @param null $displayName
* @return bool Whether the display name could get set
@ -329,6 +343,7 @@ class OC_User {
/**
* Check if the user is logged in, considers also the HTTP basic credentials
*
* @return bool
*/
public static function isLoggedIn() {
@ -341,6 +356,7 @@ class OC_User {
/**
* set incognito mode, e.g. if a user wants to open a public link
*
* @param bool $status
*/
public static function setIncognitoMode($status) {
@ -349,6 +365,7 @@ class OC_User {
/**
* get incognito mode status
*
* @return bool
*/
public static function isIncognitoMode() {
@ -373,6 +390,7 @@ class OC_User {
/**
* Check if the user is an admin user
*
* @param string $uid uid of the admin
* @return bool
*/
@ -386,6 +404,7 @@ class OC_User {
/**
* get the user id of the user currently logged in.
*
* @return string uid or false
*/
public static function getUser() {
@ -399,6 +418,7 @@ class OC_User {
/**
* get the display name of the user currently logged in.
*
* @param string $uid
* @return string uid or false
*/
@ -422,6 +442,7 @@ class OC_User {
/**
* Autogenerate a password
*
* @return string
*
* generates a password
@ -432,6 +453,7 @@ class OC_User {
/**
* Set password
*
* @param string $uid The username
* @param string $password The new password
* @param string $recoveryPassword for the encryption app to reset encryption keys
@ -450,6 +472,7 @@ class OC_User {
/**
* Check whether user can change his avatar
*
* @param string $uid The username
* @return bool
*
@ -466,6 +489,7 @@ class OC_User {
/**
* Check whether user can change his password
*
* @param string $uid The username
* @return bool
*
@ -482,6 +506,7 @@ class OC_User {
/**
* Check whether user can change his display name
*
* @param string $uid The username
* @return bool
*
@ -498,6 +523,7 @@ class OC_User {
/**
* Check if the password is correct
*
* @param string $uid The username
* @param string $password The password
* @return string|false user id a string on success, false otherwise
@ -532,6 +558,7 @@ class OC_User {
/**
* Get a list of all users
*
* @return array an array of all uids
*
* Get a list of all users.
@ -550,6 +577,7 @@ class OC_User {
/**
* Get a list of all users display name
*
* @param string $search
* @param int $limit
* @param int $offset
@ -569,6 +597,7 @@ class OC_User {
/**
* check if a user exists
*
* @param string $uid the username
* @return boolean
*/
@ -617,6 +646,7 @@ class OC_User {
/**
* Set cookie value to use in next page load
*
* @param string $username username to be set
* @param string $token
*/
@ -633,6 +663,7 @@ class OC_User {
/**
* Returns the first active backend from self::$_usedBackends.
*
* @return OCP\Authentication\IApacheBackend|null if no backend active, otherwise OCP\Authentication\IApacheBackend
*/
private static function findFirstActiveUsedBackend() {

View File

@ -63,6 +63,10 @@ class OC_Util {
private static $rootMounted = false;
private static $fsSetup = false;
protected static function getAppManager() {
return \OC::$server->getAppManager();
}
private static function initLocalStorageRootFS() {
// mount local file backend as root
$configDataDirectory = OC_Config::getValue("datadirectory", OC::$SERVERROOT . "/data");
@ -1010,7 +1014,7 @@ class OC_Util {
// find the first app that is enabled for the current user
foreach ($defaultApps as $defaultApp) {
$defaultApp = OC_App::cleanAppId(strip_tags($defaultApp));
if (OC_App::isEnabled($defaultApp)) {
if (static::getAppManager()->isEnabledForUser($defaultApp)) {
$appId = $defaultApp;
break;
}

View File

@ -20,6 +20,8 @@
*/
namespace OCP\App;
use OCP\IUser;
interface IAppManager {
/**
* Check if an app is enabled for user
@ -59,4 +61,19 @@ interface IAppManager {
* @param string $appId
*/
public function disableApp($appId);
/**
* List all apps enabled for a user
*
* @param \OCP\IUser $user
* @return string[]
*/
public function getEnabledAppsForUser(IUser $user);
/**
* List all installed apps
*
* @return string[]
*/
public function getInstalledApps();
}

View File

@ -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';
@ -401,7 +399,6 @@ class Test_App extends \Test\TestCase {
);
$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);
}
/**
@ -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

View File

@ -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));
}
}

View File

@ -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'));

View File

@ -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();
@ -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'),
@ -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;
class Dummy_OC_Util extends OC_Util {
/**
* @var \OCP\App\IAppManager
*/
public static $appManager;
public static function setEnabledApps($enabledApps) {
self::$enabledAppsCacheBackup = self::$enabledAppsCache;
self::$enabledAppsCache = $enabledApps;
}
public static function restore() {
self::$enabledAppsCache = self::$enabledAppsCacheBackup;
protected static function getAppManager() {
return self::$appManager;
}
}