From 65aa5215fe0c9a7d8d984f33b14946a288aee3a0 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 27 May 2020 00:29:25 -0400 Subject: [PATCH] finalize engine refactor --- lib/ash/actions/update.ex | 2 +- lib/ash/engine/request.ex | 81 +++++++++---------------------- lib/ash/engine/request_handler.ex | 7 ++- lib/ash/engine/runner.ex | 36 ++++++++++++-- 4 files changed, 62 insertions(+), 64 deletions(-) diff --git a/lib/ash/actions/update.ex b/lib/ash/actions/update.ex index eafb59fb..9847cbfd 100644 --- a/lib/ash/actions/update.ex +++ b/lib/ash/actions/update.ex @@ -53,7 +53,7 @@ defmodule Ash.Actions.Update do %Ecto.Changeset{} = changeset -> {:error, Ash.Error.Changeset.changeset_to_errors(resource, changeset)} - %Ash.Engine{errors: errors} -> + %{errors: errors} -> {:error, Ash.to_ash_error(errors)} {:error, error} -> diff --git a/lib/ash/engine/request.ex b/lib/ash/engine/request.ex index 7c84ea95..82082f6e 100644 --- a/lib/ash/engine/request.ex +++ b/lib/ash/engine/request.ex @@ -114,23 +114,19 @@ defmodule Ash.Engine.Request do add_initial_authorizer_state(request) end - @spec next(%{state: :check | :complete | :fetch_data | :strict_check}) :: - {:already_complete, any, any} - | {:complete, any, any} - | {:continue, any, any} - | {:error, any, any} - | {:wait, any, any, any} def next(request) do case do_next(request) do - {:complete, new_request, notifications} -> + {:complete, new_request, notifications, dependencies} -> + dependencies = new_dependencies(dependencies) + if request.state != :complete do - {:complete, new_request, notifications} + {:complete, new_request, notifications, dependencies} else - {:already_complete, new_request, notifications} + {:already_complete, new_request, notifications, dependencies} end {:waiting, new_request, notifications, dependencies} -> - {new_request, dependencies} = new_dependencies(new_request, dependencies) + dependencies = new_dependencies(dependencies) {:wait, new_request, notifications, dependencies} @@ -186,21 +182,21 @@ defmodule Ash.Engine.Request do def do_next(%{state: :check, authorize?: false} = request) do log(request, "Skipping check due to `authorize?: false`") - {:complete, %{request | state: :complete}, []} + {:complete, %{request | state: :complete}, [], []} end def do_next(%{state: :check} = request) do case Ash.authorizers(request.resource) do [] -> log(request, "No authorizers found, skipping check") - {:complete, %{request | state: :complete}, []} + {:complete, %{request | state: :complete}, [], []} authorizers -> case check(authorizers, request) do {:ok, new_request, notifications, []} -> log(new_request, "Check complete") - {:complete, %{request | state: :complete}, notifications} + {:complete, %{request | state: :complete}, notifications, []} {:ok, new_request, notifications, waiting} -> log(request, "Check incomplete, waiting on dependencies") @@ -215,7 +211,7 @@ defmodule Ash.Engine.Request do def do_next(%{state: :complete} = request) do if request.dependencies_to_send == %{} do - {:complete, request, []} + {:complete, request, [], []} else Enum.reduce_while(request.dependencies_to_send, {:complete, request, [], []}, fn {field, _paths}, {:complete, request, notifications, deps} -> @@ -255,7 +251,7 @@ defmodule Ash.Engine.Request do {:ok, new_request, notifications} {:waiting, new_request, notifications, waiting_for} -> - {new_request, dependency_requests} = new_dependencies(new_request, waiting_for) + dependency_requests = new_dependencies(waiting_for) {:waiting, new_request, notifications, dependency_requests} @@ -273,52 +269,20 @@ defmodule Ash.Engine.Request do {:continue, new_request} end - defp new_dependencies(request, waiting_for) do - {non_optional, optional} = Enum.split_with(waiting_for, &is_list/1) + defp new_dependencies(waiting_for) do + Enum.map( + waiting_for, + fn + {:optional, dep} -> + {dep, false} - {new_request, deps} = - Enum.reduce(non_optional, {request, []}, fn dep, {request, deps} -> - {new_request, new_dep} = new_dependency(request, dep, false) + {:required, dep} -> + {dep, true} - if new_dep do - {new_request, [new_dep | deps]} - else - {new_request, deps} - end - end) - - Enum.reduce(optional, {new_request, deps}, fn dep, {request, deps} -> - {new_request, new_dep} = new_dependency(request, dep, true) - - if new_dep do - {new_request, [new_dep | deps]} - else - {new_request, deps} + other -> + {other, true} end - end) - end - - defp new_dependency(request, dep, optional?) do - if Enum.any?(request.dependencies_requested, fn {dep_requested, _} -> dep_requested == dep end) do - new_request = %{ - request - | dependencies_requested: - Enum.map(request.dependencies_requested, fn {dep_requested, requested_optional?} -> - if dep_requested == dep do - {dep_requested, optional? || requested_optional?} - else - {dep_requested, requested_optional?} - end - end) - } - - {new_request, nil} - else - dependency_request = {dep, optional?} - - {%{request | dependencies_requested: [dependency_request | request.dependencies_requested]}, - dependency_request} - end + ) end def put_dependency_data(request, dep, value) do @@ -630,6 +594,7 @@ defmodule Ash.Engine.Request do defp notifications(request, field, value) do case Map.fetch(request.dependencies_to_send, field) do + # TODO: This logic could technically cause double sends? {:ok, paths} -> new_request = %{ request diff --git a/lib/ash/engine/request_handler.ex b/lib/ash/engine/request_handler.ex index 7cbacff9..4d939413 100644 --- a/lib/ash/engine/request_handler.ex +++ b/lib/ash/engine/request_handler.ex @@ -44,16 +44,19 @@ defmodule Ash.Engine.RequestHandler do {:error, error, new_request} -> {:stop, {:error, error, %{new_request | state: :error}}, state} - {:already_complete, new_request, notifications} -> + {:already_complete, new_request, notifications, dependencies} -> new_state = %{state | request: new_request} + Enum.each(dependencies, ®ister_dependency(new_state, &1)) notify(new_state, notifications) {:noreply, new_state} - {:complete, new_request, notifications} -> + {:complete, new_request, notifications, dependencies} -> new_state = %{state | request: new_request} notify(new_state, notifications) + Enum.each(dependencies, ®ister_dependency(new_state, &1)) + complete(new_state) {:noreply, new_state} diff --git a/lib/ash/engine/runner.ex b/lib/ash/engine/runner.ex index fafab67f..b9d86dd5 100644 --- a/lib/ash/engine/runner.ex +++ b/lib/ash/engine/runner.ex @@ -55,7 +55,7 @@ defmodule Ash.Engine.Runner do if new_state.engine_pid do wait_for_engine(new_state, false) else - log(state, "Synchronous engine stuck.") + log(state, "Synchronous engine stuck:\n\n#{stuck_report(state)}") add_error(state, :__engine__, "Synchronous engine stuck") end @@ -65,6 +65,36 @@ defmodule Ash.Engine.Runner do end end + defp stuck_report(state) do + Enum.map_join(state.requests, "\n", fn request -> + if request.state in [:complete, :error] do + request.name <> ": " <> "#{request.state}" + else + case Request.next(request) do + {:wait, _, _, []} -> + request.name <> ": Waiting on nothing in state #{inspect(request.state)}" + + {:wait, _, _, dependencies} -> + request.name <> ": Waiting on #{dependency_names(dependencies, state)}" + + _other -> + request.name <> ": Not waiting, not complete" + end + end + end) + end + + defp dependency_names(dependencies, state) do + Enum.map_join(dependencies, ", ", fn dependency -> + path = :lists.droplast(dependency) + field = List.last(dependency) + + request = Enum.find(state.requests, &(&1.path == path)) + + request.name <> ": " <> to_string(field) + end) + end + defp wait_for_engine(state, complete?) do engine_pid = state.engine_pid log(state, "waiting for engine") @@ -310,9 +340,9 @@ defmodule Ash.Engine.Runner do defp advance_request(request, notifications \\ [], dependencies \\ []) do case Request.next(request) do - {complete, new_request, new_notifications} + {complete, new_request, new_notifications, new_dependencies} when complete in [:complete, :already_complete] -> - {:ok, new_request, new_notifications ++ notifications, dependencies} + {:ok, new_request, new_notifications ++ notifications, new_dependencies ++ dependencies} {:continue, new_request, new_notifications} -> {:ok, new_request, new_notifications ++ notifications, []}