fix: don't reuse requested relationship loads for calculations

this is less efficient, and there are still some cases where
we could combine queries, but we need to first solve the behavioral
issues where relationships loaded for calculations could sometimes be loaded
in an "authorized" state when they should not be. We can improve the
speed/efficiency later, correctness is more important.
This commit is contained in:
Zach Daniel 2024-07-03 18:30:01 -04:00
parent a2cee3cf84
commit 49da3638f1
3 changed files with 74 additions and 99 deletions

View file

@ -1221,6 +1221,7 @@ defmodule Ash.Actions.Read.Calculations do
related_query =
relationship.destination
|> Ash.Query.set_context(%{private: %{lazy?: true}})
|> Ash.Query.select([])
|> merge_query_load(
further,
relationship.domain || domain,
@ -1252,33 +1253,6 @@ defmodule Ash.Actions.Read.Calculations do
Ash.Query.load(query, [{relationship.name, related_query}])
end
else
case query.load[relationship.name] do
nil ->
Ash.Query.load(query, [{relationship.name, further}])
current_load ->
if compatible_relationships?(current_load, further) do
%{
query
| load:
Keyword.put(
query.load,
relationship.name,
merge_query_load(
current_load,
further,
relationship.domain || domain,
calc_path,
calc_name,
calc_load,
relationship_path ++ [relationship.name],
initial_data,
strict_loads?,
reuse_values?
)
)
}
else
{type, constraints} =
case relationship.cardinality do
@ -1294,10 +1268,7 @@ defmodule Ash.Actions.Read.Calculations do
end) do
nil ->
new_calc_name =
{:__calc_dep__,
[
{calc_path, {:rel, relationship.name}, calc_name, calc_load}
]}
{:__calc_dep__, [{calc_path, {:rel, relationship.name}, calc_name, calc_load}]}
query
|> Ash.Query.calculate(
@ -1306,6 +1277,9 @@ defmodule Ash.Actions.Read.Calculations do
{Ash.Resource.Calculation.LoadRelationship,
relationship: relationship.name,
query: further,
opts: [
authorize?: false
],
domain: relationship.domain || domain},
%{},
constraints
@ -1344,8 +1318,6 @@ defmodule Ash.Actions.Read.Calculations do
|> add_calculation_dependency(calc_name, new_calculation.name)
end
end
end
end
true ->
raise "unknown load for #{inspect(query)}: #{inspect(load)}"
@ -1700,9 +1672,9 @@ defmodule Ash.Actions.Read.Calculations do
calc_load,
calc_path,
strict_loads?,
relationship_path,
calc_path,
can_expression_calculation?,
[],
relationship_path,
initial_data,
reuse_values?
)

View file

@ -57,7 +57,8 @@ defmodule Mix.Tasks.Ash.Patch.Extend do
end
defp kind_of_thing(zipper) do
with {:ok, zipper} <- Igniter.Code.Common.move_to_do_block(zipper) do
case Igniter.Code.Common.move_to_do_block(zipper) do
{:ok, zipper} ->
with {_, :error} <-
{Ash.Resource, Igniter.Code.Module.move_to_using(zipper, Ash.Resource)},
{_, :error} <-
@ -67,8 +68,9 @@ defmodule Mix.Tasks.Ash.Patch.Extend do
{kind_of_thing, {:ok, _}} ->
{:ok, kind_of_thing}
end
else
_ -> :error
_ ->
:error
end
end

View file

@ -1,4 +1,5 @@
defmodule Ash.Test.Support.PolicySimple.Calculations.PostTexts do
@moduledoc false
use Ash.Resource.Calculation
@impl true