From 7ca4d35e8896c0fae2e9713c969a3a3ee43c8716 Mon Sep 17 00:00:00 2001 From: lekss361 Date: Wed, 13 May 2026 09:11:41 +0300 Subject: [PATCH] =?UTF-8?q?fix(site-finder):=20rename=20=5Fmake=5Fempty=5F?= =?UTF-8?q?result=20=E2=86=92=20make=5Fempty=5Fresult=20(public)=20per=20P?= =?UTF-8?q?R=20#116=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M1 residual fix: parcels.py exception path использовал EMPTY_DUMP_RESULT singleton ref вместо factory. Сейчас readonly access, но нарушает documented invariant модуля. Rename `_make_empty_result` → `make_empty_result` (public API), import в parcels.py, использовать в try/except fallback. Каждый request получает независимый dict — никаких shared references. M4 (Redis SETNX dedup) + M5 (test fragility) — deferred per review, documented в code/issue. Acceptable trade-offs: - M4: UPSERT idempotency делает данные safe; burst-duplicate task'и тратят WAF traffic впустую но не повреждают данные. - M5: docstring contractually describes positional-call order. 17/17 tests pass. ruff/format clean. Per auto-review on aef8308. --- backend/app/api/v1/parcels.py | 8 ++++++-- .../site_finder/quarter_dump_lookup.py | 10 +++++----- backend/tests/test_quarter_dump_lookup.py | 20 +++++++++---------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/backend/app/api/v1/parcels.py b/backend/app/api/v1/parcels.py index 834565c2..60d0ee4d 100644 --- a/backend/app/api/v1/parcels.py +++ b/backend/app/api/v1/parcels.py @@ -22,7 +22,10 @@ from app.services.site_finder.cadastre_fetch import ( from app.services.site_finder.cadastre_fetch import ( fetch_status as _fetch_status, ) -from app.services.site_finder.quarter_dump_lookup import EMPTY_DUMP_RESULT, get_quarter_dump_data +from app.services.site_finder.quarter_dump_lookup import ( + get_quarter_dump_data, + make_empty_result, +) logger = logging.getLogger(__name__) @@ -1535,7 +1538,8 @@ def analyze_parcel( nspd_dump_data = get_quarter_dump_data(db, cad_num, geom_wkt) except Exception as e: logger.warning("nspd quarter dump lookup failed for %s: %s", cad_num, e) - nspd_dump_data = EMPTY_DUMP_RESULT + # Independent dict per request — never mutate module singleton. + nspd_dump_data = make_empty_result() # 10) Market trend — динамика цен ДДУ в радиусе 3 км за 6 vs предыдущие 6 месяцев market_trend: dict[str, Any] | None = None diff --git a/backend/app/services/site_finder/quarter_dump_lookup.py b/backend/app/services/site_finder/quarter_dump_lookup.py index 6701cf9b..72bd03ab 100644 --- a/backend/app/services/site_finder/quarter_dump_lookup.py +++ b/backend/app/services/site_finder/quarter_dump_lookup.py @@ -30,7 +30,7 @@ _DUMP_MAX_AGE_DAYS = 180 _ENGINEERING_RADIUS_M = 200 # Sentinel для isinstance-проверок и read-only fallback в parcels.py try/except. -# НИКОГДА не мутировать — использовать _make_empty_result() для новых dict. +# НИКОГДА не мутировать — использовать make_empty_result() для новых dict. EMPTY_DUMP_RESULT: dict[str, Any] = { "nspd_zoning": None, "nspd_zouit_overlaps": [], @@ -45,7 +45,7 @@ EMPTY_DUMP_RESULT: dict[str, Any] = { } -def _make_empty_result( +def make_empty_result( *, fetched_at_utc: str | None = None, stale: bool = False, @@ -109,7 +109,7 @@ def get_quarter_dump_data( quarter = derive_quarter_cad(cad_num) if quarter is None: logger.warning("quarter_dump_lookup: cannot derive quarter from cad=%s", cad_num) - return _make_empty_result() + return make_empty_result() # Читаем строку дампа из БД. Денормализованные счётчики слоёв используются # для early-exit в spatial helpers (M2 mitigation). @@ -136,7 +136,7 @@ def get_quarter_dump_data( if row is None: # Дампа нет — ставим harvest в очередь harvest_triggered = _trigger_harvest(quarter) - return _make_empty_result(harvest_triggered=harvest_triggered) + return make_empty_result(harvest_triggered=harvest_triggered) fetched_at: datetime = row[1] # Убедимся что timezone-aware для корректного сравнения @@ -155,7 +155,7 @@ def get_quarter_dump_data( # Устаревший или с ошибкой — триггерим повторный harvest if is_stale or has_error: harvest_triggered = _trigger_harvest(quarter) - return _make_empty_result( + return make_empty_result( fetched_at_utc=fetched_at.isoformat(), stale=is_stale, harvest_triggered=harvest_triggered, diff --git a/backend/tests/test_quarter_dump_lookup.py b/backend/tests/test_quarter_dump_lookup.py index c66df3c1..d5c09d23 100644 --- a/backend/tests/test_quarter_dump_lookup.py +++ b/backend/tests/test_quarter_dump_lookup.py @@ -7,7 +7,7 @@ Mock-based — без реальной БД и Celery. Проверяет: - get_quarter_dump_data: свежий дамп с territorial_zones → nspd_zoning populated - get_quarter_dump_data: свежий дамп с 2 zouit → nspd_zouit_overlaps length=2 - get_quarter_dump_data: parcel_wkt=None → metadata only, no spatial queries -- _make_empty_result: mutation-safety — каждый вызов возвращает независимый dict +- make_empty_result: mutation-safety — каждый вызов возвращает независимый dict - early-exit: layer_counts == 0 → spatial query не вызывается """ @@ -19,9 +19,9 @@ from unittest.mock import MagicMock, patch from app.services.site_finder.quarter_dump_lookup import ( EMPTY_DUMP_RESULT, - _make_empty_result, derive_quarter_cad, get_quarter_dump_data, + make_empty_result, ) # ── derive_quarter_cad ──────────────────────────────────────────────────────── @@ -303,17 +303,17 @@ def test_get_quarter_dump_includes_nspd_fields() -> None: assert "total_features" in dump -# ── _make_empty_result — mutation safety ───────────────────────────────────── +# ── make_empty_result — mutation safety ───────────────────────────────────── -def test_make_empty_result_returns_independent_copies() -> None: - """Каждый вызов _make_empty_result() возвращает независимый dict. +def testmake_empty_result_returns_independent_copies() -> None: + """Каждый вызов make_empty_result() возвращает независимый dict. Мутация nspd_dump в одном результате не должна влиять на другой и на модульный sentinel EMPTY_DUMP_RESULT. """ - a = _make_empty_result() - b = _make_empty_result() + a = make_empty_result() + b = make_empty_result() # Вложенный dict — независимый объект assert a["nspd_dump"] is not b["nspd_dump"] @@ -329,9 +329,9 @@ def test_make_empty_result_returns_independent_copies() -> None: assert EMPTY_DUMP_RESULT["nspd_zouit_overlaps"] == [] -def test_make_empty_result_overrides() -> None: - """_make_empty_result принимает override-аргументы.""" - r = _make_empty_result(harvest_triggered=True, stale=True, total_features=42) +def testmake_empty_result_overrides() -> None: + """make_empty_result принимает override-аргументы.""" + r = make_empty_result(harvest_triggered=True, stale=True, total_features=42) assert r["nspd_dump"]["harvest_triggered"] is True assert r["nspd_dump"]["stale"] is True assert r["nspd_dump"]["total_features"] == 42