Skip to content

Commit 7cd5d3f

Browse files
anth-volkanth-volk
committed
Pin legacy report IDs to migrated runs
1 parent 94228b3 commit 7cd5d3f

9 files changed

Lines changed: 297 additions & 49 deletions

‎alembic/versions/20260511_558935decda5_add_report_run_canonical_schema.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def upgrade() -> None:
2626
"legacy_report_output_id_map",
2727
sa.Column("legacy_report_output_id", sa.INTEGER(), nullable=False),
2828
sa.Column("canonical_report_output_id", sa.INTEGER(), nullable=False),
29+
sa.Column("display_report_output_run_id", sa.CHAR(length=36), nullable=False),
2930
sa.PrimaryKeyConstraint("legacy_report_output_id"),
3031
)
3132
op.create_index(

‎policyengine_api/data/initialise.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ CREATE TABLE IF NOT EXISTS legacy_report_output_aliases (
204204

205205
CREATE TABLE IF NOT EXISTS legacy_report_output_id_map (
206206
legacy_report_output_id INT PRIMARY KEY,
207-
canonical_report_output_id INT NOT NULL
207+
canonical_report_output_id INT NOT NULL,
208+
display_report_output_run_id CHAR(36) NOT NULL
208209
);
209210

210211
CREATE INDEX legacy_report_output_id_map_canonical_idx

‎policyengine_api/data/initialise_local.sql‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ CREATE TABLE IF NOT EXISTS legacy_report_output_aliases (
217217

218218
CREATE TABLE IF NOT EXISTS legacy_report_output_id_map (
219219
legacy_report_output_id INT PRIMARY KEY,
220-
canonical_report_output_id INT NOT NULL
220+
canonical_report_output_id INT NOT NULL,
221+
display_report_output_run_id CHAR(36) NOT NULL
221222
);
222223

223224
CREATE INDEX legacy_report_output_id_map_canonical_idx

‎policyengine_api/services/report_output_id_map_service.py‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ def _get_report_output_row(
2626
row: Row | None = queryer.query(query, tuple(params)).fetchone()
2727
return dict(row) if row is not None else None
2828

29+
def _get_report_output_run_row(
30+
self,
31+
report_output_run_id: str,
32+
*,
33+
canonical_report_output_id: int,
34+
queryer=None,
35+
) -> dict | None:
36+
queryer = queryer or database
37+
row: Row | None = queryer.query(
38+
"""
39+
SELECT id, report_output_id
40+
FROM report_output_runs
41+
WHERE id = ? AND report_output_id = ?
42+
""",
43+
(report_output_run_id, canonical_report_output_id),
44+
).fetchone()
45+
return dict(row) if row is not None else None
46+
2947
def _validate_mapping_identity_compatibility(
3048
self,
3149
legacy_report_output: dict,
@@ -106,9 +124,22 @@ def resolve_report_output_id(
106124
and canonical_report_output["country_id"] != country_id
107125
):
108126
return None
127+
display_report_output_run_id = mapping["display_report_output_run_id"]
128+
display_run = self._get_report_output_run_row(
129+
display_report_output_run_id,
130+
canonical_report_output_id=canonical_report_output_id,
131+
queryer=queryer,
132+
)
133+
if display_run is None:
134+
raise ValueError(
135+
"Legacy ID mapping points to missing display report output run "
136+
f"#{display_report_output_run_id} for canonical report output "
137+
f"#{canonical_report_output_id}"
138+
)
109139
return {
110140
"requested_report_output_id": requested_report_output_id,
111141
"canonical_report_output_id": canonical_report_output_id,
142+
"display_report_output_run_id": display_report_output_run_id,
112143
"is_legacy_id": True,
113144
}
114145

@@ -123,6 +154,7 @@ def resolve_report_output_id(
123154
return {
124155
"requested_report_output_id": requested_report_output_id,
125156
"canonical_report_output_id": requested_report_output_id,
157+
"display_report_output_run_id": None,
126158
"is_legacy_id": False,
127159
}
128160

@@ -146,6 +178,7 @@ def set_mapping(
146178
self,
147179
legacy_report_output_id: int,
148180
canonical_report_output_id: int,
181+
display_report_output_run_id: str,
149182
) -> bool:
150183
if legacy_report_output_id == canonical_report_output_id:
151184
raise ValueError("Legacy and canonical report outputs must be different")
@@ -163,12 +196,15 @@ def set_mapping(
163196
if (
164197
existing_mapping["canonical_report_output_id"]
165198
== canonical_report_output_id
199+
and existing_mapping["display_report_output_run_id"]
200+
== display_report_output_run_id
166201
):
167202
return True
168203

169204
raise ValueError(
170205
"Legacy report output ID already maps to canonical report output "
171-
f"#{existing_mapping['canonical_report_output_id']}"
206+
f"#{existing_mapping['canonical_report_output_id']} and display "
207+
f"run #{existing_mapping['display_report_output_run_id']}"
172208
)
173209

174210
legacy_report_output = self._get_report_output_row(legacy_report_output_id)
@@ -178,12 +214,31 @@ def set_mapping(
178214
canonical_report_output,
179215
)
180216

217+
display_run = self._get_report_output_run_row(
218+
display_report_output_run_id,
219+
canonical_report_output_id=canonical_report_output_id,
220+
)
221+
if display_run is None:
222+
raise ValueError(
223+
"Display report output run "
224+
f"#{display_report_output_run_id} not found for canonical report "
225+
f"#{canonical_report_output_id}"
226+
)
227+
181228
database.query(
182229
"""
183230
INSERT INTO legacy_report_output_id_map
184-
(legacy_report_output_id, canonical_report_output_id)
185-
VALUES (?, ?)
231+
(
232+
legacy_report_output_id,
233+
canonical_report_output_id,
234+
display_report_output_run_id
235+
)
236+
VALUES (?, ?, ?)
186237
""",
187-
(legacy_report_output_id, canonical_report_output_id),
238+
(
239+
legacy_report_output_id,
240+
canonical_report_output_id,
241+
display_report_output_run_id,
242+
),
188243
)
189244
return True

‎policyengine_api/services/report_output_service.py‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,23 +1491,36 @@ def get_report_output(self, country_id: str, report_output_id: int) -> dict | No
14911491
if canonical_report_output is None:
14921492
return None
14931493

1494-
display_run = self.report_run_service.select_display_run(
1495-
canonical_report_output
1496-
)
1497-
if display_run is None or (
1498-
run_matches_report_result(display_run, canonical_report_output)
1499-
and self._run_needs_timestamp_sync(
1500-
display_run,
1501-
canonical_report_output["status"],
1502-
)
1503-
):
1504-
canonical_report_output = self.ensure_report_output_dual_write_state(
1505-
canonical_report_output_id,
1506-
country_id=country_id,
1494+
if resolution["is_legacy_id"]:
1495+
display_run = self._get_report_run_row(
1496+
resolution["display_report_output_run_id"],
1497+
report_output_id=canonical_report_output_id,
15071498
)
1499+
if display_run is None:
1500+
raise ValueError(
1501+
"Legacy ID mapping points to missing display report output "
1502+
f"run #{resolution['display_report_output_run_id']}"
1503+
)
1504+
else:
15081505
display_run = self.report_run_service.select_display_run(
15091506
canonical_report_output
15101507
)
1508+
if display_run is None or (
1509+
run_matches_report_result(display_run, canonical_report_output)
1510+
and self._run_needs_timestamp_sync(
1511+
display_run,
1512+
canonical_report_output["status"],
1513+
)
1514+
):
1515+
canonical_report_output = (
1516+
self.ensure_report_output_dual_write_state(
1517+
canonical_report_output_id,
1518+
country_id=country_id,
1519+
)
1520+
)
1521+
display_run = self.report_run_service.select_display_run(
1522+
canonical_report_output
1523+
)
15111524
resolved_report_output = self._merge_display_run_into_report_output(
15121525
canonical_report_output,
15131526
display_run,
@@ -1818,6 +1831,19 @@ def tx_callback(tx):
18181831
f"#{report_output_run_id} not found for report "
18191832
f"#{canonical_report_id}"
18201833
)
1834+
elif resolution["is_legacy_id"]:
1835+
mutable_run = self._get_report_run_row(
1836+
resolution["display_report_output_run_id"],
1837+
queryer=tx,
1838+
report_output_id=canonical_report_id,
1839+
for_update=True,
1840+
)
1841+
if mutable_run is None:
1842+
raise ValueError(
1843+
"Legacy ID mapping points to missing display report "
1844+
"output run "
1845+
f"#{resolution['display_report_output_run_id']}"
1846+
)
18211847
else:
18221848
mutable_run = self._select_mutable_run(
18231849
canonical_report,

‎tests/unit/data/test_run_schema.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ def test_stage_one_run_schema_is_initialized_in_local_test_db(test_db):
7474
}.issubset(simulation_run_columns)
7575

7676
id_map_columns = _column_names(test_db, "legacy_report_output_id_map")
77-
assert {"legacy_report_output_id", "canonical_report_output_id"} == id_map_columns
77+
assert {
78+
"legacy_report_output_id",
79+
"canonical_report_output_id",
80+
"display_report_output_run_id",
81+
} == id_map_columns
7882

7983
legacy_alias_columns = _column_names(test_db, "legacy_report_output_aliases")
8084
assert {
@@ -100,6 +104,7 @@ def test_stage_one_schema_is_defined_in_both_sql_initializers():
100104
"CREATE TABLE IF NOT EXISTS simulation_runs",
101105
"CREATE TABLE IF NOT EXISTS legacy_report_output_aliases",
102106
"CREATE TABLE IF NOT EXISTS legacy_report_output_id_map",
107+
"display_report_output_run_id",
103108
"CREATE INDEX report_outputs_identity_idx",
104109
"report_spec_json",
105110
"report_spec_status",

‎tests/unit/services/test_report_output_id_map_service.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,19 @@ def _insert_legacy_report_output(
4343
),
4444
)
4545

46+
def _display_run_id(self, test_db, report_output_id: int) -> str:
47+
row = test_db.query(
48+
"""
49+
SELECT id FROM report_output_runs
50+
WHERE report_output_id = ?
51+
ORDER BY run_sequence DESC
52+
LIMIT 1
53+
""",
54+
(report_output_id,),
55+
).fetchone()
56+
assert row is not None
57+
return row["id"]
58+
4659
def test_resolves_to_canonical_report_output_id_when_mapping_exists(self, test_db):
4760
simulation = simulation_service.create_simulation(
4861
country_id="us",
@@ -61,6 +74,9 @@ def test_resolves_to_canonical_report_output_id_when_mapping_exists(self, test_d
6174
id_map_service.set_mapping(
6275
legacy_report_output_id=999,
6376
canonical_report_output_id=canonical_report["id"],
77+
display_report_output_run_id=self._display_run_id(
78+
test_db, canonical_report["id"]
79+
),
6480
)
6581

6682
resolved_id = id_map_service.resolve_canonical_report_output_id(999)
@@ -109,13 +125,19 @@ def test_set_mapping_is_idempotent_for_same_canonical_report_output(self, test_d
109125
id_map_service.set_mapping(
110126
legacy_report_output_id=1001,
111127
canonical_report_output_id=canonical_report["id"],
128+
display_report_output_run_id=self._display_run_id(
129+
test_db, canonical_report["id"]
130+
),
112131
)
113132
is True
114133
)
115134
assert (
116135
id_map_service.set_mapping(
117136
legacy_report_output_id=1001,
118137
canonical_report_output_id=canonical_report["id"],
138+
display_report_output_run_id=self._display_run_id(
139+
test_db, canonical_report["id"]
140+
),
119141
)
120142
is True
121143
)
@@ -139,10 +161,35 @@ def test_rejects_mapping_to_missing_canonical_report_output(self, test_db):
139161
id_map_service.set_mapping(
140162
legacy_report_output_id=1002,
141163
canonical_report_output_id=999999,
164+
display_report_output_run_id="missing-run",
142165
)
143166

144167
assert "Canonical report output #999999 not found" in str(exc_info.value)
145168

169+
def test_rejects_mapping_to_missing_display_report_output_run(self, test_db):
170+
simulation = simulation_service.create_simulation(
171+
country_id="us",
172+
population_id="household_3b",
173+
population_type="household",
174+
policy_id=3,
175+
)
176+
canonical_report = report_output_service.create_report_output(
177+
country_id="us",
178+
simulation_1_id=simulation["id"],
179+
simulation_2_id=None,
180+
year="2025",
181+
)
182+
self._insert_legacy_report_output(test_db, 10020, canonical_report)
183+
184+
with pytest.raises(ValueError) as exc_info:
185+
id_map_service.set_mapping(
186+
legacy_report_output_id=10020,
187+
canonical_report_output_id=canonical_report["id"],
188+
display_report_output_run_id="missing-run",
189+
)
190+
191+
assert "Display report output run #missing-run not found" in str(exc_info.value)
192+
146193
def test_rejects_conflicting_mapping_remap(self, test_db):
147194
simulation = simulation_service.create_simulation(
148195
country_id="us",
@@ -166,12 +213,18 @@ def test_rejects_conflicting_mapping_remap(self, test_db):
166213
id_map_service.set_mapping(
167214
legacy_report_output_id=1003,
168215
canonical_report_output_id=canonical_report["id"],
216+
display_report_output_run_id=self._display_run_id(
217+
test_db, canonical_report["id"]
218+
),
169219
)
170220

171221
with pytest.raises(ValueError) as exc_info:
172222
id_map_service.set_mapping(
173223
legacy_report_output_id=1003,
174224
canonical_report_output_id=other_report["id"],
225+
display_report_output_run_id=self._display_run_id(
226+
test_db, other_report["id"]
227+
),
175228
)
176229

177230
assert (
@@ -197,6 +250,9 @@ def test_allows_mapping_when_legacy_report_output_is_missing(self, test_db):
197250
id_map_service.set_mapping(
198251
legacy_report_output_id=10030,
199252
canonical_report_output_id=canonical_report["id"],
253+
display_report_output_run_id=self._display_run_id(
254+
test_db, canonical_report["id"]
255+
),
200256
)
201257
is True
202258
)
@@ -205,6 +261,9 @@ def test_allows_mapping_when_legacy_report_output_is_missing(self, test_db):
205261
assert resolved == {
206262
"requested_report_output_id": 10030,
207263
"canonical_report_output_id": canonical_report["id"],
264+
"display_report_output_run_id": self._display_run_id(
265+
test_db, canonical_report["id"]
266+
),
208267
"is_legacy_id": True,
209268
}
210269

@@ -270,6 +329,9 @@ def test_rejects_mapping_when_reports_do_not_share_canonical_identity(
270329
id_map_service.set_mapping(
271330
legacy_report_output_id=distinct_report["id"],
272331
canonical_report_output_id=canonical_report["id"],
332+
display_report_output_run_id=self._display_run_id(
333+
test_db, canonical_report["id"]
334+
),
273335
)
274336

275337
assert "must share canonical report identity" in str(exc_info.value)
@@ -307,6 +369,9 @@ def test_rejects_mapping_when_legacy_report_output_has_no_identity(self, test_db
307369
id_map_service.set_mapping(
308370
legacy_report_output_id=10031,
309371
canonical_report_output_id=canonical_report["id"],
372+
display_report_output_run_id=self._display_run_id(
373+
test_db, canonical_report["id"]
374+
),
310375
)
311376

312377
assert "must have canonical report identity" in str(exc_info.value)
@@ -329,6 +394,9 @@ def test_rejects_mapping_when_legacy_and_canonical_ids_match(self, test_db):
329394
id_map_service.set_mapping(
330395
legacy_report_output_id=canonical_report["id"],
331396
canonical_report_output_id=canonical_report["id"],
397+
display_report_output_run_id=self._display_run_id(
398+
test_db, canonical_report["id"]
399+
),
332400
)
333401

334402
assert "must be different" in str(exc_info.value)
@@ -352,6 +420,9 @@ def test_rejects_mapping_resolution_when_canonical_report_output_is_missing(
352420
id_map_service.set_mapping(
353421
legacy_report_output_id=1004,
354422
canonical_report_output_id=canonical_report["id"],
423+
display_report_output_run_id=self._display_run_id(
424+
test_db, canonical_report["id"]
425+
),
355426
)
356427
test_db.query(
357428
"DELETE FROM report_outputs WHERE id = ?",

0 commit comments

Comments
 (0)