From dc51a961c150df44e28cc20b48e4173ec5b8f646 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Thu, 19 Jan 2023 18:04:48 -0500 Subject: [PATCH] improvement: include value in invalid error messages --- lib/ash/changeset/changeset.ex | 33 ++++++++++------- lib/ash/data_layer/mnesia/mnesia.ex | 35 +++++++++++++++++++ lib/ash/error/changes/invalid_argument.ex | 8 +++-- lib/ash/error/changes/invalid_attribute.ex | 8 +++-- lib/ash/error/query/invalid_argument.ex | 8 +++-- lib/ash/query/query.ex | 10 +++--- .../validation/attribute_does_not_equal.ex | 5 ++- .../resource/validation/attribute_equals.ex | 5 ++- lib/ash/resource/validation/compare.ex | 7 +++- lib/ash/resource/validation/confirm.ex | 1 + lib/ash/resource/validation/match.ex | 2 ++ lib/ash/resource/validation/one_of.ex | 1 + lib/ash/resource/validation/present.ex | 10 +++--- lib/ash/resource/validation/string_length.ex | 5 +++ 14 files changed, 108 insertions(+), 30 deletions(-) diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex index c593dd35..bcf1f5a3 100644 --- a/lib/ash/changeset/changeset.ex +++ b/lib/ash/changeset/changeset.ex @@ -988,10 +988,14 @@ defmodule Ash.Changeset do |> Enum.reject(fn {key, _value} -> key in accepted_attributes end) - |> Enum.reduce(changeset, fn {key, _}, changeset -> + |> Enum.reduce(changeset, fn {key, value}, changeset -> add_error( changeset, - InvalidAttribute.exception(field: key, message: "cannot be changed") + InvalidAttribute.exception( + field: key, + message: "cannot be changed", + value: value + ) ) end) end @@ -1308,11 +1312,12 @@ defmodule Ash.Changeset do defp override_validation_message(error, message) do case error do - %{field: field} when not is_nil(field) -> + %{field: field} = error when not is_nil(field) -> error |> Map.take([:field, :vars]) |> Map.to_list() |> Keyword.put(:message, message) + |> Keyword.put(:value, Map.get(error, :value)) |> InvalidAttribute.exception() %{fields: fields} when fields not in [nil, []] -> @@ -1320,6 +1325,7 @@ defmodule Ash.Changeset do |> Map.take([:fields, :vars]) |> Map.to_list() |> Keyword.put(:message, message) + |> Keyword.put(:value, Map.get(error, :value)) |> InvalidChanges.exception() _ -> @@ -2706,7 +2712,7 @@ defmodule Ash.Changeset do | arguments: Map.put(changeset.arguments, argument.name, value) } - add_invalid_errors(:argument, changeset, argument, error) + add_invalid_errors(value, :argument, changeset, argument, error) {:error, error} -> changeset = %{ @@ -2714,7 +2720,7 @@ defmodule Ash.Changeset do | arguments: Map.put(changeset.arguments, argument.name, value) } - add_invalid_errors(:argument, changeset, argument, error) + add_invalid_errors(value, :argument, changeset, argument, error) end else %{changeset | arguments: Map.put(changeset.arguments, argument, value)} @@ -2807,7 +2813,7 @@ defmodule Ash.Changeset do | attributes: Map.put(changeset.attributes, attribute.name, value) } - add_invalid_errors(:attribute, changeset, attribute, "Attribute is not writable") + add_invalid_errors(value, :attribute, changeset, attribute, "Attribute is not writable") attribute -> with value <- handle_indexed_maps(attribute.type, value), @@ -2859,7 +2865,7 @@ defmodule Ash.Changeset do defaults: changeset.defaults -- [attribute.name] } - add_invalid_errors(:attribute, changeset, attribute, error_or_errors) + add_invalid_errors(value, :attribute, changeset, attribute, error_or_errors) :error -> changeset = %{ @@ -2868,7 +2874,7 @@ defmodule Ash.Changeset do defaults: changeset.defaults -- [attribute.name] } - add_invalid_errors(:attribute, changeset, attribute) + add_invalid_errors(value, :attribute, changeset, attribute) {:error, error_or_errors} -> changeset = %{ @@ -2877,7 +2883,7 @@ defmodule Ash.Changeset do defaults: changeset.defaults -- [attribute.name] } - add_invalid_errors(:attribute, changeset, attribute, error_or_errors) + add_invalid_errors(value, :attribute, changeset, attribute, error_or_errors) end end end @@ -3034,7 +3040,7 @@ defmodule Ash.Changeset do | attributes: Map.put(changeset.attributes, attribute.name, value) } - add_invalid_errors(:attribute, changeset, attribute) + add_invalid_errors(value, :attribute, changeset, attribute) {:error, error_or_errors} -> changeset = %{ @@ -3042,7 +3048,7 @@ defmodule Ash.Changeset do | attributes: Map.put(changeset.attributes, attribute.name, value) } - add_invalid_errors(:attribute, changeset, attribute, error_or_errors) + add_invalid_errors(value, :attribute, changeset, attribute, error_or_errors) end end end @@ -3289,12 +3295,14 @@ defmodule Ash.Changeset do InvalidAttribute.exception( field: keyword[:field], message: keyword[:message], + value: keyword[:value], vars: keyword ) else InvalidChanges.exception( fields: keyword[:fields] || [], message: keyword[:message], + value: keyword[:value], vars: keyword ) end @@ -3320,7 +3328,7 @@ defmodule Ash.Changeset do Ash.Type.handle_change(attribute.type, old_value, value, constraints) end - defp add_invalid_errors(type, changeset, attribute, message \\ nil) do + defp add_invalid_errors(value, type, changeset, attribute, message \\ nil) do messages = if Keyword.keyword?(message) do [message] @@ -3340,6 +3348,7 @@ defmodule Ash.Changeset do Enum.reduce(opts, changeset, fn opts, changeset -> error = exception.exception( + value: value, field: Keyword.get(opts, :field), message: Keyword.get(opts, :message), vars: opts diff --git a/lib/ash/data_layer/mnesia/mnesia.ex b/lib/ash/data_layer/mnesia/mnesia.ex index 7132f78a..15161bfa 100644 --- a/lib/ash/data_layer/mnesia/mnesia.ex +++ b/lib/ash/data_layer/mnesia/mnesia.ex @@ -85,6 +85,7 @@ defmodule Ash.DataLayer.Mnesia do @doc false @impl true def can?(_, :async_engine), do: true + def can?(_, :multitenancy), do: true def can?(_, :composite_primary_key), do: true def can?(_, :upsert), do: true def can?(_, :create), do: true @@ -94,6 +95,7 @@ defmodule Ash.DataLayer.Mnesia do def can?(_, :sort), do: true def can?(_, :filter), do: true def can?(_, {:filter_relationship, _}), do: true + def can?(_, {:query_aggregate, :count}), do: true def can?(_, :limit), do: true def can?(_, :offset), do: true def can?(_, :boolean_filter), do: true @@ -150,6 +152,39 @@ defmodule Ash.DataLayer.Mnesia do {:ok, %{query | sort: sort}} end + @doc false + @impl true + def run_aggregate_query(%{api: api} = query, aggregates, resource) do + case run_query(query, resource) do + {:ok, results} -> + Enum.reduce_while(aggregates, {:ok, %{}}, fn + %{kind: :count, name: name, query: query}, {:ok, acc} -> + api + |> Ash.Filter.Runtime.filter_matches(results, Map.get(query || %{}, :filter)) + |> case do + {:ok, matches} -> + {:cont, {:ok, Map.put(acc, name, Enum.count(matches))}} + + {:error, error} -> + {:halt, {:error, error}} + end + + _, _ -> + {:halt, {:error, "unsupported aggregate"}} + end) + + {:error, error} -> + {:error, error} + end + |> case do + {:error, error} -> + {:error, Ash.Error.to_ash_error(error)} + + other -> + other + end + end + @doc false @impl true def run_query( diff --git a/lib/ash/error/changes/invalid_argument.ex b/lib/ash/error/changes/invalid_argument.ex index 4eb4a997..091923ea 100644 --- a/lib/ash/error/changes/invalid_argument.ex +++ b/lib/ash/error/changes/invalid_argument.ex @@ -2,7 +2,7 @@ defmodule Ash.Error.Changes.InvalidArgument do @moduledoc "Used when an invalid value is provided for an action argument" use Ash.Error.Exception - def_ash_error([:field, :message], class: :invalid) + def_ash_error([:field, :message, :value], class: :invalid) defimpl Ash.ErrorKind do def id(_), do: Ash.UUID.generate() @@ -10,7 +10,11 @@ defmodule Ash.Error.Changes.InvalidArgument do def code(_), do: "invalid_argument" def message(error) do - "Invalid value provided#{for_field(error)}#{do_message(error)}" + """ + Invalid value provided#{for_field(error)}#{do_message(error)} + + #{inspect(error.value)} + """ end defp for_field(%{field: field}) when not is_nil(field), do: " for #{field}" diff --git a/lib/ash/error/changes/invalid_attribute.ex b/lib/ash/error/changes/invalid_attribute.ex index 528c0a70..a32f9c1b 100644 --- a/lib/ash/error/changes/invalid_attribute.ex +++ b/lib/ash/error/changes/invalid_attribute.ex @@ -2,7 +2,7 @@ defmodule Ash.Error.Changes.InvalidAttribute do @moduledoc "Used when an invalid value is provided for an attribute change" use Ash.Error.Exception - def_ash_error([:field, :message, :private_vars], class: :invalid) + def_ash_error([:field, :message, :private_vars, :value], class: :invalid) defimpl Ash.ErrorKind do def id(_), do: Ash.UUID.generate() @@ -10,7 +10,11 @@ defmodule Ash.Error.Changes.InvalidAttribute do def code(_), do: "invalid_attribute" def message(error) do - "Invalid value provided#{for_field(error)}#{do_message(error)}" + """ + Invalid value provided#{for_field(error)}#{do_message(error)} + + #{inspect(error.value)} + """ end defp for_field(%{field: field}) when not is_nil(field), do: " for #{field}" diff --git a/lib/ash/error/query/invalid_argument.ex b/lib/ash/error/query/invalid_argument.ex index dc1410b9..f15fd876 100644 --- a/lib/ash/error/query/invalid_argument.ex +++ b/lib/ash/error/query/invalid_argument.ex @@ -2,7 +2,7 @@ defmodule Ash.Error.Query.InvalidArgument do @moduledoc "Used when an invalid value is provided for an action argument" use Ash.Error.Exception - def_ash_error([:field, :message], class: :invalid) + def_ash_error([:field, :message, :value], class: :invalid) defimpl Ash.ErrorKind do def id(_), do: Ash.UUID.generate() @@ -10,7 +10,11 @@ defmodule Ash.Error.Query.InvalidArgument do def code(_), do: "invalid_argument" def message(error) do - "Invalid value provided#{for_field(error)}#{do_message(error)}" + """ + Invalid value provided#{for_field(error)}#{do_message(error)} + + #{inspect(error.value)} + """ end defp for_field(%{field: field}) when not is_nil(field), do: " for #{field}" diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index fc91ceb5..687b25d4 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -1182,15 +1182,15 @@ defmodule Ash.Query do {:constrained, {:error, error}, argument} -> query = %{query | arguments: Map.put(query.arguments, argument.name, value)} - add_invalid_errors(query, argument, error) + add_invalid_errors(value, query, argument, error) {:error, error} -> query = %{query | arguments: Map.put(query.arguments, argument.name, value)} - add_invalid_errors(query, argument, error) + add_invalid_errors(value, query, argument, error) :error -> query = %{query | arguments: Map.put(query.arguments, argument.name, value)} - add_invalid_errors(query, argument, "is invalid") + add_invalid_errors(value, query, argument, "is invalid") end else %{query | arguments: Map.put(query.arguments, argument, value)} @@ -1205,7 +1205,7 @@ defmodule Ash.Query do defp reset_arguments(query), do: query - defp add_invalid_errors(query, argument, error) do + defp add_invalid_errors(value, query, argument, error) do messages = if Keyword.keyword?(error) do [error] @@ -1218,7 +1218,7 @@ defmodule Ash.Query do message |> Ash.Changeset.error_to_exception_opts(argument) |> Enum.reduce(query, fn opts, query -> - add_error(query, InvalidArgument.exception(opts)) + add_error(query, InvalidArgument.exception(Keyword.put(opts, :value, value))) end) end) end diff --git a/lib/ash/resource/validation/attribute_does_not_equal.ex b/lib/ash/resource/validation/attribute_does_not_equal.ex index 424020c6..c2a6e7e0 100644 --- a/lib/ash/resource/validation/attribute_does_not_equal.ex +++ b/lib/ash/resource/validation/attribute_does_not_equal.ex @@ -31,10 +31,13 @@ defmodule Ash.Resource.Validation.AttributeDoesNotEqual do @impl true def validate(changeset, opts) do - if Ash.Changeset.get_attribute(changeset, opts[:attribute]) == opts[:value] do + value = Ash.Changeset.get_attribute(changeset, opts[:attribute]) + + if value == opts[:value] do {:error, InvalidAttribute.exception( field: opts[:attribute], + value: value, message: "must not equal %{value}", vars: [field: opts[:attribute], value: opts[:value]] )} diff --git a/lib/ash/resource/validation/attribute_equals.ex b/lib/ash/resource/validation/attribute_equals.ex index cec9c4f8..4684ae29 100644 --- a/lib/ash/resource/validation/attribute_equals.ex +++ b/lib/ash/resource/validation/attribute_equals.ex @@ -30,9 +30,12 @@ defmodule Ash.Resource.Validation.AttributeEquals do @impl true def validate(changeset, opts) do - if Ash.Changeset.get_attribute(changeset, opts[:attribute]) != opts[:value] do + value = Ash.Changeset.get_attribute(changeset, opts[:attribute]) + + if value != opts[:value] do {:error, InvalidAttribute.exception( + value: value, field: opts[:attribute], message: "must equal %{value}", vars: [field: opts[:attribute], value: opts[:value]] diff --git a/lib/ash/resource/validation/compare.ex b/lib/ash/resource/validation/compare.ex index c63368f6..7539d44d 100644 --- a/lib/ash/resource/validation/compare.ex +++ b/lib/ash/resource/validation/compare.ex @@ -66,6 +66,7 @@ defmodule Ash.Resource.Validation.Compare do else: invalid_attribute_error( Keyword.put(opts, :value, attribute), + value, "must be greater than %{value}" ) @@ -75,6 +76,7 @@ defmodule Ash.Resource.Validation.Compare do else: invalid_attribute_error( Keyword.put(opts, :value, attribute), + value, "must be greater than or equal to %{value}" ) @@ -84,6 +86,7 @@ defmodule Ash.Resource.Validation.Compare do else: invalid_attribute_error( Keyword.put(opts, :value, attribute), + value, "must be less than %{value}" ) @@ -93,6 +96,7 @@ defmodule Ash.Resource.Validation.Compare do else: invalid_attribute_error( Keyword.put(opts, :value, attribute), + value, "must be less than or equal to %{value}" ) @@ -116,11 +120,12 @@ defmodule Ash.Resource.Validation.Compare do defp attribute_value(_, attribute), do: attribute - defp invalid_attribute_error(opts, message) do + defp invalid_attribute_error(opts, attribute_value, message) do {:error, InvalidAttribute.exception( field: opts[:attribute], message: opts[:message] || message, + value: attribute_value, vars: [ value: case opts[:value] do diff --git a/lib/ash/resource/validation/confirm.ex b/lib/ash/resource/validation/confirm.ex index 6ca2eaf5..1d5597fe 100644 --- a/lib/ash/resource/validation/confirm.ex +++ b/lib/ash/resource/validation/confirm.ex @@ -41,6 +41,7 @@ defmodule Ash.Resource.Validation.Confirm do {:error, InvalidAttribute.exception( field: opts[:confirmation], + value: confirmation_value, message: "Confirmation did not match value" )} end diff --git a/lib/ash/resource/validation/match.ex b/lib/ash/resource/validation/match.ex index f7d80622..2da6aad8 100644 --- a/lib/ash/resource/validation/match.ex +++ b/lib/ash/resource/validation/match.ex @@ -46,6 +46,7 @@ defmodule Ash.Resource.Validation.Match do {:error, InvalidAttribute.exception( field: opts[:attribute], + value: changing_to, message: opts[:message], vars: [match: opts[:match]] )} @@ -66,6 +67,7 @@ defmodule Ash.Resource.Validation.Match do _ -> {:error, InvalidAttribute.exception( + value: opts[:value], field: opts[:attribute], message: opts[:message], vars: [match: opts[:match]] diff --git a/lib/ash/resource/validation/one_of.ex b/lib/ash/resource/validation/one_of.ex index 9678f77b..de134b5a 100644 --- a/lib/ash/resource/validation/one_of.ex +++ b/lib/ash/resource/validation/one_of.ex @@ -43,6 +43,7 @@ defmodule Ash.Resource.Validation.OneOf do else {:error, InvalidAttribute.exception( + value: changing_to, field: opts[:attribute], message: "expected one of %{values}", vars: [values: Enum.map_join(opts[:values], ", ", &to_string/1)] diff --git a/lib/ash/resource/validation/present.ex b/lib/ash/resource/validation/present.ex index c16f7303..e818eb25 100644 --- a/lib/ash/resource/validation/present.ex +++ b/lib/ash/resource/validation/present.ex @@ -60,9 +60,10 @@ defmodule Ash.Resource.Validation.Present do changes_error(opts, count, "must be absent") else if count == 1 do - attribute_error(opts, count, "must be present") + attribute_error(changeset, opts, count, "must be present") else attribute_error( + changeset, opts, count, "exactly %{exactly} of %{keys} must be present" @@ -72,14 +73,14 @@ defmodule Ash.Resource.Validation.Present do opts[:at_least] && present < opts[:at_least] -> if count == 1 do - attribute_error(opts, count, "must be present") + attribute_error(changeset, opts, count, "must be present") else changes_error(opts, count, "at least %{at_least} of %{keys} must be present") end opts[:at_most] && present > opts[:at_most] -> if count == 1 do - attribute_error(opts, count, "must not be present") + attribute_error(changeset, opts, count, "must not be present") else changes_error(opts, count, "at least %{at_most} of %{keys} must be present") end @@ -98,13 +99,14 @@ defmodule Ash.Resource.Validation.Present do )} end - defp attribute_error(opts, _count, message) do + defp attribute_error(changeset, opts, _count, message) do {:error, opts[:attributes] |> List.wrap() |> Enum.map(fn attribute -> InvalidAttribute.exception( field: attribute, + value: Ash.Changeset.get_attribute(changeset, attribute), message: message, vars: opts ) diff --git a/lib/ash/resource/validation/string_length.ex b/lib/ash/resource/validation/string_length.ex index eb2efeae..eafca016 100644 --- a/lib/ash/resource/validation/string_length.ex +++ b/lib/ash/resource/validation/string_length.ex @@ -50,6 +50,7 @@ defmodule Ash.Resource.Validation.StringLength do _ -> {:error, InvalidAttribute.exception( + value: value, field: opts[:attribute], message: "%{field} could not be parsed", vars: [field: opts[:attribute]] @@ -72,6 +73,7 @@ defmodule Ash.Resource.Validation.StringLength do else {:error, InvalidAttribute.exception( + value: value, field: opts[:attribute], message: "%{field} must have length of exactly %{exact}", vars: [field: opts[:attribute], exact: exact] @@ -87,6 +89,7 @@ defmodule Ash.Resource.Validation.StringLength do else {:error, InvalidAttribute.exception( + value: value, field: opts[:attribute], message: "%{field} must have length of between %{min} and %{max}", vars: [field: opts[:attribute], min: min, max: max] @@ -100,6 +103,7 @@ defmodule Ash.Resource.Validation.StringLength do else {:error, InvalidAttribute.exception( + value: value, field: opts[:attribute], message: "%{field} must have length of at least %{min}", vars: [field: opts[:attribute], min: min] @@ -113,6 +117,7 @@ defmodule Ash.Resource.Validation.StringLength do else {:error, InvalidAttribute.exception( + value: value, field: opts[:attribute], message: "%{field} must have length of no more than %{max}", vars: [field: opts[:attribute], max: max]