Actor suspension refactoring

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

lib/federation/activity_pub/activity.ex

@@ -4,7 +4,7 @@ defmodule Mobilizon.Federation.ActivityPub.Activity do
   """
 
   @type t :: %__MODULE__{
-          data: String.t(),
+          data: map(),
           local: boolean,
           actor: Actor.t(),
           recipients: [String.t()]

lib/federation/activity_pub/activity_pub.ex

@@ -143,11 +143,11 @@ defmodule Mobilizon.Federation.ActivityPub do
         {:ok, _activity, entity} ->
           {:ok, entity}
 
-        {:error, "Gone"} ->
+        {:error, :http_gone} ->
-          {:error, "Gone", entity}
+          {:error, :http_gone, entity}
 
-        {:error, "Not found"} ->
+        {:error, :http_not_found} ->
-          {:error, "Not found", entity}
+          {:error, :http_not_found, entity}
 
         {:error, "Object origin check failed"} ->
           {:error, "Object origin check failed"}

lib/federation/activity_pub/actor.ex

@@ -14,65 +14,62 @@ defmodule Mobilizon.Federation.ActivityPub.Actor do
   @doc """
   Getting an actor from url, eventually creating it if we don't have it locally or if it needs an update
   """
-  @spec get_or_fetch_actor_by_url(String.t(), boolean) :: {:ok, Actor.t()} | {:error, String.t()}
+  @spec get_or_fetch_actor_by_url(url :: String.t(), preload :: boolean()) ::
+          {:ok, Actor.t()}
+          | {:error, make_actor_errors}
+          | {:error, :no_internal_relay_actor}
+          | {:error, :url_nil}
   def get_or_fetch_actor_by_url(url, preload \\ false)
 
-  def get_or_fetch_actor_by_url(nil, _preload), do: {:error, "Can't fetch a nil url"}
+  def get_or_fetch_actor_by_url(nil, _preload), do: {:error, :url_nil}
 
   def get_or_fetch_actor_by_url("https://www.w3.org/ns/activitystreams#Public", _preload) do
-    with %Actor{url: url} <- Relay.get_actor() do
+    case Relay.get_actor() do
-      get_or_fetch_actor_by_url(url)
+      %Actor{url: url} ->
+        get_or_fetch_actor_by_url(url)
+
+      {:error, %Ecto.Changeset{}} ->
+        {:error, :no_internal_relay_actor}
     end
   end
 
-  @spec get_or_fetch_actor_by_url(String.t(), boolean()) :: {:ok, Actor.t()} | {:error, any()}
   def get_or_fetch_actor_by_url(url, preload) do
-    with {:ok, %Actor{} = cached_actor} <- Actors.get_actor_by_url(url, preload),
+    case Actors.get_actor_by_url(url, preload) do
-         false <- Actors.needs_update?(cached_actor) do
+      {:ok, %Actor{} = cached_actor} ->
-      {:ok, cached_actor}
+        unless Actors.needs_update?(cached_actor) do
-    else
+          {:ok, cached_actor}
-      _ ->
+        else
-        # For tests, see https://github.com/jjh42/mock#not-supported---mocking-internal-function-calls and Mobilizon.Federation.ActivityPubTest
+          __MODULE__.make_actor_from_url(url, preload)
-        case __MODULE__.make_actor_from_url(url, preload) do
-          {:ok, %Actor{} = actor} ->
-            {:ok, actor}
-
-          {:error, err} ->
-            Logger.debug("Could not fetch by AP id")
-            Logger.debug(inspect(err))
-            {:error, "Could not fetch by AP id"}
         end
+
+      {:error, :actor_not_found} ->
+        # For tests, see https://github.com/jjh42/mock#not-supported---mocking-internal-function-calls and Mobilizon.Federation.ActivityPubTest
+        __MODULE__.make_actor_from_url(url, preload)
     end
   end
 
+  @type make_actor_errors :: Fetcher.fetch_actor_errors() | :actor_is_local
+
   @doc """
   Create an actor locally by its URL (AP ID)
   """
-  @spec make_actor_from_url(String.t(), boolean()) ::
+  @spec make_actor_from_url(url :: String.t(), preload :: boolean()) ::
-          {:ok, %Actor{}} | {:error, :actor_deleted} | {:error, :http_error} | {:error, any()}
+          {:ok, Actor.t()} | {:error, make_actor_errors}
   def make_actor_from_url(url, preload \\ false) do
     if are_same_origin?(url, Endpoint.url()) do
-      {:error, "Can't make a local actor from URL"}
+      {:error, :actor_is_local}
     else
       case Fetcher.fetch_and_prepare_actor_from_url(url) do
-        # Just in case
+        {:ok, data} when is_map(data) ->
-        {:ok, {:error, _e}} ->
-          raise ArgumentError, message: "Failed to make actor from url #{url}"
-
-        {:ok, data} ->
           Actors.upsert_actor(data, preload)
 
         # Request returned 410
         {:error, :actor_deleted} ->
-          Logger.info("Actor was deleted")
+          Logger.info("Actor #{url} was deleted")
           {:error, :actor_deleted}
 
-        {:error, :http_error} ->
+        {:error, err} when err in [:http_error, :json_decode_error] ->
-          {:error, :http_error}
+          {:error, err}
-
-        {:error, e} ->
-          Logger.warn("Failed to make actor from url #{url}")
-          {:error, e}
       end
     end
   end
@@ -80,8 +77,8 @@ defmodule Mobilizon.Federation.ActivityPub.Actor do
   @doc """
   Find an actor in our local database or call WebFinger to find what's its AP ID is and then fetch it
   """
-  @spec find_or_make_actor_from_nickname(String.t(), atom() | nil) ::
+  @spec find_or_make_actor_from_nickname(nickname :: String.t(), type :: atom() | nil) ::
-          {:ok, Actor.t()} | {:error, any()}
+          {:ok, Actor.t()} | {:error, make_actor_errors | WebFinger.finger_errors()}
   def find_or_make_actor_from_nickname(nickname, type \\ nil) do
     case Actors.get_actor_by_name_with_preload(nickname, type) do
       %Actor{url: actor_url} = actor ->
@@ -96,20 +93,22 @@ defmodule Mobilizon.Federation.ActivityPub.Actor do
     end
   end
 
-  @spec find_or_make_group_from_nickname(String.t()) :: tuple()
+  @spec find_or_make_group_from_nickname(nick :: String.t()) ::
+          {:error, make_actor_errors | WebFinger.finger_errors()}
   def find_or_make_group_from_nickname(nick), do: find_or_make_actor_from_nickname(nick, :Group)
 
   @doc """
   Create an actor inside our database from username, using WebFinger to find out its AP ID and then fetch it
   """
-  @spec make_actor_from_nickname(String.t()) :: {:ok, %Actor{}} | {:error, any()}
+  @spec make_actor_from_nickname(nickname :: String.t(), preload :: boolean) ::
+          {:ok, Actor.t()} | {:error, make_actor_errors | WebFinger.finger_errors()}
   def make_actor_from_nickname(nickname, preload \\ false) do
     case WebFinger.finger(nickname) do
       {:ok, url} when is_binary(url) ->
         make_actor_from_url(url, preload)
 
-      {:error, _e} ->
+      {:error, e} ->
-        {:error, "No ActivityPub URL found in WebFinger"}
+        {:error, e}
     end
   end
 end

lib/federation/activity_pub/fetcher.ex

@@ -15,39 +15,48 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
   import Mobilizon.Federation.ActivityPub.Utils,
     only: [maybe_date_fetch: 2, sign_fetch: 4, origin_check?: 2]
 
-  @spec fetch(String.t(), Keyword.t()) :: {:ok, map()}
+  import Mobilizon.Service.Guards, only: [is_valid_string: 1]
+
+  @spec fetch(String.t(), Keyword.t()) ::
+          {:ok, map()}
+          | {:ok, Tesla.Env.t()}
+          | {:error, String.t()}
+          | {:error, any()}
+          | {:error, :invalid_url}
   def fetch(url, options \\ []) do
     on_behalf_of = Keyword.get(options, :on_behalf_of, Relay.get_actor())
+    date = Signature.generate_date_header()
 
-    with false <- address_invalid(url),
+    headers =
-         date <- Signature.generate_date_header(),
+      [{:Accept, "application/activity+json"}]
-         headers <-
+      |> maybe_date_fetch(date)
-           [{:Accept, "application/activity+json"}]
+      |> sign_fetch(on_behalf_of, url, date)
-           |> maybe_date_fetch(date)
+
-           |> sign_fetch(on_behalf_of, url, date),
+    client = ActivityPubClient.client(headers: headers)
-         client <-
+
-           ActivityPubClient.client(headers: headers),
+    if address_valid?(url) do
-         {:ok, %Tesla.Env{body: data, status: code}} when code in 200..299 <-
+      case ActivityPubClient.get(client, url) do
-           ActivityPubClient.get(client, url) do
+        {:ok, %Tesla.Env{body: data, status: code}} when code in 200..299 ->
-      {:ok, data}
+          {:ok, data}
+
+        {:ok, %Tesla.Env{status: 410}} ->
+          Logger.debug("Resource at #{url} is 410 Gone")
+          {:error, :http_gone}
+
+        {:ok, %Tesla.Env{status: 404}} ->
+          Logger.debug("Resource at #{url} is 404 Gone")
+          {:error, :http_not_found}
+
+        {:ok, %Tesla.Env{} = res} ->
+          {:error, res}
+      end
     else
-      {:ok, %Tesla.Env{status: 410}} ->
+      {:error, :invalid_url}
-        Logger.debug("Resource at #{url} is 410 Gone")
-        {:error, "Gone"}
-
-      {:ok, %Tesla.Env{status: 404}} ->
-        Logger.debug("Resource at #{url} is 404 Gone")
-        {:error, "Not found"}
-
-      {:ok, %Tesla.Env{} = res} ->
-        {:error, res}
-
-      {:error, err} ->
-        {:error, err}
     end
   end
 
-  @spec fetch_and_create(String.t(), Keyword.t()) :: {:ok, map(), struct()}
+  @spec fetch_and_create(String.t(), Keyword.t()) ::
+          {:ok, map(), struct()} | {:error, :invalid_url} | {:error, String.t()} | {:error, any}
   def fetch_and_create(url, options \\ []) do
     with {:ok, data} when is_map(data) <- fetch(url, options),
          {:origin_check, true} <- {:origin_check, origin_check?(url, data)},
@@ -69,12 +78,16 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
       {:ok, data} when is_binary(data) ->
         {:error, "Failed to parse content as JSON"}
 
+      {:error, :invalid_url} ->
+        {:error, :invalid_url}
+
       {:error, err} ->
         {:error, err}
     end
   end
 
-  @spec fetch_and_update(String.t(), Keyword.t()) :: {:ok, map(), struct()}
+  @spec fetch_and_update(String.t(), Keyword.t()) ::
+          {:ok, map(), struct()} | {:error, String.t()} | :error | {:error, any}
   def fetch_and_update(url, options \\ []) do
     with {:ok, data} when is_map(data) <- fetch(url, options),
          {:origin_check, true} <- {:origin_check, origin_check(url, data)},
@@ -96,44 +109,46 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
     end
   end
 
+  @type fetch_actor_errors :: :json_decode_error | :actor_deleted | :http_error
+
   @doc """
   Fetching a remote actor's information through its AP ID
   """
-  @spec fetch_and_prepare_actor_from_url(String.t()) :: {:ok, map()} | {:error, atom()} | any()
+  @spec fetch_and_prepare_actor_from_url(String.t()) ::
+          {:ok, map()} | {:error, fetch_actor_errors}
   def fetch_and_prepare_actor_from_url(url) do
     Logger.debug("Fetching and preparing actor from url")
     Logger.debug(inspect(url))
 
-    res =
+    case Tesla.get(url,
-      with {:ok, %{status: 200, body: body}} <-
+           headers: [{"Accept", "application/activity+json"}],
-             Tesla.get(url,
+           follow_redirect: true
-               headers: [{"Accept", "application/activity+json"}],
+         ) do
-               follow_redirect: true
+      {:ok, %{status: 200, body: body}} ->
-             ),
+        Logger.debug("response okay, now decoding json")
-           :ok <- Logger.debug("response okay, now decoding json"),
-           {:ok, data} <- Jason.decode(body) do
-        Logger.debug("Got activity+json response at actor's endpoint, now converting data")
-        {:ok, ActorConverter.as_to_model_data(data)}
-      else
-        # Actor is gone, probably deleted
-        {:ok, %{status: 410}} ->
-          Logger.info("Response HTTP 410")
-          {:error, :actor_deleted}
 
-        {:ok, %Tesla.Env{}} ->
+        case Jason.decode(body) do
-          Logger.info("Non 200 HTTP Code")
+          {:ok, data} when is_map(data) ->
-          {:error, :http_error}
+            Logger.debug("Got activity+json response at actor's endpoint, now converting data")
+            {:ok, ActorConverter.as_to_model_data(data)}
 
-        {:error, e} ->
+          {:error, %Jason.DecodeError{} = e} ->
-          Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
+            Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
-          {:error, e}
+            {:error, :json_decode_error}
+        end
 
-        e ->
+      {:ok, %{status: 410}} ->
-          Logger.warn("Could not decode actor at fetch #{url}, #{inspect(e)}")
+        Logger.info("Response HTTP 410")
-          {:error, e}
+        {:error, :actor_deleted}
-      end
 
-    res
+      {:ok, %Tesla.Env{}} ->
+        Logger.info("Non 200 HTTP Code")
+        {:error, :http_error}
+
+      {:error, error} ->
+        Logger.warn("Could not fetch actor at fetch #{url}, #{inspect(error)}")
+        {:error, :http_error}
+    end
   end
 
   @spec origin_check(String.t(), map()) :: boolean()
@@ -147,11 +162,14 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
     end
   end
 
-  @spec address_invalid(String.t()) :: false | {:error, :invalid_url}
+  @spec address_valid?(String.t()) :: boolean
-  defp address_invalid(address) do
+  defp address_valid?(address) do
-    with %URI{host: host, scheme: scheme} <- URI.parse(address),
+    case URI.parse(address) do
-         true <- is_nil(host) or is_nil(scheme) do
+      %URI{host: host, scheme: scheme} ->
-      {:error, :invalid_url}
+        is_valid_string(host) and is_valid_string(scheme)
+
+      _ ->
+        false
     end
   end
 end

lib/federation/activity_pub/permission.ex

@@ -15,6 +15,15 @@ defmodule Mobilizon.Federation.ActivityPub.Permission do
 
   @type object :: %{id: String.t(), url: String.t()}
 
+  @type permissions_member_role :: nil | :member | :moderator | :administrator
+
+  @type t :: %__MODULE__{
+          access: permissions_member_role,
+          create: permissions_member_role,
+          update: permissions_member_role,
+          delete: permissions_member_role
+        }
+
   @doc """
   Check that actor can access the object
   """

lib/federation/activity_pub/refresher.ex

@@ -8,13 +8,12 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
   alias Mobilizon.Federation.ActivityPub
   alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
   alias Mobilizon.Federation.ActivityPub.{Fetcher, Relay, Transmogrifier, Utils}
-  alias Mobilizon.Service.ErrorReporting.Sentry
   require Logger
 
   @doc """
   Refresh a remote profile
   """
-  @spec refresh_profile(Actor.t()) :: {:ok, Actor.t()}
+  @spec refresh_profile(Actor.t()) :: {:ok, Actor.t()} | {:error, fetch_actor_errors()} | {:error}
   def refresh_profile(%Actor{domain: nil}), do: {:error, "Can only refresh remote actors"}
 
   def refresh_profile(%Actor{type: :Group, url: url, id: group_id} = group) do
@@ -33,74 +32,84 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
   end
 
   def refresh_profile(%Actor{type: type, url: url}) when type in [:Person, :Application] do
-    with {:ok, %Actor{outbox_url: outbox_url} = actor} <-
+    case ActivityPubActor.make_actor_from_url(url) do
-           ActivityPubActor.make_actor_from_url(url),
+      {:ok, %Actor{outbox_url: outbox_url} = actor} ->
-         :ok <- fetch_collection(outbox_url, Relay.get_actor()) do
+        case fetch_collection(outbox_url, Relay.get_actor()) do
-      {:ok, actor}
+          :ok -> {:ok, actor}
+          {:error, error} -> {:error, error}
+        end
+
+      {:error, error} ->
+        {:error, error}
     end
   end
 
-  @spec fetch_group(String.t(), Actor.t()) :: :ok
+  @type fetch_actor_errors :: ActivityPubActor.make_actor_errors() | fetch_collection_errors()
+
+  @spec fetch_group(String.t(), Actor.t()) :: :ok | {:error, fetch_actor_errors}
   def fetch_group(group_url, %Actor{} = on_behalf_of) do
-    with {:ok,
+    case ActivityPubActor.make_actor_from_url(group_url) do
-          %Actor{
+      {:ok,
-            outbox_url: outbox_url,
+       %Actor{
-            resources_url: resources_url,
+         outbox_url: outbox_url,
-            members_url: members_url,
+         resources_url: resources_url,
-            posts_url: posts_url,
+         members_url: members_url,
-            todos_url: todos_url,
+         posts_url: posts_url,
-            discussions_url: discussions_url,
+         todos_url: todos_url,
-            events_url: events_url
+         discussions_url: discussions_url,
-          }} <-
+         events_url: events_url
-           ActivityPubActor.make_actor_from_url(group_url),
+       }} ->
-         :ok <- fetch_collection(outbox_url, on_behalf_of),
+        Logger.debug("Fetched group OK, now doing collections")
-         :ok <- fetch_collection(members_url, on_behalf_of),
-         :ok <- fetch_collection(resources_url, on_behalf_of),
-         :ok <- fetch_collection(posts_url, on_behalf_of),
-         :ok <- fetch_collection(todos_url, on_behalf_of),
-         :ok <- fetch_collection(discussions_url, on_behalf_of),
-         :ok <- fetch_collection(events_url, on_behalf_of) do
-      :ok
-    else
-      {:error, :actor_deleted} ->
-        {:error, :actor_deleted}
 
-      {:error, :http_error} ->
+        with :ok <- fetch_collection(outbox_url, on_behalf_of),
-        {:error, :http_error}
+             :ok <- fetch_collection(members_url, on_behalf_of),
+             :ok <- fetch_collection(resources_url, on_behalf_of),
+             :ok <- fetch_collection(posts_url, on_behalf_of),
+             :ok <- fetch_collection(todos_url, on_behalf_of),
+             :ok <- fetch_collection(discussions_url, on_behalf_of),
+             :ok <- fetch_collection(events_url, on_behalf_of) do
+          :ok
+        else
+          {:error, err}
+          when err in [:error, :process_error, :fetch_error, :collection_url_nil] ->
+            Logger.debug("Error while fetching actor collection")
+            {:error, err}
+        end
 
-      {:error, err} ->
+      {:error, err}
-        Logger.error("Error while refreshing a group")
+      when err in [:actor_deleted, :http_error, :json_decode_error, :actor_is_local] ->
-
+        Logger.debug("Error while making actor")
-        Sentry.capture_message("Error while refreshing a group",
-          extra: %{group_url: group_url}
-        )
-
-        Logger.debug(inspect(err))
         {:error, err}
-
-      err ->
-        Logger.error("Error while refreshing a group")
-
-        Sentry.capture_message("Error while refreshing a group",
-          extra: %{group_url: group_url}
-        )
-
-        Logger.debug(inspect(err))
-        err
     end
   end
 
-  def fetch_collection(nil, _on_behalf_of), do: :error
+  @typep fetch_collection_errors :: :process_error | :fetch_error | :collection_url_nil
+
+  @spec fetch_collection(String.t() | nil, any) ::
+          :ok | {:error, fetch_collection_errors}
+  def fetch_collection(nil, _on_behalf_of), do: {:error, :collection_url_nil}
 
   def fetch_collection(collection_url, on_behalf_of) do
     Logger.debug("Fetching and preparing collection from url")
     Logger.debug(inspect(collection_url))
 
-    with {:ok, data} <- Fetcher.fetch(collection_url, on_behalf_of: on_behalf_of),
+    case Fetcher.fetch(collection_url, on_behalf_of: on_behalf_of) do
-         :ok <- Logger.debug("Fetch ok, passing to process_collection"),
+      {:ok, data} when is_map(data) ->
-         :ok <- process_collection(data, on_behalf_of) do
+        Logger.debug("Fetch ok, passing to process_collection")
-      Logger.debug("Finished processing a collection")
+
-      :ok
+        case process_collection(data, on_behalf_of) do
+          :ok ->
+            Logger.debug("Finished processing a collection")
+            :ok
+
+          :error ->
+            Logger.debug("Failed to process collection #{collection_url}")
+            {:error, :process_error}
+        end
+
+      {:error, _err} ->
+        Logger.debug("Failed to fetch collection #{collection_url}")
+        {:error, :fetch_error}
     end
   end
 
@@ -127,6 +136,7 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
     |> Enum.each(&refresh_profile/1)
   end
 
+  @spec process_collection(map(), any()) :: :ok | :error
   defp process_collection(%{"type" => type, "orderedItems" => items}, _on_behalf_of)
        when type in ["OrderedCollection", "OrderedCollectionPage"] do
     Logger.debug(
@@ -168,6 +178,8 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
   defp process_collection(_, _), do: :error
 
   # If we're handling an activity
+  @spec handling_element(map()) :: {:ok, any, struct} | :error
+  @spec handling_element(String.t()) :: {:ok, struct} | {:error, any()}
   defp handling_element(%{"type" => activity_type} = data)
        when activity_type in ["Create", "Update", "Delete"] do
     object = get_in(data, ["object"])

lib/federation/activity_pub/relay.ex

@@ -138,7 +138,8 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
 
   defp fetch_object(object) when is_binary(object), do: {object, object}
 
-  @spec fetch_actor(String.t()) :: {:ok, String.t()} | {:error, String.t()}
+  @spec fetch_actor(String.t()) ::
+          {:ok, String.t()} | {:error, WebFinger.finger_errors() | :bad_url}
   # Dirty hack
   defp fetch_actor("https://" <> address), do: fetch_actor(address)
   defp fetch_actor("http://" <> address), do: fetch_actor(address)
@@ -154,26 +155,15 @@ defmodule Mobilizon.Federation.ActivityPub.Relay do
         check_actor("relay@#{host}")
 
       true ->
-        {:error, "Bad URL"}
+        {:error, :bad_url}
     end
   end
 
-  @spec check_actor(String.t()) :: {:ok, String.t()} | {:error, String.t()}
+  @spec check_actor(String.t()) :: {:ok, String.t()} | {:error, WebFinger.finger_errors()}
   defp check_actor(username_and_domain) do
     case Actors.get_actor_by_name(username_and_domain) do
       %Actor{url: url} -> {:ok, url}
-      nil -> finger_actor(username_and_domain)
+      nil -> WebFinger.finger(username_and_domain)
-    end
-  end
-
-  @spec finger_actor(String.t()) :: {:ok, String.t()} | {:error, String.t()}
-  defp finger_actor(nickname) do
-    case WebFinger.finger(nickname) do
-      {:ok, url} when is_binary(url) ->
-        {:ok, url}
-
-      _e ->
-        {:error, "No ActivityPub URL found in WebFinger"}
     end
   end
 end

lib/federation/activity_pub/transmogrifier.ex

@@ -32,6 +32,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
   @doc """
   Handle incoming activities
   """
+  @spec handle_incoming(map()) :: :error | {:ok, any(), struct()}
   def handle_incoming(%{"id" => nil}), do: :error
   def handle_incoming(%{"id" => ""}), do: :error
 
@@ -1107,7 +1108,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
 
   defp is_group_object_gone(object_id) do
     case ActivityPub.fetch_object_from_url(object_id, force: true) do
-      {:error, error_message, object} when error_message in ["Gone", "Not found"] ->
+      {:error, error_message, object} when error_message in [:http_gone, :http_not_found] ->
         {:ok, object}
 
       # comments are just emptied

lib/federation/web_finger/web_finger.ex

@@ -19,6 +19,10 @@ defmodule Mobilizon.Federation.WebFinger do
   require Logger
   import SweetXml
 
+  @doc """
+  Returns the Web Host Metadata (for `/.well-known/host-meta`) representation for the instance, following RFC6414.
+  """
+  @spec host_meta :: String.t()
   def host_meta do
     base_url = Endpoint.url()
     %URI{host: host} = URI.parse(base_url)
@@ -47,6 +51,10 @@ defmodule Mobilizon.Federation.WebFinger do
     |> XmlBuilder.to_doc()
   end
 
+  @doc """
+  Returns the Webfinger representation for the instance, following RFC7033.
+  """
+  @spec webfinger(String.t(), String.t()) :: {:ok, map} | {:error, :actor_not_found}
   def webfinger(resource, "JSON") do
     host = Endpoint.host()
     regex = ~r/(acct:)?(?<name>\w+)@#{host}/
@@ -61,11 +69,14 @@ defmodule Mobilizon.Federation.WebFinger do
             {:ok, represent_actor(actor, "JSON")}
 
           _e ->
-            {:error, "Couldn't find actor"}
+            {:error, :actor_not_found}
         end
     end
   end
 
+  @doc """
+  Return an `Mobilizon.Actors.Actor` Webfinger representation (as JSON)
+  """
   @spec represent_actor(Actor.t()) :: map()
   @spec represent_actor(Actor.t(), String.t()) :: map()
   def represent_actor(%Actor{} = actor), do: represent_actor(actor, "JSON")
@@ -89,6 +100,7 @@ defmodule Mobilizon.Federation.WebFinger do
     }
   end
 
+  @spec maybe_add_avatar(list(map()), Actor.t()) :: list(map())
   defp maybe_add_avatar(data, %Actor{avatar: avatar}) when not is_nil(avatar) do
     data ++
       [
@@ -102,6 +114,7 @@ defmodule Mobilizon.Federation.WebFinger do
 
   defp maybe_add_avatar(data, _actor), do: data
 
+  @spec maybe_add_profile_page(list(map()), Actor.t()) :: list(map())
   defp maybe_add_profile_page(data, %Actor{type: :Group, url: url}) do
     data ++
       [
@@ -115,35 +128,69 @@ defmodule Mobilizon.Federation.WebFinger do
 
   defp maybe_add_profile_page(data, _actor), do: data
 
+  @type finger_errors ::
+          :host_not_found | :address_invalid | :http_error | :webfinger_information_not_json
+
   @doc """
   Finger an actor to retreive it's ActivityPub ID/URL
 
-  Fetches the Extensible Resource Descriptor endpoint `/.well-known/host-meta` to find the Webfinger endpoint (usually `/.well-known/webfinger?resource=`) with `find_webfinger_endpoint/1` and then performs a Webfinger query to get the ActivityPub ID associated to an actor.
+  Fetches the Extensible Resource Descriptor endpoint `/.well-known/host-meta` to find the Webfinger endpoint (usually `/.well-known/webfinger?resource=`) and then performs a Webfinger query to get the ActivityPub ID associated to an actor.
   """
-  @spec finger(String.t()) :: {:ok, String.t()} | {:error, atom()}
+  @spec finger(String.t()) ::
+          {:ok, String.t()}
+          | {:error, finger_errors}
   def finger(actor) do
     actor = String.trim_leading(actor, "@")
 
-    with address when is_binary(address) <- apply_webfinger_endpoint(actor),
+    case validate_endpoint(actor) do
-         false <- address_invalid(address),
+      {:ok, address} ->
-         {:ok, %{body: body, status: code}} when code in 200..299 <-
+        case fetch_webfinger_data(address) do
-           WebfingerClient.get(address),
+          {:ok, %{"url" => url}} ->
-         {:ok, %{"url" => url}} <- webfinger_from_json(body) do
+            {:ok, url}
-      {:ok, url}
+
-    else
+          {:error, err} ->
-      e ->
+            Logger.debug("Couldn't process webfinger data for #{actor}")
-        Logger.debug("Couldn't finger #{actor}")
+            err
-        Logger.debug(inspect(e))
+        end
-        {:error, e}
+
+      {:error, err} ->
+        Logger.debug("Couldn't find webfinger endpoint for #{actor}")
+        {:error, err}
     end
   end
 
-  @doc """
+  @spec fetch_webfinger_data(String.t()) ::
-  Fetches the Extensible Resource Descriptor endpoint `/.well-known/host-meta` to find the Webfinger endpoint (usually `/.well-known/webfinger?resource=`)
+          {:ok, map()} | {:error, :webfinger_information_not_json | :http_error}
-  """
+  defp fetch_webfinger_data(address) do
+    case WebfingerClient.get(address) do
+      {:ok, %{body: body, status: code}} when code in 200..299 ->
+        webfinger_from_json(body)
+
+      _ ->
+        {:error, :http_error}
+    end
+  end
+
+  @spec validate_endpoint(String.t()) ::
+          {:ok, String.t()} | {:error, :address_invalid | :host_not_found}
+  defp validate_endpoint(actor) do
+    case apply_webfinger_endpoint(actor) do
+      address when is_binary(address) ->
+        if address_invalid(address) do
+          {:error, :address_invalid}
+        else
+          {:ok, address}
+        end
+
+      _ ->
+        {:error, :host_not_found}
+    end
+  end
+
+  # Fetches the Extensible Resource Descriptor endpoint `/.well-known/host-meta` to find the Webfinger endpoint (usually `/.well-known/webfinger?resource=`)
   @spec find_webfinger_endpoint(String.t()) ::
           {:ok, String.t()} | {:error, :link_not_found} | {:error, any()}
-  def find_webfinger_endpoint(domain) when is_binary(domain) do
+  defp find_webfinger_endpoint(domain) when is_binary(domain) do
     with {:ok, %{body: body}} <- fetch_document("http://#{domain}/.well-known/host-meta"),
          link_template when is_binary(link_template) <- find_link_from_template(body) do
       {:ok, link_template}

lib/mobilizon/actors/actor.ex

@@ -324,7 +324,7 @@ defmodule Mobilizon.Actors.Actor do
   Changeset for group creation
   """
   @spec group_creation_changeset(t, map) :: Ecto.Changeset.t()
-  def group_creation_changeset(%__MODULE__{} = actor, params) do
+  def group_creation_changeset(actor, params) do
     actor
     |> cast(params, @group_creation_attrs)
     |> build_urls(:Group)

lib/mobilizon/actors/actors.ex

@@ -12,16 +12,12 @@ defmodule Mobilizon.Actors do
 
   alias Mobilizon.Actors.{Actor, Bot, Follower, Member}
   alias Mobilizon.Addresses.Address
-  alias Mobilizon.{Crypto, Events}
+  alias Mobilizon.Crypto
   alias Mobilizon.Events.FeedToken
-  alias Mobilizon.Federation.ActivityPub
-  alias Mobilizon.Medias.File
-  alias Mobilizon.Service.ErrorReporting.Sentry
   alias Mobilizon.Service.Workers
   alias Mobilizon.Storage.{Page, Repo}
   alias Mobilizon.Users
   alias Mobilizon.Users.User
-  alias Mobilizon.Web.Email.Group
   alias Mobilizon.Web.Upload
 
   require Logger
@@ -61,7 +57,6 @@ defmodule Mobilizon.Actors do
   @administrator_roles [:creator, :administrator]
   @moderator_roles [:moderator] ++ @administrator_roles
   @member_roles [:member] ++ @moderator_roles
-  @actor_preloads [:user, :organized_events, :comments]
 
   @doc """
   Gets a single actor.
@@ -151,7 +146,7 @@ defmodule Mobilizon.Actors do
   @doc """
   Gets an actor by name.
   """
-  @spec get_actor_by_name(String.t(), atom | nil) :: Actor.t() | nil
+  @spec get_actor_by_name(String.t(), ActorType.t() | nil) :: Actor.t() | nil
   def get_actor_by_name(name, type \\ nil) do
     query = from(a in Actor)
 
@@ -311,98 +306,6 @@ defmodule Mobilizon.Actors do
     })
   end
 
-  @doc """
-  Deletes an actor.
-  """
-  @spec perform(atom(), Actor.t()) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
-  def perform(:delete_actor, %Actor{type: type} = actor, options \\ @delete_actor_default_options) do
-    Logger.info("Going to delete actor #{actor.url}")
-    actor = Repo.preload(actor, @actor_preloads)
-
-    delete_actor_options = Keyword.merge(@delete_actor_default_options, options)
-    Logger.debug(inspect(delete_actor_options))
-
-    if type == :Group do
-      delete_eventual_local_members(actor, delete_actor_options)
-    end
-
-    multi =
-      Multi.new()
-      |> Multi.run(:delete_organized_events, fn _, _ -> delete_actor_organized_events(actor) end)
-      |> Multi.run(:empty_comments, fn _, _ -> delete_actor_empty_comments(actor) end)
-      |> Multi.run(:remove_banner, fn _, _ -> remove_banner(actor) end)
-      |> Multi.run(:remove_avatar, fn _, _ -> remove_avatar(actor) end)
-
-    multi =
-      case type do
-        :Group ->
-          multi
-          |> Multi.run(:delete_remote_members, fn _, _ ->
-            delete_group_elements(actor, :remote_members)
-          end)
-          |> Multi.run(:delete_group_organized_events, fn _, _ ->
-            delete_group_elements(actor, :events)
-          end)
-          |> Multi.run(:delete_group_posts, fn _, _ ->
-            delete_group_elements(actor, :posts)
-          end)
-          |> Multi.run(:delete_group_resources, fn _, _ ->
-            delete_group_elements(actor, :resources)
-          end)
-          |> Multi.run(:delete_group_todo_lists, fn _, _ ->
-            delete_group_elements(actor, :todo_lists)
-          end)
-          |> Multi.run(:delete_group_discussions, fn _, _ ->
-            delete_group_elements(actor, :discussions)
-          end)
-
-        :Person ->
-          # When deleting a profile, reset default_actor_id
-          Multi.run(multi, :reset_default_actor_id, fn _, _ ->
-            reset_default_actor_id(actor)
-          end)
-
-        _ ->
-          multi
-      end
-
-    multi =
-      if Keyword.get(delete_actor_options, :reserve_username, true) do
-        Multi.update(multi, :actor, Actor.delete_changeset(actor))
-      else
-        Multi.delete(multi, :actor, actor)
-      end
-
-    Logger.debug("Going to run the transaction")
-
-    case Repo.transaction(multi) do
-      {:ok, %{actor: %Actor{} = actor}} ->
-        {:ok, true} = Cachex.del(:activity_pub, "actor_#{actor.preferred_username}")
-        Logger.info("Deleted actor #{actor.url}")
-        {:ok, actor}
-
-      {:error, remove, error, _} when remove in [:remove_banner, :remove_avatar] ->
-        Logger.error("Error while deleting actor's banner or avatar")
-
-        Sentry.capture_message("Error while deleting actor's banner or avatar",
-          extra: %{err: error}
-        )
-
-        Logger.debug(inspect(error, pretty: true))
-        {:error, error}
-
-      err ->
-        Logger.error("Unknown error while deleting actor")
-
-        Sentry.capture_message("Error while deleting actor's banner or avatar",
-          extra: %{err: err}
-        )
-
-        Logger.debug(inspect(err, pretty: true))
-        {:error, err}
-    end
-  end
-
   @spec actor_key_rotation(Actor.t()) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
   def actor_key_rotation(%Actor{} = actor) do
     actor
@@ -536,8 +439,9 @@ defmodule Mobilizon.Actors do
         limit \\ nil
       ) do
     anonymous_actor_id = Mobilizon.Config.anonymous_actor_id()
+    query = from(a in Actor)
 
-    Actor
+    query
     |> actor_by_username_or_name_query(term)
     |> actors_for_location(Keyword.get(options, :location), Keyword.get(options, :radius))
     |> filter_by_types(Keyword.get(options, :actor_type, :Group))
@@ -610,21 +514,25 @@ defmodule Mobilizon.Actors do
   """
   @spec create_group(map) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
   def create_group(attrs \\ %{}) do
-    local = Map.get(attrs, :local, true)
+    if Map.get(attrs, :local, true) do
+      multi =
+        Multi.new()
+        |> Multi.insert(:insert_group, Actor.group_creation_changeset(%Actor{}, attrs))
+        |> Multi.insert(:add_admin_member, fn %{insert_group: group} ->
+          Member.changeset(%Member{}, %{
+            parent_id: group.id,
+            actor_id: attrs.creator_actor_id,
+            role: :administrator
+          })
+        end)
+        |> Repo.transaction()
 
-    if local do
+      case multi do
-      with {:ok, %{insert_group: %Actor{} = group, add_admin_member: %Member{} = _admin_member}} <-
+        {:ok, %{insert_group: %Actor{} = group, add_admin_member: %Member{} = _admin_member}} ->
-             Multi.new()
+          {:ok, group}
-             |> Multi.insert(:insert_group, Actor.group_creation_changeset(%Actor{}, attrs))
+
-             |> Multi.insert(:add_admin_member, fn %{insert_group: group} ->
+        {:error, %Ecto.Changeset{} = err} ->
-               Member.changeset(%Member{}, %{
+          {:error, err}
-                 parent_id: group.id,
-                 actor_id: attrs.creator_actor_id,
-                 role: :administrator
-               })
-             end)
-             |> Repo.transaction() do
-        {:ok, group}
       end
     else
       %Actor{}
@@ -818,18 +726,21 @@ defmodule Mobilizon.Actors do
   """
   @spec create_member(map) :: {:ok, Member.t()} | {:error, Ecto.Changeset.t()}
   def create_member(attrs \\ %{}) do
-    with {:ok, %Member{} = member} <-
+    case %Member{}
-           %Member{}
+         |> Member.changeset(attrs)
-           |> Member.changeset(attrs)
+         |> Repo.insert(
-           |> Repo.insert(
+           on_conflict: {:replace_all_except, [:id, :url, :actor_id, :parent_id]},
-             on_conflict: {:replace_all_except, [:id, :url, :actor_id, :parent_id]},
+           conflict_target: [:actor_id, :parent_id],
-             conflict_target: [:actor_id, :parent_id],
+           # See https://hexdocs.pm/ecto/Ecto.Repo.html#c:insert/2-upserts,
-             # See https://hexdocs.pm/ecto/Ecto.Repo.html#c:insert/2-upserts,
+           # when doing an upsert with on_conflict, PG doesn't return whether it's an insert or upsert
-             # when doing an upsert with on_conflict, PG doesn't return whether it's an insert or upsert
+           # so we need to refresh the fields
-             # so we need to refresh the fields
+           returning: true
-             returning: true
+         ) do
-           ) do
+      {:ok, %Member{} = member} ->
-      {:ok, Repo.preload(member, [:actor, :parent, :invited_by])}
+        {:ok, Repo.preload(member, [:actor, :parent, :invited_by])}
+
+      {:error, %Ecto.Changeset{} = err} ->
+        {:error, err}
     end
   end
 
@@ -881,6 +792,13 @@ defmodule Mobilizon.Actors do
     |> Page.build_page(page, limit)
   end
 
+  @spec list_all_local_members_for_group(Actor.t()) :: Member.t()
+  def list_all_local_members_for_group(%Actor{id: group_id, type: :Group} = _group) do
+    group_id
+    |> group_internal_member_query()
+    |> Repo.all()
+  end
+
   @spec list_local_members_for_group(Actor.t(), integer | nil, integer | nil) :: Page.t()
   def list_local_members_for_group(
         %Actor{id: group_id, type: :Group} = _group,
@@ -995,7 +913,7 @@ defmodule Mobilizon.Actors do
   @doc """
   Creates a bot.
   """
-  @spec create_bot(map) :: {:ok, Bot.t()} | {:error, Ecto.Changeset.t()}
+  @spec create_bot(attrs :: map) :: {:ok, Bot.t()} | {:error, Ecto.Changeset.t()}
   def create_bot(attrs \\ %{}) do
     %Bot{}
     |> Bot.changeset(attrs)
@@ -1005,7 +923,8 @@ defmodule Mobilizon.Actors do
   @doc """
   Registers a new bot.
   """
-  @spec register_bot(map) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
+  @spec register_bot(%{name: String.t(), summary: String.t()}) ::
+          {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
   def register_bot(%{name: name, summary: summary}) do
     attrs = %{
       preferred_username: name,
@@ -1020,7 +939,8 @@ defmodule Mobilizon.Actors do
     |> Repo.insert()
   end
 
-  @spec get_or_create_internal_actor(String.t()) :: {:ok, Actor.t()}
+  @spec get_or_create_internal_actor(String.t()) ::
+          {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
   def get_or_create_internal_actor(username) do
     case username |> Actor.build_url(:page) |> get_actor_by_url() do
       {:ok, %Actor{} = actor} ->
@@ -1105,13 +1025,16 @@ defmodule Mobilizon.Actors do
   @doc """
   Creates a follower.
   """
-  @spec create_follower(map) :: {:ok, Follower.t()} | {:error, Ecto.Changeset.t()}
+  @spec create_follower(attrs :: map) :: {:ok, Follower.t()} | {:error, Ecto.Changeset.t()}
   def create_follower(attrs \\ %{}) do
-    with {:ok, %Follower{} = follower} <-
+    case %Follower{}
-           %Follower{}
+         |> Follower.changeset(attrs)
-           |> Follower.changeset(attrs)
+         |> Repo.insert() do
-           |> Repo.insert() do
+      {:ok, %Follower{} = follower} ->
-      {:ok, Repo.preload(follower, [:actor, :target_actor])}
+        {:ok, Repo.preload(follower, [:actor, :target_actor])}
+
+      {:error, %Ecto.Changeset{} = err} ->
+        {:error, err}
     end
   end
 
@@ -1345,7 +1268,8 @@ defmodule Mobilizon.Actors do
     end
   end
 
-  @spec schedule_key_rotation(Actor.t(), integer()) :: nil
+  # TODO: Move me otherwhere
+  @spec schedule_key_rotation(Actor.t(), integer()) :: :ok
   def schedule_key_rotation(%Actor{id: actor_id} = actor, delay) do
     Cachex.put(:actor_key_rotation, actor_id, true)
 
@@ -1354,36 +1278,6 @@ defmodule Mobilizon.Actors do
     :ok
   end
 
-  @spec remove_banner(Actor.t()) :: {:ok, Actor.t()}
-  defp remove_banner(%Actor{banner: nil} = actor), do: {:ok, actor}
-
-  defp remove_banner(%Actor{banner: %File{url: url}} = actor) do
-    safe_remove_file(url, actor)
-    {:ok, actor}
-  end
-
-  @spec remove_avatar(Actor.t()) :: {:ok, Actor.t()}
-  defp remove_avatar(%Actor{avatar: nil} = actor), do: {:ok, actor}
-
-  defp remove_avatar(%Actor{avatar: %File{url: url}} = actor) do
-    safe_remove_file(url, actor)
-    {:ok, actor}
-  end
-
-  @spec safe_remove_file(String.t(), Actor.t()) :: {:ok, Actor.t()}
-  defp safe_remove_file(url, %Actor{} = actor) do
-    case Upload.remove(url) do
-      {:ok, _value} ->
-        {:ok, actor}
-
-      {:error, error} ->
-        Logger.error("Error while removing an upload file")
-        Logger.debug(inspect(error))
-
-        {:ok, actor}
-    end
-  end
-
   @spec delete_files_if_media_changed(Ecto.Changeset.t()) :: Ecto.Changeset.t()
   defp delete_files_if_media_changed(%Ecto.Changeset{changes: changes, data: data} = changeset) do
     Enum.each([:avatar, :banner], fn key ->
@@ -1748,9 +1642,9 @@ defmodule Mobilizon.Actors do
     from(a in query, where: a.visibility == ^:public)
   end
 
-  @spec filter_by_name(Ecto.Query.t(), [String.t()]) :: Ecto.Query.t()
+  @spec filter_by_name(query :: Ecto.Query.t(), [String.t()]) :: Ecto.Query.t()
   defp filter_by_name(query, [name]) do
-    from(a in query, where: a.preferred_username == ^name and is_nil(a.domain))
+    where(query, [a], a.preferred_username == ^name and is_nil(a.domain))
   end
 
   defp filter_by_name(query, [name, domain]) do
@@ -1761,6 +1655,7 @@ defmodule Mobilizon.Actors do
     end
   end
 
+  @spec filter_followed_by_approved_status(Ecto.Query.t(), boolean() | nil) :: Ecto.Query.t()
   defp filter_followed_by_approved_status(query, nil), do: query
 
   defp filter_followed_by_approved_status(query, approved) do
@@ -1770,141 +1665,4 @@ defmodule Mobilizon.Actors do
   @spec preload_followers(Actor.t(), boolean) :: Actor.t()
   defp preload_followers(actor, true), do: Repo.preload(actor, [:followers])
   defp preload_followers(actor, false), do: actor
-
-  defp delete_actor_organized_events(%Actor{organized_events: organized_events} = actor) do
-    res =
-      Enum.map(organized_events, fn event ->
-        event =
-          Repo.preload(event, [
-            :organizer_actor,
-            :participants,
-            :picture,
-            :mentions,
-            :comments,
-            :attributed_to,
-            :tags,
-            :physical_address,
-            :contacts,
-            :media
-          ])
-
-        ActivityPub.delete(event, actor, false)
-      end)
-
-    if Enum.all?(res, fn {status, _, _} -> status == :ok end) do
-      {:ok, res}
-    else
-      {:error, res}
-    end
-  end
-
-  defp delete_actor_empty_comments(%Actor{comments: comments} = actor) do
-    res =
-      Enum.map(comments, fn comment ->
-        comment =
-          Repo.preload(comment, [
-            :actor,
-            :mentions,
-            :event,
-            :in_reply_to_comment,
-            :origin_comment,
-            :attributed_to,
-            :tags
-          ])
-
-        ActivityPub.delete(comment, actor, false)
-      end)
-
-    if Enum.all?(res, fn {status, _, _} -> status == :ok end) do
-      {:ok, res}
-    else
-      {:error, res}
-    end
-  end
-
-  defp delete_group_elements(%Actor{type: :Group} = actor, type) do
-    Logger.debug("delete_group_elements #{inspect(type)}")
-
-    method =
-      case type do
-        :remote_members -> &list_remote_members_for_group/3
-        :events -> &Events.list_simple_organized_events_for_group/3
-        :posts -> &Mobilizon.Posts.get_posts_for_group/3
-        :resources -> &Mobilizon.Resources.get_resources_for_group/3
-        :todo_lists -> &Mobilizon.Todos.get_todo_lists_for_group/3
-        :discussions -> &Mobilizon.Discussions.find_discussions_for_actor/3
-      end
-
-    res =
-      actor
-      |> accumulate_paginated_elements(method)
-      |> Enum.map(fn element -> ActivityPub.delete(element, actor, false) end)
-
-    if Enum.all?(res, fn {status, _, _} -> status == :ok end) do
-      Logger.debug("Return OK for all #{to_string(type)}")
-      {:ok, res}
-    else
-      Logger.debug("Something failed #{inspect(res)}")
-      {:error, res}
-    end
-  end
-
-  @spec reset_default_actor_id(Actor.t()) :: {:ok, User.t()} | {:error, :user_not_found}
-  defp reset_default_actor_id(%Actor{type: :Person, user: %User{id: user_id} = user, id: actor_id}) do
-    Logger.debug("reset_default_actor_id")
-
-    new_actor_id =
-      user
-      |> Users.get_actors_for_user()
-      |> Enum.map(& &1.id)
-      |> Enum.find(&(&1 !== actor_id))
-
-    {:ok, Users.update_user_default_actor(user_id, new_actor_id)}
-  rescue
-    _e in Ecto.NoResultsError ->
-      {:error, :user_not_found}
-  end
-
-  defp reset_default_actor_id(%Actor{type: :Person, user: nil}), do: {:ok, nil}
-
-  defp accumulate_paginated_elements(
-         %Actor{} = actor,
-         method,
-         elements \\ [],
-         page \\ 1,
-         limit \\ 10
-       ) do
-    Logger.debug("accumulate_paginated_elements")
-    %Page{total: total, elements: new_elements} = method.(actor, page, limit)
-    elements = elements ++ new_elements
-    count = length(elements)
-
-    if count < total do
-      accumulate_paginated_elements(actor, method, elements, page + 1, limit)
-    else
-      Logger.debug("Found #{count} group elements to delete")
-      elements
-    end
-  end
-
-  # This one is not in the Multi transaction because it sends activities
-  defp delete_eventual_local_members(%Actor{} = group, options) do
-    suspended? = Keyword.get(options, :suspension, false)
-
-    group
-    |> accumulate_paginated_elements(&list_local_members_for_group/3)
-    |> Enum.map(fn member ->
-      if suspended? do
-        Group.send_group_suspension_notification(member)
-      else
-        with author_id when not is_nil(author_id) <- Keyword.get(options, :author_id),
-             %Actor{} = author <- get_actor(author_id) do
-          Group.send_group_deletion_notification(member, author)
-        end
-      end
-
-      member
-    end)
-    |> Enum.map(fn member -> ActivityPub.delete(member, group, false) end)
-  end
 end

lib/mobilizon/actors/follower.ex

@@ -12,10 +12,13 @@ defmodule Mobilizon.Actors.Follower do
   alias Mobilizon.Web.Endpoint
 
   @type t :: %__MODULE__{
+          id: String.t(),
           approved: boolean,
           url: String.t(),
           target_actor: Actor.t(),
-          actor: Actor.t()
+          actor: Actor.t(),
+          inserted_at: DateTime.t(),
+          updated_at: DateTime.t()
         }
 
   @required_attrs [:url, :approved, :target_actor_id, :actor_id]
@@ -35,7 +38,7 @@ defmodule Mobilizon.Actors.Follower do
   end
 
   @doc false
-  @spec changeset(t, map) :: Ecto.Changeset.t()
+  @spec changeset(follower :: t, attrs :: map) :: Ecto.Changeset.t()
   def changeset(follower, attrs) do
     follower
     |> cast(attrs, @attrs)

lib/mobilizon/config.ex

@@ -5,6 +5,7 @@ defmodule Mobilizon.Config do
 
   alias Mobilizon.Actors
   alias Mobilizon.Service.GitStatus
+  require Logger
 
   @spec instance_config :: keyword
   def instance_config, do: Application.get_env(:mobilizon, :instance)
@@ -35,10 +36,10 @@ defmodule Mobilizon.Config do
         "instance_long_description"
       )
 
-  @spec instance_slogan :: String.t()
+  @spec instance_slogan :: String.t() | nil
   def instance_slogan, do: Mobilizon.Admin.get_admin_setting_value("instance", "instance_slogan")
 
-  @spec contact :: String.t()
+  @spec contact :: String.t() | nil
   def contact do
     Mobilizon.Admin.get_admin_setting_value("instance", "contact")
   end
@@ -53,7 +54,7 @@ defmodule Mobilizon.Config do
     Mobilizon.Admin.get_admin_setting_value("instance", "instance_terms_type", "DEFAULT")
   end
 
-  @spec instance_terms_url :: String.t()
+  @spec instance_terms_url :: String.t() | nil
   def instance_terms_url do
     Mobilizon.Admin.get_admin_setting_value("instance", "instance_terms_url")
   end
@@ -225,6 +226,7 @@ defmodule Mobilizon.Config do
   @spec ldap_enabled? :: boolean()
   def ldap_enabled?, do: get([:ldap, :enabled], false)
 
+  @spec instance_resource_providers :: list(%{type: atom, software: atom, endpoint: String.t()})
   def instance_resource_providers do
     types = get_in(Application.get_env(:mobilizon, Mobilizon.Service.ResourceProviders), [:types])
 
@@ -248,20 +250,25 @@ defmodule Mobilizon.Config do
     end
   end
 
+  @spec instance_group_feature_enabled? :: boolean
   def instance_group_feature_enabled?,
     do: :mobilizon |> Application.get_env(:groups) |> Keyword.get(:enabled)
 
+  @spec instance_event_creation_enabled? :: boolean
   def instance_event_creation_enabled?,
     do: :mobilizon |> Application.get_env(:events) |> Keyword.get(:creation)
 
+  @spec anonymous_actor_id :: binary | integer
   def anonymous_actor_id, do: get_cached_value(:anonymous_actor_id)
+  @spec relay_actor_id :: binary | integer
   def relay_actor_id, do: get_cached_value(:relay_actor_id)
+  @spec admin_settings :: map
   def admin_settings, do: get_cached_value(:admin_config)
 
-  @spec get(module | atom) :: any
+  @spec get(key :: module | atom) :: any
   def get(key), do: get(key, nil)
 
-  @spec get([module | atom]) :: any
+  @spec get(keys :: [module | atom], default :: any) :: any
   def get([key], default), do: get(key, default)
 
   def get([parent_key | keys], default) do
@@ -271,10 +278,10 @@ defmodule Mobilizon.Config do
     end
   end
 
-  @spec get(module | atom, any) :: any
+  @spec get(key :: module | atom, default :: any) :: any
   def get(key, default), do: Application.get_env(:mobilizon, key, default)
 
-  @spec get!(module | atom) :: any
+  @spec get!(key :: module | atom) :: any
   def get!(key) do
     value = get(key, nil)
 
@@ -285,6 +292,7 @@ defmodule Mobilizon.Config do
     end
   end
 
+  @spec put(keys :: [module | atom], value :: any) :: :ok
   def put([key], value), do: put(key, value)
 
   def put([parent_key | keys], value) do
@@ -295,6 +303,7 @@ defmodule Mobilizon.Config do
     Application.put_env(:mobilizon, parent_key, parent)
   end
 
+  @spec put(keys :: module | atom, value :: any) :: :ok
   def put(key, value) do
     Application.put_env(:mobilizon, key, value)
   end
@@ -302,11 +311,16 @@ defmodule Mobilizon.Config do
   @spec to_boolean(boolean | String.t()) :: boolean
   defp to_boolean(boolean), do: "true" == String.downcase("#{boolean}")
 
+  @spec get_cached_value(atom) :: String.t() | integer | map
   defp get_cached_value(key) do
     case Cachex.fetch(:config, key, fn key ->
            case create_cache(key) do
-             value when not is_nil(value) -> {:commit, value}
+             {:ok, value} when not is_nil(value) ->
-             err -> {:ignore, err}
+               {:commit, value}
+
+             {:error, err} ->
+               Logger.debug("Failed to cache config value, returned: #{inspect(err)}")
+               {:ignore, err}
            end
          end) do
       {status, value} when status in [:ok, :commit] -> value
@@ -314,23 +328,29 @@ defmodule Mobilizon.Config do
     end
   end
 
-  @spec create_cache(atom()) :: integer()
+  @spec create_cache(atom()) :: {:ok, integer() | map()} | {:error, Ecto.Changeset.t()}
   defp create_cache(:anonymous_actor_id) do
-    with {:ok, %{id: actor_id}} <- Actors.get_or_create_internal_actor("anonymous") do
+    case Actors.get_or_create_internal_actor("anonymous") do
-      actor_id
+      {:ok, %{id: actor_id}} ->
+        {:ok, actor_id}
+
+      {:error, err} ->
+        {:error, err}
     end
   end
 
-  @spec create_cache(atom()) :: integer()
   defp create_cache(:relay_actor_id) do
-    with {:ok, %{id: actor_id}} <- Actors.get_or_create_internal_actor("relay") do
+    case Actors.get_or_create_internal_actor("relay") do
-      actor_id
+      {:ok, %{id: actor_id}} ->
+        {:ok, actor_id}
+
+      {:error, err} ->
+        {:error, err}
     end
   end
 
-  @spec create_cache(atom()) :: map()
   defp create_cache(:admin_config) do
-    %{
+    data = %{
       instance_description: instance_description(),
       instance_long_description: instance_long_description(),
       instance_name: instance_name(),
@@ -346,12 +366,16 @@ defmodule Mobilizon.Config do
       instance_rules: instance_rules(),
       instance_languages: instance_languages()
     }
+
+    {:ok, data}
   end
 
+  @spec clear_config_cache :: {:ok | :error, integer}
   def clear_config_cache do
     Cachex.clear(:config)
   end
 
+  @spec generate_terms(String.t()) :: String.t()
   def generate_terms(locale) do
     Gettext.put_locale(locale)
 
@@ -366,6 +390,7 @@ defmodule Mobilizon.Config do
     )
   end
 
+  @spec generate_privacy(String.t()) :: String.t()
   def generate_privacy(locale) do
     Gettext.put_locale(locale)
 
@@ -376,6 +401,7 @@ defmodule Mobilizon.Config do
     )
   end
 
+  @spec instance_contact_html :: String.t()
   defp instance_contact_html do
     contact = contact()
 

lib/mobilizon/users/push_subscription.ex

@@ -6,6 +6,14 @@ defmodule Mobilizon.Users.PushSubscription do
   alias Mobilizon.Users.User
   import Ecto.Changeset
 
+  @type t :: %__MODULE__{
+          digest: String.t(),
+          user: User.t(),
+          endpoint: String.t(),
+          auth: String.t(),
+          p256dh: String.t()
+        }
+
   @primary_key {:id, :binary_id, autogenerate: true}
   schema "user_push_subscriptions" do
     field(:digest, :string)

lib/mobilizon/users/users.ex

@@ -42,7 +42,7 @@ defmodule Mobilizon.Users do
     end
   end
 
-  @spec create_external(String.t(), String.t(), Map.t()) ::
+  @spec create_external(String.t(), String.t(), map()) ::
           {:ok, User.t()} | {:error, Ecto.Changeset.t()}
   def create_external(email, provider, args \\ %{}) do
     with {:ok, %User{} = user} <-

lib/service/actor_suspension.ex

@@ -0,0 +1,256 @@
+defmodule Mobilizon.Service.ActorSuspension do
+  @moduledoc """
+  Handle actor suspensions
+  """
+
+  alias Ecto.Multi
+  alias Mobilizon.{Actors, Events, Users}
+  alias Mobilizon.Actors.{Actor, Member}
+  alias Mobilizon.Discussions.{Comment, Discussion}
+  alias Mobilizon.Events.{Event, Participant}
+  alias Mobilizon.Medias.File
+  alias Mobilizon.Posts.Post
+  alias Mobilizon.Resources.Resource
+  alias Mobilizon.Storage.Repo
+  alias Mobilizon.Users.User
+  alias Mobilizon.Web.Email.Actor, as: ActorEmail
+  alias Mobilizon.Web.Email.Group
+  alias Mobilizon.Web.Upload
+  require Logger
+  import Ecto.Query
+
+  @actor_preloads [:user, :organized_events, :comments]
+  @delete_actor_default_options [reserve_username: true, suspension: false]
+
+  @doc """
+  Deletes an actor.
+  """
+  @spec suspend_actor(Actor.t(), Keyword.t()) :: {:ok, Actor.t()} | {:error, Ecto.Changeset.t()}
+  def suspend_actor(%Actor{} = actor, options \\ @delete_actor_default_options) do
+    Logger.info("Going to delete actor #{actor.url}")
+    actor = Repo.preload(actor, @actor_preloads)
+
+    delete_actor_options = Keyword.merge(@delete_actor_default_options, options)
+    Logger.debug(inspect(delete_actor_options))
+
+    send_suspension_notification(actor)
+    notify_event_participants_from_suspension(actor)
+    delete_participations(actor)
+
+    multi =
+      Multi.new()
+      |> maybe_reset_actor_id(actor)
+      |> delete_actor_empty_comments(actor)
+      |> Multi.run(:remove_banner, fn _, _ -> remove_banner(actor) end)
+      |> Multi.run(:remove_avatar, fn _, _ -> remove_avatar(actor) end)
+
+    multi =
+      if Keyword.get(delete_actor_options, :reserve_username, true) do
+        multi
+        |> delete_actor_events(actor)
+        |> delete_posts(actor)
+        |> delete_ressources(actor)
+        |> delete_discussions(actor)
+        |> delete_members(actor)
+        |> Multi.update(:actor, Actor.delete_changeset(actor))
+      else
+        Multi.delete(multi, :actor, actor)
+      end
+
+    Logger.debug("Going to run the transaction")
+
+    case Repo.transaction(multi) do
+      {:ok, %{actor: %Actor{} = actor}} ->
+        {:ok, true} = Cachex.del(:activity_pub, "actor_#{actor.preferred_username}")
+        Logger.info("Deleted actor #{actor.url}")
+        {:ok, actor}
+
+      {:error, remove, error, _} when remove in [:remove_banner, :remove_avatar] ->
+        Logger.error("Error while deleting actor's banner or avatar")
+
+        Sentry.capture_message("Error while deleting actor's banner or avatar",
+          extra: %{err: error}
+        )
+
+        Logger.debug(inspect(error, pretty: true))
+        {:error, error}
+
+      err ->
+        Logger.error("Unknown error while deleting actor")
+
+        Sentry.capture_message("Error while deleting actor's banner or avatar",
+          extra: %{err: err}
+        )
+
+        Logger.debug(inspect(err, pretty: true))
+        {:error, err}
+    end
+  end
+
+  # When deleting a profile, reset default_actor_id
+  @spec maybe_reset_actor_id(Multi.t(), Actor.t()) :: Multi.t()
+  defp maybe_reset_actor_id(%Multi{} = multi, %Actor{type: :Person} = actor) do
+    Multi.run(multi, :reset_default_actor_id, fn _, _ ->
+      reset_default_actor_id(actor)
+    end)
+  end
+
+  defp maybe_reset_actor_id(%Multi{} = multi, %Actor{type: :Group} = _actor) do
+    multi
+  end
+
+  defp delete_actor_empty_comments(%Multi{} = multi, %Actor{id: actor_id}) do
+    now = DateTime.utc_now() |> DateTime.truncate(:second)
+
+    Multi.update_all(multi, :empty_comments, where(Comment, [c], c.actor_id == ^actor_id),
+      set: [
+        text: nil,
+        actor_id: nil,
+        deleted_at: now
+      ]
+    )
+  end
+
+  @spec notify_event_participants_from_suspension(Actor.t()) :: :ok
+  defp notify_event_participants_from_suspension(%Actor{id: actor_id} = actor) do
+    actor
+    |> get_actor_organizer_events_participations()
+    |> preload([:actor, :event])
+    |> Repo.all()
+    |> Enum.filter(fn %Participant{actor: %Actor{id: participant_actor_id}} ->
+      participant_actor_id != actor_id
+    end)
+    |> Enum.each(&ActorEmail.send_notification_event_participants_from_suspension(&1, actor))
+  end
+
+  @spec get_actor_organizer_events_participations(Actor.t()) :: Ecto.Query.t()
+  defp get_actor_organizer_events_participations(%Actor{type: :Person, id: actor_id}) do
+    do_get_actor_organizer_events_participations()
+    |> where([_p, e], e.organizer_actor_id == ^actor_id)
+  end
+
+  defp get_actor_organizer_events_participations(%Actor{type: :Group, id: actor_id}) do
+    do_get_actor_organizer_events_participations()
+    |> where([_p, e], e.attributed_to_id == ^actor_id)
+  end
+
+  @spec do_get_actor_organizer_events_participations :: Ecto.Query.t()
+  defp do_get_actor_organizer_events_participations do
+    Participant
+    |> join(:inner, [p], e in Event, on: p.event_id == e.id)
+    |> where([_p, e], e.begins_on > ^DateTime.utc_now())
+    |> where([p, _e], p.role in [:participant, :moderator, :administrator])
+  end
+
+  @spec delete_actor_events(Ecto.Multi.t(), Actor.t()) :: Ecto.Multi.t()
+  defp delete_actor_events(%Multi{} = multi, %Actor{type: :Person, id: actor_id}) do
+    Multi.delete_all(multi, :delete_events, where(Event, [e], e.organizer_actor_id == ^actor_id))
+  end
+
+  defp delete_actor_events(%Multi{} = multi, %Actor{type: :Group, id: actor_id}) do
+    Multi.delete_all(multi, :delete_events, where(Event, [e], e.attributed_to_id == ^actor_id))
+  end
+
+  defp delete_posts(%Multi{} = multi, %Actor{type: :Person, id: actor_id}) do
+    Multi.delete_all(multi, :delete_posts, where(Post, [e], e.author_id == ^actor_id))
+  end
+
+  defp delete_posts(%Multi{} = multi, %Actor{type: :Group, id: actor_id}) do
+    Multi.delete_all(multi, :delete_posts, where(Post, [e], e.attributed_to_id == ^actor_id))
+  end
+
+  defp delete_ressources(%Multi{} = multi, %Actor{type: :Person, id: actor_id}) do
+    Multi.delete_all(multi, :delete_resources, where(Resource, [e], e.creator_id == ^actor_id))
+  end
+
+  defp delete_ressources(%Multi{} = multi, %Actor{type: :Group, id: actor_id}) do
+    Multi.delete_all(multi, :delete_resources, where(Resource, [e], e.actor_id == ^actor_id))
+  end
+
+  # Keep discussions just in case, comments are already emptied
+  defp delete_discussions(%Multi{} = multi, %Actor{type: :Person}) do
+    multi
+  end
+
+  defp delete_discussions(%Multi{} = multi, %Actor{type: :Group, id: actor_id}) do
+    Multi.delete_all(multi, :delete_discussions, where(Discussion, [e], e.actor_id == ^actor_id))
+  end
+
+  @spec delete_participations(Actor.t()) :: :ok
+  defp delete_participations(%Actor{type: :Person} = actor) do
+    %Actor{participations: participations} = Repo.preload(actor, [:participations])
+    Enum.each(participations, &Events.delete_participant/1)
+  end
+
+  defp delete_participations(%Actor{type: :Group}), do: :ok
+
+  @spec delete_members(Multi.t(), Actor.t()) :: Multi.t()
+  defp delete_members(%Multi{} = multi, %Actor{type: :Person, id: actor_id}) do
+    Multi.delete_all(multi, :delete_members, where(Member, [e], e.actor_id == ^actor_id))
+  end
+
+  defp delete_members(%Multi{} = multi, %Actor{type: :Group, id: actor_id}) do
+    Multi.delete_all(multi, :delete_members, where(Member, [e], e.parent_id == ^actor_id))
+  end
+
+  @spec reset_default_actor_id(Actor.t()) :: {:ok, User.t() | nil} | {:error, :user_not_found}
+  defp reset_default_actor_id(%Actor{type: :Person, user: %User{id: user_id} = user, id: actor_id}) do
+    Logger.debug("reset_default_actor_id")
+
+    new_actor_id =
+      user
+      |> Users.get_actors_for_user()
+      |> Enum.map(& &1.id)
+      |> Enum.find(&(&1 !== actor_id))
+
+    {:ok, Users.update_user_default_actor(user_id, new_actor_id)}
+  rescue
+    _e in Ecto.NoResultsError ->
+      {:error, :user_not_found}
+  end
+
+  defp reset_default_actor_id(%Actor{type: :Person, user: nil}), do: {:ok, nil}
+
+  @spec remove_banner(Actor.t()) :: {:ok, Actor.t()}
+  defp remove_banner(%Actor{banner: nil} = actor), do: {:ok, actor}
+
+  defp remove_banner(%Actor{banner: %File{url: url}} = actor) do
+    safe_remove_file(url, actor)
+    {:ok, actor}
+  end
+
+  @spec remove_avatar(Actor.t()) :: {:ok, Actor.t()}
+  defp remove_avatar(%Actor{avatar: avatar} = actor) do
+    case avatar do
+      %File{url: url} ->
+        safe_remove_file(url, actor)
+        {:ok, actor}
+
+      nil ->
+        {:ok, actor}
+    end
+  end
+
+  @spec safe_remove_file(String.t(), Actor.t()) :: {:ok, Actor.t()}
+  defp safe_remove_file(url, %Actor{} = actor) do
+    case Upload.remove(url) do
+      {:ok, _value} ->
+        {:ok, actor}
+
+      {:error, error} ->
+        Logger.error("Error while removing an upload file")
+        Logger.debug(inspect(error))
+
+        {:ok, actor}
+    end
+  end
+
+  @spec send_suspension_notification(Actor.t()) :: :ok
+  defp send_suspension_notification(%Actor{type: :Group} = group) do
+    group
+    |> Actors.list_all_local_members_for_group()
+    |> Enum.each(&Group.send_group_suspension_notification/1)
+  end
+
+  defp send_suspension_notification(%Actor{} = _actor), do: :ok
+end

lib/service/workers/background.ex

@@ -6,6 +6,7 @@ defmodule Mobilizon.Service.Workers.Background do
   alias Mobilizon.Actors
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub.Refresher
+  alias Mobilizon.Service.ActorSuspension
 
   use Mobilizon.Service.Workers.Helper, queue: "background"
 
@@ -14,7 +15,7 @@ defmodule Mobilizon.Service.Workers.Background do
     with reserve_username when is_boolean(reserve_username) <-
            Map.get(args, "reserve_username", true),
          %Actor{} = actor <- Actors.get_actor(actor_id) do
-      Actors.perform(:delete_actor, actor, reserve_username: reserve_username)
+      ActorSuspension.suspend_actor(actor, reserve_username: reserve_username)
     end
   end
 

lib/service/workers/clean_suspended_actors.ex

@@ -0,0 +1,18 @@
+defmodule Mobilizon.Service.Workers.CleanUnconfirmedUsersWorker do
+  @moduledoc """
+  Worker to clean unattached media
+  """
+
+  use Oban.Worker, queue: "background"
+  alias Mobilizon.Actors
+  alias Mobilizon.Service.ActorSuspension
+
+  @suspention_days 30
+
+  @impl Oban.Worker
+  def perform(%Job{}) do
+    [suspension: @suspention_days]
+    |> Actors.list_suspended_actors_to_purge()
+    |> Enum.each(&ActorSuspension.suspend_actor(&1, reserve_username: true, suspension: true))
+  end
+end

lib/web/cache/activity_pub.ex

@@ -21,7 +21,7 @@ defmodule Mobilizon.Web.Cache.ActivityPub do
   Gets a actor by username and eventually domain.
   """
   @spec get_actor_by_name(String.t()) ::
-          {:commit, Actor.t()} | {:ignore, nil}
+          {:commit, ActorModel.t()} | {:ignore, nil}
   def get_actor_by_name(name) do
     Cachex.fetch(@cache, "actor_" <> name, fn "actor_" <> name ->
       case Actor.find_or_make_actor_from_nickname(name) do
@@ -38,7 +38,7 @@ defmodule Mobilizon.Web.Cache.ActivityPub do
   Gets a local actor by username.
   """
   @spec get_local_actor_by_name(String.t()) ::
-          {:commit, Actor.t()} | {:ignore, nil}
+          {:commit, ActorModel.t()} | {:ignore, nil}
   def get_local_actor_by_name(name) do
     Cachex.fetch(@cache, "local_actor_" <> name, fn "local_actor_" <> name ->
       case Actors.get_local_actor_by_name(name) do
@@ -195,7 +195,7 @@ defmodule Mobilizon.Web.Cache.ActivityPub do
   @doc """
   Gets a relay.
   """
-  @spec get_relay :: {:commit, Actor.t()} | {:ignore, nil}
+  @spec get_relay :: {:commit, ActorModel.t()} | {:ignore, nil}
   def get_relay do
     Cachex.fetch(@cache, "relay_actor", &Relay.get_actor/0)
   end

lib/web/email/group.ex

@@ -119,41 +119,57 @@ defmodule Mobilizon.Web.Email.Group do
       when role not in @member_roles,
       do: :ok
 
+  @spec send_group_deletion_notification(Member.t(), Actor.t()) :: :ok
   def send_group_deletion_notification(
         %Member{
-          actor: %Actor{user_id: user_id, id: actor_id},
+          actor: %Actor{user_id: user_id, id: actor_id} = member
-          parent: %Actor{domain: nil} = group,
-          role: member_role
         },
         %Actor{id: author_id} = author
       ) do
     with %User{email: email, locale: locale} <- Users.get_user!(user_id),
          {:member_not_author, true} <- {:member_not_author, author_id !== actor_id} do
-      Gettext.put_locale(locale)
+      do_send_group_deletion_notification(member, author: author, email: email, locale: locale)
-      instance = Config.instance_name()
-
-      subject =
-        gettext(
-          "The group %{group} has been deleted on %{instance}",
-          group: group.name,
-          instance: instance
-        )
-
-      Email.base_email(to: email, subject: subject)
-      |> assign(:locale, locale)
-      |> assign(:group, group)
-      |> assign(:role, member_role)
-      |> assign(:subject, subject)
-      |> assign(:instance, instance)
-      |> assign(:author, author)
-      |> render(:group_deletion)
-      |> Email.Mailer.send_email_later()
-
-      :ok
     else
       # Skip if it's the author itself
       {:member_not_author, _} ->
         :ok
     end
   end
+
+  @spec send_group_deletion_notification(Member.t()) :: :ok
+  def send_group_deletion_notification(%Member{actor: %Actor{user_id: user_id}} = member) do
+    case Users.get_user!(user_id) do
+      %User{email: email, locale: locale} ->
+        do_send_group_deletion_notification(member, email: email, locale: locale)
+    end
+  end
+
+  defp do_send_group_deletion_notification(
+         %Member{role: member_role, parent: %Actor{domain: nil} = group},
+         options
+       ) do
+    locale = Keyword.get(options, :locale)
+    Gettext.put_locale(locale)
+    instance = Config.instance_name()
+    author = Keyword.get(options, :author)
+
+    subject =
+      gettext(
+        "The group %{group} has been deleted on %{instance}",
+        group: group.name,
+        instance: instance
+      )
+
+    Email.base_email(to: Keyword.get(options, :email), subject: subject)
+    |> assign(:locale, locale)
+    |> assign(:group, group)
+    |> assign(:role, member_role)
+    |> assign(:subject, subject)
+    |> assign(:instance, instance)
+    |> assign(:author, author)
+    |> render(:group_deletion)
+    |> Email.Mailer.send_email_later()
+
+    :ok
+  end
 end

test/federation/activity_pub/transmogrifier_test.exs

@@ -202,7 +202,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
       with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
-            @mobilizon_group_url -> {:error, "Not found"}
+            @mobilizon_group_url -> {:error, :http_not_found}
             ^actor_url -> {:ok, actor}
           end
         end do
@@ -308,7 +308,7 @@ defmodule Mobilizon.Federation.ActivityPub.TransmogrifierTest do
       with_mock ActivityPubActor, [:passthrough],
         get_or_fetch_actor_by_url: fn url ->
           case url do
-            @mobilizon_group_url -> {:error, "Not found"}
+            @mobilizon_group_url -> {:error, :http_not_found}
             ^actor_url -> {:ok, actor}
           end
         end do

test/mobilizon/actors/actors_test.exs

@@ -5,7 +5,7 @@ defmodule Mobilizon.ActorsTest do
 
   import Mobilizon.Factory
 
-  alias Mobilizon.{Actors, Config, Discussions, Events, Tombstone, Users}
+  alias Mobilizon.{Actors, Config, Discussions, Events, Users}
   alias Mobilizon.Actors.{Actor, Bot, Follower, Member}
   alias Mobilizon.Discussions.Comment
   alias Mobilizon.Events.Event
@@ -292,57 +292,7 @@ defmodule Mobilizon.ActorsTest do
     test "update_actor/2 with invalid data returns error changeset", %{actor: actor} do
       assert {:error, %Ecto.Changeset{}} = Actors.update_actor(actor, @invalid_attrs)
       actor_fetched = Actors.get_actor!(actor.id)
-      assert actor = actor_fetched
+      assert actor.id == actor_fetched.id
-    end
-
-    test "perform delete the actor actually deletes the actor", %{
-      actor: %Actor{avatar: %{url: avatar_url}, banner: %{url: banner_url}, id: actor_id} = actor
-    } do
-      %Event{url: event1_url} = event1 = insert(:event, organizer_actor: actor)
-      insert(:event, organizer_actor: actor)
-
-      %Comment{url: comment1_url} = comment1 = insert(:comment, actor: actor)
-      insert(:comment, actor: actor)
-
-      %URI{path: "/media/" <> avatar_path} = URI.parse(avatar_url)
-      %URI{path: "/media/" <> banner_path} = URI.parse(banner_url)
-
-      assert File.exists?(
-               Config.get!([Uploader.Local, :uploads]) <>
-                 "/" <> avatar_path
-             )
-
-      assert File.exists?(
-               Config.get!([Uploader.Local, :uploads]) <>
-                 "/" <> banner_path
-             )
-
-      assert {:ok, %Actor{}} = Actors.perform(:delete_actor, actor)
-
-      assert %Actor{
-               name: nil,
-               summary: nil,
-               suspended: true,
-               avatar: nil,
-               banner: nil,
-               user_id: nil
-             } = Actors.get_actor(actor_id)
-
-      assert {:error, :event_not_found} = Events.get_event(event1.id)
-      assert %Tombstone{} = Tombstone.find_tombstone(event1_url)
-      assert %Comment{deleted_at: deleted_at} = Discussions.get_comment(comment1.id)
-      refute is_nil(deleted_at)
-      assert %Tombstone{} = Tombstone.find_tombstone(comment1_url)
-
-      refute File.exists?(
-               Config.get!([Uploader.Local, :uploads]) <>
-                 "/" <> avatar_path
-             )
-
-      refute File.exists?(
-               Config.get!([Uploader.Local, :uploads]) <>
-                 "/" <> banner_path
-             )
     end
 
     test "delete_actor/1 deletes the actor", %{
@@ -392,10 +342,8 @@ defmodule Mobilizon.ActorsTest do
              } = Actors.get_actor(actor_id)
 
       assert {:error, :event_not_found} = Events.get_event(event1.id)
-      assert %Tombstone{} = Tombstone.find_tombstone(event1_url)
       assert %Comment{deleted_at: deleted_at} = Discussions.get_comment(comment1.id)
       refute is_nil(deleted_at)
-      assert %Tombstone{} = Tombstone.find_tombstone(comment1_url)
 
       refute File.exists?(
                Config.get!([Uploader.Local, :uploads]) <>

test/service/actor_suspension_test.exs

@@ -0,0 +1,103 @@
+defmodule Mobilizon.Service.ActorSuspensionTest do
+  use Mobilizon.DataCase
+
+  import Mobilizon.Factory
+
+  alias Mobilizon.{Actors, Config, Discussions, Events}
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Discussions.Comment
+  alias Mobilizon.Events.{Event, Participant}
+  alias Mobilizon.Service.ActorSuspension
+  alias Mobilizon.Web.Upload.Uploader
+
+  describe "suspend a person" do
+    setup do
+      %Actor{} = actor = insert(:actor)
+
+      %Comment{} = comment = insert(:comment, actor: actor)
+      %Event{} = event = insert(:event, organizer_actor: actor)
+      %Event{} = insert(:event)
+      insert(:participant, event: event)
+
+      %Participant{} =
+        participant = insert(:participant, actor: actor, event: event, role: :participant)
+
+      {:ok, actor: actor, comment: comment, event: event, participant: participant}
+    end
+
+    test "local", %{actor: actor, comment: comment, event: event, participant: participant} do
+      assert actor
+             |> media_paths()
+             |> media_exists?()
+
+      assert {:ok, %Actor{}} = ActorSuspension.suspend_actor(actor)
+      assert %Actor{suspended: true} = Actors.get_actor(actor.id)
+      assert %Comment{deleted_at: %DateTime{}} = Discussions.get_comment(comment.id)
+      assert {:error, :event_not_found} = Events.get_event(event.id)
+      assert nil == Events.get_participant(participant.id)
+
+      refute actor
+             |> media_paths()
+             |> media_exists?()
+    end
+  end
+
+  describe "delete a person" do
+    setup do
+      %Actor{} = actor = insert(:actor)
+
+      %Comment{} = comment = insert(:comment, actor: actor)
+      %Event{} = event = insert(:event, organizer_actor: actor)
+
+      {:ok, actor: actor, comment: comment, event: event}
+    end
+
+    test "local", %{actor: actor, comment: comment, event: event} do
+      assert actor
+             |> media_paths()
+             |> media_exists?()
+
+      assert {:ok, %Actor{}} = ActorSuspension.suspend_actor(actor, reserve_username: false)
+      assert nil == Actors.get_actor(actor.id)
+      assert %Comment{deleted_at: %DateTime{}} = Discussions.get_comment(comment.id)
+      assert {:error, :event_not_found} = Events.get_event(event.id)
+
+      refute actor
+             |> media_paths()
+             |> media_exists?()
+    end
+  end
+
+  describe "suspend a group" do
+    setup do
+      %Actor{} = group = insert(:group)
+
+      %Event{} = event = insert(:event, attributed_to: group)
+
+      {:ok, group: group, event: event}
+    end
+
+    test "local", %{group: group, event: event} do
+      assert {:ok, %Actor{}} = ActorSuspension.suspend_actor(group)
+      assert %Actor{suspended: true} = Actors.get_actor(group.id)
+      assert {:error, :event_not_found} = Events.get_event(event.id)
+    end
+  end
+
+  defp media_paths(%Actor{avatar: %{url: avatar_url}, banner: %{url: banner_url}}) do
+    %URI{path: "/media/" <> avatar_path} = URI.parse(avatar_url)
+    %URI{path: "/media/" <> banner_path} = URI.parse(banner_url)
+    %{avatar: avatar_path, banner: banner_path}
+  end
+
+  defp media_exists?(%{avatar: avatar_path, banner: banner_path}) do
+    File.exists?(
+      Config.get!([Uploader.Local, :uploads]) <>
+        "/" <> avatar_path
+    ) &&
+      File.exists?(
+        Config.get!([Uploader.Local, :uploads]) <>
+          "/" <> banner_path
+      )
+  end
+end