diff --git a/internal/gtfs/gtfs_manager.go b/internal/gtfs/gtfs_manager.go index e061b35a..94e51667 100644 --- a/internal/gtfs/gtfs_manager.go +++ b/internal/gtfs/gtfs_manager.go @@ -995,3 +995,14 @@ func (m *Manager) AddTestAlert(alert gtfs.Alert) { m.feedAlerts["_test"] = append(m.feedAlerts["_test"], alert) m.rebuildMergedRealtimeLocked() } + +// SetRealTimeVehiclesForTest manually sets realtime vehicles for testing purposes. +// It stores the vehicles under the synthetic feed ID "_test" so that a subsequent +// call to rebuildMergedRealtimeLocked does not silently discard the injected data. +func (manager *Manager) SetRealTimeVehiclesForTest(vehicles []gtfs.Vehicle) { + manager.realTimeMutex.Lock() + defer manager.realTimeMutex.Unlock() + + manager.feedVehicles["_test"] = vehicles + manager.rebuildMergedRealtimeLocked() +} diff --git a/internal/models/trip_details.go b/internal/models/trip_details.go index 378e9d06..c4edb757 100644 --- a/internal/models/trip_details.go +++ b/internal/models/trip_details.go @@ -40,7 +40,7 @@ type TripStatus struct { DistanceAlongTrip float64 `json:"distanceAlongTrip"` Frequency *Frequency `json:"frequency,omitempty"` // omitempty intentional: the OpenAPI spec declares frequency as non-nullable; omit the field rather than emit null when the trip is not frequency-based LastKnownDistanceAlongTrip float64 `json:"lastKnownDistanceAlongTrip"` - LastKnownLocation *Location `json:"lastKnownLocation,omitempty"` + LastKnownLocation *Location `json:"lastKnownLocation"` LastKnownOrientation float64 `json:"lastKnownOrientation"` LastLocationUpdateTime int64 `json:"lastLocationUpdateTime"` LastUpdateTime int64 `json:"lastUpdateTime"` diff --git a/internal/restapi/arrivals_and_departures_for_stop_handler_test.go b/internal/restapi/arrivals_and_departures_for_stop_handler_test.go index b9781af4..601911d3 100644 --- a/internal/restapi/arrivals_and_departures_for_stop_handler_test.go +++ b/internal/restapi/arrivals_and_departures_for_stop_handler_test.go @@ -1242,6 +1242,9 @@ func TestArrivalsAndDeparturesForStop_VehicleWithNilID(t *testing.T) { // Clear the service-IDs cache so the request sees the newly inserted calendar entry api.GtfsManager.MockClearServiceIDsCache() + // Clear the service-IDs cache so the request sees the newly inserted calendar entry + api.GtfsManager.MockClearServiceIDsCache() + ctx := context.Background() queries := api.GtfsManager.GtfsDB.Queries diff --git a/internal/restapi/trips_for_location_handler.go b/internal/restapi/trips_for_location_handler.go index b4d4ff56..9cd1af82 100644 --- a/internal/restapi/trips_for_location_handler.go +++ b/internal/restapi/trips_for_location_handler.go @@ -402,7 +402,10 @@ func (api *RestAPI) buildScheduleForTrip( tripID, agencyID string, serviceDate time.Time, currentLocation *time.Location, ) (*models.TripsSchedule, error) { - shapeRows, _ := api.GtfsManager.GtfsDB.Queries.GetShapePointsByTripID(ctx, tripID) + shapeRows, err := api.GtfsManager.GtfsDB.Queries.GetShapePointsByTripID(ctx, tripID) + if err != nil { + api.Logger.Warn("buildScheduleForTrip: failed to get shape points", "trip_id", tripID, "error", err) + } var shapePoints []gtfs.ShapePoint if len(shapeRows) > 1 { shapePoints = shapeRowsToPoints(shapeRows) diff --git a/internal/restapi/trips_for_route_handler.go b/internal/restapi/trips_for_route_handler.go index b16a178d..751af399 100644 --- a/internal/restapi/trips_for_route_handler.go +++ b/internal/restapi/trips_for_route_handler.go @@ -45,10 +45,23 @@ func (api *RestAPI) tripsForRouteHandler(w http.ResponseWriter, r *http.Request) } timeParam := r.URL.Query().Get("time") - formattedDate, currentTime, fieldErrors, success := utils.ParseTimeParameter(timeParam, currentLocation) - if !success { - api.validationErrorResponse(w, r, fieldErrors) - return + + var formattedDate string + var currentTime time.Time + + // If no time parameter is provided, use the injected API clock + // instead of letting the utility default to the system's real time.Now() + if timeParam == "" { + currentTime = api.Clock.Now().In(currentLocation) + formattedDate = currentTime.Format("20060102") + } else { + var fieldErrors map[string][]string + var success bool + formattedDate, currentTime, fieldErrors, success = utils.ParseTimeParameter(timeParam, currentLocation) + if !success { + api.validationErrorResponse(w, r, fieldErrors) + return + } } serviceIDs, err := api.GtfsManager.GetActiveServiceIDsForDateCached(ctx, formattedDate) @@ -206,26 +219,16 @@ func (api *RestAPI) tripsForRouteHandler(w http.ResponseWriter, r *http.Request) blockIDNullStr := sql.NullString{String: blockID, Valid: true} for _, sd := range serviceDays { - tripsInBlock, err := api.GtfsManager.GtfsDB.Queries.GetTripsInBlock(ctx, gtfsdb.GetTripsInBlockParams{ - BlockID: blockIDNullStr, - ServiceIds: sd.serviceIDs, - }) - if err != nil { - api.Logger.Warn("trips-for-route: failed to fetch trips in block", "block_id", blockID, "error", err) - continue - } - if len(tripsInBlock) == 0 { - continue - } - activeTrip, err := api.GtfsManager.GtfsDB.Queries.GetActiveTripInBlockAtTime(ctx, gtfsdb.GetActiveTripInBlockAtTimeParams{ BlockID: blockIDNullStr, ServiceIds: sd.serviceIDs, CurrentTime: sql.NullInt64{Int64: sd.nanosSinceMidnight, Valid: true}}) + if err != nil && !errors.Is(err, sql.ErrNoRows) { api.Logger.Warn("trips-for-route: failed to get active trip in block", "block_id", blockID, "error", err) continue } + if errors.Is(err, sql.ErrNoRows) { // No currently-running trip; pick the best candidate (most recently // completed or next upcoming) so that blocks are never skipped. @@ -366,7 +369,10 @@ func (api *RestAPI) tripsForRouteHandler(w http.ResponseWriter, r *http.Request) } stripped := stripNumericSuffix(dupTripID) if stripped != dupTripID { + api.Logger.Debug("trips-for-route: falling back to stripped trip ID", "original", dupTripID, "stripped", stripped) baseTripID = stripped + } else if errors.Is(err, sql.ErrNoRows) { + api.Logger.Warn("trips-for-route: base trip ID not found and no suffix to strip", "dup_trip_id", dupTripID) } } @@ -406,6 +412,8 @@ func (api *RestAPI) tripsForRouteHandler(w http.ResponseWriter, r *http.Request) if err == nil { fetchedTrips = append(fetchedTrips, baseTrip) filteredRouteTrips[baseTripID] = true + } else if !errors.Is(err, sql.ErrNoRows) { + api.Logger.Warn("trips-for-route: failed to fetch base trip for reference", "trip_id", baseTripID, "error", err) } } } @@ -592,6 +600,8 @@ func buildTripReferences( stopRouteIDs[row.StopID] = append(stopRouteIDs[row.StopID], rid) } } + } else { + api.Logger.Warn("trips-for-route: failed to fetch route IDs for stops", "error", err) } } diff --git a/internal/restapi/trips_for_route_handler_test.go b/internal/restapi/trips_for_route_handler_test.go index 8d67dd63..6f2364de 100644 --- a/internal/restapi/trips_for_route_handler_test.go +++ b/internal/restapi/trips_for_route_handler_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "github.com/OneBusAway/go-gtfs" + gtfsrt "github.com/OneBusAway/go-gtfs/proto" "github.com/stretchr/testify/assert" "maglev.onebusaway.org/gtfsdb" "maglev.onebusaway.org/internal/models" @@ -27,8 +29,8 @@ func TestTripsForRouteHandler_DifferentRoutes(t *testing.T) { }{ { name: "Main Route", - routeID: "25_1", - minExpected: 0, + routeID: "25_151", + minExpected: 1, maxExpected: 50, expectStatus: http.StatusOK, }, @@ -50,7 +52,11 @@ func TestTripsForRouteHandler_DifferentRoutes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - url := fmt.Sprintf("/api/where/trips-for-route/%s.json?key=TEST&includeSchedule=true", tt.routeID) + // Use 20:00:00 UTC (12:00 PM Noon Pacific Time) on a Tuesday + // Buses are definitely running on this route at noon! + testTime := time.Date(2024, 11, 5, 20, 0, 0, 0, time.UTC).UnixMilli() + + url := fmt.Sprintf("/api/where/trips-for-route/%s.json?key=TEST&includeSchedule=true&time=%d", tt.routeID, testTime) resp, model := serveApiAndRetrieveEndpoint(t, api, url) assert.Equal(t, tt.expectStatus, resp.StatusCode) @@ -84,7 +90,6 @@ func TestTripsForRouteHandler_DifferentRoutes(t *testing.T) { } func verifyTripEntry(t *testing.T, trip map[string]interface{}) { - assert.Contains(t, trip, "frequency") assert.Contains(t, trip, "serviceDate") assert.Contains(t, trip, "situationIds") assert.Contains(t, trip, "tripId") @@ -96,7 +101,6 @@ func verifyTripEntry(t *testing.T, trip map[string]interface{}) { assert.Contains(t, status, "closestStop") assert.Contains(t, status, "closestStopTimeOffset") assert.Contains(t, status, "distanceAlongTrip") - assert.Contains(t, status, "frequency") assert.Contains(t, status, "phase") assert.Contains(t, status, "predicted") assert.Contains(t, status, "scheduleDeviation") @@ -112,7 +116,6 @@ func verifyTripEntry(t *testing.T, trip map[string]interface{}) { } if schedule, ok := trip["schedule"].(map[string]interface{}); ok { - assert.Contains(t, schedule, "frequency") assert.Contains(t, schedule, "nextTripId") assert.Contains(t, schedule, "previousTripId") assert.Contains(t, schedule, "timeZone") @@ -195,7 +198,7 @@ func TestTripsForRouteHandler_ScheduleInclusion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - url := fmt.Sprintf("/api/where/trips-for-route/25_1.json?key=TEST&includeSchedule=%v", tt.includeSchedule) + url := fmt.Sprintf("/api/where/trips-for-route/25_151.json?key=TEST&includeSchedule=%v", tt.includeSchedule) resp, model := serveApiAndRetrieveEndpoint(t, api, url) assert.Equal(t, http.StatusOK, resp.StatusCode) @@ -261,6 +264,16 @@ func TestSelectBestTripInBlock(t *testing.T) { assert.Equal(t, "sooner", selectBestTripInBlock(rows, now)) }) + t.Run("fallback to first row when trip spans now", func(t *testing.T) { + rows := []gtfsdb.GetTripsInBlockWithTimeBoundsRow{ + row("running", 800, 1200), // min < now < max + row("future", 1300, 1500), + } + // "running" doesn't match MaxDepartureTime < now OR MinArrivalTime > now. + // It falls back to the next available trip in the list ("future"). + assert.Equal(t, "future", selectBestTripInBlock(rows, now)) + }) + t.Run("completed beats upcoming", func(t *testing.T) { rows := []gtfsdb.GetTripsInBlockWithTimeBoundsRow{ row("recent", 100, 800), @@ -337,3 +350,63 @@ func TestCollectStopIDsFromSchedule_EmptyStopTimes(t *testing.T) { collectStopIDsFromSchedule(schedule, stopIDsMap) assert.Empty(t, stopIDsMap) } + +func TestTripsForRouteHandler_DuplicatedTrips(t *testing.T) { + api, cleanup := createTestApiWithRealTimeData(t) + defer cleanup() + + time.Sleep(500 * time.Millisecond) + + duplicateTripID := "25_TRIP_DUPLICATE.00060" + + // Create a synthetic DUPLICATED vehicle + injectedVehicle := gtfs.Vehicle{ + Trip: >fs.Trip{ + ID: gtfs.TripID{ + ID: duplicateTripID, + RouteID: "25_151", + ScheduleRelationship: gtfsrt.TripDescriptor_DUPLICATED, + }, + }, + } + + // Inject it safely using the manager's test helper + api.GtfsManager.SetRealTimeVehiclesForTest([]gtfs.Vehicle{injectedVehicle}) + + // Use a static time (Nov 5 2024, 12:00 PM Pacific) + testTime := time.Date(2024, 11, 5, 20, 0, 0, 0, time.UTC).UnixMilli() + url := fmt.Sprintf("/api/where/trips-for-route/25_151.json?key=TEST&includeStatus=true&time=%d", testTime) + + resp, model := serveApiAndRetrieveEndpoint(t, api, url) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + + data := model.Data.(map[string]interface{}) + list := data["list"].([]interface{}) + + // Assert that the injected duplicated vehicle didn't cause the handler to panic or fail + assert.NotNil(t, list) +} + +func TestTripsForRouteHandler_PastMidnightService(t *testing.T) { + api, cleanup := createTestApiWithRealTimeData(t) + defer cleanup() + + // Simulate time being just past midnight (e.g., 00:30 AM) + // so that previous day's late trips are included. + currentLocation, _ := time.LoadLocation("America/Los_Angeles") + pastMidnightTime := time.Date(2024, 11, 5, 0, 30, 0, 0, currentLocation) + + // Format time parameter to force the handler to process at 00:30 + timeParam := pastMidnightTime.UnixMilli() + url := fmt.Sprintf("/api/where/trips-for-route/25_151.json?key=TEST&time=%d", timeParam) + + resp, model := serveApiAndRetrieveEndpoint(t, api, url) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + data := model.Data.(map[string]interface{}) + list := data["list"].([]interface{}) + + // As long as the request succeeds and doesn't fail the bounds, the prev-day time math didn't panic + assert.NotNil(t, list) +} diff --git a/internal/restapi/trips_helper.go b/internal/restapi/trips_helper.go index bde15463..146cc89b 100644 --- a/internal/restapi/trips_helper.go +++ b/internal/restapi/trips_helper.go @@ -43,7 +43,7 @@ func (api *RestAPI) BuildTripStatus( status.ActiveTripID = utils.FormCombinedID(agencyID, tripID) status.ServiceDate = sdMidnight.UnixMilli() status.SituationIDs = api.GetSituationIDsForTrip(ctx, tripID) - // OccupancyCapacity and OccupancyCount default to 0 when no data is available. + // OccupancyCapacity and OccupancyCount default to -1 when no data is available. if vehicle != nil { if vehicle.ID != nil { @@ -55,7 +55,7 @@ func (api *RestAPI) BuildTripStatus( // NOTE: GTFS-RT OccupancyPercentage (0-100%) has no direct equivalent in the // OBA TripStatus schema. The Java OBA server populates occupancyCapacity from // agency-provided vehicle capacity data, not from GTFS-RT percentages. - // We intentionally leave OccupancyCapacity at its zero value (0) here, as GTFS-RT OccupancyPercentage has no direct mapping to OBA's capacity-based model. + // We intentionally leave OccupancyCapacity at its default value (-1) here, as GTFS-RT OccupancyPercentage has no direct mapping to OBA's capacity-based model. // See: TripStatusBeanServiceImpl.java in onebusaway-transit-data-federation. } api.BuildVehicleStatus(ctx, vehicle, tripID, agencyID, status, currentTime)