diff --git a/lib/ash/actions/attributes.ex b/lib/ash/actions/attributes.ex index 0781c452..837194fb 100644 --- a/lib/ash/actions/attributes.ex +++ b/lib/ash/actions/attributes.ex @@ -16,7 +16,7 @@ defmodule Ash.Actions.Attributes do changeset: changeset, action_type: action.type, data: - Ash.Engine.Request.UnresolvedField.data( + Ash.Engine.Request.resolve( [[:data, :data]], fn %{data: %{data: data}} -> {:ok, data} diff --git a/lib/ash/actions/create.ex b/lib/ash/actions/create.ex index 5e1d9f7c..ecf8cdec 100644 --- a/lib/ash/actions/create.ex +++ b/lib/ash/actions/create.ex @@ -70,7 +70,7 @@ defmodule Ash.Actions.Create do action_type: action.type, strict_access?: false, data: - Ash.Engine.Request.UnresolvedField.data( + Ash.Engine.Request.resolve( [[:data, :changeset]], fn %{data: %{changeset: changeset}} -> resource @@ -115,14 +115,16 @@ defmodule Ash.Actions.Create do relationship_read_requests ++ relationship_change_requests ++ side_load_requests, api, user: params[:authorization][:user], - bypass_strict_access?: params[:bypass_strict_access?] + bypass_strict_access?: params[:bypass_strict_access?], + verbose?: params[:verbose?] ) else Engine.run( [create_request | attribute_requests] ++ relationship_read_requests ++ relationship_change_requests ++ side_load_requests, api, - fetch_only?: true + fetch_only?: true, + verbose?: params[:verbose?] ) end end diff --git a/lib/ash/actions/destroy.ex b/lib/ash/actions/destroy.ex index 19b4fa4f..f269f7fc 100644 --- a/lib/ash/actions/destroy.ex +++ b/lib/ash/actions/destroy.ex @@ -12,7 +12,7 @@ defmodule Ash.Actions.Destroy do strict_access: false, path: [:data], data: - Ash.Engine.Request.UnresolvedField.data([], fn _ -> + Ash.Engine.Request.resolve(fn _ -> case Ash.data_layer(resource).destroy(record) do :ok -> {:ok, record} {:error, error} -> {:error, error} @@ -28,10 +28,11 @@ defmodule Ash.Actions.Destroy do [auth_request], api, user: params[:authorization][:user], - bypass_strict_access?: params[:bypass_strict_access?] + bypass_strict_access?: params[:bypass_strict_access?], + verbose?: params[:verbose?] ) else - Engine.run([auth_request], api, fetch_only?: true) + Engine.run([auth_request], api, fetch_only?: true, verbose?: params[:verbose?]) end case result do diff --git a/lib/ash/actions/read.ex b/lib/ash/actions/read.ex index ca9d0d38..7b1a350e 100644 --- a/lib/ash/actions/read.ex +++ b/lib/ash/actions/read.ex @@ -59,7 +59,7 @@ defmodule Ash.Actions.Read do action_type: action.type, strict_access?: !Ash.Filter.primary_key_filter?(filter), data: - Request.UnresolvedField.data( + Request.resolve( [[:data, :filter]], Ash.Filter.optional_paths(filter), fn %{data: %{filter: filter}} = data -> @@ -84,10 +84,11 @@ defmodule Ash.Actions.Read do [request | requests], api, user: params[:authorization][:user], - bypass_strict_access?: params[:bypass_strict_access?] + bypass_strict_access?: params[:bypass_strict_access?], + verbose?: params[:verbose?] ) else - Engine.run([request | requests], api, fetch_only?: true) + Engine.run([request | requests], api, fetch_only?: true, verbose?: params[:verbose?]) end end end diff --git a/lib/ash/actions/relationships.ex b/lib/ash/actions/relationships.ex index d0e8128b..e87a8713 100644 --- a/lib/ash/actions/relationships.ex +++ b/lib/ash/actions/relationships.ex @@ -84,7 +84,7 @@ defmodule Ash.Actions.Relationships do changeset: changeset(changeset, api, relationships), action_type: action.type, data: - Ash.Engine.Request.UnresolvedField.data(dependencies, fn + Ash.Engine.Request.resolve(dependencies, fn %{data: %{data: data}} -> {:ok, data} end), @@ -227,7 +227,7 @@ defmodule Ash.Actions.Relationships do resolve_when_fetch_only?: true, path: [:relationships, relationship_name, type], data: - Ash.Engine.Request.UnresolvedField.data([], fn _data -> + Ash.Engine.Request.resolve(fn _data -> case api.read(destination, filter: filter, paginate: false) do {:ok, %{results: results}} -> {:ok, results} {:error, error} -> {:error, error} @@ -360,7 +360,7 @@ defmodule Ash.Actions.Relationships do else dependencies = Map.get(changeset, :__changes_depend_on__, []) - Ash.Engine.Request.UnresolvedField.field(dependencies, fn data -> + Ash.Engine.Request.resolve(dependencies, fn data -> new_changeset = data |> Map.get(:relationships, %{}) @@ -887,7 +887,7 @@ defmodule Ash.Actions.Relationships do resolve_when_fetch_only?: true, filter: filter, data: - Ash.Engine.Request.UnresolvedField.data([], fn _data -> + Ash.Engine.Request.resolve(fn _data -> case api.read(destination, filter: filter, paginate: false) do {:ok, %{results: results}} -> {:ok, results} {:error, error} -> {:error, error} @@ -924,7 +924,7 @@ defmodule Ash.Actions.Relationships do filter: filter, resolve_when_fetch_only?: true, data: - Ash.Engine.Request.UnresolvedField.data([], fn _data -> + Ash.Engine.Request.resolve(fn _data -> case api.read(through, filter: filter_statement) do {:ok, %{results: results}} -> {:ok, results} {:error, error} -> {:error, error} @@ -951,7 +951,7 @@ defmodule Ash.Actions.Relationships do resolve_when_fetch_only?: true, path: [:relationships, name, :current], filter: - Ash.Engine.Request.UnresolvedField.field( + Ash.Engine.Request.resolve( [[:relationships, name, :current_join, :data]], fn %{relationships: %{^name => %{current_join: %{data: current_join}}}} -> field_values = @@ -963,7 +963,7 @@ defmodule Ash.Actions.Relationships do end ), data: - Ash.Engine.Request.UnresolvedField.field( + Ash.Engine.Request.resolve( [[:relationships, name, :current_join, :data]], fn %{relationships: %{^name => %{current_join: %{data: current_join}}}} -> field_values = diff --git a/lib/ash/actions/side_load.ex b/lib/ash/actions/side_load.ex index d507ea82..99ad71b4 100644 --- a/lib/ash/actions/side_load.ex +++ b/lib/ash/actions/side_load.ex @@ -194,7 +194,7 @@ defmodule Ash.Actions.SideLoad do if seed_data do [] else - [[:data]] + [[:data, :data]] end dependencies = @@ -229,7 +229,7 @@ defmodule Ash.Actions.SideLoad do ), strict_access?: true, data: - Ash.Engine.Request.UnresolvedField.data(dependencies, fn data -> + Ash.Engine.Request.resolve(dependencies, fn data -> data = if seed_data do Map.update(data, :data, %{data: seed_data}, fn data_request -> @@ -294,7 +294,7 @@ defmodule Ash.Actions.SideLoad do ), strict_access?: true, data: - Ash.Engine.Request.UnresolvedField.data(dependencies, fn data -> + Ash.Engine.Request.resolve(dependencies, fn data -> if seed_data do Map.update(data, :data, %{data: seed_data}, fn data_request -> Map.put(data_request, :data, seed_data) @@ -342,7 +342,7 @@ defmodule Ash.Actions.SideLoad do _, _ ) do - Ash.Engine.Request.UnresolvedField.field([], fn _ -> + Ash.Engine.Request.resolve(fn _ -> {:error, "Required reverse relationship for #{inspect(relationship)}"} end) end @@ -356,7 +356,7 @@ defmodule Ash.Actions.SideLoad do seed_data ) when root_filter in [:update, :create] do - Ash.Engine.Request.UnresolvedField.field(data_dependency, fn data -> + Ash.Engine.Request.resolve(data_dependency, fn data -> data = if seed_data do seed_data diff --git a/lib/ash/actions/update.ex b/lib/ash/actions/update.ex index f158e985..e368d04b 100644 --- a/lib/ash/actions/update.ex +++ b/lib/ash/actions/update.ex @@ -80,7 +80,7 @@ defmodule Ash.Actions.Update do ), action_type: action.type, data: - Ash.Engine.Request.UnresolvedField.data( + Ash.Engine.Request.resolve( [[:data, :changeset]], fn %{data: %{changeset: changeset}} -> resource @@ -113,13 +113,15 @@ defmodule Ash.Actions.Update do api, strict_access?: false, user: params[:authorization][:user], - bypass_strict_access?: params[:bypass_strict_access?] + bypass_strict_access?: params[:bypass_strict_access?], + verbose?: params[:verbose?] ) else Engine.run( [update_request | attribute_requests] ++ relationship_requests ++ side_load_requests, api, - fetch_only?: true + fetch_only?: true, + verbose?: params[:verbose?] ) end end diff --git a/lib/ash/api/interface.ex b/lib/ash/api/interface.ex index 10e67434..f5cf8d3f 100644 --- a/lib/ash/api/interface.ex +++ b/lib/ash/api/interface.ex @@ -20,13 +20,16 @@ defmodule Ash.Api.Interface do @global_opts Ashton.schema( opts: [ - authorization: [{:const, false}, @authorization_schema] + authorization: [{:const, false}, @authorization_schema], + verbose?: :boolean ], defaults: [ - authorization: false + authorization: false, + verbose?: false ], describe: [ - authorization: "# TODO describe" + authorization: "# TODO describe", + verbose?: "Debug log engine operation" ] ) diff --git a/lib/ash/authorization/checker.ex b/lib/ash/authorization/checker.ex index f7473064..e4812e76 100644 --- a/lib/ash/authorization/checker.ex +++ b/lib/ash/authorization/checker.ex @@ -13,46 +13,39 @@ defmodule Ash.Authorization.Checker do If you need to write your own checks see #TODO: Link to a guide about writing checks here. """ - require Logger alias Ash.Authorization.Clause def strict_check(user, request, facts) do - if Ash.Engine.Request.can_strict_check?(request) do - new_facts = - request.rules - |> Enum.reduce(facts, fn {_step, clause}, facts -> - case Clause.find(facts, clause) do - {:ok, _boolean_result} -> - facts + new_facts = + request.rules + |> Enum.reduce(facts, fn {_step, clause}, facts -> + case Clause.find(facts, clause) do + {:ok, _boolean_result} -> + facts - :error -> - case do_strict_check(clause, user, request) do - {:error, _error} -> - # TODO: Surface this error - facts + :error -> + case do_strict_check(clause, user, request) do + {:error, _error} -> + # TODO: Surface this error + facts - :unknown -> - facts + :unknown -> + facts - :unknowable -> - Map.put(facts, clause, :unknowable) + :unknowable -> + Map.put(facts, clause, :unknowable) - :irrelevant -> - Map.put(facts, clause, :irrelevant) + :irrelevant -> + Map.put(facts, clause, :irrelevant) - boolean -> - Map.put(facts, clause, boolean) - end - end - end) + boolean -> + Map.put(facts, clause, boolean) + end + end + end) - Logger.debug("Completed strict_check for #{request.name}") - - {Map.put(request, :strict_check_complete?, true), new_facts} - else - {request, facts} - end + {Map.put(request, :strict_check_complete?, true), new_facts} end def run_checks(engine, %{data: []}, _clause) do @@ -76,41 +69,38 @@ defmodule Ash.Authorization.Checker do end) end) - # TODO: Handle the possibility of error here - {:ok, authorized_values} = - Ash.Actions.PrimaryKeyHelpers.values_to_primary_key_filters( - request.resource, - authorized - ) - - authorized_filter = - Ash.Filter.parse(request.resource, [or: authorized_values], engine.api) - - {:ok, unauthorized_values} = - Ash.Actions.PrimaryKeyHelpers.values_to_primary_key_filters( - request.resource, - unauthorized - ) - - unauthorized_filter = - Ash.Filter.parse(request.resource, [or: unauthorized_values], engine.api) - - authorized_clause = %{clause | filter: authorized_filter} - unauthorized_clause = %{clause | filter: unauthorized_filter} - case {authorized, unauthorized} do {_, []} -> - {:ok, %{engine | facts: Map.put(engine.facts, authorized_clause, true)}} + {:ok, %{engine | facts: Map.put(engine.facts, clause, true)}} {[], _} -> - {:ok, %{engine | facts: Map.put(engine.facts, unauthorized_clause, false)}} + {:ok, %{engine | facts: Map.put(engine.facts, clause, false)}} - {_authorized, _unauthorized} -> - # TODO: Handle the possibility of error here + {authorized, unauthorized} -> + # TODO: Handle this error + {:ok, authorized_values} = + Ash.Actions.PrimaryKeyHelpers.values_to_primary_key_filters( + request.resource, + authorized + ) + + authorized_filter = + Ash.Filter.parse(request.resource, [or: authorized_values], engine.api) + + {:ok, unauthorized_values} = + Ash.Actions.PrimaryKeyHelpers.values_to_primary_key_filters( + request.resource, + unauthorized + ) + + unauthorized_filter = + Ash.Filter.parse(request.resource, [or: unauthorized_values], engine.api) + + authorized_clause = %{clause | filter: authorized_filter} + unauthorized_clause = %{clause | filter: unauthorized_filter} new_facts = engine.facts - |> Map.delete(clause) |> Map.put(authorized_clause, true) |> Map.put(unauthorized_clause, false) @@ -141,7 +131,7 @@ defmodule Ash.Authorization.Checker do :unknown true -> - :unknowable + :unknowabl end end end diff --git a/lib/ash/authorization/clause.ex b/lib/ash/authorization/clause.ex index b565bc99..3da2590d 100644 --- a/lib/ash/authorization/clause.ex +++ b/lib/ash/authorization/clause.ex @@ -94,6 +94,8 @@ defimpl Inspect, for: Ash.Authorization.Clause do concat([ "#Clause<", + inspect(clause.resource), + ": ", filter, terminator, to_doc(clause.check_module.describe(clause.check_opts), opts), diff --git a/lib/ash/engine/engine.ex b/lib/ash/engine/engine.ex index 6d86a80b..3a2b517d 100644 --- a/lib/ash/engine/engine.ex +++ b/lib/ash/engine/engine.ex @@ -12,7 +12,7 @@ defmodule Ash.Engine do :api, :requests, :user, - :log_transitions?, + :verbose?, :failure_mode, errors: %{}, completed_preparations: %{}, @@ -86,15 +86,15 @@ defmodule Ash.Engine do |> transition(:check) end - defp next(%{state: :check} = engine) do - new_engine = - if Enum.all?(engine.requests, & &1.strict_check_complete?) do - check(engine) - else - strict_check(engine) - end + defp next(%{state: :check, authorized?: true} = engine) do + transition(engine, :resolve_complete, %{message: "Request is already authorized"}) + end - transition(new_engine, :generate_scenarios) + defp next(%{state: :check} = engine) do + engine + |> strict_check() + |> check() + |> transition(:generate_scenarios) end defp next(%{state: :generate_scenarios} = engine) do @@ -118,6 +118,13 @@ defmodule Ash.Engine do |> transition(:check) [] -> + # engine.requests + # |> Enum.reject(&Request.data_resolved?/1) + # |> Enum.map(& &1.name) + # |> IO.inspect(label: "unresolved") + + # IO.inspect(engine.requests) + transition(engine, :complete, %{message: "No requests to resolve"}) end end @@ -125,6 +132,7 @@ defmodule Ash.Engine do defp next(%{state: :resolve_complete} = engine) do case Enum.find(engine.requests, &resolve_for_resolve_complete?(&1, engine)) do nil -> + # case Enum.find(engine.requests, &) transition(engine, :complete, %{message: "No remaining requests that must be resolved"}) request -> @@ -138,26 +146,18 @@ defmodule Ash.Engine do if Request.data_resolved?(request) do false else - case Request.all_dependencies_met?(request, engine.data) do - {true, _must_resolve} -> - request.resolve_when_fetch_only? || is_hard_depended_on?(request, engine.requests) - - false -> - false - end + Request.all_dependencies_met?(request, engine.data, true) && + (request.resolve_when_fetch_only? || is_hard_depended_on?(request, engine.requests)) end end - defp is_hard_depended_on?(request, all_requests) do - remaining_requests = all_requests -- [request] - - all_requests + defp is_hard_depended_on?(request, requests) do + requests |> Enum.reject(& &1.error?) |> Enum.reject(&Request.data_resolved?/1) |> Enum.filter(&Request.depends_on?(&1, request)) |> Enum.any?(fn other_request -> - other_request.resolve_when_fetch_only? || - is_hard_depended_on?(other_request, remaining_requests) + other_request.resolve_when_fetch_only? end) end @@ -209,15 +209,28 @@ defmodule Ash.Engine do end defp resolvable_requests(engine) do - Enum.filter(engine.requests, fn request -> - !request.error? && not request.strict_access? && - match?(%Request.UnresolvedField{}, request.data) && - match?({true, _}, Request.all_dependencies_met?(request, engine.data)) && - maybe_passes_strict_check_in_isolation?(request, engine) + # TODO: Sort these by whether or not their optional deps have been met + # perhaps by the count of unmet optional deps? + # Also, sort them by whether or not they *should* be fetched + Enum.filter(engine.requests, fn + %{data: %Request.UnresolvedField{}} = request -> + !request.error? && Request.all_dependencies_met?(request, engine.data, true) && + allowed_access?(engine, request) + + _request -> + false end) end - defp maybe_passes_strict_check_in_isolation?(request, engine) do + defp allowed_access?(engine, request) do + if request.strict_access? do + passes_strict_check_in_isolation?(request, engine, :definitely) + else + passes_strict_check_in_isolation?(request, engine, :maybe) + end + end + + defp passes_strict_check_in_isolation?(request, engine, condition) do rules_with_data = if Request.data_resolved?(request) do request.data @@ -230,8 +243,12 @@ defmodule Ash.Engine do end case SatSolver.solve(rules_with_data, engine.facts) do - {:ok, _scenarios} -> - true + {:ok, scenarios} -> + if condition == :definitely do + find_real_scenario(scenarios, engine.facts) != nil + else + true + end {:error, :unsatisfiable} -> false @@ -239,13 +256,13 @@ defmodule Ash.Engine do end defp resolve_data(engine, request) do - with {:ok, requirements_resolved} <- resolve_required_paths(engine, request), - {:ok, resolved_request} <- Request.resolve_data(requirements_resolved.data, request), + with {:ok, engine, request} <- resolve_dependencies(request, engine, true), + {:ok, resolved_request} <- Request.resolve_data(engine.data, request), {:ok, prepared_request} <- prepare(engine, resolved_request) do replace_request(engine, prepared_request) else - {:error, path, message, engine} -> - add_error(engine, path, message) + {:error, %__MODULE__{} = engine} -> + engine {:error, error} -> new_request = %{request | error?: true} @@ -256,133 +273,208 @@ defmodule Ash.Engine do end end - defp resolve_required_paths(engine, request) do - case Request.all_dependencies_met?(request, engine.data) do - false -> - raise "Unreachable case" + # defp resolve_required_paths(engine, request, data? \\ true) do + # case Request.all_dependencies_met?(request, engine.data, data?) do + # false -> + # raise "Unreachable case" - {true, dependency_paths} -> - do_resolve_required_paths(dependency_paths, engine, request) - end - end + # {true, dependency_paths} -> + # do_resolve_required_paths(dependency_paths, engine, request) + # end + # end - defp do_resolve_required_paths(dependency_paths, engine, request) do - resolution_result = - dependency_paths - |> Enum.sort_by(&Enum.count/1) - |> Enum.reduce_while({:ok, engine, []}, fn path, {:ok, engine, skipped} -> - case resolve_by_path(path, engine.data, engine.data) do - {data, requests} -> - {:cont, - {:ok, Enum.reduce(requests, %{engine | data: data}, &replace_request(&2, &1, false)), - skipped}} + # defp do_resolve_required_paths(dependency_paths, engine, request) do + # resolution_result = + # dependency_paths + # |> Enum.sort_by(&Enum.count/1) + # |> Enum.reduce_while({:ok, engine}, fn path, {:ok, engine} -> + # dependent_request = Enum.find(engine.requests, &List.starts_with?(path, &1.path)) - {:unmet_dependencies, new_data, new_requests} -> - new_engine = - Enum.reduce( - new_requests, - %{engine | data: new_data}, - &replace_request(&2, &1, false) - ) + # if dependent_request do + # replace_request(engine, %{dependent_request | }) + # end - {:cont, {:ok, new_engine, skipped ++ [path]}} + # # case resolve_by_path(path, engine.data, engine.data) do + # # {data, requests} -> + # # {:cont, + # # {:ok, Enum.reduce(requests, %{engine | data: data}, &replace_request(&2, &1, false)), + # # skipped}} - {:error, new_data, new_requests, path, error} -> - new_engine = - engine - |> Map.put(:data, new_data) - |> replace_request(%{request | error?: true}) - |> add_error(request.path, error) + # # {:unmet_dependencies, new_data, new_requests} -> + # # new_engine = + # # Enum.reduce( + # # new_requests, + # # %{engine | data: new_data}, + # # &replace_request(&2, &1, false) + # # ) - {:halt, - {:error, path, error, - Enum.reduce(new_requests, new_engine, &replace_request(&2, &1, false))}} - end - end) + # # {:cont, {:ok, new_engine, skipped ++ [path]}} - case resolution_result do - {:ok, engine, ^dependency_paths} when dependency_paths != [] -> - [first | rest] = dependency_paths + # # {:error, new_data, new_requests, path, error} -> + # # new_engine = + # # engine + # # |> Map.put(:data, new_data) + # # |> replace_request(%{request | error?: true}) + # # |> add_error(request.path, error) - {:error, first, "Codependent requests.", - Enum.reduce(rest, engine, &add_error(&2, &1, "Codependent requests."))} + # # {:halt, + # # {:error, path, error, + # # Enum.reduce(new_requests, new_engine, &replace_request(&2, &1, false))}} + # # end + # end) - {:ok, engine, []} -> - {:ok, engine} + # case resolution_result do + # {:ok, engine, ^dependency_paths} when dependency_paths != [] -> + # [first | rest] = dependency_paths - {:ok, engine, skipped} -> - do_resolve_required_paths(skipped, engine, request) - end - end + # {:error, first, "Codependent requests.", + # Enum.reduce(rest, engine, &add_error(&2, &1, "Codependent requests."))} - defp resolve_by_path(path, current_data, all_data, requests \\ [], path_prefix \\ []) + # {:ok, engine, []} -> + # {:ok, engine} - defp resolve_by_path([head | tail], current_data, all_data, requests, path_prefix) - when is_map(current_data) do - case Map.fetch(current_data, head) do - {:ok, %Request{} = request} -> - case resolve_by_path(tail, request, all_data, requests, [head | path_prefix]) do - {:error, new_request, new_requests, error_path, message} -> - {:error, Map.put(current_data, request, new_request), - [%{new_request | error?: true} | new_requests], error_path, message} + # {:ok, engine, skipped} -> + # do_resolve_required_paths(skipped, engine, request) + # end + # end - {new_request, new_requests} -> - {Map.put(current_data, head, new_request), [new_request | new_requests]} + # defp resolve_dependency_path(engine, requests, path) do + # requests + # |> Enum.reduce(&resolve_path(engine, &1, path)) + # end - {:unmet_dependencies, new_request, new_requests} -> - {:unmet_dependencies, Map.put(current_data, request, new_request), - [new_request | new_requests]} - end + # defp resolve_path(engine, request, path) do + # if List.starts_with?(path, request.path) do + # case Enum.drop(path, Enum.count(request.path)) do + # [] -> + # true - {:ok, %Request.UnresolvedField{}} when tail != [] -> - {:error, current_data, requests, Enum.reverse(path_prefix) ++ [head], - "Unresolved field while resolving path"} + # remaining_path -> + # case resolve_request_value(engine, request, remaining_path) do + # {:cont, engine, _request} -> + # {:ok, engine} - {:ok, value} -> - case resolve_by_path(tail, value, all_data, requests, [head | path_prefix]) do - {:error, nested_data, new_requests, error_path, message} -> - {:error, Map.put(current_data, value, nested_data), new_requests, error_path, message} + # {:halt, engine} -> + # {:error, engine} + # end + # end + # else + # false + # end + # end - {new_value, new_requests} -> - {Map.put(current_data, head, new_value), new_requests} + # def resolve_request_value(engine, request, path, trail \\ []) - {:unmet_dependencies, new_value, new_requests} -> - {:unmet_dependencies, Map.put(current_data, head, new_value), new_requests} - end + # def resolve_request_value(engine, %Request{} = request, [], _trail) do + # {:cont, engine, request} + # end - nil -> - {:error, current_data, requests, Enum.reverse(path_prefix) ++ [head], - "Missing field while resolving path"} - end - end + # def resolve_request_value(engine, %Request.UnresolvedField{} = unresolved, path, trail) do + # resolved = Request.resolve_field(engine.data, unresolved) - defp resolve_by_path([], value, all_data, requests, path_prefix) do - case value do - %Request.UnresolvedField{} = unresolved -> - case Request.dependencies_met?(all_data, unresolved.depends_on) do - {true, []} -> - case Request.resolve_field(all_data, unresolved) do - {:ok, value} -> {value, requests} - {:error, error} -> {:error, value, requests, Enum.reverse(path_prefix), error} - end + # case resolved do + # {:ok, value} -> + # resolve_request_value(engine, value, path, trail) - {true, _needs} -> - {:unmet_dependencies, unresolved, requests} + # {:error, error} -> + # add_error(engine, Enum.reverse(trail), error) + # end + # end - false -> - {:error, value, requests, Enum.reverse(path_prefix), - "Unmet dependencies while resolving path"} - end + # def resolve_request_value(engine, value, [head | tail], trail) do + # case Map.fetch(value, head) do + # {:ok, nested_value} -> + # case resolve_request_value(engine, nested_value, tail, [head | trail]) do + # {:halt, engine} -> + # {:halt, engine} - other -> - {other, requests} - end - end + # {:cont, new_engine, new_nested_value} -> + # new_value = Map.put(value, head, new_nested_value) - defp resolve_by_path(path, current_data, _all_data, requests, path_prefix) do - {:error, current_data, requests, Enum.reverse(path_prefix) ++ path, - "Invalid data while resolving path."} - end + # new_engine = + # case new_value do + # %Request{} = request -> + # replace_request(engine, request) + + # _ -> + # new_engine + # end + + # {:cont, new_engine, Map.put(value, head, new_nested_value)} + # end + + # :error -> + # {:halt, engine} + # end + # end + + # defp resolve_by_path(path, current_data, all_data, requests \\ [], path_prefix \\ []) + + # defp resolve_by_path([head | tail], current_data, all_data, requests, path_prefix) + # when is_map(current_data) + # when tail != [] do + # case Map.fetch(current_data, head) do + # {:ok, %Request{} = request} -> + # request = Enum.find(requests, &(&1.id == request.id)) || request + + # case resolve_by_path(tail, request, all_data, requests, [head | path_prefix]) do + # {:error, new_request, new_requests, error_path, message} -> + # {:error, Map.put(current_data, head, new_request), + # [%{new_request | error?: true} | new_requests], error_path, message} + + # {new_request, new_requests} -> + # {Map.put(current_data, head, new_request), [new_request | new_requests]} + + # {:unmet_dependencies, new_request, new_requests} -> + # {:unmet_dependencies, Map.put(current_data, head, new_request), + # [new_request | new_requests]} + # end + + # {:ok, value} -> + # case resolve_by_path(tail, value, all_data, requests, [head | path_prefix]) do + # {:error, nested_data, new_requests, error_path, message} -> + # {:error, Map.put(current_data, head, nested_data), new_requests, error_path, message} + + # {new_value, new_requests} -> + # {Map.put(current_data, head, new_value), new_requests} + + # {:unmet_dependencies, new_value, new_requests} -> + # {:unmet_dependencies, Map.put(current_data, head, new_value), new_requests} + # end + + # nil -> + # {:error, current_data, requests, Enum.reverse(path_prefix) ++ [head], + # "Missing field while resolving path"} + # end + # end + + # defp resolve_by_path([], value, all_data, requests, path_prefix) do + # case value do + # %Request.UnresolvedField{} = unresolved -> + # case Request.dependencies_met?(all_data, unresolved.depends_on) do + # {true, []} -> + # case Request.resolve_field(all_data, unresolved) do + # {:ok, value} -> {value, requests} + # {:error, error} -> {:error, value, requests, Enum.reverse(path_prefix), error} + # end + + # {true, _needs} -> + # {:unmet_dependencies, unresolved, requests} + + # false -> + # {:error, value, requests, Enum.reverse(path_prefix), + # "Unmet dependencies while resolving path"} + # end + + # other -> + # {other, requests} + # end + # end + + # defp resolve_by_path(path, current_data, _all_data, requests, path_prefix) do + # {:error, current_data, requests, Enum.reverse(path_prefix) ++ path, + # "Invalid data while resolving path."} + # end defp reality_check(%{authorized?: true} = engine) do transition(engine, :resolve_complete) @@ -482,14 +574,16 @@ defmodule Ash.Engine do Ash.Filter.parse(resource, pkey_filter, api) end - defp check(%{authorized?: true} = engine) do - transition(engine, :resolve_complete) - end - defp check(engine) do case checkable_request(engine) do {:ok, request} -> - run_checks(engine, request) + case resolve_dependencies(request, engine, true) do + {:ok, engine, request} -> + run_checks(engine, request) + + {:error, engine} -> + {:error, engine} + end _ -> engine @@ -540,60 +634,181 @@ defmodule Ash.Engine do end end - defp strict_check(%{authorized?: true} = engine) do - transition(engine, :generate_scenarios) + defp strict_check(engine) do + engine.requests + |> Enum.filter(&Request.can_strict_check(&1, engine.data)) + |> Enum.reduce_while(engine, fn request, engine -> + case resolve_dependencies(request, engine, false) do + {:ok, new_engine, new_request} -> + {new_request, new_facts} = + Checker.strict_check(new_engine.user, new_request, new_engine.facts) + + new_engine = + new_engine + |> replace_request(new_request) + |> Map.put(:facts, new_facts) + + {:cont, new_engine} + + {:error, engine} -> + {:halt, engine} + end + end) end - defp strict_check(engine) do - {requests, facts} = - Enum.reduce(engine.requests, {[], engine.facts}, fn request, {requests, facts} -> - {new_request, new_facts} = Checker.strict_check(engine.user, request, facts) + defp resolve_dependencies(request, engine, data?) do + case do_resolve_dependencies(request, engine, data?) do + :done -> + {:ok, engine, request} - {[new_request | requests], new_facts} + {:ok, engine, request} -> + resolve_dependencies(request, engine, data?) + + {:error, engine, dep, error} -> + {:error, add_error(engine, dep, error)} + end + end + + defp do_resolve_dependencies(request, engine, data?) do + request + |> Map.from_struct() + |> Enum.find(&match?({_, %Request.UnresolvedField{}}, &1)) + |> case do + nil -> + :done + + {key, unresolved} -> + case resolve_field(engine, unresolved, data?) do + {:ok, new_engine, resolved} -> + new_request = Map.put(request, key, resolved) + new_engine = replace_request(new_engine, new_request) + {:ok, new_engine, new_request} + + {:error, new_engine, dep, error} -> + new_engine = replace_request(new_engine, %{request | error?: true}) + {:error, new_engine, dep || request.path ++ [key], error} + end + end + end + + defp resolve_field(engine, %{deps: deps} = unresolved, data?, dep \\ nil) do + result = + Enum.reduce_while(deps, {:ok, engine}, fn dep, {:ok, engine} -> + # TODO: this is inneficient + path = :lists.droplast(dep) + key = List.last(dep) + + other_request = Enum.find(engine.requests, &(&1.path == path)) + + case Map.get(other_request, key) do + %Request.UnresolvedField{data?: field_is_data?} = other_value -> + if field_is_data? and not data? do + {:cont, {:ok, engine}} + else + case resolve_field(engine, other_value, data?, dep) do + {:ok, new_engine, resolved} -> + new_request = Map.put(other_request, key, resolved) + {:cont, {:ok, replace_request(new_engine, new_request)}} + + {:error, engine, dep, error} -> + new_engine = replace_request(engine, %{other_request | error?: true}) + {:halt, {:error, new_engine, dep, error}} + end + end + + _ -> + {:cont, {:ok, engine}} + end end) - transition(engine, :generate_scenarios, %{requests: Enum.reverse(requests), facts: facts}) + case result do + {:ok, new_engine} -> + case Request.resolve_field(new_engine.data, unresolved) do + {:ok, value} -> + {:ok, new_engine, value} + + {:error, error} -> + {:error, engine, dep, error} + end + + {:error, engine, dep, error} -> + {:error, engine, dep, error} + end end defp new(request, api, opts) when not is_list(request), do: new([request], api, opts) defp new(requests, api, opts) do - requests = - if opts[:bypass_strict_access?] do - Enum.map(requests, &Map.put(&1, :strict_access?, false)) - else - requests - end + requests + |> new_engine(api, opts) + |> validate_unique_paths() + |> bypass_strict_access(opts) + |> validate_dependencies() + |> validate_has_rules() + |> log_init() + end - engine = %__MODULE__{ - requests: requests, - user: opts[:user], - api: api, - authorized?: opts[:fetch_only?], - failure_mode: opts[:failure_mode] || :complete, - log_transitions?: Keyword.get(opts, :log_transitions, true) - } + defp validate_dependencies(engine) do + case Request.build_dependencies(engine.requests) do + :impossible -> + add_error(engine, [:__engine__], "Request dependencies are not possible") - if engine.log_transitions? do - Logger.debug( - "Initializing engine with requests: #{ - Enum.map_join(requests, ", ", &(to_string(&1.resource) <> ": " <> &1.name)) - }" - ) + {:ok, _requests} -> + # TODO: no need to aggregate the full dependencies of + engine end + end + defp validate_has_rules(%{authorized?: true} = engine), do: engine + + defp validate_has_rules(engine) do case Enum.find(engine.requests, &Enum.empty?(&1.rules)) do nil -> engine request -> - if opts[:fetch_only?] do - engine - else - exception = Ash.Error.Forbidden.exception(no_steps_configured: request) + add_error(engine, request.path, "No authorization steps configured") + end + end - transition(engine, :complete, %{errors: {:__engine__, exception}}) - end + defp validate_unique_paths(engine) do + case Request.validate_unique_paths(engine.requests) do + :ok -> + engine + + {:error, paths} -> + Enum.reduce(paths, engine, &add_error(&2, &1, "Duplicate requests at path")) + end + end + + defp new_engine(requests, api, opts) do + %__MODULE__{ + requests: requests, + user: opts[:user], + api: api, + authorized?: !!opts[:fetch_only?], + failure_mode: opts[:failure_mode] || :complete, + verbose?: Keyword.get(opts, :verbose?, false) + } + end + + defp log_init(engine) do + if engine.verbose? do + Logger.debug( + "Initializing engine with requests: #{ + Enum.map_join(engine.requests, ", ", &(to_string(&1.resource) <> ": " <> &1.name)) + }" + ) + end + + engine + end + + defp bypass_strict_access(engine, opts) do + if opts[:bypass_strict_access?] do + %{engine | requests: Enum.map(engine.requests, &Map.put(&1, :strict_access?, false))} + else + engine end end @@ -631,7 +846,7 @@ defmodule Ash.Engine do end defp remain(engine, args) do - if engine.log_transitions? do + if engine.verbose? do Logger.debug("Remaining in #{engine.state}#{format_args(args)}") end @@ -640,7 +855,7 @@ defmodule Ash.Engine do end defp transition(engine, state, args \\ %{}) do - if engine.log_transitions? do + if engine.verbose? do Logger.debug("Moving from #{engine.state} to #{state}#{format_args(args)}") end diff --git a/lib/ash/engine/request.ex b/lib/ash/engine/request.ex index a7bd337d..07393d0c 100644 --- a/lib/ash/engine/request.ex +++ b/lib/ash/engine/request.ex @@ -2,23 +2,14 @@ defmodule Ash.Engine.Request do alias Ash.Authorization.{Check, Clause} defmodule UnresolvedField do - defstruct [:resolver, depends_on: [], can_use: [], data?: false] + # TODO: Add some kind of optional dependency? + defstruct [:resolver, deps: [], optional_deps: [], data?: false] - def data(dependencies, can_use \\ [], func) do + def new(dependencies, optional_deps, func) do %__MODULE__{ resolver: func, - depends_on: deps(dependencies), - can_use: deps(can_use), - data?: true - } - end - - def field(dependencies, can_use \\ [], func) do - %__MODULE__{ - resolver: func, - depends_on: deps(dependencies), - can_use: deps(can_use), - data?: false + deps: deps(dependencies), + optional_deps: deps(optional_deps) } end @@ -33,20 +24,9 @@ defmodule Ash.Engine.Request do import Inspect.Algebra def inspect(field, opts) do - data = - if field.data? do - "data! " - else - "" - end - concat([ "#UnresolvedField<", - data, - "needs: ", - to_doc(field.depends_on, opts), - ", can_use: ", - to_doc(field.can_use, opts), + to_doc(field.deps, opts), ">" ]) end @@ -76,6 +56,10 @@ defmodule Ash.Engine.Request do prepared?: false ] + def resolve(dependencies \\ [], optional_dependencies \\ [], func) do + UnresolvedField.new(dependencies, optional_dependencies, func) + end + def new(opts) do filter = case opts[:filter] do @@ -102,6 +86,15 @@ defmodule Ash.Engine.Request do )} end) + data = + case opts[:data] do + %UnresolvedField{} = unresolved -> + %{unresolved | data?: true} + + other -> + other + end + %__MODULE__{ id: Ecto.UUID.generate(), rules: rules, @@ -110,7 +103,7 @@ defmodule Ash.Engine.Request do changeset: opts[:changeset], path: List.wrap(opts[:path]), action_type: opts[:action_type], - data: opts[:data], + data: data, resolve_when_fetch_only?: opts[:resolve_when_fetch_only?], filter: filter, name: opts[:name], @@ -119,14 +112,10 @@ defmodule Ash.Engine.Request do } end - def can_strict_check?(%__MODULE__{strict_check_complete?: true}), do: false + def can_strict_check(%__MODULE__{strict_check_complete?: true}, _state), do: false - def can_strict_check?(request) do - request - |> Map.from_struct() - |> Enum.all?(fn {_key, value} -> - !match?(%UnresolvedField{data?: false}, value) - end) + def can_strict_check(request, state) do + all_dependencies_met?(request, state, false) end def authorize_always(request) do @@ -154,7 +143,6 @@ defmodule Ash.Engine.Request do end def resolve_data(data, %{data: %UnresolvedField{resolver: resolver} = unresolved} = request) do - # {new_data = resolve_ context = resolver_context(data, unresolved) case resolver.(context) do @@ -186,57 +174,46 @@ defmodule Ash.Engine.Request do fetch_nested_value(state, request.path) end - defp resolver_context(state, %{depends_on: depends_on, can_use: can_use}) do + defp resolver_context(state, %{deps: depends_on, optional_deps: optional_deps}) do with_dependencies = Enum.reduce(depends_on, %{}, fn dependency, acc -> {:ok, value} = fetch_nested_value(state, dependency) + put_nested_key(acc, dependency, value) end) - Enum.reduce(can_use, with_dependencies, fn can_use, acc -> - case fetch_nested_value(state, can_use) do - {:ok, value} -> put_nested_key(acc, can_use, value) + Enum.reduce(optional_deps, with_dependencies, fn optional_dep, acc -> + case fetch_nested_value(state, optional_dep) do + {:ok, value} -> put_nested_key(acc, optional_dep, value) _ -> acc end end) end - def all_dependencies_met?(request, state) do - dependencies_met?(state, get_dependencies(request)) + def all_dependencies_met?(request, state, data? \\ true) do + dependencies_met?(state, get_dependencies(request, data?), data?) end - def dependencies_met?(state, dependencies, sources \\ []) - def dependencies_met?(_state, [], _sources), do: {true, []} - def dependencies_met?(_state, nil, _sources), do: {true, []} + def dependencies_met?(state, deps, data? \\ true) + def dependencies_met?(_state, [], _), do: true + def dependencies_met?(_state, nil, _), do: true - def dependencies_met?(state, dependencies, sources) do - Enum.reduce(dependencies, {true, []}, fn - _, false -> - false - - dependency, {true, if_resolved} -> - if dependency in sources do - # Prevent infinite loop on co-dependent requests - # Does it make sense to have to do this? - false - else - case fetch_nested_value(state, dependency) do - {:ok, %UnresolvedField{depends_on: nested_dependencies}} -> - case dependencies_met?(state, nested_dependencies, [dependency | sources]) do - {true, nested_if_resolved} -> - {true, [dependency | if_resolved] ++ nested_if_resolved} - - false -> - false - end - - {:ok, _} -> - {true, if_resolved} - - _ -> - false + def dependencies_met?(state, dependencies, data?) do + Enum.all?(dependencies, fn dependency -> + case fetch_nested_value(state, dependency) do + {:ok, %UnresolvedField{deps: nested_dependencies, data?: dep_is_data?}} -> + if dep_is_data? and not data? do + false + else + dependencies_met?(state, nested_dependencies, data?) end - end + + {:ok, _} -> + true + + _ -> + false + end end) end @@ -265,17 +242,127 @@ defmodule Ash.Engine.Request do Map.fetch(state, key) end - defp get_dependencies(request) do + # Debugging utility + def deps_report(requests) when is_list(requests) do + Enum.map_join(requests, &deps_report/1) + end + + def deps_report(request) do + header = "#{request.name}: \n" + + body = + request + |> Map.from_struct() + |> Enum.filter(&match?({_, %UnresolvedField{}}, &1)) + |> Enum.map_join("\n", fn {key, value} -> + " #{key}: #{inspect(value)}" + end) + + header <> body <> "\n" + end + + def validate_unique_paths(requests) do + requests + |> Enum.group_by(& &1.path) + |> Enum.filter(fn {_path, value} -> + Enum.count(value, & &1.write_to_data?) > 1 + end) + |> case do + [] -> + :ok + + invalid_paths -> + invalid_paths = Enum.map(invalid_paths, &elem(&1, 0)) + + {:error, invalid_paths} + end + end + + def build_dependencies(requests) do + result = + Enum.reduce_while(requests, {:ok, []}, fn request, {:ok, new_requests} -> + case do_build_dependencies(request, requests) do + {:ok, new_request} -> {:cont, {:ok, [new_request | new_requests]}} + {:error, error} -> {:halt, {:error, request.path, error}} + end + end) + + case result do + {:ok, requests} -> {:ok, Enum.reverse(requests)} + other -> other + end + end + + defp do_build_dependencies(request, requests, trail \\ []) do request |> Map.from_struct() - |> Enum.flat_map(fn {_key, value} -> - case value do - %UnresolvedField{depends_on: values} -> - values + |> Enum.reduce_while({:ok, request}, fn + {key, %UnresolvedField{deps: deps} = unresolved}, {:ok, request} -> + case expand_deps(deps, requests, trail) do + {:error, error} -> + {:halt, {:error, error}} + + {:ok, new_deps} -> + {:cont, {:ok, Map.put(request, key, %{unresolved | deps: Enum.uniq(new_deps)})}} + end + + _, {:ok, request} -> + {:cont, {:ok, request}} + end) + end + + defp expand_deps([], _, _), do: {:ok, []} + + defp expand_deps(deps, requests, trail) do + Enum.reduce_while(deps, {:ok, []}, fn dep, {:ok, all_new_deps} -> + case do_expand_dep(dep, requests, trail) do + {:ok, new_deps} -> {:cont, {:ok, all_new_deps ++ new_deps}} + {:error, error} -> {:halt, {:error, error}} + end + end) + end + + defp do_expand_dep(dep, requests, trail) do + if dep in trail do + {:error, {:circular, dep}} + else + # TODO: this is inneficient + request_path = :lists.droplast(dep) + request_key = List.last(dep) + + case Enum.find(requests, &(&1.path == request_path)) do + nil -> + {:error, {:impossible, dep}} + + %{^request_key => %UnresolvedField{deps: nested_deps}} -> + case expand_deps(nested_deps, requests, [dep | trail]) do + {:ok, new_deps} -> {:ok, [dep | new_deps]} + other -> other + end _ -> - [] + {:ok, [dep]} end + end + end + + defp get_dependencies(request, data? \\ true) do + keys_to_drop = + if data? do + [] + else + [:data] + end + + request + |> Map.from_struct() + |> Map.drop(keys_to_drop) + |> Enum.flat_map(fn + {_key, %UnresolvedField{deps: values}} -> + values + + _ -> + [] end) |> Enum.uniq() end diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index 46970c4c..a8694008 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -70,7 +70,7 @@ defmodule Ash.Filter do path: [:filter, path], resolve_when_fetch_only?: false, data: - Ash.Engine.Request.UnresolvedField.data( + Ash.Engine.Request.resolve( [[:filter, path, :filter]], fn %{filter: %{^path => %{filter: filter}}} -> query = Ash.DataLayer.resource_to_query(resource) @@ -435,7 +435,11 @@ defmodule Ash.Filter do do: false def strict_subset_of(filter, candidate) do + # IO.inspect(filter, label: "filter") + # IO.inspect(candidate, label: "candidate") {filter, candidate} = cosimplify(filter, candidate) + # IO.inspect(filter, label: "cosimplified") + # IO.inspect(candidate, label: "cosimplified") Ash.Authorization.SatSolver.strict_filter_subset(filter, candidate) end diff --git a/test/authorization/read_authorization_test.exs b/test/authorization/read_authorization_test.exs index bd1e5aa4..604d427e 100644 --- a/test/authorization/read_authorization_test.exs +++ b/test/authorization/read_authorization_test.exs @@ -105,7 +105,8 @@ defmodule Ash.Test.Authorization.ReadAuthorizationTest do Api.read!(Post, authorization: [user: user], - filter: [authors: [id: author.id]] + filter: [authors: [id: author.id]], + verbose?: true ) end diff --git a/test/test_helper.exs b/test/test_helper.exs index a9a02041..da952fa1 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1,5 +1,5 @@ ExUnit.start() -Logger.configure(level: :error) +Logger.configure(level: :debug) # We compile modules with the same name often while testing the DSL Code.compiler_options(ignore_module_conflict: true)