From 7cb4401d8e937850130709d9b5a651b23571f9a1 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Sat, 13 Nov 2021 13:48:25 -0500 Subject: [PATCH] improvement: support do/else blocks in if improvement: support `cond` --- lib/ash/filter/filter.ex | 38 ++++++++++++++++++++-- lib/ash/filter/runtime.ex | 10 ++++++ lib/ash/query/call.ex | 47 +++++++++++++++++++++++++++ lib/ash/query/function/function.ex | 21 +++++++++--- lib/ash/query/function/if.ex | 18 ++++++++++- lib/ash/query/query.ex | 39 +++++++++++++++++++++- lib/sat_solver.ex | 4 +++ test/calculation_test.exs | 52 ++++++++++++++++++++++++++++++ 8 files changed, 220 insertions(+), 9 deletions(-) diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index 7f8fbc85..94e4de1a 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -793,7 +793,17 @@ defmodule Ash.Filter do %{op | left: do_map(left, func), right: do_map(right, func)} %{__function__?: true, arguments: arguments} = func -> - %{func | arguments: Enum.map(arguments, &do_map(&1, func))} + %{ + func + | arguments: + Enum.map(arguments, fn + {key, arg} when is_atom(key) -> + {key, do_map(arg, func)} + + arg -> + do_map(arg, func) + end) + } other -> func.(other) @@ -806,6 +816,9 @@ defmodule Ash.Filter do def update_aggregates(expression, mapper) do case expression do + {key, value} when is_atom(key) -> + {key, update_aggregates(value, mapper)} + %Not{expression: expression} = not_expr -> %{not_expr | expression: update_aggregates(expression, mapper)} @@ -1234,6 +1247,10 @@ defmodule Ash.Filter do [do_relationship_paths(left, kind), do_relationship_paths(right, kind)] end + defp do_relationship_paths({key, value}, kind) when is_atom(key) do + do_relationship_paths(value, kind) + end + defp do_relationship_paths(%{__function__?: true, arguments: arguments}, kind) do Enum.map(arguments, &do_relationship_paths(&1, kind)) end @@ -1327,6 +1344,8 @@ defmodule Ash.Filter do Enum.flat_map(list, &list_refs/1) end + def list_refs({key, value}) when is_atom(key), do: list_refs(value) + def list_refs(%__MODULE__{expression: expression}) do list_refs(expression) end @@ -1419,7 +1438,7 @@ defmodule Ash.Filter do path ) do arguments - |> Enum.filter(&match?(%Ref{}, &1)) + |> Enum.filter(fn arg -> match?(%Ref{}, arg) || match?({_, %Ref{}}, arg) end) |> Enum.any?(&List.starts_with?(&1.relationship_path, path)) |> case do true -> @@ -1434,6 +1453,8 @@ defmodule Ash.Filter do other end + defp scope_ref({key, %Ref{} = ref}, path), do: {key, scope_ref(ref, path)} + defp scope_ref(%Ref{} = ref, path) do if List.starts_with?(ref.relationship_path, path) do %{ref | relationship_path: Enum.drop(ref.relationship_path, Enum.count(path))} @@ -2006,6 +2027,16 @@ defmodule Ash.Filter do defp module_and_opts({module, opts}), do: {module, opts} defp module_and_opts(module), do: {module, []} + def hydrate_refs({key, value}, context) when is_atom(key) do + case hydrate_refs(value, context) do + {:ok, hydrated} -> + {:ok, {key, hydrated}} + + other -> + other + end + end + def hydrate_refs( %Ref{attribute: attribute} = ref, %{aggregates: aggregates, calculations: calculations} = context @@ -2234,6 +2265,9 @@ defmodule Ash.Filter do defp add_to_predicate_path(expression, context) do case expression do + {key, value} when is_atom(key) -> + {key, add_to_predicate_path(value, context)} + %Not{expression: expression} = not_expr -> %{not_expr | expression: add_to_predicate_path(expression, context)} diff --git a/lib/ash/filter/runtime.ex b/lib/ash/filter/runtime.ex index 85eed54b..145e5c9e 100644 --- a/lib/ash/filter/runtime.ex +++ b/lib/ash/filter/runtime.ex @@ -214,6 +214,16 @@ defmodule Ash.Filter.Runtime do end end + defp resolve_expr({key, value}, record) when is_atom(key) do + case resolve_expr(value, record) do + {:ok, resolved} -> + {:ok, {key, resolved}} + + other -> + other + end + end + defp resolve_expr(%Ref{} = ref, record) do {:ok, resolve_ref(ref, record)} end diff --git a/lib/ash/query/call.ex b/lib/ash/query/call.ex index da55a03a..d97c7daf 100644 --- a/lib/ash/query/call.ex +++ b/lib/ash/query/call.ex @@ -6,7 +6,54 @@ defmodule Ash.Query.Call do defimpl Inspect do import Inspect.Algebra + def inspect(%{name: :if, operator?: false, args: [condition, blocks]} = call, opts) do + if Keyword.keyword?(blocks) do + if Keyword.has_key?(blocks, :else) do + concat([ + nest( + concat([ + group(concat(["if ", to_doc(condition, opts), " do"])), + line(), + to_doc(blocks[:do], opts) + ]), + 2 + ), + line(), + "else", + nest( + concat([ + line(), + to_doc(blocks[:else], opts) + ]), + 2 + ), + line(), + "end" + ]) + else + concat([ + nest( + concat([ + group(concat(["if ", to_doc(condition, opts), " do"])), + line(), + to_doc(blocks[:do], opts) + ]), + 2 + ), + line(), + "end" + ]) + end + else + do_inspect(call, opts) + end + end + def inspect(call, inspect_opts) do + do_inspect(call, inspect_opts) + end + + defp do_inspect(call, inspect_opts) do if call.operator? do concat([ to_doc(Enum.at(call.args, 0), inspect_opts), diff --git a/lib/ash/query/function/function.ex b/lib/ash/query/function/function.ex index 4803f1ed..ec76155a 100644 --- a/lib/ash/query/function/function.ex +++ b/lib/ash/query/function/function.ex @@ -27,23 +27,34 @@ defmodule Ash.Query.Function do mod_args -> configured_args = List.wrap(mod_args) - configured_arg_count = Enum.count(Enum.at(configured_args, 0)) + allowed_arg_counts = Enum.map(configured_args, &Enum.count/1) given_arg_count = Enum.count(args) - if configured_arg_count == given_arg_count do + if given_arg_count in allowed_arg_counts do mod_args + |> Enum.filter(fn args -> + Enum.count(args) == given_arg_count + end) |> Enum.find_value(&try_cast_arguments(&1, args)) |> case do nil -> - {:error, - "Could not cast function arguments for #{mod.name()}/#{configured_arg_count}"} + {:error, "Could not cast function arguments for #{mod.name()}/#{given_arg_count}"} casted -> mod.new(casted) end else + did_you_mean = + Enum.map_join(allowed_arg_counts, "\n", fn arg_count -> + " . * #{mod.name()}/#{arg_count}" + end) + {:error, - "function #{mod.name()}/#{configured_arg_count} takes #{configured_arg_count} arguments, provided #{given_arg_count}"} + """ + No such function #{mod.name()}/#{given_arg_count}. Did you mean one of: + + #{did_you_mean} + """} end end end diff --git a/lib/ash/query/function/if.ex b/lib/ash/query/function/if.ex index 63cf21b9..981356f5 100644 --- a/lib/ash/query/function/if.ex +++ b/lib/ash/query/function/if.ex @@ -4,7 +4,23 @@ defmodule Ash.Query.Function.If do """ use Ash.Query.Function, name: :if - def args, do: [[:boolean, :any, :any]] + def args, do: [[:boolean, :any], [:boolean, :any, :any]] + + def new([condition, block]) do + if Keyword.keyword?(block) && Keyword.has_key?(block, :do) do + if Keyword.has_key?(block, :else) do + super([condition, block[:do], block[:else]]) + else + super([condition, block[:do], nil]) + end + else + super([condition, block, nil]) + end + end + + def new(other) do + super(other) + end def evaluate(%{arguments: [condition, when_true, when_false]}) do if condition do diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index eb5c40bf..da7b7e4b 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -548,8 +548,29 @@ defmodule Ash.Query do soft_escape(Not.new(expression), escape?) end + defp do_expr({:cond, _, [[do: options]]}, escape?) do + options + |> Enum.map(fn {:->, _, [condition, result]} -> + {condition, result} + end) + |> cond_to_if_tree() + |> do_expr(escape?) + end + defp do_expr({op, _, args}, escape?) when is_atom(op) and is_list(args) do - args = Enum.map(args, &do_expr(&1, false)) + last_arg = List.last(args) + + args = + if Keyword.keyword?(last_arg) && Keyword.has_key?(last_arg, :do) do + Enum.map(:lists.droplast(args), &do_expr(&1, false)) ++ + [ + Enum.map(last_arg, fn {key, arg_value} -> + {key, do_expr(arg_value, false)} + end) + ] + else + Enum.map(args, &do_expr(&1, false)) + end soft_escape(%Ash.Query.Call{name: op, args: args, operator?: false}, escape?) end @@ -558,6 +579,22 @@ defmodule Ash.Query do defp do_expr(other, _), do: other + defp cond_to_if_tree([{condition, result}]) do + {:if, [], [cond_condition(condition), [do: result]]} + end + + defp cond_to_if_tree([{condition, result} | rest]) do + {:if, [], [cond_condition(condition), [do: result, else: cond_to_if_tree(rest)]]} + end + + defp cond_condition([condition]) do + condition + end + + defp cond_condition([condition | rest]) do + {:and, [], [condition, cond_condition(rest)]} + end + defp soft_escape(%_{} = val, _) do {:%{}, [], Map.to_list(val)} end diff --git a/lib/sat_solver.ex b/lib/sat_solver.ex index 38b26549..f6fd5023 100644 --- a/lib/sat_solver.ex +++ b/lib/sat_solver.ex @@ -149,6 +149,10 @@ defmodule Ash.SatSolver do defp upgrade_related_filters_to_join_keys(expr, _), do: expr + defp upgrade_ref({key, ref}, resource) when is_atom(key) do + {key, upgrade_ref(ref, resource)} + end + defp upgrade_ref( %Ash.Query.Ref{attribute: attribute, relationship_path: path} = ref, resource diff --git a/test/calculation_test.exs b/test/calculation_test.exs index a105e9c3..29134690 100644 --- a/test/calculation_test.exs +++ b/test/calculation_test.exs @@ -60,6 +60,28 @@ defmodule Ash.Test.CalculationTest do calculate :conditional_full_name, :string, expr(if(first_name and last_name, first_name <> " " <> last_name, "(none)")) + + calculate :conditional_full_name_block, + :string, + expr( + if first_name and last_name do + first_name <> " " <> last_name + else + "(none)" + end + ) + + calculate :conditional_full_name_cond, + :string, + expr( + cond do + first_name and last_name -> + first_name <> " " <> last_name + + true -> + "(none)" + end + ) end end @@ -185,4 +207,34 @@ defmodule Ash.Test.CalculationTest do assert full_names == ["(none)", "brian cranston", "zach daniel"] end + + test "the `if` calculation can use the `do` style syntax" do + User + |> Ash.Changeset.new(%{first_name: "bob"}) + |> Api.create!() + + full_names = + User + |> Ash.Query.load(:conditional_full_name_block) + |> Api.read!() + |> Enum.map(& &1.conditional_full_name_block) + |> Enum.sort() + + assert full_names == ["(none)", "brian cranston", "zach daniel"] + end + + test "the `if` calculation can use the `cond` style syntax" do + User + |> Ash.Changeset.new(%{first_name: "bob"}) + |> Api.create!() + + full_names = + User + |> Ash.Query.load(:conditional_full_name_cond) + |> Api.read!() + |> Enum.map(& &1.conditional_full_name_cond) + |> Enum.sort() + + assert full_names == ["(none)", "brian cranston", "zach daniel"] + end end