Protocol.receive: bridge replies to followers where in-reply-to already is

...ie if it's on the follower's protocol, either native or bridged.

for #981, #950, #949

not confident in this, despite the tests. worried it will either over- or under-deliver. guess we'll see.

activitypub.py

@@ -619,6 +619,10 @@ def postprocess_as2(activity, orig_obj=None, wrap=True):
             activity['inReplyTo'] = orig_id
         elif isinstance(in_reply_to, list):
             if len(in_reply_to) > 1:
+                # this isn't actually true, AS2 inReplyTo can be multiply
+                # valued. Why do we truncate it to one value? interop somewhere?
+                # it's not marked Functional:
+                # https://www.w3.org/TR/activitystreams-vocabulary/#dfn-inreplyto
                 logger.warning(
                     "AS2 doesn't support multiple inReplyTo URLs! "
                     f'Only using the first: {in_reply_to[0]}')

protocol.py

@@ -1061,18 +1061,34 @@ class Protocol:
 
         target_uris = sorted(set(as1.targets(obj.as1)))
         logger.info(f'Raw targets: {target_uris}')
-        orig_obj = None
         targets = {}  # maps Target to Object or None
         owner = as1.get_owner(obj.as1)
 
+        in_reply_to_protocols = set()  # protocol kinds, eg 'MagicKey'
         in_reply_to_owners = []
         in_reply_tos = as1.get_ids(as1.get_object(obj.as1), 'inReplyTo')
         for in_reply_to in in_reply_tos:
-            if protocol := Protocol.for_id(in_reply_to):
-                if in_reply_to_obj := protocol.load(in_reply_to):
+            if proto := Protocol.for_id(in_reply_to):
+                if in_reply_to_obj := proto.load(in_reply_to):
+                    if proto.LABEL != obj.source_protocol:
+                        in_reply_to_protocols.add(proto._get_kind())
+                    else:
+                        proto_labels = proto.DEFAULT_ENABLED_PROTOCOLS + tuple(
+                            c.protocol for c in in_reply_to_obj.copies)
+                        in_reply_to_protocols.update(PROTOCOLS[c.protocol]._get_kind()
+                                                     for c in in_reply_to_obj.copies)
+
                     if reply_owner := as1.get_owner(in_reply_to_obj.as1):
                         in_reply_to_owners.append(reply_owner)
-        is_self_reply = False
+
+        is_reply = obj.type == 'comment' or in_reply_tos
+        if is_reply:
+            logger.info(f"It's a reply...")
+            if in_reply_to_protocols:
+                logger.info(f'  ...delivering to these protocols where the in-reply-to post was native or bridged: {in_reply_to_protocols}')
+            else:
+                logger.info(f"  ...skipping, in-reply-to post(s) are same protocol and weren't bridged anywhere")
+                return {}
 
         for id in sorted(target_uris):
             protocol = Protocol.for_id(id)
@@ -1088,12 +1104,6 @@ class Protocol:
                 logger.info(f"Couldn't load {id}")
                 continue
 
-            # deliver self-replies to followers
-            # https://github.com/snarfed/bridgy-fed/issues/639
-            if owner == as1.get_owner(orig_obj.as1):
-                is_self_reply = True
-                logger.info(f'Looks like a self reply! Delivering to all followers')
-
             if protocol == cls and cls.LABEL != 'fake':
                 logger.info(f'Skipping same-protocol target {id}')
                 continue
@@ -1122,14 +1132,16 @@ class Protocol:
             logger.info("Can't tell who this is from! Skipping followers.")
             return targets
 
-        is_reply = obj.type == 'comment' or in_reply_tos
         if (obj.type in ('post', 'update', 'delete', 'share')
-                and (is_self_reply or not is_reply)):
+                and (not is_reply or in_reply_to_protocols)):
             logger.info(f'Delivering to followers of {user_key}')
             followers = Follower.query(Follower.to == user_key,
                                        Follower.status == 'active'
                                        ).fetch()
-            users = [u for u in ndb.get_multi(f.from_ for f in followers) if u]
+            user_keys = [f.from_ for f in followers]
+            if is_reply:
+                user_keys = [k for k in user_keys if k.kind() in in_reply_to_protocols]
+            users = [u for u in ndb.get_multi(user_keys) if u]
             User.load_multi(users)
 
             # which object should we add to followers' feeds, if any
@@ -1168,7 +1180,9 @@ class Protocol:
                 feed_obj.put()
 
         # de-dupe targets, discard same-domain
+        # maps string target URL to (Target, Object) tuple
         candidates = {t.uri: (t, obj) for t, obj in targets.items()}
+        # maps Target to Object or None
         targets = {}
         source_domains = [
             util.domain_from_link(url) for url in

tests/test_protocol.py

@@ -429,7 +429,7 @@ class ProtocolTest(TestCase):
         self.store_object(id='at://did:plc:foo/co.ll/post', our_as1={'foo': 'bar'})
         self.store_object(id='fake:post', our_as1={'foo': 'baz'})
 
-        obj = Object(our_as1={
+        obj = Object(source_protocol='other', our_as1={
             'id': 'other:reply',
             'objectType': 'note',
             'inReplyTo': [
@@ -450,7 +450,7 @@ class ProtocolTest(TestCase):
             'objectType': 'note',
         }
         self.assertEqual({Target(protocol='fake', uri='fake:post:target')},
-                         Fake.targets(Object(our_as1={
+                         OtherFake.targets(Object(our_as1={
             'objectType': 'activity',
             'verb': 'post',
             'object': {
@@ -799,23 +799,23 @@ class ProtocolReceiveTest(TestCase):
         self.assertEqual([], Fake.sent)
 
     def test_create_reply(self):
-        self.make_followers()
+        eve = self.make_user('other:eve', cls=OtherFake, obj_id='other:eve')
 
-        Fake.fetchable['fake:post'] = {
+        OtherFake.fetchable['other:post'] = {
             'objectType': 'note',
-            'author': 'fake:bob',
+            'author': 'other:eve',
         }
         reply_as1 = {
             'id': 'fake:reply',
             'objectType': 'note',
-            'inReplyTo': 'fake:post',
+            'inReplyTo': 'other:post',
             'author': 'fake:alice',
         }
         create_as1 = {
             'id': 'fake:create',
             'objectType': 'activity',
             'verb': 'post',
-            'actor': 'fake:user',
+            'actor': 'fake:alice',
             'object': reply_as1,
         }
         self.assertEqual(('OK', 202), Fake.receive_as1(create_as1))
@@ -827,27 +827,28 @@ class ProtocolReceiveTest(TestCase):
         obj = self.assert_object('fake:create',
                                  status='complete',
                                  our_as1=create_as1,
-                                 delivered=['fake:post:target'],
+                                 delivered=['other:post:target'],
+                                 delivered_protocol='other',
                                  type='post',
-                                 users=[self.user.key, self.alice.key],
-                                 notify=[self.bob.key],
+                                 users=[self.alice.key],
+                                 notify=[eve.key],
                                  )
 
-        self.assertEqual([(obj.key.id(), 'fake:post:target')], Fake.sent)
+        self.assertEqual([(obj.key.id(), 'other:post:target')], OtherFake.sent)
 
     def test_create_reply_bare_object(self):
-        self.make_followers()
+        eve = self.make_user('other:eve', cls=OtherFake, obj_id='other:eve')
 
         reply_as1 = {
             'id': 'fake:reply',
             'objectType': 'note',
-            'inReplyTo': 'fake:post',
+            'inReplyTo': 'other:post',
             'author': 'fake:alice',
         }
-        Fake.fetchable['fake:post'] = {
+        OtherFake.fetchable['other:post'] = {
             'objectType': 'note',
-            'id': 'fake:post',
-            'author': 'fake:bob',
+            'id': 'other:post',
+            'author': 'other:eve',
         }
         self.assertEqual(('OK', 202), Fake.receive_as1(reply_as1))
 
@@ -867,16 +868,42 @@ class ProtocolReceiveTest(TestCase):
         obj = self.assert_object('fake:reply#bridgy-fed-create',
                                  status='complete',
                                  our_as1=create_as1,
-                                 delivered=['fake:post:target'],
+                                 delivered=['other:post:target'],
+                                 delivered_protocol='other',
                                  type='post',
                                  users=[self.alice.key],
-                                 notify=[self.bob.key],
+                                 notify=[eve.key],
                                  )
 
-        self.assertEqual([(obj.key.id(), 'fake:post:target')], Fake.sent)
+        self.assertEqual([(obj.key.id(), 'other:post:target')], OtherFake.sent)
 
     def test_create_reply_to_self_delivers_to_followers(self):
-        self.make_followers()
+        eve = self.make_user('other:eve', cls=OtherFake, obj_id='other:eve')
+        Follower.get_or_create(to=self.user, from_=eve)
+
+        reply_as1 = {
+            'id': 'fake:reply',
+            'objectType': 'note',
+            'inReplyTo': 'fake:post',
+            'author': 'fake:user',
+        }
+        self.store_object(id='fake:post', source_protocol='fake',
+                          copies=[Target(protocol='other', uri='other:post')],
+                          our_as1={
+                              'objectType': 'note',
+                              'id': 'fake:post',
+                              'author': 'fake:user',
+                          })
+        self.assertEqual(('OK', 202), Fake.receive_as1(reply_as1))
+
+        self.assert_object('fake:reply', our_as1=reply_as1, type='note',
+                           feed=[eve.key])
+
+        obj = Object.get_by_id(id='fake:reply#bridgy-fed-create')
+        self.assertEqual([(obj.key.id(), 'fake:post:target')], Fake.sent)
+        self.assertEqual([(obj.key.id(), 'other:eve:target')], OtherFake.sent)
+
+    def test_create_reply_isnt_bridged_if_original_isnt_bridged(self):
         eve = self.make_user('other:eve', cls=OtherFake, obj_id='other:eve')
         Follower.get_or_create(to=self.user, from_=eve)
 
@@ -891,19 +918,14 @@ class ProtocolReceiveTest(TestCase):
             'id': 'fake:post',
             'author': 'fake:user',
         }
-        self.assertEqual(('OK', 202), Fake.receive_as1(reply_as1))
 
-        self.assert_object('fake:reply', our_as1=reply_as1, type='note',
-                           feed=[self.alice.key, self.bob.key, eve.key])
+        with self.assertRaises(NoContent):
+            Fake.receive_as1(reply_as1)
 
-        obj = Object.get_by_id(id='fake:reply#bridgy-fed-create')
-        self.assertEqual([
-            (obj.key.id(), 'fake:post:target'),
-            (obj.key.id(), 'shared:target'),
-        ], Fake.sent)
-        self.assertEqual([
-            (obj.key.id(), 'other:eve:target'),
-        ], OtherFake.sent)
+        reply = self.assert_object('fake:reply', our_as1=reply_as1, type='note')
+        self.assertEqual([], reply.copies)
+        self.assertEqual([], Fake.sent)
+        self.assertEqual([], OtherFake.sent)
 
     def test_reply_skips_mention_of_original_post_author(self):
         bob = self.store_object(id='fake:bob', our_as1={'foo': 1})
@@ -941,16 +963,16 @@ class ProtocolReceiveTest(TestCase):
         ], Fake.sent)
 
     def test_update_reply(self):
-        self.make_followers()
+        eve = self.make_user('other:eve', cls=OtherFake, obj_id='other:eve')
 
-        Fake.fetchable['fake:post'] = {
+        OtherFake.fetchable['other:post'] = {
             'objectType': 'note',
-            'author': 'fake:bob',
+            'author': 'other:eve',
         }
         reply_as1 = {
             'id': 'fake:reply',
             'objectType': 'note',
-            'inReplyTo': 'fake:post',
+            'inReplyTo': 'other:post',
             'author': 'fake:alice',
         }
         self.store_object(id='fake:reply', our_as1=reply_as1)
@@ -971,12 +993,13 @@ class ProtocolReceiveTest(TestCase):
         obj = self.assert_object('fake:update',
                                  status='complete',
                                  our_as1=update_as1,
-                                 delivered=['fake:post:target'],
+                                 delivered=['other:post:target'],
+                                 delivered_protocol='other',
                                  type='update',
                                  users=[self.user.key, self.alice.key],
-                                 notify=[self.bob.key],
+                                 notify=[eve.key],
                                  )
-        self.assertEqual([(obj.key.id(), 'fake:post:target')], Fake.sent)
+        self.assertEqual([(obj.key.id(), 'other:post:target')], OtherFake.sent)
 
     def test_repost(self):
         self.make_followers()
@@ -2042,17 +2065,18 @@ class ProtocolReceiveTest(TestCase):
     @patch('oauth_dropins.webutil.appengine_config.tasks_client.create_task')
     def test_reply_send_tasks_orig_obj(self, mock_create_task):
         common.RUN_TASKS_INLINE = False
+        eve = self.make_user('other:eve', cls=OtherFake, obj_id='other:eve')
 
         reply_as1 = {
             'id': 'fake:reply',
             'objectType': 'note',
-            'inReplyTo': 'fake:post',
+            'inReplyTo': 'other:post',
             'author': 'fake:user',
         }
-        Fake.fetchable['fake:post'] = {
+        OtherFake.fetchable['other:post'] = {
             'objectType': 'note',
-            'id': 'fake:post',
-            'author': 'fake:bob',
+            'id': 'other:post',
+            'author': 'other:eve',
         }
         self.assertEqual(('OK', 202), Fake.receive_as1(reply_as1))
 
@@ -2062,10 +2086,10 @@ class ProtocolReceiveTest(TestCase):
                            )
 
         create_key = Object(id='fake:reply#bridgy-fed-create').key.urlsafe()
-        orig_obj_key = Object(id='fake:post').key.urlsafe()
-        self.assert_task(mock_create_task, 'send', '/queue/send', protocol='fake',
-                         obj=create_key, orig_obj=orig_obj_key, url='fake:post:target',
-                         user=self.user.key.urlsafe())
+        orig_obj_key = Object(id='other:post').key.urlsafe()
+        self.assert_task(mock_create_task, 'send', '/queue/send', protocol='other',
+                         obj=create_key, orig_obj=orig_obj_key,
+                         url='other:post:target', user=self.user.key.urlsafe())
 
         self.assertEqual([], OtherFake.sent)
         self.assertEqual([], Fake.sent)

tests/test_web.py

@@ -195,8 +195,8 @@ REPLY_HTML = """\
 <div class="h-entry">
 <a class="u-url" href="https://user.com/reply"></a>
 <p class="e-content p-name">
-<a class="u-in-reply-to" href="http://no.t/fediverse"></a>
 <a class="u-in-reply-to" href="https://mas.to/toot">foo ☕ bar</a>
+<a class="u-in-reply-to" href="http://no.t/fediverse"></a>
 <a href="http://localhost/"></a>
 </p>
 <a class="p-author h-card" href="https://user.com/">Ms. ☕ Baz</a>
@@ -252,13 +252,13 @@ AS2_CREATE = {
         'url': 'http://localhost/r/https://user.com/reply',
         'name': 'foo ☕ bar',
         'content': """\
-<a class="u-in-reply-to" href="http://no.t/fediverse"></a>
 <a class="u-in-reply-to" href="https://mas.to/toot">foo ☕ bar</a>
+<a class="u-in-reply-to" href="http://no.t/fediverse"></a>
 <a href="http://localhost/"></a>""",
         'contentMap': {
             'en': """\
-<a class="u-in-reply-to" href="http://no.t/fediverse"></a>
 <a class="u-in-reply-to" href="https://mas.to/toot">foo ☕ bar</a>
+<a class="u-in-reply-to" href="http://no.t/fediverse"></a>
 <a href="http://localhost/"></a>""",
         },
         'inReplyTo': 'https://mas.to/toot/id',
@@ -794,10 +794,16 @@ class WebTest(TestCase):
         })
         self.assertEqual(202, got.status_code)
 
-        self.assertEqual(1, mock_post.call_count)
-        args, kwargs = mock_post.call_args
-        self.assertEqual(('https://mas.to/inbox',), args)
-        self.assert_equals(AS2_UPDATE, json_loads(kwargs['data']))
+        self.assertEqual(['https://inbox/', 'https://mas.to/inbox',
+                          'https://public/inbox', 'https://shared/inbox'],
+                         [args[0] for args, _ in mock_post.call_args_list])
+
+        for args, kwargs in mock_post.call_args_list:
+            expected = copy.deepcopy(AS2_UPDATE)
+            if args[0] != 'https://mas.to/inbox':
+                del expected['object']['tag']  # Mention
+                expected['object']['inReplyTo'] = 'https://mas.to/toot'
+            self.assert_equals(expected, json_loads(kwargs['data']), ignore=['cc'])
 
     def test_redo_repost_isnt_update(self, mock_get, mock_post):
         """Like and Announce shouldn't use Update, they should just resend as is."""