From 9b652ed5d50a40a1edde0f1b8c859f49ff8248c9 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Sun, 16 Aug 2015 17:49:45 +0200 Subject: [PATCH] [App Code Check] add check for version and mandatory fields * ref #17598 * including unit tests for mandatory fields/versions --- core/command/app/checkcode.php | 69 ++++++++- core/register_command.php | 3 +- lib/private/app/codechecker/infochecker.php | 141 ++++++++++++++++++ .../appinfo/info.xml | 9 ++ .../appinfo/version | 1 + .../testapp-infoxml-version/appinfo/info.xml | 9 ++ .../testapp-infoxml-version/appinfo/version | 1 + tests/apps/testapp-infoxml/appinfo/info.xml | 9 ++ .../testapp-name-missing/appinfo/info.xml | 8 + .../testapp-version-missing/appinfo/info.xml | 8 + tests/apps/testapp-version/appinfo/info.xml | 8 + tests/apps/testapp-version/appinfo/version | 1 + tests/lib/app/codechecker/infocheckertest.php | 73 +++++++++ 13 files changed, 338 insertions(+), 2 deletions(-) create mode 100644 lib/private/app/codechecker/infochecker.php create mode 100644 tests/apps/testapp-infoxml-version-different/appinfo/info.xml create mode 100644 tests/apps/testapp-infoxml-version-different/appinfo/version create mode 100644 tests/apps/testapp-infoxml-version/appinfo/info.xml create mode 100644 tests/apps/testapp-infoxml-version/appinfo/version create mode 100644 tests/apps/testapp-infoxml/appinfo/info.xml create mode 100644 tests/apps/testapp-name-missing/appinfo/info.xml create mode 100644 tests/apps/testapp-version-missing/appinfo/info.xml create mode 100644 tests/apps/testapp-version/appinfo/info.xml create mode 100644 tests/apps/testapp-version/appinfo/version create mode 100644 tests/lib/app/codechecker/infocheckertest.php diff --git a/core/command/app/checkcode.php b/core/command/app/checkcode.php index a4e7322460f..0db9958f387 100644 --- a/core/command/app/checkcode.php +++ b/core/command/app/checkcode.php @@ -26,6 +26,8 @@ namespace OC\Core\Command\App; use OC\App\CodeChecker\CodeChecker; use OC\App\CodeChecker\EmptyCheck; +use OC\App\CodeChecker\InfoChecker; +use OC\App\InfoParser; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -33,12 +35,21 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class CheckCode extends Command { + + /** @var InfoParser */ + private $infoParser; + protected $checkers = [ 'private' => '\OC\App\CodeChecker\PrivateCheck', 'deprecation' => '\OC\App\CodeChecker\DeprecationCheck', 'strong-comparison' => '\OC\App\CodeChecker\StrongComparisonCheck', ]; + public function __construct(InfoParser $infoParser) { + parent::__construct(); + $this->infoParser = $infoParser; + } + protected function configure() { $this ->setName('app:check-code') @@ -54,6 +65,12 @@ class CheckCode extends Command { InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'enable the specified checker(s)', [ 'private', 'deprecation', 'strong-comparison' ] + ) + ->addOption( + '--skip-validate-info', + null, + InputOption::VALUE_NONE, + 'skips the info.xml/version check' ); } @@ -84,7 +101,7 @@ class CheckCode extends Command { $output->writeln("Analysing {$filename}"); } - // show error count if there are errros present or the verbosity is high + // show error count if there are errors present or the verbosity is high if($count > 0 || OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) { $output->writeln(" {$count} errors"); } @@ -98,6 +115,56 @@ class CheckCode extends Command { } }); $errors = $codeChecker->analyse($appId); + + if(!$input->getOption('skip-validate-info')) { + $infoChecker = new InfoChecker($this->infoParser); + + $infoChecker->listen('InfoChecker', 'mandatoryFieldMissing', function($key) use ($output) { + $output->writeln("Mandatory field missing: $key"); + }); + + $infoChecker->listen('InfoChecker', 'deprecatedFieldFound', function($key, $value) use ($output) { + if($value === [] || is_null($value) || $value === '') { + $output->writeln("Deprecated field available: $key"); + } else { + if(is_array($value)) { + $value = 'Array of ' . count($value) . ' element(s)'; + } + $output->writeln("Deprecated field available: $key => $value"); + } + }); + + $infoChecker->listen('InfoChecker', 'differentVersions', function($versionFile, $infoXML) use ($output) { + $output->writeln("Different versions provided (appinfo/version: $versionFile - appinfo/info.xml: $infoXML)"); + }); + + $infoChecker->listen('InfoChecker', 'sameVersions', function($path) use ($output) { + $output->writeln("Version file isn't needed anymore and can be safely removed ($path)"); + }); + + $infoChecker->listen('InfoChecker', 'migrateVersion', function($version) use ($output) { + $output->writeln("Migrate the app version to appinfo/info.xml (add $version to appinfo/info.xml and remove appinfo/version)"); + }); + + if(OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) { + $infoChecker->listen('InfoChecker', 'mandatoryFieldFound', function($key, $value) use ($output) { + $output->writeln("Mandatory field available: $key => $value"); + }); + + $infoChecker->listen('InfoChecker', 'optionalFieldFound', function($key, $value) use ($output) { + $output->writeln("Optional field available: $key => $value"); + }); + + $infoChecker->listen('InfoChecker', 'unusedFieldFound', function($key, $value) use ($output) { + $output->writeln("Unused field available: $key => $value"); + }); + } + + $infoErrors = $infoChecker->analyse($appId); + + $errors = array_merge($errors, $infoErrors); + } + if (empty($errors)) { $output->writeln('App is compliant - awesome job!'); return 0; diff --git a/core/register_command.php b/core/register_command.php index 114e115c491..878542f72c9 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -28,7 +28,8 @@ /** @var $application Symfony\Component\Console\Application */ $application->add(new OC\Core\Command\Status); $application->add(new OC\Core\Command\Check(\OC::$server->getConfig())); -$application->add(new OC\Core\Command\App\CheckCode()); +$infoParser = new \OC\App\InfoParser(\OC::$server->getHTTPHelper(), \OC::$server->getURLGenerator()); +$application->add(new OC\Core\Command\App\CheckCode($infoParser)); $application->add(new OC\Core\Command\L10n\CreateJs()); if (\OC::$server->getConfig()->getSystemValue('installed', false)) { diff --git a/lib/private/app/codechecker/infochecker.php b/lib/private/app/codechecker/infochecker.php new file mode 100644 index 00000000000..2af72ebedc3 --- /dev/null +++ b/lib/private/app/codechecker/infochecker.php @@ -0,0 +1,141 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\App\CodeChecker; + +use OC\App\InfoParser; +use OC\Hooks\BasicEmitter; + +class InfoChecker extends BasicEmitter { + + /** @var InfoParser */ + private $infoParser; + + private $mandatoryFields = [ + 'author', + 'description', + 'id', + 'licence', + 'name', + ]; + private $optionalFields = [ + 'bugs', + 'category', + 'documentation', + 'namespace', + 'ocsid', + 'repository', + 'require', + 'requiremin', + 'types', + 'version', + 'website', + ]; + private $deprecatedFields = [ + 'default_enable', + 'public', + 'remote', + 'shipped', + 'standalone', + ]; + + public function __construct(InfoParser $infoParser) { + $this->infoParser = $infoParser; + } + + /** + * @param string $appId + * @return array + */ + public function analyse($appId) { + $appPath = \OC_App::getAppPath($appId); + if ($appPath === false) { + throw new \RuntimeException("No app with given id <$appId> known."); + } + + $errors = []; + + $info = $this->infoParser->parse($appPath . '/appinfo/info.xml'); + + foreach ($info as $key => $value) { + if (in_array($key, $this->mandatoryFields)) { + $this->emit('InfoChecker', 'mandatoryFieldFound', [$key, $value]); + continue; + } + + if (in_array($key, $this->optionalFields)) { + $this->emit('InfoChecker', 'optionalFieldFound', [$key, $value]); + continue; + } + + if (in_array($key, $this->deprecatedFields)) { + // skip empty arrays - empty arrays for remote and public are always added + if($value === []) { + continue; + } + $this->emit('InfoChecker', 'deprecatedFieldFound', [$key, $value]); + continue; + } + + $this->emit('InfoChecker', 'unusedFieldFound', [$key, $value]); + } + + foreach ($this->mandatoryFields as $key) { + if(!isset($info[$key])) { + $this->emit('InfoChecker', 'mandatoryFieldMissing', [$key]); + $errors[] = [ + 'type' => 'mandatoryFieldMissing', + 'field' => $key, + ]; + } + } + + $versionFile = $appPath . '/appinfo/version'; + if (is_file($versionFile)) { + $version = trim(file_get_contents($versionFile)); + if(isset($info['version'])) { + if($info['version'] !== $version) { + $this->emit('InfoChecker', 'differentVersions', + [$version, $info['version']]); + $errors[] = [ + 'type' => 'differentVersions', + 'message' => 'appinfo/version: ' . $version . + ' - appinfo/info.xml: ' . $info['version'], + ]; + } else { + $this->emit('InfoChecker', 'sameVersions', [$versionFile]); + } + } else { + $this->emit('InfoChecker', 'migrateVersion', [$version]); + } + } else { + if(!isset($info['version'])) { + $this->emit('InfoChecker', 'mandatoryFieldMissing', ['version']); + $errors[] = [ + 'type' => 'mandatoryFieldMissing', + 'field' => 'version', + ]; + } + } + + return $errors; + } +} diff --git a/tests/apps/testapp-infoxml-version-different/appinfo/info.xml b/tests/apps/testapp-infoxml-version-different/appinfo/info.xml new file mode 100644 index 00000000000..c765400a76f --- /dev/null +++ b/tests/apps/testapp-infoxml-version-different/appinfo/info.xml @@ -0,0 +1,9 @@ + + + testapp-infoxml-version + 1.2.3 + Jane + A b c + Abc + Test app + diff --git a/tests/apps/testapp-infoxml-version-different/appinfo/version b/tests/apps/testapp-infoxml-version-different/appinfo/version new file mode 100644 index 00000000000..e8ea05db814 --- /dev/null +++ b/tests/apps/testapp-infoxml-version-different/appinfo/version @@ -0,0 +1 @@ +1.2.4 diff --git a/tests/apps/testapp-infoxml-version/appinfo/info.xml b/tests/apps/testapp-infoxml-version/appinfo/info.xml new file mode 100644 index 00000000000..c765400a76f --- /dev/null +++ b/tests/apps/testapp-infoxml-version/appinfo/info.xml @@ -0,0 +1,9 @@ + + + testapp-infoxml-version + 1.2.3 + Jane + A b c + Abc + Test app + diff --git a/tests/apps/testapp-infoxml-version/appinfo/version b/tests/apps/testapp-infoxml-version/appinfo/version new file mode 100644 index 00000000000..0495c4a88ca --- /dev/null +++ b/tests/apps/testapp-infoxml-version/appinfo/version @@ -0,0 +1 @@ +1.2.3 diff --git a/tests/apps/testapp-infoxml/appinfo/info.xml b/tests/apps/testapp-infoxml/appinfo/info.xml new file mode 100644 index 00000000000..cb63a0fc76e --- /dev/null +++ b/tests/apps/testapp-infoxml/appinfo/info.xml @@ -0,0 +1,9 @@ + + + testapp-infoxml + 1.2.3 + Jane + A b c + Abc + Test app + diff --git a/tests/apps/testapp-name-missing/appinfo/info.xml b/tests/apps/testapp-name-missing/appinfo/info.xml new file mode 100644 index 00000000000..f0a62b8d380 --- /dev/null +++ b/tests/apps/testapp-name-missing/appinfo/info.xml @@ -0,0 +1,8 @@ + + + testapp-version + 1.1.1 + Jane + A b c + Abc + diff --git a/tests/apps/testapp-version-missing/appinfo/info.xml b/tests/apps/testapp-version-missing/appinfo/info.xml new file mode 100644 index 00000000000..d7da3e07e36 --- /dev/null +++ b/tests/apps/testapp-version-missing/appinfo/info.xml @@ -0,0 +1,8 @@ + + + testapp-version + Jane + A b c + Abc + Test app + diff --git a/tests/apps/testapp-version/appinfo/info.xml b/tests/apps/testapp-version/appinfo/info.xml new file mode 100644 index 00000000000..d7da3e07e36 --- /dev/null +++ b/tests/apps/testapp-version/appinfo/info.xml @@ -0,0 +1,8 @@ + + + testapp-version + Jane + A b c + Abc + Test app + diff --git a/tests/apps/testapp-version/appinfo/version b/tests/apps/testapp-version/appinfo/version new file mode 100644 index 00000000000..0495c4a88ca --- /dev/null +++ b/tests/apps/testapp-version/appinfo/version @@ -0,0 +1 @@ +1.2.3 diff --git a/tests/lib/app/codechecker/infocheckertest.php b/tests/lib/app/codechecker/infocheckertest.php new file mode 100644 index 00000000000..59c1316b769 --- /dev/null +++ b/tests/lib/app/codechecker/infocheckertest.php @@ -0,0 +1,73 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\App\CodeChecker; + +use OC\App\InfoParser; +use Test\TestCase; + +class InfoCheckerTest extends TestCase { + /** @var InfoChecker */ + protected $infoChecker; + + public static function setUpBeforeClass() { + \OC::$APPSROOTS[] = [ + 'path' => \OC::$SERVERROOT . '/tests/apps', + 'url' => '/apps-test', + 'writable' => false, + ]; + } + + public static function tearDownAfterClass() { + // remove last element + array_pop(\OC::$APPSROOTS); + } + + protected function setUp() { + parent::setUp(); + $infoParser = new InfoParser(\OC::$server->getHTTPHelper(), \OC::$server->getURLGenerator()); + + $this->infoChecker = new InfoChecker($infoParser); + } + + public function appInfoData() { + return [ + ['testapp-infoxml', []], + ['testapp-version', []], + ['testapp-infoxml-version', []], + ['testapp-infoxml-version-different', [['type' => 'differentVersions', 'message' => 'appinfo/version: 1.2.4 - appinfo/info.xml: 1.2.3']]], + ['testapp-version-missing', [['type' => 'mandatoryFieldMissing', 'field' => 'version']]], + ['testapp-name-missing', [['type' => 'mandatoryFieldMissing', 'field' => 'name']]], + ]; + } + + /** + * @dataProvider appInfoData + * + * @param $appId + * @param $expectedErrors + */ + public function testApps($appId, $expectedErrors) { + $errors = $this->infoChecker->analyse($appId); + + $this->assertEquals($expectedErrors, $errors); + } +}