Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nesting for includes #87

Merged
merged 8 commits into from
Jan 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 30 additions & 28 deletions lib/jsonapi/serializer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)))
Expand Down Expand Up @@ -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
Expand All @@ -179,4 +181,4 @@ defmodule JSONAPI.Serializer do
end

defp remove_links?, do: Application.get_env(:jsonapi, :remove_links, false)
end
end
147 changes: 140 additions & 7 deletions test/jsonapi_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ defmodule JSONAPITest do
def type, do: "mytype"
def relationships do
[author: {JSONAPITest.UserView, :include},
other_user: {JSONAPITest.UserView, :include}]
other_user: JSONAPITest.UserView]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these changes result in a breaking change? If not, let's leave the existing tests alone and add new ones to cover these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this changes the functionality. Although, I think it fixes the original intended functionality of :include meaning that the relationship will be included by default. I'm basing this on this bit from the readme:

  def relationships do
    # The post's author will be included by default
    [author: {MyApp.UserView, :include},
     comments: MyApp.CommentView]
  end

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end
end

Expand All @@ -18,6 +18,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

Expand Down Expand Up @@ -74,7 +104,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"
Expand All @@ -84,7 +114,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",
Expand Down Expand Up @@ -112,14 +142,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
Expand Down
110 changes: 110 additions & 0 deletions test/serializer_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down