Make situation references resolve on every endpoint

internal/restapi/arrival_and_departure_for_stop_handler.go

@@ -357,7 +357,7 @@ func (api *RestAPI) arrivalAndDepartureForStopHandler(w http.ResponseWriter, r *
 	// carry a wall-clock time portion) for all schedule math — matches Java's
 	// BlockInstance contract ("midnight time relative to stop times"; see
 	// BlockInstance.java:69-72) and the plural handler's convention.
-	status, snapshot, statusErr := api.BuildTripStatus(ctx, route.AgencyID, tripID, nil, serviceMidnight, currentTime)
+	status, statusExtras, statusErr := api.BuildTripStatus(ctx, route.AgencyID, tripID, nil, serviceMidnight, currentTime)
 	if statusErr != nil {
 		api.Logger.Warn("BuildTripStatus failed",
 			"tripID", tripID, "error", statusErr)
@@ -385,8 +385,8 @@ func (api *RestAPI) arrivalAndDepartureForStopHandler(w http.ResponseWriter, r *
 		// Reuse the snapshot BuildTripStatus already computed for this trip.
 		// It applies the same schedule-deviation shift internally, so
 		// recomputing here just to run metricsForStop was duplicating work.
-		if snapshot != nil {
-			if d, n, ok := snapshot.metricsForStop(tripID, int(targetStopTime.StopSequence)); ok {
+		if statusExtras.snapshot != nil {
+			if d, n, ok := statusExtras.snapshot.metricsForStop(tripID, int(targetStopTime.StopSequence)); ok {
 				distanceFromStop = d
 				numberOfStopsAway = n
 			}
@@ -398,7 +398,7 @@ func (api *RestAPI) arrivalAndDepartureForStopHandler(w http.ResponseWriter, r *
 	blockTripSequence := api.calculateBlockTripSequence(ctx, tripID, serviceMidnight)
 
 	lastUpdateTime := api.GtfsManager.GetVehicleLastUpdateTime(vehicle)
-	situationIDs := api.GetSituationIDsForTrip(r.Context(), tripID)
+	situationIDs, situationRefs := api.situationsFromRefs(statusExtras.situations)
 
 	arrival := models.NewArrivalAndDeparture(
 		utils.FormCombinedID(route.AgencyID, route.ID), // routeID
@@ -595,13 +595,7 @@ func (api *RestAPI) arrivalAndDepartureForStopHandler(w http.ResponseWriter, r *
 	}
 	references.Routes = utils.MapValues(routeRefs)
 
-	if len(situationIDs) > 0 {
-		alerts := api.GtfsManager.GetAlertsForTrip(r.Context(), tripID)
-		if len(alerts) > 0 {
-			situations := api.BuildSituationReferences(alerts)
-			references.Situations = append(references.Situations, situations...)
-		}
-	}
+	references.Situations = append(references.Situations, situationRefs...)
 
 	response := models.NewEntryResponse(arrival, *references, api.Clock)
 	api.sendResponse(w, r, response)

internal/restapi/arrivals_and_departures_for_stop_handler.go

@@ -10,7 +10,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/OneBusAway/go-gtfs"
 	"maglev.onebusaway.org/gtfsdb"
 	internalgtfs "maglev.onebusaway.org/internal/gtfs"
 	"maglev.onebusaway.org/internal/models"
@@ -144,7 +143,7 @@ func (api *RestAPI) arrivalsAndDeparturesForStopHandler(w http.ResponseWriter, r
 	addedAgencyIDs := make(map[string]bool)
 	addedAgencyIDs[agency.ID] = true
 
-	collectedAlerts := make(map[string]gtfs.Alert)
+	situations := newSituationCollector()
 	alertAgencyID := stopAgencyID
 
 	type activeStopTime struct {
@@ -366,7 +365,7 @@ func (api *RestAPI) arrivalsAndDeparturesForStopHandler(w http.ResponseWriter, r
 
 		// Always built — Java attaches a BlockLocation (real-time or scheduled) to
 		// every arrival, so tripStatus is always non-null.
-		status, snapshot, statusErr := api.BuildTripStatus(ctx, route.AgencyID, st.TripID, nil, serviceMidnight, params.Time)
+		status, statusExtras, statusErr := api.BuildTripStatus(ctx, route.AgencyID, st.TripID, nil, serviceMidnight, params.Time)
 		if statusErr != nil {
 			api.Logger.Warn("BuildTripStatus failed for arrival",
 				"tripID", st.TripID, "error", statusErr)
@@ -392,8 +391,8 @@ func (api *RestAPI) arrivalsAndDeparturesForStopHandler(w http.ResponseWriter, r
 			// so recomputing here just to run metricsForStop was doubling every
 			// per-arrival snapshot cost — a real problem on the plural handler
 			// where minutesBefore/minutesAfter can be 24h in each direction.
-			if snapshot != nil {
-				if d, n, ok := snapshot.metricsForStop(st.TripID, int(st.StopSequence)); ok {
+			if statusExtras.snapshot != nil {
+				if d, n, ok := statusExtras.snapshot.metricsForStop(st.TripID, int(st.StopSequence)); ok {
 					distanceFromStop = d
 					numberOfStopsAway = n
 				}
@@ -444,17 +443,7 @@ func (api *RestAPI) arrivalsAndDeparturesForStopHandler(w http.ResponseWriter, r
 		lastUpdateTime := api.GtfsManager.GetVehicleLastUpdateTime(vehicle)
 
 		tripAlerts := api.GtfsManager.GetAlertsForTrip(r.Context(), st.TripID)
-		situationIDs := make([]string, 0, len(tripAlerts))
-		for _, alert := range tripAlerts {
-			if alert.ID == "" {
-				continue
-			}
-
-			situationIDs = append(situationIDs, utils.FormCombinedID(route.AgencyID, alert.ID))
-			if _, seen := collectedAlerts[alert.ID]; !seen {
-				collectedAlerts[alert.ID] = alert
-			}
-		}
+		situationIDs := situations.add(tripAlerts, route.AgencyID)
 
 		if alertAgencyID == "" && route.AgencyID != "" {
 			alertAgencyID = route.AgencyID
@@ -629,30 +618,15 @@ func (api *RestAPI) arrivalsAndDeparturesForStopHandler(w http.ResponseWriter, r
 		}
 	}
 
-	for _, alert := range api.GtfsManager.GetAlertsForStop(stopCode) {
-		if alert.ID != "" {
-			if _, seen := collectedAlerts[alert.ID]; !seen {
-				collectedAlerts[alert.ID] = alert
-			}
-		}
-	}
+	situations.add(api.GtfsManager.GetAlertsForStop(stopCode), alertAgencyID)
 
-	if len(collectedAlerts) > 0 {
-		alertSlice := make([]gtfs.Alert, 0, len(collectedAlerts))
-		for _, a := range collectedAlerts {
-			alertSlice = append(alertSlice, a)
-		}
-		situations := api.BuildSituationReferences(alertSlice)
-		references.Situations = append(references.Situations, situations...)
-	}
+	references.Situations = append(references.Situations, api.situationReferences(situations.refs)...)
 
-	topLevelSituationIDSet := make(map[string]struct{}, len(collectedAlerts))
-	for alertID := range collectedAlerts {
-		topLevelSituationIDSet[utils.FormCombinedID(alertAgencyID, alertID)] = struct{}{}
-	}
-	topLevelSituationIDs := make([]string, 0, len(topLevelSituationIDSet))
-	for id := range topLevelSituationIDSet {
-		topLevelSituationIDs = append(topLevelSituationIDs, id)
+	// The top-level list covers every alert reachable from this stop, whether it
+	// was matched through an arrival's trip or through the stop itself.
+	topLevelSituationIDs := make([]string, 0, len(situations.refs))
+	for _, ref := range situations.refs {
+		topLevelSituationIDs = append(topLevelSituationIDs, ref.ID)
 	}
 
 	nearbyStopIDs := getNearbyStopIDs(api, ctx, stop.Lat, stop.Lon, stopCode, stopAgencyID)

internal/restapi/reference_utils.go

@@ -2,12 +2,15 @@ package restapi
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/OneBusAway/go-gtfs"
 	"maglev.onebusaway.org/gtfsdb"
+	"maglev.onebusaway.org/internal/logging"
 	"maglev.onebusaway.org/internal/models"
 	"maglev.onebusaway.org/internal/nulls"
 	"maglev.onebusaway.org/internal/utils"
@@ -448,3 +451,167 @@ func (api *RestAPI) buildStopModel(ctx context.Context, agencyID string, stop gt
 		StaticRouteIDs:     combinedRouteIDs,
 	}
 }
+
+// Situation references
+//
+// An entry's situationIds and the response's references.situations must use the
+// same ID, or the IDs resolve to nothing. Everything that derives one derives
+// the other, so the two cannot drift apart.
+// situationRef pairs an alert with the situation ID that both list entries and
+// situation references use to refer to it.
+type situationRef struct {
+	ID    string
+	Alert gtfs.Alert
+}
+
+// situationRefsFromAlerts drops alerts with no ID and pairs the rest with the
+// situation ID used to refer to them.
+func situationRefsFromAlerts(alerts []gtfs.Alert, agencyID string) []situationRef {
+	refs := make([]situationRef, 0, len(alerts))
+	for _, alert := range alerts {
+		if alert.ID == "" {
+			continue
+		}
+		refs = append(refs, situationRef{ID: situationID(alert.ID, agencyIDForAlert(alert, agencyID)), Alert: alert})
+	}
+	return refs
+}
+
+// agencyIDForAlert returns the agency whose prefix an alert's situation ID
+// carries, preferring the alert's own informed entity over the caller's agency.
+//
+// One alert is reachable by more than one path: a stop served by another
+// agency's route matches the same alert through both the route and the stop, and
+// each path knows a different agency. Scoping the ID to the caller's agency
+// would then publish that alert twice under two IDs, and an entry's
+// situationIds would resolve to whichever one it happened to be built from.
+// The informed entity is the only source that does not vary by lookup path.
+func agencyIDForAlert(alert gtfs.Alert, fallbackAgencyID string) string {
+	for _, entity := range alert.InformedEntities {
+		if entity.AgencyID != nil && *entity.AgencyID != "" {
+			return *entity.AgencyID
+		}
+	}
+	return fallbackAgencyID
+}
+
+// situationID returns the combined-form ID for an alert.
+//
+// Prefixing is idempotent, not unconditional: a GTFS-RT feed exported by an OBA
+// instance already carries the agency prefix on its alert IDs — Puget Sound
+// ships "1_92239" — and prefixing again would publish "1_1_92239", which
+// resolves against nothing a client has seen. Upstream reports "1_92239".
+func situationID(alertID, agencyID string) string {
+	if agencyID == "" || strings.HasPrefix(alertID, agencyID+"_") {
+		return alertID
+	}
+	return utils.FormCombinedID(agencyID, alertID)
+}
+
+// situationCollector deduplicates the alerts referenced by list entries so the
+// response emits one situation reference per distinct situation ID.
+type situationCollector struct {
+	seen map[string]struct{}
+	refs []situationRef
+}
+
+func newSituationCollector() *situationCollector {
+	return &situationCollector{seen: make(map[string]struct{})}
+}
+
+// add records the alerts affecting one trip and returns their situation IDs.
+func (c *situationCollector) add(alerts []gtfs.Alert, agencyID string) []string {
+	return c.addRefs(situationRefsFromAlerts(alerts, agencyID))
+}
+
+// addRefs records already-resolved situation references and returns their IDs.
+func (c *situationCollector) addRefs(refs []situationRef) []string {
+	ids := make([]string, 0, len(refs))
+	for _, ref := range refs {
+		ids = append(ids, ref.ID)
+		if _, ok := c.seen[ref.ID]; ok {
+			continue
+		}
+		c.seen[ref.ID] = struct{}{}
+		c.refs = append(c.refs, ref)
+	}
+	return ids
+}
+
+// situationReferences converts collected alerts into situation references,
+// stamping the same IDs the list entries use. BuildSituationReferences emits raw
+// alert IDs and preserves input order one-for-one.
+func (api *RestAPI) situationReferences(refs []situationRef) []models.Situation {
+	if len(refs) == 0 {
+		return []models.Situation{}
+	}
+
+	alerts := make([]gtfs.Alert, 0, len(refs))
+	for _, ref := range refs {
+		alerts = append(alerts, ref.Alert)
+	}
+
+	situations := api.BuildSituationReferences(alerts)
+	if len(situations) != len(refs) {
+		// The ID stamping below pairs by position, so a length change in
+		// BuildSituationReferences would silently mislabel every situation.
+		logging.LogError(api.Logger, "situation reference count does not match the alerts it was built from",
+			fmt.Errorf("built %d situations from %d alerts", len(situations), len(refs)))
+		return situations
+	}
+
+	for i := range situations {
+		situations[i].ID = refs[i].ID
+	}
+	return situations
+}
+
+func (rb *referenceBuilder) getAgenciesList() []models.AgencyReference {
+	agencies := make([]models.AgencyReference, 0, len(rb.presentAgencies))
+	for _, agency := range rb.presentAgencies {
+		agencies = append(agencies, agency)
+	}
+	return agencies
+}
+
+func (rb *referenceBuilder) getRoutesList() []models.Route {
+	routes := make([]models.Route, 0, len(rb.presentRoutes))
+	for _, route := range rb.presentRoutes {
+		if route.ID != "" {
+			routes = append(routes, route)
+		}
+	}
+	return routes
+}
+
+// situationIDsFromRefs returns the situation IDs of already-resolved references,
+// so entry IDs are always derived the same way the references are.
+func situationIDsFromRefs(refs []situationRef) []string {
+	ids := make([]string, 0, len(refs))
+	for _, ref := range refs {
+		ids = append(ids, ref.ID)
+	}
+	return ids
+}
+
+// TripSituations returns a trip's situation IDs together with the matching
+// situation references, so an entry's situationIds always resolve.
+func (api *RestAPI) TripSituations(ctx context.Context, tripID string) ([]string, []models.Situation) {
+	return api.situationsFromRefs(api.situationRefsForTrip(ctx, tripID))
+}
+
+// situationsFromRefs splits already-resolved references into the entry IDs and
+// the reference block built from the same lookup.
+func (api *RestAPI) situationsFromRefs(refs []situationRef) ([]string, []models.Situation) {
+	return situationIDsFromRefs(refs), api.situationReferences(refs)
+}
+
+// tripSituationsFor returns a trip's situations, reusing the references
+// BuildTripStatus already resolved for the same trip rather than querying for
+// them a second time. Extras is nil when the caller skipped the status.
+func (api *RestAPI) tripSituationsFor(ctx context.Context, tripID string, extras *tripStatusExtras) ([]string, []models.Situation) {
+	if extras == nil {
+		return api.TripSituations(ctx, tripID)
+	}
+	return api.situationsFromRefs(extras.situations)
+}