From c871759fc183a3c5975c45bd9ce31124ed6c5c1f Mon Sep 17 00:00:00 2001 From: hq1 Date: Mon, 1 Jul 2024 12:39:55 +0200 Subject: [PATCH 1/4] Bugfix: opening details UTM sources modal (#4295) --- assets/js/dashboard/util/url.js | 2 +- priv/repo/seeds.exs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/js/dashboard/util/url.js b/assets/js/dashboard/util/url.js index 609f116123747..f201c6fe0b22b 100644 --- a/assets/js/dashboard/util/url.js +++ b/assets/js/dashboard/util/url.js @@ -5,7 +5,7 @@ export function apiPath(site, path = '') { } export function sitePath(path = '') { - return `/${path}` + window.location.search + return (path.startsWith('/') ? path : '/' + path) + window.location.search } export function setQuery(key, value) { diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 4d2dc4c99f20d..44e1db967060e 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -168,6 +168,7 @@ native_stats_range screen_size: Enum.random(["Mobile", "Tablet", "Desktop", "Laptop"]), operating_system: Enum.random(["Windows", "Mac", "Linux"]), operating_system_version: to_string(Enum.random(0..15)), + utm_campaign: Enum.random(["", "Referral", "Advertisement", "Email"]), pathname: Enum.random([ "/", From 839691d29ff427afc2c8bfb9c9793f30ffe51ec6 Mon Sep 17 00:00:00 2001 From: hq1 Date: Tue, 2 Jul 2024 14:45:48 +0200 Subject: [PATCH 2/4] Bugfix: custom props modal opens up with no filters (#4301) --- assets/js/dashboard/stats/behaviours/props.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/dashboard/stats/behaviours/props.js b/assets/js/dashboard/stats/behaviours/props.js index 4986b215df77b..8ba2d7074fa7e 100644 --- a/assets/js/dashboard/stats/behaviours/props.js +++ b/assets/js/dashboard/stats/behaviours/props.js @@ -98,7 +98,7 @@ export default function Properties(props) { BUILD_EXTRA && { name: 'total_revenue', label: 'Revenue', hiddenOnMobile: true }, BUILD_EXTRA && { name: 'average_revenue', label: 'Average', hiddenOnMobile: true } ]} - detailsLink={`/custom-prop-values/${propKey}`} + detailsLink={url.sitePath(`custom-prop-values/${propKey}`)} maybeHideDetails={true} query={query} color="bg-red-50" From 790984e1adaafb2f7ad564e25642574ae457d280 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Tue, 2 Jul 2024 15:09:23 +0200 Subject: [PATCH 3/4] Refactor Sites and Stats API authorization logic (#4297) * Refactor and unify auth plugs for Stats and Sites APIs * Expose get site Sites API endpoint to all API keys * Test the new plug * Add test for endpoint with modified scope * Fix typos Co-authored-by: hq1 * Rename plug for consistency (h/t @aerosol) --------- Co-authored-by: hq1 --- .../lib/plausible_web/authorize_sites_api.ex | 57 ----- .../plugs/authorize_public_api.ex | 206 +++++++++++++++ .../plugs/authorize_stats_api.ex | 115 --------- lib/plausible_web/router.ex | 35 ++- .../api/external_sites_controller_test.exs | 11 + .../plugs/authorize_public_api_test.exs | 235 ++++++++++++++++++ 6 files changed, 474 insertions(+), 185 deletions(-) delete mode 100644 extra/lib/plausible_web/authorize_sites_api.ex create mode 100644 lib/plausible_web/plugs/authorize_public_api.ex delete mode 100644 lib/plausible_web/plugs/authorize_stats_api.ex create mode 100644 test/plausible_web/plugs/authorize_public_api_test.exs diff --git a/extra/lib/plausible_web/authorize_sites_api.ex b/extra/lib/plausible_web/authorize_sites_api.ex deleted file mode 100644 index b55796e3d67de..0000000000000 --- a/extra/lib/plausible_web/authorize_sites_api.ex +++ /dev/null @@ -1,57 +0,0 @@ -defmodule PlausibleWeb.AuthorizeSitesApiPlug do - import Plug.Conn - use Plausible.Repo - alias Plausible.Auth.ApiKey - alias PlausibleWeb.Api.Helpers, as: H - - def init(options) do - options - end - - def call(conn, _opts) do - with {:ok, raw_api_key} <- get_bearer_token(conn), - {:ok, api_key} <- verify_access(raw_api_key) do - user = Repo.get_by(Plausible.Auth.User, id: api_key.user_id) - assign(conn, :current_user, user) - else - {:error, :missing_api_key} -> - H.unauthorized( - conn, - "Missing API key. Please use a valid Plausible API key as a Bearer Token." - ) - - {:error, :invalid_api_key} -> - H.unauthorized( - conn, - "Invalid API key. Please make sure you're using a valid API key with access to the resource you've requested." - ) - end - end - - defp verify_access(api_key) do - hashed_key = ApiKey.do_hash(api_key) - - found_key = - Repo.one( - from a in ApiKey, - where: a.key_hash == ^hashed_key, - where: fragment("? @> ?", a.scopes, ["sites:provision:*"]) - ) - - cond do - found_key -> {:ok, found_key} - true -> {:error, :invalid_api_key} - end - end - - defp get_bearer_token(conn) do - authorization_header = - Plug.Conn.get_req_header(conn, "authorization") - |> List.first() - - case authorization_header do - "Bearer " <> token -> {:ok, String.trim(token)} - _ -> {:error, :missing_api_key} - end - end -end diff --git a/lib/plausible_web/plugs/authorize_public_api.ex b/lib/plausible_web/plugs/authorize_public_api.ex new file mode 100644 index 0000000000000..45956d1dbfcb1 --- /dev/null +++ b/lib/plausible_web/plugs/authorize_public_api.ex @@ -0,0 +1,206 @@ +defmodule PlausibleWeb.Plugs.AuthorizePublicAPI do + @moduledoc """ + Plug for authorizing access to Stats and Sites APIs. + + The plug expects `:api_scope` to be provided in the assigns. The scope + will then be used to check for API key validity. The assign can be + provided in the router configuration in a following way: + + scope "/api/v1/stats", PlausibleWeb.Api, assigns: %{api_scope: "some:scope:*"} do + pipe_through [:public_api, #{inspect(__MODULE__)}] + + # route definitions follow + # ... + end + + The scope from `:api_scope` is checked for match against all scopes from API key's + `scopes` field. If the scope is among `@implicit_scopes`, it's considered to be + present for any valid API key. Scopes are checked for match by prefix, so if we have + `some:scope:*` in matching route `:api_scope` and the API key has `some:*` in its + `scopes` field, they will match. + + After a match is found, additional verification can be conducted, like in case of + `stats:read:*`, where valid site ID is expected among parameters too. + + All API requests are rate limited per API key, enforcing a given hourly request limit. + """ + + use Plausible.Repo + + import Plug.Conn + + alias Plausible.Auth + alias Plausible.RateLimit + alias Plausible.Sites + alias PlausibleWeb.Api.Helpers, as: H + + # Scopes permitted implicitly for every API key. Existing API keys + # have _either_ `["stats:read:*"]` (the default) or `["sites:provision:*"]` + # set as their valid scopes. We always consider implicit scopes as + # present in addition to whatever else is provided for a particular + # API key. + @implicit_scopes ["stats:read:*", "sites:read:*"] + + def init(opts) do + opts + end + + def call(conn, _opts) do + requested_scope = Map.fetch!(conn.assigns, :api_scope) + + with {:ok, token} <- get_bearer_token(conn), + {:ok, api_key} <- Auth.find_api_key(token), + :ok <- check_api_key_rate_limit(api_key), + {:ok, conn} <- verify_by_scope(conn, api_key, requested_scope) do + assign(conn, :current_user, api_key.user) + else + error -> send_error(conn, requested_scope, error) + end + end + + ### Verification dispatched by scope + + defp verify_by_scope(conn, api_key, "stats:read:" <> _ = scope) do + with :ok <- check_scope(api_key, scope), + {:ok, site} <- find_site(conn.params["site_id"]), + :ok <- verify_site_access(api_key, site) do + Plausible.OpenTelemetry.add_site_attributes(site) + site = Plausible.Imported.load_import_data(site) + {:ok, assign(conn, :site, site)} + end + end + + defp verify_by_scope(conn, api_key, scope) do + with :ok <- check_scope(api_key, scope) do + {:ok, conn} + end + end + + defp check_scope(_api_key, required_scope) when required_scope in @implicit_scopes do + :ok + end + + defp check_scope(api_key, required_scope) do + found? = + Enum.any?(api_key.scopes, fn scope -> + scope = String.trim_trailing(scope, "*") + + String.starts_with?(required_scope, scope) + end) + + if found? do + :ok + else + {:error, :invalid_api_key} + end + end + + defp get_bearer_token(conn) do + authorization_header = + conn + |> Plug.Conn.get_req_header("authorization") + |> List.first() + + case authorization_header do + "Bearer " <> token -> {:ok, String.trim(token)} + _ -> {:error, :missing_api_key} + end + end + + defp check_api_key_rate_limit(api_key) do + case RateLimit.check_rate( + "api_request:#{api_key.id}", + to_timeout(hour: 1), + api_key.hourly_request_limit + ) do + {:allow, _} -> :ok + {:deny, _} -> {:error, :rate_limit, api_key.hourly_request_limit} + end + end + + defp find_site(nil), do: {:error, :missing_site_id} + + defp find_site(site_id) do + domain_based_search = + from s in Plausible.Site, where: s.domain == ^site_id or s.domain_changed_from == ^site_id + + case Repo.one(domain_based_search) do + %Plausible.Site{} = site -> + {:ok, site} + + nil -> + {:error, :invalid_api_key} + end + end + + defp verify_site_access(api_key, site) do + is_member? = Sites.is_member?(api_key.user_id, site) + is_super_admin? = Auth.is_super_admin?(api_key.user_id) + + cond do + is_super_admin? -> + :ok + + Sites.locked?(site) -> + {:error, :site_locked} + + Plausible.Billing.Feature.StatsAPI.check_availability(api_key.user) !== :ok -> + {:error, :upgrade_required} + + is_member? -> + :ok + + true -> + {:error, :invalid_api_key} + end + end + + defp send_error(conn, _, {:error, :missing_api_key}) do + H.unauthorized( + conn, + "Missing API key. Please use a valid Plausible API key as a Bearer Token." + ) + end + + defp send_error(conn, "stats:read:" <> _, {:error, :invalid_api_key}) do + H.unauthorized( + conn, + "Invalid API key or site ID. Please make sure you're using a valid API key with access to the site you've requested." + ) + end + + defp send_error(conn, _, {:error, :invalid_api_key}) do + H.unauthorized( + conn, + "Invalid API key. Please make sure you're using a valid API key with access to the resource you've requested." + ) + end + + defp send_error(conn, _, {:error, :rate_limit, limit}) do + H.too_many_requests( + conn, + "Too many API requests. Your API key is limited to #{limit} requests per hour. Please contact us to request more capacity." + ) + end + + defp send_error(conn, _, {:error, :missing_site_id}) do + H.bad_request( + conn, + "Missing site ID. Please provide the required site_id parameter with your request." + ) + end + + defp send_error(conn, _, {:error, :upgrade_required}) do + H.payment_required( + conn, + "The account that owns this API key does not have access to Stats API. Please make sure you're using the API key of a subscriber account and that the subscription plan includes Stats API" + ) + end + + defp send_error(conn, _, {:error, :site_locked}) do + H.payment_required( + conn, + "This Plausible site is locked due to missing active subscription. In order to access it, the site owner should subscribe to a suitable plan" + ) + end +end diff --git a/lib/plausible_web/plugs/authorize_stats_api.ex b/lib/plausible_web/plugs/authorize_stats_api.ex deleted file mode 100644 index 4dc68b5bd3558..0000000000000 --- a/lib/plausible_web/plugs/authorize_stats_api.ex +++ /dev/null @@ -1,115 +0,0 @@ -defmodule PlausibleWeb.AuthorizeStatsApiPlug do - import Plug.Conn - use Plausible.Repo - alias Plausible.Auth - alias Plausible.Sites - alias Plausible.RateLimit - alias PlausibleWeb.Api.Helpers, as: H - - def init(options) do - options - end - - def call(conn, _opts) do - with {:ok, token} <- get_bearer_token(conn), - {:ok, api_key} <- Auth.find_api_key(token), - :ok <- check_api_key_rate_limit(api_key), - {:ok, site} <- verify_access(api_key, conn.params["site_id"]) do - Plausible.OpenTelemetry.add_site_attributes(site) - site = Plausible.Imported.load_import_data(site) - assign(conn, :site, site) - else - {:error, :missing_api_key} -> - H.unauthorized( - conn, - "Missing API key. Please use a valid Plausible API key as a Bearer Token." - ) - - {:error, :missing_site_id} -> - H.bad_request( - conn, - "Missing site ID. Please provide the required site_id parameter with your request." - ) - - {:error, :rate_limit, limit} -> - H.too_many_requests( - conn, - "Too many API requests. Your API key is limited to #{limit} requests per hour. Please contact us to request more capacity." - ) - - {:error, :invalid_api_key} -> - H.unauthorized( - conn, - "Invalid API key or site ID. Please make sure you're using a valid API key with access to the site you've requested." - ) - - {:error, :upgrade_required} -> - H.payment_required( - conn, - "The account that owns this API key does not have access to Stats API. Please make sure you're using the API key of a subscriber account and that the subscription plan includes Stats API" - ) - - {:error, :site_locked} -> - H.payment_required( - conn, - "This Plausible site is locked due to missing active subscription. In order to access it, the site owner should subscribe to a suitable plan" - ) - end - end - - defp verify_access(_api_key, nil), do: {:error, :missing_site_id} - - defp verify_access(api_key, site_id) do - domain_based_search = - from s in Plausible.Site, where: s.domain == ^site_id or s.domain_changed_from == ^site_id - - case Repo.one(domain_based_search) do - %Plausible.Site{} = site -> - is_member? = Sites.is_member?(api_key.user_id, site) - is_super_admin? = Plausible.Auth.is_super_admin?(api_key.user_id) - - cond do - is_super_admin? -> - {:ok, site} - - Sites.locked?(site) -> - {:error, :site_locked} - - Plausible.Billing.Feature.StatsAPI.check_availability(api_key.user) !== :ok -> - {:error, :upgrade_required} - - is_member? -> - {:ok, site} - - true -> - {:error, :invalid_api_key} - end - - nil -> - {:error, :invalid_api_key} - end - end - - defp get_bearer_token(conn) do - authorization_header = - Plug.Conn.get_req_header(conn, "authorization") - |> List.first() - - case authorization_header do - "Bearer " <> token -> {:ok, String.trim(token)} - _ -> {:error, :missing_api_key} - end - end - - @one_hour 60 * 60 * 1000 - defp check_api_key_rate_limit(api_key) do - case RateLimit.check_rate( - "api_request:#{api_key.id}", - @one_hour, - api_key.hourly_request_limit - ) do - {:allow, _} -> :ok - {:deny, _} -> {:error, :rate_limit, api_key.hourly_request_limit} - end - end -end diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index bd7179c13f168..7c281e06c54a4 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -166,8 +166,8 @@ defmodule PlausibleWeb.Router do get "/:domain/suggestions/:filter_name", StatsController, :filter_suggestions end - scope "/api/v1/stats", PlausibleWeb.Api do - pipe_through [:public_api, PlausibleWeb.AuthorizeStatsApiPlug] + scope "/api/v1/stats", PlausibleWeb.Api, assigns: %{api_scope: "stats:read:*"} do + pipe_through [:public_api, PlausibleWeb.Plugs.AuthorizePublicAPI] get "/realtime/visitors", ExternalStatsController, :realtime_visitors get "/aggregate", ExternalStatsController, :aggregate @@ -175,23 +175,32 @@ defmodule PlausibleWeb.Router do get "/timeseries", ExternalStatsController, :timeseries end - scope "/api/v2", PlausibleWeb.Api do - pipe_through [:public_api, PlausibleWeb.AuthorizeStatsApiPlug] + scope "/api/v2", PlausibleWeb.Api, assigns: %{api_scope: "stats:read:*"} do + pipe_through [:public_api, PlausibleWeb.Plugs.AuthorizePublicAPI] post "/query", ExternalQueryApiController, :query end on_ee do scope "/api/v1/sites", PlausibleWeb.Api do - pipe_through [:public_api, PlausibleWeb.AuthorizeSitesApiPlug] - - post "/", ExternalSitesController, :create_site - put "/shared-links", ExternalSitesController, :find_or_create_shared_link - put "/goals", ExternalSitesController, :find_or_create_goal - delete "/goals/:goal_id", ExternalSitesController, :delete_goal - get "/:site_id", ExternalSitesController, :get_site - put "/:site_id", ExternalSitesController, :update_site - delete "/:site_id", ExternalSitesController, :delete_site + pipe_through :public_api + + scope assigns: %{api_scope: "sites:read:*"} do + pipe_through PlausibleWeb.Plugs.AuthorizePublicAPI + + get "/:site_id", ExternalSitesController, :get_site + end + + scope assigns: %{api_scope: "sites:provision:*"} do + pipe_through PlausibleWeb.Plugs.AuthorizePublicAPI + + post "/", ExternalSitesController, :create_site + put "/shared-links", ExternalSitesController, :find_or_create_shared_link + put "/goals", ExternalSitesController, :find_or_create_goal + delete "/goals/:goal_id", ExternalSitesController, :delete_goal + put "/:site_id", ExternalSitesController, :update_site + delete "/:site_id", ExternalSitesController, :delete_site + end end end diff --git a/test/plausible_web/controllers/api/external_sites_controller_test.exs b/test/plausible_web/controllers/api/external_sites_controller_test.exs index e454a6bd463ff..0a079648ac8bd 100644 --- a/test/plausible_web/controllers/api/external_sites_controller_test.exs +++ b/test/plausible_web/controllers/api/external_sites_controller_test.exs @@ -509,6 +509,17 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do assert json_response(conn, 200) == %{"domain" => new_domain, "timezone" => site.timezone} end + test "get a site with basic scope config", %{conn: conn, user: user, site: site} do + api_key = insert(:api_key, user: user, scopes: ["stats:read:*"]) + + conn = + conn + |> Plug.Conn.put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/api/v1/sites/" <> site.domain) + + assert json_response(conn, 200) == %{"domain" => site.domain, "timezone" => site.timezone} + end + test "is 404 when site cannot be found", %{conn: conn} do conn = get(conn, "/api/v1/sites/foobar.baz") diff --git a/test/plausible_web/plugs/authorize_public_api_test.exs b/test/plausible_web/plugs/authorize_public_api_test.exs new file mode 100644 index 0000000000000..a7501e89c5ad1 --- /dev/null +++ b/test/plausible_web/plugs/authorize_public_api_test.exs @@ -0,0 +1,235 @@ +defmodule PlausibleWeb.Plugs.AuthorizePublicAPITest do + use PlausibleWeb.ConnCase, async: false + + alias PlausibleWeb.Plugs.AuthorizePublicAPI + + setup %{conn: conn} do + conn = + conn + |> put_private(PlausibleWeb.FirstLaunchPlug, :skip) + |> bypass_through(PlausibleWeb.Router) + + {:ok, conn: conn} + end + + test "halts with error when bearer token is missing", %{conn: conn} do + conn = + conn + |> get("/") + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 401)["error"] =~ "Missing API key." + end + + test "halts with error when bearer token is invalid against read-only Stats API", %{conn: conn} do + conn = + conn + |> put_req_header("authorization", "Bearer invalid") + |> get("/") + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 401)["error"] =~ "Invalid API key or site ID." + end + + test "halts with error when bearer token is invalid", %{conn: conn} do + conn = + conn + |> put_req_header("authorization", "Bearer invalid") + |> get("/") + |> assign(:api_scope, "sites:provision:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 401)["error"] =~ "Invalid API key." + end + + test "halts with error on missing site ID when request made to Stats API", %{conn: conn} do + api_key = insert(:api_key, user: build(:user)) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/") + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 400)["error"] =~ "Missing site ID." + end + + @tag :ee_only + test "halts with error when upgrade is required", %{conn: conn} do + user = insert(:user, trial_expiry_date: nil) + site = insert(:site, members: [user]) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => site.domain}) + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + + assert json_response(conn, 402)["error"] =~ + "The account that owns this API key does not have access" + end + + test "halts with error when site is locked", %{conn: conn} do + user = insert(:user) + site = insert(:site, members: [user], locked: true) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => site.domain}) + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 402)["error"] =~ "This Plausible site is locked" + end + + test "halts with error when site ID is invalid", %{conn: conn} do + user = insert(:user) + _site = insert(:site, members: [user]) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => "invalid.domain"}) + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 401)["error"] =~ "Invalid API key or site ID." + end + + test "halts with error when API key owner does not have access to the requested site", %{ + conn: conn + } do + user = insert(:user) + site = insert(:site) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => site.domain}) + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 401)["error"] =~ "Invalid API key or site ID." + end + + test "halts with error when API lacks required scope", %{conn: conn} do + user = insert(:user) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/") + |> assign(:api_scope, "sites:provision:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + assert json_response(conn, 401)["error"] =~ "Invalid API key." + end + + test "halts with error when API rate limit hit", %{conn: conn} do + user = insert(:user) + api_key = insert(:api_key, user: user, hourly_request_limit: 1) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/") + |> assign(:api_scope, "sites:read:*") + + first_resp = AuthorizePublicAPI.call(conn, nil) + second_resp = AuthorizePublicAPI.call(conn, nil) + + refute first_resp.halted + assert second_resp.halted + assert json_response(second_resp, 429)["error"] =~ "Too many API requests." + end + + test "passes and sets current user when valid API key with required scope provided", %{ + conn: conn + } do + user = insert(:user) + api_key = insert(:api_key, user: user, scopes: ["sites:provision:*"]) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/") + |> assign(:api_scope, "sites:provision:*") + |> AuthorizePublicAPI.call(nil) + + refute conn.halted + assert conn.assigns.current_user.id == user.id + end + + test "passes and sets current user and site when valid API key and site ID provided", %{ + conn: conn + } do + user = insert(:user) + site = insert(:site, members: [user]) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => site.domain}) + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + refute conn.halted + assert conn.assigns.current_user.id == user.id + assert conn.assigns.site.id == site.id + end + + @tag :ee_only + test "passes for super admin user even if not a member of the requested site", %{conn: conn} do + user = insert(:user) + patch_env(:super_admin_user_ids, [user.id]) + site = insert(:site, locked: true) + api_key = insert(:api_key, user: user) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => site.domain}) + |> assign(:api_scope, "stats:read:*") + |> AuthorizePublicAPI.call(nil) + + refute conn.halted + assert conn.assigns.current_user.id == user.id + assert conn.assigns.site.id == site.id + end + + test "passes for subscope match", %{conn: conn} do + user = insert(:user) + api_key = insert(:api_key, user: user, scopes: ["funnels:*"]) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/") + |> assign(:api_scope, "funnels:read:*") + |> AuthorizePublicAPI.call(nil) + + refute conn.halted + assert conn.assigns.current_user.id == user.id + end +end From 05ac840078cf9fa31c70dfd3ca140f725fb80b59 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Wed, 3 Jul 2024 16:32:25 +0300 Subject: [PATCH 4/4] APIv2: macros, SQL cleanup (#4286) * Move fragments module under Plausible.Stats.SQL * Introduce select_merge_as macro This simplifies some select_merge calls * Simplify select_join_fields * Remove a needless dynamic * wrap_select_columns macro * Move metrics from base.ex to expression.ex * Move WhereBuilder under Plausible.Stats.SQL * Moduledoc * Improved macros * Wrap more code * select_merge_as more * Move defp to the end * wrap_alias --- extra/lib/plausible/stats/funnel.ex | 2 +- extra/lib/plausible/stats/goal/revenue.ex | 20 -- lib/plausible/exports.ex | 2 +- lib/plausible/stats/aggregate.ex | 5 +- lib/plausible/stats/base.ex | 261 ++-------------- lib/plausible/stats/breakdown.ex | 2 +- lib/plausible/stats/clickhouse.ex | 2 +- lib/plausible/stats/current_visitors.ex | 2 +- lib/plausible/stats/filter_suggestions.ex | 2 +- lib/plausible/stats/imported/base.ex | 7 +- lib/plausible/stats/imported/imported.ex | 137 +++------ lib/plausible/stats/sql/expression.ex | 286 +++++++++++++----- lib/plausible/stats/{ => sql}/fragments.ex | 79 ++++- lib/plausible/stats/sql/query_builder.ex | 82 +++-- .../stats/{filters => sql}/where_builder.ex | 4 +- lib/plausible/stats/timeseries.ex | 2 +- lib/plausible/stats/util.ex | 1 - test/plausible/stats/sql/fragments_test.exs | 10 + 18 files changed, 407 insertions(+), 499 deletions(-) rename lib/plausible/stats/{ => sql}/fragments.ex (65%) rename lib/plausible/stats/{filters => sql}/where_builder.ex (99%) create mode 100644 test/plausible/stats/sql/fragments_test.exs diff --git a/extra/lib/plausible/stats/funnel.ex b/extra/lib/plausible/stats/funnel.ex index d9c497cee609e..121a699fae80a 100644 --- a/extra/lib/plausible/stats/funnel.ex +++ b/extra/lib/plausible/stats/funnel.ex @@ -10,7 +10,7 @@ defmodule Plausible.Stats.Funnel do alias Plausible.Funnels import Ecto.Query - import Plausible.Stats.Fragments + import Plausible.Stats.SQL.Fragments alias Plausible.ClickhouseRepo alias Plausible.Stats.Base diff --git a/extra/lib/plausible/stats/goal/revenue.ex b/extra/lib/plausible/stats/goal/revenue.ex index 7443a1758d37b..1ba564e3ff340 100644 --- a/extra/lib/plausible/stats/goal/revenue.ex +++ b/extra/lib/plausible/stats/goal/revenue.ex @@ -12,26 +12,6 @@ defmodule Plausible.Stats.Goal.Revenue do @revenue_metrics end - def total_revenue_query() do - dynamic( - [e], - selected_as( - fragment("toDecimal64(sum(?) * any(_sample_factor), 3)", e.revenue_reporting_amount), - :total_revenue - ) - ) - end - - def average_revenue_query() do - dynamic( - [e], - selected_as( - fragment("toDecimal64(avg(?) * any(_sample_factor), 3)", e.revenue_reporting_amount), - :average_revenue - ) - ) - end - @spec get_revenue_tracking_currency(Plausible.Site.t(), Plausible.Stats.Query.t(), [atom()]) :: {atom() | nil, [atom()]} @doc """ diff --git a/lib/plausible/exports.ex b/lib/plausible/exports.ex index d5cb20ada20de..04e6a420218f0 100644 --- a/lib/plausible/exports.ex +++ b/lib/plausible/exports.ex @@ -4,7 +4,7 @@ defmodule Plausible.Exports do """ use Plausible - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments import Ecto.Query @doc "Schedules CSV export job to S3 storage" diff --git a/lib/plausible/stats/aggregate.ex b/lib/plausible/stats/aggregate.ex index 9a2e65bab6074..8c5cdc4f7bbba 100644 --- a/lib/plausible/stats/aggregate.ex +++ b/lib/plausible/stats/aggregate.ex @@ -3,7 +3,7 @@ defmodule Plausible.Stats.Aggregate do use Plausible import Plausible.Stats.Base import Ecto.Query - alias Plausible.Stats.{Query, Util} + alias Plausible.Stats.{Query, Util, SQL} def aggregate(site, query, metrics) do {currency, metrics} = @@ -64,8 +64,7 @@ defmodule Plausible.Stats.Aggregate do timed_page_transitions_q = from e in Ecto.Query.subquery(windowed_pages_q), group_by: [e.pathname, e.next_pathname, e.session_id], - where: - ^Plausible.Stats.Filters.WhereBuilder.build_condition(:pathname, event_page_filter), + where: ^SQL.WhereBuilder.build_condition(:pathname, event_page_filter), where: e.next_timestamp != 0, select: %{ pathname: e.pathname, diff --git a/lib/plausible/stats/base.ex b/lib/plausible/stats/base.ex index 5354833a1f63e..8335308cce2a9 100644 --- a/lib/plausible/stats/base.ex +++ b/lib/plausible/stats/base.ex @@ -1,14 +1,12 @@ defmodule Plausible.Stats.Base do use Plausible.ClickhouseRepo use Plausible - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments - alias Plausible.Stats.{Query, Filters, TableDecider} + alias Plausible.Stats.{Query, TableDecider, SQL} alias Plausible.Timezones import Ecto.Query - @uniq_users_expression "toUInt64(round(uniq(?) * any(_sample_factor)))" - def base_event_query(site, query) do events_q = query_events(site, query) @@ -32,7 +30,7 @@ defmodule Plausible.Stats.Base do end def query_events(site, query) do - q = from(e in "events_v2", where: ^Filters.WhereBuilder.build(:events, site, query)) + q = from(e in "events_v2", where: ^SQL.WhereBuilder.build(:events, site, query)) on_ee do q = Plausible.Stats.Sampling.add_query_hint(q, query) @@ -42,7 +40,7 @@ defmodule Plausible.Stats.Base do end def query_sessions(site, query) do - q = from(s in "sessions_v2", where: ^Filters.WhereBuilder.build(:sessions, site, query)) + q = from(s in "sessions_v2", where: ^SQL.WhereBuilder.build(:sessions, site, query)) on_ee do q = Plausible.Stats.Sampling.add_query_hint(q, query) @@ -53,206 +51,16 @@ defmodule Plausible.Stats.Base do def select_event_metrics(metrics) do metrics - |> Enum.map(&select_event_metric/1) + |> Enum.map(&SQL.Expression.event_metric/1) |> Enum.reduce(%{}, &Map.merge/2) end - defp select_event_metric(:pageviews) do - %{ - pageviews: - dynamic( - [e], - selected_as( - fragment("toUInt64(round(countIf(? = 'pageview') * any(_sample_factor)))", e.name), - :pageviews - ) - ) - } - end - - defp select_event_metric(:events) do - %{ - events: - dynamic( - [], - selected_as(fragment("toUInt64(round(count(*) * any(_sample_factor)))"), :events) - ) - } - end - - defp select_event_metric(:visitors) do - %{ - visitors: dynamic([e], selected_as(fragment(@uniq_users_expression, e.user_id), :visitors)) - } - end - - defp select_event_metric(:visits) do - %{ - visits: - dynamic( - [e], - selected_as( - fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", e.session_id), - :visits - ) - ) - } - end - - on_ee do - defp select_event_metric(:total_revenue) do - %{total_revenue: Plausible.Stats.Goal.Revenue.total_revenue_query()} - end - - defp select_event_metric(:average_revenue) do - %{average_revenue: Plausible.Stats.Goal.Revenue.average_revenue_query()} - end - end - - defp select_event_metric(:sample_percent) do - %{ - sample_percent: - dynamic( - [], - fragment("if(any(_sample_factor) > 1, round(100 / any(_sample_factor)), 100)") - ) - } - end - - defp select_event_metric(:percentage), do: %{} - defp select_event_metric(:conversion_rate), do: %{} - defp select_event_metric(:group_conversion_rate), do: %{} - defp select_event_metric(:total_visitors), do: %{} - - defp select_event_metric(unknown), do: raise("Unknown metric: #{unknown}") - def select_session_metrics(metrics, query) do metrics - |> Enum.map(&select_session_metric(&1, query)) + |> Enum.map(&SQL.Expression.session_metric(&1, query)) |> Enum.reduce(%{}, &Map.merge/2) end - defp select_session_metric(:bounce_rate, query) do - # :TRICKY: If page is passed to query, we only count bounce rate where users _entered_ at page. - event_page_filter = Query.get_filter(query, "event:page") - condition = Filters.WhereBuilder.build_condition(:entry_page, event_page_filter) - - %{ - bounce_rate: - dynamic( - [], - selected_as( - fragment( - "toUInt32(ifNotFinite(round(sumIf(is_bounce * sign, ?) / sumIf(sign, ?) * 100), 0))", - ^condition, - ^condition - ), - :bounce_rate - ) - ), - __internal_visits: dynamic([], fragment("toUInt32(sum(sign))")) - } - end - - defp select_session_metric(:visits, _query) do - %{ - visits: - dynamic( - [s], - selected_as( - fragment("toUInt64(round(sum(?) * any(_sample_factor)))", s.sign), - :visits - ) - ) - } - end - - defp select_session_metric(:pageviews, _query) do - %{ - pageviews: - dynamic( - [s], - selected_as( - fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.pageviews), - :pageviews - ) - ) - } - end - - defp select_session_metric(:events, _query) do - %{ - events: - dynamic( - [s], - selected_as( - fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.events), - :events - ) - ) - } - end - - defp select_session_metric(:visitors, _query) do - %{ - visitors: - dynamic( - [s], - selected_as( - fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", s.user_id), - :visitors - ) - ) - } - end - - defp select_session_metric(:visit_duration, _query) do - %{ - visit_duration: - dynamic( - [], - selected_as( - fragment("toUInt32(ifNotFinite(round(sum(duration * sign) / sum(sign)), 0))"), - :visit_duration - ) - ), - __internal_visits: dynamic([], fragment("toUInt32(sum(sign))")) - } - end - - defp select_session_metric(:views_per_visit, _query) do - %{ - views_per_visit: - dynamic( - [s], - selected_as( - fragment( - "ifNotFinite(round(sum(? * ?) / sum(?), 2), 0)", - s.sign, - s.pageviews, - s.sign - ), - :views_per_visit - ) - ), - __internal_visits: dynamic([], fragment("toUInt32(sum(sign))")) - } - end - - defp select_session_metric(:sample_percent, _query) do - %{ - sample_percent: - dynamic( - [], - fragment("if(any(_sample_factor) > 1, round(100 / any(_sample_factor)), 100)") - ) - } - end - - defp select_session_metric(:percentage, _query), do: %{} - defp select_session_metric(:conversion_rate, _query), do: %{} - defp select_session_metric(:group_conversion_rate, _query), do: %{} - def filter_converted_sessions(db_query, site, query) do if Query.has_event_filters?(query) do converted_sessions = @@ -334,7 +142,9 @@ defmodule Plausible.Stats.Base do defp total_visitors(site, query) do base_event_query(site, query) - |> select([e], total_visitors: fragment(@uniq_users_expression, e.user_id)) + |> select([e], + total_visitors: fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", e.user_id) + ) end # `total_visitors_subquery` returns a subquery which selects `total_visitors` - @@ -350,18 +160,17 @@ defmodule Plausible.Stats.Base do def total_visitors_subquery(site, query, include_imported) def total_visitors_subquery(site, query, true = _include_imported) do - dynamic( - [e], - selected_as( + wrap_alias([], %{ + total_visitors: subquery(total_visitors(site, query)) + - subquery(Plausible.Stats.Imported.total_imported_visitors(site, query)), - :__total_visitors - ) - ) + subquery(Plausible.Stats.Imported.total_imported_visitors(site, query)) + }) end def total_visitors_subquery(site, query, false = _include_imported) do - dynamic([e], selected_as(subquery(total_visitors(site, query)), :__total_visitors)) + wrap_alias([], %{ + total_visitors: subquery(total_visitors(site, query)) + }) end def add_percentage_metric(q, site, query, metrics) do @@ -369,19 +178,14 @@ defmodule Plausible.Stats.Base do total_query = Query.set_dimensions(query, []) q - |> select_merge( - ^%{__total_visitors: total_visitors_subquery(site, total_query, query.include_imported)} - ) - |> select_merge(%{ + |> select_merge_as([], total_visitors_subquery(site, total_query, query.include_imported)) + |> select_merge_as([], %{ percentage: - selected_as( - fragment( - "if(? > 0, round(? / ? * 100, 1), null)", - selected_as(:__total_visitors), - selected_as(:visitors), - selected_as(:__total_visitors) - ), - :percentage + fragment( + "if(? > 0, round(? / ? * 100, 1), null)", + selected_as(:total_visitors), + selected_as(:visitors), + selected_as(:total_visitors) ) }) else @@ -401,19 +205,14 @@ defmodule Plausible.Stats.Base do # :TRICKY: Subquery is used due to event:goal breakdown above doing an UNION ALL subquery(q) - |> select_merge( - ^%{total_visitors: total_visitors_subquery(site, total_query, query.include_imported)} - ) - |> select_merge([e], %{ + |> select_merge_as([], total_visitors_subquery(site, total_query, query.include_imported)) + |> select_merge_as([e], %{ conversion_rate: - selected_as( - fragment( - "if(? > 0, round(? / ? * 100, 1), 0)", - selected_as(:__total_visitors), - e.visitors, - selected_as(:__total_visitors) - ), - :conversion_rate + fragment( + "if(? > 0, round(? / ? * 100, 1), 0)", + selected_as(:total_visitors), + e.visitors, + selected_as(:total_visitors) ) }) else diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index a0bdaa1509173..abf78beac382f 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -1,7 +1,7 @@ defmodule Plausible.Stats.Breakdown do use Plausible.ClickhouseRepo use Plausible - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments import Plausible.Stats.Base import Ecto.Query diff --git a/lib/plausible/stats/clickhouse.ex b/lib/plausible/stats/clickhouse.ex index ed112b9a93c20..d22a4bb0c569d 100644 --- a/lib/plausible/stats/clickhouse.ex +++ b/lib/plausible/stats/clickhouse.ex @@ -2,7 +2,7 @@ defmodule Plausible.Stats.Clickhouse do use Plausible use Plausible.Repo use Plausible.ClickhouseRepo - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments import Ecto.Query, only: [from: 2] diff --git a/lib/plausible/stats/current_visitors.ex b/lib/plausible/stats/current_visitors.ex index 4af146691238d..3249e510d7e5a 100644 --- a/lib/plausible/stats/current_visitors.ex +++ b/lib/plausible/stats/current_visitors.ex @@ -1,6 +1,6 @@ defmodule Plausible.Stats.CurrentVisitors do use Plausible.ClickhouseRepo - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments def current_visitors(site) do first_datetime = diff --git a/lib/plausible/stats/filter_suggestions.ex b/lib/plausible/stats/filter_suggestions.ex index 8713919e3e4a4..c4c59a373602e 100644 --- a/lib/plausible/stats/filter_suggestions.ex +++ b/lib/plausible/stats/filter_suggestions.ex @@ -1,7 +1,7 @@ defmodule Plausible.Stats.FilterSuggestions do use Plausible.Repo use Plausible.ClickhouseRepo - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments import Plausible.Stats.Base import Ecto.Query diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index 5b774d9ea9600..72a888d186d19 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -6,8 +6,7 @@ defmodule Plausible.Stats.Imported.Base do import Ecto.Query alias Plausible.Imported - alias Plausible.Stats.Filters - alias Plausible.Stats.Query + alias Plausible.Stats.{Filters, Query, SQL} @property_to_table_mappings %{ "visit:source" => "imported_sources", @@ -213,9 +212,9 @@ defmodule Plausible.Stats.Imported.Base do defp apply_filter(q, %Query{filters: filters}) do Enum.reduce(filters, q, fn [_, filter_key | _] = filter, q -> - db_field = Plausible.Stats.Filters.without_prefix(filter_key) + db_field = Filters.without_prefix(filter_key) mapped_db_field = Map.get(@db_field_mappings, db_field, db_field) - condition = Filters.WhereBuilder.build_condition(mapped_db_field, filter) + condition = SQL.WhereBuilder.build_condition(mapped_db_field, filter) where(q, ^condition) end) diff --git a/lib/plausible/stats/imported/imported.ex b/lib/plausible/stats/imported/imported.ex index 57cd6737bb192..4fcdd04a66dee 100644 --- a/lib/plausible/stats/imported/imported.ex +++ b/lib/plausible/stats/imported/imported.ex @@ -1,11 +1,11 @@ defmodule Plausible.Stats.Imported do - alias Plausible.Stats.Filters use Plausible.ClickhouseRepo + use Plausible.Stats.SQL.Fragments import Ecto.Query - import Plausible.Stats.Fragments import Plausible.Stats.Util, only: [shortname: 2] + alias Plausible.Stats.Filters alias Plausible.Stats.Imported alias Plausible.Stats.Query alias Plausible.Stats.SQL.QueryBuilder @@ -290,12 +290,8 @@ defmodule Plausible.Stats.Imported do "imported_custom_events" -> Imported.Base.query_imported("imported_custom_events", site, query) |> where([i], i.visitors > 0) - |> select_merge([i], %{ - dim0: - selected_as( - fragment("-indexOf(?, ?)", type(^events, {:array, :string}), i.name), - :dim0 - ) + |> select_merge_as([i], %{ + dim0: fragment("-indexOf(?, ?)", type(^events, {:array, :string}), i.name) }) |> select_imported_metrics(metrics) |> group_by([], selected_as(:dim0)) @@ -314,8 +310,8 @@ defmodule Plausible.Stats.Imported do ) |> join(:array, index in fragment("indices")) |> group_by([_i, index], index) - |> select_merge([_i, index], %{ - dim0: selected_as(type(fragment("?", index), :integer), :dim0) + |> select_merge_as([_i, index], %{ + dim0: type(fragment("?", index), :integer) }) |> select_imported_metrics(metrics) end) @@ -563,17 +559,8 @@ defmodule Plausible.Stats.Imported do defp group_imported_by(q, dim, key) when dim in [:source, :referrer] do q |> group_by([i], field(i, ^dim)) - |> select_merge([i], %{ - ^key => - selected_as( - fragment( - "if(empty(?), ?, ?)", - field(i, ^dim), - @no_ref, - field(i, ^dim) - ), - ^key - ) + |> select_merge_as([i], %{ + key => fragment("if(empty(?), ?, ?)", field(i, ^dim), @no_ref, field(i, ^dim)) }) end @@ -582,90 +569,70 @@ defmodule Plausible.Stats.Imported do q |> group_by([i], field(i, ^dim)) |> where([i], fragment("not empty(?)", field(i, ^dim))) - |> select_merge([i], %{^key => selected_as(field(i, ^dim), ^key)}) + |> select_merge_as([i], %{key => field(i, ^dim)}) end defp group_imported_by(q, :page, key) do q |> group_by([i], i.page) - |> select_merge([i], %{^key => selected_as(i.page, ^key), time_on_page: sum(i.time_on_page)}) + |> select_merge_as([i], %{key => i.page, time_on_page: sum(i.time_on_page)}) end defp group_imported_by(q, :country, key) do q |> group_by([i], i.country) |> where([i], i.country != "ZZ") - |> select_merge([i], %{^key => selected_as(i.country, ^key)}) + |> select_merge_as([i], %{key => i.country}) end defp group_imported_by(q, :region, key) do q |> group_by([i], i.region) |> where([i], i.region != "") - |> select_merge([i], %{^key => selected_as(i.region, ^key)}) + |> select_merge_as([i], %{key => i.region}) end defp group_imported_by(q, :city, key) do q |> group_by([i], i.city) |> where([i], i.city != 0 and not is_nil(i.city)) - |> select_merge([i], %{^key => selected_as(i.city, ^key)}) + |> select_merge_as([i], %{key => i.city}) end defp group_imported_by(q, dim, key) when dim in [:device, :browser] do q |> group_by([i], field(i, ^dim)) - |> select_merge([i], %{ - ^key => - selected_as( - fragment("if(empty(?), ?, ?)", field(i, ^dim), @not_set, field(i, ^dim)), - ^key - ) + |> select_merge_as([i], %{ + key => fragment("if(empty(?), ?, ?)", field(i, ^dim), @not_set, field(i, ^dim)) }) end defp group_imported_by(q, :browser_version, key) do q |> group_by([i], [i.browser_version]) - |> select_merge([i], %{ - ^key => - selected_as( - fragment( - "if(empty(?), ?, ?)", - i.browser_version, - @not_set, - i.browser_version - ), - ^key - ) + |> select_merge_as([i], %{ + key => fragment("if(empty(?), ?, ?)", i.browser_version, @not_set, i.browser_version) }) end defp group_imported_by(q, :os, key) do q |> group_by([i], i.operating_system) - |> select_merge([i], %{ - ^key => - selected_as( - fragment("if(empty(?), ?, ?)", i.operating_system, @not_set, i.operating_system), - ^key - ) + |> select_merge_as([i], %{ + key => fragment("if(empty(?), ?, ?)", i.operating_system, @not_set, i.operating_system) }) end defp group_imported_by(q, :os_version, key) do q |> group_by([i], [i.operating_system_version]) - |> select_merge([i], %{ - ^key => - selected_as( - fragment( - "if(empty(?), ?, ?)", - i.operating_system_version, - @not_set, - i.operating_system_version - ), - ^key + |> select_merge_as([i], %{ + key => + fragment( + "if(empty(?), ?, ?)", + i.operating_system_version, + @not_set, + i.operating_system_version ) }) end @@ -673,28 +640,28 @@ defmodule Plausible.Stats.Imported do defp group_imported_by(q, dim, key) when dim in [:entry_page, :exit_page] do q |> group_by([i], field(i, ^dim)) - |> select_merge([i], %{^key => selected_as(field(i, ^dim), ^key)}) + |> select_merge_as([i], %{key => field(i, ^dim)}) end defp group_imported_by(q, :name, key) do q |> group_by([i], i.name) - |> select_merge([i], %{^key => selected_as(i.name, ^key)}) + |> select_merge_as([i], %{key => i.name}) end defp group_imported_by(q, :url, key) do q |> group_by([i], i.link_url) - |> select_merge([i], %{ - ^key => selected_as(fragment("if(not empty(?), ?, ?)", i.link_url, i.link_url, @none), ^key) + |> select_merge_as([i], %{ + key => fragment("if(not empty(?), ?, ?)", i.link_url, i.link_url, @none) }) end defp group_imported_by(q, :path, key) do q |> group_by([i], i.path) - |> select_merge([i], %{ - ^key => selected_as(fragment("if(not empty(?), ?, ?)", i.path, i.path, @none), ^key) + |> select_merge_as([i], %{ + key => fragment("if(not empty(?), ?, ?)", i.path, i.path, @none) }) end @@ -705,23 +672,14 @@ defmodule Plausible.Stats.Imported do end defp select_joined_dimension(q, "visit:city", key) do - select_merge(q, [s, i], %{ - ^key => selected_as(fragment("greatest(?,?)", field(i, ^key), field(s, ^key)), ^key) + select_merge_as(q, [s, i], %{ + key => fragment("greatest(?,?)", field(i, ^key), field(s, ^key)) }) end defp select_joined_dimension(q, _dimension, key) do - select_merge(q, [s, i], %{ - ^key => - selected_as( - fragment( - "if(empty(?), ?, ?)", - field(s, ^key), - field(i, ^key), - field(s, ^key) - ), - ^key - ) + select_merge_as(q, [s, i], %{ + key => fragment("if(empty(?), ?, ?)", field(s, ^key), field(i, ^key), field(s, ^key)) }) end @@ -734,31 +692,31 @@ defmodule Plausible.Stats.Imported do defp select_joined_metrics(q, [:visits | rest]) do q - |> select_merge([s, i], %{visits: selected_as(s.visits + i.visits, :visits)}) + |> select_merge_as([s, i], %{visits: s.visits + i.visits}) |> select_joined_metrics(rest) end defp select_joined_metrics(q, [:visitors | rest]) do q - |> select_merge([s, i], %{visitors: selected_as(s.visitors + i.visitors, :visitors)}) + |> select_merge_as([s, i], %{visitors: s.visitors + i.visitors}) |> select_joined_metrics(rest) end defp select_joined_metrics(q, [:events | rest]) do q - |> select_merge([s, i], %{events: selected_as(s.events + i.events, :events)}) + |> select_merge_as([s, i], %{events: s.events + i.events}) |> select_joined_metrics(rest) end defp select_joined_metrics(q, [:pageviews | rest]) do q - |> select_merge([s, i], %{pageviews: selected_as(s.pageviews + i.pageviews, :pageviews)}) + |> select_merge_as([s, i], %{pageviews: s.pageviews + i.pageviews}) |> select_joined_metrics(rest) end defp select_joined_metrics(q, [:views_per_visit | rest]) do q - |> select_merge([s, i], %{ + |> select_merge_as([s, i], %{ views_per_visit: fragment( "if(? + ? > 0, round((? + ? * ?) / (? + ?), 2), 0)", @@ -776,7 +734,7 @@ defmodule Plausible.Stats.Imported do defp select_joined_metrics(q, [:bounce_rate | rest]) do q - |> select_merge([s, i], %{ + |> select_merge_as([s, i], %{ bounce_rate: fragment( "if(? + ? > 0, round(100 * (? + (? * ? / 100)) / (? + ?)), 0)", @@ -794,7 +752,7 @@ defmodule Plausible.Stats.Imported do defp select_joined_metrics(q, [:visit_duration | rest]) do q - |> select_merge([s, i], %{ + |> select_merge_as([s, i], %{ visit_duration: fragment( """ @@ -818,7 +776,7 @@ defmodule Plausible.Stats.Imported do defp select_joined_metrics(q, [:sample_percent | rest]) do q - |> select_merge([s, i], %{sample_percent: s.sample_percent}) + |> select_merge_as([s, i], %{sample_percent: s.sample_percent}) |> select_joined_metrics(rest) end @@ -831,10 +789,11 @@ defmodule Plausible.Stats.Imported do from(a in subquery(q1), full_join: b in subquery(q2), on: a.dim0 == b.dim0, - select: %{ - dim0: selected_as(fragment("if(? != 0, ?, ?)", a.dim0, a.dim0, b.dim0), :dim0) - } + select: %{} ) + |> select_merge_as([a, b], %{ + dim0: fragment("if(? != 0, ?, ?)", a.dim0, a.dim0, b.dim0) + }) |> select_joined_metrics(metrics) end end diff --git a/lib/plausible/stats/sql/expression.ex b/lib/plausible/stats/sql/expression.ex index d3f38a1428151..daea6791f47bc 100644 --- a/lib/plausible/stats/sql/expression.ex +++ b/lib/plausible/stats/sql/expression.ex @@ -1,131 +1,253 @@ defmodule Plausible.Stats.SQL.Expression do @moduledoc """ This module is responsible for generating SQL/Ecto expressions - for dimensions used in query select, group_by and order_by. + for dimensions and metrics used in query SELECT statement. + + Each dimension and metric is tagged with with selected_as for easier + usage down the line. """ + use Plausible + use Plausible.Stats.SQL.Fragments + import Ecto.Query - use Plausible.Stats.Fragments + alias Plausible.Stats.{Query, SQL} @no_ref "Direct / None" @not_set "(not set)" - defmacrop field_or_blank_value(expr, empty_value, select_alias) do + defmacrop field_or_blank_value(key, expr, empty_value) do quote do - dynamic( - [t], - selected_as( - fragment("if(empty(?), ?, ?)", unquote(expr), unquote(empty_value), unquote(expr)), - ^unquote(select_alias) - ) - ) + wrap_alias([t], %{ + unquote(key) => + fragment("if(empty(?), ?, ?)", unquote(expr), unquote(empty_value), unquote(expr)) + }) end end - def dimension("time:hour", query, select_alias) do - dynamic( - [t], - selected_as( - fragment("toStartOfHour(toTimeZone(?, ?))", t.timestamp, ^query.timezone), - ^select_alias - ) - ) + def dimension(key, "time:hour", query) do + wrap_alias([t], %{ + key => fragment("toStartOfHour(toTimeZone(?, ?))", t.timestamp, ^query.timezone) + }) end - def dimension("time:day", query, select_alias) do - dynamic( - [t], - selected_as( - fragment("toDate(toTimeZone(?, ?))", t.timestamp, ^query.timezone), - ^select_alias - ) - ) + def dimension(key, "time:day", query) do + wrap_alias([t], %{ + key => fragment("toDate(toTimeZone(?, ?))", t.timestamp, ^query.timezone) + }) end - def dimension("time:month", query, select_alias) do - dynamic( - [t], - selected_as( - fragment("toStartOfMonth(toTimeZone(?, ?))", t.timestamp, ^query.timezone), - ^select_alias - ) - ) + def dimension(key, "time:month", query) do + wrap_alias([t], %{ + key => fragment("toStartOfMonth(toTimeZone(?, ?))", t.timestamp, ^query.timezone) + }) end - def dimension("event:name", _query, select_alias), - do: dynamic([t], selected_as(t.name, ^select_alias)) + def dimension(key, "event:name", _query), + do: wrap_alias([t], %{key => t.name}) - def dimension("event:page", _query, select_alias), - do: dynamic([t], selected_as(t.pathname, ^select_alias)) + def dimension(key, "event:page", _query), + do: wrap_alias([t], %{key => t.pathname}) - def dimension("event:hostname", _query, select_alias), - do: dynamic([t], selected_as(t.hostname, ^select_alias)) + def dimension(key, "event:hostname", _query), + do: wrap_alias([t], %{key => t.hostname}) - def dimension("event:props:" <> property_name, _query, select_alias) do - dynamic( - [t], - selected_as( + def dimension(key, "event:props:" <> property_name, _query) do + wrap_alias([t], %{ + key => fragment( "if(not empty(?), ?, '(none)')", get_by_key(t, :meta, ^property_name), get_by_key(t, :meta, ^property_name) - ), - ^select_alias - ) - ) + ) + }) end - def dimension("visit:entry_page", _query, select_alias), - do: dynamic([t], selected_as(t.entry_page, ^select_alias)) + def dimension(key, "visit:entry_page", _query), + do: wrap_alias([t], %{key => t.entry_page}) + + def dimension(key, "visit:exit_page", _query), + do: wrap_alias([t], %{key => t.exit_page}) + + def dimension(key, "visit:utm_medium", _query), + do: field_or_blank_value(key, t.utm_medium, @not_set) + + def dimension(key, "visit:utm_source", _query), + do: field_or_blank_value(key, t.utm_source, @not_set) - def dimension("visit:exit_page", _query, select_alias), - do: dynamic([t], selected_as(t.exit_page, ^select_alias)) + def dimension(key, "visit:utm_campaign", _query), + do: field_or_blank_value(key, t.utm_campaign, @not_set) - def dimension("visit:utm_medium", _query, select_alias), - do: field_or_blank_value(t.utm_medium, @not_set, select_alias) + def dimension(key, "visit:utm_content", _query), + do: field_or_blank_value(key, t.utm_content, @not_set) - def dimension("visit:utm_source", _query, select_alias), - do: field_or_blank_value(t.utm_source, @not_set, select_alias) + def dimension(key, "visit:utm_term", _query), + do: field_or_blank_value(key, t.utm_term, @not_set) - def dimension("visit:utm_campaign", _query, select_alias), - do: field_or_blank_value(t.utm_campaign, @not_set, select_alias) + def dimension(key, "visit:source", _query), + do: field_or_blank_value(key, t.source, @no_ref) - def dimension("visit:utm_content", _query, select_alias), - do: field_or_blank_value(t.utm_content, @not_set, select_alias) + def dimension(key, "visit:referrer", _query), + do: field_or_blank_value(key, t.referrer, @no_ref) - def dimension("visit:utm_term", _query, select_alias), - do: field_or_blank_value(t.utm_term, @not_set, select_alias) + def dimension(key, "visit:device", _query), + do: field_or_blank_value(key, t.device, @not_set) - def dimension("visit:source", _query, select_alias), - do: field_or_blank_value(t.source, @no_ref, select_alias) + def dimension(key, "visit:os", _query), + do: field_or_blank_value(key, t.os, @not_set) - def dimension("visit:referrer", _query, select_alias), - do: field_or_blank_value(t.referrer, @no_ref, select_alias) + def dimension(key, "visit:os_version", _query), + do: field_or_blank_value(key, t.os_version, @not_set) - def dimension("visit:device", _query, select_alias), - do: field_or_blank_value(t.device, @not_set, select_alias) + def dimension(key, "visit:browser", _query), + do: field_or_blank_value(key, t.browser, @not_set) - def dimension("visit:os", _query, select_alias), - do: field_or_blank_value(t.os, @not_set, select_alias) + def dimension(key, "visit:browser_version", _query), + do: field_or_blank_value(key, t.browser_version, @not_set) - def dimension("visit:os_version", _query, select_alias), - do: field_or_blank_value(t.os_version, @not_set, select_alias) + def dimension(key, "visit:country", _query), + do: wrap_alias([t], %{key => t.country}) - def dimension("visit:browser", _query, select_alias), - do: field_or_blank_value(t.browser, @not_set, select_alias) + def dimension(key, "visit:region", _query), + do: wrap_alias([t], %{key => t.region}) - def dimension("visit:browser_version", _query, select_alias), - do: field_or_blank_value(t.browser_version, @not_set, select_alias) + def dimension(key, "visit:city", _query), + do: wrap_alias([t], %{key => t.city}) - def dimension("visit:country", _query, select_alias), - do: dynamic([t], selected_as(t.country, ^select_alias)) + def event_metric(:pageviews) do + wrap_alias([e], %{ + pageviews: + fragment("toUInt64(round(countIf(? = 'pageview') * any(_sample_factor)))", e.name) + }) + end + + def event_metric(:events) do + wrap_alias([], %{ + events: fragment("toUInt64(round(count(*) * any(_sample_factor)))") + }) + end + + def event_metric(:visitors) do + wrap_alias([e], %{ + visitors: fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", e.user_id) + }) + end + + def event_metric(:visits) do + wrap_alias([e], %{ + visits: fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", e.session_id) + }) + end - def dimension("visit:region", _query, select_alias), - do: dynamic([t], selected_as(t.region, ^select_alias)) + on_ee do + def event_metric(:total_revenue) do + wrap_alias( + [e], + %{ + total_revenue: + fragment("toDecimal64(sum(?) * any(_sample_factor), 3)", e.revenue_reporting_amount) + } + ) + end + + def event_metric(:average_revenue) do + wrap_alias( + [e], + %{ + average_revenue: + fragment("toDecimal64(avg(?) * any(_sample_factor), 3)", e.revenue_reporting_amount) + } + ) + end + end + + def event_metric(:sample_percent) do + wrap_alias([], %{ + sample_percent: + fragment("if(any(_sample_factor) > 1, round(100 / any(_sample_factor)), 100)") + }) + end + + def event_metric(:percentage), do: %{} + def event_metric(:conversion_rate), do: %{} + def event_metric(:group_conversion_rate), do: %{} + def event_metric(:total_visitors), do: %{} + + def event_metric(unknown), do: raise("Unknown metric: #{unknown}") + + def session_metric(:bounce_rate, query) do + # :TRICKY: If page is passed to query, we only count bounce rate where users _entered_ at page. + event_page_filter = Query.get_filter(query, "event:page") + condition = SQL.WhereBuilder.build_condition(:entry_page, event_page_filter) + + wrap_alias([], %{ + bounce_rate: + fragment( + "toUInt32(ifNotFinite(round(sumIf(is_bounce * sign, ?) / sumIf(sign, ?) * 100), 0))", + ^condition, + ^condition + ), + __internal_visits: fragment("toUInt32(sum(sign))") + }) + end + + def session_metric(:visits, _query) do + wrap_alias([s], %{ + visits: fragment("toUInt64(round(sum(?) * any(_sample_factor)))", s.sign) + }) + end + + def session_metric(:pageviews, _query) do + wrap_alias([s], %{ + pageviews: + fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.pageviews) + }) + end + + def session_metric(:events, _query) do + wrap_alias([s], %{ + events: fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.events) + }) + end + + def session_metric(:visitors, _query) do + wrap_alias([s], %{ + visitors: fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", s.user_id) + }) + end + + def session_metric(:visit_duration, _query) do + wrap_alias([], %{ + visit_duration: + fragment("toUInt32(ifNotFinite(round(sum(duration * sign) / sum(sign)), 0))"), + __internal_visits: fragment("toUInt32(sum(sign))") + }) + end + + def session_metric(:views_per_visit, _query) do + wrap_alias([s], %{ + views_per_visit: + fragment( + "ifNotFinite(round(sum(? * ?) / sum(?), 2), 0)", + s.sign, + s.pageviews, + s.sign + ), + __internal_visits: fragment("toUInt32(sum(sign))") + }) + end + + def session_metric(:sample_percent, _query) do + wrap_alias([], %{ + sample_percent: + fragment("if(any(_sample_factor) > 1, round(100 / any(_sample_factor)), 100)") + }) + end - def dimension("visit:city", _query, select_alias), - do: dynamic([t], selected_as(t.city, ^select_alias)) + def session_metric(:percentage, _query), do: %{} + def session_metric(:conversion_rate, _query), do: %{} + def session_metric(:group_conversion_rate, _query), do: %{} defmacro event_goal_join(events, page_regexes) do quote do diff --git a/lib/plausible/stats/fragments.ex b/lib/plausible/stats/sql/fragments.ex similarity index 65% rename from lib/plausible/stats/fragments.ex rename to lib/plausible/stats/sql/fragments.ex index 27f49839f51cc..526682f9ef6c3 100644 --- a/lib/plausible/stats/fragments.ex +++ b/lib/plausible/stats/sql/fragments.ex @@ -1,4 +1,15 @@ -defmodule Plausible.Stats.Fragments do +defmodule Plausible.Stats.SQL.Fragments do + @moduledoc """ + Various macros and common SQL fragments used in Stats code. + """ + + defmacro __using__(_) do + quote do + import Plausible.Stats.SQL.Fragments + require Plausible.Stats.SQL.Fragments + end + end + defmacro uniq(user_id) do quote do fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", unquote(user_id)) @@ -56,21 +67,23 @@ defmodule Plausible.Stats.Fragments do `not_before` boundary is set to the past Saturday, which is before the weekstart, therefore the cap does not apply. - iex> this_wednesday = ~D[2022-11-09] - ...> past_saturday = ~D[2022-11-05] - ...> weekstart_not_before(this_wednesday, past_saturday) + ``` + > this_wednesday = ~D[2022-11-09] + > past_saturday = ~D[2022-11-05] + > weekstart_not_before(this_wednesday, past_saturday) ~D[2022-11-07] - + ``` In this other example, the fragment returns Tuesday and not the weekstart. The `not_before` boundary is set to Tuesday, which is past the weekstart, therefore the cap applies. - iex> this_wednesday = ~D[2022-11-09] - ...> this_tuesday = ~D[2022-11-08] - ...> weekstart_not_before(this_wednesday, this_tuesday) + ``` + > this_wednesday = ~D[2022-11-09] + > this_tuesday = ~D[2022-11-08] + > weekstart_not_before(this_wednesday, this_tuesday) ~D[2022-11-08] - + ``` """ defmacro weekstart_not_before(date, not_before) do quote do @@ -85,7 +98,7 @@ defmodule Plausible.Stats.Fragments do end @doc """ - Same as Plausible.Stats.Fragments.weekstart_not_before/2 but converts dates to + Same as Plausible.Stats.SQL.Fragments.weekstart_not_before/2 but converts dates to the specified timezone. """ defmacro weekstart_not_before(date, not_before, timezone) do @@ -143,9 +156,51 @@ defmodule Plausible.Stats.Fragments do def meta_value_column(:meta), do: :"meta.value" def meta_value_column(:entry_meta), do: :"entry_meta.value" - defmacro __using__(_) do + @doc """ + Convenience Ecto macro for wrapping a map passed to select_merge_as such that each + expression gets wrapped in dynamic and set as selected_as. + + ### Examples + + iex> wrap_alias([t], %{ foo: t.column }) |> expand_macro_once + "%{foo: dynamic([t], selected_as(t.column, :foo))}" + """ + defmacro wrap_alias(binding, map_literal) do + update_literal_map_values(map_literal, fn {key, expr} -> + key_expr = + if Macro.quoted_literal?(key) do + key + else + quote(do: ^unquote(key)) + end + + quote(do: dynamic(unquote(binding), selected_as(unquote(expr), unquote(key_expr)))) + end) + end + + @doc """ + Convenience Ecto macro for wrapping select_merge where each value gets in turn passed to selected_as. + + ### Examples + + iex> select_merge_as(q, [t], %{ foo: t.column }) |> expand_macro_once + "select_merge(q, [], ^wrap_alias([t], %{foo: t.column}))" + """ + defmacro select_merge_as(q, binding, map_literal) do quote do - import Plausible.Stats.Fragments + select_merge(unquote(q), [], ^wrap_alias(unquote(binding), unquote(map_literal))) end end + + defp update_literal_map_values({:%{}, ctx, keyword_list}, mapper_fn) do + { + :%{}, + ctx, + Enum.map(keyword_list, fn {key, expr} -> + {key, mapper_fn.({key, expr})} + end) + } + end + + defp update_literal_map_values(ast, _), do: ast end diff --git a/lib/plausible/stats/sql/query_builder.ex b/lib/plausible/stats/sql/query_builder.ex index 7e5e49a887f1f..8cf92725bcace 100644 --- a/lib/plausible/stats/sql/query_builder.ex +++ b/lib/plausible/stats/sql/query_builder.ex @@ -2,12 +2,13 @@ defmodule Plausible.Stats.SQL.QueryBuilder do @moduledoc false use Plausible + use Plausible.Stats.SQL.Fragments import Ecto.Query import Plausible.Stats.Imported import Plausible.Stats.Util - alias Plausible.Stats.{Base, Query, QueryOptimizer, TableDecider, Filters} + alias Plausible.Stats.{Base, Filters, Query, QueryOptimizer, TableDecider, SQL} alias Plausible.Stats.SQL.Expression require Plausible.Stats.SQL.Expression @@ -30,7 +31,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do q = from( e in "events_v2", - where: ^Filters.WhereBuilder.build(:events, site, events_query), + where: ^SQL.WhereBuilder.build(:events, site, events_query), select: ^Base.select_event_metrics(events_query.metrics) ) @@ -73,7 +74,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do q = from( e in "sessions_v2", - where: ^Filters.WhereBuilder.build(:sessions, site, sessions_query), + where: ^SQL.WhereBuilder.build(:sessions, site, sessions_query), select: ^Base.select_session_metrics(sessions_query.metrics, sessions_query) ) @@ -94,7 +95,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do if Query.has_event_filters?(query) do events_q = from(e in "events_v2", - where: ^Filters.WhereBuilder.build(:events, site, query), + where: ^SQL.WhereBuilder.build(:events, site, query), select: %{ session_id: fragment("DISTINCT ?", e.session_id), _sample_factor: fragment("_sample_factor") @@ -135,7 +136,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do key = shortname(query, dimension) q - |> select_merge(^%{key => Expression.dimension(dimension, query, key)}) + |> select_merge_as([], Expression.dimension(key, dimension, query)) |> group_by([], selected_as(^key)) end @@ -147,9 +148,9 @@ defmodule Plausible.Stats.SQL.QueryBuilder do order_by( q, [t], - ^{ - order_direction, - dynamic([], selected_as(^shortname(query, metric_or_dimension))) + { + ^order_direction, + selected_as(^shortname(query, metric_or_dimension)) } ) end @@ -157,19 +158,11 @@ defmodule Plausible.Stats.SQL.QueryBuilder do defmacrop select_join_fields(q, query, list, table_name) do quote do Enum.reduce(unquote(list), unquote(q), fn metric_or_dimension, q -> - select_merge( - q, - ^%{ - shortname(unquote(query), metric_or_dimension) => - dynamic( - [e, s], - selected_as( - field(unquote(table_name), ^shortname(unquote(query), metric_or_dimension)), - ^shortname(unquote(query), metric_or_dimension) - ) - ) - } - ) + key = shortname(unquote(query), metric_or_dimension) + + select_merge_as(q, [e, s], %{ + key => field(unquote(table_name), ^key) + }) end) end end @@ -185,21 +178,17 @@ defmodule Plausible.Stats.SQL.QueryBuilder do |> Query.set_dimensions([]) q - |> select_merge( - ^%{ - total_visitors: Base.total_visitors_subquery(site, total_query, query.include_imported) - } + |> select_merge_as( + [], + Base.total_visitors_subquery(site, total_query, query.include_imported) ) - |> select_merge([e], %{ + |> select_merge_as([e], %{ conversion_rate: - selected_as( - fragment( - "if(? > 0, round(? / ? * 100, 1), 0)", - selected_as(:__total_visitors), - selected_as(:visitors), - selected_as(:__total_visitors) - ), - :conversion_rate + fragment( + "if(? > 0, round(? / ? * 100, 1), 0)", + selected_as(:total_visitors), + selected_as(:visitors), + selected_as(:total_visitors) ) }) else @@ -228,21 +217,18 @@ defmodule Plausible.Stats.SQL.QueryBuilder do from(e in subquery(q), left_join: c in subquery(build(group_totals_query, site)), - on: ^build_group_by_join(query), - select_merge: %{ - total_visitors: c.visitors, - group_conversion_rate: - selected_as( - fragment( - "if(? > 0, round(? / ? * 100, 1), 0)", - c.visitors, - e.visitors, - c.visitors - ), - :group_conversion_rate - ) - } + on: ^build_group_by_join(query) ) + |> select_merge_as([e, c], %{ + total_visitors: c.visitors, + group_conversion_rate: + fragment( + "if(? > 0, round(? / ? * 100, 1), 0)", + c.visitors, + e.visitors, + c.visitors + ) + }) |> select_join_fields(query, query.dimensions, e) |> select_join_fields(query, List.delete(query.metrics, :group_conversion_rate), e) else diff --git a/lib/plausible/stats/filters/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex similarity index 99% rename from lib/plausible/stats/filters/where_builder.ex rename to lib/plausible/stats/sql/where_builder.ex index 257d4257a027d..71bcdad495199 100644 --- a/lib/plausible/stats/filters/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -1,4 +1,4 @@ -defmodule Plausible.Stats.Filters.WhereBuilder do +defmodule Plausible.Stats.SQL.WhereBuilder do @moduledoc """ A module for building am ecto where clause of a query out of a query. """ @@ -8,7 +8,7 @@ defmodule Plausible.Stats.Filters.WhereBuilder do alias Plausible.Stats.Query - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments require Logger diff --git a/lib/plausible/stats/timeseries.ex b/lib/plausible/stats/timeseries.ex index 69e3dc528f4c7..4914023bfc4a1 100644 --- a/lib/plausible/stats/timeseries.ex +++ b/lib/plausible/stats/timeseries.ex @@ -4,7 +4,7 @@ defmodule Plausible.Stats.Timeseries do alias Plausible.Stats.{Query, Util, Imported} import Plausible.Stats.{Base} import Ecto.Query - use Plausible.Stats.Fragments + use Plausible.Stats.SQL.Fragments @typep metric :: :pageviews diff --git a/lib/plausible/stats/util.ex b/lib/plausible/stats/util.ex index 60bdd7a931ab1..de1411f637e29 100644 --- a/lib/plausible/stats/util.ex +++ b/lib/plausible/stats/util.ex @@ -6,7 +6,6 @@ defmodule Plausible.Stats.Util do @manually_removable_metrics [ :__internal_visits, :visitors, - :__total_visitors, :__breakdown_value, :total_visitors ] diff --git a/test/plausible/stats/sql/fragments_test.exs b/test/plausible/stats/sql/fragments_test.exs new file mode 100644 index 0000000000000..0932d7e59894c --- /dev/null +++ b/test/plausible/stats/sql/fragments_test.exs @@ -0,0 +1,10 @@ +defmodule Plausible.Stats.SQL.FragmentsTest do + use ExUnit.Case, async: true + use Plausible.Stats.SQL.Fragments + + defmacro expand_macro_once(ast) do + ast |> Macro.expand_once(__ENV__) |> Macro.to_string() + end + + doctest Plausible.Stats.SQL.Fragments +end