feat: GTFS Frequencies Phase 3 (API Integration & Handlers)

[3rabiii]

Description

This PR implements Phase 3 (the final phase) of the GTFS Frequencies feature. It wires the database storage and core logic (built in Phases 1 & 2) #582 directly into the API handlers and response models.

Key Changes

This PR is cleanly separated into 3 logical commits to make reviewing easier:

1. Core Models & Single-Trip Handlers:

• Added the new ScheduleFrequency struct to stop_time_schedule.go for the schedule endpoints.
• Integrated NewFrequencyFromDB for proper Nanosecond to Epoch MS time conversion.
• Updated trip_details_handler.go, trip_for_vehicle_handler.go, and arrivals_and_departure_for_stop.go to look up and populate the Frequency object for frequency-based trips (handling both exact_times=0 active window matching and exact_times=1).

2. Batch Queries & Schedule Handlers:

• Updated schedule_for_stop and schedule_for_route handlers to build ScheduleFrequencies arrays and expand virtual stop times for exact_times=1 trips.
• Updated trips_for_route and trips_for_location to use batch queries (GetFrequenciesForTrips) to populate the headway *int64 field efficiently without N+1 query issues.

3. Testing:

• Consolidated all frequency testing into a new comprehensive suite: frequency_integration_test.go.
• Includes subtests for all affected handlers, covering exact_times=0, exact_times=1, and null/empty fallbacks.
• Added a regression suite to ensure endpoints still behave perfectly when no frequency data is present.

@aaronbrethorst
closes : #666

[aaronbrethorst]

Adel, this is a clean Phase 3 — wiring the frequency data through every handler that needs it, with batch queries to avoid N+1 problems and a comprehensive test suite covering both exact_times modes. The ScheduleFrequency type for the schedule endpoints is the right call since those responses need serviceId and tripId context that the trip-details Frequency struct doesn't carry. The test structure with insertFrequencyData + deferred cleanup is solid.

Two items need fixing before merge, plus a suggestion for a follow-up cleanup.

Important

1. time.Now() used instead of api.Clock.Now() in two places — breaks mock clock testing

   • internal/restapi/trips_helper.go:223:

     active := gtfsInternal.GetActiveHeadwayForTime(frequencies, serviceDate, time.Now().In(serviceDate.Location()))

     Should be:

     active := gtfsInternal.GetActiveHeadwayForTime(frequencies, serviceDate, api.Clock.Now().In(serviceDate.Location()))

   • internal/restapi/trips_for_location_handler.go:821:

     serviceDate := time.Date(time.Now().Year(), time.Now().Month(), time.Now().Day(), 0, 0, 0, 0, currentLocation)

     Should use api.Clock.Now(). The function already receives a currentLocation — it just needs the clock-aware time.

   The rest of the codebase consistently uses api.Clock.Now() (e.g., trips_for_location_handler.go:40). Using time.Now() means these code paths will produce wrong results in tests and make the frequency window lookup non-deterministic relative to the mock clock.

2. Tests silently pass when the endpoint doesn't return 200 (frequency_integration_test.go, multiple locations)

   Many test cases guard their assertions with if model.Code == 200 { ... }, which means the test passes without verifying anything if the endpoint returns an error. Examples:

   • TestTripForVehicleFrequencyIntegration lines 510, 546
   • TestTripsForLocationFrequencyIntegration lines 442, 460
   • TestSingularArrivalAndDepartureFrequencyIntegration lines 862, 879

   These should use require.Equal(t, 200, model.Code) before the assertions. If the endpoint can legitimately return non-200 (e.g., trip not serving this stop), use t.Skip() with a clear reason rather than silently passing.

Suggestions

3. Extract the repeated frequency lookup into a helper (trips_helper.go)

   The same ~15-line block appears in four places:

   • trip_details_handler.go:175-207
   • trip_for_vehicle_handler.go:113-143
   • trips_helper.go:214-232
   • arrival_and_departure_for_stop_handler.go:389-404

   Something like:

   func (api *RestAPI) lookupTripFrequency(ctx context.Context, tripID string, serviceDate time.Time, now time.Time) *models.Frequency {
       frequencies, err := api.GtfsManager.GetFrequenciesForTrip(ctx, tripID)
       if err != nil || len(frequencies) == 0 {
           return nil
       }
       active := gtfsInternal.GetActiveHeadwayForTime(frequencies, serviceDate, now)
       if active == nil {
           active = &frequencies[0]
       }
       freq := models.NewFrequencyFromDB(*active, serviceDate)
       return &freq
   }

   This would reduce each call site to a single line and make the fallback-to-first-entry behavior consistent everywhere.