Refactor Atom feed caching

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

Fixes
Fix tests
Fix tests

lib/mobilizon/application.ex

@@ -3,6 +3,7 @@ defmodule Mobilizon.Application do
   The Mobilizon application
   """
   use Application
+  import Cachex.Spec
 
   # See https://hexdocs.pm/elixir/Application.html
   # for more information on OTP Applications
@@ -17,7 +18,32 @@ defmodule Mobilizon.Application do
       supervisor(MobilizonWeb.Endpoint, []),
       # Start your own worker by calling: Mobilizon.Worker.start_link(arg1, arg2, arg3)
       # worker(Mobilizon.Worker, [arg1, arg2, arg3]),
-      worker(Cachex, [:mobilizon, []]),
+      worker(
+        Cachex,
+        [
+          :feed,
+          [
+            limit: 2500,
+            expiration:
+              expiration(
+                default: :timer.minutes(60),
+                interval: :timer.seconds(60)
+              ),
+            fallback: fallback(default: &Mobilizon.Service.Feed.create_cache/1)
+          ]
+        ],
+        id: :cache_feed
+      ),
+      worker(
+        Cachex,
+        [
+          :json,
+          [
+            limit: 2500
+          ]
+        ],
+        id: :cache_actor
+      ),
       worker(Guardian.DB.Token.SweeperServer, []),
       worker(Mobilizon.Service.Federator, [])
     ]

lib/mobilizon_web/controllers/activity_pub_controller.ex

@@ -21,10 +21,6 @@ defmodule MobilizonWeb.ActivityPubController do
     "application/activity+json, application/ld+json"
   ]
 
-  def actor(conn, %{"name" => _name, "_format" => "atom"} = params) do
-    MobilizonWeb.FeedController.actor(conn, params)
-  end
-
   def actor(conn, %{"name" => name}) do
     with %Actor{} = actor <- Actors.get_local_actor_by_name(name) do
       if conn |> get_req_header("accept") |> is_ap_header() do

lib/mobilizon_web/controllers/feed_controller.ex

@@ -4,102 +4,17 @@ defmodule MobilizonWeb.FeedController do
   """
   use MobilizonWeb, :controller
 
-  alias Mobilizon.Actors
+  def actor(conn, %{"name" => name, "format" => "atom"}) do
-  alias Mobilizon.Actors.Actor
-  alias Mobilizon.Events
-  alias Mobilizon.Events.Event
-  alias Atomex.{Feed, Entry}
-  import MobilizonWeb.Gettext
-
-  @version Mix.Project.config()[:version]
-  def version(), do: @version
-
-  def actor(conn, %{"name" => name, "_format" => format}) when format in ["atom"] do
-    name = String.replace_suffix(name, ".atom", "")
-
     with {status, data} when status in [:ok, :commit] <-
-           Cachex.fetch(:mobilizon, "actor_" <> format <> "_" <> name, &create_cache/1) do
+           Cachex.fetch(:feed, "actor_" <> name) do
       conn
       |> put_resp_content_type("application/atom+xml")
       |> send_resp(200, data)
     else
       _err ->
-        send_resp(conn, 404, "Not found")
+        conn
-    end
+        |> put_resp_content_type("text/html")
-  end
+        |> send_file(404, "priv/static/index.html")
-
-  @spec create_cache(String.t()) :: {:commit, String.t()} | {:ignore, any()}
-  defp create_cache(key) do
-    with ["actor", type, name] <- String.split(key, "_", parts: 3),
-         {:ok, res} <- fetch_actor_event_feed(type, name) do
-      {:commit, res}
-    else
-      err ->
-        {:ignore, err}
-    end
-  end
-
-  @spec fetch_actor_event_feed(String.t(), String.t()) :: String.t()
-  defp fetch_actor_event_feed(type, name) do
-    with %Actor{} = actor <- Actors.get_local_actor_by_name(name),
-         {:ok, events, _count} <- Events.get_public_events_for_actor(actor) do
-      {:ok, build_actor_feed(actor, events, type)}
-    else
-      err ->
-        {:error, err}
-    end
-  end
-
-  @spec build_actor_feed(Actor.t(), list(), String.t()) :: String.t()
-  defp build_actor_feed(%Actor{} = actor, events, type) do
-    display_name = Actor.display_name(actor)
-
-    # Title uses default instance language
-    feed =
-      Feed.new(
-        actor.url <> ".rss",
-        DateTime.utc_now(),
-        gettext("%{actor}'s public events feed", actor: display_name)
-      )
-      |> Feed.author(display_name, uri: actor.url)
-      |> Feed.link(actor.url <> "." <> type, rel: "self")
-      |> Feed.link(actor.url, rel: "alternate")
-      |> Feed.generator("Mobilizon", uri: "https://joinmobilizon.org", version: version())
-      |> Feed.entries(Enum.map(events, &get_entry/1))
-
-    feed = if actor.avatar_url, do: Feed.icon(feed, actor.avatar_url), else: feed
-
-    feed =
-      if actor.banner_url,
-        do: Feed.logo(feed, actor.banner_url),
-        else: feed
-
-    feed
-    |> Feed.build()
-    |> Atomex.generate_document()
-  end
-
-  defp get_entry(%Event{} = event) do
-    with {:ok, html, []} <- Earmark.as_html(event.description) do
-      entry =
-        Entry.new(event.url, event.inserted_at, event.title)
-        |> Entry.link(event.url, rel: "alternate", type: "text/html")
-        |> Entry.content({:cdata, html}, type: "html")
-
-      entry = if event.publish_at, do: Entry.published(entry, event.publish_at), else: entry
-
-      # Add tags
-      entry =
-        event.tags
-        |> Enum.map(& &1.title)
-        |> Enum.uniq()
-        |> Enum.reduce(entry, fn tag, acc -> Entry.category(acc, tag) end)
-
-      Entry.build(entry)
-    else
-      {:error, _html, error_messages} ->
-        require Logger
-        Logger.error("Unable to produce HTML for Markdown", details: inspect(error_messages))
     end
   end
 end

lib/mobilizon_web/router.ex

@@ -22,9 +22,8 @@ defmodule MobilizonWeb.Router do
     plug(:accepts, ["activity-json", "html"])
   end
 
-  pipeline :activity_pub_rss do
+  pipeline :rss do
-    plug(TrailingFormatPlug)
+    plug(:accepts, ["atom", "html"])
-    plug(:accepts, ["activity-json", "html", "atom"])
   end
 
   pipeline :browser do
@@ -57,14 +56,15 @@ defmodule MobilizonWeb.Router do
   end
 
   scope "/", MobilizonWeb do
-    pipe_through(:activity_pub_rss)
+    pipe_through(:rss)
 
-    get("/@:name", ActivityPubController, :actor)
+    get("/@:name/feed/:format", FeedController, :actor)
   end
 
   scope "/", MobilizonWeb do
     pipe_through(:activity_pub)
 
+    get("/@:name", ActivityPubController, :actor)
     get("/@:name/outbox", ActivityPubController, :outbox)
     get("/@:name/following", ActivityPubController, :following)
     get("/@:name/followers", ActivityPubController, :followers)

lib/service/feed.ex

@@ -0,0 +1,94 @@
+defmodule Mobilizon.Service.Feed do
+  @moduledoc """
+  Serve Atom Syndication Feeds
+  """
+
+  alias Mobilizon.Actors
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Events
+  alias Mobilizon.Events.Event
+  alias Atomex.{Feed, Entry}
+  import MobilizonWeb.Gettext
+  alias MobilizonWeb.Router.Helpers, as: Routes
+  alias MobilizonWeb.Endpoint
+
+  @version Mix.Project.config()[:version]
+  def version(), do: @version
+
+  @spec create_cache(String.t()) :: {:commit, String.t()} | {:ignore, any()}
+  def create_cache("actor_" <> name) do
+    with {:ok, res} <- fetch_actor_event_feed(name) do
+      {:commit, res}
+    else
+      err ->
+        {:ignore, err}
+    end
+  end
+
+  @spec fetch_actor_event_feed(String.t()) :: String.t()
+  defp fetch_actor_event_feed(name) do
+    with %Actor{} = actor <- Actors.get_local_actor_by_name(name),
+         {:ok, events, _count} <- Events.get_public_events_for_actor(actor) do
+      {:ok, build_actor_feed(actor, events)}
+    else
+      err ->
+        {:error, err}
+    end
+  end
+
+  # Build an atom feed from actor and it's public events
+  @spec build_actor_feed(Actor.t(), list()) :: String.t()
+  defp build_actor_feed(%Actor{} = actor, events) do
+    display_name = Actor.display_name(actor)
+    self_url = Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom") |> URI.decode()
+
+    # Title uses default instance language
+    feed =
+      Feed.new(
+        self_url,
+        DateTime.utc_now(),
+        gettext("%{actor}'s public events feed", actor: display_name)
+      )
+      |> Feed.author(display_name, uri: actor.url)
+      |> Feed.link(self_url, rel: "self")
+      |> Feed.link(actor.url, rel: "alternate")
+      |> Feed.generator("Mobilizon", uri: "https://joinmobilizon.org", version: version())
+      |> Feed.entries(Enum.map(events, &get_entry/1))
+
+    feed = if actor.avatar_url, do: Feed.icon(feed, actor.avatar_url), else: feed
+
+    feed =
+      if actor.banner_url,
+        do: Feed.logo(feed, actor.banner_url),
+        else: feed
+
+    feed
+    |> Feed.build()
+    |> Atomex.generate_document()
+  end
+
+  # Create an entry for the Atom feed
+  @spec get_entry(Event.t()) :: any()
+  defp get_entry(%Event{} = event) do
+    with {:ok, html, []} <- Earmark.as_html(event.description) do
+      entry =
+        Entry.new(event.url, event.inserted_at, event.title)
+        |> Entry.link(event.url, rel: "alternate", type: "text/html")
+        |> Entry.content({:cdata, html}, type: "html")
+
+      entry = if event.publish_at, do: Entry.published(entry, event.publish_at), else: entry
+
+      # Add tags
+      entry =
+        event.tags
+        |> Enum.uniq()
+        |> Enum.reduce(entry, fn tag, acc -> Entry.category(acc, tag.slug, label: tag.title) end)
+
+      Entry.build(entry)
+    else
+      {:error, _html, error_messages} ->
+        require Logger
+        Logger.error("Unable to produce HTML for Markdown", details: inspect(error_messages))
+    end
+  end
+end

mix.exs

@@ -87,7 +87,6 @@ defmodule Mobilizon.Mixfile do
       {:plug_cowboy, "~> 2.0"},
       {:atomex, "0.3.0"},
       {:cachex, "~> 3.1"},
-      {:trailing_format_plug, "~> 0.0.5"},
       {:earmark, "~> 1.3.1"},
       # Dev and test dependencies
       {:phoenix_live_reload, "~> 1.2", only: :dev},
@@ -100,7 +99,7 @@ defmodule Mobilizon.Mixfile do
       {:exvcr, "~> 0.10", only: :test},
       {:credo, "~> 1.0.0", only: [:dev, :test], runtime: false},
       {:mock, "~> 0.3.0", only: :test},
-      {:feeder_ex, "~> 1.1", only: :test}
+      {:elixir_feed_parser, "~> 2.1.0", only: :test}
     ]
   end
 

mix.lock

@@ -29,13 +29,14 @@
   "ecto_autoslug_field": {:hex, :ecto_autoslug_field, "1.0.0", "577eed25e6d045b8d783f82c9872f97c3a84017a4feae50eaf3cf4e1334a19e2", [:mix], [{:ecto, ">= 2.1.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:slugger, ">= 0.2.0", [hex: :slugger, repo: "hexpm", optional: false]}], "hexpm"},
   "ecto_enum": {:hex, :ecto_enum, "1.2.0", "9ead3ee04efc4cb68a50560a9d9ebb665dd697f957f1c3df8e81bf863cf7a4e9", [:mix], [{:ecto, ">= 2.0.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, ">= 0.0.0", [hex: :mariaex, repo: "hexpm", optional: true]}, {:postgrex, ">= 0.0.0", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm"},
   "ecto_sql": {:hex, :ecto_sql, "3.0.5", "7e44172b4f7aca4469f38d7f6a3da394dbf43a1bcf0ca975e958cb957becd74e", [:mix], [{:db_connection, "~> 2.0", [hex: :db_connection, repo: "hexpm", optional: false]}, {:ecto, "~> 3.0.6", [hex: :ecto, repo: "hexpm", optional: false]}, {:mariaex, "~> 0.9.1", [hex: :mariaex, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.14.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.3.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm"},
+  "elixir_feed_parser": {:hex, :elixir_feed_parser, "2.1.0", "bb96fb6422158dc7ad59de62ef211cc69d264acbbe63941a64a5dce97bbbc2e6", [:mix], [{:timex, "~> 3.4", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm"},
   "elixir_make": {:hex, :elixir_make, "0.5.2", "96a28c79f5b8d34879cd95ebc04d2a0d678cfbbd3e74c43cb63a76adf0ee8054", [:mix], [], "hexpm"},
   "erlex": {:hex, :erlex, "0.2.1", "cee02918660807cbba9a7229cae9b42d1c6143b768c781fa6cee1eaf03ad860b", [:mix], [], "hexpm"},
   "eternal": {:hex, :eternal, "1.2.1", "d5b6b2499ba876c57be2581b5b999ee9bdf861c647401066d3eeed111d096bc4", [:mix], [], "hexpm"},
   "ex_crypto": {:hex, :ex_crypto, "0.10.0", "af600a89b784b36613a989da6e998c1b200ff1214c3cfbaf8deca4aa2f0a1739", [:mix], [{:poison, ">= 2.0.0", [hex: :poison, repo: "hexpm", optional: false]}], "hexpm"},
   "ex_doc": {:hex, :ex_doc, "0.19.3", "3c7b0f02851f5fc13b040e8e925051452e41248f685e40250d7e40b07b9f8c10", [:mix], [{:earmark, "~> 1.2", [hex: :earmark, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.10", [hex: :makeup_elixir, repo: "hexpm", optional: false]}], "hexpm"},
   "ex_ical": {:hex, :ex_ical, "0.2.0", "4b928b554614704016cc0c9ee226eb854da9327a1cc460457621ceacb1ac29a6", [:mix], [{:timex, "~> 3.1", [hex: :timex, repo: "hexpm", optional: false]}], "hexpm"},
-  "ex_machina": {:hex, :ex_machina, "2.2.2", "d84217a6fb7840ff771d2561b8aa6d74a0d8968e4b10ecc0d7e9890dc8fb1c6a", [:mix], [{:ecto, "~> 2.2 or ~> 3.0", [hex: :ecto, repo: "hexpm", optional: true]}, {:ecto_sql, "~> 3.0", [hex: :ecto_sql, repo: "hexpm", optional: true]}], "hexpm"},
+  "ex_machina": {:hex, :ex_machina, "2.3.0", "92a5ad0a8b10ea6314b876a99c8c9e3f25f4dde71a2a835845b136b9adaf199a", [:mix], [{:ecto, "~> 2.2 or ~> 3.0", [hex: :ecto, repo: "hexpm", optional: true]}, {:ecto_sql, "~> 3.0", [hex: :ecto_sql, repo: "hexpm", optional: true]}], "hexpm"},
   "ex_unit_notifier": {:hex, :ex_unit_notifier, "0.1.4", "36a2dcab829f506e01bf17816590680dd1474407926d43e64c1263e627c364b8", [:mix], [], "hexpm"},
   "exactor": {:hex, :exactor, "2.2.4", "5efb4ddeb2c48d9a1d7c9b465a6fffdd82300eb9618ece5d34c3334d5d7245b1", [:mix], [], "hexpm"},
   "excoveralls": {:hex, :excoveralls, "0.10.5", "7c912c4ec0715a6013647d835c87cde8154855b9b84e256bc7a63858d5f284e3", [:mix], [{:hackney, "~> 1.13", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm"},
@@ -43,7 +44,6 @@
   "exjsx": {:hex, :exjsx, "4.0.0", "60548841e0212df401e38e63c0078ec57b33e7ea49b032c796ccad8cde794b5c", [:mix], [{:jsx, "~> 2.8.0", [hex: :jsx, repo: "hexpm", optional: false]}], "hexpm"},
   "exvcr": {:hex, :exvcr, "0.10.3", "1ae3b97560430acfa88ebc737c85b2b7a9dbacd8a2b26789a19718b51ae3522c", [:mix], [{:exactor, "~> 2.2", [hex: :exactor, repo: "hexpm", optional: false]}, {:exjsx, "~> 4.0", [hex: :exjsx, repo: "hexpm", optional: false]}, {:httpoison, "~> 1.0", [hex: :httpoison, repo: "hexpm", optional: true]}, {:httpotion, "~> 3.1", [hex: :httpotion, repo: "hexpm", optional: true]}, {:ibrowse, "~> 4.4", [hex: :ibrowse, repo: "hexpm", optional: true]}, {:meck, "~> 0.8", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm"},
   "feeder": {:hex, :feeder, "2.2.4", "56ec535cf2f79719bc53b5c2abe5f6cf481fc01e5ae6229ab7cc829644f039ec", [:make], [], "hexpm"},
-  "feeder_ex": {:hex, :feeder_ex, "1.1.0", "0be3732255cdb45dec949e0ede6852b5261c9ff173360e8274a6ac65183b2b55", [:mix], [{:feeder, "~> 2.2", [hex: :feeder, repo: "hexpm", optional: false]}], "hexpm"},
   "file_system": {:hex, :file_system, "0.2.6", "fd4dc3af89b9ab1dc8ccbcc214a0e60c41f34be251d9307920748a14bf41f1d3", [:mix], [], "hexpm"},
   "gen_smtp": {:hex, :gen_smtp, "0.12.0", "97d44903f5ca18ca85cb39aee7d9c77e98d79804bbdef56078adcf905cb2ef00", [:rebar3], [], "hexpm"},
   "geo": {:hex, :geo, "3.1.0", "727e005262430d037e870ff364e65d80ca5ca21d5ac8eddd57a1ada72c3f83b0", [:mix], [], "hexpm"},
@@ -92,7 +92,6 @@
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.4", "f0eafff810d2041e93f915ef59899c923f4568f4585904d010387ed74988e77b", [:make, :mix, :rebar3], [], "hexpm"},
   "telemetry": {:hex, :telemetry, "0.3.0", "099a7f3ce31e4780f971b4630a3c22ec66d22208bc090fe33a2a3a6a67754a73", [:rebar3], [], "hexpm"},
   "timex": {:hex, :timex, "3.5.0", "b0a23167da02d0fe4f1a4e104d1f929a00d348502b52432c05de875d0b9cffa5", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:gettext, "~> 0.10", [hex: :gettext, repo: "hexpm", optional: false]}, {:tzdata, "~> 0.1.8 or ~> 0.5", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm"},
-  "trailing_format_plug": {:hex, :trailing_format_plug, "0.0.7", "64b877f912cf7273bed03379936df39894149e35137ac9509117e59866e10e45", [:mix], [{:plug, "> 0.12.0", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm"},
   "tzdata": {:hex, :tzdata, "0.5.19", "7962a3997bf06303b7d1772988ede22260f3dae1bf897408ebdac2b4435f4e6a", [:mix], [{:hackney, "~> 1.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},
   "unicode_util_compat": {:hex, :unicode_util_compat, "0.4.1", "d869e4c68901dd9531385bb0c8c40444ebf624e60b6962d95952775cac5e90cd", [:rebar3], [], "hexpm"},
   "unsafe": {:hex, :unsafe, "1.0.1", "a27e1874f72ee49312e0a9ec2e0b27924214a05e3ddac90e91727bc76f8613d8", [:mix], [], "hexpm"},

test/mobilizon_web/controllers/feed_controller_test.exs

@@ -1,37 +1,64 @@
 defmodule MobilizonWeb.FeedControllerTest do
   use MobilizonWeb.ConnCase
   import Mobilizon.Factory
+  alias MobilizonWeb.Router.Helpers, as: Routes
+  alias MobilizonWeb.Endpoint
 
   describe "/@:preferred_username.atom" do
     test "it returns an RSS representation of the actor's public events", %{conn: conn} do
       actor = insert(:actor)
-      event1 = insert(:event, organizer_actor: actor)
+      tag1 = insert(:tag, title: "RSS", slug: "rss")
-      event2 = insert(:event, organizer_actor: actor)
+      tag2 = insert(:tag, title: "ATOM", slug: "atom")
+      event1 = insert(:event, organizer_actor: actor, tags: [tag1])
+      event2 = insert(:event, organizer_actor: actor, tags: [tag1, tag2])
 
       conn =
         conn
-        |> put_req_header("accept", "application/atom+xml")
+        |> get(
-        |> get("/@#{actor.preferred_username}.atom")
+          Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom")
+          |> URI.decode()
+        )
 
       assert response(conn, 200) =~ "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
       assert response_content_type(conn, :xml) =~ "charset=utf-8"
 
-      {:ok, feed, _} = FeederEx.parse(conn.resp_body)
+      {:ok, feed} = ElixirFeedParser.parse(conn.resp_body)
 
       assert feed.title == actor.preferred_username <> "'s public events feed"
 
-      Enum.each(feed.entries, fn entry ->
+      [entry1, entry2] = entries = feed.entries
+
+      Enum.each(entries, fn entry ->
         assert entry.title in [event1.title, event2.title]
       end)
+
+      assert entry1.categories == [tag2.slug, tag1.slug]
+      assert entry2.categories == [tag1.slug]
+    end
+
+    test "it returns an RSS representation of the actor's public events with the proper accept header",
+         %{conn: conn} do
+      actor = insert(:actor)
+
+      conn =
+        conn
+        |> put_req_header("accept", "application/atom+xml")
+        |> get(
+          Routes.feed_url(Endpoint, :actor, actor.preferred_username, "atom")
+          |> URI.decode()
+        )
+
+      assert response(conn, 200) =~ "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"
+      assert response_content_type(conn, :xml) =~ "charset=utf-8"
     end
 
     test "it doesn't return anything for an not existing actor", %{conn: conn} do
       conn =
         conn
         |> put_req_header("accept", "application/atom+xml")
-        |> get("/@notexistent.atom")
+        |> get("/@notexistent/feed/atom")
 
-      assert response(conn, 404) == "Not found"
+      assert response(conn, 404)
     end
   end
 end