fix(tradein/avito): экранировать _ в LIKE-паттернах + честная эмуляция LIKE в тесте (#2576)
All checks were successful
CI / changes (pull_request) Successful in 10s
CI Trade-In / changes (pull_request) Successful in 12s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 2m15s
All checks were successful
CI / changes (pull_request) Successful in 10s
CI Trade-In / changes (pull_request) Successful in 12s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 2m15s
This commit is contained in:
parent
10442d0187
commit
5a66c2df51
2 changed files with 105 additions and 22 deletions
|
|
@ -79,7 +79,22 @@ __all__ = [
|
||||||
# города (может отличаться от нашего city_slug: kamensk-uralskiy через дефис,
|
# города (может отличаться от нашего city_slug: kamensk-uralskiy через дефис,
|
||||||
# verhnyaya_pyshma без "kh") -- дублировать список тут вместо импорта было бы
|
# verhnyaya_pyshma без "kh") -- дублировать список тут вместо импорта было бы
|
||||||
# risk дрейфа при добавлении новых oblast-городов.
|
# risk дрейфа при добавлении новых oblast-городов.
|
||||||
_OBLAST_AVITO_URL_PATTERNS = tuple(f"%/{loc.avito_slug}/%" for loc in CITY_LOCATIONS.values())
|
#
|
||||||
|
# #2578 review: Postgres LIKE трактует '_' как wildcard "один любой символ" (не
|
||||||
|
# литерал) и '%' как wildcard "любая последовательность" -- два слага из пяти
|
||||||
|
# (nizhniy_tagil, verhnyaya_pyshma) содержат '_', без экранирования это латентная
|
||||||
|
# дыра: город с похожим слагом (напр. nizhniyXtagil) молча совпал бы. Сегодня
|
||||||
|
# коллизий нет (проверено на проде: raw vs escaped паттерны дают одинаковые 776
|
||||||
|
# совпадений), но экранируем сейчас, а не когда появится реальная коллизия.
|
||||||
|
# LIKE по умолчанию использует '\' как escape-символ (без явного ESCAPE) —
|
||||||
|
# подтверждено на живом Postgres 16.4 (см. коммит #2578-fixup): 'nizhniyXtagil'
|
||||||
|
# матчит неэкранированный '%/nizhniy_tagil/%' (LIKE default '_'=wildcard) и НЕ
|
||||||
|
# матчит экранированный '%/nizhniy\_tagil/%' (LIKE '\_' = литерал '_'); точный
|
||||||
|
# слаг 'nizhniy_tagil' матчит оба варианта -- позитивный кейс не сломан.
|
||||||
|
_OBLAST_AVITO_URL_PATTERNS = tuple(
|
||||||
|
"%/" + loc.avito_slug.replace("\\", "\\\\").replace("_", "\\_").replace("%", "\\%") + "/%"
|
||||||
|
for loc in CITY_LOCATIONS.values()
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
|
import fnmatch
|
||||||
import os
|
import os
|
||||||
import sys
|
import sys
|
||||||
from unittest.mock import AsyncMock, MagicMock, patch
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
@ -398,42 +399,109 @@ def test_oblast_avito_url_patterns_cover_region66_cities() -> None:
|
||||||
"""#2576: _OBLAST_AVITO_URL_PATTERNS строится из CITY_LOCATIONS.avito_slug —
|
"""#2576: _OBLAST_AVITO_URL_PATTERNS строится из CITY_LOCATIONS.avito_slug —
|
||||||
список должен покрывать реальные Avito-слаги oblast-городов (в т.ч. те, что
|
список должен покрывать реальные Avito-слаги oblast-городов (в т.ч. те, что
|
||||||
ОТЛИЧАЮТСЯ от нашего city_slug: kamensk-uralskiy через дефис, а не
|
ОТЛИЧАЮТСЯ от нашего city_slug: kamensk-uralskiy через дефис, а не
|
||||||
kamensk_uralskiy)."""
|
kamensk_uralskiy).
|
||||||
|
|
||||||
|
#2578 review: '_' в слаге -- LIKE wildcard, экранируем при построении паттерна
|
||||||
|
('_' -> '\\_') -- nizhniy_tagil/verhnyaya_pyshma здесь ожидаются С обратным
|
||||||
|
слэшем перед '_', НЕ голым подчёркиванием."""
|
||||||
assert "%/kamensk-uralskiy/%" in _OBLAST_AVITO_URL_PATTERNS
|
assert "%/kamensk-uralskiy/%" in _OBLAST_AVITO_URL_PATTERNS
|
||||||
assert "%/nizhniy_tagil/%" in _OBLAST_AVITO_URL_PATTERNS
|
assert "%/nizhniy\\_tagil/%" in _OBLAST_AVITO_URL_PATTERNS
|
||||||
assert "%/pervouralsk/%" in _OBLAST_AVITO_URL_PATTERNS
|
assert "%/pervouralsk/%" in _OBLAST_AVITO_URL_PATTERNS
|
||||||
assert "%/verhnyaya_pyshma/%" in _OBLAST_AVITO_URL_PATTERNS
|
assert "%/verhnyaya\\_pyshma/%" in _OBLAST_AVITO_URL_PATTERNS
|
||||||
assert "%/serov/%" in _OBLAST_AVITO_URL_PATTERNS
|
assert "%/serov/%" in _OBLAST_AVITO_URL_PATTERNS
|
||||||
# ЕКБ обрабатывается отдельным жёстко закодированным паттерном (ekb CTE),
|
# ЕКБ обрабатывается отдельным жёстко закодированным паттерном (ekb CTE),
|
||||||
# НЕ через этот oblast-список — не должен в него затесаться.
|
# НЕ через этот oblast-список — не должен в него затесаться.
|
||||||
assert not any("ekaterinburg" in p for p in _OBLAST_AVITO_URL_PATTERNS)
|
assert not any("ekaterinburg" in p for p in _OBLAST_AVITO_URL_PATTERNS)
|
||||||
|
|
||||||
|
|
||||||
|
def _like_pattern_to_fnmatch(pattern: str) -> str:
|
||||||
|
"""Точный перевод семантики Postgres `LIKE` (default `ESCAPE '\\'`) в fnmatch-
|
||||||
|
паттерн -- посимвольно, а НЕ наивным `.replace()`.
|
||||||
|
|
||||||
|
LIKE: `%` = любая последовательность символов, `_` = РОВНО один любой символ,
|
||||||
|
`\\%`/`\\_`/`\\\\` = литералы (экранирование). fnmatch: `*` = любая
|
||||||
|
последовательность, `?` = один любой символ; голые `_`/`%` в fnmatch не
|
||||||
|
специальны (можно вставлять как литерал без экранирования).
|
||||||
|
|
||||||
|
#2578 review: наивный `pat.replace("%", "*")` (как было раньше) НЕ отражал бы
|
||||||
|
семантику `_` вообще -- fnmatch трактует `_` как литерал, LIKE -- как wildcard.
|
||||||
|
Из-за этого расхождения прежний тест не поймал бы латентный баг (нет
|
||||||
|
экранирования `_` в продовых паттернах). Посимвольный разбор здесь корректно
|
||||||
|
различает голый `_` (-> `?` wildcard) и экранированный `\\_` (-> литерал `_`).
|
||||||
|
"""
|
||||||
|
out: list[str] = []
|
||||||
|
i = 0
|
||||||
|
n = len(pattern)
|
||||||
|
while i < n:
|
||||||
|
ch = pattern[i]
|
||||||
|
if ch == "\\" and i + 1 < n and pattern[i + 1] in ("%", "_", "\\"):
|
||||||
|
out.append(pattern[i + 1]) # экранированный символ -> литерал as-is
|
||||||
|
i += 2
|
||||||
|
continue
|
||||||
|
if ch == "%":
|
||||||
|
out.append("*")
|
||||||
|
elif ch == "_":
|
||||||
|
out.append("?")
|
||||||
|
else:
|
||||||
|
out.append(ch)
|
||||||
|
i += 1
|
||||||
|
return "".join(out)
|
||||||
|
|
||||||
|
|
||||||
|
def _in_oblast_or_ekb_scope(source_url: str) -> bool:
|
||||||
|
"""Локальная реплика WHERE-условия snapshot-запроса (ekb CTE OR oblast CTE)
|
||||||
|
через корректную LIKE-эмуляцию -- без поднятия БД."""
|
||||||
|
if fnmatch.fnmatchcase(source_url, _like_pattern_to_fnmatch("%/ekaterinburg/%")):
|
||||||
|
return True
|
||||||
|
return any(
|
||||||
|
fnmatch.fnmatchcase(source_url, _like_pattern_to_fnmatch(pat))
|
||||||
|
for pat in _OBLAST_AVITO_URL_PATTERNS
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_oblast_avito_url_patterns_include_oblast_and_ekb_exclude_foreign_region() -> None:
|
def test_oblast_avito_url_patterns_include_oblast_and_ekb_exclude_foreign_region() -> None:
|
||||||
"""#2576 DoD: листинг города области и екатеринбургский листинг проходят
|
"""#2576 DoD: листинг города области и екатеринбургский листинг проходят
|
||||||
scope-фильтр; листинг чужого региона (Москва/СПб) — нет.
|
scope-фильтр; листинг чужого региона (Москва/СПб) — нет.
|
||||||
|
|
||||||
Постгресовый `LIKE '%pat%'` эквивалентен fnmatch с `%` -> `*` (сам паттерн
|
Использует корректную LIKE-эмуляцию (_like_pattern_to_fnmatch), а не наивный
|
||||||
без иных SQL-метасимволов) — реплицируем ту же семантику локально, чтобы
|
`%` -> `*` replace (#2578 review — тот не различал бы `_`-семантику)."""
|
||||||
проверить реальные продовые паттерны (_OBLAST_AVITO_URL_PATTERNS) без
|
|
||||||
поднятия БД (юнит-тесты этого файла её не используют)."""
|
|
||||||
import fnmatch
|
|
||||||
|
|
||||||
def _in_scope(source_url: str) -> bool:
|
|
||||||
if fnmatch.fnmatchcase(source_url, "*/ekaterinburg/*"):
|
|
||||||
return True
|
|
||||||
return any(
|
|
||||||
fnmatch.fnmatchcase(source_url, pat.replace("%", "*"))
|
|
||||||
for pat in _OBLAST_AVITO_URL_PATTERNS
|
|
||||||
)
|
|
||||||
|
|
||||||
# Область (Каменск-Уральский, #2576 — реальный кейс из тикета) -- проходит.
|
# Область (Каменск-Уральский, #2576 — реальный кейс из тикета) -- проходит.
|
||||||
assert _in_scope("https://www.avito.ru/kamensk-uralskiy/kvartiry/prodam_123")
|
assert _in_oblast_or_ekb_scope("https://www.avito.ru/kamensk-uralskiy/kvartiry/prodam_123")
|
||||||
# ЕКБ — по-прежнему проходит (не деградировал).
|
# ЕКБ — по-прежнему проходит (не деградировал).
|
||||||
assert _in_scope("https://www.avito.ru/ekaterinburg/kvartiry/prodam_456")
|
assert _in_oblast_or_ekb_scope("https://www.avito.ru/ekaterinburg/kvartiry/prodam_456")
|
||||||
# Чужой регион — НЕ проходит (иначе поехали бы Москва/СПб/Тюмень legacy-строки).
|
# Чужой регион — НЕ проходит (иначе поехали бы Москва/СПб/Тюмень legacy-строки).
|
||||||
assert not _in_scope("https://www.avito.ru/moskva/kvartiry/prodam_789")
|
assert not _in_oblast_or_ekb_scope("https://www.avito.ru/moskva/kvartiry/prodam_789")
|
||||||
assert not _in_scope("https://www.avito.ru/sankt-peterburg/kvartiry/prodam_000")
|
assert not _in_oblast_or_ekb_scope("https://www.avito.ru/sankt-peterburg/kvartiry/prodam_000")
|
||||||
|
|
||||||
|
|
||||||
|
def test_like_underscore_wildcard_regression_caught_by_escaped_patterns() -> None:
|
||||||
|
"""#2578 deep-review latent bug: Postgres `LIKE` трактует `_` как wildcard
|
||||||
|
"ровно один любой символ", а НЕ литерал. Два слага из пяти (nizhniy_tagil,
|
||||||
|
verhnyaya_pyshma) содержат `_` -- БЕЗ экранирования 'nizhniy_tagil' молча
|
||||||
|
совпал бы с 'nizhniyXtagil' (X = любой символ), т.е. коллизия слагов при
|
||||||
|
появлении похожего города. Сегодня коллизий нет (проверено на проде: raw vs
|
||||||
|
escaped паттерны дают одинаковые 776 совпадений), но дыра латентная.
|
||||||
|
|
||||||
|
Этот тест ДОЛЖЕН падать на RAW (неэкранированном) варианте паттерна -- именно
|
||||||
|
так выглядели продовые паттерны ДО фикса #2578 (`%/nizhniy_tagil/%`, без
|
||||||
|
`\\`). Экранированный прод-паттерн (_OBLAST_AVITO_URL_PATTERNS, ПОСЛЕ фикса)
|
||||||
|
коллизию отклоняет, точный слаг по-прежнему матчит (позитивный кейс жив).
|
||||||
|
"""
|
||||||
|
raw_pattern = "%/nizhniy_tagil/%" # как было бы БЕЗ фикса #2578 (голый '_')
|
||||||
|
escaped_pattern = next(p for p in _OBLAST_AVITO_URL_PATTERNS if "nizhniy" in p)
|
||||||
|
# Сам факт экранирования: прод-паттерн ДОЛЖЕН отличаться от raw ('_' -> '\_').
|
||||||
|
assert escaped_pattern != raw_pattern, "фикс #2578 должен экранировать '_' в avito_slug"
|
||||||
|
|
||||||
|
collision_url = "https://www.avito.ru/nizhniyXtagil/kvartiry/prodam_1"
|
||||||
|
exact_url = "https://www.avito.ru/nizhniy_tagil/kvartiry/prodam_1"
|
||||||
|
|
||||||
|
# RAW: '_' -- wildcard -> ложное совпадение с ЛЮБЫМ символом на его месте.
|
||||||
|
assert fnmatch.fnmatchcase(collision_url, _like_pattern_to_fnmatch(raw_pattern))
|
||||||
|
# Экранированный прод-паттерн (после фикса): '_' -- литерал -> коллизия отклонена.
|
||||||
|
assert not fnmatch.fnmatchcase(collision_url, _like_pattern_to_fnmatch(escaped_pattern))
|
||||||
|
# Позитивный кейс не сломан: точный слаг матчит ОБА варианта паттерна.
|
||||||
|
assert fnmatch.fnmatchcase(exact_url, _like_pattern_to_fnmatch(raw_pattern))
|
||||||
|
assert fnmatch.fnmatchcase(exact_url, _like_pattern_to_fnmatch(escaped_pattern))
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue