From 791d34fb321f40a52e8f0ec25659863c577b4236 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Mon, 4 Nov 2024 11:59:22 -0500 Subject: [PATCH] fix: don't call `.name` on a potentially `nil` action --- lib/ash/actions/read/read.ex | 130 ++++++++++++++-------------- lib/ash/expr/expr.ex | 1 - test/ash/custom_expression_test.exs | 47 ++++++++++ 3 files changed, 113 insertions(+), 65 deletions(-) diff --git a/lib/ash/actions/read/read.ex b/lib/ash/actions/read/read.ex index e2195a6e0..975902339 100644 --- a/lib/ash/actions/read/read.ex +++ b/lib/ash/actions/read/read.ex @@ -48,80 +48,82 @@ defmodule Ash.Actions.Read do action = get_action(query.resource, action || query.action) - tracer = - if opts[:initial_data] do - nil - else - opts[:tracer] - end + try do + tracer = + if opts[:initial_data] do + nil + else + opts[:tracer] + end - Ash.Tracer.span :action, - fn -> - Ash.Domain.Info.span_name(query.domain, query.resource, action.name) - end, - tracer do - metadata = fn -> - %{ - domain: query.domain, - resource: query.resource, - resource_short_name: Ash.Resource.Info.short_name(query.resource), - actor: opts[:actor], - tenant: opts[:tenant], - action: action.name, - authorize?: opts[:authorize?] - } - end + Ash.Tracer.span :action, + fn -> + Ash.Domain.Info.span_name(query.domain, query.resource, action.name) + end, + tracer do + metadata = fn -> + %{ + domain: query.domain, + resource: query.resource, + resource_short_name: Ash.Resource.Info.short_name(query.resource), + actor: opts[:actor], + tenant: opts[:tenant], + action: action.name, + authorize?: opts[:authorize?] + } + end - Ash.Tracer.telemetry_span [:ash, Ash.Domain.Info.short_name(query.domain), :read], - metadata, - skip?: !!opts[:initial_data] do - Ash.Tracer.set_metadata(tracer, :action, metadata) - - run_around_transaction_hooks(query, fn query -> - case do_run(query, action, opts) do - {:error, error} -> - error = - Ash.Error.to_error_class( - error, - bread_crumbs: "Error returned from: #{inspect(query.resource)}.#{action.name}" - ) + Ash.Tracer.telemetry_span [:ash, Ash.Domain.Info.short_name(query.domain), :read], + metadata, + skip?: !!opts[:initial_data] do + Ash.Tracer.set_metadata(tracer, :action, metadata) - if opts[:tracer] do - stacktrace = - case error do - %{stacktrace: %{stacktrace: stacktrace}} -> - stacktrace || [] + run_around_transaction_hooks(query, fn query -> + case do_run(query, action, opts) do + {:error, error} -> + error = + Ash.Error.to_error_class( + error, + bread_crumbs: "Error returned from: #{inspect(query.resource)}.#{action.name}" + ) - _ -> - {:current_stacktrace, stacktrace} = - Process.info(self(), :current_stacktrace) + if opts[:tracer] do + stacktrace = + case error do + %{stacktrace: %{stacktrace: stacktrace}} -> + stacktrace || [] - stacktrace - end + _ -> + {:current_stacktrace, stacktrace} = + Process.info(self(), :current_stacktrace) - Ash.Tracer.set_handled_error(opts[:tracer], Ash.Error.to_error_class(error), - stacktrace: stacktrace - ) - end + stacktrace + end - {:error, error} + Ash.Tracer.set_handled_error(opts[:tracer], Ash.Error.to_error_class(error), + stacktrace: stacktrace + ) + end - other -> - other - end - end) + {:error, error} + + other -> + other + end + end) + end end + rescue + e -> + reraise Ash.Error.to_error_class(e, + query: query, + stacktrace: __STACKTRACE__, + bread_crumbs: [ + "Exception raised in: #{inspect(query.resource)}.#{action.name}" + ] + ), + __STACKTRACE__ end - rescue - e -> - reraise Ash.Error.to_error_class(e, - query: query, - stacktrace: __STACKTRACE__, - bread_crumbs: [ - "Exception raised in: #{inspect(query.resource)}.#{action.name}" - ] - ), - __STACKTRACE__ end defp run_around_transaction_hooks(%{around_transaction: []} = query, func), diff --git a/lib/ash/expr/expr.ex b/lib/ash/expr/expr.ex index 283e44eb9..ffedaabaa 100644 --- a/lib/ash/expr/expr.ex +++ b/lib/ash/expr/expr.ex @@ -776,7 +776,6 @@ defmodule Ash.Expr do {:&, _, _} = expr, _ ) do - IO.inspect(expr) raise """ The only kind of anonymous functions allowed in expressions are in the format `&Module.function/arity`. diff --git a/test/ash/custom_expression_test.exs b/test/ash/custom_expression_test.exs index 3a32930eb..5165576a0 100644 --- a/test/ash/custom_expression_test.exs +++ b/test/ash/custom_expression_test.exs @@ -3,7 +3,54 @@ defmodule Ash.CustomExpressionTest do import Ash.Expr + defmodule ExampleResource do + use Ash.Resource, domain: Ash.Test.Domain, data_layer: Ash.DataLayer.Ets + + ets do + private? true + end + + attributes do + uuid_primary_key :id + attribute :name, :string, allow_nil?: false + end + + actions do + defaults create: [:name] + + read :matches_query_with_calc do + argument :query, :string, allow_nil?: false + filter expr(matches_query?(query: ^arg(:query))) + end + + read :matches_query_with_explicit_filter do + argument :query, :string, allow_nil?: false + filter expr(jaro_distance(name, ^arg(:query)) >= 0.6) + end + end + + calculations do + calculate :matches_query?, :boolean, expr(jaro_distance(name, ^arg(:query)) >= 0.6) do + argument :query, :string, allow_nil?: false + end + end + end + test "custom expressions are callable and evaluate" do assert eval!(expr(jaro_distance("foo", "bar"))) == String.jaro_distance("foo", "bar") end + + test "custom expressions work the same in calculations and without" do + Ash.create!(ExampleResource, %{name: "foo"}) + + assert [_] = + ExampleResource + |> Ash.Query.for_read(:matches_query_with_calc, %{query: "fop"}) + |> Ash.read!() + + assert [_] = + ExampleResource + |> Ash.Query.for_read(:matches_query_with_explicit_filter, %{query: "fop"}) + |> Ash.read!() + end end