Fixed error messages, and error tests

apps/trip_plan/lib/trip_plan/api/open_trip_planner.ex

@@ -3,14 +3,16 @@ defmodule TripPlan.Api.OpenTripPlanner do
   @behaviour TripPlan.Api
   require Logger
   import __MODULE__.Builder, only: [build_params: 3]
-  import __MODULE__.Parser, only: [parse_ql: 1]
+  import __MODULE__.Parser, only: [parse_ql: 2]
 
   def plan(from, to, connection_opts, opts, _parent) do
     plan(from, to, connection_opts, opts)
   end
 
   @impl true
   def plan(from, to, connection_opts, opts) do
+    accessible? = Keyword.get(opts, :wheelchair_accessible?, false)
+
     with {:ok, params} <- build_params(from, to, opts) do
       param_string = Enum.map_join(params, "\n", fn {key, val} -> ~s{#{key}: #{val}} end)
 
@@ -23,12 +25,10 @@ defmodule TripPlan.Api.OpenTripPlanner do
       }
       """
 
-      IO.inspect(graph_ql_query)
-
       root_url = Keyword.get(opts, :root_url, nil) || pick_url(connection_opts)
       graphql_url = "#{root_url}/otp/routers/default/index/"
 
-      send_request(graphql_url, graph_ql_query, &parse_ql/1)
+      send_request(graphql_url, graph_ql_query, accessible?, &parse_ql/2)
     end
   end
 
@@ -86,10 +86,10 @@ defmodule TripPlan.Api.OpenTripPlanner do
     Util.config(:trip_plan, OpenTripPlanner, key)
   end
 
-  defp send_request(url, query, parser) do
+  defp send_request(url, query, accessible?, parser) do
     with {:ok, response} <- log_response(url, query),
          %{status: 200, body: body} <- response do
-      parser.(body)
+      parser.(body, accessible?)
     else
       %{status: _} = response ->
         {:error, response}
@@ -116,7 +116,7 @@ defmodule TripPlan.Api.OpenTripPlanner do
         "#{__MODULE__}.plan_response url=#{url} is_otp2=#{String.contains?(url, config(:otp2_url))} query=#{inspect(query)} #{status_text(response)} duration=#{duration / :timer.seconds(1)}"
       end)
 
-    IO.inspect(response)
+    response
   end
 
   defp build_headers("true") do
@@ -138,6 +138,10 @@ defmodule TripPlan.Api.OpenTripPlanner do
   defp itinerary_shape() do
     """
     {
+      routingErrors {
+        code
+        description
+      }
       itineraries {
         startTime
         endTime

apps/trip_plan/lib/trip_plan/api/open_trip_planner/parser.ex

@@ -14,42 +14,22 @@ defmodule TripPlan.Api.OpenTripPlanner.Parser do
 
   @transit_modes ~w(SUBWAY TRAM BUS RAIL FERRY)s
 
-  @spec parse_ql(map) :: {:ok, [Itinerary.t()]} | {:error, TripPlan.Api.error()}
-  def parse_ql(%{"data" => data}) do
-    parse_map(data)
+  @spec parse_ql(map(), boolean()) :: {:ok, [Itinerary.t()]} | {:error, TripPlan.Api.error()}
+  def parse_ql(%{"data" => data}, accessible?) do
+    parse_map(data, accessible?)
   end
 
-  @doc """
-  Parse the JSON output from the plan endpoint.
-  """
-  @spec parse_json(binary) :: {:ok, [Itinerary.t()]} | {:error, TripPlan.Api.error()}
-  def parse_json(json_binary) do
-    with {:ok, json} <- Poison.decode(json_binary) do
-      parse_map(json)
-    end
-  rescue
-    e in FunctionClauseError ->
-      _ =
-        Logger.warn(fn ->
-          "#{__MODULE__} exception=FunctionClauseError function=#{e.function} json=#{json_binary}"
-        end)
-
-      {:error, :invalid_json}
-  end
-
-  @spec parse_map(map) :: {:ok, [Itinerary.t()]} | {:error, TripPlan.Api.error()}
-  defp parse_map(%{"error" => %{"message" => error_message}, "requestParameters" => params}) do
-    accessible? = params["wheelchair"] == "true"
-
+  @spec parse_map(map(), boolean()) :: {:ok, [Itinerary.t()]} | {:error, TripPlan.Api.error()}
+  defp parse_map(%{"plan" => %{"routingErrors" => [head | _]}}, accessible?) do
     _ =
       Logger.warn(fn ->
-        "#{__MODULE__} trip_plan=error message=#{inspect(error_message)}"
+        "#{__MODULE__} trip_plan=error message=#{inspect(head["code"])}"
       end)
 
-    {:error, error_message_atom(error_message, accessible?: accessible?)}
+    {:error, error_message_atom(head["code"], accessible?: accessible?)}
   end
 
-  defp parse_map(json) do
+  defp parse_map(json, _) do
     _ =
       Logger.info(fn ->
         "#{__MODULE__} trip_plan=success count=#{Enum.count(json["plan"]["itineraries"])}"
@@ -59,9 +39,13 @@ defmodule TripPlan.Api.OpenTripPlanner.Parser do
   end
 
   @doc "Test helper which matches off the :ok"
-  def parse_json!(json_binary) do
-    {:ok, itineraries} = parse_json(json_binary)
-    itineraries
+  def parse_json(json_binary, accessible? \\ false) do
+    with {:ok, json} <- Poison.decode(json_binary) do
+      parse_map(json, accessible?)
+    end
+  rescue
+    _e in FunctionClauseError ->
+      {:error, :invalid_json}
   end
 
   @spec error_message_atom(String.t(), Keyword.t()) :: TripPlan.Api.error()
@@ -72,6 +56,7 @@ defmodule TripPlan.Api.OpenTripPlanner.Parser do
   defp error_message_atom("PATH_NOT_FOUND", accessible?: true), do: :location_not_accessible
   defp error_message_atom("PATH_NOT_FOUND", accessible?: false), do: :path_not_found
   defp error_message_atom("LOCATION_NOT_ACCESSIBLE", _opts), do: :location_not_accessible
+  defp error_message_atom("NO_STOPS_IN_RANGE", _opts), do: :location_not_accessible
   defp error_message_atom(_, _opts), do: :unknown
 
   defp parse_itinerary(json, request_params) do

apps/trip_plan/test/api/open_trip_planner/parser_test.exs

@@ -4,19 +4,21 @@ defmodule TripPlan.Api.OpenTripPlanner.ParserTest do
   alias TripPlan.{Itinerary, NamedPosition, PersonalDetail, PersonalDetail.Step, TransitDetail}
 
   @fixture File.read!("test/fixture/north_station_to_park_plaza.json")
-  @parsed parse_json!(@fixture)
+  @parsed parse_json(@fixture)
 
   describe "parse_json/1" do
     test "returns an error with invalid JSON" do
       assert {:error, _} = parse_json("")
     end
 
     test "returns a list of Itinerary structs" do
-      for i <- @parsed do
+      {:ok, parsed} = @parsed
+
+      for i <- parsed do
         assert %Itinerary{} = i
       end
 
-      assert [first, _, _] = @parsed
+      assert [first, _, _] = parsed
       assert first.start == Timex.to_datetime(~N[2017-05-19T13:50:59], "America/New_York")
       assert first.stop == Timex.to_datetime(~N[2017-05-19T14:03:19], "America/New_York")
     end
@@ -36,7 +38,8 @@ defmodule TripPlan.Api.OpenTripPlanner.ParserTest do
     end
 
     test "an itinerary has legs" do
-      first = List.first(@parsed)
+      {:ok, parsed} = @parsed
+      first = List.first(parsed)
       [subway_leg, walk_leg] = first.legs
 
       assert %TransitDetail{
@@ -59,14 +62,16 @@ defmodule TripPlan.Api.OpenTripPlanner.ParserTest do
     end
 
     test "positions can use stopId instead of stopCode" do
+      {:ok, parsed} = @parsed
       stop_code_regex = ~r/"stopCode": "[^"]",/
       data = String.replace(@fixture, stop_code_regex, "")
-      parsed = parse_json!(data)
-      assert parsed == @parsed
+      {:ok, parsed_data} = parse_json(data)
+      assert parsed_data == parsed
     end
 
     test "walk legs have distance and step plans" do
-      [_, walk_leg] = List.first(@parsed).legs
+      {:ok, parsed} = @parsed
+      [_, walk_leg] = List.first(parsed).legs
       assert walk_leg.mode.distance == 329.314
 
       assert walk_leg.mode.steps == [
@@ -92,7 +97,8 @@ defmodule TripPlan.Api.OpenTripPlanner.ParserTest do
     end
 
     test "subway legs have trip information" do
-      [subway_leg, _] = List.first(@parsed).legs
+      {:ok, parsed} = @parsed
+      [subway_leg, _] = List.first(parsed).legs
       assert subway_leg.mode.route_id == "Orange"
       assert subway_leg.mode.trip_id == "33932853"
       assert subway_leg.mode.intermediate_stop_ids == ~w(
@@ -104,18 +110,15 @@ defmodule TripPlan.Api.OpenTripPlanner.ParserTest do
     end
 
     test "parses path_not_found error as location_not_accessible when accessiblity is checked" do
-      decoded_json = %{
-        "error" => %{"message" => "PATH_NOT_FOUND"},
-        "requestParameters" => %{"wheelchair" => "true"}
-      }
+      decoded_json = %{"plan" => %{"routingErrors" => [%{"code" => "PATH_NOT_FOUND"}]}}
 
       {:ok, json} = Poison.encode(decoded_json)
-      parsed_json = parse_json(json)
+      parsed_json = parse_json(json, true)
       assert parsed_json == {:error, :location_not_accessible}
     end
 
     test "parses path_not_found error as normally when accessibility is not checked" do
-      decoded_json = %{"error" => %{"message" => "PATH_NOT_FOUND"}, "requestParameters" => %{}}
+      decoded_json = %{"plan" => %{"routingErrors" => [%{"code" => "PATH_NOT_FOUND"}]}}
       {:ok, json} = Poison.encode(decoded_json)
       parsed_json = parse_json(json)
       assert parsed_json == {:error, :path_not_found}