From fc3cbc4e2c9162463b5146e4827b547a0316f64a Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 31 Jan 2024 15:36:36 -0500 Subject: [PATCH] improvement: change nested field filtering to be done with `at_path` --- lib/ash/filter/filter.ex | 153 +++++++++++++++++++----------------- test/filter/filter_test.exs | 2 +- 2 files changed, 80 insertions(+), 75 deletions(-) diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index 8b0f4ba9..b58f4644 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -3579,27 +3579,43 @@ defmodule Ash.Filter do end end - defp parse_predicates(value, field, context, inner_expr \\ nil) - - defp parse_predicates(value, field, context, inner_expr) + defp parse_predicates(value, field, context) when not is_list(value) and not is_map(value) do - parse_predicates([eq: value], field, context, inner_expr) + parse_predicates([eq: value], field, context) end - defp parse_predicates(value, field, context, inner_expr) when value == %{} do - parse_predicates([eq: true], field, context, inner_expr) + defp parse_predicates(value, field, context) when value == %{} do + parse_predicates([eq: true], field, context) end - defp parse_predicates(%struct{} = value, field, context, inner_expr) + defp parse_predicates(%struct{} = value, field, context) when struct not in [Not, BooleanExpression, Ref, Call] do - parse_predicates([eq: value], field, context, inner_expr) + parse_predicates([eq: value], field, context) end - defp parse_predicates(values, attr, context, inner_expr) do + defp parse_predicates(values, attr, context) do if is_struct(values) && Map.has_key?(values, :__predicate__) do - parse_predicates([eq: values], attr, context, inner_expr) + parse_predicates([eq: values], attr, context) else if is_map(values) || Keyword.keyword?(values) do + at_path = + if is_map(values) do + Map.get(values, :at_path) || Map.get(values, "at_path") + else + Keyword.get(values, :at_path) + end + + {values, at_path} = + if is_list(at_path) do + if is_map(values) do + {Map.drop(values, [:at_path, "at_path"]), at_path} + else + {Keyword.delete(values, :at_path), at_path} + end + else + {values, nil} + end + Enum.reduce_while(values, {:ok, true}, fn {key, value}, {:ok, expression} when key in [:not, "not"] -> case parse_predicates(List.wrap(value), attr, context) do @@ -3615,73 +3631,62 @@ defmodule Ash.Filter do end {key, value}, {:ok, expression} -> - if !match?({:array, _}, attr.type) && - Ash.Type.embedded_type?(attr.type) && Ash.Resource.Info.attribute(attr.type, key) do - get_path = %Call{ - name: :get_path, - args: [ - %Ref{ - attribute: attr, - relationship_path: context[:relationship_path] || [], - resource: context.resource, - input?: true - }, - [key] - ] - } + case get_operator(key) do + nil -> + error = NoSuchFilterPredicate.exception(key: key, resource: context.resource) + {:halt, {:error, error}} - {:cont, - parse_predicates( - value, - attr, - context, - get_path - )} - else - case get_operator(key) do - nil -> - error = NoSuchFilterPredicate.exception(key: key, resource: context.resource) - {:halt, {:error, error}} - - operator_module -> - left = - inner_expr || - %Ref{ - attribute: attr, - relationship_path: context[:relationship_path] || [], - resource: context.resource, - input?: true - } - - with {:ok, [left, right]} <- - hydrate_refs([left, value], context), - refs <- list_refs([left, right]), - :ok <- - validate_refs( - refs, - context.root_resource, - {attr, value} - ), - {:ok, operator} <- Operator.new(operator_module, left, right) do - if is_boolean(operator) do - {:cont, {:ok, operator}} - else - if is_nil(context.resource) || - Ash.DataLayer.data_layer_can?( - context.resource, - {:filter_expr, operator} - ) do - {:cont, - {:ok, BooleanExpression.optimized_new(:and, expression, operator)}} - else - {:halt, - {:error, "data layer does not support the operator #{inspect(operator)}"}} - end - end + operator_module -> + left = + if is_list(at_path) do + %Call{ + name: :get_path, + args: [ + %Ref{ + attribute: attr, + relationship_path: context[:relationship_path] || [], + resource: context.resource, + input?: true + }, + at_path + ] + } else - {:error, error} -> {:halt, {:error, error}} + %Ref{ + attribute: attr, + relationship_path: context[:relationship_path] || [], + resource: context.resource, + input?: true + } end - end + + with {:ok, [left, right]} <- + hydrate_refs([left, value], context), + refs <- list_refs([left, right]), + :ok <- + validate_refs( + refs, + context.root_resource, + {attr, value} + ), + {:ok, operator} <- Operator.new(operator_module, left, right) do + if is_boolean(operator) do + {:cont, {:ok, operator}} + else + if is_nil(context.resource) || + Ash.DataLayer.data_layer_can?( + context.resource, + {:filter_expr, operator} + ) do + {:cont, {:ok, BooleanExpression.optimized_new(:and, expression, operator)}} + else + {:halt, + {:error, "data layer does not support the operator #{inspect(operator)}"}} + end + end + else + {:error, error} -> {:halt, {:error, error}} + end end end) else diff --git a/test/filter/filter_test.exs b/test/filter/filter_test.exs index fe124743..47a59fa0 100644 --- a/test/filter/filter_test.exs +++ b/test/filter/filter_test.exs @@ -920,7 +920,7 @@ defmodule Ash.Test.Filter.FilterTest do assert [%{embedded_bio: %{title: "Dr."}}] = Profile - |> Ash.Query.filter_input(%{embedded_bio: %{title: "Dr."}}) + |> Ash.Query.filter_input(%{embedded_bio: %{at_path: [:title], eq: "Dr."}}) |> Api.read!() end end