diff --git a/atproto.py b/atproto.py index c790ea82..b36e5b59 100644 --- a/atproto.py +++ b/atproto.py @@ -578,9 +578,12 @@ def send(to_cls, obj, url, from_user=None, orig_obj_id=None): base_obj = obj base_obj_as1 = obj.as1 allow_opt_out = (type == 'delete') - if type in ('post', 'update', 'delete', 'undo'): + if type in as1.CRUD_VERBS: base_obj_as1 = as1.get_object(obj.as1) - base_id = base_obj_as1['id'] + base_id = base_obj_as1.get('id') + if not base_id: + logger.info(f'{type} object has no id!') + return False base_obj = PROTOCOLS[obj.source_protocol].load(base_id, remote=False) if type not in ('delete', 'undo'): if not base_obj: # probably a new repo diff --git a/protocol.py b/protocol.py index 7b19c147..f2e616db 100644 --- a/protocol.py +++ b/protocol.py @@ -917,6 +917,11 @@ def receive(from_cls, obj, authed_as=None, internal=False, received_at=None): obj.add('users', from_user.key) inner_obj_as1 = as1.get_object(obj.as1) + inner_obj_id = inner_obj_as1.get('id') + if obj.type in as1.CRUD_VERBS | set(('like', 'share')): + if not inner_obj_id: + error(f'{obj.type} object has no id!') + if obj.type in as1.CRUD_VERBS: if inner_owner := as1.get_owner(inner_obj_as1): if inner_owner_key := from_cls.key_for(inner_owner): @@ -926,7 +931,6 @@ def receive(from_cls, obj, authed_as=None, internal=False, received_at=None): obj.put() # store inner object - inner_obj_id = inner_obj_as1.get('id') if obj.type in ('post', 'update') and inner_obj_as1.keys() > set(['id']): Object.get_or_create(inner_obj_id, our_as1=inner_obj_as1, source_protocol=from_cls.LABEL, authed_as=actor) @@ -959,16 +963,10 @@ def receive(from_cls, obj, authed_as=None, internal=False, received_at=None): # TODO: do we convert stop-following to webmention 410 of original # follow? - elif obj.type in ('update', 'like', 'share'): # require object - if not inner_obj_id: - error("Couldn't find id of object to update") - # fall through to deliver to followers elif obj.type in ('delete', 'undo'): - if not inner_obj_id: - error("Couldn't find id of object to delete") - + assert inner_obj_id logger.info(f'Marking Object {inner_obj_id} deleted') Object.get_or_create(inner_obj_id, deleted=True, authed_as=authed_as) diff --git a/tests/test_atproto.py b/tests/test_atproto.py index 87dad606..549943f5 100644 --- a/tests/test_atproto.py +++ b/tests/test_atproto.py @@ -2320,6 +2320,21 @@ def test_send_chat_recipient_disabled(self, mock_get): 'https://chat.local/xrpc/chat.bsky.convo.getConvoForMembers?members=did%3Aplc%3Aalice', json=None, data=None, headers=ANY) + def test_send_object_without_id(self): + user = self.make_user_and_repo() + + # got an activity like this from Pandacap + # https://github.com/IsaacSchemm/Pandacap + dm = Object(id='fake:post', source_protocol='fake', our_as1={ + 'objectType': 'activity', + 'verb': 'post', + 'object': [{ + 'Item1': 'id', + 'Item2': 'https://pandacap.azurewebsites.net/#transient-abc-123', + }], + }) + self.assertFalse(ATProto.send(dm, 'https://bsky.brid.gy/')) + def test_datastore_client_get_record_datastore_object(self): self.make_user_and_repo() post = { diff --git a/tests/test_protocol.py b/tests/test_protocol.py index ed9c20d9..6ca848b4 100644 --- a/tests/test_protocol.py +++ b/tests/test_protocol.py @@ -982,6 +982,26 @@ def test_create_post(self): ('other:bob:target', obj.as1), ], OtherFake.sent) + def test_create_post_object_missing_id(self): + self.make_followers() + + # got an activity like this from Pandacap + # https://github.com/IsaacSchemm/Pandacap + create_as1 = { + 'id': 'fake:create', + 'objectType': 'activity', + 'verb': 'post', + 'actor': 'fake:user', + 'object': [{ + 'Item1': 'id', + 'Item2': 'https://pandacap.azurewebsites.net/#transient-abc-123', + }], + } + with self.assertRaises(ErrorButDoNotRetryTask): + Fake.receive_as1(create_as1) + + self.assertEqual([], OtherFake.sent) + def test_create_post_bare_object(self): self.make_followers() diff --git a/tests/test_web.py b/tests/test_web.py index bf45892f..cfff329c 100644 --- a/tests/test_web.py +++ b/tests/test_web.py @@ -1120,9 +1120,10 @@ def test_post_type_discovery_multiple_types(self, mock_get, mock_post): mock_get.return_value = requests_response( NOTE_HTML.replace('', """ - + + -"""), url='https://user.com/post') +""")) got = self.post('/queue/webmention', data={ 'source': 'https://user.com/multiple', @@ -1131,21 +1132,19 @@ def test_post_type_discovery_multiple_types(self, mock_get, mock_post): self.assertEqual(202, got.status_code) inboxes = ['https://inbox', 'https://public/inbox', 'https://shared/inbox'] - expected = { - **NOTE_AS2, - 'attributedTo': None, - 'type': 'Create', + self.assert_deliveries(mock_post, inboxes, { + '@context': ['https://www.w3.org/ns/activitystreams'], + 'type': 'Announce', + 'id': 'http://localhost/r/https://user.com/multiple', 'actor': 'http://localhost/user.com', - 'content': 'hello i am a post', - # TODO: this is an awkward wart left over from the multi-type mf2. - # remove it eventually. - 'object': { - 'targetUrl': 'http://bob.com/post', - 'to': ['https://www.w3.org/ns/activitystreams#Public'], - }, - } - del expected['contentMap'] - self.assert_deliveries(mock_post, inboxes, expected) + 'content': '

hello i am a post

', + 'contentMap': {'en': '

hello i am a post

'}, + 'name': 'hello i am a post', + 'object': 'http://localhost/r/http://bob.com/post', + 'to': ['https://www.w3.org/ns/activitystreams#Public'], + 'cc': ['http://localhost/user.com', + 'https://www.w3.org/ns/activitystreams#Public'], + }) def test_create_default_url_to_wm_source(self, mock_get, mock_post): """Source post has no u-url. AS2 id should default to webmention source.""" diff --git a/tests/testutil.py b/tests/testutil.py index f262b65e..78d56ce4 100644 --- a/tests/testutil.py +++ b/tests/testutil.py @@ -158,8 +158,11 @@ def send(to, obj, url, from_user=None, orig_obj_id=None): from_ = PROTOCOLS.get(obj.source_protocol) if (from_ and from_ != to and to.HAS_COPIES and obj.type not in ('update', 'delete')): + obj_id = as1.get_object(obj.as1).get('id') + if not obj_id: + return False if obj.type == 'post': - obj = Object.get_by_id(as1.get_object(obj.as1)['id']) + obj = Object.get_by_id(obj_id) copy_id = ids.translate_object_id( id=obj.key.id(), from_=from_, to=to) util.add(obj.copies, Target(uri=copy_id, protocol=to.LABEL))