From 8e7815388ed68deee37c61e16268935db51faf3e Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Mon, 5 Jun 2023 16:50:11 -0400 Subject: [PATCH] improvement: handle `nil`s in memory the same way sql would have --- lib/ash/data_layer/ets/ets.ex | 15 ++++++++-- lib/ash/filter/filter.ex | 43 +++++++++++++++++----------- lib/ash/filter/runtime.ex | 10 +++++-- lib/ash/query/boolean_expression.ex | 14 ++++++--- test/actions/read_test.exs | 32 ++++++++++----------- test/support/flow/flows/branching.ex | 2 +- test/support/flow/flows/halting.ex | 26 ++++++++--------- 7 files changed, 87 insertions(+), 55 deletions(-) diff --git a/lib/ash/data_layer/ets/ets.ex b/lib/ash/data_layer/ets/ets.ex index 4b40d75e..c0794cc7 100644 --- a/lib/ash/data_layer/ets/ets.ex +++ b/lib/ash/data_layer/ets/ets.ex @@ -373,11 +373,15 @@ defmodule Ash.DataLayer.Ets do case Ash.Expr.eval_hydrated(expression, record: record, resource: resource) do {:ok, value} -> if calculation.load do - {:cont, {:ok, Map.put(record, calculation.load, value)}} + {:cont, {:ok, Map.put(record, calculation.load, value || calculation.default)}} else {:cont, {:ok, - Map.update!(record, :calculations, &Map.put(&1, calculation.name, value))}} + Map.update!( + record, + :calculations, + &Map.put(&1, calculation.name, value || calculation.default) + )}} end :unknown -> @@ -385,7 +389,12 @@ defmodule Ash.DataLayer.Ets do {:cont, {:ok, Map.put(record, calculation.load, nil)}} else {:cont, - {:ok, Map.update!(record, :calculations, &Map.put(&1, calculation.name, nil))}} + {:ok, + Map.update!( + record, + :calculations, + &Map.put(&1, calculation.name, calculation.default) + )}} end {:error, error} -> diff --git a/lib/ash/filter/filter.ex b/lib/ash/filter/filter.ex index d7fe5f38..e266a901 100644 --- a/lib/ash/filter/filter.ex +++ b/lib/ash/filter/filter.ex @@ -193,7 +193,7 @@ defmodule Ash.Filter do use Ash.Api resources do - allow_unregistered? true + allow_unregistered?(true) end end @@ -1717,7 +1717,7 @@ defmodule Ash.Filter do end defp records_to_expression(records, relationship, path) do - Enum.reduce_while(records, {:ok, nil}, fn record, {:ok, expression} -> + Enum.reduce_while(records, {:ok, true}, fn record, {:ok, expression} -> case records_to_expression([record], relationship, path) do {:ok, operator} -> {:cont, {:ok, BooleanExpression.optimized_new(:and, expression, operator)}} @@ -2074,7 +2074,7 @@ defmodule Ash.Filter do do: {:ok, move_to_relationship_path(expression, context[:relationship_path] || [])} defp parse_expression(statement, context) when is_list(statement) do - Enum.reduce_while(statement, {:ok, nil}, fn expression_part, {:ok, expression} -> + Enum.reduce_while(statement, {:ok, true}, fn expression_part, {:ok, expression} -> case add_expression_part(expression_part, context, expression) do {:ok, new_expression} -> {:cont, {:ok, new_expression}} @@ -2089,8 +2089,13 @@ defmodule Ash.Filter do parse_expression([statement], context) end - defp add_expression_part(boolean, _context, expression) when is_boolean(boolean), - do: {:ok, BooleanExpression.optimized_new(:and, expression, boolean)} + defp add_expression_part(boolean, context, nil) do + add_expression_part(boolean, context, true) + end + + defp add_expression_part(boolean, _context, expression) when is_boolean(boolean) do + {:ok, BooleanExpression.optimized_new(:and, expression, boolean)} + end defp add_expression_part(%__MODULE__{expression: adding_expression}, context, expression) do {:ok, @@ -2544,7 +2549,7 @@ defmodule Ash.Filter do value |> Map.to_list() - |> Enum.reduce_while({:ok, nil}, fn {key, value}, {:ok, expression} -> + |> Enum.reduce_while({:ok, true}, fn {key, value}, {:ok, expression} -> case add_expression_part({key, value}, context, expression) do {:ok, new_expression} -> {:cont, {:ok, new_expression}} @@ -3240,16 +3245,22 @@ defmodule Ash.Filter do defp add_to_ref_path(other, _), do: other - defp parse_and_join(statements, op, context) do - Enum.reduce_while(statements, {:ok, nil}, fn statement, {:ok, expression} -> - case parse_expression(statement, context) do - {:ok, nested_expression} -> - {:cont, {:ok, BooleanExpression.optimized_new(op, expression, nested_expression)}} + defp parse_and_join([statement | statements], op, context) do + case parse_expression(statement, context) do + {:ok, nested_expression} -> + Enum.reduce_while(statements, {:ok, nested_expression}, fn statement, {:ok, expression} -> + case parse_expression(statement, context) do + {:ok, nested_expression} -> + {:cont, {:ok, BooleanExpression.optimized_new(op, expression, nested_expression)}} - {:error, error} -> - {:halt, {:error, error}} - end - end) + {:error, error} -> + {:halt, {:error, error}} + end + end) + + {:error, error} -> + {:halt, {:error, error}} + end end defp parse_predicates(value, field, context) when not is_list(value) and not is_map(value) do @@ -3266,7 +3277,7 @@ defmodule Ash.Filter do parse_predicates([eq: values], attr, context) else if is_map(values) || Keyword.keyword?(values) do - Enum.reduce_while(values, {:ok, nil}, fn + Enum.reduce_while(values, {:ok, true}, fn {:not, value}, {:ok, expression} -> case parse_predicates(List.wrap(value), attr, context) do {:ok, not_expression} -> diff --git a/lib/ash/filter/runtime.ex b/lib/ash/filter/runtime.ex index 4e974ec2..cc648beb 100644 --- a/lib/ash/filter/runtime.ex +++ b/lib/ash/filter/runtime.ex @@ -279,11 +279,17 @@ defmodule Ash.Filter.Runtime do :unknown end + %Not{expression: nil} -> + {:ok, nil} + %Not{expression: expression} -> case do_match(record, expression, parent, resource) do :unknown -> :unknown + {:ok, nil} -> + {:ok, nil} + {:ok, match?} -> {:ok, !match?} @@ -744,7 +750,7 @@ defmodule Ash.Filter.Runtime do {:ok, false} {:ok, nil} -> - {:ok, false} + {:ok, nil} {:ok, true} -> case do_match(record, right, parent) do @@ -752,7 +758,7 @@ defmodule Ash.Filter.Runtime do {:ok, false} {:ok, nil} -> - {:ok, false} + {:ok, nil} {:ok, _} -> {:ok, true} diff --git a/lib/ash/query/boolean_expression.ex b/lib/ash/query/boolean_expression.ex index 1a18628e..48c6b70e 100644 --- a/lib/ash/query/boolean_expression.ex +++ b/lib/ash/query/boolean_expression.ex @@ -8,8 +8,10 @@ defmodule Ash.Query.BooleanExpression do alias Ash.Query.Ref def new(_, nil, nil), do: nil - def new(_, left, nil), do: left - def new(_, nil, right), do: right + def new(:or, left, nil), do: left + def new(:or, nil, right), do: right + def new(:and, _, nil), do: nil + def new(:and, nil, _), do: nil def new(op, left, right) do %__MODULE__{op: op, left: left, right: right} @@ -27,8 +29,12 @@ defmodule Ash.Query.BooleanExpression do def optimized_new(:and, _, false), do: false def optimized_new(:or, true, _), do: true def optimized_new(:or, _, true), do: true - def optimized_new(_, nil, right), do: right - def optimized_new(_, left, nil), do: left + def optimized_new(:or, nil, right), do: right + def optimized_new(:or, left, nil), do: left + def optimized_new(:and, true, right), do: right + def optimized_new(:and, left, true), do: left + def optimized_new(:and, nil, _), do: nil + def optimized_new(:and, _, nil), do: nil def optimized_new( op, diff --git a/test/actions/read_test.exs b/test/actions/read_test.exs index 7fa77823..ba6c9cb0 100644 --- a/test/actions/read_test.exs +++ b/test/actions/read_test.exs @@ -29,16 +29,16 @@ defmodule Ash.Test.Actions.ReadTest do end actions do - defaults [:read, :create, :update, :destroy] + defaults([:read, :create, :update, :destroy]) end attributes do - uuid_primary_key :id - attribute :name, :string + uuid_primary_key(:id) + attribute(:name, :string) end relationships do - has_many :posts, Ash.Test.Actions.ReadTest.Post, destination_attribute: :author1_id + has_many(:posts, Ash.Test.Actions.ReadTest.Post, destination_attribute: :author1_id) end end @@ -47,39 +47,39 @@ defmodule Ash.Test.Actions.ReadTest do use Ash.Resource, data_layer: Ash.DataLayer.Ets identities do - identity :backup_id, [:uuid], pre_check_with: Api + identity(:backup_id, [:uuid], pre_check_with: Api) end ets do - private? true + private?(true) end actions do - defaults [:read, :create, :update, :destroy] + defaults([:read, :create, :update, :destroy]) read :read_with_after_action do - prepare PostPreparation + prepare(PostPreparation) end read :get_by_id do - get_by :id + get_by(:id) end read :get_by_id_and_uuid do - get_by [:id, :uuid] + get_by([:id, :uuid]) end end attributes do - uuid_primary_key :id - attribute :uuid, :uuid, default: &Ash.UUID.generate/0 - attribute :title, :string - attribute :contents, :string + uuid_primary_key(:id) + attribute(:uuid, :uuid, default: &Ash.UUID.generate/0) + attribute(:title, :string) + attribute(:contents, :string) end relationships do - belongs_to :author1, Ash.Test.Actions.ReadTest.Author - belongs_to :author2, Ash.Test.Actions.ReadTest.Author + belongs_to(:author1, Ash.Test.Actions.ReadTest.Author) + belongs_to(:author2, Ash.Test.Actions.ReadTest.Author) end end diff --git a/test/support/flow/flows/branching.ex b/test/support/flow/flows/branching.ex index 1751f3aa..7c5a45ab 100644 --- a/test/support/flow/flows/branching.ex +++ b/test/support/flow/flows/branching.ex @@ -29,7 +29,7 @@ defmodule Ash.Test.Flow.Flows.Branching do end end - branch :inner_branch_alt, expr(not (^arg(:do_inner_branch))) do + branch :inner_branch_alt, expr(not (^arg(:do_inner_branch) || false)) do custom :inner_branch_alt_return, Ash.Test.Flow.Steps.SimpleReturn do input %{return: "inner_branch didn't happen"} end diff --git a/test/support/flow/flows/halting.ex b/test/support/flow/flows/halting.ex index 627e91f2..b0e0e012 100644 --- a/test/support/flow/flows/halting.ex +++ b/test/support/flow/flows/halting.ex @@ -4,31 +4,31 @@ defmodule Ash.Test.Flow.Flows.Halting do flow do argument :on_step, :atom do - constraints one_of: [:a, :b, :c] + constraints(one_of: [:a, :b, :c]) end - returns :c + returns(:c) end steps do custom :a, Ash.Test.Flow.Steps.SimpleReturn do - input %{return: "a"} - halt_if expr(not (^arg(:on_step) == :a)) - halt_reason :not_on_step_a + input(%{return: "a"}) + halt_if(expr(not (^arg(:on_step) == :a || false))) + halt_reason(:not_on_step_a) end custom :b, Ash.Test.Flow.Steps.SimpleReturn do - input %{return: "b"} - halt_if expr(not (^arg(:on_step) == :b)) - wait_for result(:a) - halt_reason :not_on_step_b + input(%{return: "b"}) + halt_if(expr(not (^arg(:on_step) == :b || false))) + wait_for(result(:a)) + halt_reason(:not_on_step_b) end custom :c, Ash.Test.Flow.Steps.SimpleReturn do - input %{return: "c"} - halt_if expr(not (^arg(:on_step) == :c)) - wait_for result(:b) - halt_reason :not_on_step_c + input(%{return: "c"}) + halt_if(expr(not (^arg(:on_step) == :c || false))) + wait_for(result(:b)) + halt_reason(:not_on_step_c) end end end