aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Reschke <lukas@owncloud.com>2014-09-04 13:56:41 +0200
committerLukas Reschke <lukas@owncloud.com>2014-09-04 13:56:41 +0200
commit1a7df3323391f6d6f7ef04de2687b3a566ab6351 (patch)
tree9e3bc1db6aa3f05886a747bcf728040a79a4cdcd
parentd261ca616b2ffec54dafb83818cfabef5306944b (diff)
parent4a93a6e0600e4445333bd22015fc9d22d4251219 (diff)
downloadnextcloud-server-1a7df3323391f6d6f7ef04de2687b3a566ab6351.tar.gz
nextcloud-server-1a7df3323391f6d6f7ef04de2687b3a566ab6351.zip
Merge pull request #10818 from owncloud/enableappforgroupfix
Fix upgrade process when apps enabled for specific groups
-rw-r--r--core/ajax/update.php4
-rw-r--r--lib/private/app.php31
-rwxr-xr-xlib/private/util.php16
-rw-r--r--tests/lib/app.php226
-rw-r--r--tests/lib/util.php22
5 files changed, 286 insertions, 13 deletions
diff --git a/core/ajax/update.php b/core/ajax/update.php
index 627ada080c8..1e280a82227 100644
--- a/core/ajax/update.php
+++ b/core/ajax/update.php
@@ -3,6 +3,10 @@ set_time_limit(0);
require_once '../../lib/base.php';
if (OC::checkUpgrade(false)) {
+ // if a user is currently logged in, their session must be ignored to
+ // avoid side effects
+ \OC_User::setIncognitoMode(true);
+
$l = new \OC_L10N('core');
$eventSource = new OC_EventSource();
$updater = new \OC\Updater(\OC_Log::$object);
diff --git a/lib/private/app.php b/lib/private/app.php
index d10d352b432..3eed9e3c443 100644
--- a/lib/private/app.php
+++ b/lib/private/app.php
@@ -172,17 +172,29 @@ class OC_App {
*/
protected static $enabledAppsCache = array();
- public static function getEnabledApps($forceRefresh = false) {
+ /**
+ * Returns apps enabled for the current user.
+ *
+ * @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
+ */
+ public static function getEnabledApps($forceRefresh = false, $all = false) {
if (!OC_Config::getValue('installed', false)) {
return array();
}
- if (!$forceRefresh && !empty(self::$enabledAppsCache)) {
+ // in incognito mode or when logged out, $user will be false,
+ // which is also the case during an upgrade
+ $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');
- $user = \OC_User::getUser();
foreach ($appStatus as $app => $enabled) {
if ($app === 'files') {
continue;
@@ -192,11 +204,16 @@ class OC_App {
} else if ($enabled !== 'no') {
$groups = json_decode($enabled);
if (is_array($groups)) {
- foreach ($groups as $group) {
- if (\OC_Group::inGroup($user, $group)) {
- $apps[] = $app;
- break;
+ 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;
}
}
}
diff --git a/lib/private/util.php b/lib/private/util.php
index 2a0f9197e3c..6f45e00215d 100755
--- a/lib/private/util.php
+++ b/lib/private/util.php
@@ -1461,9 +1461,11 @@ class OC_Util {
}
/**
- * Check whether the instance needs to preform an upgrade
+ * Check whether the instance needs to perform an upgrade,
+ * either when the core version is higher or any app requires
+ * an upgrade.
*
- * @return bool
+ * @return bool whether the core or any app needs an upgrade
*/
public static function needUpgrade() {
if (OC_Config::getValue('installed', false)) {
@@ -1473,14 +1475,16 @@ class OC_Util {
return true;
}
- // also check for upgrades for apps
- $apps = \OC_App::getEnabledApps();
+ // also check for upgrades for apps (independently from the user)
+ $apps = \OC_App::getEnabledApps(false, true);
+ $shouldUpgrade = false;
foreach ($apps as $app) {
if (\OC_App::shouldUpgrade($app)) {
- return true;
+ $shouldUpgrade = true;
+ break;
}
}
- return false;
+ return $shouldUpgrade;
} else {
return false;
}
diff --git a/tests/lib/app.php b/tests/lib/app.php
index e2b578fe6b9..e538ebec8a0 100644
--- a/tests/lib/app.php
+++ b/tests/lib/app.php
@@ -9,6 +9,14 @@
class Test_App extends PHPUnit_Framework_TestCase {
+ private $oldAppConfigService;
+
+ const TEST_USER1 = 'user1';
+ const TEST_USER2 = 'user2';
+ const TEST_USER3 = 'user3';
+ const TEST_GROUP1 = 'group1';
+ const TEST_GROUP2 = 'group2';
+
function appVersionsProvider() {
return array(
// exact match
@@ -236,4 +244,222 @@ class Test_App extends PHPUnit_Framework_TestCase {
array_unshift($sortedApps, 'files');
$this->assertEquals($sortedApps, $apps);
}
+
+ /**
+ * Providers for the app config values
+ */
+ function appConfigValuesProvider() {
+ return array(
+ // logged in user1
+ array(
+ self::TEST_USER1,
+ array(
+ 'files',
+ 'app1',
+ 'app3',
+ 'appforgroup1',
+ 'appforgroup12',
+ ),
+ false
+ ),
+ // logged in user2
+ array(
+ self::TEST_USER2,
+ array(
+ 'files',
+ 'app1',
+ 'app3',
+ 'appforgroup12',
+ 'appforgroup2',
+ ),
+ false
+ ),
+ // logged in user3
+ array(
+ self::TEST_USER3,
+ array(
+ 'files',
+ 'app1',
+ 'app3',
+ 'appforgroup1',
+ 'appforgroup12',
+ 'appforgroup2',
+ ),
+ false
+ ),
+ // no user, returns all apps
+ array(
+ null,
+ array(
+ 'files',
+ 'app1',
+ 'app3',
+ 'appforgroup1',
+ 'appforgroup12',
+ 'appforgroup2',
+ ),
+ false,
+ ),
+ // user given, but ask for all
+ array(
+ self::TEST_USER1,
+ array(
+ 'files',
+ 'app1',
+ 'app3',
+ 'appforgroup1',
+ 'appforgroup12',
+ 'appforgroup2',
+ ),
+ true,
+ ),
+ );
+ }
+
+ /**
+ * Test enabled apps
+ *
+ * @dataProvider appConfigValuesProvider
+ */
+ public function testEnabledApps($user, $expectedApps, $forceAll) {
+ $userManager = \OC::$server->getUserManager();
+ $groupManager = \OC::$server->getGroupManager();
+ $user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
+ $user2 = $userManager->createUser(self::TEST_USER2, self::TEST_USER2);
+ $user3 = $userManager->createUser(self::TEST_USER3, self::TEST_USER3);
+
+ $group1 = $groupManager->createGroup(self::TEST_GROUP1);
+ $group1->addUser($user1);
+ $group1->addUser($user3);
+ $group2 = $groupManager->createGroup(self::TEST_GROUP2);
+ $group2->addUser($user2);
+ $group2->addUser($user3);
+
+ \OC_User::setUserId($user);
+
+ $this->setupAppConfigMock()->expects($this->once())
+ ->method('getValues')
+ ->will($this->returnValue(
+ array(
+ 'app3' => 'yes',
+ 'app2' => 'no',
+ 'app1' => 'yes',
+ 'appforgroup1' => '["group1"]',
+ 'appforgroup2' => '["group2"]',
+ 'appforgroup12' => '["group2","group1"]',
+ )
+ )
+ );
+
+ $apps = \OC_App::getEnabledApps(true, $forceAll);
+ $this->assertEquals($expectedApps, $apps);
+
+ $this->restoreAppConfig();
+ \OC_User::setUserId(null);
+
+ $user1->delete();
+ $user2->delete();
+ $user3->delete();
+ // clear user cache...
+ $userManager->delete(self::TEST_USER1);
+ $userManager->delete(self::TEST_USER2);
+ $userManager->delete(self::TEST_USER3);
+ $group1->delete();
+ $group2->delete();
+ }
+
+ /**
+ * Test isEnabledApps() with cache, not re-reading the list of
+ * enabled apps more than once when a user is set.
+ */
+ public function testEnabledAppsCache() {
+ $userManager = \OC::$server->getUserManager();
+ $user1 = $userManager->createUser(self::TEST_USER1, self::TEST_USER1);
+
+ \OC_User::setUserId(self::TEST_USER1);
+
+ $this->setupAppConfigMock()->expects($this->once())
+ ->method('getValues')
+ ->will($this->returnValue(
+ array(
+ 'app3' => 'yes',
+ 'app2' => 'no',
+ )
+ )
+ );
+
+ $apps = \OC_App::getEnabledApps(true);
+ $this->assertEquals(array('files', 'app3'), $apps);
+
+ // mock should not be called again here
+ $apps = \OC_App::getEnabledApps(false);
+ $this->assertEquals(array('files', 'app3'), $apps);
+
+ $this->restoreAppConfig();
+ \OC_User::setUserId(null);
+
+ $user1->delete();
+ // clear user cache...
+ $userManager->delete(self::TEST_USER1);
+ }
+
+ /**
+ * 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(
+ '\OC\AppConfig',
+ array('getValues'),
+ array(\OC_DB::getConnection()),
+ '',
+ false
+ );
+
+ $this->registerAppConfig($appConfig);
+ return $appConfig;
+ }
+
+ /**
+ * 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;
+ });
+ }
+
+ /**
+ * Restore the original app config service.
+ */
+ private function restoreAppConfig() {
+ $oldService = $this->oldAppConfigService;
+ \OC::$server->registerService('AppConfig', function ($c) use ($oldService){
+ return $oldService;
+ });
+ }
}
+
diff --git a/tests/lib/util.php b/tests/lib/util.php
index c2bb99c3b2e..a2efcca2603 100644
--- a/tests/lib/util.php
+++ b/tests/lib/util.php
@@ -303,6 +303,8 @@ class Test_Util extends PHPUnit_Framework_TestCase {
\OC::$WEBROOT = '';
Dummy_OC_App::setEnabledApps($enabledApps);
+ // need to set a user id to make sure enabled apps are read from cache
+ \OC_User::setUserId(uniqid());
\OCP\Config::setSystemValue('defaultapp', $defaultAppConfig);
$this->assertEquals('http://localhost/' . $expectedPath, \OC_Util::getDefaultPageUrl());
@@ -310,6 +312,7 @@ class Test_Util extends PHPUnit_Framework_TestCase {
\OC::$WEBROOT = $oldWebRoot;
Dummy_OC_App::restore();
\OCP\Config::setSystemValue('defaultapp', $oldDefaultApps);
+ \OC_User::setUserId(null);
}
function defaultAppsProvider() {
@@ -341,6 +344,25 @@ class Test_Util extends PHPUnit_Framework_TestCase {
);
}
+ /**
+ * Test needUpgrade() when the core version is increased
+ */
+ public function testNeedUpgradeCore() {
+ $oldConfigVersion = OC_Config::getValue('version', '0.0.0');
+ $oldSessionVersion = \OC::$server->getSession()->get('OC_Version');
+
+ $this->assertFalse(\OCP\Util::needUpgrade());
+
+ OC_Config::setValue('version', '7.0.0.0');
+ \OC::$server->getSession()->set('OC_Version', array(7, 0, 0, 1));
+
+ $this->assertTrue(\OCP\Util::needUpgrade());
+
+ OC_Config::setValue('version', $oldConfigVersion);
+ $oldSessionVersion = \OC::$server->getSession()->set('OC_Version', $oldSessionVersion);
+
+ $this->assertFalse(\OCP\Util::needUpgrade());
+ }
}
/**