From 641ac227866dcee0c740daaea9f73948b172ade1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 24 Nov 2020 16:26:28 -0800 Subject: [PATCH 1/3] remove outdated tests --- bookwyrm/activitypub/base_activity.py | 2 + .../tests/connectors/test_self_connector.py | 7 ++- bookwyrm/tests/test_remote_user.py | 43 ------------------- 3 files changed, 5 insertions(+), 47 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 62fce70b2..185fc775f 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -230,6 +230,8 @@ def image_attachments_formatter(images_json): caption = image.get('name') attachment = models.Attachment(caption=caption) image_field = image_formatter(image) + if not image_field: + continue attachment.image.save(*image_field, save=False) attachments.append(attachment) return attachments diff --git a/bookwyrm/tests/connectors/test_self_connector.py b/bookwyrm/tests/connectors/test_self_connector.py index b80ad202c..dd638137f 100644 --- a/bookwyrm/tests/connectors/test_self_connector.py +++ b/bookwyrm/tests/connectors/test_self_connector.py @@ -57,10 +57,9 @@ class SelfConnector(TestCase): def test_search_rank(self): results = self.connector.search('Anonymous') - self.assertEqual(len(results), 3) - self.assertEqual(results[0].title, 'Edition of Example Work') - self.assertEqual(results[1].title, 'More Editions') - self.assertEqual(results[2].title, 'Another Edition') + self.assertEqual(len(results), 2) + self.assertEqual(results[0].title, 'More Editions') + self.assertEqual(results[1].title, 'Edition of Example Work') def test_search_default_filter(self): diff --git a/bookwyrm/tests/test_remote_user.py b/bookwyrm/tests/test_remote_user.py index 3af8f59c0..febf9f67c 100644 --- a/bookwyrm/tests/test_remote_user.py +++ b/bookwyrm/tests/test_remote_user.py @@ -21,50 +21,7 @@ class RemoteUser(TestCase): self.user_data = json.loads(datafile.read_bytes()) - def test_get_remote_user(self): actor = 'https://example.com/users/rat' user = remote_user.get_or_create_remote_user(actor) self.assertEqual(user, self.remote_user) - - - def test_create_remote_user(self): - user = remote_user.create_remote_user(self.user_data) - self.assertFalse(user.local) - self.assertEqual(user.remote_id, 'https://example.com/user/mouse') - self.assertEqual(user.username, 'mouse@example.com') - self.assertEqual(user.name, 'MOUSE?? MOUSE!!') - self.assertEqual(user.inbox, 'https://example.com/user/mouse/inbox') - self.assertEqual(user.outbox, 'https://example.com/user/mouse/outbox') - self.assertEqual(user.shared_inbox, 'https://example.com/inbox') - self.assertEqual( - user.public_key, - self.user_data['publicKey']['publicKeyPem'] - ) - self.assertEqual(user.local, False) - self.assertEqual(user.bookwyrm_user, True) - self.assertEqual(user.manually_approves_followers, False) - - - def test_create_remote_user_missing_inbox(self): - del self.user_data['inbox'] - self.assertRaises( - TypeError, - remote_user.create_remote_user, - self.user_data - ) - - - def test_create_remote_user_missing_outbox(self): - del self.user_data['outbox'] - self.assertRaises( - TypeError, - remote_user.create_remote_user, - self.user_data - ) - - - def test_create_remote_user_default_fields(self): - del self.user_data['manuallyApprovesFollowers'] - user = remote_user.create_remote_user(self.user_data) - self.assertEqual(user.manually_approves_followers, False) From 9b57cfd331a898c9fb01eacdc008f59dc4211d93 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 25 Nov 2020 10:44:49 -0800 Subject: [PATCH 2/3] Fixes default lists on activitypub dataclasses --- bookwyrm/activitypub/book.py | 2 +- bookwyrm/activitypub/note.py | 4 ++-- bookwyrm/activitypub/person.py | 2 +- bookwyrm/tests/data/ap_quotation.json | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index 60d36bd02..bdc30edd2 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -25,7 +25,7 @@ class Book(ActivityObject): librarything_key: str goodreads_key: str - attachment: List[Image] = field(default=lambda: []) + attachment: List[Image] = field(default_factory=lambda: []) type: str = 'Book' diff --git a/bookwyrm/activitypub/note.py b/bookwyrm/activitypub/note.py index ebc0cf3ce..9eab952d3 100644 --- a/bookwyrm/activitypub/note.py +++ b/bookwyrm/activitypub/note.py @@ -24,8 +24,8 @@ class Note(ActivityObject): cc: List[str] content: str replies: Dict - tag: List[Link] = field(default=lambda: []) - attachment: List[Image] = field(default=lambda: []) + tag: List[Link] = field(default_factory=lambda: []) + attachment: List[Image] = field(default_factory=lambda: []) sensitive: bool = False type: str = 'Note' diff --git a/bookwyrm/activitypub/person.py b/bookwyrm/activitypub/person.py index 118774a27..324d68e32 100644 --- a/bookwyrm/activitypub/person.py +++ b/bookwyrm/activitypub/person.py @@ -15,7 +15,7 @@ class Person(ActivityObject): summary: str publicKey: PublicKey endpoints: Dict - icon: Image = field(default=lambda: {}) + icon: Image = field(default_factory=lambda: {}) bookwyrmUser: bool = False manuallyApprovesFollowers: str = False discoverable: str = True diff --git a/bookwyrm/tests/data/ap_quotation.json b/bookwyrm/tests/data/ap_quotation.json index 5085547a6..089bc85fd 100644 --- a/bookwyrm/tests/data/ap_quotation.json +++ b/bookwyrm/tests/data/ap_quotation.json @@ -19,8 +19,8 @@ "mediaType": "image//images/covers/2b4e4712-5a4d-4ac1-9df4-634cc9c7aff3jpg", "url": "https://example.com/images/covers/2b4e4712-5a4d-4ac1-9df4-634cc9c7aff3jpg", "name": "Cover of \"This Is How You Lose the Time War\"" - } - ], + } + ], "replies": { "id": "https://example.com/user/mouse/quotation/13/replies", "type": "Collection", From aed360d07ea9abd07887dbb13c8797302ccd0deb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 25 Nov 2020 11:15:14 -0800 Subject: [PATCH 3/3] Fixes serializer handling default dataclass fields --- bookwyrm/activitypub/base_activity.py | 11 ++++++++--- bookwyrm/models/base_model.py | 15 +++++++++++---- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 185fc775f..e8efeeac5 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -74,7 +74,8 @@ class ActivityObject: try: value = kwargs[field.name] except KeyError: - if field.default == MISSING: + if field.default == MISSING and \ + field.default_factory == MISSING: raise ActivitySerializerError(\ 'Missing required field: %s' % field.name) value = field.default @@ -143,6 +144,8 @@ class ActivityObject: # add images for (model_key, value) in image_fields.items(): + if not value: + continue getattr(instance, model_key).save(*value, save=True) # add one to many fields @@ -188,6 +191,8 @@ def resolve_foreign_key(model, remote_id): def tag_formatter(tags, tag_type): ''' helper function to extract foreign keys from tag activity json ''' + if not isinstance(tags, list): + return [] items = [] types = { 'Book': models.Book, @@ -207,9 +212,9 @@ def tag_formatter(tags, tag_type): def image_formatter(image_json): ''' helper function to load images and format them for a model ''' - url = image_json.get('url') - if not url: + if not image_json or not hasattr(image_json, 'url'): return None + url = image_json.get('url') try: response = requests.get(url) diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index e56d21f66..8c28c8abb 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -59,24 +59,31 @@ class ActivitypubMixin: def to_activity(self, pure=False): ''' convert from a model to an activity ''' if pure: + # works around bookwyrm-specific fields for vanilla AP services mappings = self.pure_activity_mappings else: + # may include custom fields that bookwyrm instances will understand mappings = self.activity_mappings fields = {} for mapping in mappings: if not hasattr(self, mapping.model_key) or not mapping.activity_key: + # this field on the model isn't serialized continue value = getattr(self, mapping.model_key) if hasattr(value, 'remote_id'): + # this is probably a foreign key field, which we want to + # serialize as just the remote_id url reference value = value.remote_id - if isinstance(value, datetime): + elif isinstance(value, datetime): value = value.isoformat() + + # run the custom formatter function set in the model result = mapping.activity_formatter(value) if mapping.activity_key in fields and \ isinstance(fields[mapping.activity_key], list): - # there are two database fields that map to the same AP list - # this happens in status, which combines user and book tags + # there can be two database fields that map to the same AP list + # this happens in status tags, which combines user and book tags fields[mapping.activity_key] += result else: fields[mapping.activity_key] = result @@ -265,7 +272,7 @@ def tag_formatter(items, name_field, activity_type): def image_formatter(image, default_path=None): ''' convert images into activitypub json ''' - if image: + if image and hasattr(image, 'url'): url = image.url elif default_path: url = default_path