From 4b74832b7acd1a01e37527d5fc7193088e4f41d4 Mon Sep 17 00:00:00 2001
From: Ivan Bochkarev <ivanx86@gmail.com>
Date: Sun, 16 Aug 2026 17:52:47 +0600
Subject: [PATCH] perf(web-api): batch-load product Data on catalog list

Avoid per-row getOne(Data) on product/list, drop useless COUNT DISTINCT for the 1:1 Data join, and skip selecting content when include_content is off.
---
 .../Product/ProductCatalogService.php         | 72 ++++++++++++++++---
 .../tests/ProductCatalogPerfTest.php          | 57 +++++++++++++++
 2 files changed, 121 insertions(+), 8 deletions(-)
 create mode 100644 core/components/minishop3/tests/ProductCatalogPerfTest.php

diff --git a/core/components/minishop3/src/Services/Product/ProductCatalogService.php b/core/components/minishop3/src/Services/Product/ProductCatalogService.php
index 2a0276c9..ed0ddd69 100644
--- a/core/components/minishop3/src/Services/Product/ProductCatalogService.php
+++ b/core/components/minishop3/src/Services/Product/ProductCatalogService.php
@@ -211,20 +211,21 @@ public function getList(array $params): array
         $total = $this->countList($params);
 
         $listQuery = $this->buildListQuery($params);
+        $this->applyListSelect($listQuery, $includeContent);
         $this->applySort($listQuery, $params);
         $listQuery->limit($limit, $offset);
 
-        /** @var list<msProduct> $products */
+        /** @var array<int|string, msProduct> $products */
         $products = $this->modx->getCollection(msProduct::class, $listQuery) ?: [];
+        $productList = array_values($products);
+        $ids = $this->prefetchAndAttachProductData($productList);
 
-        $optionsByProduct = [];
-        if ($includeOptions && $products !== []) {
-            $ids = array_map(static fn (msProduct $p) => (int) $p->get('id'), array_values($products));
-            $optionsByProduct = $this->loadOptionsForProducts($ids);
-        }
+        $optionsByProduct = ($includeOptions && $ids !== [])
+            ? $this->loadOptionsForProducts($ids)
+            : [];
 
         $items = [];
-        foreach ($products as $product) {
+        foreach ($productList as $product) {
             $productId = (int) $product->get('id');
             $options = $includeOptions ? ($optionsByProduct[$productId] ?? []) : null;
             $items[] = $this->formatProduct($product, $includeContent, $options);
@@ -269,7 +270,8 @@ private function resolveContext(array $params): string
     private function countList(array $params): int
     {
         $countQuery = $this->buildListQuery($params);
-        $countQuery->select('COUNT(DISTINCT msProduct.id)');
+        // 1:1 join on Data — DISTINCT is unnecessary until many-joins are added.
+        $countQuery->select('COUNT(msProduct.id)');
         if (!$countQuery->prepare() || !$countQuery->stmt->execute()) {
             return 0;
         }
@@ -307,6 +309,60 @@ private function buildListQuery(array $params): xPDOQuery
         return $c;
     }
 
+    /**
+     * Limit selected resource columns; skip content blob on PLP when not requested.
+     * Same pattern as ms3_products snippet.
+     */
+    private function applyListSelect(xPDOQuery $query, bool $includeContent): void
+    {
+        $query->select(
+            $includeContent
+                ? $this->modx->getSelectColumns(msProduct::class, 'msProduct')
+                : $this->modx->getSelectColumns(msProduct::class, 'msProduct', '', ['content'], true)
+        );
+    }
+
+    /**
+     * Batch-load msProductData and attach via addOne so loadData() skips getOne N+1.
+     *
+     * List query already JOINs Data for WHERE/ORDER only (no related hydrate from that JOIN).
+     * One IN-query here is O(1) vs L× getOne; total SQL ≈ count + list + data (+ options).
+     *
+     * @param list<msProduct> $products
+     * @return list<int>
+     */
+    private function prefetchAndAttachProductData(array $products): array
+    {
+        $byId = [];
+        foreach ($products as $product) {
+            $id = (int) $product->get('id');
+            if ($id > 0) {
+                $byId[$id] = $product;
+            }
+        }
+
+        if ($byId === []) {
+            return [];
+        }
+
+        $ids = array_keys($byId);
+        $c = $this->modx->newQuery(msProductData::class);
+        $c->where(['id:IN' => $ids]);
+
+        /** @var msProductData $data */
+        foreach ($this->modx->getCollection(msProductData::class, $c) ?: [] as $data) {
+            $id = (int) $data->get('id');
+            if (!isset($byId[$id])) {
+                continue;
+            }
+            // addOne requires a by-ref argument (xPDO signature).
+            $attached = $data;
+            $byId[$id]->addOne($attached, 'Data');
+        }
+
+        return $ids;
+    }
+
     /**
      * @param array<string, mixed> $params
      */
diff --git a/core/components/minishop3/tests/ProductCatalogPerfTest.php b/core/components/minishop3/tests/ProductCatalogPerfTest.php
new file mode 100644
index 00000000..8a059543
--- /dev/null
+++ b/core/components/minishop3/tests/ProductCatalogPerfTest.php
@@ -0,0 +1,57 @@
+<?php
+
+/**
+ * #575: ProductCatalogService list avoids N+1 Data and useless COUNT DISTINCT.
+ *
+ * Run: php tests/ProductCatalogPerfTest.php
+ */
+
+declare(strict_types=1);
+
+$fail = static function (string $message): never {
+    fwrite(STDERR, "FAIL: {$message}\n");
+    exit(1);
+};
+
+$src = file_get_contents(__DIR__ . '/../src/Services/Product/ProductCatalogService.php');
+$modelSrc = file_get_contents(__DIR__ . '/../src/Model/msProduct.php');
+if ($src === false || $modelSrc === false) {
+    $fail('unable to read source files');
+}
+
+if (str_contains($modelSrc, 'function attachData')) {
+    $fail('do not add attachData on msProduct — use addOne($data, \'Data\')');
+}
+
+if (!str_contains($src, "addOne(\$attached, 'Data')") && !str_contains($src, 'addOne($attached, "Data")')) {
+    $fail('prefetch must attach Data via addOne(..., Data)');
+}
+
+if (!str_contains($src, "where(['id:IN'")) {
+    $fail('Data prefetch must use id:IN batch query');
+}
+
+if (str_contains($src, 'COUNT(DISTINCT msProduct.id)')) {
+    $fail('countList must not use COUNT(DISTINCT) for 1:1 Data join');
+}
+
+if (!str_contains($src, "select('COUNT(msProduct.id)')")) {
+    $fail('countList must COUNT(msProduct.id)');
+}
+
+if (!preg_match(
+    "/getSelectColumns\(\s*msProduct::class\s*,\s*'msProduct'\s*,\s*''\s*,\s*\['content'\]\s*,\s*true\s*\)/",
+    $src
+)) {
+    $fail('include_content=0 must exclude content via getSelectColumns(..., [content], true)');
+}
+
+// Prefetch must run on getList path before format (batch, not per-row getOne).
+if (!preg_match('/function getList[\s\S]*?id:IN[\s\S]*?formatProduct/m', $src)
+    && !preg_match('/function getList[\s\S]*?prefetchAndAttachProductData[\s\S]*?formatProduct/m', $src)
+) {
+    $fail('getList must batch-load Data before formatProduct');
+}
+
+fwrite(STDOUT, "OK ProductCatalogPerfTest\n");
+exit(0);