fix(vehicles): add logging if a non-shuttle vehicle has a null direct…

…ion_id (#854)

Problem: As described in the linked ticket, sometimes we return vehicles with "direction_id": null. We aren't sure why. I spent a couple of days looking into this issue and didn't make any progress.

Solution: We are adding logs for when an invalid direction_id is returned. Hopefully these can allow us to better ascertain what conditions lead to this happening.

apps/state/lib/state/vehicle.ex

@@ -38,9 +38,22 @@ defmodule State.Vehicle do
 
   @impl State.Server
   def pre_insert_hook(vehicle) do
+    if has_invalid_dir(vehicle) do
+      Logger.warning("Found vehicle with invalid direction: #{inspect(vehicle)}")
+    end
+
     update_effective_route_id(vehicle)
   end
 
+  @spec has_invalid_dir(Vehicle.t()) :: boolean()
+  defp has_invalid_dir(vehicle) do
+    not_shuttle = vehicle.route_id != nil and not String.starts_with?(vehicle.route_id, "Shuttle")
+
+    invalid_dir = vehicle.direction_id not in [0, 1]
+
+    not_shuttle and invalid_dir
+  end
+
   defp update_effective_route_id(%Vehicle{trip_id: trip_id} = vehicle) do
     case Trip.by_id(trip_id) do
       [] ->

apps/state/test/state/vehicle_test.exs

@@ -1,6 +1,7 @@
 defmodule State.VehicleTest do
   @moduledoc false
   use ExUnit.Case
+  import ExUnit.CaptureLog
 
   alias Model.{Route, Trip, Vehicle}
   import State.Vehicle
@@ -114,4 +115,29 @@ defmodule State.VehicleTest do
 
     assert size() == 1
   end
+
+  test "logs a message for non-shuttle vehicles with an invalid direction" do
+    # direction_id is not assigned but this is not a shuttle
+    vehicle = %Vehicle{id: "42", trip_id: "2", route_id: "504"}
+    log = capture_log(fn -> new_state([vehicle]) end)
+
+    assert %Vehicle{id: "42"} = by_id("42")
+    assert log =~ "Found vehicle with invalid direction"
+    assert log =~ "42"
+
+    # direction_id is an invalid number
+    vehicle = %Vehicle{id: "43", trip_id: "2", route_id: "504", direction_id: -1}
+    log = capture_log(fn -> new_state([vehicle]) end)
+
+    assert %Vehicle{id: "43"} = by_id("43")
+    assert log =~ "Found vehicle with invalid direction"
+    assert log =~ "43"
+
+    # it's a shuttle, so we don't care
+    vehicle = %Vehicle{id: "44", trip_id: "2", route_id: "Shuttle-ToTheMoon"}
+    log = capture_log(fn -> new_state([vehicle]) end)
+
+    assert %Vehicle{id: "44"} = by_id("44")
+    refute log =~ "Found vehicle with invalid direction"
+  end
 end