[ActivityPub] Properly handle Actor URIs by using events correctly

This should fix nulls on explorer lookups inputed by postman after generate_followers/getSubscribers, that I think were caused by calling common_profile_uri that, curiously, only handles local profiles

lib/util/util.php

@@ -2330,7 +2330,8 @@ function common_profile_uri($profile)
             $user = User::getKV('id', $profile->id);
             if ($user instanceof User) {
                 $uri = $user->getUri();
-            }
+            } // FIXME: might be a remote profile, by this function name, I would guess it would be fine to call this
+              // On the other hand, there's Profile->getUri
             Event::handle('EndCommonProfileURI', [$profile, &$uri]);
         }
     }

plugins/ActivityPub/ActivityPubPlugin.php

@@ -60,29 +60,22 @@ class ActivityPubPlugin extends Plugin
     /**
      * Returns a Actor's URI from its local $profile
      * Works both for local and remote users.
+     * This is a discovery event but it seems more logical to have it separated.
+     * This ensures that Profile->getUri() will always return the intended for a remote AP profile.
      *
      * @param Profile $profile Actor's local profile
-     * @return string Actor's URI
+     * @param string &$uri I/O Actor's URI
-     * @throws Exception
      * @author Diogo Cordeiro <diogo@fc.up.pt>
+     * @return bool event hook
      */
-    public static function actor_uri($profile)
+    public function onStartGetProfileUri(Profile $profile, &$uri): bool
     {
-        return common_profile_uri($profile);
+        $aprofile = Activitypub_profile::getKV('profile_id', $profile->id);
+        if ($aprofile instanceof Activitypub_profile) {
+            $uri = $aprofile->getUri();
+            return false;
         }
-
+        return true;
-    /**
-     * Returns a Actor's URL from its local $profile
-     * Works both for local and remote users.
-     *
-     * @param Profile $profile Actor's local profile
-     * @return string Actor's URL
-     * @throws Exception
-     * @author Diogo Cordeiro <diogo@fc.up.pt>
-     */
-    public static function actor_url($profile)
-    {
-        return ActivityPubPlugin::actor_uri($profile)."/";
     }
 
     /**
@@ -621,7 +614,7 @@ class ActivityPubPlugin extends Plugin
         if ($object->isPerson()) {
             $link = new XML_XRD_Element_Link(
                 'self',
-                ActivityPubPlugin::actor_uri($object->getProfile()),
+                $object->getProfile()->getUri(),
                 'application/activity+json'
             );
             $xrd->links[] = clone($link);
@@ -743,25 +736,6 @@ class ActivityPubPlugin extends Plugin
      *                   Discovery Events                   *
      ********************************************************/
 
-    /**
-     * Profile URI for remote profiles.
-     *
-     * @author GNU social
-     * @author Diogo Cordeiro <diogo@fc.up.pt>
-     * @param Profile $profile
-     * @param string $uri in/out
-     * @return mixed hook return code
-     */
-    public function onStartGetProfileUri(Profile $profile, &$uri)
-    {
-        $aprofile = Activitypub_profile::getKV('profile_id', $profile->id);
-        if ($aprofile instanceof Activitypub_profile) {
-            $uri = $aprofile->getUri();
-            return false;
-        }
-        return true;
-    }
-
     /**
      * Profile from URI.
      *

plugins/ActivityPub/actions/apactorfollowers.php

@@ -122,7 +122,7 @@ class apActorFollowersAction extends ManagedAction
 
             /* Get followers' URLs */
             foreach ($sub as $s) {
-                $subs[] = ActivityPubPlugin::actor_uri($s);
+                $subs[] = $s->getUri();
             }
         } catch (NoResultException $e) {
             // Just let the exception go on its merry way

plugins/ActivityPub/actions/apactorfollowing.php

@@ -122,7 +122,7 @@ class apActorFollowingAction extends ManagedAction
 
             /* Get followed' URLs */
             foreach ($sub as $s) {
-                $subs[] = ActivityPubPlugin::actor_uri($s);
+                $subs[] = $s->getUri();
             }
         } catch (NoResultException $e) {
             // Just let the exception go on its merry way

plugins/ActivityPub/actions/apactoroutbox.php

@@ -123,7 +123,7 @@ class apActorOutboxAction extends ManagedAction
             // TODO: Handle other types
             if ($note->object_type == 'http://activitystrea.ms/schema/1.0/note') {
                 $notices[] = Activitypub_create::create_to_array(
-                    ActivityPubPlugin::actor_uri($note->getProfile()),
+                    $note->getProfile()->getUri(),
                     Activitypub_notice::notice_to_array($note)
                 );
             }

plugins/ActivityPub/classes/Activitypub_profile.php

@@ -86,7 +86,7 @@ class Activitypub_profile extends Managed_DataObject
      */
     public static function profile_to_array($profile)
     {
-        $uri = ActivityPubPlugin::actor_uri($profile);
+        $uri = $profile->getUri();
         $id = $profile->getID();
         $rsa = new Activitypub_rsa();
         $public_key = $rsa->ensure_public_key($profile);
@@ -589,7 +589,6 @@ class Activitypub_profile extends Managed_DataObject
         }
 
         $subs = Subscription::getSubscriberIDs($profile->id, $offset, $limit);
-        try {
         $profiles = [];
 
         $users = User::multiGet('id', $subs);
@@ -601,9 +600,6 @@ class Activitypub_profile extends Managed_DataObject
         foreach ($ap_profiles->fetchAll() as $ap) {
             $profiles[$ap->getID()] = $ap->local_profile();
         }
-        } catch (NoResultException $e) {
-            return $e->obj;
-        }
 
         if ($cache) {
             self::cacheSet(sprintf('activitypub_profile:subscriberCollection:%d', $profile->id), $profiles);

plugins/ActivityPub/classes/Activitypub_rsa.php

@@ -115,7 +115,7 @@ class Activitypub_rsa extends Managed_DataObject
                 // ASSERT: This should never happen, but try to recover!
                 common_log(LOG_ERR, "Activitypub_rsa: An impossible thing has happened... Please let the devs know that it entered in line 116 at Activitypub_rsa.php");
                 if ($fetch) {
-                    $res = Activitypub_explorer::get_remote_user_activity(ActivityPubPlugin::actor_uri($profile));
+                    $res = Activitypub_explorer::get_remote_user_activity($profile->getUri());
                     Activitypub_rsa::update_public_key($profile, $res['publicKey']['publicKeyPem']);
                     return self::ensure_public_key($profile, false);
                 } else {

plugins/ActivityPub/lib/httpsignature.php

@@ -36,7 +36,7 @@ class HttpSignature
         $key = openssl_pkey_get_private($actor_private_key);
         openssl_sign($stringToSign, $signature, $key, OPENSSL_ALGO_SHA256);
         $signature = base64_encode($signature);
-        $signatureHeader = 'keyId="' . ActivityPubPlugin::actor_uri($user).'#public-key' . '",headers="' . $signedHeaders . '",algorithm="rsa-sha256",signature="' . $signature . '"';
+        $signatureHeader = 'keyId="' . $user->getUri() . '#public-key' . '",headers="' . $signedHeaders . '",algorithm="rsa-sha256",signature="' . $signature . '"';
         unset($headers['(request-target)']);
         $headers['Signature'] = $signatureHeader;
 

plugins/ActivityPub/lib/models/Activitypub_announce.php

@@ -46,7 +46,7 @@ class Activitypub_announce
      */
     public static function announce_to_array(Profile $actor, Notice $notice): array
     {
-        $actor_uri = ActivityPubPlugin::actor_uri($actor);
+        $actor_uri = $actor->getUri();
         $notice_url = Activitypub_notice::getUrl($notice);
 
         $to = [common_local_url('apActorFollowers', ['id' => $actor->getID()])];

plugins/ActivityPub/lib/models/Activitypub_follow.php

@@ -85,13 +85,13 @@ class Activitypub_follow
         if (!Subscription::exists($actor_profile, $object_profile)) {
             Subscription::start($actor_profile, $object_profile);
             Activitypub_profile::subscribeCacheUpdate($actor_profile, $object_profile);
-            common_debug('ActivityPubPlugin: Accepted Follow request from '.ActivityPubPlugin::actor_uri($actor_profile).' to '.$object);
+            common_debug('ActivityPubPlugin: Accepted Follow request from '.$actor_profile->getUri().' to '.$object);
         } else {
-            common_debug('ActivityPubPlugin: Received a repeated Follow request from '.ActivityPubPlugin::actor_uri($actor_profile).' to '.$object);
+            common_debug('ActivityPubPlugin: Received a repeated Follow request from '.$actor_profile->getUri().' to '.$object);
         }
 
         // Notify remote instance that we have accepted their request
-        common_debug('ActivityPubPlugin: Notifying remote instance that we have accepted their Follow request request from '.ActivityPubPlugin::actor_uri($actor_profile).' to '.$object);
+        common_debug('ActivityPubPlugin: Notifying remote instance that we have accepted their Follow request request from '.$actor_profile->getUri().' to '.$object);
         $postman = new Activitypub_postman($object_profile, [$actor_aprofile]);
         $postman->accept_follow($id);
     }

plugins/ActivityPub/lib/models/Activitypub_message.php

@@ -63,7 +63,7 @@ class Activitypub_message
             'id'            => common_local_url('showmessage', ['message' => $message->getID()]),
             'type'          => 'Note',
             'published'     => str_replace(' ', 'T', $message->created).'Z',
-            'attributedTo'  => ActivityPubPlugin::actor_uri($from),
+            'attributedTo'  => $from->getUri(),
             'to'            => $to,
             'cc'            => [],
             'content'       => $message->getRendered(),

plugins/ActivityPub/lib/models/Activitypub_notice.php

@@ -85,7 +85,7 @@ class Activitypub_notice
             'type' => 'Note',
             'published' => str_replace(' ', 'T', $notice->getCreated()) . 'Z',
             'url' => self::getUrl($notice),
-            'attributedTo' => ActivityPubPlugin::actor_uri($profile),
+            'attributedTo' => $profile->getUri(),
             'to' => $to,
             'cc' => $cc,
             'conversation' => $notice->getConversationUrl(),
@@ -205,7 +205,7 @@ class Activitypub_notice
 
         foreach ($mentions_profiles as $mp) {
             if (!$mp->hasBlocked($actor_profile)) {
-                $act->context->attention[ActivityPubPlugin::actor_uri($mp)] = 'http://activitystrea.ms/schema/1.0/person';
+                $act->context->attention[$mp->getUri()] = 'http://activitystrea.ms/schema/1.0/person';
             }
         }
 

plugins/ActivityPub/lib/postman.php

@@ -58,7 +58,7 @@ class Activitypub_postman
         $this->actor = $from;
         $this->to = $to;
 
-        $this->actor_uri = ActivityPubPlugin::actor_uri($this->actor);
+        $this->actor_uri = $this->actor->getUri();
 
         $this->client = new HTTPClient();
     }
@@ -364,7 +364,7 @@ class Activitypub_postman
     public function delete_note($notice)
     {
         $data = Activitypub_delete::delete_to_array(
-            ActivityPubPlugin::actor_uri($notice->getProfile()),
+            $notice->getProfile()->getUri(),
             Activitypub_notice::getUrl($notice)
                 );
         $errors = [];