From f41cc77549e4b386eba50821bb7c40be47260858 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Sat, 19 Sep 2020 15:46:34 -0400 Subject: [PATCH] Various Improvements (#113) --- .credo.exs | 3 +- .formatter.exs | 2 + lib/ash/actions/destroy.ex | 7 ++ lib/ash/actions/update.ex | 2 +- lib/ash/data_layer/ets.ex | 29 ++++++++- lib/ash/filter/filter.ex | 60 ++++++++++++++++- lib/ash/filter/predicate.ex | 9 ++- .../filter/predicate/greater_than_or_equal.ex | 64 +++++++++++++++++++ lib/ash/filter/predicate/in.ex | 6 +- lib/ash/filter/predicate/less_than.ex | 2 +- .../filter/predicate/less_than_or_equal.ex | 64 +++++++++++++++++++ lib/ash/query/query.ex | 38 +++++++++-- lib/ash/resource.ex | 5 ++ lib/ash/resource/actions/destroy.ex | 12 +++- lib/ash/resource/change/builtins.ex | 4 ++ lib/ash/resource/change/set_attribute.ex | 34 ++++++++++ lib/ash/resource/dsl.ex | 14 +++- test/filter/filter_test.exs | 50 +++++++++++++++ 18 files changed, 385 insertions(+), 20 deletions(-) create mode 100644 lib/ash/filter/predicate/greater_than_or_equal.ex create mode 100644 lib/ash/filter/predicate/less_than_or_equal.ex create mode 100644 lib/ash/resource/change/set_attribute.ex diff --git a/.credo.exs b/.credo.exs index 5c7c18ae..1487c375 100644 --- a/.credo.exs +++ b/.credo.exs @@ -81,8 +81,7 @@ # You can customize the priority of any check # Priority values are: `low, normal, high, higher` # - {Credo.Check.Design.AliasUsage, - [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]}, + {Credo.Check.Design.AliasUsage, false}, # You can also customize the exit_status of each check. # If you don't want TODO comments to cause `mix credo` to fail, just # set this value to 0 (zero). diff --git a/.formatter.exs b/.formatter.exs index 30ef5c22..19654cd8 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -7,6 +7,7 @@ locals_without_parens = [ argument: 3, attribute: 2, attribute: 3, + base_filter: 1, belongs_to: 2, belongs_to: 3, calculate: 2, @@ -51,6 +52,7 @@ locals_without_parens = [ required?: 1, resource: 1, resource: 2, + soft?: 1, source_field: 1, source_field_on_join_table: 1, table: 1, diff --git a/lib/ash/actions/destroy.ex b/lib/ash/actions/destroy.ex index 0487e14f..d7829096 100644 --- a/lib/ash/actions/destroy.ex +++ b/lib/ash/actions/destroy.ex @@ -5,6 +5,13 @@ defmodule Ash.Actions.Destroy do @spec run(Ash.api(), Ash.Changeset.t(), Ash.action(), Keyword.t()) :: :ok | {:error, Ash.Changeset.t()} | {:error, Ash.error()} + def run(api, changeset, %{soft?: true} = action, opts) do + case Ash.Actions.Update.run(api, %{changeset | action_type: :destroy}, action, opts) do + {:ok, _} -> :ok + other -> other + end + end + def run(api, %{data: record, resource: resource} = changeset, action, opts) do engine_opts = opts diff --git a/lib/ash/actions/update.ex b/lib/ash/actions/update.ex index 5da42a65..d966d651 100644 --- a/lib/ash/actions/update.ex +++ b/lib/ash/actions/update.ex @@ -90,7 +90,7 @@ defmodule Ash.Actions.Update do defp add_validations(changeset) do changeset.resource() - |> Ash.Resource.validations(:update) + |> Ash.Resource.validations(changeset.action_type) |> Enum.reduce(changeset, fn validation, changeset -> Ash.Changeset.before_action(changeset, &do_validation(&1, validation)) end) diff --git a/lib/ash/data_layer/ets.ex b/lib/ash/data_layer/ets.ex index cfdb039f..23e918e2 100644 --- a/lib/ash/data_layer/ets.ex +++ b/lib/ash/data_layer/ets.ex @@ -7,7 +7,16 @@ defmodule Ash.DataLayer.Ets do alias Ash.Actions.Sort alias Ash.Filter.{Expression, Not, Predicate} - alias Ash.Filter.Predicate.{Eq, GreaterThan, In, IsNil, LessThan} + + alias Ash.Filter.Predicate.{ + Eq, + GreaterThan, + GreaterThanOrEqual, + In, + IsNil, + LessThan, + LessThanOrEqual + } @behaviour Ash.DataLayer @@ -58,6 +67,8 @@ defmodule Ash.DataLayer.Ets do def can?(_, {:filter_predicate, _, %Eq{}}), do: true def can?(_, {:filter_predicate, _, %LessThan{}}), do: true def can?(_, {:filter_predicate, _, %GreaterThan{}}), do: true + def can?(_, {:filter_predicate, _, %LessThanOrEqual{}}), do: true + def can?(_, {:filter_predicate, _, %GreaterThanOrEqual{}}), do: true def can?(_, {:filter_predicate, _, %IsNil{}}), do: true def can?(_, {:sort, _}), do: true def can?(_, _), do: false @@ -180,6 +191,20 @@ defmodule Ash.DataLayer.Ets do end end + defp matches_predicate?(record, field, %LessThanOrEqual{value: predicate_value}) do + case Map.fetch(record, field) do + {:ok, value} -> value <= predicate_value + :error -> false + end + end + + defp matches_predicate?(record, field, %GreaterThanOrEqual{value: predicate_value}) do + case Map.fetch(record, field) do + {:ok, value} -> value >= predicate_value + :error -> false + end + end + defp matches_predicate?(record, field, %In{values: predicate_values}) do case Map.fetch(record, field) do {:ok, value} -> value in predicate_values @@ -197,7 +222,7 @@ defmodule Ash.DataLayer.Ets do @impl true def upsert(resource, changeset) do - create(resource, changeset) + update(resource, changeset) end @impl true diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index e6540976..d07694d7 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -14,6 +14,51 @@ defmodule Ash.Filter do You can pass a filter template to `build_filter_from_template/2` with an actor, and it will return the new result Additionally, you can ask if the filter template contains an actor reference via `template_references_actor?/1` + + ## Writing a filter: + + A filter is a nested keyword list (with some exceptions, like `true` for everything and `false` for nothing). + + The key is the "predicate" (A.K.A condition) and the value is the parameter. You can use `and` and `or` to create + nested filters. Datalayers can expose custom predicates. Eventually, you will be able to define your own custom + predicates, which will be a mechanism for you to attach complex filters supported by the data layer to your queries. + + ** Important ** + In a given keyword list, all predicates are considered to be "ands". So `[or: [first_name: "Tom", last_name: "Bombadil"]]` doesn't + mean 'First name == "tom" or last_name == "bombadil"'. To say that, you want to provide a list of filters, + like so: `[or: [[first_name: "Tom"], [last_name: "Bombadil"]]]` + + The builtin predicates are: + + * eq - shorthand for equals + * equals + * in + * lt - shorthand for less_than + * gt - shorthand for greater_than + * lte - shorthand for less_than_or_equal + * gte - shorthand for greater_than_or_equal + * less_than + * greater_than + * less_than_or_equal + * greater_than_or_equal + * is_nil + + Some example filters: + + ```elixir + [name: "Zardoz"] + [first_name: "Zar", last_name: "Doz"] + [first_name: "Zar", last_name: [in: ["Doz", "Daz"]], high_score: [greater_than: 10]] + [first_name: "Zar", last_name: [in: ["Doz", "Daz"]], high_score: [greater_than: 10]] + [or: [ + [first_name: "Zar"], + [last_name: "Doz"], + [or: [ + [high_score: [greater_than: 10]]], + [high_score: [less_than: -10]] + ] + ]] + ``` """ alias Ash.Actions.SideLoad alias Ash.Engine.Request @@ -26,7 +71,16 @@ defmodule Ash.Filter do ReadActionRequired } - alias Ash.Filter.Predicate.{Eq, GreaterThan, In, IsNil, LessThan} + alias Ash.Filter.Predicate.{ + Eq, + GreaterThan, + GreaterThanOrEqual, + In, + IsNil, + LessThan, + LessThanOrEqual + } + alias Ash.Filter.{Expression, Not, Predicate} alias Ash.Query.Aggregate @@ -36,8 +90,12 @@ defmodule Ash.Filter do in: In, lt: LessThan, gt: GreaterThan, + lte: LessThanOrEqual, + gte: GreaterThanOrEqual, less_than: LessThan, greater_than: GreaterThan, + less_than_or_equal: LessThanOrEqual, + greater_than_or_equal: GreaterThanOrEqual, is_nil: IsNil ] diff --git a/lib/ash/filter/predicate.ex b/lib/ash/filter/predicate.ex index cd8d8671..ec6db3af 100644 --- a/lib/ash/filter/predicate.ex +++ b/lib/ash/filter/predicate.ex @@ -1,7 +1,12 @@ defmodule Ash.Filter.Predicate do - @moduledoc "Represents a filter predicate" + @moduledoc """ + Represents a filter predicate - defstruct [:resource, :attribute, :relationship_path, :predicate, :value] + The `embedded` flag is set to true for predicates that are present in the `base_filter`. + Datalayers may optionally use this information. + """ + + defstruct [:resource, :attribute, :relationship_path, :predicate, :value, embedded: false] alias Ash.Error.Query.UnsupportedPredicate alias Ash.Filter diff --git a/lib/ash/filter/predicate/greater_than_or_equal.ex b/lib/ash/filter/predicate/greater_than_or_equal.ex new file mode 100644 index 00000000..b2b29df0 --- /dev/null +++ b/lib/ash/filter/predicate/greater_than_or_equal.ex @@ -0,0 +1,64 @@ +defmodule Ash.Filter.Predicate.GreaterThanOrEqual do + @moduledoc "A predicate for a value being greater than the provided value" + defstruct [:field, :value, :type] + + alias Ash.Filter.Predicate.Eq + + alias Ash.Error.Query.InvalidFilterValue + + use Ash.Filter.Predicate + + def new(_resource, attribute, value) do + case Ash.Type.cast_input(attribute.type, value) do + {:ok, value} -> + {:ok, %__MODULE__{field: attribute.name, value: value}} + + _ -> + {:error, + InvalidFilterValue.exception( + value: value, + context: %__MODULE__{field: attribute.name, value: value}, + message: "Could not be casted to type #{inspect(attribute.type)}" + )} + end + end + + def match?(%{value: predicate_value}, value, _) do + value >= predicate_value + end + + def compare(%__MODULE__{value: value}, %__MODULE__{value: value}), do: :mutually_inclusive + + def compare(%__MODULE__{value: value}, %__MODULE__{value: other_value}) + when value > other_value do + :right_includes_left + end + + def compare(%__MODULE__{value: value}, %__MODULE__{value: other_value}) + when value < other_value do + :left_includes_right + end + + def compare(%__MODULE__{value: value}, %Eq{value: eq_value}) when eq_value >= value do + :left_includes_right + end + + def compare(%__MODULE__{}, %Eq{}) do + :mutually_exclusive + end + + def compare(_, _), do: :unknown + + defimpl Inspect do + import Inspect.Algebra + alias Ash.Filter.Predicate + + def inspect(predicate, opts) do + concat([ + Predicate.add_inspect_path(opts, predicate.field), + " >= ", + to_doc(predicate.value, opts) + ]) + end + end +end diff --git a/lib/ash/filter/predicate/in.ex b/lib/ash/filter/predicate/in.ex index 88d643ed..ccf17134 100644 --- a/lib/ash/filter/predicate/in.ex +++ b/lib/ash/filter/predicate/in.ex @@ -50,16 +50,16 @@ defmodule Ash.Filter.Predicate.In do end def compare(%__MODULE__{} = left, %__MODULE__{} = right) do - {:simplify, in_to_or_equals(left), in_to_or_equals(right)} + {:simplify, into_or_equals(left), into_or_equals(right)} end def compare(%__MODULE__{} = in_expr, _) do - {:simplify, in_to_or_equals(in_expr)} + {:simplify, into_or_equals(in_expr)} end def compare(_, _), do: :unknown - defp in_to_or_equals(%{field: field, values: values}) do + defp into_or_equals(%{field: field, values: values}) do Enum.reduce(values, nil, fn value, expression -> Expression.new(:or, expression, %Eq{field: field, value: value}) end) diff --git a/lib/ash/filter/predicate/less_than.ex b/lib/ash/filter/predicate/less_than.ex index f8d41ff8..18e257b3 100644 --- a/lib/ash/filter/predicate/less_than.ex +++ b/lib/ash/filter/predicate/less_than.ex @@ -24,7 +24,7 @@ defmodule Ash.Filter.Predicate.LessThan do end def match?(%{value: predicate_value}, value, _) do - value > predicate_value + value < predicate_value end def compare(%__MODULE__{value: value}, %__MODULE__{value: value}), do: :mutually_inclusive diff --git a/lib/ash/filter/predicate/less_than_or_equal.ex b/lib/ash/filter/predicate/less_than_or_equal.ex new file mode 100644 index 00000000..b6bccb68 --- /dev/null +++ b/lib/ash/filter/predicate/less_than_or_equal.ex @@ -0,0 +1,64 @@ +defmodule Ash.Filter.Predicate.LessThanOrEqual do + @moduledoc "A predicate for a value being greater than the provided value" + defstruct [:field, :value, :type] + + alias Ash.Filter.Predicate.Eq + + alias Ash.Error.Query.InvalidFilterValue + + use Ash.Filter.Predicate + + def new(_resource, attribute, value) do + case Ash.Type.cast_input(attribute.type, value) do + {:ok, value} -> + {:ok, %__MODULE__{field: attribute.name, value: value}} + + _ -> + {:error, + InvalidFilterValue.exception( + value: value, + context: %__MODULE__{field: attribute.name, value: value}, + message: "Could not be casted type type #{inspect(attribute.type)}" + )} + end + end + + def match?(%{value: predicate_value}, value, _) do + value <= predicate_value + end + + def compare(%__MODULE__{value: value}, %__MODULE__{value: value}), do: :mutually_inclusive + + def compare(%__MODULE__{value: value}, %__MODULE__{value: other_value}) + when value < other_value do + :right_includes_left + end + + def compare(%__MODULE__{value: value}, %__MODULE__{value: other_value}) + when value > other_value do + :left_includes_right + end + + def compare(%__MODULE__{value: value}, %Eq{value: eq_value}) when eq_value <= value do + :left_includes_right + end + + def compare(%__MODULE__{}, %Eq{}) do + :mutually_exclusive + end + + def compare(_, _), do: :unknown + + defimpl Inspect do + import Inspect.Algebra + alias Ash.Filter.Predicate + + def inspect(predicate, opts) do + concat([ + Predicate.add_inspect_path(opts, predicate.field), + " <= ", + to_doc(predicate.value, opts) + ]) + end + end +end diff --git a/lib/ash/query/query.ex b/lib/ash/query/query.ex index 09232d2d..4971b3d4 100644 --- a/lib/ash/query/query.ex +++ b/lib/ash/query/query.ex @@ -66,12 +66,32 @@ defmodule Ash.Query do @doc "Create a new query." def new(resource, api \\ nil) when is_atom(resource) do - %__MODULE__{ - api: api, - filter: nil, - resource: resource - } - |> set_data_layer_query() + query = + %__MODULE__{ + api: api, + filter: nil, + resource: resource + } + |> set_data_layer_query() + + case Ash.Resource.base_filter(resource) do + nil -> + query + + filter -> + filter = Ash.Filter.parse!(resource, filter) + + filter = + Ash.Filter.map(filter, fn + %Ash.Filter.Predicate{} = pred -> + %{pred | embedded: true} + + other -> + other + end) + + filter(query, filter) + end end @spec load(t(), atom | list(atom) | Keyword.t()) :: t() @@ -515,6 +535,12 @@ defmodule Ash.Query do end) end + @doc """ + Attach a filter statement to the query. + + The filter is applied as an "and" to any filters currently on the query. + For more information on writing filters, see: `Ash.Filter`. + """ @spec filter(t() | Ash.resource(), nil | false | Ash.filter() | Keyword.t()) :: t() def filter(query, nil), do: to_query(query) diff --git a/lib/ash/resource.ex b/lib/ash/resource.ex index d4d68033..6f7b87f7 100644 --- a/lib/ash/resource.ex +++ b/lib/ash/resource.ex @@ -89,6 +89,11 @@ defmodule Ash.Resource do Extension.get_opt(resource, [:resource], :description, "no description") end + @spec base_filter(Ash.resource()) :: term + def base_filter(resource) do + Extension.get_opt(resource, [:resource], :base_filter, nil) + end + @doc "A list of identities for the resource" @spec identities(Ash.resource()) :: [Ash.Resource.Identity.t()] def identities(resource) do diff --git a/lib/ash/resource/actions/destroy.ex b/lib/ash/resource/actions/destroy.ex index 1c101125..dc7da1f8 100644 --- a/lib/ash/resource/actions/destroy.ex +++ b/lib/ash/resource/actions/destroy.ex @@ -1,7 +1,7 @@ defmodule Ash.Resource.Actions.Destroy do @moduledoc "Represents a destroy action on a resource." - defstruct [:name, :primary?, type: :destroy] + defstruct [:name, :primary?, :changes, :accept, :soft?, type: :destroy] @type t :: %__MODULE__{ type: :destroy, @@ -18,6 +18,16 @@ defmodule Ash.Resource.Actions.Destroy do type: :boolean, default: false, doc: "Whether or not this action should be used when no action is specified by the caller." + ], + accept: [ + type: {:custom, Ash.OptionsHelpers, :list_of_atoms, []}, + doc: + "The list of attributes and relationships to accept. Defaults to all attributes on the resource. Has no effect unless `soft?` is specified." + ], + soft?: [ + type: :atom, + doc: + "If specified, the destroy action calls the datalayer's update function with any specified changes." ] ] diff --git a/lib/ash/resource/change/builtins.ex b/lib/ash/resource/change/builtins.ex index ec287e10..de4b8e4c 100644 --- a/lib/ash/resource/change/builtins.ex +++ b/lib/ash/resource/change/builtins.ex @@ -8,5 +8,9 @@ defmodule Ash.Resource.Change.Builtins do {Ash.Resource.Change.RelateActor, relationship: relationship} end + def set_attribute(attribute, value) do + {Ash.Resource.Change.SetAttribute, attribute: attribute, value: value} + end + def actor(value), do: {:_actor, value} end diff --git a/lib/ash/resource/change/set_attribute.ex b/lib/ash/resource/change/set_attribute.ex new file mode 100644 index 00000000..efbcf82e --- /dev/null +++ b/lib/ash/resource/change/set_attribute.ex @@ -0,0 +1,34 @@ +defmodule Ash.Resource.Change.SetAttribute do + @moduledoc """ + Sets the attribute to the value provided. If a zero argument function is provided, it is called to determine the value. + """ + use Ash.Resource.Change + alias Ash.Changeset + + def init(opts) do + with :ok <- validate_attribute(opts[:attribute]), + :ok <- validate_value(opts[:value]) do + {:ok, opts} + end + end + + defp validate_attribute(nil), do: {:error, "attribute is required"} + defp validate_attribute(value) when is_atom(value), do: :ok + defp validate_attribute(other), do: {:error, "attribute is invalid: #{inspect(other)}"} + defp validate_value(value) when is_function(value, 0), do: :ok + + defp validate_value(value) when is_function(value), + do: {:error, "only 0 argument functions are supported"} + + defp validate_value(_), do: :ok + + def change(changeset, opts, _) do + value = + case opts[:value] do + value when is_function(value) -> value.() + value -> value + end + + Changeset.force_change_attribute(changeset, opts[:attribute], value) + end +end diff --git a/lib/ash/resource/dsl.ex b/lib/ash/resource/dsl.ex index ee5145f3..43898a00 100644 --- a/lib/ash/resource/dsl.ex +++ b/lib/ash/resource/dsl.ex @@ -182,6 +182,8 @@ defmodule Ash.Resource.Dsl do so you can say something like: `change my_change(1)` + + For destroys, `changes` are not applied unless `soft?` is set to true. """, examples: [ "change relate_actor(:reporter)", @@ -250,6 +252,11 @@ defmodule Ash.Resource.Dsl do examples: [ "destroy :soft_delete, primary?: true" ], + entities: [ + changes: [ + @change + ] + ], target: Ash.Resource.Actions.Destroy, schema: Ash.Resource.Actions.Destroy.opt_schema(), args: [:name] @@ -322,7 +329,12 @@ defmodule Ash.Resource.Dsl do ], schema: [ description: [ - type: :string + type: :string, + doc: "A human readable description of the resource, to be used in generated documentation" + ], + base_filter: [ + type: :any, + doc: "A filter statement to be applied to any queries on the resource" ] ] } diff --git a/test/filter/filter_test.exs b/test/filter/filter_test.exs index 863caadf..89efa454 100644 --- a/test/filter/filter_test.exs +++ b/test/filter/filter_test.exs @@ -117,12 +117,42 @@ defmodule Ash.Test.Filter.FilterTest do end end + defmodule SoftDeletePost do + @moduledoc false + use Ash.Resource, data_layer: Ash.DataLayer.Ets + + ets do + private? true + end + + resource do + base_filter is_nil: :deleted_at + end + + actions do + read :default + create :default + + destroy :default do + soft? true + + change set_attribute(:deleted_at, &DateTime.utc_now/0) + end + end + + attributes do + attribute :id, :uuid, primary_key?: true, default: &Ecto.UUID.generate/0 + attribute :deleted_at, :utc_datetime + end + end + defmodule Api do @moduledoc false use Ash.Api resources do resource(Post) + resource(SoftDeletePost) resource(User) resource(Profile) resource(PostLink) @@ -374,4 +404,24 @@ defmodule Ash.Test.Filter.FilterTest do assert Filter.strict_subset_of?(filter, candidate) end end + + describe "base_filter" do + test "resources that apply to the base filter are returned" do + %{id: id} = + SoftDeletePost + |> new(%{}) + |> Api.create!() + + assert [%{id: ^id}] = Api.read!(SoftDeletePost) + end + + test "resources that don't apply to the base filter are not returned" do + SoftDeletePost + |> new(%{}) + |> Api.create!() + |> Api.destroy() + + assert [] = Api.read!(SoftDeletePost) + end + end end