Improve how avoid duplicate includes

Author: Raphexion

lib/hawk/json_api/document.ex

@@ -238,10 +238,7 @@ defmodule Hawk.JsonApi.Document do
   defp put_included(document, models, opts) do
     primary_resources = MapSet.new(models, &resource_identity/1)
 
-    included =
-      models
-      |> included_resources(Keyword.get(opts, :preloads, []), opts)
-      |> Enum.reject(&MapSet.member?(primary_resources, {&1.type, &1.id}))
+    {included, _seen} = included_resources(models, Keyword.get(opts, :preloads, []), opts, primary_resources)
 
     if included == [] do
       document
@@ -254,28 +251,40 @@ defmodule Hawk.JsonApi.Document do
     {Schema.metadata(model).type, to_string(Map.get(model, Schema.identity(model)))}
   end
 
-  defp included_resources(models, preloads, opts) do
-    models
-    |> Enum.flat_map(&included_resources_for_model(&1, preloads, opts))
-    |> Enum.uniq_by(&{&1.type, &1.id})
+  defp included_resources(models, preloads, opts, seen) do
+    Enum.reduce(models, {[], seen}, fn model, {included, seen} ->
+      {model_included, seen} = included_resources_for_model(model, preloads, opts, seen)
+      {included ++ model_included, seen}
+    end)
   end
 
-  defp included_resources_for_model(model, preloads, opts) do
-    Enum.flat_map(preloads, fn
-      name when is_atom(name) ->
-        direct_included_resources(model, name, [], opts)
+  defp included_resources_for_model(model, preloads, opts, seen) do
+    Enum.reduce(preloads, {[], seen}, fn
+      name, {included, seen} when is_atom(name) ->
+        {direct, seen} = direct_included_resources(model, name, [], opts, seen)
+        {included ++ direct, seen}
 
-      {name, nested} when is_atom(name) ->
-        direct_included_resources(model, name, nested, opts)
+      {name, nested}, {included, seen} when is_atom(name) ->
+        {direct, seen} = direct_included_resources(model, name, nested, opts, seen)
+        {included ++ direct, seen}
     end)
   end
 
-  defp direct_included_resources(model, name, nested, opts) do
+  defp direct_included_resources(model, name, nested, opts, seen) do
     model
     |> related_models(name)
-    |> Enum.flat_map(fn related ->
-      [resource_object(related, Keyword.put(opts, :preloads, nested))] ++
-        included_resources_for_model(related, nested, opts)
+    |> Enum.reduce({[], seen}, fn related, {included, seen} ->
+      identity = resource_identity(related)
+
+      if MapSet.member?(seen, identity) do
+        {nested_included, seen} = included_resources_for_model(related, nested, opts, seen)
+        {included ++ nested_included, seen}
+      else
+        seen = MapSet.put(seen, identity)
+        object = resource_object(related, Keyword.put(opts, :preloads, nested))
+        {nested_included, seen} = included_resources_for_model(related, nested, opts, seen)
+        {included ++ [object | nested_included], seen}
+      end
     end)
   end
 

test/hawk/json_api/document_test.exs

@@ -1,7 +1,44 @@
+defmodule Hawk.JsonApi.DocumentTest.Author do
+  @moduledoc false
+
+  use Hawk.Model
+  use Hawk.JsonApi.Resource
+
+  model "document_test_authors" do
+    field(:name, :string)
+  end
+
+  type("document-test-authors")
+  attribute(:name, resolver: &__MODULE__.render_name/1)
+
+  def render_name(%{name: :raise_if_rendered}) do
+    raise "duplicate author was rendered"
+  end
+
+  def render_name(author), do: author.name
+end
+
+defmodule Hawk.JsonApi.DocumentTest.Article do
+  @moduledoc false
+
+  use Hawk.Model
+  use Hawk.JsonApi.Resource
+
+  model "document_test_articles" do
+    field(:title, :string)
+    belongs_to(:author, Hawk.JsonApi.DocumentTest.Author)
+  end
+
+  type("document-test-articles")
+  attribute(:title, [])
+  relationship(:author, [])
+end
+
 defmodule Hawk.JsonApi.DocumentTest do
   use ExUnit.Case, async: true
 
   alias Hawk.JsonApi.Document
+  alias Hawk.JsonApi.DocumentTest.{Article, Author}
 
   test "renders a collection without a json_api_by_model override, resolving per record" do
     courses = [
@@ -21,6 +58,31 @@ defmodule Hawk.JsonApi.DocumentTest do
     assert Enum.map(document.data, & &1.attributes.title) == ["Math", "History"]
   end
 
+  test "renders duplicate included resources once" do
+    author_id = "00000000-0000-0000-0000-000000000021"
+    author = %Author{id: author_id, name: "Ada"}
+    duplicate_author = %Author{id: author_id, name: :raise_if_rendered}
+
+    articles = [
+      %Article{
+        id: "00000000-0000-0000-0000-000000000001",
+        title: "One",
+        author_id: author.id,
+        author: author
+      },
+      %Article{
+        id: "00000000-0000-0000-0000-000000000002",
+        title: "Two",
+        author_id: duplicate_author.id,
+        author: duplicate_author
+      }
+    ]
+
+    document = Document.document(articles, preloads: [:author])
+
+    assert [%{type: "document-test-authors", id: ^author_id}] = document.included
+  end
+
   test "renders sparse fieldsets for attributes and relationships" do
     course = %Videdal.Course{
       id: "00000000-0000-0000-0000-000000000007",