Remove internal query api schema (#5969)

* move goal name validation test

* move realtime query construction tests

* Drop include.compare parsing tests

These are focused solely on the parsing part and not the utc_time_range
construction which is tested in Plausible.Stats.ComparisonsTest.

* Move filter validation to QueryBuilder + tests

* move relative_date param tests

* make tests use public schema where possible

* fixup: remove duplicate describe block

* add tests for passing filter validation

...and remove internal operators to stick to testing only the public
schema in QueryParseAndBuildTest

* remove time:minute dimension validation test

* move exit_rate metric validation tests

* make test meaningful again

has_not_done is available in the public API too. Use matches_wildcard
instead.

* move consolidated_site_ids tests

* use QueryBuilder.build for ad-hoc query building where possible

* adjust api_query_parser error messages

* use QueryBuilder for validating segments too

* remove :internal query api schema

* clean up ApiQueryParser of internal-only behaviour

* add segment validtion comment

Author: RobertJoonas

assets/js/types/query-api.d.ts

@@ -12,13 +12,10 @@ export type Metric =
   | "conversion_rate"
   | "group_conversion_rate"
   | "time_on_page"
-  | "exit_rate"
   | "total_revenue"
   | "average_revenue"
   | "scroll_depth";
 export type DateRangeShorthand =
-  | "30m"
-  | "realtime"
   | "all"
   | "day"
   | "7d"
@@ -71,7 +68,7 @@ export type SimpleFilterDimensions =
   | "visit:exit_page_hostname";
 export type CustomPropertyFilterDimensions = string;
 export type GoalDimension = "event:goal";
-export type TimeDimensions = ("time" | "time:month" | "time:week" | "time:day" | "time:hour") | "time:minute";
+export type TimeDimensions = "time" | "time:month" | "time:week" | "time:day" | "time:hour";
 export type FilterTree = FilterEntry | FilterAndOr | FilterNot | FilterHasDone;
 export type FilterEntry = FilterWithoutGoals | FilterWithIs | FilterWithContains | FilterWithPattern;
 /**
@@ -126,7 +123,7 @@ export type FilterWithContains =
  * @maxItems 3
  */
 export type FilterWithPattern = [
-  FilterOperationRegex | ("matches_wildcard" | "matches_wildcard_not"),
+  FilterOperationRegex,
   SimpleFilterDimensions | CustomPropertyFilterDimensions,
   Clauses
 ];
@@ -157,7 +154,6 @@ export type OrderByEntry = [
   Metric | SimpleFilterDimensions | CustomPropertyFilterDimensions | TimeDimensions,
   "asc" | "desc"
 ];
-export type ComparisonMode = "previous_period" | "year_over_year";
 
 export interface QueryApiSchema {
   /**
@@ -170,7 +166,6 @@ export interface QueryApiSchema {
    * @minItems 1
    */
   metrics: [Metric, ...Metric[]];
-  date?: string;
   /**
    * Date range to query
    */
@@ -198,14 +193,6 @@ export interface QueryApiSchema {
      * If set and using `day`, `month` or `year` date_ranges, the query will be trimmed to the current date
      */
     trim_relative_date_range?: boolean;
-    /**
-     * If set, executes the same query but over a comparison date range
-     */
-    compare?: DateRange | ComparisonMode | never;
-    /**
-     * With the `compare` option, if set and using time:day dimensions, day-of-week of comparison query is matched
-     */
-    compare_match_day_of_week?: boolean;
   };
   pagination?: {
     /**

extra/lib/plausible/stats/consolidated_view.ex

@@ -86,25 +86,16 @@ defmodule Plausible.Stats.ConsolidatedView do
   end
 
   defp query_24h_intervals(view, now) do
+    from_datetime = now |> NaiveDateTime.shift(hour: -24) |> DateTime.from_naive!("Etc/UTC")
+    to_datetime = now |> DateTime.from_naive!("Etc/UTC")
+
     graph_query =
-      Stats.Query.parse_and_build!(
-        view,
-        :internal,
-        %{
-          "site_id" => view.domain,
-          "metrics" => ["visitors"],
-          "date_range" => [
-            NaiveDateTime.shift(now, hour: -24)
-            |> DateTime.from_naive!("Etc/UTC")
-            |> DateTime.to_iso8601(),
-            now
-            |> DateTime.from_naive!("Etc/UTC")
-            |> DateTime.to_iso8601()
-          ],
-          "dimensions" => ["time:hour"],
-          "order_by" => [["time:hour", "asc"]]
-        }
-      )
+      QueryBuilder.build!(view, %ParsedQueryParams{
+        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,6 +10,7 @@ defmodule PlausibleWeb.Live.FunnelSettings.Form do
 
   import PlausibleWeb.Live.Components.Form
   alias Plausible.{Goals, Funnels}
+  alias Plausible.Stats.{QueryBuilder, ParsedQueryParams}
 
   def mount(_params, %{"domain" => domain} = session, socket) do
     site =
@@ -349,15 +350,10 @@ defmodule PlausibleWeb.Live.FunnelSettings.Form do
       )
 
     query =
-      Plausible.Stats.Query.parse_and_build!(
-        site,
-        :internal,
-        %{
-          "site_id" => site.domain,
-          "date_range" => "month",
-          "metrics" => ["pageviews"]
-        }
-      )
+      QueryBuilder.build!(site, %ParsedQueryParams{
+        metrics: [:pageviews],
+        input_date_range: :month
+      })
 
     {:ok, {definition, query}}
   end

lib/plausible/segments/segment.ex

@@ -5,6 +5,7 @@ defmodule Plausible.Segments.Segment do
   use Plausible
   use Ecto.Schema
   import Ecto.Changeset
+  alias Plausible.Stats.{ApiQueryParser, QueryBuilder, ParsedQueryParams}
 
   @segment_types [:personal, :site]
 
@@ -102,77 +103,33 @@ defmodule Plausible.Segments.Segment do
       ),
       do: :ok
 
-  def validate_segment_data_if_exists(%Plausible.Site{} = site, segment_data, restricted_depth?),
-    do: validate_segment_data(site, segment_data, restricted_depth?)
-
-  def validate_segment_data(
-        %Plausible.Site{} = site,
-        %{"filters" => filters},
-        restricted_depth?
-      ) do
-    with {:ok, %Plausible.Stats.Query{filters: parsed_filters}} <-
-           build_naive_query_from_segment_data(site, filters),
-         :ok <- maybe_validate_filters_depth(parsed_filters, restricted_depth?) do
-      :ok
-    else
-      {:error, message} ->
-        reformat_filters_errors(message)
-
-      :error_deep_filters_not_supported ->
-        reformat_filters_errors("Invalid filters. Deep filters are not supported.")
-    end
+  def validate_segment_data_if_exists(%Plausible.Site{} = site, segment_data, restricted_depth?) do
+    validate_segment_data(site, segment_data, restricted_depth?)
   end
 
+  @spec validate_segment_data(Plausible.Site.t(), map(), boolean()) ::
+          :ok | {:error, {:invalid_filters, String.t()}}
   @doc """
-    This function builds a simple query using the filters from Plausibe.Segment.segment_data
-    to test whether the filters used in the segment stand as legitimate query filters.
-    If they don't, it indicates an error with the filters that must be passed to the client,
-    so they could reconfigure the filters.
+  Checks whether the filters in segment_data are valid by attempting to build
+  a `%Query{}` struct with them. The `metrics` and `input_date_range` values
+  do not matter as long as they don't affect filters being the only field
+  affecting query validity.
   """
-  def build_naive_query_from_segment_data(%Plausible.Site{} = site, filters),
-    do:
-      Plausible.Stats.Query.parse_and_build(
-        site,
-        :internal,
-        %{
-          "site_id" => site.domain,
-          "metrics" => ["visitors"],
-          "date_range" => "7d",
-          "filters" => filters
-        },
-        %{}
-      )
-
-  @doc """
-    This function handles the error from building the naive query that is used to validate segment filters.
-    If the error is only about filters, it's marked as :invalid_filters error and ultimately forwarded to client.
-    If the error is not only about filters, the client can't do anything about the situation,
-    and the error message is returned as-is.
-
-    ### Examples
-    iex> reformat_filters_errors(~s(#/metrics/0 Invalid metric "Visitors"\\n#/filters/0 Invalid filter "A"))
-    {:error, ~s(#/metrics/0 Invalid metric "Visitors"\\n#/filters/0 Invalid filter "A")}
-
-    iex> reformat_filters_errors(~s(#/filters/0 Invalid filter "A"\\n#/filters/1 Invalid filter "B"))
-    {:error, {:invalid_filters, ~s(#/filters/0 Invalid filter "A"\\n#/filters/1 Invalid filter "B")}}
-
-    iex> reformat_filters_errors("Invalid filters. Dimension `event:goal` can only be filtered at the top level.")
-    {:error, {:invalid_filters, "Invalid filters. Dimension `event:goal` can only be filtered at the top level."}}
-  """
-  def reformat_filters_errors(message) do
-    lines = String.split(message, "\n")
-
-    if Enum.all?(lines, fn line ->
-         String.starts_with?(line, "#/filters/") or String.starts_with?(line, "Invalid filters.")
-       end) do
-      {:error, {:invalid_filters, message}}
+  def validate_segment_data(%Plausible.Site{} = site, %{"filters" => filters}, restricted_depth?) do
+    with {:ok, parsed_filters} <- ApiQueryParser.parse_filters(filters),
+         {:ok, _} <-
+           QueryBuilder.build(site, %ParsedQueryParams{
+             metrics: [:visitors],
+             input_date_range: {:last_n_days, 7},
+             filters: parsed_filters
+           }),
+         :ok <- maybe_validate_filters_depth(parsed_filters, restricted_depth?) do
+      :ok
     else
-      {:error, message}
+      {:error, message} -> {:error, {:invalid_filters, message}}
     end
   end
 
-  @spec maybe_validate_filters_depth([any()], boolean()) ::
-          :ok | :error_deep_filters_not_supported
   defp maybe_validate_filters_depth(filters, restricted_depth?)
 
   defp maybe_validate_filters_depth(_filters, false), do: :ok
@@ -181,7 +138,7 @@ defmodule Plausible.Segments.Segment do
     if Enum.all?(filters, &dashboard_compatible_filter?/1) do
       :ok
     else
-      :error_deep_filters_not_supported
+      {:error, "Invalid filters. Deep filters are not supported."}
     end
   end
 

lib/plausible/stats/api_query_parser.ex

@@ -22,11 +22,9 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   def default_pagination(), do: @default_pagination
 
-  def parse(schema_type, params) when is_map(params) do
-    input_date_range = Map.get(params, "date_range")
-
-    with :ok <- JSONSchema.validate(schema_type, params),
-         {:ok, input_date_range} <- parse_input_date_range(input_date_range),
+  def parse(params) when is_map(params) do
+    with :ok <- JSONSchema.validate(params),
+         {:ok, input_date_range} <- parse_input_date_range(params["date_range"]),
          {:ok, metrics} <- parse_metrics(Map.fetch!(params, "metrics")),
          {:ok, filters} <- parse_filters(params["filters"]),
          {:ok, dimensions} <- parse_dimensions(params["dimensions"]),
@@ -96,7 +94,7 @@ defmodule Plausible.Stats.ApiQueryParser do
   def parse_filter_second(_operator, filter), do: parse_filter_dimension(filter)
 
   defp parse_filter_dimension([_operator, filter_dimension | _rest] = filter) do
-    parse_filter_dimension_string(filter_dimension, "Invalid filter '#{i(filter)}")
+    parse_filter_dimension_string(filter_dimension, "Invalid filter '#{i(filter)}'.")
   end
 
   defp parse_filter_dimension(filter), do: {:error, "Invalid filter '#{i(filter)}'."}
@@ -149,7 +147,7 @@ defmodule Plausible.Stats.ApiQueryParser do
     end
   end
 
-  defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'"}
+  defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'."}
 
   defp parse_filter_modifiers(modifiers) when is_map(modifiers) do
     {:ok, [atomize_keys(modifiers)]}
@@ -159,8 +157,6 @@ defmodule Plausible.Stats.ApiQueryParser do
     {:ok, []}
   end
 
-  defp parse_input_date_range("realtime"), do: {:ok, :realtime}
-  defp parse_input_date_range("30m"), do: {:ok, :realtime_30m}
   defp parse_input_date_range("day"), do: {:ok, :day}
   defp parse_input_date_range("month"), do: {:ok, :month}
   defp parse_input_date_range("year"), do: {:ok, :year}
@@ -280,24 +276,12 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   @allowed_include_keys Enum.map(Map.keys(@default_include), &Atom.to_string/1)
 
-  defp parse_include_entry("compare", value) do
-    with {:ok, parsed_compare} <- parse_compare(value) do
-      {:ok, {:compare, parsed_compare}}
-    end
-  end
-
   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_compare(false), do: {:ok, nil}
-  defp parse_compare("previous_period"), do: {:ok, :previous_period}
-  defp parse_compare("year_over_year"), do: {:ok, :year_over_year}
-  defp parse_compare([from_date, to_date]), do: parse_date_strings(from_date, to_date)
-  defp parse_compare(compare), do: {:error, "Invalid include.compare '#{i(compare)}'."}
-
   defp parse_pagination(pagination) when is_map(pagination) do
     {:ok, Map.merge(@default_pagination, atomize_keys(pagination))}
   end
@@ -306,13 +290,10 @@ defmodule Plausible.Stats.ApiQueryParser do
 
   defp atomize_keys(map) when is_map(map) do
     Map.new(map, fn {key, value} ->
-      key = String.to_existing_atom(key)
-      {key, atomize_keys(value)}
+      {String.to_existing_atom(key), value}
     end)
   end
 
-  defp atomize_keys(value), do: value
-
   defp parse_filter_dimension_string(dimension, error_message \\ "") do
     case dimension do
       "event:props:" <> property_name ->

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
+  alias Plausible.Stats.{Query, QueryBuilder, ParsedQueryParams, QueryInclude}
   import Plausible.Stats.Base
   import Ecto.Query
 
@@ -39,16 +39,11 @@ defmodule Plausible.Stats.GoalSuggestions do
     from_date = Date.shift(to_date, month: -6)
 
     query =
-      Plausible.Stats.Query.parse_and_build!(
-        site,
-        :internal,
-        %{
-          "site_id" => site.domain,
-          "date_range" => [Date.to_iso8601(from_date), Date.to_iso8601(to_date)],
-          "metrics" => ["pageviews"],
-          "include" => %{"imports" => true}
-        }
-      )
+      QueryBuilder.build!(site, %ParsedQueryParams{
+        input_date_range: {:date_range, from_date, to_date},
+        metrics: [:pageviews],
+        include: %QueryInclude{imports: true}
+      })
 
     native_q =
       from(e in base_event_query(query),