diff --git a/lib/jsonapi/plugs/query_parser.ex b/lib/jsonapi/plugs/query_parser.ex index 465fc548..d9458901 100644 --- a/lib/jsonapi/plugs/query_parser.ex +++ b/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()) diff --git a/lib/jsonapi/utils/include_tree.ex b/lib/jsonapi/utils/include_tree.ex index d5ead334..90d56a31 100644 --- a/lib/jsonapi/utils/include_tree.ex +++ b/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) diff --git a/test/jsonapi/plugs/query_parser_test.exs b/test/jsonapi/plugs/query_parser_test.exs index 0462301a..00f48225 100644 --- a/test/jsonapi/plugs/query_parser_test.exs +++ b/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) diff --git a/test/utils/include_tree_test.exs b/test/utils/include_tree_test.exs index cc304381..28e97730 100644 --- a/test/utils/include_tree_test.exs +++ b/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