refactor(tradein/tasks): newbuilding_enrich_backfill на kit cian.newbuilding + config= (#2397 Part D3) #2415
3 changed files with 105 additions and 32 deletions
|
|
@ -356,21 +356,26 @@ async def backfill_newbuilding_enrichment(
|
|||
# when the task actually runs — mirrors cian_history_backfill's lazy import and keeps
|
||||
# unit tests able to patch fetch_newbuilding cheaply.
|
||||
#
|
||||
# NOT migrated to scraper_kit (issue #2310, Group C): kit's
|
||||
# scraper_kit.providers.cian.newbuilding.fetch_newbuilding() constructs
|
||||
# BrowserFetcher(source="cian") WITHOUT the now-mandatory endpoint= kwarg —
|
||||
# every call raises TypeError, with no caller-side fix possible (the kit
|
||||
# provider function doesn't expose a config/endpoint hook at all). This is
|
||||
# issue #2322, verified STILL OPEN by reading the actual provider source
|
||||
# (not just the issue's open/closed status) at the time of this migration.
|
||||
# Left on legacy entirely, exactly like Group A (#2305) did for the same
|
||||
# bug in admin.py's debug endpoints.
|
||||
from app.services.scrapers.cian_newbuilding import (
|
||||
# Migrated to scraper_kit (#2397 Part D3): issue #2322 (BrowserFetcher(source="cian")
|
||||
# constructed WITHOUT the mandatory endpoint= kwarg) was fixed for this provider —
|
||||
# fetch_newbuilding() / resolve_cian_zhk_url_via_search() now accept an explicit
|
||||
# `config: ScraperConfig | None` kwarg (see test_scraper_kit_newbuilding_endpoint.py).
|
||||
# config=RealScraperConfig() is REQUIRED below for both calls:
|
||||
# - fetch_newbuilding: without it, BrowserFetcher.__aenter__ raises AssertionError.
|
||||
# - resolve_cian_zhk_url_via_search: this one goes through curl_cffi (not
|
||||
# BrowserFetcher), so a missing config does NOT crash — it silently drops
|
||||
# cian_proxy_url and falls back to a direct (non-proxied) connection. Both call
|
||||
# sites below pass config= explicitly to avoid that silent footgun.
|
||||
from scraper_kit.providers.cian.newbuilding import (
|
||||
fetch_newbuilding,
|
||||
resolve_cian_zhk_url_via_search,
|
||||
save_newbuilding_enrichment,
|
||||
)
|
||||
|
||||
from app.services.scraper_adapters import RealScraperConfig
|
||||
|
||||
scraper_config = RealScraperConfig()
|
||||
|
||||
result = NewbuildingEnrichBackfillResult()
|
||||
t0 = time.time()
|
||||
delay = request_delay_sec if request_delay_sec is not None else get_scraper_delay("cian")
|
||||
|
|
@ -444,7 +449,7 @@ async def backfill_newbuilding_enrichment(
|
|||
continue
|
||||
|
||||
try:
|
||||
resolved = await resolve_cian_zhk_url_via_search(nb_id)
|
||||
resolved = await resolve_cian_zhk_url_via_search(nb_id, config=scraper_config)
|
||||
except Exception as exc: # defensive — resolver already catches internally
|
||||
logger.warning(
|
||||
"zhk-url resolve raised house_id=%s nb_id=%s: %s", house_id, nb_id, exc
|
||||
|
|
@ -491,7 +496,7 @@ async def backfill_newbuilding_enrichment(
|
|||
# ── Fetch (network; anti-bot surface) ──────────────────────────────
|
||||
enrichment = None
|
||||
try:
|
||||
enrichment = await fetch_newbuilding(zhk_url)
|
||||
enrichment = await fetch_newbuilding(zhk_url, config=scraper_config)
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"newbuilding fetch failed house_id=%s url=%s: %s", house_id, zhk_url, exc
|
||||
|
|
|
|||
|
|
@ -19,8 +19,8 @@ _wp_mock = MagicMock()
|
|||
sys.modules.setdefault("weasyprint", _wp_mock)
|
||||
|
||||
import pytest # noqa: E402
|
||||
from scraper_kit.providers.cian.newbuilding import NewbuildingEnrichment # noqa: E402
|
||||
|
||||
from app.services.scrapers.cian_newbuilding import NewbuildingEnrichment # noqa: E402
|
||||
from app.tasks.newbuilding_enrich_backfill import ( # noqa: E402
|
||||
NewbuildingEnrichBackfillResult,
|
||||
_save_cian_reviews,
|
||||
|
|
@ -233,7 +233,7 @@ def test_save_cian_reviews_clamps_bad_score() -> None:
|
|||
async def test_backfill_dry_run_counts_only() -> None:
|
||||
db = FakeDB(pending_rows=[{"house_id": 1, "cian_zhk_url": "https://zhk-a.cian.ru/"}])
|
||||
with patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding", new_callable=AsyncMock
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding", new_callable=AsyncMock
|
||||
) as mock_fetch:
|
||||
result = await backfill_newbuilding_enrichment(db, limit=5, dry_run=True)
|
||||
mock_fetch.assert_not_called()
|
||||
|
|
@ -261,11 +261,11 @@ async def test_backfill_populates_all_three_tables() -> None:
|
|||
|
||||
with (
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding",
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding",
|
||||
side_effect=_fetch,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
@ -293,12 +293,12 @@ async def test_backfill_force_rerun_no_dup() -> None:
|
|||
|
||||
with (
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding",
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding",
|
||||
new_callable=AsyncMock,
|
||||
return_value=enr,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
@ -335,9 +335,9 @@ async def test_backfill_default_rerun_skips_and_no_reliability_dup() -> None:
|
|||
return enr
|
||||
|
||||
with (
|
||||
patch("app.services.scrapers.cian_newbuilding.fetch_newbuilding", side_effect=_fetch),
|
||||
patch("scraper_kit.providers.cian.newbuilding.fetch_newbuilding", side_effect=_fetch),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
@ -370,11 +370,11 @@ async def test_backfill_fetch_failure_does_not_abort_batch() -> None:
|
|||
|
||||
with (
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding",
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding",
|
||||
side_effect=_fetch,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
@ -411,15 +411,15 @@ async def test_backfill_resolves_zhk_url_from_ext_id_then_enriches() -> None:
|
|||
|
||||
with (
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.resolve_cian_zhk_url_via_search",
|
||||
"scraper_kit.providers.cian.newbuilding.resolve_cian_zhk_url_via_search",
|
||||
side_effect=_resolve,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding",
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding",
|
||||
side_effect=_fetch,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
@ -450,16 +450,16 @@ async def test_backfill_unresolved_ext_id_counts_fail_and_skips_fetch() -> None:
|
|||
|
||||
with (
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.resolve_cian_zhk_url_via_search",
|
||||
"scraper_kit.providers.cian.newbuilding.resolve_cian_zhk_url_via_search",
|
||||
new_callable=AsyncMock,
|
||||
return_value=None,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding",
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding",
|
||||
side_effect=_fetch,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
@ -494,15 +494,15 @@ async def test_backfill_resolve_then_idempotent_rerun_no_reresolve() -> None:
|
|||
|
||||
with (
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.resolve_cian_zhk_url_via_search",
|
||||
"scraper_kit.providers.cian.newbuilding.resolve_cian_zhk_url_via_search",
|
||||
side_effect=_resolve,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.fetch_newbuilding",
|
||||
"scraper_kit.providers.cian.newbuilding.fetch_newbuilding",
|
||||
side_effect=_fetch,
|
||||
),
|
||||
patch(
|
||||
"app.services.scrapers.cian_newbuilding.save_newbuilding_enrichment",
|
||||
"scraper_kit.providers.cian.newbuilding.save_newbuilding_enrichment",
|
||||
side_effect=_fake_save_newbuilding_enrichment,
|
||||
),
|
||||
):
|
||||
|
|
|
|||
|
|
@ -14,6 +14,15 @@ These tests prove `config.browser_http_endpoint` is actually threaded through to
|
|||
`BrowserFetcher(...)` constructor call args — not merely that the function "doesn't
|
||||
crash" on a happy path. `BrowserFetcher` itself is replaced by a spy so the constructor
|
||||
call is directly inspectable.
|
||||
|
||||
Also covers a SIBLING but importantly DIFFERENT footgun (#2397 Part D3, migrating
|
||||
`app.tasks.newbuilding_enrich_backfill` onto this kit provider):
|
||||
`resolve_cian_zhk_url_via_search` does NOT use `BrowserFetcher` at all — it goes
|
||||
through `build_curl_cffi_session` — so a missing `config=` does NOT raise like the
|
||||
`BrowserFetcher.__aenter__` assertion above. It silently degrades to a direct
|
||||
(non-proxied) connection (`cian_proxy_url` dropped). That is much harder to notice in
|
||||
prod (no crash, just quietly unproxied traffic that anti-bot can throttle/block), so a
|
||||
caller that forgets `config=` here gets no signal at all.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -22,13 +31,17 @@ from types import SimpleNamespace
|
|||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
from scraper_kit.providers.cian.newbuilding import fetch_newbuilding
|
||||
from scraper_kit.providers.cian.newbuilding import (
|
||||
fetch_newbuilding,
|
||||
resolve_cian_zhk_url_via_search,
|
||||
)
|
||||
from scraper_kit.providers.yandex.newbuilding import (
|
||||
YandexNewbuildingScraper,
|
||||
resolve_yandex_jk_slug,
|
||||
)
|
||||
|
||||
_TEST_ENDPOINT = "http://test-tradein-browser:9009/fetch"
|
||||
_TEST_PROXY = "http://mobile-proxy.example:8080"
|
||||
|
||||
|
||||
def _spy_browser_fetcher(html: str) -> MagicMock:
|
||||
|
|
@ -138,3 +151,58 @@ async def test_resolve_yandex_jk_slug_endpoint_none_without_config(monkeypatch):
|
|||
_, kwargs = spy.call_args
|
||||
assert kwargs.get("source") == "yandex"
|
||||
assert kwargs.get("endpoint") is None
|
||||
|
||||
|
||||
# ── cian.newbuilding.resolve_cian_zhk_url_via_search ─────────────────────────
|
||||
# NOTE (#2397 Part D3): this provider does NOT use BrowserFetcher (unlike
|
||||
# fetch_newbuilding above) — it builds its own curl_cffi session via
|
||||
# build_curl_cffi_session(proxy_url=...). Unlike the BrowserFetcher assertion, a
|
||||
# missing config= here does NOT crash — it silently drops the proxy and falls back to
|
||||
# a direct (non-proxied) connection. These tests prove the threading in BOTH directions
|
||||
# so a caller forgetting config= has a red test instead of quiet unproxied traffic.
|
||||
|
||||
_SERP_HTML = (
|
||||
'<h1 data-name="Title">Купить квартиру в '
|
||||
'<a href="https://zhk-parkovyy-kvartal-ekb-i.cian.ru/">ЖК «Парковый квартал»</a></h1>'
|
||||
)
|
||||
|
||||
|
||||
def _spy_curl_session(html: str) -> MagicMock:
|
||||
"""build_curl_cffi_session spy: records the (proxy_url=...) call kwargs and returns
|
||||
a fake curl_cffi AsyncSession whose .get() resolves to a 200 response carrying `html`."""
|
||||
mock_resp = SimpleNamespace(status_code=200, text=html)
|
||||
mock_session = MagicMock()
|
||||
mock_session.get = AsyncMock(return_value=mock_resp)
|
||||
mock_session.close = AsyncMock()
|
||||
return MagicMock(return_value=mock_session)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_cian_zhk_url_via_search_passes_proxy_from_config(monkeypatch):
|
||||
"""config.cian_proxy_url должен попасть в build_curl_cffi_session(proxy_url=...)."""
|
||||
spy = _spy_curl_session(_SERP_HTML)
|
||||
monkeypatch.setattr("scraper_kit.providers.cian.newbuilding.build_curl_cffi_session", spy)
|
||||
|
||||
config = SimpleNamespace(cian_proxy_url=_TEST_PROXY)
|
||||
url = await resolve_cian_zhk_url_via_search(48853, config=config)
|
||||
|
||||
assert url == "https://zhk-parkovyy-kvartal-ekb-i.cian.ru"
|
||||
spy.assert_called_once()
|
||||
_, kwargs = spy.call_args
|
||||
assert kwargs.get("proxy_url") == _TEST_PROXY
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resolve_cian_zhk_url_via_search_proxy_none_without_config(monkeypatch):
|
||||
"""Без config= (SILENT footgun): вызов НЕ падает (в отличие от BrowserFetcher
|
||||
assertion выше в этом файле), но proxy_url молча становится None — резолвер уходит
|
||||
на прямое (непроксированное) соединение вместо настроенного мобильного прокси."""
|
||||
spy = _spy_curl_session(_SERP_HTML)
|
||||
monkeypatch.setattr("scraper_kit.providers.cian.newbuilding.build_curl_cffi_session", spy)
|
||||
|
||||
url = await resolve_cian_zhk_url_via_search(48853)
|
||||
|
||||
assert url == "https://zhk-parkovyy-kvartal-ekb-i.cian.ru" # "works" — silently unproxied
|
||||
spy.assert_called_once()
|
||||
_, kwargs = spy.call_args
|
||||
assert kwargs.get("proxy_url") is None
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue