From a5f51e8f1bc4aee803896198d4953e7db84a6ca4 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 14 Sep 2022 22:31:32 -0400 Subject: [PATCH] fix: properly error on types when evaluating expressions at runtime --- lib/ash/filter/runtime.ex | 46 +++++++++++++++++++++++++++--- lib/ash/query/function/function.ex | 2 +- lib/ash/query/operator/operator.ex | 3 +- lib/ash/query/type.ex | 2 ++ test/calculation_test.exs | 14 ++++++--- 5 files changed, 57 insertions(+), 10 deletions(-) diff --git a/lib/ash/filter/runtime.ex b/lib/ash/filter/runtime.ex index c071cc86..e51a7677 100644 --- a/lib/ash/filter/runtime.ex +++ b/lib/ash/filter/runtime.ex @@ -195,12 +195,21 @@ defmodule Ash.Filter.Runtime do nil -> {:ok, true} - %op{__operator__?: true, left: left, right: right} = operator -> + %op{__operator__?: true, left: left, right: right} -> with {:ok, [left, right]} <- resolve_exprs([left, right], record), - {:known, val} <- op.evaluate(%{operator | left: left, right: right}) do + {:op, {:ok, %op{} = new_operator}} <- + {:op, Ash.Query.Operator.try_cast_with_ref(op, left, right)}, + {:known, val} <- + op.evaluate(new_operator) do {:ok, val} else + {:op, {:error, error}} -> + {:error, error} + + {:op, {:ok, expr}} -> + do_match(record, expr) + {:error, error} -> {:error, error} @@ -213,9 +222,15 @@ defmodule Ash.Filter.Runtime do %func{__function__?: true, arguments: arguments} = function -> with {:ok, args} <- resolve_exprs(arguments, record), + {:args, args} when not is_nil(args) <- + {:args, try_cast_arguments(func.args(), args)}, {:known, val} <- func.evaluate(%{function | arguments: args}) do {:ok, val} else + {:args, nil} -> + {:error, + "Could not cast function arguments for #{func.name()}/#{Enum.count(arguments)}"} + {:error, error} -> {:error, error} @@ -333,11 +348,19 @@ defmodule Ash.Filter.Runtime do end) end - defp resolve_expr(%mod{__predicate__?: _, left: left, right: right} = pred, record) do + defp resolve_expr(%mod{__predicate__?: _, left: left, right: right}, record) do with {:ok, [left, right]} <- resolve_exprs([left, right], record), - {:known, val} <- mod.evaluate(%{pred | left: left, right: right}) do + {:op, {:ok, %mod{} = new_pred}} <- + {:op, Ash.Query.Operator.try_cast_with_ref(mod, left, right)}, + {:known, val} <- mod.evaluate(new_pred) do {:ok, val} else + {:op, {:error, error}} -> + {:error, error} + + {:op, {:ok, expr}} -> + resolve_expr(expr, record) + {:error, error} -> {:error, error} @@ -351,9 +374,14 @@ defmodule Ash.Filter.Runtime do defp resolve_expr(%mod{__predicate__?: _, arguments: args} = pred, record) do with {:ok, args} <- resolve_exprs(args, record), + {:args, args} when not is_nil(args) <- + {:args, try_cast_arguments(mod.args(), args)}, {:known, val} <- mod.evaluate(%{pred | arguments: args}) do {:ok, val} else + {:args, nil} -> + {:error, "Could not cast function arguments for #{mod.name()}/#{Enum.count(args)}"} + {:error, error} -> {:error, error} @@ -367,6 +395,16 @@ defmodule Ash.Filter.Runtime do defp resolve_expr(other, _), do: {:ok, other} + defp try_cast_arguments(configured_args, args) do + given_arg_count = Enum.count(args) + + configured_args + |> Enum.filter(fn args -> + Enum.count(args) == given_arg_count + end) + |> Enum.find_value(&Ash.Query.Function.try_cast_arguments(&1, args)) + end + defp resolve_ref(%Ref{attribute: attribute, relationship_path: path}, record) do name = case attribute do diff --git a/lib/ash/query/function/function.ex b/lib/ash/query/function/function.ex index e12c27ac..284cdb64 100644 --- a/lib/ash/query/function/function.ex +++ b/lib/ash/query/function/function.ex @@ -80,7 +80,7 @@ defmodule Ash.Query.Function do end end - defp try_cast_arguments(configured_args, args) do + def try_cast_arguments(configured_args, args) do args |> Enum.zip(configured_args) |> Enum.reduce_while({:ok, []}, fn diff --git a/lib/ash/query/operator/operator.ex b/lib/ash/query/operator/operator.ex index e2786fcb..f362bfb1 100644 --- a/lib/ash/query/operator/operator.ex +++ b/lib/ash/query/operator/operator.ex @@ -71,7 +71,8 @@ defmodule Ash.Query.Operator do end end - defp try_cast_with_ref(mod, left, right) do + @doc false + def try_cast_with_ref(mod, left, right) do Enum.find_value(mod.types(), fn type -> try_cast(left, right, type) end) diff --git a/lib/ash/query/type.ex b/lib/ash/query/type.ex index e1d77f02..9d853fb2 100644 --- a/lib/ash/query/type.ex +++ b/lib/ash/query/type.ex @@ -14,6 +14,8 @@ defmodule Ash.Query.Type do :error -> :error {:ok, val} -> {:ok, val} end + else + :error end end diff --git a/test/calculation_test.exs b/test/calculation_test.exs index 3e183a47..411b9013 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -104,12 +104,18 @@ defmodule Ash.Test.CalculationTest do calculate :conditional_full_name, :string, - expr(if(first_name and last_name, first_name <> " " <> last_name, "(none)")) + expr( + if( + not is_nil(first_name) and not is_nil(last_name), + first_name <> " " <> last_name, + "(none)" + ) + ) calculate :conditional_full_name_block, :string, expr( - if first_name and last_name do + if not is_nil(first_name) and not is_nil(last_name) do first_name <> " " <> last_name else "(none)" @@ -120,10 +126,10 @@ defmodule Ash.Test.CalculationTest do :string, expr( cond do - first_name and last_name -> + not is_nil(first_name) and not is_nil(last_name) -> first_name <> " " <> last_name - first_name -> + not is_nil(first_name) -> first_name true ->