QueryBuilder follow-up (#5971)

* add QueryError struct

Also adds back the behaviour that segment update/insert will crash when
an unexpected non-filters-related query builder error is encountered.

* add querybuilder convenience fn

* remove unreachable case clause

* rename file

* remove redundant braces and type def

Author: RobertJoonas

extra/lib/plausible/stats/consolidated_view.ex

@@ -90,12 +90,12 @@ defmodule Plausible.Stats.ConsolidatedView do
     to_datetime = now |> DateTime.from_naive!("Etc/UTC")
 
     graph_query =
-      QueryBuilder.build!(view, %ParsedQueryParams{
+      QueryBuilder.build!(view,
         metrics: [:visitors],
         input_date_range: {:datetime_range, from_datetime, to_datetime},
         dimensions: ["time:hour"],
         order_by: [{"time:hour", :asc}]
-      })
+      )
 
     %Stats.QueryResult{results: results} = Stats.query(view, graph_query)
 

extra/lib/plausible_web/live/funnel_settings/form.ex

@@ -10,7 +10,7 @@ defmodule PlausibleWeb.Live.FunnelSettings.Form do
 
   import PlausibleWeb.Live.Components.Form
   alias Plausible.{Goals, Funnels}
-  alias Plausible.Stats.{QueryBuilder, ParsedQueryParams}
+  alias Plausible.Stats.QueryBuilder
 
   def mount(_params, %{"domain" => domain} = session, socket) do
     site =
@@ -350,10 +350,10 @@ defmodule PlausibleWeb.Live.FunnelSettings.Form do
       )
 
     query =
-      QueryBuilder.build!(site, %ParsedQueryParams{
+      QueryBuilder.build!(site,
         metrics: [:pageviews],
         input_date_range: :month
-      })
+      )
 
     {:ok, {definition, query}}
   end

lib/plausible/segments/filters.ex

@@ -3,7 +3,7 @@ defmodule Plausible.Segments.Filters do
   This module contains functions that enable resolving segments in filters.
   """
   alias Plausible.Segments
-  alias Plausible.Stats.{Filters, ApiQueryParser}
+  alias Plausible.Stats.{Filters, ApiQueryParser, QueryError}
 
   @max_segment_filters_count 10
 
@@ -15,7 +15,7 @@ defmodule Plausible.Segments.Filters do
     {:ok, [10, 20]}
 
     iex> get_segment_ids([[:and, [[:is, "segment", Enum.to_list(1..6)], [:is, "segment", Enum.to_list(1..6)]]]])
-    {:error, "Invalid filters. You can only use up to 10 segment filters in a query."}
+    {:error, %Plausible.Stats.QueryError{code: :invalid_filters, message: "Invalid filters. You can only use up to 10 segment filters in a query."}}
   """
   def get_segment_ids(filters) do
     ids =
@@ -28,7 +28,11 @@ defmodule Plausible.Segments.Filters do
 
     if length(ids) > @max_segment_filters_count do
       {:error,
-       "Invalid filters. You can only use up to #{@max_segment_filters_count} segment filters in a query."}
+       %QueryError{
+         code: :invalid_filters,
+         message:
+           "Invalid filters. You can only use up to #{@max_segment_filters_count} segment filters in a query."
+       }}
     else
       {:ok, Enum.uniq(ids)}
     end
@@ -56,7 +60,12 @@ defmodule Plausible.Segments.Filters do
             )},
          :ok <-
            if(Enum.any?(segment_ids, fn id -> is_nil(Map.get(segments_by_id, id)) end),
-             do: {:error, "Invalid filters. Some segments don't exist or aren't accessible."},
+             do:
+               {:error,
+                %QueryError{
+                  code: :invalid_filters,
+                  message: "Invalid filters. Some segments don't exist or aren't accessible."
+                }},
              else: :ok
            ) do
       {:ok, segments_by_id}

lib/plausible/segments/segment.ex

@@ -5,7 +5,7 @@ defmodule Plausible.Segments.Segment do
   use Plausible
   use Ecto.Schema
   import Ecto.Changeset
-  alias Plausible.Stats.{ApiQueryParser, QueryBuilder, ParsedQueryParams}
+  alias Plausible.Stats.{ApiQueryParser, QueryBuilder, ParsedQueryParams, QueryError}
 
   @segment_types [:personal, :site]
 
@@ -126,7 +126,11 @@ defmodule Plausible.Segments.Segment do
          :ok <- maybe_validate_filters_depth(parsed_filters, restricted_depth?) do
       :ok
     else
-      {:error, message} -> {:error, {:invalid_filters, message}}
+      {:error, %QueryError{code: :invalid_filters, message: message}} ->
+        {:error, {:invalid_filters, message}}
+
+      {:error, :deep_filters_not_supported} ->
+        {:error, {:invalid_filters, "Invalid filters. Deep filters are not supported."}}
     end
   end
 
@@ -138,7 +142,7 @@ defmodule Plausible.Segments.Segment do
     if Enum.all?(filters, &dashboard_compatible_filter?/1) do
       :ok
     else
-      {:error, "Invalid filters. Deep filters are not supported."}
+      {:error, :deep_filters_not_supported}
     end
   end
 

lib/plausible/stats/api_query_parser.ex

@@ -3,7 +3,7 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   use Plausible
 
-  alias Plausible.Stats.{Filters, Metrics, JSONSchema}
+  alias Plausible.Stats.{Filters, Metrics, JSONSchema, QueryError}
 
   @default_include %Plausible.Stats.QueryInclude{
     imports: false,
@@ -50,8 +50,12 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   defp parse_metric(metric_str) do
     case Metrics.from_string(metric_str) do
-      {:ok, metric} -> {:ok, metric}
-      _ -> {:error, "Unknown metric '#{i(metric_str)}'."}
+      {:ok, metric} ->
+        {:ok, metric}
+
+      _ ->
+        {:error,
+         %QueryError{code: :invalid_metrics, message: "Unknown metric '#{i(metric_str)}'."}}
     end
   end
 
@@ -82,7 +86,11 @@ defmodule Plausible.Stats.ApiQueryParser do
   defp parse_operator(["not" | _rest]), do: {:ok, :not}
   defp parse_operator(["has_done" | _rest]), do: {:ok, :has_done}
   defp parse_operator(["has_not_done" | _rest]), do: {:ok, :has_not_done}
-  defp parse_operator(filter), do: {:error, "Unknown operator for filter '#{i(filter)}'."}
+
+  defp parse_operator(filter),
+    do:
+      {:error,
+       %QueryError{code: :invalid_filters, message: "Unknown operator for filter '#{i(filter)}'."}}
 
   def parse_filter_second(operator, [_, filters | _rest]) when operator in [:and, :or],
     do: parse_filters(filters)
@@ -97,7 +105,8 @@ defmodule Plausible.Stats.ApiQueryParser do
     parse_filter_dimension_string(filter_dimension, "Invalid filter '#{i(filter)}'.")
   end
 
-  defp parse_filter_dimension(filter), do: {:error, "Invalid filter '#{i(filter)}'."}
+  defp parse_filter_dimension(filter),
+    do: {:error, %QueryError{code: :invalid_filters, message: "Invalid filter '#{i(filter)}'."}}
 
   defp parse_filter_rest(operator, filter)
        when operator in [
@@ -133,7 +142,11 @@ defmodule Plausible.Stats.ApiQueryParser do
           {:ok, list}
         else
           {:error,
-           "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code."}
+           %QueryError{
+             code: :invalid_filters,
+             message:
+               "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code."
+           }}
         end
 
       {"segment", _} when all_integers? ->
@@ -143,11 +156,12 @@ defmodule Plausible.Stats.ApiQueryParser do
         {:ok, list}
 
       _ ->
-        {:error, "Invalid filter '#{i(filter)}'."}
+        {:error, %QueryError{code: :invalid_filters, message: "Invalid filter '#{i(filter)}'."}}
     end
   end
 
-  defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'."}
+  defp parse_clauses_list(filter),
+    do: {:error, %QueryError{code: :invalid_filters, message: "Invalid filter '#{i(filter)}'."}}
 
   defp parse_filter_modifiers(modifiers) when is_map(modifiers) do
     {:ok, [atomize_keys(modifiers)]}
@@ -164,9 +178,15 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   defp parse_input_date_range(shorthand) when is_binary(shorthand) do
     case Integer.parse(shorthand) do
-      {n, "d"} when n > 0 and n <= 5_000 -> {:ok, {:last_n_days, n}}
-      {n, "mo"} when n > 0 and n <= 100 -> {:ok, {:last_n_months, n}}
-      _ -> {:error, "Invalid date_range #{i(shorthand)}"}
+      {n, "d"} when n > 0 and n <= 5_000 ->
+        {:ok, {:last_n_days, n}}
+
+      {n, "mo"} when n > 0 and n <= 100 ->
+        {:ok, {:last_n_months, n}}
+
+      _ ->
+        {:error,
+         %QueryError{code: :invalid_date_range, message: "Invalid date_range #{i(shorthand)}"}}
     end
   end
 
@@ -178,7 +198,7 @@ defmodule Plausible.Stats.ApiQueryParser do
   end
 
   defp parse_input_date_range(unknown) do
-    {:error, "Invalid date_range #{i(unknown)}"}
+    {:error, %QueryError{code: :invalid_date_range, message: "Invalid date_range #{i(unknown)}"}}
   end
 
   defp parse_date_strings(from, to) do
@@ -193,7 +213,9 @@ defmodule Plausible.Stats.ApiQueryParser do
          {:ok, to_datetime, _offset} <- DateTime.from_iso8601(to) do
       {:ok, {:datetime_range, from_datetime, to_datetime}}
     else
-      _ -> {:error, "Invalid date_range '#{i([from, to])}'."}
+      _ ->
+        {:error,
+         %QueryError{code: :invalid_date_range, message: "Invalid date_range '#{i([from, to])}'."}}
     end
   end
 
@@ -211,7 +233,11 @@ defmodule Plausible.Stats.ApiQueryParser do
   end
 
   def parse_order_by(nil), do: {:ok, nil}
-  def parse_order_by(order_by), do: {:error, "Invalid order_by '#{i(order_by)}'."}
+
+  def parse_order_by(order_by),
+    do:
+      {:error,
+       %QueryError{code: :invalid_order_by, message: "Invalid order_by '#{i(order_by)}'."}}
 
   defp parse_order_by_entry(entry) do
     with {:ok, value} <- parse_metric_or_dimension(entry),
@@ -227,7 +253,7 @@ defmodule Plausible.Stats.ApiQueryParser do
     } do
       {{:ok, time}, _} -> {:ok, time}
       {_, {:ok, dimension}} -> {:ok, dimension}
-      _ -> {:error, error_message}
+      _ -> {:error, %QueryError{code: :invalid_dimensions, message: error_message}}
     end
   end
 
@@ -237,10 +263,18 @@ defmodule Plausible.Stats.ApiQueryParser do
       parse_metric(value),
       parse_filter_dimension_string(value)
     } do
-      {{:ok, time}, _, _} -> {:ok, time}
-      {_, {:ok, metric}, _} -> {:ok, metric}
-      {_, _, {:ok, dimension}} -> {:ok, dimension}
-      _ -> {:error, "Invalid order_by entry '#{i(entry)}'."}
+      {{:ok, time}, _, _} ->
+        {:ok, time}
+
+      {_, {:ok, metric}, _} ->
+        {:ok, metric}
+
+      {_, _, {:ok, dimension}} ->
+        {:ok, dimension}
+
+      _ ->
+        {:error,
+         %QueryError{code: :invalid_order_by, message: "Invalid order_by entry '#{i(entry)}'."}}
     end
   end
 
@@ -254,15 +288,19 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   defp parse_order_direction([_, "asc"]), do: {:ok, :asc}
   defp parse_order_direction([_, "desc"]), do: {:ok, :desc}
-  defp parse_order_direction(entry), do: {:error, "Invalid order_by entry '#{i(entry)}'."}
+
+  defp parse_order_direction(entry),
+    do:
+      {:error,
+       %QueryError{code: :invalid_order_by, message: "Invalid order_by entry '#{i(entry)}'."}}
 
   def parse_include(include) when is_map(include) do
     parsed_include_params_or_error =
       include
       |> Enum.reduce_while({:ok, []}, fn {key, value}, {:ok, acc} ->
         case parse_include_entry(key, value) do
           {:ok, parsed_tuple} -> {:cont, {:ok, acc ++ [parsed_tuple]}}
-          {:error, msg} -> {:halt, {:error, msg}}
+          {:error, query_error} -> {:halt, {:error, query_error}}
         end
       end)
 
@@ -272,15 +310,18 @@ defmodule Plausible.Stats.ApiQueryParser do
   end
 
   def parse_include(nil), do: {:ok, @default_include}
-  def parse_include(include), do: {:error, "Invalid include '#{i(include)}'."}
+
+  def parse_include(include),
+    do: {:error, %QueryError{code: :invalid_include, message: "Invalid include '#{i(include)}'."}}
 
   @allowed_include_keys Enum.map(Map.keys(@default_include), &Atom.to_string/1)
 
   defp parse_include_entry(key, value) when key in @allowed_include_keys do
     {:ok, {String.to_existing_atom(key), value}}
   end
 
-  defp parse_include_entry(key, _value), do: {:error, "Invalid include key'#{i(key)}'."}
+  defp parse_include_entry(key, _value),
+    do: {:error, %QueryError{code: :invalid_include, message: "Invalid include key'#{i(key)}'."}}
 
   defp parse_pagination(pagination) when is_map(pagination) do
     {:ok, Map.merge(@default_pagination, atomize_keys(pagination))}
@@ -300,28 +341,28 @@ defmodule Plausible.Stats.ApiQueryParser do
         if String.length(property_name) > 0 do
           {:ok, dimension}
         else
-          {:error, error_message}
+          {:error, %QueryError{code: :invalid_filters, message: error_message}}
         end
 
       "event:" <> key ->
         if key in Filters.event_props() do
           {:ok, dimension}
         else
-          {:error, error_message}
+          {:error, %QueryError{code: :invalid_filters, message: error_message}}
         end
 
       "visit:" <> key ->
         if key in Filters.visit_props() do
           {:ok, dimension}
         else
-          {:error, error_message}
+          {:error, %QueryError{code: :invalid_filters, message: error_message}}
         end
 
       "segment" ->
         {:ok, dimension}
 
       _ ->
-        {:error, error_message}
+        {:error, %QueryError{code: :invalid_filters, message: error_message}}
     end
   end
 

lib/plausible/stats/goal_suggestions.ex

@@ -2,7 +2,7 @@ defmodule Plausible.Stats.GoalSuggestions do
   @moduledoc false
 
   alias Plausible.{Repo, ClickhouseRepo}
-  alias Plausible.Stats.{Query, QueryBuilder, ParsedQueryParams, QueryInclude}
+  alias Plausible.Stats.{Query, QueryBuilder}
   import Plausible.Stats.Base
   import Ecto.Query
 
@@ -39,11 +39,11 @@ defmodule Plausible.Stats.GoalSuggestions do
     from_date = Date.shift(to_date, month: -6)
 
     query =
-      QueryBuilder.build!(site, %ParsedQueryParams{
+      QueryBuilder.build!(site,
         input_date_range: {:date_range, from_date, to_date},
         metrics: [:pageviews],
-        include: %QueryInclude{imports: true}
-      })
+        include: [imports: true]
+      )
 
     native_q =
       from(e in base_event_query(query),

lib/plausible/stats/json_schema.ex

@@ -7,6 +7,7 @@ defmodule Plausible.Stats.JSONSchema do
   """
   use Plausible
   alias Plausible.Stats.JSONSchema.Utils
+  alias Plausible.Stats.QueryError
 
   @json_schema_filepath "priv/json-schemas/query-api-schema.json"
   @external_resource @json_schema_filepath
@@ -28,8 +29,12 @@ defmodule Plausible.Stats.JSONSchema do
 
   def validate(params) do
     case ExJsonSchema.Validator.validate(@query_schema, params) do
-      :ok -> :ok
-      {:error, errors} -> {:error, format_errors(errors, params)}
+      :ok ->
+        :ok
+
+      {:error, errors} ->
+        {:error,
+         %QueryError{code: :failed_schema_validation, message: format_errors(errors, params)}}
     end
   end