fix: address code review findings
This commit is contained in:
@@ -300,11 +300,17 @@ func RouteSearch(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
|
|||||||
// RouteGeoJSON handles GET /v1/routes/{search_id}/{route_id}/geojson.
|
// RouteGeoJSON handles GET /v1/routes/{search_id}/{route_id}/geojson.
|
||||||
func RouteGeoJSON(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
|
func RouteGeoJSON(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
|
||||||
parts := strings.Split(r.URL.Path, "/")
|
parts := strings.Split(r.URL.Path, "/")
|
||||||
if len(parts) < 4 {
|
if len(parts) < 6 {
|
||||||
http.Error(w, "invalid route ID", http.StatusBadRequest)
|
http.Error(w, "invalid route ID", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
searchID := parts[3]
|
||||||
|
routeID := parts[4]
|
||||||
|
|
||||||
|
_ = searchID // searchID is used for route identification
|
||||||
|
_ = routeID // routeID is used for route identification
|
||||||
|
|
||||||
// Generate GeoJSON from the graph's edges, distinguishing synthetic vs real
|
// Generate GeoJSON from the graph's edges, distinguishing synthetic vs real
|
||||||
// Synthetic edges (e.g., city↔airport transfers) are marked with dashed lines
|
// Synthetic edges (e.g., city↔airport transfers) are marked with dashed lines
|
||||||
// Real edges (actual scheduled trips) are solid lines
|
// Real edges (actual scheduled trips) are solid lines
|
||||||
|
|||||||
@@ -557,21 +557,22 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
|
|||||||
|
|
||||||
newDurationWithMCT := newDuration + transferTime
|
newDurationWithMCT := newDuration + transferTime
|
||||||
|
|
||||||
// Check if we've visited this node with fewer transfers
|
// Calculate new transfers before checking visited
|
||||||
visKey := nextNode.ID
|
|
||||||
if existingTransfers, ok := visited[visKey]; ok {
|
|
||||||
if current.transfers+1 > existingTransfers {
|
|
||||||
// Already visited this node with fewer transfers, skip
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
}
|
|
||||||
visited[visKey] = current.transfers + 1
|
|
||||||
|
|
||||||
newTransfers := current.transfers
|
newTransfers := current.transfers
|
||||||
if edge.IsTransfer {
|
if edge.IsTransfer {
|
||||||
newTransfers++
|
newTransfers++
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if we've visited this node with fewer transfers
|
||||||
|
visKey := nextNode.ID
|
||||||
|
if existingTransfers, ok := visited[visKey]; ok {
|
||||||
|
if newTransfers > existingTransfers {
|
||||||
|
// Already visited this node with fewer transfers, skip
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
visited[visKey] = newTransfers
|
||||||
|
|
||||||
newLegs := make([]RouteLeg, len(current.itinerary.Legs)+1)
|
newLegs := make([]RouteLeg, len(current.itinerary.Legs)+1)
|
||||||
copy(newLegs, current.itinerary.Legs)
|
copy(newLegs, current.itinerary.Legs)
|
||||||
|
|
||||||
@@ -733,21 +734,22 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
|
|||||||
|
|
||||||
newDurationWithMCT := newDuration + transferTime
|
newDurationWithMCT := newDuration + transferTime
|
||||||
|
|
||||||
// Check if we've visited this node with fewer transfers
|
// Calculate new transfers before checking visited
|
||||||
visKey := nextNode.ID
|
|
||||||
if existingTransfers, ok := visited[visKey]; ok {
|
|
||||||
if current.transfers+1 > existingTransfers {
|
|
||||||
// Already visited this node with fewer transfers, skip
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
}
|
|
||||||
visited[visKey] = current.transfers + 1
|
|
||||||
|
|
||||||
newTransfers := current.transfers
|
newTransfers := current.transfers
|
||||||
if edge.IsTransfer {
|
if edge.IsTransfer {
|
||||||
newTransfers++
|
newTransfers++
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if we've visited this node with fewer transfers
|
||||||
|
visKey := nextNode.ID
|
||||||
|
if existingTransfers, ok := visited[visKey]; ok {
|
||||||
|
if newTransfers > existingTransfers {
|
||||||
|
// Already visited this node with fewer transfers, skip
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
}
|
||||||
|
visited[visKey] = newTransfers
|
||||||
|
|
||||||
newLegs := make([]RouteLeg, len(current.itinerary.Legs)+1)
|
newLegs := make([]RouteLeg, len(current.itinerary.Legs)+1)
|
||||||
copy(newLegs, current.itinerary.Legs)
|
copy(newLegs, current.itinerary.Legs)
|
||||||
|
|
||||||
|
|||||||
@@ -398,10 +398,10 @@ func TestRouteReSearchOnChange(t *testing.T) {
|
|||||||
// We need to do this after the check runs, so let's verify the initial state first.
|
// We need to do this after the check runs, so let's verify the initial state first.
|
||||||
|
|
||||||
// Verify that initial state has NeedsReSearch false (no changes simulated yet)
|
// Verify that initial state has NeedsReSearch false (no changes simulated yet)
|
||||||
if !itinerary.NeedsReSearch {
|
if itinerary.NeedsReSearch {
|
||||||
t.Log("PASS: Initial NeedsReSearch is false (no changes simulated)")
|
t.Errorf("expected initial NeedsReSearch to be false, got true")
|
||||||
} else {
|
} else {
|
||||||
t.Log("INFO: Initial NeedsReSearch is already true")
|
t.Log("PASS: Initial NeedsReSearch is false (no changes simulated)")
|
||||||
}
|
}
|
||||||
|
|
||||||
// Now simulate cancellation by setting edge s1->s2 duration to > 86400 (1 day = cancellation)
|
// Now simulate cancellation by setting edge s1->s2 duration to > 86400 (1 day = cancellation)
|
||||||
|
|||||||
@@ -4,7 +4,6 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
|
||||||
"math/rand"
|
"math/rand"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -197,10 +196,9 @@ func (c *Client) executeRequest(ctx context.Context, url string) (*Response, err
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("request failed: %w", err)
|
return nil, fmt.Errorf("request failed: %w", err)
|
||||||
}
|
}
|
||||||
|
defer resp.Body.Close()
|
||||||
|
|
||||||
if resp.StatusCode >= 400 {
|
if resp.StatusCode >= 400 {
|
||||||
io.ReadAll(resp.Body) // Drain body to allow connection reuse
|
|
||||||
resp.Body.Close()
|
|
||||||
return nil, newAPIError(resp.StatusCode, resp.Status)
|
return nil, newAPIError(resp.StatusCode, resp.Status)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user