refactor(category): streamline product list query and enhance option field handling

Ibochkarev · Ibochkarev · commit af276ff2a430 · 2026-04-19T11:21:35.000+06:00
- Consolidated product list query logic into a new method, buildProductListQuery, for better readability and maintainability.
- Updated option field handling to use GROUP_CONCAT for compliance with MySQL ONLY_FULL_GROUP_BY.
- Enhanced formatProduct method to prevent leaking internal columns by validating option field names.
- Added validation for option keys in GridConfigService to ensure data integrity.
diff --git a/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php b/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php
@@ -62,93 +62,9 @@ public function getList(array $params = []): array
         $gridFields = $gridConfig ? $gridConfig->getGridConfig('category-products', true) : [];
         $optionFields = $gridConfig ? $gridConfig->extractOptionFields($gridFields) : [];
 
-        $c = $this->modx->newQuery(msProduct::class);
-        $c->innerJoin(msProductData::class, 'Data', 'msProduct.id = Data.id');
+        $c = $this->buildProductListQuery($categoryId, $params, $nested, $optionFields);
 
-        foreach ($optionFields as $opt) {
-            $alias = $opt['alias'];
-            $key = $opt['key'];
-            $c->leftJoin(
-                msProductOption::class,
-                $alias,
-                "`{$alias}`.product_id = msProduct.id AND `{$alias}`.key = '{$key}'"
-            );
-        }
-
-        $c->where(['msProduct.class_key' => msProduct::class]);
-
-        // Parent filter
-        if ($nested) {
-            // Get all child category IDs
-            $categoryIds = $this->getChildCategories($categoryId);
-            $categoryIds[] = $categoryId;
-            $c->where(['msProduct.parent:IN' => $categoryIds]);
-        } else {
-            $c->where(['msProduct.parent' => $categoryId]);
-        }
-
-        // Search filter
-        if (!empty($query)) {
-            $c->where([
-                'msProduct.pagetitle:LIKE' => "%{$query}%",
-                'OR:Data.article:LIKE' => "%{$query}%",
-            ]);
-        }
-
-        // Boolean filters for msProduct fields
-        $productBooleanFields = ['published', 'deleted', 'hidemenu', 'isfolder'];
-        foreach ($productBooleanFields as $field) {
-            if (isset($params[$field]) && $params[$field] !== '') {
-                $c->where(["msProduct.{$field}" => (int)$params[$field]]);
-            }
-        }
-
-        // Boolean filters for msProductData fields
-        $dataBooleanFields = ['new', 'popular', 'favorite'];
-        foreach ($dataBooleanFields as $field) {
-            if (isset($params[$field]) && $params[$field] !== '') {
-                $c->where(["Data.{$field}" => (int)$params[$field]]);
-            }
-        }
-
-        // Text filters for msProduct fields (LIKE search)
-        $productTextFields = ['pagetitle', 'longtitle', 'alias', 'description', 'introtext', 'content'];
-        foreach ($productTextFields as $field) {
-            if (!empty($params[$field])) {
-                $c->where(["msProduct.{$field}:LIKE" => "%{$params[$field]}%"]);
-            }
-        }
-
-        // Text filters for msProductData fields (LIKE search)
-        $dataTextFields = ['article', 'made_in'];
-        foreach ($dataTextFields as $field) {
-            if (!empty($params[$field])) {
-                $c->where(["Data.{$field}:LIKE" => "%{$params[$field]}%"]);
-            }
-        }
-
-        // Numeric filters for msProductData fields (exact match)
-        $dataNumericFields = ['price', 'old_price', 'weight', 'vendor_id'];
-        foreach ($dataNumericFields as $field) {
-            if (isset($params[$field]) && $params[$field] !== '') {
-                $c->where(["Data.{$field}" => $params[$field]]);
-            }
-        }
-
-        foreach ($optionFields as $opt) {
-            $paramKey = 'filter_' . $opt['fieldName'];
-            if (isset($params[$paramKey]) && $params[$paramKey] !== '') {
-                $filterValue = $this->modx->escape($params[$paramKey]);
-                $c->where(["`{$opt['alias']}`.value:LIKE" => "%{$filterValue}%"]);
-            }
-        }
-
-        // Default: hide deleted if not explicitly filtered
-        if (!isset($params['deleted']) || $params['deleted'] === '') {
-            $c->where(['msProduct.deleted' => 0]);
-        }
-
-        $countQuery = clone $c;
+        $countQuery = $this->buildProductListQuery($categoryId, $params, $nested, $optionFields);
         $countQuery->select('COUNT(DISTINCT msProduct.id)');
         $countQuery->prepare();
         $countQuery->stmt->execute();
@@ -173,16 +89,20 @@ public function getList(array $params = []): array
             'Data.favorite',
         ];
         foreach ($optionFields as $opt) {
-            $selectParts[] = "`{$opt['alias']}`.value AS `{$opt['fieldName']}`";
+            $selectParts[] = "GROUP_CONCAT(DISTINCT `{$opt['alias']}`.value) AS `{$opt['fieldName']}`";
         }
         $c->select($selectParts);
+        if (!empty($optionFields)) {
+            $c->groupby('msProduct.id');
+        }
 
         $c->prepare();
         $rows = $c->stmt->execute() ? $c->stmt->fetchAll(\PDO::FETCH_ASSOC) : [];
 
+        $optionFieldNames = array_column($optionFields, 'fieldName');
         $results = [];
         foreach ($rows as $row) {
-            $results[] = $this->formatProduct($row, $nested);
+            $results[] = $this->formatProduct($row, $nested, $optionFieldNames);
         }
 
         return Response::success([
@@ -424,6 +344,8 @@ public function publish(array $params = []): array
     /**
      * Map sort field to SQL expression (supports option fields)
      *
+     * For option fields uses GROUP_CONCAT to comply with MySQL ONLY_FULL_GROUP_BY.
+     *
      * @param string $sortBy
      * @param array $optionFields
      * @return string
@@ -432,7 +354,7 @@ protected function mapSortField(string $sortBy, array $optionFields): string
     {
         foreach ($optionFields as $opt) {
             if ($opt['fieldName'] === $sortBy) {
-                return "`{$opt['alias']}`.value";
+                return "GROUP_CONCAT(DISTINCT `{$opt['alias']}`.value)";
             }
         }
         $productFields = ['id', 'pagetitle', 'menuindex', 'published', 'createdon', 'editedon'];
@@ -451,9 +373,10 @@ protected function mapSortField(string $sortBy, array $optionFields): string
      *
      * @param array $row Raw row from query (includes joined option values)
      * @param bool $nested
+     * @param array $optionFieldNames Allowed option field names (prevents leaking internal xPDO/MySQL columns)
      * @return array
      */
-    protected function formatProduct(array $row, bool $nested = false): array
+    protected function formatProduct(array $row, bool $nested = false, array $optionFieldNames = []): array
     {
         $id = (int)$row['id'];
         $data = [
@@ -482,8 +405,9 @@ protected function formatProduct(array $row, bool $nested = false): array
             'preview_url' => $this->modx->makeUrl($id, '', '', 'full'),
         ];
 
+        $allowedOptionFields = array_flip($optionFieldNames);
         foreach ($row as $key => $value) {
-            if (!array_key_exists($key, $data)) {
+            if (!array_key_exists($key, $data) && isset($allowedOptionFields[$key])) {
                 $data[$key] = $value;
             }
         }
@@ -498,6 +422,97 @@ protected function formatProduct(array $row, bool $nested = false): array
         return $data;
     }
 
+    /**
+     * Build base product list query with JOINs and filters (no select/sort/limit)
+     *
+     * @param int $categoryId
+     * @param array $params
+     * @param bool $nested
+     * @param array $optionFields
+     * @return \xPDO\Om\xPDOQuery
+     */
+    protected function buildProductListQuery(int $categoryId, array $params, bool $nested, array $optionFields): \xPDO\Om\xPDOQuery
+    {
+        $query = trim($params['query'] ?? '');
+        $c = $this->modx->newQuery(msProduct::class);
+        $c->innerJoin(msProductData::class, 'Data', 'msProduct.id = Data.id');
+
+        foreach ($optionFields as $opt) {
+            $alias = $opt['alias'];
+            $key = $opt['key'];
+            $c->leftJoin(
+                msProductOption::class,
+                $alias,
+                "`{$alias}`.product_id = msProduct.id AND `{$alias}`.key = '{$key}'"
+            );
+        }
+
+        $c->where(['msProduct.class_key' => msProduct::class]);
+
+        if ($nested) {
+            $categoryIds = $this->getChildCategories($categoryId);
+            $categoryIds[] = $categoryId;
+            $c->where(['msProduct.parent:IN' => $categoryIds]);
+        } else {
+            $c->where(['msProduct.parent' => $categoryId]);
+        }
+
+        if (!empty($query)) {
+            $c->where([
+                'msProduct.pagetitle:LIKE' => "%{$query}%",
+                'OR:Data.article:LIKE' => "%{$query}%",
+            ]);
+        }
+
+        $productBooleanFields = ['published', 'deleted', 'hidemenu', 'isfolder'];
+        foreach ($productBooleanFields as $field) {
+            if (isset($params[$field]) && $params[$field] !== '') {
+                $c->where(["msProduct.{$field}" => (int)$params[$field]]);
+            }
+        }
+
+        $dataBooleanFields = ['new', 'popular', 'favorite'];
+        foreach ($dataBooleanFields as $field) {
+            if (isset($params[$field]) && $params[$field] !== '') {
+                $c->where(["Data.{$field}" => (int)$params[$field]]);
+            }
+        }
+
+        $productTextFields = ['pagetitle', 'longtitle', 'alias', 'description', 'introtext', 'content'];
+        foreach ($productTextFields as $field) {
+            if (!empty($params[$field])) {
+                $c->where(["msProduct.{$field}:LIKE" => "%{$params[$field]}%"]);
+            }
+        }
+
+        $dataTextFields = ['article', 'made_in'];
+        foreach ($dataTextFields as $field) {
+            if (!empty($params[$field])) {
+                $c->where(["Data.{$field}:LIKE" => "%{$params[$field]}%"]);
+            }
+        }
+
+        $dataNumericFields = ['price', 'old_price', 'weight', 'vendor_id'];
+        foreach ($dataNumericFields as $field) {
+            if (isset($params[$field]) && $params[$field] !== '') {
+                $c->where(["Data.{$field}" => $params[$field]]);
+            }
+        }
+
+        foreach ($optionFields as $opt) {
+            $paramKey = 'filter_' . $opt['fieldName'];
+            if (isset($params[$paramKey]) && $params[$paramKey] !== '') {
+                $c->where(["`{$opt['alias']}`.value:LIKE" => "%{$params[$paramKey]}%"]);
+            }
+        }
+
+        if (!isset($params['deleted']) || $params['deleted'] === '') {
+            $c->where(['msProduct.deleted' => 0]);
+        }
+
+        return $c;
+    }
+
     /**
      * Get all child category IDs recursively
      *
diff --git a/core/components/minishop3/src/Services/GridConfigService.php b/core/components/minishop3/src/Services/GridConfigService.php
@@ -762,6 +762,8 @@ protected function validateOptionConfig(array $config): array
     /**
      * Extract option fields from grid config for JOIN building
      *
+     * Re-validates option.key on read (defense in depth) — config can be modified directly in DB.
+     *
      * @param array $gridFields Array of grid field configs
      * @return array List of option field definitions: [['fieldName' => 'option_length', 'key' => 'length', 'alias' => 'opt_length'], ...]
      */
@@ -780,6 +782,10 @@ public function extractOptionFields(array $gridFields): array
             }
 
             $key = $option['key'];
+            if (!preg_match('/^[a-z0-9_]+$/i', $key)) {
+                continue;
+            }
+
             $fieldName = $field['name'];
             $alias = 'opt_' . $key;
 
