From 847014720e06ca3ee120cc1645553cf127952e69 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 24 Nov 2020 16:05:00 -0800 Subject: [PATCH 01/32] Refactors bookwyrm connector to use activitypub serializer --- bookwyrm/activitypub/base_activity.py | 10 ++- bookwyrm/connectors/abstract_connector.py | 32 +++---- bookwyrm/connectors/bookwyrm_connector.py | 100 +++++----------------- bookwyrm/connectors/openlibrary.py | 3 +- bookwyrm/models/book.py | 4 +- 5 files changed, 48 insertions(+), 101 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 62fce70b2..d120e1115 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -143,7 +143,8 @@ class ActivityObject: # add images for (model_key, value) in image_fields.items(): - getattr(instance, model_key).save(*value, save=True) + if value: + getattr(instance, model_key).save(*value, save=True) # add one to many fields for (model_key, values) in one_to_many_fields.items(): @@ -207,6 +208,13 @@ def tag_formatter(tags, tag_type): def image_formatter(image_json): ''' helper function to load images and format them for a model ''' + if isinstance(image_json, list): + try: + image_json = image_json[0] + except IndexError: + return None + if not image_json: + return None url = image_json.get('url') if not url: return None diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 7fc4596b3..d709b075a 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -157,7 +157,7 @@ class AbstractConnector(ABC): def update_book_from_data(self, book, data, update_cover=True): ''' for creating a new book or syncing with data ''' - book = update_from_mappings(book, data, self.book_mappings) + book = self.update_from_mappings(book, data, self.book_mappings) author_text = [] for author in self.get_authors_from_data(data): @@ -262,23 +262,23 @@ class AbstractConnector(ABC): ''' get more info on a book ''' -def update_from_mappings(obj, data, mappings): - ''' assign data to model with mappings ''' - for mapping in mappings: - # check if this field is present in the data - value = data.get(mapping.remote_field) - if not value: - continue + def update_from_mappings(self, obj, data, mappings): + ''' assign data to model with mappings ''' + for mapping in mappings: + # check if this field is present in the data + value = data.get(mapping.remote_field) + if not value: + continue - # extract the value in the right format - try: - value = mapping.formatter(value) - except: - continue + # extract the value in the right format + try: + value = mapping.formatter(value) + except: + continue - # assign the formatted value to the model - obj.__setattr__(mapping.local_field, value) - return obj + # assign the formatted value to the model + obj.__setattr__(mapping.local_field, value) + return obj def get_date(date_string): diff --git a/bookwyrm/connectors/bookwyrm_connector.py b/bookwyrm/connectors/bookwyrm_connector.py index 6ed9dda1d..f1e539719 100644 --- a/bookwyrm/connectors/bookwyrm_connector.py +++ b/bookwyrm/connectors/bookwyrm_connector.py @@ -1,55 +1,22 @@ ''' using another bookwyrm instance as a source of book data ''' -from uuid import uuid4 - from django.core.exceptions import ObjectDoesNotExist -from django.core.files.base import ContentFile from django.db import transaction -import requests -from bookwyrm import models -from .abstract_connector import AbstractConnector, SearchResult, Mapping -from .abstract_connector import update_from_mappings, get_date, get_data +from bookwyrm import activitypub, models +from .abstract_connector import AbstractConnector, SearchResult +from .abstract_connector import get_data class Connector(AbstractConnector): ''' interact with other instances ''' - def __init__(self, identifier): - super().__init__(identifier) - self.key_mappings = [ - Mapping('isbn_13', model=models.Edition), - Mapping('isbn_10', model=models.Edition), - Mapping('lccn', model=models.Work), - Mapping('oclc_number', model=models.Edition), - Mapping('openlibrary_key'), - Mapping('goodreads_key'), - Mapping('asin'), - ] - self.book_mappings = self.key_mappings + [ - Mapping('sort_title'), - Mapping('subtitle'), - Mapping('description'), - Mapping('languages'), - Mapping('series'), - Mapping('series_number'), - Mapping('subjects'), - Mapping('subject_places'), - Mapping('first_published_date'), - Mapping('published_date'), - Mapping('pages'), - Mapping('physical_format'), - Mapping('publishers'), - ] - - self.author_mappings = [ - Mapping('name'), - Mapping('bio'), - Mapping('openlibrary_key'), - Mapping('wikipedia_link'), - Mapping('aliases'), - Mapping('born', formatter=get_date), - Mapping('died', formatter=get_date), - ] + def update_from_mappings(self, obj, data, mappings): + ''' serialize book data into a model ''' + if self.is_work_data(data): + work_data = activitypub.Work(**data) + return work_data.to_model(models.Work, instance=obj) + edition_data = activitypub.Edition(**data) + return edition_data.to_model(models.Edition, instance=obj) def get_remote_id_from_data(self, data): @@ -71,46 +38,19 @@ class Connector(AbstractConnector): def get_authors_from_data(self, data): - for author_url in data.get('authors', []): - yield self.get_or_create_author(author_url) + ''' load author data ''' + for author_id in data.get('authors', []): + try: + yield models.Author.objects.get(origin_id=author_id) + except models.Author.DoesNotExist: + continue + data = get_data(author_id) + author_data = activitypub.Author(**data) + yield author_data.to_model(models.Author) def get_cover_from_data(self, data): - cover_data = data.get('attachment') - if not cover_data: - return None - try: - cover_url = cover_data[0].get('url') - except IndexError: - return None - try: - response = requests.get(cover_url) - except ConnectionError: - return None - - if not response.ok: - return None - - image_name = str(uuid4()) + '.' + cover_url.split('.')[-1] - image_content = ContentFile(response.content) - return [image_name, image_content] - - - def get_or_create_author(self, remote_id): - ''' load that author ''' - try: - return models.Author.objects.get(origin_id=remote_id) - except ObjectDoesNotExist: - pass - - data = get_data(remote_id) - - # ingest a new author - author = models.Author(origin_id=remote_id) - author = update_from_mappings(author, data, self.author_mappings) - author.save() - - return author + pass def parse_search_data(self, data): diff --git a/bookwyrm/connectors/openlibrary.py b/bookwyrm/connectors/openlibrary.py index 5c26ad45b..5e18616d5 100644 --- a/bookwyrm/connectors/openlibrary.py +++ b/bookwyrm/connectors/openlibrary.py @@ -7,7 +7,6 @@ from django.core.files.base import ContentFile from bookwyrm import models from .abstract_connector import AbstractConnector, SearchResult, Mapping from .abstract_connector import ConnectorException -from .abstract_connector import update_from_mappings from .abstract_connector import get_date, get_data from .openlibrary_languages import languages @@ -185,7 +184,7 @@ class Connector(AbstractConnector): data = get_data(url) author = models.Author(openlibrary_key=olkey) - author = update_from_mappings(author, data, self.author_mappings) + author = self.update_from_mappings(author, data, self.author_mappings) name = data.get('name') # TODO this is making some BOLD assumption if name: diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index c8643f07e..642b5bfe0 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -102,7 +102,7 @@ class Book(ActivitypubMixin, BookWyrmModel): 'attachment', 'cover', # this expects an iterable and the field is just an image lambda x: image_attachments_formatter([x]), - lambda x: activitypub.image_attachments_formatter(x)[0] + activitypub.image_formatter ), ] @@ -190,7 +190,7 @@ class Edition(Book): if self.isbn_10 and not self.isbn_13: self.isbn_13 = isbn_10_to_13(self.isbn_10) - super().save(*args, **kwargs) + return super().save(*args, **kwargs) def isbn_10_to_13(isbn_10): From 641ac227866dcee0c740daaea9f73948b172ade1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 24 Nov 2020 16:26:28 -0800 Subject: [PATCH 02/32] 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 03/32] 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 04/32] 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 From 4f6ce0307d3171a30e1a9afde8d03b8ec89bc920 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 25 Nov 2020 11:24:36 -0800 Subject: [PATCH 05/32] fixes import in abstract connector unit test --- bookwyrm/tests/connectors/test_abstract_connector.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bookwyrm/tests/connectors/test_abstract_connector.py b/bookwyrm/tests/connectors/test_abstract_connector.py index f4af8a1ac..b207faf62 100644 --- a/bookwyrm/tests/connectors/test_abstract_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_connector.py @@ -2,8 +2,7 @@ from django.test import TestCase from bookwyrm import models -from bookwyrm.connectors.abstract_connector import Mapping,\ - update_from_mappings +from bookwyrm.connectors.abstract_connector import Mapping from bookwyrm.connectors.bookwyrm_connector import Connector @@ -79,7 +78,7 @@ class AbstractConnector(TestCase): Mapping('physical_format', remote_field='format'), Mapping('series', formatter=lambda x: x[0]), ] - book = update_from_mappings(self.book, data, mappings) + book = self.connector.update_from_mappings(self.book, data, mappings) self.assertEqual(book.title, 'Example Edition') self.assertEqual(book.isbn_10, '1234567890') self.assertEqual(book.isbn_13, None) From 9b79eb5fc3d12e0469dd1fed2419d8d889c21e92 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 25 Nov 2020 11:44:19 -0800 Subject: [PATCH 06/32] Removes outdated test of bookwyrm connector updater --- bookwyrm/connectors/bookwyrm_connector.py | 2 +- .../connectors/test_abstract_connector.py | 23 ------------------- bookwyrm/tests/data/fr_work.json | 4 +--- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/bookwyrm/connectors/bookwyrm_connector.py b/bookwyrm/connectors/bookwyrm_connector.py index f1e539719..78ae84c32 100644 --- a/bookwyrm/connectors/bookwyrm_connector.py +++ b/bookwyrm/connectors/bookwyrm_connector.py @@ -24,7 +24,7 @@ class Connector(AbstractConnector): def is_work_data(self, data): - return data['type'] == 'Work' + return data.get('type') == 'Work' def get_edition_from_work_data(self, data): diff --git a/bookwyrm/tests/connectors/test_abstract_connector.py b/bookwyrm/tests/connectors/test_abstract_connector.py index b207faf62..c01b00dba 100644 --- a/bookwyrm/tests/connectors/test_abstract_connector.py +++ b/bookwyrm/tests/connectors/test_abstract_connector.py @@ -63,29 +63,6 @@ class AbstractConnector(TestCase): self.assertEqual(mapping.formatter('bb'), 'aabb') - def test_update_from_mappings(self): - data = { - 'title': 'Unused title', - 'isbn_10': '1234567890', - 'isbn_13': 'blahhh', - 'blah': 'bip', - 'format': 'hardcover', - 'series': ['one', 'two'], - } - mappings = [ - Mapping('isbn_10'), - Mapping('blah'),# not present on self.book - Mapping('physical_format', remote_field='format'), - Mapping('series', formatter=lambda x: x[0]), - ] - book = self.connector.update_from_mappings(self.book, data, mappings) - self.assertEqual(book.title, 'Example Edition') - self.assertEqual(book.isbn_10, '1234567890') - self.assertEqual(book.isbn_13, None) - self.assertEqual(book.physical_format, 'hardcover') - self.assertEqual(book.series, 'one') - - def test_match_from_mappings(self): edition = models.Edition.objects.create( title='Blah', diff --git a/bookwyrm/tests/data/fr_work.json b/bookwyrm/tests/data/fr_work.json index 4e759a194..3a36fc640 100644 --- a/bookwyrm/tests/data/fr_work.json +++ b/bookwyrm/tests/data/fr_work.json @@ -28,9 +28,7 @@ ], "lccn": null, "editions": [ - "https://bookwyrm.social/book/5989", - "OL28439584M", - "OL28300471M" + "https://bookwyrm.social/book/5989" ], "@context": "https://www.w3.org/ns/activitystreams" } From efa5d5ef2cac7510ae1e2b743124634939643dce Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 11:37:06 -0800 Subject: [PATCH 07/32] Adds ci test runner --- .github/workflows/django-tests.yml | 68 ++++++++++++++++++++++++++++++ bw-dev | 3 +- 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/django-tests.yml diff --git a/.github/workflows/django-tests.yml b/.github/workflows/django-tests.yml new file mode 100644 index 000000000..3ce368ecd --- /dev/null +++ b/.github/workflows/django-tests.yml @@ -0,0 +1,68 @@ +name: Run Python Tests +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + build: + + runs-on: ubuntu-20.04 + strategy: + max-parallel: 4 + matrix: + db: [postgres] + python-version: [3.9] + include: + - db: postgres + db_port: 5432 + + services: + postgres: + image: postgres:10 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: hunter2 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + steps: + - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Install Dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + - name: Run Tests + env: + DB: ${{ matrix.db }} + DB_HOST: 127.0.0.1 + DB_PORT: ${{ matrix.db_port }} + DB_PASSWORD: hunter2 + SECRET_KEY: beepbeep + DEBUG: true + DOMAIN: your.domain.here + OL_URL: https://openlibrary.org + BOOKWYRM_DATABASE_BACKEND: postgres + MEDIA_ROOT: images/ + POSTGRES_PASSWORD: hunter2 + POSTGRES_USER: postgres + POSTGRES_DB: github_actions + POSTGRES_HOST: 127.0.0.1 + CELERY_BROKER: "" + CELERY_RESULT_BACKEND: "" + EMAIL_HOST: "smtp.mailgun.org" + EMAIL_PORT: 587 + EMAIL_HOST_USER: "" + EMAIL_HOST_PASSWORD: "" + EMAIL_USE_TLS: true + run: | + python manage.py test diff --git a/bw-dev b/bw-dev index 6d74924ff..53c8e52d9 100755 --- a/bw-dev +++ b/bw-dev @@ -61,7 +61,8 @@ case "$1" in ;; migrate) execweb python manage.py rename_app fedireads bookwyrm - execweb python manage.py "$@" + shift 1 + execweb python manage.py migrate "$@" ;; bash) execweb bash From 257a29dcfdc2576f6d19d623f29cf0d7aafdee72 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 11:53:30 -0800 Subject: [PATCH 08/32] Comment out failing tests Obviously this is not a SOLUTION, it's an intermediary step in resolving the redis dependency issues. this PR isn't mergable until the tests are restored. --- bookwyrm/tests/incoming/test_follow.py | 46 +++---- bookwyrm/tests/outgoing/test_follow.py | 102 +++++++-------- bookwyrm/tests/outgoing/test_shelving.py | 72 +++++------ bookwyrm/tests/test_signing.py | 156 +++++++++++------------ 4 files changed, 188 insertions(+), 188 deletions(-) diff --git a/bookwyrm/tests/incoming/test_follow.py b/bookwyrm/tests/incoming/test_follow.py index 51ab3c43a..377c70205 100644 --- a/bookwyrm/tests/incoming/test_follow.py +++ b/bookwyrm/tests/incoming/test_follow.py @@ -18,29 +18,29 @@ class IncomingFollow(TestCase): self.local_user.save() - def test_handle_follow(self): - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/users/rat", - "object": "http://local.com/user/mouse" - } - - incoming.handle_follow(activity) - - # notification created - notification = models.Notification.objects.get() - self.assertEqual(notification.user, self.local_user) - self.assertEqual(notification.notification_type, 'FOLLOW') - - # the request should have been deleted - requests = models.UserFollowRequest.objects.all() - self.assertEqual(list(requests), []) - - # the follow relationship should exist - follow = models.UserFollows.objects.get(user_object=self.local_user) - self.assertEqual(follow.user_subject, self.remote_user) +# def test_handle_follow(self): +# activity = { +# "@context": "https://www.w3.org/ns/activitystreams", +# "id": "https://example.com/users/rat/follows/123", +# "type": "Follow", +# "actor": "https://example.com/users/rat", +# "object": "http://local.com/user/mouse" +# } +# +# incoming.handle_follow(activity) +# +# # notification created +# notification = models.Notification.objects.get() +# self.assertEqual(notification.user, self.local_user) +# self.assertEqual(notification.notification_type, 'FOLLOW') +# +# # the request should have been deleted +# requests = models.UserFollowRequest.objects.all() +# self.assertEqual(list(requests), []) +# +# # the follow relationship should exist +# follow = models.UserFollows.objects.get(user_object=self.local_user) +# self.assertEqual(follow.user_subject, self.remote_user) def test_handle_follow_manually_approved(self): diff --git a/bookwyrm/tests/outgoing/test_follow.py b/bookwyrm/tests/outgoing/test_follow.py index 82a476f62..a1ea3f690 100644 --- a/bookwyrm/tests/outgoing/test_follow.py +++ b/bookwyrm/tests/outgoing/test_follow.py @@ -19,54 +19,54 @@ class Following(TestCase): ) - def test_handle_follow(self): - self.assertEqual(models.UserFollowRequest.objects.count(), 0) - - outgoing.handle_follow(self.local_user, self.remote_user) - rel = models.UserFollowRequest.objects.get() - - self.assertEqual(rel.user_subject, self.local_user) - self.assertEqual(rel.user_object, self.remote_user) - self.assertEqual(rel.status, 'follow_request') - - - def test_handle_unfollow(self): - self.remote_user.followers.add(self.local_user) - self.assertEqual(self.remote_user.followers.count(), 1) - outgoing.handle_unfollow(self.local_user, self.remote_user) - - self.assertEqual(self.remote_user.followers.count(), 0) - - - def test_handle_accept(self): - rel = models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - rel_id = rel.id - - outgoing.handle_accept(rel) - # request should be deleted - self.assertEqual( - models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 - ) - # follow relationship should exist - self.assertEqual(self.remote_user.followers.first(), self.local_user) - - - def test_handle_reject(self): - rel = models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - rel_id = rel.id - - outgoing.handle_reject(rel) - # request should be deleted - self.assertEqual( - models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 - ) - # follow relationship should not exist - self.assertEqual( - models.UserFollows.objects.filter(id=rel_id).count(), 0 - ) +# def test_handle_follow(self): +# self.assertEqual(models.UserFollowRequest.objects.count(), 0) +# +# outgoing.handle_follow(self.local_user, self.remote_user) +# rel = models.UserFollowRequest.objects.get() +# +# self.assertEqual(rel.user_subject, self.local_user) +# self.assertEqual(rel.user_object, self.remote_user) +# self.assertEqual(rel.status, 'follow_request') +# +# +# def test_handle_unfollow(self): +# self.remote_user.followers.add(self.local_user) +# self.assertEqual(self.remote_user.followers.count(), 1) +# outgoing.handle_unfollow(self.local_user, self.remote_user) +# +# self.assertEqual(self.remote_user.followers.count(), 0) +# +# +# def test_handle_accept(self): +# rel = models.UserFollowRequest.objects.create( +# user_subject=self.local_user, +# user_object=self.remote_user +# ) +# rel_id = rel.id +# +# outgoing.handle_accept(rel) +# # request should be deleted +# self.assertEqual( +# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 +# ) +# # follow relationship should exist +# self.assertEqual(self.remote_user.followers.first(), self.local_user) +# +# +# def test_handle_reject(self): +# rel = models.UserFollowRequest.objects.create( +# user_subject=self.local_user, +# user_object=self.remote_user +# ) +# rel_id = rel.id +# +# outgoing.handle_reject(rel) +# # request should be deleted +# self.assertEqual( +# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 +# ) +# # follow relationship should not exist +# self.assertEqual( +# models.UserFollows.objects.filter(id=rel_id).count(), 0 +# ) diff --git a/bookwyrm/tests/outgoing/test_shelving.py b/bookwyrm/tests/outgoing/test_shelving.py index 0b85b671b..c26bec84f 100644 --- a/bookwyrm/tests/outgoing/test_shelving.py +++ b/bookwyrm/tests/outgoing/test_shelving.py @@ -25,39 +25,39 @@ class Shelving(TestCase): ) - def test_handle_shelve(self): - outgoing.handle_shelve(self.user, self.book, self.shelf) - # make sure the book is on the shelf - self.assertEqual(self.shelf.books.get(), self.book) - - - def test_handle_shelve_to_read(self): - shelf = models.Shelf.objects.get(identifier='to-read') - - outgoing.handle_shelve(self.user, self.book, shelf) - # make sure the book is on the shelf - self.assertEqual(shelf.books.get(), self.book) - - - def test_handle_shelve_reading(self): - shelf = models.Shelf.objects.get(identifier='reading') - - outgoing.handle_shelve(self.user, self.book, shelf) - # make sure the book is on the shelf - self.assertEqual(shelf.books.get(), self.book) - - - def test_handle_shelve_read(self): - shelf = models.Shelf.objects.get(identifier='read') - - outgoing.handle_shelve(self.user, self.book, shelf) - # make sure the book is on the shelf - self.assertEqual(shelf.books.get(), self.book) - - - def test_handle_unshelve(self): - self.shelf.books.add(self.book) - self.shelf.save() - self.assertEqual(self.shelf.books.count(), 1) - outgoing.handle_unshelve(self.user, self.book, self.shelf) - self.assertEqual(self.shelf.books.count(), 0) +# def test_handle_shelve(self): +# outgoing.handle_shelve(self.user, self.book, self.shelf) +# # make sure the book is on the shelf +# self.assertEqual(self.shelf.books.get(), self.book) +# +# +# def test_handle_shelve_to_read(self): +# shelf = models.Shelf.objects.get(identifier='to-read') +# +# outgoing.handle_shelve(self.user, self.book, shelf) +# # make sure the book is on the shelf +# self.assertEqual(shelf.books.get(), self.book) +# +# +# def test_handle_shelve_reading(self): +# shelf = models.Shelf.objects.get(identifier='reading') +# +# outgoing.handle_shelve(self.user, self.book, shelf) +# # make sure the book is on the shelf +# self.assertEqual(shelf.books.get(), self.book) +# +# +# def test_handle_shelve_read(self): +# shelf = models.Shelf.objects.get(identifier='read') +# +# outgoing.handle_shelve(self.user, self.book, shelf) +# # make sure the book is on the shelf +# self.assertEqual(shelf.books.get(), self.book) +# +# +# def test_handle_unshelve(self): +# self.shelf.books.add(self.book) +# self.shelf.save() +# self.assertEqual(self.shelf.books.count(), 1) +# outgoing.handle_unshelve(self.user, self.book, self.shelf) +# self.assertEqual(self.shelf.books.count(), 0) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 5170b4167..bdd82585a 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -70,9 +70,9 @@ class Signature(TestCase): signer or sender, self.rat.inbox, now, digest) return self.send(signature, now, send_data or data, digest) - def test_correct_signature(self): - response = self.send_test_request(sender=self.mouse) - self.assertEqual(response.status_code, 200) +# def test_correct_signature(self): +# response = self.send_test_request(sender=self.mouse) +# self.assertEqual(response.status_code, 200) def test_wrong_signature(self): ''' Messages must be signed by the right actor. @@ -80,82 +80,82 @@ class Signature(TestCase): response = self.send_test_request(sender=self.mouse, signer=self.cat) self.assertEqual(response.status_code, 401) - @responses.activate - def test_remote_signer(self): - ''' signtures for remote users ''' - datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') - data = json.loads(datafile.read_bytes()) - data['id'] = self.fake_remote.remote_id - data['publicKey']['publicKeyPem'] = self.fake_remote.public_key - del data['icon'] # Avoid having to return an avatar. - responses.add( - responses.GET, - self.fake_remote.remote_id, - json=data, - status=200) - responses.add( - responses.GET, - 'https://localhost/.well-known/nodeinfo', - status=404) - responses.add( - responses.GET, - 'https://example.com/user/mouse/outbox?page=true', - json={'orderedItems': []}, - status=200 - ) +# @responses.activate +# def test_remote_signer(self): +# ''' signtures for remote users ''' +# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') +# data = json.loads(datafile.read_bytes()) +# data['id'] = self.fake_remote.remote_id +# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key +# del data['icon'] # Avoid having to return an avatar. +# responses.add( +# responses.GET, +# self.fake_remote.remote_id, +# json=data, +# status=200) +# responses.add( +# responses.GET, +# 'https://localhost/.well-known/nodeinfo', +# status=404) +# responses.add( +# responses.GET, +# 'https://example.com/user/mouse/outbox?page=true', +# json={'orderedItems': []}, +# status=200 +# ) +# +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 200) - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) - - @responses.activate - def test_key_needs_refresh(self): - datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') - data = json.loads(datafile.read_bytes()) - data['id'] = self.fake_remote.remote_id - data['publicKey']['publicKeyPem'] = self.fake_remote.public_key - del data['icon'] # Avoid having to return an avatar. - responses.add( - responses.GET, - self.fake_remote.remote_id, - json=data, - status=200) - responses.add( - responses.GET, - 'https://localhost/.well-known/nodeinfo', - status=404) - responses.add( - responses.GET, - 'https://example.com/user/mouse/outbox?page=true', - json={'orderedItems': []}, - status=200 - ) - - # Second and subsequent fetches get a different key: - new_private_key, new_public_key = create_key_pair() - new_sender = Sender( - self.fake_remote.remote_id, new_private_key, new_public_key) - data['publicKey']['publicKeyPem'] = new_public_key - responses.add( - responses.GET, - self.fake_remote.remote_id, - json=data, - status=200) - - # Key correct: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) - - # Old key is cached, so still works: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 200) - - # Try with new key: - response = self.send_test_request(sender=new_sender) - self.assertEqual(response.status_code, 200) - - # Now the old key will fail: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 401) +# @responses.activate +# def test_key_needs_refresh(self): +# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') +# data = json.loads(datafile.read_bytes()) +# data['id'] = self.fake_remote.remote_id +# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key +# del data['icon'] # Avoid having to return an avatar. +# responses.add( +# responses.GET, +# self.fake_remote.remote_id, +# json=data, +# status=200) +# responses.add( +# responses.GET, +# 'https://localhost/.well-known/nodeinfo', +# status=404) +# responses.add( +# responses.GET, +# 'https://example.com/user/mouse/outbox?page=true', +# json={'orderedItems': []}, +# status=200 +# ) +# +# # Second and subsequent fetches get a different key: +# new_private_key, new_public_key = create_key_pair() +# new_sender = Sender( +# self.fake_remote.remote_id, new_private_key, new_public_key) +# data['publicKey']['publicKeyPem'] = new_public_key +# responses.add( +# responses.GET, +# self.fake_remote.remote_id, +# json=data, +# status=200) +# +# # Key correct: +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 200) +# +# # Old key is cached, so still works: +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 200) +# +# # Try with new key: +# response = self.send_test_request(sender=new_sender) +# self.assertEqual(response.status_code, 200) +# +# # Now the old key will fail: +# response = self.send_test_request(sender=self.fake_remote) +# self.assertEqual(response.status_code, 401) @responses.activate From 44a0ef3b0b6775eadb0e2632fc7e37c7271adc5c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 12:25:01 -0800 Subject: [PATCH 09/32] Fixes nondeterministic order of query causing test failure --- bookwyrm/tests/models/test_user_model.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 203c558d7..611e1fd68 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -27,7 +27,9 @@ class User(TestCase): shelves = models.Shelf.objects.filter(user=self.user).all() self.assertEqual(len(shelves), 3) names = [s.name for s in shelves] - self.assertEqual(names, ['To Read', 'Currently Reading', 'Read']) + self.assertTrue('To Read' in names) + self.assertTrue('Currently Reading' in names) + self.assertTrue('Read' in names) ids = [s.identifier for s in shelves] self.assertEqual(ids, ['to-read', 'reading', 'read']) From 0c01af4042f13913e7c3b4ac8ba79fbc33f5af4a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 12:42:01 -0800 Subject: [PATCH 10/32] Another nondeterministic list order problem --- bookwyrm/tests/models/test_user_model.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 611e1fd68..e82f91be1 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -31,7 +31,9 @@ class User(TestCase): self.assertTrue('Currently Reading' in names) self.assertTrue('Read' in names) ids = [s.identifier for s in shelves] - self.assertEqual(ids, ['to-read', 'reading', 'read']) + self.assertTrue('to-read' in ids) + self.assertTrue('reading' in ids) + self.assertTrue('read' in ids) def test_activitypub_serialize(self): From 48ab993861ddb400338e2c46f8d14ecfc719ddbc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:02:26 -0800 Subject: [PATCH 11/32] Mocks celery task for follow request --- bookwyrm/tests/test_signing.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index bdd82585a..35c68cf5c 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -2,6 +2,7 @@ import time from collections import namedtuple from urllib.parse import urlsplit import pathlib +from unittest.mock import patch import json import responses @@ -63,16 +64,18 @@ class Signature(TestCase): send_data=None, digest=None, date=None): + ''' sends a follow request to the "rat" user ''' now = date or http_date() data = json.dumps(get_follow_data(sender, self.rat)) digest = digest or make_digest(data) signature = make_signature( signer or sender, self.rat.inbox, now, digest) - return self.send(signature, now, send_data or data, digest) + with patch('bookwyrm.incoming.handle_follow.delay') as _: + return self.send(signature, now, send_data or data, digest) -# def test_correct_signature(self): -# response = self.send_test_request(sender=self.mouse) -# self.assertEqual(response.status_code, 200) + def test_correct_signature(self): + response = self.send_test_request(sender=self.mouse) + self.assertEqual(response.status_code, 200) def test_wrong_signature(self): ''' Messages must be signed by the right actor. From 73279d65d7af633c8a3674a1006123e06994cfdf Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:08:01 -0800 Subject: [PATCH 12/32] Fix test remote signer and comment out failing tests --- bookwyrm/tests/test_signing.py | 95 +++++++++++++++++----------------- 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 35c68cf5c..d57a34392 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -83,32 +83,33 @@ class Signature(TestCase): response = self.send_test_request(sender=self.mouse, signer=self.cat) self.assertEqual(response.status_code, 401) -# @responses.activate -# def test_remote_signer(self): -# ''' signtures for remote users ''' -# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') -# data = json.loads(datafile.read_bytes()) -# data['id'] = self.fake_remote.remote_id -# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key -# del data['icon'] # Avoid having to return an avatar. -# responses.add( -# responses.GET, -# self.fake_remote.remote_id, -# json=data, -# status=200) -# responses.add( -# responses.GET, -# 'https://localhost/.well-known/nodeinfo', -# status=404) -# responses.add( -# responses.GET, -# 'https://example.com/user/mouse/outbox?page=true', -# json={'orderedItems': []}, -# status=200 -# ) -# -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 200) + @responses.activate + def test_remote_signer(self): + ''' signtures for remote users ''' + datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') + data = json.loads(datafile.read_bytes()) + data['id'] = self.fake_remote.remote_id + data['publicKey']['publicKeyPem'] = self.fake_remote.public_key + del data['icon'] # Avoid having to return an avatar. + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + responses.add( + responses.GET, + 'https://localhost/.well-known/nodeinfo', + status=404) + responses.add( + responses.GET, + 'https://example.com/user/mouse/outbox?page=true', + json={'orderedItems': []}, + status=200 + ) + + with patch('bookwyrm.remote_user.get_remote_reviews.delay') as _: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) # @responses.activate # def test_key_needs_refresh(self): @@ -172,26 +173,26 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 401) - @pytest.mark.integration - def test_changed_data(self): - '''Message data must match the digest header.''' - response = self.send_test_request( - self.mouse, - send_data=get_follow_data(self.mouse, self.cat)) - self.assertEqual(response.status_code, 401) +# @pytest.mark.integration +# def test_changed_data(self): +# '''Message data must match the digest header.''' +# response = self.send_test_request( +# self.mouse, +# send_data=get_follow_data(self.mouse, self.cat)) +# self.assertEqual(response.status_code, 401) - @pytest.mark.integration - def test_invalid_digest(self): - response = self.send_test_request( - self.mouse, - digest='SHA-256=AAAAAAAAAAAAAAAAAA') - self.assertEqual(response.status_code, 401) +# @pytest.mark.integration +# def test_invalid_digest(self): +# response = self.send_test_request( +# self.mouse, +# digest='SHA-256=AAAAAAAAAAAAAAAAAA') +# self.assertEqual(response.status_code, 401) - @pytest.mark.integration - def test_old_message(self): - '''Old messages should be rejected to prevent replay attacks.''' - response = self.send_test_request( - self.mouse, - date=http_date(time.time() - 301) - ) - self.assertEqual(response.status_code, 401) +# @pytest.mark.integration +# def test_old_message(self): +# '''Old messages should be rejected to prevent replay attacks.''' +# response = self.send_test_request( +# self.mouse, +# date=http_date(time.time() - 301) +# ) +# self.assertEqual(response.status_code, 401) From 829615cdd729ac2537d216fb35cee3046faf1047 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:18:10 -0800 Subject: [PATCH 13/32] Fixes celery mocks on more signature unit tests --- bookwyrm/tests/test_signing.py | 113 +++++++++++++++++---------------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index d57a34392..05cb8ea09 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -111,55 +111,56 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 200) -# @responses.activate -# def test_key_needs_refresh(self): -# datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') -# data = json.loads(datafile.read_bytes()) -# data['id'] = self.fake_remote.remote_id -# data['publicKey']['publicKeyPem'] = self.fake_remote.public_key -# del data['icon'] # Avoid having to return an avatar. -# responses.add( -# responses.GET, -# self.fake_remote.remote_id, -# json=data, -# status=200) -# responses.add( -# responses.GET, -# 'https://localhost/.well-known/nodeinfo', -# status=404) -# responses.add( -# responses.GET, -# 'https://example.com/user/mouse/outbox?page=true', -# json={'orderedItems': []}, -# status=200 -# ) -# -# # Second and subsequent fetches get a different key: -# new_private_key, new_public_key = create_key_pair() -# new_sender = Sender( -# self.fake_remote.remote_id, new_private_key, new_public_key) -# data['publicKey']['publicKeyPem'] = new_public_key -# responses.add( -# responses.GET, -# self.fake_remote.remote_id, -# json=data, -# status=200) -# -# # Key correct: -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 200) -# -# # Old key is cached, so still works: -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 200) -# -# # Try with new key: -# response = self.send_test_request(sender=new_sender) -# self.assertEqual(response.status_code, 200) -# -# # Now the old key will fail: -# response = self.send_test_request(sender=self.fake_remote) -# self.assertEqual(response.status_code, 401) + @responses.activate + def test_key_needs_refresh(self): + datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') + data = json.loads(datafile.read_bytes()) + data['id'] = self.fake_remote.remote_id + data['publicKey']['publicKeyPem'] = self.fake_remote.public_key + del data['icon'] # Avoid having to return an avatar. + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + responses.add( + responses.GET, + 'https://localhost/.well-known/nodeinfo', + status=404) + responses.add( + responses.GET, + 'https://example.com/user/mouse/outbox?page=true', + json={'orderedItems': []}, + status=200 + ) + + # Second and subsequent fetches get a different key: + new_private_key, new_public_key = create_key_pair() + new_sender = Sender( + self.fake_remote.remote_id, new_private_key, new_public_key) + data['publicKey']['publicKeyPem'] = new_public_key + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + + with patch('bookwyrm.remote_user.get_remote_reviews.delay') as _: + # Key correct: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) + + # Old key is cached, so still works: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) + + # Try with new key: + response = self.send_test_request(sender=new_sender) + self.assertEqual(response.status_code, 200) + + # Now the old key will fail: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 401) @responses.activate @@ -173,13 +174,13 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 401) -# @pytest.mark.integration -# def test_changed_data(self): -# '''Message data must match the digest header.''' -# response = self.send_test_request( -# self.mouse, -# send_data=get_follow_data(self.mouse, self.cat)) -# self.assertEqual(response.status_code, 401) + @pytest.mark.integration + def test_changed_data(self): + '''Message data must match the digest header.''' + response = self.send_test_request( + self.mouse, + send_data=get_follow_data(self.mouse, self.cat)) + self.assertEqual(response.status_code, 401) # @pytest.mark.integration # def test_invalid_digest(self): From a8f3ddec05db7c9ee766a4e670fb2843d1688424 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:39:33 -0800 Subject: [PATCH 14/32] Trying to avoid issues from execusing http requests --- bookwyrm/tests/test_signing.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 05cb8ea09..817e55890 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -155,8 +155,8 @@ class Signature(TestCase): self.assertEqual(response.status_code, 200) # Try with new key: - response = self.send_test_request(sender=new_sender) - self.assertEqual(response.status_code, 200) +# response = self.send_test_request(sender=new_sender) +# self.assertEqual(response.status_code, 200) # Now the old key will fail: response = self.send_test_request(sender=self.fake_remote) @@ -177,17 +177,19 @@ class Signature(TestCase): @pytest.mark.integration def test_changed_data(self): '''Message data must match the digest header.''' - response = self.send_test_request( - self.mouse, - send_data=get_follow_data(self.mouse, self.cat)) - self.assertEqual(response.status_code, 401) + with patch('bookwyrm.remote_user.refresh_remote_user') as _: + response = self.send_test_request( + self.mouse, + send_data=get_follow_data(self.mouse, self.cat)) + self.assertEqual(response.status_code, 401) -# @pytest.mark.integration -# def test_invalid_digest(self): -# response = self.send_test_request( -# self.mouse, -# digest='SHA-256=AAAAAAAAAAAAAAAAAA') -# self.assertEqual(response.status_code, 401) + @pytest.mark.integration + def test_invalid_digest(self): + with patch('bookwyrm.remote_user.refresh_remote_user') as _: + response = self.send_test_request( + self.mouse, + digest='SHA-256=AAAAAAAAAAAAAAAAAA') + self.assertEqual(response.status_code, 401) # @pytest.mark.integration # def test_old_message(self): From f173d674ac383b677843bfeacef8d81ae864f42c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 13:53:38 -0800 Subject: [PATCH 15/32] Mock fetch_user function which makes http request --- bookwyrm/tests/test_signing.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 817e55890..e393062ed 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -155,12 +155,12 @@ class Signature(TestCase): self.assertEqual(response.status_code, 200) # Try with new key: -# response = self.send_test_request(sender=new_sender) -# self.assertEqual(response.status_code, 200) + #response = self.send_test_request(sender=new_sender) + #self.assertEqual(response.status_code, 200) # Now the old key will fail: - response = self.send_test_request(sender=self.fake_remote) - self.assertEqual(response.status_code, 401) + #response = self.send_test_request(sender=self.fake_remote) + #self.assertEqual(response.status_code, 401) @responses.activate @@ -177,7 +177,7 @@ class Signature(TestCase): @pytest.mark.integration def test_changed_data(self): '''Message data must match the digest header.''' - with patch('bookwyrm.remote_user.refresh_remote_user') as _: + with patch('bookwyrm.remote_user.fetch_user_data') as _: response = self.send_test_request( self.mouse, send_data=get_follow_data(self.mouse, self.cat)) @@ -185,17 +185,18 @@ class Signature(TestCase): @pytest.mark.integration def test_invalid_digest(self): - with patch('bookwyrm.remote_user.refresh_remote_user') as _: + with patch('bookwyrm.remote_user.fetch_user_data') as _: response = self.send_test_request( self.mouse, digest='SHA-256=AAAAAAAAAAAAAAAAAA') self.assertEqual(response.status_code, 401) -# @pytest.mark.integration -# def test_old_message(self): -# '''Old messages should be rejected to prevent replay attacks.''' -# response = self.send_test_request( -# self.mouse, -# date=http_date(time.time() - 301) -# ) -# self.assertEqual(response.status_code, 401) + @pytest.mark.integration + def test_old_message(self): + '''Old messages should be rejected to prevent replay attacks.''' + with patch('bookwyrm.remote_user.fetch_user_data') as _: + response = self.send_test_request( + self.mouse, + date=http_date(time.time() - 301) + ) + self.assertEqual(response.status_code, 401) From 4ec557fc5dec8d75b0e634f3941c87e151b142f2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 14:15:13 -0800 Subject: [PATCH 16/32] fixes unit tests for incoming and outgoing follows --- bookwyrm/tests/incoming/test_follow.py | 54 +++++++------ bookwyrm/tests/outgoing/test_follow.py | 108 +++++++++++++------------ bookwyrm/tests/test_signing.py | 8 +- 3 files changed, 90 insertions(+), 80 deletions(-) diff --git a/bookwyrm/tests/incoming/test_follow.py b/bookwyrm/tests/incoming/test_follow.py index 377c70205..50e47d88b 100644 --- a/bookwyrm/tests/incoming/test_follow.py +++ b/bookwyrm/tests/incoming/test_follow.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from django.test import TestCase from bookwyrm import models, incoming @@ -18,29 +19,30 @@ class IncomingFollow(TestCase): self.local_user.save() -# def test_handle_follow(self): -# activity = { -# "@context": "https://www.w3.org/ns/activitystreams", -# "id": "https://example.com/users/rat/follows/123", -# "type": "Follow", -# "actor": "https://example.com/users/rat", -# "object": "http://local.com/user/mouse" -# } -# -# incoming.handle_follow(activity) -# -# # notification created -# notification = models.Notification.objects.get() -# self.assertEqual(notification.user, self.local_user) -# self.assertEqual(notification.notification_type, 'FOLLOW') -# -# # the request should have been deleted -# requests = models.UserFollowRequest.objects.all() -# self.assertEqual(list(requests), []) -# -# # the follow relationship should exist -# follow = models.UserFollows.objects.get(user_object=self.local_user) -# self.assertEqual(follow.user_subject, self.remote_user) + def test_handle_follow(self): + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/users/rat", + "object": "http://local.com/user/mouse" + } + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + incoming.handle_follow(activity) + + # notification created + notification = models.Notification.objects.get() + self.assertEqual(notification.user, self.local_user) + self.assertEqual(notification.notification_type, 'FOLLOW') + + # the request should have been deleted + requests = models.UserFollowRequest.objects.all() + self.assertEqual(list(requests), []) + + # the follow relationship should exist + follow = models.UserFollows.objects.get(user_object=self.local_user) + self.assertEqual(follow.user_subject, self.remote_user) def test_handle_follow_manually_approved(self): @@ -55,7 +57,8 @@ class IncomingFollow(TestCase): self.local_user.manually_approves_followers = True self.local_user.save() - incoming.handle_follow(activity) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + incoming.handle_follow(activity) # notification created notification = models.Notification.objects.get() @@ -81,7 +84,8 @@ class IncomingFollow(TestCase): "object": "http://local.com/user/nonexistent-user" } - incoming.handle_follow(activity) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + incoming.handle_follow(activity) # do nothing notifications = models.Notification.objects.all() diff --git a/bookwyrm/tests/outgoing/test_follow.py b/bookwyrm/tests/outgoing/test_follow.py index a1ea3f690..89e677abf 100644 --- a/bookwyrm/tests/outgoing/test_follow.py +++ b/bookwyrm/tests/outgoing/test_follow.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from django.test import TestCase from bookwyrm import models, outgoing @@ -19,54 +20,59 @@ class Following(TestCase): ) -# def test_handle_follow(self): -# self.assertEqual(models.UserFollowRequest.objects.count(), 0) -# -# outgoing.handle_follow(self.local_user, self.remote_user) -# rel = models.UserFollowRequest.objects.get() -# -# self.assertEqual(rel.user_subject, self.local_user) -# self.assertEqual(rel.user_object, self.remote_user) -# self.assertEqual(rel.status, 'follow_request') -# -# -# def test_handle_unfollow(self): -# self.remote_user.followers.add(self.local_user) -# self.assertEqual(self.remote_user.followers.count(), 1) -# outgoing.handle_unfollow(self.local_user, self.remote_user) -# -# self.assertEqual(self.remote_user.followers.count(), 0) -# -# -# def test_handle_accept(self): -# rel = models.UserFollowRequest.objects.create( -# user_subject=self.local_user, -# user_object=self.remote_user -# ) -# rel_id = rel.id -# -# outgoing.handle_accept(rel) -# # request should be deleted -# self.assertEqual( -# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 -# ) -# # follow relationship should exist -# self.assertEqual(self.remote_user.followers.first(), self.local_user) -# -# -# def test_handle_reject(self): -# rel = models.UserFollowRequest.objects.create( -# user_subject=self.local_user, -# user_object=self.remote_user -# ) -# rel_id = rel.id -# -# outgoing.handle_reject(rel) -# # request should be deleted -# self.assertEqual( -# models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 -# ) -# # follow relationship should not exist -# self.assertEqual( -# models.UserFollows.objects.filter(id=rel_id).count(), 0 -# ) + def test_handle_follow(self): + self.assertEqual(models.UserFollowRequest.objects.count(), 0) + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_follow(self.local_user, self.remote_user) + + rel = models.UserFollowRequest.objects.get() + + self.assertEqual(rel.user_subject, self.local_user) + self.assertEqual(rel.user_object, self.remote_user) + self.assertEqual(rel.status, 'follow_request') + + + def test_handle_unfollow(self): + self.remote_user.followers.add(self.local_user) + self.assertEqual(self.remote_user.followers.count(), 1) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_unfollow(self.local_user, self.remote_user) + + self.assertEqual(self.remote_user.followers.count(), 0) + + + def test_handle_accept(self): + rel = models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) + rel_id = rel.id + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_accept(rel) + # request should be deleted + self.assertEqual( + models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 + ) + # follow relationship should exist + self.assertEqual(self.remote_user.followers.first(), self.local_user) + + + def test_handle_reject(self): + rel = models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) + rel_id = rel.id + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_reject(rel) + # request should be deleted + self.assertEqual( + models.UserFollowRequest.objects.filter(id=rel_id).count(), 0 + ) + # follow relationship should not exist + self.assertEqual( + models.UserFollows.objects.filter(id=rel_id).count(), 0 + ) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index e393062ed..a5039306b 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -155,12 +155,12 @@ class Signature(TestCase): self.assertEqual(response.status_code, 200) # Try with new key: - #response = self.send_test_request(sender=new_sender) - #self.assertEqual(response.status_code, 200) + response = self.send_test_request(sender=new_sender) + self.assertEqual(response.status_code, 200) # Now the old key will fail: - #response = self.send_test_request(sender=self.fake_remote) - #self.assertEqual(response.status_code, 401) + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 401) @responses.activate From 9e48328e9e3285d5dc3dfe80a04c20ec063b9e60 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 14:18:45 -0800 Subject: [PATCH 17/32] Mocks broadcast task for outgoing shelve tests --- bookwyrm/tests/outgoing/test_shelving.py | 78 +++++++++++++----------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/bookwyrm/tests/outgoing/test_shelving.py b/bookwyrm/tests/outgoing/test_shelving.py index c26bec84f..5567784e5 100644 --- a/bookwyrm/tests/outgoing/test_shelving.py +++ b/bookwyrm/tests/outgoing/test_shelving.py @@ -1,3 +1,4 @@ +from unittest.mock import patch from django.test import TestCase from bookwyrm import models, outgoing @@ -25,39 +26,44 @@ class Shelving(TestCase): ) -# def test_handle_shelve(self): -# outgoing.handle_shelve(self.user, self.book, self.shelf) -# # make sure the book is on the shelf -# self.assertEqual(self.shelf.books.get(), self.book) -# -# -# def test_handle_shelve_to_read(self): -# shelf = models.Shelf.objects.get(identifier='to-read') -# -# outgoing.handle_shelve(self.user, self.book, shelf) -# # make sure the book is on the shelf -# self.assertEqual(shelf.books.get(), self.book) -# -# -# def test_handle_shelve_reading(self): -# shelf = models.Shelf.objects.get(identifier='reading') -# -# outgoing.handle_shelve(self.user, self.book, shelf) -# # make sure the book is on the shelf -# self.assertEqual(shelf.books.get(), self.book) -# -# -# def test_handle_shelve_read(self): -# shelf = models.Shelf.objects.get(identifier='read') -# -# outgoing.handle_shelve(self.user, self.book, shelf) -# # make sure the book is on the shelf -# self.assertEqual(shelf.books.get(), self.book) -# -# -# def test_handle_unshelve(self): -# self.shelf.books.add(self.book) -# self.shelf.save() -# self.assertEqual(self.shelf.books.count(), 1) -# outgoing.handle_unshelve(self.user, self.book, self.shelf) -# self.assertEqual(self.shelf.books.count(), 0) + def test_handle_shelve(self): + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, self.shelf) + # make sure the book is on the shelf + self.assertEqual(self.shelf.books.get(), self.book) + + + def test_handle_shelve_to_read(self): + shelf = models.Shelf.objects.get(identifier='to-read') + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, shelf) + # make sure the book is on the shelf + self.assertEqual(shelf.books.get(), self.book) + + + def test_handle_shelve_reading(self): + shelf = models.Shelf.objects.get(identifier='reading') + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, shelf) + # make sure the book is on the shelf + self.assertEqual(shelf.books.get(), self.book) + + + def test_handle_shelve_read(self): + shelf = models.Shelf.objects.get(identifier='read') + + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_shelve(self.user, self.book, shelf) + # make sure the book is on the shelf + self.assertEqual(shelf.books.get(), self.book) + + + def test_handle_unshelve(self): + self.shelf.books.add(self.book) + self.shelf.save() + self.assertEqual(self.shelf.books.count(), 1) + with patch('bookwyrm.broadcast.broadcast_task.delay') as _: + outgoing.handle_unshelve(self.user, self.book, self.shelf) + self.assertEqual(self.shelf.books.count(), 0) From 7b6035898bed76e0690b805d1be79b4f3073e225 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 14:54:08 -0800 Subject: [PATCH 18/32] Serialize activitypub authors from data in connector --- bookwyrm/activitypub/book.py | 12 +++--- bookwyrm/connectors/bookwyrm_connector.py | 6 +-- bookwyrm/models/__init__.py | 3 +- bookwyrm/models/author.py | 50 +++++++++++++++++++++++ bookwyrm/models/book.py | 41 ------------------- 5 files changed, 61 insertions(+), 51 deletions(-) create mode 100644 bookwyrm/models/author.py diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index bdc30edd2..2bfafba70 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -56,10 +56,10 @@ class Work(Book): class Author(ActivityObject): ''' author of a book ''' name: str - born: str - died: str - aliases: str - bio: str - openlibrary_key: str - wikipedia_link: str + born: str = '' + died: str = '' + aliases: str = '' + bio: str = '' + openlibraryKey: str = '' + wikipediaLink: str = '' type: str = 'Person' diff --git a/bookwyrm/connectors/bookwyrm_connector.py b/bookwyrm/connectors/bookwyrm_connector.py index 78ae84c32..1bc81450d 100644 --- a/bookwyrm/connectors/bookwyrm_connector.py +++ b/bookwyrm/connectors/bookwyrm_connector.py @@ -1,5 +1,4 @@ ''' using another bookwyrm instance as a source of book data ''' -from django.core.exceptions import ObjectDoesNotExist from django.db import transaction from bookwyrm import activitypub, models @@ -43,10 +42,11 @@ class Connector(AbstractConnector): try: yield models.Author.objects.get(origin_id=author_id) except models.Author.DoesNotExist: - continue + pass data = get_data(author_id) author_data = activitypub.Author(**data) - yield author_data.to_model(models.Author) + author = author_data.to_model(models.Author) + yield author def get_cover_from_data(self, data): diff --git a/bookwyrm/models/__init__.py b/bookwyrm/models/__init__.py index 8a93b805a..81c64831b 100644 --- a/bookwyrm/models/__init__.py +++ b/bookwyrm/models/__init__.py @@ -2,7 +2,8 @@ import inspect import sys -from .book import Book, Work, Edition, Author +from .book import Book, Work, Edition +from .author import Author from .connector import Connector from .relationship import UserFollows, UserFollowRequest, UserBlocks from .shelf import Shelf, ShelfBook diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py new file mode 100644 index 000000000..1d7017974 --- /dev/null +++ b/bookwyrm/models/author.py @@ -0,0 +1,50 @@ +''' database schema for info about authors ''' +from django.db import models +from django.utils import timezone + +from bookwyrm import activitypub +from bookwyrm.utils.fields import ArrayField + +from .base_model import ActivitypubMixin, ActivityMapping, BookWyrmModel + + +class Author(ActivitypubMixin, BookWyrmModel): + ''' basic biographic info ''' + origin_id = models.CharField(max_length=255, null=True) + ''' copy of an author from OL ''' + openlibrary_key = models.CharField(max_length=255, blank=True, null=True) + sync = models.BooleanField(default=True) + last_sync_date = models.DateTimeField(default=timezone.now) + wikipedia_link = models.CharField(max_length=255, blank=True, null=True) + # idk probably other keys would be useful here? + born = models.DateTimeField(blank=True, null=True) + died = models.DateTimeField(blank=True, null=True) + name = models.CharField(max_length=255) + last_name = models.CharField(max_length=255, blank=True, null=True) + first_name = models.CharField(max_length=255, blank=True, null=True) + aliases = ArrayField( + models.CharField(max_length=255), blank=True, default=list + ) + bio = models.TextField(null=True, blank=True) + + @property + def display_name(self): + ''' Helper to return a displayable name''' + if self.name: + return self.name + # don't want to return a spurious space if all of these are None + if self.first_name and self.last_name: + return self.first_name + ' ' + self.last_name + return self.last_name or self.first_name + + activity_mappings = [ + ActivityMapping('id', 'remote_id'), + ActivityMapping('name', 'name'), + ActivityMapping('born', 'born'), + ActivityMapping('died', 'died'), + ActivityMapping('aliases', 'aliases'), + ActivityMapping('bio', 'bio'), + ActivityMapping('openlibraryKey', 'openlibrary_key'), + ActivityMapping('wikipediaLink', 'wikipedia_link'), + ] + activity_serializer = activitypub.Author diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 642b5bfe0..7ec330da2 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -234,44 +234,3 @@ def isbn_13_to_10(isbn_13): if checkdigit == 10: checkdigit = 'X' return converted + str(checkdigit) - - -class Author(ActivitypubMixin, BookWyrmModel): - origin_id = models.CharField(max_length=255, null=True) - ''' copy of an author from OL ''' - openlibrary_key = models.CharField(max_length=255, blank=True, null=True) - sync = models.BooleanField(default=True) - last_sync_date = models.DateTimeField(default=timezone.now) - wikipedia_link = models.CharField(max_length=255, blank=True, null=True) - # idk probably other keys would be useful here? - born = models.DateTimeField(blank=True, null=True) - died = models.DateTimeField(blank=True, null=True) - name = models.CharField(max_length=255) - last_name = models.CharField(max_length=255, blank=True, null=True) - first_name = models.CharField(max_length=255, blank=True, null=True) - aliases = ArrayField( - models.CharField(max_length=255), blank=True, default=list - ) - bio = models.TextField(null=True, blank=True) - - @property - def display_name(self): - ''' Helper to return a displayable name''' - if self.name: - return self.name - # don't want to return a spurious space if all of these are None - if self.first_name and self.last_name: - return self.first_name + ' ' + self.last_name - return self.last_name or self.first_name - - activity_mappings = [ - ActivityMapping('id', 'remote_id'), - ActivityMapping('name', 'display_name'), - ActivityMapping('born', 'born'), - ActivityMapping('died', 'died'), - ActivityMapping('aliases', 'aliases'), - ActivityMapping('bio', 'bio'), - ActivityMapping('openlibrary_key', 'openlibrary_key'), - ActivityMapping('wikipedia_link', 'wikipedia_link'), - ] - activity_serializer = activitypub.Author From 69a664401145e3dd8937a211603d07f61d665782 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 15:34:47 -0800 Subject: [PATCH 19/32] Corrects activitypub fields to camelcase in Book --- bookwyrm/activitypub/book.py | 34 +++++++++++++++++----------------- bookwyrm/models/book.py | 24 ++++++++++++------------ 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index 2bfafba70..01bdcca91 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -7,23 +7,23 @@ from .base_activity import ActivityObject, Image @dataclass(init=False) class Book(ActivityObject): ''' serializes an edition or work, abstract ''' - authors: List[str] - first_published_date: str - published_date: str - title: str - sort_title: str - subtitle: str - description: str + sortTitle: str = '' + subtitle: str = '' + description: str = '' languages: List[str] - series: str - series_number: str + series: str = '' + seriesNumber: str = '' subjects: List[str] - subject_places: List[str] + subjectPlaces: List[str] - openlibrary_key: str - librarything_key: str - goodreads_key: str + authors: List[str] + firstPublishedDate: str = '' + publishedDate: str = '' + + openlibraryKey: str = '' + librarythingKey: str = '' + goodreadsKey: str = '' attachment: List[Image] = field(default_factory=lambda: []) type: str = 'Book' @@ -32,12 +32,12 @@ class Book(ActivityObject): @dataclass(init=False) class Edition(Book): ''' Edition instance of a book object ''' - isbn_10: str - isbn_13: str - oclc_number: str + isbn10: str + isbn13: str + oclcNumber: str asin: str pages: str - physical_format: str + physicalFormat: str publishers: List[str] work: str diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 7ec330da2..7a6528327 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -70,30 +70,30 @@ class Book(ActivitypubMixin, BookWyrmModel): ActivityMapping('id', 'remote_id'), ActivityMapping('authors', 'ap_authors'), - ActivityMapping('first_published_date', 'first_published_date'), - ActivityMapping('published_date', 'published_date'), + ActivityMapping('firstPublishedDate', 'firstpublished_date'), + ActivityMapping('publishedDate', 'published_date'), ActivityMapping('title', 'title'), - ActivityMapping('sort_title', 'sort_title'), + ActivityMapping('sortTitle', 'sort_title'), ActivityMapping('subtitle', 'subtitle'), ActivityMapping('description', 'description'), ActivityMapping('languages', 'languages'), ActivityMapping('series', 'series'), - ActivityMapping('series_number', 'series_number'), + ActivityMapping('seriesNumber', 'series_number'), ActivityMapping('subjects', 'subjects'), - ActivityMapping('subject_places', 'subject_places'), + ActivityMapping('subjectPlaces', 'subject_places'), - ActivityMapping('openlibrary_key', 'openlibrary_key'), - ActivityMapping('librarything_key', 'librarything_key'), - ActivityMapping('goodreads_key', 'goodreads_key'), + ActivityMapping('openlibraryKey', 'openlibrary_key'), + ActivityMapping('librarythingKey', 'librarything_key'), + ActivityMapping('goodreadsKey', 'goodreads_key'), ActivityMapping('work', 'ap_parent_work'), - ActivityMapping('isbn_10', 'isbn_10'), - ActivityMapping('isbn_13', 'isbn_13'), - ActivityMapping('oclc_number', 'oclc_number'), + ActivityMapping('isbn10', 'isbn_10'), + ActivityMapping('isbn13', 'isbn_13'), + ActivityMapping('oclcNumber', 'oclc_number'), ActivityMapping('asin', 'asin'), ActivityMapping('pages', 'pages'), - ActivityMapping('physical_format', 'physical_format'), + ActivityMapping('physicalFormat', 'physical_format'), ActivityMapping('publishers', 'publishers'), ActivityMapping('lccn', 'lccn'), From d8b2afff3dcaeef56cb28792f595539ba02e3a4d Mon Sep 17 00:00:00 2001 From: Joel Bradshaw Date: Fri, 27 Nov 2020 16:24:53 -0800 Subject: [PATCH 20/32] Replace naive datetimes with aware ones --- bookwyrm/forms.py | 3 ++- ...20200221_1702_squashed_0064_merge_20201101_1913.py | 5 ++--- bookwyrm/models/import_job.py | 6 ++++-- bookwyrm/models/site.py | 2 +- bookwyrm/status.py | 4 ++-- bookwyrm/tests/connectors/test_self_connector.py | 3 ++- bookwyrm/tests/models/test_import_model.py | 11 ++++++----- bookwyrm/wellknown.py | 7 +++---- 8 files changed, 22 insertions(+), 19 deletions(-) diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py index 29c6b6de7..784f10382 100644 --- a/bookwyrm/forms.py +++ b/bookwyrm/forms.py @@ -5,6 +5,7 @@ from collections import defaultdict from django import forms from django.forms import ModelForm, PasswordInput, widgets from django.forms.widgets import Textarea +from django.utils import timezone from bookwyrm import models @@ -143,7 +144,7 @@ class ExpiryWidget(widgets.Select): else: return selected_string # "This will raise - return datetime.datetime.now() + interval + return timezone.now() + interval class CreateInviteForm(CustomForm): class Meta: diff --git a/bookwyrm/migrations/0006_auto_20200221_1702_squashed_0064_merge_20201101_1913.py b/bookwyrm/migrations/0006_auto_20200221_1702_squashed_0064_merge_20201101_1913.py index 980b66142..13cb1406a 100644 --- a/bookwyrm/migrations/0006_auto_20200221_1702_squashed_0064_merge_20201101_1913.py +++ b/bookwyrm/migrations/0006_auto_20200221_1702_squashed_0064_merge_20201101_1913.py @@ -3,7 +3,6 @@ import bookwyrm.models.connector import bookwyrm.models.site import bookwyrm.utils.fields -import datetime from django.conf import settings import django.contrib.postgres.operations import django.core.validators @@ -37,7 +36,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name='status', name='published_date', - field=models.DateTimeField(default=datetime.datetime.now), + field=models.DateTimeField(default=django.utils.timezone.now), ), migrations.CreateModel( name='Edition', @@ -129,7 +128,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name='book', name='last_sync_date', - field=models.DateTimeField(default=datetime.datetime.now), + field=models.DateTimeField(default=django.utils.timezone.now), ), migrations.AddField( model_name='book', diff --git a/bookwyrm/models/import_job.py b/bookwyrm/models/import_job.py index b35f79921..fe39325f0 100644 --- a/bookwyrm/models/import_job.py +++ b/bookwyrm/models/import_job.py @@ -132,14 +132,16 @@ class ImportItem(models.Model): def date_added(self): ''' when the book was added to this dataset ''' if self.data['Date Added']: - return dateutil.parser.parse(self.data['Date Added']) + return timezone.make_aware( + dateutil.parser.parse(self.data['Date Added'])) return None @property def date_read(self): ''' the date a book was completed ''' if self.data['Date Read']: - return dateutil.parser.parse(self.data['Date Read']) + return timezone.make_aware( + dateutil.parser.parse(self.data['Date Read'])) return None @property diff --git a/bookwyrm/models/site.py b/bookwyrm/models/site.py index 24eb673c4..aa2e2a675 100644 --- a/bookwyrm/models/site.py +++ b/bookwyrm/models/site.py @@ -54,7 +54,7 @@ class SiteInvite(models.Model): def get_passowrd_reset_expiry(): ''' give people a limited time to use the link ''' - now = datetime.datetime.now() + now = timezone.now() return now + datetime.timedelta(days=1) diff --git a/bookwyrm/status.py b/bookwyrm/status.py index c950f4ab3..6a86209f4 100644 --- a/bookwyrm/status.py +++ b/bookwyrm/status.py @@ -1,5 +1,5 @@ ''' Handle user activity ''' -from datetime import datetime +from django.utils import timezone from bookwyrm import activitypub, books_manager, models from bookwyrm.sanitize_html import InputHtmlParser @@ -8,7 +8,7 @@ from bookwyrm.sanitize_html import InputHtmlParser def delete_status(status): ''' replace the status with a tombstone ''' status.deleted = True - status.deleted_date = datetime.now() + status.deleted_date = timezone.now() status.save() diff --git a/bookwyrm/tests/connectors/test_self_connector.py b/bookwyrm/tests/connectors/test_self_connector.py index dd638137f..4627bc8c8 100644 --- a/bookwyrm/tests/connectors/test_self_connector.py +++ b/bookwyrm/tests/connectors/test_self_connector.py @@ -1,6 +1,7 @@ ''' testing book data connectors ''' import datetime from django.test import TestCase +from django.utils import timezone from bookwyrm import models from bookwyrm.connectors.self_connector import Connector @@ -27,7 +28,7 @@ class SelfConnector(TestCase): self.edition = models.Edition.objects.create( title='Edition of Example Work', author_text='Anonymous', - published_date=datetime.datetime(1980, 5, 10), + published_date=datetime.datetime(1980, 5, 10, tzinfo=timezone.utc), parent_work=self.work, ) models.Edition.objects.create( diff --git a/bookwyrm/tests/models/test_import_model.py b/bookwyrm/tests/models/test_import_model.py index 5e9a9e304..b56d9a441 100644 --- a/bookwyrm/tests/models/test_import_model.py +++ b/bookwyrm/tests/models/test_import_model.py @@ -1,5 +1,6 @@ ''' testing models ''' import datetime +from django.utils import timezone from django.test import TestCase from bookwyrm import models @@ -77,29 +78,29 @@ class ImportJob(TestCase): def test_date_added(self): ''' converts to the local shelf typology ''' - expected = datetime.datetime(2019, 4, 9, 0, 0) + expected = datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc) item = models.ImportItem.objects.get(index=1) self.assertEqual(item.date_added, expected) def test_date_read(self): ''' converts to the local shelf typology ''' - expected = datetime.datetime(2019, 4, 12, 0, 0) + expected = datetime.datetime(2019, 4, 12, 0, 0, tzinfo=timezone.utc) item = models.ImportItem.objects.get(index=2) self.assertEqual(item.date_read, expected) def test_currently_reading_reads(self): expected = [models.ReadThrough( - start_date=datetime.datetime(2019, 4, 9, 0, 0))] + start_date=datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc))] actual = models.ImportItem.objects.get(index=1) self.assertEqual(actual.reads[0].start_date, expected[0].start_date) self.assertEqual(actual.reads[0].finish_date, expected[0].finish_date) def test_read_reads(self): actual = models.ImportItem.objects.get(index=2) - self.assertEqual(actual.reads[0].start_date, datetime.datetime(2019, 4, 9, 0, 0)) - self.assertEqual(actual.reads[0].finish_date, datetime.datetime(2019, 4, 12, 0, 0)) + self.assertEqual(actual.reads[0].start_date, datetime.datetime(2019, 4, 9, 0, 0, tzinfo=timezone.utc)) + self.assertEqual(actual.reads[0].finish_date, datetime.datetime(2019, 4, 12, 0, 0, tzinfo=timezone.utc)) def test_unread_reads(self): expected = [] diff --git a/bookwyrm/wellknown.py b/bookwyrm/wellknown.py index 29cee707d..ec8557ddf 100644 --- a/bookwyrm/wellknown.py +++ b/bookwyrm/wellknown.py @@ -1,10 +1,9 @@ ''' responds to various requests to /.well-know ''' -from datetime import datetime - from dateutil.relativedelta import relativedelta from django.http import HttpResponseNotFound from django.http import JsonResponse +from django.utils import timezone from bookwyrm import models from bookwyrm.settings import DOMAIN @@ -60,13 +59,13 @@ def nodeinfo(request): status_count = models.Status.objects.filter(user__local=True).count() user_count = models.User.objects.filter(local=True).count() - month_ago = datetime.now() - relativedelta(months=1) + month_ago = timezone.now() - relativedelta(months=1) last_month_count = models.User.objects.filter( local=True, last_active_date__gt=month_ago ).count() - six_months_ago = datetime.now() - relativedelta(months=6) + six_months_ago = timezone.now() - relativedelta(months=6) six_month_count = models.User.objects.filter( local=True, last_active_date__gt=six_months_ago From 624ff71a112dacd298c02fa6fdf447775c60a6f5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 17:20:01 -0800 Subject: [PATCH 21/32] Makes attachment Images a serializable class --- .../migrations/0014_auto_20201128_0118.py | 17 ++++++++++ bookwyrm/models/__init__.py | 10 ++++-- bookwyrm/models/attachment.py | 31 +++++++++++++++++++ bookwyrm/models/status.py | 11 ------- 4 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 bookwyrm/migrations/0014_auto_20201128_0118.py create mode 100644 bookwyrm/models/attachment.py diff --git a/bookwyrm/migrations/0014_auto_20201128_0118.py b/bookwyrm/migrations/0014_auto_20201128_0118.py new file mode 100644 index 000000000..babdd7805 --- /dev/null +++ b/bookwyrm/migrations/0014_auto_20201128_0118.py @@ -0,0 +1,17 @@ +# Generated by Django 3.0.7 on 2020-11-28 01:18 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookwyrm', '0013_book_origin_id'), + ] + + operations = [ + migrations.RenameModel( + old_name='Attachment', + new_name='Image', + ), + ] diff --git a/bookwyrm/models/__init__.py b/bookwyrm/models/__init__.py index 81c64831b..3d8544788 100644 --- a/bookwyrm/models/__init__.py +++ b/bookwyrm/models/__init__.py @@ -5,15 +5,21 @@ import sys from .book import Book, Work, Edition from .author import Author from .connector import Connector -from .relationship import UserFollows, UserFollowRequest, UserBlocks + from .shelf import Shelf, ShelfBook + from .status import Status, GeneratedNote, Review, Comment, Quotation -from .status import Attachment, Favorite, Boost, Notification, ReadThrough +from .status import Favorite, Boost, Notification, ReadThrough +from .attachment import Image + from .tag import Tag + from .user import User +from .relationship import UserFollows, UserFollowRequest, UserBlocks from .federated_server import FederatedServer from .import_job import ImportJob, ImportItem + from .site import SiteSettings, SiteInvite, PasswordReset cls_members = inspect.getmembers(sys.modules[__name__], inspect.isclass) diff --git a/bookwyrm/models/attachment.py b/bookwyrm/models/attachment.py new file mode 100644 index 000000000..b3e8bdb72 --- /dev/null +++ b/bookwyrm/models/attachment.py @@ -0,0 +1,31 @@ +''' media that is posted in the app ''' +from django.db import models + +from bookwyrm import activitypub +from .base_model import ActivitypubMixin +from .base_model import ActivityMapping, BookWyrmModel + + +class Attachment(ActivitypubMixin, BookWyrmModel): + ''' an image (or, in the future, video etc) associated with a status ''' + status = models.ForeignKey( + 'Status', + on_delete=models.CASCADE, + related_name='attachments' + ) + class Meta: + ''' one day we'll have other types of attachments besides images ''' + abstract = True + + activity_mappings = [ + ActivityMapping('id', 'remote_id'), + ActivityMapping('url', 'image'), + ActivityMapping('name', 'caption'), + ] + +class Image(Attachment): + ''' an image attachment ''' + image = models.ImageField(upload_to='status/', null=True, blank=True) + caption = models.TextField(null=True, blank=True) + + activity_serializer = activitypub.Image diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 6f534f506..1b8923891 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -151,17 +151,6 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): return super().save(*args, **kwargs) -class Attachment(BookWyrmModel): - ''' an image (or, in the future, video etc) associated with a status ''' - status = models.ForeignKey( - 'Status', - on_delete=models.CASCADE, - related_name='attachments' - ) - image = models.ImageField(upload_to='status/', null=True, blank=True) - caption = models.TextField(null=True, blank=True) - - class GeneratedNote(Status): ''' these are app-generated messages about user activity ''' @property From 4ae785a7f7e76ec38f0909063cfa109367b74495 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 17:58:21 -0800 Subject: [PATCH 22/32] move image activity to its own file --- bookwyrm/activitypub/__init__.py | 3 ++- bookwyrm/activitypub/base_activity.py | 19 +++---------------- bookwyrm/activitypub/book.py | 5 +++-- bookwyrm/activitypub/image.py | 9 +++++++++ bookwyrm/activitypub/note.py | 3 ++- bookwyrm/activitypub/person.py | 3 ++- 6 files changed, 21 insertions(+), 21 deletions(-) create mode 100644 bookwyrm/activitypub/image.py diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 852db345c..3dccc31a4 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -2,11 +2,12 @@ import inspect import sys -from .base_activity import ActivityEncoder, Image, PublicKey, Signature +from .base_activity import ActivityEncoder, PublicKey, Signature from .base_activity import Link, Mention from .base_activity import ActivitySerializerError from .base_activity import tag_formatter from .base_activity import image_formatter, image_attachments_formatter +from .image import Image from .note import Note, GeneratedNote, Article, Comment, Review, Quotation from .note import Tombstone from .interaction import Boost, Like diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 54c2baea9..3fe44f8b7 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -23,13 +23,6 @@ class ActivityEncoder(JSONEncoder): return o.__dict__ -@dataclass -class Image: - ''' image block ''' - url: str - type: str = 'Image' - - @dataclass class Link(): ''' for tagging a book in a status ''' @@ -146,6 +139,7 @@ class ActivityObject: for (model_key, value) in image_fields.items(): if not value: continue + #formatted_value = image_formatter(value) getattr(instance, model_key).save(*value, save=True) # add one to many fields @@ -212,16 +206,9 @@ def tag_formatter(tags, tag_type): def image_formatter(image_json): ''' helper function to load images and format them for a model ''' - if isinstance(image_json, list): - try: - image_json = image_json[0] - except IndexError: - return None - - if not image_json or not hasattr(image_json, 'url'): + url = image.get('url') + if not url: return None - url = image_json.get('url') - try: response = requests.get(url) except ConnectionError: diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index 01bdcca91..02cab2818 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -2,7 +2,8 @@ from dataclasses import dataclass, field from typing import List -from .base_activity import ActivityObject, Image +from .base_activity import ActivityObject +from .image import Image @dataclass(init=False) class Book(ActivityObject): @@ -25,7 +26,7 @@ class Book(ActivityObject): librarythingKey: str = '' goodreadsKey: str = '' - attachment: List[Image] = field(default_factory=lambda: []) + cover: Image = field(default_factory=lambda: {}) type: str = 'Book' diff --git a/bookwyrm/activitypub/image.py b/bookwyrm/activitypub/image.py new file mode 100644 index 000000000..c0ad3ea30 --- /dev/null +++ b/bookwyrm/activitypub/image.py @@ -0,0 +1,9 @@ +''' an image, nothing fancy ''' +from dataclasses import dataclass + +@dataclass +class Image: + ''' image block ''' + url: str + name: str = '' + type: str = 'Image' diff --git a/bookwyrm/activitypub/note.py b/bookwyrm/activitypub/note.py index 9eab952d3..aeb078dcc 100644 --- a/bookwyrm/activitypub/note.py +++ b/bookwyrm/activitypub/note.py @@ -2,7 +2,8 @@ from dataclasses import dataclass, field from typing import Dict, List -from .base_activity import ActivityObject, Image, Link +from .base_activity import ActivityObject, Link +from .image import Image @dataclass(init=False) class Tombstone(ActivityObject): diff --git a/bookwyrm/activitypub/person.py b/bookwyrm/activitypub/person.py index 324d68e32..e7d720ecf 100644 --- a/bookwyrm/activitypub/person.py +++ b/bookwyrm/activitypub/person.py @@ -2,7 +2,8 @@ from dataclasses import dataclass, field from typing import Dict -from .base_activity import ActivityObject, Image, PublicKey +from .base_activity import ActivityObject, PublicKey +from .image import Image @dataclass(init=False) class Person(ActivityObject): From 24806903781dd634cf95de2b2039037f9ed64e19 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 18:26:07 -0800 Subject: [PATCH 23/32] Automatically handle image fields in model serializer --- bookwyrm/activitypub/base_activity.py | 2 +- bookwyrm/models/base_model.py | 13 +++++++------ bookwyrm/models/book.py | 15 ++------------- bookwyrm/models/user.py | 6 +----- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 3fe44f8b7..33820e1e5 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -139,7 +139,7 @@ class ActivityObject: for (model_key, value) in image_fields.items(): if not value: continue - #formatted_value = image_formatter(value) + formatted_value = image_formatter(value) getattr(instance, model_key).save(*value, save=True) # add one to many fields diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index 8c28c8abb..4109a49b9 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -10,6 +10,7 @@ from Crypto.PublicKey import RSA from Crypto.Signature import pkcs1_15 from Crypto.Hash import SHA256 from django.db import models +from django.db.models.fields.files import ImageFieldFile from django.dispatch import receiver from bookwyrm import activitypub @@ -77,16 +78,18 @@ class ActivitypubMixin: value = value.remote_id elif isinstance(value, datetime): value = value.isoformat() + elif isinstance(value, ImageFieldFile): + value = image_formatter(value) # run the custom formatter function set in the model - result = mapping.activity_formatter(value) + formatted_value = mapping.activity_formatter(value) if mapping.activity_key in fields and \ isinstance(fields[mapping.activity_key], list): # 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 + fields[mapping.activity_key] += formatted_value else: - fields[mapping.activity_key] = result + fields[mapping.activity_key] = formatted_value if pure: return self.pure_activity_serializer( @@ -270,12 +273,10 @@ def tag_formatter(items, name_field, activity_type): return tags -def image_formatter(image, default_path=None): +def image_formatter(image): ''' convert images into activitypub json ''' if image and hasattr(image, 'url'): url = image.url - elif default_path: - url = default_path else: return None url = 'https://%s%s' % (DOMAIN, url) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 7a6528327..132b4c070 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -12,7 +12,6 @@ from bookwyrm.utils.fields import ArrayField from .base_model import ActivityMapping, BookWyrmModel from .base_model import ActivitypubMixin, OrderedCollectionPageMixin -from .base_model import image_attachments_formatter class Book(ActivitypubMixin, BookWyrmModel): ''' a generic book, which can mean either an edition or a work ''' @@ -61,11 +60,6 @@ class Book(ActivitypubMixin, BookWyrmModel): ''' the activitypub serialization should be a list of author ids ''' return [a.remote_id for a in self.authors.all()] - @property - def ap_parent_work(self): - ''' reference the work via local id not remote ''' - return self.parent_work.remote_id - activity_mappings = [ ActivityMapping('id', 'remote_id'), @@ -87,7 +81,7 @@ class Book(ActivitypubMixin, BookWyrmModel): ActivityMapping('librarythingKey', 'librarything_key'), ActivityMapping('goodreadsKey', 'goodreads_key'), - ActivityMapping('work', 'ap_parent_work'), + ActivityMapping('work', 'parent_work'), ActivityMapping('isbn10', 'isbn_10'), ActivityMapping('isbn13', 'isbn_13'), ActivityMapping('oclcNumber', 'oclc_number'), @@ -98,12 +92,7 @@ class Book(ActivitypubMixin, BookWyrmModel): ActivityMapping('lccn', 'lccn'), ActivityMapping('editions', 'editions_path'), - ActivityMapping( - 'attachment', 'cover', - # this expects an iterable and the field is just an image - lambda x: image_attachments_formatter([x]), - activitypub.image_formatter - ), + ActivityMapping('cover', 'cover'), ] def save(self, *args, **kwargs): diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index b38a4b192..4d511d561 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -112,11 +112,7 @@ class User(OrderedCollectionPageMixin, AbstractUser): activity_formatter=lambda x: {'sharedInbox': x}, model_formatter=lambda x: x.get('sharedInbox') ), - ActivityMapping( - 'icon', 'avatar', - lambda x: image_formatter(x, '/static/images/default_avi.jpg'), - activitypub.image_formatter - ), + ActivityMapping('icon', 'avatar'), ActivityMapping( 'manuallyApprovesFollowers', 'manually_approves_followers' From 4626d94ab933fe37314210a3cbec359d8c45096f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 20:11:22 -0800 Subject: [PATCH 24/32] handle image attachments recursively --- bookwyrm/activitypub/__init__.py | 1 - bookwyrm/activitypub/base_activity.py | 94 +++++++++---------- bookwyrm/activitypub/image.py | 6 +- .../migrations/0015_auto_20201128_0349.py | 19 ++++ bookwyrm/models/attachment.py | 3 +- bookwyrm/models/status.py | 1 - bookwyrm/signatures.py | 2 +- 7 files changed, 71 insertions(+), 55 deletions(-) create mode 100644 bookwyrm/migrations/0015_auto_20201128_0349.py diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 3dccc31a4..85245929b 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -6,7 +6,6 @@ from .base_activity import ActivityEncoder, PublicKey, Signature from .base_activity import Link, Mention from .base_activity import ActivitySerializerError from .base_activity import tag_formatter -from .base_activity import image_formatter, image_attachments_formatter from .image import Image from .note import Note, GeneratedNote, Article, Comment, Review, Quotation from .note import Tombstone diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 33820e1e5..31e6d2e0e 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -4,6 +4,7 @@ from json import JSONEncoder from uuid import uuid4 from django.core.files.base import ContentFile +from django.db import transaction from django.db.models.fields.related_descriptors \ import ForwardManyToOneDescriptor, ManyToManyDescriptor, \ ReverseManyToOneDescriptor @@ -106,14 +107,15 @@ class ActivityObject: formatted_value = mapping.model_formatter(value) if isinstance(model_field, ForwardManyToOneDescriptor) and \ formatted_value: - # foreign key remote id reolver + # foreign key remote id reolver (work on Edition, for example) fk_model = model_field.field.related_model reference = resolve_foreign_key(fk_model, formatted_value) mapped_fields[mapping.model_key] = reference elif isinstance(model_field, ManyToManyDescriptor): + # status mentions book/users many_to_many_fields[mapping.model_key] = formatted_value elif isinstance(model_field, ReverseManyToOneDescriptor): - # attachments on statuses, for example + # attachments on Status, for example one_to_many_fields[mapping.model_key] = formatted_value elif isinstance(model_field, ImageFileDescriptor): # image fields need custom handling @@ -121,38 +123,39 @@ class ActivityObject: else: mapped_fields[mapping.model_key] = formatted_value - if instance: - # updating an existing model isntance - for k, v in mapped_fields.items(): - setattr(instance, k, v) - instance.save() - else: - # creating a new model instance - instance = model.objects.create(**mapped_fields) - - # add many-to-many fields - for (model_key, values) in many_to_many_fields.items(): - getattr(instance, model_key).set(values) - instance.save() - - # add images - for (model_key, value) in image_fields.items(): - if not value: - continue - formatted_value = image_formatter(value) - getattr(instance, model_key).save(*value, save=True) - - # add one to many fields - for (model_key, values) in one_to_many_fields.items(): - items = [] - for item in values: - # the reference id wasn't available at creation time - setattr(item, instance.__class__.__name__.lower(), instance) - item.save() - items.append(item) - if items: - getattr(instance, model_key).set(items) + with transaction.atomic(): + if instance: + # updating an existing model isntance + for k, v in mapped_fields.items(): + setattr(instance, k, v) instance.save() + else: + # creating a new model instance + instance = model.objects.create(**mapped_fields) + + # add images + for (model_key, value) in image_fields.items(): + if not value: + continue + formatted_value = image_formatter(value) + getattr(instance, model_key).save(*formatted_value, save=True) + + for (model_key, values) in many_to_many_fields.items(): + # mention books, mention users + getattr(instance, model_key).set(values) + + # add one to many fields + for (model_key, values) in one_to_many_fields.items(): + model_field = getattr(instance, model_key) + model = model_field.model + for item in values: + item = model.activity_serializer(**item) + field_name = instance.__class__.__name__.lower() + with transaction.atomic(): + item = item.to_model(model) + setattr(item, field_name, instance) + item.save() + return instance @@ -204,9 +207,16 @@ def tag_formatter(tags, tag_type): return items -def image_formatter(image_json): +def image_formatter(image_slug): ''' helper function to load images and format them for a model ''' - url = image.get('url') + # when it's an inline image (User avatar/icon, Book cover), it's a json + # blob, but when it's an attached image, it's just a url + if isinstance(image_slug, dict): + url = image_slug.get('url') + elif isinstance(image_slug, str): + url = image_slug + else: + return None if not url: return None try: @@ -219,17 +229,3 @@ def image_formatter(image_json): image_name = str(uuid4()) + '.' + url.split('.')[-1] image_content = ContentFile(response.content) return [image_name, image_content] - - -def image_attachments_formatter(images_json): - ''' deserialize a list of images ''' - attachments = [] - for image in 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/activitypub/image.py b/bookwyrm/activitypub/image.py index c0ad3ea30..569f83c5d 100644 --- a/bookwyrm/activitypub/image.py +++ b/bookwyrm/activitypub/image.py @@ -1,9 +1,11 @@ ''' an image, nothing fancy ''' from dataclasses import dataclass +from .base_activity import ActivityObject -@dataclass -class Image: +@dataclass(init=False) +class Image(ActivityObject): ''' image block ''' url: str name: str = '' type: str = 'Image' + id: str = '' diff --git a/bookwyrm/migrations/0015_auto_20201128_0349.py b/bookwyrm/migrations/0015_auto_20201128_0349.py new file mode 100644 index 000000000..52b155186 --- /dev/null +++ b/bookwyrm/migrations/0015_auto_20201128_0349.py @@ -0,0 +1,19 @@ +# Generated by Django 3.0.7 on 2020-11-28 03:49 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookwyrm', '0014_auto_20201128_0118'), + ] + + operations = [ + migrations.AlterField( + model_name='image', + name='status', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='attachments', to='bookwyrm.Status'), + ), + ] diff --git a/bookwyrm/models/attachment.py b/bookwyrm/models/attachment.py index b3e8bdb72..7329e65d6 100644 --- a/bookwyrm/models/attachment.py +++ b/bookwyrm/models/attachment.py @@ -11,7 +11,8 @@ class Attachment(ActivitypubMixin, BookWyrmModel): status = models.ForeignKey( 'Status', on_delete=models.CASCADE, - related_name='attachments' + related_name='attachments', + null=True ) class Meta: ''' one day we'll have other types of attachments besides images ''' diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 1b8923891..9d45379cc 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -90,7 +90,6 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): ActivityMapping( 'attachment', 'attachments', lambda x: image_attachments_formatter(x.all()), - activitypub.image_attachments_formatter ) ] diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index 57c181dfb..dbb88d8a2 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -89,7 +89,7 @@ class Signature: def verify(self, public_key, request): ''' verify rsa signature ''' - if http_date_age(request.headers['date']) > MAX_SIGNATURE_AGE: + if False:#http_date_age(request.headers['date']) > MAX_SIGNATURE_AGE: raise ValueError( "Request too old: %s" % (request.headers['date'],)) public_key = RSA.import_key(public_key) From 5638597112d1b1fb71508f0a1f35c9d8f1ef6fc9 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Nov 2020 20:24:19 -0800 Subject: [PATCH 25/32] Fixes errors caught in tests --- bookwyrm/activitypub/base_activity.py | 6 ++++-- bookwyrm/signatures.py | 2 +- bookwyrm/tests/data/ap_quotation.json | 8 -------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 31e6d2e0e..caa4aeb80 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -135,9 +135,9 @@ class ActivityObject: # add images for (model_key, value) in image_fields.items(): - if not value: - continue formatted_value = image_formatter(value) + if not formatted_value: + continue getattr(instance, model_key).save(*formatted_value, save=True) for (model_key, values) in many_to_many_fields.items(): @@ -146,6 +146,8 @@ class ActivityObject: # add one to many fields for (model_key, values) in one_to_many_fields.items(): + if values == MISSING: + continue model_field = getattr(instance, model_key) model = model_field.model for item in values: diff --git a/bookwyrm/signatures.py b/bookwyrm/signatures.py index dbb88d8a2..57c181dfb 100644 --- a/bookwyrm/signatures.py +++ b/bookwyrm/signatures.py @@ -89,7 +89,7 @@ class Signature: def verify(self, public_key, request): ''' verify rsa signature ''' - if False:#http_date_age(request.headers['date']) > MAX_SIGNATURE_AGE: + if http_date_age(request.headers['date']) > MAX_SIGNATURE_AGE: raise ValueError( "Request too old: %s" % (request.headers['date'],)) public_key = RSA.import_key(public_key) diff --git a/bookwyrm/tests/data/ap_quotation.json b/bookwyrm/tests/data/ap_quotation.json index 089bc85fd..36a4112be 100644 --- a/bookwyrm/tests/data/ap_quotation.json +++ b/bookwyrm/tests/data/ap_quotation.json @@ -13,14 +13,6 @@ "sensitive": false, "content": "commentary", "type": "Quotation", - "attachment": [ - { - "type": "Document", - "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 78fa949237bf1621f038a107e70a738583677148 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 28 Nov 2020 07:55:31 -0800 Subject: [PATCH 26/32] Inline form to add description to book --- bookwyrm/templates/book.html | 33 +++++++++++++++++++++++++++++++-- bookwyrm/urls.py | 1 + bookwyrm/view_actions.py | 21 +++++++++++++++++++-- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/bookwyrm/templates/book.html b/bookwyrm/templates/book.html index b0064e1fd..f7456367a 100644 --- a/bookwyrm/templates/book.html +++ b/bookwyrm/templates/book.html @@ -57,6 +57,35 @@ {% include 'snippets/trimmed_text.html' with full=book|book_description %} + {% if request.user.is_authenticated and not book|book_description %} +
+ + +
+ +
+ + +
+ {% endif %} + + {% if book.parent_work.edition_set.count > 1 %}

{{ book.parent_work.edition_set.count }} editions

{% endif %} @@ -112,7 +141,7 @@
- +
@@ -135,7 +164,7 @@ -