All checks were successful
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m55s
CI Trade-In / changes (pull_request) Successful in 13s
CI / changes (pull_request) Successful in 15s
CI Trade-In / browser-tests (pull_request) Has been skipped
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
avito/imv.py сам собирал curl_cffi AsyncSession с третьей копией document-заголовков (_DOC_HEADERS) — из-за того, что parity-тесты патчили curl_cffi.requests.AsyncSession, а _base импортирует класс на уровне модуля и такой патч до него не долетает. Взят вариант B из issue: в кодовой базе уже принят патч scraper_kit.providers._base.AsyncSession (test_scraper_proxy.py, test_pipeline_browser_routing.py, test_kit_serp_proxy_pool.py), а вариант A (живой lookup в _base) сломал бы эти тесты — атрибута _base.AsyncSession не стало бы. Сессия теперь build_document_session(proxy_url, timeout=25); _DOC_HEADERS удалён (идентичен DOCUMENT_HEADERS, проверено до правки), мёртвый try/except ImportError убран — curl_cffi и так импортируется через _base. Тесты патчат _base.AsyncSession; ассерт с config дополнительно фиксирует impersonate, timeout=25 и заголовки — параметры сессии не изменились. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
138 lines
6.5 KiB
Python
138 lines
6.5 KiB
Python
"""Config-footgun regression for kit `evaluate_via_imv` — issue #2334.
|
||
|
||
Group E1 of the scraper_kit migration epic (#2277 → #2308 → #2334 → final deletion
|
||
step). Legacy `app/services/scrapers/avito_imv.py` was deleted once the kit provider
|
||
(`scraper_kit.providers.avito.imv`) reached golden parity and no runtime caller
|
||
referenced the legacy module anymore — the legacy-vs-kit comparison tests that used
|
||
to live here were removed together with the legacy import.
|
||
|
||
What remains is a standalone kit-only regression, still worth keeping on its own:
|
||
|
||
**Config-footgun regression** — CONFIRMED by recon before issue #2334 started:
|
||
kit `evaluate_via_imv(config: ScraperConfig | None = None, ...)` only reads
|
||
`config.scraper_proxy_url` when `config is not None`; the deleted legacy module
|
||
unconditionally read `settings.scraper_proxy_url` itself. Call the kit fn WITHOUT
|
||
`config=` → own curl_cffi session gets built with NO proxy (`proxies=None`) even
|
||
though a real proxy is configured — a silent behaviour change vs the old code. WITH
|
||
`config=RealScraperConfig()` (the established DI pattern, see
|
||
`app/services/cian_price_history.py` / `tests/test_scraper_kit_pricehistory_session_parity.py::
|
||
test_fetch_detail_own_session_proxy_wiring_parity` for precedent) the proxy is wired
|
||
correctly. NOTE: neither `estimator.py` nor `house_imv_backfill.py` needs this
|
||
config= kwarg wired for `evaluate_via_imv` specifically — these tests exist to prove
|
||
the KIT SIDE stays safe against a "simplify away the config kwarg" regression.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import asyncio
|
||
import os
|
||
from typing import Any
|
||
from unittest.mock import AsyncMock, MagicMock, patch
|
||
|
||
import pytest
|
||
|
||
# app.core.config.Settings() requires DATABASE_URL at import time; app.services.
|
||
# scraper_adapters (RealScraperConfig) imports app.core.config transitively.
|
||
# Mirror tests/test_scraper_kit_pricehistory_session_parity.py.
|
||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
|
||
|
||
from scraper_kit.providers._base import DEFAULT_IMPERSONATE, DOCUMENT_HEADERS
|
||
from scraper_kit.providers.avito.imv import (
|
||
IMVAddressNotFoundError as KitIMVAddressNotFoundError,
|
||
)
|
||
from scraper_kit.providers.avito.imv import evaluate_via_imv as kit_evaluate_via_imv
|
||
|
||
from app.core.config import settings
|
||
from app.services.scraper_adapters import RealScraperConfig
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# Config-footgun regression (#2334 CONFIRMED recon finding)
|
||
# ---------------------------------------------------------------------------
|
||
#
|
||
# evaluate_via_imv's own-curl_cffi-session path (browser_fetcher=None, the default)
|
||
# is the one that reads proxy config. Mirrors
|
||
# tests/test_scraper_proxy.py::test_avito_imv_own_session_receives_proxies (legacy
|
||
# reference, now gone with the legacy module) and
|
||
# tests/test_scraper_kit_pricehistory_session_parity.py::
|
||
# test_fetch_detail_own_session_proxy_wiring_parity (kit config= precedent for a
|
||
# sibling provider, #2306).
|
||
|
||
|
||
def _mock_own_session_flow() -> MagicMock:
|
||
"""Session double that lets warm-up + geocode-A pass, then fails at geocode-B
|
||
(missing geoFieldsHash) — raises IMVAddressNotFoundError AFTER the curl_cffi
|
||
session constructor already ran, which is all these tests need to capture.
|
||
"""
|
||
mock_session = MagicMock()
|
||
mock_warmup_resp = MagicMock(status_code=200)
|
||
mock_session.get = AsyncMock(return_value=mock_warmup_resp) # warm-up + geocode A
|
||
mock_geo_resp = MagicMock(status_code=200)
|
||
mock_geo_resp.json = MagicMock(return_value={"result": {"point": None}}) # no geoFieldsHash
|
||
mock_session.post = AsyncMock(return_value=mock_geo_resp) # geocode B
|
||
mock_session.close = AsyncMock()
|
||
return mock_session
|
||
|
||
|
||
async def _call_kit_own_session(*, config: Any = None) -> None:
|
||
with pytest.raises(KitIMVAddressNotFoundError):
|
||
await kit_evaluate_via_imv(
|
||
address="ул. Тургенева, 4",
|
||
rooms=2,
|
||
area_m2=50.0,
|
||
floor=3,
|
||
floor_at_home=9,
|
||
house_type="panel",
|
||
renovation_type="cosmetic",
|
||
has_balcony=False,
|
||
has_loggia=False,
|
||
config=config,
|
||
)
|
||
|
||
|
||
def test_kit_evaluate_via_imv_without_config_drops_proxy(monkeypatch: pytest.MonkeyPatch) -> None:
|
||
"""CONFIRMED footgun: kit evaluate_via_imv called WITHOUT config= builds its own
|
||
curl_cffi session with proxies=None — even though a real proxy IS configured in
|
||
settings — because `_env = config.scraper_proxy_url if config is not None else None`
|
||
never reads settings when config is absent (no fallback to env/settings inside kit).
|
||
"""
|
||
monkeypatch.setattr(settings, "scraper_proxy_url_env", "http://test-proxy.local:8080")
|
||
mock_session = _mock_own_session_flow()
|
||
captured: dict[str, Any] = {}
|
||
|
||
def _fake_session(*args: Any, **kwargs: Any) -> MagicMock:
|
||
captured.update(kwargs)
|
||
return mock_session
|
||
|
||
with patch("scraper_kit.providers._base.AsyncSession", _fake_session):
|
||
asyncio.run(_call_kit_own_session(config=None))
|
||
|
||
assert captured.get("proxies") is None
|
||
|
||
|
||
def test_kit_evaluate_via_imv_with_config_uses_proxy(monkeypatch: pytest.MonkeyPatch) -> None:
|
||
"""With config=RealScraperConfig() (the established DI pattern for this epic) the
|
||
kit path reads the SAME settings.scraper_proxy_url as legacy used to and wires it
|
||
into the curl_cffi session identically — this is the regression guard: if someone
|
||
later "simplifies away" the config kwarg threading in evaluate_via_imv, this test
|
||
fails on `captured.get("proxies") is None` instead of the expected proxy dict.
|
||
"""
|
||
monkeypatch.setattr(settings, "scraper_proxy_url_env", "http://test-proxy.local:8080")
|
||
mock_session = _mock_own_session_flow()
|
||
captured: dict[str, Any] = {}
|
||
|
||
def _fake_session(*args: Any, **kwargs: Any) -> MagicMock:
|
||
captured.update(kwargs)
|
||
return mock_session
|
||
|
||
with patch("scraper_kit.providers._base.AsyncSession", _fake_session):
|
||
asyncio.run(_call_kit_own_session(config=RealScraperConfig()))
|
||
|
||
assert captured.get("proxies") == {
|
||
"http": "http://test-proxy.local:8080",
|
||
"https": "http://test-proxy.local:8080",
|
||
}
|
||
# #2386: сессия строится общей build_document_session — параметры те же, что были у
|
||
# собственной (impersonate/timeout=25/document-заголовки).
|
||
assert captured.get("impersonate") == DEFAULT_IMPERSONATE
|
||
assert captured.get("timeout") == 25
|
||
assert captured.get("headers") == DOCUMENT_HEADERS
|