From b29fefe26251d94c1fcce99ca06ade3635d7b128 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Tue, 27 Jul 2021 10:55:35 +0200 Subject: [PATCH 1/8] IBX-201: Refactored ExportParametersFactory --- .../Resources/config/services/factory.yaml | 22 +- .../Resources/config/services/values.yaml | 3 - .../CredentialsNotFoundException.php | 20 +- .../ExportCredentialsNotFoundException.php | 13 - .../MissingExportParameterException.php | 29 +- .../ConfigurableExportParametersFactory.php | 130 ------- src/lib/Factory/Export/ParametersFactory.php | 341 ++++++++++++++++++ .../Export/ParametersFactoryInterface.php | 32 ++ src/lib/Factory/ExportParametersFactory.php | 39 -- .../ExportParametersFactoryInterface.php | 21 -- .../SPI/ExportParametersFactoryDecorator.php | 31 -- src/lib/Value/Export/Parameters.php | 128 +++++++ .../Factory/Export/ParametersFactoryTest.php | 265 ++++++++++++++ 13 files changed, 827 insertions(+), 247 deletions(-) delete mode 100644 src/lib/Exception/ExportCredentialsNotFoundException.php delete mode 100644 src/lib/Factory/ConfigurableExportParametersFactory.php create mode 100644 src/lib/Factory/Export/ParametersFactory.php create mode 100644 src/lib/Factory/Export/ParametersFactoryInterface.php delete mode 100644 src/lib/Factory/ExportParametersFactory.php delete mode 100644 src/lib/Factory/ExportParametersFactoryInterface.php delete mode 100644 src/lib/SPI/ExportParametersFactoryDecorator.php create mode 100644 src/lib/Value/Export/Parameters.php create mode 100644 tests/lib/Factory/Export/ParametersFactoryTest.php diff --git a/src/bundle/Resources/config/services/factory.yaml b/src/bundle/Resources/config/services/factory.yaml index 87651350..b2720b4f 100644 --- a/src/bundle/Resources/config/services/factory.yaml +++ b/src/bundle/Resources/config/services/factory.yaml @@ -4,12 +4,22 @@ services: autoconfigure: true public: false - EzSystems\EzRecommendationClient\Factory\: - resource: '../../../../src/lib/Factory/*' + EzSystems\EzRecommendationClient\Factory\EzRecommendationClientAPIFactory: ~ - EzSystems\EzRecommendationClient\Factory\ExportParametersFactory: ~ + EzSystems\EzRecommendationClient\Factory\FakeRequestFactory: ~ - EzSystems\EzRecommendationClient\Factory\ConfigurableExportParametersFactory: + EzSystems\EzRecommendationClient\Factory\RequestFactoryInterface: + '@EzSystems\EzRecommendationClient\Factory\FakeRequestFactory' + + EzSystems\EzRecommendationClient\Factory\TokenFactory: ~ + + EzSystems\EzRecommendationClient\Factory\TokenFactoryInterface: + '@EzSystems\EzRecommendationClient\Factory\TokenFactory' + + Ibexa\Personalization\Factory\Export\ParametersFactory: arguments: - $innerService: '@EzSystems\EzRecommendationClient\Factory\ExportParametersFactory' - $credentialsResolver: '@EzSystems\EzRecommendationClient\Config\ExportCredentialsResolver' + $siteAccessService: '@ezpublish.siteaccess_service' + $credentialsResolver: '@EzSystems\EzRecommendationClient\Config\EzRecommendationClientCredentialsResolver' + + Ibexa\Personalization\Factory\Export\ParametersFactoryInterface: + '@Ibexa\Personalization\Factory\Export\ParametersFactory' diff --git a/src/bundle/Resources/config/services/values.yaml b/src/bundle/Resources/config/services/values.yaml index f4b7eb52..af6aedaf 100644 --- a/src/bundle/Resources/config/services/values.yaml +++ b/src/bundle/Resources/config/services/values.yaml @@ -4,9 +4,6 @@ services: autoconfigure: true public: false - EzSystems\EzRecommendationClient\Value\: - resource: '../../../../src/lib/Value/*' - EzSystems\EzRecommendationClient\Value\ContentDataVisitor: tags: - { name: ezpublish_rest.output.value_object_visitor, type: EzSystems\EzRecommendationClient\Value\ContentData } diff --git a/src/lib/Exception/CredentialsNotFoundException.php b/src/lib/Exception/CredentialsNotFoundException.php index e9e87eba..75290217 100644 --- a/src/lib/Exception/CredentialsNotFoundException.php +++ b/src/lib/Exception/CredentialsNotFoundException.php @@ -8,10 +8,24 @@ namespace EzSystems\EzRecommendationClient\Exception; -class CredentialsNotFoundException extends NotFoundException +use Throwable; + +final class CredentialsNotFoundException extends NotFoundException { - public function __construct(int $code = 0, ?Throwable $previous = null) + public function __construct(?string $siteAccess = null, int $code = 0, Throwable $previous = null) { - parent::__construct('Credentials for recommendation client are not set', $code, $previous); + $message = 'Credentials for recommendation client are not set'; + + if (null !== $siteAccess) { + $message .= sprintf( + ' for siteAccess: %s', $siteAccess + ); + } + + parent::__construct( + $message, + $code, + $previous + ); } } diff --git a/src/lib/Exception/ExportCredentialsNotFoundException.php b/src/lib/Exception/ExportCredentialsNotFoundException.php deleted file mode 100644 index 986b9ccf..00000000 --- a/src/lib/Exception/ExportCredentialsNotFoundException.php +++ /dev/null @@ -1,13 +0,0 @@ - $missingParameters + */ + public function __construct(array $missingParameters, string $type, int $code = 0, Throwable $previous = null) + { + $parameters = []; + + if ($type === ParametersFactoryInterface::COMMAND_TYPE) { + foreach ($missingParameters as $parameter) { + $parameters[] = str_replace('_', '-', $parameter); + } + } else { + $parameters = $missingParameters; + } + + parent::__construct( + sprintf( + 'Required parameters: %s are missing', + implode(', ', $parameters) + ), + $code, + $previous + ); + } } diff --git a/src/lib/Factory/ConfigurableExportParametersFactory.php b/src/lib/Factory/ConfigurableExportParametersFactory.php deleted file mode 100644 index f1444417..00000000 --- a/src/lib/Factory/ConfigurableExportParametersFactory.php +++ /dev/null @@ -1,130 +0,0 @@ -credentialsResolver = $credentialsResolver; - $this->configResolver = $configResolver; - $this->siteAccessHelper = $siteAccessHelper; - - parent::__construct($innerService); - } - - /** - * {@inheritdoc} - * - * @throws \EzSystems\EzRecommendationClient\Exception\ExportCredentialsNotFoundException - * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException - * @throws \eZ\Publish\Core\Base\Exceptions\NotFoundException - */ - public function create(array $properties = []): ExportParameters - { - if (!empty($this->getMissingRequiredOptions($properties))) { - throw new MissingExportParameterException(sprintf( - 'Required parameters %s are missing', - implode(', ', $this->getMissingRequiredOptions($properties)) - )); - } - - $properties['siteaccess'] = $properties['siteaccess'] ?? $this->getSiteAccess(); - - if (!isset($properties['customerId']) && !isset($properties['licenseKey'])) { - /** @var \EzSystems\EzRecommendationClient\Value\Config\ExportCredentials $credentials */ - $credentials = $this->credentialsResolver->getCredentials($properties['siteaccess']); - - if (!$this->credentialsResolver->hasCredentials()) { - throw new ExportCredentialsNotFoundException(sprintf( - 'Recommendation client export credentials are not set for siteAccess: %s', - $properties['siteaccess'] - )); - } - - $properties['customerId'] = $credentials->getLogin(); - $properties['licenseKey'] = $credentials->getPassword(); - } - - $properties['host'] = $properties['host'] ?? $this->getHostUri($properties['siteaccess']); - $properties['webHook'] = $properties['webHook'] ?? $this->getWebHook( - (int)$properties['customerId'], - $properties['siteaccess'] - ); - - return $this->innerService->create($properties); - } - - private function getSiteAccess(): string - { - return current($this->siteAccessHelper->getSiteAccesses()); - } - - private function getHostUri(string $siteAccess): string - { - return $this->configResolver->getParameter( - 'host_uri', - Parameters::NAMESPACE, - $siteAccess - ); - } - - private function getWebHook(int $customerId, string $siteAccess): string - { - return $this->configResolver->getParameter( - 'api.notifier.endpoint', - Parameters::NAMESPACE, - $siteAccess - ) . sprintf(Notifier::ENDPOINT_PATH, $customerId); - } - - private function getMissingRequiredOptions(array $options): array - { - $missingOptions = []; - - if (isset($options['customerId']) || isset($options['licenseKey'])) { - if (!isset($options['customerId'])) { - $missingOptions[] = 'customerId'; - } - - if (!isset($options['licenseKey'])) { - $missingOptions[] = 'licenseKey'; - } - - if (!isset($options['siteaccess'])) { - $missingOptions[] = 'siteaccess'; - } - } - - return $missingOptions; - } -} diff --git a/src/lib/Factory/Export/ParametersFactory.php b/src/lib/Factory/Export/ParametersFactory.php new file mode 100644 index 00000000..dc28761f --- /dev/null +++ b/src/lib/Factory/Export/ParametersFactory.php @@ -0,0 +1,341 @@ +credentialsResolver = $credentialsResolver; + $this->configResolver = $configResolver; + $this->siteAccessService = $siteAccessService; + } + + public function create(array $options, string $type): Parameters + { + $this->exportParametersType = $type; + + return Parameters::fromArray( + $this->getExportParameters($options) + ); + } + + /** + * @phpstan-param array{ + * item_type_identifier_list: string, + * languages: string, + * page_size: string, + * customer_id: ?string, + * license_key: ?string, + * siteaccess: ?string, + * web_hook: ?string, + * host: ?string, + * } $options + * + * @phpstan-return array{ + * item_type_identifier_list: string, + * languages: string, + * page_size: string, + * customer_id: string, + * license_key: string, + * siteaccess: string, + * web_hook: string, + * host: string, + * } + * + * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException + * @throws \EzSystems\EzRecommendationClient\Exception\InvalidArgumentException + */ + private function getExportParameters(array $options): array + { + if (isset($options['siteaccess']) && !$this->hasConfiguredSiteAccess($options['siteaccess'])) { + throw new InvalidArgumentException(sprintf( + 'SiteAccess %s doesn\'t exists', + $options['siteaccess'] + )); + } + + $configuration = $this->countConfiguredCustomerSettings() === 1 + ? $this->getCredentialsAndSiteAccessForSingleConfiguration($options) + : $this->getCredentialsAndSiteAccessForMultiCustomerConfiguration($options); + + $siteAccess = $configuration['siteaccess']; + $customerId = $configuration['customer_id']; + + return [ + 'customer_id' => $customerId, + 'license_key' => $configuration['license_key'], + 'siteaccess' => $configuration['siteaccess'], + 'item_type_identifier_list' => $options['item_type_identifier_list'], + 'languages' => $options['languages'], + 'host' => $options['host'] ?? $this->getHostUri($siteAccess), + 'web_hook' => $options['web_hook'] ?? $this->getWebHook( + (int)$customerId, + $siteAccess), + 'page_size' => $options['page_size'], + ]; + } + + private function hasConfiguredSiteAccess(string $siteAccessName): bool + { + foreach ($this->siteAccessService->getAll() as $siteAccess) { + if ($siteAccessName === $siteAccess->name) { + return true; + } + } + + return false; + } + + private function countConfiguredCustomerSettings(): int + { + $configuredCounter = 0; + + foreach ($this->siteAccessService->getAll() as $siteAccess) { + if ($this->credentialsResolver->hasCredentials($siteAccess->name)) { + ++$configuredCounter; + } + } + + return $configuredCounter; + } + + /** + * @phpstan-param array{ + * item_type_identifier_list: string, + * languages: string, + * page_size: string, + * customer_id: ?string, + * license_key: ?string, + * siteaccess: ?string, + * web_hook: ?string, + * host: ?string, + * } $options + * + * @phpstan-return array{ + * customer_id: string, + * license_key: string, + * siteaccess: string, + * } + * + * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException + */ + private function getCredentialsAndSiteAccessForSingleConfiguration(array $options): array + { + if ( + (isset($options['customer_id']) || isset($options['license_key'])) + && $this->hasMissingRequiredOptions($options) + ) { + throw new MissingExportParameterException( + $this->getMissingRequiredOptions($options), + $this->exportParametersType + ); + } + + if (null !== $options['customer_id'] && null !== $options['license_key'] && null !== $options['siteaccess']) { + return [ + 'customer_id' => $options['customer_id'], + 'license_key' => $options['license_key'], + 'siteaccess' => $options['siteaccess'], + ]; + } + + return $this->getCredentialsAndSiteAccess(); + } + + /** + * @phpstan-return array{ + * customer_id: string, + * license_key: string, + * siteaccess: string, + * } + */ + private function getCredentialsAndSiteAccess(): array + { + $siteAccess = $this->getSingleConfiguredSiteAccess(); + $configuredCredentials = $this->getCredentialsForScope($siteAccess); + + return [ + 'customer_id' => $configuredCredentials['customer_id'], + 'license_key' => $configuredCredentials['license_key'], + 'siteaccess' => $siteAccess, + ]; + } + + private function getSingleConfiguredSiteAccess(): string + { + foreach ($this->siteAccessService->getAll() as $siteAccess) { + if ($this->credentialsResolver->hasCredentials($siteAccess->name)) { + return $siteAccess->name; + } + } + + throw new CredentialsNotFoundException(); + } + + /** + * @phpstan-return array{ + * customer_id: string, + * license_key: string, + * } + */ + private function getCredentialsForScope(string $siteAccess): array + { + if (!$this->credentialsResolver->hasCredentials($siteAccess)) { + throw new CredentialsNotFoundException($siteAccess); + } + + /** @var \EzSystems\EzRecommendationClient\Value\Config\EzRecommendationClientCredentials $credentials */ + $credentials = $this->credentialsResolver->getCredentials($siteAccess); + /** @var int $customerId */ + $customerId = $credentials->getCustomerId(); + /** @var string $licenseKey */ + $licenseKey = $credentials->getLicenseKey(); + + return [ + 'customer_id' => (string)$customerId, + 'license_key' => $licenseKey, + ]; + } + + /** + * @phpstan-param array{ + * item_type_identifier_list: string, + * languages: string, + * page_size: string, + * customer_id: ?string, + * license_key: ?string, + * siteaccess: ?string, + * web_hook: ?string, + * host: ?string, + * } $options + * + * @phpstan-return array{ + * customer_id: string, + * license_key: string, + * siteaccess: string, + * } + * + * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException + */ + private function getCredentialsAndSiteAccessForMultiCustomerConfiguration(array $options): array + { + if ($this->hasMissingRequiredOptions($options)) { + throw new MissingExportParameterException( + $this->getMissingRequiredOptions($options), + $this->exportParametersType + ); + } + + /** @var string $customerId */ + $customerId = $options['customer_id']; + /** @var string $licenseKey */ + $licenseKey = $options['license_key']; + /** @var string $siteAccess */ + $siteAccess = $options['siteaccess']; + + return [ + 'customer_id' => $customerId, + 'license_key' => $licenseKey, + 'siteaccess' => $siteAccess, + ]; + } + + private function getHostUri(string $siteAccess): string + { + return $this->configResolver->getParameter( + 'host_uri', + 'ezrecommendation', + $siteAccess + ); + } + + private function getWebHook(int $customerId, string $siteAccess): string + { + return $this->configResolver->getParameter( + 'api.notifier.endpoint', + 'ezrecommendation', + $siteAccess + ) . sprintf(Notifier::ENDPOINT_PATH, $customerId); + } + + /** + * @phpstan-param array{ + * item_type_identifier_list: string, + * languages: string, + * page_size: string, + * customer_id: ?string, + * license_key: ?string, + * siteaccess: ?string, + * web_hook: ?string, + * host: ?string, + * } $options + */ + private function hasMissingRequiredOptions(array $options): bool + { + return !empty($this->getMissingRequiredOptions($options)); + } + + /** + * Checks if one of the required option is missing. + * For single configuration user doesn't need to provide any of these options, + * but if one of them is provided then rest of it are needed. + * + * @phpstan-param array{ + * item_type_identifier_list: string, + * languages: string, + * page_size: string, + * customer_id: ?string, + * license_key: ?string, + * siteaccess: ?string, + * web_hook: ?string, + * host: ?string, + * } $options + * + * @return array + */ + private function getMissingRequiredOptions(array $options): array + { + $missingOptions = []; + + if (!isset($options['customer_id'])) { + $missingOptions[] = 'customer_id'; + } + + if (!isset($options['license_key'])) { + $missingOptions[] = 'license_key'; + } + + if (!isset($options['siteaccess'])) { + $missingOptions[] = 'siteaccess'; + } + + return $missingOptions; + } +} diff --git a/src/lib/Factory/Export/ParametersFactoryInterface.php b/src/lib/Factory/Export/ParametersFactoryInterface.php new file mode 100644 index 00000000..1280f9b4 --- /dev/null +++ b/src/lib/Factory/Export/ParametersFactoryInterface.php @@ -0,0 +1,32 @@ +siteAccessHelper = $siteAccessHelper; - } - - public function create(array $properties = []): ExportParameters - { - $properties['contentTypeIdList'] = ParamsConverterHelper::getIdListFromString( - $properties['contentTypeIdList'] - ); - $properties['languages'] = $this->siteAccessHelper->getLanguages( - (int)$properties['customerId'], - $properties['siteaccess'] - ); - - isset($properties['fields']) ? ParamsConverterHelper::getArrayFromString($properties['fields']) : null; - - return new ExportParameters($properties); - } -} diff --git a/src/lib/Factory/ExportParametersFactoryInterface.php b/src/lib/Factory/ExportParametersFactoryInterface.php deleted file mode 100644 index 677c4d2b..00000000 --- a/src/lib/Factory/ExportParametersFactoryInterface.php +++ /dev/null @@ -1,21 +0,0 @@ -innerService = $innerService; - } - - /** - * {@inheritdoc} - */ - public function create(array $properties = []): ExportParameters - { - return $this->innerService->create($properties); - } -} diff --git a/src/lib/Value/Export/Parameters.php b/src/lib/Value/Export/Parameters.php new file mode 100644 index 00000000..9f712f1b --- /dev/null +++ b/src/lib/Value/Export/Parameters.php @@ -0,0 +1,128 @@ + */ + public array $itemTypeIdentifierList; + + /** @var array */ + public array $languages; + + public string $siteaccess; + + public string $webHook; + + public string $host; + + public int $pageSize; + + /** + * @param array $itemTypeIdentifierList + * @param array $languages + */ + private function __construct( + string $customerId, + string $licenseKey, + array $itemTypeIdentifierList, + array $languages, + string $siteaccess, + string $webHook, + string $host, + int $pageSize + ) { + $this->customerId = $customerId; + $this->licenseKey = $licenseKey; + $this->itemTypeIdentifierList = $itemTypeIdentifierList; + $this->languages = $languages; + $this->siteaccess = $siteaccess; + $this->webHook = $webHook; + $this->host = $host; + $this->pageSize = $pageSize; + } + + public function getCustomerId(): string + { + return $this->customerId; + } + + public function getLicenseKey(): string + { + return $this->licenseKey; + } + + /** + * @return array + */ + public function getItemTypeIdentifierList(): array + { + return $this->itemTypeIdentifierList; + } + + /** + * @return array + */ + public function getLanguages(): array + { + return $this->languages; + } + + public function getSiteaccess(): string + { + return $this->siteaccess; + } + + public function getWebHook(): string + { + return $this->webHook; + } + + public function getHost(): string + { + return $this->host; + } + + public function getPageSize(): int + { + return $this->pageSize; + } + + /** + * @phpstan-param array{ + * customer_id: string, + * license_key: string, + * item_type_identifier_list: string, + * languages: string, + * siteaccess: string, + * web_hook: string, + * host: string, + * page_size: string, + * } $properties + */ + public static function fromArray(array $properties): self + { + return new self( + $properties['customer_id'], + $properties['license_key'], + ParamsConverterHelper::getArrayFromString($properties['item_type_identifier_list']), + ParamsConverterHelper::getArrayFromString($properties['languages']), + $properties['siteaccess'], + $properties['web_hook'], + $properties['host'], + (int)$properties['page_size'], + ); + } +} diff --git a/tests/lib/Factory/Export/ParametersFactoryTest.php b/tests/lib/Factory/Export/ParametersFactoryTest.php new file mode 100644 index 00000000..cf2798da --- /dev/null +++ b/tests/lib/Factory/Export/ParametersFactoryTest.php @@ -0,0 +1,265 @@ +credentialsResolver = $this->createMock(CredentialsResolverInterface::class); + $this->configResolver = $this->createMock(ConfigResolverInterface::class); + $this->siteAccessService = $this->createMock(SiteAccessServiceInterface::class); + $this->parametersFactory = new ParametersFactory( + $this->credentialsResolver, + $this->configResolver, + $this->siteAccessService, + ); + $this->options = [ + 'customer_id' => '12345', + 'license_key' => '12345-12345-12345-12345', + 'siteaccess' => 'test', + 'item_type_identifier_list' => 'article, product, blog', + 'languages' => 'eng-GB', + 'web_hook' => 'https://reco-engine.com/api/12345/items', + 'host' => 'https://127.0.0.1', + 'page_size' => '500', + ]; + } + + /** + * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException + */ + public function testCreateFromAllOptions(): void + { + $this->getAllSiteAccesses(); + + self::assertEquals( + Parameters::fromArray($this->options), + $this->parametersFactory->create($this->options, ParametersFactoryInterface::COMMAND_TYPE) + ); + } + + public function testCreateWithAutocomplete(): void + { + $siteAccess = 'test'; + $options = [ + 'customer_id' => null, + 'license_key' => null, + 'siteaccess' => null, + 'item_type_identifier_list' => 'article, product, blog', + 'languages' => 'eng-GB', + 'page_size' => '500', + 'web_hook' => null, + 'host' => null, + ]; + + $this->credentialsResolver + ->expects(self::atLeastOnce()) + ->method('hasCredentials') + ->with($siteAccess) + ->willReturn(true); + + $this->siteAccessService + ->expects(self::atLeastOnce()) + ->method('getAll') + ->willReturn( + [ + new SiteAccess($siteAccess), + ] + ); + + $this->credentialsResolver + ->expects(self::once()) + ->method('getCredentials') + ->with($siteAccess) + ->willReturn(new EzRecommendationClientCredentials( + 12345, + '12345-12345-12345-12345' + )); + + $this->getHostUriAndApiNotifierUri($siteAccess); + + self::assertEquals( + Parameters::fromArray($this->options), + $this->parametersFactory->create($options, ParametersFactoryInterface::COMMAND_TYPE) + ); + } + + public function testCreateForSingleConfiguration(): void + { + $siteAccess = 'test'; + $options = [ + 'customer_id' => '12345', + 'license_key' => '12345-12345-12345-12345', + 'siteaccess' => $siteAccess, + 'item_type_identifier_list' => 'article, product, blog', + 'languages' => 'eng-GB', + 'page_size' => '500', + 'web_hook' => null, + 'host' => null, + ]; + + $this->siteAccessService + ->method('getAll') + ->willReturn( + [ + new SiteAccess($siteAccess), + ] + ); + + $this->credentialsResolver + ->expects(self::once()) + ->method('hasCredentials') + ->with($siteAccess) + ->willReturn(true); + + $this->getHostUriAndApiNotifierUri($siteAccess); + + self::assertEquals( + Parameters::fromArray($this->options), + $this->parametersFactory->create($options, ParametersFactoryInterface::COMMAND_TYPE) + ); + } + + public function testCreateForMultiCustomerConfiguration(): void + { + $firstSiteAccess = 'test'; + $secondSiteAccess = 'second_siteaccess'; + + $options = [ + 'customer_id' => '12345', + 'license_key' => '12345-12345-12345-12345', + 'siteaccess' => $firstSiteAccess, + 'item_type_identifier_list' => 'article, product, blog', + 'languages' => 'eng-GB', + 'page_size' => '500', + 'web_hook' => null, + 'host' => null, + ]; + + $this->getAllSiteAccesses(); + + $this->credentialsResolver + ->expects(self::at(0)) + ->method('hasCredentials') + ->with($firstSiteAccess) + ->willReturn(true); + + $this->credentialsResolver + ->expects(self::at(1)) + ->method('hasCredentials') + ->with($secondSiteAccess) + ->willReturn(true); + + $this->getHostUriAndApiNotifierUri($firstSiteAccess); + + self::assertEquals( + Parameters::fromArray($this->options), + $this->parametersFactory->create($options, ParametersFactoryInterface::COMMAND_TYPE) + ); + } + + /** + * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException + */ + public function testThrowExportCredentialsNotFoundException(): void + { + $siteAccess = 'invalid'; + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('SiteAccess %s doesn\'t exists', $siteAccess)); + + $this->getAllSiteAccesses(); + + $this->parametersFactory->create( + [ + 'customer_id' => null, + 'license_key' => null, + 'siteaccess' => $siteAccess, + 'item_type_identifier_list' => 'article, product, blog', + 'languages' => 'eng-GB', + 'page_size' => '500', + 'web_hook' => null, + 'host' => null, + ], + ParametersFactoryInterface::COMMAND_TYPE + ); + } + + private function getAllSiteAccesses(): void + { + $this->siteAccessService + ->method('getAll') + ->willReturn( + [ + new SiteAccess('test'), + new SiteAccess('second_siteaccess'), + ] + ); + } + + private function getHostUriAndApiNotifierUri(string $siteAccess): void + { + $this->configResolver + ->expects(self::at(0)) + ->method('getParameter') + ->with( + 'host_uri', + 'ezrecommendation', + $siteAccess + ) + ->willReturn('https://127.0.0.1'); + + $this->configResolver + ->expects(self::at(1)) + ->method('getParameter') + ->with( + 'api.notifier.endpoint', + 'ezrecommendation', + $siteAccess + ) + ->willReturn('https://reco-engine.com'); + } +} From 3409df3904a7935d1f6a22219a96561b8599172e Mon Sep 17 00:00:00 2001 From: ciastektk Date: Tue, 3 Aug 2021 13:29:27 +0200 Subject: [PATCH 2/8] Update src/lib/Factory/Export/ParametersFactory.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- src/lib/Factory/Export/ParametersFactory.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/lib/Factory/Export/ParametersFactory.php b/src/lib/Factory/Export/ParametersFactory.php index dc28761f..54563b4f 100644 --- a/src/lib/Factory/Export/ParametersFactory.php +++ b/src/lib/Factory/Export/ParametersFactory.php @@ -96,8 +96,9 @@ private function getExportParameters(array $options): array 'languages' => $options['languages'], 'host' => $options['host'] ?? $this->getHostUri($siteAccess), 'web_hook' => $options['web_hook'] ?? $this->getWebHook( - (int)$customerId, - $siteAccess), + (int)$customerId, + $siteAccess + ), 'page_size' => $options['page_size'], ]; } From f6280804a759bdea52e9e94c9b729a0926a054ef Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Tue, 3 Aug 2021 14:53:23 +0200 Subject: [PATCH 3/8] Fixes after review --- src/lib/Factory/Export/ParametersFactory.php | 33 +++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/lib/Factory/Export/ParametersFactory.php b/src/lib/Factory/Export/ParametersFactory.php index 54563b4f..8b378d07 100644 --- a/src/lib/Factory/Export/ParametersFactory.php +++ b/src/lib/Factory/Export/ParametersFactory.php @@ -19,6 +19,15 @@ final class ParametersFactory implements ParametersFactoryInterface { + private const PARAMETERS_NAMESPACE = 'ezrecommendation'; + private const HOST_URI_PARAMETER_NAME = 'host_uri'; + private const NOTIFIER_ENDPOINT_PARAMETER_NAME = 'api.notifier.endpoint'; + private const REQUIRED_OPTIONS = [ + 'customer_id', + 'license_key', + 'siteaccess', + ]; + private CredentialsResolverInterface $credentialsResolver; private ConfigResolverInterface $configResolver; @@ -271,8 +280,8 @@ private function getCredentialsAndSiteAccessForMultiCustomerConfiguration(array private function getHostUri(string $siteAccess): string { return $this->configResolver->getParameter( - 'host_uri', - 'ezrecommendation', + self::HOST_URI_PARAMETER_NAME, + self::PARAMETERS_NAMESPACE, $siteAccess ); } @@ -280,8 +289,8 @@ private function getHostUri(string $siteAccess): string private function getWebHook(int $customerId, string $siteAccess): string { return $this->configResolver->getParameter( - 'api.notifier.endpoint', - 'ezrecommendation', + self::NOTIFIER_ENDPOINT_PARAMETER_NAME, + self::PARAMETERS_NAMESPACE, $siteAccess ) . sprintf(Notifier::ENDPOINT_PATH, $customerId); } @@ -323,20 +332,6 @@ private function hasMissingRequiredOptions(array $options): bool */ private function getMissingRequiredOptions(array $options): array { - $missingOptions = []; - - if (!isset($options['customer_id'])) { - $missingOptions[] = 'customer_id'; - } - - if (!isset($options['license_key'])) { - $missingOptions[] = 'license_key'; - } - - if (!isset($options['siteaccess'])) { - $missingOptions[] = 'siteaccess'; - } - - return $missingOptions; + return array_diff(self::REQUIRED_OPTIONS, $options); } } From d59e8a4ab4a25b54adb27884fb4e9122b2d3b0e4 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Wed, 4 Aug 2021 10:37:00 +0200 Subject: [PATCH 4/8] Fixed getMissingRequiredOptions method --- src/lib/Factory/Export/ParametersFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Factory/Export/ParametersFactory.php b/src/lib/Factory/Export/ParametersFactory.php index 8b378d07..02e979ee 100644 --- a/src/lib/Factory/Export/ParametersFactory.php +++ b/src/lib/Factory/Export/ParametersFactory.php @@ -332,6 +332,6 @@ private function hasMissingRequiredOptions(array $options): bool */ private function getMissingRequiredOptions(array $options): array { - return array_diff(self::REQUIRED_OPTIONS, $options); + return array_diff(self::REQUIRED_OPTIONS, array_keys($options)); } } From b7a32a20cb72e1e15d989f11406b9c969bcc812c Mon Sep 17 00:00:00 2001 From: ciastektk Date: Wed, 4 Aug 2021 11:34:49 +0200 Subject: [PATCH 5/8] Update src/lib/Exception/CredentialsNotFoundException.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adam Wójs --- src/lib/Exception/CredentialsNotFoundException.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/lib/Exception/CredentialsNotFoundException.php b/src/lib/Exception/CredentialsNotFoundException.php index 75290217..8dc1d3fe 100644 --- a/src/lib/Exception/CredentialsNotFoundException.php +++ b/src/lib/Exception/CredentialsNotFoundException.php @@ -17,9 +17,7 @@ public function __construct(?string $siteAccess = null, int $code = 0, Throwable $message = 'Credentials for recommendation client are not set'; if (null !== $siteAccess) { - $message .= sprintf( - ' for siteAccess: %s', $siteAccess - ); + $message .= ' for siteAccess: ' . $siteAccess; } parent::__construct( From 6a9388365cc6efad569cd02cc16f17c246905124 Mon Sep 17 00:00:00 2001 From: ciastektk Date: Wed, 4 Aug 2021 12:06:36 +0200 Subject: [PATCH 6/8] Update src/lib/Factory/Export/ParametersFactory.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- src/lib/Factory/Export/ParametersFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Factory/Export/ParametersFactory.php b/src/lib/Factory/Export/ParametersFactory.php index 02e979ee..68043c38 100644 --- a/src/lib/Factory/Export/ParametersFactory.php +++ b/src/lib/Factory/Export/ParametersFactory.php @@ -168,7 +168,7 @@ private function getCredentialsAndSiteAccessForSingleConfiguration(array $option ); } - if (null !== $options['customer_id'] && null !== $options['license_key'] && null !== $options['siteaccess']) { + if (isset($options['customer_id'], $options['license_key'], $options['siteaccess']) { return [ 'customer_id' => $options['customer_id'], 'license_key' => $options['license_key'], From 6f0352a47a009acb0c8f5e11a407b0b2673ee726 Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Wed, 4 Aug 2021 13:23:15 +0200 Subject: [PATCH 7/8] Fixes after review --- src/lib/Factory/Export/ParametersFactory.php | 6 ++- .../Factory/Export/ParametersFactoryTest.php | 40 +++++++------------ 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/lib/Factory/Export/ParametersFactory.php b/src/lib/Factory/Export/ParametersFactory.php index 68043c38..0bfdde7b 100644 --- a/src/lib/Factory/Export/ParametersFactory.php +++ b/src/lib/Factory/Export/ParametersFactory.php @@ -46,6 +46,10 @@ public function __construct( $this->siteAccessService = $siteAccessService; } + /** + * @throws \EzSystems\EzRecommendationClient\Exception\MissingExportParameterException + * @throws \EzSystems\EzRecommendationClient\Exception\InvalidArgumentException + */ public function create(array $options, string $type): Parameters { $this->exportParametersType = $type; @@ -168,7 +172,7 @@ private function getCredentialsAndSiteAccessForSingleConfiguration(array $option ); } - if (isset($options['customer_id'], $options['license_key'], $options['siteaccess']) { + if (isset($options['customer_id'], $options['license_key'], $options['siteaccess'])) { return [ 'customer_id' => $options['customer_id'], 'license_key' => $options['license_key'], diff --git a/tests/lib/Factory/Export/ParametersFactoryTest.php b/tests/lib/Factory/Export/ParametersFactoryTest.php index cf2798da..dbe867a5 100644 --- a/tests/lib/Factory/Export/ParametersFactoryTest.php +++ b/tests/lib/Factory/Export/ParametersFactoryTest.php @@ -73,7 +73,7 @@ public function setUp(): void */ public function testCreateFromAllOptions(): void { - $this->getAllSiteAccesses(); + $this->configureSiteAccessServiceToReturnAllSiteAccesses(); self::assertEquals( Parameters::fromArray($this->options), @@ -119,7 +119,7 @@ public function testCreateWithAutocomplete(): void '12345-12345-12345-12345' )); - $this->getHostUriAndApiNotifierUri($siteAccess); + $this->configureConfigResolverToReturnHostUriAndApiNotifierUri($siteAccess); self::assertEquals( Parameters::fromArray($this->options), @@ -155,7 +155,7 @@ public function testCreateForSingleConfiguration(): void ->with($siteAccess) ->willReturn(true); - $this->getHostUriAndApiNotifierUri($siteAccess); + $this->configureConfigResolverToReturnHostUriAndApiNotifierUri($siteAccess); self::assertEquals( Parameters::fromArray($this->options), @@ -179,7 +179,7 @@ public function testCreateForMultiCustomerConfiguration(): void 'host' => null, ]; - $this->getAllSiteAccesses(); + $this->configureSiteAccessServiceToReturnAllSiteAccesses(); $this->credentialsResolver ->expects(self::at(0)) @@ -193,7 +193,7 @@ public function testCreateForMultiCustomerConfiguration(): void ->with($secondSiteAccess) ->willReturn(true); - $this->getHostUriAndApiNotifierUri($firstSiteAccess); + $this->configureConfigResolverToReturnHostUriAndApiNotifierUri($firstSiteAccess); self::assertEquals( Parameters::fromArray($this->options), @@ -211,7 +211,7 @@ public function testThrowExportCredentialsNotFoundException(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage(sprintf('SiteAccess %s doesn\'t exists', $siteAccess)); - $this->getAllSiteAccesses(); + $this->configureSiteAccessServiceToReturnAllSiteAccesses(); $this->parametersFactory->create( [ @@ -228,7 +228,7 @@ public function testThrowExportCredentialsNotFoundException(): void ); } - private function getAllSiteAccesses(): void + private function configureSiteAccessServiceToReturnAllSiteAccesses(): void { $this->siteAccessService ->method('getAll') @@ -240,26 +240,16 @@ private function getAllSiteAccesses(): void ); } - private function getHostUriAndApiNotifierUri(string $siteAccess): void + private function configureConfigResolverToReturnHostUriAndApiNotifierUri(string $siteAccess): void { $this->configResolver - ->expects(self::at(0)) - ->method('getParameter') - ->with( - 'host_uri', - 'ezrecommendation', - $siteAccess - ) - ->willReturn('https://127.0.0.1'); - - $this->configResolver - ->expects(self::at(1)) + ->expects(self::atLeastOnce()) ->method('getParameter') - ->with( - 'api.notifier.endpoint', - 'ezrecommendation', - $siteAccess - ) - ->willReturn('https://reco-engine.com'); + ->willReturn(self::returnValueMap( + [ + ['host_uri', 'ezrecommendation', $siteAccess, 'https://127.0.0.1'], + ['api.notifier.endpoint', 'ezrecommendation', $siteAccess, 'https://reco-engine.com'], + ] + )); } } From a9043566be2499a91831841236d8d2c7d9824a8d Mon Sep 17 00:00:00 2001 From: Tomasz Kryszan Date: Fri, 6 Aug 2021 10:39:52 +0200 Subject: [PATCH 8/8] Used willReturnMap instead of willReturn --- tests/lib/Factory/Export/ParametersFactoryTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/Factory/Export/ParametersFactoryTest.php b/tests/lib/Factory/Export/ParametersFactoryTest.php index dbe867a5..debefd5a 100644 --- a/tests/lib/Factory/Export/ParametersFactoryTest.php +++ b/tests/lib/Factory/Export/ParametersFactoryTest.php @@ -245,11 +245,11 @@ private function configureConfigResolverToReturnHostUriAndApiNotifierUri(string $this->configResolver ->expects(self::atLeastOnce()) ->method('getParameter') - ->willReturn(self::returnValueMap( + ->willReturnMap( [ ['host_uri', 'ezrecommendation', $siteAccess, 'https://127.0.0.1'], ['api.notifier.endpoint', 'ezrecommendation', $siteAccess, 'https://reco-engine.com'], ] - )); + ); } }