From 695da72f5159b26d86ff0e30ef587a8facdb64c3 Mon Sep 17 00:00:00 2001 From: tejasva-vardhan Date: Sun, 29 Mar 2026 02:21:04 +0530 Subject: [PATCH 1/4] fix: register missing Situation endpoint route (#798) --- internal/gtfs/realtime.go | 30 +++++++++ internal/restapi/routes.go | 1 + internal/restapi/situation_handler.go | 42 ++++++++++++ internal/restapi/situation_handler_test.go | 76 ++++++++++++++++++++++ 4 files changed, 149 insertions(+) create mode 100644 internal/restapi/situation_handler.go create mode 100644 internal/restapi/situation_handler_test.go diff --git a/internal/gtfs/realtime.go b/internal/gtfs/realtime.go index 25cfe0382..2a0a12f7a 100644 --- a/internal/gtfs/realtime.go +++ b/internal/gtfs/realtime.go @@ -201,6 +201,36 @@ func (manager *Manager) GetAlertsForStop(stopID string) []gtfs.Alert { return out } +// GetAllAlerts returns all deduplicated realtime alerts across all feeds. +// Deduplication is by alert ID, preserving first-seen order by sorted feed ID. +func (manager *Manager) GetAllAlerts() []gtfs.Alert { + manager.realTimeMutex.RLock() + defer manager.realTimeMutex.RUnlock() + + feedIDs := make([]string, 0, len(manager.feedAlerts)) + for feedID := range manager.feedAlerts { + feedIDs = append(feedIDs, feedID) + } + sort.Strings(feedIDs) + + seen := make(map[string]struct{}) + alerts := make([]gtfs.Alert, 0) + for _, feedID := range feedIDs { + for _, alert := range manager.feedAlerts[feedID] { + if alert.ID == "" { + continue + } + if _, exists := seen[alert.ID]; exists { + continue + } + seen[alert.ID] = struct{}{} + alerts = append(alerts, alert) + } + } + + return alerts +} + // Fetches GTFS-RT data from a URL with per-feed headers. func loadRealtimeData(ctx context.Context, source string, headers map[string]string) (*gtfs.Realtime, error) { req, err := http.NewRequestWithContext(ctx, "GET", source, nil) diff --git a/internal/restapi/routes.go b/internal/restapi/routes.go index 81a25a570..b311a7e49 100644 --- a/internal/restapi/routes.go +++ b/internal/restapi/routes.go @@ -96,6 +96,7 @@ func (api *RestAPI) SetRoutes(mux *http.ServeMux) { // Real-time simple ID endpoints (no ETag) mux.Handle("GET /api/where/vehicles-for-agency/{id}", CacheControlMiddleware(models.CacheDurationShort, rateLimitAndValidateAPIKey(api, api.vehiclesForAgencyHandler))) + mux.Handle("GET /api/where/situation/{id}", CacheControlMiddleware(models.CacheDurationShort, rateLimitAndValidateAPIKey(api, api.situationHandler))) // --- Routes with combined ID validation (agency_id_code format) --- mux.Handle("GET /api/where/trip/{id}", CacheControlMiddleware(models.CacheDurationLong, rateLimitAndValidateAPIKey(api, etagStatic(api, api.tripHandler)))) diff --git a/internal/restapi/situation_handler.go b/internal/restapi/situation_handler.go new file mode 100644 index 000000000..fdad212b6 --- /dev/null +++ b/internal/restapi/situation_handler.go @@ -0,0 +1,42 @@ +package restapi + +import ( + "fmt" + "net/http" + + "github.com/OneBusAway/go-gtfs" + "maglev.onebusaway.org/internal/models" +) + +// situationHandler serves a single GTFS-RT service alert (OneBusAway "Situation") +// by its alert id. +func (api *RestAPI) situationHandler(w http.ResponseWriter, r *http.Request) { + situationID, ok := api.extractAndValidateID(w, r) + if !ok { + return + } + + var alert gtfs.Alert + found := false + for _, candidate := range api.GtfsManager.GetAllAlerts() { + if candidate.ID == situationID { + alert = candidate + found = true + break + } + } + if !found { + api.sendNotFound(w, r) + return + } + + situations := api.BuildSituationReferences([]gtfs.Alert{alert}) + if len(situations) == 0 { + api.serverErrorResponse(w, r, fmt.Errorf("unexpected empty situation build for id %q", situationID)) + return + } + + references := models.NewEmptyReferences() + response := models.NewEntryResponse(situations[0], *references, api.Clock) + api.sendResponse(w, r, response) +} diff --git a/internal/restapi/situation_handler_test.go b/internal/restapi/situation_handler_test.go new file mode 100644 index 000000000..2499934b6 --- /dev/null +++ b/internal/restapi/situation_handler_test.go @@ -0,0 +1,76 @@ +package restapi + +import ( + "net/http" + "testing" + + "github.com/OneBusAway/go-gtfs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSituationHandlerRequiresValidAPIKey(t *testing.T) { + api := createTestApi(t) + defer api.Shutdown() + + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/test-alert.json?key=invalid") + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode) + assert.Equal(t, http.StatusUnauthorized, model.Code) + assert.Equal(t, "permission denied", model.Text) +} + +func TestSituationHandlerNotFound(t *testing.T) { + api := createTestApi(t) + defer api.Shutdown() + + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/nonexistent-alert.json?key=TEST") + assert.Equal(t, http.StatusNotFound, resp.StatusCode) + assert.Equal(t, http.StatusNotFound, model.Code) + assert.Equal(t, "resource not found", model.Text) +} + +func TestSituationHandlerWithSituation(t *testing.T) { + api := createTestApi(t) + defer api.Shutdown() + + alert := gtfs.Alert{ + ID: "test-alert-123", + Header: []gtfs.AlertText{ + {Text: "Service disruption", Language: "en"}, + }, + Description: []gtfs.AlertText{ + {Text: "Detour in effect", Language: "en"}, + }, + } + api.GtfsManager.AddTestAlert(alert) + + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/test-alert-123.json?key=TEST") + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, http.StatusOK, model.Code) + assert.Equal(t, "OK", model.Text) + assert.Equal(t, 2, model.Version) + + data, ok := model.Data.(map[string]interface{}) + require.True(t, ok, "response should include data object") + + entry, ok := data["entry"].(map[string]interface{}) + require.True(t, ok, "response should include data.entry object") + assert.Equal(t, "test-alert-123", entry["id"]) + assert.Equal(t, "UNKNOWN_CAUSE", entry["reason"]) + assert.Equal(t, "noImpact", entry["severity"]) + + references, ok := data["references"].(map[string]interface{}) + require.True(t, ok, "response should include data.references object") + + agencies, ok := references["agencies"].([]interface{}) + require.True(t, ok) + assert.Len(t, agencies, 0) + + routes, ok := references["routes"].([]interface{}) + require.True(t, ok) + assert.Len(t, routes, 0) + + stops, ok := references["stops"].([]interface{}) + require.True(t, ok) + assert.Len(t, stops, 0) +} From f20054e36bf67d7a57a7fc470e943de0fe2a3006 Mon Sep 17 00:00:00 2001 From: tejasva-vardhan Date: Sat, 15 Aug 2026 13:34:37 +0530 Subject: [PATCH 2/4] Fix situation endpoint after rebase onto main Use slices.Sort and AddAlertForTest so the branch compiles against current main. Drop the unreachable empty-situation guard and assert summary and description on the handler test. --- internal/gtfs/realtime.go | 2 +- internal/restapi/situation_handler.go | 6 ------ internal/restapi/situation_handler_test.go | 12 +++++++++++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/gtfs/realtime.go b/internal/gtfs/realtime.go index 2a0a12f7a..d2150a561 100644 --- a/internal/gtfs/realtime.go +++ b/internal/gtfs/realtime.go @@ -211,7 +211,7 @@ func (manager *Manager) GetAllAlerts() []gtfs.Alert { for feedID := range manager.feedAlerts { feedIDs = append(feedIDs, feedID) } - sort.Strings(feedIDs) + slices.Sort(feedIDs) seen := make(map[string]struct{}) alerts := make([]gtfs.Alert, 0) diff --git a/internal/restapi/situation_handler.go b/internal/restapi/situation_handler.go index fdad212b6..118ce5e8b 100644 --- a/internal/restapi/situation_handler.go +++ b/internal/restapi/situation_handler.go @@ -1,7 +1,6 @@ package restapi import ( - "fmt" "net/http" "github.com/OneBusAway/go-gtfs" @@ -31,11 +30,6 @@ func (api *RestAPI) situationHandler(w http.ResponseWriter, r *http.Request) { } situations := api.BuildSituationReferences([]gtfs.Alert{alert}) - if len(situations) == 0 { - api.serverErrorResponse(w, r, fmt.Errorf("unexpected empty situation build for id %q", situationID)) - return - } - references := models.NewEmptyReferences() response := models.NewEntryResponse(situations[0], *references, api.Clock) api.sendResponse(w, r, response) diff --git a/internal/restapi/situation_handler_test.go b/internal/restapi/situation_handler_test.go index 2499934b6..8c8c49965 100644 --- a/internal/restapi/situation_handler_test.go +++ b/internal/restapi/situation_handler_test.go @@ -42,7 +42,7 @@ func TestSituationHandlerWithSituation(t *testing.T) { {Text: "Detour in effect", Language: "en"}, }, } - api.GtfsManager.AddTestAlert(alert) + api.GtfsManager.AddAlertForTest(alert) resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/test-alert-123.json?key=TEST") assert.Equal(t, http.StatusOK, resp.StatusCode) @@ -59,6 +59,16 @@ func TestSituationHandlerWithSituation(t *testing.T) { assert.Equal(t, "UNKNOWN_CAUSE", entry["reason"]) assert.Equal(t, "noImpact", entry["severity"]) + summary, ok := entry["summary"].(map[string]interface{}) + require.True(t, ok, "entry should include summary") + assert.Equal(t, "Service disruption", summary["value"]) + assert.Equal(t, "en", summary["lang"]) + + description, ok := entry["description"].(map[string]interface{}) + require.True(t, ok, "entry should include description") + assert.Equal(t, "Detour in effect", description["value"]) + assert.Equal(t, "en", description["lang"]) + references, ok := data["references"].(map[string]interface{}) require.True(t, ok, "response should include data.references object") From a7882b23e400548fa31c946438cd2973f27f42d2 Mon Sep 17 00:00:00 2001 From: tejasva-vardhan Date: Sat, 15 Aug 2026 13:48:39 +0530 Subject: [PATCH 3/4] Use a unique alert ID in the situation handler test Tests share one GTFS manager, and route-handler tests already seed test-alert-123. GetAllAlerts keeps the first ID, so the situation test was asserting against the wrong alert. --- internal/restapi/situation_handler_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/restapi/situation_handler_test.go b/internal/restapi/situation_handler_test.go index 8c8c49965..fa8306056 100644 --- a/internal/restapi/situation_handler_test.go +++ b/internal/restapi/situation_handler_test.go @@ -33,8 +33,9 @@ func TestSituationHandlerWithSituation(t *testing.T) { api := createTestApi(t) defer api.Shutdown() + const alertID = "situation-handler-alert" alert := gtfs.Alert{ - ID: "test-alert-123", + ID: alertID, Header: []gtfs.AlertText{ {Text: "Service disruption", Language: "en"}, }, @@ -44,7 +45,7 @@ func TestSituationHandlerWithSituation(t *testing.T) { } api.GtfsManager.AddAlertForTest(alert) - resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/test-alert-123.json?key=TEST") + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/"+alertID+".json?key=TEST") assert.Equal(t, http.StatusOK, resp.StatusCode) assert.Equal(t, http.StatusOK, model.Code) assert.Equal(t, "OK", model.Text) @@ -55,7 +56,7 @@ func TestSituationHandlerWithSituation(t *testing.T) { entry, ok := data["entry"].(map[string]interface{}) require.True(t, ok, "response should include data.entry object") - assert.Equal(t, "test-alert-123", entry["id"]) + assert.Equal(t, alertID, entry["id"]) assert.Equal(t, "UNKNOWN_CAUSE", entry["reason"]) assert.Equal(t, "noImpact", entry["severity"]) From 44bf34276b24ecb2a04017d791dc719deafcf2aa Mon Sep 17 00:00:00 2001 From: tejasva-vardhan Date: Fri, 21 Aug 2026 11:45:41 +0530 Subject: [PATCH 4/4] Match situation lookups to agency-qualified IDs Clients following situationIds from trip-details use the agency-prefixed form. Dedup GetAllAlerts on that same ID, and reject malformed IDs with 400 instead of 404. --- internal/gtfs/realtime.go | 26 +++++++- internal/gtfs/realtime_test.go | 38 +++++++++++ internal/restapi/situation_handler.go | 17 ++++- internal/restapi/situation_handler_test.go | 75 +++++++++++++++++++--- 4 files changed, 142 insertions(+), 14 deletions(-) diff --git a/internal/gtfs/realtime.go b/internal/gtfs/realtime.go index d2150a561..a03ec6adc 100644 --- a/internal/gtfs/realtime.go +++ b/internal/gtfs/realtime.go @@ -10,12 +10,14 @@ import ( "math/rand" "net/http" "slices" + "strings" "sync" "time" "github.com/OneBusAway/go-gtfs" gtfsrt "github.com/OneBusAway/go-gtfs/proto" "maglev.onebusaway.org/internal/logging" + "maglev.onebusaway.org/internal/utils" ) // alertIndex holds pre-built maps for O(1) alert lookups, keyed by trip, route, agency, and stop IDs. @@ -202,7 +204,8 @@ func (manager *Manager) GetAlertsForStop(stopID string) []gtfs.Alert { } // GetAllAlerts returns all deduplicated realtime alerts across all feeds. -// Deduplication is by alert ID, preserving first-seen order by sorted feed ID. +// Deduplication is by agency-qualified alert ID, preserving first-seen order +// by sorted feed ID, so two agencies publishing the same raw ID both survive. func (manager *Manager) GetAllAlerts() []gtfs.Alert { manager.realTimeMutex.RLock() defer manager.realTimeMutex.RUnlock() @@ -220,10 +223,11 @@ func (manager *Manager) GetAllAlerts() []gtfs.Alert { if alert.ID == "" { continue } - if _, exists := seen[alert.ID]; exists { + key := qualifiedAlertID(alert) + if _, exists := seen[key]; exists { continue } - seen[alert.ID] = struct{}{} + seen[key] = struct{}{} alerts = append(alerts, alert) } } @@ -231,6 +235,22 @@ func (manager *Manager) GetAllAlerts() []gtfs.Alert { return alerts } +// qualifiedAlertID is the agency-and-id form used as the GetAllAlerts dedup +// key. It matches restapi.situationID(alert.ID, agencyIDForAlert(alert, "")). +func qualifiedAlertID(alert gtfs.Alert) string { + agencyID := "" + for _, entity := range alert.InformedEntities { + if entity.AgencyID != nil && *entity.AgencyID != "" { + agencyID = *entity.AgencyID + break + } + } + if agencyID == "" || strings.HasPrefix(alert.ID, agencyID+"_") { + return alert.ID + } + return utils.FormCombinedID(agencyID, alert.ID) +} + // Fetches GTFS-RT data from a URL with per-feed headers. func loadRealtimeData(ctx context.Context, source string, headers map[string]string) (*gtfs.Realtime, error) { req, err := http.NewRequestWithContext(ctx, "GET", source, nil) diff --git a/internal/gtfs/realtime_test.go b/internal/gtfs/realtime_test.go index bc369fd1d..e953ac2cc 100644 --- a/internal/gtfs/realtime_test.go +++ b/internal/gtfs/realtime_test.go @@ -71,6 +71,44 @@ func TestGetAlertsForStop(t *testing.T) { assert.Equal(t, "alert1", alerts[0].ID) } +func TestGetAllAlerts(t *testing.T) { + agency1 := "1" + agency40 := "40" + manager := &Manager{ + realTimeMutex: sync.RWMutex{}, + feedAlerts: map[string][]gtfs.Alert{ + "feed-b": { + { + ID: "shared-raw", + InformedEntities: []gtfs.AlertInformedEntity{{AgencyID: &agency1}}, + Header: []gtfs.AlertText{{Text: "from-b"}}, + }, + {ID: ""}, + }, + "feed-a": { + { + ID: "shared-raw", + InformedEntities: []gtfs.AlertInformedEntity{{AgencyID: &agency1}}, + Header: []gtfs.AlertText{{Text: "from-a"}}, + }, + {ID: "shared-raw", InformedEntities: []gtfs.AlertInformedEntity{{AgencyID: &agency40}}}, + }, + }, + } + + alerts := manager.GetAllAlerts() + + require.Len(t, alerts, 2) + assert.Equal(t, "shared-raw", alerts[0].ID) + require.NotNil(t, alerts[0].InformedEntities[0].AgencyID) + assert.Equal(t, "1", *alerts[0].InformedEntities[0].AgencyID) + require.NotEmpty(t, alerts[0].Header) + assert.Equal(t, "from-a", alerts[0].Header[0].Text) + assert.Equal(t, "shared-raw", alerts[1].ID) + require.NotNil(t, alerts[1].InformedEntities[0].AgencyID) + assert.Equal(t, "40", *alerts[1].InformedEntities[0].AgencyID) +} + func TestRebuildRealTimeTripLookup(t *testing.T) { manager := &Manager{ realTimeMutex: sync.RWMutex{}, diff --git a/internal/restapi/situation_handler.go b/internal/restapi/situation_handler.go index 118ce5e8b..a1c0023e0 100644 --- a/internal/restapi/situation_handler.go +++ b/internal/restapi/situation_handler.go @@ -5,20 +5,22 @@ import ( "github.com/OneBusAway/go-gtfs" "maglev.onebusaway.org/internal/models" + "maglev.onebusaway.org/internal/utils" ) // situationHandler serves a single GTFS-RT service alert (OneBusAway "Situation") // by its alert id. func (api *RestAPI) situationHandler(w http.ResponseWriter, r *http.Request) { - situationID, ok := api.extractAndValidateID(w, r) + agencyID, codeID, ok := api.extractAndValidateAgencyCodeID(w, r) if !ok { return } + requestID := utils.FormCombinedID(agencyID, codeID) var alert gtfs.Alert found := false for _, candidate := range api.GtfsManager.GetAllAlerts() { - if candidate.ID == situationID { + if situationAlertMatches(candidate, requestID) { alert = candidate found = true break @@ -30,7 +32,18 @@ func (api *RestAPI) situationHandler(w http.ResponseWriter, r *http.Request) { } situations := api.BuildSituationReferences([]gtfs.Alert{alert}) + situations[0].ID = situationID(alert.ID, agencyIDForAlert(alert, "")) references := models.NewEmptyReferences() response := models.NewEntryResponse(situations[0], *references, api.Clock) api.sendResponse(w, r, response) } + +// situationAlertMatches reports whether a GTFS-RT alert is the situation named +// by requestID. Clients following situationIds from trip-details use the +// agency-qualified form; some feeds already store that form as alert.ID. +func situationAlertMatches(alert gtfs.Alert, requestID string) bool { + if alert.ID == requestID { + return true + } + return situationID(alert.ID, agencyIDForAlert(alert, "")) == requestID +} diff --git a/internal/restapi/situation_handler_test.go b/internal/restapi/situation_handler_test.go index fa8306056..86e45586d 100644 --- a/internal/restapi/situation_handler_test.go +++ b/internal/restapi/situation_handler_test.go @@ -19,23 +19,58 @@ func TestSituationHandlerRequiresValidAPIKey(t *testing.T) { assert.Equal(t, "permission denied", model.Text) } -func TestSituationHandlerNotFound(t *testing.T) { +func TestSituationHandlerErrors(t *testing.T) { api := createTestApi(t) defer api.Shutdown() - resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/nonexistent-alert.json?key=TEST") - assert.Equal(t, http.StatusNotFound, resp.StatusCode) - assert.Equal(t, http.StatusNotFound, model.Code) - assert.Equal(t, "resource not found", model.Text) + tests := []struct { + name string + pathID string + expectedStatus int + expectedText string + }{ + { + name: "unknown but well-formed ID", + pathID: "25_nonexistent-alert", + expectedStatus: http.StatusNotFound, + expectedText: "resource not found", + }, + { + name: "malformed ID without agency separator", + pathID: "nonexistent-alert", + expectedStatus: http.StatusBadRequest, + }, + { + name: "invalid characters", + pathID: "bad*id", + expectedStatus: http.StatusBadRequest, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/"+tt.pathID+".json?key=TEST") + assert.Equal(t, tt.expectedStatus, resp.StatusCode) + assert.Equal(t, tt.expectedStatus, model.Code) + if tt.expectedText != "" { + assert.Equal(t, tt.expectedText, model.Text) + } + }) + } } func TestSituationHandlerWithSituation(t *testing.T) { api := createTestApi(t) defer api.Shutdown() - const alertID = "situation-handler-alert" + agencyID := "25" + const rawAlertID = "situation-handler-alert" + qualifiedID := "25_situation-handler-alert" alert := gtfs.Alert{ - ID: alertID, + ID: rawAlertID, + InformedEntities: []gtfs.AlertInformedEntity{ + {AgencyID: &agencyID}, + }, Header: []gtfs.AlertText{ {Text: "Service disruption", Language: "en"}, }, @@ -45,7 +80,7 @@ func TestSituationHandlerWithSituation(t *testing.T) { } api.GtfsManager.AddAlertForTest(alert) - resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/"+alertID+".json?key=TEST") + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/"+qualifiedID+".json?key=TEST") assert.Equal(t, http.StatusOK, resp.StatusCode) assert.Equal(t, http.StatusOK, model.Code) assert.Equal(t, "OK", model.Text) @@ -56,7 +91,7 @@ func TestSituationHandlerWithSituation(t *testing.T) { entry, ok := data["entry"].(map[string]interface{}) require.True(t, ok, "response should include data.entry object") - assert.Equal(t, alertID, entry["id"]) + assert.Equal(t, qualifiedID, entry["id"]) assert.Equal(t, "UNKNOWN_CAUSE", entry["reason"]) assert.Equal(t, "noImpact", entry["severity"]) @@ -85,3 +120,25 @@ func TestSituationHandlerWithSituation(t *testing.T) { require.True(t, ok) assert.Len(t, stops, 0) } + +func TestSituationHandlerMatchesPrefixedAlertID(t *testing.T) { + api := createTestApi(t) + defer api.Shutdown() + + const prefixedID = "40_situation-handler-prefixed" + api.GtfsManager.AddAlertForTest(gtfs.Alert{ + ID: prefixedID, + Header: []gtfs.AlertText{ + {Text: "Already prefixed", Language: "en"}, + }, + }) + + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/situation/"+prefixedID+".json?key=TEST") + assert.Equal(t, http.StatusOK, resp.StatusCode) + + data, ok := model.Data.(map[string]interface{}) + require.True(t, ok) + entry, ok := data["entry"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, prefixedID, entry["id"]) +}