Repository: jeregrine/jsonapi
Pull request #203 (2019-05-10 05:15:34)

Fix JSONApi sparse fieldset compliance for relationships
Changes:
- query_parser: change fields resolution to be based on type instead of relationship name.
  see: https://jsonapi.org/format/#fetching-sparse-fieldsets
- query_parser_test: fix a typo in test relationship definition

Relevant section from the spec:  "A client MAY request that an endpoint return only specific fields in the response on a per-type basis by including a fields[TYPE] parameter."


leepfrog: Thx!  Will make these changes and update this PR shortly.

Patch:
diff --git a/README.md b/README.md
index a7d6249..d742187 100644
--- a/README.md
+++ b/README.md
@@ -203,8 +203,8 @@ defmodule PageBasedPaginator do
     total_pages = Keyword.get(options, :total_pages, 0)
 
     %{
-      first: view.url_for_pagination(data, conn, %{page | "page" => "1"}),
-      last: view.url_for_pagination(data, conn, %{page | "page" => total_pages}),
+      first: view.url_for_pagination(data, conn, Map.put(page, "page", "1")),
+      last: view.url_for_pagination(data, conn, Map.put(page, "page", total_pages)),
       next: next_link(data, view, conn, number, size, total_pages),
       prev: previous_link(data, view, conn, number, size)
     }
diff --git a/lib/jsonapi/deprecation.ex b/lib/jsonapi/deprecation.ex
new file mode 100644
index 0000000..bc16aae
--- /dev/null
+++ b/lib/jsonapi/deprecation.ex
@@ -0,0 +1,17 @@
+defmodule JSONAPI.Deprecation do
+  @moduledoc """
+    Generate warnings in places where we want to deprecate functions or struct parameters
+  """
+
+  @doc """
+    Generates a deprecation warning for using `fields[relationship_key]` instead of `fields[type]` when
+    parsing query parameters.
+  """
+  def warn(:query_parser_fields) do
+    IO.warn(
+      "`JSONAPI.QueryParser` will no longer accept `fields` query params that refer to the relationship key of a `JSONAPI.View`.  Please use the `type` of the resource to perform filtering.
+      See: https://github.com/jeregrine/jsonapi/pull/203.",
+      Macro.Env.stacktrace(__ENV__)
+    )
+  end
+end
diff --git a/lib/jsonapi/plugs/query_parser.ex b/lib/jsonapi/plugs/query_parser.ex
index 7e4a0dc..b598112 100644
--- a/lib/jsonapi/plugs/query_parser.ex
+++ b/lib/jsonapi/plugs/query_parser.ex
@@ -1,6 +1,6 @@
 defmodule JSONAPI.QueryParser do
   @behaviour Plug
-  alias JSONAPI.Config
+  alias JSONAPI.{Config, Deprecation}
   alias JSONAPI.Exceptions.InvalidQuery
   alias Plug.Conn
   import JSONAPI.Utils.IncludeTree
@@ -246,12 +246,29 @@ defmodule JSONAPI.QueryParser do
 
   @spec get_view_for_type(module(), String.t()) :: module() | no_return()
   def get_view_for_type(view, type) do
-    case Enum.find(view.relationships, fn {k, _v} -> Atom.to_string(k) == type end) do
+    case Enum.find(view.relationships, fn relationship ->
+           is_field_valid_for_relationship(relationship, type)
+         end) do
       {_, view} -> view
       nil -> raise_invalid_field_names(type, view.type())
     end
   end
 
+  @spec is_field_valid_for_relationship({atom(), module()}, String.t()) :: boolean()
+  defp is_field_valid_for_relationship({key, view}, expected_type) do
+    cond do
+      view.type == expected_type ->
+        true
+
+      Atom.to_string(key) == expected_type ->
+        Deprecation.warn(:query_parser_fields)
+        true
+
+      true ->
+        false
+    end
+  end
+
   @spec raise_invalid_include_query(param :: String.t(), resource_type :: String.t()) ::
           no_return()
   defp raise_invalid_include_query(param, resource_type) do


Test patch:
diff --git a/test/jsonapi/plugs/query_parser_test.exs b/test/jsonapi/plugs/query_parser_test.exs
index f51cba3..29c1ef5 100644
--- a/test/jsonapi/plugs/query_parser_test.exs
+++ b/test/jsonapi/plugs/query_parser_test.exs
@@ -14,7 +14,7 @@ defmodule JSONAPI.QueryParserTest do
       [
         author: JSONAPI.QueryParserTest.UserView,
         comments: JSONAPI.QueryParserTest.CommentView,
-        best_friends: JSONAPI.QueryParsertTest.UserView
+        best_friends: JSONAPI.QueryParserTest.UserView
       ]
     end
   end
@@ -125,7 +125,11 @@ defmodule JSONAPI.QueryParserTest do
     end
   end
 
-  test "get_view_for_type/2" do
+  test "get_view_for_type/2 using view.type as key" do
+    assert get_view_for_type(MyView, "comment") == JSONAPI.QueryParserTest.CommentView
+  end
+
+  test "DEPRECATED: get_view_for_type/2 using relationship name as key" do
     assert get_view_for_type(MyView, "comments") == JSONAPI.QueryParserTest.CommentView
   end
 
@@ -140,8 +144,8 @@ defmodule JSONAPI.QueryParserTest do
   end
 
   test "get_view_for_type/2 raises on invalid fields" do
-    assert_raise InvalidQuery, "invalid fields, comment for type mytype", fn ->
-      get_view_for_type(MyView, "comment")
+    assert_raise InvalidQuery, "invalid fields, cupcake for type mytype", fn ->
+      get_view_for_type(MyView, "cupcake")
     end
   end
 end