From 5ff8c40e706ea906c3d82267499b61c13f1e5b3b Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Wed, 6 Nov 2024 03:57:33 +0300 Subject: [PATCH 1/4] fix: unique identifiers --- manifest.php | 4 +- .../Service/ResourceMetadataRetriever.php | 37 ++++++++++++++ .../TranslationServiceProvider.php | 10 +--- .../Service/ResourceUniqueIdRetriever.php | 49 +++++++++++++++++++ .../UniqueIdServiceProvider.php | 44 +++++++++++++++++ 5 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 models/classes/Translation/Service/ResourceMetadataRetriever.php create mode 100644 models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php create mode 100644 models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php diff --git a/manifest.php b/manifest.php index 5370f732cf..f827ecd986 100755 --- a/manifest.php +++ b/manifest.php @@ -60,6 +60,7 @@ use oat\tao\model\search\ServiceProvider\SearchServiceProvider; use oat\tao\model\StatisticalMetadata\StatisticalMetadataServiceProvider; use oat\tao\model\Translation\ServiceProvider\TranslationServiceProvider; +use oat\tao\model\UniqueId\ServiceProvider\UniqueIdServiceProvider; use oat\tao\model\user\TaoRoles; use oat\tao\model\user\UserSettingsServiceProvider; use oat\tao\model\webhooks\WebhookServiceProvider; @@ -424,7 +425,8 @@ DynamicConfigServiceProvider::class, TranslationServiceProvider::class, TaoFormServiceProvider::class, - IdentifierGeneratorServiceProvider::class + IdentifierGeneratorServiceProvider::class, + UniqueIdServiceProvider::class, ], 'middlewares' => [ MiddlewareConfig::class, diff --git a/models/classes/Translation/Service/ResourceMetadataRetriever.php b/models/classes/Translation/Service/ResourceMetadataRetriever.php new file mode 100644 index 0000000000..f12a7b2037 --- /dev/null +++ b/models/classes/Translation/Service/ResourceMetadataRetriever.php @@ -0,0 +1,37 @@ +getProperty(TaoOntology::PROPERTY_TRANSLATION_ORIGINAL_RESOURCE_URI); + $originalResourceUri = $resource->getOnePropertyValue($property); + + return !empty($originalResourceUri) ? $originalResourceUri->getUri() : null; + } +} diff --git a/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php b/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php index 72c8c283fb..19c248e53b 100644 --- a/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php +++ b/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php @@ -35,9 +35,9 @@ use oat\tao\model\Translation\Form\Modifier\TranslationFormModifier; use oat\tao\model\Translation\Repository\ResourceTranslatableRepository; use oat\tao\model\Translation\Repository\ResourceTranslationRepository; -use oat\tao\model\Translation\Service\Listener\ResourceDeletedEventListener; use oat\tao\model\Translation\Service\ResourceLanguageRetriever; use oat\tao\model\Translation\Service\ResourceMetadataPopulateService; +use oat\tao\model\Translation\Service\ResourceMetadataRetriever; use oat\tao\model\Translation\Service\ResourceTranslatableRetriever; use oat\tao\model\Translation\Service\ResourceTranslationRetriever; use oat\tao\model\Translation\Service\TranslationCreationService; @@ -169,12 +169,6 @@ public function __invoke(ContainerConfigurator $configurator): void ]) ->public(); - $services - ->set(ResourceDeletedEventListener::class, ResourceDeletedEventListener::class) - ->args([ - service(FeatureFlagChecker::class), - service(TranslationDeletionService::class), - ]) - ->public(); + $services->set(ResourceMetadataRetriever::class, ResourceMetadataRetriever::class); } } diff --git a/models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php b/models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php new file mode 100644 index 0000000000..f504363909 --- /dev/null +++ b/models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php @@ -0,0 +1,49 @@ +featureFlagChecker = $featureFlagChecker; + } + + public function retrieve(core_kernel_classes_Resource $resource): ?string + { + if (!$this->featureFlagChecker->isEnabled('FEATURE_FLAG_UNIQUE_NUMERIC_QTI_IDENTIFIER')) { + return null; + } + + $property = $resource->getProperty(TaoOntology::PROPERTY_UNIQUE_IDENTIFIER); + $uniqueId = $resource->getOnePropertyValue($property); + + return !empty($uniqueId) ? $uniqueId->literal : null; + } +} diff --git a/models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php b/models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php new file mode 100644 index 0000000000..f74a5b40be --- /dev/null +++ b/models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php @@ -0,0 +1,44 @@ +services(); + + $services + ->set(ResourceUniqueIdRetriever::class, ResourceUniqueIdRetriever::class) + ->args([ + service(FeatureFlagChecker::class), + ]); + } +} From 58bf96a68d805cf242469a9ddd603631ca9dc5f3 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Wed, 6 Nov 2024 13:29:14 +0300 Subject: [PATCH 2/4] chore: refactor --- manifest.php | 2 - .../Service/AbstractQtiIdentifierSetter.php | 74 ++++++++++++ .../Service/ResourceMetadataRetriever.php | 37 ------ .../Service/TranslationCreationService.php | 2 +- .../Service/TranslationUniqueIdSetter.php | 111 ++++++++++++++++++ .../TranslationServiceProvider.php | 8 +- .../Service/ResourceUniqueIdRetriever.php | 49 -------- .../UniqueIdServiceProvider.php | 44 ------- 8 files changed, 192 insertions(+), 135 deletions(-) create mode 100644 models/classes/Translation/Service/AbstractQtiIdentifierSetter.php delete mode 100644 models/classes/Translation/Service/ResourceMetadataRetriever.php create mode 100644 models/classes/Translation/Service/TranslationUniqueIdSetter.php delete mode 100644 models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php delete mode 100644 models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php diff --git a/manifest.php b/manifest.php index f827ecd986..d0bce7c64d 100755 --- a/manifest.php +++ b/manifest.php @@ -60,7 +60,6 @@ use oat\tao\model\search\ServiceProvider\SearchServiceProvider; use oat\tao\model\StatisticalMetadata\StatisticalMetadataServiceProvider; use oat\tao\model\Translation\ServiceProvider\TranslationServiceProvider; -use oat\tao\model\UniqueId\ServiceProvider\UniqueIdServiceProvider; use oat\tao\model\user\TaoRoles; use oat\tao\model\user\UserSettingsServiceProvider; use oat\tao\model\webhooks\WebhookServiceProvider; @@ -426,7 +425,6 @@ TranslationServiceProvider::class, TaoFormServiceProvider::class, IdentifierGeneratorServiceProvider::class, - UniqueIdServiceProvider::class, ], 'middlewares' => [ MiddlewareConfig::class, diff --git a/models/classes/Translation/Service/AbstractQtiIdentifierSetter.php b/models/classes/Translation/Service/AbstractQtiIdentifierSetter.php new file mode 100644 index 0000000000..99ef752d70 --- /dev/null +++ b/models/classes/Translation/Service/AbstractQtiIdentifierSetter.php @@ -0,0 +1,74 @@ +logger = $logger; + } + + public function set(array $options): void + { + try { + if (!isset($options[self::OPTION_RESOURCE], $options[self::OPTION_IDENTIFIER])) { + throw new InvalidArgumentException( + sprintf( + 'Options %s and %s are required to set QTI Identifier.', + self::OPTION_RESOURCE, + self::OPTION_IDENTIFIER + ) + ); + } + + $this->applyIdentifier($options); + } catch (Throwable $exception) { + $this->logger->error('An error occurred while setting QTI identifier: ' . $exception->getMessage()); + + throw $exception; + } + } + + abstract protected function applyIdentifier(array $options): void; + + protected function getResource(array $options): core_kernel_classes_Resource + { + return $options[self::OPTION_RESOURCE]; + } + + protected function getIdentifier(array $options): string + { + return $options[self::OPTION_IDENTIFIER]; + } +} diff --git a/models/classes/Translation/Service/ResourceMetadataRetriever.php b/models/classes/Translation/Service/ResourceMetadataRetriever.php deleted file mode 100644 index f12a7b2037..0000000000 --- a/models/classes/Translation/Service/ResourceMetadataRetriever.php +++ /dev/null @@ -1,37 +0,0 @@ -getProperty(TaoOntology::PROPERTY_TRANSLATION_ORIGINAL_RESOURCE_URI); - $originalResourceUri = $resource->getOnePropertyValue($property); - - return !empty($originalResourceUri) ? $originalResourceUri->getUri() : null; - } -} diff --git a/models/classes/Translation/Service/TranslationCreationService.php b/models/classes/Translation/Service/TranslationCreationService.php index 4c54f62039..816e88187a 100644 --- a/models/classes/Translation/Service/TranslationCreationService.php +++ b/models/classes/Translation/Service/TranslationCreationService.php @@ -188,7 +188,7 @@ public function create(CreateTranslationCommand $command): core_kernel_classes_R ); foreach ($this->callables[$rootId] ?? [] as $callable) { - $clonedInstance = $callable($clonedInstance); + $callable($clonedInstance); } return $clonedInstance; diff --git a/models/classes/Translation/Service/TranslationUniqueIdSetter.php b/models/classes/Translation/Service/TranslationUniqueIdSetter.php new file mode 100644 index 0000000000..64c450b6a5 --- /dev/null +++ b/models/classes/Translation/Service/TranslationUniqueIdSetter.php @@ -0,0 +1,111 @@ +featureFlagChecker = $featureFlagChecker; + } + + public function addQtiIdentifierSetter(AbstractQtiIdentifierSetter $qtiIdentifierSetter, string $resourceType): void + { + if (isset($this->qtiIdentifierSetters[$resourceType])) { + throw new InvalidArgumentException( + 'QTI Identifier setter already exists for resource type ' . $resourceType + ); + } + + $this->qtiIdentifierSetters[$resourceType] = $qtiIdentifierSetter; + } + + public function __invoke(core_kernel_classes_Resource $item): void + { + if ( + !$this->featureFlagChecker->isEnabled('FEATURE_FLAG_UNIQUE_NUMERIC_QTI_IDENTIFIER') + && !$this->featureFlagChecker->isEnabled('FEATURE_FLAG_TRANSLATION_ENABLED') + ) { + return; + } + + $originalResource = $this->getOriginalResource($item); + $uniqueIdentifier = $this->getUniqueId($originalResource); + + $item->editPropertyValues($item->getProperty(TaoOntology::PROPERTY_UNIQUE_IDENTIFIER), $uniqueIdentifier); + $this->getQtiIdentifierSetter($item)->set([ + AbstractQtiIdentifierSetter::OPTION_RESOURCE => $item, + AbstractQtiIdentifierSetter::OPTION_IDENTIFIER => $uniqueIdentifier, + ]); + } + + private function getOriginalResource(core_kernel_classes_Resource $resource): core_kernel_classes_Resource + { + $property = $resource->getProperty(TaoOntology::PROPERTY_TRANSLATION_ORIGINAL_RESOURCE_URI); + $originalResourceUri = $resource->getOnePropertyValue($property); + + if (empty($originalResourceUri)) { + throw new InvalidArgumentException( + sprintf( + 'Resource %s is not a translation - original resource URI is empty', + $originalResourceUri + ) + ); + } + + return $resource->getResource($originalResourceUri); + } + + private function getUniqueId(core_kernel_classes_Resource $resource): ?string + { + $property = $resource->getProperty(TaoOntology::PROPERTY_UNIQUE_IDENTIFIER); + $uniqueId = $resource->getOnePropertyValue($property); + + if (empty($uniqueId)) { + throw new InvalidArgumentException('Unique ID must exists for resource URI ' . $resource->getUri()); + } + + return $uniqueId->literal; + } + + private function getQtiIdentifierSetter(core_kernel_classes_Resource $resource): AbstractQtiIdentifierSetter + { + $resourceType = $resource->getRootId(); + + if (!isset($this->qtiIdentifierSetters[$resourceType])) { + throw new InvalidArgumentException( + 'QTI Identifier setter does not exist for resource type ' . $resourceType + ); + } + + return $this->qtiIdentifierSetters[$resourceType]; + } +} diff --git a/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php b/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php index 19c248e53b..1314ae7745 100644 --- a/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php +++ b/models/classes/Translation/ServiceProvider/TranslationServiceProvider.php @@ -37,12 +37,12 @@ use oat\tao\model\Translation\Repository\ResourceTranslationRepository; use oat\tao\model\Translation\Service\ResourceLanguageRetriever; use oat\tao\model\Translation\Service\ResourceMetadataPopulateService; -use oat\tao\model\Translation\Service\ResourceMetadataRetriever; use oat\tao\model\Translation\Service\ResourceTranslatableRetriever; use oat\tao\model\Translation\Service\ResourceTranslationRetriever; use oat\tao\model\Translation\Service\TranslationCreationService; use oat\tao\model\Translation\Service\TranslationDeletionService; use oat\tao\model\Translation\Service\TranslationSyncService; +use oat\tao\model\Translation\Service\TranslationUniqueIdSetter; use oat\tao\model\Translation\Service\TranslationUpdateService; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -169,6 +169,10 @@ public function __invoke(ContainerConfigurator $configurator): void ]) ->public(); - $services->set(ResourceMetadataRetriever::class, ResourceMetadataRetriever::class); + $services + ->set(TranslationUniqueIdSetter::class, TranslationUniqueIdSetter::class) + ->args([ + service(FeatureFlagChecker::class), + ]); } } diff --git a/models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php b/models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php deleted file mode 100644 index f504363909..0000000000 --- a/models/classes/UniqueId/Service/ResourceUniqueIdRetriever.php +++ /dev/null @@ -1,49 +0,0 @@ -featureFlagChecker = $featureFlagChecker; - } - - public function retrieve(core_kernel_classes_Resource $resource): ?string - { - if (!$this->featureFlagChecker->isEnabled('FEATURE_FLAG_UNIQUE_NUMERIC_QTI_IDENTIFIER')) { - return null; - } - - $property = $resource->getProperty(TaoOntology::PROPERTY_UNIQUE_IDENTIFIER); - $uniqueId = $resource->getOnePropertyValue($property); - - return !empty($uniqueId) ? $uniqueId->literal : null; - } -} diff --git a/models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php b/models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php deleted file mode 100644 index f74a5b40be..0000000000 --- a/models/classes/UniqueId/ServiceProvider/UniqueIdServiceProvider.php +++ /dev/null @@ -1,44 +0,0 @@ -services(); - - $services - ->set(ResourceUniqueIdRetriever::class, ResourceUniqueIdRetriever::class) - ->args([ - service(FeatureFlagChecker::class), - ]); - } -} From f640dff15b0915bed177beb17fa129a994b42837 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Wed, 6 Nov 2024 13:49:38 +0300 Subject: [PATCH 3/4] fix: require both FF to be enabled --- .../classes/Translation/Service/TranslationUniqueIdSetter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/classes/Translation/Service/TranslationUniqueIdSetter.php b/models/classes/Translation/Service/TranslationUniqueIdSetter.php index 64c450b6a5..a549ee1bf6 100644 --- a/models/classes/Translation/Service/TranslationUniqueIdSetter.php +++ b/models/classes/Translation/Service/TranslationUniqueIdSetter.php @@ -52,7 +52,7 @@ public function __invoke(core_kernel_classes_Resource $item): void { if ( !$this->featureFlagChecker->isEnabled('FEATURE_FLAG_UNIQUE_NUMERIC_QTI_IDENTIFIER') - && !$this->featureFlagChecker->isEnabled('FEATURE_FLAG_TRANSLATION_ENABLED') + || !$this->featureFlagChecker->isEnabled('FEATURE_FLAG_TRANSLATION_ENABLED') ) { return; } From 801481f6a24e08ec40d4e1579a0013a06d5fcbb9 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Wed, 6 Nov 2024 13:54:48 +0300 Subject: [PATCH 4/4] chore: fix variable name --- .../Service/TranslationUniqueIdSetter.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/models/classes/Translation/Service/TranslationUniqueIdSetter.php b/models/classes/Translation/Service/TranslationUniqueIdSetter.php index a549ee1bf6..d123ce4f67 100644 --- a/models/classes/Translation/Service/TranslationUniqueIdSetter.php +++ b/models/classes/Translation/Service/TranslationUniqueIdSetter.php @@ -48,7 +48,7 @@ public function addQtiIdentifierSetter(AbstractQtiIdentifierSetter $qtiIdentifie $this->qtiIdentifierSetters[$resourceType] = $qtiIdentifierSetter; } - public function __invoke(core_kernel_classes_Resource $item): void + public function __invoke(core_kernel_classes_Resource $resource): void { if ( !$this->featureFlagChecker->isEnabled('FEATURE_FLAG_UNIQUE_NUMERIC_QTI_IDENTIFIER') @@ -57,12 +57,15 @@ public function __invoke(core_kernel_classes_Resource $item): void return; } - $originalResource = $this->getOriginalResource($item); + $originalResource = $this->getOriginalResource($resource); $uniqueIdentifier = $this->getUniqueId($originalResource); - $item->editPropertyValues($item->getProperty(TaoOntology::PROPERTY_UNIQUE_IDENTIFIER), $uniqueIdentifier); - $this->getQtiIdentifierSetter($item)->set([ - AbstractQtiIdentifierSetter::OPTION_RESOURCE => $item, + $resource->editPropertyValues( + $resource->getProperty(TaoOntology::PROPERTY_UNIQUE_IDENTIFIER), + $uniqueIdentifier + ); + $this->getQtiIdentifierSetter($resource)->set([ + AbstractQtiIdentifierSetter::OPTION_RESOURCE => $resource, AbstractQtiIdentifierSetter::OPTION_IDENTIFIER => $uniqueIdentifier, ]); }