feat: add /route/{id} endpoint

[uloamaka]

fix: #156

This PR adds the /route/{id} endpoint, with proper (unit)end to end testing ensuring that the Java production server’s output is followed and also follows the codebase coding style.

[uloamaka]

@aaronbrethorst kindly help review this PR.

[Ahmedhossamdev]

Great work!

I think it's ready to merge @aaronbrethorst

[aaronbrethorst]

Thanks for implementing the /route/{id} endpoint! This is a clean, well-structured implementation that follows the established patterns in the codebase. The test coverage is solid, and the response format matches the OneBusAway API specification.

---

Issues to Fix

1. AgencyID in Response Uses Uncombined ID

In internal/restapi/route_handler.go, the AgencyID field is set directly from the database without forming a combined ID:

routeData := &models.Route{
    ID:       utils.FormCombinedID(route.AgencyID, route.ID),
    AgencyID: route.AgencyID,  // ← Not combined
    ...
}

Looking at the test assertion in route_handler_test.go:52:

assert.Equal(t, routes[0].Agency.Id, entry["agencyId"])

This suggests the current behavior matches the in-memory GTFS data, so this may be intentional. However, I wanted to flag it for verification—other endpoints like stop_handler.go handle agency IDs differently. If the OneBusAway API expects a combined agencyId in the response, this would need adjustment.

Action: Verify against a production OneBusAway server response to confirm the expected agencyId format.

---

Suggestions

1. Consider Using the NewRoute Constructor

There's already a NewRoute constructor in internal/models/routes.go that other handlers use. Using it would improve consistency:

// Current approach (direct struct initialization)
routeData := &models.Route{
    ID:                utils.FormCombinedID(route.AgencyID, route.ID),
    AgencyID:          route.AgencyID,
    // ... 8 more fields
}

// Alternative (using constructor)
routeData := models.NewRoute(
    utils.FormCombinedID(route.AgencyID, route.ID),
    route.AgencyID,
    route.ShortName.String,
    route.LongName.String,
    route.Desc.String,
    models.RouteType(route.Type),
    route.Url.String,
    route.Color.String,
    route.TextColor.String,
    utils.NullStringOrEmpty(route.ShortName),
)

This is non-blocking—both approaches work correctly.

2. Test Assumption About Agency-Route Relationship

In the tests, the route ID is formed by combining the first agency with the first route:

routeID := utils.FormCombinedID(agencies[0].Id, routes[0].Id)

This works because the test data has only one agency. Consider adding a comment or using routes[0].Agency.Id directly for clarity:

// Use the route's actual agency to form the combined ID
routeID := utils.FormCombinedID(routes[0].Agency.Id, routes[0].Id)

---

Summary

The implementation correctly:

• Validates the ID format
• Queries the database for route data
• Includes agency references in the response
• Handles not-found cases appropriately
• Passes all new tests

The new GetRoutes() method in gtfs_manager.go is a useful addition for test setup.

Once you verify the agencyId format question above, this is ready to merge!

[uloamaka]

In internal/restapi/route_handler.go, the AgencyID field is set directly from the database without forming a combined ID:

From production OneBusAway response, the agencyID field is not combined, but the routeId is the combined form.

This works because the test data has only one agency. Consider adding a comment or using routes[0].Agency.Id directly for clarity:

This has been resolved in the last commit.

There's already a NewRoute constructor in internal/models/routes.go that other handlers use. Using it would improve consistency:

The route_handler has been updated to use theNewRoute constructor,
Thanks for the review.