From 2c2c207e68cff4a7c69afd13eb6b802b0e13ab21 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Thu, 15 Sep 2022 00:54:30 -0400 Subject: [PATCH] fix: use `Comp.equal?/2` when finding loaded data matches --- lib/ash/actions/load.ex | 98 +++++++++++++++++++----- lib/ash/actions/managed_relationships.ex | 2 +- test/actions/load_test.exs | 43 +++++++++++ 3 files changed, 124 insertions(+), 19 deletions(-) diff --git a/lib/ash/actions/load.ex b/lib/ash/actions/load.ex index 4e023e3f..785e428d 100644 --- a/lib/ash/actions/load.ex +++ b/lib/ash/actions/load.ex @@ -147,30 +147,57 @@ defmodule Ash.Actions.Load do defp attach_to_many_loads(value, last_relationship, data, lead_path) when is_map(value) do primary_key = Ash.Resource.Info.primary_key(last_relationship.source) + value = Map.to_list(value) map_or_update(data, lead_path, fn record -> case primary_key do [field] -> + related = + Enum.find(value, fn {key, _value} -> + Comp.equal?(key, Map.get(record, field)) || + Comp.equal?(key, Map.take(record, [field])) + end) + |> case do + nil -> + nil + + {_, value} -> + value + end + Map.put( record, last_relationship.name, - Map.get(value, Map.take(record, primary_key)) || - Map.get(value, Map.get(record, field)) || [] + related ) - _ -> - Map.put(record, last_relationship.name, Map.get(value, Map.take(record, primary_key))) || - [] + primary_key -> + related = + Enum.find(value, fn {key, _value} -> + Comp.equal?(key, Map.take(record, primary_key)) + end) + |> case do + nil -> + nil + + {_, value} -> + value + end + + Map.put(record, last_relationship.name, related) end end) end defp attach_to_many_loads(value, last_relationship, data, lead_path) do - values = Enum.group_by(value, &Map.get(&1, last_relationship.destination_attribute)) - map_or_update(data, lead_path, fn record -> source_key = Map.get(record, last_relationship.source_attribute) - related_records = Map.get(values, source_key, []) + + related_records = + Enum.filter(value, fn maybe_related -> + Comp.equal?(Map.get(maybe_related, last_relationship.destination_attribute), source_key) + end) + Map.put(record, last_relationship.name, related_records) end) end @@ -185,21 +212,54 @@ defmodule Ash.Actions.Load do primary_key = Ash.Resource.Info.primary_key(last_relationship.source) map_or_update(data, lead_path, fn record -> - Map.put(record, last_relationship.name, Map.get(value, Map.take(record, primary_key))) + case primary_key do + [field] -> + related = + Enum.find(value, fn {key, _value} -> + Comp.equal?(key, Map.get(record, field)) || + Comp.equal?(key, Map.take(record, [field])) + end) + |> case do + nil -> + nil + + {_, value} -> + value + end + + Map.put( + record, + last_relationship.name, + related + ) + + primary_key -> + related = + Enum.find(value, fn {key, _value} -> + Comp.equal?(key, Map.take(record, primary_key)) + end) + |> case do + nil -> + nil + + {_, value} -> + value + end + + Map.put(record, last_relationship.name, related) + end end) end defp attach_to_one_loads(value, last_relationship, data, lead_path) do - values = - value - |> Enum.reverse() - |> Enum.into(%{}, fn item -> - {Map.get(item, last_relationship.destination_attribute), item} - end) - map_or_update(data, lead_path, fn record -> source_key = Map.get(record, last_relationship.source_attribute) - related_record = Map.get(values, source_key) + + related_record = + Enum.find(value, fn maybe_related -> + Comp.equal?(Map.get(maybe_related, last_relationship.destination_attribute), source_key) + end) + Map.put(record, last_relationship.name, related_record) end) end @@ -218,8 +278,10 @@ defmodule Ash.Actions.Load do join_values = join_data |> Enum.filter(fn join_row -> - Map.get(join_row, last_relationship.source_attribute_on_join_resource) == + Comp.equal?( + Map.get(join_row, last_relationship.source_attribute_on_join_resource), source_value + ) end) |> Enum.map(&Map.get(&1, last_relationship.destination_attribute_on_join_resource)) diff --git a/lib/ash/actions/managed_relationships.ex b/lib/ash/actions/managed_relationships.ex index 9065037a..1a2892da 100644 --- a/lib/ash/actions/managed_relationships.ex +++ b/lib/ash/actions/managed_relationships.ex @@ -1394,7 +1394,7 @@ defmodule Ash.Actions.ManagedRelationships do defp do_matches?(current_value, input, field) do with {:ok, current_val} when not is_nil(current_val) <- Map.fetch(current_value, field), {:ok, input_val} when not is_nil(input_val) <- fetch_field(input, field) do - current_val == input_val + Comp.equal?(current_val, input_val) else _ -> false diff --git a/test/actions/load_test.exs b/test/actions/load_test.exs index 01d87a19..18a3b411 100644 --- a/test/actions/load_test.exs +++ b/test/actions/load_test.exs @@ -5,6 +5,25 @@ defmodule Ash.Test.Actions.LoadTest do import Ash.Changeset require Ash.Query + defmodule Campaign do + @moduledoc false + use Ash.Resource, + data_layer: Ash.DataLayer.Ets + + ets do + private?(true) + end + + actions do + defaults [:create, :read, :update, :destroy] + end + + attributes do + uuid_primary_key :id + attribute :name, :ci_string + end + end + defmodule Author do @moduledoc false use Ash.Resource, @@ -32,6 +51,13 @@ defmodule Ash.Test.Actions.LoadTest do has_one :latest_post, Ash.Test.Actions.LoadTest.Post, destination_attribute: :author_id, sort: [inserted_at: :desc] + + belongs_to :campaign, Ash.Test.Actions.LoadTest.Campaign do + attribute_type :ci_string + source_attribute :campaign_name + destination_attribute :name + attribute_writable? true + end end end @@ -176,6 +202,7 @@ defmodule Ash.Test.Actions.LoadTest do entry(Post) entry(Category) entry(PostCategory) + entry(Campaign) end end @@ -241,6 +268,22 @@ defmodule Ash.Test.Actions.LoadTest do |> Map.get(:posts_in_same_category) end + test "it uses `Comp.equal?/2` to support things like ci_string foreign keys" do + author = + Author + |> new(%{name: "zerg", campaign_name: "FrEd"}) + |> Api.create!() + + Campaign + |> new(%{name: "fReD"}) + |> Api.create!() + + assert %{ + campaign: %{name: %Ash.CiString{string: "fReD"}}, + campaign_name: %Ash.CiString{string: "FrEd"} + } = Api.load!(author, :campaign) + end + test "it allows loading related data" do author = Author