From 4bc51dca82ce4664e8f308e142013c61f5fa1b6e Mon Sep 17 00:00:00 2001 From: gggeek Date: Thu, 1 Aug 2019 08:02:40 +0000 Subject: [PATCH] fix issue #216, improve tests --- Core/Executor/ContentManager.php | 4 + Core/Executor/LocationManager.php | 2 +- Core/Executor/RepositoryExecutor.php | 20 ++ Core/Executor/UserGroupManager.php | 2 +- Core/MigrationService.php | 2 +- Resources/doc/DSL/Contents.yml | 4 +- .../bad/execution/UnitTestKO009_language.yml | 6 +- .../bad/execution/UnitTestKO010_cleanup.yml | 4 +- .../UnitTestKO013_multilang_fields.yml | 2 +- ...tTestKO014_multilang_fields_wrong_lang.yml | 2 +- ...stKO015_multilang_fields_missing_field.yml | 6 +- Tests/dsl/good/UnitTestOK005_location.yml | 6 +- .../good/UnitTestOK032_matchByuserGroup.yml | 50 ++-- Tests/phpunit/6_MassMigrateTest.php_ | 271 ++++++++++++++++++ WHATSNEW.md | 17 +- 15 files changed, 353 insertions(+), 45 deletions(-) create mode 100644 Tests/phpunit/6_MassMigrateTest.php_ diff --git a/Core/Executor/ContentManager.php b/Core/Executor/ContentManager.php index 13ff54c2..f3200f20 100644 --- a/Core/Executor/ContentManager.php +++ b/Core/Executor/ContentManager.php @@ -403,6 +403,10 @@ protected function getReferencesValues($content, array $references, $step) case 'main_location_id': $value = $content->contentInfo->mainLocationId; break; + case 'location_remote_id': + $locationService = $this->repository->getLocationService(); + $value = $locationService->loadLocation($content->contentInfo->mainLocationId)->remoteId; + break; case 'main_language_code': $value = $content->contentInfo->mainLanguageCode; break; diff --git a/Core/Executor/LocationManager.php b/Core/Executor/LocationManager.php index bdace7c9..0188718c 100644 --- a/Core/Executor/LocationManager.php +++ b/Core/Executor/LocationManager.php @@ -184,7 +184,7 @@ protected function update($step) $parentLocationId = isset($step->dsl['parent_location']) ? $step->dsl['parent_location'] : $step->dsl['parent_location_id']; $parentLocationId = $this->referenceResolver->resolveReference($parentLocationId); - $newParentLocation = $locationService->loadLocation($parentLocationId); + $newParentLocation = $this->matchLocationByKey($parentLocationId); $locationService->moveSubtree($location, $newParentLocation); diff --git a/Core/Executor/RepositoryExecutor.php b/Core/Executor/RepositoryExecutor.php index f0ab0a54..a01e4828 100644 --- a/Core/Executor/RepositoryExecutor.php +++ b/Core/Executor/RepositoryExecutor.php @@ -29,6 +29,8 @@ abstract class RepositoryExecutor extends AbstractExecutor /** Used if not specified by the migration */ const USER_CONTENT_TYPE = 'user'; + /** Used if not specified by the migration */ + const USERGROUP_CONTENT_TYPE = 'user_group'; const REFERENCE_TYPE_SCALAR = 'scalar'; const REFERENCE_TYPE_ARRAY = 'array'; @@ -160,6 +162,15 @@ protected function getUserContentType($step) return isset($step->dsl['user_content_type']) ? $this->referenceResolver->resolveReference($step->dsl['user_content_type']) : $this->getUserContentTypeFromContext($step->context); } + /** + * @param MigrationStep $step + * @return string + */ + protected function getUserGroupContentType($step) + { + return isset($step->dsl['usergroup_content_type']) ? $this->referenceResolver->resolveReference($step->dsl['usergroup_content_type']) : $this->getUserGroupContentTypeFromContext($step->context); + } + /** * @param array $context * @return string @@ -169,6 +180,15 @@ protected function getUserContentTypeFromContext($context) return isset($context['userContentType']) ? $context['userContentType'] : self::USER_CONTENT_TYPE; } + /** + * @param array $context + * @return string + */ + protected function getUserGroupContentTypeFromContext($context) + { + return isset($context['userGroupContentType']) ? $context['userGroupContentType'] : self::USERGROUP_CONTENT_TYPE; + } + /** * @param array $context we have to return FALSE if that is set as adminUserLogin, whereas if NULL is set, we return the default admin * @return int|string|false diff --git a/Core/Executor/UserGroupManager.php b/Core/Executor/UserGroupManager.php index ce0df43d..d15ce736 100644 --- a/Core/Executor/UserGroupManager.php +++ b/Core/Executor/UserGroupManager.php @@ -39,7 +39,7 @@ protected function create($step) $parentGroupId = $this->referenceResolver->resolveReference($parentGroupId); $parentGroup = $this->userGroupMatcher->matchOneByKey($parentGroupId); - $contentType = $this->repository->getContentTypeService()->loadContentTypeByIdentifier("user_group"); + $contentType = $this->repository->getContentTypeService()->loadContentTypeByIdentifier($this->getUserGroupContentType($step)); $userGroupCreateStruct = $userService->newUserGroupCreateStruct($this->getLanguageCode($step), $contentType); $userGroupCreateStruct->setField('name', $step->dsl['name']); diff --git a/Core/MigrationService.php b/Core/MigrationService.php index 21f57a6f..ae1c3d38 100644 --- a/Core/MigrationService.php +++ b/Core/MigrationService.php @@ -267,7 +267,7 @@ public function executeMigration(MigrationDefinition $migrationDefinition, $useT throw new \Exception("Can not execute " . $this->getEntityName($migrationDefinition). " '{$migrationDefinition->name}': {$migrationDefinition->parsingError}"); } - /// @todo add support for setting in $migrationContext a userContentType ? + /// @todo add support for setting in $migrationContext a userContentType, userGroupContentType ? $migrationContext = $this->migrationContextFromParameters($defaultLanguageCode, $adminLogin, $forceSigchildEnabled); // set migration as begun - has to be in own db transaction diff --git a/Resources/doc/DSL/Contents.yml b/Resources/doc/DSL/Contents.yml index 04994b4d..efcc4fc5 100644 --- a/Resources/doc/DSL/Contents.yml +++ b/Resources/doc/DSL/Contents.yml @@ -95,7 +95,7 @@ attribute: attributeId # The attribute to get the value of for the reference. # Supports: content_id, content_remote_id, # always_available, content_type_id, content_type_identifier, current_version_no, - # location_id, main_language_code, modification_date, name, object_state.$stategroup, + # location_id, location_remote_id, main_language_code, modification_date, name, object_state.$stategroup, # owner_id, path, publication_date, section_id as well as attributes.$name (for scalar # attributes) and complex expressions for non-scalar attributes, such as f.e. # attributes.objectrelationlistattr.destinationContentIds[0] (1st element in an object-relation) @@ -173,7 +173,7 @@ attribute: attributeId # The attribute to get the value of for the reference # supports: content_id, content_remote_id, # always_available, content_type_id, content_type_identifier, current_version_no, - # location_id, main_language_code, modification_date, name, object_state.$stategroup, + # location_id, location_remote_id, main_language_code, modification_date, name, object_state.$stategroup, # owner_id, path, publication_date, section_id as well as attributes.$name (for scalar # attributes) and complex expressions for non-scalar attributes, such as f.e. # attributes.objectrelationlistattr.destinationContentIds[0] (1st element in an object-relation) diff --git a/Tests/dsl/bad/execution/UnitTestKO009_language.yml b/Tests/dsl/bad/execution/UnitTestKO009_language.yml index a6c0f0ae..0272b677 100644 --- a/Tests/dsl/bad/execution/UnitTestKO009_language.yml +++ b/Tests/dsl/bad/execution/UnitTestKO009_language.yml @@ -5,7 +5,7 @@ type: content_type mode: create content_type_group: 1 - identifier: kmb_test_9 + identifier: kmb_test_9k name: Kaliop Migration Bundle Test Class 9 name_pattern: '' attributes: @@ -15,13 +15,13 @@ identifier: ezstring references: - - identifier: kmb_test_9 + identifier: kmb_test_9k attribute: identifier - type: content mode: create - content_type: kmb_test_9 + content_type: kmb_test_9k parent_location: 2 lang: fre-IT attributes: diff --git a/Tests/dsl/bad/execution/UnitTestKO010_cleanup.yml b/Tests/dsl/bad/execution/UnitTestKO010_cleanup.yml index ce59438e..7b966bb9 100644 --- a/Tests/dsl/bad/execution/UnitTestKO010_cleanup.yml +++ b/Tests/dsl/bad/execution/UnitTestKO010_cleanup.yml @@ -3,9 +3,9 @@ - type: content_type mode: delete - identifier: reference:kmb_test_9 + identifier: reference:kmb_test_9k - type: content_type mode: delete - identifier: reference:kmb_test_9 + identifier: reference:kmb_test_9k diff --git a/Tests/dsl/bad/execution/UnitTestKO013_multilang_fields.yml b/Tests/dsl/bad/execution/UnitTestKO013_multilang_fields.yml index 951f4349..1433b721 100644 --- a/Tests/dsl/bad/execution/UnitTestKO013_multilang_fields.yml +++ b/Tests/dsl/bad/execution/UnitTestKO013_multilang_fields.yml @@ -33,7 +33,7 @@ - type: content mode: create - content_type: kmb_test_9 + content_type: kmb_test_13 parent_location: 2 attributes: ezstring: diff --git a/Tests/dsl/bad/execution/UnitTestKO014_multilang_fields_wrong_lang.yml b/Tests/dsl/bad/execution/UnitTestKO014_multilang_fields_wrong_lang.yml index b7083815..9fc10dfb 100644 --- a/Tests/dsl/bad/execution/UnitTestKO014_multilang_fields_wrong_lang.yml +++ b/Tests/dsl/bad/execution/UnitTestKO014_multilang_fields_wrong_lang.yml @@ -23,7 +23,7 @@ - type: content mode: create - content_type: kmb_test_9 + content_type: kmb_test_14 parent_location: 2 attributes: ezstring: diff --git a/Tests/dsl/bad/execution/UnitTestKO015_multilang_fields_missing_field.yml b/Tests/dsl/bad/execution/UnitTestKO015_multilang_fields_missing_field.yml index 4c505aa9..ffeb6bb7 100644 --- a/Tests/dsl/bad/execution/UnitTestKO015_multilang_fields_missing_field.yml +++ b/Tests/dsl/bad/execution/UnitTestKO015_multilang_fields_missing_field.yml @@ -1,4 +1,4 @@ -# Fails by attempting creation of content in non-existing language. +# Fails by attempting creation of content with non-existing field. # NB: needs a follow-up migration to remove the created content type - @@ -34,10 +34,10 @@ - type: content mode: create - content_type: kmb_test_9 + content_type: kmb_test_15 parent_location: 2 attributes: - ezstring: + ezstring_notexist: abe-TA: hello world 4 abe-TA abe-TB: hello world 4 abe-TB ezstring_2: diff --git a/Tests/dsl/good/UnitTestOK005_location.yml b/Tests/dsl/good/UnitTestOK005_location.yml index 479092f6..f9d4efd7 100644 --- a/Tests/dsl/good/UnitTestOK005_location.yml +++ b/Tests/dsl/good/UnitTestOK005_location.yml @@ -77,7 +77,9 @@ - identifier: kmb_test_005_4_rid attribute: remote_id - + - + identifier: kmb_test_005_4_loc_rid + attribute: location_remote_id - type: content mode: create @@ -260,7 +262,7 @@ mode: update # deprecated location_id: reference:kmb_test_005_3_loc_3 - parent_location: reference:kmb_test_005_4_loc + parent_location: reference:kmb_test_005_4_loc_rid priority: 1998 is_hidden: false is_main: true diff --git a/Tests/dsl/good/UnitTestOK032_matchByuserGroup.yml b/Tests/dsl/good/UnitTestOK032_matchByuserGroup.yml index e75b55fe..760f6e41 100644 --- a/Tests/dsl/good/UnitTestOK032_matchByuserGroup.yml +++ b/Tests/dsl/good/UnitTestOK032_matchByuserGroup.yml @@ -7,30 +7,30 @@ roles: [1, 2] # Anon, admin references: - - identifier: kmb_test_31_g + identifier: kmb_test_32_g attribute: id - - identifier: kmb_test_31_gr + identifier: kmb_test_32_gr attribute: remote_id - type: user mode: create first_name: Kaliop - last_name: Test User 31 - username: kmb_test_user_31 - email: kmb_test_user_31@test.com + last_name: Test User 32 + username: kmb_test_user_32 + email: kmb_test_user_32@test.com password: anUnsafePassword123 - groups: [reference:kmb_test_31_g] + groups: [reference:kmb_test_32_g] references: - - identifier: kmb_test_31_u + identifier: kmb_test_32_u attribute: id - type: content_type mode: create content_type_group: 1 - identifier: kmb_test_31_cti - name: Kaliop Migration Bundle Test Class 31 + identifier: kmb_test_32_cti + name: Kaliop Migration Bundle Test Class 32 name_pattern: '' attributes: - @@ -39,71 +39,71 @@ identifier: ezstring references: - - identifier: kmb_test_31_ct + identifier: kmb_test_32_ct attribute: id - type: content mode: create - content_type: kmb_test_31_cti + content_type: kmb_test_32_cti parent_location: 2 - owner: reference:kmb_test_31_u + owner: reference:kmb_test_32_u attributes: - - ezstring: hello world 31 + - ezstring: hello world 32 references: - - identifier: kmb_test_31_c + identifier: kmb_test_32_c attribute: id - type: content mode: load match: - group: reference:kmb_test_31_g + group: reference:kmb_test_32_g references: - - identifier: kmb_test_31_count_1 + identifier: kmb_test_32_count_1 attribute: count - type: assert target: reference - identifier: reference:kmb_test_31_count_1 + identifier: reference:kmb_test_32_count_1 test: equals: 1 - type: content mode: delete match: - content_type: kmb_test_31_cti + content_type: kmb_test_32_cti - type: content_type mode: delete - identifier: kmb_test_31_cti + identifier: kmb_test_32_cti - type: user mode: delete match: - usergroup_id: reference:kmb_test_31_g + usergroup_id: reference:kmb_test_32_g references: - - identifier: kmb_test_31_count_2 + identifier: kmb_test_32_count_2 attribute: count - type: assert target: reference - identifier: reference:kmb_test_31_count_2 + identifier: reference:kmb_test_32_count_2 test: equals: 1 - type: user_group mode: delete match: - content_remote_id: reference:kmb_test_31_gr + content_remote_id: reference:kmb_test_32_gr references: - - identifier: kmb_test_31_count_3 + identifier: kmb_test_32_count_3 attribute: count - type: assert target: reference - identifier: reference:kmb_test_31_count_3 + identifier: reference:kmb_test_32_count_3 test: equals: 1 diff --git a/Tests/phpunit/6_MassMigrateTest.php_ b/Tests/phpunit/6_MassMigrateTest.php_ new file mode 100644 index 00000000..08e25daa --- /dev/null +++ b/Tests/phpunit/6_MassMigrateTest.php_ @@ -0,0 +1,271 @@ +markTestSkipped(); + return; + } + + $this->prepareMigration($filePath); + + $count1 = BeforeStepExecutionListener::getExecutions(); + $count2 = StepExecutedListener::getExecutions(); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migrate', '--path' => array($filePath), '-n' => true, '-u' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + // check that there are no notes after adding the migration + $this->assertRegexp('?\| ' . basename($filePath) . ' +\| +\|?', $output); + + // simplistic check on the event listeners having fired off correctly + $this->assertGreaterThanOrEqual($count1 + 1, BeforeStepExecutionListener::getExecutions(), "Migration 'before step' listener did not fire"); + $this->assertGreaterThanOrEqual($count2 + 1, StepExecutedListener::getExecutions(), "Migration 'step executed' listener did not fire"); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migration', 'migration' => basename($filePath), '--delete' => true, '-n' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + } + + /** + * @param string $filePath + * @dataProvider invalidDSLProvider + */ + public function testExecuteInvalidDSL($filePath = '') + { + if ($filePath == '') { + $this->markTestSkipped(); + return; + } + + $this->prepareMigration($filePath); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migrate', '--path' => array($filePath), '-n' => true, '-u' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + // check that there are no notes after adding the migration + $this->assertRegexp('?Skipping ' . basename($filePath) . '?', $output); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migration', 'migration' => basename($filePath), '--delete' => true, '-n' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + } + + /** + * @param string $filePath + * @dataProvider badDSLProvider + */ + public function testExecuteBadDSL($filePath = '') + { + if ($filePath == '') { + $this->markTestSkipped(); + return; + } + + $this->prepareMigration($filePath); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migrate', '--path' => array($filePath), '-n' => true, '-u' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertNotSame(0, $exitCode, 'CLI Command should have failed. Output: ' . $output); + // check that there are no notes after adding the migration + $this->assertRegexp('?Migration failed!?', $output); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migration', 'migration' => basename($filePath), '--delete' => true, '-n' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + } + + /** + * Tests executing a very simple migration with all the different cli flags enabled or not + * @param array $options + * @dataProvider migrateOptionsProvider + */ + public function testExecuteWithDifferentOptions(array $options = array()) + { + $filePath = $this->dslDir . '/UnitTestOK031_helloworld.yml'; + + $this->prepareMigration($filePath); + + $count1 = BeforeStepExecutionListener::getExecutions(); + $count2 = StepExecutedListener::getExecutions(); + + $input = new ArrayInput(array_merge(array('command' => 'kaliop:migration:migrate', '--path' => array($filePath), '-n' => true), $options)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + // check that there are no notes after adding the migration + $this->assertRegexp('?\| ' . basename($filePath) . ' +\| +\|?', $output); + + // simplistic check on the event listeners having fired off correctly + $this->assertGreaterThanOrEqual($count1 + 1, BeforeStepExecutionListener::getExecutions(), "Migration 'before step' listener did not fire"); + $this->assertGreaterThanOrEqual($count2 + 1, StepExecutedListener::getExecutions(), "Migration 'step executed' listener did not fire"); + + $input = new ArrayInput(array('command' => 'kaliop:migration:migration', 'migration' => basename($filePath), '--delete' => true, '-n' => true)); + $exitCode = $this->app->run($input, $this->output); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + } + + public function goodDSLProvider() + { + $dslDir = $this->dslDir.'/good'; + if (!is_dir($dslDir)) { + return array(); + } + + $out = array(); + foreach (scandir($dslDir) as $fileName) { + $filePath = $dslDir . '/' . $fileName; + if (is_file($filePath)) { + $out[] = array($filePath); + } + } + return $out; + } + + public function invalidDSLProvider() + { + $dslDir = $this->dslDir.'/bad/parsing'; + if (!is_dir($dslDir)) { + return array(); + } + + $out = array(); + foreach (scandir($dslDir) as $fileName) { + $filePath = $dslDir . '/' . $fileName; + if (is_file($filePath)) { + $out[] = array($filePath); + } + } + return $out; + } + + public function badDSLProvider() + { + $dslDir = $this->dslDir.'/bad/execution'; + if (!is_dir($dslDir)) { + return array(); + } + + $out = array(); + foreach (scandir($dslDir) as $fileName) { + $filePath = $dslDir . '/' . $fileName; + if (is_file($filePath)) { + $out[] = array($filePath); + } + } + return $out; + } + + public function migrateOptionsProvider() + { + return array( + array(), + array('-c' => true), + array('clear-cache' => true), + array('-f' => true), + array('--force' => true), + array('-i' => true), + array('--ignore-failures' => true), + array('-u' => true), + array('--no-transactions' => true), + array('-p' => true), + array('--separate-process' => true), + array('--force-sigchild-enabled' => true), + array('--survive-disconnected-tty' => true), + ); + } + + /** + * Add a migration from a file to the migration service. + * @param string $filePath + */ + protected function addMigration($filePath) + { + $exitCode = $this->runCommand('kaliop:migration:migration', [ + 'migration' => $filePath, + '--add' => true, + '-n' => true, + ]); + $output = $this->fetchOutput(); + $this->assertSame(0, $exitCode, 'CLI Command failed. Output: ' . $output); + $this->assertRegexp('?Added migration?', $output); + } + + /** + * Delete the migration from the database table + * @param string $filePath + * @return string + */ + protected function deleteMigration($filePath) + { + $this->runCommand('kaliop:migration:migration', [ + 'migration' => basename($filePath), + '--delete' => true, + '-n' => true, + ]); + + return $this->fetchOutput(); + } + + /** + * Prepare a migration file for a test. + * @param string $filePath + */ + protected function prepareMigration($filePath) + { + // Make user migration is not in the db: delete it, ignoring errors + $this->deleteMigration($filePath); + $this->addMigration($filePath); + } + + /** + * Run a symfony command + * @param string $commandName + * @param array $params + * @return int + */ + protected function runCommand($commandName, array $params) + { + $params = array_merge(['command' => $commandName], $params); + $input = new ArrayInput($params); + + return $this->app->run($input, $this->output); + } + + /** + * Get the eZ repository + * @param int $loginUserId + * @return \eZ\Publish\Core\SignalSlot\Repository + */ + protected function getRepository($loginUserId = \Kaliop\eZMigrationBundle\Core\MigrationService::ADMIN_USER_ID) + { + $repository = $this->getContainer()->get('ezpublish.api.repository'); + if ($loginUserId !== false && (is_null($repository->getCurrentUser()) || $repository->getCurrentUser()->id != $loginUserId)) { + $repository->setCurrentUser($repository->getUserService()->loadUser($loginUserId)); + } + + return $repository; + } +} diff --git a/WHATSNEW.md b/WHATSNEW.md index fbf53a9a..ee727532 100644 --- a/WHATSNEW.md +++ b/WHATSNEW.md @@ -1,5 +1,15 @@ +Version 5.10.1 +============== + +* Fix issue #216: cannot update a location's parent matching it by remote id + +* Improved: when creating/updating content, allow to set references to `location_remote_id` + +* Improved: add plumbing to allow future usage of custom content types for UserGroups + + Version 5.10.0 -============= +============== * Fix issue #210: cannot match locations by group @@ -138,7 +148,8 @@ Version 5.8.0 * Fix: the `if` element was not giving a fatal error for all migration steps affecting repository elements (Content, Location, etc...), at least for Symfony version 2.7.10 -ezjscserverfunctionsjs.phprate` now accept a `--force` flag that will execute + +* New: the command `kaliop:migration:migrate` now accepts a `--force` flag that will execute migrations that were previously executed or skipped or failed. *NB* this flag is useful when testing migrations, but should be used sparingly in production context, as replaying migrations that had already been executed can wreak havoc to your database. *you have been warned* @@ -148,7 +159,7 @@ ezjscserverfunctionsjs.phprate` now accept a `--force` flag that will execute - the `kaliop:migration:generate` command now uses as default language for the generated migrations the default one of the current siteaccess, instead of 'eng-GB' - + Version 5.7.3 =============