better error responses

.github/workflows/test.yml

@@ -20,17 +20,6 @@ jobs:
         elixir: [1.12.1]
         otp: [24.2.1]
 
-    services:
-      postgres:
-        image: postgres
-        ports:
-          - 5432:5432
-        env:
-          POSTGRES_PASSWORD: postgres
-          POSTGRES_USER: postgres
-        options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
-
-
     steps:
     - uses: actions/checkout@v3
 

.gitignore

@@ -27,3 +27,5 @@ quarry-*.tar
 
 # Vim swap files
 *.swp
+
+*.db*

CHANGELOG.md

@@ -4,6 +4,8 @@
 <!-- next-header -->
 
 ## [Unreleased] - ReleaseDate
+Fix: Nested filter breaks without load option
+Add: Included `load_path` on sort and filter error types to show which subquery these errors belong to
 
 ## [0.3.0] - 2022-4-4
 ### Added

config/test.exs

@@ -8,9 +8,7 @@ config :quarry,
 
 config :quarry, Quarry.Repo,
   log: :debug,
-  username: "postgres",
-  password: "postgres",
-  database: "quarry_test",
-  hostname: "localhost",
+  adapter: Ecto.Adapters.SQLite3,
+  database: "#{Mix.env()}.db",
   pool: Ecto.Adapters.SQL.Sandbox,
   priv: "test/support"

lib/quarry.ex

@@ -128,6 +128,7 @@ defmodule Quarry do
   def build(schema, opts \\ []) do
     default_opts = %{
       binding_prefix: nil,
+      load_path: [],
       filter: %{},
       load: [],
       sort: [],
@@ -139,9 +140,9 @@ defmodule Quarry do
 
     {schema, []}
     |> From.build(opts.binding_prefix)
-    |> Filter.build(opts.filter)
-    |> Load.build(opts.load)
-    |> Sort.build(opts.sort)
+    |> Filter.build(opts.filter, opts.load_path)
+    |> Load.build(opts.load, opts.load_path)
+    |> Sort.build(opts.sort, opts.load_path)
     |> limit(opts.limit)
     |> offset(opts.offset)
   end

lib/quarry/filter.ex

@@ -6,13 +6,18 @@ defmodule Quarry.Filter do
 
   @type filter :: %{optional(atom()) => String.t() | number() | filter()}
 
-  @spec build({Ecto.Query.t(), [Quarry.error()]}, Quarry.filter()) ::
+  @spec build({Ecto.Query.t(), [Quarry.error()]}, Quarry.filter(), [atom()]) ::
           {Ecto.Query.t(), [Quarry.error()]}
-  def build({query, errors}, filters) do
+  def build({query, errors}, filters, load_path \\ []) do
     root_binding = From.get_root_binding(query)
     schema = From.get_root_schema(query)
 
-    filter({query, errors}, filters, binding: root_binding, schema: schema, path: [])
+    filter({query, errors}, filters,
+      binding: root_binding,
+      schema: schema,
+      path: [],
+      load_path: load_path
+    )
   end
 
   defp filter(acc, filters, state) do
@@ -26,17 +31,19 @@ defmodule Quarry.Filter do
     if (is_map(value) && field_name in association) || field_name in fields do
       filter_field(entry, {query, errors}, state)
     else
-      error = %{
-        type: :filter,
-        path: Enum.reverse([field_name | state[:path]]),
-        message:
-          "Quarry couldn't find field \"#{field_name}\" on Ecto schema \"#{state[:schema]}\""
-      }
-
-      {query, [error | errors]}
+      {query, [build_error(field_name, state) | errors]}
     end
   end
 
+  defp build_error(field_name, state) do
+    %{
+      type: :filter,
+      path: Enum.reverse([field_name | state[:path]]),
+      load_path: Enum.reverse(state[:load_path]),
+      message: "Quarry couldn't find field \"#{field_name}\" on Ecto schema \"#{state[:schema]}\""
+    }
+  end
+
   defp filter_field({field_name, child_filter}, acc, state) when is_map(child_filter) do
     child_schema = state[:schema].__schema__(:association, field_name).related
 

lib/quarry/load.ex

@@ -4,13 +4,15 @@ defmodule Quarry.Load do
 
   alias Quarry.{Join, From, QueryStruct}
 
-  @spec build({Ecto.Query.t(), [Quarry.error()]}, Quarry.load()) ::
+  @quarry_opts [:filter, :load, :sort, :limit, :offset]
+
+  @spec build({Ecto.Query.t(), [Quarry.error()]}, Quarry.load()[atom()]) ::
           {Ecto.Query.t(), [Quarry.error()]}
-  def build({query, errors}, load_params) do
+  def build({query, errors}, load_params, load_path \\ []) do
     root_binding = From.get_root_binding(query)
     schema = From.get_root_schema(query)
 
-    state = [binding: root_binding, schema: schema, path: []]
+    state = [binding: root_binding, schema: schema, local_path: [], path: load_path]
 
     load({query, errors}, load_params, state)
   end
@@ -31,56 +33,61 @@ defmodule Quarry.Load do
     if association do
       preload_tree({query, errors}, association, children, state)
     else
-      path =
-        state[:path]
-        |> List.insert_at(0, assoc)
-        |> Enum.reverse()
-
-      error = %{
-        type: :load,
-        path: path,
-        message:
-          "Quarry couldn't find filtering field \"#{Enum.join(path, ".")}\" on Ecto schema \"#{state[:schema]}\""
-      }
-
-      {query, [error | errors]}
+      {query, [build_error(assoc, state) | errors]}
     end
   end
 
+  defp build_error(field_name, state) do
+    %{
+      type: :load,
+      path: Enum.reverse([field_name | state[:local_path] ++ state[:path]]),
+      message: "Quarry couldn't find field \"#{field_name}\" on Ecto schema \"#{state[:schema]}\""
+    }
+  end
+
   defp preload_tree({query, errors}, %{cardinality: :one} = association, children, state) do
     %{queryable: child_schema, field: assoc} = association
     binding = Keyword.get(state, :binding)
-    path = [assoc | state[:path]]
+    local_path = [assoc | state[:local_path]]
 
     {query, join_binding} = Join.with_join(query, binding, assoc)
 
     query
-    |> QueryStruct.add_assoc(Enum.reverse(path), join_binding)
+    |> QueryStruct.add_assoc(Enum.reverse(local_path), join_binding)
     |> then(&{&1, errors})
-    |> load(children, binding: join_binding, schema: child_schema, path: path)
+    |> load(children,
+      binding: join_binding,
+      schema: child_schema,
+      local_path: local_path,
+      path: state[:path]
+    )
   end
 
   defp preload_tree({query, errors}, %{cardinality: :many} = association, children, state) do
     %{queryable: child_schema, field: assoc} = association
     binding = Keyword.get(state, :binding)
 
-    ordered_path =
-      state[:path]
-      |> List.insert_at(0, assoc)
-      |> Enum.reverse()
-
-    children = List.wrap(children)
-
-    {subquery, sub_errors} =
-      Quarry.build(child_schema,
-        filter: Keyword.get(children, :filter, %{}),
-        load: Keyword.get(children, :load, children),
-        sort: Keyword.get(children, :sort, []),
-        limit: Keyword.get(children, :limit),
-        offset: Keyword.get(children, :offset),
-        binding_prefix: binding
+    quarry_opts =
+      Keyword.merge(extract_nested_opts(children),
+        binding_prefix: binding,
+        load_path: [assoc | state[:local_path] ++ state[:path]]
       )
 
-    {QueryStruct.add_preload(query, ordered_path, subquery), sub_errors ++ errors}
+    {subquery, sub_errors} = Quarry.build(child_schema, quarry_opts)
+
+    ordered_local_path = Enum.reverse([assoc | state[:local_path]])
+
+    {QueryStruct.add_preload(query, ordered_local_path, subquery), sub_errors ++ errors}
+  end
+
+  defp extract_nested_opts(children) do
+    children
+    |> List.wrap()
+    |> Enum.filter(&is_tuple(&1))
+    |> Keyword.take(@quarry_opts)
+    |> case do
+      [] -> [load: children]
+      opts -> opts
+    end
   end
 end

lib/quarry/sort.ex

@@ -8,29 +8,37 @@ defmodule Quarry.Sort do
 
   @spec build({Ecto.Query.t(), [Quarry.error()]}, Quarry.sort()) ::
           {Ecto.Query.t(), [Qurry.error()]}
-  def build({query, errors}, keys) do
+  def build({query, errors}, keys, load_path \\ []) do
     root_binding = From.get_root_binding(query)
     schema = From.get_root_schema(query)
-    sort({query, errors}, root_binding, schema, [], keys)
+
+    state = [
+      schema: schema,
+      binding: root_binding,
+      load_path: load_path
+    ]
+
+    sort({query, errors}, [], keys, state)
   end
 
-  defp sort(acc, binding, schema, join_deps, keys) when is_list(keys) do
+  defp sort(acc, join_deps, keys, state) when is_list(keys) do
     Enum.reduce(
       keys,
       acc,
       fn entry, {query, errors} ->
         sort_key(entry, join_deps,
           query: query,
-          schema: schema,
-          binding: binding,
+          schema: state[:schema],
+          binding: state[:binding],
+          load_path: state[:load_path],
           errors: errors
         )
       end
     )
   end
 
-  defp sort(acc, binding, schema, join_deps, key),
-    do: sort(acc, binding, schema, join_deps, [key])
+  defp sort(acc, join_deps, key, state),
+    do: sort(acc, join_deps, [key], state)
 
   defp sort_key({dir, path}, join_deps, state), do: sort_key(path, dir, join_deps, state)
   defp sort_key(path, join_deps, state), do: sort_key(path, :asc, join_deps, state)
@@ -47,7 +55,7 @@ defmodule Quarry.Sort do
       state = Keyword.put(state, :schema, child_schema)
       sort_key(path, dir, [assoc | join_deps], state)
     else
-      error = build_error(assoc, join_deps, state[:schema])
+      error = build_error(assoc, join_deps, state)
       {state[:query], [error | state[:errors]]}
     end
   end
@@ -58,16 +66,17 @@ defmodule Quarry.Sort do
       query = Ecto.Query.order_by(query, [{^dir, field(as(^join_binding), ^field_name)}])
       {query, state[:errors]}
     else
-      error = build_error(field_name, join_deps, state[:schema])
+      error = build_error(field_name, join_deps, state)
       {state[:query], [error | state[:errors]]}
     end
   end
 
-  defp build_error(field, path, schema) do
+  defp build_error(field, path, state) do
     %{
       type: :sort,
       path: Enum.reverse([field | path]),
-      message: "Quarry couldn't find field \"#{field}\" on Ecto schema \"#{schema}\""
+      load_path: Enum.reverse(state[:load_path]),
+      message: "Quarry couldn't find field \"#{field}\" on Ecto schema \"#{state[:schema]}\""
     }
   end
 end

mix.exs

@@ -39,7 +39,7 @@ defmodule Quarry.MixProject do
       {:ex_doc, "~> 0.24", only: :dev, runtime: false},
       {:version_release, "~> 0.1.0", only: :dev, runtime: false},
       {:ecto_sql, "~> 3.5", only: [:dev, :test]},
-      {:postgrex, "~> 0.14", only: [:test]},
+      {:ecto_sqlite3, "~> 0.7", only: :test},
       {:ex_machina, "~> 2.3", only: [:test]},
       {:excoveralls, "~> 0.10", only: :test}
     ]

mix.lock

@@ -6,9 +6,12 @@
   "earmark_parser": {:hex, :earmark_parser, "1.4.18", "e1b2be73eb08a49fb032a0208bf647380682374a725dfb5b9e510def8397f6f2", [:mix], [], "hexpm", "114a0e85ec3cf9e04b811009e73c206394ffecfcc313e0b346de0d557774ee97"},
   "ecto": {:hex, :ecto, "3.7.1", "a20598862351b29f80f285b21ec5297da1181c0442687f9b8329f0445d228892", [:mix], [{:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "d36e5b39fc479e654cffd4dbe1865d9716e4a9b6311faff799b6f90ab81b8638"},
   "ecto_sql": {:hex, :ecto_sql, "3.7.1", "8de624ef50b2a8540252d8c60506379fbbc2707be1606853df371cf53df5d053", [:mix], [{:db_connection, "~> 2.2", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.4.0 or ~> 0.5.0", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.15.0 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:tds, "~> 2.1.1 or ~> 2.2", [hex: :tds, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "2b42a32e2ce92f64aba5c88617891ab3b0ba34f3f3a503fa20009eae1a401c81"},
+  "ecto_sqlite3": {:hex, :ecto_sqlite3, "0.7.4", "6128effc8a1e21f223a6f3875e156887a9e7f98e9d7bf1544e397f3d12a69318", [:mix], [{:decimal, "~> 1.6 or ~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.7", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.7", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:exqlite, "~> 0.9", [hex: :exqlite, repo: "hexpm", optional: false]}], "hexpm", "36235576dc14fed3a81f057b6c12a2c6192f3760aa7f61c29a016af525ab4b17"},
+  "elixir_make": {:hex, :elixir_make, "0.6.3", "bc07d53221216838d79e03a8019d0839786703129599e9619f4ab74c8c096eac", [:mix], [], "hexpm", "f5cbd651c5678bcaabdbb7857658ee106b12509cd976c2c2fca99688e1daf716"},
   "ex_doc": {:hex, :ex_doc, "0.26.0", "1922164bac0b18b02f84d6f69cab1b93bc3e870e2ad18d5dacb50a9e06b542a3", [:mix], [{:earmark_parser, "~> 1.4.0", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "2775d66e494a9a48355db7867478ffd997864c61c65a47d31c4949459281c78d"},
   "ex_machina": {:hex, :ex_machina, "2.7.0", "b792cc3127fd0680fecdb6299235b4727a4944a09ff0fa904cc639272cd92dc7", [:mix], [{:ecto, "~> 2.2 or ~> 3.0", [hex: :ecto, repo: "hexpm", optional: true]}, {:ecto_sql, "~> 3.0", [hex: :ecto_sql, repo: "hexpm", optional: true]}], "hexpm", "419aa7a39bde11894c87a615c4ecaa52d8f107bbdd81d810465186f783245bf8"},
   "excoveralls": {:hex, :excoveralls, "0.14.4", "295498f1ae47bdc6dce59af9a585c381e1aefc63298d48172efaaa90c3d251db", [:mix], [{:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "e3ab02f2df4c1c7a519728a6f0a747e71d7d6e846020aae338173619217931c1"},
+  "exqlite": {:hex, :exqlite, "0.10.2", "bb609c45ef5c8215dfe98dbf41a3b964ed0efc492f207c063e0584966b8f8d44", [:make, :mix], [{:db_connection, "~> 2.1", [hex: :db_connection, repo: "hexpm", optional: false]}, {:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "9727e053ba524e7526c801a7d8a2ac04ceb09fb4ab9b0b81c4128feb1e61d106"},
   "faker": {:hex, :faker, "0.17.0", "671019d0652f63aefd8723b72167ecdb284baf7d47ad3a82a15e9b8a6df5d1fa", [:mix], [], "hexpm", "a7d4ad84a93fd25c5f5303510753789fc2433ff241bf3b4144d3f6f291658a6a"},
   "hackney": {:hex, :hackney, "1.18.1", "f48bf88f521f2a229fc7bae88cf4f85adc9cd9bcf23b5dc8eb6a1788c662c4f6", [:rebar3], [{:certifi, "~>2.9.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~>6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~>1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~>1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "a4ecdaff44297e9b5894ae499e9a070ea1888c84afdd1fd9b7b2bc384950128e"},
   "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},

test/integration_test.exs

@@ -1,14 +1,9 @@
 defmodule Quarry.IntegrationTest do
-  use ExUnit.Case
+  use Quarry.DataCase
   doctest Quarry
 
   import Quarry.Factory
-  alias Quarry.{Context, Repo}
-
-  setup do
-    # Explicitly get a connection before each test
-    :ok = Ecto.Adapters.SQL.Sandbox.checkout(Repo)
-  end
+  alias Quarry.Context
 
   test "returns empty when entities" do
     assert Context.list_posts() == []

test/quarry/filter_test.exs

@@ -56,6 +56,11 @@ defmodule Quarry.FilterTest do
     assert %{path: [:author, :bad]} = error
   end
 
+  test "returns passed in load_path on errors", %{base: base} do
+    {_, [error]} = Filter.build(base, %{author: %{bad: "test"}}, [:post, :comments])
+    assert %{type: :filter, path: [:author, :bad], load_path: [:comments, :post]} = error
+  end
+
   test "can filter at multiple levels without duplicate join", %{base: base} do
     expected =
       from(