From 465f2e39294efc19fbcfdcafc7635ab3f3fdec23 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Tue, 18 Aug 2026 07:55:37 +0600 Subject: [PATCH 1/3] feat(web-api): product gallery images[] for headless PDP Add include_images on product/get so Nuxt can render a slider from ordered active files without mgr processors or leaking file internals. --- .../Controllers/Api/Web/ProductController.php | 2 + .../minishop3/src/ServiceRegistry.php | 4 + .../src/ServiceRegistryFactories.php | 1 + .../Product/ProductCatalogService.php | 51 ++++++++- .../ProductGalleryPublicSerializer.php | 102 +++++++++++++++++ .../Product/ProductGalleryPublicService.php | 105 ++++++++++++++++++ .../tests/ProductCatalogImagesRoutesTest.php | 67 +++++++++++ .../tests/ProductCatalogServiceTest.php | 22 +++- .../ProductGalleryPublicSerializerTest.php | 84 ++++++++++++++ 9 files changed, 435 insertions(+), 3 deletions(-) create mode 100644 core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php create mode 100644 core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php create mode 100644 core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php create mode 100644 core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php diff --git a/core/components/minishop3/src/Controllers/Api/Web/ProductController.php b/core/components/minishop3/src/Controllers/Api/Web/ProductController.php index 35a2164c0..17b2f00e7 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/ProductController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/ProductController.php @@ -29,6 +29,8 @@ public function __construct(modX $modx) /** * GET /api/v1/product/get/{id} * + * Query: context, include_images (0|1, default 0 — omit images[]; name→alt, no DB alt). + * * @param array $params */ public function get(array $params = []): Response diff --git a/core/components/minishop3/src/ServiceRegistry.php b/core/components/minishop3/src/ServiceRegistry.php index 2e1e25643..4004cb6a5 100644 --- a/core/components/minishop3/src/ServiceRegistry.php +++ b/core/components/minishop3/src/ServiceRegistry.php @@ -162,6 +162,10 @@ class ServiceRegistry 'class' => \MiniShop3\Services\Product\ProductFacetService::class, 'interface' => null, ], + 'ms3_product_gallery_public' => [ + 'class' => \MiniShop3\Services\Product\ProductGalleryPublicService::class, + 'interface' => null, + ], 'ms3_category_catalog' => [ 'class' => \MiniShop3\Services\Category\CategoryCatalogService::class, 'interface' => null, diff --git a/core/components/minishop3/src/ServiceRegistryFactories.php b/core/components/minishop3/src/ServiceRegistryFactories.php index 522b9ac88..012fa9216 100644 --- a/core/components/minishop3/src/ServiceRegistryFactories.php +++ b/core/components/minishop3/src/ServiceRegistryFactories.php @@ -40,6 +40,7 @@ public static function map(): array 'ms3_product_link_service' => $modxOnly(), 'ms3_product_catalog' => $modxOnly(), 'ms3_product_facets' => $modxOnly(), + 'ms3_product_gallery_public' => $modxOnly(), 'ms3_category_catalog' => $modxOnly(), 'ms3_delivery_catalog' => $modxOnly(), 'ms3_payment_catalog' => $modxOnly(), diff --git a/core/components/minishop3/src/Services/Product/ProductCatalogService.php b/core/components/minishop3/src/Services/Product/ProductCatalogService.php index 2eb6e11b8..f28064054 100644 --- a/core/components/minishop3/src/Services/Product/ProductCatalogService.php +++ b/core/components/minishop3/src/Services/Product/ProductCatalogService.php @@ -130,6 +130,7 @@ public static function whitelistPublicPayload( array $payload, bool $includeContent, bool $includeOptions, + bool $includeImages = false, ): array { $allowed = array_merge(self::RESOURCE_FIELDS, self::DATA_FIELDS); if ($includeContent) { @@ -147,6 +148,10 @@ public static function whitelistPublicPayload( $result['options'] = self::stripOptionMetadata($payload['options']); } + if ($includeImages && is_array($payload['images'] ?? null)) { + $result['images'] = ProductGalleryPublicSerializer::whitelistItems($payload['images']); + } + return $result; } @@ -184,7 +189,21 @@ public function getById(int $productId, array $params = []): ?array $this->optionService()->loadOptionsForProduct($productId, false) ); - return $this->formatProduct($product, true, $options); + $includeImages = self::toBool($params['include_images'] ?? false); + $images = null; + if ($includeImages) { + $data = $product->loadData(); + $previewFileId = $data + ? $this->imageService()->resolvePreviewFileId($data) + : 0; + $images = $this->gallery()->loadForProduct( + $productId, + (string) $product->get('pagetitle'), + $previewFileId, + ); + } + + return $this->formatProduct($product, true, $options, $images); } /** @@ -199,6 +218,7 @@ public function getById(int $productId, array $params = []): ?array * - options: JSON object or bracket map (AND between keys, OR within key) * - limit, offset | page, sort, dir, query, context * - include_options, include_content + * - include_images (get only; default 0) * * @param array $params * @return array{items: list>, total: int, limit: int, offset: int} @@ -439,12 +459,14 @@ private function optionService(): OptionService /** * @param array|null $options null = omit options key; array = include + * @param list>|null $images null = omit images key; array = include * @return array */ private function formatProduct( msProduct $product, bool $includeContent, ?array $options = null, + ?array $images = null, ): array { $data = $product->loadData(); $payload = []; @@ -482,11 +504,36 @@ private function formatProduct( $payload['options'] = $options; } + if ($images !== null) { + $payload['images'] = $images; + } + $modified = $product->modifyFields($payload); if (is_array($modified)) { $payload = $modified; } - return self::whitelistPublicPayload($payload, $includeContent, $options !== null); + return self::whitelistPublicPayload( + $payload, + $includeContent, + $options !== null, + $images !== null, + ); + } + + private function gallery(): ProductGalleryPublicService + { + /** @var ProductGalleryPublicService $service */ + $service = $this->modx->services->get('ms3_product_gallery_public'); + + return $service; + } + + private function imageService(): ProductImageService + { + /** @var ProductImageService $service */ + $service = $this->modx->services->get('ms3_product_image'); + + return $service; } } diff --git a/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php b/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php new file mode 100644 index 000000000..abcd7f9a6 --- /dev/null +++ b/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php @@ -0,0 +1,102 @@ + */ + public const ITEM_KEYS = [ + 'id', + 'url', + 'thumb', + 'name', + 'description', + 'alt', + 'position', + 'is_preview', + ]; + + /** + * @param list> $originals Top-level files (already filtered/sorted/capped) + * @param array $thumbByParentId parent file id => thumb url + * @return list + */ + public static function serializeGallery( + array $originals, + array $thumbByParentId, + string $pagetitle, + int $previewFileId, + ): array { + $items = []; + foreach ($originals as $row) { + $id = (int) ($row['id'] ?? 0); + if ($id <= 0) { + continue; + } + + $url = (string) ($row['url'] ?? ''); + $name = trim((string) ($row['name'] ?? '')); + + $items[] = [ + 'id' => $id, + 'url' => $url, + 'thumb' => $thumbByParentId[$id] ?? $url, + 'name' => $name, + 'description' => (string) ($row['description'] ?? ''), + 'alt' => $name !== '' ? $name : $pagetitle, + 'position' => (int) ($row['position'] ?? 0), + 'is_preview' => $previewFileId > 0 && $id === $previewFileId, + ]; + } + + return $items; + } + + /** + * Drop leaked file internals if a plugin mutates images[]. + * + * @param list $images + * @return list> + */ + public static function whitelistItems(array $images): array + { + $out = []; + foreach ($images as $item) { + if (!is_array($item)) { + continue; + } + + $clean = []; + foreach (self::ITEM_KEYS as $key) { + if (array_key_exists($key, $item)) { + $clean[$key] = $item[$key]; + } + } + + if ($clean !== []) { + $out[] = $clean; + } + } + + return $out; + } +} diff --git a/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php b/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php new file mode 100644 index 000000000..7194364f9 --- /dev/null +++ b/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php @@ -0,0 +1,105 @@ +> + */ + public function loadForProduct(int $productId, string $pagetitle, int $previewFileId): array + { + if ($productId <= 0) { + return []; + } + + $originals = $this->fetchOriginals($productId); + if ($originals === []) { + return []; + } + + return ProductGalleryPublicSerializer::serializeGallery( + $originals, + $this->fetchThumbsByParent(array_map(intval(...), array_column($originals, 'id'))), + $pagetitle, + $previewFileId, + ); + } + + /** + * @return list> + */ + private function fetchOriginals(int $productId): array + { + $c = $this->modx->newQuery(msProductFile::class); + $c->where([ + 'product_id' => $productId, + 'parent_id' => 0, + 'type' => 'image', + 'active' => 1, + ]); + $c->select('id, url, name, description, position'); + $c->sortby('position', 'ASC'); + $c->sortby('id', 'ASC'); + $c->limit(ProductGalleryPublicSerializer::MAX_IMAGES); + + if (!$c->prepare() || !$c->stmt->execute()) { + return []; + } + + return $c->stmt->fetchAll(\PDO::FETCH_ASSOC) ?: []; + } + + /** + * First child image url per original (lowest id). + * + * @param list $parentIds + * @return array + */ + private function fetchThumbsByParent(array $parentIds): array + { + if ($parentIds === []) { + return []; + } + + $c = $this->modx->newQuery(msProductFile::class); + $c->where([ + 'parent_id:IN' => $parentIds, + 'type' => 'image', + 'active' => 1, + ]); + $c->select('id, parent_id, url'); + $c->sortby('id', 'ASC'); + + if (!$c->prepare() || !$c->stmt->execute()) { + return []; + } + + $thumbs = []; + foreach ($c->stmt->fetchAll(\PDO::FETCH_ASSOC) ?: [] as $row) { + $parentId = (int) ($row['parent_id'] ?? 0); + if ($parentId <= 0 || isset($thumbs[$parentId])) { + continue; + } + $thumbs[$parentId] = (string) ($row['url'] ?? ''); + } + + return $thumbs; + } +} diff --git a/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php b/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php new file mode 100644 index 000000000..3fa088ba5 --- /dev/null +++ b/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php @@ -0,0 +1,67 @@ + $catalog, 'controller' => $controller, 'service' => $service, 'serializer' => $serializer] as $label => $src) { + if ($src === false || $src === '') { + $fail("unable to read {$label}"); + } +} + +if (!str_contains($catalog, 'include_images')) { + $fail('ProductCatalogService must honor include_images'); +} +if (!str_contains($catalog, 'ProductGalleryPublicSerializer::whitelistItems')) { + $fail('images must pass serializer allowlist after plugins'); +} +if (!str_contains($catalog, 'resolvePreviewFileId')) { + $fail('is_preview must use ProductImageService::resolvePreviewFileId'); +} +if (!str_contains($catalog, "ms3_product_gallery_public")) { + $fail('gallery service must come from ServiceRegistry'); +} +if (!preg_match('/function getById\([\s\S]*?gallery\(\)->loadForProduct/', $catalog)) { + $fail('gallery load must run inside getById after product is found'); +} +if (preg_match('/public function getList\(array \$params\): array\s*\{([\s\S]*?)\n public function /', $catalog, $m) + && str_contains($m[1], 'loadForProduct')) { + $fail('product/list must not load gallery in MVP'); +} + +$registry = file_get_contents(__DIR__ . '/../src/ServiceRegistry.php'); +$factories = file_get_contents(__DIR__ . '/../src/ServiceRegistryFactories.php'); +if ($registry === false || $factories === false + || !str_contains($registry, "'ms3_product_gallery_public'") + || !str_contains($factories, "'ms3_product_gallery_public'")) { + $fail('ms3_product_gallery_public must be registered'); +} + +if (!str_contains($service, "'parent_id' => 0") || !str_contains($service, "'active' => 1")) { + $fail('gallery query must restrict parent_id=0 and active=1'); +} +if (!str_contains($service, 'parent_id:IN')) { + $fail('thumbs must batch-load via parent_id IN'); +} + +if (!str_contains($serializer, 'no DB `alt`')) { + $fail('serializer must document name → alt mapping'); +} + +fwrite(STDOUT, "OK ProductCatalogImagesRoutesTest\n"); +exit(0); diff --git a/core/components/minishop3/tests/ProductCatalogServiceTest.php b/core/components/minishop3/tests/ProductCatalogServiceTest.php index 30d36c0b3..90e36054e 100644 --- a/core/components/minishop3/tests/ProductCatalogServiceTest.php +++ b/core/components/minishop3/tests/ProductCatalogServiceTest.php @@ -100,9 +100,29 @@ 'pagetitle' => 'Coffee', 'content' => '

hidden

', 'options' => ['size' => ['L']], + 'images' => [['id' => 1, 'hash' => 'x']], 'tv_private' => 'x', ], false, false), - 'whitelist omits content/options when flags off' + 'whitelist omits content/options/images when flags off' +); + +$assertSame( + [ + 'id' => 3, + 'pagetitle' => 'Mug', + 'images' => [ + ['id' => 9, 'url' => '/m.jpg'], + ], + ], + ProductCatalogService::whitelistPublicPayload([ + 'id' => 3, + 'pagetitle' => 'Mug', + 'images' => [ + ['id' => 9, 'url' => '/m.jpg', 'hash' => 'secret', 'path' => '/fs', 'createdby' => 1], + ], + 'hash' => 'nope', + ], false, false, true), + 'images flag keeps allowlisted gallery rows' ); fwrite(STDOUT, "OK ProductCatalogServiceTest\n"); diff --git a/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php b/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php new file mode 100644 index 000000000..eabfe7bf2 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php @@ -0,0 +1,84 @@ + 10, 'url' => '/a.jpg', 'name' => 'Front', 'description' => 'wide', 'position' => 0], + ['id' => 11, 'url' => '/b.jpg', 'name' => '', 'description' => '', 'position' => 1], + ['id' => 12, 'url' => '/c.jpg', 'name' => 'Side', 'description' => '', 'position' => 2], + ], + [10 => '/a_small.jpg', 12 => '/c_small.jpg'], + 'Kettle', + 12, + ); + + self::assertSame([10, 11, 12], array_column($items, 'id')); + self::assertSame('/a_small.jpg', $items[0]['thumb']); + self::assertSame('/b.jpg', $items[1]['thumb']); + self::assertTrue($items[2]['is_preview']); + self::assertFalse($items[0]['is_preview']); + self::assertSame('Front', $items[0]['alt']); + self::assertSame('Kettle', $items[1]['alt']); + self::assertSame('wide', $items[0]['description']); + } + + public function testSerializeSkipsInvalidIdsAndDoesNotUseHash(): void + { + $items = ProductGalleryPublicSerializer::serializeGallery( + [ + ['id' => 0, 'url' => '/skip.jpg', 'hash' => 'abc', 'path' => '/secret'], + ['id' => 5, 'url' => '/ok.jpg', 'hash' => 'leak', 'path' => '/fs', 'createdby' => 3, 'name' => 'Ok'], + ], + [], + 'Tea', + 5, + ); + + self::assertCount(1, $items); + self::assertSame(['id', 'url', 'thumb', 'name', 'description', 'alt', 'position', 'is_preview'], array_keys($items[0])); + self::assertArrayNotHasKey('hash', $items[0]); + self::assertArrayNotHasKey('path', $items[0]); + self::assertArrayNotHasKey('createdby', $items[0]); + self::assertTrue($items[0]['is_preview']); + } + + public function testEmptyOriginalsYieldEmptyGallery(): void + { + self::assertSame( + [], + ProductGalleryPublicSerializer::serializeGallery([], [], 'Tea', 0) + ); + } + + public function testWhitelistItemsDropsInternalsAndNonArrays(): void + { + $clean = ProductGalleryPublicSerializer::whitelistItems([ + 'nope', + [ + 'id' => 1, + 'url' => '/x.jpg', + 'hash' => 'abc', + 'path' => '/fs', + 'createdby' => 9, + 'alt' => 'X', + ], + ]); + + self::assertCount(1, $clean); + self::assertSame(1, $clean[0]['id']); + self::assertSame('/x.jpg', $clean[0]['url']); + self::assertArrayNotHasKey('hash', $clean[0]); + self::assertArrayNotHasKey('path', $clean[0]); + self::assertArrayNotHasKey('createdby', $clean[0]); + } +} From a9d0e5370dfbfc496c9fbe584270420ffb98f1b3 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Tue, 18 Aug 2026 08:34:34 +0600 Subject: [PATCH 2/3] feat(web-api): gallery list flag, /images route, and size-keyed thumbs Nuxt PDP/PLP need a dedicated gallery URL, a capped list payload, and the same thumbnail size as mgr GetList without a second media-source hit. --- .../minishop3/config/routes/web.php | 5 + .../Controllers/Api/Web/ProductController.php | 33 ++++- .../Product/ProductCatalogService.php | 100 +++++++++++--- .../ProductGalleryPublicSerializer.php | 88 +++++++++--- .../Product/ProductGalleryPublicService.php | 129 ++++++++++++++---- .../tests/ProductCatalogImagesRoutesTest.php | 36 ++++- .../ProductGalleryPublicSerializerTest.php | 40 +++++- 7 files changed, 362 insertions(+), 69 deletions(-) diff --git a/core/components/minishop3/config/routes/web.php b/core/components/minishop3/config/routes/web.php index 39dfd9ed1..a2e998765 100644 --- a/core/components/minishop3/config/routes/web.php +++ b/core/components/minishop3/config/routes/web.php @@ -284,6 +284,11 @@ $controller = new \MiniShop3\Controllers\Api\Web\ProductController($modx); return $controller->filters($params); }); + + $router->get('/{id}/images', function ($params) use ($modx) { + $controller = new \MiniShop3\Controllers\Api\Web\ProductController($modx); + return $controller->getImages($params); + }); }); // Public category catalog — no TokenMiddleware (headless nav / PLP) diff --git a/core/components/minishop3/src/Controllers/Api/Web/ProductController.php b/core/components/minishop3/src/Controllers/Api/Web/ProductController.php index 17b2f00e7..a9c308312 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/ProductController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/ProductController.php @@ -61,7 +61,8 @@ public function get(array $params = []): Response * * Query: parent|category, parents, nested, price_min, price_max, in_stock, stock_min, * vendor_id, new, popular, favorite, options (JSON), - * limit, offset|page, sort, dir, query, context, include_options, include_content + * limit, offset|page, sort, dir, query, context, include_options, include_content, + * include_images (0|1, default 0, cap 10 files per item) * * @param array $params Route + query params (Router merges $_GET) */ @@ -101,6 +102,36 @@ public function filters(array $params = []): Response return Response::success($result); } + /** + * GET /api/v1/product/{id}/images + * + * Same gallery serializer as include_images=1 on get. 404 if the product is not storefront-visible. + * + * @param array $params + */ + public function getImages(array $params = []): Response + { + $productId = (int) ($params['id'] ?? 0); + + if ($productId <= 0) { + return Response::error( + $this->modx->lexicon('ms3_err_product_id_ns'), + HttpStatus::BAD_REQUEST + ); + } + + $result = $this->catalog()->getPublicImages($productId, $params); + + if ($result === null) { + return Response::error( + $this->modx->lexicon('ms3_err_product_nf'), + HttpStatus::NOT_FOUND + ); + } + + return Response::success($result); + } + private function catalog(): ProductCatalogService { /** @var ProductCatalogService $service */ diff --git a/core/components/minishop3/src/Services/Product/ProductCatalogService.php b/core/components/minishop3/src/Services/Product/ProductCatalogService.php index f28064054..652ba434c 100644 --- a/core/components/minishop3/src/Services/Product/ProductCatalogService.php +++ b/core/components/minishop3/src/Services/Product/ProductCatalogService.php @@ -163,10 +163,48 @@ public static function toBool(mixed $value): bool /** * Single published product by ID (same visibility rules as list). * + * Query: context, include_images (0|1, default 0). + * * @param array $params Optional context override * @return array|null */ public function getById(int $productId, array $params = []): ?array + { + $product = $this->findVisibleProduct($productId, $params); + if ($product === null) { + return null; + } + + $options = self::stripOptionMetadata( + $this->optionService()->loadOptionsForProduct($productId, false) + ); + + $includeImages = self::toBool($params['include_images'] ?? false); + $images = $includeImages ? $this->loadImagesForProduct($product) : null; + + return $this->formatProduct($product, true, $options, $images); + } + + /** + * Gallery only: same visibility as get. Always returns images[] (may be empty). + * + * @param array $params + * @return array{images: list>}|null + */ + public function getPublicImages(int $productId, array $params = []): ?array + { + $product = $this->findVisibleProduct($productId, $params); + if ($product === null) { + return null; + } + + return ['images' => $this->loadImagesForProduct($product)]; + } + + /** + * @param array $params + */ + private function findVisibleProduct(int $productId, array $params): ?msProduct { if ($productId <= 0) { return null; @@ -181,29 +219,46 @@ public function getById(int $productId, array $params = []): ?array /** @var msProduct|null $product */ $product = $this->modx->getObject(msProduct::class, $this->publicCriteria($criteria)); - if (!$product) { - return null; - } + return $product ?: null; + } - $options = self::stripOptionMetadata( - $this->optionService()->loadOptionsForProduct($productId, false) + /** + * @return list> + */ + private function loadImagesForProduct(msProduct $product): array + { + $data = $product->loadData(); + $previewFileId = $data + ? $this->imageService()->resolvePreviewFileId($data) + : 0; + + return $this->gallery()->loadForProduct( + (int) $product->get('id'), + (string) $product->get('pagetitle'), + $previewFileId, ); + } - $includeImages = self::toBool($params['include_images'] ?? false); - $images = null; - if ($includeImages) { + /** + * @param list $products + * @return array>> + */ + private function loadImagesForProducts(array $products): array + { + $meta = []; + foreach ($products as $product) { + $id = (int) $product->get('id'); + if ($id <= 0) { + continue; + } $data = $product->loadData(); - $previewFileId = $data - ? $this->imageService()->resolvePreviewFileId($data) - : 0; - $images = $this->gallery()->loadForProduct( - $productId, - (string) $product->get('pagetitle'), - $previewFileId, - ); + $meta[$id] = [ + 'pagetitle' => (string) $product->get('pagetitle'), + 'preview_file_id' => $data ? (int) $data->get('preview_file_id') : 0, + ]; } - return $this->formatProduct($product, true, $options, $images); + return $this->gallery()->loadForProducts($meta, ProductGalleryPublicSerializer::MAX_IMAGES_LIST); } /** @@ -217,8 +272,7 @@ public function getById(int $productId, array $params = []): ?array * - in_stock, stock_min, vendor_id, new, popular, favorite * - options: JSON object or bracket map (AND between keys, OR within key) * - limit, offset | page, sort, dir, query, context - * - include_options, include_content - * - include_images (get only; default 0) + * - include_options, include_content, include_images (default 0; cap 10 files / product) * * @param array $params * @return array{items: list>, total: int, limit: int, offset: int} @@ -233,6 +287,7 @@ public function getList(array $params): array $offset = self::resolveOffset($params, $limit); $includeOptions = self::toBool($params['include_options'] ?? false); $includeContent = self::toBool($params['include_content'] ?? false); + $includeImages = self::toBool($params['include_images'] ?? false); $total = $this->countList($params, $filters); @@ -250,11 +305,16 @@ public function getList(array $params): array ? $this->loadOptionsForProducts($ids) : []; + $galleries = ($includeImages && $ids !== []) + ? $this->loadImagesForProducts($productList) + : []; + $items = []; foreach ($productList as $product) { $productId = (int) $product->get('id'); $options = $includeOptions ? ($optionsByProduct[$productId] ?? []) : null; - $items[] = $this->formatProduct($product, $includeContent, $options); + $images = $includeImages ? ($galleries[$productId] ?? []) : null; + $items[] = $this->formatProduct($product, $includeContent, $options, $images); } return [ diff --git a/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php b/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php index abcd7f9a6..ced7b702a 100644 --- a/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php +++ b/core/components/minishop3/src/Services/Product/ProductGalleryPublicSerializer.php @@ -9,16 +9,19 @@ * * There is no DB `alt` column: alt is derived from name, then product pagetitle. * include_images=1 with no files yields `images: []` (key present, empty list). + * `thumb` prefers ms3_product_thumbnail_size (mgr GetList path match), else first child, else url. */ final class ProductGalleryPublicSerializer { public const MAX_IMAGES = 50; + public const MAX_IMAGES_LIST = 10; /** @var list */ public const ITEM_KEYS = [ 'id', 'url', 'thumb', + 'thumbs', 'name', 'description', 'alt', @@ -26,26 +29,50 @@ final class ProductGalleryPublicSerializer 'is_preview', ]; + /** + * Size folder from child path (`{productId}/{size}/`) or url (`/{id}/{size}/`). + */ + public static function sizeKeyFromChild(string $path, string $url, int $productId): string + { + $normalized = trim(str_replace('\\', '/', $path), '/'); + if ($normalized !== '') { + $parts = explode('/', $normalized); + $last = (string) end($parts); + if ($last !== '' && ($productId <= 0 || $last !== (string) $productId)) { + return $last; + } + } + + if ($productId > 0 && preg_match('#/' . preg_quote((string) $productId, '#') . '/([^/]+)/#', $url, $m) === 1) { + return $m[1]; + } + + return ''; + } + /** * @param list> $originals Top-level files (already filtered/sorted/capped) - * @param array $thumbByParentId parent file id => thumb url - * @return list + * @param array}> $thumbsByParent + * @return list> */ public static function serializeGallery( array $originals, - array $thumbByParentId, + array $thumbsByParent, string $pagetitle, int $previewFileId, ): array { + $validIds = []; + foreach ($originals as $row) { + $id = (int) ($row['id'] ?? 0); + if ($id > 0) { + $validIds[] = $id; + } + } + $effectivePreview = $previewFileId; + if ($effectivePreview <= 0 || !in_array($effectivePreview, $validIds, true)) { + $effectivePreview = $validIds[0] ?? 0; + } + $items = []; foreach ($originals as $row) { $id = (int) ($row['id'] ?? 0); @@ -55,16 +82,20 @@ public static function serializeGallery( $url = (string) ($row['url'] ?? ''); $name = trim((string) ($row['name'] ?? '')); + $bundle = $thumbsByParent[$id] ?? []; + $thumb = trim((string) ($bundle['thumb'] ?? '')); + $thumbs = self::whitelistThumbs($bundle['thumbs'] ?? []); $items[] = [ 'id' => $id, 'url' => $url, - 'thumb' => $thumbByParentId[$id] ?? $url, + 'thumb' => $thumb !== '' ? $thumb : $url, + 'thumbs' => $thumbs, 'name' => $name, 'description' => (string) ($row['description'] ?? ''), 'alt' => $name !== '' ? $name : $pagetitle, 'position' => (int) ($row['position'] ?? 0), - 'is_preview' => $previewFileId > 0 && $id === $previewFileId, + 'is_preview' => $effectivePreview > 0 && $id === $effectivePreview, ]; } @@ -87,9 +118,14 @@ public static function whitelistItems(array $images): array $clean = []; foreach (self::ITEM_KEYS as $key) { - if (array_key_exists($key, $item)) { - $clean[$key] = $item[$key]; + if (!array_key_exists($key, $item)) { + continue; + } + if ($key === 'thumbs') { + $clean['thumbs'] = self::whitelistThumbs($item['thumbs']); + continue; } + $clean[$key] = $item[$key]; } if ($clean !== []) { @@ -99,4 +135,24 @@ public static function whitelistItems(array $images): array return $out; } + + /** + * @return array + */ + public static function whitelistThumbs(mixed $thumbs): array + { + if (!is_array($thumbs)) { + return []; + } + + $out = []; + foreach ($thumbs as $size => $url) { + if ($size === '' || (!is_scalar($url) && $url !== null)) { + continue; + } + $out[(string) $size] = (string) $url; + } + + return $out; + } } diff --git a/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php b/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php index 7194364f9..fcf80c14e 100644 --- a/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php +++ b/core/components/minishop3/src/Services/Product/ProductGalleryPublicService.php @@ -8,10 +8,11 @@ use MODX\Revolution\modX; /** - * Batch-load public gallery rows for product/get?include_images=1 (#566). + * Batch-load public gallery rows for product get/list/images (#566). * * Only active top-level images (parent_id=0, type=image). Child thumbs are - * joined in a second IN-query, not per file. + * joined in a second IN-query, not per file. Preferred `thumb` matches + * ms3_product_thumbnail_size (same path folder as mgr Gallery GetList). */ final class ProductGalleryPublicService { @@ -25,39 +26,73 @@ public function __construct( */ public function loadForProduct(int $productId, string $pagetitle, int $previewFileId): array { - if ($productId <= 0) { + return $this->loadForProducts( + [$productId => ['pagetitle' => $pagetitle, 'preview_file_id' => $previewFileId]], + ProductGalleryPublicSerializer::MAX_IMAGES, + )[$productId] ?? []; + } + + /** + * @param array $products + * @return array>> + */ + public function loadForProducts(array $products, int $perProductLimit): array + { + $ids = []; + foreach (array_keys($products) as $id) { + $id = (int) $id; + if ($id > 0) { + $ids[] = $id; + } + } + if ($ids === []) { return []; } - $originals = $this->fetchOriginals($productId); - if ($originals === []) { - return []; + $limit = max(1, $perProductLimit); + $grouped = $this->groupOriginals($this->fetchOriginalsForProducts($ids), $limit); + $parentIds = []; + foreach ($grouped as $rows) { + foreach ($rows as $row) { + $parentIds[] = (int) ($row['id'] ?? 0); + } + } + $thumbs = $this->fetchThumbsByParent(array_values(array_filter($parentIds))); + + $out = []; + foreach ($products as $id => $meta) { + $id = (int) $id; + if ($id <= 0) { + continue; + } + $out[$id] = ProductGalleryPublicSerializer::serializeGallery( + $grouped[$id] ?? [], + $thumbs, + (string) ($meta['pagetitle'] ?? ''), + (int) ($meta['preview_file_id'] ?? 0), + ); } - return ProductGalleryPublicSerializer::serializeGallery( - $originals, - $this->fetchThumbsByParent(array_map(intval(...), array_column($originals, 'id'))), - $pagetitle, - $previewFileId, - ); + return $out; } /** + * @param list $productIds * @return list> */ - private function fetchOriginals(int $productId): array + private function fetchOriginalsForProducts(array $productIds): array { $c = $this->modx->newQuery(msProductFile::class); $c->where([ - 'product_id' => $productId, + 'product_id:IN' => $productIds, 'parent_id' => 0, 'type' => 'image', 'active' => 1, ]); - $c->select('id, url, name, description, position'); + $c->select('id, product_id, url, name, description, position'); + $c->sortby('product_id', 'ASC'); $c->sortby('position', 'ASC'); $c->sortby('id', 'ASC'); - $c->limit(ProductGalleryPublicSerializer::MAX_IMAGES); if (!$c->prepare() || !$c->stmt->execute()) { return []; @@ -67,10 +102,32 @@ private function fetchOriginals(int $productId): array } /** - * First child image url per original (lowest id). - * + * @param list> $rows + * @return array>> + */ + private function groupOriginals(array $rows, int $perProductLimit): array + { + $grouped = []; + foreach ($rows as $row) { + $productId = (int) ($row['product_id'] ?? 0); + if ($productId <= 0) { + continue; + } + if (!isset($grouped[$productId])) { + $grouped[$productId] = []; + } + if (count($grouped[$productId]) >= $perProductLimit) { + continue; + } + $grouped[$productId][] = $row; + } + + return $grouped; + } + + /** * @param list $parentIds - * @return array + * @return array}> */ private function fetchThumbsByParent(array $parentIds): array { @@ -84,22 +141,46 @@ private function fetchThumbsByParent(array $parentIds): array 'type' => 'image', 'active' => 1, ]); - $c->select('id, parent_id, url'); + $c->select('id, parent_id, product_id, url, path'); $c->sortby('id', 'ASC'); if (!$c->prepare() || !$c->stmt->execute()) { return []; } - $thumbs = []; + $preferred = $this->preferredThumbSize(); + $out = []; foreach ($c->stmt->fetchAll(\PDO::FETCH_ASSOC) ?: [] as $row) { $parentId = (int) ($row['parent_id'] ?? 0); - if ($parentId <= 0 || isset($thumbs[$parentId])) { + if ($parentId <= 0) { continue; } - $thumbs[$parentId] = (string) ($row['url'] ?? ''); + $url = (string) ($row['url'] ?? ''); + $size = ProductGalleryPublicSerializer::sizeKeyFromChild( + (string) ($row['path'] ?? ''), + $url, + (int) ($row['product_id'] ?? 0), + ); + if (!isset($out[$parentId])) { + $out[$parentId] = ['thumb' => '', 'thumbs' => []]; + } + if ($size !== '' && !isset($out[$parentId]['thumbs'][$size])) { + $out[$parentId]['thumbs'][$size] = $url; + } + if ($size === $preferred) { + $out[$parentId]['thumb'] = $url; + } elseif ($out[$parentId]['thumb'] === '') { + $out[$parentId]['thumb'] = $url; + } } - return $thumbs; + return $out; + } + + private function preferredThumbSize(): string + { + $size = trim((string) $this->modx->getOption('ms3_product_thumbnail_size', null, 'small')); + + return $size !== '' ? $size : 'small'; } } diff --git a/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php b/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php index 3fa088ba5..68f837a22 100644 --- a/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php +++ b/core/components/minishop3/tests/ProductCatalogImagesRoutesTest.php @@ -1,7 +1,7 @@ $catalog, 'controller' => $controller, 'service' => $service, 'serializer' => $serializer] as $label => $src) { +foreach ( + [ + 'catalog' => $catalog, + 'controller' => $controller, + 'service' => $service, + 'serializer' => $serializer, + 'webRoutes' => $webRoutes, + ] as $label => $src +) { if ($src === false || $src === '') { $fail("unable to read {$label}"); } @@ -36,12 +45,21 @@ if (!str_contains($catalog, "ms3_product_gallery_public")) { $fail('gallery service must come from ServiceRegistry'); } -if (!preg_match('/function getById\([\s\S]*?gallery\(\)->loadForProduct/', $catalog)) { +if (!preg_match('/function getById\([\s\S]*?loadImagesForProduct/', $catalog)) { $fail('gallery load must run inside getById after product is found'); } -if (preg_match('/public function getList\(array \$params\): array\s*\{([\s\S]*?)\n public function /', $catalog, $m) - && str_contains($m[1], 'loadForProduct')) { - $fail('product/list must not load gallery in MVP'); +if (!preg_match('/function getList\([\s\S]*?loadImagesForProducts/', $catalog)) { + $fail('product/list must batch-load gallery when include_images is on'); +} +if (!str_contains($catalog, 'function getPublicImages')) { + $fail('catalog must expose getPublicImages for /product/{id}/images'); +} + +if (!str_contains($controller, 'function getImages')) { + $fail('ProductController must expose getImages'); +} +if (!str_contains($webRoutes, "'/{id}/images'") && !str_contains($webRoutes, '"/{id}/images"')) { + $fail('web.php must register GET /product/{id}/images'); } $registry = file_get_contents(__DIR__ . '/../src/ServiceRegistry.php'); @@ -58,10 +76,16 @@ if (!str_contains($service, 'parent_id:IN')) { $fail('thumbs must batch-load via parent_id IN'); } +if (!str_contains($service, 'ms3_product_thumbnail_size')) { + $fail('thumb must prefer ms3_product_thumbnail_size'); +} if (!str_contains($serializer, 'no DB `alt`')) { $fail('serializer must document name → alt mapping'); } +if (!str_contains($serializer, "'thumbs'")) { + $fail('serializer must expose multi-size thumbs map'); +} fwrite(STDOUT, "OK ProductCatalogImagesRoutesTest\n"); exit(0); diff --git a/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php b/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php index eabfe7bf2..65c88ecf8 100644 --- a/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php +++ b/core/components/minishop3/tests/Unit/Services/Product/ProductGalleryPublicSerializerTest.php @@ -17,14 +17,19 @@ public function testSerializeKeepsPositionOrderAndPreviewFlag(): void ['id' => 11, 'url' => '/b.jpg', 'name' => '', 'description' => '', 'position' => 1], ['id' => 12, 'url' => '/c.jpg', 'name' => 'Side', 'description' => '', 'position' => 2], ], - [10 => '/a_small.jpg', 12 => '/c_small.jpg'], + [ + 10 => ['thumb' => '/a_small.jpg', 'thumbs' => ['small' => '/a_small.jpg']], + 12 => ['thumb' => '/c_small.jpg', 'thumbs' => ['small' => '/c_small.jpg']], + ], 'Kettle', 12, ); self::assertSame([10, 11, 12], array_column($items, 'id')); self::assertSame('/a_small.jpg', $items[0]['thumb']); + self::assertSame(['small' => '/a_small.jpg'], $items[0]['thumbs']); self::assertSame('/b.jpg', $items[1]['thumb']); + self::assertSame([], $items[1]['thumbs']); self::assertTrue($items[2]['is_preview']); self::assertFalse($items[0]['is_preview']); self::assertSame('Front', $items[0]['alt']); @@ -32,6 +37,22 @@ public function testSerializeKeepsPositionOrderAndPreviewFlag(): void self::assertSame('wide', $items[0]['description']); } + public function testStalePreviewFallsBackToFirstOriginal(): void + { + $items = ProductGalleryPublicSerializer::serializeGallery( + [ + ['id' => 10, 'url' => '/a.jpg', 'name' => 'A', 'position' => 0], + ['id' => 11, 'url' => '/b.jpg', 'name' => 'B', 'position' => 1], + ], + [], + 'Tea', + 99, + ); + + self::assertTrue($items[0]['is_preview']); + self::assertFalse($items[1]['is_preview']); + } + public function testSerializeSkipsInvalidIdsAndDoesNotUseHash(): void { $items = ProductGalleryPublicSerializer::serializeGallery( @@ -45,7 +66,10 @@ public function testSerializeSkipsInvalidIdsAndDoesNotUseHash(): void ); self::assertCount(1, $items); - self::assertSame(['id', 'url', 'thumb', 'name', 'description', 'alt', 'position', 'is_preview'], array_keys($items[0])); + self::assertSame( + ['id', 'url', 'thumb', 'thumbs', 'name', 'description', 'alt', 'position', 'is_preview'], + array_keys($items[0]) + ); self::assertArrayNotHasKey('hash', $items[0]); self::assertArrayNotHasKey('path', $items[0]); self::assertArrayNotHasKey('createdby', $items[0]); @@ -60,6 +84,16 @@ public function testEmptyOriginalsYieldEmptyGallery(): void ); } + public function testSizeKeyFromChildUsesPathThenUrl(): void + { + self::assertSame('small', ProductGalleryPublicSerializer::sizeKeyFromChild('22/small/', '', 22)); + self::assertSame( + 'medium', + ProductGalleryPublicSerializer::sizeKeyFromChild('', '/assets/products/22/medium/a.jpg', 22) + ); + self::assertSame('', ProductGalleryPublicSerializer::sizeKeyFromChild('', '/a.jpg', 22)); + } + public function testWhitelistItemsDropsInternalsAndNonArrays(): void { $clean = ProductGalleryPublicSerializer::whitelistItems([ @@ -71,12 +105,14 @@ public function testWhitelistItemsDropsInternalsAndNonArrays(): void 'path' => '/fs', 'createdby' => 9, 'alt' => 'X', + 'thumbs' => ['small' => '/x_s.jpg', 'leak' => ['nope']], ], ]); self::assertCount(1, $clean); self::assertSame(1, $clean[0]['id']); self::assertSame('/x.jpg', $clean[0]['url']); + self::assertSame(['small' => '/x_s.jpg'], $clean[0]['thumbs']); self::assertArrayNotHasKey('hash', $clean[0]); self::assertArrayNotHasKey('path', $clean[0]); self::assertArrayNotHasKey('createdby', $clean[0]); From 00ec65241f0cb46a88aa7bc7a627da123b257643 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 19 Aug 2026 08:32:35 +0600 Subject: [PATCH 3/3] =?UTF-8?q?fix(web-api):=20=D0=BF=D1=83=D0=B1=D0=BB?= =?UTF-8?q?=D0=B8=D1=87=D0=BD=D1=8B=D0=B9=20/product/{id}/images=20=D0=B1?= =?UTF-8?q?=D0=B5=D0=B7=20widening=20prefix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Оставляем явный allow-list после #584. Галерея матчится шаблоном */images, а не префиксом /api/v1/product/. --- .../src/Middleware/TokenMiddleware.php | 39 +++++++++++++ .../tests/TokenMiddlewarePublicRoutesTest.php | 8 +++ .../TokenMiddlewarePublicPatternTest.php | 57 +++++++++++++++++++ 3 files changed, 104 insertions(+) create mode 100644 core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index 5c035a923..6550d8a07 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -48,6 +48,16 @@ class TokenMiddleware implements MiddlewareInterface '/api/v1/health', ]; + /** + * Glob patterns (`*` = one path segment). Keeps /{id}/images public without + * opening the whole `/api/v1/product/*` group via a prefix (#584). + * + * @var list + */ + private array $publicRoutePatterns = [ + '/api/v1/product/*/images', + ]; + /** * @param modX $modx MODX instance */ @@ -196,15 +206,44 @@ private function isPublicRoute(string $uri): bool $route = preg_replace('#^/assets/components/minishop3/api\.php#', '', $path); } + $route = $this->normalizePublicPath((string) $route); + foreach ($this->publicRoutes as $publicRoute) { if (str_starts_with($route, $publicRoute)) { return true; } } + foreach ($this->publicRoutePatterns as $pattern) { + if (self::matchesSegmentPattern($route, $pattern)) { + return true; + } + } + return false; } + private function normalizePublicPath(string $route): string + { + $qPos = strpos($route, '?'); + if ($qPos !== false) { + $route = substr($route, 0, $qPos); + } + + if ($route !== '/') { + $route = rtrim($route, '/'); + } + + return $route; + } + + private static function matchesSegmentPattern(string $path, string $pattern): bool + { + $regex = '#^' . str_replace('\\*', '[^/]+', preg_quote($pattern, '#')) . '$#'; + + return (bool) preg_match($regex, $path); + } + /** * Add public route * diff --git a/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php b/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php index cd04e6389..fe39388f9 100644 --- a/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php +++ b/core/components/minishop3/tests/TokenMiddlewarePublicRoutesTest.php @@ -35,6 +35,14 @@ $fail('cart/get must not be public — otherwise guest GET never auto-mints a token (#408)'); } +if (in_array('/api/v1/product/', $publicRoutes, true)) { + $fail('wide /api/v1/product/ prefix would publish the whole product group (#584)'); +} + +if (!str_contains($middlewareSrc, "'/api/v1/product/*/images'")) { + $fail('publicRoutePatterns must include /api/v1/product/*/images'); +} + foreach ( [ '/api/v1/product/get/', diff --git a/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php b/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php new file mode 100644 index 000000000..618608d35 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Middleware/TokenMiddlewarePublicPatternTest.php @@ -0,0 +1,57 @@ +middleware = new TokenMiddleware(new modX()); + $this->isPublic = new ReflectionMethod(TokenMiddleware::class, 'isPublicRoute'); + $this->isPublic->setAccessible(true); + } + + protected function tearDown(): void + { + unset($_REQUEST['route']); + } + + #[DataProvider('routes')] + public function testPublicPatternDoesNotWidenProductGroup(string $route, bool $expected): void + { + $_REQUEST['route'] = $route; + + self::assertSame($expected, $this->isPublic->invoke($this->middleware, '/')); + } + + /** + * @return iterable + */ + public static function routes(): iterable + { + yield 'images' => ['/api/v1/product/42/images', true]; + yield 'images trailing slash' => ['/api/v1/product/42/images/', true]; + yield 'images query' => ['/api/v1/product/42/images?include_thumbs=1', true]; + yield 'filters prefix' => ['/api/v1/product/filters', true]; + yield 'list prefix' => ['/api/v1/product/list', true]; + yield 'unknown product sibling' => ['/api/v1/product/42/reviews', false]; + yield 'product root' => ['/api/v1/product/42', false]; + yield 'nested after images' => ['/api/v1/product/42/images/raw', false]; + } +}