fix: resolve race condition and double HTTP write in location APIs

[AhmedAlian7]

Description

• follow-up (replaces Refactor location parameter parsing to shared helper #644)

The changes address the race condition, double HTTP response write, and error aggregation issues mentioned in the previous review.

Changes Made

• Fixed Race Condition: Moved api.GtfsManager.RLock() and defer api.GtfsManager.RUnlock() above the parseAndValidateRequest call in trips_for_location_handler.go to safely protect GetAgencies().
• Resolved Double HTTP Write: Refactored the parseLocationParams signature in location_params.go to accept and return the fieldErrors map. Removed the code that was directly writing to w (ResponseWriter) mid-process.
• Unified Validation Errors: Updated stops_for_location_handler.go, routes_for_location_handler.go, and trips_for_location_handler.go to properly chain the fieldErrors map using the new parseLocationParams signature. This ensures that all validation errors (e.g., location, maxCount, routeType) are successfully aggregated and returned as a single list to the user.

[aaronbrethorst]

Ahmed, this is a well-structured refactoring that tackles three distinct problems — race condition, double HTTP write, and scattered validation — with a single cohesive extraction. The LocationParams struct and parseLocationParams method are the right abstraction for consolidating the duplicated parsing logic across the three location handlers.

What works well

• Race condition fix: Moving RLock()/RUnlock() above parseAndValidateRequest in trips_for_location_handler.go:22-25 ensures GetAgencies() is safely called under the read lock.
• No more double writes: Removing the direct ResponseWriter writes from inside parseLocationParams and instead returning errors for the caller to handle is the correct pattern.
• Error aggregation: The fieldErrors map threading through parseLocationParams → ParseMaxCount → etc. means clients see all validation problems in a single response instead of having to fix-and-retry one at a time.
• err → serverErr rename in parseAndValidateRequest makes the distinction between field validation errors and server errors immediately clear.

Fit and Finish (1 found)

1. Return nil instead of the fieldErrors map on the happy path — location_params.go:47

When all parsing and validation succeeds, line 47 returns:

return &LocationParams{...}, fieldErrors

At this point fieldErrors is either nil or an empty map (depending on what the upstream ParseFloatParam calls returned). Both work correctly at call sites since callers check len(fieldErrors) > 0, but returning nil explicitly on the success path is more idiomatic and removes any ambiguity:

return &LocationParams{
    Lat:     lat,
    Lon:     lon,
    Radius:  radius,
    LatSpan: latSpan,
    LonSpan: lonSpan,
}, nil

This is cosmetic — the current code is functionally correct.

Checklist

• Tests pass
• Lint clean (pre-existing errcheck in gtfs_manager_test.go is on main)
• go fmt clean
• Race condition addressed
• Double HTTP write resolved
• Error aggregation unified across all three handlers

Merge Conflicts

This branch currently has merge conflicts with main that will need to be resolved before merging. Please rebase or merge main into your branch and resolve the conflicts.

Recommended Action

Merge (after resolving conflicts). The fit-and-finish item is optional and can be addressed at your discretion.