Refactor: Use common current_visitors code (#4071)

* Use common module for counting current visitors in external stats controller

* Refactor spike notifier, remove now-dead code

lib/plausible/stats/clickhouse.ex

@@ -120,12 +120,8 @@ defmodule Plausible.Stats.Clickhouse do
     ClickhouseRepo.all(referrers)
   end
 
-  def current_visitors(site, query) do
+  def current_visitors(site) do
-    Plausible.ClickhouseRepo.one(
+    Plausible.Stats.current_visitors(site)
-      from(e in base_query(site, query),
-        select: uniq(e.user_id)
-      )
-    )
   end
 
   def has_pageviews?(site) do
@@ -251,140 +247,6 @@ defmodule Plausible.Stats.Clickhouse do
     end
   end
 
-  defp base_query_bare(site, query) do
-    {first_datetime, last_datetime} = utc_boundaries(query, site)
-
-    q =
-      from(e in "events_v2",
-        where: e.site_id == ^site.id,
-        where: e.timestamp >= ^first_datetime and e.timestamp < ^last_datetime
-      )
-
-    on_ee do
-      q = Plausible.Stats.Sampling.add_query_hint(q, 10_000_000)
-    end
-
-    q =
-      if query.filters["screen"] do
-        size = query.filters["screen"]
-        from(e in q, where: e.screen_size == ^size)
-      else
-        q
-      end
-
-    q =
-      if query.filters["browser"] do
-        browser = query.filters["browser"]
-        from(s in q, where: s.browser == ^browser)
-      else
-        q
-      end
-
-    q =
-      if query.filters["browser_version"] do
-        version = query.filters["browser_version"]
-        from(s in q, where: s.browser_version == ^version)
-      else
-        q
-      end
-
-    q =
-      if query.filters["os"] do
-        os = query.filters["os"]
-        from(s in q, where: s.operating_system == ^os)
-      else
-        q
-      end
-
-    q =
-      if query.filters["os_version"] do
-        version = query.filters["os_version"]
-        from(s in q, where: s.operating_system_version == ^version)
-      else
-        q
-      end
-
-    q =
-      if query.filters["country"] do
-        country = query.filters["country"]
-        from(s in q, where: s.country_code == ^country)
-      else
-        q
-      end
-
-    q =
-      if query.filters["utm_medium"] do
-        utm_medium = query.filters["utm_medium"]
-        from(e in q, where: e.utm_medium == ^utm_medium)
-      else
-        q
-      end
-
-    q =
-      if query.filters["utm_source"] do
-        utm_source = query.filters["utm_source"]
-        from(e in q, where: e.utm_source == ^utm_source)
-      else
-        q
-      end
-
-    q =
-      if query.filters["utm_campaign"] do
-        utm_campaign = query.filters["utm_campaign"]
-        from(e in q, where: e.utm_campaign == ^utm_campaign)
-      else
-        q
-      end
-
-    q =
-      if query.filters["utm_content"] do
-        utm_content = query.filters["utm_content"]
-        from(e in q, where: e.utm_content == ^utm_content)
-      else
-        q
-      end
-
-    q =
-      if query.filters["utm_term"] do
-        utm_term = query.filters["utm_term"]
-        from(e in q, where: e.utm_term == ^utm_term)
-      else
-        q
-      end
-
-    q =
-      if query.filters["referrer"] do
-        ref = query.filters["referrer"]
-        from(e in q, where: e.referrer == ^ref)
-      else
-        q
-      end
-
-    q = include_path_filter(q, query.filters[:page])
-
-    if query.filters["props"] do
-      [{key, val}] = query.filters["props"] |> Enum.into([])
-
-      if val == "(none)" do
-        from(
-          e in q,
-          where: not has_key(e, :meta, ^key)
-        )
-      else
-        from(
-          e in q,
-          where: has_key(e, :meta, ^key) and get_by_key(e, :meta, ^key) == ^val
-        )
-      end
-    else
-      q
-    end
-  end
-
-  defp base_query(site, query) do
-    base_query_bare(site, query) |> include_goal_conversions(query)
-  end
-
   defp utc_boundaries(%Query{now: now, period: "30m"}, site) do
     last_datetime = now |> NaiveDateTime.truncate(:second)
 
@@ -425,83 +287,6 @@ defmodule Plausible.Stats.Clickhouse do
     {first_datetime, last_datetime}
   end
 
-  defp event_name_for_goal(query) do
-    case query.filters["goal"] do
-      "Visit " <> page ->
-        {"pageview", page}
-
-      goal when is_binary(goal) ->
-        {goal, nil}
-
-      _ ->
-        {nil, nil}
-    end
-  end
-
-  defp include_goal_conversions(db_query, query) do
-    {goal_event, path} = event_name_for_goal(query)
-
-    q =
-      if goal_event do
-        from(e in db_query, where: e.name == ^goal_event)
-      else
-        from(e in db_query, where: e.name == "pageview")
-      end
-
-    if path do
-      {contains_regex, path_regex} = convert_path_regex(path)
-
-      if contains_regex do
-        from(e in q, where: fragment("match(?, ?)", e.pathname, ^path_regex))
-      else
-        from(e in q, where: e.pathname == ^path)
-      end
-    else
-      q
-    end
-  end
-
-  defp check_negated_filter(filter) do
-    negated = String.at(filter, 0) == "!"
-    updated_filter = if negated, do: String.slice(filter, 1..-1), else: filter
-
-    {negated, updated_filter}
-  end
-
-  defp convert_path_regex(path) do
-    contains_regex = String.match?(path, ~r/\*/)
-
-    regex =
-      "^#{path}\/?$"
-      |> String.replace(~r/\*\*/, ".*")
-      |> String.replace(~r/(?<!\.)\*/, "[^/]*")
-
-    {contains_regex, regex}
-  end
-
-  defp include_path_filter(db_query, path) do
-    if path do
-      {negated, path} = check_negated_filter(path)
-      {contains_regex, path_regex} = convert_path_regex(path)
-
-      if contains_regex do
-        if negated do
-          from(e in db_query, where: fragment("not(match(?, ?))", e.pathname, ^path_regex))
-        else
-          from(e in db_query, where: fragment("match(?, ?)", e.pathname, ^path_regex))
-        end
-      else
-        if negated do
-          from(e in db_query, where: e.pathname != ^path)
-        else
-          from(e in db_query, where: e.pathname == ^path)
-        end
-      end
-    else
-      db_query
-    end
-  end
-
   defp beginning_of_time(candidate, site_creation_date) do
     if Timex.after?(site_creation_date, candidate) do
       site_creation_date

lib/plausible_web/controllers/api/external_stats_controller.ex

@@ -6,8 +6,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do
 
   def realtime_visitors(conn, _params) do
     site = conn.assigns.site
-    query = Query.from(site, %{"period" => "realtime"})
+    json(conn, Plausible.Stats.current_visitors(site))
-    json(conn, Plausible.Stats.Clickhouse.current_visitors(site, query))
   end
 
   def aggregate(conn, params) do

lib/workers/spike_notifier.ex

@@ -19,10 +19,10 @@ defmodule Plausible.Workers.SpikeNotifier do
       )
 
     for notification <- notifications do
-      query = Query.from(notification.site, %{"period" => "realtime"})
+      current_visitors = clickhouse.current_visitors(notification.site)
-      current_visitors = clickhouse.current_visitors(notification.site, query)
 
       if current_visitors >= notification.threshold do
+        query = Query.from(notification.site, %{"period" => "realtime"})
         sources = clickhouse.top_sources_for_spike(notification.site, query, 3, 1)
         notify(notification, current_visitors, sources)
       end

test/workers/spike_notifier_test.exs

@@ -14,7 +14,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do
     )
 
     clickhouse_stub =
-      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 5 end)
+      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 5 end)
       |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end)
 
     SpikeNotifier.perform(nil, clickhouse_stub)
@@ -32,7 +32,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do
     )
 
     clickhouse_stub =
-      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end)
+      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end)
       |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end)
 
     SpikeNotifier.perform(nil, clickhouse_stub)
@@ -58,7 +58,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do
     )
 
     clickhouse_stub =
-      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end)
+      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end)
       |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end)
 
     SpikeNotifier.perform(nil, clickhouse_stub)
@@ -71,7 +71,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do
     insert(:spike_notification, site: site, threshold: 10, recipients: ["uku@example.com"])
 
     clickhouse_stub =
-      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end)
+      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end)
       |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end)
 
     SpikeNotifier.perform(nil, clickhouse_stub)
@@ -92,7 +92,7 @@ defmodule Plausible.Workers.SpikeNotifierTest do
     insert(:spike_notification, site: site, threshold: 10, recipients: ["robert@example.com"])
 
     clickhouse_stub =
-      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site, _query -> 10 end)
+      stub(Plausible.Stats.Clickhouse, :current_visitors, fn _site -> 10 end)
       |> stub(:top_sources_for_spike, fn _site, _query, _limit, _page -> [] end)
 
     SpikeNotifier.perform(nil, clickhouse_stub)