Validate the same metric isnt queried multiple times in external stats API (#3871)

* Validate the same metric isnt queried multiple times in external stats API

Issue: https://3.basecamp.com/5308029/buckets/35611491/card_tables/cards/7161347855

* Changelog entry

* Make credo happy
This commit is contained in:
Karl-Aksel Puulmann 2024-03-08 10:46:18 +02:00 committed by GitHub
parent 26d41ddbb9
commit a9d3c03782
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 26 additions and 2 deletions

View File

@ -43,6 +43,7 @@ All notable changes to this project will be documented in this file.
- Require custom properties to be explicitly added from Site Settings > Custom Properties in order for them to show up on the dashboard - Require custom properties to be explicitly added from Site Settings > Custom Properties in order for them to show up on the dashboard
- GA/SC sections moved to new settings: Integrations - GA/SC sections moved to new settings: Integrations
- Replace `CLICKHOUSE_MAX_BUFFER_SIZE` with `CLICKHOUSE_MAX_BUFFER_SIZE_BYTES` - Replace `CLICKHOUSE_MAX_BUFFER_SIZE` with `CLICKHOUSE_MAX_BUFFER_SIZE_BYTES`
- Validate metric isn't queried multiple times
### Fixed ### Fixed
- Using `VersionedCollapsingMergeTree` to store visit data to avoid rare race conditions that led to wrong visit data being shown - Using `VersionedCollapsingMergeTree` to store visit data to avoid rare race conditions that led to wrong visit data being shown

View File

@ -111,7 +111,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do
Map.get(params, "metrics", "visitors") Map.get(params, "metrics", "visitors")
|> String.split(",") |> String.split(",")
case validate_all_metrics(metrics, property, query) do case validate_metrics(metrics, property, query) do
{:error, reason} -> {:error, reason} ->
{:error, reason} {:error, reason}
@ -149,7 +149,15 @@ defmodule PlausibleWeb.Api.ExternalStatsController do
end end
end end
defp validate_all_metrics(metrics, property, query) do defp validate_metrics(metrics, property, query) do
if length(metrics) == length(Enum.uniq(metrics)) do
validate_each_metric(metrics, property, query)
else
{:error, "Metrics cannot be queried multiple times."}
end
end
defp validate_each_metric(metrics, property, query) do
Enum.reduce_while(metrics, [], fn metric, acc -> Enum.reduce_while(metrics, [], fn metric, acc ->
case validate_metric(metric, property, query) do case validate_metric(metric, property, query) do
{:ok, metric} -> {:cont, acc ++ [metric]} {:ok, metric} -> {:cont, acc ++ [metric]}

View File

@ -189,6 +189,21 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do
"Session metric `views_per_visit` cannot be queried when using a filter on `event:name`." "Session metric `views_per_visit` cannot be queried when using a filter on `event:name`."
} }
end end
test "validates a metric isn't asked multiple times", %{
conn: conn,
site: site
} do
conn =
get(conn, "/api/v1/stats/aggregate", %{
"site_id" => site.domain,
"metrics" => "visitors,visitors"
})
assert json_response(conn, 400) == %{
"error" => "Metrics cannot be queried multiple times."
}
end
end end
test "aggregates a single metric", %{conn: conn, site: site} do test "aggregates a single metric", %{conn: conn, site: site} do