Skip to content

Web API: resolve category/product по alias|uri - #644

Open
Ibochkarev wants to merge 3 commits into
betafrom
feat/issue-579-catalog-resolve-alias-uri
Open

Web API: resolve category/product по alias|uri#644
Ibochkarev wants to merge 3 commits into
betafrom
feat/issue-579-catalog-resolve-alias-uri

Conversation

@Ibochkarev

Copy link
Copy Markdown
Member

Описание

Публичный lookup категории и товара по alias или uri (+ context) для headless SSR без numeric MODX id. Ответ тот же, что у GET …/get/{id}.

Контракт: ровно один из alias/uri. Ошибки lookup → 400 с lexicon. Невидимый / чужой context / неоднозначный alias → 404.

Кэш каталога (#579 cache-часть) — out of scope, follow-up. Docs #573 — вне репозитория, follow-up.

Тип изменений

  • Новая функциональность (non-breaking change)
  • Исправление бага (non-breaking change)
  • Breaking change (изменение, ломающее обратную совместимость)
  • Рефакторинг (без изменения функциональности)
  • Документация
  • Другое (опишите):

Связанные Issues

Closes #579 (resolve-only; cache и docs.modx.pro/#573 — follow-up)

Как это было протестировано?

cd core/components/minishop3
php -l src/Services/Catalog/CatalogResolve.php
# … и остальные затронутые PHP — exit 0

vendor/bin/phpunit tests/Unit/Services/Catalog/CatalogResolveTest.php
# OK — 24 tests, exit 0

php tests/CategoryCatalogRoutesTest.php
php tests/ProductCatalogRoutesTest.php
php tests/TokenMiddlewarePublicRoutesTest.php
# OK, exit 0

vendor/bin/phpunit tests/Integration/WebApi/HeadlessStorefrontErrorsTest.php \
  --filter testProductGetWithoutLookupParamsReturns400
# OK — 1 test, exit 0
  • Ручное тестирование
  • Автоматические тесты (unit CatalogResolve, smoke routes/TokenMiddleware, integration 400)
  • Тестирование на разных версиях PHP/MODX

Конфигурация тестирования:

  • MiniShop3: feat/issue-579-catalog-resolve-alias-uri
  • MODX: n/a (unit/smoke)
  • PHP: 8.4.23

Скриншоты (если применимо)

До После
n/a n/a

Чеклист

  • Код соответствует стилю проекта
  • Добавлены/обновлены комментарии в сложных местах
  • Изменения не ломают существующую функциональность
  • Лексиконы добавлены на двух языках (ru/en)
  • PHPStan проходит без новых ошибок (composer stan / CI job PHPStan)
  • ESLint проходит без ошибок (npm run lint:ci для Vue) — Vue не трогали
  • Обновлён CHANGELOG.md — на релиз

Дополнительные заметки

Endpoints

GET /api/v1/category/get?uri=catalog/shoes/&context=web
GET /api/v1/category/get?alias=shoes&context=web
GET /api/v1/product/get?alias=red-sneakers&context=web

Реализация

  • CatalogResolve — parse/normalize + findUniqueId (LIMIT 2)
  • CategoryCatalogService / ProductCatalogServiceresolveByLookupgetById
  • Маршруты GET /get зарегистрированы перед GET /get/{id}
  • TokenMiddleware: exact path или prefix/ (без ложного match на getting)

Вне scope

@AgelxNash AgelxNash mentioned this pull request Sep 6, 2026
16 tasks
AgelxNash pushed a commit to AgelxNash/MiniShop3 that referenced this pull request Sep 6, 2026
Conflict resolution: keep seo (599) + menuindex (627) imports alongside
new CatalogResolve.
@AgelxNash

Copy link
Copy Markdown

Этот PR включён в тестовую интеграционную сборку всех открытых PR MiniShop3: AgelxNash/MiniShop3, ветка integration/open-prs-20260906 (28/28 открытых).

Сборка нужна, чтобы проверить совместимость взаимозависимых серий PR до их мержа — при последовательном слиянии они конфликтуют друг с другом. Это не ревью и не конкурирующий PR: авторство сохранено (1 PR = 1 коммит с исходным автором), ветка пересобирается по мере обновления PR.

Как вошёл в сборку: Конфликты импортов в CategoryCatalogService/ProductCatalogService разрешены: CatalogResolve добавлен рядом с уже слитыми PublicSeoService (#599) и CategoryProductMenuindexService (#627).

@AgelxNash

Copy link
Copy Markdown

Удачи с PR! Пусть дойдёт до релиза как можно скорее — спасибо за вклад в MiniShop3! 🚀

@biz87

biz87 commented Sep 7, 2026

Copy link
Copy Markdown
Member

По существу PR хороший, к вливанию почти готов — возвращаю ради одной небольшой правки в списке публичных роутов.

Что подтвердилось по коду

  • SQL-инъекция исключенаCatalogResolve::findUniqueId() строит запрос через newQuery() с criteria-массивом, сырого SQL нет.
  • class_key проставляется явно в publicCriteria() обоих сервисов. Известная ловушка проекта (getIterator() не вызывает addDerivativeCriteria(), в отличие от getCollection()) здесь неприменима — код использует ручной newQuery() с явным class_key.
  • published=1, deleted=0, context_key учтены и в самом резолве, и повторно в финальном getById(). LIMIT 2 требует ровно одного совпадения, иначе 404 — неоднозначный alias не отдаёт случайный ресурс.
  • whitelistPublicPayload() применяется после modifyFields() — плагин не может протащить лишнее поле мимо whitelist.
  • Порядок роутов верный: GET /get зарегистрирован перед GET /get/{id} и не проглатывается им, это подтверждено интеграционным тестом.
  • Нагрузка: rate limit висит на всей группе /api/v1, а alias и uri в схеме ядра MODX проиндексированы BTREE — резолв не даёт full table scan. Решение оставить кэш вне scope обоснованно, нового вектора дешёвой нагрузки сверх уже существующего для публичного каталога нет.

Отдельно отмечу CatalogResolve::sanitizeContext() — ограничение длины, набор символов и явная блокировка префикса mgr. У существующих getById()/getList()/getTree() такой санитизации нет вовсе, то есть новый путь защищён лучше соседних. Заведём отдельную задачу на выравнивание.

Локально на ветке с домердженной актуальной beta (мержил дважды): smoke 92/92, PHPUnit 299 тестов / 718 assertions, WebApi 23/23, CatalogResolveTest 24/24, PHPStan по шести файлам без ошибок.

Что просьба поправить

Новая логика сравнения корректна:

if ($route === $publicRoute || str_starts_with($route, $publicRoute . '/')) {

Но в $publicRoutes ты привёл к ней только product/get и category/get, сняв завершающий слэш, а delivery/get/ и payment/get/ остались как были. Нормализации пути в этом PR нет, поэтому для них новая логика не срабатывает ни одной веткой.

Прогнал обе логики на реальном содержимом списка:

маршрут                            было      стало
/api/v1/product/get                public    public
/api/v1/product/get/5              public    public
/api/v1/category/get/7             public    public
/api/v1/delivery/get/5             public    closed   <--
/api/v1/payment/get/3              public    closed   <--
/api/v1/product/getting-started    public    closed   <-- это целевой фикс, всё верно

getting-started закрылся правильно — ради этого правка и делалась. А вот delivery/get/{id} и payment/get/{id} (роуты реальные, config/routes/web.php строки ~309 и ~321) выпали случайно: для /api/v1/delivery/get/5 не проходит ни точное равенство с /api/v1/delivery/get/, ни str_starts_with с /api/v1/delivery/get//.

Практического эффекта сегодня нет — группы /delivery и /payment, как и каталожные, объявлены без $tokenMiddleware, так что список их не гейтит. Но он фиксирует намерение и покрыт тестом, а при подключении middleware эти роуты молча потребуют токен. Починка — снять два завершающих слэша, чтобы записи стали единообразны с остальными.

Заодно стоит добавить в TokenMiddlewarePublicRoutesTest кейс на delivery/get/{id} — сейчас регрессия этим тестом не ловится.

К сведению

Конфликта с #598 (галерея, уже влит в beta) нет — проверил трёхсторонним прогоном: normalizePublicPath() из #598, твоё точное сравнение и glob-канал для /product/*/images складываются вместе и работают корректно, ни один из двух фиксов не теряется. GitHub после вливания #598 по-прежнему показывает PR как MERGEABLE.

Ещё одно наблюдение, не к тебе и не к этому PR: ни один публичный каталожный эндпоинт не проверяет resource_groups. Если закрытые для веб-группы ресурсы предполагаются поддерживаться — это дыра во всём публичном API, заведу отдельно.

После правки слэшей — вливаем.

Add public GET /category/get and /product/get lookup for headless SSR
routing without numeric resource ids. Cache remains a follow-up.
getSelectColumns with the FQCN as table alias produced invalid SQL for
namespaced msProduct/msCategory, so alias/uri lookup always returned 404.
Align publicRoutes with segment matching so delivery/get/{id} and
payment/get/{id} stay public after the exact-or-prefix rewrite.
@Ibochkarev

Copy link
Copy Markdown
Member Author

Адрес ревью:

  • Сняты завершающие слэши у /api/v1/delivery/get и /api/v1/payment/get в $publicRoutes
  • Runtime-кейсы в TokenMiddlewarePublicRoutesTest + PHPUnit: delivery/get/{id}, payment/get/{id} public; product/getting-started по-прежнему closed

Rebase на актуальную beta. composer ci:php зелёный.

@Ibochkarev
Ibochkarev force-pushed the feat/issue-579-catalog-resolve-alias-uri branch from 318af42 to 3c8f3f4 Compare September 7, 2026 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Web API: resolve category/product по alias|uri + cache

3 participants