fix(openapi): align arrivals-and-departures-for-stop time + arrival m…

[Adityatorgal17]

Align arrivals-and-departures-for-stop with OpenAPI spec + fix ArrivalAndDeparture schema drift

Fixes #724

Problem

• Handler parsed time as Unix milliseconds instead of RFC3339 date-time as the OpenAPI spec requires, breaking spec-compliant requests.
• ArrivalAndDeparture model had drifted from the OpenAPI ArrivalDepartureForStop schema (interface{}/null shapes, missing omitempty on optional fields).
• No conformance test coverage existed for this endpoint.

Changes

Handler (arrivals_and_departure_for_stop.go)

• Parse time as RFC3339 only; return 400 on invalid input.

Handler Tests (arrivals_and_departures_for_stop_handler_test.go)

• Pass time as RFC3339 and URL-escape values so +05:30 offsets aren't decoded as spaces.

Model (arrival_and_departure.go)

• Replace interface{}/null fields with string; add omitempty on optional fields to match spec.

Model Tests (arrival_and_departure_test.go)

• Assert optional fields are absent in marshaled JSON when empty.

Conformance (openapi_conformance_test.go)

• Add missing conformance test for this endpoint against testdata/openapi.yml.

Notes

• testdata/openapi.yml is unchanged — all fixes are implementation-side only.

Validation Passed

• make test

[aaronbrethorst]

Aditya, thanks for digging into the spec and finding the time parameter format mismatch and the interface{} fields — both are legitimate spec-compliance issues worth fixing. However, there's a conflict with an in-flight PR that needs to be resolved first.

Critical Issues (1 found)

1. Frequency field change from *Frequency to string conflicts with open PR #680

   This PR changes ArrivalAndDeparture.Frequency from *Frequency (struct pointer) to string with omitempty. While the OpenAPI spec does say frequency: type: string, PR #680 (GTFS Frequencies Phase 3) is actively building on the *Frequency type — it populates this field with real frequency data:

   // PR #680, arrivals_and_departure_for_stop.go: arrival.Frequency = api.lookupTripFrequency(ctx, tripID, serviceMidnight, currentTime) // returns *models.Frequency

   // PR #680, arrival_and_departure_for_stop_handler.go: arrival.Frequency = &freq // where freq is models.Frequency

   If this PR merges first, PR #680 will fail to compile. If #680 merges first, this PR will have merge conflicts. These two PRs need to be coordinated.

   The OpenAPI spec says type: string, but this appears to be an under-specification — the production OBA Java server returns frequency as a structured object (FrequencyBean) when present, not a string. The spec likely inherited a simplified representation. The real behavior is closer to what #680 implements.

   Resolution options:

   • Wait for #680 to merge, then update this PR to skip the Frequency field change (or update the spec to use an object schema)
   • Coordinate with the #680 author to agree on the final type

Important Issues (1 found)

2. The singular arrival-and-departure-for-stop handler still parses time as Unix ms

   This PR fixes the plural handler (arrivals_and_departure_for_stop.go) to parse time as RFC3339, but the singular handler (arrival_and_departure_for_stop_handler.go:58) still uses strconv.ParseInt:

   // arrival_and_departure_for_stop_handler.go:58 — still Unix ms if timeMs, err := strconv.ParseInt(timeStr, 10, 64); err == nil {

   Interestingly, the OpenAPI spec itself has different types for these two endpoints:

   • Plural endpoint: time: type: string, format: date-time (RFC3339)
   • Singular endpoint: time: type: integer (Unix ms)

   So the singular handler is actually correct per its own spec entry. However, this inconsistency between the two endpoints is worth noting in the PR description so future readers understand it's intentional, not an oversight.

Suggestions (1 found)

3. The omitempty changes on interval fields are correct but deserve a note

   Changing PredictedArrivalInterval, PredictedDepartureInterval, ScheduledArrivalInterval, and ScheduledDepartureInterval from interface{} (serialized as null) to string with omitempty (omitted entirely) changes the JSON wire format. Previously clients received "predictedArrivalInterval": null; now they won't see the field at all when empty. This is spec-compliant (the fields aren't marked required), but any clients relying on the key being present may break. Worth noting in the PR description.

Strengths

• The time parameter fix to RFC3339 is correct per the OpenAPI spec for this endpoint
• Good use of url.QueryEscape in tests for RFC3339 values containing + characters
• The new conformance test for arrivals-and-departures-for-stop is a valuable addition
• The omitempty test (TestArrivalAndDepartureJSON) properly verifies the fields are absent from marshaled JSON

Recommended Action

1. Resolve the Frequency type conflict with PR #680 before merging
2. Add a note to the PR description about the singular vs plural time format difference
3. After resolving #680 coordination, this should be good to merge