Protocol.translate: distinguish between object and user ids

atproto.py

@@ -213,6 +213,7 @@ class ATProto(User, Protocol):
 
         Object.get_or_create(did_plc.did, raw=did_plc.doc)
         user.atproto_did = did_plc.did
+        # TODO: move this to ATProto.get_or_create?
         add(user.copies, Target(uri=did_plc.did, protocol='atproto'))
         handle = user.handle_as('atproto')
 

ids.py

@@ -28,24 +28,32 @@ def translate_user_id(*, id, from_proto, to_proto):
     if from_proto == to_proto:
         return id
 
+    def copy_or_original():
+        if user := from_proto.get_by_id(id):
+            if copy := user.get_copy(to_proto):
+                return copy
+        if orig := models.get_original(id):
+            if isinstance(orig, to_proto):
+                return orig.key.id()
+
     match from_proto.LABEL, to_proto.LABEL:
-        case _, 'atproto':
-            user = from_proto.get_by_id(id)
-            return user.atproto_did if user else None
-        case 'atproto', _:
-            user = models.get_original(id)
-            return user.key.id() if user else None
+        case ('atproto', _) | (_, 'atproto'):
+            if found := copy_or_original():
+                return found
+            logger.warning(f"Can't translate user id {id} to {to_proto} , haven't copied it to/from there yet!")
+            return None
         case _, 'activitypub':
             return subdomain_wrap(from_proto, f'/ap/{id}')
         case 'activitypub', 'web':
             return id
+
         # only for unit tests
         case _, 'fake':
-            return f'fake:{id}'
+            return copy_or_original() or f'fake:u:{id}'
         case _, 'other':
-            return f'other:{id}'
+            return copy_or_original() or f'other:u:{id}'
         case 'fake' | 'other', _:
-            return id
+            return copy_or_original() or id
 
     assert False, (id, from_proto, to_proto)
 
@@ -110,18 +118,18 @@ def translate_object_id(*, id, from_proto, to_proto):
     if from_proto == to_proto:
         return id
 
-    # fall back subdomain-wrapped /convert/ URLs
+    def copy_or_original():
+        if obj := from_proto.load(id, remote=False):
+            if copy := obj.get_copy(to_proto):
+                return copy
+        if orig := models.get_original(id):
+            return orig.key.id()
+
     match from_proto.LABEL, to_proto.LABEL:
         case ('atproto' | 'nostr', _) | (_, 'atproto' | 'nostr'):
-            obj = from_proto.load(id, remote=False)
-            if obj:
-                for copy in obj.copies:
-                    if copy.protocol in (to_proto.LABEL, to_proto.ABBREV):
-                        return copy.uri
-            orig = models.get_original(id)
-            if orig:
-                return orig.key.id()
-            logger.warning(f"Can't translate {id} to {to_proto} , haven't copied it to/from there yet!")
+            if found := copy_or_original():
+                return found
+            logger.warning(f"Can't translate object id {id} to {to_proto} , haven't copied it to/from there yet!")
             return id
 
         case _, 'activitypub' | 'web':
@@ -129,8 +137,8 @@ def translate_object_id(*, id, from_proto, to_proto):
 
         # only for unit tests
         case _, 'fake':
-            return f'fake:{from_proto.ABBREV}:{id}'
+            return copy_or_original() or f'fake:o:{from_proto.ABBREV}:{id}'
         case _, 'other':
-            return f'other:{from_proto.ABBREV}:{id}'
+            return copy_or_original() or f'other:o:{from_proto.ABBREV}:{id}'
 
     assert False, (id, from_proto, to_proto)

protocol.py

@@ -17,7 +17,7 @@ import werkzeug.exceptions
 import common
 from common import add, DOMAIN_BLOCKLIST, DOMAINS, error, subdomain_wrap
 from flask_app import app
-from ids import translate_object_id
+from ids import translate_object_id, translate_user_id
 from models import Follower, get_originals, Object, PROTOCOLS, Target, User
 
 SUPPORTED_TYPES = (
@@ -497,7 +497,7 @@ class Protocol:
         outer_obj = copy.deepcopy(obj)
         inner_obj = outer_obj['object'] = as1.get_object(outer_obj)
 
-        def translate(elem, field):
+        def translate(elem, field, fn):
             elem[field] = as1.get_object(elem, field)
             id = elem[field].get('id')
             if id and util.domain_from_link(id) not in DOMAINS:
@@ -505,19 +505,26 @@ class Protocol:
                 # TODO: what if from_cls is None? relax translate_object_id,
                 # make it a noop if we don't know enough about from/to?
                 if from_cls and from_cls != to_cls:
-                    elem[field]['id'] = translate_object_id(
-                        id=id, from_proto=from_cls, to_proto=to_cls)
+                    elem[field]['id'] = fn(id=id, from_proto=from_cls, to_proto=to_cls)
             if elem[field].keys() == {'id'}:
                 elem[field] = elem[field]['id']
 
-        for o in outer_obj, inner_obj:
-            for field in ('actor', 'author', 'id', 'inReplyTo'):
-                translate(o, field)
+        type = as1.object_type(outer_obj)
+        translate(outer_obj, 'id',
+                  translate_user_id if type in as1.ACTOR_TYPES
+                  else translate_object_id)
 
-        for tag in (as1.get_objects(outer_obj, 'tags')
-                    + as1.get_objects(inner_obj, 'tags')):
-            if tag.get('objectType') == 'mention':
-                translate(tag, 'url')
+        translate(inner_obj, 'id',
+                  translate_user_id if type in ('follow', 'stop-following')
+                  else translate_object_id)
+
+        for o in outer_obj, inner_obj:
+            translate(o, 'inReplyTo', translate_object_id)
+            for field in 'actor', 'author':
+                translate(o, field, translate_user_id)
+            for tag in as1.get_objects(o, 'tags'):
+                if tag.get('objectType') == 'mention':
+                    translate(tag, 'url', translate_user_id)
 
         outer_obj = util.trim_nulls(outer_obj)
         if outer_obj.get('object', {}).keys() == {'id'}:

tests/test_ids.py

@@ -19,7 +19,7 @@ class IdsTest(TestCase):
         for from_, id, to, expected in [
             (ActivityPub, 'https://inst/user', ActivityPub, 'https://inst/user'),
             (ActivityPub, 'https://inst/user', ATProto, 'did:plc:456'),
-            (ActivityPub, 'https://inst/user', Fake, 'fake:https://inst/user'),
+            (ActivityPub, 'https://inst/user', Fake, 'fake:u:https://inst/user'),
             (ActivityPub, 'https://inst/user', Web, 'https://inst/user'),
             (ATProto, 'did:plc:123', Web, 'user.com'),
             (ATProto, 'did:plc:456', ActivityPub, 'https://inst/user'),
@@ -31,14 +31,14 @@ class IdsTest(TestCase):
             (Fake, 'fake:user', Web, 'fake:user'),
             (Web, 'user.com', ActivityPub, 'https://web.brid.gy/ap/user.com'),
             (Web, 'user.com', ATProto, 'did:plc:123'),
-            (Web, 'user.com', Fake, 'fake:user.com'),
+            (Web, 'user.com', Fake, 'fake:u:user.com'),
             (Web, 'user.com', Web, 'user.com'),
         ]:
             with self.subTest(from_=from_.LABEL, to=to.LABEL):
                 self.assertEqual(expected, translate_user_id(
                     id=id, from_proto=from_, to_proto=to))
 
-    def test_translate_user_id_no_atproto_did_stored(self):
+    def test_translate_user_id_no_copy_did_stored(self):
         for proto, id in [
             (Web, 'user.com'),
             (ActivityPub, 'https://instance/user'),
@@ -98,7 +98,7 @@ class IdsTest(TestCase):
         for from_, id, to, expected in [
             (ActivityPub, 'https://inst/post', ActivityPub, 'https://inst/post'),
             (ActivityPub, 'https://inst/post', ATProto, 'at://did/ap/post'),
-            (ActivityPub, 'https://inst/post', Fake, 'fake:ap:https://inst/post'),
+            (ActivityPub, 'https://inst/post', Fake, 'fake:o:ap:https://inst/post'),
             (ActivityPub, 'https://inst/post',
              Web, 'https://ap.brid.gy/convert/web/https:/inst/post'),
             (ATProto, 'at://did/web/post', Web, 'http://post'),
@@ -113,7 +113,7 @@ class IdsTest(TestCase):
             (Web, 'http://post',
              ActivityPub, 'https://web.brid.gy/convert/ap/http:/post'),
             (Web, 'http://post', ATProto, 'at://did/web/post'),
-            (Web, 'http://post', Fake, 'fake:web:http://post'),
+            (Web, 'http://post', Fake, 'fake:o:web:http://post'),
             (Web, 'http://post', Web, 'http://post'),
         ]:
             with self.subTest(from_=from_.LABEL, to=to.LABEL):

tests/test_protocol.py

@@ -424,11 +424,11 @@ class ProtocolTest(TestCase):
 
     def test_translate_ids_follow(self):
         self.assert_equals({
-            'id': 'other:fa:fake:follow',
+            'id': 'other:o:fa:fake:follow',
             'objectType': 'activity',
             'verb': 'follow',
-            'actor': 'other:fa:fake:alice',
-            'object': 'other:fa:fake:bob',
+            'actor': 'other:u:fake:alice',
+            'object': 'other:u:fake:bob',
         }, OtherFake.translate_ids({
             'id': 'fake:follow',
             'objectType': 'activity',
@@ -442,13 +442,13 @@ class ProtocolTest(TestCase):
             'objectType': 'activity',
             'verb': 'create',
             'object': {
-                'id': 'other:fa:fake:reply',
+                'id': 'other:o:fa:fake:reply',
                 'objectType': 'note',
-                'inReplyTo': 'other:fa:fake:post',
-                'author': 'other:fa:fake:alice',
+                'inReplyTo': 'other:o:fa:fake:post',
+                'author': 'other:u:fake:alice',
                 'tags': [{
                     'objectType': 'mention',
-                    'url': 'other:fa:fake:bob',
+                    'url': 'other:u:fake:bob',
                 }],
             },
         }, OtherFake.translate_ids({
@@ -475,10 +475,10 @@ class ProtocolTest(TestCase):
         self.assert_equals({
             'objectType': 'activity',
             'verb': 'create',
-            'actor': 'other:fa:fake:user',
+            'actor': 'other:user',
             'object': {
-                'id': 'other:fa:fake:reply',
-                'inReplyTo': 'other:fa:fake:post',
+                'id': 'other:o:fa:fake:reply',
+                'inReplyTo': 'other:post',
             },
         }, OtherFake.translate_ids({
             'objectType': 'activity',