From dddf316f295853d3a08820e379cda54c6cf76668 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Fri, 31 May 2013 10:16:27 +0200 Subject: [PATCH] Address github comments --- .../Core/Provider/PluginServiceProvider.php | 4 +- .../TemporaryFilesystemServiceProvider.php | 9 +++ .../Plugin/Management/AutoloaderGenerator.php | 57 ++++++++++++++----- .../Plugin/Schema/ManifestValidator.php | 52 ++++++++++++----- .../Phrasea/Plugin/Schema/PluginValidator.php | 16 +++++- .../Provider/PluginServiceProviderTest.php | 2 +- .../Fixtures/manifest-wrong5-min-version.json | 5 ++ .../Fixtures/manifest-wrong6-max-version.json | 5 ++ .../Management/AutoloaderGeneratorTest.php | 2 +- .../Tests/Phrasea/Plugin/PluginTestCase.php | 9 ++- .../Plugin/Schema/ManifestValidatorTest.php | 8 ++- .../Plugin/Schema/PluginValidatorTest.php | 8 +-- 12 files changed, 131 insertions(+), 46 deletions(-) create mode 100644 tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong5-min-version.json create mode 100644 tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong6-max-version.json diff --git a/lib/Alchemy/Phrasea/Core/Provider/PluginServiceProvider.php b/lib/Alchemy/Phrasea/Core/Provider/PluginServiceProvider.php index aeb508df29..9c77ef5729 100644 --- a/lib/Alchemy/Phrasea/Core/Provider/PluginServiceProvider.php +++ b/lib/Alchemy/Phrasea/Core/Provider/PluginServiceProvider.php @@ -32,12 +32,12 @@ class PluginServiceProvider implements ServiceProviderInterface $app['plugins.directory'] = realpath(__DIR__ . '/../../../../../plugins'); $app['plugins.schema'] = realpath(__DIR__ . '/../../../../conf.d/plugin-schema.json'); - $app['json-validator'] = $app->share(function (Application $app) { + $app['plugins.json-validator'] = $app->share(function (Application $app) { return new JsonValidator(); }); $app['plugins.manifest-validator'] = $app->share(function (Application $app) { - return ManifestValidator::create($app['json-validator'], $app['plugins.schema']); + return ManifestValidator::create($app); }); $app['plugins.plugins-validator'] = $app->share(function (Application $app) { diff --git a/lib/Alchemy/Phrasea/Core/Provider/TemporaryFilesystemServiceProvider.php b/lib/Alchemy/Phrasea/Core/Provider/TemporaryFilesystemServiceProvider.php index 4003b056a9..33ebf9d38a 100644 --- a/lib/Alchemy/Phrasea/Core/Provider/TemporaryFilesystemServiceProvider.php +++ b/lib/Alchemy/Phrasea/Core/Provider/TemporaryFilesystemServiceProvider.php @@ -1,5 +1,14 @@ getName() . DIRECTORY_SEPARATOR . "vendor" . DIRECTORY_SEPARATOR . "autoload.php';\n"; + $autoloader = '/' . $manifest->getName() . DIRECTORY_SEPARATOR . "vendor" . DIRECTORY_SEPARATOR . "autoload.php"; + $buffer .= <<getServices() as $service) { - $buffer .= " \$app->register(\\".$service['class']."::create(\$app));\n"; + $class = $service['class']; + $buffer .= <<register($class::create(\$app)); +EOF; } } - $buffer .= "\n return \$app;\n}, \$app);\n"; + $buffer .= <<validator = $validator; + $this->version = $version; $this->schemaData = $schemaData; } @@ -36,12 +40,12 @@ class ManifestValidator throw new InvalidArgumentException('Json Schema must be an object'); } - $validator = clone $this->validator; - $validator->check($data, $this->schemaData); + $this->validator->reset(); + $this->validator->check($data, $this->schemaData); - if (!$validator->isValid()) { + if (!$this->validator->isValid()) { $errors = array(); - foreach ((array) $validator->getErrors() as $error) { + foreach ((array) $this->validator->getErrors() as $error) { $errors[] = ($error['property'] ? $error['property'].' : ' : '').$error['message']; } throw new JsonValidationException('Manifest file does not match the expected JSON schema', $errors); @@ -51,17 +55,35 @@ class ManifestValidator throw new JsonValidationException('Does not match the expected JSON schema', array('"name" must not contains only alphanumeric caracters')); } - // validate gainst versions - } - - public static function create(JsonValidator $jsonValidator, $path) - { - $data = @json_decode(@file_get_contents($path)); - - if (JSON_ERROR_NONE !== json_last_error()) { - throw new InvalidArgumentException(sprintf('Unable to read %s', $path)); + if (isset($data->{'minimum-phraseanet-version'})) { + if (true !== version_compare($this->version->getNumber(), $data->{'minimum-phraseanet-version'}, '>=')) { + throw new JsonValidationException(sprintf( + 'Version incomptibility : Minimum Phraseanet version required is %s, current version is %s', + $data->{'minimum-phraseanet-version'}, + $this->version->getNumber() + )); + } } - return new static($jsonValidator, $data); + if (isset($data->{'maximum-phraseanet-version'})) { + if (true !== version_compare($this->version->getNumber(), $data->{'maximum-phraseanet-version'}, '<')) { + throw new JsonValidationException(sprintf( + 'Version incomptibility : Maximum Phraseanet version required is %s, current version is %s', + $data->{'maximum-phraseanet-version'}, + $this->version->getNumber() + )); + } + } + } + + public static function create(Application $app) + { + $data = @json_decode(@file_get_contents($app['plugins.schema'])); + + if (JSON_ERROR_NONE !== json_last_error()) { + throw new InvalidArgumentException(sprintf('Unable to read %s', $app['plugins.schema'])); + } + + return new static($app['plugins.json-validator'], $data, $app['phraseanet.version']); } } diff --git a/lib/Alchemy/Phrasea/Plugin/Schema/PluginValidator.php b/lib/Alchemy/Phrasea/Plugin/Schema/PluginValidator.php index f28445c69c..0b0cadbdde 100644 --- a/lib/Alchemy/Phrasea/Plugin/Schema/PluginValidator.php +++ b/lib/Alchemy/Phrasea/Plugin/Schema/PluginValidator.php @@ -43,8 +43,7 @@ class PluginValidator throw new PluginValidationException('Manifest file is invalid', $e->getCode(), $e); } - // a ameliorer - return new Manifest(@json_decode(@file_get_contents($manifest), true)); + return new Manifest($this->objectToArray($data)); } private function ensureManifest($directory) @@ -65,4 +64,17 @@ class PluginValidator throw new PluginValidationException(sprintf('Required file %s is not present.', $file)); } } + + private function objectToArray($data) + { + if (is_object($data)) { + $data = get_object_vars($data); + } + + if (is_array($data)) { + return array_map(array($this, 'objectToArray'), $data); + } + + return $data; + } } diff --git a/tests/Alchemy/Tests/Phrasea/Core/Provider/PluginServiceProviderTest.php b/tests/Alchemy/Tests/Phrasea/Core/Provider/PluginServiceProviderTest.php index 076f29fc00..df94de41b7 100644 --- a/tests/Alchemy/Tests/Phrasea/Core/Provider/PluginServiceProviderTest.php +++ b/tests/Alchemy/Tests/Phrasea/Core/Provider/PluginServiceProviderTest.php @@ -16,7 +16,7 @@ class PluginServiceProvidertest extends ServiceProviderTestCase return array( array( 'Alchemy\Phrasea\Core\Provider\PluginServiceProvider', - 'json-validator', + 'plugins.json-validator', 'JsonSchema\Validator' ), array( diff --git a/tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong5-min-version.json b/tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong5-min-version.json new file mode 100644 index 0000000000..c63b1f8eda --- /dev/null +++ b/tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong5-min-version.json @@ -0,0 +1,5 @@ +{ + "name": "TestPlugin", + "description" : "A custom class connector", + "minimum-phraseanet-version": "14" +} diff --git a/tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong6-max-version.json b/tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong6-max-version.json new file mode 100644 index 0000000000..c8dbb299f4 --- /dev/null +++ b/tests/Alchemy/Tests/Phrasea/Plugin/Fixtures/manifest-wrong6-max-version.json @@ -0,0 +1,5 @@ +{ + "name": "TestPlugin", + "description" : "A custom class connector", + "maximum-phraseanet-version": "3.8" +} diff --git a/tests/Alchemy/Tests/Phrasea/Plugin/Management/AutoloaderGeneratorTest.php b/tests/Alchemy/Tests/Phrasea/Plugin/Management/AutoloaderGeneratorTest.php index 8ee8241189..267cba71dd 100644 --- a/tests/Alchemy/Tests/Phrasea/Plugin/Management/AutoloaderGeneratorTest.php +++ b/tests/Alchemy/Tests/Phrasea/Plugin/Management/AutoloaderGeneratorTest.php @@ -33,7 +33,7 @@ class AutoloaderGeneratorTest extends \PHPUnit_Framework_TestCase $this->assertFileExists($file); $process = ProcessBuilder::create(array($php, '-l', $file))->getProcess(); $process->run(); - $this->assertTrue($process->isSuccessful()); + $this->assertTrue($process->isSuccessful(), basename($file) . ' is valid'); } // test autoload diff --git a/tests/Alchemy/Tests/Phrasea/Plugin/PluginTestCase.php b/tests/Alchemy/Tests/Phrasea/Plugin/PluginTestCase.php index 8603550843..e1cd027f17 100644 --- a/tests/Alchemy/Tests/Phrasea/Plugin/PluginTestCase.php +++ b/tests/Alchemy/Tests/Phrasea/Plugin/PluginTestCase.php @@ -2,8 +2,15 @@ namespace Alchemy\Tests\Phrasea\Plugin; -class PluginTestCase extends \PHPUnit_Framework_TestCase +use Alchemy\Phrasea\Plugin\Schema\ManifestValidator; + +class PluginTestCase extends \PhraseanetPHPUnitAbstract { + protected function createManifestValidator() + { + return ManifestValidator::create(self::$DI['app']); + } + protected function getPluginDirectory() { return __DIR__ . DIRECTORY_SEPARATOR . 'PluginFolder'; diff --git a/tests/Alchemy/Tests/Phrasea/Plugin/Schema/ManifestValidatorTest.php b/tests/Alchemy/Tests/Phrasea/Plugin/Schema/ManifestValidatorTest.php index dae749933a..5fb5a1bc99 100644 --- a/tests/Alchemy/Tests/Phrasea/Plugin/Schema/ManifestValidatorTest.php +++ b/tests/Alchemy/Tests/Phrasea/Plugin/Schema/ManifestValidatorTest.php @@ -42,6 +42,8 @@ class ManifestValidatorTest extends PluginTestCase array(__DIR__ . '/../Fixtures/manifest-wrong2.json'), array(__DIR__ . '/../Fixtures/manifest-wrong3.json'), array(__DIR__ . '/../Fixtures/manifest-wrong4.json'), + array(__DIR__ . '/../Fixtures/manifest-wrong5-min-version.json'), + array(__DIR__ . '/../Fixtures/manifest-wrong6-max-version.json'), ); } @@ -59,12 +61,12 @@ class ManifestValidatorTest extends PluginTestCase */ public function testConstructWithInvalidSchema() { - $validator = new ManifestValidator(new JsonSchemaValidator(), array()); + new ManifestValidator(new JsonSchemaValidator(), array(), self::$DI['app']['phraseanet.version']); } public function testCreate() { - $validator = ManifestValidator::create(new JsonSchemaValidator(), $this->getSchemaPath()); + $validator = ManifestValidator::create(self::$DI['app']); $this->assertInstanceOf('Alchemy\Phrasea\Plugin\Schema\ManifestValidator', $validator); } @@ -73,6 +75,6 @@ class ManifestValidatorTest extends PluginTestCase { $schema = json_decode($this->getSchema()); - return new ManifestValidator(new JsonSchemaValidator(), $schema); + return new ManifestValidator(new JsonSchemaValidator(), $schema, self::$DI['app']['phraseanet.version']); } } diff --git a/tests/Alchemy/Tests/Phrasea/Plugin/Schema/PluginValidatorTest.php b/tests/Alchemy/Tests/Phrasea/Plugin/Schema/PluginValidatorTest.php index 2d9dfaa1be..b477faa68f 100644 --- a/tests/Alchemy/Tests/Phrasea/Plugin/Schema/PluginValidatorTest.php +++ b/tests/Alchemy/Tests/Phrasea/Plugin/Schema/PluginValidatorTest.php @@ -15,9 +15,7 @@ class PluginValidatorTest extends PluginTestCase */ public function testValidateInvalidPlugin($directory) { - $schema = json_decode($this->getSchema()); - - $validator = new PluginValidator(new ManifestValidator(new JsonValidator(), $schema)); + $validator = new PluginValidator($this->createManifestValidator()); $validator->validatePlugin($directory); } @@ -26,9 +24,7 @@ class PluginValidatorTest extends PluginTestCase */ public function testValidatePlugin($directory) { - $schema = json_decode($this->getSchema()); - - $validator = new PluginValidator(new ManifestValidator(new JsonValidator(), $schema)); + $validator = new PluginValidator($this->createManifestValidator()); $validator->validatePlugin($directory); }