Skip to content

Commit

Permalink
feat(ShapeWithStops): Return all routes on routes field. No more conn…
Browse files Browse the repository at this point in the history
…ections field (#2165)
  • Loading branch information
KaylaBrady authored Aug 4, 2023
1 parent 051f388 commit f177d8f
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 100 deletions.
19 changes: 2 additions & 17 deletions lib/schedule/data.ex
Original file line number Diff line number Diff line change
Expand Up @@ -228,22 +228,7 @@ defmodule Schedule.Data do

@spec shape_with_stops_for_trip(t(), Schedule.Trip.id()) :: Schedule.ShapeWithStops.t()
def shape_with_stops_for_trip(data, trip_id) do
%{trips: trips_by_id} = data

trip = Map.get(trips_by_id, trip_id)
trip_route_id = if trip, do: trip.route_id, else: nil

stops_for_trip =
case trip_route_id do
# This trip doesn't have a route - no connections to filter
nil ->
stops_for_trip(data, trip_id)

trip_route_id ->
data
|> stops_for_trip(trip_id)
|> Enum.map(&Stop.reject_connections_for_route(&1, trip_route_id))
end
stops_for_trip = stops_for_trip(data, trip_id)

case shape_for_trip(data, trip_id) do
nil -> nil
Expand Down Expand Up @@ -376,7 +361,7 @@ defmodule Schedule.Data do
bus_routes = Garage.add_garages_to_routes(gtfs_data.bus_only.routes, schedule_trips_by_id)

stops =
Stop.stops_with_connections(
Stop.stops_with_routes(
gtfs_data.all_modes.stops_by_id,
gtfs_data.all_modes.routes,
gtfs_data.all_modes.route_patterns,
Expand Down
32 changes: 12 additions & 20 deletions lib/schedule/gtfs/stop.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ defmodule Schedule.Gtfs.Stop do
parent_station_id: id() | nil,
latitude: float() | nil,
longitude: float() | nil,
connections: [Route.t()],
routes: [Route.t()],
location_type: location_type()
}
@type by_id :: %{id() => t()}
Expand All @@ -28,7 +28,7 @@ defmodule Schedule.Gtfs.Stop do
:parent_station_id,
:latitude,
:longitude,
connections: [],
routes: [],
location_type: :stop
]

Expand Down Expand Up @@ -85,23 +85,15 @@ defmodule Schedule.Gtfs.Stop do
def is_station?(stop), do: stop.location_type == :station

@doc """
Remove any stop connections with the given route_id
Add routes to a map of stops by id based on the given route, pattern, and stop time data.
The list of routes for a stop will include routes for all sibling and parent stops.
"""
@spec reject_connections_for_route(t(), Route.id()) :: t()
def reject_connections_for_route(stop, route_id) do
%{stop | connections: Enum.reject(stop.connections, &(&1.id == route_id))}
end

@doc """
Add connections to a map of stops by id based on the given route, pattern, and stop time data.
The list of connections for a stop will include connections for all sibling and parent stops.
"""
@spec stops_with_connections(by_id(), [Route.t()], [RoutePattern.t()], StopTime.by_trip_id()) ::
@spec stops_with_routes(by_id(), [Route.t()], [RoutePattern.t()], StopTime.by_trip_id()) ::
by_id()
def stops_with_connections(stops_by_id, routes, route_patterns, stop_times_by_trip_id) do
def stops_with_routes(stops_by_id, routes, route_patterns, stop_times_by_trip_id) do
routes_by_id = routes |> Enum.reject(&Route.shuttle_route?(&1)) |> Map.new(&{&1.id, &1})

connections_by_parent_or_stop_id =
routes_by_parent_or_stop_id =
route_patterns
|> Enum.reduce(%{}, fn route_pattern, acc_stop_id_to_routes ->
route_pattern
Expand All @@ -115,18 +107,18 @@ defmodule Schedule.Gtfs.Stop do
end)
end)

stops_with_connections(stops_by_id, connections_by_parent_or_stop_id)
stops_with_routes(stops_by_id, routes_by_parent_or_stop_id)
end

@spec stops_with_connections(by_id(), %{id() => MapSet.t(Route.t())}) :: by_id()
defp stops_with_connections(stops_by_id, connections_by_parent_or_stop_id) do
@spec stops_with_routes(by_id(), %{id() => MapSet.t(Route.t())}) :: by_id()
defp stops_with_routes(stops_by_id, routes_by_parent_or_stop_id) do
Map.new(stops_by_id, fn {stop_id, stop} ->
{
stop_id,
%{
stop
| connections:
connections_by_parent_or_stop_id
| routes:
routes_by_parent_or_stop_id
|> Map.get(stop_id_for_route_association(stop), MapSet.new())
|> MapSet.to_list()
}
Expand Down
68 changes: 20 additions & 48 deletions test/schedule/gtfs/stop_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -128,35 +128,7 @@ defmodule Schedule.Gtfs.StopTest do
end
end

describe "reject_connections_for_route/2" do
test "only rejects connections with the matching route id " do
matching_route = %Route{
id: "39",
name: "first_bus",
description: "bus_route",
direction_names: %{}
}

other_route = %Route{
id: "86",
name: "second_bus",
description: "bus_route",
direction_names: %{}
}

stop = %Stop{
id: "1",
name: "name",
parent_station_id: nil,
connections: [matching_route, other_route]
}

assert %{connections: [^other_route]} =
Stop.reject_connections_for_route(stop, matching_route.id)
end
end

describe "stops_with_connections/4" do
describe "stops_with_routes/4" do
setup do
%{
stop_1: %Stop{id: 1, name: "stop 1", parent_station_id: nil},
Expand Down Expand Up @@ -213,12 +185,12 @@ defmodule Schedule.Gtfs.StopTest do
}
end

test "connections is empty for a stop when no connections found", data do
test "routes is empty for a stop when no routes found", data do
trip_id = data.route_1_pattern_1.representative_trip_id
stop_id = data.stop_1.id

assert %{^stop_id => %{connections: []}} =
Stop.stops_with_connections(
assert %{^stop_id => %{routes: []}} =
Stop.stops_with_routes(
%{stop_id => data.stop_1},
[data.route_1, data.route_2],
[data.route_1_pattern_1],
Expand All @@ -237,10 +209,10 @@ defmodule Schedule.Gtfs.StopTest do
route_2_trip_id = data.route_2_pattern_1.representative_trip_id

assert %{
^stop_1_id => %{connections: [^route_1, ^route_2]},
^stop_2_id => %{connections: [^route_1]}
^stop_1_id => %{routes: [^route_1, ^route_2]},
^stop_2_id => %{routes: [^route_1]}
} =
Stop.stops_with_connections(
Stop.stops_with_routes(
%{stop_1_id => data.stop_1, stop_2_id => data.stop_2},
[data.route_1, data.route_2],
[data.route_1_pattern_1, data.route_2_pattern_1],
Expand All @@ -266,9 +238,9 @@ defmodule Schedule.Gtfs.StopTest do
route_2_trip_id = data.shuttle_route_pattern_1.representative_trip_id

assert %{
^stop_1_id => %{connections: [^route_1]}
^stop_1_id => %{routes: [^route_1]}
} =
Stop.stops_with_connections(
Stop.stops_with_routes(
%{stop_1_id => data.stop_1},
[route_1, route_2],
[data.route_1_pattern_1, data.shuttle_route_pattern_1],
Expand All @@ -279,7 +251,7 @@ defmodule Schedule.Gtfs.StopTest do
)
end

test "connections include routes that stop at sibling stops", data do
test "routes include routes that stop at sibling stops", data do
sibling_stop_1_id = data.child_stop_1.id
sibling_stop_2_id = data.child_stop_2.id

Expand All @@ -290,10 +262,10 @@ defmodule Schedule.Gtfs.StopTest do
route_2_trip_id = data.route_2_pattern_1.representative_trip_id

assert %{
^sibling_stop_1_id => %{connections: [^route_1, ^route_2]},
^sibling_stop_2_id => %{connections: [^route_1, ^route_2]}
^sibling_stop_1_id => %{routes: [^route_1, ^route_2]},
^sibling_stop_2_id => %{routes: [^route_1, ^route_2]}
} =
Stop.stops_with_connections(
Stop.stops_with_routes(
%{
sibling_stop_1_id => data.child_stop_1,
sibling_stop_2_id => data.child_stop_2
Expand All @@ -311,7 +283,7 @@ defmodule Schedule.Gtfs.StopTest do
)
end

test "connections include routes that stop at parent stops", data do
test "routes include routes that stop at parent stops", data do
child_stop_id = data.child_stop_1.id
parent_stop_id = data.parent_stop.id

Expand All @@ -322,10 +294,10 @@ defmodule Schedule.Gtfs.StopTest do
route_2_trip_id = data.route_2_pattern_1.representative_trip_id

assert %{
^child_stop_id => %{connections: [^route_1, ^route_2]},
^parent_stop_id => %{connections: [^route_1, ^route_2]}
^child_stop_id => %{routes: [^route_1, ^route_2]},
^parent_stop_id => %{routes: [^route_1, ^route_2]}
} =
Stop.stops_with_connections(
Stop.stops_with_routes(
%{child_stop_id => data.child_stop_1, parent_stop_id => data.parent_stop},
[data.route_1, data.route_2],
[data.route_1_pattern_1, data.route_2_pattern_1],
Expand All @@ -340,7 +312,7 @@ defmodule Schedule.Gtfs.StopTest do
)
end

test "connections are unique when multiple route patterns go to same stop", data do
test "routes are unique when multiple route patterns go to same stop", data do
stop_1_id = data.stop_1.id

route_1 = data.route_1
Expand All @@ -349,9 +321,9 @@ defmodule Schedule.Gtfs.StopTest do
route_1_trip_id_2 = data.route_1_pattern_2.representative_trip_id

assert %{
^stop_1_id => %{connections: [^route_1]}
^stop_1_id => %{routes: [^route_1]}
} =
Stop.stops_with_connections(
Stop.stops_with_routes(
%{stop_1_id => data.stop_1},
[data.route_1],
[data.route_1_pattern_1, data.route_1_pattern_2],
Expand Down
15 changes: 11 additions & 4 deletions test/schedule_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ defmodule ScheduleTest do
assert Schedule.stop("id", pid) == nil
end

test "bus stop has included bus and subway connections" do
test "bus stop has included bus and subway routes" do
pid =
Schedule.start_mocked(%{
gtfs: %{
Expand Down Expand Up @@ -307,7 +307,7 @@ defmodule ScheduleTest do
%Stop{
id: "stop1_id",
name: "One",
connections: [
routes: [
%Route{
id: "route",
name: "route",
Expand Down Expand Up @@ -796,7 +796,7 @@ defmodule ScheduleTest do
end

describe "shape_with_stops_for_trip" do
test "returns the shape for the trip with stops and connections excluding the trip's route" do
test "returns the shape for the trip with stops and routes" do
pid =
Schedule.start_mocked(%{
gtfs: %{
Expand Down Expand Up @@ -847,7 +847,14 @@ defmodule ScheduleTest do
name: "One",
latitude: 1.0,
longitude: 1.5,
connections: [
routes: [
%Schedule.Gtfs.Route{
description: "Key Bus",
direction_names: %{0 => nil, 1 => nil},
id: "route",
name: "route",
type: 3
},
%Schedule.Gtfs.Route{
id: "subway_route",
name: "subway_route_name",
Expand Down
8 changes: 4 additions & 4 deletions test/skate_web/controllers/route_pattern_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defmodule SkateWeb.RoutePatternControllerTest do
name: "One",
latitude: 42.01,
longitude: -71.01,
connections: [
routes: [
%Route{
id: "route_1",
name: "route_1_name",
Expand All @@ -33,7 +33,7 @@ defmodule SkateWeb.RoutePatternControllerTest do
}
]
},
%Stop{id: "stop_2", name: "Two", latitude: 42.02, longitude: -71.02, connections: []}
%Stop{id: "stop_2", name: "Two", latitude: 42.02, longitude: -71.02, routes: []}
]
}

Expand All @@ -54,7 +54,7 @@ defmodule SkateWeb.RoutePatternControllerTest do
"lat" => 42.01,
"lon" => -71.01,
"location_type" => "stop",
"connections" => [
"routes" => [
%{
"id" => "route_1",
"name" => "route_1_name",
Expand All @@ -71,7 +71,7 @@ defmodule SkateWeb.RoutePatternControllerTest do
"lat" => 42.02,
"lon" => -71.02,
"location_type" => "stop",
"connections" => []
"routes" => []
}
]
}
Expand Down
8 changes: 4 additions & 4 deletions test/skate_web/controllers/shape_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ defmodule SkateWeb.ShapeControllerTest do
name: "One",
latitude: 42.01,
longitude: -71.01,
connections: [
routes: [
%Route{
id: "route_1",
name: "route_1_name",
Expand All @@ -55,7 +55,7 @@ defmodule SkateWeb.ShapeControllerTest do
}
]
},
%Stop{id: "stop_2", name: "Two", latitude: 42.02, longitude: -71.02, connections: []}
%Stop{id: "stop_2", name: "Two", latitude: 42.02, longitude: -71.02, routes: []}
]
}

Expand All @@ -76,7 +76,7 @@ defmodule SkateWeb.ShapeControllerTest do
"lat" => 42.01,
"lon" => -71.01,
"location_type" => "stop",
"connections" => [
"routes" => [
%{
"id" => "route_1",
"name" => "route_1_name",
Expand All @@ -93,7 +93,7 @@ defmodule SkateWeb.ShapeControllerTest do
"lat" => 42.02,
"lon" => -71.02,
"location_type" => "stop",
"connections" => []
"routes" => []
}
]
}
Expand Down
6 changes: 3 additions & 3 deletions test/skate_web/controllers/stop_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule SkateWeb.StopControllerTest do
import Skate.Factory

@stations [
build(:gtfs_stop, %{location_type: :station, connections: [build(:gtfs_route)]}),
build(:gtfs_stop, %{location_type: :station, routes: [build(:gtfs_route)]}),
build(:gtfs_stop, %{
id: "stop2",
name: "Stop 2",
Expand All @@ -29,7 +29,7 @@ defmodule SkateWeb.StopControllerTest do
assert %{
"data" => [
%{
"connections" => [
"routes" => [
%{
"description" => "Key Bus",
"direction_names" => %{"0" => "Outbound", "1" => "Inbound"},
Expand All @@ -46,7 +46,7 @@ defmodule SkateWeb.StopControllerTest do
"name" => "Stop 1"
},
%{
"connections" => [],
"routes" => [],
"id" => "stop2",
"lat" => 42.01,
"location_type" => "station",
Expand Down

0 comments on commit f177d8f

Please sign in to comment.