Merge branch 'issue/733' into 'develop'

[#733] added test for Ostatus

See merge request pleroma/pleroma!1657

lib/pleroma/web/activity_pub/transmogrifier.ex

@@ -1042,8 +1042,8 @@ def upgrade_user_from_ap_id(ap_id) do
     with %User{local: false} = user <- User.get_cached_by_ap_id(ap_id),
          {:ok, data} <- ActivityPub.fetch_and_prepare_user_from_ap_id(ap_id),
          already_ap <- User.ap_enabled?(user),
-         {:ok, user} <- user |> User.upgrade_changeset(data, true) |> User.update_and_set_cache() do
+         {:ok, user} <- upgrade_user(user, data) do
-      unless already_ap do
+      if not already_ap do
         TransmogrifierWorker.enqueue("user_upgrade", %{"user_id" => user.id})
       end
 
@@ -1054,6 +1054,12 @@ def upgrade_user_from_ap_id(ap_id) do
     end
   end
 
+  defp upgrade_user(user, data) do
+    user
+    |> User.upgrade_changeset(data, true)
+    |> User.update_and_set_cache()
+  end
+
   def maybe_retire_websub(ap_id) do
     # some sanity checks
     if is_binary(ap_id) && String.length(ap_id) > 8 do

lib/pleroma/web/ostatus/ostatus.ex

@@ -3,14 +3,12 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.OStatus do
-  import Ecto.Query
   import Pleroma.Web.XML
   require Logger
 
   alias Pleroma.Activity
   alias Pleroma.HTTP
   alias Pleroma.Object
-  alias Pleroma.Repo
   alias Pleroma.User
   alias Pleroma.Web
   alias Pleroma.Web.ActivityPub.ActivityPub
@@ -38,21 +36,13 @@ def is_representable?(%Activity{} = activity) do
     end
   end
 
-  def feed_path(user) do
+  def feed_path(user), do: "#{user.ap_id}/feed.atom"
-    "#{user.ap_id}/feed.atom"
-  end
 
-  def pubsub_path(user) do
+  def pubsub_path(user), do: "#{Web.base_url()}/push/hub/#{user.nickname}"
-    "#{Web.base_url()}/push/hub/#{user.nickname}"
-  end
 
-  def salmon_path(user) do
+  def salmon_path(user), do: "#{user.ap_id}/salmon"
-    "#{user.ap_id}/salmon"
-  end
 
-  def remote_follow_path do
+  def remote_follow_path, do: "#{Web.base_url()}/ostatus_subscribe?acct={uri}"
-    "#{Web.base_url()}/ostatus_subscribe?acct={uri}"
-  end
 
   def handle_incoming(xml_string, options \\ []) do
     with doc when doc != :error <- parse_document(xml_string) do
@@ -217,10 +207,9 @@ def get_content(entry) do
     Get the cw that mastodon uses.
   """
   def get_cw(entry) do
-    with cw when not is_nil(cw) <- string_from_xpath("/*/summary", entry) do
+    case string_from_xpath("/*/summary", entry) do
-      cw
+      cw when not is_nil(cw) -> cw
-    else
+      _ -> nil
-      _e -> nil
     end
   end
 
@@ -232,19 +221,17 @@ def get_tags(entry) do
   end
 
   def maybe_update(doc, user) do
-    if "true" == string_from_xpath("//author[1]/ap_enabled", doc) do
+    case string_from_xpath("//author[1]/ap_enabled", doc) do
+      "true" ->
         Transmogrifier.upgrade_user_from_ap_id(user.ap_id)
-    else
+
+      _ ->
         maybe_update_ostatus(doc, user)
     end
   end
 
   def maybe_update_ostatus(doc, user) do
-    old_data = %{
+    old_data = Map.take(user, [:bio, :avatar, :name])
-      avatar: user.avatar,
-      bio: user.bio,
-      name: user.name
-    }
 
     with false <- user.local,
          avatar <- make_avatar_object(doc),
@@ -279,21 +266,28 @@ def find_make_or_update_actor(doc) do
     end
   end
 
+  @spec find_or_make_user(String.t()) :: {:ok, User.t()}
   def find_or_make_user(uri) do
-    query = from(user in User, where: user.ap_id == ^uri)
+    case User.get_by_ap_id(uri) do
-
+      %User{} = user -> {:ok, user}
-    user = Repo.one(query)
+      _ -> make_user(uri)
-
-    if is_nil(user) do
-      make_user(uri)
-    else
-      {:ok, user}
     end
   end
 
+  @spec make_user(String.t(), boolean()) :: {:ok, User.t()} | {:error, any()}
   def make_user(uri, update \\ false) do
     with {:ok, info} <- gather_user_info(uri) do
-      data = %{
+      with false <- update,
+           %User{} = user <- User.get_cached_by_ap_id(info["uri"]) do
+        {:ok, user}
+      else
+        _e -> User.insert_or_update_user(build_user_data(info))
+      end
+    end
+  end
+
+  defp build_user_data(info) do
+    %{
       name: info["name"],
       nickname: info["nickname"] <> "@" <> info["host"],
       ap_id: info["uri"],
@@ -301,14 +295,6 @@ def make_user(uri, update \\ false) do
       avatar: info["avatar"],
       bio: info["bio"]
     }
-
-      with false <- update,
-           %User{} = user <- User.get_cached_by_ap_id(data.ap_id) do
-        {:ok, user}
-      else
-        _e -> User.insert_or_update_user(data)
-      end
-    end
   end
 
   # TODO: Just takes the first one for now.
@@ -319,23 +305,23 @@ def make_avatar_object(author_doc, rel \\ "avatar") do
     if href do
       %{
         "type" => "Image",
-        "url" => [
+        "url" => [%{"type" => "Link", "mediaType" => type, "href" => href}]
-          %{
-            "type" => "Link",
-            "mediaType" => type,
-            "href" => href
-          }
-        ]
       }
     else
       nil
     end
   end
 
+  @spec gather_user_info(String.t()) :: {:ok, map()} | {:error, any()}
   def gather_user_info(username) do
     with {:ok, webfinger_data} <- WebFinger.finger(username),
          {:ok, feed_data} <- Websub.gather_feed_data(webfinger_data["topic"]) do
-      {:ok, Map.merge(webfinger_data, feed_data) |> Map.put("fqn", username)}
+      data =
+        webfinger_data
+        |> Map.merge(feed_data)
+        |> Map.put("fqn", username)
+
+      {:ok, data}
     else
       e ->
         Logger.debug(fn -> "Couldn't gather info for #{username}" end)
@@ -371,10 +357,7 @@ def get_atom_url(body) do
   def fetch_activity_from_atom_url(url, options \\ []) do
     with true <- String.starts_with?(url, "http"),
          {:ok, %{body: body, status: code}} when code in 200..299 <-
-           HTTP.get(
+           HTTP.get(url, [{:Accept, "application/atom+xml"}]) do
-             url,
-             [{:Accept, "application/atom+xml"}]
-           ) do
       Logger.debug("Got document from #{url}, handling...")
       handle_incoming(body, options)
     else

lib/pleroma/web/ostatus/ostatus_controller.ex

@@ -55,12 +55,11 @@ def feed_redirect(conn, %{"nickname" => nickname}) do
 
   def feed(conn, %{"nickname" => nickname} = params) do
     with {_, %User{} = user} <- {:fetch_user, User.get_cached_by_nickname(nickname)} do
-      query_params =
-        Map.take(params, ["max_id"])
-        |> Map.merge(%{"whole_db" => true, "actor_id" => user.ap_id})
-
       activities =
-        ActivityPub.fetch_public_activities(query_params)
+        params
+        |> Map.take(["max_id"])
+        |> Map.merge(%{"whole_db" => true, "actor_id" => user.ap_id})
+        |> ActivityPub.fetch_public_activities()
         |> Enum.reverse()
 
       response =
@@ -98,8 +97,7 @@ def salmon_incoming(conn, _) do
 
     Federator.incoming_doc(doc)
 
-    conn
+    send_resp(conn, 200, "")
-    |> send_resp(200, "")
   end
 
   def object(%{assigns: %{format: format}} = conn, %{"uuid" => _uuid})

test/web/ostatus/ostatus_test.exs

@@ -628,4 +628,18 @@ test "Article objects are not representable" do
       refute OStatus.is_representable?(note_activity)
     end
   end
+
+  describe "make_user/2" do
+    test "creates new user" do
+      {:ok, user} = OStatus.make_user("https://social.heldscal.la/user/23211")
+
+      created_user =
+        User
+        |> Repo.get_by(ap_id: "https://social.heldscal.la/user/23211")
+        |> Map.put(:last_digest_emailed_at, nil)
+
+      assert user.info
+      assert user == created_user
+    end
+  end
 end