Skip to content

Commit

Permalink
improvement: warning on args, preparations or filters on primary reads
Browse files Browse the repository at this point in the history
Probably one of the most common mistakes made in early days of Ash is
accidentally adding preparations, filters, or arguments to the primary
read actions. This adds a dismissable warning to help prevent this case.
  • Loading branch information
zachdaniel committed Jan 30, 2025
1 parent a325ac7 commit e5656b6
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 6 deletions.
13 changes: 7 additions & 6 deletions lib/ash/resource.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ defmodule Ash.Resource do
[Resource DSL documentation](dsl-ash-resource.html)
"""

def foo do
# x + y + z

1 - 2 - 3
end

@type t :: module
@type record :: struct()

Expand Down Expand Up @@ -39,6 +33,12 @@ defmodule Ash.Resource do
doc: "Whether or not to validate that this resource is included in a domain.",
default: true
],
primary_read_warning?: [
type: :boolean,
doc:
"Set to `false` to silence warnings about arguments, preparations and filters on the primary read action.",
default: true
],
domain: [
type: {:behaviour, Ash.Domain},
doc:
Expand Down Expand Up @@ -123,6 +123,7 @@ defmodule Ash.Resource do
embed_nil_values?: opts[:embed_nil_values?]
] do
@persist {:simple_notifiers, List.wrap(opts[:simple_notifiers])}
@persist {:primary_read_warning?, Keyword.get(opts, :primary_read_warning?, true)}

if !(embedded? || has_domain?) do
IO.warn("""
Expand Down
1 change: 1 addition & 0 deletions lib/ash/resource/dsl.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,7 @@ defmodule Ash.Resource.Dsl do
Ash.Resource.Verifiers.ValidateRelationshipAttributesMatch,
Ash.Resource.Verifiers.VerifyReservedCalculationArguments,
Ash.Resource.Verifiers.VerifyIdentityFields,
Ash.Resource.Verifiers.VerifyPrimaryReadActionHasNoArguments,
Ash.Resource.Verifiers.VerifySelectedByDefault,
Ash.Resource.Verifiers.EnsureAggregateFieldIsAttributeOrCalculation,
Ash.Resource.Verifiers.ValidateRelationshipAttributes,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
defmodule Ash.Resource.Verifiers.VerifyPrimaryReadActionHasNoArguments do
@moduledoc "Verifies that primary read actions do not have any arguments"
use Spark.Dsl.Verifier

def verify(dsl) do
resource = Spark.Dsl.Verifier.get_persisted(dsl, :module)

if Spark.Dsl.Verifier.get_persisted(dsl, :primary_read_warning?) do
dsl
|> Ash.Resource.Info.actions()
|> Enum.find(&(&1.type == :read && &1.primary?))
|> case do
nil ->
:ok

action ->
if action.arguments != [] do
IO.warn(warning(resource, action, "arguments"))
end

if action.filter not in [nil, []] do
IO.warn(warning(resource, action, "filters"))
end

if action.preparations != [] &&
Ash.Resource.Info.data_layer(resource) != Ash.DataLayer.Simple do
IO.warn(warning(resource, action, "preparations"))
end

:ok
end
else
:ok
end
end

defp warning(resource, action, things) do
"""
Primary read action `#{inspect(resource)}.#{action.name}` has #{things}.
This is often done by mistake, but can also be done intentionally.
Primary read actions are used when loading relationships, checking policies and more.
It is okay to have optional arguments, but required arguments are almost never desired.
If you intended to have #{things} on your primary read action, add `primary_read_warning?: false`
to `use Ash.Resource`. For example:
use Ash.Resource,
primary_read_warning?: false
"""
end
end

0 comments on commit e5656b6

Please sign in to comment.