[Pkg-owncloud-commits] [owncloud] 01/10: Fix totally broken AppStore
David Prévot
taffit at moszumanska.debian.org
Wed Mar 11 15:50:03 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to annotated tag v8.0.2
in repository owncloud.
commit 8961967fecf5e860552bcb633524385d5d12a32c
Author: Lukas Reschke <lukas at owncloud.com>
Date: Thu Mar 5 20:42:13 2015 +0100
Fix totally broken AppStore
As it turned out the AppStore code was completely broken when it came from apps delivered from the appstore, this meant:
1. You could not disable and then re-enable an application that was installed from the AppStore. It simply failed hard.
2. You could not disable apps from the categories but only from the "Activated" page
3. It did not show the activation state from any category page
This code is completely static and thus testing it is impossible. We really have to stop with "let's add yet another feature in already existing static code". Such stuff has to get refactored first.
That said, this code works from what I can say when clicking around in the AppStore page GUI. However, it may easily be that it does not work with updates or whatsever as I have no chance to test that since the AppStore code is not open-source and it is impossible to write unit-tests for that.
Fixes https://github.com/owncloud/core/issues/14711
---
lib/private/allconfig.php | 30 ++++++++++++++++++++++++++++++
lib/private/app.php | 28 ++++++++++++++++++++++++----
lib/public/iconfig.php | 9 +++++++++
settings/ajax/disableapp.php | 3 +--
settings/ajax/enableapp.php | 3 +--
tests/lib/allconfig.php | 27 +++++++++++++++++++++++++++
6 files changed, 92 insertions(+), 8 deletions(-)
diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php
index 00defd9..ec0461f 100644
--- a/lib/private/allconfig.php
+++ b/lib/private/allconfig.php
@@ -161,6 +161,36 @@ class AllConfig implements \OCP\IConfig {
\OC::$server->getAppConfig()->deleteApp($appName);
}
+ /**
+ * Determines the apps that have the set the value for the specified
+ * keys
+ *
+ * @param string $value the value to get the app for
+ * @param string $key the key too lookup
+ * @return array of app IDs
+ */
+ public function getAppsForKeyValue($key, $value) {
+ // TODO - FIXME
+ $this->fixDIInit();
+
+ $qb = $this->connection->createQueryBuilder();
+ $qb
+ ->select('appid')
+ ->from('`*PREFIX*appconfig`')
+ ->where('configvalue = ?')
+ ->andWhere('configkey = ?')
+ ->setParameter(0, $value)
+ ->setParameter(1, $key)
+ ;
+ $result = $qb->execute();
+
+ $appIDs = [];
+ while ($row = $result->fetch()) {
+ $appIDs[] = $row['appid'];
+ }
+
+ return $appIDs;
+ }
/**
* Set a user defined value
diff --git a/lib/private/app.php b/lib/private/app.php
index 9e9a388..c12447d 100644
--- a/lib/private/app.php
+++ b/lib/private/app.php
@@ -277,6 +277,7 @@ class OC_App {
*/
public static function enable($app, $groups = null) {
self::$enabledAppsCache = array(); // flush
+
if (!OC_Installer::isInstalled($app)) {
$app = self::installApp($app);
}
@@ -327,6 +328,12 @@ 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));
+
+ // Convert OCS ID to regular application identifier
+ if(is_numeric($app)) {
+ $app = \OC::$server->getConfig()->getAppsForKeyValue('ocsid', $app)[0];
+ }
+
OC_Appconfig::setValue($app, 'enabled', 'no' );
}
@@ -925,7 +932,8 @@ class OC_App {
$app1[$i] = OC_App::parseAppInfo($app);
$app1[$i]['author'] = $app['personid'];
$app1[$i]['ocs_id'] = $app['id'];
- $app1[$i]['internal'] = $app1[$i]['active'] = 0;
+ $app1[$i]['internal'] = 0;
+ $app1[$i]['active'] = self::isEnabled(\OC::$server->getConfig()->getAppsForKeyValue('ocsid', $app['id'])[0]);
$app1[$i]['update'] = false;
$app1[$i]['groups'] = false;
$app1[$i]['score'] = $app['score'];
@@ -1055,7 +1063,6 @@ class OC_App {
}
}
-
/**
* @param mixed $app
* @return bool
@@ -1075,8 +1082,21 @@ class OC_App {
} else {
$app = OC_Installer::installShippedApp($app);
}
- }else{
- $app = self::downloadApp($app);
+ } else {
+ // Maybe the app is already installed - compare the version in this
+ // case and use the local already installed one.
+ // FIXME: This is a horrible hack. I feel sad. The god of code cleanness may forgive me.
+ $internalAppId = $config->getAppsForKeyValue('ocsid', $app);
+ if(isset($internalAppId[0])) {
+ if($appData && version_compare(\OC_App::getAppVersion($internalAppId[0]), $appData['version'], '<')) {
+ $app = self::downloadApp($app);
+ } else {
+ self::enable($internalAppId[0]);
+ $app = $internalAppId[0];
+ }
+ } else {
+ $app = self::downloadApp($app);
+ }
}
if($app!==false) {
diff --git a/lib/public/iconfig.php b/lib/public/iconfig.php
index 868a413..06638d0 100644
--- a/lib/public/iconfig.php
+++ b/lib/public/iconfig.php
@@ -109,6 +109,15 @@ interface IConfig {
*/
public function deleteAppValues($appName);
+ /**
+ * Determines the apps that have the set the value for the specified
+ * keys
+ *
+ * @param string $value the value to get the app for
+ * @param string $key the key too lookup
+ * @return array of app IDs
+ */
+ public function getAppsForKeyValue($key, $value);
/**
* Set a user defined value
diff --git a/settings/ajax/disableapp.php b/settings/ajax/disableapp.php
index 1a133ea..be717e2 100644
--- a/settings/ajax/disableapp.php
+++ b/settings/ajax/disableapp.php
@@ -11,8 +11,7 @@ $appId = $_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::$server->getMemCacheFactory()->create('settings')->clear('listApps-');
OC_App::disable($appId);
OC_JSON::success();
diff --git a/settings/ajax/enableapp.php b/settings/ajax/enableapp.php
index 88abff4..fcfa278 100644
--- a/settings/ajax/enableapp.php
+++ b/settings/ajax/enableapp.php
@@ -8,8 +8,7 @@ $groups = isset($_POST['groups']) ? $_POST['groups'] : null;
try {
OC_App::enable(OC_App::cleanAppId($_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::$server->getMemCacheFactory()->create('settings')->clear('listApps-');
OC_JSON::success();
} catch (Exception $e) {
OC_Log::write('core', $e->getMessage(), OC_Log::ERROR);
diff --git a/tests/lib/allconfig.php b/tests/lib/allconfig.php
index 7f8ad5e..290e40e 100644
--- a/tests/lib/allconfig.php
+++ b/tests/lib/allconfig.php
@@ -26,6 +26,33 @@ class TestAllConfig extends \Test\TestCase {
return new \OC\AllConfig($systemConfig, $connection);
}
+ // FIXME: Actually an integration test... – Shouldn't be in the unit test at all.
+ public function testGetAppsForKeyValue() {
+ $config = $this->getConfig();
+
+ // preparation - add something to the database
+ $data = [
+ ['myFirstApp', 'key1', 'value1'],
+ ['mySecondApp', 'key1', 'value2'],
+ ['mySecondApp', 'key3', 'value2'],
+ ['myThirdApp', 'key3', 'value3'],
+ ['myThirdApp', 'key3', 'value2'],
+ ];
+ foreach ($data as $entry) {
+ $config->setAppValue($entry[0], $entry[1], $entry[2]);
+ }
+
+ $this->assertEquals(['mySecondApp'], $config->getAppsForKeyValue('key1', 'value2'));
+ $this->assertEquals(['mySecondApp', 'myThirdApp'], $config->getAppsForKeyValue('key3', 'value2'));
+ $this->assertEquals([], $config->getAppsForKeyValue('NotExisting', 'NotExistingValue'));
+
+ // cleanup
+ foreach ($data as $entry) {
+ $config->deleteAppValue($entry[0], $entry[1]);
+
+ }
+ }
+
public function testDeleteUserValue() {
$config = $this->getConfig();
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list