Deduplicate resource objects in includeds

lib/jsonapi_plug/normalizer.ex

@@ -2,7 +2,7 @@ defmodule JSONAPIPlug.Normalizer do
   @moduledoc """
   Transforms user data to and from a `JSON:API` Document.
 
-  The default implementation transforms `JSON:API`documents in requests to an ecto
+  The default implementation transforms `JSON:API` documents in requests to an ecto
   friendly format and expects `Ecto.Schema` instances when rendering data in responses.
   The data it produces is stored under the `:params` key of the `JSONAPIPlug` struct
   that will be stored in the `Plug.Conn` private assign `:jsonapi_plug`.
@@ -305,11 +305,46 @@ defmodule JSONAPIPlug.Normalizer do
       data: normalize_data(conn, resource_or_resources, options),
       links: normalize_links(conn, resource_or_resources, links || %{}, options),
       included:
-        normalize_included(MapSet.new(), conn, resource_or_resources, options)
-        |> MapSet.to_list()
+        normalize_included(%{}, conn, resource_or_resources, options)
+        |> Map.values()
     }
   end
 
+  defp put_included(included, %ResourceObject{} = resource_object) do
+    key = {resource_object.type, resource_object.id || resource_object.lid}
+    Map.update(included, key, resource_object, &merge_resource_objects(&1, resource_object))
+  end
+
+  defp merge_resource_objects(%ResourceObject{} = base, %ResourceObject{} = incoming) do
+    %ResourceObject{
+      base
+      | relationships: merge_relationships(base.relationships, incoming.relationships)
+    }
+  end
+
+  defp merge_relationships(base, incoming) do
+    Map.merge(base || %{}, incoming || %{}, fn _name, base_relationship, incoming_relationship ->
+      merge_relationship(base_relationship, incoming_relationship)
+    end)
+  end
+
+  defp merge_relationship(
+         %RelationshipObject{data: base_data} = base_relationship,
+         %RelationshipObject{data: incoming_data}
+       )
+       when is_list(base_data) and is_list(incoming_data) do
+    %RelationshipObject{base_relationship | data: Enum.uniq(base_data ++ incoming_data)}
+  end
+
+  defp merge_relationship(
+         %RelationshipObject{data: nil},
+         %RelationshipObject{} = incoming_relationship
+       ),
+       do: incoming_relationship
+
+  defp merge_relationship(%RelationshipObject{} = base_relationship, %RelationshipObject{}),
+    do: base_relationship
+
   defp normalize_data(_conn, nil, _options), do: nil
 
   defp normalize_data(conn, resources, options) when is_list(resources),
@@ -472,8 +507,9 @@ defmodule JSONAPIPlug.Normalizer do
 
     case {related_loaded?, related_many, related_data} do
       {true, true, related_data} when is_list(related_data) ->
-        MapSet.new(related_data, &normalize_resource(conn, &1, options))
-        |> MapSet.union(included)
+        Enum.reduce(related_data, included, fn related, included ->
+          put_included(included, normalize_resource(conn, related, options))
+        end)
 
       {true, _related_many, related_data} when is_list(related_data) ->
         raise InvalidDocument,
@@ -486,10 +522,7 @@ defmodule JSONAPIPlug.Normalizer do
           reference: nil
 
       {true, _related_many, _related_data} ->
-        MapSet.put(
-          included,
-          normalize_resource(conn, related_data, options)
-        )
+        put_included(included, normalize_resource(conn, related_data, options))
 
       {false, _related_many, _related_data} ->
         included

test/jsonapi_plug/normalizer_test.exs

@@ -0,0 +1,67 @@
+defmodule JSONAPIPlug.NormalizerTest do
+  use ExUnit.Case
+  import Plug.Test
+
+  alias JSONAPIPlug.{Document, Document.ResourceObject}
+  alias JSONAPIPlug.TestSupport.Plugs.UnderscoringPostPlug
+  alias JSONAPIPlug.TestSupport.Resources.{Comment, Company, Industry, Post, Tag, User}
+
+  defp included_with_id(%Document{included: included}, type, id),
+    do: Enum.filter(included, &(&1.type == type and &1.id == id))
+
+  describe "normalize included deduplication" do
+    test "collapses a resource reached via multiple include paths into a single object" do
+      conn =
+        conn(:get, "/?include=author.company,best_comments.user") |> UnderscoringPostPlug.call([])
+
+      author = %User{id: 1, first_name: "Ada", company: %Company{id: 9, name: "Initech"}}
+      comment = %Comment{id: 5, body: "hi", user: %User{id: 1, first_name: "Ada"}}
+      post = %Post{id: 1, text: "Hello", author: author, best_comments: [comment]}
+
+      document = JSONAPIPlug.render(conn, post)
+
+      assert [%ResourceObject{relationships: relationships}] =
+               included_with_id(document, "user", "1")
+
+      assert Map.has_key?(relationships, "company")
+      assert [%ResourceObject{}] = included_with_id(document, "company", "9")
+    end
+
+    test "leaves a document without duplicate resources untouched" do
+      conn = conn(:get, "/?include=author,best_comments") |> UnderscoringPostPlug.call([])
+
+      post = %Post{
+        id: 1,
+        text: "Hello",
+        author: %User{id: 1, first_name: "Ada"},
+        best_comments: [%Comment{id: 5, body: "hi"}]
+      }
+
+      assert %Document{included: included} = JSONAPIPlug.render(conn, post)
+
+      assert included |> Enum.map(&{&1.type, &1.id}) |> Enum.sort() == [
+               {"comment", "5"},
+               {"user", "1"}
+             ]
+    end
+
+    test "merges to-many relationship identifiers into one" do
+      conn =
+        conn(:get, "/?include=author.company.industry.tags,other_user.company.industry.tags")
+        |> UnderscoringPostPlug.call([])
+
+      industry_a = %Industry{id: 7, name: "Tech", tags: [%Tag{id: 100, name: "a"}]}
+      industry_b = %Industry{id: 7, name: "Tech", tags: [%Tag{id: 200, name: "b"}]}
+      author = %User{id: 1, company: %Company{id: 9, industry: industry_a}}
+      other_user = %User{id: 2, company: %Company{id: 10, industry: industry_b}}
+      post = %Post{id: 1, text: "Hello", author: author, other_user: other_user}
+
+      document = JSONAPIPlug.render(conn, post)
+
+      assert [%ResourceObject{relationships: %{"tags" => tags}}] =
+               included_with_id(document, "industry", "7")
+
+      assert tags.data |> Enum.map(& &1.id) |> Enum.sort() == ["100", "200"]
+    end
+  end
+end