From: Kunal Mehta Date: Thu, 1 Dec 2016 07:04:27 +0000 (-0800) Subject: registration: Refactor validation logic to avoid duplication X-Git-Tag: 1.31.0-rc.0~4662^2 X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/banques/?a=commitdiff_plain;h=da68c0ae8256e357ff9a532c434a07788128d759;p=lhc%2Fweb%2Fwiklou.git registration: Refactor validation logic to avoid duplication 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 --- diff --git a/autoload.php b/autoload.php index 0d6407bc20..7604414254 100644 --- a/autoload.php +++ b/autoload.php @@ -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 index 0000000000..897d2840ed --- /dev/null +++ b/includes/registration/ExtensionJsonValidationError.php @@ -0,0 +1,22 @@ +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 '; + } + } + + $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 ); + } + } +} diff --git a/maintenance/validateRegistrationFile.php b/maintenance/validateRegistrationFile.php index b9baf8ce72..9906990bb5 100644 --- a/maintenance/validateRegistrationFile.php +++ b/maintenance/validateRegistrationFile.php @@ -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 '; - } - } - - $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 ); } } } diff --git a/tests/phpunit/structure/ExtensionJsonValidationTest.php b/tests/phpunit/structure/ExtensionJsonValidationTest.php index e11fd8a548..8ba2aeb241 100644 --- a/tests/phpunit/structure/ExtensionJsonValidationTest.php +++ b/tests/phpunit/structure/ExtensionJsonValidationTest.php @@ -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 '; - } - } - - $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() ); } } }