Should change and preparation modules validate arguments?

Questions & Help Questions
1

If a change or preparation module uses Ash.Changeset.get_argument/2 or Ash.Query.get_argument/2 (or any of the other get_argument functions) should such a module then check the existence and validity of the arguments again?

Example:

# in the resource
defmodule MyResource do
  actions do
    update :do_something do
      argument :foobar, :string, allow_nil?: false
      change NaiveCustomChange # or the other one, whatever
    end
  end
end

defmodule NaiveCustomChange do
  use Ash.Resource.Change

  @impl true
  def change(changeset, opts, _context) do
    foobar_from_args = Ash.Changeset.get_argument(changeset, :foobar)
    Ash.Changeset.change_attribute(changeset, :foobar, foobar_from_args)
  end
end

vs

defmodule PessimisticCustomChange do
  use Ash.Resource.Change

  @impl true
  def change(changeset, opts, _context) do
    case Ash.Changeset.get_argument(changeset, :foobar) do
      foobar when is_binary(foobar) ->
        Ash.Changeset.change_attribute(changeset, :foobar, foobar)

      _ ->
        Ash.Changeset.add_error(changeset,
          field: :foobar,
          message: "needs the argument :foobar"
        )
    end
  end
end

Does anybody have any reason to prefer one over the other?

2

The values from Ash.Changeset.get_attribute and Ash.Changeset.get_argument are only provided when the original value is actually valid according to the type and constraints declared. however, by default, validations are run regardless of if previous validations passed or failed. For example:

argument :foo, :string, constraints: [max_length: 10]

validate Something # <- will only ever see `foo` as `nil` or a string with max length 10
change SomethingElse # <- called even if the previous validation failed

The concept here is that you want to provide multiple validation errors to the caller in one pass.

So in general, you just want your changes to be conditional when an action is written this way. i.e something like:

value = Ash.Changeset.get_argument(changeset, :foo)

if is_nil(value) do
  ...
end

or

case Ash.Changeset.fetch_argument(changeset, :foo) do
  {:ok, value} -> ...
  :error -> ...
end

However, what you can do instead is this:

validate Something, only_when_valid?: true
change SomethingElse, only_when_valid?: true

or, programmatically in the change module

if changeset.valid? do
  ...
else
  ...
end

Which ensures that in each call, the changeset is “fully valid” up until that point.

We are considering making only_when_valid?: true the default in Ash 4.0