fix: trips-for-route logging, test coverage, and performance follow-ups

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()
+}

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"`

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
 

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)

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)
 		}
 	}
 

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: &gtfs.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)
+}

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)