Move advert update server to app namespace and convert to GenServer

lib/philomena/adverts.ex

@@ -8,6 +8,7 @@ defmodule Philomena.Adverts do
 
   alias Philomena.Adverts.Advert
   alias Philomena.Adverts.Restrictions
+  alias Philomena.Adverts.Server
   alias Philomena.Adverts.Uploader
 
   @doc """
@@ -64,6 +65,32 @@ defmodule Philomena.Adverts do
     Repo.one(query)
   end
 
+  @doc """
+  Asynchronously records a new impression.
+
+  ## Example
+
+      iex> record_impression(%Advert{})
+      :ok
+
+  """
+  def record_impression(%Advert{id: id}) do
+    Server.record_impression(id)
+  end
+
+  @doc """
+  Asynchronously records a new click.
+
+  ## Example
+
+      iex> record_click(%Advert{})
+      :ok
+
+  """
+  def record_click(%Advert{id: id}) do
+    Server.record_click(id)
+  end
+
   @doc """
   Gets a single advert.
 

lib/philomena/adverts/recorder.ex

@@ -1,33 +1,9 @@
-defmodule PhilomenaWeb.AdvertUpdater do
+defmodule Philomena.Adverts.Recorder do
   alias Philomena.Adverts.Advert
   alias Philomena.Repo
   import Ecto.Query
 
-  def child_spec([]) do
+  def run(%{impressions: impressions, clicks: clicks}) do
-    %{
-      id: PhilomenaWeb.AdvertUpdater,
-      start: {PhilomenaWeb.AdvertUpdater, :start_link, [[]]}
-    }
-  end
-
-  def start_link([]) do
-    {:ok, spawn_link(&init/0)}
-  end
-
-  def cast(type, advert_id) when type in [:impression, :click] do
-    pid = Process.whereis(:advert_updater)
-    if pid, do: send(pid, {type, advert_id})
-  end
-
-  defp init do
-    Process.register(self(), :advert_updater)
-    run()
-  end
-
-  defp run do
-    # Read impression counts from mailbox
-    {impressions, clicks} = receive_all()
-
     now = DateTime.utc_now() |> DateTime.truncate(:second)
 
     # Create insert statements for Ecto
@@ -41,24 +17,7 @@ defmodule PhilomenaWeb.AdvertUpdater do
     Repo.insert_all(Advert, impressions, on_conflict: impressions_update, conflict_target: [:id])
     Repo.insert_all(Advert, clicks, on_conflict: clicks_update, conflict_target: [:id])
 
-    :timer.sleep(:timer.seconds(10))
+    :ok
-
-    run()
-  end
-
-  defp receive_all(impressions \\ %{}, clicks \\ %{}) do
-    receive do
-      {:impression, advert_id} ->
-        impressions = Map.update(impressions, advert_id, 1, &(&1 + 1))
-        receive_all(impressions, clicks)
-
-      {:click, advert_id} ->
-        clicks = Map.update(clicks, advert_id, 1, &(&1 + 1))
-        receive_all(impressions, clicks)
-    after
-      0 ->
-        {impressions, clicks}
-    end
   end
 
   defp impressions_insert_all({advert_id, impressions}, now) do

lib/philomena/adverts/server.ex

@@ -0,0 +1,94 @@
+defmodule Philomena.Adverts.Server do
+  @moduledoc """
+  Advert impression and click aggregator.
+
+  Updating the impression count for adverts and clicks on every pageload is unnecessary
+  and slows down requests. This module collects the adverts and clicks and submits a batch
+  of updates to the database after every 10 seconds asynchronously, reducing the amount of
+  work to be done.
+  """
+
+  use GenServer
+  alias Philomena.Adverts.Recorder
+
+  @type advert_id :: integer()
+
+  @doc """
+  Starts the GenServer.
+
+  See `GenServer.start_link/2` for more information.
+  """
+  def start_link(_) do
+    GenServer.start_link(__MODULE__, [], name: __MODULE__)
+  end
+
+  @doc """
+  Asynchronously records a new impression.
+
+  ## Example
+
+      iex> record_impression(advert.id)
+      :ok
+
+  """
+  @spec record_impression(advert_id()) :: :ok
+  def record_impression(advert_id) do
+    GenServer.cast(__MODULE__, {:impressions, advert_id})
+  end
+
+  @doc """
+  Asynchronously records a new click.
+
+  ## Example
+
+      iex> record_click(advert.id)
+      :ok
+
+  """
+  @spec record_click(advert_id()) :: :ok
+  def record_click(advert_id) do
+    GenServer.cast(__MODULE__, {:clicks, advert_id})
+  end
+
+  # Used to force the GenServer to immediately sleep when no
+  # messages are available.
+  @timeout 0
+  @sleep :timer.seconds(10)
+
+  @impl true
+  @doc false
+  def init(_) do
+    {:ok, initial_state(), @timeout}
+  end
+
+  @impl true
+  @doc false
+  def handle_cast({type, advert_id}, state) do
+    # Update the counter described by the message
+    state = update_in(state[type], &increment_counter(&1, advert_id))
+
+    # Return to GenServer event loop
+    {:noreply, state, @timeout}
+  end
+
+  @impl true
+  @doc false
+  def handle_info(:timeout, state) do
+    # Process all updates from state now
+    Recorder.run(state)
+
+    # Sleep for the specified delay
+    :timer.sleep(@sleep)
+
+    # Return to GenServer event loop
+    {:noreply, initial_state(), @timeout}
+  end
+
+  defp increment_counter(map, advert_id) do
+    Map.update(map, advert_id, 1, &(&1 + 1))
+  end
+
+  defp initial_state do
+    %{impressions: %{}, clicks: %{}}
+  end
+end

lib/philomena/application.ex

@@ -28,8 +28,10 @@ defmodule Philomena.Application do
          node_name: valid_node_name(node())
        ]},
 
+      # Advert update batching
+      Philomena.Adverts.Server,
+
       # Start the endpoint when the application starts
-      PhilomenaWeb.AdvertUpdater,
       PhilomenaWeb.UserFingerprintUpdater,
       PhilomenaWeb.UserIpUpdater,
       PhilomenaWeb.Endpoint

lib/philomena_web/controllers/advert_controller.ex

@@ -1,15 +1,15 @@
 defmodule PhilomenaWeb.AdvertController do
   use PhilomenaWeb, :controller
 
-  alias PhilomenaWeb.AdvertUpdater
   alias Philomena.Adverts.Advert
+  alias Philomena.Adverts
 
   plug :load_resource, model: Advert
 
   def show(conn, _params) do
     advert = conn.assigns.advert
 
-    AdvertUpdater.cast(:click, advert.id)
+    Adverts.record_click(advert)
 
     redirect(conn, external: advert.link)
   end

lib/philomena_web/plugs/advert_plug.ex

@@ -1,5 +1,4 @@
 defmodule PhilomenaWeb.AdvertPlug do
-  alias PhilomenaWeb.AdvertUpdater
   alias Philomena.Adverts
   alias Plug.Conn
 
@@ -33,7 +32,7 @@ defmodule PhilomenaWeb.AdvertPlug do
   defp record_impression(nil), do: nil
 
   defp record_impression(advert) do
-    AdvertUpdater.cast(:impression, advert.id)
+    Adverts.record_impression(advert)
 
     advert
   end