Refactor DB lookups in FarmEvent and Regimen managers.

lib/farmbot/asset/asset.ex

@@ -9,6 +9,7 @@ defmodule Farmbot.Asset do
     Point,
     Sensor,
     Sequence,
+    Regimen,
     Tool
   }
 
@@ -29,6 +30,15 @@ defmodule Farmbot.Asset do
     repo().one(from s in Sequence, where: s.id == ^sequence_id)
   end
 
+  @doc "Same as `get_sequence_by_id/1` but raises if no Sequence is found."
+  def get_sequence_by_id!(sequence_id) do
+    case get_sequence_by_id(sequence_id) do
+      nil -> raise "Could not find sequence by id #{sequence_id}"
+      %Sequence{} = seq -> seq
+    end
+  end
+
+
   @doc "Get a Point by it's id."
   def get_point_by_id(point_id) do
     repo().one(from p in Point, where: p.id == ^point_id)
@@ -39,10 +49,23 @@ defmodule Farmbot.Asset do
     repo().one(from p in Point, where: p.tool_id == ^tool_id)
   end
 
-  @doc "Get a tool by it's id."
+  @doc "Get a Tool by it's id."
   def get_tool_by_id(tool_id) do
     repo().one(from t in Tool, where: t.id == ^tool_id)
   end
 
+  @doc "Get a Regimen by it's id."
+  def get_regimen_by_id(regimen_id) do
+    repo().one(from r in Regimen, where: r.id == ^regimen_id)
+  end
+
+  @doc "Same as `get_regimen_by_id/1` but raises if no Regimen is found."
+  def get_regimen_by_id!(regimen_id) do
+    case get_regimen_by_id(regimen_id) do
+      nil -> raise "Could not find regimen by id #{regimen_id}"
+      %Regimen{} = reg -> reg
+    end
+  end
+
   defp repo, do: Farmbot.Repo.current_repo()
 end

lib/farmbot/celery_script/ast/node/config_update.ex

@@ -10,7 +10,7 @@ defmodule Farmbot.CeleryScript.AST.Node.ConfigUpdate do
     {:error, msg, env}
   end
 
-  def execute(%{package: :arduino_firmware}, body, env) do
+  def execute(%{package: :arduino_firmware}, _body, env) do
     msg = "`config_update` for Arduino Firmware is depricated."
     env = mutate_env(env)
     {:error, msg, env}

lib/farmbot/farm_event/manager.ex

@@ -18,7 +18,8 @@ defmodule Farmbot.FarmEvent.Manager do
   use GenServer
   use Farmbot.Logger
   alias Farmbot.FarmEvent.Execution
-  alias Farmbot.Asset.FarmEvent
+  alias Farmbot.Asset
+  alias Farmbot.Asset.{FarmEvent, Sequence, Regimen}
 
   # @checkup_time 100
   @checkup_time 30_000
@@ -81,7 +82,7 @@ defmodule Farmbot.FarmEvent.Manager do
 
   def async_checkup(_manager, state) do
     now = get_now()
-    alias Farmbot.Asset.FarmEvent
+
     # maybe_farm_event_log "Rebuilding calendar."
     all_events = Enum.map(state.events, &FarmEvent.build_calendar(&1))
     # maybe_farm_event_log "Rebuilding calendar complete."
@@ -117,10 +118,8 @@ defmodule Farmbot.FarmEvent.Manager do
 
   defp check_event(%FarmEvent{} = f, now, last_time) do
     # Get the executable out of the database this may fail.
-    # mod_list = ["Farmbot", "Asset", f.executable_type]
     mod      = Module.safe_concat([f.executable_type])
-
-    event    = lookup(mod, f.executable_id)
+    event    = lookup!(mod, f.executable_id)
 
     # build a local start time and end time
     start_time = Timex.parse! f.start_time, "{ISO:Extended}"
@@ -131,9 +130,9 @@ defmodule Farmbot.FarmEvent.Manager do
     started?  = Timex.after? now, start_time
     finished? = Timex.after? now, end_time
 
-    case f.executable_type do
-      "Elixir.Farmbot.Asset.Regimen"  -> maybe_start_regimen(started?, start_time, last_time, event, now)
-      "Elixir.Farmbot.Asset.Sequence" -> maybe_start_sequence(started?, finished?, f, last_time, event, now)
+    case mod do
+      Regimen  -> maybe_start_regimen(started?, start_time, last_time, event, now)
+      Sequence -> maybe_start_sequence(started?, finished?, f, last_time, event, now)
     end
   end
 
@@ -154,10 +153,11 @@ defmodule Farmbot.FarmEvent.Manager do
     {nil, last_time}
   end
 
-  defp lookup(module, sr_id) do
-    case Farmbot.Repo.current_repo().get(module, sr_id) do
-      nil -> raise "Could not find #{module} by id: #{sr_id}"
-      item -> item
+  defp lookup!(module, sr_id) when is_atom(module) and is_number(sr_id) do
+    case module do
+      Sequence -> Asset.get_sequence_by_id!(sr_id)
+      Regimen ->  Asset.get_regimen_by_id!(sr_id)
+      _ -> raise "unknown executable type: #{module}"
     end
   end
 

lib/farmbot/regimen/manager.ex

@@ -3,7 +3,10 @@ defmodule Farmbot.Regimen.Manager do
 
   use Farmbot.Logger
   use GenServer
-  alias Farmbot.Asset.Regimen
+  alias Farmbot.CeleryScript
+  alias Farmbot.Asset
+  alias Asset.Regimen
+  import Farmbot.System.ConfigStorage, only: [get_config_value: 3]
 
   defmodule Error do
     @moduledoc false
@@ -15,7 +18,7 @@ defmodule Farmbot.Regimen.Manager do
     @type t :: %__MODULE__{
       name:        binary,
       time_offset: integer,
-      sequence:    Farmbot.CeleryScript.AST.t
+      sequence:    CeleryScript.AST.t
     }
 
     defstruct [:time_offset, :sequence, :name]
@@ -23,7 +26,7 @@ defmodule Farmbot.Regimen.Manager do
     def parse(%{time_offset: offset, sequence_id: sequence_id})
     do
       sequence = fetch_sequence(sequence_id)
-      {:ok, ast} = Farmbot.CeleryScript.AST.decode(sequence)
+      {:ok, ast} = CeleryScript.AST.decode(sequence)
       ast_with_label = %{ast | args: Map.put(ast.args, :label, sequence.name)}
 
       %__MODULE__{
@@ -32,12 +35,7 @@ defmodule Farmbot.Regimen.Manager do
         sequence:    ast_with_label}
     end
 
-    def fetch_sequence(id) do
-      case Farmbot.Repo.current_repo().get(Farmbot.Asset.Sequence, id) do
-        nil -> raise "Could not find sequence by id: #{inspect id}"
-        obj -> obj
-      end
-    end
+    def fetch_sequence(id), do: Asset.get_sequence_by_id!(id)
   end
 
   @doc false
@@ -106,7 +104,7 @@ defmodule Farmbot.Regimen.Manager do
   defp do_item(item, regimen, state) do
     if item do
       Logger.busy 2, "[#{regimen.name}] is going to execute: #{item.name}"
-      Farmbot.CeleryScript.execute(item.sequence)
+      CeleryScript.execute(item.sequence)
     end
     next_item = List.first(regimen.regimen_items)
     if next_item do
@@ -123,7 +121,7 @@ defmodule Farmbot.Regimen.Manager do
     pid, state)
   do
     next_dt         = Timex.shift(state.epoch, milliseconds: nx_itm.time_offset)
-    timezone        = Farmbot.System.ConfigStorage.get_config_value(:string, "settings", "timezone")
+    timezone        = get_config_value(:string, "settings", "timezone")
     now             = Timex.now(timezone)
     offset_from_now = Timex.diff(next_dt, now, :milliseconds)
 
@@ -159,7 +157,7 @@ defmodule Farmbot.Regimen.Manager do
   # returns midnight of today
   @spec build_epoch(DateTime.t) :: DateTime.t
   def build_epoch(time) do
-    tz = Farmbot.System.ConfigStorage.get_config_value(:string, "settings", "timezone")
+    tz = get_config_value(:string, "settings", "timezone")
     n  = Timex.Timezone.convert(time, tz)
     Timex.shift(n, hours: -n.hour, seconds: -n.second, minutes: -n.minute)
   end

test/farmbot/asset/asset_test.exs

@@ -41,6 +41,13 @@ defmodule Farmbot.AssetTest do
   test "Gets a sequence", %{repo: repo} do
     s = sequence(120, "Acuate LEDS", "sequence", %{}, []) |> repo.insert!()
     assert Asset.get_sequence_by_id(s.id) == s
+    assert Asset.get_sequence_by_id!(s.id) == s
+  end
+
+  test "Raises if no sequence", %{repo: repo} do
+    assert_raise fn() ->
+      Asset.get_sequence_by_id!(1000)
+    end
   end
 
   defp sequence(id, name, kind, args, body) do
@@ -61,7 +68,7 @@ defmodule Farmbot.AssetTest do
   end
 
   test "Returns nil when there is no point for a tool" do
-    refute Asset.get_point_from_tool(123)  
+    refute Asset.get_point_from_tool(123)
   end
 
   test "Gets a tool from a point", %{repo: repo} do

test/farmbot/celery_script/ast/node/config_update_test.exs

@@ -1,9 +0,0 @@
-defmodule Farmbot.CeleryScript.AST.Node.ConfigUpdateTest do
-  alias Farmbot.CeleryScript.AST.Node.ConfigUpdate
-  use FarmbotTestSupport.AST.NodeTestCase, async: false
-
-  test "mutates env", %{env: env} do
-    {:ok, env} = ConfigUpdate.execute(%{package: :farmbot_os}, [], env)
-    assert_cs_env_mutation(ConfigUpdate, env)
-  end
-end

test/support/test_update_handler.ex

@@ -21,4 +21,6 @@ defmodule FarmbotTestSupport.TestUpdateHandler do
   def setup(_env) do
     :ok
   end
+
+  def requires_reboot?, do: false
 end