Create dedicated methods in repositories with single anguage to optim…

…ize a bit more the SQL requests for form choice providers
jolelievre · Nov 20, 2023 · 1b04661 · 1b04661
1 parent b72452a
commit 1b04661
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 79 deletions.
diff --git 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<int, array<string, mixed>>
+     */
+    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,40 +153,16 @@ 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
      *
      * @return QueryBuilder
      */
     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
@@ -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
@@ -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
@@ -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
@@ -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'