From 52b94842c07ae6b214b89ce22c8038b1d9d8c096 Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Tue, 10 Sep 2024 18:01:42 +0300 Subject: [PATCH] Assert filters are tuples, simplify schema (#4541) --- lib/plausible/stats/json_schema.ex | 4 +- priv/json-schemas/query-api-schema.json | 204 ++++++++------------- test/plausible/stats/query_parser_test.exs | 54 ++++++ 3 files changed, 135 insertions(+), 127 deletions(-) diff --git a/lib/plausible/stats/json_schema.ex b/lib/plausible/stats/json_schema.ex index 64db0da90cd8..b219a55089f2 100644 --- a/lib/plausible/stats/json_schema.ex +++ b/lib/plausible/stats/json_schema.ex @@ -17,11 +17,11 @@ defmodule Plausible.Stats.JSONSchema do @internal_query_schema @raw_public_schema # Add overrides for things allowed in the internal API |> JSONPointer.add!( - "#/definitions/filter_entry/oneOf/0/items/0/enum/0", + "#/definitions/filter_operation_without_goals/enum/0", "matches_wildcard" ) |> JSONPointer.add!( - "#/definitions/filter_entry/oneOf/0/items/0/enum/0", + "#/definitions/filter_operation_without_goals/enum/0", "matches_wildcard_not" ) |> JSONPointer.add!("#/definitions/metric/oneOf/0", %{ diff --git a/priv/json-schemas/query-api-schema.json b/priv/json-schemas/query-api-schema.json index 15f5114884f5..9b80aac218ba 100644 --- a/priv/json-schemas/query-api-schema.json +++ b/priv/json-schemas/query-api-schema.json @@ -29,9 +29,7 @@ }, "filters": { "type": "array", - "items": { - "$ref": "#/definitions/filter_entry" - }, + "items": { "$ref": "#/definitions/filter_tree" }, "description": "How to drill into your data" }, "order_by": { @@ -54,11 +52,7 @@ } } }, - "required": [ - "site_id", - "metrics", - "date_range" - ], + "required": ["site_id", "metrics", "date_range"], "additionalProperties": false, "definitions": { "date_range": { @@ -97,23 +91,21 @@ }, { "type": "array", + "additionalItems": false, + "minItems": 2, + "maxItems": 2, "items": { "type": "string", "pattern": "^\\d{4}-\\d{2}-\\d{2}(?:T\\d{2}:\\d{2}:\\d{2}\\s[A-Za-z/_]+)?$" }, "markdownDescription": "A list of two elements to determine the query date range. Both elements should have the same format - either `YYYY-MM-DD` or `YYYY-MM-DDThh:mm:ss `", "examples": [ - [ - "2024-01-01", - "2024-01-31" - ], + ["2024-01-01", "2024-01-31"], [ "2024-01-01T00:00:00 Europe/Tallinn", "2024-01-01T12:00:00 Europe/Tallinn" ] - ], - "minItems": 2, - "maxItems": 2 + ] } ] }, @@ -196,10 +188,7 @@ "type": "string", "pattern": "^event:props:.+", "markdownDescription": "Custom property. See [documentation](https://plausible.io/docs/custom-props/introduction) for more information", - "examples": [ - "event:props:url", - "event:props:path" - ] + "examples": ["event:props:url", "event:props:path"] }, "goal_dimension": { "const": "event:goal", @@ -207,28 +196,14 @@ }, "time_dimensions": { "type": "string", - "enum": [ - "time", - "time:month", - "time:week", - "time:day", - "time:hour" - ] + "enum": ["time", "time:month", "time:week", "time:day", "time:hour"] }, "dimensions": { "oneOf": [ - { - "$ref": "#/definitions/simple_filter_dimensions" - }, - { - "$ref": "#/definitions/custom_property_filter_dimensions" - }, - { - "$ref": "#/definitions/goal_dimension" - }, - { - "$ref": "#/definitions/time_dimensions" - } + { "$ref": "#/definitions/simple_filter_dimensions" }, + { "$ref": "#/definitions/custom_property_filter_dimensions" }, + { "$ref": "#/definitions/goal_dimension" }, + { "$ref": "#/definitions/time_dimensions" } ] }, "clauses": { @@ -237,129 +212,108 @@ "type": ["string", "integer"] } }, - "filter_entry": { - "oneOf": [ + "filter_operation_without_goals": { + "type": "string", + "enum": ["is_not", "contains_not", "matches", "matches_not"], + "description": "filter operation" + }, + "filter_operation_with_goals": { + "type": "string", + "enum": ["is", "contains"], + "description": "filter operation" + }, + "filter_without_goals": { + "type": "array", + "additionalItems": false, + "minItems": 3, + "maxItems": 3, + "items": [ + { "$ref": "#/definitions/filter_operation_without_goals" }, { - "type": "array", - "items": [ - { - "type": "string", - "enum": [ - "is_not", - "contains_not", - "matches", - "matches_not" - ], - "description": "filter operation" - }, - { - "oneOf": [ - { - "$ref": "#/definitions/simple_filter_dimensions" - }, - { - "$ref": "#/definitions/custom_property_filter_dimensions" - } - ] - }, - { - "$ref": "#/definitions/clauses" - } + "oneOf": [ + { "$ref": "#/definitions/simple_filter_dimensions" }, + { "$ref": "#/definitions/custom_property_filter_dimensions" } ] }, + { "$ref": "#/definitions/clauses" } + ] + }, + "filter_with_goals": { + "type": "array", + "additionalItems": false, + "minItems": 3, + "maxItems": 3, + "items": [ { - "type": "array", - "items": [ - { - "type": "string", - "enum": [ - "is", - "contains" - ], - "description": "filter operation" - }, - { - "oneOf": [ - { - "$ref": "#/definitions/goal_dimension" - }, - { - "$ref": "#/definitions/simple_filter_dimensions" - }, - { - "$ref": "#/definitions/custom_property_filter_dimensions" - } - ] - }, - { - "$ref": "#/definitions/clauses" - } - ] + "$ref": "#/definitions/filter_operation_with_goals" }, { - "$ref": "#/definitions/filter_and_or" + "oneOf": [ + { "$ref": "#/definitions/goal_dimension" }, + { "$ref": "#/definitions/simple_filter_dimensions" }, + { "$ref": "#/definitions/custom_property_filter_dimensions" } + ] }, { - "$ref": "#/definitions/filter_not" + "$ref": "#/definitions/clauses" } ] }, + "filter_entry": { + "oneOf": [ + { "$ref": "#/definitions/filter_without_goals" }, + { "$ref": "#/definitions/filter_with_goals" } + ] + }, + "filter_tree": { + "oneOf": [ + { "$ref": "#/definitions/filter_entry" }, + { "$ref": "#/definitions/filter_and_or" }, + { "$ref": "#/definitions/filter_not" } + ] + }, "filter_not": { "type": "array", - "items": [ - { - "const": "not" - }, - { - "$ref": "#/definitions/filter_entry" - } - ] + "additionalItems": false, + "minItems": 2, + "maxItems": 2, + "items": [{ "const": "not" }, { "$ref": "#/definitions/filter_tree" }] }, "filter_and_or": { "type": "array", + "additionalItems": false, + "minItems": 2, + "maxItems": 2, "items": [ { "type": "string", - "enum": [ - "and", - "or" - ] + "enum": ["and", "or"] }, { "type": "array", - "items": { - "$ref": "#/definitions/filter_entry" - }, + "items": { "$ref": "#/definitions/filter_tree" }, "minItems": 1 } ] }, "order_by_entry": { "type": "array", + "additionalItems": false, + "minItems": 2, + "maxItems": 2, "items": [ { "oneOf": [ - { - "$ref": "#/definitions/metric" - }, - { - "$ref": "#/definitions/simple_filter_dimensions" - }, - { - "$ref": "#/definitions/custom_property_filter_dimensions" - }, - { - "$ref": "#/definitions/time_dimensions" - } + { "$ref": "#/definitions/metric" }, + { "$ref": "#/definitions/simple_filter_dimensions" }, + { "$ref": "#/definitions/custom_property_filter_dimensions" }, + { "$ref": "#/definitions/time_dimensions" } ], "markdownDescription": "Metric or dimension to order by. Must be listed under `metrics` or `dimensions`" }, { "type": "string", - "enum": [ - "asc", - "desc" - ], + "enum": ["asc", "desc"], "description": "Sorting order" } ] diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index 657c017d7958..14f4e5ec8d13 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -238,6 +238,60 @@ defmodule Plausible.Stats.Filters.QueryParserTest do end end + for too_short_filter <- [ + [], + ["and"], + ["or"], + ["and", []], + ["or", []], + ["not"], + ["is_not"], + ["is_not", "event:name"] + ] do + test "errors on too short filter #{inspect(too_short_filter)}", %{ + site: site + } do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + unquote(too_short_filter) + ] + } + |> check_error( + site, + ~s(#/filters/0: Invalid filter #{inspect(unquote(too_short_filter))}) + ) + end + end + + valid_filter = ["is", "event:props:foobar", ["value"]] + + for too_long_filter <- [ + ["and", [valid_filter], "extra"], + ["or", [valid_filter], []], + ["not", valid_filter, 1], + Enum.concat(valid_filter, [true]) + ] do + test "errors on too long filter #{inspect(too_long_filter)}", %{ + site: site + } do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + unquote(too_long_filter) + ] + } + |> check_error( + site, + ~s(#/filters/0: Invalid filter #{inspect(unquote(too_long_filter))}) + ) + end + end + test "filtering by invalid operation", %{site: site} do %{ "site_id" => site.domain,