refactor(tradein/tasks): newbuilding_enrich_backfill на kit cian.newbuilding + config= (#2397 Part D3)

tradein-mvp/backend/app/tasks/newbuilding_enrich_backfill.py

@@ -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

tradein-mvp/backend/tests/tasks/test_newbuilding_enrich_backfill.py

@@ -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,
         ),
     ):

tradein-mvp/backend/tests/test_scraper_kit_newbuilding_endpoint.py

@@ -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