improvement: improve behavior of lazy?: true option

before, any calculation that determined that it needed to load
would always load its dependencies, but now if they are already loaded
then dependencies won't be reloaded
This commit is contained in:
Zach Daniel 2022-07-11 21:31:50 -04:00
parent a9f93c1ca3
commit 0f3521b2f4
4 changed files with 105 additions and 36 deletions

View file

@ -154,6 +154,28 @@ defmodule Ash.Actions.Read do
error_path = request_opts[:error_path]
lazy? = request_opts[:lazy?]
query =
if initial_data && query && lazy? do
new_load =
query.load
|> List.wrap()
|> Enum.reject(fn load ->
case load do
{key, _value} ->
calculation_or_aggregate?(resource, key) &&
Ash.Resource.Info.loaded?(resource, key)
key ->
calculation_or_aggregate?(resource, key) &&
Ash.Resource.Info.loaded?(resource, key)
end
end)
%{query | load: new_load}
else
query
end
fetch =
Request.new(
resource: resource,
@ -326,7 +348,8 @@ defmodule Ash.Actions.Read do
path,
Map.get(fetched_data, :ultimate_query) || query,
Map.get(fetched_data, :calculations_at_runtime) || [],
get_in(context, path ++ [:calculation_results]) || :error
get_in(context, path ++ [:calculation_results]) || :error,
lazy?
)
|> case do
{:ok, values} ->
@ -377,6 +400,11 @@ defmodule Ash.Actions.Read do
|> Keyword.put(:verbose?, verbose?)
end
defp calculation_or_aggregate?(resource, field) do
!!(Ash.Resource.Info.aggregate(resource, field) ||
Ash.Resource.Info.calculation(resource, field))
end
defp handle_attribute_multitenancy(query) do
multitenancy_attribute = Ash.Resource.Info.multitenancy_attribute(query.resource)
@ -1150,7 +1178,8 @@ defmodule Ash.Actions.Read do
path,
results,
calculation,
query
query,
lazy?
) do
all_calcs = Enum.map(all_calcs, & &1.name)
@ -1166,6 +1195,7 @@ defmodule Ash.Actions.Read do
|> calculation.module.load(calculation.opts, calculation.context)
|> List.wrap()
|> Enum.concat(resource_load)
|> reject_loaded(results, lazy?)
|> Ash.Actions.Helpers.validate_calculation_load!(calculation.module)
|> Enum.map(fn
{key, _} ->
@ -1278,6 +1308,18 @@ defmodule Ash.Actions.Read do
end
end
defp reject_loaded(loads, results, true) do
loads
|> List.wrap()
|> Enum.reject(fn load ->
Ash.Resource.Info.loaded?(results, load)
end)
end
defp reject_loaded(loads, _, _) do
loads
end
defp add_calculation_values(
results,
resource,
@ -1287,7 +1329,8 @@ defmodule Ash.Actions.Read do
path,
query,
calculations,
:error
:error,
lazy?
)
when calculations != [] do
{:requests,
@ -1302,7 +1345,8 @@ defmodule Ash.Actions.Read do
path,
results,
&1,
query
query,
lazy?
)
)}
end
@ -1316,7 +1360,8 @@ defmodule Ash.Actions.Read do
_path,
_query,
_calculations,
calculation_values
calculation_values,
_lazy?
) do
if calculation_values == :error do
{:ok, results}

View file

@ -52,26 +52,29 @@ defmodule Ash.DocIndex do
@behaviour Ash.DocIndex
if guides_from do
@files guides_from
|> Path.wildcard()
|> Enum.map(fn path ->
path
|> Path.split()
|> Enum.drop(1)
|> case do
[category, file] ->
%{
name: Ash.DocIndex.to_name(Path.rootname(file)),
category: Ash.DocIndex.to_name(category),
text: Ash.DocIndex.read!(otp_app, path),
route: "#{Ash.DocIndex.to_path(category)}/#{Ash.DocIndex.to_path(file)}"
}
end
end)
@impl Ash.DocIndex
# sobelow_skip ["Traversal.FileModule"]
def guides do
@files
unquote(otp_app)
|> :code.priv_dir()
|> Path.join(unquote(guides_from))
|> Path.wildcard()
|> Enum.map(fn path ->
path
|> Path.split()
|> Enum.reverse()
|> Enum.take(2)
|> Enum.reverse()
|> case do
[category, file] ->
%{
name: Ash.DocIndex.to_name(Path.rootname(file)),
category: Ash.DocIndex.to_name(category),
text: File.read!(path),
route: "#{Ash.DocIndex.to_path(category)}/#{Ash.DocIndex.to_path(file)}"
}
end
end)
end
defoverridable guides: 0

View file

@ -302,8 +302,6 @@ defmodule Ash.Resource.Dsl do
`change {MyChange, foo: 1}`
`change MyChange`
For destroys, `changes` are not applied unless `soft?` is set to true.
""",
examples: [
"change relate_actor(:reporter)",
@ -319,9 +317,6 @@ defmodule Ash.Resource.Dsl do
name: :argument,
describe: """
Declares an argument on the action
The type can be either a built in type (see `Ash.Type`) for more, or a module implementing
the `Ash.Type` behaviour.
""",
examples: [
"argument :password_confirmation, :string"
@ -357,16 +352,10 @@ defmodule Ash.Resource.Dsl do
A change to be applied to the changeset after it is generated. They are run in order, from top to bottom.
To implement your own, see `Ash.Resource.Change`.
To use it, you can simply refer to the module and its options, like so:
To use it, simply refer to the module and its options or just the module if there are no options, like so:
`change {MyChange, foo: 1}`
But for readability, you may want to define a function elsewhere and import it,
so you can say something like:
`change my_change(1)`
For destroys, `changes` are not applied unless `soft?` is set to true.
`change MyChange`
""",
examples: [
"change relate_actor(:reporter)",

View file

@ -196,6 +196,38 @@ defmodule Ash.Test.CalculationTest do
assert full_names == ["brian cranston brian cranston", "zach daniel zach daniel"]
end
test "it doesn't reload anything specified by the load callback if its already been loaded when using `lazy?: true`" do
full_names =
User
|> Ash.Query.load(:full_name_plus_full_name)
|> Api.read!()
|> Enum.map(&%{&1 | full_name: &1.full_name <> " more"})
|> Api.load!(:full_name_plus_full_name, lazy?: true)
|> Enum.map(& &1.full_name_plus_full_name)
|> Enum.sort()
assert full_names == [
"brian cranston more brian cranston more",
"zach daniel more zach daniel more"
]
end
test "it reloads anything specified by the load callback if its already been loaded when using `lazy?: false`" do
full_names =
User
|> Ash.Query.load(:full_name_plus_full_name)
|> Api.read!()
|> Enum.map(&%{&1 | full_name: &1.full_name <> " more"})
|> Api.load!(:full_name_plus_full_name)
|> Enum.map(& &1.full_name_plus_full_name)
|> Enum.sort()
assert full_names == [
"brian cranston brian cranston",
"zach daniel zach daniel"
]
end
test "nested calculations are loaded if necessary" do
best_friends_names =
User