From 70410dfafd272bd1f38602446cc4f6e83645326f Mon Sep 17 00:00:00 2001
From: Maksim Pechnikov <parallel588@gmail.com>
Date: Wed, 8 Jan 2020 16:40:38 +0300
Subject: [PATCH] fix create service actor

---
 lib/pleroma/user.ex                   | 53 ++++++++++++++++++++-------
 lib/pleroma/web/activity_pub/relay.ex |  4 +-
 test/user_test.exs                    | 37 +++++++++++++++++++
 3 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index 706aee2ff..7ce9e17df 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -1430,20 +1430,47 @@ defmodule Pleroma.User do
   Creates an internal service actor by URI if missing.
   Optionally takes nickname for addressing.
   """
-  def get_or_create_service_actor_by_ap_id(uri, nickname \\ nil) do
-    with user when is_nil(user) <- get_cached_by_ap_id(uri) do
-      {:ok, user} =
-        %User{
-          invisible: true,
-          local: true,
-          ap_id: uri,
-          nickname: nickname,
-          follower_address: uri <> "/followers"
-        }
-        |> Repo.insert()
+  @spec get_or_create_service_actor_by_ap_id(String.t(), String.t()) :: User.t() | nil
+  def get_or_create_service_actor_by_ap_id(uri, nickname) do
+    {_, user} =
+      case get_cached_by_ap_id(uri) do
+        nil ->
+          with {:error, %{errors: errors}} <- create_service_actor(uri, nickname) do
+            Logger.error("Cannot create service actor: #{uri}/.\n#{inspect(errors)}")
+            {:error, nil}
+          end
 
-      user
-    end
+        %User{invisible: false} = user ->
+          set_invisible(user)
+
+        user ->
+          {:ok, user}
+      end
+
+    user
+  end
+
+  @spec set_invisible(User.t()) :: {:ok, User.t()}
+  defp set_invisible(user) do
+    user
+    |> change(%{invisible: true})
+    |> update_and_set_cache()
+  end
+
+  @spec create_service_actor(String.t(), String.t()) ::
+          {:ok, User.t()} | {:error, Ecto.Changeset.t()}
+  defp create_service_actor(uri, nickname) do
+    %User{
+      invisible: true,
+      local: true,
+      ap_id: uri,
+      nickname: nickname,
+      follower_address: uri <> "/followers"
+    }
+    |> change
+    |> unique_constraint(:nickname)
+    |> Repo.insert()
+    |> set_cache()
   end
 
   # AP style
diff --git a/lib/pleroma/web/activity_pub/relay.ex b/lib/pleroma/web/activity_pub/relay.ex
index 99a804568..48a1b71e0 100644
--- a/lib/pleroma/web/activity_pub/relay.ex
+++ b/lib/pleroma/web/activity_pub/relay.ex
@@ -9,10 +9,12 @@ defmodule Pleroma.Web.ActivityPub.Relay do
   alias Pleroma.Web.ActivityPub.ActivityPub
   require Logger
 
+  @relay_nickname "relay"
+
   def get_actor do
     actor =
       relay_ap_id()
-      |> User.get_or_create_service_actor_by_ap_id()
+      |> User.get_or_create_service_actor_by_ap_id(@relay_nickname)
 
     actor
   end
diff --git a/test/user_test.exs b/test/user_test.exs
index d7ab63463..9da1e02a9 100644
--- a/test/user_test.exs
+++ b/test/user_test.exs
@@ -17,6 +17,7 @@ defmodule Pleroma.UserTest do
 
   import Mock
   import Pleroma.Factory
+  import ExUnit.CaptureLog
 
   setup_all do
     Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
@@ -26,6 +27,42 @@ defmodule Pleroma.UserTest do
   clear_config([:instance, :account_activation_required])
 
   describe "service actors" do
+    test "returns updated invisible actor" do
+      uri = "#{Pleroma.Web.Endpoint.url()}/relay"
+      followers_uri = "#{uri}/followers"
+
+      insert(
+        :user,
+        %{
+          nickname: "relay",
+          invisible: false,
+          local: true,
+          ap_id: uri,
+          follower_address: followers_uri
+        }
+      )
+
+      actor = User.get_or_create_service_actor_by_ap_id(uri, "relay")
+      assert actor.invisible
+    end
+
+    test "returns relay user" do
+      uri = "#{Pleroma.Web.Endpoint.url()}/relay"
+      followers_uri = "#{uri}/followers"
+
+      assert %User{
+               nickname: "relay",
+               invisible: true,
+               local: true,
+               ap_id: ^uri,
+               follower_address: ^followers_uri
+             } = User.get_or_create_service_actor_by_ap_id(uri, "relay")
+
+      assert capture_log(fn ->
+               refute User.get_or_create_service_actor_by_ap_id("/relay", "relay")
+             end) =~ "Cannot create service actor:"
+    end
+
     test "returns invisible actor" do
       uri = "#{Pleroma.Web.Endpoint.url()}/internal/fetch-test"
       followers_uri = "#{uri}/followers"
-- 
2.45.2