Defining `get_by` on a code interface doesn't do type-checking of the argument
Describe the bug
A code interface to get a single record by UUID primary key:
define :get_artist_by_id, action: :read, get_by: :id
Calling the generated function with an invalid UUID raises an error from within Ecto:
iex(8)> Tunez.Music.get_artist_by_id("not a uuid")
{:error,
%Ash.Error.Invalid{
bread_crumbs: ["Error returned from: Tunez.Music.Artist.read"],
query: "#Query<>",
errors: [
%Ash.Error.Query.InvalidFilterValue{
message: nil,
value: "not a uuid",
context: #Ecto.Query<from a0 in Tunez.Music.Artist, as: 0,
where: type(
type(as(0).id, {:parameterized, {Ash.Type.UUID.EctoType, []}}),
{:parameterized, {Ash.Type.UUID.EctoType, []}}
) ==
type(
type(^"not a uuid", {:parameterized, {Ash.Type.UUID.EctoType, []}}),
{:parameterized, {Ash.Type.UUID.EctoType, []}}
),
select: struct(a0, [
:id,
:name,
:inserted_at,
:updated_at,
:biography,
:created_by_id,
:previous_names,
:updated_by_id
])>,
splode: Ash.Error,
bread_crumbs: ["Error returned from: Tunez.Music.Artist.read"],
vars: [],
path: [],
stacktrace: #Splode.Stacktrace<>,
class: :invalid
}
]
}}
Whereas if the code interface used a real action that applied a get_by filter:
# in the domain
define :get_artist_by_id, action: :get_by_id, args: [:id]
# in the resource
actions do
read :get_by_id do
get_by :id
end
end
Calling the function would properly validate the UUID before running the action:
iex(5)> Tunez.Music.get_artist_by_id("not a uuid")
{:error,
%Ash.Error.Invalid{
errors: [
%Ash.Error.Query.InvalidArgument{
field: :id,
message: "is invalid",
value: "not a uuid",
splode: Ash.Error,
bread_crumbs: [],
vars: [],
path: [],
stacktrace: #Splode.Stacktrace<>,
class: :invalid
}
]
}}
I expected these two function definitions to be exactly equivalent, but that is not the case. The get_by code interface version skips past the UUID type check, and the non-UUID value gets all the way into the Ecto query.
Runtime
- Elixir version 1.18.2-otp-27
- Erlang version 27.2.1
- OS macOS Sequoia
- Ash version 3.5.4
Good point. We need to add some level of validation either in the code interface definition or somewhere in filter parsing. I thought we already had that kind of thing, but maybe its not coming into play as it should.
I did look into this a bit, and the difference is that for the get_by inside the read action, a transformer adds the field as an argument and adds the filter with an arg template, which is therefore checked as an argument.
For the code_interface, we only supply those values to the filter, and they are validated on the filter path and surface the InvalidFilterValue Error.
So, it's doing the right thing, even though it's not entirely clear from the outside. We can't really modify the action from the code interface, so the only option would be to add some form of validation to the code interface, if you think it's worth it.
Good point. 🤔 I think realistically what we should do is find some way to improve this error in general. For example, perhaps we should eagerly validate certain common operators somewhere etc. Or just make the error less ugly coming back from AshPostgres. Not sure yet TBH.