Make shipped apps updatable via appstore #8808
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,12 +89,11 @@ public function findClass($class) { | |
| } elseif (strpos($class, 'OCA\\') === 0) { | ||
| list(, $app, $rest) = explode('\\', $class, 3); | ||
| $app = strtolower($app); | ||
| $appPath = \OC_App::getAppPath($app); | ||
| if ($appPath && stream_resolve_include_path($appPath)) { | ||
| $paths[] = $appPath . '/' . strtolower(str_replace('\\', '/', $rest) . '.php'); | ||
| // If not found in the root of the app directory, insert '/lib' after app id and try again. | ||
| $paths[] = $appPath . '/lib/' . strtolower(str_replace('\\', '/', $rest) . '.php'); | ||
| } | ||
|
There was a problem hiding this comment. @bantu @icewind1991 can you help double-checking these auto-loader changes ? (I'm not familiar with autoloader, so not sure if this could break something else) |
||
| } elseif (strpos($class, 'Test_') === 0) { | ||
| $paths[] = 'tests/lib/' . strtolower(str_replace('_', '/', substr($class, 5)) . '.php'); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,13 @@ | |
| * ownCloud | ||
| * | ||
| * @author Frank Karlitschek | ||
| * @copyright 2012 Frank Karlitschek <frank@owncloud.org> | ||
| * | ||
| * @author Jakob Sack | ||
| * @copyright 2012 Jakob Sack <mail@jakobsack.de> | ||
| * | ||
| * @author Georg Ehrke | ||
| * @copyright 2014 Georg Ehrke <georg@ownCloud.com> | ||
| * | ||
| * This library is free software; you can redistribute it and/or | ||
| * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE | ||
|
|
@@ -221,64 +226,53 @@ public static function isEnabled($app) { | |
| public static function enable($app, $groups = null) { | ||
| self::$enabledAppsCache = array(); // flush | ||
| if (!OC_Installer::isInstalled($app)) { | ||
| $app = self::installApp($app); | ||
| } | ||
|
|
||
| if (!is_null($groups)) { | ||
| OC_Appconfig::setValue($app, 'enabled', json_encode($groups)); | ||
| }else{ | ||
| OC_Appconfig::setValue($app, 'enabled', 'yes'); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param string $app | ||
| * @return int | ||
| */ | ||
| public static function downloadApp($app) { | ||
| $appdata=OC_OCSClient::getApplication($app); | ||
| $download=OC_OCSClient::getApplicationDownload($app, 1); | ||
| if(isset($download['downloadlink']) and $download['downloadlink']!='') { | ||
| // Replace spaces in download link without encoding entire URL | ||
| $download['downloadlink'] = str_replace(' ', '%20', $download['downloadlink']); | ||
|
There was a problem hiding this comment. Are we sure there aren't any other special characters in the download links? There was a problem hiding this comment. just from moved this code here from another place (https://github.com/owncloud/core/pull/8808/files#diff-4828f39ae4935d14b1dc1a6ffe323725L232) works in current versions, so apparently it's just fine to escape spaces |
||
| $info = array('source'=>'http', 'href'=>$download['downloadlink'], 'appdata'=>$appdata); | ||
| $app=OC_Installer::installApp($info); | ||
| } | ||
| return $app; | ||
| } | ||
|
|
||
| /** | ||
| * @param string $app | ||
| * @return bool | ||
| */ | ||
| public static function removeApp($app) { | ||
| if (self::isShipped($app)) { | ||
| return false; | ||
| } | ||
|
|
||
| return OC_Installer::removeApp($app); | ||
| } | ||
|
|
||
| /** | ||
| * This function set an app as disabled in appconfig. | ||
|
There was a problem hiding this comment. Merge this line with the line above |
||
| * @param string $app app | ||
| */ | ||
| public static function disable($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' ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -461,17 +455,46 @@ public static function getInstallPath() { | |
| } | ||
|
|
||
|
|
||
| /** | ||
| * search for an app in all app-directories | ||
| * @param $appId | ||
| * @return mixed (bool|string) | ||
| */ | ||
| protected static function findAppInDirectories($appId) { | ||
| static $app_dir = array(); | ||
|
There was a problem hiding this comment. static should be in the object instead of the method? There was a problem hiding this comment. works both, didn't write this piece of code. This is how it is in master |
||
|
|
||
| if (isset($app_dir[$appId])) { | ||
| return $app_dir[$appId]; | ||
| } | ||
|
|
||
| $possibleApps = array(); | ||
| foreach(OC::$APPSROOTS as $dir) { | ||
| if(file_exists($dir['path'] . '/' . $appId)) { | ||
| $possibleApps[] = $dir; | ||
| } | ||
| } | ||
|
|
||
| if (empty($possibleApps)) { | ||
| return false; | ||
| } elseif(count($possibleApps) === 1) { | ||
| $dir = array_shift($possibleApps); | ||
| $app_dir[$appId] = $dir; | ||
|
There was a problem hiding this comment. Remove this line, There was a problem hiding this comment. |
||
| return $dir; | ||
| } else { | ||
| $versionToLoad = array(); | ||
| foreach($possibleApps as $possibleApp) { | ||
| $version = self::getAppVersionByPath($possibleApp['path']); | ||
| if (empty($versionToLoad) || version_compare($version, $versionToLoad['version'], '>')) { | ||
| $versionToLoad = array( | ||
|
There was a problem hiding this comment. Just There was a problem hiding this comment. but we want to load the newest version, that's why we need to check all There was a problem hiding this comment. We should load the "running" version, not the newest one? There was a problem hiding this comment. you can't run multiple app versions that use the same appid, that's why we search for the app with the highest version num There was a problem hiding this comment. Ah okay, is that the same way the core finds the classes of apps? |
||
| 'dir' => $possibleApp, | ||
| 'version' => $version, | ||
| ); | ||
| } | ||
| } | ||
| $app_dir[$appId] = $versionToLoad['dir']; | ||
|
There was a problem hiding this comment. invalid (it's a static variable of the method) |
||
| return $versionToLoad['dir']; | ||
| //TODO - write test | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -487,6 +510,17 @@ public static function getAppPath($appid) { | |
| return false; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * check if an app's directory is writable | ||
| * @param $appid | ||
| * @return bool | ||
| */ | ||
| public static function isAppDirWritable($appid) { | ||
| $path = self::getAppPath($appid); | ||
| return ($path !== false) ? is_writable($path) : false; | ||
| } | ||
|
|
||
| /** | ||
| * Get the path for the given app on the access | ||
| * If the app is defined in multiple directories, the first one is taken. (false if not found) | ||
|
|
@@ -506,15 +540,27 @@ public static function getAppWebPath($appid) { | |
| * @return string | ||
| */ | ||
| public static function getAppVersion($appid) { | ||
| $file = self::getAppPath($appid); | ||
| return ($file !== false) ? self::getAppVersionByPath($file) : '0'; | ||
| } | ||
|
|
||
| /** | ||
| * get app's version based on it's path | ||
| * @param string $path | ||
| * @return string | ||
| */ | ||
| public static function getAppVersionByPath($path) { | ||
| $versionFile = $path . '/appinfo/version'; | ||
| $infoFile = $path . '/appinfo/info.xml'; | ||
| if(is_file($versionFile)) { | ||
| return trim(file_get_contents($versionFile)); | ||
| }else{ | ||
| $appData = self::getAppInfo($infoFile, true); | ||
| return isset($appData['version']) ? $appData['version'] : ''; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Read all app metadata from the info.xml file | ||
| * @param string $appid id of the app or the path of the info.xml file | ||
|
|
@@ -619,7 +665,6 @@ public static function getCurrentApp() { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * get the forms for either settings, admin or personal | ||
| */ | ||
|
|
@@ -745,18 +790,20 @@ public static function listAllApps() { | |
|
|
||
| $info['active'] = $active; | ||
|
|
||
| if(isset($info['shipped']) and ($info['shipped'] == 'true')) { | ||
| $info['internal'] = true; | ||
| $info['internallabel'] = 'Internal App'; | ||
| $info['internalclass'] = ''; | ||
| $info['removable'] = false; | ||
| } else { | ||
| $info['internal'] = false; | ||
| $info['internallabel'] = '3rd Party'; | ||
| $info['internalclass'] = 'externalapp'; | ||
| $info['removable'] = true; | ||
| } | ||
|
|
||
| $info['update'] = OC_Installer::isUpdateAvailable($app); | ||
|
|
||
| $info['preview'] = OC_Helper::imagePath('settings', 'trans.png'); | ||
| $info['version'] = OC_App::getAppVersion($app); | ||
| $appList[] = $info; | ||
|
|
@@ -841,6 +888,7 @@ public static function getAppstoreApps($filter = 'approved') { | |
| $app1[$i]['ocs_id'] = $app['id']; | ||
| $app1[$i]['internal'] = $app1[$i]['active'] = 0; | ||
| $app1[$i]['update'] = false; | ||
| $app1[$i]['removable'] = false; | ||
| if ($app['label'] == 'recommended') { | ||
| $app1[$i]['internallabel'] = 'Recommended'; | ||
| $app1[$i]['internalclass'] = 'recommendedapp'; | ||
|
|
@@ -851,17 +899,29 @@ public static function getAppstoreApps($filter = 'approved') { | |
|
|
||
|
|
||
| // rating img | ||
| if ($app['score'] < 5) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s1.png" ); | ||
| } elseif ($app['score'] < 15) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s2.png" ); | ||
| } elseif($app['score'] < 25) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s3.png" ); | ||
| } elseif($app['score'] < 35) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s4.png" ); | ||
| } elseif($app['score'] < 45) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s5.png" ); | ||
| } elseif($app['score'] < 55) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s6.png" ); | ||
| } elseif($app['score'] < 65) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s7.png" ); | ||
| } elseif($app['score'] < 75) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s8.png" ); | ||
| } elseif($app['score'] < 85) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s9.png" ); | ||
| } elseif($app['score'] < 95) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s10.png" ); | ||
| } elseif($app['score'] < 100) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s11.png" ); | ||
| } | ||
|
|
||
| $app1[$i]['score'] = '<img src="' . $img . '"> Score: ' . $app['score'] . '%'; | ||
| $i++; | ||
|
|
@@ -1043,10 +1103,58 @@ public static function getAppVersions() { | |
| return $versions; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * @param mixed $app | ||
| * @return bool | ||
|
There was a problem hiding this comment. Add There was a problem hiding this comment. just @throws \Exception or is it supposed to have a description in which certain cases an exception is thrown There was a problem hiding this comment. the case would be great |
||
| * @throws Exception if app is not compatible with this version of ownCloud | ||
| * @throws Exception if no app-name was specified | ||
| */ | ||
| public static function installApp($app) { | ||
| $l = OC_L10N::get('core'); | ||
| $appdata=OC_OCSClient::getApplication($app); | ||
|
|
||
| // check if app is a shipped app or not. OCS apps have an integer as id, shipped apps use a string | ||
| if(!is_numeric($app)) { | ||
| $shippedVersion=self::getAppVersion($app); | ||
| if($appdata && version_compare($shippedVersion, $appdata['version'], '<')) { | ||
| $app = self::downloadApp($app); | ||
| } else { | ||
| $app = OC_Installer::installShippedApp($app); | ||
| } | ||
| }else{ | ||
| $app = self::downloadApp($app); | ||
| } | ||
|
|
||
| if($app!==false) { | ||
| // check if the app is compatible with this version of ownCloud | ||
| $info = self::getAppInfo($app); | ||
| $version=OC_Util::getVersion(); | ||
| if(!self::isAppCompatible($version, $info)) { | ||
| throw new \Exception( | ||
| $l->t('App \"%s\" can\'t be installed because it is not compatible with this version of ownCloud.', | ||
| array($info['name']) | ||
| ) | ||
| ); | ||
| }else{ | ||
| OC_Appconfig::setValue( $app, 'enabled', 'yes' ); | ||
| if(isset($appdata['id'])) { | ||
| OC_Appconfig::setValue( $app, 'ocsid', $appdata['id'] ); | ||
| } | ||
| \OC_Hook::emit('OC_App', 'post_enable', array('app' => $app)); | ||
| } | ||
| }else{ | ||
| throw new \Exception($l->t("No app name specified")); | ||
| } | ||
|
|
||
| return $app; | ||
| } | ||
|
|
||
| /** | ||
| * update the database for the app and call the update script | ||
| * | ||
| * @param string $appid | ||
| * @return bool | ||
|
There was a problem hiding this comment. I can't comment on the line bellow: In line 1166 there is just a There was a problem hiding this comment. fixed with 6bf0689 |
||
| */ | ||
| public static function updateApp($appid) { | ||
| if (file_exists(self::getAppPath($appid) . '/appinfo/preupdate.php')) { | ||
|
|
@@ -1057,7 +1165,7 @@ public static function updateApp($appid) { | |
| OC_DB::updateDbFromStructure(self::getAppPath($appid) . '/appinfo/database.xml'); | ||
| } | ||
| if (!self::isEnabled($appid)) { | ||
| return false; | ||
| } | ||
| if (file_exists(self::getAppPath($appid) . '/appinfo/update.php')) { | ||
| self::loadApp($appid); | ||
|
|
@@ -1074,6 +1182,8 @@ public static function updateApp($appid) { | |
| } | ||
|
|
||
| self::setAppTypes($appid); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like $appPath defined by \OC_App::getAppPath($app) on line 92 can also be of type false; however, stream_resolve_include_path() does only seem to accept string, maybe add an additional type check?