From 5990633fb9cbe966755838f88b4a96c3c86ea0a9 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Mon, 15 Jun 2020 11:13:20 +0200 Subject: [PATCH] Use ActivityPub.get_or_fetch_actor_by_url/2 instead of directly Actors.get_actor_by_url So that we can refresh actors when they're stale Signed-off-by: Thomas Citharel --- lib/federation/activity_pub/activity_pub.ex | 3 +- lib/federation/activity_pub/federator.ex | 4 +-- lib/federation/activity_pub/transmogrifier.ex | 29 ++++++++++++------- lib/federation/activity_pub/utils.ex | 4 +-- .../activity_stream/converter/flag.ex | 6 ++-- lib/federation/web_finger/web_finger.ex | 3 +- lib/graphql/schema/event.ex | 2 +- 7 files changed, 30 insertions(+), 21 deletions(-) diff --git a/lib/federation/activity_pub/activity_pub.ex b/lib/federation/activity_pub/activity_pub.ex index a444dab1b..1895d0afb 100644 --- a/lib/federation/activity_pub/activity_pub.ex +++ b/lib/federation/activity_pub/activity_pub.ex @@ -163,6 +163,7 @@ defmodule Mobilizon.Federation.ActivityPub do 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), false <- Actors.needs_update?(cached_actor) do @@ -295,7 +296,7 @@ defmodule Mobilizon.Federation.ActivityPub do local \\ true, public \\ true ) do - with {:ok, %Actor{id: object_owner_actor_id}} <- Actors.get_actor_by_url(object["actor"]), + with {:ok, %Actor{id: object_owner_actor_id}} <- get_or_fetch_actor_by_url(object["actor"]), {:ok, %Share{} = _share} <- Share.create(object["id"], actor.id, object_owner_actor_id), announce_data <- make_announce_data(actor, object, activity_id, public), {:ok, activity} <- create_activity(announce_data, local), diff --git a/lib/federation/activity_pub/federator.ex b/lib/federation/activity_pub/federator.ex index f52d1531f..2c2b03de7 100644 --- a/lib/federation/activity_pub/federator.ex +++ b/lib/federation/activity_pub/federator.ex @@ -11,7 +11,7 @@ defmodule Mobilizon.Federation.ActivityPub.Federator do use GenServer alias Mobilizon.Actors - + alias Mobilizon.Actors.Actor alias Mobilizon.Federation.ActivityPub alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier} @@ -43,7 +43,7 @@ defmodule Mobilizon.Federation.ActivityPub.Federator do Logger.debug(inspect(activity)) Logger.debug(fn -> "Running publish for #{activity.data["id"]}" end) - with actor when not is_nil(actor) <- Actors.get_actor_by_url!(activity.data["actor"]) do + with {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(activity.data["actor"]) do Logger.info(fn -> "Sending #{activity.data["id"]} out via AP" end) ActivityPub.publish(actor, activity) end diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index c3a7f3f57..561a3203c 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -175,7 +175,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:member, Actors.is_member?(object_data.creator_id, object_data.actor_id)}, {:ok, %Activity{} = activity, %Resource{} = resource} <- ActivityPub.create(:resource, object_data, false), - {:ok, %Actor{type: :Group, id: group_id} = group} <- Actors.get_actor_by_url(group_url), + {:ok, %Actor{type: :Group, id: group_id} = group} <- + ActivityPub.get_or_fetch_actor_by_url(group_url), announce_id <- "#{object_url}/announces/#{group_id}", {:ok, _activity, _resource} <- ActivityPub.announce(group, object, announce_id) do {:ok, activity, resource} @@ -262,7 +263,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do ) do with actor <- Utils.get_actor(data), # TODO: Is the following line useful? - {:ok, %Actor{id: actor_id} = _actor} <- ActivityPub.get_or_fetch_actor_by_url(actor), + {:ok, %Actor{id: actor_id, suspended: false} = _actor} <- + ActivityPub.get_or_fetch_actor_by_url(actor), :ok <- Logger.debug("Fetching contained object"), {:ok, object} <- fetch_obj_helper_as_activity_streams(object), :ok <- Logger.debug("Handling contained object"), @@ -271,7 +273,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do {:ok, _activity, entity} <- handle_incoming(create_data), :ok <- Logger.debug("Finished processing contained object"), {:ok, activity} <- ActivityPub.create_activity(data, false), - {:ok, %Actor{id: object_owner_actor_id}} <- Actors.get_actor_by_url(object["actor"]), + {:ok, %Actor{id: object_owner_actor_id}} <- + ActivityPub.get_or_fetch_actor_by_url(object["actor"]), {:ok, %Mobilizon.Share{} = _share} <- Mobilizon.Share.create(object["id"], actor_id, object_owner_actor_id) do {:ok, activity, entity} @@ -288,7 +291,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do "actor" => _actor_id }) when object_type in ["Person", "Group", "Application", "Service", "Organization"] do - with {:ok, %Actor{} = old_actor} <- Actors.get_actor_by_url(object["id"]), + with {:ok, %Actor{suspended: false} = old_actor} <- + ActivityPub.get_or_fetch_actor_by_url(object["id"]), object_data <- object |> Converter.Actor.as_to_model_data(), {:ok, %Activity{} = activity, %Actor{} = new_actor} <- @@ -306,7 +310,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do update_data ) do with actor <- Utils.get_actor(update_data), - {:ok, %Actor{url: actor_url}} <- Actors.get_actor_by_url(actor), + {:ok, %Actor{url: actor_url, suspended: false}} <- + ActivityPub.get_or_fetch_actor_by_url(actor), {:ok, %Event{} = old_event} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(), object_data <- Converter.Event.as_to_model_data(object), @@ -351,8 +356,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do "id" => id } = _data ) do - with {:ok, %Actor{domain: nil} = followed} <- Actors.get_actor_by_url(followed), - {:ok, %Actor{} = follower} <- Actors.get_actor_by_url(follower), + with {:ok, %Actor{domain: nil} = followed} <- ActivityPub.get_or_fetch_actor_by_url(followed), + {:ok, %Actor{} = follower} <- ActivityPub.get_or_fetch_actor_by_url(follower), {:ok, activity, object} <- ActivityPub.unfollow(follower, followed, id, false) do {:ok, activity, object} else @@ -371,7 +376,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do %{"type" => "Delete", "object" => object, "actor" => _actor, "id" => _id} = data ) do with actor <- Utils.get_actor(data), - {:ok, %Actor{url: actor_url}} <- Actors.get_actor_by_url(actor), + {:ok, %Actor{url: actor_url}} <- ActivityPub.get_or_fetch_actor_by_url(actor), object_id <- Utils.get_url(object), {:origin_check, true} <- {:origin_check, Utils.origin_check_from_id?(actor_url, object_id)}, @@ -399,7 +404,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do } = data ) do with actor <- Utils.get_actor(data), - {:ok, %Actor{url: _actor_url} = actor} <- Actors.get_actor_by_url(actor), + {:ok, %Actor{url: _actor_url, suspended: false} = actor} <- + ActivityPub.get_or_fetch_actor_by_url(actor), object <- Utils.get_url(object), {:ok, object} <- ActivityPub.fetch_object_from_url(object), {:ok, activity, object} <- @@ -416,7 +422,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do %{"type" => "Leave", "object" => object, "actor" => actor, "id" => _id} = data ) do with actor <- Utils.get_actor(data), - {:ok, %Actor{} = actor} <- Actors.get_actor_by_url(actor), + {:ok, %Actor{} = actor} <- ActivityPub.get_or_fetch_actor_by_url(actor), object <- Utils.get_url(object), {:ok, object} <- ActivityPub.fetch_object_from_url(object), {:ok, activity, object} <- ActivityPub.leave(object, actor, false) do @@ -443,7 +449,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do "target" => target } = data ) do - with {:ok, %Actor{} = actor} <- data |> Utils.get_actor() |> Actors.get_actor_by_url(), + with {:ok, %Actor{} = actor} <- + data |> Utils.get_actor() |> ActivityPub.get_or_fetch_actor_by_url(), {:ok, object} <- object |> Utils.get_url() |> ActivityPub.fetch_object_from_url(), {:ok, %Actor{} = target} <- target |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url(), diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex index 5cb487d10..5e969eb9d 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/lib/federation/activity_pub/utils.ex @@ -8,10 +8,10 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do Various ActivityPub related utils. """ - alias Mobilizon.Actors alias Mobilizon.Actors.Actor alias Mobilizon.Media.Picture + alias Mobilizon.Federation.ActivityPub alias Mobilizon.Federation.ActivityPub.{Activity, Federator, Relay} alias Mobilizon.Federation.ActivityStream.Converter alias Mobilizon.Federation.HTTPSignatures @@ -118,7 +118,7 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do to = to ++ (data["cc"] || []) to - |> Enum.map(fn url -> Actors.get_actor_by_url(url) end) + |> Enum.map(fn url -> ActivityPub.get_or_fetch_actor_by_url(url) end) |> Enum.map(fn {status, actor} -> case status do :ok -> diff --git a/lib/federation/activity_stream/converter/flag.ex b/lib/federation/activity_stream/converter/flag.ex index 87f893e14..f3abb7a74 100644 --- a/lib/federation/activity_stream/converter/flag.ex +++ b/lib/federation/activity_stream/converter/flag.ex @@ -8,13 +8,13 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Flag do Note: Reports are named Flag in AS. """ - alias Mobilizon.Actors alias Mobilizon.Actors.Actor alias Mobilizon.Conversations alias Mobilizon.Events alias Mobilizon.Events.Event alias Mobilizon.Reports.Report + alias Mobilizon.Federation.ActivityPub alias Mobilizon.Federation.ActivityPub.Relay alias Mobilizon.Federation.ActivityStream.{Converter, Convertible} @@ -65,10 +65,10 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Flag do @spec as_to_model(map) :: map def as_to_model(%{"object" => objects} = object) do - with {:ok, %Actor{} = reporter} <- Actors.get_actor_by_url(object["actor"]), + with {:ok, %Actor{} = reporter} <- ActivityPub.get_or_fetch_actor_by_url(object["actor"]), %Actor{} = reported <- Enum.reduce_while(objects, nil, fn url, _ -> - case Actors.get_actor_by_url(url) do + case ActivityPub.get_or_fetch_actor_by_url(url) do {:ok, %Actor{} = actor} -> {:halt, actor} diff --git a/lib/federation/web_finger/web_finger.ex b/lib/federation/web_finger/web_finger.ex index 70141c783..1b939aeba 100644 --- a/lib/federation/web_finger/web_finger.ex +++ b/lib/federation/web_finger/web_finger.ex @@ -10,6 +10,7 @@ defmodule Mobilizon.Federation.WebFinger do alias Mobilizon.Actors alias Mobilizon.Actors.Actor + alias Mobilizon.Federation.ActivityPub alias Mobilizon.Federation.WebFinger.XmlBuilder alias Mobilizon.Web.Endpoint alias Mobilizon.Web.Router.Helpers, as: Routes @@ -48,7 +49,7 @@ defmodule Mobilizon.Federation.WebFinger do {:ok, represent_actor(actor, "JSON")} else _e -> - case Actors.get_actor_by_url(resource) do + case ActivityPub.get_or_fetch_actor_by_url(resource) do {:ok, %Actor{} = actor} when not is_nil(actor) -> {:ok, represent_actor(actor, "JSON")} diff --git a/lib/graphql/schema/event.ex b/lib/graphql/schema/event.ex index 67b47e573..a77e4f3e9 100644 --- a/lib/graphql/schema/event.ex +++ b/lib/graphql/schema/event.ex @@ -8,7 +8,7 @@ defmodule Mobilizon.GraphQL.Schema.EventType do import Absinthe.Resolution.Helpers, only: [dataloader: 1] import Mobilizon.GraphQL.Helpers.Error - alias Mobilizon.{Actors, Addresses, Conversations, Events} + alias Mobilizon.{Actors, Addresses, Conversations} alias Mobilizon.GraphQL.Resolvers.{Event, Picture, Tag} alias Mobilizon.GraphQL.Schema