fix(tradein-matching): pg_advisory_xact_lock to prevent duplicate house INSERTs #501

Merged
lekss361 merged 2 commits from feat/tradein-houses-advisory-lock into main 2026-05-24 10:52:11 +00:00
2 changed files with 52 additions and 9 deletions

View file

@ -34,11 +34,11 @@ def match_or_create_house(
) -> tuple[int, float, str]:
"""Match existing house or create new canonical record.
KNOWN LIMITATION: Concurrent calls for the same address can race
two scrapers calling simultaneously for an unknown address can both
miss Tier 0-3 and each INSERT a new house row. Mitigation (Stage 8.x):
gate scraper concurrency to 1 per fingerprint OR add
pg_advisory_xact_lock(hashtext(normalized_address)) at function entry.
Concurrency-safe: serializes concurrent calls for the same address fingerprint
via pg_advisory_xact_lock(42, hashtext(fp)). The lock is transaction-scoped,
released on the caller's COMMIT/ROLLBACK. Without this, two scrapers calling
for an unknown address could both miss Tier 0-3 and each INSERT a duplicate
house row. Closes finding #1 from 2026-05-24 audit.
Returns:
(house_id, confidence [0.0, 1.0], method {
@ -54,6 +54,18 @@ def match_or_create_house(
"""
cad = building_cadastral_number or cadastral_number
# Compute fingerprint early so we can acquire the advisory lock before any tier reads.
fp = address_fingerprint(address, lat, lon)
# Serialize concurrent calls for the same address fingerprint to prevent
# racing INSERTs into houses (finding #1 from 2026-05-24 audit).
# pg_advisory_xact_lock takes (int4, int4) — namespace 42 = "tradein house matching".
# Lock is released automatically on caller's COMMIT/ROLLBACK.
db.execute(
text('SELECT pg_advisory_xact_lock(42, hashtext(:fp))'),
{'fp': fp},
)
# Tier 0: cadastral number exact match
if cad:
row = db.execute(
@ -83,9 +95,6 @@ def match_or_create_house(
ext_id)
return (house_id, 1.0, 'source_exact')
# Compute fingerprint once for Tier 2 and reuse for new house registration
fp = address_fingerprint(address, lat, lon)
# Tier 2: address fingerprint lookup
row = db.execute(
text(

View file

@ -148,6 +148,7 @@ def test_match_house_tier0_cadastr():
from app.services.matching.houses import match_or_create_house
db = _make_db([
None, # pg_advisory_xact_lock
{'id': 42}, # cadastral match
None, # _upsert_house_source (INSERT ... ON CONFLICT)
])
@ -171,7 +172,8 @@ def test_match_house_tier1_source_exact():
from app.services.matching.houses import match_or_create_house
db = _make_db([
{'house_id': 7}, # house_sources hit (first and only execute call)
None, # pg_advisory_xact_lock
{'house_id': 7}, # house_sources hit
])
house_id, conf, method = match_or_create_house(
db, 'avito', 'ext-999',
@ -186,6 +188,7 @@ def test_match_house_tier2_fingerprint():
from app.services.matching.houses import match_or_create_house
db = _make_db([
None, # pg_advisory_xact_lock
None, # house_sources miss
{'house_id': 15}, # fingerprint hit
None, # _upsert_house_source
@ -204,6 +207,7 @@ def test_match_house_tier3_geo():
from app.services.matching.houses import match_or_create_house
db = _make_db([
None, # pg_advisory_xact_lock
None, # house_sources miss
None, # fingerprint miss
{'id': 22, 'dist': 15.5}, # geo hit
@ -224,6 +228,7 @@ def test_match_house_new():
from app.services.matching.houses import match_or_create_house
db = _make_db([
None, # pg_advisory_xact_lock
None, # house_sources miss
None, # fingerprint miss
None, # geo miss
@ -240,6 +245,35 @@ def test_match_house_new():
assert method == 'new'
def test_match_house_advisory_lock_called_first():
"""Regression: pg_advisory_xact_lock must be the first execute() call
on match_or_create_house. Re-ordering would re-introduce the race
(finding #1 from 2026-05-24 audit). PR #501."""
from app.services.matching.houses import match_or_create_house
db = _make_db([
None, # pg_advisory_xact_lock
{'house_id': 7}, # house_sources Tier 1 hit (short path)
])
match_or_create_house(
db, 'avito', 'regress-001',
address='Тестовая 1', lat=56.8, lon=60.5,
)
# First execute MUST be the lock acquisition, with bind {'fp': <hex>}.
first_call = db.execute.call_args_list[0]
sql_obj = first_call[0][0] # TextClause
bind = first_call[0][1] # dict
assert 'pg_advisory_xact_lock' in str(sql_obj), (
f'first execute must be advisory lock, got: {sql_obj}'
)
assert 'fp' in bind, f"lock bind must include fp, got: {bind}"
assert isinstance(bind['fp'], str) and len(bind['fp']) == 32, (
f"fp must be 32-char sha256 hex, got: {bind.get('fp')!r}"
)
# ---------------------------------------------------------------------------
# match_or_create_listing — mock DB tier routing
# ---------------------------------------------------------------------------