ATProto.convert: fill in strongRef URIs with DIDs as well as CID

also error handling in ATProto.fetch for failed getRecord requests

atproto.py

@@ -20,6 +20,7 @@ from google.cloud import ndb
 from granary import as1, bluesky
 from lexrpc import Client
 import requests
+from requests import RequestException
 from oauth_dropins.webutil.appengine_info import DEBUG
 from oauth_dropins.webutil import util
 from oauth_dropins.webutil.util import json_dumps, json_loads
@@ -105,6 +106,8 @@ class ATProto(User, Protocol):
     def handle_to_id(cls, handle):
         assert cls.owns_handle(handle) is not False
 
+        # TODO: shortcut our own handles? eg snarfed.org.web.brid.gy
+
         user = ATProto.query(ATProto.handle == handle).get()
         if user:
             return user.key.id()
@@ -370,7 +373,7 @@ class ATProto(User, Protocol):
                 return False
             obj.key = ndb.Key(Object, id)
 
-        # at:// URI
+        # at:// URI. if it has a handle, resolve and replace with DID.
         # examples:
         # at://did:plc:s2koow7r6t7tozgd4slc3dsg/app.bsky.feed.post/3jqcpv7bv2c2q
         # https://bsky.social/xrpc/com.atproto.repo.getRecord?repo=did:plc:s2koow7r6t7tozgd4slc3dsg&collection=app.bsky.feed.post&rkey=3jqcpv7bv2c2q
@@ -378,12 +381,20 @@ class ATProto(User, Protocol):
         if not repo.startswith('did:'):
             handle = repo
             repo = cls.handle_to_id(repo)
+            if not repo:
+                return False
+            assert repo.startswith('did:')
             obj.key = ndb.Key(Object, id.replace(f'at://{handle}', f'at://{repo}'))
 
         client = Client(f'https://{os.environ["APPVIEW_HOST"]}',
                         headers={'User-Agent': USER_AGENT})
-        ret = client.com.atproto.repo.getRecord(
+        try:
-            repo=repo, collection=collection, rkey=rkey)
+            ret = client.com.atproto.repo.getRecord(
+                repo=repo, collection=collection, rkey=rkey)
+        except RequestException as e:
+            util.interpret_http_exception(e)
+            return False
+
         # TODO: verify sig?
         obj.bsky = {
             **ret['value'],
@@ -429,8 +440,12 @@ class ATProto(User, Protocol):
         # fill in CIDs from Objects
         def populate_cid(strong_ref):
             if uri := strong_ref.get('uri'):
+                # TODO: fail if this load fails? since we don't populate CID
                 if ref_obj := ATProto.load(uri):
-                    strong_ref['cid'] = ref_obj.bsky.get('cid')
+                    strong_ref.update({
+                        'cid': ref_obj.bsky.get('cid'),
+                        'uri': ref_obj.key.id(),
+                    })
 
         match ret.get('$type'):
             case 'app.bsky.feed.like' |  'app.bsky.feed.repost':

tests/test_atproto.py

@@ -266,10 +266,20 @@ class ATProtoTest(TestCase):
             },
         )
 
+    @patch('requests.get', return_value=requests_response({
+        'error':'InvalidRequest',
+        'message':'Could not locate record: at://did:plc:abc/app.bsky.feed.post/123',
+    }, status=400))
+    def test_fetch_at_uri_record_error(self, mock_get):
+        obj = Object(id='at://did:plc:abc/app.bsky.feed.post/123')
+        self.assertFalse(ATProto.fetch(obj))
+        mock_get.assert_called_once_with(
+            'https://api.bsky-sandbox.dev/xrpc/com.atproto.repo.getRecord?repo=did%3Aplc%3Aabc&collection=app.bsky.feed.post&rkey=123',
+            json=None, data=None, headers=ANY)
+
     @patch('dns.resolver.resolve', side_effect=dns.resolver.NXDOMAIN())
     @patch('requests.get', side_effect=[
         # resolving handle, HTTPS method
-
         requests_response('did:plc:abc', content_type='text/plain'),
         # AppView getRecord
         requests_response({
@@ -294,6 +304,12 @@ class ATProtoTest(TestCase):
             },
         )
 
+    @patch('dns.resolver.resolve', side_effect=dns.resolver.NXDOMAIN())
+    @patch('requests.get', return_value=requests_response(status=404))
+    def test_fetch_resolve_handle_fails(self, mock_get, _):
+        obj = Object(id='https://bsky.app/profile/bad.com/post/789')
+        self.assertFalse(ATProto.fetch(obj))
+
     def test_convert_bsky_pass_through(self):
         self.assertEqual({
             'foo': 'bar',
@@ -358,39 +374,58 @@ class ATProtoTest(TestCase):
             'inReplyTo': 'at://did:plc:bob/app.bsky.feed.post/tid',
         })))
 
-    @patch('requests.get', return_value=requests_response({
+    @patch('dns.resolver.resolve', side_effect=dns.resolver.NXDOMAIN())
-        'uri': 'at://did:plc:bob/app.bsky.feed.post/tid',
+    @patch('requests.get', side_effect=[
-        'cid': 'my sidd',
+        # resolving handle, HTTPS method
-        'value': {
+        requests_response('did:plc:user', content_type='text/plain'),
-            '$type': 'app.bsky.feed.post',
+        # AppView getRecord
-            'foo': 'bar',
+        requests_response({
-        },
+            'uri': 'at://did:plc:bob/app.bsky.feed.post/tid',
-    }))
+            'cid': 'my sidd',
-    def test_convert_populate_cid_fetch_remote_record(self, mock_get):
+            'value': {
-        self.store_object(id='did:plc:bob', raw={
+                '$type': 'app.bsky.feed.post',
-            **DID_DOC,
+                'foo': 'bar',
-            'id': 'did:plc:bob',
+            },
-        })
+        }),
+    ])
+    def test_convert_populate_cid_fetch_remote_record_handle(self, mock_get, _):
+        self.store_object(id='did:plc:user', raw=DID_DOC)
 
         self.assertEqual({
             '$type': 'app.bsky.feed.like',
             'subject': {
-                'uri': 'at://did:plc:bob/app.bsky.feed.post/tid',
+                'uri': 'at://did:plc:user/app.bsky.feed.post/tid',
                 'cid': 'my sidd',
             },
             'createdAt': '2022-01-02T03:04:05.000Z',
         }, ATProto.convert(Object(our_as1={
             'objectType': 'activity',
             'verb': 'like',
-            'object': 'at://did:plc:bob/app.bsky.feed.post/tid',
+            # handle here should be replaced with DID in returned record's URI
+            'object': 'at://han.dull/app.bsky.feed.post/tid',
         })))
         mock_get.assert_called_with(
-            'https://api.bsky-sandbox.dev/xrpc/com.atproto.repo.getRecord?repo=did%3Aplc%3Abob&collection=app.bsky.feed.post&rkey=tid',
+            'https://api.bsky-sandbox.dev/xrpc/com.atproto.repo.getRecord?repo=did%3Aplc%3Auser&collection=app.bsky.feed.post&rkey=tid',
-            json=None, data=None, headers={
+            json=None, data=None, headers=ANY)
-                'Content-Type': 'application/json',
+
-                'User-Agent': common.USER_AGENT,
+    @patch('dns.resolver.resolve', side_effect=dns.resolver.NXDOMAIN())
+    # resolving handle, HTTPS method
+    @patch('requests.get', return_value=requests_response(status=404))
+    def test_convert_populate_cid_fetch_remote_record_bad_handle(self, _, __):
+        # skips getRecord because handle didn't resolve
+        self.assertEqual({
+            '$type': 'app.bsky.feed.like',
+            'subject': {
+                # preserves handle here since it couldn't be resolved to a DID
+                'uri': 'at://bob.net/app.bsky.feed.post/tid',
+                'cid': '',
             },
-        )
+            'createdAt': '2022-01-02T03:04:05.000Z',
+        }, ATProto.convert(Object(our_as1={
+            'objectType': 'activity',
+            'verb': 'like',
+            'object': 'at://bob.net/app.bsky.feed.post/tid',
+        })))
 
     def test_convert_blobs_false(self):
         self.assertEqual({