Repository: jeregrine/jsonapi
Pull request #87 (2017-11-27 17:49:40)

Fix nesting for includes
fixes nested includes. query strings like this should now include data as expected:
`?include=user.company.industry.tags`

jeregrine: cc @doomspork if I could get a second set of eyes on this
doomspork: @jeregrine I'll take a peek today 👍 
StareIntoTheBeard: @doomspork @jeregrine @jlvallelonga bump, any other thoughts on this one? What's needed to get this merged in?
doomspork: Sorry @jlvallelonga, I've been away on a business trip and didn't have too much time to go through GH.  Since these _are_ a breaking change, I'm going to have to spend some time looking through them closer.  That said, if @jeregrine wants to go forward with it 👍 

Patch:
diff --git a/README.md b/README.md
index 90db1d2..0fee2b9 100644
--- a/README.md
+++ b/README.md
@@ -26,7 +26,11 @@ defmodule MyApp.PostView do
   use JSONAPI.View, type: "posts"
 
   def fields do
-    [:text, :body]
+    [:text, :body, :excerpt]
+  end
+
+  def excerpt(post, _conn) do
+    String.slice(post.body, 0..5)
   end
 
   def relationships do
diff --git a/lib/jsonapi/plugs/format_required.ex b/lib/jsonapi/plugs/format_required.ex
index 308dae3..4fd61e1 100644
--- a/lib/jsonapi/plugs/format_required.ex
+++ b/lib/jsonapi/plugs/format_required.ex
@@ -8,6 +8,7 @@ defmodule JSONAPI.FormatRequired do
   def init(opts), do: opts
 
   def call(%{method: method} = conn, _opts) when method in ["DELETE", "GET", "HEAD"], do: conn
+  def call(%{params: %{"data" => %{"relationships" => _}}} = conn, _), do: conn
   def call(%{params: %{"data" => %{"attributes" => _}}} = conn, _), do: conn
   def call(%{params: %{"data" => _}} = conn, _), do: send_error(conn, missing_data_attributes_param())
   def call(conn, _), do: send_error(conn, missing_data_param())
diff --git a/lib/jsonapi/serializer.ex b/lib/jsonapi/serializer.ex
index 1f6fa07..b134846 100644
--- a/lib/jsonapi/serializer.ex
+++ b/lib/jsonapi/serializer.ex
@@ -81,7 +81,11 @@ defmodule JSONAPI.Serializer do
     if {rel_view, :include} == valid_include_view && is_data_loaded?(rel_data) do
       rel_query_includes =
         if is_list(query_includes) do
-          Keyword.get(query_includes, key, [])
+          Enum.reduce(query_includes, [], fn
+            {^key, value}, acc -> acc ++ [value]
+            _, acc -> acc
+          end)
+          |> List.flatten
         else
           []
         end
@@ -94,13 +98,14 @@ defmodule JSONAPI.Serializer do
   end
 
   defp include_view(valid_includes, key) when is_list(valid_includes) do
-    case Keyword.get(valid_includes, key) do
-      {view, :include} -> {view, :include}
-      _view -> false
-    end
+    valid_includes
+    |> Keyword.get(key)
+    |> generate_view_tuple
   end
-  defp include_view({view, :include}, _key), do: {view, :include}
-  defp include_view(_view, _key), do: false
+  defp include_view(view, _key), do: generate_view_tuple(view)
+
+  defp generate_view_tuple({view, :include}), do: {view, :include}
+  defp generate_view_tuple(view) when is_atom(view), do: {view, :include}
 
   def is_data_loaded?(rel_data) do
     assoc_loaded?(rel_data) && (is_map(rel_data) || (is_list(rel_data) && !Enum.empty?(rel_data)))
@@ -142,29 +147,26 @@ defmodule JSONAPI.Serializer do
     |> Enum.uniq
   end
 
-  # This makes a mapping between includes from the query parser and includes in the view.
-  defp get_includes(view, nil), do: view.relationships()
-  defp get_includes(view, []), do: view.relationships()
-  defp get_includes(view, query_includes) when is_list(query_includes) do
-    base = view.relationships()
-    Enum.reduce(query_includes, [], &(handle_include(base, &1, &2)))
-  end
-  defp get_includes(view, include) do
-    base = view.relationships()
-    Keyword.get(base, include)
+  defp get_includes(view, query_includes) do
+    includes = get_default_includes(view) ++ get_query_includes(view, query_includes)
+    Enum.uniq(includes)
   end
 
-  defp handle_include(base, {parent, child}, acc) do
-    view = Keyword.get(base, parent)
-    acc = Keyword.put(acc, parent, view)
-    handle_include(view, child, acc)
-  end
-  defp handle_include({base, :include}, child, acc) do
-    handle_include(base.relationships(), child, acc)
+  defp get_default_includes(view) do
+    rels = view.relationships()
+    default_includes = rels |> Enum.filter(fn
+      {_k, {_v, :include}} -> true
+      _ -> false
+    end)
   end
-  defp handle_include(base, child, acc) do
-    view = if is_list(base), do: Keyword.get(base, child), else: base
-    Keyword.put(acc, child, view)
+
+  defp get_query_includes(view, query_includes) do
+    rels = view.relationships()
+    Enum.map(query_includes, fn
+      {include, _} -> Keyword.take(rels, [include])
+      include -> Keyword.take(rels, [include])
+    end)
+    |> List.flatten
   end
 
   def get_view({view, :include}), do: view
@@ -179,4 +181,4 @@ defmodule JSONAPI.Serializer do
   end
 
   defp remove_links?, do: Application.get_env(:jsonapi, :remove_links, false)
-end
\ No newline at end of file
+end
diff --git a/lib/jsonapi/view.ex b/lib/jsonapi/view.ex
index b1ccea2..0cc1cf5 100644
--- a/lib/jsonapi/view.ex
+++ b/lib/jsonapi/view.ex
@@ -91,7 +91,17 @@ defmodule JSONAPI.View do
       end
 
       def attributes(data, conn) do
-        visible_fields(data)
+        visible_fields = fields() -- hidden()
+
+        Enum.reduce(visible_fields, %{}, fn field, intermediate_map ->
+          value =
+            try do
+              apply(__MODULE__, field, [data, conn])
+            rescue
+              _e -> Map.get(data, field)
+            end
+          Map.put(intermediate_map, field, value)
+        end)
       end
 
       def meta(_data, _conn), do: nil
@@ -143,8 +153,6 @@ defmodule JSONAPI.View do
           do: index(data, conn, params)
       end
 
-      defp visible_fields(data), do: Map.take(data, fields() -- hidden())
-
       defp host(conn), do: Application.get_env(:jsonapi, :host, conn.host)
 
       defp scheme(conn), do: Application.get_env(:jsonapi, :scheme, to_string(conn.scheme))


Test patch:
diff --git a/test/jsonapi/plugs/format_required_test.exs b/test/jsonapi/plugs/format_required_test.exs
index 5557eef..3657da1 100644
--- a/test/jsonapi/plugs/format_required_test.exs
+++ b/test/jsonapi/plugs/format_required_test.exs
@@ -32,6 +32,15 @@ defmodule JSONAPI.FormatRequiredTest do
     assert %{"source" => %{"pointer" => "/data/attributes"}, "title" => "Missing attributes in data parameter"} = error
   end
 
+  test "does not halt if only relationships member is present" do
+    conn =
+      :post
+      |> conn("/example", Poison.encode!(%{data: %{relationships: %{}}}))
+      |> call_plug
+
+    refute conn.halted
+  end
+
   test "passes request through" do
     conn =
       :post
diff --git a/test/jsonapi/view_test.exs b/test/jsonapi/view_test.exs
index 0fe58d4..f285748 100644
--- a/test/jsonapi/view_test.exs
+++ b/test/jsonapi/view_test.exs
@@ -23,7 +23,11 @@ defmodule JSONAPI.ViewTest do
       namespace: "/api"
 
     def fields do
-      [:age, :first_name, :last_name, :password]
+      [:age, :first_name, :last_name, :full_name, :password]
+    end
+
+    def full_name(user, _conn) do
+      "#{user.first_name} #{user.last_name}"
     end
 
     def hidden do
@@ -69,7 +73,7 @@ defmodule JSONAPI.ViewTest do
   end
 
   test "attributes/2 does not display hidden fields" do
-    expected_map = %{age: 100, first_name: "Jason", last_name: "S"}
+    expected_map = %{age: 100, first_name: "Jason", last_name: "S", full_name: "Jason S"}
     assert expected_map == UserView.attributes(%{age: 100, first_name: "Jason", last_name: "S", password: "securepw"}, nil)
   end
 end
diff --git a/test/jsonapi_test.exs b/test/jsonapi_test.exs
index 6948dbf..f9dbfbf 100644
--- a/test/jsonapi_test.exs
+++ b/test/jsonapi_test.exs
@@ -5,11 +5,15 @@ defmodule JSONAPITest do
   defmodule PostView do
     use JSONAPI.View
 
-    def fields, do: [:text, :body]
+    def fields, do: [:text, :body, :excerpt]
     def type, do: "mytype"
     def relationships do
       [author: {JSONAPITest.UserView, :include},
-       other_user: {JSONAPITest.UserView, :include}]
+       other_user: JSONAPITest.UserView]
+    end
+    def excerpt(post, _conn) do
+      letter = String.slice(post.text, 0..1)
+      letter
     end
   end
 
@@ -18,6 +22,36 @@ defmodule JSONAPITest do
 
     def fields, do: [:username]
     def type, do: "user"
+    def relationships do
+      [company: JSONAPITest.CompanyView]
+    end
+  end
+
+  defmodule CompanyView do
+    use JSONAPI.View
+
+    def fields, do: [:name]
+    def type, do: "company"
+    def relationships do
+      [industry: JSONAPITest.IndustryView]
+    end
+  end
+
+  defmodule IndustryView do
+    use JSONAPI.View
+
+    def fields, do: [:name]
+    def type, do: "industry"
+    def relationships do
+      [tags: JSONAPITest.TagView]
+    end
+  end
+
+  defmodule TagView do
+    use JSONAPI.View
+
+    def fields, do: [:name]
+    def type, do: "tag"
     def relationships, do: []
   end
 
@@ -60,6 +94,9 @@ defmodule JSONAPITest do
 
     assert Enum.count(data_list) == 1
     [data | _] = data_list
+    assert Map.get(data["attributes"], "body") == "Hi"
+    assert Map.get(data["attributes"], "text") == "Hello"
+    assert Map.get(data["attributes"], "excerpt") == "He"
     assert Map.get(data, "type") == "mytype"
     assert Map.get(data, "id") == "1"
 
@@ -74,7 +111,7 @@ defmodule JSONAPITest do
     assert Map.has_key?(json, "included")
     included = Map.get(json, "included")
     assert is_list(included)
-    assert Enum.count(included) == 2
+    assert Enum.count(included) == 1
 
     [author | _] = included
     assert Map.get(author, "type") == "user"
@@ -84,7 +121,7 @@ defmodule JSONAPITest do
   end
 
   test "handles includes properly" do
-    conn = conn(:get, "/posts?include=author")
+    conn = conn(:get, "/posts?include=other_user")
     |> Plug.Conn.assign(:data, [%{
       id: 1,
       text: "Hello",
@@ -112,14 +149,117 @@ defmodule JSONAPITest do
     assert get_in(author_rel, ["data", "type"]) == "user"
     assert get_in(author_rel, ["data", "id"]) == "2"
 
+    other_user = Map.get(relationships, "other_user")
+
+    assert get_in(other_user, ["data", "type"]) == "user"
+    assert get_in(other_user, ["data", "id"]) == "3"
+
     assert Map.has_key?(json, "included")
     included = Map.get(json, "included")
     assert is_list(included)
-    assert Enum.count(included) == 1
+    assert Enum.count(included) == 2
 
-    [author] = included
-    assert Map.get(author, "type") == "user"
-    assert Map.get(author, "id") == "2"
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "user" &&
+      Map.get(include, "id") == "2"
+    end)
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "user" &&
+      Map.get(include, "id") == "3"
+    end)
+
+    assert Map.has_key?(json, "links")
+  end
+
+  test "handles deep nested includes properly" do
+    data = [%{
+      id: 1,
+      text: "Hello",
+      body: "Hi",
+      author: %{username: "jason", id: 2},
+      other_user: %{
+        id: 1,
+        username: "jim",
+        first_name: "Jimmy",
+        last_name: "Beam",
+        company: %{
+          id: 2,
+          name: "acme",
+          industry: %{
+            id: 4,
+            name: "stuff",
+            tags: [
+              %{id: 3, name: "a tag"},
+              %{id: 4, name: "another tag"}
+            ]
+          }
+        }
+      }
+    }]
+
+    conn = conn(:get, "/posts?include=other_user.company.industry.tags")
+    |> Plug.Conn.assign(:data, data)
+    |> Plug.Conn.fetch_query_params()
+    |> MyPostPlug.call([])
+
+    json = conn.resp_body |> Poison.decode!
+
+    assert Map.has_key?(json, "data")
+    data_list = Map.get(json, "data")
+
+    assert Enum.count(data_list) == 1
+    [data | _] = data_list
+    assert Map.get(data, "type") == "mytype"
+    assert Map.get(data, "id") == "1"
+
+    relationships = Map.get(data, "relationships")
+    assert map_size(relationships) == 2
+    assert Enum.sort(Map.keys(relationships)) == ["author", "other_user"]
+    author_rel = Map.get(relationships, "author")
+
+    assert get_in(author_rel, ["data", "type"]) == "user"
+    assert get_in(author_rel, ["data", "id"]) == "2"
+
+    other_user = Map.get(relationships, "other_user")
+
+    assert get_in(other_user, ["data", "type"]) == "user"
+    assert get_in(other_user, ["data", "id"]) == "1"
+
+    assert Map.has_key?(json, "included")
+    included = Map.get(json, "included")
+    assert is_list(included)
+    assert Enum.count(included) == 6
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "user" &&
+      Map.get(include, "id") == "2"
+    end)
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "user" &&
+      Map.get(include, "id") == "1"
+    end)
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "company" &&
+      Map.get(include, "id") == "2"
+    end)
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "industry" &&
+      Map.get(include, "id") == "4"
+    end)
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "tag" &&
+      Map.get(include, "id") == "3"
+    end)
+
+    assert Enum.find(included, fn include ->
+      Map.get(include, "type") == "tag" &&
+      Map.get(include, "id") == "4"
+    end)
 
     assert Map.has_key?(json, "links")
   end
diff --git a/test/serializer_test.exs b/test/serializer_test.exs
index 898d4eb..473ba0a 100644
--- a/test/serializer_test.exs
+++ b/test/serializer_test.exs
@@ -20,6 +20,36 @@ defmodule JSONAPISerializerTest do
 
     def fields, do: [:username, :first_name, :last_name]
     def type, do: "user"
+    def relationships do
+      [company: JSONAPISerializerTest.CompanyView]
+    end
+  end
+
+  defmodule CompanyView do
+    use JSONAPI.View
+
+    def fields, do: [:name]
+    def type, do: "company"
+    def relationships do
+      [industry: JSONAPISerializerTest.IndustryView]
+    end
+  end
+
+  defmodule IndustryView do
+    use JSONAPI.View
+
+    def fields, do: [:name]
+    def type, do: "industry"
+    def relationships do
+      [tags: JSONAPISerializerTest.TagView]
+    end
+  end
+
+  defmodule TagView do
+    use JSONAPI.View
+
+    def fields, do: [:name]
+    def type, do: "tag"
     def relationships, do: []
   end
 
@@ -211,6 +241,86 @@ defmodule JSONAPISerializerTest do
     assert Enum.count(encoded.included) == 4
   end
 
+  test "includes from the query when not included by default" do
+    data = %{
+      id: 1,
+      username: "jim",
+      first_name: "Jimmy",
+      last_name: "Beam",
+      company: %{ id: 2, name: "acme"}
+    }
+
+    conn = %Plug.Conn{
+      assigns: %{
+        jsonapi_query: %{
+          includes: [:company]
+        }
+      }
+    }
+
+    encoded = Serializer.serialize(UserView, data, conn)
+
+    assert encoded.data.relationships.company.links.self == "http://www.example.com/user/1/relationships/company"
+    assert Enum.count(encoded.included) == 1
+  end
+
+  test "includes nested items from the query when not included by default" do
+    data = %{
+      id: 1,
+      username: "jim",
+      first_name: "Jimmy",
+      last_name: "Beam",
+      company: %{id: 2, name: "acme", industry: %{id: 4, name: "stuff"}}
+    }
+
+    conn = %Plug.Conn{
+      assigns: %{
+        jsonapi_query: %{
+          includes: [company: :industry]
+        }
+      }
+    }
+
+    encoded = Serializer.serialize(UserView, data, conn)
+
+    assert encoded.data.relationships.company.links.self == "http://www.example.com/user/1/relationships/company"
+    assert Enum.count(encoded.included) == 2
+  end
+
+  test "includes items nested 2 layers deep from the query when not included by default" do
+    data = %{
+      id: 1,
+      username: "jim",
+      first_name: "Jimmy",
+      last_name: "Beam",
+      company: %{
+        id: 2,
+        name: "acme",
+        industry: %{
+          id: 4,
+          name: "stuff",
+          tags: [
+            %{id: 3, name: "a tag"},
+            %{id: 4, name: "another tag"}
+          ]
+        }
+      }
+    }
+
+    conn = %Plug.Conn{
+      assigns: %{
+        jsonapi_query: %{
+          includes: [company: [industry: :tags]]
+        }
+      }
+    }
+
+    encoded = Serializer.serialize(UserView, data, conn)
+
+    assert encoded.data.relationships.company.links.self == "http://www.example.com/user/1/relationships/company"
+    assert Enum.count(encoded.included) == 4
+  end
+
   test "serialize properly uses underscore_to_dash on both attributes and relationships" do
     data = %{
       id: 1,