From 1b0466112b3da2e913c7d4faab633979db391d22 Mon Sep 17 00:00:00 2001 From: Jonathan Lelievre Date: Tue, 21 Nov 2023 00:22:57 +0100 Subject: [PATCH] Create dedicated methods in repositories with single anguage to optimize a bit more the SQL requests for form choice providers --- .../Feature/Repository/FeatureRepository.php | 65 +++++++++++-------- .../Repository/FeatureValueRepository.php | 22 ++++++- .../FeatureValuesChoiceProvider.php | 28 +------- .../ChoiceProvider/FeaturesChoiceProvider.php | 28 +------- .../config/services/adapter/form.yml | 2 - 5 files changed, 66 insertions(+), 79 deletions(-) diff --git a/src/Adapter/Feature/Repository/FeatureRepository.php b/src/Adapter/Feature/Repository/FeatureRepository.php index e9e3d60c4015a..55fce7c56c935 100644 --- a/src/Adapter/Feature/Repository/FeatureRepository.php +++ b/src/Adapter/Feature/Repository/FeatureRepository.php @@ -77,6 +77,23 @@ public function assertExists(FeatureId $featureId): void ); } + /** + * @param int $langId + * + * @return array> + */ + public function getFeaturesByLang(int $langId): array + { + $qb = $this->getFeaturesQueryBuilder(['id_lang' => $langId]) + ->leftJoin('f', $this->dbPrefix . 'feature_lang', 'fl', 'fl.id_feature = f.id_feature AND fl.id_lang = :languageId') + ->setParameter('languageId', $langId) + ->select('f.*, fl.*') + ->addOrderBy('fl.name', 'ASC') + ; + + return $this->formatResult($qb->execute()->fetchAllAssociative()); + } + /** * @param int|null $limit * @param int|null $offset @@ -88,11 +105,31 @@ public function getFeatures(?int $limit = null, ?int $offset = null, ?array $fil { $qb = $this->getFeaturesQueryBuilder($filters) ->select('f.*, fl.*') + ->addOrderBy('f.position', 'ASC') ->setFirstResult($offset) ->setMaxResults($limit) ; - $results = $qb->execute()->fetchAll(); + return $this->formatResult($qb->execute()->fetchAllAssociative()); + } + + /** + * @param array|null $filters + * + * @return int + */ + public function getFeaturesCount(?array $filters = []): int + { + $qb = $this->getFeaturesQueryBuilder($filters) + ->select('COUNT(f.id_feature_value) AS total_feature_values') + ->addGroupBy('f.id_feature_value') + ; + + return (int) $qb->execute()->fetch()['total_feature_values']; + } + + private function formatResult(array $results): array + { $localizedNames = []; $featuresById = []; foreach ($results as $result) { @@ -116,21 +153,6 @@ public function getFeatures(?int $limit = null, ?int $offset = null, ?array $fil return $features; } - /** - * @param array|null $filters - * - * @return int - */ - public function getFeaturesCount(?array $filters = []): int - { - $qb = $this->getFeaturesQueryBuilder($filters) - ->select('COUNT(f.id_feature_value) AS total_feature_values') - ->addGroupBy('f.id_feature_value') - ; - - return (int) $qb->execute()->fetch()['total_feature_values']; - } - /** * @param array|null $filters * @@ -138,18 +160,9 @@ public function getFeaturesCount(?array $filters = []): int */ private function getFeaturesQueryBuilder(?array $filters): QueryBuilder { + // Filters not handled yet $qb = $this->connection->createQueryBuilder(); $qb->from($this->dbPrefix . 'feature', 'f'); - if (!empty($filters['id_lang'])) { - $languageIds = is_array($filters['id_lang']) ? $filters['id_lang'] : [$filters['id_lang']]; - $qb - ->leftJoin('f', $this->dbPrefix . 'feature_lang', 'fl', 'fl.id_feature = f.id_feature AND fl.id_lang IN (:languageIds)') - ->setParameter('languageIds', $languageIds, Connection::PARAM_INT_ARRAY) - ; - } else { - $qb->leftJoin('f', $this->dbPrefix . 'feature_lang', 'fl', 'fl.id_feature = f.id_feature'); - } - $qb->addOrderBy('f.position', 'ASC'); return $qb; } diff --git a/src/Adapter/Feature/Repository/FeatureValueRepository.php b/src/Adapter/Feature/Repository/FeatureValueRepository.php index 099f2d950b4b5..21e2eaaf1bc89 100644 --- a/src/Adapter/Feature/Repository/FeatureValueRepository.php +++ b/src/Adapter/Feature/Repository/FeatureValueRepository.php @@ -153,7 +153,26 @@ public function assertExists(FeatureValueId $featureValueId): void */ public function getProductFeatureValues(ProductId $productId, ?int $limit = null, ?int $offset = null, ?array $filters = []): array { - return $this->getFeatureValues($limit, $offset, array_merge($filters, ['id_product' => $productId->getValue()])); + return $this->getFeatureValues($limit, $offset, array_merge($filters ?? [], ['id_product' => $productId->getValue()])); + } + + /** + * @param int $langId + * @param array $filters + * + * @return array + */ + public function getFeatureValuesByLang(int $langId, array $filters): array + { + $qb = $this->getFeatureValuesQueryBuilder(array_merge($filters, ['id_lang' => $langId])) + ->leftJoin('f', $this->dbPrefix . 'feature_value_lang', 'fvl', 'fvl.id_feature_value = fv.id_feature_value AND fvl.id_lang = :langId') + ->setParameter('langId', $langId) + ->select('fv.*, fvl.value') + // Override the default order by feature position and ID + ->orderBy('fvl.value') + ; + + return $qb->execute()->fetchAllAssociative(); } /** @@ -170,6 +189,7 @@ public function getFeatureValues(?int $limit = null, ?int $offset = null, ?array ->setFirstResult($offset) ->setMaxResults($limit) ; + $featureValues = $qb->execute()->fetchAllAssociative(); $indexedFeatureValues = []; diff --git a/src/Adapter/Form/ChoiceProvider/FeatureValuesChoiceProvider.php b/src/Adapter/Form/ChoiceProvider/FeatureValuesChoiceProvider.php index f83c4b07d4fe8..d4a5a5a9e4a41 100644 --- a/src/Adapter/Form/ChoiceProvider/FeatureValuesChoiceProvider.php +++ b/src/Adapter/Form/ChoiceProvider/FeatureValuesChoiceProvider.php @@ -44,11 +44,6 @@ class FeatureValuesChoiceProvider implements ConfigurableFormChoiceProviderInter */ private $contextLanguageId; - /** - * @var int - */ - private $defaultLanguageId; - /** * Cache value to avoid performing the same request multiple times as the value should remain the same inside a request. * @@ -58,12 +53,10 @@ class FeatureValuesChoiceProvider implements ConfigurableFormChoiceProviderInter public function __construct( FeatureValueRepository $featureValueRepository, - LegacyContext $legacyContext, - int $defaultLanguageId + LegacyContext $legacyContext ) { $this->featureValueRepository = $featureValueRepository; $this->contextLanguageId = (int) $legacyContext->getLanguage()->getId(); - $this->defaultLanguageId = $defaultLanguageId; } /** @@ -81,32 +74,17 @@ public function getChoices(array $options) if (isset($options['custom'])) { $filters['custom'] = $options['custom']; } - $filters['id_lang'] = [ - $this->contextLanguageId, - $this->defaultLanguageId, - ]; $cacheKey = md5(serialize($filters)); - if (!empty($this->cacheFeatureValueChoices[$cacheKey])) { return $this->cacheFeatureValueChoices[$cacheKey]; } - $featureValues = $this->featureValueRepository->getFeatureValues(null, null, $filters); + $featureValues = $this->featureValueRepository->getFeatureValuesByLang($this->contextLanguageId, $filters); $this->cacheFeatureValueChoices[$cacheKey] = []; foreach ($featureValues as $feature) { - if (!empty($feature['localized_values'][$this->contextLanguageId])) { - $featureValueName = $feature['localized_values'][$this->contextLanguageId]; - } elseif (!empty($feature['localized_values'][$this->defaultLanguageId])) { - $featureValueName = $feature['localized_values'][$this->defaultLanguageId]; - } else { - $featureValueName = reset($feature['localized_values']); - } - $this->cacheFeatureValueChoices[$cacheKey][$featureValueName] = (int) $feature['id_feature_value']; + $this->cacheFeatureValueChoices[$cacheKey][$feature['value']] = (int) $feature['id_feature_value']; } - // Order alphabetically - ksort($this->cacheFeatureValueChoices[$cacheKey]); - return $this->cacheFeatureValueChoices[$cacheKey]; } } diff --git a/src/Adapter/Form/ChoiceProvider/FeaturesChoiceProvider.php b/src/Adapter/Form/ChoiceProvider/FeaturesChoiceProvider.php index b111bb8d43eb1..081d890991e24 100644 --- a/src/Adapter/Form/ChoiceProvider/FeaturesChoiceProvider.php +++ b/src/Adapter/Form/ChoiceProvider/FeaturesChoiceProvider.php @@ -44,11 +44,6 @@ class FeaturesChoiceProvider implements FormChoiceProviderInterface */ private $contextLanguageId; - /** - * @var int - */ - private $defaultLanguageId; - /** * Cache value to avoid performing the same request multiple times as the value should remain the same inside a request. * @@ -58,12 +53,10 @@ class FeaturesChoiceProvider implements FormChoiceProviderInterface public function __construct( FeatureRepository $featureRepository, - LegacyContext $legacyContext, - int $defaultLanguageId + LegacyContext $legacyContext ) { $this->featureRepository = $featureRepository; $this->contextLanguageId = (int) $legacyContext->getLanguage()->getId(); - $this->defaultLanguageId = $defaultLanguageId; } /** @@ -75,27 +68,12 @@ public function getChoices() return $this->cacheFeatureChoices; } - $features = $this->featureRepository->getFeatures(null, null, [ - 'id_lang' => [ - $this->contextLanguageId, - $this->defaultLanguageId, - ], - ]); + $features = $this->featureRepository->getFeaturesByLang($this->contextLanguageId); $this->cacheFeatureChoices = []; foreach ($features as $feature) { - if (!empty($feature['localized_names'][$this->contextLanguageId])) { - $featureName = $feature['localized_names'][$this->contextLanguageId]; - } elseif (!empty($feature['localized_names'][$this->defaultLanguageId])) { - $featureName = $feature['localized_names'][$this->defaultLanguageId]; - } else { - $featureName = reset($feature['localized_names']); - } - $this->cacheFeatureChoices[$featureName] = $feature['id_feature']; + $this->cacheFeatureChoices[$feature['localized_names'][$this->contextLanguageId]] = $feature['id_feature']; } - // Order alphabetically - ksort($this->cacheFeatureChoices); - return $this->cacheFeatureChoices; } } diff --git a/src/PrestaShopBundle/Resources/config/services/adapter/form.yml b/src/PrestaShopBundle/Resources/config/services/adapter/form.yml index 658755813866d..06b0a48b3d6d6 100644 --- a/src/PrestaShopBundle/Resources/config/services/adapter/form.yml +++ b/src/PrestaShopBundle/Resources/config/services/adapter/form.yml @@ -73,14 +73,12 @@ services: arguments: - '@PrestaShop\PrestaShop\Adapter\Feature\Repository\FeatureRepository' - '@prestashop.adapter.legacy.context' - - "@=service('prestashop.adapter.legacy.configuration').getInt('PS_LANG_DEFAULT')" prestashop.adapter.form.choice_provider.feature_values_choice_provider: class: 'PrestaShop\PrestaShop\Adapter\Form\ChoiceProvider\FeatureValuesChoiceProvider' arguments: - '@PrestaShop\PrestaShop\Adapter\Feature\Repository\FeatureValueRepository' - '@prestashop.adapter.legacy.context' - - "@=service('prestashop.adapter.legacy.configuration').getInt('PS_LANG_DEFAULT')" prestashop.adapter.form.choice_provider.supplier_name_by_id_choice_provider: class: 'PrestaShop\PrestaShop\Adapter\Form\ChoiceProvider\SupplierNameByIdChoiceProvider'