diff --git a/internal/models/agency.go b/internal/models/agency.go index 54566f31..4bd101eb 100644 --- a/internal/models/agency.go +++ b/internal/models/agency.go @@ -23,13 +23,13 @@ func NewAgencyCoverage(agencyID string, lat, latSpan, lon, lonSpan float64) Agen } type AgencyReference struct { - Disclaimer string `json:"disclaimer"` - Email string `json:"email"` - FareUrl string `json:"fareUrl"` + Disclaimer string `json:"disclaimer,omitempty"` + Email string `json:"email,omitempty"` + FareUrl string `json:"fareUrl,omitempty"` ID string `json:"id"` - Lang string `json:"lang"` + Lang string `json:"lang,omitempty"` Name string `json:"name"` - Phone string `json:"phone"` + Phone string `json:"phone,omitempty"` PrivateService bool `json:"privateService"` Timezone string `json:"timezone"` URL string `json:"url"` diff --git a/internal/models/routes.go b/internal/models/routes.go index 0ad64823..7d06745e 100644 --- a/internal/models/routes.go +++ b/internal/models/routes.go @@ -4,15 +4,15 @@ type RouteType int type Route struct { AgencyID string `json:"agencyId"` - Color string `json:"color"` - Description string `json:"description"` + Color string `json:"color,omitempty"` + Description string `json:"description,omitempty"` ID string `json:"id"` - LongName string `json:"longName"` - NullSafeShortName string `json:"nullSafeShortName"` - ShortName string `json:"shortName"` - TextColor string `json:"textColor"` + LongName string `json:"longName,omitempty"` + NullSafeShortName string `json:"-"` + ShortName string `json:"shortName,omitempty"` + TextColor string `json:"textColor,omitempty"` Type RouteType `json:"type"` - URL string `json:"url"` + URL string `json:"url,omitempty"` } func NewRoute(id, agencyID, shortName, longName, description string, routeType RouteType, url, color, textColor string) Route { diff --git a/internal/models/routes_test.go b/internal/models/routes_test.go index 053d848e..970768b3 100644 --- a/internal/models/routes_test.go +++ b/internal/models/routes_test.go @@ -36,16 +36,15 @@ func TestRouteCreation(t *testing.T) { func TestRouteJSON(t *testing.T) { route := Route{ - ID: "2", - AgencyID: "agency-2", - ShortName: "B", - LongName: "Airport Shuttle", - Description: "Service to the airport", - Type: RouteType(2), - URL: "https://transit.org/routes/2", - Color: "00FF00", - TextColor: "000000", - NullSafeShortName: "B", + ID: "2", + AgencyID: "agency-2", + ShortName: "B", + LongName: "Airport Shuttle", + Description: "Service to the airport", + Type: RouteType(2), + URL: "https://transit.org/routes/2", + Color: "00FF00", + TextColor: "000000", } jsonData, err := json.Marshal(route) @@ -64,7 +63,6 @@ func TestRouteJSON(t *testing.T) { assert.Equal(t, route.URL, unmarshaledRoute.URL) assert.Equal(t, route.Color, unmarshaledRoute.Color) assert.Equal(t, route.TextColor, unmarshaledRoute.TextColor) - assert.Equal(t, route.NullSafeShortName, unmarshaledRoute.NullSafeShortName) } func TestRouteWithEmptyValues(t *testing.T) { @@ -94,9 +92,10 @@ func TestRouteWithNilValuesJSON(t *testing.T) { jsonStr := string(jsonData) assert.Contains(t, jsonStr, `"id":"3"`) assert.Contains(t, jsonStr, `"agencyId":"agency-3"`) - assert.Contains(t, jsonStr, `"shortName":""`) - assert.Contains(t, jsonStr, `"longName":""`) assert.Contains(t, jsonStr, `"type":0`) + assert.NotContains(t, jsonStr, `"shortName"`) + assert.NotContains(t, jsonStr, `"longName"`) + assert.NotContains(t, jsonStr, `"nullSafeShortName"`) } func TestRouteDataJSON(t *testing.T) { diff --git a/internal/restapi/agencies_with_coverage_handler_test.go b/internal/restapi/agencies_with_coverage_handler_test.go index 1e53e23a..9d50f865 100644 --- a/internal/restapi/agencies_with_coverage_handler_test.go +++ b/internal/restapi/agencies_with_coverage_handler_test.go @@ -52,12 +52,12 @@ func TestAgenciesWithCoverageHandlerEndToEnd(t *testing.T) { assert.Equal(t, "America/Los_Angeles", agencyRef["timezone"]) assert.Equal(t, "en", agencyRef["lang"]) assert.Equal(t, "530-241-2877", agencyRef["phone"]) - assert.Equal(t, "", agencyRef["email"]) - assert.Equal(t, "", agencyRef["fareUrl"]) - assert.Equal(t, "", agencyRef["disclaimer"]) + assert.Nil(t, agencyRef["email"]) + assert.Nil(t, agencyRef["fareUrl"]) + assert.Nil(t, agencyRef["disclaimer"]) assert.False(t, agencyRef["privateService"].(bool)) - // Ensure no extra fields - assert.Len(t, agencyRef, 10) + // Required fields (id, name, url, timezone, privateService) plus present optional fields (lang, phone) + assert.Len(t, agencyRef, 7) assert.Empty(t, refs["routes"]) assert.Empty(t, refs["situations"]) diff --git a/internal/restapi/reference_utils.go b/internal/restapi/reference_utils.go index b842caca..ff839d2b 100644 --- a/internal/restapi/reference_utils.go +++ b/internal/restapi/reference_utils.go @@ -47,16 +47,15 @@ func (api *RestAPI) BuildRouteReferences(ctx context.Context, agencyID string, s } routeModel := models.Route{ - ID: utils.FormCombinedID(agencyID, route.ID), - AgencyID: agencyID, - ShortName: route.ShortName.String, - LongName: route.LongName.String, - Description: route.Desc.String, - Type: models.RouteType(route.Type), - URL: route.Url.String, - Color: route.Color.String, - TextColor: route.TextColor.String, - NullSafeShortName: route.ShortName.String, + ID: utils.FormCombinedID(agencyID, route.ID), + AgencyID: agencyID, + ShortName: route.ShortName.String, + LongName: route.LongName.String, + Description: route.Desc.String, + Type: models.RouteType(route.Type), + URL: route.Url.String, + Color: route.Color.String, + TextColor: route.TextColor.String, } modelRoutes = append(modelRoutes, routeModel) } diff --git a/internal/restapi/route_handler_test.go b/internal/restapi/route_handler_test.go index 00cfc6cf..5e96780d 100644 --- a/internal/restapi/route_handler_test.go +++ b/internal/restapi/route_handler_test.go @@ -50,14 +50,22 @@ func TestRouteHandlerEndToEnd(t *testing.T) { entry, ok := data["entry"].(map[string]interface{}) assert.True(t, ok) + assertOptional := func(expected string, got interface{}) { + if expected == "" { + assert.Nil(t, got) + } else { + assert.Equal(t, expected, got) + } + } + assert.Equal(t, routeID, entry["id"]) assert.Equal(t, routes[0].AgencyID, entry["agencyId"]) - assert.Equal(t, routes[0].ShortName.String, entry["shortName"]) - assert.Equal(t, routes[0].LongName.String, entry["longName"]) - assert.Equal(t, routes[0].Desc.String, entry["description"]) - assert.Equal(t, routes[0].Url.String, entry["url"]) - assert.Equal(t, routes[0].Color.String, entry["color"]) - assert.Equal(t, routes[0].TextColor.String, entry["textColor"]) + assertOptional(routes[0].ShortName.String, entry["shortName"]) + assertOptional(routes[0].LongName.String, entry["longName"]) + assertOptional(routes[0].Desc.String, entry["description"]) + assertOptional(routes[0].Url.String, entry["url"]) + assertOptional(routes[0].Color.String, entry["color"]) + assertOptional(routes[0].TextColor.String, entry["textColor"]) if typeVal, typeOk := entry["type"].(float64); typeOk { assert.Equal(t, int(routes[0].Type), int(typeVal)) } else { diff --git a/internal/restapi/routes_for_agency_handler.go b/internal/restapi/routes_for_agency_handler.go index 943f1f39..530304c4 100644 --- a/internal/restapi/routes_for_agency_handler.go +++ b/internal/restapi/routes_for_agency_handler.go @@ -14,6 +14,8 @@ func (api *RestAPI) routesForAgencyHandler(w http.ResponseWriter, r *http.Reques return } + includeReferences := r.URL.Query().Get("includeReferences") != "false" + api.GtfsManager.RLock() defer api.GtfsManager.RUnlock() @@ -24,7 +26,7 @@ func (api *RestAPI) routesForAgencyHandler(w http.ResponseWriter, r *http.Reques return } if agency == nil { - api.sendNull(w, r) + api.sendNotFound(w, r) return } @@ -34,9 +36,6 @@ func (api *RestAPI) routesForAgencyHandler(w http.ResponseWriter, r *http.Reques return } - // Apply pagination - offset, limit := utils.ParsePaginationParams(r) - routesForAgency, limitExceeded := utils.PaginateSlice(routesForAgency, offset, limit) routesList := make([]models.Route, 0, len(routesForAgency)) for _, route := range routesForAgency { @@ -53,10 +52,12 @@ func (api *RestAPI) routesForAgencyHandler(w http.ResponseWriter, r *http.Reques } references := models.NewEmptyReferences() - references.Agencies = []models.AgencyReference{ - models.AgencyReferenceFromDatabase(agency), + if includeReferences { + references.Agencies = []models.AgencyReference{ + models.AgencyReferenceFromDatabase(agency), + } } - response := models.NewListResponse(routesList, *references, limitExceeded, api.Clock) + response := models.NewListResponse(routesList, *references, false, api.Clock) api.sendResponse(w, r, response) } diff --git a/internal/restapi/routes_for_agency_handler_test.go b/internal/restapi/routes_for_agency_handler_test.go index a2aa95b4..dad5fedd 100644 --- a/internal/restapi/routes_for_agency_handler_test.go +++ b/internal/restapi/routes_for_agency_handler_test.go @@ -39,16 +39,35 @@ func TestRoutesForAgencyHandlerEndToEnd(t *testing.T) { data, ok := model.Data.(map[string]interface{}) require.True(t, ok) - // Check that we have a list of routes - _, ok = data["list"].([]interface{}) + routes, ok := data["list"].([]interface{}) require.True(t, ok) + require.NotEmpty(t, routes) + + for _, r := range routes { + route, ok := r.(map[string]interface{}) + require.True(t, ok) + assert.Nil(t, route["nullSafeShortName"], "nullSafeShortName must not appear in API responses") + assert.NotEmpty(t, route["id"], "route id must be present") + assert.NotEmpty(t, route["agencyId"], "route agencyId must be present") + _, hasType := route["type"] + assert.True(t, hasType, "route type must be present") + } refs, ok := data["references"].(map[string]interface{}) require.True(t, ok) refAgencies, ok := refs["agencies"].([]interface{}) require.True(t, ok) - assert.Len(t, refAgencies, 1) + require.Len(t, refAgencies, 1) + + agency, ok := refAgencies[0].(map[string]interface{}) + require.True(t, ok) + assert.NotEmpty(t, agency["id"], "agency id must be present") + assert.NotEmpty(t, agency["name"], "agency name must be present") + assert.NotEmpty(t, agency["url"], "agency url must be present") + assert.NotEmpty(t, agency["timezone"], "agency timezone must be present") + _, hasPrivateService := agency["privateService"] + assert.True(t, hasPrivateService, "agency privateService must be present") } func TestRoutesForAgencyHandlerInvalidID(t *testing.T) { @@ -71,9 +90,9 @@ func TestRoutesForAgencyHandlerNonExistentAgency(t *testing.T) { resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/routes-for-agency/non-existent-agency.json?key=TEST") - assert.Equal(t, http.StatusOK, resp.StatusCode) - assert.Equal(t, "", model.Text) - assert.Nil(t, model.Data) + assert.Equal(t, http.StatusNotFound, resp.StatusCode) + assert.Equal(t, http.StatusNotFound, model.Code) + assert.Equal(t, "resource not found", model.Text) } func TestRoutesForAgencyHandlerReturnsCompoundRouteIDs(t *testing.T) { @@ -107,7 +126,7 @@ func TestRoutesForAgencyHandlerReturnsCompoundRouteIDs(t *testing.T) { } } -func TestRoutesForAgencyHandlerPagination(t *testing.T) { +func TestRoutesForAgencyHandlerLimitExceededAlwaysFalse(t *testing.T) { api := createTestApi(t) defer api.Shutdown() @@ -115,41 +134,33 @@ func TestRoutesForAgencyHandlerPagination(t *testing.T) { require.NotEmpty(t, agencies) agencyId := agencies[0].ID - // Case 1: Limit 5 (Should return 5 items, limitExceeded=true) - resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/routes-for-agency/"+agencyId+".json?key=TEST&limit=5") + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/routes-for-agency/"+agencyId+".json?key=TEST") assert.Equal(t, http.StatusOK, resp.StatusCode) data, ok := model.Data.(map[string]interface{}) require.True(t, ok) - list, ok := data["list"].([]interface{}) - require.True(t, ok) - assert.Len(t, list, 5) - assert.True(t, data["limitExceeded"].(bool), "limitExceeded should be true when more items exist") - - // Case 2: Offset 5, Limit 5 (Should return next 5 items) - resp2, model2 := serveApiAndRetrieveEndpoint(t, api, "/api/where/routes-for-agency/"+agencyId+".json?key=TEST&offset=5&limit=5") - assert.Equal(t, http.StatusOK, resp2.StatusCode) - data2, ok := model2.Data.(map[string]interface{}) + limitExceeded, ok := data["limitExceeded"].(bool) require.True(t, ok) - list2, ok := data2["list"].([]interface{}) - require.True(t, ok) - assert.Len(t, list2, 5) + assert.False(t, limitExceeded) +} - // Verify items are different - firstItem1 := list[0].(map[string]interface{}) - firstItem2 := list2[0].(map[string]interface{}) - assert.NotEqual(t, firstItem1["id"], firstItem2["id"]) +func TestRoutesForAgencyHandlerIncludeReferencesFalse(t *testing.T) { + api := createTestApi(t) + defer api.Shutdown() - // Case 3: Limit 100 (Should return all 13 items, limitExceeded=false) - resp3, model3 := serveApiAndRetrieveEndpoint(t, api, "/api/where/routes-for-agency/"+agencyId+".json?key=TEST&limit=100") - assert.Equal(t, http.StatusOK, resp3.StatusCode) + agencies := mustGetAgencies(t, api) + require.NotEmpty(t, agencies) + agencyId := agencies[0].ID - data3, ok := model3.Data.(map[string]interface{}) - require.True(t, ok) - list3, ok := data3["list"].([]interface{}) + resp, model := serveApiAndRetrieveEndpoint(t, api, "/api/where/routes-for-agency/"+agencyId+".json?key=TEST&includeReferences=false") + assert.Equal(t, http.StatusOK, resp.StatusCode) + + data, ok := model.Data.(map[string]interface{}) require.True(t, ok) - // RABA has 13 routes - assert.Len(t, list3, 13) - assert.False(t, data3["limitExceeded"].(bool), "limitExceeded should be false when all items returned") + + if refs, ok := data["references"].(map[string]interface{}); ok { + refAgencies, _ := refs["agencies"].([]interface{}) + assert.Empty(t, refAgencies, "agencies must be empty when includeReferences=false") + } } diff --git a/internal/restapi/testdata/models.go b/internal/restapi/testdata/models.go index 868c1efe..ba3eb4ba 100644 --- a/internal/restapi/testdata/models.go +++ b/internal/restapi/testdata/models.go @@ -16,66 +16,55 @@ var Raba = models.AgencyReference{ } var Route19 = models.Route{ - AgencyID: "25", - Color: "2738ec", - Description: "", - ID: "25_3779", - LongName: "Route 19", - NullSafeShortName: "19", - ShortName: "19", - TextColor: "ffffff", - Type: 3, - URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/SRTA_BeachBus_20230302_ful.pdf", + AgencyID: "25", + Color: "2738ec", + ID: "25_3779", + LongName: "Route 19", + ShortName: "19", + TextColor: "ffffff", + Type: 3, + URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/SRTA_BeachBus_20230302_ful.pdf", } var Route44x = models.Route{ - AgencyID: "25", - Color: "F84DF7", - Description: "", - ID: "25_44X", - LongName: "Shingletown Flex", - NullSafeShortName: "44X", - ShortName: "44X", - TextColor: "000000", - Type: 3, - URL: "", + AgencyID: "25", + Color: "F84DF7", + ID: "25_44X", + LongName: "Shingletown Flex", + ShortName: "44X", + TextColor: "000000", + Type: 3, } var Route15 = models.Route{ - AgencyID: "25", - Color: "800000", - Description: "", - ID: "25_15", - LongName: "Churn Creek/Knightson/Airport", - NullSafeShortName: "15", - ShortName: "15", - TextColor: "FFFFFF", - Type: 3, - URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/Bus/15-18.pdf", + AgencyID: "25", + Color: "800000", + ID: "25_15", + LongName: "Churn Creek/Knightson/Airport", + ShortName: "15", + TextColor: "FFFFFF", + Type: 3, + URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/Bus/15-18.pdf", } var Route14 = models.Route{ - AgencyID: "25", - Color: "8c08b1", - Description: "", - ID: "25_160", - LongName: "Route 14", - NullSafeShortName: "14", - ShortName: "14", - TextColor: "ffffff", - Type: 3, - URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/Bus/Route14.pdf", + AgencyID: "25", + Color: "8c08b1", + ID: "25_160", + LongName: "Route 14", + ShortName: "14", + TextColor: "ffffff", + Type: 3, + URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/Bus/Route14.pdf", } var Route11 = models.Route{ - AgencyID: "25", - Color: "93ea5f", - Description: "", - ID: "25_159", - LongName: "Route 11", - NullSafeShortName: "11", - ShortName: "11", - TextColor: "000000", - Type: 3, - URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/Bus/Route11.pdf", + AgencyID: "25", + Color: "93ea5f", + ID: "25_159", + LongName: "Route 11", + ShortName: "11", + TextColor: "000000", + Type: 3, + URL: "https://cms3.revize.com/revize/reddingbusauthority/Document%20Center/Services/Bus/Route11.pdf", } diff --git a/internal/restapi/trip_details_handler.go b/internal/restapi/trip_details_handler.go index 384f29e1..e754caf3 100644 --- a/internal/restapi/trip_details_handler.go +++ b/internal/restapi/trip_details_handler.go @@ -532,16 +532,15 @@ func (api *RestAPI) BuildRouteReference(ctx context.Context, agencyID string, st } routeModel := models.Route{ - ID: utils.FormCombinedID(agencyID, route.ID), - AgencyID: agencyID, - ShortName: route.ShortName.String, - LongName: route.LongName.String, - Description: route.Desc.String, - Type: models.RouteType(route.Type), - URL: route.Url.String, - Color: route.Color.String, - TextColor: route.TextColor.String, - NullSafeShortName: route.ShortName.String, + ID: utils.FormCombinedID(agencyID, route.ID), + AgencyID: agencyID, + ShortName: route.ShortName.String, + LongName: route.LongName.String, + Description: route.Desc.String, + Type: models.RouteType(route.Type), + URL: route.Url.String, + Color: route.Color.String, + TextColor: route.TextColor.String, } modelRoutes = append(modelRoutes, routeModel) }