Repository navigation
Build stops-for-agency records with shared code - #1809
aaronbrethorst wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
|
@aaronbrethorst, the PR description does not follow the PR template:
Keep this PR as Draft until these are fixed. This comment updates when the description changes. |
Performance Smoke Test ResultsStatus: PASSED
Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%. Full results uploaded as workflow artifact: k6-smoke-summary. |
stops-for-agency was the last endpoint with its own copy of the stop record code. It copied the stops.direction column as is, listed each stop's routes in database order, and found parent stations and route references through two more helpers of its own. Take the stops, their parent stations and their routes from stopModelsByIDs and missingParentStops, and build route references with routeReferencesForStops, as block does. A stop's direction now goes through DirectionCalculator, which reads the same column first and turns a feed value like "North" into "N". Its routeIds follow the shared builder's route-name order.
routeReferencesForStops built its list from a map, so references.routes came back in a different order on every request for stops-for-agency and block, where the old stops-for-agency code sorted by route ID. Sort by ID. stops-for-agency repeated BuildStopReferencesAndRouteIDsForStops line for line to keep parent stations apart from the stops. Both now use stopsAndParentStations. The stops-for-agency test also pins one stop's route order and direction, so it no longer only compares the handler with the builder it calls.
2571546 to
bcc942e
Compare
2133a5b to
0158ed7
Compare
|
Performance Smoke Test ResultsStatus: PASSED
Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%. Full results uploaded as workflow artifact: k6-smoke-summary. |
Performance Smoke Test ResultsStatus: PASSED
Smoke test config: 5 VUs x 30s. Thresholds: p(95) < 300ms, error rate < 1%. Full results uploaded as workflow artifact: k6-smoke-summary. |
|
Closing as superseded. The multi-feed static GTFS spec (#1811, |
Stacked on #1807. Part of the stop consolidation work (#1771–#1779). No issue filed yet.
What it does
stops-for-agencynow builds its stop records, parent stations and route references with the shared stop builder. It was the last endpoint with its own copy of that code, so every stop record in Maglev now comes from one place (29 lines added, 106 removed).Root cause
buildStopsListForAgencyhand-builtmodels.Stopvalues:stops.directionas is;buildParentStationReferencesandBuildRouteReferencesfor the references.Now the stops come from
stopModelsByIDsand the parent stations frommissingParentStops, together with their route rows. Route references come fromrouteReferencesForStops, asblockdoes since #1798. The two helpers are deleted;BuildRouteReferencesstays for its other callers.Testing
TestStopsForAgency_UsesSharedStopRecords, which requires the list to equal the shared builder's records for the agency's stops. On RABA it failed on route order: for stop25_1001, before["25_159","25_160","25_157"], after["25_157","25_159","25_160"].references.stopsserializing asnullwhen there are no parent stations; that's fixed.Response comparison
Production Puget Sound returns 404 for
stops-for-agency, so there's no Java response to compare against. On the synthetic parity feed,stops-for-agency/40and/1still match Java's IDs.Checklist
mainand passes on this branch.gtfsdb/(db.go,models.go,query.sql.go) ortestdata/openapi.yml(synced from upstream).r.Context()to DB/service calls, notcontext.Background().testdata/openapi.yml, has an issue open on OneBusAway/sdk-config, or is outside the SDK and the description says why.Notes for reviewers
DirectionCalculator, as on every other endpoint. That costs little: directions are computed at import and stored instops.direction, which the calculator reads first. The visible change is that a feed value likeNorthbecomesN, matching other endpoints. RABA's directions are unchanged.GetStopIDsForAgencyorder instead ofGetStopsByIDsorder.After review
routeReferencesForStopsbuilt its list from a map, soreferences.routeschanged order on every request here and inblock. It now sorts by route ID.stopsAndParentStationsis now shared withBuildStopReferencesAndRouteIDsForStops, instead of being repeated here.25_1001's route order and direction, so it no longer only compares the handler with the builder it calls.