Merge branch '1808-pleroma-sucks' into 'develop'

Resolve "Don't fail message ingestions when we can't update a user"

Closes #1808

See merge request pleroma/pleroma!2576

lib/pleroma/user.ex

@@ -1618,12 +1618,19 @@ def html_filter_policy(_), do: Pleroma.Config.get([:markup, :scrub_policy])
   def fetch_by_ap_id(ap_id), do: ActivityPub.make_user_from_ap_id(ap_id)
 
   def get_or_fetch_by_ap_id(ap_id) do
-    user = get_cached_by_ap_id(ap_id)
+    cached_user = get_cached_by_ap_id(ap_id)
 
-    if !is_nil(user) and !needs_update?(user) do
+    maybe_fetched_user = needs_update?(cached_user) && fetch_by_ap_id(ap_id)
+
+    case {cached_user, maybe_fetched_user} do
+      {_, {:ok, %User{} = user}} ->
         {:ok, user}
-    else
-      fetch_by_ap_id(ap_id)
+
+      {%User{} = user, _} ->
+        {:ok, user}
+
+      _ ->
+        {:error, :not_found}
     end
   end
 

test/support/factory.ex

@@ -34,7 +34,8 @@ def user_factory do
       last_digest_emailed_at: NaiveDateTime.utc_now(),
       last_refreshed_at: NaiveDateTime.utc_now(),
       notification_settings: %Pleroma.User.NotificationSetting{},
-      multi_factor_authentication_settings: %Pleroma.MFA.Settings{}
+      multi_factor_authentication_settings: %Pleroma.MFA.Settings{},
+      ap_enabled: true
     }
 
     %{

test/user_test.exs

@@ -586,6 +586,26 @@ test "updates an existing user, if stale" do
 
       refute user.last_refreshed_at == orig_user.last_refreshed_at
     end
+
+    @tag capture_log: true
+    test "it returns the old user if stale, but unfetchable" do
+      a_week_ago = NaiveDateTime.add(NaiveDateTime.utc_now(), -604_800)
+
+      orig_user =
+        insert(
+          :user,
+          local: false,
+          nickname: "admin@mastodon.example.org",
+          ap_id: "http://mastodon.example.org/users/raymoo",
+          last_refreshed_at: a_week_ago
+        )
+
+      assert orig_user.last_refreshed_at == a_week_ago
+
+      {:ok, user} = User.get_or_fetch_by_ap_id("http://mastodon.example.org/users/raymoo")
+
+      assert user.last_refreshed_at == orig_user.last_refreshed_at
+    end
   end
 
   test "returns an ap_id for a user" do

test/web/activity_pub/activity_pub_controller_test.exs

@@ -451,6 +451,36 @@ test "it inserts an incoming activity into the database", %{conn: conn} do
       assert Activity.get_by_ap_id(data["id"])
     end
 
+    @tag capture_log: true
+    test "it inserts an incoming activity into the database" <>
+           "even if we can't fetch the user but have it in our db",
+         %{conn: conn} do
+      user =
+        insert(:user,
+          ap_id: "https://mastodon.example.org/users/raymoo",
+          ap_enabled: true,
+          local: false,
+          last_refreshed_at: nil
+        )
+
+      data =
+        File.read!("test/fixtures/mastodon-post-activity.json")
+        |> Poison.decode!()
+        |> Map.put("actor", user.ap_id)
+        |> put_in(["object", "attridbutedTo"], user.ap_id)
+
+      conn =
+        conn
+        |> assign(:valid_signature, true)
+        |> put_req_header("content-type", "application/activity+json")
+        |> post("/inbox", data)
+
+      assert "ok" == json_response(conn, 200)
+
+      ObanHelpers.perform(all_enqueued(worker: ReceiverWorker))
+      assert Activity.get_by_ap_id(data["id"])
+    end
+
     test "it clears `unreachable` federation status of the sender", %{conn: conn} do
       data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!()