From 9a6890de6aaace10144e38d55c20968d1bfeb7f1 Mon Sep 17 00:00:00 2001 From: lekss361 Date: Sun, 24 May 2026 13:15:50 +0300 Subject: [PATCH 1/2] fix(tradein-matching): pg_advisory_xact_lock to prevent duplicate house INSERTs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two scrapers calling match_or_create_house() concurrently for the same address could both miss Tier 0-3 and each INSERT a new canonical house — splitting listings across duplicate house rows and breaking aggregations. Serialize via pg_advisory_xact_lock(42, hashtext(fingerprint)). Lock is transaction-scoped (released on caller's COMMIT/ROLLBACK). After unblock, the losing scraper's Tier 2 fingerprint lookup hits the winner's alias and returns the same house_id. Closes finding #1 from 2026-05-24 trade-in audit. --- .../backend/app/services/matching/houses.py | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tradein-mvp/backend/app/services/matching/houses.py b/tradein-mvp/backend/app/services/matching/houses.py index 87d78c84..64a1fa67 100644 --- a/tradein-mvp/backend/app/services/matching/houses.py +++ b/tradein-mvp/backend/app/services/matching/houses.py @@ -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( -- 2.45.3 From cea2af8d3dd628a4512026b584372fd465f58f6b Mon Sep 17 00:00:00 2001 From: lekss361 Date: Sun, 24 May 2026 13:37:46 +0300 Subject: [PATCH 2/2] test(tradein-matching): fix off-by-one in 5 match_house tests + add lock regression Advisory lock in match_or_create_house() is now the first db.execute() call; existing tests built side_effect lists assuming the tier query was first, causing each to read off-by-one (Tier 0 cadastr lookup gets None, falls through, etc.). Prepend a None row to all 5 test_match_house_* _make_db lists. Add test_match_house_advisory_lock_called_first as a regression guard against future re-ordering of the lock call. Per PR #501 reviewer feedback. --- tradein-mvp/backend/tests/test_matching.py | 36 +++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/tradein-mvp/backend/tests/test_matching.py b/tradein-mvp/backend/tests/test_matching.py index 1f9c7b0c..51c38bd9 100644 --- a/tradein-mvp/backend/tests/test_matching.py +++ b/tradein-mvp/backend/tests/test_matching.py @@ -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': }. + 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 # --------------------------------------------------------------------------- -- 2.45.3