From 27ffcdb4c1bdbd080e50bd3f0488fff0834bcf0e Mon Sep 17 00:00:00 2001 From: biz87 Date: Fri, 16 Oct 2026 01:23:47 +0500 Subject: [PATCH] =?UTF-8?q?perf(mgr-api):=20=D0=B2=D0=B5=D1=80=D0=BD=D1=83?= =?UTF-8?q?=D1=82=D1=8C=20LIMIT/OFFSET=20=D0=B2=20=D0=BB=D0=B8=D1=81=D1=82?= =?UTF-8?q?=D0=B8=D0=BD=D0=B3=20=D1=82=D0=BE=D0=B2=D0=B0=D1=80=D0=BE=D0=B2?= =?UTF-8?q?=20=D0=BA=D0=B0=D1=82=D0=B5=D0=B3=D0=BE=D1=80=D0=B8=D0=B8=20(#4?= =?UTF-8?q?73)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Листинг был обычной постраничной выборкой, пока #473 не добавил ACL уровня документа. Проверять checkPolicy('view') можно только по загруженному объекту, поэтому прямой вызов getPage() заменили сканированием: collectVisibleListPage() — сканирует от начала батчами и пропускает start записей в PHP; countVisibleListResults() — проходит всю категорию батчами по 200 и считает видимые, ради одного числа total. На категории с 2114 товарами это 18 запросов и 953 мс на первой странице, 33 запроса и 1350 мс на start=800 — цена растёт с номером страницы. Сервис при этом всё умеет сам: COUNT(DISTINCT msProduct.id) одним запросом и limit($limit, $start). Контроллер просто перестал этим пользоваться. Ограничить видимость ресурса может только группа ресурсов с записью доступа: modResource::findPolicy() собирает политику джойном modAccessResourceGroup + modResourceGroupResource и отдаёт пустой набор, когда настройка контекста выключена. Значит нет ни одной записи доступа — нет ни одного закрытого товара, и построчный фильтр заведомо пропускает всё; для sudo он пропускает всё всегда. В этих случаях включается быстрый путь, то есть ровно тот код, что был до #473. Если группы ресурсов настроены, остаётся прежнее сканирование: права важнее скорости. Замер после правки на той же категории: nested=false 1 запрос, 24-31 мс на любой странице (было 18-33 / 953-1350) nested=true 22 запроса, 32 мс (22 — обход дерева категорий, он был и раньше) total совпадает с прежним сканирующим подсчётом в обоих режимах: 2114. Проверено и обратное — при появлении записи modAccessResourceGroup условие переключается на false, после уборки обратно на true. Фильтрация листинга по ACL тестами не покрыта (ни collectVisibleListPage, ни countVisibleListResults, ни filterListResultsByDocumentView в тестах не упоминаются), поэтому добавлен тест на само условие переключения: ошибка в нём либо вернёт медленный листинг, либо обойдёт проверку прав. Стабу modX добавлены getCount() и optionOverrides. Подпись getCount() намеренно без типов: тесты переопределяют метод в анонимных подклассах со своей, более слабой. --- .../Manager/CategoryProductsController.php | 63 +++++++++++++ .../CategoryProductsListFastPathTest.php | 88 +++++++++++++++++++ .../minishop3/tests/stubs/ModxStub.php | 20 ++++- 3 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 core/components/minishop3/tests/Unit/Controllers/CategoryProductsListFastPathTest.php diff --git a/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php b/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php index 27b55ef5c..073a9c38a 100644 --- a/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php +++ b/core/components/minishop3/src/Controllers/Api/Manager/CategoryProductsController.php @@ -12,6 +12,8 @@ use MiniShop3\Services\Category\CategoryProductScopeService; use MiniShop3\Services\Category\CategoryProductsListService; use MiniShop3\Services\FilterConfigManager; +use MODX\Revolution\modAccessResourceGroup; +use MODX\Revolution\modUser; use MODX\Revolution\modX; /** @@ -25,6 +27,9 @@ class CategoryProductsController { protected modX $modx; + /** Кэш на запрос: ACL документов проверяется один раз. */ + private ?bool $documentAclInert = null; + public function __construct(modX $modx) { $this->modx = $modx; @@ -85,6 +90,26 @@ public function getList(array $params = []): array return $this->errorResponse('ms3_err_category_products_list_service', HttpStatus::INTERNAL_SERVER_ERROR); } + // Когда ACL документов никого не ограничивает, страницу отдаёт один + // запрос с LIMIT и один COUNT — так листинг и работал до #473. + if ($this->documentAclIsInert()) { + $page = $listService->getPage( + $categoryId, + $params, + $nested, + $gridFields, + $start, + $limit, + (string) $sortBy, + $sortDir + ); + + return Response::success([ + 'results' => $page['results'], + 'total' => $page['total'], + ])->getData(); + } + $results = $this->collectVisibleListPage( $listService, $categoryId, @@ -113,6 +138,44 @@ public function getList(array $params = []): array ])->getData(); } + /** + * Может ли построчная проверка checkPolicy('view') хоть что-то отбросить. + * + * #473 добавил ACL уровня документа в грид товаров категории, и ради него + * прямой LIMIT/OFFSET заменили сканированием: проверять политику можно + * только по загруженному объекту. Из-за этого на каждый запрос страницы + * вычитывалась вся категория — для total всегда, плюс от начала до start + * для самой страницы. На 2114 товарах это около секунды, и чем дальше + * страница, тем дороже. + * + * Ограничить видимость ресурса может только группа ресурсов с записью + * доступа: modResource::findPolicy() собирает политику джойном + * modAccessResourceGroup + modResourceGroupResource и возвращает пустой + * набор, когда настройка контекста выключена. Значит ни одной записи + * доступа по группам ресурсов — ни одного закрытого товара, и фильтр + * заведомо пропускает всё. Для sudo он пропускает всё всегда. + * + * В этих случаях работает быстрый путь. Если группы ресурсов на сайте + * настроены, остаётся прежнее сканирование: права важнее скорости. + */ + private function documentAclIsInert(): bool + { + if ($this->documentAclInert !== null) { + return $this->documentAclInert; + } + + $user = $this->modx->user; + if ($user instanceof modUser && $user->get('sudo')) { + return $this->documentAclInert = true; + } + + if (!$this->modx->getOption('access_resource_group_enabled', null, true)) { + return $this->documentAclInert = true; + } + + return $this->documentAclInert = $this->modx->getCount(modAccessResourceGroup::class) === 0; + } + /** * Get filters configuration * GET /api/mgr/categories/{id}/products/filters diff --git a/core/components/minishop3/tests/Unit/Controllers/CategoryProductsListFastPathTest.php b/core/components/minishop3/tests/Unit/Controllers/CategoryProductsListFastPathTest.php new file mode 100644 index 000000000..edb847278 --- /dev/null +++ b/core/components/minishop3/tests/Unit/Controllers/CategoryProductsListFastPathTest.php @@ -0,0 +1,88 @@ +isInert(aclRows: 0, groupsEnabled: true)); + } + + public function testNotInertWhenResourceGroupAclExists(): void + { + // Появилась хоть одна запись доступа — возвращаемся к проверке прав. + self::assertFalse($this->isInert(aclRows: 1, groupsEnabled: true)); + self::assertFalse($this->isInert(aclRows: 25, groupsEnabled: true)); + } + + public function testInertWhenResourceGroupAccessDisabledInContext(): void + { + // modResource::findPolicy() при выключенной настройке отдаёт пустую + // политику, то есть ограничений нет даже при наличии записей. + self::assertTrue($this->isInert(aclRows: 5, groupsEnabled: false)); + } + + /** + * Результат кэшируется на запрос: условие дёргается один раз на листинг. + */ + public function testResultIsMemoisedPerRequest(): void + { + $modx = $this->makeModx(aclRows: 0, groupsEnabled: true); + $controller = new CategoryProductsController($modx); + $method = new ReflectionMethod($controller, 'documentAclIsInert'); + $method->setAccessible(true); + + self::assertTrue($method->invoke($controller)); + + // Меняем ответ хранилища: закэшированное решение не должно поехать. + $modx->counts[modAccessResourceGroup::class] = 3; + + self::assertTrue($method->invoke($controller)); + } + + private function isInert(int $aclRows, bool $groupsEnabled): bool + { + $controller = new CategoryProductsController($this->makeModx($aclRows, $groupsEnabled)); + $method = new ReflectionMethod($controller, 'documentAclIsInert'); + $method->setAccessible(true); + + return (bool) $method->invoke($controller); + } + + private function makeModx(int $aclRows, bool $groupsEnabled): modX + { + $modx = new modX(); + $modx->optionOverrides['access_resource_group_enabled'] = $groupsEnabled ? '1' : '0'; + $modx->counts[modAccessResourceGroup::class] = $aclRows; + + return $modx; + } +} diff --git a/core/components/minishop3/tests/stubs/ModxStub.php b/core/components/minishop3/tests/stubs/ModxStub.php index 11de16574..584428da9 100644 --- a/core/components/minishop3/tests/stubs/ModxStub.php +++ b/core/components/minishop3/tests/stubs/ModxStub.php @@ -69,13 +69,31 @@ public function load(string ...$topics): void }; } + /** @var array Переопределения для getOption(). */ + public array $optionOverrides = []; + + /** @var array Ответы getCount() по имени класса. */ + public array $counts = []; + /** * @param mixed $default * @return mixed */ public function getOption(string $key, $options = null, $default = null) { - return $default; + return array_key_exists($key, $this->optionOverrides) ? $this->optionOverrides[$key] : $default; + } + + /** + * Подпись намеренно без типов — тесты переопределяют метод в анонимных + * подклассах со своей, более слабой. + * + * @param mixed $className + * @param mixed $criteria + */ + public function getCount($className, $criteria = null) + { + return $this->counts[(string) $className] ?? 0; } /**