From 157ad30cf68f42fa4e5f97c7f74a3628044cb9a0 Mon Sep 17 00:00:00 2001 From: Leo Batyuk Date: Tue, 18 May 2021 12:53:01 +0200 Subject: [PATCH] Support associations on composite foreign keys --- Earthfile | 2 +- integration_test/cases/assoc.exs | 46 +++ integration_test/cases/preload.exs | 76 +++++ integration_test/cases/repo.exs | 18 ++ integration_test/support/schemas.exs | 6 + lib/ecto.ex | 13 +- lib/ecto/association.ex | 400 ++++++++++++++++++++------- lib/ecto/changeset.ex | 10 +- lib/ecto/repo/preloader.ex | 72 +++-- lib/ecto/repo/schema.ex | 29 +- lib/ecto/schema.ex | 16 +- lib/ecto/util.ex | 19 ++ test/ecto/query/planner_test.exs | 44 +++ test/ecto/repo/belongs_to_test.exs | 96 ++++++- test/ecto/repo/has_assoc_test.exs | 53 ++++ test/ecto/repo/many_to_many_test.exs | 119 ++++++++ test/ecto/schema_test.exs | 65 +++-- 17 files changed, 922 insertions(+), 162 deletions(-) create mode 100644 lib/ecto/util.ex diff --git a/Earthfile b/Earthfile index 45757abce2..04f146e98c 100644 --- a/Earthfile +++ b/Earthfile @@ -60,7 +60,7 @@ integration-test-base: apk del .build-dependencies && rm -f msodbcsql*.sig mssql-tools*.apk ENV PATH="/opt/mssql-tools/bin:${PATH}" - GIT CLONE https://github.com/elixir-ecto/ecto_sql.git /src/ecto_sql + GIT CLONE --branch composite_foreign_keys https://github.com/soundmonster/ecto_sql.git /src/ecto_sql WORKDIR /src/ecto_sql RUN mix deps.get diff --git a/integration_test/cases/assoc.exs b/integration_test/cases/assoc.exs index e2089f4f30..9bbbfd86a0 100644 --- a/integration_test/cases/assoc.exs +++ b/integration_test/cases/assoc.exs @@ -10,6 +10,7 @@ defmodule Ecto.Integration.AssocTest do alias Ecto.Integration.PostUser alias Ecto.Integration.Comment alias Ecto.Integration.Permalink + alias Ecto.Integration.CompositePk test "has_many assoc" do p1 = TestRepo.insert!(%Post{title: "1"}) @@ -55,6 +56,22 @@ defmodule Ecto.Integration.AssocTest do assert p2.id == pid2 end + test "belongs_to assoc with composite key" do + TestRepo.insert!(%CompositePk{a: 2, b: 1, name: "foo"}) + TestRepo.insert!(%CompositePk{a: 2, b: 2, name: "bar"}) + TestRepo.insert!(%CompositePk{a: 2, b: 3, name: "unused"}) + + p1 = TestRepo.insert!(%Post{title: "first", composite_a: 2, composite_b: 1}) + p2 = TestRepo.insert!(%Post{title: "none"}) + p3 = TestRepo.insert!(%Post{title: "second", composite_a: 2, composite_b: 2}) + + assert [c1, c2] = TestRepo.all Ecto.assoc([p1, p2, p3], :composite) + assert c1.a == 2 + assert c1.b == 1 + assert c2.a == 2 + assert c2.b == 2 + end + test "has_many through assoc" do p1 = TestRepo.insert!(%Post{}) p2 = TestRepo.insert!(%Post{}) @@ -725,6 +742,27 @@ defmodule Ecto.Integration.AssocTest do assert perma.post_id == nil end + test "belongs_to changeset assoc on composite key" do + changeset = + %CompositePk{a: 1, b: 2} + |> Ecto.Changeset.change() + |> Ecto.Changeset.put_assoc(:posts, [%Post{title: "1"}]) + + composite = TestRepo.insert!(changeset) + assert [post] = composite.posts + assert post.id + assert post.composite_a == composite.a + assert post.composite_b == composite.b + assert post.title == "1" + # TODO Repo.get should work with composite keys somehow, right? + # composite = TestRepo.get! from(Composite, preload: [:post]), [composite.a, composite.b] + # assert composite.post.title == "1" + + post = TestRepo.get! from(Post, preload: [:composite]), post.id + assert post.composite.a == 1 + assert post.composite.b == 2 + end + test "inserting struct with associations" do tree = %Permalink{ url: "root", @@ -750,6 +788,14 @@ defmodule Ecto.Integration.AssocTest do assert Enum.all?(tree.post.comments, & &1.id) end + test "inserting struct with associations on composite keys" do + # creates nested belongs_to + %Post{composite: composite} = + TestRepo.insert! %Post{title: "1", composite: %CompositePk{a: 1, b: 2, name: "name"}} + + assert %CompositePk{a: 1, b: 2, name: "name"} = composite + end + test "inserting struct with empty associations" do permalink = TestRepo.insert!(%Permalink{url: "root", post: nil}) assert permalink.post == nil diff --git a/integration_test/cases/preload.exs b/integration_test/cases/preload.exs index da81a9198e..fe9d3f9926 100644 --- a/integration_test/cases/preload.exs +++ b/integration_test/cases/preload.exs @@ -6,6 +6,7 @@ defmodule Ecto.Integration.PreloadTest do alias Ecto.Integration.Post alias Ecto.Integration.Comment + alias Ecto.Integration.CompositePk alias Ecto.Integration.Item alias Ecto.Integration.Permalink alias Ecto.Integration.User @@ -341,6 +342,25 @@ defmodule Ecto.Integration.PreloadTest do assert [] = pe3.comments end + test "preload composite foreign key with function" do + c11 = TestRepo.insert!(%CompositePk{a: 1, b: 1, name: "11"}) + c12 = TestRepo.insert!(%CompositePk{a: 1, b: 2, name: "12"}) + c22 = TestRepo.insert!(%CompositePk{a: 2, b: 2, name: "22"}) + c33 = TestRepo.insert!(%CompositePk{a: 3, b: 3, name: "33"}) + + TestRepo.insert!(%Post{title: "1", composite_a: 1, composite_b: 1}) + TestRepo.insert!(%Post{title: "2", composite_a: 1, composite_b: 1}) + TestRepo.insert!(%Post{title: "3", composite_a: 1, composite_b: 2}) + TestRepo.insert!(%Post{title: "4", composite_a: 2, composite_b: 2}) + + assert [ce12, ce11, ce33, ce22] = TestRepo.preload([c12, c11, c33, c22], + posts: fn _ -> TestRepo.all(Post) end) + assert [%Post{title: "1"}, %Post{title: "2"}] = ce11.posts + assert [%Post{title: "3"}] = ce12.posts + assert [%Post{title: "4"}] = ce22.posts + assert [] = ce33.posts + end + test "preload many_to_many with function" do p1 = TestRepo.insert!(%Post{title: "1"}) p2 = TestRepo.insert!(%Post{title: "2"}) @@ -389,6 +409,50 @@ defmodule Ecto.Integration.PreloadTest do assert p3.users == [%{id: uid1}, %{id: uid4}] end + test "preload many_to_many on composite foreign keys with function" do + c11 = TestRepo.insert!(%CompositePk{a: 1, b: 1, name: "11"}) + c12 = TestRepo.insert!(%CompositePk{a: 1, b: 2, name: "12"}) + c22 = TestRepo.insert!(%CompositePk{a: 2, b: 2, name: "22"}) + + TestRepo.insert_all "composite_pk_composite_pk", [[a_1: 1, b_1: 1, a_2: 1, b_2: 2], + [a_1: 1, b_1: 1, a_2: 2, b_2: 2], + [a_1: 1, b_1: 2, a_2: 1, b_2: 1], + [a_1: 2, b_1: 2, a_2: 2, b_2: 2]] + + wrong_preloader = fn composite_ids -> + composite_ids_a = Enum.map(composite_ids, &Enum.at(&1, 0)) + composite_ids_b = Enum.map(composite_ids, &Enum.at(&1, 1)) + TestRepo.all( + from c in CompositePk, + join: cc in "composite_pk_composite_pk", + where: cc.a_1 in ^composite_ids_a and cc.b_1 in ^composite_ids_b and cc.a_2 == c.a and cc.b_2 == c.b, + order_by: [c.a, c.b], + select: map(c, [:name]) + ) + end + + assert_raise RuntimeError, ~r/invalid custom preload for `composites` on `Ecto.Integration.CompositePk`/, fn -> + TestRepo.preload([c11, c12, c22], composites: wrong_preloader) + end + + right_preloader = fn composite_ids -> + composite_ids_a = Enum.map(composite_ids, &Enum.at(&1, 0)) + composite_ids_b = Enum.map(composite_ids, &Enum.at(&1, 1)) + TestRepo.all( + from c in CompositePk, + join: cc in "composite_pk_composite_pk", + where: cc.a_1 in ^composite_ids_a and cc.b_1 in ^composite_ids_b and cc.a_2 == c.a and cc.b_2 == c.b, + order_by: [c.a, c.b], + select: {[cc.a_1, cc.b_1], map(c, [:name])} + ) + end + + [c11, c12, c22] = TestRepo.preload([c11, c12, c22], composites: right_preloader) + assert c11.composites == [%{name: "12"}, %{name: "22"}] + assert c12.composites == [%{name: "11"}] + assert c22.composites == [%{name: "22"}] + end + test "preload with query" do p1 = TestRepo.insert!(%Post{title: "1"}) p2 = TestRepo.insert!(%Post{title: "2"}) @@ -604,6 +668,18 @@ defmodule Ecto.Integration.PreloadTest do assert TestRepo.preload(updated, [:author], force: true).author == nil end + test "preload raises with association over composite foreign key is set but without id" do + p1 = TestRepo.insert!(%Post{title: "1"}) + c11 = TestRepo.insert!(%CompositePk{a: 1, b: 1, name: "11"}) + updated = %{p1 | composite: c11, composite_a: nil, composite_b: nil} + + assert ExUnit.CaptureLog.capture_log(fn -> + assert TestRepo.preload(updated, [:composite]).composite == c11 + end) =~ ~r/its association keys `\[composite_a, composite_b\]` are nil/ + + assert TestRepo.preload(updated, [:composite], force: true).composite == nil + end + test "preload skips already loaded for cardinality one" do %Post{id: pid} = TestRepo.insert!(%Post{title: "1"}) diff --git a/integration_test/cases/repo.exs b/integration_test/cases/repo.exs index 005bbde5c5..cfb3f7085c 100644 --- a/integration_test/cases/repo.exs +++ b/integration_test/cases/repo.exs @@ -152,6 +152,24 @@ defmodule Ecto.Integration.RepoTest do assert TestRepo.all(PostUserCompositePk) == [] end + @tag :composite_pk + # TODO this needs a better name + test "insert, update and delete with associated composite pk #2" do + composite = TestRepo.insert!(%CompositePk{a: 1, b: 2, name: "name"}) + post = TestRepo.insert!(%Post{title: "post title", composite: composite}) + + assert post.composite_a == 1 + assert post.composite_b == 2 + assert TestRepo.get_by!(CompositePk, [a: 1, b: 2]) == composite + + post = post |> Ecto.Changeset.change(composite: nil) |> TestRepo.update! + assert is_nil(post.composite_a) + assert is_nil(post.composite_b) + + TestRepo.delete!(post) + assert TestRepo.all(CompositePk) == [composite] + end + @tag :invalid_prefix test "insert, update and delete with invalid prefix" do post = TestRepo.insert!(%Post{}) diff --git a/integration_test/support/schemas.exs b/integration_test/support/schemas.exs index b97d63a0f6..d6817c09ea 100644 --- a/integration_test/support/schemas.exs +++ b/integration_test/support/schemas.exs @@ -54,6 +54,8 @@ defmodule Ecto.Integration.Post do has_one :update_permalink, Ecto.Integration.Permalink, foreign_key: :post_id, on_delete: :delete_all, on_replace: :update has_many :comments_authors, through: [:comments, :author] belongs_to :author, Ecto.Integration.User + belongs_to :composite, Ecto.Integration.CompositePk, + foreign_key: [:composite_a, :composite_b], references: [:a, :b], type: [:integer, :integer], on_replace: :nilify many_to_many :users, Ecto.Integration.User, join_through: "posts_users", on_delete: :delete_all, on_replace: :delete many_to_many :ordered_users, Ecto.Integration.User, join_through: "posts_users", preload_order: [desc: :name] @@ -291,6 +293,10 @@ defmodule Ecto.Integration.CompositePk do field :a, :integer, primary_key: true field :b, :integer, primary_key: true field :name, :string + has_many :posts, Ecto.Integration.Post, foreign_key: [:composite_a, :composite_b], references: [:a, :b] + many_to_many :composites, Ecto.Integration.CompositePk, + join_through: "composite_pk_composite_pk", join_keys: [[a_1: :a, b_1: :b], [a_2: :a, b_2: :b]], + on_delete: :delete_all, on_replace: :delete end def changeset(schema, params) do cast(schema, params, ~w(a b name)a) diff --git a/lib/ecto.ex b/lib/ecto.ex index b16e2deceb..29112c2b4d 100644 --- a/lib/ecto.ex +++ b/lib/ecto.ex @@ -510,10 +510,15 @@ defmodule Ecto do refl = %{owner_key: owner_key} = Ecto.Association.association_from_schema!(schema, assoc) values = - Enum.uniq for(struct <- structs, - assert_struct!(schema, struct), - key = Map.fetch!(struct, owner_key), - do: key) + structs + |> Enum.filter(&assert_struct!(schema, &1)) + |> Enum.map(fn struct -> + owner_key + # TODO remove List.wrap once all assocs use lists + |> List.wrap + |> Enum.map(&Map.fetch!(struct, &1)) + end) + |> Enum.uniq case assocs do [] -> diff --git a/lib/ecto/association.ex b/lib/ecto/association.ex index 3169be3f9b..d2c7694aee 100644 --- a/lib/ecto/association.ex +++ b/lib/ecto/association.ex @@ -1,5 +1,5 @@ -import Ecto.Query, only: [from: 1, from: 2, join: 4, join: 5, distinct: 3, where: 3] - +import Ecto.Query, only: [from: 1, from: 2, join: 4, join: 5, distinct: 3, where: 3, dynamic: 2] +require Util # TODO delete defmodule Ecto.Association.NotLoaded do @moduledoc """ Struct returned by associations when they are not loaded. @@ -35,7 +35,7 @@ defmodule Ecto.Association do required(:cardinality) => :one | :many, required(:relationship) => :parent | :child, required(:owner) => atom, - required(:owner_key) => atom, + required(:owner_key) => list(atom), required(:field) => atom, required(:unique) => boolean, optional(atom) => any} @@ -71,7 +71,8 @@ defmodule Ecto.Association do * `:owner` - the owner module of the association - * `:owner_key` - the key in the owner with the association value + * `:owner_key` - the key in the owner with the association value, or a + list of keys for composite keys * `:relationship` - if the relationship to the specified schema is of a `:child` or a `:parent` @@ -235,8 +236,15 @@ defmodule Ecto.Association do # for the final WHERE clause with values. {_, query, _, dest_out_key} = Enum.reduce(joins, {source, query, counter, source.out_key}, fn curr_rel, {prev_rel, query, counter, _} -> related_queryable = curr_rel.schema - - next = join(query, :inner, [{src, counter}], dest in ^related_queryable, on: field(src, ^prev_rel.out_key) == field(dest, ^curr_rel.in_key)) + # TODO remove this once all relations store keys in lists + in_keys = List.wrap(curr_rel.in_key) + out_keys = List.wrap(prev_rel.out_key) + next = query + # join on the first field of the foreign key + |> join(:inner, [{src, counter}], dest in ^related_queryable, on: field(src, ^hd(out_keys)) == field(dest, ^hd(in_keys))) + # add the rest of the foreign key fields, if any + |> composite_joins_query(counter, counter + 1, tl(out_keys), tl(in_keys)) + # consider where clauses on assocs |> combine_joins_query(curr_rel.where, counter + 1) {curr_rel, next, counter + 1, curr_rel.out_key} @@ -247,12 +255,20 @@ defmodule Ecto.Association do values = List.wrap(values) query = case {join_to, values} do {nil, [single_value]} -> - query - |> where([{dest, final_bind}], field(dest, ^dest_out_key) == ^single_value) + dest_out_key + |> Enum.zip(single_value) + |> Enum.reduce(query, fn {dest_out_key_field, value}, query -> + query + |> where([{dest, final_bind}], field(dest, ^dest_out_key_field) == ^value) + end) {nil, values} -> - query - |> where([{dest, final_bind}], field(dest, ^dest_out_key) in ^values) + dest_out_key + |> Enum.zip(transpose_values(values)) + |> Enum.reduce(query, fn {dest_out_key_field, values}, query -> + query + |> where([{dest, final_bind}], field(dest, ^dest_out_key_field) in ^values) + end) {_, _} -> query @@ -261,6 +277,13 @@ defmodule Ecto.Association do combine_assoc_query(query, source.where || []) end + def transpose_values(values) do + values + |> List.wrap() + |> Enum.zip() + |> Enum.map(&Tuple.to_list/1) + end + defp flatten_through_chain(owner, [], acc), do: {owner, acc} defp flatten_through_chain(owner, [assoc | tl], acc) do refl = association_from_schema!(owner, assoc) @@ -290,11 +313,18 @@ defmodule Ecto.Association do table_list = case refl do %{join_through: join_through, join_keys: join_keys, join_where: join_where, where: where} -> - [{owner_join_key, owner_key}, {related_join_key, related_key}] = join_keys - - owner_map = %{owner_map | in_key: owner_key} - join_map = %{schema: join_through, out_key: owner_join_key, in_key: related_join_key, where: join_where} - related_map = %{schema: refl.related, out_key: related_key, in_key: nil, where: where} + # [[{owner_join_key, owner_key}], [{related_join_key, related_key}]] = join_keys + # TODO does this support more than one key on many to many? + %{ + owner_keys: owner_keys, + owner_join_keys: owner_join_keys, + related_keys: related_keys, + related_join_keys: related_join_keys + } = resolve_join_keys(join_keys) + + owner_map = %{owner_map | in_key: owner_keys} + join_map = %{schema: join_through, out_key: owner_join_keys, in_key: related_join_keys, where: join_where} + related_map = %{schema: refl.related, out_key: related_keys, in_key: nil, where: where} [related_map, join_map, owner_map | table_list] @@ -320,6 +350,25 @@ defmodule Ecto.Association do end) end + defp resolve_join_keys([owner_join_keys, related_join_keys]) do + owner_keys = Keyword.values(owner_join_keys) + owner_join_keys = Keyword.keys(owner_join_keys) + + related_keys = Keyword.values(related_join_keys) + related_join_keys = Keyword.keys(related_join_keys) + %{owner_keys: owner_keys, owner_join_keys: owner_join_keys, related_keys: related_keys, related_join_keys: related_join_keys} + end + + # TODO docs + def composite_joins_query(query, _binding_src, _binding_dst, [], []) do + query + end + def composite_joins_query(query, binding_src, binding_dst, [src_key | src_keys], [dst_key | dst_keys]) do + # TODO + [query, binding_src, binding_dst, [src_key | src_keys], [dst_key | dst_keys]] |> Util.inspect() + query + end + @doc """ Add the default assoc query where clauses to a join. @@ -335,6 +384,39 @@ defmodule Ecto.Association do %{query | joins: joins ++ [%{join_expr | on: %{join_on | expr: expr, params: params}}]} end + # TODO docs + def composite_assoc_query(query, _binding_src, [], _values) do + query + end + + def composite_assoc_query(query, binding_dst, [dst_key | dst_keys], [[single_value | values_rest]]) do + query + |> where([binding_dst], field(binding_dst, ^dst_key) == ^single_value) + |> composite_assoc_query(binding_dst, dst_keys, values_rest) + end + + def composite_assoc_query(query, binding_dst, [dst_key | dst_keys], [[_|_] | _] = values) do + # is there a better way of matching on a nonempty list of lists? + values_for_this_key = Enum.map(values, &hd/1) + values_rest = Enum.map(values, &tl/1) + + query + |> where([binding_dst], field(binding_dst, ^dst_key) in ^values_for_this_key) + |> composite_assoc_query(binding_dst, dst_keys, values_rest) + end + + # TODO why do we need this extra case? + def composite_assoc_query(query, binding_dst, [dst_key | dst_keys], [single_value | values_rest]) do + query + |> where([binding_dst], field(binding_dst, ^dst_key) == ^single_value) + |> composite_assoc_query(binding_dst, dst_keys, values_rest) + end + + def composite_assoc_query(query, binding_src, dst_keys, values) do + # TODO remove before merging as this case should not happen + raise FunctionClauseError + end + @doc """ Add the default assoc query where clauses a provided query. """ @@ -632,6 +714,15 @@ defmodule Ecto.Association do defp primary_key!(nil), do: [] defp primary_key!(struct), do: Ecto.primary_key!(struct) + + def missing_fields(queryable, related_key) do + Enum.filter related_key, &is_nil(queryable.__schema__(:type, &1)) + end + + def label(env) do + {fun, arity} = env.function + "#{env.module}.#{fun}/#{arity} #{env.line}" + end end defmodule Ecto.Association.Has do @@ -644,8 +735,8 @@ defmodule Ecto.Association.Has do * `field` - The name of the association field on the schema * `owner` - The schema where the association was defined * `related` - The schema that is associated - * `owner_key` - The key on the `owner` schema used for the association - * `related_key` - The key on the `related` schema used for the association + * `owner_key` - The list of columns that form the key on the `owner` schema used for the association + * `related_key` - The list of columns that form the key on the `related` schema used for the association * `queryable` - The real query to use for querying association * `on_delete` - The action taken on associations when schema is deleted * `on_replace` - The action taken on associations when schema is replaced @@ -673,8 +764,8 @@ defmodule Ecto.Association.Has do {:error, "associated schema #{inspect queryable} does not exist"} not function_exported?(queryable, :__schema__, 2) -> {:error, "associated module #{inspect queryable} is not an Ecto schema"} - is_nil queryable.__schema__(:type, related_key) -> - {:error, "associated schema #{inspect queryable} does not have field `#{related_key}`"} + [] != (missing_fields = Ecto.Association.missing_fields(queryable, related_key)) -> + {:error, "associated schema #{inspect queryable} does not have field(s) `#{inspect missing_fields}`"} true -> :ok end @@ -686,14 +777,17 @@ defmodule Ecto.Association.Has do cardinality = Keyword.fetch!(opts, :cardinality) related = Ecto.Association.related_from_query(queryable, name) - ref = + refs = module |> Module.get_attribute(:primary_key) |> get_ref(opts[:references], name) + |> List.wrap() - unless Module.get_attribute(module, :ecto_fields)[ref] do - raise ArgumentError, "schema does not have the field #{inspect ref} used by " <> - "association #{inspect name}, please set the :references option accordingly" + for ref <- refs do + unless Module.get_attribute(module, :ecto_fields)[ref] do + raise ArgumentError, "schema does not have the field #{inspect ref} used by " <> + "association #{inspect name}, please set the :references option accordingly" + end end if opts[:through] do @@ -725,13 +819,19 @@ defmodule Ecto.Association.Has do raise ArgumentError, "expected `:where` for #{inspect name} to be a keyword list, got: `#{inspect where}`" end + foreign_key = case opts[:foreign_key] do + nil -> Enum.map(refs, &Ecto.Association.association_key(module, &1)) + key when is_atom(key) -> [key] + keys when is_list(keys) -> keys + end + %__MODULE__{ field: name, cardinality: cardinality, owner: module, related: related, - owner_key: ref, - related_key: opts[:foreign_key] || Ecto.Association.association_key(module, ref), + owner_key: refs, + related_key: foreign_key, queryable: queryable, on_delete: on_delete, on_replace: on_replace, @@ -756,19 +856,23 @@ defmodule Ecto.Association.Has do @impl true def joins_query(%{related_key: related_key, owner: owner, owner_key: owner_key, queryable: queryable} = assoc) do - from(o in owner, join: q in ^queryable, on: field(q, ^related_key) == field(o, ^owner_key)) + # TODO find out how to handle a dynamic list of fields here + from(o in owner, join: q in ^queryable, on: field(q, ^hd(related_key)) == field(o, ^hd(owner_key))) + |> Ecto.Association.composite_joins_query(0, 1, tl(related_key), tl(owner_key)) |> Ecto.Association.combine_joins_query(assoc.where, 1) end @impl true def assoc_query(%{related_key: related_key, queryable: queryable} = assoc, query, [value]) do - from(x in (query || queryable), where: field(x, ^related_key) == ^value) + from(x in (query || queryable), where: field(x, ^hd(related_key)) == ^hd(value)) + |> Ecto.Association.composite_assoc_query(0, tl(related_key), tl(value)) |> Ecto.Association.combine_assoc_query(assoc.where) end @impl true def assoc_query(%{related_key: related_key, queryable: queryable} = assoc, query, values) do - from(x in (query || queryable), where: field(x, ^related_key) in ^values) + from(x in (query || queryable), where: field(x, ^hd(related_key)) in ^Enum.map(values, &hd/1)) + |> Ecto.Association.composite_assoc_query(0, tl(related_key), Enum.map(values, &tl/1)) |> Ecto.Association.combine_assoc_query(assoc.where) end @@ -807,16 +911,21 @@ defmodule Ecto.Association.Has do %{data: parent, repo: repo} = parent_changeset %{action: action, changes: changes} = changeset - {key, value} = parent_key(assoc, parent) - changeset = update_parent_key(changeset, action, key, value) - changeset = Ecto.Association.update_parent_prefix(changeset, parent) + parent_keys = parent_keys(assoc, parent) + changeset = Enum.reduce parent_keys, changeset, fn {key, value}, changeset -> + changeset = update_parent_key(changeset, action, key, value) + Ecto.Association.update_parent_prefix(changeset, parent) + end case apply(repo, action, [changeset, opts]) do {:ok, _} = ok -> if action == :delete, do: {:ok, nil}, else: ok {:error, changeset} -> - original = Map.get(changes, key) - {:error, put_in(changeset.changes[key], original)} + changeset = Enum.reduce parent_keys, changeset, fn {key, _}, changeset -> + original = Map.get(changes, key) + put_in(changeset.changes[key], original) + end + {:error, changeset} end end @@ -825,11 +934,21 @@ defmodule Ecto.Association.Has do defp update_parent_key(changeset, _action, key, value), do: Ecto.Changeset.put_change(changeset, key, value) - defp parent_key(%{related_key: related_key}, nil) do - {related_key, nil} + defp parent_keys(%{related_key: related_keys}, nil) when is_list(related_keys) do + Enum.map related_keys, fn related_key -> {related_key, nil} end + end + defp parent_keys(%{related_key: related_key}, nil) do + [{related_key, nil}] + end + defp parent_keys(%{owner_key: owner_keys, related_key: related_keys}, owner) when is_list(owner_keys) and is_list(related_keys) do + owner_keys + |> Enum.zip(related_keys) + |> Enum.map(fn {owner_key, related_key} -> + {related_key, Map.get(owner, owner_key)} + end) end - defp parent_key(%{owner_key: owner_key, related_key: related_key}, owner) do - {related_key, Map.get(owner, owner_key)} + defp parent_keys(%{owner_key: owner_key, related_key: related_key}, owner) do + [{related_key, Map.get(owner, owner_key)}] end ## Relation callbacks @@ -852,18 +971,36 @@ defmodule Ecto.Association.Has do end @doc false - def nilify_all(%{related_key: related_key} = refl, parent, repo_name, opts) do + def nilify_all(%{related_key: [related_key]} = refl, parent, repo_name, opts) do if query = on_delete_query(refl, parent) do Ecto.Repo.Queryable.update_all repo_name, query, [set: [{related_key, nil}]], opts end end + def nilify_all(%{related_key: related_keys} = refl, parent, repo_name, opts) do + if query = on_delete_query(refl, parent) do + set = Enum.map(related_keys, fn related_key -> {related_key, nil} end) + Ecto.Repo.Queryable.update_all repo_name, query, [set: set], opts + end + end - defp on_delete_query(%{owner_key: owner_key, related_key: related_key, + defp on_delete_query(%{owner_key: [owner_key], related_key: [related_key], queryable: queryable}, parent) do if value = Map.get(parent, owner_key) do from x in queryable, where: field(x, ^related_key) == ^value end end + + defp on_delete_query(%{owner_key: owner_key, related_key: related_key, + queryable: queryable}, parent) do + values = Enum.map(owner_key, &Map.get(parent, &1)) + if values != [] && Enum.all?(values) do + related_key + |> Enum.zip(values) + |> Enum.reduce(from(x in queryable), fn {related_key_field, value}, query -> + query |> where([x], field(x, ^related_key_field) == ^ value) + end) + end + end end defmodule Ecto.Association.HasThrough do @@ -875,7 +1012,7 @@ defmodule Ecto.Association.HasThrough do * `cardinality` - The association cardinality * `field` - The name of the association field on the schema * `owner` - The schema where the association was defined - * `owner_key` - The key on the `owner` schema used for the association + * `owner_key` - The list of columns that form the key on the `owner` schema used for the association * `through` - The through associations * `relationship` - The relationship to the specified schema, default `:child` """ @@ -942,7 +1079,7 @@ defmodule Ecto.Association.HasThrough do end @impl true - def assoc_query(%{owner: owner, through: through}, _, values) do + def assoc_query(%{owner: owner, through: through}, _query, values) do Ecto.Association.filter_through_chain(owner, through, values) end end @@ -982,8 +1119,8 @@ defmodule Ecto.Association.BelongsTo do {:error, "associated schema #{inspect queryable} does not exist"} not function_exported?(queryable, :__schema__, 2) -> {:error, "associated module #{inspect queryable} is not an Ecto schema"} - is_nil queryable.__schema__(:type, related_key) -> - {:error, "associated schema #{inspect queryable} does not have field `#{related_key}`"} + [] != (missing_fields = Ecto.Association.missing_fields(queryable, related_key)) -> + {:error, "associated schema #{inspect queryable} does not have field(s) `#{inspect missing_fields}`"} true -> :ok end @@ -991,7 +1128,7 @@ defmodule Ecto.Association.BelongsTo do @impl true def struct(module, name, opts) do - ref = if ref = opts[:references], do: ref, else: :id + refs = if ref = opts[:references], do: List.wrap(ref), else: [:id] queryable = Keyword.fetch!(opts, :queryable) related = Ecto.Association.related_from_query(queryable, name) on_replace = Keyword.get(opts, :on_replace, :raise) @@ -1013,8 +1150,8 @@ defmodule Ecto.Association.BelongsTo do field: name, owner: module, related: related, - owner_key: Keyword.fetch!(opts, :foreign_key), - related_key: ref, + owner_key: List.wrap(Keyword.fetch!(opts, :foreign_key)), + related_key: refs, queryable: queryable, on_replace: on_replace, defaults: defaults, @@ -1031,19 +1168,22 @@ defmodule Ecto.Association.BelongsTo do @impl true def joins_query(%{related_key: related_key, owner: owner, owner_key: owner_key, queryable: queryable} = assoc) do - from(o in owner, join: q in ^queryable, on: field(q, ^related_key) == field(o, ^owner_key)) + from(o in owner, join: q in ^queryable, on: field(q, ^hd(related_key)) == field(o, ^hd(owner_key))) + |> Ecto.Association.composite_joins_query(0, 1, tl(related_key), tl(owner_key)) |> Ecto.Association.combine_joins_query(assoc.where, 1) end @impl true def assoc_query(%{related_key: related_key, queryable: queryable} = assoc, query, [value]) do - from(x in (query || queryable), where: field(x, ^related_key) == ^value) + from(x in (query || queryable), where: field(x, ^hd(related_key)) == ^hd(value)) + |> Ecto.Association.composite_assoc_query(0, tl(related_key), tl(value)) |> Ecto.Association.combine_assoc_query(assoc.where) end @impl true def assoc_query(%{related_key: related_key, queryable: queryable} = assoc, query, values) do - from(x in (query || queryable), where: field(x, ^related_key) in ^values) + from(x in (query || queryable), where: field(x, ^hd(related_key)) in ^Enum.map(values, &hd/1)) + |> Ecto.Association.composite_assoc_query(0, tl(related_key), Enum.map(values, &tl/1)) |> Ecto.Association.combine_assoc_query(assoc.where) end @@ -1109,7 +1249,7 @@ defmodule Ecto.Association.ManyToMany do * `field` - The name of the association field on the schema * `owner` - The schema where the association was defined * `related` - The schema that is associated - * `owner_key` - The key on the `owner` schema used for the association + * `owner_key` - The list of fields that form the key on the `owner` schema used for the association * `queryable` - The real query to use for querying association * `on_delete` - The action taken on associations when schema is deleted * `on_replace` - The action taken on associations when schema is replaced @@ -1159,17 +1299,22 @@ defmodule Ecto.Association.ManyToMany do related = Ecto.Association.related_from_query(queryable, name) join_keys = opts[:join_keys] + validate_join_keys!(name, join_keys) join_through = opts[:join_through] - validate_join_through(name, join_through) + validate_join_through!(name, join_through) {owner_key, join_keys} = case join_keys do [{join_owner_key, owner_key}, {join_related_key, related_key}] when is_atom(join_owner_key) and is_atom(owner_key) and is_atom(join_related_key) and is_atom(related_key) -> - {owner_key, join_keys} + join_keys = [[{join_owner_key, owner_key}], [{join_related_key, related_key}]] + {[owner_key], join_keys} + [join_keys_to_through, join_keys_to_related] + when is_list(join_keys_to_through) and is_list(join_keys_to_related) -> + {Keyword.values(join_keys_to_through), join_keys} nil -> - {:id, default_join_keys(module, related)} + {[:id], default_join_keys(module, related)} _ -> raise ArgumentError, "many_to_many #{inspect name} expect :join_keys to be a keyword list " <> @@ -1178,10 +1323,12 @@ defmodule Ecto.Association.ManyToMany do "the associated schema. For example: #{inspect default_join_keys(module, related)}" end - unless Module.get_attribute(module, :ecto_fields)[owner_key] do - raise ArgumentError, "schema does not have the field #{inspect owner_key} used by " <> - "association #{inspect name}, please set the :join_keys option accordingly" - end + Enum.each(owner_key, fn owner_key_field -> + unless Module.get_attribute(module, :ecto_fields)[owner_key_field] do + raise ArgumentError, "schema does not have the field #{inspect owner_key_field} used by " <> + "association #{inspect name}, please set the :join_keys option accordingly" + end + end) on_delete = Keyword.get(opts, :on_delete, :nothing) on_replace = Keyword.get(opts, :on_replace, :raise) @@ -1237,18 +1384,29 @@ defmodule Ecto.Association.ManyToMany do end defp default_join_keys(module, related) do - [{Ecto.Association.association_key(module, :id), :id}, - {Ecto.Association.association_key(related, :id), :id}] + [ + [{Ecto.Association.association_key(module, :id), :id}], + [{Ecto.Association.association_key(related, :id), :id}] + ] + end + + defp on_fields([{dst_key, src_key}] = _fields) do + dynamic([..., dst, src], field(dst, ^dst_key) == field(src, ^src_key)) + end + + defp on_fields([{dst_key, src_key} | fields]) do + dynamic([..., dst, src], field(dst, ^dst_key) == field(src, ^src_key) and ^on_fields(fields)) end @impl true def joins_query(%{owner: owner, queryable: queryable, join_through: join_through, join_keys: join_keys} = assoc) do - [{join_owner_key, owner_key}, {join_related_key, related_key}] = join_keys + [join_through_keys, join_related_keys] = join_keys + join_through_keys = Enum.map(join_through_keys, fn {from, to} -> {to, from} end) from(o in owner, - join: j in ^join_through, on: field(j, ^join_owner_key) == field(o, ^owner_key), - join: q in ^queryable, on: field(j, ^join_related_key) == field(q, ^related_key)) + join: j in ^join_through, on: ^on_fields(join_through_keys), + join: q in ^queryable, on: ^on_fields(join_related_keys)) |> Ecto.Association.combine_joins_query(assoc.where, 2) |> Ecto.Association.combine_joins_query(assoc.join_where, 1) end @@ -1259,17 +1417,25 @@ defmodule Ecto.Association.ManyToMany do @impl true def assoc_query(assoc, query, values) do + values = Ecto.Association.transpose_values(values) %{queryable: queryable, join_through: join_through, join_keys: join_keys, owner: owner} = assoc - [{join_owner_key, owner_key}, {join_related_key, related_key}] = join_keys - - owner_key_type = owner.__schema__(:type, owner_key) + # TODO does this support composite keys? + [[{_join_owner_key, _owner_key}] = join_through_keys, join_related_keys] = join_keys + join_related_keys = Enum.map(join_related_keys, fn {from, to} -> {to, from} end) # We only need to join in the "join table". Preload and Ecto.assoc expressions can then filter # by &1.join_owner_key in ^... to filter down to the associated entries in the related table. - from(q in (query || queryable), - join: j in ^join_through, on: field(q, ^related_key) == field(j, ^join_related_key), - where: field(j, ^join_owner_key) in type(^values, {:in, ^owner_key_type}) - ) + query = + from(q in (query || queryable), + join: j in ^join_through, on: ^on_fields(join_related_keys) + ) + + values + |> Enum.zip(join_through_keys) + |> Enum.reduce(query, fn {col_values, {join_owner_key_col, owner_key_col}}, query -> + owner_key_type = owner.__schema__(:type, owner_key_col) + where(query, [_, j], field(j, ^join_owner_key_col) in type(^col_values, {:in, ^owner_key_type})) + end) |> Ecto.Association.combine_assoc_query(assoc.where) |> Ecto.Association.combine_joins_query(assoc.join_where, 1) end @@ -1282,12 +1448,14 @@ defmodule Ecto.Association.ManyToMany do end @impl true - def preload_info(%{join_keys: [{join_owner_key, owner_key}, {_, _}], owner: owner} = refl) do - owner_key_type = owner.__schema__(:type, owner_key) + def preload_info(%{join_keys: [join_through_keys, _], owner: owner} = refl) do + join_owner_keys = Keyword.keys(join_through_keys) + owner_key_types = join_through_keys |> Keyword.values |> Enum.map(& owner.__schema__(:type, &1)) + # owner_key_type = owner.__schema__(:type, owner_key) # When preloading use the last bound table (which is the join table) and the join_owner_key # to filter out related entities to the owner structs we're preloading with. - {:assoc, refl, {-1, join_owner_key, owner_key_type}} + {:assoc, refl, {-1, join_owner_keys, owner_key_types}} end @impl true @@ -1298,14 +1466,19 @@ defmodule Ecto.Association.ManyToMany do def on_repo_change(%{join_keys: join_keys, join_through: join_through}, %{repo: repo, data: owner}, %{action: :delete, data: related}, adapter, opts) do - [{join_owner_key, owner_key}, {join_related_key, related_key}] = join_keys - owner_value = dump! :delete, join_through, owner, owner_key, adapter - related_value = dump! :delete, join_through, related, related_key, adapter + [join_through_keys, join_related_keys] = join_keys + owner_values = dump! :delete, join_through, owner, Keyword.values(join_through_keys), adapter + related_values = dump! :delete, join_through, related, Keyword.values(join_related_keys), adapter - query = - from j in join_through, - where: field(j, ^join_owner_key) == ^owner_value and - field(j, ^join_related_key) == ^related_value + + join_fields = Keyword.keys(join_through_keys) ++ Keyword.keys(join_related_keys) + join_values = owner_values ++ related_values + + query = join_fields + |> Enum.zip(join_values) + |> Enum.reduce(from(j in join_through), fn {field, value}, query -> + where(query, [j], field(j, ^field) == ^value) + end) query = %{query | prefix: owner.__meta__.prefix} repo.delete_all(query, opts) @@ -1319,12 +1492,18 @@ defmodule Ecto.Association.ManyToMany do case apply(repo, action, [changeset, opts]) do {:ok, related} -> - [{join_owner_key, owner_key}, {join_related_key, related_key}] = join_keys - - if insert_join?(parent_changeset, changeset, field, related_key) do - owner_value = dump! :insert, join_through, owner, owner_key, adapter - related_value = dump! :insert, join_through, related, related_key, adapter - data = %{join_owner_key => owner_value, join_related_key => related_value} + [join_through_keys, join_related_keys] = join_keys + owner_keys = Keyword.values(join_through_keys) + related_keys = Keyword.values(join_related_keys) + # [[{join_owner_key, owner_key}], [{join_related_key, related_key}]] = join_keys + + if insert_join?(parent_changeset, changeset, field, related_keys) do + owner_values = dump! :insert, join_through, owner, owner_keys, adapter + related_values = dump! :insert, join_through, related, related_keys, adapter + data = + (Keyword.keys(join_through_keys) ++ Keyword.keys(join_related_keys)) + |> Enum.zip(owner_values ++ related_values) + |> Map.new case insert_join(join_through, refl, parent_changeset, data, opts) do {:error, join_changeset} -> @@ -1342,24 +1521,30 @@ defmodule Ecto.Association.ManyToMany do end end - defp validate_join_through(name, nil) do + defp validate_join_keys!(_name, _join_keys) do + # TODO + :ok + end + defp validate_join_through!(name, nil) do raise ArgumentError, "many_to_many #{inspect name} associations require the :join_through option to be given" end - defp validate_join_through(_, join_through) when is_atom(join_through) or is_binary(join_through) do + defp validate_join_through!(_, join_through) when is_atom(join_through) or is_binary(join_through) do :ok end - defp validate_join_through(name, _join_through) do + defp validate_join_through!(name, _join_through) do raise ArgumentError, "many_to_many #{inspect name} associations require the :join_through option to be " <> "an atom (representing a schema) or a string (representing a table)" end - defp insert_join?(%{action: :insert}, _, _field, _related_key), do: true - defp insert_join?(_, %{action: :insert}, _field, _related_key), do: true - defp insert_join?(%{data: owner}, %{data: related}, field, related_key) do - current_key = Map.fetch!(related, related_key) - not Enum.any? Map.fetch!(owner, field), fn child -> - Map.get(child, related_key) == current_key + defp insert_join?(%{action: :insert}, _, _field, _related_keys), do: true + defp insert_join?(_, %{action: :insert}, _field, _related_keys), do: true + defp insert_join?(%{data: owner}, %{data: related}, field, related_keys) do + Enum.all? related_keys, fn related_key -> + current_key = Map.fetch!(related, related_key) + not Enum.any? Map.fetch!(owner, field), fn child -> + Map.get(child, related_key) == current_key + end end end @@ -1384,6 +1569,11 @@ defmodule Ecto.Association.ManyToMany do Map.get(struct, field) || raise "could not #{op} join entry because `#{field}` is nil in #{inspect struct}" end + defp dump!(action, join_through, struct, fields, adapter) when is_list(fields) do + Enum.map(fields, &dump!(action, join_through, struct, &1, adapter)) + end + + # TODO optimize away single field dump! defp dump!(action, join_through, struct, field, adapter) when is_binary(join_through) do value = field!(action, struct, field) type = struct.__struct__.__schema__(:type, field) @@ -1417,12 +1607,26 @@ defmodule Ecto.Association.ManyToMany do @doc false def delete_all(refl, parent, repo_name, opts) do %{join_through: join_through, join_keys: join_keys, owner: owner} = refl - [{join_owner_key, owner_key}, {_, _}] = join_keys + [join_through_keys, _join_related_keys] = join_keys + values = join_through_keys |> Keyword.values() |> Enum.map(&Map.get(parent, &1)) - if value = Map.get(parent, owner_key) do - owner_type = owner.__schema__(:type, owner_key) - query = from j in join_through, where: field(j, ^join_owner_key) == type(^value, ^owner_type) + + unless Enum.all?(values, &is_nil/1) do + query = from j in join_through, where: ^where_fields(owner, join_through_keys, values) Ecto.Repo.Queryable.delete_all repo_name, query, opts end end + + defp where_fields(owner, [{join_owner_key, owner_key}] = _fields, [value]) do + owner_type = owner.__schema__(:type, owner_key) + dynamic([..., join_through], field(join_through, ^join_owner_key) == type(^value, ^owner_type)) + end + + defp where_fields(owner, [{join_owner_key, owner_key} | fields], [value | values]) do + owner_type = owner.__schema__(:type, owner_key) + dynamic( + [..., join_through], + field(join_through, ^join_owner_key) == type(^value, ^owner_type) and ^where_fields(owner, fields, values) + ) + end end diff --git a/lib/ecto/changeset.ex b/lib/ecto/changeset.ex index fcf32336c8..dc9e7a905c 100644 --- a/lib/ecto/changeset.ex +++ b/lib/ecto/changeset.ex @@ -2794,7 +2794,7 @@ defmodule Ecto.Changeset do constraint = opts[:name] || case get_assoc(changeset, assoc) do %Ecto.Association.BelongsTo{owner_key: owner_key} -> - "#{get_source(changeset)}_#{owner_key}_fkey" + "#{get_source(changeset)}_#{atom_concat owner_key}_fkey" other -> raise ArgumentError, "assoc_constraint can only be added to belongs to associations, got: #{inspect other}" @@ -2845,7 +2845,7 @@ defmodule Ecto.Changeset do case get_assoc(changeset, assoc) do %Ecto.Association.Has{cardinality: cardinality, related_key: related_key, related: related} -> - {opts[:name] || "#{related.__schema__(:source)}_#{related_key}_fkey", + {opts[:name] || "#{related.__schema__(:source)}_#{atom_concat related_key}_fkey", message(opts, no_assoc_message(cardinality))} other -> raise ArgumentError, @@ -3062,6 +3062,12 @@ defmodule Ecto.Changeset do |> merge_keyword_keys(msg_func, changeset) |> merge_related_keys(changes, types, msg_func, &traverse_validations/2) end + + defp atom_concat(atoms) do + atoms + |> Enum.map(&to_string/1) + |> Enum.join("_") + end end defimpl Inspect, for: Ecto.Changeset do diff --git a/lib/ecto/repo/preloader.ex b/lib/ecto/repo/preloader.ex index b9713ae39f..aa3b24e8fe 100644 --- a/lib/ecto/repo/preloader.ex +++ b/lib/ecto/repo/preloader.ex @@ -170,15 +170,24 @@ defmodule Ecto.Repo.Preloader do acc struct, {fetch_ids, loaded_ids, loaded_structs} -> assert_struct!(module, struct) - %{^owner_key => id, ^field => value} = struct + %{^field => value} = struct + # TODO remove List.wrap + ids = owner_key |> List.wrap |> Enum.map(fn key -> + %{^key => id} = struct + id + end) loaded? = Ecto.assoc_loaded?(value) and not force? - if loaded? and is_nil(id) and not Ecto.Changeset.Relation.empty?(assoc, value) do + if loaded? and Enum.any?(ids, &is_nil/1) and not Ecto.Changeset.Relation.empty?(assoc, value) do + {key_noun, key_verb_present, key_verb_past, key_list} = case owner_key do + [single_key] -> {"key", "is", "was", single_key} + many_keys -> {"keys", "are", "were", "[#{Enum.join(many_keys, ", ")}]"} + end Logger.warn """ association `#{field}` for `#{inspect(module)}` has a loaded value but \ - its association key `#{owner_key}` is nil. This usually means one of: + its association #{key_noun} `#{key_list}` #{key_verb_present} nil. This usually means one of: - * `#{owner_key}` was not selected in a query + * `#{key_list}` #{key_verb_past} not selected in a query * the struct was set with default values for `#{field}` which now you want to override If this is intentional, set force: true to disable this warning @@ -187,13 +196,13 @@ defmodule Ecto.Repo.Preloader do cond do card == :one and loaded? -> - {fetch_ids, [id | loaded_ids], [value | loaded_structs]} + {fetch_ids, [ids | loaded_ids], [value | loaded_structs]} card == :many and loaded? -> - {fetch_ids, [{id, length(value)} | loaded_ids], value ++ loaded_structs} - is_nil(id) -> + {fetch_ids, [{ids, length(value)} | loaded_ids], value ++ loaded_structs} + Enum.any? ids, &is_nil/1 -> {fetch_ids, loaded_ids, loaded_structs} true -> - {[id | fetch_ids], loaded_ids, loaded_structs} + {[ids | fetch_ids], loaded_ids, loaded_structs} end end end @@ -211,20 +220,20 @@ defmodule Ecto.Repo.Preloader do defp fetch_query(ids, %{cardinality: card} = assoc, repo_name, query, prefix, related_key, take, opts) do query = assoc.__struct__.assoc_query(assoc, query, Enum.uniq(ids)) - field = related_key_to_field(query, related_key) + fields = related_key_to_fields(query, related_key) # Normalize query query = %{Ecto.Query.Planner.ensure_select(query, take || true) | prefix: prefix} # Add the related key to the query results - query = update_in query.select.expr, &{:{}, [], [field, &1]} + query = update_in query.select.expr, &{:{}, [], [fields, &1]} # If we are returning many results, we must sort by the key too query = case card do :many -> update_in query.order_bys, fn order_bys -> - [%Ecto.Query.QueryExpr{expr: preload_order(assoc, query, field), params: [], + [%Ecto.Query.QueryExpr{expr: preload_order(assoc, query, fields), params: [], file: __ENV__.file, line: __ENV__.line}|order_bys] end :one -> @@ -237,11 +246,20 @@ defmodule Ecto.Repo.Preloader do defp fetched_records_to_tuple_ids([], _assoc, _related_key), do: [] - defp fetched_records_to_tuple_ids([%{} | _] = entries, _assoc, {0, key}), - do: Enum.map(entries, &{Map.fetch!(&1, key), &1}) + defp fetched_records_to_tuple_ids([%{} | _] = entries, _assoc, {0, [single_key]}) do + Enum.map(entries, &{[Map.fetch!(&1, single_key)], &1}) + end - defp fetched_records_to_tuple_ids([{_, %{}} | _] = entries, _assoc, _related_key), - do: entries + defp fetched_records_to_tuple_ids([%{} | _] = entries, _assoc, {0, keys}) do + Enum.map(entries, fn entry -> + key = Enum.map(keys, &Map.fetch!(entry, &1)) + {key, entry} + end) + end + + defp fetched_records_to_tuple_ids([{_, %{}} | _] = entries, _assoc, _related_key) do + Enum.map(entries, fn {key, value} -> {List.wrap(key), value} end) + end defp fetched_records_to_tuple_ids([entry | _], assoc, _), do: raise """ @@ -280,22 +298,28 @@ defmodule Ecto.Repo.Preloader do defp preload_order(assoc, query, related_field) do custom_order_by = Enum.map(assoc.preload_order, fn {direction, field} -> - {direction, related_key_to_field(query, {0, field})} + {direction, related_key_to_fields(query, {0, [field]})} field -> - {:asc, related_key_to_field(query, {0, field})} + {:asc, related_key_to_fields(query, {0, [field]})} end) [{:asc, related_field} | custom_order_by] end - defp related_key_to_field(query, {pos, key, field_type}) do - field_ast = related_key_to_field(query, {pos, key}) - - {:type, [], [field_ast, field_type]} + defp related_key_to_fields(query, {pos, keys}) do + Enum.map keys, fn key -> + {{:., [], [{:&, [], [related_key_pos(query, pos)]}, key]}, [], []} + end end - defp related_key_to_field(query, {pos, key}) do - {{:., [], [{:&, [], [related_key_pos(query, pos)]}, key]}, [], []} + # TODO deprecated + defp related_key_to_fields(query, {pos, keys, types}) do + keys + |> Enum.zip(types) + |> Enum.map(fn {key, field_type} -> + field_ast = {{:., [], [{:&, [], [related_key_pos(query, pos)]}, key]}, [], []} + {:type, [], [field_ast, field_type]} + end) end defp related_key_pos(_query, pos) when pos >= 0, do: pos @@ -352,7 +376,7 @@ defmodule Ecto.Repo.Preloader do defp load_assoc({:assoc, assoc, ids}, struct) do %{field: field, owner_key: owner_key, cardinality: cardinality} = assoc - key = Map.fetch!(struct, owner_key) + key = Enum.map(owner_key, fn owner_key_field -> Map.fetch!(struct, owner_key_field) end) loaded = case ids do diff --git a/lib/ecto/repo/schema.ex b/lib/ecto/repo/schema.ex index ab05f0b0e1..97712a436d 100644 --- a/lib/ecto/repo/schema.ex +++ b/lib/ecto/repo/schema.ex @@ -884,20 +884,25 @@ defmodule Ecto.Repo.Schema do end defp change_parents(changes, struct, assocs) do - Enum.reduce assocs, changes, fn {refl, _}, acc -> + Enum.reduce assocs, changes, fn {refl, _}, changes -> %{field: field, owner_key: owner_key, related_key: related_key} = refl related = Map.get(struct, field) - value = related && Map.fetch!(related, related_key) - - case Map.fetch(changes, owner_key) do - {:ok, current} when current != value -> - raise ArgumentError, - "cannot change belongs_to association `#{field}` because there is " <> - "already a change setting its foreign key `#{owner_key}` to `#{inspect current}`" - - _ -> - Map.put(acc, owner_key, value) - end + # TODO remove List.wrap + List.wrap(owner_key) + |> Enum.zip(List.wrap(related_key)) + |> Enum.reduce(changes, fn {owner_key, related_key}, changes -> + value = related && Map.fetch!(related, related_key) + + case Map.fetch(changes, owner_key) do + {:ok, current} when current != value -> + raise ArgumentError, + "cannot change belongs_to association `#{field}` because there is " <> + "already a change setting its foreign key `#{owner_key}` to `#{inspect(current)}`" + + _ -> + Map.put(changes, owner_key, value) + end + end) end end diff --git a/lib/ecto/schema.ex b/lib/ecto/schema.ex index 7a5a806ea2..ac9bf3b0a2 100644 --- a/lib/ecto/schema.ex +++ b/lib/ecto/schema.ex @@ -1976,15 +1976,25 @@ defmodule Ecto.Schema do def __belongs_to__(mod, name, queryable, opts) do check_options!(opts, @valid_belongs_to_options, "belongs_to/3") - opts = Keyword.put_new(opts, :foreign_key, :"#{name}_id") + opts = Keyword.update(opts, :foreign_key, :"#{name}_id", &List.wrap/1) foreign_key_type = opts[:type] || Module.get_attribute(mod, :foreign_key_type) - if name == Keyword.get(opts, :foreign_key) do + if [name] == Keyword.get(opts, :foreign_key) do raise ArgumentError, "foreign_key #{inspect name} must be distinct from corresponding association name" end if Keyword.get(opts, :define_field, true) do - __field__(mod, opts[:foreign_key], foreign_key_type, opts) + foreign_keys = List.wrap(opts[:foreign_key]) + foreign_key_types = if is_list(foreign_key_type) do + foreign_key_type + else + # TODO add a test for this branch + List.duplicate(foreign_key_type, length(foreign_keys)) + end + + for {fk, type} <- Enum.zip(foreign_keys, foreign_key_types) do + __field__(mod, fk, type, opts) + end end struct = diff --git a/lib/ecto/util.ex b/lib/ecto/util.ex new file mode 100644 index 0000000000..da1b0c1cf0 --- /dev/null +++ b/lib/ecto/util.ex @@ -0,0 +1,19 @@ +# TODO delete this file +defmodule Util do + defmacro inspect_unstruct(term) do + quote do + IO.inspect(unquote(term), label: Util.label(__ENV__), charlists: :as_lists, structs: false) + end + end + + defmacro inspect(term) do + quote do + IO.inspect(unquote(term), label: Util.label(__ENV__), charlists: :as_lists) + end + end + + def label(env) do + {fun, arity} = env.function + "#{env.module}.#{fun}/#{arity} #{env.line}" + end +end diff --git a/test/ecto/query/planner_test.exs b/test/ecto/query/planner_test.exs index 4d520f63b3..eed76f0945 100644 --- a/test/ecto/query/planner_test.exs +++ b/test/ecto/query/planner_test.exs @@ -67,6 +67,18 @@ defmodule Ecto.Query.PlannerTest do end end + defmodule CompositePk do + use Ecto.Schema + + @primary_key false + schema "composites" do + field :id_1, :string, primary_key: true + field :id_2, :integer, primary_key: true + + many_to_many :posts, Ecto.Query.PlannerTest.Post, join_through: "composites_posts", join_keys: [[composite_id_1: :id_1, composite_id_2: :id_2], [post_id: :id]], join_where: [deleted: true] + end + end + defmodule Post do use Ecto.Schema @@ -92,6 +104,8 @@ defmodule Ecto.Query.PlannerTest do many_to_many :crazy_comments, Comment, join_through: CommentPost, where: [text: "crazycomment"] many_to_many :crazy_comments_with_list, Comment, join_through: CommentPost, where: [text: {:in, ["crazycomment1", "crazycomment2"]}], join_where: [deleted: true] many_to_many :crazy_comments_without_schema, Comment, join_through: "comment_posts", join_where: [deleted: true] + + many_to_many :composites, CompositePk, join_through: "composites_posts", join_keys: [[post_id: :id], [composite_id_1: :id_1, composite_id_2: :id_2]], join_where: [deleted: true] end end @@ -968,6 +982,36 @@ defmodule Ecto.Query.PlannerTest do assert params == [true, 1] end + test "normalize: many_to_many assoc join with composite keys on association" do + {query, params, _select} = from(post in Post, join: comment in assoc(post, :composites)) |> normalize_with_params() + + assert inspect(query) =~ "join: c1 in Ecto.Query.PlannerTest.CompositePk, on: c2.composite_id_1 == c1.id_1 and c2.composite_id_2 == c1.id_2 and c2.deleted == ^..." + assert params == [true] + + {query, params, _} = + Ecto.assoc(%Post{id: 1}, :composites) + |> normalize_with_params() + + assert inspect(query) =~ "join: c1 in \"composites_posts\", on: c0.id_1 == c1.composite_id_1 and c0.id_2 == c1.composite_id_2 and c1.deleted == ^..." + assert inspect(query) =~ "where: c1.post_id in ^..." + assert params == [true, 1] + end + + test "normalize: many_to_many assoc join with composite keys on owner" do + {query, params, _select} = from(compo in CompositePk, join: post in assoc(compo, :posts)) |> normalize_with_params() + + assert inspect(query) =~ "join: p1 in Ecto.Query.PlannerTest.Post, on: c2.post_id == p1.id and c2.deleted == ^..." + assert params == [true] + + {query, params, _} = + Ecto.assoc(%Post{id: 1}, :composites) + |> normalize_with_params() + + assert inspect(query) =~ "join: c1 in \"composites_posts\", on: c0.id_1 == c1.composite_id_1 and c0.id_2 == c1.composite_id_2 and c1.deleted == ^..." + assert inspect(query) =~ "where: c1.post_id in ^..." + assert params == [true, 1] + end + test "normalize: dumps in query expressions" do assert_raise Ecto.QueryError, ~r"cannot be dumped", fn -> normalize(from p in Post, where: p.posted == "2014-04-17 00:00:00") diff --git a/test/ecto/repo/belongs_to_test.exs b/test/ecto/repo/belongs_to_test.exs index db0d295296..0225f71a46 100644 --- a/test/ecto/repo/belongs_to_test.exs +++ b/test/ecto/repo/belongs_to_test.exs @@ -24,6 +24,20 @@ defmodule Ecto.Repo.BelongsToTest do end end + defmodule MyCompositeAssoc do + use Ecto.Schema + + @primary_key false + schema "my_composite_assoc" do + field :id_1, :id, primary_key: true + field :id_2, :string, primary_key: true + field :name, :string + has_one :my_schema, MySchema, + foreign_key: [:composite_id_1, :composite_id_2], references: [:id_1, :id_2] + timestamps() + end + end + defmodule MySchema do use Ecto.Schema @@ -32,6 +46,8 @@ defmodule Ecto.Repo.BelongsToTest do field :y, :binary belongs_to :assoc, MyAssoc, on_replace: :delete belongs_to :nilify_assoc, MyAssoc, on_replace: :nilify + belongs_to :composite_assoc, MyCompositeAssoc, + foreign_key: [:composite_id_1, :composite_id_2], references: [:id_1, :id_2], type: [:id, :string] end end @@ -50,6 +66,22 @@ defmodule Ecto.Repo.BelongsToTest do assert assoc.inserted_at end + test "handles assocs with composite keys on insert" do + sample = %MyCompositeAssoc{id_1: 123, id_2: "xyz"} + + changeset = + %MySchema{} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:composite_assoc, sample) + schema = TestRepo.insert!(changeset) + assoc = schema.composite_assoc + assert assoc.id_1 == 123 + assert assoc.id_2 == "xyz" + assert assoc.id_1 == schema.composite_id_1 + assert assoc.id_2 == schema.composite_id_2 + assert assoc.inserted_at + end + test "handles assocs on insert preserving parent schema prefix" do sample = %MyAssoc{x: "xyz"} @@ -106,6 +138,32 @@ defmodule Ecto.Repo.BelongsToTest do end end + test "checks dual changes to composite keys on insert" do + # values are the same + changeset = + %MySchema{} + |> Ecto.Changeset.change(composite_id_1: 13) + |> Ecto.Changeset.put_assoc(:composite_assoc, %MyCompositeAssoc{id_1: 13, id_2: "xyz"}) + TestRepo.insert!(changeset) + + # values are different + changeset = + %MySchema{} + |> Ecto.Changeset.change(composite_id_1: 13) + |> Ecto.Changeset.put_assoc(:composite_assoc, %MyCompositeAssoc{id_2: "xyz"}) + assert_raise ArgumentError, ~r"there is already a change setting its foreign key", fn -> + TestRepo.insert!(changeset) + end + + changeset = + %MySchema{} + |> Ecto.Changeset.change(composite_id_2: "abc") + |> Ecto.Changeset.put_assoc(:composite_assoc, %MyCompositeAssoc{id_1: 13}) + assert_raise ArgumentError, ~r"there is already a change setting its foreign key", fn -> + TestRepo.insert!(changeset) + end + end + test "returns untouched changeset on invalid children on insert" do assoc = %MyAssoc{x: "xyz"} assoc_changeset = %{Ecto.Changeset.change(assoc) | valid?: false} @@ -242,7 +300,23 @@ defmodule Ecto.Repo.BelongsToTest do assert assoc.updated_at end - test "inserting assocs on update preserving parent schema prefix" do + test "inserting assocs with composite keys on update" do + sample = %MyCompositeAssoc{id_1: 123, id_2: "xyz"} + + changeset = + %MySchema{id: 1} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:composite_assoc, sample) + schema = TestRepo.update!(changeset) + assoc = schema.composite_assoc + assert assoc.id_1 == 123 + assert assoc.id_2 == "xyz" + assert assoc.id_1 == schema.composite_id_1 + assert assoc.id_2 == schema.composite_id_2 + assert assoc.inserted_at + end + + test "inserting assocs on update preserving parent schema prefix" do sample = %MyAssoc{x: "xyz"} changeset = @@ -349,6 +423,26 @@ defmodule Ecto.Repo.BelongsToTest do refute_received {:delete, _} # Same assoc should not emit delete end + test "changing assocs with composite keys on update" do + sample = %MyCompositeAssoc{id_1: 13, id_2: "xyz", name: "old name"} + sample = put_meta sample, state: :loaded + + # Changing the assoc + sample_changeset = Ecto.Changeset.change(sample, name: "new name") + changeset = + %MySchema{id: 1, composite_assoc: sample} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:composite_assoc, sample_changeset) + schema = TestRepo.update!(changeset) + assoc = schema.composite_assoc + assert assoc.id_1 == 13 + assert assoc.id_2 == "xyz" + assert assoc.name == "new name" + refute assoc.inserted_at + assert assoc.updated_at + refute_received {:delete, _} # Same assoc should not emit delete + end + test "removing assocs on update raises if there is no id" do assoc = %MyAssoc{x: "xyz"} |> Ecto.put_meta(state: :loaded) diff --git a/test/ecto/repo/has_assoc_test.exs b/test/ecto/repo/has_assoc_test.exs index b7758adb39..3995308089 100644 --- a/test/ecto/repo/has_assoc_test.exs +++ b/test/ecto/repo/has_assoc_test.exs @@ -563,4 +563,57 @@ defmodule Ecto.Repo.HasAssocTest do refute_received {:transaction, _} refute_received {:rollback, _} end + + defmodule MyCompositeAssoc do + use Ecto.Schema + + schema "my_composite_assoc" do + field :name, :binary + belongs_to :composite_schema, MyCompositeSchema, + foreign_key: [:composite_x, :composite_y], references: [:x, :y], type: [:id, :string] + timestamps() + end + end + + defmodule MyCompositeSchema do + use Ecto.Schema + + @primary_key false + schema "my_composite_schema" do + field :x, :id, primary_key: true + field :y, :string, primary_key: true + has_one :assoc, MyCompositeAssoc, foreign_key: [:composite_x, :composite_y], references: [:x, :y] + has_many :assocs, MyCompositeAssoc, foreign_key: [:composite_x, :composite_y], references: [:x, :y] + end + end + + test "handles assocs with composite keys on insert" do + sample = %MyCompositeAssoc{name: "xyz"} + + changeset = + %MyCompositeSchema{} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:assoc, sample) + schema = TestRepo.insert!(changeset) + assoc = schema.assoc + assert assoc.id + assert assoc.name == "xyz" + assert assoc.composite_x == schema.x + assert assoc.composite_y == schema.y + assert assoc.inserted_at + + changeset = + %MyCompositeSchema{} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:assocs, [sample]) + schema = TestRepo.insert!(changeset) + [assoc] = schema.assocs + assert assoc.id + assert assoc.name == "xyz" + assert assoc.composite_x == schema.x + assert assoc.composite_y == schema.y + assert assoc.inserted_at + end + + # TODO add tests for update end diff --git a/test/ecto/repo/many_to_many_test.exs b/test/ecto/repo/many_to_many_test.exs index d00400e38d..bc0e5cc2a5 100644 --- a/test/ecto/repo/many_to_many_test.exs +++ b/test/ecto/repo/many_to_many_test.exs @@ -34,6 +34,33 @@ defmodule Ecto.Repo.ManyToManyTest do end end + defmodule MyCompositeAssoc do + use Ecto.Schema + + @primary_key false + schema "my_composite_assoc" do + field :id_1, :id, primary_key: true + field :id_2, :string, primary_key: true + field :name, :string + many_to_many :my_schemas, Ecto.Repo.ManyToManyTest.MySchema, + join_through: "schemas_composite_assocs", + join_keys: [[composite_id_1: :id_1, composite_id_2: :id_2],[my_schema_id: :id]] + timestamps() + end + end + + defmodule MySchemaCompositeAssoc do + use Ecto.Schema + + schema "schemas_composite_assocs" do + field :public, :boolean, default: false + belongs_to :my_schema, MySchema + belongs_to :my_assoc, MyCompositeAssoc, + foreign_key: [:composite_id_1, :composite_id_2], references: [:id_1, :id_2], type: [:id, :string] + timestamps() + end + end + defmodule MySchema do use Ecto.Schema @@ -42,7 +69,15 @@ defmodule Ecto.Repo.ManyToManyTest do field :y, :binary many_to_many :assocs, MyAssoc, join_through: "schemas_assocs", on_replace: :delete many_to_many :schema_assocs, MyAssoc, join_through: MySchemaAssoc, join_defaults: [public: true] + many_to_many :schema_key_assocs, MyAssoc, join_through: MySchemaAssoc, join_keys: [my_schema_id: :id, my_assoc_id: :id] many_to_many :mfa_schema_assocs, MyAssoc, join_through: MySchemaAssoc, join_defaults: {__MODULE__, :send_to_self, [:extra]} + many_to_many :composite_assocs, MyCompositeAssoc, + join_through: "schemas_composite_assocs", + join_keys: [[my_schema_id: :id], [composite_id_1: :id_1, composite_id_2: :id_2]] + many_to_many :schema_composite_assocs, MyCompositeAssoc, + join_through: MySchemaCompositeAssoc, + join_keys: [[my_schema_id: :id], [composite_id_1: :id_1, composite_id_2: :id_2]], + join_defaults: [public: true] end def send_to_self(struct, owner, extra) do @@ -103,6 +138,28 @@ defmodule Ecto.Repo.ManyToManyTest do assert join.fields[:public] end + test "handles assocs on insert with schema and join keys" do + sample = %MyAssoc{x: "xyz"} + + changeset = + %MySchema{} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:schema_key_assocs, [sample]) + + schema = TestRepo.insert!(changeset) + [assoc] = schema.schema_key_assocs + assert assoc.id + assert assoc.x == "xyz" + assert assoc.inserted_at + assert_received {:insert, _child} + assert_received {:insert, _parent} + assert_received {:insert, join} + + # Available from defaults + assert join.fields[:my_schema_id] == schema.id + assert join.fields[:my_assoc_id] == assoc.id + end + test "handles assocs on insert with schema and MFA defaults" do sample = %MyAssoc{x: "xyz"} @@ -128,6 +185,68 @@ defmodule Ecto.Repo.ManyToManyTest do assert_received {:defaults, %MySchemaAssoc{}, %MySchema{x: "abc"}, :extra} end + test "handles assocs with composite keys on insert (parent has normal keys)" do + sample = %MyCompositeAssoc{id_1: 1, id_2: "a", name: "xyz"} + + changeset = + %MySchema{x: "abc"} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:composite_assocs, [sample]) + schema = TestRepo.insert!(changeset) + [assoc] = schema.composite_assocs + assert assoc.id_1 == 1 + assert assoc.id_2 == "a" + assert assoc.name == "xyz" + assert assoc.inserted_at + assert_received {:insert, _} + assert_received {:insert_all, %{source: "schemas_composite_assocs"}, [ + [composite_id_1: 1, composite_id_2: "a", my_schema_id: 1] + ]} + end + + test "handles assocs with composite keys on insert (parent has composite keys)" do + sample = %MySchema{x: "abc"} + + changeset = + %MyCompositeAssoc{id_1: 1, id_2: "a", name: "xyz"} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:my_schemas, [sample]) + schema = TestRepo.insert!(changeset) + [assoc] = schema.my_schemas + assert assoc.id + assert assoc.x == "abc" + assert_received {:insert, _} + assert_received {:insert_all, %{source: "schemas_composite_assocs"}, [ + [composite_id_1: 1, composite_id_2: "a", my_schema_id: 1] + ]} + end + + test "handles assocs with composite keys and schema on insert" do + sample = %MyCompositeAssoc{id_1: 1, id_2: "a", name: "xyz"} + + changeset = + %MySchema{x: "abc"} + |> Ecto.Changeset.change + |> Ecto.Changeset.put_assoc(:schema_composite_assocs, [sample]) + schema = TestRepo.insert!(changeset) + [assoc] = schema.schema_composite_assocs + assert assoc.id_1 == 1 + assert assoc.id_2 == "a" + assert assoc.name == "xyz" + assert assoc.inserted_at + + assert_received {:insert, _child} + assert_received {:insert, _parent} + assert_received {:insert, join} + + # Available from defaults + assert join.fields[:my_schema_id] == schema.id + assert join.fields[:composite_id_1] == assoc.id_1 + assert join.fields[:composite_id_2] == assoc.id_2 + assert join.fields[:public] + end + + test "handles assocs from struct on insert" do schema = TestRepo.insert!(%MySchema{assocs: [%MyAssoc{x: "xyz"}]}) [assoc] = schema.assocs diff --git a/test/ecto/schema_test.exs b/test/ecto/schema_test.exs index 669e999570..509dfea134 100644 --- a/test/ecto/schema_test.exs +++ b/test/ecto/schema_test.exs @@ -564,7 +564,7 @@ defmodule Ecto.SchemaTest do test "has_many association" do struct = %Ecto.Association.Has{field: :posts, owner: AssocSchema, cardinality: :many, on_delete: :nothing, - related: Post, owner_key: :id, related_key: :assoc_schema_id, queryable: Post, + related: Post, owner_key: [:id], related_key: [:assoc_schema_id], queryable: Post, on_replace: :raise} assert AssocSchema.__schema__(:association, :posts) == struct @@ -578,7 +578,7 @@ defmodule Ecto.SchemaTest do test "has_many association via {source schema}" do struct = %Ecto.Association.Has{field: :emails, owner: AssocSchema, cardinality: :many, on_delete: :nothing, - related: Email, owner_key: :id, related_key: :assoc_schema_id, + related: Email, owner_key: [:id], related_key: [:assoc_schema_id], queryable: {"users_emails", Email}, on_replace: :delete} assert AssocSchema.__schema__(:association, :emails) == struct @@ -592,7 +592,7 @@ defmodule Ecto.SchemaTest do test "has_many through association" do assert AssocSchema.__schema__(:association, :comment_authors) == %Ecto.Association.HasThrough{field: :comment_authors, owner: AssocSchema, cardinality: :many, - through: [:comment, :authors], owner_key: :comment_id} + through: [:comment, :authors], owner_key: [:comment_id]} refute Map.has_key?(AssocSchema.__changeset__(), :comment_authors) @@ -604,7 +604,7 @@ defmodule Ecto.SchemaTest do test "has_one association" do struct = %Ecto.Association.Has{field: :author, owner: AssocSchema, cardinality: :one, on_delete: :nothing, - related: User, owner_key: :id, related_key: :assoc_schema_id, queryable: User, + related: User, owner_key: [:id], related_key: [:assoc_schema_id], queryable: User, on_replace: :raise} assert AssocSchema.__schema__(:association, :author) == struct @@ -618,7 +618,7 @@ defmodule Ecto.SchemaTest do test "has_one association via {source, schema}" do struct = %Ecto.Association.Has{field: :profile, owner: AssocSchema, cardinality: :one, on_delete: :nothing, - related: Profile, owner_key: :id, related_key: :assoc_schema_id, + related: Profile, owner_key: [:id], related_key: [:assoc_schema_id], queryable: {"users_profiles", Profile}, on_replace: :raise} assert AssocSchema.__schema__(:association, :profile) == struct @@ -632,7 +632,7 @@ defmodule Ecto.SchemaTest do test "has_one through association" do assert AssocSchema.__schema__(:association, :comment_main_author) == %Ecto.Association.HasThrough{field: :comment_main_author, owner: AssocSchema, cardinality: :one, - through: [:comment, :main_author], owner_key: :comment_id} + through: [:comment, :main_author], owner_key: [:comment_id]} refute Map.has_key?(AssocSchema.__changeset__(), :comment_main_author) @@ -644,7 +644,7 @@ defmodule Ecto.SchemaTest do test "belongs_to association" do struct = %Ecto.Association.BelongsTo{field: :comment, owner: AssocSchema, cardinality: :one, - related: Comment, owner_key: :comment_id, related_key: :id, queryable: Comment, + related: Comment, owner_key: [:comment_id], related_key: [:id], queryable: Comment, on_replace: :raise, defaults: []} assert AssocSchema.__schema__(:association, :comment) == struct @@ -658,7 +658,7 @@ defmodule Ecto.SchemaTest do test "belongs_to association via {source, schema}" do struct = %Ecto.Association.BelongsTo{field: :summary, owner: AssocSchema, cardinality: :one, - related: Summary, owner_key: :summary_id, related_key: :id, + related: Summary, owner_key: [:summary_id], related_key: [:id], queryable: {"post_summary", Summary}, on_replace: :raise, defaults: []} assert AssocSchema.__schema__(:association, :summary) == struct @@ -672,7 +672,7 @@ defmodule Ecto.SchemaTest do test "belongs_to association via Ecto.ParameterizedType" do struct = %Ecto.Association.BelongsTo{field: :reference, owner: AssocSchema, cardinality: :one, - related: SchemaWithParameterizedPrimaryKey, owner_key: :reference_id, related_key: :id, queryable: SchemaWithParameterizedPrimaryKey, + related: SchemaWithParameterizedPrimaryKey, owner_key: [:reference_id], related_key: [:id], queryable: SchemaWithParameterizedPrimaryKey, on_replace: :raise, defaults: []} assert AssocSchema.__schema__(:association, :reference) == struct @@ -693,32 +693,63 @@ defmodule Ecto.SchemaTest do has_one :author, User, references: :pk, foreign_key: :fk belongs_to :permalink1, Permalink, references: :pk, foreign_key: :fk belongs_to :permalink2, Permalink, references: :pk, type: :string + belongs_to :publication, Publication, references: [:id_1, :id_2], + foreign_key: [:publication_id_1, :publication_id_2], type: [:integer, :string] + end + end + + defmodule Publication do + use Ecto.Schema + + @primary_key false + schema "publication" do + field :id_1, :integer, primary_key: true + field :id_2, :string, primary_key: true + has_many :custom_assoc_schemas, CustomAssocSchema, references: [:id_1, :id_2], + foreign_key: [:publication_id_1, :publication_id_2] + has_one :custom_assoc_schema, CustomAssocSchema, references: [:id_1, :id_2], + foreign_key: [:pub_id_1, :pub_id_2] end end test "has_many options" do refl = CustomAssocSchema.__schema__(:association, :posts) - assert :pk == refl.owner_key - assert :fk == refl.related_key + assert [:pk] == refl.owner_key + assert [:fk] == refl.related_key + + refl = Publication.__schema__(:association, :custom_assoc_schemas) + assert [:id_1, :id_2] == refl.owner_key + assert [:publication_id_1, :publication_id_2] == refl.related_key end test "has_one options" do refl = CustomAssocSchema.__schema__(:association, :author) - assert :pk == refl.owner_key - assert :fk == refl.related_key + assert [:pk] == refl.owner_key + assert [:fk] == refl.related_key + + refl = Publication.__schema__(:association, :custom_assoc_schema) + assert [:id_1, :id_2] == refl.owner_key + assert [:pub_id_1, :pub_id_2] == refl.related_key end test "belongs_to options" do refl = CustomAssocSchema.__schema__(:association, :permalink1) - assert :fk == refl.owner_key - assert :pk == refl.related_key + assert [:fk] == refl.owner_key + assert [:pk] == refl.related_key refl = CustomAssocSchema.__schema__(:association, :permalink2) - assert :permalink2_id == refl.owner_key - assert :pk == refl.related_key + assert [:permalink2_id] == refl.owner_key + assert [:pk] == refl.related_key assert CustomAssocSchema.__schema__(:type, :fk) == :string assert CustomAssocSchema.__schema__(:type, :permalink2_id) == :string + + refl = CustomAssocSchema.__schema__(:association, :publication) + assert [:publication_id_1, :publication_id_2] == refl.owner_key + assert [:id_1, :id_2] == refl.related_key + + assert CustomAssocSchema.__schema__(:type, :publication_id_1) == :integer + assert CustomAssocSchema.__schema__(:type, :publication_id_2) == :string end test "has_* validates option" do