registration: Refactor validation logic to avoid duplication
authorKunal Mehta <legoktm@member.fsf.org>
Thu, 1 Dec 2016 07:04:27 +0000 (23:04 -0800)
committerKunal Mehta <legoktm@member.fsf.org>
Thu, 1 Dec 2016 07:04:27 +0000 (23:04 -0800)
Previously, logic to validate extension.json files was in two places:
validateRegistrationFile.php maintenance script, and the
ExtensionJsonValidationTest.php structure test. This caused duplication
as validation became more complex (e.g. usage of spdx-licenses library).

A generic ExtensionJsonValidator class now handles most of the
validation work, while the maintenance script and test case just wrap
around it for their output formats.

Change-Id: I47062a4ae19c58ee1b1f2bb4877913259bf19c8b

autoload.php
includes/registration/ExtensionJsonValidationError.php [new file with mode: 0644]
includes/registration/ExtensionJsonValidator.php [new file with mode: 0644]
maintenance/validateRegistrationFile.php
tests/phpunit/structure/ExtensionJsonValidationTest.php

index 0d6407b..7604414 100644 (file)
@@ -428,6 +428,8 @@ $wgAutoloadLocalClasses = [
        'ExplodeIterator' => __DIR__ . '/includes/libs/ExplodeIterator.php',
        'ExportProgressFilter' => __DIR__ . '/maintenance/backup.inc',
        'ExportSites' => __DIR__ . '/maintenance/exportSites.php',
+       'ExtensionJsonValidationError' => __DIR__ . '/includes/registration/ExtensionJsonValidationError.php',
+       'ExtensionJsonValidator' => __DIR__ . '/includes/registration/ExtensionJsonValidator.php',
        'ExtensionLanguages' => __DIR__ . '/maintenance/language/languages.inc',
        'ExtensionProcessor' => __DIR__ . '/includes/registration/ExtensionProcessor.php',
        'ExtensionRegistry' => __DIR__ . '/includes/registration/ExtensionRegistry.php',
diff --git a/includes/registration/ExtensionJsonValidationError.php b/includes/registration/ExtensionJsonValidationError.php
new file mode 100644 (file)
index 0000000..897d284
--- /dev/null
@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+class ExtensionJsonValidationError extends Exception {
+}
diff --git a/includes/registration/ExtensionJsonValidator.php b/includes/registration/ExtensionJsonValidator.php
new file mode 100644 (file)
index 0000000..f6e76af
--- /dev/null
@@ -0,0 +1,122 @@
+<?php
+
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+use Composer\Spdx\SpdxLicenses;
+use JsonSchema\Validator;
+
+/**
+ * @since 1.29
+ */
+class ExtensionJsonValidator {
+
+       /**
+        * @var callable
+        */
+       private $missingDepCallback;
+
+       /**
+        * @param callable $missingDepCallback
+        */
+       public function __construct( callable $missingDepCallback ) {
+               $this->missingDepCallback = $missingDepCallback;
+       }
+
+       /**
+        * @return bool
+        */
+       public function checkDependencies() {
+               if ( !class_exists( Validator::class ) ) {
+                       call_user_func( $this->missingDepCallback,
+                               'The JsonSchema library cannot be found, please install it through composer.'
+                       );
+                       return false;
+               } elseif ( !class_exists( SpdxLicenses::class ) ) {
+                       call_user_func( $this->missingDepCallback,
+                               'The spdx-licenses library cannot be found, please install it through composer.'
+                       );
+                       return false;
+               }
+
+               return true;
+       }
+
+       /**
+        * @param string $path file to validate
+        * @return bool true if passes validation
+        * @throws ExtensionJsonValidationError on any failure
+        */
+       public function validate( $path ) {
+               $data = json_decode( file_get_contents( $path ) );
+               if ( !is_object( $data ) ) {
+                       throw new ExtensionJsonValidationError( "$path is not valid JSON" );
+               }
+
+               if ( !isset( $data->manifest_version ) ) {
+                       throw new ExtensionJsonValidationError(
+                               "$path does not have manifest_version set." );
+               }
+
+               $version = $data->manifest_version;
+               if ( $version !== ExtensionRegistry::MANIFEST_VERSION ) {
+                       $schemaPath = __DIR__ . "/../../docs/extension.schema.v$version.json";
+               } else {
+                       $schemaPath = __DIR__ . '/../../docs/extension.schema.json';
+               }
+
+               // Not too old
+               if ( $version < ExtensionRegistry::OLDEST_MANIFEST_VERSION ) {
+                       throw new ExtensionJsonValidationError(
+                               "$path is using a non-supported schema version"
+                       );
+               } elseif ( $version > ExtensionRegistry::MANIFEST_VERSION ) {
+                       throw new ExtensionJsonValidationError(
+                               "$path is using a non-supported schema version"
+                       );
+               }
+
+               $licenseError = false;
+               // Check if it's a string, if not, schema validation will display an error
+               if ( isset( $data->{'license-name'} ) && is_string( $data->{'license-name'} ) ) {
+                       $licenses = new SpdxLicenses();
+                       $valid = $licenses->validate( $data->{'license-name'} );
+                       if ( !$valid ) {
+                               $licenseError = '[license-name] Invalid SPDX license identifier, '
+                                       . 'see <https://spdx.org/licenses/>';
+                       }
+               }
+
+               $validator = new Validator;
+               $validator->check( $data, (object)[ '$ref' => 'file://' . $schemaPath ] );
+               if ( $validator->isValid() && !$licenseError ) {
+                       // All good.
+                       return true;
+               } else {
+                       $out = "$path did pass validation.\n";
+                       foreach ( $validator->getErrors() as $error ) {
+                               $out .= "[{$error['property']}] {$error['message']}\n";
+                       }
+                       if ( $licenseError ) {
+                               $out .= "$licenseError\n";
+                       }
+                       throw new ExtensionJsonValidationError( $out );
+               }
+       }
+}
index b9baf8c..9906990 100644 (file)
@@ -2,73 +2,22 @@
 
 require_once __DIR__ . '/Maintenance.php';
 
-use Composer\Spdx\SpdxLicenses;
-use JsonSchema\Validator;
-
 class ValidateRegistrationFile extends Maintenance {
        public function __construct() {
                parent::__construct();
                $this->addArg( 'path', 'Path to extension.json/skin.json file.', true );
        }
        public function execute() {
-               if ( !class_exists( Validator::class ) ) {
-                       $this->error( 'The JsonSchema library cannot be found, please install it through composer.', 1 );
-               } elseif ( !class_exists( SpdxLicenses::class ) ) {
-                       $this->error(
-                               'The spdx-licenses library cannot be found, please install it through composer.', 1
-                       );
-               }
-
+               $validator = new ExtensionJsonValidator( function( $msg ) {
+                       $this->error( $msg, 1 );
+               } );
+               $validator->checkDependencies();
                $path = $this->getArg( 0 );
-               $data = json_decode( file_get_contents( $path ) );
-               if ( !is_object( $data ) ) {
-                       $this->error( "$path is not a valid JSON file.", 1 );
-               }
-               if ( !isset( $data->manifest_version ) ) {
-                       $this->output( "Warning: No manifest_version set, assuming 1.\n" );
-                       // For backwards-compatability assume 1
-                       $data->manifest_version = 1;
-               }
-               $version = $data->manifest_version;
-               if ( $version !== ExtensionRegistry::MANIFEST_VERSION ) {
-                       $schemaPath = dirname( __DIR__ ) . "/docs/extension.schema.v$version.json";
-               } else {
-                       $schemaPath = dirname( __DIR__ ) . '/docs/extension.schema.json';
-               }
-
-               if ( $version < ExtensionRegistry::OLDEST_MANIFEST_VERSION
-                       || $version > ExtensionRegistry::MANIFEST_VERSION
-               ) {
-                       $this->error( "Error: $path is using a non-supported schema version, it should use "
-                               . ExtensionRegistry::MANIFEST_VERSION, 1 );
-               } elseif ( $version < ExtensionRegistry::MANIFEST_VERSION ) {
-                       $this->output( "Warning: $path is using a deprecated schema, and should be updated to "
-                               . ExtensionRegistry::MANIFEST_VERSION . "\n" );
-               }
-
-               $licenseError = false;
-               // Check if it's a string, if not, schema validation will display an error
-               if ( isset( $data->{'license-name'} ) && is_string( $data->{'license-name'} ) ) {
-                       $licenses = new SpdxLicenses();
-                       $valid = $licenses->validate( $data->{'license-name'} );
-                       if ( !$valid ) {
-                               $licenseError = '[license-name] Invalid SPDX license identifier, '
-                                       . 'see <https://spdx.org/licenses/>';
-                       }
-               }
-
-               $validator = new Validator;
-               $validator->check( $data, (object)[ '$ref' => 'file://' . $schemaPath ] );
-               if ( $validator->isValid() && !$licenseError ) {
-                       $this->output( "$path validates against the version $version schema!\n" );
-               } else {
-                       foreach ( $validator->getErrors() as $error ) {
-                               $this->output( "[{$error['property']}] {$error['message']}\n" );
-                       }
-                       if ( $licenseError ) {
-                               $this->output( "$licenseError\n" );
-                       }
-                       $this->error( "$path does not validate.", 1 );
+               try {
+                       $validator->validate( $path );
+                       $this->output( "$path validates against the schema!\n" );
+               } catch ( ExtensionJsonValidationError $e ) {
+                       $this->error( $e->getMessage(), 1 );
                }
        }
 }
index e11fd8a..8ba2aeb 100644 (file)
@@ -25,14 +25,16 @@ use JsonSchema\Validator;
  */
 class ExtensionJsonValidationTest extends PHPUnit_Framework_TestCase {
 
+       /**
+        * @var ExtensionJsonValidator
+        */
+       protected $validator;
+
        public function setUp() {
                parent::setUp();
-               if ( !class_exists( Validator::class ) ) {
-                       $this->markTestSkipped(
-                               'The JsonSchema library cannot be found,' .
-                               ' please install it through composer to run extension.json validation tests.'
-                       );
-               }
+
+               $this->validator = new ExtensionJsonValidator( [ $this, 'markTestSkipped' ] );
+               $this->validator->checkDependencies();
 
                if ( !ExtensionRegistry::getInstance()->getAllThings() ) {
                        $this->markTestSkipped(
@@ -55,56 +57,12 @@ class ExtensionJsonValidationTest extends PHPUnit_Framework_TestCase {
         * @param string $path Path to thing's json file
         */
        public function testPassesValidation( $path ) {
-               $data = json_decode( file_get_contents( $path ) );
-               $this->assertInstanceOf( 'stdClass', $data, "$path is not valid JSON" );
-
-               $this->assertObjectHasAttribute( 'manifest_version', $data,
-                       "$path does not have manifest_version set." );
-               $version = $data->manifest_version;
-               if ( $version !== ExtensionRegistry::MANIFEST_VERSION ) {
-                       $schemaPath = __DIR__ . "/../../../docs/extension.schema.v$version.json";
-               } else {
-                       $schemaPath = __DIR__ . '/../../../docs/extension.schema.json';
-               }
-
-               // Not too old
-               $this->assertTrue(
-                       $version >= ExtensionRegistry::OLDEST_MANIFEST_VERSION,
-                       "$path is using a non-supported schema version"
-               );
-               // Not too new
-               $this->assertTrue(
-                       $version <= ExtensionRegistry::MANIFEST_VERSION,
-                       "$path is using a non-supported schema version"
-               );
-
-               $licenseError = false;
-               if ( class_exists( SpdxLicenses::class ) && isset( $data->{'license-name'} )
-                       // Check if it's a string, if not, schema validation will display an error
-                       && is_string( $data->{'license-name'} )
-               ) {
-                       $licenses = new SpdxLicenses();
-                       $valid = $licenses->validate( $data->{'license-name'} );
-                       if ( !$valid ) {
-                               $licenseError = '[license-name] Invalid SPDX license identifier, '
-                                       . 'see <https://spdx.org/licenses/>';
-                       }
-               }
-
-               $validator = new Validator;
-               $validator->check( $data, (object)[ '$ref' => 'file://' . $schemaPath ] );
-               if ( $validator->isValid() && !$licenseError ) {
-                       // All good.
+               try {
+                       $this->validator->validate( $path );
+                       // All good
                        $this->assertTrue( true );
-               } else {
-                       $out = "$path did pass validation.\n";
-                       foreach ( $validator->getErrors() as $error ) {
-                               $out .= "[{$error['property']}] {$error['message']}\n";
-                       }
-                       if ( $licenseError ) {
-                               $out .= "$licenseError\n";
-                       }
-                       $this->assertTrue( false, $out );
+               } catch ( ExtensionJsonValidationError $e ) {
+                       $this->assertEquals( false, $e->getMessage() );
                }
        }
 }