Merge pull request #300 from philomena-dev/ad-extraction

Ads logic cleanup

lib/philomena/adverts.ex

@@ -7,53 +7,88 @@ defmodule Philomena.Adverts do
   alias Philomena.Repo
 
   alias Philomena.Adverts.Advert
+  alias Philomena.Adverts.Restrictions
+  alias Philomena.Adverts.Server
   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()
+    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
+
+    Repo.one(query)
   end
 
-  def random_live_for(image) do
-    image = Repo.preload(image, :tags)
-    now = DateTime.utc_now()
+  @doc """
+  Asynchronously records a new impression.
 
-    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()
+  ## Example
+
+      iex> record_impression(%Advert{})
+      :ok
+
+  """
+  def record_impression(%Advert{id: id}) do
+    Server.record_impression(id)
   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)
+  @doc """
+  Asynchronously records a new click.
 
-    MapSet.size(intersect) > 0
-  end
+  ## Example
 
-  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)
+      iex> record_click(%Advert{})
+      :ok
 
-    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
+  """
+  def record_click(%Advert{id: id}) do
+    Server.record_click(id)
   end
 
   @doc """
@@ -102,7 +137,7 @@ defmodule Philomena.Adverts do
   end
 
   @doc """
-  Updates an advert.
+  Updates an Advert without updating its image.
 
   ## Examples
 
@@ -119,6 +154,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)

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

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

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
 
@@ -19,7 +18,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)
@@ -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