diff --git a/documentation/dsls/DSL:-Ash.Resource.md b/documentation/dsls/DSL:-Ash.Resource.md index d1c21b87..b212c089 100644 --- a/documentation/dsls/DSL:-Ash.Resource.md +++ b/documentation/dsls/DSL:-Ash.Resource.md @@ -1528,7 +1528,7 @@ update :flag_for_review, primary?: true |------|------|---------|------| | [`manual`](#actions-update-manual){: #actions-update-manual } | `(any, any -> any) \| module` | | Override the update behavior. Accepts a module or module and opts, or a function that takes the changeset and context. See the [manual actions guide](/documentation/topics/manual-actions.md) for more. | | [`require_atomic?`](#actions-update-require_atomic?){: #actions-update-require_atomic? } | `boolean` | `true` | Require that the update be atomic. This means that all changes and validations implement the `atomic` callback. See the guide on atomic updates for more. | -| [`atomic_upgrade?`](#actions-update-atomic_upgrade?){: #actions-update-atomic_upgrade? } | `boolean` | `true` | If set to `true`, atomic upgrades will be performed. See the update actions guide for more. | +| [`atomic_upgrade?`](#actions-update-atomic_upgrade?){: #actions-update-atomic_upgrade? } | `boolean` | `false` | If set to `true`, atomic upgrades will be performed. Ignored if `required_atomic?` is `true`. See the update actions guide for more. | | [`atomic_upgrade_with`](#actions-update-atomic_upgrade_with){: #actions-update-atomic_upgrade_with } | `:atom \| nil` | | Configure the read action used when performing atomic upgrades. Defaults to the primary read action. | | [`primary?`](#actions-update-primary?){: #actions-update-primary? } | `boolean` | `false` | Whether or not this action should be used when no action is specified by the caller. | | [`description`](#actions-update-description){: #actions-update-description } | `String.t` | | An optional description for the action | @@ -1770,7 +1770,7 @@ end | [`soft?`](#actions-destroy-soft?){: #actions-destroy-soft? } | `boolean` | `false` | If specified, the destroy action behaves as an update internally | | [`manual`](#actions-destroy-manual){: #actions-destroy-manual } | `(any, any -> any) \| module` | | Override the update behavior. Accepts a module or module and opts, or a function that takes the changeset and context. See the [manual actions guide](/documentation/topics/manual-actions.md) for more. | | [`require_atomic?`](#actions-destroy-require_atomic?){: #actions-destroy-require_atomic? } | `boolean` | `true` | Require that the update be atomic. Only relevant if `soft?` is set to `true`. This means that all changes and validations implement the `atomic` callback. See the guide on atomic updates for more. | -| [`atomic_upgrade?`](#actions-destroy-atomic_upgrade?){: #actions-destroy-atomic_upgrade? } | `boolean` | `true` | If set to `true`, atomic upgrades will be performed. See the update actions guide for more. | +| [`atomic_upgrade?`](#actions-destroy-atomic_upgrade?){: #actions-destroy-atomic_upgrade? } | `boolean` | `false` | If set to `true`, atomic upgrades will be performed. See the update actions guide for more. | | [`atomic_upgrade_with`](#actions-destroy-atomic_upgrade_with){: #actions-destroy-atomic_upgrade_with } | `:atom \| nil` | | Configure the read action used when performing atomic upgrades. Defaults to the primary read action. | | [`primary?`](#actions-destroy-primary?){: #actions-destroy-primary? } | `boolean` | `false` | Whether or not this action should be used when no action is specified by the caller. | | [`description`](#actions-destroy-description){: #actions-destroy-description } | `String.t` | | An optional description for the action | diff --git a/lib/ash/actions/read/calculations.ex b/lib/ash/actions/read/calculations.ex index 0e232aaa..7638751a 100644 --- a/lib/ash/actions/read/calculations.ex +++ b/lib/ash/actions/read/calculations.ex @@ -31,6 +31,21 @@ defmodule Ash.Actions.Read.Calculations do end end) + primary_key = + case Ash.Resource.Info.primary_key(resource) do + [] -> + nil + + primary_key -> + Enum.reduce_while(primary_key, %{}, fn key, acc -> + case Map.get(record, key) do + nil -> {:halt, nil} + %Ash.NotLoaded{} -> {:halt, nil} + other -> {:cont, Map.put(acc, key, other)} + end + end) + end + calc_context = %Ash.Resource.Calculation.Context{ actor: opts[:actor], @@ -56,105 +71,109 @@ defmodule Ash.Actions.Read.Calculations do with {:ok, expr} <- expr, {:ok, expr} <- Ash.Filter.hydrate_refs(expr, %{resource: resource}) do - case Ash.Expr.eval(expr, - record: record, - resource: resource, - unknown_on_unknown_refs?: true - ) do - {:ok, result} -> - {:ok, result} + if is_nil(primary_key) && requires_primary_key?(expr) do + {:error, + Ash.Error.Query.CalculationRequiresPrimaryKey.exception( + resource: resource, + calculation: calculation + )} + else + if !is_nil(primary_key) do + case Ash.load(record, [{calculation, arguments}], + actor: opts[:actor], + domain: opts[:domain], + tenant: opts[:tenant], + authorize?: opts[:authorize?], + tracer: opts[:tracer], + resource: opts[:resource], + context: opts[:context] || %{} + ) do + {:ok, record} -> {:ok, Map.get(record, calculation)} + {:error, error} -> {:error, error} + end + else + expr = replace_refs(expr, Keyword.put(opts, :record, record)) - :unknown -> - expr = - Ash.Filter.map(expr, fn - %Ash.Query.Ref{relationship_path: path, attribute: attribute} -> - name = - case attribute do - %{name: name} -> name - name -> name - end - - get_in(opts[:refs] || %{}, path ++ [name]) - - other -> - other - end) - - data_layer_result = - if Ash.DataLayer.data_layer_can?(resource, :calculate) do - Ash.DataLayer.calculate(resource, [expr], opts[:context] || %{}) - else - :cant_calculate + evaled = + try do + Ash.Expr.eval(expr, + record: record, + resource: resource, + unknown_on_unknown_refs?: true + ) + rescue + _ -> + :unknown end - case data_layer_result do + case evaled do {:ok, result} -> - {:ok, Enum.at(result, 0)} + {:ok, result} - :cant_calculate -> - {:error, - "Failed to run calculation in memory, or in the data layer, and no `calculate/3` is defined on #{inspect(module)}. Data layer does not support one-off calculations."} + :unknown -> + data_layer_result = + if Ash.DataLayer.data_layer_can?(resource, :calculate) do + Ash.DataLayer.calculate(resource, [expr], %{ + calculation_context: calc_context, + primary_key: primary_key + }) + else + :cant_calculate + end + + case data_layer_result do + {:ok, result} -> + {:ok, Enum.at(result, 0)} + + :cant_calculate -> + {:error, + "Failed to run calculation in memory, or in the data layer, and no `calculate/3` is defined on #{inspect(module)}. Data layer does not support one-off calculations."} + + {:error, error} -> + if module.has_calculate?() do + case module.calculate([record], calc_opts, calc_context) do + [result] -> + result + + {:ok, [result]} -> + {:ok, result} + + {:ok, _} -> + {:error, "Invalid calculation return"} + + {:error, error} -> + {:error, error} + + :unknown -> + {:error, + "Failed to run calculation in memory, or in the data layer. Data layer returned #{inspect(error)}"} + end + else + {:error, + "Failed to run calculation in memory, or in the data layer, and no `calculate/3` is defined on #{inspect(module)}. Data layer returned #{inspect(error)}"} + end + end {:error, error} -> - if module.has_calculate?() do - case module.calculate([record], calc_opts, calc_context) do - [result] -> - result - - {:ok, [result]} -> - {:ok, result} - - {:ok, _} -> - {:error, "Invalid calculation return"} - - {:error, error} -> - {:error, error} - - :unknown -> - {:error, - "Failed to run calculation in memory, or in the data layer. Data layer returned #{inspect(error)}"} - end - else - {:error, - "Failed to run calculation in memory, or in the data layer, and no `calculate/3` is defined on #{inspect(module)}. Data layer returned #{inspect(error)}"} - end + {:error, error} end - - {:error, error} -> - {:error, error} + end end end else - primary_key = Ash.Resource.Info.primary_key(resource) - if module.has_calculate?() do - if Enum.all?(primary_key, &(not is_nil(Map.get(record, &1)))) do - case Ash.load(record, [{calculation, arguments}], - actor: opts[:actor], - domain: opts[:domain], - tenant: opts[:tenant], - authorize?: opts[:authorize?], - tracer: opts[:tracer], - resource: opts[:resource], - context: opts[:context] || %{} - ) do - {:ok, record} -> {:ok, Map.get(record, calculation)} - {:error, error} -> {:error, error} - end - else - case module.calculate([record], calc_opts, calc_context) do - [result] -> - {:ok, result} + case module.calculate([record], calc_opts, calc_context) do + [result] -> + {:ok, result} - {:ok, [result]} -> - {:ok, result} + {:ok, [result]} -> + {:ok, result} - {:ok, _} -> - {:error, "Invalid calculation return"} + {:ok, _} -> + {:error, "Invalid calculation return"} - {:error, error} -> - {:error, error} - end + {:error, error} -> + {:error, error} end else {:error, "Module #{inspect(module)} does not have an expression or calculate function"} @@ -169,6 +188,53 @@ defmodule Ash.Actions.Read.Calculations do end end + defp requires_primary_key?(expr) do + Ash.Filter.find_value(expr, fn + %Ash.Query.Ref{attribute: %agg_struct{}} + when agg_struct in [Ash.Query.Aggregate, Ash.Resource.Aggregate] -> + true + + %Ash.Query.Exists{} -> + true + + %Ash.Query.Parent{} -> + true + + _ -> + false + end) || + false + end + + defp replace_refs(expr, opts) do + Ash.Filter.map(expr, fn + %Ash.Query.Ref{relationship_path: path, attribute: %Ash.Resource.Attribute{} = attribute} -> + name = + case attribute do + %{name: name} -> name + name -> name + end + + Ash.Expr.get_path(opts[:record] || %{}, path ++ [name]) + + %Ash.Query.Exists{expr: expr} = exists -> + %{ + exists + | expr: + Ash.Filter.map(expr, fn + %Ash.Query.Parent{expr: expr} = parent -> + %{parent | expr: replace_refs(expr, opts)} + + other -> + other + end) + } + + other -> + other + end) + end + def run([], _, _, _calculations_in_query), do: {:ok, []} def run(records, ash_query, calculations_at_runtime, calculations_in_query) do diff --git a/lib/ash/error/query/calculation_requires_primary_key.ex b/lib/ash/error/query/calculation_requires_primary_key.ex new file mode 100644 index 00000000..3824411a --- /dev/null +++ b/lib/ash/error/query/calculation_requires_primary_key.ex @@ -0,0 +1,21 @@ +defmodule Ash.Error.Query.CalculationRequiresPrimaryKey do + @moduledoc "Used when a calculation requires a primary key but was not supplied with one" + use Ash.Error.Exception + + use Splode.Error, fields: [:resource, :calculation], class: :invalid + + def message(error) do + identifier = + if String.Chars.impl_for(error.calculation) do + "#{inspect(error.resource)}.#{error.calculation}" + else + "#{inspect(error.resource)}.#{inspect(error.calculation)}" + end + + """ + Primary key is required for #{identifier}, as it uses aggregates or `exists` expressions. + + In practice, this means accepting a record, or adding arguments for each key in the primary key. + """ + end +end diff --git a/lib/ash/error/query/invalid_calculation_argument.ex b/lib/ash/error/query/invalid_calculation_argument.ex index 9e9f4e55..1d5f7ed6 100644 --- a/lib/ash/error/query/invalid_calculation_argument.ex +++ b/lib/ash/error/query/invalid_calculation_argument.ex @@ -6,7 +6,7 @@ defmodule Ash.Error.Query.InvalidCalculationArgument do def message(error) do """ - Invalid value provided for calculation argument #{error.field} in #{error.calculation}: #{do_message(error)} + Invalid value provided for calculation argument #{error.field} in #{error.calculation}#{do_message(error)} #{inspect(error.value)} """ diff --git a/lib/ash/expr/expr.ex b/lib/ash/expr/expr.ex index 42f1d8a5..c544fe23 100644 --- a/lib/ash/expr/expr.ex +++ b/lib/ash/expr/expr.ex @@ -204,19 +204,20 @@ defmodule Ash.Expr do end) end - defp get_path(map, [key]) when is_struct(map) do + @doc false + def get_path(map, [key]) when is_struct(map) do Map.get(map, key) end - defp get_path(map, [key]) when is_map(map) do + def get_path(map, [key]) when is_map(map) do Map.get(map, key) end - defp get_path(map, [key | rest]) when is_map(map) do + def get_path(map, [key | rest]) when is_map(map) do get_path(get_path(map, [key]), rest) end - defp get_path(_, _), do: nil + def get_path(_, _), do: nil @doc false def template_references_actor?(template) do diff --git a/lib/ash/filter/runtime.ex b/lib/ash/filter/runtime.ex index 8a0b7e8d..f8cc0ae2 100644 --- a/lib/ash/filter/runtime.ex +++ b/lib/ash/filter/runtime.ex @@ -258,6 +258,8 @@ defmodule Ash.Filter.Runtime do defp load_unflattened(record, []), do: record defp load_unflattened(nil, _), do: nil + defp load_unflattened(%Ash.NotLoaded{}, _), do: nil + defp load_unflattened(records, path) when is_list(records) do Enum.map(records, &load_unflattened(&1, path)) end diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index c41aa450..27802046 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -1727,8 +1727,34 @@ defmodule Ash.Query do expr(type(^casted, ^argument.type, ^argument.constraints)) )}} else + cond do + is_nil(casted) && argument.allow_nil? -> + {:cont, {:ok, Map.put(arg_values, argument.name, nil)}} + + is_nil(casted) && is_nil(argument.default) -> + {:halt, + {:error, + InvalidCalculationArgument.exception( + field: argument.name, + calculation: calculation.name, + message: "is required", + value: value + )}} + + is_nil(Map.get(args, argument.name, Map.get(args, to_string(argument.name)))) && + not is_nil(value) -> + {:cont, + {:ok, + Map.put( + arg_values, + argument.name, + value + )}} + true -> + {:cont, {:ok, Map.put(arg_values, argument.name, casted)}} end + end else {:error, error} when is_binary(error) -> {:halt, diff --git a/test/code_interface_test.exs b/test/code_interface_test.exs index 6722d19e..a376848e 100644 --- a/test/code_interface_test.exs +++ b/test/code_interface_test.exs @@ -81,7 +81,7 @@ defmodule Ash.Test.CodeInterfaceTest do calculations do calculate :full_name, :string, expr(first_name <> ^arg(:separator) <> last_name) do public?(true) - argument :separator, :string, default: " ", allow_nil?: false + argument :separator, :string, default: " ", allow_nil?: false, constraints: [allow_empty?: true, trim?: false] end calculate :full_name_functional,