Indexer.Block.Catchup.BoundIntervalSupervisor is proper supervisor

Fixes #592

Ensure that Indexer.Block.Catchup.BoundIntervalSupervisor acts as a
proper Supervisor by ensure it supports all Supervisor and :supervisor
functions.

apps/indexer/lib/indexer/block/catchup/bound_interval_supervisor.ex

@@ -11,6 +11,8 @@ defmodule Indexer.Block.Catchup.BoundIntervalSupervisor do
   alias Indexer.{Block, BoundInterval}
   alias Indexer.Block.Catchup
 
+  @type named_arguments :: %{required(:block_fetcher) => Block.Fetcher.t(), optional(:block_interval) => pos_integer}
+
   # milliseconds
   @block_interval 5_000
 
@@ -19,10 +21,17 @@ defmodule Indexer.Block.Catchup.BoundIntervalSupervisor do
             fetcher: %Catchup.Fetcher{},
             task: nil
 
-  def child_spec(arg) do
+  @spec child_spec([named_arguments | GenServer.options(), ...]) :: Supervisor.child_spec()
+  def child_spec([named_arguments]) when is_map(named_arguments), do: child_spec([named_arguments, []])
+
+  def child_spec([named_arguments, gen_server_options] = start_link_arguments)
+      when is_map(named_arguments) and is_list(gen_server_options) do
     # The `child_spec` from `use Supervisor` because the one from `use GenServer` will set the `type` to `:worker`
     # instead of `:supervisor` and use the wrong shutdown timeout
-    Supervisor.child_spec(%{id: __MODULE__, start: {__MODULE__, :start_link, [arg]}, type: :supervisor}, [])
+    Supervisor.child_spec(
+      %{id: __MODULE__, start: {__MODULE__, :start_link, start_link_arguments}, type: :supervisor},
+      []
+    )
   end
 
   @doc """
@@ -30,8 +39,10 @@ defmodule Indexer.Block.Catchup.BoundIntervalSupervisor do
 
   For `named_arguments` see `Indexer.BlockFetcher.new/1`.  For `t:GenServer.options/0` see `GenServer.start_link/3`.
   """
-  @spec start_link([named_arguments :: list() | GenServer.options()]) :: {:ok, pid}
-  def start_link([named_arguments, gen_server_options]) when is_map(named_arguments) and is_list(gen_server_options) do
+  @spec start_link(named_arguments :: map()) :: {:ok, pid}
+  @spec start_link(named_arguments :: %{}, GenServer.options()) :: {:ok, pid}
+  def start_link(named_arguments, gen_server_options \\ [])
+      when is_map(named_arguments) and is_list(gen_server_options) do
     GenServer.start_link(__MODULE__, named_arguments, gen_server_options)
   end
 
@@ -57,6 +68,108 @@ defmodule Indexer.Block.Catchup.BoundIntervalSupervisor do
     }
   end
 
+  @impl GenServer
+  def handle_call(:count_children, _from, %__MODULE__{task: task} = state) do
+    active =
+      case task do
+        nil -> 0
+        %Task{} -> 1
+      end
+
+    {:reply, [specs: 1, active: active, supervisors: 0, workers: 1], state}
+  end
+
+  def handle_call({:delete_child, :task}, _from, %__MODULE__{task: task} = state) do
+    reason =
+      case task do
+        nil -> :restarting
+        %Task{} -> :running
+      end
+
+    {:reply, {:error, reason}, state}
+  end
+
+  def handle_call({:delete_child, _}, _from, %__MODULE__{} = state) do
+    {:reply, {:error, :not_found}, state}
+  end
+
+  @task_shutdown_timeout 5_000
+
+  def handle_call({:get_childspec, :task}, _from, %__MODULE__{fetcher: %Catchup.Fetcher{} = catchup} = state) do
+    {:reply,
+     {:ok,
+      %{
+        id: :task,
+        start: {Catchup.Fetcher, :task, [catchup]},
+        restart: :transient,
+        shutdown: @task_shutdown_timeout,
+        type: :worker,
+        modules: [Catchup.Fetcher]
+      }}, state}
+  end
+
+  def handle_call({:get_childspec, _}, _from, %__MODULE__{} = state) do
+    {:reply, {:error, :not_found}, state}
+  end
+
+  def handle_call({:restart_child, :task}, _from, %__MODULE__{fetcher: %Catchup.Fetcher{} = catchup, task: nil} = state) do
+    %Task{pid: pid} = task = Task.Supervisor.async_nolink(Catchup.TaskSupervisor, Catchup.Fetcher, :task, [catchup])
+
+    {:reply, {:ok, pid}, %__MODULE__{state | task: task}}
+  end
+
+  def handle_call({:restart_child, :task}, _from, %__MODULE__{task: %Task{}} = state) do
+    {:reply, {:error, :running}, state}
+  end
+
+  def handle_call({:restart_child, _}, _from, %__MODULE__{} = state) do
+    {:reply, {:error, :not_found}, state}
+  end
+
+  def handle_call({:start_child, %{id: :task}}, _from, %__MODULE__{task: nil} = state) do
+    {:reply, {:error, :already_present}, state}
+  end
+
+  def handle_call({:start_child, %{id: :task}}, _from, %__MODULE__{task: %Task{pid: pid}} = state) do
+    {:reply, {:error, :already_present, pid}, state}
+  end
+
+  def handle_call({:start_child, {:task, _, _, _, _, _}}, _from, %__MODULE__{task: nil} = state) do
+    {:reply, {:error, :already_present}, state}
+  end
+
+  def handle_call({:start_child, {:task, _, _, _, _, _}}, _from, %__MODULE__{task: %Task{pid: pid}} = state) do
+    {:reply, {:error, :already_present, pid}, state}
+  end
+
+  def handle_call({:start_child, _}, _from, %__MODULE__{} = state) do
+    {:reply, {:error, :not_supported}, state}
+  end
+
+  def handle_call({:terminate_child, :task}, _from, %__MODULE__{task: nil} = state) do
+    {:reply, :ok, state}
+  end
+
+  def handle_call({:terminate_child, :task}, _from, %__MODULE__{task: %Task{} = task} = state) do
+    Task.shutdown(task, @task_shutdown_timeout)
+
+    {:reply, :ok, %__MODULE__{state | task: nil}}
+  end
+
+  def handle_call({:terminate_child, _}, _from, %__MODULE__{} = state) do
+    {:reply, {:error, :not_found}, state}
+  end
+
+  def handle_call(:which_children, _from, %__MODULE__{task: task} = state) do
+    child =
+      case task do
+        nil -> :restarting
+        %Task{pid: pid} -> pid
+      end
+
+    {:reply, [{:task, child, :worker, [Catchup.Fetcher]}], state}
+  end
+
   @impl GenServer
   def handle_info(:catchup_index, %__MODULE__{fetcher: %Catchup.Fetcher{} = catchup} = state) do
     {:noreply,

apps/indexer/test/indexer/block/catchup/bound_interval_supervisor_test.exs

@@ -228,6 +228,159 @@ defmodule Indexer.Block.Catchup.BoundIntervalSupervisorTest do
     end
   end
 
+  describe "Supervisor.count_children/1" do
+    setup :supervisor
+
+    test "without task running returns 0 active", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      assert Supervisor.count_children(pid) == %{active: 0, specs: 1, supervisors: 0, workers: 1}
+    end
+
+    test "with task running returns 1 active", %{pid: pid} do
+      assert Supervisor.count_children(pid) == %{active: 1, specs: 1, supervisors: 0, workers: 1}
+    end
+  end
+
+  describe "Supervisor.delete_child/2" do
+    setup :supervisor
+
+    test "with task running returns {:error, :running}", %{pid: pid} do
+      assert {:error, :running} = Supervisor.delete_child(pid, :task)
+    end
+
+    test "without task running returns {:error, :restarting}", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      assert {:error, :restarting} = Supervisor.delete_child(pid, :task)
+    end
+
+    test "with unknown child_id returns {:error, :not_found}", %{pid: pid} do
+      assert {:error, :not_found} = Supervisor.delete_child(pid, :other)
+    end
+  end
+
+  describe ":supervisor.get_childspec/2" do
+    setup :supervisor
+
+    test "with :task", %{pid: pid} do
+      assert {:ok, %{id: :task, modules: [Catchup.Fetcher], restart: _, shutdown: _, start: _, type: :worker}} =
+               :supervisor.get_childspec(pid, :task)
+    end
+
+    test "with unknown child_id returns {:error, :not_found}", %{pid: pid} do
+      assert {:error, :not_found} = :supervisor.get_childspec(pid, :other)
+    end
+  end
+
+  describe "Supervisor.restart_child/2" do
+    setup :supervisor
+
+    test "without :task running restarts task", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      assert {:ok, child_pid} = Supervisor.restart_child(pid, :task)
+
+      assert is_pid(child_pid)
+    end
+
+    test "with :task running returns {:error, :running}", %{pid: pid} do
+      assert {:error, :running} = Supervisor.restart_child(pid, :task)
+    end
+
+    test "with unknown child_id returns {:error, :not_found}", %{pid: pid} do
+      assert {:error, :not_found} = Supervisor.restart_child(pid, :other)
+    end
+  end
+
+  describe "Supervisor.start_child/2" do
+    setup :supervisor
+
+    test "with map with :task without running returns {:error, :already_present}", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      {:ok, child_spec} = :supervisor.get_childspec(pid, :task)
+
+      assert is_map(child_spec)
+
+      assert {:error, :already_present} = Supervisor.start_child(pid, child_spec)
+    end
+
+    test "with map with :task with running returns {:error, :already_present, pid}", %{pid: pid} do
+      {:ok, child_spec} = :supervisor.get_childspec(pid, :task)
+
+      assert is_map(child_spec)
+
+      assert {:error, :already_present, child_pid} = Supervisor.start_child(pid, child_spec)
+      assert is_pid(child_pid)
+    end
+
+    test "with tuple with :task without running returns {:error, :already_present}", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      {:ok, %{id: id, start: start, restart: restart, shutdown: shutdown, type: type, modules: modules}} =
+        :supervisor.get_childspec(pid, :task)
+
+      assert {:error, :already_present} = Supervisor.start_child(pid, {id, start, restart, shutdown, type, modules})
+    end
+
+    test "with tuple with :task with running returns {:error, :already_present, pid}", %{pid: pid} do
+      {:ok, %{id: id, start: start, restart: restart, shutdown: shutdown, type: type, modules: modules}} =
+        :supervisor.get_childspec(pid, :task)
+
+      assert {:error, :already_present, child_pid} =
+               Supervisor.start_child(pid, {id, start, restart, shutdown, type, modules})
+
+      assert is_pid(child_pid)
+    end
+
+    test "with other child_spec returns {:error, :not_supported}", %{pid: pid} do
+      assert {:error, :not_supported} = Supervisor.start_child(pid, %{})
+    end
+  end
+
+  describe "Supervisor.terminate_child/2" do
+    setup :supervisor
+
+    test "with :task without running returns :ok", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      assert :ok = Supervisor.terminate_child(pid, :task)
+    end
+
+    test "with :task running returns :ok after shutting down child", %{pid: pid} do
+      [{:task, child_pid, _, _}] = Supervisor.which_children(pid)
+
+      assert is_pid(child_pid)
+
+      reference = Process.monitor(child_pid)
+
+      assert :ok = Supervisor.terminate_child(pid, :task)
+
+      assert_receive {:DOWN, ^reference, :process, ^child_pid, :shutdown}
+    end
+
+    test "with other child_id returns {:error, :not_found}", %{pid: pid} do
+      assert {:error, :not_found} = Supervisor.terminate_child(pid, :other)
+    end
+  end
+
+  describe "Supervisor.which_children/1" do
+    setup :supervisor
+
+    test "without task running returns child as :restarting", %{pid: pid} do
+      Supervisor.terminate_child(pid, :task)
+
+      assert [{:task, :restarting, :worker, [Catchup.Fetcher]}] = Supervisor.which_children(pid)
+    end
+
+    test "with task running returns child as pid", %{pid: pid} do
+      assert [{:task, child_pid, :worker, [Catchup.Fetcher]}] = Supervisor.which_children(pid)
+
+      assert is_pid(child_pid)
+    end
+  end
+
   describe "handle_info(:catchup_index, state)" do
     setup context do
       # force to use `Mox`, so we can manipulate `lastest_block_number`
@@ -376,4 +529,16 @@ defmodule Indexer.Block.Catchup.BoundIntervalSupervisorTest do
 
     %{state: state}
   end
+
+  defp supervisor(%{json_rpc_named_arguments: json_rpc_named_arguments}) do
+    start_supervised!({Task.Supervisor, name: Indexer.Block.Catchup.TaskSupervisor})
+
+    pid =
+      start_supervised!(
+        {Catchup.BoundIntervalSupervisor,
+         [%{block_fetcher: %Indexer.Block.Fetcher{json_rpc_named_arguments: json_rpc_named_arguments}}]}
+      )
+
+    {:ok, %{pid: pid}}
+  end
 end