All checks were successful
Deploy Trade-In / changes (push) Successful in 10s
Deploy Trade-In / build-browser (push) Has been skipped
Deploy Trade-In / build-frontend (push) Successful in 28s
Deploy Trade-In / test (push) Successful in 1m24s
Deploy Trade-In / build-backend (push) Successful in 53s
Deploy Trade-In / deploy (push) Successful in 51s
337 lines
13 KiB
Python
337 lines
13 KiB
Python
"""Tests for issue #849 Parts 2 and 3.
|
|
|
|
Part 2: same normalized_address → exactly 1 house and 1 alias key.
|
|
Part 3: batch save_listings with one failing lot → siblings persist,
|
|
no orphan SAVEPOINT / no whole-batch rollback.
|
|
|
|
All tests use mock DB sessions — no real Postgres required.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from contextlib import contextmanager
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
from app.services.matching.houses import _insert_alias, match_or_create_house
|
|
from app.services.matching.normalize import address_fingerprint, normalize_address
|
|
from app.services.scrapers.base import ScrapedLot, save_listings
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Helpers
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _make_db(rows: list[dict | None]) -> MagicMock:
|
|
"""Build a Session mock returning rows sequentially for each execute() call."""
|
|
db = MagicMock()
|
|
side_effects = []
|
|
for row in rows:
|
|
result = MagicMock()
|
|
mappings = MagicMock()
|
|
if row is None:
|
|
mappings.first.return_value = None
|
|
mappings.one.return_value = None
|
|
else:
|
|
mappings.first.return_value = row
|
|
mappings.one.return_value = row
|
|
result.mappings.return_value = mappings
|
|
side_effects.append(result)
|
|
db.execute.side_effect = side_effects
|
|
return db
|
|
|
|
|
|
def _mock_save_db(inserted: bool = True, listing_id: int = 42) -> MagicMock:
|
|
"""Mock Session for save_listings: INSERT RETURNING + SAVEPOINTs as no-op."""
|
|
row = MagicMock()
|
|
row.id = listing_id
|
|
row.inserted = inserted
|
|
|
|
result = MagicMock()
|
|
result.fetchone.return_value = row
|
|
|
|
db = MagicMock()
|
|
db.execute.return_value = result
|
|
|
|
@contextmanager
|
|
def _nested():
|
|
yield MagicMock()
|
|
|
|
db.begin_nested.side_effect = _nested
|
|
return db
|
|
|
|
|
|
def _base_lot(**overrides) -> ScrapedLot:
|
|
defaults = {
|
|
"source": "avito",
|
|
"source_url": "https://www.avito.ru/ekaterinburg/kvartiry/1-k_11111",
|
|
"source_id": "11111",
|
|
"price_rub": 4_000_000,
|
|
}
|
|
defaults.update(overrides)
|
|
return ScrapedLot(**defaults)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Part 2: fingerprint↔alias sync
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestFingerprintAliasSync:
|
|
"""Two listings with the same address → exactly 1 house and 1 alias key.
|
|
|
|
Scenario: two scrapers submit the same street address but with coords that
|
|
differ beyond the 4-decimal rounding boundary — they produce different
|
|
fingerprints. The matcher must still resolve to the same canonical house
|
|
and NOT create a second alias for the same normalized_address.
|
|
"""
|
|
|
|
def test_same_normalized_address_resolves_to_same_house_via_tier2b(self):
|
|
"""First call misses all tiers → creates house + alias (fp=fp1).
|
|
Second call with same address but different coords:
|
|
- Tier 2a: fp2 ≠ fp1 → miss
|
|
- Tier 2b: normalized_address hit → returns existing house_id
|
|
Result: same house_id, NOT a new house row.
|
|
"""
|
|
address = "Екатеринбург, ул. Ленина, 5"
|
|
lat1, lon1 = 56.838000, 60.594600 # yields fp1
|
|
lat2, lon2 = 56.839100, 60.594600 # 4dp rounds to 56.8391 ≠ 56.8380 → fp2 ≠ fp1
|
|
|
|
fp1 = address_fingerprint(address, lat1, lon1)
|
|
fp2 = address_fingerprint(address, lat2, lon2)
|
|
assert fp1 != fp2, "precondition: coords produce different fingerprints"
|
|
|
|
# ── First call: all tiers miss → INSERT new house ───────────────────
|
|
db1 = _make_db(
|
|
[
|
|
None, # pg_advisory_xact_lock
|
|
None, # house_sources miss (Tier 1)
|
|
None, # fingerprint miss (Tier 2a)
|
|
None, # normalized_address miss (Tier 2b) — first time
|
|
None, # geo miss (Tier 3; no lat/lon provided here)
|
|
{"id": 7}, # INSERT RETURNING id
|
|
None, # _upsert_house_source
|
|
None, # _insert_alias
|
|
]
|
|
)
|
|
house_id_1, _, method_1 = match_or_create_house(
|
|
db1,
|
|
"avito",
|
|
"ext-001",
|
|
address=address,
|
|
lat=lat1,
|
|
lon=lon1,
|
|
)
|
|
assert method_1 == "new"
|
|
assert house_id_1 == 7
|
|
|
|
# ── Second call (same address, different coords) ────────────────────
|
|
# Tier 2a misses (fp2 ≠ stored fp1), Tier 2b hits on normalized_address.
|
|
db2 = _make_db(
|
|
[
|
|
None, # pg_advisory_xact_lock
|
|
None, # house_sources miss (Tier 1)
|
|
None, # fingerprint miss (Tier 2a — fp2 not registered yet)
|
|
{"house_id": 7}, # normalized_address hit (Tier 2b)
|
|
None, # _upsert_house_source
|
|
None, # _insert_alias (register fp2)
|
|
]
|
|
)
|
|
house_id_2, _, method_2 = match_or_create_house(
|
|
db2,
|
|
"cian",
|
|
"ext-002",
|
|
address=address,
|
|
lat=lat2,
|
|
lon=lon2,
|
|
)
|
|
assert (
|
|
method_2 == "fingerprint"
|
|
), f"Expected 'fingerprint' (Tier 2b match), got {method_2!r}"
|
|
assert (
|
|
house_id_2 == 7
|
|
), f"Expected same house_id=7, got {house_id_2} — second scraper created a duplicate"
|
|
|
|
def test_same_normalized_address_tier2a_hits_directly_when_fp_matches(self):
|
|
"""When the fingerprint IS already registered (same address + coords within 11 m),
|
|
Tier 2a hits directly and normalized_address fallback is not reached."""
|
|
address = "ул Мира 10"
|
|
lat, lon = 56.84, 60.59
|
|
|
|
db = _make_db(
|
|
[
|
|
None, # pg_advisory_xact_lock
|
|
None, # house_sources miss
|
|
{"house_id": 99}, # fingerprint hit (Tier 2a)
|
|
None, # _upsert_house_source
|
|
None, # _insert_alias
|
|
]
|
|
)
|
|
house_id, conf, method = match_or_create_house(
|
|
db,
|
|
"yandex",
|
|
"ext-yz",
|
|
address=address,
|
|
lat=lat,
|
|
lon=lon,
|
|
)
|
|
assert method == "fingerprint"
|
|
assert house_id == 99
|
|
assert conf == 0.9
|
|
|
|
def test_insert_alias_upsert_keeps_fingerprint_in_sync(self):
|
|
"""_insert_alias with ON CONFLICT DO UPDATE ensures the stored fingerprint
|
|
is refreshed, not left stale from the first writer.
|
|
|
|
Verify the emitted SQL contains 'DO UPDATE SET' (not 'DO NOTHING').
|
|
"""
|
|
db = MagicMock()
|
|
result = MagicMock()
|
|
db.execute.return_value = result
|
|
|
|
_insert_alias(
|
|
db,
|
|
house_id=5,
|
|
address="ул. Ленина 5",
|
|
fp="aabbccdd" * 4,
|
|
source="avito",
|
|
)
|
|
|
|
assert db.execute.called
|
|
sql_str = str(db.execute.call_args[0][0])
|
|
assert (
|
|
"DO UPDATE SET" in sql_str
|
|
), "_insert_alias must use ON CONFLICT DO UPDATE SET fingerprint, not DO NOTHING"
|
|
assert "fingerprint" in sql_str
|
|
|
|
def test_two_listings_same_address_exactly_one_alias_key(self):
|
|
"""Integration check: normalize_address produces identical key for two scrapers
|
|
submitting the same address in different formats, so at most one alias row can
|
|
exist (UNIQUE normalized_address constraint holds).
|
|
|
|
This test verifies the normalization layer — the DB constraint is structural.
|
|
addr_a uses the abbreviated form 'ул.' which expands to 'улица';
|
|
addr_b already uses the full form. Both should normalize identically.
|
|
"""
|
|
addr_a = "ул. Ленина 5" # abbreviated 'ул.' → expands to 'улица'
|
|
addr_b = "улица ленина 5" # already expanded, lowercase
|
|
assert normalize_address(addr_a) == normalize_address(addr_b), (
|
|
"normalize_address must produce identical key for both address forms "
|
|
"so the UNIQUE(normalized_address) constraint prevents duplicate aliases"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Part 3: single SAVEPOINT level — batch isolation
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestSingleSavepointBatchIsolation:
|
|
"""save_listings: one failing lot → siblings persist; no orphan SAVEPOINT."""
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _patch_matching(self):
|
|
"""Stub matching so save_listings tests don't hit real DB queries."""
|
|
with (
|
|
patch("app.services.scrapers.base.match_or_create_house") as m_house,
|
|
patch("app.services.scrapers.base.upsert_listing_source") as m_link,
|
|
):
|
|
m_house.return_value = (101, 1.0, "new")
|
|
m_link.return_value = None
|
|
self.m_house = m_house
|
|
self.m_link = m_link
|
|
yield
|
|
|
|
def test_failing_lot_does_not_abort_batch(self, caplog):
|
|
"""Lot whose match_or_create_house raises → that lot's hook is rolled back
|
|
(outer SAVEPOINT), but other lots in the batch are fully inserted.
|
|
|
|
No whole-batch rollback, no orphan SAVEPOINTs.
|
|
"""
|
|
# First lot: house match succeeds; second lot: house match raises
|
|
self.m_house.side_effect = [
|
|
(10, 1.0, "new"),
|
|
RuntimeError("simulated house-match DB error"),
|
|
(12, 1.0, "new"),
|
|
]
|
|
|
|
db = _mock_save_db(inserted=True, listing_id=50)
|
|
|
|
lots = [
|
|
_base_lot(
|
|
source_id="ok-1",
|
|
address="Екатеринбург, Ленина 5",
|
|
lat=56.84,
|
|
lon=60.59,
|
|
),
|
|
_base_lot(
|
|
source_id="fail-1",
|
|
address="Екатеринбург, Мира 10",
|
|
lat=56.85,
|
|
lon=60.60,
|
|
),
|
|
_base_lot(
|
|
source_id="ok-2",
|
|
address="Екатеринбург, Куйбышева 3",
|
|
lat=56.83,
|
|
lon=60.58,
|
|
),
|
|
]
|
|
|
|
with caplog.at_level(logging.WARNING, logger="app.services.scrapers.base"):
|
|
inserted, _updated = save_listings(db, lots)
|
|
|
|
# All 3 listings INSERT'd (listing INSERT is outside any SAVEPOINT)
|
|
assert inserted == 3
|
|
|
|
# House match attempted on all 3 (all have address+coords)
|
|
assert self.m_house.call_count == 3
|
|
|
|
# upsert_listing_source: lots ok-1 and ok-2 succeed; fail-1's hook was aborted
|
|
# by the outer SAVEPOINT so link not called for it.
|
|
assert self.m_link.call_count == 2
|
|
|
|
# Warning logged for the failing lot
|
|
warns = [r for r in caplog.records if "match_failed" in r.getMessage()]
|
|
assert len(warns) == 1
|
|
|
|
def test_no_begin_nested_inside_link_listing_to_house(self):
|
|
"""Verify that _link_listing_to_house does NOT open its own begin_nested.
|
|
|
|
The outer per-row SAVEPOINT (one call per lot in save_listings) is the only
|
|
SAVEPOINT level allowed. If begin_nested is called more than N lots times,
|
|
that indicates a double-nesting.
|
|
"""
|
|
db = _mock_save_db(inserted=True, listing_id=77)
|
|
lots = [_base_lot(source_id=str(i)) for i in range(3)]
|
|
|
|
save_listings(db, lots)
|
|
|
|
# save_listings calls begin_nested THREE times per lot:
|
|
# 1) upsert SAVEPOINT — ловит IntegrityError на UNIQUE(source,source_id)
|
|
# (migration 133) при dedup_hash-дрейфе → rollback + прямой UPDATE (#1921);
|
|
# 2) snapshot SAVEPOINT;
|
|
# 3) hook SAVEPOINT (_link_listing_to_house).
|
|
# If _link_listing_to_house opened an INNER begin_nested too,
|
|
# the count would be > 3 * len(lots). The intent of this test (no double-nesting
|
|
# INSIDE the hook) holds — base.py keeps exactly one SAVEPOINT level in the hook.
|
|
assert db.begin_nested.call_count == 3 * len(lots), (
|
|
f"Expected exactly 3 begin_nested per lot (upsert + snapshot + hook outer), "
|
|
f"got {db.begin_nested.call_count} — inner SAVEPOINT may still be present"
|
|
)
|
|
|
|
def test_whole_batch_not_rolled_back_on_single_hook_failure(self, caplog):
|
|
"""When one lot's hook fails, db.commit() is still called (batch persisted)."""
|
|
self.m_house.side_effect = RuntimeError("boom")
|
|
|
|
db = _mock_save_db(inserted=True, listing_id=10)
|
|
lots = [_base_lot(source_id="only-one", address="x", lat=56.8, lon=60.5)]
|
|
|
|
with caplog.at_level(logging.WARNING, logger="app.services.scrapers.base"):
|
|
save_listings(db, lots)
|
|
|
|
# db.commit() called despite the hook failure — no whole-batch rollback
|
|
db.commit.assert_called_once()
|
|
db.rollback.assert_not_called()
|