From dc999bfdb862bec5bcab24010a652b02fb8b4a85 Mon Sep 17 00:00:00 2001
From: Ryan Barrett <git@ryanb.org>
Date: Fri, 18 Oct 2024 13:38:54 -0700
Subject: [PATCH] bluesky.from_as1: bug fix: handle attachments with no `id` or
 `url`

fixes https://console.cloud.google.com/errors/detail/CLOcmfD-4NSy1QE;locations=global;time=P30D?project=bridgy-federated
---
 README.md                     |  1 +
 granary/bluesky.py            | 11 ++++++-----
 granary/tests/test_bluesky.py | 15 +++++++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index 0baef1a9..7950c552 100644
--- a/README.md
+++ b/README.md
@@ -314,6 +314,7 @@ Changelog
     * When a `flag` has multiple objects, use the first one that's an ATProto record.
     * Handle URLs more carefully, don't add link facets with invalid `uri`s.
     * Bug fix: handle HTML links with `title` in `content` correctly.
+    * Bug fix: handle attachments with no `id` or `url`.
   * `to_as1`:
     * Extract links from `app.bsky.actor.profile#description` into `url`/`urls` fields
     * Bug fix: first URL (singular) goes in `url`, list of URLs goes in `urls`.
diff --git a/granary/bluesky.py b/granary/bluesky.py
index a159e0d4..d350dfd6 100644
--- a/granary/bluesky.py
+++ b/granary/bluesky.py
@@ -749,12 +749,13 @@ def from_as1(obj, out_type=None, blobs=None, client=None,
     record_embed = record_record_embed = external_embed = external_record_embed = None
 
     for att in attachments:
-      if not att.get('objectType') in ('article', 'link', 'note'):
+      att_id = att.get('id') or ''
+      att_url = as1.get_url(att) or ''
+      if not (att.get('objectType') in ('article', 'link', 'note')
+              and (att_url or att_id)):
         continue
 
-      id = att.get('id') or ''
-      att_url = att.get('url') or ''
-      if (id.startswith('at://') or id.startswith(Bluesky.BASE_URL) or
+      if (att_id.startswith('at://') or att_id.startswith(Bluesky.BASE_URL) or
           att_url.startswith('at://') or att_url.startswith(Bluesky.BASE_URL)):
         # quoted Bluesky post
         embed = from_as1(att, **kwargs).get('post') or {}
@@ -995,7 +996,7 @@ def from_as1(obj, out_type=None, blobs=None, client=None,
       'labels': labels,
     }
 
-    if as_embed or type == 'article':
+    if as_embed or (type == 'article' and url):
       # render articles with just an external link embed, no text
       ret.update({
         'text': '',
diff --git a/granary/tests/test_bluesky.py b/granary/tests/test_bluesky.py
index 96ad62e4..5cb88eee 100644
--- a/granary/tests/test_bluesky.py
+++ b/granary/tests/test_bluesky.py
@@ -1720,6 +1720,21 @@ def test_from_as1_article_to_embed_with_image_blobs(self):
       'image': NEW_BLOB_URL,
     }, blobs={NEW_BLOB_URL: BLOB}))
 
+  def test_from_as1_attachment_without_url_isnt_embed(self):
+    self.assertEqual({
+      '$type': 'app.bsky.feed.post',
+      'text': 'foo bar',
+      'createdAt': '2022-01-02T03:04:05.000Z',
+    }, from_as1({
+      'objectType': 'article',
+      'displayName': 'An article',
+      'content': 'foo bar',
+      'attachments': [{
+        'objectType': 'note',
+        'content': 'a citation? or footnote? ...'
+      }],
+    }))
+
   def test_from_as1_note_display_name_as_embed(self):
     self.assert_equals({
       '$type': 'app.bsky.feed.post',