[App Code Check] add check for version and mandatory fields

* ref #17598
* including unit tests for mandatory fields/versions

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("<info>Analysing {$filename}</info>");
 			}
 
-			// 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("<error>Mandatory field missing: $key</error>");
+			});
+
+			$infoChecker->listen('InfoChecker', 'deprecatedFieldFound', function($key, $value) use ($output) {
+				if($value === [] || is_null($value) || $value === '') {
+					$output->writeln("<info>Deprecated field available: $key</info>");
+				} else {
+					if(is_array($value)) {
+						$value = 'Array of ' . count($value) . ' element(s)';
+					}
+					$output->writeln("<info>Deprecated field available: $key => $value</info>");
+				}
+			});
+
+			$infoChecker->listen('InfoChecker', 'differentVersions', function($versionFile, $infoXML) use ($output) {
+				$output->writeln("<error>Different versions provided (appinfo/version: $versionFile - appinfo/info.xml: $infoXML)</error>");
+			});
+
+			$infoChecker->listen('InfoChecker', 'sameVersions', function($path) use ($output) {
+				$output->writeln("<info>Version file isn't needed anymore and can be safely removed ($path)</info>");
+			});
+
+			$infoChecker->listen('InfoChecker', 'migrateVersion', function($version) use ($output) {
+				$output->writeln("<info>Migrate the app version to appinfo/info.xml (add <version>$version</version> to appinfo/info.xml and remove appinfo/version)</info>");
+			});
+
+			if(OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
+				$infoChecker->listen('InfoChecker', 'mandatoryFieldFound', function($key, $value) use ($output) {
+					$output->writeln("<info>Mandatory field available: $key => $value</info>");
+				});
+
+				$infoChecker->listen('InfoChecker', 'optionalFieldFound', function($key, $value) use ($output) {
+					$output->writeln("<info>Optional field available: $key => $value</info>");
+				});
+
+				$infoChecker->listen('InfoChecker', 'unusedFieldFound', function($key, $value) use ($output) {
+					$output->writeln("<info>Unused field available: $key => $value</info>");
+				});
+			}
+
+			$infoErrors = $infoChecker->analyse($appId);
+
+			$errors = array_merge($errors, $infoErrors);
+		}
+
 		if (empty($errors)) {
 			$output->writeln('<info>App is compliant - awesome job!</info>');
 			return 0;

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)) {

lib/private/app/codechecker/infochecker.php

@@ -0,0 +1,141 @@
+<?php
+/**
+ * @author Morris Jobke <hey@morrisjobke.de>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+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;
+	}
+}

tests/apps/testapp-infoxml-version-different/appinfo/info.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<info>
+	<id>testapp-infoxml-version</id>
+	<version>1.2.3</version>
+	<author>Jane</author>
+	<description>A b c</description>
+	<licence>Abc</licence>
+	<name>Test app</name>
+</info>

tests/apps/testapp-infoxml-version-different/appinfo/version

@@ -0,0 +1 @@
+1.2.4

tests/apps/testapp-infoxml-version/appinfo/info.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<info>
+	<id>testapp-infoxml-version</id>
+	<version>1.2.3</version>
+	<author>Jane</author>
+	<description>A b c</description>
+	<licence>Abc</licence>
+	<name>Test app</name>
+</info>

tests/apps/testapp-infoxml-version/appinfo/version

@@ -0,0 +1 @@
+1.2.3

tests/apps/testapp-infoxml/appinfo/info.xml

@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<info>
+	<id>testapp-infoxml</id>
+	<version>1.2.3</version>
+	<author>Jane</author>
+	<description>A b c</description>
+	<licence>Abc</licence>
+	<name>Test app</name>
+</info>

tests/apps/testapp-name-missing/appinfo/info.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<info>
+	<id>testapp-version</id>
+	<version>1.1.1</version>
+	<author>Jane</author>
+	<description>A b c</description>
+	<licence>Abc</licence>
+</info>

tests/apps/testapp-version-missing/appinfo/info.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<info>
+	<id>testapp-version</id>
+	<author>Jane</author>
+	<description>A b c</description>
+	<licence>Abc</licence>
+	<name>Test app</name>
+</info>

tests/apps/testapp-version/appinfo/info.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<info>
+	<id>testapp-version</id>
+	<author>Jane</author>
+	<description>A b c</description>
+	<licence>Abc</licence>
+	<name>Test app</name>
+</info>

tests/apps/testapp-version/appinfo/version

@@ -0,0 +1 @@
+1.2.3

tests/lib/app/codechecker/infocheckertest.php

@@ -0,0 +1,73 @@
+<?php
+/**
+ * @author Morris Jobke <hey@morrisjobke.de>
+ *
+ * @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 <http://www.gnu.org/licenses/>
+ *
+ */
+
+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);
+	}
+}