Make shipped apps updatable via appstore #8808
There are no files selected for viewing
This file contains hidden or 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 <[email protected]> | ||
| * | ||
| * @author Jakob Sack | ||
| * @copyright 2012 Jakob Sack <[email protected]> | ||
| * | ||
| * @author Georg Ehrke | ||
| * @copyright 2014 Georg Ehrke <[email protected]> | ||
| * | ||
| * This library is free software; you can redistribute it and/or | ||
| * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE | ||
|
|
@@ -211,48 +216,45 @@ public static function isEnabled( $app ) { | |
| */ | ||
| public static function enable( $app ) { | ||
| self::$enabledAppsCache = array(); // flush | ||
| if (!OC_Installer::isInstalled($app)) { | ||
| $app = self::installApp($app); | ||
| } | ||
|
|
||
| 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']); | ||
| $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); | ||
| } | ||
|
|
||
| /** | ||
| * disables an app | ||
| * @param string $app app | ||
| * @return null | ||
|
Contributor
There was a problem hiding this comment. drop the return statement, because there is no return inside of this function |
||
| * | ||
| * This function set an app as disabled in appconfig. | ||
|
Contributor
There was a problem hiding this comment. Merge this line with the line above |
||
| */ | ||
|
|
@@ -261,11 +263,6 @@ public static function disable( $app ) { | |
| // 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' ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -446,18 +443,50 @@ 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(); | ||
|
Contributor
There was a problem hiding this comment. static should be in the object instead of the method?
Contributor
Author
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 (count($possibleApps) === 0) { | ||
|
Contributor
There was a problem hiding this comment.
|
||
| return false; | ||
| } elseif(count($possibleApps) === 1) { | ||
| reset($possibleApps); | ||
| $dir = current($possibleApps); | ||
|
Contributor
There was a problem hiding this comment.
Contributor
There was a problem hiding this comment. Or even better, just |
||
| $app_dir[$appId] = $dir; | ||
|
Contributor
There was a problem hiding this comment. Remove this line,
Contributor
Author
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( | ||
|
Contributor
There was a problem hiding this comment. Just
Contributor
Author
There was a problem hiding this comment. but we want to load the newest version, that's why we need to check all
Contributor
There was a problem hiding this comment. We should load the "running" version, not the newest one?
Contributor
Author
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
Contributor
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']; | ||
|
Contributor
There was a problem hiding this comment. invalid (it's a static variable of the method) |
||
| return $versionToLoad['dir']; | ||
| //TODO - write test | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * Get the directory for the given app. | ||
| * If the app is defined in multiple directories, the first one is taken. (false if not found) | ||
|
|
@@ -471,6 +500,18 @@ 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 is_writable($path); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * 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) | ||
|
|
@@ -490,15 +531,28 @@ public static function getAppWebPath($appid) { | |
| * @return string | ||
| */ | ||
| public static function getAppVersion($appid) { | ||
| $file = self::getAppPath($appid); | ||
| return self::getAppVersionByPath($file); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * 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) && $version = trim(file_get_contents($versionFile))) { | ||
|
Contributor
There was a problem hiding this comment. don't use inline assignments, just do |
||
| return $version; | ||
| }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 | ||
|
|
@@ -513,7 +567,7 @@ public static function getAppInfo($appid, $path=false) { | |
| if(isset(self::$appInfo[$appid])) { | ||
| return self::$appInfo[$appid]; | ||
| } | ||
| $file = self::getAppPath($appid) . '/appinfo/info.xml'; | ||
| } | ||
| $data=array(); | ||
| $content=@file_get_contents($file); | ||
|
|
@@ -602,7 +656,6 @@ public static function getCurrentApp() { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * get the forms for either settings, admin or personal | ||
| */ | ||
|
|
@@ -726,20 +779,22 @@ public static function listAllApps() { | |
| $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; | ||
| } | ||
| } | ||
| $remoteApps = self::getAppstoreApps(); | ||
| if ( $remoteApps ) { | ||
| // Remove duplicates | ||
| foreach ( $appList as $app ) { | ||
|
|
@@ -818,6 +873,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'; | ||
|
|
@@ -828,17 +884,29 @@ public static function getAppstoreApps( $filter = 'approved' ) { | |
|
|
||
|
|
||
| // rating img | ||
| if ($app['score'] >= 0 && $app['score'] < 5) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s1.png" ); | ||
| } elseif ($app['score'] >= 5 && $app['score'] < 15) { | ||
|
Contributor
There was a problem hiding this comment. Please remove all |
||
| $img = OC_Helper::imagePath( "core", "rating/s2.png" ); | ||
| } elseif($app['score'] >= 15 && $app['score'] < 25) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s3.png" ); | ||
| } elseif($app['score'] >= 25 && $app['score'] < 35) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s4.png" ); | ||
| } elseif($app['score'] >= 35 && $app['score'] < 45) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s5.png" ); | ||
| } elseif($app['score'] >= 45 && $app['score'] < 55) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s6.png" ); | ||
| } elseif($app['score'] >= 55 && $app['score'] < 65) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s7.png" ); | ||
| } elseif($app['score'] >= 65 && $app['score'] < 75) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s8.png" ); | ||
| } elseif($app['score'] >= 75 && $app['score'] < 85) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s9.png" ); | ||
| } elseif($app['score'] >= 85 && $app['score'] < 95) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s10.png" ); | ||
| } elseif($app['score'] >= 95 && $app['score'] < 100) { | ||
| $img = OC_Helper::imagePath( "core", "rating/s11.png" ); | ||
| } | ||
|
|
||
| $app1[$i]['score'] = '<img src="'.$img.'"> Score: '.$app['score'].'%'; | ||
| $i++; | ||
|
|
@@ -1007,9 +1075,55 @@ public static function getAppVersions() { | |
| return $versions; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * @param mixed $app | ||
| * @return bool | ||
|
Contributor
There was a problem hiding this comment. Add
Contributor
Author
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
Contributor
There was a problem hiding this comment. the case would be great |
||
| */ | ||
| 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.", | ||
|
Contributor
There was a problem hiding this comment. Use single quotes, so you dont need to escape the double quotes: |
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. I can't comment on the line bellow: In line 1166 there is just a |
||
| */ | ||
| public static function updateApp($appid) { | ||
| if(file_exists(self::getAppPath($appid).'/appinfo/preupdate.php')) { | ||
|
|
@@ -1020,7 +1134,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); | ||
|
|
@@ -1037,6 +1151,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.
Are we sure there aren't any other special characters in the download links?
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.
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