feat: support nested filtering in the query parser (#366)

* feat: support nested filtering

Also renames a few private functions to clarify the difference between "validity" of a filter or include (represents a real option as determined by a view) vs. "allowed" (is specified in the allow-list passed to the QueryParser plug).

* chore: add requested more deeply nested tests

* fix: simplify and correct deep_merge conflict handling

* chore: add test that string/string conflicts result in the second string being chosen

Author: mattpolzin

lib/jsonapi/plugs/query_parser.ex

@@ -119,12 +119,15 @@ defmodule JSONAPI.QueryParser do
     opts_filter = Keyword.get(opts, :filter, [])
 
     Enum.reduce(filter, config, fn {key, val}, acc ->
-      check_filter_validity!(opts_filter, key, config)
-      %{acc | filter: Keyword.put(acc.filter, String.to_existing_atom(key), val)}
+      check_filter_allowed!(opts_filter, key, config)
+
+      keys = key |> String.split(".") |> Enum.map(&String.to_existing_atom/1)
+      filter = deep_merge(acc.filter, put_as_tree([], keys, val))
+      %{acc | filter: filter}
     end)
   end
 
-  defp check_filter_validity!(filters, key, config) do
+  defp check_filter_allowed!(filters, key, config) do
     unless key in filters do
       raise InvalidQuery, resource: config.view.type(), param: key, param_type: :filter
     end
@@ -215,7 +218,9 @@ defmodule JSONAPI.QueryParser do
   end
 
   defp include_reducer(config, valid_includes, inc, acc) do
-    check_include_validity!(inc, config)
+    # if an explicit list of allowed includes was specified, check this include
+    # against it:
+    check_include_allowed!(inc, config)
 
     if inc =~ ~r/\w+\.\w+/ do
       acc ++ handle_nested_include(inc, valid_includes, config)
@@ -236,25 +241,25 @@ defmodule JSONAPI.QueryParser do
     end
   end
 
-  defp check_include_validity!(key, %Config{opts: opts, view: view}) do
+  defp check_include_allowed!(key, %Config{opts: opts, view: view}) do
     if opts do
-      check_include_validity!(key, Keyword.get(opts, :include), view)
+      check_include_allowed!(key, Keyword.get(opts, :include), view)
     end
   end
 
-  defp check_include_validity!(key, allowed_includes, view) when is_list(allowed_includes) do
+  defp check_include_allowed!(key, allowed_includes, view) when is_list(allowed_includes) do
     unless key in allowed_includes do
       raise_invalid_include_query(key, view.type())
     end
   end
 
-  defp check_include_validity!(_key, nil, _view) do
+  defp check_include_allowed!(_key, nil, _view) do
     # all includes are allowed if none are specified in input config
   end
 
   @spec handle_nested_include(key :: String.t(), valid_include :: list(), config :: Config.t()) ::
           list() | no_return()
-  def handle_nested_include(key, valid_include, config) do
+  def handle_nested_include(key, valid_includes, config) do
     keys =
       try do
         key
@@ -267,7 +272,7 @@ defmodule JSONAPI.QueryParser do
     last = List.last(keys)
     path = Enum.slice(keys, 0, Enum.count(keys) - 1)
 
-    if member_of_tree?(keys, valid_include) do
+    if member_of_tree?(keys, valid_includes) do
       put_as_tree([], path, last)
     else
       raise_invalid_include_query(key, config.view.type())

lib/jsonapi/utils/include_tree.ex

@@ -3,6 +3,22 @@ defmodule JSONAPI.Utils.IncludeTree do
   Internal utility for building trees of resource relationships
   """
 
+  @spec deep_merge(Keyword.t(), Keyword.t()) :: Keyword.t()
+  def deep_merge(acc, []), do: acc
+
+  def deep_merge(acc, [{key, val} | tail]) do
+    acc
+    |> Keyword.update(
+      key,
+      val,
+      fn
+        [_first | _rest] = old_val when is_list(val) -> deep_merge(old_val, val)
+        _ -> val
+      end
+    )
+    |> deep_merge(tail)
+  end
+
   @spec put_as_tree(term(), term(), term()) :: term()
   def put_as_tree(acc, items, val) do
     [head | tail] = Enum.reverse(items)

test/jsonapi/plugs/query_parser_test.exs

@@ -64,12 +64,31 @@ defmodule JSONAPI.QueryParserTest do
     end
   end
 
-  test "parse_filter/2 turns filters key/val pairs" do
+  test "parse_filter/2 returns filters key/val pairs" do
     config = struct(Config, opts: [filter: ~w(name)], view: MyView)
     filter = parse_filter(config, %{"name" => "jason"}).filter
     assert filter[:name] == "jason"
   end
 
+  test "parse_filter/2 handles nested filters" do
+    config = struct(Config, opts: [filter: ~w(author.username)], view: MyView)
+    filter = parse_filter(config, %{"author.username" => "jason"}).filter
+    assert filter[:author][:username] == "jason"
+  end
+
+  test "parse_filter/2 handles nested filters two deep" do
+    config = struct(Config, opts: [filter: ~w(author.top_posts.text)], view: MyView)
+    filter = parse_filter(config, %{"author.top_posts.text" => "some post"}).filter
+    assert filter[:author][:top_posts][:text] == "some post"
+  end
+
+  test "parse_filter/2 handles nested filters with overlap" do
+    config = struct(Config, opts: [filter: ~w(author.username author.id)], view: MyView)
+    filter = parse_filter(config, %{"author.username" => "jason", "author.id" => "123"}).filter
+    assert filter[:author][:username] == "jason"
+    assert filter[:author][:id] == "123"
+  end
+
   test "parse_filter/2 raises on invalid filters" do
     config = struct(Config, opts: [], view: MyView)
 
@@ -84,11 +103,24 @@ defmodule JSONAPI.QueryParserTest do
     assert parse_include(config, "author").include == [:author]
     assert parse_include(config, "comments,author").include == [:comments, :author]
     assert parse_include(config, "comments.user").include == [comments: :user]
+    assert parse_include(config, "comments.user.top_posts").include == [comments: [user: :top_posts]]
     assert parse_include(config, "best_friends").include == [:best_friends]
     assert parse_include(config, "author.top-posts").include == [author: :top_posts]
     assert parse_include(config, "").include == []
   end
 
+  test "parse_include/2 succeds given valid nested include specified in allowed list" do
+    config = struct(Config, view: MyView, opts: [include: ~w(comments.user)])
+
+    assert parse_include(config, "comments.user").include == [comments: :user]
+  end
+
+  test "parse_include/2 succeds given valid twice-nested include specified in allowed list" do
+    config = struct(Config, view: MyView, opts: [include: ~w(comments.user.top_posts)])
+
+    assert parse_include(config, "comments.user.top_posts").include == [comments: [user: :top_posts]]
+  end
+
   test "parse_include/2 errors with invalid includes" do
     config = struct(Config, view: MyView)
 

test/utils/include_tree_test.exs

@@ -6,4 +6,18 @@ defmodule JSONAPI.IncludeTreeTest do
     items = [:test, :the, :path]
     assert put_as_tree([], items, :boo) == [test: [the: [path: :boo]]]
   end
+
+  test "deep_merge/2 handles string/keyword conflict by choosing second value" do
+    # one direction
+    assert [other: "thing", hi: [hello: "there"]] = deep_merge([other: "thing", hi: "there"], hi: [hello: "there"])
+    # the other direction
+    assert [hi: "there", other: "thing"] = deep_merge([hi: [hello: "there"]], other: "thing", hi: "there")
+  end
+
+  test "deep_merge/2 handles string/string conflict by choosing second value" do
+    # one direction
+    assert [hi: "there"] = deep_merge([hi: "hello"], hi: "there")
+    # the other direction
+    assert [hi: "hello"] = deep_merge([hi: "there"], hi: "hello")
+  end
 end