publisher: don't mangle between string and atom

Oban jobs only can have string args and there’s no reason to insist on atoms here.

Plus this used unchecked string_to_atom

lib/pleroma/web/activity_pub/publisher.ex

@@ -47,7 +47,9 @@ def is_representable?(%Activity{} = activity) do
   * `actor`: the actor which is signing the message
   * `id`: the ActivityStreams URI of the message
   """
-  def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = params) do
+  def publish_one(
+        %{"inbox" => inbox, "json" => json, "actor" => %User{} = actor, "id" => id} = params
+      ) do
     Logger.debug("Federating #{id} to #{inbox}")
     uri = %{path: path} = URI.parse(inbox)
     digest = "SHA-256=" <> (:crypto.hash(:sha256, json) |> Base.encode64())
@@ -74,24 +76,24 @@ def publish_one(%{inbox: inbox, json: json, actor: %User{} = actor, id: id} = pa
                {"digest", digest}
              ]
            ) do
-      if not Map.has_key?(params, :unreachable_since) || params[:unreachable_since] do
+      if not Map.has_key?(params, "unreachable_since") || params["unreachable_since"] do
         Instances.set_reachable(inbox)
       end
 
       result
     else
       {_post_result, response} ->
-        unless params[:unreachable_since], do: Instances.set_unreachable(inbox)
+        unless params["unreachable_since"], do: Instances.set_unreachable(inbox)
         {:error, response}
     end
   end
 
-  def publish_one(%{actor_id: actor_id} = params) do
+  def publish_one(%{"actor_id" => actor_id} = params) do
     actor = User.get_cached_by_id(actor_id)
 
     params
-    |> Map.delete(:actor_id)
-    |> Map.put(:actor, actor)
+    |> Map.delete("actor_id")
+    |> Map.put("actor", actor)
     |> publish_one()
   end
 
@@ -237,11 +239,11 @@ def publish(%User{} = actor, %{data: %{"bcc" => bcc}} = activity)
           |> Jason.encode!()
 
         Pleroma.Web.Federator.Publisher.enqueue_one(__MODULE__, %{
-          inbox: inbox,
-          json: json,
-          actor_id: actor.id,
-          id: activity.data["id"],
-          unreachable_since: unreachable_since
+          "inbox" => inbox,
+          "json" => json,
+          "actor_id" => actor.id,
+          "id" => activity.data["id"],
+          "unreachable_since" => unreachable_since
         })
       end)
     end)
@@ -270,11 +272,11 @@ def publish(%User{} = actor, %Activity{} = activity) do
       Pleroma.Web.Federator.Publisher.enqueue_one(
         __MODULE__,
         %{
-          inbox: inbox,
-          json: json,
-          actor_id: actor.id,
-          id: activity.data["id"],
-          unreachable_since: unreachable_since
+          "inbox" => inbox,
+          "json" => json,
+          "actor_id" => actor.id,
+          "id" => activity.data["id"],
+          "unreachable_since" => unreachable_since
         }
       )
     end)

lib/pleroma/workers/publisher_worker.ex

@@ -30,7 +30,6 @@ def perform(%Job{
   end
 
   def perform(%Job{args: %{"op" => "publish_one", "module" => module_name, "params" => params}}) do
-    params = Map.new(params, fn {k, v} -> {String.to_atom(k), v} end)
-    Federator.perform(:publish_one, String.to_atom(module_name), params)
+    Federator.perform(:publish_one, String.to_existing_atom(module_name), params)
   end
 end

test/pleroma/web/activity_pub/publisher_test.exs

@@ -146,20 +146,20 @@ test "publish to url with with different ports" do
 
       assert {:ok, %{body: "port 42"}} =
                Publisher.publish_one(%{
-                 inbox: inbox42,
-                 json: "{}",
-                 actor: actor,
-                 id: 1,
-                 unreachable_since: true
+                 "inbox" => inbox42,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1,
+                 "unreachable_since" => true
                })
 
       assert {:ok, %{body: "port 80"}} =
                Publisher.publish_one(%{
-                 inbox: inbox80,
-                 json: "{}",
-                 actor: actor,
-                 id: 1,
-                 unreachable_since: true
+                 "inbox" => inbox80,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1,
+                 "unreachable_since" => true
                })
     end
 
@@ -173,7 +173,14 @@ test "publish to url with with different ports" do
 
       inbox = "http://200.site/users/nick1/inbox"
 
-      assert {:ok, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1})
+      assert {:ok, _} =
+               Publisher.publish_one(%{
+                 "inbox" => inbox,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1
+               })
+
       assert called(Instances.set_reachable(inbox))
     end
 
@@ -189,11 +196,11 @@ test "publish to url with with different ports" do
 
       assert {:ok, _} =
                Publisher.publish_one(%{
-                 inbox: inbox,
-                 json: "{}",
-                 actor: actor,
-                 id: 1,
-                 unreachable_since: NaiveDateTime.utc_now()
+                 "inbox" => inbox,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1,
+                 "unreachable_since" => NaiveDateTime.utc_now()
                })
 
       assert called(Instances.set_reachable(inbox))
@@ -211,11 +218,11 @@ test "publish to url with with different ports" do
 
       assert {:ok, _} =
                Publisher.publish_one(%{
-                 inbox: inbox,
-                 json: "{}",
-                 actor: actor,
-                 id: 1,
-                 unreachable_since: nil
+                 "inbox" => inbox,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1,
+                 "unreachable_since" => nil
                })
 
       refute called(Instances.set_reachable(inbox))
@@ -231,7 +238,13 @@ test "publish to url with with different ports" do
 
       inbox = "http://404.site/users/nick1/inbox"
 
-      assert {:error, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1})
+      assert {:error, _} =
+               Publisher.publish_one(%{
+                 "inbox" => inbox,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1
+               })
 
       assert called(Instances.set_unreachable(inbox))
     end
@@ -248,7 +261,12 @@ test "publish to url with with different ports" do
 
       assert capture_log(fn ->
                assert {:error, _} =
-                        Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1})
+                        Publisher.publish_one(%{
+                          "inbox" => inbox,
+                          "json" => "{}",
+                          "actor" => actor,
+                          "id" => 1
+                        })
              end) =~ "connrefused"
 
       assert called(Instances.set_unreachable(inbox))
@@ -264,7 +282,13 @@ test "publish to url with with different ports" do
 
       inbox = "http://200.site/users/nick1/inbox"
 
-      assert {:ok, _} = Publisher.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1})
+      assert {:ok, _} =
+               Publisher.publish_one(%{
+                 "inbox" => inbox,
+                 "json" => "{}",
+                 "actor" => actor,
+                 "id" => 1
+               })
 
       refute called(Instances.set_unreachable(inbox))
     end
@@ -282,11 +306,11 @@ test "publish to url with with different ports" do
       assert capture_log(fn ->
                assert {:error, _} =
                         Publisher.publish_one(%{
-                          inbox: inbox,
-                          json: "{}",
-                          actor: actor,
-                          id: 1,
-                          unreachable_since: NaiveDateTime.utc_now()
+                          "inbox" => inbox,
+                          "json" => "{}",
+                          "actor" => actor,
+                          "id" => 1,
+                          "unreachable_since" => NaiveDateTime.utc_now()
                         })
              end) =~ "connrefused"
 
@@ -344,33 +368,33 @@ test "publish to url with with different ports" do
 
       assert not called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://domain.com/users/nick1/inbox",
-                 actor_id: actor.id,
-                 id: note_activity.data["id"]
+                 "inbox" => "https://domain.com/users/nick1/inbox",
+                 "actor_id" => actor.id,
+                 "id" => note_activity.data["id"]
                })
              )
 
       assert not called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://domain.com/users/nick1/inbox",
-                 actor_id: actor.id,
-                 id: public_note_activity.data["id"]
+                 "inbox" => "https://domain.com/users/nick1/inbox",
+                 "actor_id" => actor.id,
+                 "id" => public_note_activity.data["id"]
                })
              )
 
       assert not called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://rejected.com/users/nick2/inbox",
-                 actor_id: actor.id,
-                 id: note_activity.data["id"]
+                 "inbox" => "https://rejected.com/users/nick2/inbox",
+                 "actor_id" => actor.id,
+                 "id" => note_activity.data["id"]
                })
              )
 
       assert not called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://rejected.com/users/nick2/inbox",
-                 actor_id: actor.id,
-                 id: public_note_activity.data["id"]
+                 "inbox" => "https://rejected.com/users/nick2/inbox",
+                 "actor_id" => actor.id,
+                 "id" => public_note_activity.data["id"]
                })
              )
     end
@@ -406,9 +430,9 @@ test "publish to url with with different ports" do
 
       assert called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://domain.com/users/nick1/inbox",
-                 actor_id: actor.id,
-                 id: note_activity.data["id"]
+                 "inbox" => "https://domain.com/users/nick1/inbox",
+                 "actor_id" => actor.id,
+                 "id" => note_activity.data["id"]
                })
              )
     end
@@ -441,9 +465,9 @@ test "publish to url with with different ports" do
 
       assert called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://domain.com/users/nick1/inbox",
-                 actor_id: actor.id,
-                 id: note_activity.data["id"]
+                 "inbox" => "https://domain.com/users/nick1/inbox",
+                 "actor_id" => actor.id,
+                 "id" => note_activity.data["id"]
                })
              )
     end
@@ -493,17 +517,17 @@ test "publish to url with with different ports" do
 
       assert called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://domain.com/users/nick1/inbox",
-                 actor_id: actor.id,
-                 id: delete.data["id"]
+                 "inbox" => "https://domain.com/users/nick1/inbox",
+                 "actor_id" => actor.id,
+                 "id" => delete.data["id"]
                })
              )
 
       assert called(
                Pleroma.Web.Federator.Publisher.enqueue_one(Publisher, %{
-                 inbox: "https://domain2.com/users/nick1/inbox",
-                 actor_id: actor.id,
-                 id: delete.data["id"]
+                 "inbox" => "https://domain2.com/users/nick1/inbox",
+                 "actor_id" => actor.id,
+                 "id" => delete.data["id"]
                })
              )
     end