From 180aa23478a2321e190fefe3b7b4dbb9d4bace21 Mon Sep 17 00:00:00 2001
From: Liam <byteslice@airmail.cc>
Date: Sat, 22 Jun 2024 12:06:48 -0400
Subject: [PATCH 1/2] Extract ad restriction application to new module

---
 lib/philomena/adverts.ex               | 104 +++++++++++++++----------
 lib/philomena/adverts/restrictions.ex  |  47 +++++++++++
 lib/philomena_web/plugs/advert_plug.ex |   2 +-
 3 files changed, 110 insertions(+), 43 deletions(-)
 create mode 100644 lib/philomena/adverts/restrictions.ex

diff --git a/lib/philomena/adverts.ex b/lib/philomena/adverts.ex
index c0dd178c..523f7e19 100644
--- a/lib/philomena/adverts.ex
+++ b/lib/philomena/adverts.ex
@@ -7,53 +7,61 @@ defmodule Philomena.Adverts do
   alias Philomena.Repo
 
   alias Philomena.Adverts.Advert
+  alias Philomena.Adverts.Restrictions
   alias Philomena.Adverts.Uploader
 
+  @doc """
+  Gets an advert that is currently live.
+
+  Returns the advert, or nil if nothing was live.
+
+      iex> random_live()
+      nil
+
+      iex> random_live()
+      %Advert{}
+
+  """
   def random_live do
+    random_live_for_tags([])
+  end
+
+  @doc """
+  Gets an advert that is currently live, matching any tagging restrictions
+  for the given image.
+
+  Returns the advert, or nil if nothing was live.
+
+  ## Examples
+
+      iex> random_live(%Image{})
+      nil
+
+      iex> random_live(%Image{})
+      %Advert{}
+
+  """
+  def random_live(image) do
+    image
+    |> Repo.preload(:tags)
+    |> Map.get(:tags)
+    |> Enum.map(& &1.name)
+    |> random_live_for_tags()
+  end
+
+  defp random_live_for_tags(tags) do
     now = DateTime.utc_now()
+    restrictions = Restrictions.tags(tags)
 
-    Advert
-    |> where(live: true, restrictions: "none")
-    |> where([a], a.start_date < ^now and a.finish_date > ^now)
-    |> order_by(asc: fragment("random()"))
-    |> limit(1)
-    |> Repo.one()
-  end
+    query =
+      from a in Advert,
+        where: a.live == true,
+        where: a.restrictions in ^restrictions,
+        where: a.start_date < ^now and a.finish_date > ^now,
+        order_by: [asc: fragment("random()")],
+        limit: 1
 
-  def random_live_for(image) do
-    image = Repo.preload(image, :tags)
-    now = DateTime.utc_now()
-
-    Advert
-    |> where(live: true)
-    |> where([a], a.restrictions in ^restrictions(image))
-    |> where([a], a.start_date < ^now and a.finish_date > ^now)
-    |> order_by(asc: fragment("random()"))
-    |> limit(1)
-    |> Repo.one()
-  end
-
-  defp sfw?(image) do
-    image_tags = MapSet.new(image.tags |> Enum.map(& &1.name))
-    sfw_tags = MapSet.new(["safe", "suggestive"])
-    intersect = MapSet.intersection(image_tags, sfw_tags)
-
-    MapSet.size(intersect) > 0
-  end
-
-  defp nsfw?(image) do
-    image_tags = MapSet.new(image.tags |> Enum.map(& &1.name))
-    nsfw_tags = MapSet.new(["questionable", "explicit"])
-    intersect = MapSet.intersection(image_tags, nsfw_tags)
-
-    MapSet.size(intersect) > 0
-  end
-
-  defp restrictions(image) do
-    restrictions = ["none"]
-    restrictions = if nsfw?(image), do: ["nsfw" | restrictions], else: restrictions
-    restrictions = if sfw?(image), do: ["sfw" | restrictions], else: restrictions
-    restrictions
+    Repo.one(query)
   end
 
   @doc """
@@ -102,7 +110,7 @@ defmodule Philomena.Adverts do
   end
 
   @doc """
-  Updates an advert.
+  Updates an Advert without updating its image.
 
   ## Examples
 
@@ -119,6 +127,18 @@ defmodule Philomena.Adverts do
     |> Repo.update()
   end
 
+  @doc """
+  Updates the image for an Advert.
+
+  ## Examples
+
+      iex> update_advert_image(advert, %{image: new_value})
+      {:ok, %Advert{}}
+
+      iex> update_advert(advert, %{image: bad_value})
+      {:error, %Ecto.Changeset{}}
+
+  """
   def update_advert_image(%Advert{} = advert, attrs) do
     advert
     |> Advert.changeset(attrs)
diff --git a/lib/philomena/adverts/restrictions.ex b/lib/philomena/adverts/restrictions.ex
new file mode 100644
index 00000000..60ef4153
--- /dev/null
+++ b/lib/philomena/adverts/restrictions.ex
@@ -0,0 +1,47 @@
+defmodule Philomena.Adverts.Restrictions do
+  @moduledoc """
+  Advert restriction application.
+  """
+
+  @type restriction :: String.t()
+  @type restriction_list :: [restriction()]
+  @type tag_list :: [String.t()]
+
+  @nsfw_tags MapSet.new(["questionable", "explicit"])
+  @sfw_tags MapSet.new(["safe", "suggestive"])
+
+  @doc """
+  Calculates the restrictions available to a given tag list.
+
+  Returns a list containing `"none"`, and neither or one of `"sfw"`, `"nsfw"`.
+
+  ## Examples
+
+      iex> tags([])
+      ["none"]
+
+      iex> tags(["safe"])
+      ["sfw", "none"]
+
+      iex> tags(["explicit"])
+      ["nsfw", "none"]
+
+  """
+  @spec tags(tag_list()) :: restriction_list()
+  def tags(tags) do
+    tags = MapSet.new(tags)
+
+    ["none"]
+    |> apply_if(tags, @nsfw_tags, "nsfw")
+    |> apply_if(tags, @sfw_tags, "sfw")
+  end
+
+  @spec apply_if(restriction_list(), MapSet.t(), MapSet.t(), restriction()) :: restriction_list()
+  defp apply_if(restrictions, tags, test, new_restriction) do
+    if MapSet.disjoint?(tags, test) do
+      restrictions
+    else
+      [new_restriction | restrictions]
+    end
+  end
+end
diff --git a/lib/philomena_web/plugs/advert_plug.ex b/lib/philomena_web/plugs/advert_plug.ex
index 37a5fe04..8ed4f543 100644
--- a/lib/philomena_web/plugs/advert_plug.ex
+++ b/lib/philomena_web/plugs/advert_plug.ex
@@ -19,7 +19,7 @@ defmodule PhilomenaWeb.AdvertPlug do
     do: Conn.assign(conn, :advert, record_impression(Adverts.random_live()))
 
   defp maybe_assign_ad(conn, image, true),
-    do: Conn.assign(conn, :advert, record_impression(Adverts.random_live_for(image)))
+    do: Conn.assign(conn, :advert, record_impression(Adverts.random_live(image)))
 
   defp maybe_assign_ad(conn, _image, _false),
     do: Conn.assign(conn, :advert, nil)

From 003ebed59a101fe416e40371ed50859f678c3897 Mon Sep 17 00:00:00 2001
From: Liam <byteslice@airmail.cc>
Date: Sat, 22 Jun 2024 13:46:36 -0400
Subject: [PATCH 2/2] Move advert update server to app namespace and convert to
 GenServer

---
 lib/philomena/adverts.ex                      | 27 ++++++
 .../adverts/recorder.ex}                      | 47 +---------
 lib/philomena/adverts/server.ex               | 94 +++++++++++++++++++
 lib/philomena/application.ex                  |  4 +-
 .../controllers/advert_controller.ex          |  4 +-
 lib/philomena_web/plugs/advert_plug.ex        |  3 +-
 6 files changed, 130 insertions(+), 49 deletions(-)
 rename lib/{philomena_web/advert_updater.ex => philomena/adverts/recorder.ex} (50%)
 create mode 100644 lib/philomena/adverts/server.ex

diff --git a/lib/philomena/adverts.ex b/lib/philomena/adverts.ex
index 523f7e19..06a1104e 100644
--- a/lib/philomena/adverts.ex
+++ b/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.
 
diff --git a/lib/philomena_web/advert_updater.ex b/lib/philomena/adverts/recorder.ex
similarity index 50%
rename from lib/philomena_web/advert_updater.ex
rename to lib/philomena/adverts/recorder.ex
index 02846f9e..19e15cf5 100644
--- a/lib/philomena_web/advert_updater.ex
+++ b/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
-    %{
-      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()
-
+  def run(%{impressions: impressions, clicks: clicks}) do
     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))
-
-    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
+    :ok
   end
 
   defp impressions_insert_all({advert_id, impressions}, now) do
diff --git a/lib/philomena/adverts/server.ex b/lib/philomena/adverts/server.ex
new file mode 100644
index 00000000..be759693
--- /dev/null
+++ b/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
diff --git a/lib/philomena/application.ex b/lib/philomena/application.ex
index f85de948..3ecfcc03 100644
--- a/lib/philomena/application.ex
+++ b/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
diff --git a/lib/philomena_web/controllers/advert_controller.ex b/lib/philomena_web/controllers/advert_controller.ex
index 9387fbd3..73c47482 100644
--- a/lib/philomena_web/controllers/advert_controller.ex
+++ b/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
diff --git a/lib/philomena_web/plugs/advert_plug.ex b/lib/philomena_web/plugs/advert_plug.ex
index 8ed4f543..797184a3 100644
--- a/lib/philomena_web/plugs/advert_plug.ex
+++ b/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