diff --git a/.credo.exs b/.credo.exs index e2da56bde..84d2bdc90 100644 --- a/.credo.exs +++ b/.credo.exs @@ -117,7 +117,7 @@ ## Refactoring Opportunities # {Credo.Check.Refactor.CondStatements, []}, - {Credo.Check.Refactor.CyclomaticComplexity, [max_complexity: 12]}, + {Credo.Check.Refactor.CyclomaticComplexity, [max_complexity: 13]}, {Credo.Check.Refactor.FunctionArity, []}, {Credo.Check.Refactor.LongQuoteBlocks, []}, {Credo.Check.Refactor.MapInto, []}, diff --git a/benchmarks/sat_solver.ex b/benchmarks/sat_solver.ex new file mode 100644 index 000000000..25f2523e6 --- /dev/null +++ b/benchmarks/sat_solver.ex @@ -0,0 +1,19 @@ +query = + AshExample.Api.query(AshExample.Representative) + |> Ash.Query.filter( + id: [in: ["35c73627-d24c-4d37-99c5-f6309202f514", "35c73627-d24c-4d37-99c5-f6309202f514"]] + ) + +candidate = + AshExample.Api.query(AshExample.Representative) + |> Ash.Query.filter(id: "35c73627-d24c-4d37-99c5-f6309202f514") + +Benchee.run( + %{ + "picosat" => fn -> + Ash.SatSolver.strict_filter_subset(query.filter, candidate.filter) + end + }, + time: 10, + memory_time: 2 +) diff --git a/lib/ash/actions/relationships.ex b/lib/ash/actions/relationships.ex index e274a92aa..ecd4d0fb0 100644 --- a/lib/ash/actions/relationships.ex +++ b/lib/ash/actions/relationships.ex @@ -170,20 +170,20 @@ defmodule Ash.Actions.Relationships do ) do relationship_name = relationship.name - filter = + {possible?, filter} = case identifiers do [single_identifier] -> if Keyword.keyword?(single_identifier) do - single_identifier + {true, single_identifier} else - [single_identifier] + {true, [single_identifier]} end [] -> - [__impossible__: true] + {false, []} many -> - [or: many] + {true, [or: many]} end query = @@ -198,10 +198,15 @@ defmodule Ash.Actions.Relationships do action: Ash.primary_action!(relationship.destination, :read), query: query, path: [:relationships, relationship_name, type], + authorize?: possible?, data: Request.resolve(fn _data -> - query - |> api.read() + if possible? do + query + |> api.read() + else + [] + end end), name: "read prior to write related #{relationship.name}" ) diff --git a/lib/ash/actions/side_load.ex b/lib/ash/actions/side_load.ex index 7d525ae7e..89a2974d8 100644 --- a/lib/ash/actions/side_load.ex +++ b/lib/ash/actions/side_load.ex @@ -432,7 +432,7 @@ defmodule Ash.Actions.SideLoad do root_filter = case data do [] -> - [__impossible__: true] + false [%resource{} = item] -> item @@ -588,8 +588,8 @@ defmodule Ash.Actions.SideLoad do end defp put_nested_relationship(request_filter, path, value, records? \\ true) - defp put_nested_relationship(_, _, [], true), do: [__impossible__: true] - defp put_nested_relationship(_, _, nil, true), do: [__impossible__: true] + defp put_nested_relationship(_, _, [], true), do: false + defp put_nested_relationship(_, _, nil, true), do: false defp put_nested_relationship(_, _, [], false), do: [] defp put_nested_relationship(_, _, nil, false), do: [] diff --git a/lib/ash/data_layer/data_layer.ex b/lib/ash/data_layer/data_layer.ex index d4efc7bd3..4dadeee4a 100644 --- a/lib/ash/data_layer/data_layer.ex +++ b/lib/ash/data_layer/data_layer.ex @@ -10,12 +10,10 @@ defmodule Ash.DataLayer do to ensure that the engine only ever tries to interact with the data layer in ways that it supports. """ - @type filter_type :: :eq | :in @type feature() :: :transact | :async_engine - | {:filter, filter_type} - | {:filter_related, Ash.relationship_cardinality()} + | {:filter_predicate, struct} | :upsert | :composite_primary_key diff --git a/lib/ash/data_layer/ets/ets.ex b/lib/ash/data_layer/ets/ets.ex index 5d10253af..82649cefc 100644 --- a/lib/ash/data_layer/ets/ets.ex +++ b/lib/ash/data_layer/ets/ets.ex @@ -5,7 +5,7 @@ defmodule Ash.DataLayer.Ets do This is used for testing. *Do not use this data layer in production* """ - alias Ash.Filter.{Predicate, Expression, Not} + alias Ash.Filter.{Expression, Not, Predicate} alias Ash.Filter.Predicate.{Eq, In} @behaviour Ash.DataLayer @@ -41,18 +41,10 @@ defmodule Ash.DataLayer.Ets do not private?(resource) end - def can?(_, :transact), do: false - def can?(_, :composite_primary_key), do: true def can?(_, :upsert), do: true - def can?(_, {:filter, :in}), do: true - def can?(_, {:filter, :not_in}), do: true - def can?(_, {:filter, :not_eq}), do: true - def can?(_, {:filter, :eq}), do: true - def can?(_, {:filter, :and}), do: true - def can?(_, {:filter, :or}), do: true - def can?(_, {:filter, :not}), do: true - def can?(_, {:filter_related, _}), do: true + def can?(_, {:filter_predicate, %In{}}), do: true + def can?(_, {:filter_predicate, %Eq{}}), do: true def can?(_, _), do: false @impl true diff --git a/lib/ash/engine/request.ex b/lib/ash/engine/request.ex index b4e4feaa4..b96c1cdb5 100644 --- a/lib/ash/engine/request.ex +++ b/lib/ash/engine/request.ex @@ -43,7 +43,6 @@ defmodule Ash.Engine.Request do :data, :name, :api, - :authorization_filter, :query, :write_to_data?, :strict_check_only?, @@ -393,6 +392,8 @@ defmodule Ash.Engine.Request do end defp do_strict_check(authorizer, request, notifications \\ []) do + strict_check_only? = request.strict_check_only? + case missing_strict_check_dependencies?(authorizer, request) do [] -> case strict_check_authorizer(authorizer, request) do @@ -402,41 +403,25 @@ defmodule Ash.Engine.Request do {:filter, filter} -> request |> Map.update!(:query, &Ash.Query.filter(&1, filter)) - |> Map.update(:authorization_filter, filter, fn authorization_filter -> - if authorization_filter do - Ash.Filter.add_to_filter(authorization_filter, filter) - else - filter - end - end) |> set_authorizer_state(authorizer, :authorized) |> try_resolve([request.path ++ [:query]], false, false) + {:filter_and_continue, _, _} when strict_check_only? -> + {:error, "Request must pass strict check"} + {:filter_and_continue, filter, new_authorizer_state} -> - if request.strict_check_only? do - {:error, "Request must pass strict check"} - else - new_request = - request - |> Map.update(:authorization_filter, filter, fn authorization_filter -> - if authorization_filter do - Ash.Filter.add_to_filter(authorization_filter, filter) - else - filter - end - end) - |> Map.update!(:query, &Ash.Query.filter(&1, filter)) - |> set_authorizer_state(authorizer, new_authorizer_state) - - {:ok, new_request} - end + new_request = + request + |> Map.update!(:query, &Ash.Query.filter(&1, filter)) + |> set_authorizer_state(authorizer, new_authorizer_state) + + {:ok, new_request} + + {:continue, _} when strict_check_only? -> + {:error, "Request must pass strict check"} {:continue, authorizer_state} -> - if request.strict_check_only? do - {:error, "Request must pass strict check"} - else - {:ok, set_authorizer_state(request, authorizer, authorizer_state)} - end + {:ok, set_authorizer_state(request, authorizer, authorizer_state)} {:error, error} -> {:error, error} @@ -499,15 +484,7 @@ defmodule Ash.Engine.Request do {:ok, set_authorizer_state(request, authorizer, :authorized)} {:filter, filter} -> - request - |> Map.update(:authorization_filter, filter, fn authorization_filter -> - if authorization_filter do - Ash.Filter.add_to_filter(authorization_filter, filter) - else - filter - end - end) - |> runtime_filter(authorizer, filter) + runtime_filter(request, authorizer, filter) {:error, error} -> {:error, error} @@ -624,7 +601,7 @@ defmodule Ash.Engine.Request do authorized? = Enum.all?(Map.values(request.authorizer_state), &(&1 == :authorized)) # Don't fetch honor requests for dat until the request is authorized - if field in [:data, :query, :authorization_filter] and not authorized? and not internal? do + if field in [:data, :query] and not authorized? and not internal? do try_resolve_dependencies_of(request, field, internal?, optional?) else case Map.get(request, field) do diff --git a/lib/ash/filter/cosimplify.ex b/lib/ash/filter/cosimplify.ex deleted file mode 100644 index e69de29bb..000000000 diff --git a/lib/ash/filter/expression.ex b/lib/ash/filter/expression.ex index c8074eb51..fcc972024 100644 --- a/lib/ash/filter/expression.ex +++ b/lib/ash/filter/expression.ex @@ -1,4 +1,6 @@ defmodule Ash.Filter.Expression do + @moduledoc "Represents a boolean expression" + defstruct [:op, :left, :right] def new(_, nil, nil), do: nil diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index ae5a804af..fe90e44c6 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -1,7 +1,14 @@ defmodule Ash.Filter do + @moduledoc """ + The representation of a filter in Ash. + + Ash filters are stored as nested `Ash.Filter.Expression{}` and `%Ash.Filter.Not{}` structs, + terminating in a `%Ash.Filter.Predicate{}` struct. An expression is simply a boolean operator + and the left and right hand side of that operator. + """ + alias Ash.Engine.Request alias Ash.Filter.Predicate.{Eq, In} alias Ash.Filter.{Expression, Not, Predicate} - alias Ash.Engine.Request @built_in_predicates [ eq: Eq, @@ -45,14 +52,27 @@ defmodule Ash.Filter do |> Enum.map(fn {path} -> path end) end + def add_to_filter!(base, op \\ :and, addition) do + case add_to_filter(base, op, addition) do + {:ok, value} -> + value + + {:error, error} -> + raise Ash.Error.to_ash_error(error) + end + end + + def add_to_filter(base, op \\ :and, addition) + def add_to_filter( %__MODULE__{resource: resource, api: api} = base, + op, %__MODULE__{resource: resource, api: api} = addition ) do - {:ok, Expression.new(:and, base.expression, addition.expression)} + {:ok, %{base | expression: Expression.new(op, base.expression, addition.expression)}} end - def add_to_filter(%__MODULE__{api: api} = base, %__MODULE__{api: api} = addition) do + def add_to_filter(%__MODULE__{api: api} = base, _, %__MODULE__{api: api} = addition) do {:error, "Cannot add filter for resource #{inspect(addition.resource)} to filter with resource #{ inspect(base.resource) @@ -61,15 +81,16 @@ defmodule Ash.Filter do def add_to_filter( %__MODULE__{resource: resource} = base, + _, %__MODULE__{resource: resource} = addition ) do {:error, "Cannot add filter for api #{inspect(addition.api)} to filter with api #{inspect(base.api)}"} end - def add_to_filter(%__MODULE__{} = base, statement) do + def add_to_filter(%__MODULE__{} = base, op, statement) do case parse(base.api, base.resource, statement) do - {:ok, filter} -> add_to_filter(base, filter) + {:ok, filter} -> add_to_filter(base, op, filter) {:error, error} -> {:error, error} end end @@ -93,44 +114,49 @@ defmodule Ash.Filter do strict_subset_of(filter, candidate) == true end - def primary_key_filter?(nil), do: false - - def primary_key_filter?(filter) do - pkey = Ash.primary_key(filter.resource) - cleared_pkey_filter = Enum.map(pkey, fn key -> {key, nil} end) - - case cleared_pkey_filter do - [] -> - false - - cleared_pkey_filter -> - case parse(filter.api, filter.resource, cleared_pkey_filter) do - {:ok, parsed_cleared_pkey_filter} -> - cleared_candidate_filter = - map(filter, fn - %Predicate{attribute: %{name: name}, predicate: %mod{}} = predicate - when mod in [Eq, In] -> - if name in pkey do - %{ - predicate - | predicate: %Predicate{ - attribute: Ash.attribute(filter.resource, name), - predicate: %Eq{field: name, value: nil} - } - } - end - - other -> - other - end) - - strict_subset_of?(parsed_cleared_pkey_filter, cleared_candidate_filter) - - _ -> - false - end - end - end + # def primary_key_filter?(nil), do: false + + # def primary_key_filter?(filter) do + # pkey = Ash.primary_key(filter.resource) + # cleared_pkey_filter = Enum.map(pkey, fn key -> {key, nil} end) + + # case cleared_pkey_filter do + # [] -> + # false + + # cleared_pkey_filter -> + # case parse(filter.api, filter.resource, cleared_pkey_filter) do + # {:ok, parsed_cleared_pkey_filter} -> + # cleared_candidate_filter = clear_primary_key(filter, pkey) + + # strict_subset_of?(parsed_cleared_pkey_filter, cleared_candidate_filter) + + # _ -> + # false + # end + # end + # end + + # defp clear_primary_key(filter, pkey) do + # map(filter, fn + # %Predicate{attribute: %{name: name}, predicate: %mod{}} = predicate + # when mod in [Eq, In] -> + # if name in pkey do + # %{ + # predicate + # | predicate: %Predicate{ + # attribute: Ash.attribute(filter.resource, name), + # predicate: %Eq{field: name, value: nil} + # } + # } + # else + # predicate + # end + + # other -> + # other + # end) + # end def relationship_filter_request_paths(filter) do filter @@ -138,6 +164,8 @@ defmodule Ash.Filter do |> Enum.map(&[:filter, &1]) end + def read_requests(nil), do: [] + def read_requests(filter) do filter |> Ash.Filter.relationship_paths() @@ -215,9 +243,13 @@ defmodule Ash.Filter do def do_reduce(expression, acc, func) do case expression do %Expression{left: left, right: right} -> + acc = func.(right, acc) + acc = func.(left, acc) do_reduce(right, do_reduce(left, acc, func), func) %Not{expression: not_expr} -> + acc = func.(not_expr, acc) + do_reduce(not_expr, acc, func) other -> diff --git a/lib/ash/filter/not.ex b/lib/ash/filter/not.ex index 36d16ddc1..b4c69395e 100644 --- a/lib/ash/filter/not.ex +++ b/lib/ash/filter/not.ex @@ -1,4 +1,5 @@ defmodule Ash.Filter.Not do + @moduledoc "Represents the negation of the contained expression" defstruct [:expression] alias Ash.Filter.Expression diff --git a/lib/ash/filter/predicate.ex b/lib/ash/filter/predicate.ex index 8f4ba810f..1e899f303 100644 --- a/lib/ash/filter/predicate.ex +++ b/lib/ash/filter/predicate.ex @@ -1,7 +1,10 @@ defmodule Ash.Filter.Predicate do + @moduledoc "Represents a filter predicate" + defstruct [:attribute, :relationship_path, :predicate] - alias Ash.Filter.Predicate.Comparison + alias Ash.Filter + alias Ash.Filter.{Expression, Not} @type predicate :: struct @@ -23,7 +26,7 @@ defmodule Ash.Filter.Predicate do } @callback new(Ash.resource(), Ash.attribute(), term) :: {:ok, struct} | {:error, term} - @callback compare(predicate(), predicate()) :: Comparison.comparison() + @callback compare(predicate(), predicate()) :: comparison() defmacro __using__(_opts) do quote do @@ -38,7 +41,25 @@ defmodule Ash.Filter.Predicate do end @spec compare(predicate(), predicate()) :: comparison - def compare(%__MODULE__{predicate: left}, right), do: compare(left, right) + def compare(%__MODULE__{predicate: left} = pred, right) do + case compare(left, right) do + {:simplify, simplification} -> + simplification = + Filter.map(simplification, fn + %struct{} = expr when struct in [__MODULE__, Not, Expression] -> + expr + + other -> + wrap_in_predicate(pred, other) + end) + + {:simplify, simplification} + + other -> + other + end + end + def compare(left, %__MODULE__{predicate: right}), do: compare(left, right) def compare(left, right) do @@ -63,44 +84,29 @@ defmodule Ash.Filter.Predicate do {_, other} -> other end end + end - # right_compared_to_left = - # case left.__struct__.compare(right) do - # :unkown -> - # right.__struct__.compare(left) - # end - # left_compared_to_right = right.__struct__.compare(left) - - # # This is not very performant, and will result in many unnecassary cycles - # # simplifying values. This will work in the short term though. It can be - # # optimized later - # case {right_compared_to_left, left_compared_to_right} do - # {:unknown, :unknown} -> :unknown - # {:exclusive, :unknown} -> :right_excludes_left - # {:unknown, :exclusive} -> :left_excludes_right - # {:inclusive, :unknown} -> :right_includes_left - # {:unknown, :inclusive} -> :left_includes_right - # {:inclusive, :inclusive} -> :mutually_inclusive - # {:exclusive, :exclusive} -> :mutually_exclusive - # {_, :exclusive} -> :left_excludes_right - # {:exclusive, _} -> :right_excludes_left - # {_, :inclusive} -> :left_includes_right - # {:inclusive, _} -> :right_includes_left - # {{:simplify, new_left, _}, _} -> {:simplify, new_left} - # {_, {:simplify, _, new_left}} -> {:simplify, new_left} - # {_, _} -> :unknown - # end + defp wrap_in_predicate(predicate, %struct{} = other) do + if Ash.implements_behaviour?(struct, Ash.Filter.Predicate) do + %{predicate | predicate: other} + else + other + end end def new(resource, attribute, predicate, value, relationship_path) do case predicate.new(resource, attribute, value) do {:ok, predicate} -> - {:ok, - %__MODULE__{ - attribute: attribute, - predicate: predicate, - relationship_path: relationship_path - }} + if Ash.data_layer_can?(resource, {:filter_predicate, predicate}) do + {:ok, + %__MODULE__{ + attribute: attribute, + predicate: predicate, + relationship_path: relationship_path + }} + else + {:error, "Data layer does not support filtering with #{inspect(predicate)}"} + end {:error, error} -> {:error, error} @@ -127,7 +133,7 @@ defmodule Ash.Filter.Predicate do atom: :yellow, binary: :green, boolean: :pink, - list: :orange, + list: :cyan, map: :magenta, number: :red, regex: :violet, diff --git a/lib/ash/filter/predicate/eq.ex b/lib/ash/filter/predicate/eq.ex index 89966af91..d6d243eac 100644 --- a/lib/ash/filter/predicate/eq.ex +++ b/lib/ash/filter/predicate/eq.ex @@ -2,10 +2,11 @@ defmodule Ash.Filter.Predicate.Eq do @moduledoc false defstruct [:field, :value] - alias Ash.Error.Filter.InvalidFilterValue - use Ash.Filter.Predicate + alias Ash.Error.Filter.InvalidFilterValue + alias Ash.Filter.Predicate + def new(_resource, attribute, value) do case Ash.Type.cast_input(attribute.type, value) do {:ok, value} -> @@ -31,7 +32,7 @@ defmodule Ash.Filter.Predicate.Eq do def inspect(predicate, opts) do concat([ - Ash.Filter.Predicate.add_inspect_path(opts, predicate.field), + Predicate.add_inspect_path(opts, predicate.field), " == ", to_doc(predicate.value, opts) ]) diff --git a/lib/ash/filter/predicate/in.ex b/lib/ash/filter/predicate/in.ex index ae7d2956c..ff6ebe3b9 100644 --- a/lib/ash/filter/predicate/in.ex +++ b/lib/ash/filter/predicate/in.ex @@ -2,12 +2,13 @@ defmodule Ash.Filter.Predicate.In do @moduledoc false defstruct [:field, :values] + use Ash.Filter.Predicate + alias Ash.Error.Filter.InvalidFilterValue alias Ash.Filter.Expression + alias Ash.Filter.Predicate alias Ash.Filter.Predicate.Eq - use Ash.Filter.Predicate - def new(_resource, attribute, []), do: {:ok, %__MODULE__{field: attribute.name, values: MapSet.new([])}} @@ -15,7 +16,8 @@ defmodule Ash.Filter.Predicate.In do def new(_resource, attribute, values) when is_list(values) do Enum.reduce_while(values, {:ok, %__MODULE__{field: attribute.name, values: []}}, fn value, - predicate -> + {:ok, + predicate} -> case Ash.Type.cast_input(attribute.type, value) do {:ok, casted} -> {:cont, {:ok, %{predicate | values: [casted | predicate.values]}}} @@ -40,49 +42,28 @@ defmodule Ash.Filter.Predicate.In do )} end - def compare(%__MODULE__{field: field, values: values}, %__MODULE__{values: other_values}) do - left = MapSet.new(values) - right = MapSet.new(other_values) - - common_members = MapSet.intersection(left, right) - - if Enum.empty?(common_members) do - :exclusive - else - different_members = MapSet.difference(left, right) - - if Enum.empty?(different_members) do - :mutually_inclusive - else - {:simplify, - Expression.new( - :or, - %__MODULE__{field: field, values: MapSet.to_list(different_members)}, - %__MODULE__{field: field, values: MapSet.to_list(common_members)} - )} - end - end + def compare(%__MODULE__{} = left, %__MODULE__{} = right) do + {:simplify, in_to_or_equals(left), in_to_or_equals(right)} end - def compare( - %__MODULE__{values: values} = in_predicate, - %Eq{value: value} = equals - ) do - if value in values do - {:simplify, Expression.new(:or, equals, %{in_predicate | values: values -- [value]})} - else - :mutually_exclusive - end + def compare(%__MODULE__{} = in_expr, _) do + {:simplify, in_to_or_equals(in_expr)} end def compare(_, _), do: :unknown + defp in_to_or_equals(%{field: field, values: values}) do + Enum.reduce(values, nil, fn value, expression -> + Expression.new(:or, expression, %Eq{field: field, value: value}) + end) + end + defimpl Inspect do import Inspect.Algebra def inspect(predicate, opts) do concat([ - Ash.Filter.Predicate.add_inspect_path(opts, predicate.field), + Predicate.add_inspect_path(opts, predicate.field), " in ", to_doc(predicate.values, opts) ]) diff --git a/lib/ash/query.ex b/lib/ash/query.ex index 48be32543..9a32a3d0c 100644 --- a/lib/ash/query.ex +++ b/lib/ash/query.ex @@ -24,6 +24,20 @@ defmodule Ash.Query do import Inspect.Algebra def inspect(query, opts) do + opts = %{ + opts + | syntax_colors: [ + atom: :yellow, + binary: :green, + boolean: :pink, + list: :cyan, + map: :magenta, + number: :red, + regex: :violet, + tuple: :white + ] + } + error_doc = if Enum.empty?(query.errors) do empty() diff --git a/lib/ash/type/type.ex b/lib/ash/type/type.ex index 6ab0a4d61..38074ce26 100644 --- a/lib/ash/type/type.ex +++ b/lib/ash/type/type.ex @@ -22,7 +22,7 @@ defmodule Ash.Type do integer: [ecto_type: :integer, filters: [:eq, :in, :not_eq, :not_in], sortable?: true], boolean: [ecto_type: :boolean, filters: [:eq, :not_eq], sortable?: true], int: [ecto_type: :integer, filters: [:eq, :in, :not_eq, :not_in], sortable?: true], - uuid: [ecto_type: Ecto.UUID, filters: [:eq, :in, :not_eq, :not_in], sortable?: true], + uuid: [ecto_type: :binary_id, filters: [:eq, :in, :not_eq, :not_in], sortable?: true], date: [ecto_type: :date, filters: [:eq, :in, :not_eq, :not_in], sortable?: true], utc_datetime: [ ecto_type: :utc_datetime, @@ -52,18 +52,6 @@ defmodule Ash.Type do value end - @spec supports_filter?(Ash.resource(), t(), Ash.DataLayer.filter_type(), Ash.data_layer()) :: - boolean - def supports_filter?(resource, type, filter_type, _data_layer) when type in @builtin_names do - Ash.data_layer_can?(resource, {:filter, filter_type}) and - filter_type in @builtins[type][:filters] - end - - def supports_filter?(resource, type, filter_type, data_layer) do - Ash.data_layer_can?(resource, {:filter, filter_type}) and - filter_type in type.supported_filter_types(data_layer) - end - @doc """ Determines whether or not this value can be sorted. """ diff --git a/lib/sat_solver.ex b/lib/sat_solver.ex index 1131d63a3..9e46d9bea 100644 --- a/lib/sat_solver.ex +++ b/lib/sat_solver.ex @@ -13,41 +13,45 @@ defmodule Ash.SatSolver do true {_filter_expr, %{expression: nil}} -> - # TODO: truesim check `filter_expr` false {filter, candidate} -> + do_strict_filter_subset(filter, candidate) + end + end + + defp do_strict_filter_subset(filter, candidate) do + case add_comparisons_and_solve_expression( + Expression.new(:and, filter.expression, candidate.expression) + ) do + {:error, :unsatisfiable} -> + false + + {:ok, _} -> case add_comparisons_and_solve_expression( - Expression.new(:and, filter.expression, candidate.expression) + Expression.new(:and, Not.new(filter.expression), candidate.expression) ) do {:error, :unsatisfiable} -> - false + true - {:ok, _} -> - case add_comparisons_and_solve_expression( - Expression.new(:and, Not.new(filter.expression), candidate.expression) - ) do - {:error, :unsatisfiable} -> - true - - _ -> - :maybe - end + _ -> + :maybe end end end defp filter_to_expr(nil), do: nil + defp filter_to_expr(false), do: false + defp filter_to_expr(true), do: true defp filter_to_expr(%Filter{expression: expression}), do: filter_to_expr(expression) defp filter_to_expr(%Predicate{} = predicate), do: predicate - defp filter_to_expr(%Not{expression: expression}), do: {:not, expression} + defp filter_to_expr(%Not{expression: expression}), do: {:not, filter_to_expr(expression)} defp filter_to_expr(%Expression{op: op, left: left, right: right}) do {op, filter_to_expr(left), filter_to_expr(right)} end def add_comparisons_and_solve_expression(expression) do - # TODO: Make a ticket to optimize/think more about this all_predicates = Filter.reduce(expression, [], fn %Predicate{} = predicate, predicates -> @@ -60,13 +64,8 @@ defmodule Ash.SatSolver do simplified = Filter.map(expression, fn %Predicate{} = predicate -> - all_predicates - |> Enum.find_value(fn other_predicate -> - case Predicate.compare(predicate, other_predicate) do - {:simplify, simplification} -> {:simplify, simplification} - _ -> false - end - end) + predicate + |> find_simplification(all_predicates) |> case do nil -> predicate @@ -80,6 +79,16 @@ defmodule Ash.SatSolver do end) if simplified == expression do + all_predicates = + Filter.reduce(expression, [], fn + %Predicate{} = predicate, predicates -> + [predicate | predicates] + + _, predicates -> + predicates + end) + |> Enum.uniq() + comparison_expressions = all_predicates |> Enum.reduce([], fn predicate, new_expressions -> @@ -98,9 +107,10 @@ defmodule Ash.SatSolver do [{:not, {:and, other_predicate, predicate}} | new_expressions] {:simplify, _} -> - new_expressions + # Filter should be fully simplified here + raise "What" - _ -> + _other -> # If we can't tell, we assume they are exclusive statements [{:not, {:and, other_predicate, predicate}} | new_expressions] end @@ -121,6 +131,16 @@ defmodule Ash.SatSolver do end end + defp find_simplification(predicate, predicates) do + predicates + |> Enum.find_value(fn other_predicate -> + case Predicate.compare(predicate, other_predicate) do + {:simplify, simplification} -> {:simplify, simplification} + _ -> false + end + end) + end + def solve_expression(expression) do expression_with_constants = {:and, true, {:and, {:not, false}, expression}} diff --git a/test/filter/filter_test.exs b/test/filter/filter_test.exs index 3ec1c7a01..a4ca7901a 100644 --- a/test/filter/filter_test.exs +++ b/test/filter/filter_test.exs @@ -2,6 +2,8 @@ defmodule Ash.Test.Filter.FilterTest do @moduledoc false use ExUnit.Case, async: true + alias Ash.Filter + defmodule Profile do @moduledoc false use Ash.Resource, data_layer: Ash.DataLayer.Ets @@ -249,4 +251,61 @@ defmodule Ash.Test.Filter.FilterTest do |> Api.read!() end end + + describe "filter subset logic" do + test "can detect a filter is a subset of itself" do + filter = Filter.parse!(Api, Post, %{points: 1}) + + assert Filter.strict_subset_of?(filter, filter) + end + + test "can detect a filter is a subset of itself *and* something else" do + filter = Filter.parse!(Api, Post, points: 1) + + candidate = Filter.add_to_filter!(filter, title: "Title") + + assert Filter.strict_subset_of?(filter, candidate) + end + + test "can detect a filter is not a subset of itself *or* something else" do + filter = Filter.parse!(Api, Post, points: 1) + + candidate = Filter.add_to_filter!(filter, :or, title: "Title") + + refute Filter.strict_subset_of?(filter, candidate) + end + + test "can detect a filter is a subset based on a simplification" do + filter = Filter.parse!(Api, Post, points: [in: [1, 2]]) + + candidate = Filter.parse!(Api, Post, points: 1) + + assert Filter.strict_subset_of?(filter, candidate) + end + + test "can detect a filter is not a subset based on a simplification" do + filter = Filter.parse!(Api, Post, points: [in: [1, 2]]) + + candidate = Filter.parse!(Api, Post, points: 3) + + refute Filter.strict_subset_of?(filter, candidate) + end + + test "can detect a more complicated scenario" do + filter = Filter.parse!(Api, Post, or: [points: [in: [1, 2, 3]], points: 4, points: 5]) + + candidate = Filter.parse!(Api, Post, or: [points: 1, points: 3, points: 5]) + + assert Filter.strict_subset_of?(filter, candidate) + end + + test "understands unrelated negations" do + filter = Filter.parse!(Api, Post, or: [points: [in: [1, 2, 3]], points: 4, points: 5]) + + candidate = + Filter.parse!(Api, Post, or: [points: 1, points: 3, points: 5], not: [points: 7]) + + assert Filter.strict_subset_of?(filter, candidate) + end + end end