From 54f45f95f51dc14d6a7126170b3277a3ad57b608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 28 Apr 2016 10:07:29 +0200 Subject: [PATCH 1/5] Adding repair steps for install and uninstall - fixes #24306 --- lib/private/App/InfoParser.php | 12 ++++++++ lib/private/legacy/app.php | 2 +- lib/private/legacy/installer.php | 51 +++++++++++-------------------- tests/data/app/expected-info.json | 4 ++- 4 files changed, 33 insertions(+), 36 deletions(-) diff --git a/lib/private/App/InfoParser.php b/lib/private/App/InfoParser.php index b7540c0424..21422d4060 100644 --- a/lib/private/App/InfoParser.php +++ b/lib/private/App/InfoParser.php @@ -74,6 +74,9 @@ class InfoParser { if (!array_key_exists('repair-steps', $array)) { $array['repair-steps'] = []; } + if (!array_key_exists('install', $array['repair-steps'])) { + $array['repair-steps']['install'] = []; + } if (!array_key_exists('pre-migration', $array['repair-steps'])) { $array['repair-steps']['pre-migration'] = []; } @@ -83,6 +86,9 @@ class InfoParser { if (!array_key_exists('live-migration', $array['repair-steps'])) { $array['repair-steps']['live-migration'] = []; } + if (!array_key_exists('uninstall', $array['repair-steps'])) { + $array['repair-steps']['uninstall'] = []; + } if (array_key_exists('documentation', $array) && is_array($array['documentation'])) { foreach ($array['documentation'] as $key => $url) { @@ -107,6 +113,9 @@ class InfoParser { $array['types'] = []; } } + if (isset($array['repair-steps']['install']['step']) && is_array($array['repair-steps']['install']['step'])) { + $array['repair-steps']['install'] = $array['repair-steps']['install']['step']; + } if (isset($array['repair-steps']['pre-migration']['step']) && is_array($array['repair-steps']['pre-migration']['step'])) { $array['repair-steps']['pre-migration'] = $array['repair-steps']['pre-migration']['step']; } @@ -116,6 +125,9 @@ class InfoParser { if (isset($array['repair-steps']['live-migration']['step']) && is_array($array['repair-steps']['live-migration']['step'])) { $array['repair-steps']['live-migration'] = $array['repair-steps']['live-migration']['step']; } + if (isset($array['repair-steps']['uninstall']['step']) && is_array($array['repair-steps']['uninstall']['step'])) { + $array['repair-steps']['uninstall'] = $array['repair-steps']['uninstall']['step']; + } return $array; } diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 246bf97ee9..a1d4b7ce26 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -1187,7 +1187,7 @@ class OC_App { * @param string[] $steps * @throws \OC\NeedsUpdateException */ - private static function executeRepairSteps($appId, array $steps) { + public static function executeRepairSteps($appId, array $steps) { if (empty($steps)) { return; } diff --git a/lib/private/legacy/installer.php b/lib/private/legacy/installer.php index 24c79b2dd8..2bf95f3319 100644 --- a/lib/private/legacy/installer.php +++ b/lib/private/legacy/installer.php @@ -45,7 +45,7 @@ use OC\OCSClient; /** * This class provides the functionality needed to install, update and remove plugins/apps */ -class OC_Installer{ +class OC_Installer { /** * @@ -134,16 +134,19 @@ class OC_Installer{ self::includeAppScript($basedir . '/appinfo/install.php'); } + $appData = OC_App::getAppInfo($appId); + OC_App::executeRepairSteps($appId, $appData['repair-steps']['install']); + //set the installed version \OC::$server->getAppConfig()->setValue($info['id'], 'installed_version', OC_App::getAppVersion($info['id'])); \OC::$server->getAppConfig()->setValue($info['id'], 'enabled', 'no'); //set remote/public handelers foreach($info['remote'] as $name=>$path) { - OCP\CONFIG::setAppValue('core', 'remote_'.$name, $info['id'].'/'.$path); + OCP\Config::setAppValue('core', 'remote_'.$name, $info['id'].'/'.$path); } foreach($info['public'] as $name=>$path) { - OCP\CONFIG::setAppValue('core', 'public_'.$name, $info['id'].'/'.$path); + OCP\Config::setAppValue('core', 'public_'.$name, $info['id'].'/'.$path); } OC_App::setAppTypes($info['id']); @@ -474,52 +477,30 @@ class OC_Installer{ /** * Removes an app * @param string $name name of the application to remove - * @param array $options options * @return boolean * - * This function removes an app. $options is an associative array. The - * following keys are optional:ja - * - keeppreferences: boolean, if true the user preferences won't be deleted - * - keepappconfig: boolean, if true the config will be kept - * - keeptables: boolean, if true the database will be kept - * - keepfiles: boolean, if true the user files will be kept * * This function works as follows - * -# including appinfo/remove.php + * -# call uninstall repair steps * -# removing the files * * The function will not delete preferences, tables and the configuration, * this has to be done by the function oc_app_uninstall(). */ - public static function removeApp( $name, $options = array()) { + public static function removeApp($appId) { - if(isset($options['keeppreferences']) and $options['keeppreferences']==false ) { - // todo - // remove preferences + $appData = OC_App::getAppInfo($appId); + if (!is_null($appData)) { + OC_App::executeRepairSteps($appId, $appData['repair-steps']['uninstall']); } - if(isset($options['keepappconfig']) and $options['keepappconfig']==false ) { - // todo - // remove app config - } - - if(isset($options['keeptables']) and $options['keeptables']==false ) { - // todo - // remove app database tables - } - - if(isset($options['keepfiles']) and $options['keepfiles']==false ) { - // todo - // remove user files - } - - if(OC_Installer::isDownloaded( $name )) { - $appdir=OC_App::getInstallPath().'/'.$name; - OC_Helper::rmdirr($appdir); + if(OC_Installer::isDownloaded( $appId )) { + $appDir=OC_App::getInstallPath() . '/' . $appId; + OC_Helper::rmdirr($appDir); return true; }else{ - \OCP\Util::writeLog('core', 'can\'t remove app '.$name.'. It is not installed.', \OCP\Util::ERROR); + \OCP\Util::writeLog('core', 'can\'t remove app '.$appId.'. It is not installed.', \OCP\Util::ERROR); return false; } @@ -590,6 +571,8 @@ class OC_Installer{ return false; } + OC_App::executeRepairSteps($app, $info['repair-steps']['install']); + $config = \OC::$server->getConfig(); $config->setAppValue($app, 'installed_version', OC_App::getAppVersion($app)); diff --git a/tests/data/app/expected-info.json b/tests/data/app/expected-info.json index 51d0c00cce..cef7a7fdab 100644 --- a/tests/data/app/expected-info.json +++ b/tests/data/app/expected-info.json @@ -69,8 +69,10 @@ } }, "repair-steps": { + "install": [], "pre-migration": [], "post-migration": [], - "live-migration": [] + "live-migration": [], + "uninstall": [] } } From e049953d1af24feeb57796ac4025f0f607ac1cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 28 Apr 2016 15:15:34 +0200 Subject: [PATCH 2/5] OC_Installer -> \OC\Installer --- lib/private/Setup.php | 2 +- lib/private/Updater.php | 7 ++- lib/private/legacy/app.php | 11 ++--- lib/private/legacy/installer.php | 44 ++++++++++--------- settings/ajax/updateapp.php | 2 +- .../lib/{installer.php => InstallerTest.php} | 29 +++++++----- 6 files changed, 52 insertions(+), 43 deletions(-) rename tests/lib/{installer.php => InstallerTest.php} (74%) diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 196ae8a8bc..23c66f98b7 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -367,7 +367,7 @@ class Setup { \OC_User::login($username, $password); //guess what this does - \OC_Installer::installShippedApps(); + Installer::installShippedApps(); // create empty file in data dir, so we can later find // out that this is indeed an ownCloud data directory diff --git a/lib/private/Updater.php b/lib/private/Updater.php index 093ebebbbe..7ca3cd0936 100644 --- a/lib/private/Updater.php +++ b/lib/private/Updater.php @@ -36,7 +36,6 @@ namespace OC; use OC\Hooks\BasicEmitter; use OC\IntegrityCheck\Checker; use OC_App; -use OC_Installer; use OCP\IConfig; use OC\Setup; use OCP\ILogger; @@ -251,7 +250,7 @@ class Updater extends BasicEmitter { // install new shipped apps on upgrade OC_App::loadApps('authentication'); - $errors = OC_Installer::installShippedApps(true); + $errors = Installer::installShippedApps(true); foreach ($errors as $appId => $exception) { /** @var \Exception $exception */ $this->log->logException($exception, ['app' => $appId]); @@ -443,11 +442,11 @@ class Updater extends BasicEmitter { private function upgradeAppStoreApps(array $disabledApps) { foreach($disabledApps as $app) { try { - if (OC_Installer::isUpdateAvailable($app)) { + if (Installer::isUpdateAvailable($app)) { $ocsId = \OC::$server->getConfig()->getAppValue($app, 'ocsid', ''); $this->emit('\OC\Updater', 'upgradeAppStoreApp', array($app)); - OC_Installer::updateAppByOCSId($ocsId); + Installer::updateAppByOCSId($ocsId); } } catch (\Exception $ex) { $this->log->logException($ex, ['app' => 'core']); diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index a1d4b7ce26..42bb441228 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -46,6 +46,7 @@ */ use OC\App\DependencyAnalyzer; use OC\App\Platform; +use OC\Installer; use OC\OCSClient; use OC\Repair; @@ -304,7 +305,7 @@ class OC_App { */ public static function enable($app, $groups = null) { self::$enabledAppsCache = array(); // flush - if (!OC_Installer::isInstalled($app)) { + if (!Installer::isInstalled($app)) { $app = self::installApp($app); } @@ -340,7 +341,7 @@ class OC_App { // 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); + $app = Installer::installApp($info); } return $app; } @@ -354,7 +355,7 @@ class OC_App { return false; } - return OC_Installer::removeApp($app); + return Installer::removeApp($app); } /** @@ -827,7 +828,7 @@ class OC_App { $info['removable'] = true; } - $info['update'] = ($includeUpdateInfo) ? OC_Installer::isUpdateAvailable($app) : null; + $info['update'] = ($includeUpdateInfo) ? Installer::isUpdateAvailable($app) : null; $appPath = self::getAppPath($app); if($appPath !== false) { @@ -1073,7 +1074,7 @@ class OC_App { if ($appData && version_compare($shippedVersion, $appData['version'], '<')) { $app = self::downloadApp($app); } else { - $app = OC_Installer::installShippedApp($app); + $app = Installer::installShippedApp($app); } } else { // Maybe the app is already installed - compare the version in this diff --git a/lib/private/legacy/installer.php b/lib/private/legacy/installer.php index 2bf95f3319..cc80423b7d 100644 --- a/lib/private/legacy/installer.php +++ b/lib/private/legacy/installer.php @@ -37,15 +37,19 @@ * */ +namespace OC; + use OC\App\CodeChecker\CodeChecker; use OC\App\CodeChecker\EmptyCheck; use OC\App\CodeChecker\PrivateCheck; -use OC\OCSClient; +use OC_App; +use OC_DB; +use OC_Helper; /** * This class provides the functionality needed to install, update and remove plugins/apps */ -class OC_Installer { +class Installer { /** * @@ -138,15 +142,15 @@ class OC_Installer { OC_App::executeRepairSteps($appId, $appData['repair-steps']['install']); //set the installed version - \OC::$server->getAppConfig()->setValue($info['id'], 'installed_version', OC_App::getAppVersion($info['id'])); - \OC::$server->getAppConfig()->setValue($info['id'], 'enabled', 'no'); + \OC::$server->getConfig()->setAppValue($info['id'], 'installed_version', OC_App::getAppVersion($info['id'])); + \OC::$server->getConfig()->setAppValue($info['id'], 'enabled', 'no'); - //set remote/public handelers + //set remote/public handlers foreach($info['remote'] as $name=>$path) { - OCP\Config::setAppValue('core', 'remote_'.$name, $info['id'].'/'.$path); + \OC::$server->getConfig()->setAppValue('core', 'remote_'.$name, $info['id'].'/'.$path); } foreach($info['public'] as $name=>$path) { - OCP\Config::setAppValue('core', 'public_'.$name, $info['id'].'/'.$path); + \OC::$server->getConfig()->setAppValue('core', 'public_'.$name, $info['id'].'/'.$path); } OC_App::setAppTypes($info['id']); @@ -161,15 +165,15 @@ class OC_Installer { * * Checks whether or not an app is installed, i.e. registered in apps table. */ - public static function isInstalled( $app ) { - return (\OC::$server->getAppConfig()->getValue($app, "installed_version") !== null); + public static function isInstalled( $app ) { + return (\OC::$server->getConfig()->getAppValue($app, "installed_version", null) !== null); } /** * @brief Update an application * @param array $info * @param bool $isShipped - * @throws Exception + * @throws \Exception * @return bool * * This function could work like described below, but currently it disables and then @@ -232,7 +236,7 @@ class OC_Installer { * * @param integer $ocsId * @return bool - * @throws Exception + * @throws \Exception */ public static function updateAppByOCSId($ocsId) { $ocsClient = new OCSClient( @@ -260,7 +264,7 @@ class OC_Installer { /** * @param array $data * @return array - * @throws Exception + * @throws \Exception */ public static function downloadApp($data = array()) { $l = \OC::$server->getL10N('lib'); @@ -296,7 +300,7 @@ class OC_Installer { $extractDir = \OC::$server->getTempManager()->getTemporaryFolder(); OC_Helper::rmdirr($extractDir); mkdir($extractDir); - if($archive=OC_Archive::open($path)) { + if($archive=\OC_Archive::open($path)) { $archive->extract($extractDir); } else { OC_Helper::rmdirr($extractDir); @@ -378,7 +382,7 @@ class OC_Installer { } // check the code for not allowed calls - if(!$isShipped && !OC_Installer::checkCode($extractDir)) { + if(!$isShipped && !Installer::checkCode($extractDir)) { OC_Helper::rmdirr($extractDir); throw new \Exception($l->t("App can't be installed because of not allowed code in the App")); } @@ -460,7 +464,7 @@ class OC_Installer { * The function will check if the app is already downloaded in the apps repository */ public static function isDownloaded( $name ) { - foreach(OC::$APPSROOTS as $dir) { + foreach(\OC::$APPSROOTS as $dir) { $dirToTest = $dir['path']; $dirToTest .= '/'; $dirToTest .= $name; @@ -494,7 +498,7 @@ class OC_Installer { OC_App::executeRepairSteps($appId, $appData['repair-steps']['uninstall']); } - if(OC_Installer::isDownloaded( $appId )) { + if(Installer::isDownloaded( $appId )) { $appDir=OC_App::getInstallPath() . '/' . $appId; OC_Helper::rmdirr($appDir); @@ -517,25 +521,25 @@ class OC_Installer { */ public static function installShippedApps($softErrors = false) { $errors = []; - foreach(OC::$APPSROOTS as $app_dir) { + foreach(\OC::$APPSROOTS as $app_dir) { if($dir = opendir( $app_dir['path'] )) { while( false !== ( $filename = readdir( $dir ))) { if( substr( $filename, 0, 1 ) != '.' and is_dir($app_dir['path']."/$filename") ) { if( file_exists( $app_dir['path']."/$filename/appinfo/info.xml" )) { - if(!OC_Installer::isInstalled($filename)) { + if(!Installer::isInstalled($filename)) { $info=OC_App::getAppInfo($filename); $enabled = isset($info['default_enable']); if (($enabled || in_array($filename, \OC::$server->getAppManager()->getAlwaysEnabledApps())) && \OC::$server->getConfig()->getAppValue($filename, 'enabled') !== 'no') { if ($softErrors) { try { - OC_Installer::installShippedApp($filename); + Installer::installShippedApp($filename); } catch (\Doctrine\DBAL\Exception\TableExistsException $e) { $errors[$filename] = $e; continue; } } else { - OC_Installer::installShippedApp($filename); + Installer::installShippedApp($filename); } \OC::$server->getConfig()->setAppValue($filename, 'enabled', 'yes'); } diff --git a/settings/ajax/updateapp.php b/settings/ajax/updateapp.php index 8521914884..7ac32468fe 100644 --- a/settings/ajax/updateapp.php +++ b/settings/ajax/updateapp.php @@ -50,7 +50,7 @@ $appId = OC_App::cleanAppId($appId); $config = \OC::$server->getConfig(); $config->setSystemValue('maintenance', true); try { - $result = OC_Installer::updateAppByOCSId($appId); + $result = \OC\Installer::updateAppByOCSId($appId); $config->setSystemValue('maintenance', false); } catch(Exception $ex) { $config->setSystemValue('maintenance', false); diff --git a/tests/lib/installer.php b/tests/lib/InstallerTest.php similarity index 74% rename from tests/lib/installer.php rename to tests/lib/InstallerTest.php index ca210ba31e..e1c17b841a 100644 --- a/tests/lib/installer.php +++ b/tests/lib/InstallerTest.php @@ -6,7 +6,12 @@ * See the COPYING-README file. */ -class Test_Installer extends \Test\TestCase { +namespace Test; + + +use OC\Installer; + +class InstallerTest extends TestCase { private static $appid = 'testapp'; private $appstore; @@ -17,11 +22,11 @@ class Test_Installer extends \Test\TestCase { $config = \OC::$server->getConfig(); $this->appstore = $config->setSystemValue('appstoreenabled', true); $config->setSystemValue('appstoreenabled', true); - OC_Installer::removeApp(self::$appid); + Installer::removeApp(self::$appid); } protected function tearDown() { - OC_Installer::removeApp(self::$appid); + Installer::removeApp(self::$appid); \OC::$server->getConfig()->setSystemValue('appstoreenabled', $this->appstore); parent::tearDown(); @@ -33,7 +38,7 @@ class Test_Installer extends \Test\TestCase { $pathOfTestApp .= 'testapp.zip'; $tmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); - OC_Helper::copyr($pathOfTestApp, $tmp); + \OC_Helper::copyr($pathOfTestApp, $tmp); $data = array( 'path' => $tmp, @@ -44,8 +49,8 @@ class Test_Installer extends \Test\TestCase { ] ); - OC_Installer::installApp($data); - $isInstalled = OC_Installer::isInstalled(self::$appid); + Installer::installApp($data); + $isInstalled = Installer::isInstalled(self::$appid); $this->assertTrue($isInstalled); } @@ -56,7 +61,7 @@ class Test_Installer extends \Test\TestCase { $pathOfOldTestApp .= 'testapp.zip'; $oldTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); - OC_Helper::copyr($pathOfOldTestApp, $oldTmp); + \OC_Helper::copyr($pathOfOldTestApp, $oldTmp); $oldData = array( 'path' => $oldTmp, @@ -72,7 +77,7 @@ class Test_Installer extends \Test\TestCase { $pathOfNewTestApp .= 'testapp2.zip'; $newTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); - OC_Helper::copyr($pathOfNewTestApp, $newTmp); + \OC_Helper::copyr($pathOfNewTestApp, $newTmp); $newData = array( 'path' => $newTmp, @@ -83,11 +88,11 @@ class Test_Installer extends \Test\TestCase { ] ); - OC_Installer::installApp($oldData); - $oldVersionNumber = OC_App::getAppVersion(self::$appid); + Installer::installApp($oldData); + $oldVersionNumber = \OC_App::getAppVersion(self::$appid); - OC_Installer::updateApp($newData); - $newVersionNumber = OC_App::getAppVersion(self::$appid); + Installer::updateApp($newData); + $newVersionNumber = \OC_App::getAppVersion(self::$appid); $this->assertNotEquals($oldVersionNumber, $newVersionNumber); } From f91e5f87d2acfd0f2505e7d581d414efcc6c5f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 2 May 2016 09:06:19 +0200 Subject: [PATCH 3/5] Fix installer file location --- lib/private/{legacy/installer.php => Installer.php} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/private/{legacy/installer.php => Installer.php} (100%) diff --git a/lib/private/legacy/installer.php b/lib/private/Installer.php similarity index 100% rename from lib/private/legacy/installer.php rename to lib/private/Installer.php From b0211a54c43187602dde314a3ec990af11595190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 2 May 2016 09:17:20 +0200 Subject: [PATCH 4/5] Repair steps no longer implement the Emitter interface --- lib/private/Repair.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/private/Repair.php b/lib/private/Repair.php index 590b0bee72..5d10cd582f 100644 --- a/lib/private/Repair.php +++ b/lib/private/Repair.php @@ -73,7 +73,6 @@ class Repair implements IOutput{ * Run a series of repair steps for common problems */ public function run() { - $self = $this; if (count($this->repairSteps) === 0) { $this->emit('\OC\Repair', 'info', array('No repair steps available')); return; @@ -82,16 +81,6 @@ class Repair implements IOutput{ foreach ($this->repairSteps as $step) { $this->currentStep = $step->getName(); $this->emit('\OC\Repair', 'step', [$this->currentStep]); - - if ($step instanceof Emitter) { - $step->listen('\OC\Repair', 'warning', function ($description) use ($self) { - $self->emit('\OC\Repair', 'warning', array($description)); - }); - $step->listen('\OC\Repair', 'info', function ($description) use ($self) { - $self->emit('\OC\Repair', 'info', array($description)); - }); - } - $step->run($this); } } From 5e055ca6c1837a762aed7fe1e09b3a834fd50f20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 2 May 2016 09:22:26 +0200 Subject: [PATCH 5/5] Move uninstall repair step execution to the correct place --- lib/private/Installer.php | 5 ----- lib/private/legacy/app.php | 14 ++++++++++++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/private/Installer.php b/lib/private/Installer.php index cc80423b7d..643590ae22 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -493,11 +493,6 @@ class Installer { */ public static function removeApp($appId) { - $appData = OC_App::getAppInfo($appId); - if (!is_null($appData)) { - OC_App::executeRepairSteps($appId, $appData['repair-steps']['uninstall']); - } - if(Installer::isDownloaded( $appId )) { $appDir=OC_App::getInstallPath() . '/' . $appId; OC_Helper::rmdirr($appDir); diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 42bb441228..41b1b79e4f 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -370,9 +370,19 @@ class OC_App { $app = self::getInternalAppIdByOcs($app); } - self::$enabledAppsCache = array(); // flush - // check if app is a shipped app or not. if not delete + // flush + self::$enabledAppsCache = array(); + + // run uninstall steps + $appData = OC_App::getAppInfo($app); + if (!is_null($appData)) { + OC_App::executeRepairSteps($app, $appData['repair-steps']['uninstall']); + } + + // emit disable hook - needed anymore ? \OC_Hook::emit('OC_App', 'pre_disable', array('app' => $app)); + + // finally disable it $appManager = \OC::$server->getAppManager(); $appManager->disableApp($app); }