From f832ab36243cc495391548254ab3da2ecb81c140 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Sat, 1 Apr 2023 01:35:12 -0400 Subject: [PATCH] improvment: loading data shouldn't call before action hooks by this, I mean that loading data shouldn't call before action hooks on the root resource you're loading data on --- lib/ash/actions/read.ex | 125 +++++++++++++++++++++++----------------- lib/ash/query/query.ex | 9 ++- 2 files changed, 77 insertions(+), 57 deletions(-) diff --git a/lib/ash/actions/read.ex b/lib/ash/actions/read.ex index de2d026a..01c91385 100644 --- a/lib/ash/actions/read.ex +++ b/lib/ash/actions/read.ex @@ -286,63 +286,81 @@ defmodule Ash.Actions.Read do query = add_calculation_context(query, actor, authorize?, tenant, tracer) - query = %{ + if Keyword.has_key?(request_opts, :initial_data) do query - | api: api, - timeout: timeout || query.timeout || Ash.Api.Info.timeout(api) - } + |> Ash.Query.set_context(%{ + initial_limit: query.limit, + initial_offset: query.offset, + page_opts: nil, + initial_query: query, + filter_requests: [], + query_opts: query_opts + }) + |> query_with_initial_data(request_opts) + |> case do + %{valid?: true} = query -> + {:ok, query} - query = - if tenant do - Ash.Query.set_tenant(query, tenant) - else - query + %{valid?: false} = query -> + {:error, query.errors, %{set: %{query: query}}} end - - with %{valid?: true} = query <- modify_query.(query, context), - %{limit: initial_limit, offset: initial_offset} <- query, - %{valid?: true} = query <- - handle_attribute_multitenancy(query), - :ok <- validate_multitenancy(query, initial_data), - %{valid?: true} = query <- - query_with_initial_data(query, request_opts), - {%{valid?: true} = query, before_notifications} <- - run_before_action(query), - {:ok, initial_query, query, page_opts} <- - paginate(query, action, page: request_opts[:page]), - page_opts <- page_opts && Keyword.delete(page_opts, :filter), - {:ok, filter_requests} <- - filter_requests(query, path, request_opts, actor, tenant), - {:ok, sort} <- - Ash.Actions.Sort.process( - query.resource, - query.sort, - query.aggregates, - query.context - ) do - {:ok, - query - |> Map.put(:sort, sort) - |> Ash.Query.set_context(%{ - initial_limit: initial_limit, - initial_offset: initial_offset, - page_opts: page_opts, - initial_query: initial_query, - filter_requests: filter_requests, - query_opts: query_opts - }), - %{ - notifications: before_notifications - }} else - %{valid?: false} = query -> - {:error, query.errors} + query = %{ + query + | api: api, + timeout: timeout || query.timeout || Ash.Api.Info.timeout(api) + } - {:error, %Ash.Query{} = query} -> - {:error, query.errors, %{set: %{query: query}}} + query = + if tenant do + Ash.Query.set_tenant(query, tenant) + else + query + end - other -> - other + with %{valid?: true} = query <- modify_query.(query, context), + %{limit: initial_limit, offset: initial_offset} <- query, + %{valid?: true} = query <- + handle_attribute_multitenancy(query), + :ok <- validate_multitenancy(query), + {%{valid?: true} = query, before_notifications} <- + run_before_action(query), + {:ok, initial_query, query, page_opts} <- + paginate(query, action, page: request_opts[:page]), + page_opts <- page_opts && Keyword.delete(page_opts, :filter), + {:ok, filter_requests} <- + filter_requests(query, path, request_opts, actor, tenant), + {:ok, sort} <- + Ash.Actions.Sort.process( + query.resource, + query.sort, + query.aggregates, + query.context + ) do + {:ok, + query + |> Map.put(:sort, sort) + |> Ash.Query.set_context(%{ + initial_limit: initial_limit, + initial_offset: initial_offset, + page_opts: page_opts, + initial_query: initial_query, + filter_requests: filter_requests, + query_opts: query_opts + }), + %{ + notifications: before_notifications + }} + else + %{valid?: false} = query -> + {:error, query.errors} + + {:error, %Ash.Query{} = query} -> + {:error, query.errors, %{set: %{query: query}}} + + other -> + other + end end end ), @@ -586,10 +604,9 @@ defmodule Ash.Actions.Read do end end - defp validate_multitenancy(query, initial_data) do + defp validate_multitenancy(query) do if is_nil(Ash.Resource.Info.multitenancy_strategy(query.resource)) || - Ash.Resource.Info.multitenancy_global?(query.resource) || query.tenant || - initial_data do + Ash.Resource.Info.multitenancy_global?(query.resource) || query.tenant do :ok else {:error, Ash.Error.Invalid.TenantRequired.exception(resource: query.resource)} diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index f2ba95e7..cd16dbdb 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -385,7 +385,8 @@ defmodule Ash.Query do end end - defp set_actor(query, opts) do + @doc false + def set_actor(query, opts) do if Keyword.has_key?(opts, :actor) do put_context(query, :private, %{actor: opts[:actor]}) else @@ -393,7 +394,8 @@ defmodule Ash.Query do end end - defp set_authorize?(query, opts) do + @doc false + def set_authorize?(query, opts) do if Keyword.has_key?(opts, :authorize?) do put_context(query, :private, %{authorize?: opts[:authorize?]}) else @@ -401,7 +403,8 @@ defmodule Ash.Query do end end - defp set_tracer(query, opts) do + @doc false + def set_tracer(query, opts) do if Keyword.has_key?(opts, :tracer) do put_context(query, :private, %{tracer: opts[:tracer]}) else