331 lines
12 KiB
Python
331 lines
12 KiB
Python
"""#2206: ре-подтверждение живого листинга двигает scraped_at (не только last_seen_at).
|
||
|
||
Корень бага: upsert листингов при ре-подтверждении живым ставил last_seen_at=NOW(),
|
||
обновлял цену/поля, но scraped_at НЕ трогал никогда. Эстиматор фильтрует свежесть
|
||
через `scraped_at > NOW() - INTERVAL '14 days'`, поэтому ре-подтверждённые (но не
|
||
пере-вставленные) листинги выпадали из видимого инвентаря — на проде до фикса
|
||
эстиматору было видно лишь ~45.6% активного инвентаря (CIAN — 5.9%).
|
||
|
||
Фикс: и ON CONFLICT DO UPDATE, и dedup-drift reconcile UPDATE теперь выставляют
|
||
scraped_at = NOW() рядом с last_seen_at = NOW(). Проверяем оба пути в
|
||
`scraper_kit.base` (единственный живой модуль — `app.services.scrapers.base`
|
||
удалён #2397 финальный шаг E, mirror-тесты через legacy убраны) и свойства
|
||
ретро-бэкфилл-миграции 161.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import os
|
||
import re
|
||
from contextlib import contextmanager
|
||
from pathlib import Path
|
||
from typing import Any
|
||
from unittest.mock import MagicMock, patch
|
||
|
||
import psycopg.errors
|
||
import pytest
|
||
from sqlalchemy.exc import IntegrityError
|
||
|
||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost/test_db")
|
||
|
||
from scraper_kit.base import ScrapedLot as KitLot
|
||
from scraper_kit.base import save_listings as kit_save_listings
|
||
|
||
|
||
@contextmanager
|
||
def _nested_ctx() -> Any:
|
||
yield MagicMock()
|
||
|
||
|
||
def _mock_db_update_path(inserted: bool = False, listing_id: int = 42) -> MagicMock:
|
||
"""Session mock для ON CONFLICT DO UPDATE (xmax != 0 → updated)."""
|
||
insert_row = MagicMock()
|
||
insert_row.id = listing_id
|
||
insert_row.inserted = inserted
|
||
|
||
db = MagicMock()
|
||
|
||
def _execute(sql: Any, params: dict[str, Any] | None = None) -> MagicMock:
|
||
s = str(sql)
|
||
res = MagicMock()
|
||
if "SELECT card_hash" in s and "WHERE dedup_hash" in s:
|
||
res.fetchone.return_value = None
|
||
elif "FROM listings_snapshots" in s:
|
||
res.fetchone.return_value = None
|
||
elif "INSERT INTO listings (" in s:
|
||
res.fetchone.return_value = insert_row
|
||
else:
|
||
res.fetchone.return_value = None
|
||
return res
|
||
|
||
db.execute.side_effect = _execute
|
||
db.begin_nested.side_effect = _nested_ctx
|
||
return db
|
||
|
||
|
||
def _mock_db_reconcile(reconcile_id: int = 77) -> MagicMock:
|
||
"""Session mock: INSERT → UniqueViolation → reconcile UPDATE by (source, source_id)."""
|
||
uv_orig = psycopg.errors.UniqueViolation()
|
||
integrity_err = IntegrityError("INSERT INTO listings ...", {}, uv_orig)
|
||
|
||
rec_row = MagicMock()
|
||
rec_row.id = reconcile_id
|
||
|
||
db = MagicMock()
|
||
|
||
def _execute(sql: Any, params: dict[str, Any] | None = None) -> MagicMock:
|
||
s = str(sql)
|
||
res = MagicMock()
|
||
if "SELECT card_hash" in s and "WHERE dedup_hash" in s:
|
||
res.fetchone.return_value = None
|
||
elif "FROM listings_snapshots" in s:
|
||
res.fetchone.return_value = None
|
||
elif "INSERT INTO listings (" in s:
|
||
raise integrity_err
|
||
elif "UPDATE listings" in s and "SET dedup_hash" in s:
|
||
res.fetchone.return_value = rec_row
|
||
else:
|
||
res.fetchone.return_value = None
|
||
return res
|
||
|
||
db.execute.side_effect = _execute
|
||
|
||
@contextmanager
|
||
def _nested() -> Any:
|
||
try:
|
||
yield MagicMock()
|
||
except IntegrityError:
|
||
raise
|
||
|
||
db.begin_nested.side_effect = _nested
|
||
return db
|
||
|
||
|
||
def _find_sql(db: MagicMock, needle: str) -> str:
|
||
for call in db.execute.call_args_list:
|
||
sql = str(call.args[0])
|
||
if needle in sql:
|
||
return sql
|
||
raise AssertionError(f"SQL containing {needle!r} not found")
|
||
|
||
|
||
# ── scraper_kit.base (единственный живой путь — legacy app.services.scrapers.base
|
||
# удалён #2397 финальный шаг E) ─────────────────────────────────────────────────
|
||
|
||
|
||
def _kit_matcher() -> MagicMock:
|
||
matcher = MagicMock()
|
||
matcher.match_or_create_house.return_value = (101, 1.0, "new")
|
||
matcher.upsert_listing_source.return_value = None
|
||
return matcher
|
||
|
||
|
||
def test_kit_on_conflict_bumps_scraped_at() -> None:
|
||
"""kit ON CONFLICT DO UPDATE двигает scraped_at = NOW()."""
|
||
db = _mock_db_update_path(inserted=False)
|
||
lot = KitLot(
|
||
source="cian",
|
||
source_url="https://ekb.cian.ru/sale/flat/1/",
|
||
source_id="1",
|
||
price_rub=5_000_000,
|
||
)
|
||
|
||
with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None):
|
||
inserted, updated = kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66)
|
||
|
||
assert (inserted, updated) == (0, 1)
|
||
sql = _find_sql(db, "INSERT INTO listings (")
|
||
assert "scraped_at = NOW()" in sql
|
||
assert "last_seen_at = NOW()" in sql
|
||
|
||
|
||
def test_kit_reconcile_bumps_scraped_at() -> None:
|
||
"""kit dedup-drift reconcile UPDATE двигает scraped_at = NOW()."""
|
||
db = _mock_db_reconcile(reconcile_id=88)
|
||
lot = KitLot(
|
||
source="avito",
|
||
source_url="https://www.avito.ru/ekaterinburg/kvartiry/1-k_1",
|
||
source_id="7960764619",
|
||
price_rub=5_000_000,
|
||
)
|
||
|
||
with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None):
|
||
kit_save_listings(db, [lot], matcher=_kit_matcher(), region_code=66)
|
||
|
||
sql = _find_sql(db, "SET dedup_hash")
|
||
assert "scraped_at = NOW()" in sql
|
||
assert "last_seen_at = NOW()" in sql
|
||
|
||
|
||
# ── DomClick house SKU (PR1/2): property_type/land_area_m2 + house-match short-circuit
|
||
# on a real Postgres. Real DB needed (not a MagicMock) because the assertion is about
|
||
# actual row state (house_id_fk IS NULL) and a genuine listing_sources row — a mock
|
||
# can't prove save_listings' SAVEPOINT/hook wiring actually persists both. ────────────
|
||
|
||
|
||
def _live_session() -> Any | None:
|
||
"""Return a SQLAlchemy Session if a non-placeholder Postgres is reachable, else None.
|
||
|
||
Same self-skipping pattern as test_house_dedup_merge.py's `_live_session()`.
|
||
"""
|
||
try:
|
||
from sqlalchemy import create_engine
|
||
from sqlalchemy.orm import sessionmaker
|
||
|
||
dsn = os.environ.get("TEST_DATABASE_URL") or os.environ.get("DATABASE_URL", "")
|
||
if not dsn or "localhost:5432/test" in dsn:
|
||
return None
|
||
engine = create_engine(dsn, future=True)
|
||
conn = engine.connect()
|
||
from sqlalchemy import text as _t
|
||
|
||
conn.execute(_t("SELECT 1"))
|
||
conn.close()
|
||
return sessionmaker(bind=engine, future=True)()
|
||
except Exception:
|
||
return None
|
||
|
||
|
||
class _HouseShortCircuitMatcher:
|
||
"""Matcher stub for the property_type='house' short-circuit test.
|
||
|
||
match_or_create_house() MUST NOT be called for a house lot — asserting that is
|
||
the whole point of the test, so this raises rather than silently no-op'ing.
|
||
upsert_listing_source() delegates to the real `app.services.matching` function
|
||
against the live DB, so the test can assert a genuine listing_sources row exists
|
||
(independent of house linkage, per the analyst plan).
|
||
"""
|
||
|
||
def match_or_create_house(self, *args: Any, **kwargs: Any) -> Any:
|
||
raise AssertionError(
|
||
"match_or_create_house MUST NOT be called for property_type='house' "
|
||
"(houses table is an apartment-BUILDING cache, not a standalone house)"
|
||
)
|
||
|
||
def upsert_listing_source(self, db: Any, **kwargs: Any) -> None:
|
||
from app.services.matching import upsert_listing_source as _real_upsert
|
||
|
||
_real_upsert(db, **kwargs)
|
||
|
||
|
||
@pytest.mark.skipif(_live_session() is None, reason="no reachable Postgres test DB")
|
||
def test_kit_house_property_type_skips_house_match_real_db() -> None:
|
||
"""property_type='house' persists land_area_m2, leaves house_id_fk NULL, and
|
||
still registers the listing in listing_sources — WITHOUT ever calling
|
||
match_or_create_house() (DomClick house SKU, PR1/2)."""
|
||
from sqlalchemy import text as _t
|
||
|
||
db = _live_session()
|
||
assert db is not None
|
||
try:
|
||
lot = KitLot(
|
||
source="domclick",
|
||
source_url="https://ekaterinburg.domclick.ru/card/sale__house__TEST-HOUSE-2206",
|
||
source_id="TEST-HOUSE-2206",
|
||
address="тестовый дом, ул. Домиковая, 1",
|
||
lat=56.84,
|
||
lon=60.60,
|
||
area_m2=180.0,
|
||
price_rub=12_000_000,
|
||
property_type="house",
|
||
land_area_m2=650.0,
|
||
)
|
||
|
||
with patch("scraper_kit.base.upsert_listing_snapshot", return_value=None):
|
||
inserted, updated = kit_save_listings(
|
||
db, [lot], matcher=_HouseShortCircuitMatcher(), region_code=66
|
||
)
|
||
assert (inserted, updated) == (1, 0)
|
||
|
||
row = db.execute(
|
||
_t(
|
||
"SELECT property_type, land_area_m2, house_id_fk FROM listings "
|
||
"WHERE source = 'domclick' AND source_id = 'TEST-HOUSE-2206'"
|
||
)
|
||
).fetchone()
|
||
assert row is not None
|
||
assert row.property_type == "house"
|
||
assert float(row.land_area_m2) == 650.0
|
||
assert row.house_id_fk is None
|
||
|
||
ls_row = db.execute(
|
||
_t(
|
||
"SELECT listing_id FROM listing_sources "
|
||
"WHERE ext_source = 'domclick' AND ext_id = 'TEST-HOUSE-2206'"
|
||
)
|
||
).fetchone()
|
||
assert ls_row is not None
|
||
finally:
|
||
db.rollback()
|
||
db.execute(
|
||
_t(
|
||
"DELETE FROM listing_sources WHERE ext_source = 'domclick' "
|
||
"AND ext_id = 'TEST-HOUSE-2206'"
|
||
)
|
||
)
|
||
db.execute(
|
||
_t(
|
||
"DELETE FROM listings WHERE source = 'domclick' "
|
||
"AND source_id = 'TEST-HOUSE-2206'"
|
||
)
|
||
)
|
||
db.commit()
|
||
db.close()
|
||
|
||
|
||
# ── Migration 161: retro-backfill scraped_at ──────────────────────────────────
|
||
|
||
_SQL_DIR = Path(__file__).resolve().parents[1] / "data" / "sql"
|
||
_MIGRATION_161 = _SQL_DIR / "161_backfill_scraped_at_active_recent.sql"
|
||
|
||
|
||
def test_migration_161_exists() -> None:
|
||
assert _MIGRATION_161.is_file(), f"missing migration: {_MIGRATION_161}"
|
||
|
||
|
||
def test_migration_161_is_transactional() -> None:
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert "BEGIN;" in sql
|
||
assert "COMMIT;" in sql
|
||
|
||
|
||
def test_migration_161_sets_scraped_at_from_last_seen_at() -> None:
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert "SET scraped_at = last_seen_at" in sql
|
||
|
||
|
||
def test_migration_161_is_idempotent_by_construction() -> None:
|
||
"""Идемпотентность: WHERE last_seen_at > scraped_at → после первого прогона 0 строк."""
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert "last_seen_at > scraped_at" in sql
|
||
|
||
|
||
def test_migration_161_only_active_rows() -> None:
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert "is_active" in sql
|
||
|
||
|
||
def test_migration_161_scoped_to_scraped_sources() -> None:
|
||
"""avito/cian/yandex включены; domklik/n1 — НЕТ (поломанный bulk-touch last_seen_at)."""
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert "IN ('avito', 'cian', 'yandex')" in sql
|
||
|
||
|
||
def test_migration_161_excludes_domklik() -> None:
|
||
"""domklik намеренно исключён: last_seen_at bulk-touch'ится ежедневно → ложное освежение."""
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
# domklik не в списке источников UPDATE (упоминание только в комментарии-обосновании)
|
||
assert "'domklik'" not in sql
|
||
assert "domklik" in sql # обоснование исключения зафиксировано в комментарии
|
||
|
||
|
||
def test_migration_161_no_psycopg_trap() -> None:
|
||
"""psycopg v3: никаких :param::type."""
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert not re.search(r":\w+::", sql)
|
||
|
||
|
||
def test_migration_161_non_destructive_update_only() -> None:
|
||
sql = _MIGRATION_161.read_text("utf-8")
|
||
assert "UPDATE listings" in sql
|
||
assert "DELETE" not in sql.upper()
|