From e76f96eb6c09fffd6059e4d914c47a03396e7295 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 12 May 2020 18:56:28 -0700 Subject: [PATCH] Unify concept of absolute_id and remote_id --- fedireads/activitypub/actor.py | 12 +-- fedireads/activitypub/book.py | 10 +-- fedireads/activitypub/create.py | 8 +- fedireads/activitypub/follow.py | 34 ++++---- fedireads/activitypub/shelve.py | 7 +- fedireads/activitypub/status.py | 60 +++++++-------- fedireads/books_manager.py | 38 +-------- fedireads/broadcast.py | 2 +- fedireads/connectors/openlibrary.py | 2 +- fedireads/connectors/self_connector.py | 2 +- fedireads/incoming.py | 2 +- .../migrations/0040_auto_20200513_0153.py | 77 +++++++++++++++++++ fedireads/migrations/0041_user_remote_id.py | 18 +++++ fedireads/models/base_model.py | 21 +++-- fedireads/models/book.py | 35 ++++++--- fedireads/models/shelf.py | 11 +-- fedireads/models/status.py | 12 --- fedireads/models/user.py | 23 ++---- fedireads/status.py | 13 ++-- fedireads/templates/notifications.html | 10 +-- .../templates/snippets/status_content.html | 2 +- .../templates/snippets/status_header.html | 4 +- fedireads/tests/test_base_model.py | 14 +--- fedireads/tests/test_book_model.py | 22 +++--- fedireads/tests/test_books_manager.py | 30 -------- fedireads/tests/test_broadcast.py | 8 +- fedireads/tests/test_comment.py | 2 +- fedireads/tests/test_quotation.py | 2 +- fedireads/tests/test_review.py | 2 +- fedireads/tests/test_status.py | 3 +- fedireads/tests/test_status_model.py | 8 +- fedireads/tests/test_user_model.py | 3 +- fedireads/wellknown.py | 2 +- 33 files changed, 263 insertions(+), 236 deletions(-) create mode 100644 fedireads/migrations/0040_auto_20200513_0153.py create mode 100644 fedireads/migrations/0041_user_remote_id.py diff --git a/fedireads/activitypub/actor.py b/fedireads/activitypub/actor.py index d26346f3..2afd6734 100644 --- a/fedireads/activitypub/actor.py +++ b/fedireads/activitypub/actor.py @@ -24,18 +24,18 @@ def get_actor(user): }, ], - 'id': user.actor, + 'id': user.remote_id, 'type': 'Person', 'preferredUsername': user.localname, 'name': user.name, 'inbox': user.inbox, - 'outbox': '%s/outbox' % user.actor, - 'followers': '%s/followers' % user.actor, - 'following': '%s/following' % user.actor, + 'outbox': '%s/outbox' % user.remote_id, + 'followers': '%s/followers' % user.remote_id, + 'following': '%s/following' % user.remote_id, 'summary': user.summary, 'publicKey': { - 'id': '%s/#main-key' % user.actor, - 'owner': user.actor, + 'id': '%s/#main-key' % user.remote_id, + 'owner': user.remote_id, 'publicKeyPem': user.public_key, }, 'endpoints': { diff --git a/fedireads/activitypub/book.py b/fedireads/activitypub/book.py index e3c0e6bf..fd2430be 100644 --- a/fedireads/activitypub/book.py +++ b/fedireads/activitypub/book.py @@ -34,9 +34,9 @@ def get_book(book, recursive=True): 'type': 'Document', 'book_type': book_type, 'name': book.title, - 'url': book.absolute_id, + 'url': book.local_id, - 'authors': [a.absolute_id for a in book.authors.all()], + 'authors': [a.local_id for a in book.authors.all()], 'first_published_date': book.first_published_date.isoformat() if \ book.first_published_date else None, 'published_date': book.published_date.isoformat() if \ @@ -79,7 +79,7 @@ def get_author(author): ] activity = { '@context': 'https://www.w3.org/ns/activitystreams', - 'url': author.absolute_id, + 'url': author.local_id, 'type': 'Person', } for field in fields: @@ -90,7 +90,7 @@ def get_author(author): def get_shelf(shelf, page=None): ''' serialize shelf object ''' - id_slug = shelf.absolute_id + id_slug = shelf.remote_id if page: return get_shelf_page(shelf, page) count = shelf.books.count() @@ -110,7 +110,7 @@ def get_shelf_page(shelf, page): start = (page - 1) * page_length end = start + page_length shelf_page = shelf.books.all()[start:end] - id_slug = shelf.absolute_id + id_slug = shelf.local_id data = { '@context': 'https://www.w3.org/ns/activitystreams', 'id': '%s?page=%d' % (id_slug, page), diff --git a/fedireads/activitypub/create.py b/fedireads/activitypub/create.py index f78f7d79..183a9205 100644 --- a/fedireads/activitypub/create.py +++ b/fedireads/activitypub/create.py @@ -14,16 +14,16 @@ def get_create(user, status_json): 'id': '%s/activity' % status_json['id'], 'type': 'Create', - 'actor': user.actor, + 'actor': user.remote_id, 'published': status_json['published'], - 'to': ['%s/followers' % user.actor], + 'to': ['%s/followers' % user.remote_id], 'cc': ['https://www.w3.org/ns/activitystreams#Public'], 'object': status_json, 'signature': { 'type': 'RsaSignature2017', - 'creator': '%s#main-key' % user.absolute_id, + 'creator': '%s#main-key' % user.remote_id, 'created': status_json['published'], 'signatureValue': b64encode(signed_message).decode('utf8'), } @@ -37,7 +37,7 @@ def get_update(user, activity_json): '@context': 'https://www.w3.org/ns/activitystreams', 'id': 'https://friend.camp/users/tripofmice#updates/1585446332', 'type': 'Update', - 'actor': user.actor, + 'actor': user.remote_id, 'to': [ 'https://www.w3.org/ns/activitystreams#Public' ], diff --git a/fedireads/activitypub/follow.py b/fedireads/activitypub/follow.py index 4d6a5dad..d7f7798d 100644 --- a/fedireads/activitypub/follow.py +++ b/fedireads/activitypub/follow.py @@ -12,22 +12,22 @@ def get_follow_request(user, to_follow): 'id': 'https://%s/%s' % (DOMAIN, str(uuid)), 'summary': '', 'type': 'Follow', - 'actor': user.actor, - 'object': to_follow.actor, + 'actor': user.remote_id, + 'object': to_follow.remote_id, } def get_unfollow(relationship): ''' undo that precious bond of friendship ''' return { '@context': 'https://www.w3.org/ns/activitystreams', - 'id': '%s/undo' % relationship.absolute_id, + 'id': '%s/undo' % relationship.remote_id, 'type': 'Undo', - 'actor': relationship.user_subject.actor, + 'actor': relationship.user_subject.remote_id, 'object': { 'id': relationship.relationship_id, 'type': 'Follow', - 'actor': relationship.user_subject.actor, - 'object': relationship.user_object.actor, + 'actor': relationship.user_subject.remote_id, + 'object': relationship.user_object.remote_id, } } @@ -36,14 +36,14 @@ def get_accept(user, relationship): ''' accept a follow request ''' return { '@context': 'https://www.w3.org/ns/activitystreams', - 'id': '%s#accepts/follows/' % user.absolute_id, + 'id': '%s#accepts/follows/' % user.remote_id, 'type': 'Accept', - 'actor': user.actor, + 'actor': user.remote_id, 'object': { 'id': relationship.relationship_id, 'type': 'Follow', - 'actor': relationship.user_subject.actor, - 'object': relationship.user_object.actor, + 'actor': relationship.user_subject.remote_id, + 'object': relationship.user_object.remote_id, } } @@ -52,27 +52,27 @@ def get_reject(user, relationship): ''' reject a follow request ''' return { '@context': 'https://www.w3.org/ns/activitystreams', - 'id': '%s#rejects/follows/' % user.absolute_id, + 'id': '%s#rejects/follows/' % user.remote_id, 'type': 'Reject', - 'actor': user.actor, + 'actor': user.remote_id, 'object': { 'id': relationship.relationship_id, 'type': 'Follow', - 'actor': relationship.user_subject.actor, - 'object': relationship.user_object.actor, + 'actor': relationship.user_subject.remote_id, + 'object': relationship.user_object.remote_id, } } def get_followers(user, page, follow_queryset): ''' list of people who follow a user ''' - id_slug = '%s/followers' % user.actor + id_slug = '%s/followers' % user.remote_id return get_follow_info(id_slug, page, follow_queryset) def get_following(user, page, follow_queryset): ''' list of people who follow a user ''' - id_slug = '%s/following' % user.actor + id_slug = '%s/following' % user.remote_id return get_follow_info(id_slug, page, follow_queryset) @@ -103,7 +103,7 @@ def get_follow_page(user_list, id_slug, page): 'type': 'OrderedCollectionPage', 'totalItems': user_list.count(), 'partOf': id_slug, - 'orderedItems': [u.actor for u in follower_page], + 'orderedItems': [u.remote_id for u in follower_page], } if end <= user_list.count(): # there are still more pages diff --git a/fedireads/activitypub/shelve.py b/fedireads/activitypub/shelve.py index 46cf4132..b776531c 100644 --- a/fedireads/activitypub/shelve.py +++ b/fedireads/activitypub/shelve.py @@ -18,16 +18,15 @@ def get_add_remove(user, book, shelf, action='Add'): '@context': 'https://www.w3.org/ns/activitystreams', 'id': str(uuid), 'type': action, - 'actor': user.actor, + 'actor': user.remote_id, 'object': { - # TODO: document?? 'type': 'Document', 'name': book.title, - 'url': book.absolute_id, + 'url': book.local_id, }, 'target': { 'type': 'Collection', 'name': shelf.name, - 'id': shelf.absolute_id, + 'id': shelf.remote_id, } } diff --git a/fedireads/activitypub/status.py b/fedireads/activitypub/status.py index 68a58a67..a5e0abca 100644 --- a/fedireads/activitypub/status.py +++ b/fedireads/activitypub/status.py @@ -7,7 +7,7 @@ from fedireads.settings import DOMAIN def get_rating(review): ''' activitypub serialize rating activity ''' status = get_status(review) - status['inReplyToBook'] = review.book.absolute_id + status['inReplyToBook'] = review.book.local_id status['fedireadsType'] = review.status_type status['rating'] = review.rating status['content'] = '%d star rating of "%s"' % ( @@ -18,7 +18,7 @@ def get_rating(review): def get_quotation(quotation): ''' fedireads json for quotations ''' status = get_status(quotation) - status['inReplyToBook'] = quotation.book.absolute_id + status['inReplyToBook'] = quotation.book.local_id status['fedireadsType'] = quotation.status_type status['quote'] = quotation.quote return status @@ -29,7 +29,7 @@ def get_quotation_article(quotation): status = get_status(quotation) content = '"%s"
-- "%s")

%s' % ( quotation.quote, - quotation.book.absolute_id, + quotation.book.local_id, quotation.book.title, quotation.content, ) @@ -40,7 +40,7 @@ def get_quotation_article(quotation): def get_review(review): ''' fedireads json for book reviews ''' status = get_status(review) - status['inReplyToBook'] = review.book.absolute_id + status['inReplyToBook'] = review.book.local_id status['fedireadsType'] = review.status_type status['name'] = review.name status['rating'] = review.rating @@ -50,7 +50,7 @@ def get_review(review): def get_comment(comment): ''' fedireads json for book reviews ''' status = get_status(comment) - status['inReplyToBook'] = comment.book.absolute_id + status['inReplyToBook'] = comment.book.local_id status['fedireadsType'] = comment.status_type return status @@ -87,15 +87,15 @@ def get_comment_article(comment): ''' a book comment formatted for a non-fedireads isntance (mastodon) ''' status = get_status(comment) status['content'] += '

(comment on "%s")' % \ - (comment.book.absolute_id, comment.book.title) + (comment.book.local_id, comment.book.title) return status def get_status(status): ''' create activitypub json for a status ''' user = status.user - uri = status.absolute_id - reply_parent_id = status.reply_parent.absolute_id \ + uri = status.remote_id + reply_parent_id = status.reply_parent.remote_id \ if status.reply_parent else None image_attachments = [] @@ -117,10 +117,10 @@ def get_status(status): 'url': uri, 'inReplyTo': reply_parent_id, 'published': status.published_date.isoformat(), - 'attributedTo': user.actor, + 'attributedTo': user.remote_id, # TODO: assuming all posts are public -- should check privacy db field 'to': ['https://www.w3.org/ns/activitystreams#Public'], - 'cc': ['%s/followers' % user.absolute_id], + 'cc': ['%s/followers' % user.remote_id], 'sensitive': status.sensitive, 'content': status.content, 'type': status.activity_type, @@ -142,7 +142,7 @@ def get_status(status): def get_replies(status, replies): ''' collection of replies ''' - id_slug = status.absolute_id + '/replies' + id_slug = status.remote_id + '/replies' return { '@context': 'https://www.w3.org/ns/activitystreams', 'id': id_slug, @@ -159,7 +159,7 @@ def get_replies(status, replies): def get_replies_page(status, replies): ''' actual reply list content ''' - id_slug = status.absolute_id + '/replies?page=true&only_other_accounts=true' + id_slug = status.remote_id + '/replies?page=true&only_other_accounts=true' items = [] for reply in replies: if reply.user.local: @@ -171,7 +171,7 @@ def get_replies_page(status, replies): 'id': id_slug, 'type': 'CollectionPage', 'next': '%s&min_id=%d' % (id_slug, replies[len(replies) - 1].id), - 'partOf': status.absolute_id + '/replies', + 'partOf': status.remote_id + '/replies', 'items': [items] } @@ -180,10 +180,10 @@ def get_favorite(favorite): ''' like a post ''' return { '@context': 'https://www.w3.org/ns/activitystreams', - 'id': favorite.absolute_id, + 'id': favorite.remote_id, 'type': 'Like', - 'actor': favorite.user.actor, - 'object': favorite.status.absolute_id, + 'actor': favorite.user.remote_id, + 'object': favorite.status.remote_id, } @@ -191,14 +191,14 @@ def get_unfavorite(favorite): ''' like a post ''' return { '@context': 'https://www.w3.org/ns/activitystreams', - 'id': '%s/undo' % favorite.absolute_id, + 'id': '%s/undo' % favorite.remote_id, 'type': 'Undo', - 'actor': favorite.user.actor, + 'actor': favorite.user.remote_id, 'object': { - 'id': favorite.absolute_id, + 'id': favorite.remote_id, 'type': 'Like', - 'actor': favorite.user.actor, - 'object': favorite.status.absolute_id, + 'actor': favorite.user.remote_id, + 'object': favorite.status.remote_id, } } @@ -207,10 +207,10 @@ def get_boost(boost): ''' boost/announce a post ''' return { '@context': 'https://www.w3.org/ns/activitystreams', - 'id': boost.absolute_id, + 'id': boost.remote_id, 'type': 'Announce', - 'actor': boost.user.actor, - 'object': boost.boosted_status.absolute_id, + 'actor': boost.user.remote_id, + 'object': boost.boosted_status.remote_id, } @@ -221,15 +221,15 @@ def get_add_tag(tag): '@context': 'https://www.w3.org/ns/activitystreams', 'id': str(uuid), 'type': 'Add', - 'actor': tag.user.actor, + 'actor': tag.user.remote_id, 'object': { 'type': 'Tag', - 'id': tag.absolute_id, + 'id': tag.remote_id, 'name': tag.name, }, 'target': { 'type': 'Book', - 'id': tag.book.absolute_id, + 'id': tag.book.local_id, } } @@ -241,14 +241,14 @@ def get_remove_tag(tag): '@context': 'https://www.w3.org/ns/activitystreams', 'id': str(uuid), 'type': 'Remove', - 'actor': tag.user.actor, + 'actor': tag.user.remote_id, 'object': { 'type': 'Tag', - 'id': tag.absolute_id, + 'id': tag.remote_id, 'name': tag.name, }, 'target': { 'type': 'Book', - 'id': tag.book.absolute_id, + 'id': tag.book.local_id, } } diff --git a/fedireads/books_manager.py b/fedireads/books_manager.py index e0053413..a97d3fb3 100644 --- a/fedireads/books_manager.py +++ b/fedireads/books_manager.py @@ -4,7 +4,7 @@ from requests import HTTPError import importlib from urllib.parse import urlparse -from fedireads import models, settings +from fedireads import models from fedireads.tasks import app @@ -18,7 +18,9 @@ def get_edition(book_id): def get_or_create_book(remote_id): ''' pull up a book record by whatever means possible ''' - book = get_by_absolute_id(remote_id, models.Book) + book = models.Book.objects.select_subclasses().filter( + remote_id=remote_id + ).first() if book: return book @@ -52,38 +54,6 @@ def get_or_create_connector(remote_id): return load_connector(connector_info) -def get_by_absolute_id(absolute_id, model): - ''' generalized function to get from a model with a remote_id field ''' - if not absolute_id: - return None - - # check if it's a remote status - try: - return model.objects.get(remote_id=absolute_id) - except model.DoesNotExist: - pass - - url = urlparse(absolute_id) - if url.netloc != settings.DOMAIN: - return None - - # try finding a local status with that id - local_id = absolute_id.split('/')[-1] - try: - if hasattr(model.objects, 'select_subclasses'): - possible_match = model.objects.select_subclasses().get(id=local_id) - else: - possible_match = model.objects.get(id=local_id) - except model.DoesNotExist: - return None - - # make sure it's not actually a remote status with an id that - # clashes with a local id - if possible_match.absolute_id == absolute_id: - return possible_match - return None - - @app.task def load_more_data(book_id): ''' background the work of getting all 10,000 editions of LoTR ''' diff --git a/fedireads/broadcast.py b/fedireads/broadcast.py index ba0aaf3a..ff7dd801 100644 --- a/fedireads/broadcast.py +++ b/fedireads/broadcast.py @@ -74,7 +74,7 @@ def make_signature(sender, destination, date): signer = pkcs1_15.new(RSA.import_key(sender.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode('utf8'))) signature = { - 'keyId': '%s#main-key' % sender.actor, + 'keyId': '%s#main-key' % sender.remote_id, 'algorithm': 'rsa-sha256', 'headers': '(request-target) host date', 'signature': b64encode(signed_message).decode('utf8'), diff --git a/fedireads/connectors/openlibrary.py b/fedireads/connectors/openlibrary.py index ca0cf33f..efabc103 100644 --- a/fedireads/connectors/openlibrary.py +++ b/fedireads/connectors/openlibrary.py @@ -124,7 +124,7 @@ class Connector(AbstractConnector): def format_search_result(self, doc): - # build the absolute id from the openlibrary key + # build the remote id from the openlibrary key key = self.books_url + doc['key'] author = doc.get('author_name') or ['Unknown'] return SearchResult( diff --git a/fedireads/connectors/self_connector.py b/fedireads/connectors/self_connector.py index b8c865ee..915c463d 100644 --- a/fedireads/connectors/self_connector.py +++ b/fedireads/connectors/self_connector.py @@ -41,7 +41,7 @@ class Connector(AbstractConnector): def format_search_result(self, book): return SearchResult( book.title, - book.absolute_id, + book.local_id, book.author_text, book.published_date.year if book.published_date else None, ) diff --git a/fedireads/incoming.py b/fedireads/incoming.py index 6827e8ee..3167031d 100644 --- a/fedireads/incoming.py +++ b/fedireads/incoming.py @@ -295,7 +295,7 @@ def handle_favorite(activity): def handle_unfavorite(activity): ''' approval of your good good post ''' favorite_id = activity['object']['id'] - fav = status_builder.get_favorite(favorite_id) + fav = models.Favorite.objects.filter(remote_id=favorite_id).first() if not fav: return False diff --git a/fedireads/migrations/0040_auto_20200513_0153.py b/fedireads/migrations/0040_auto_20200513_0153.py new file mode 100644 index 00000000..1f5ff0d4 --- /dev/null +++ b/fedireads/migrations/0040_auto_20200513_0153.py @@ -0,0 +1,77 @@ +# Generated by Django 3.0.3 on 2020-05-13 01:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('fedireads', '0039_auto_20200510_2342'), + ] + + operations = [ + migrations.RemoveField( + model_name='user', + name='actor', + ), + migrations.AddField( + model_name='connector', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='federatedserver', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='notification', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='readthrough', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='shelf', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='shelfbook', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='tag', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='userblocks', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='userfollowrequest', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AddField( + model_name='userfollows', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AlterField( + model_name='favorite', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + migrations.AlterField( + model_name='status', + name='remote_id', + field=models.CharField(max_length=255, null=True), + ), + ] diff --git a/fedireads/migrations/0041_user_remote_id.py b/fedireads/migrations/0041_user_remote_id.py new file mode 100644 index 00000000..55388d5a --- /dev/null +++ b/fedireads/migrations/0041_user_remote_id.py @@ -0,0 +1,18 @@ +# Generated by Django 3.0.3 on 2020-05-13 02:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('fedireads', '0040_auto_20200513_0153'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='remote_id', + field=models.CharField(max_length=255, null=True, unique=True), + ), + ] diff --git a/fedireads/models/base_model.py b/fedireads/models/base_model.py index 506e063f..a90312f4 100644 --- a/fedireads/models/base_model.py +++ b/fedireads/models/base_model.py @@ -1,5 +1,6 @@ ''' base model with default fields ''' from django.db import models +from django.dispatch import receiver from fedireads.settings import DOMAIN @@ -7,18 +8,24 @@ class FedireadsModel(models.Model): ''' fields and functions for every model ''' created_date = models.DateTimeField(auto_now_add=True) updated_date = models.DateTimeField(auto_now=True) + remote_id = models.CharField(max_length=255, null=True) - @property - def absolute_id(self): - ''' constructs the absolute reference to any db object ''' - if hasattr(self, 'remote_id') and self.remote_id: - return self.remote_id - + def get_remote_id(self): + ''' generate a url that resolves to the local object ''' base_path = 'https://%s' % DOMAIN if hasattr(self, 'user'): - base_path = self.user.absolute_id + base_path = self.user.remote_id model_name = type(self).__name__.lower() return '%s/%s/%d' % (base_path, model_name, self.id) class Meta: abstract = True + + +@receiver(models.signals.post_save, sender=FedireadsModel) +def execute_after_save(sender, instance, created, *args, **kwargs): + ''' set the remote_id after save (when the id is available) ''' + if not created: + return + instance.remote_id = instance.get_remote_id() + instance.save() diff --git a/fedireads/models/book.py b/fedireads/models/book.py index 2b561a1d..ca198584 100644 --- a/fedireads/models/book.py +++ b/fedireads/models/book.py @@ -5,7 +5,7 @@ from model_utils.managers import InheritanceManager from fedireads import activitypub from fedireads.settings import DOMAIN -from fedireads.utils.fields import JSONField, ArrayField +from fedireads.utils.fields import ArrayField from .base_model import FedireadsModel from fedireads.connectors.settings import CONNECTORS @@ -47,7 +47,6 @@ class Connector(FedireadsModel): class Book(FedireadsModel): ''' a generic book, which can mean either an edition or a work ''' - remote_id = models.CharField(max_length=255, null=True) # these identifiers apply to both works and editions openlibrary_key = models.CharField(max_length=255, blank=True, null=True) librarything_key = models.CharField(max_length=255, blank=True, null=True) @@ -87,20 +86,27 @@ class Book(FedireadsModel): published_date = models.DateTimeField(blank=True, null=True) objects = InheritanceManager() - @property - def absolute_id(self): - ''' constructs the absolute reference to any db object ''' - if self.sync and self.remote_id: - return self.remote_id - base_path = 'https://%s' % DOMAIN - return '%s/book/%d' % (base_path, self.id) - def save(self, *args, **kwargs): ''' can't be abstract for query reasons, but you shouldn't USE it ''' if not isinstance(self, Edition) and not isinstance(self, Work): raise ValueError('Books should be added as Editions or Works') + super().save(*args, **kwargs) + def get_remote_id(self): + ''' editions and works both use "book" instead of model_name ''' + return 'https://%s/book/%d' % (DOMAIN, self.id) + + + @property + def local_id(self): + ''' when a book is ingested from an outside source, it becomes local to + an instance, so it needs a local url for federation. but it still needs + the remote_id for easier deduplication and, if appropriate, to sync with + the remote canonical copy ''' + return 'https://%s/book/%d' % (DOMAIN, self.id) + + def __repr__(self): return "<{} key={!r} title={!r}>".format( self.__class__, @@ -152,7 +158,6 @@ class Edition(Book): class Author(FedireadsModel): ''' copy of an author from OL ''' - remote_id = models.CharField(max_length=255, null=True) openlibrary_key = models.CharField(max_length=255, blank=True, null=True) sync = models.BooleanField(default=True) last_sync_date = models.DateTimeField(default=timezone.now) @@ -168,6 +173,14 @@ class Author(FedireadsModel): ) bio = models.TextField(null=True, blank=True) + @property + def local_id(self): + ''' when a book is ingested from an outside source, it becomes local to + an instance, so it needs a local url for federation. but it still needs + the remote_id for easier deduplication and, if appropriate, to sync with + the remote canonical copy (ditto here for author)''' + return 'https://%s/book/%d' % (DOMAIN, self.id) + @property def activitypub_serialize(self): return activitypub.get_author(self) diff --git a/fedireads/models/shelf.py b/fedireads/models/shelf.py index ff625a09..e9111d6e 100644 --- a/fedireads/models/shelf.py +++ b/fedireads/models/shelf.py @@ -1,7 +1,6 @@ ''' puttin' books on shelves ''' from django.db import models -from fedireads import activitypub from .base_model import FedireadsModel @@ -17,12 +16,10 @@ class Shelf(FedireadsModel): through_fields=('shelf', 'book') ) - @property - def absolute_id(self): - ''' use shelf identifier as absolute id ''' - base_path = self.user.absolute_id - model_name = type(self).__name__.lower() - return '%s/%s/%s' % (base_path, model_name, self.identifier) + def get_remote_id(self): + ''' shelf identifier instead of id ''' + base_path = self.user.remote_id + return '%s/shelf/%s' % (base_path, self.identifier) class Meta: unique_together = ('user', 'identifier') diff --git a/fedireads/models/status.py b/fedireads/models/status.py index 171d1a67..a5af6dd4 100644 --- a/fedireads/models/status.py +++ b/fedireads/models/status.py @@ -12,7 +12,6 @@ from .base_model import FedireadsModel class Status(FedireadsModel): ''' any post, like a reply to a review, etc ''' - remote_id = models.CharField(max_length=255, unique=True, null=True) user = models.ForeignKey('User', on_delete=models.PROTECT) status_type = models.CharField(max_length=255, default='Note') content = models.TextField(blank=True, null=True) @@ -39,16 +38,6 @@ class Status(FedireadsModel): ) objects = InheritanceManager() - @property - def absolute_id(self): - ''' constructs the absolute reference to any db object ''' - if self.remote_id: - return self.remote_id - base_path = self.user.absolute_id - model_name = type(self).__name__.lower() - return '%s/%s/%d' % (base_path, model_name, self.id) - - @property def activitypub_serialize(self): return activitypub.get_status(self) @@ -111,7 +100,6 @@ class Favorite(FedireadsModel): ''' fav'ing a post ''' user = models.ForeignKey('User', on_delete=models.PROTECT) status = models.ForeignKey('Status', on_delete=models.PROTECT) - remote_id = models.CharField(max_length=255, unique=True, null=True) class Meta: unique_together = ('user', 'status') diff --git a/fedireads/models/user.py b/fedireads/models/user.py index 5c10fc26..e9bb2de0 100644 --- a/fedireads/models/user.py +++ b/fedireads/models/user.py @@ -15,7 +15,6 @@ class User(AbstractUser): ''' a user who wants to read books ''' private_key = models.TextField(blank=True, null=True) public_key = models.TextField(blank=True, null=True) - actor = models.CharField(max_length=255, unique=True) inbox = models.CharField(max_length=255, unique=True) shared_inbox = models.CharField(max_length=255, blank=True, null=True) federated_server = models.ForeignKey( @@ -63,17 +62,11 @@ class User(AbstractUser): through_fields=('user', 'status'), related_name='favorite_statuses' ) + remote_id = models.CharField(max_length=255, null=True, unique=True) created_date = models.DateTimeField(auto_now_add=True) updated_date = models.DateTimeField(auto_now=True) manually_approves_followers = models.BooleanField(default=False) - @property - def absolute_id(self): - ''' users are identified by their username, so overriding this prop ''' - model_name = type(self).__name__.lower() - username = self.localname or self.username - return 'https://%s/%s/%s' % (DOMAIN, model_name, username) - @property def activitypub_serialize(self): return activitypub.get_actor(self) @@ -107,10 +100,9 @@ class UserRelationship(FedireadsModel): ) ] - @property - def absolute_id(self): - ''' use shelf identifier as absolute id ''' - base_path = self.user_subject.absolute_id + def get_remote_id(self): + ''' use shelf identifier in remote_id ''' + base_path = self.user_subject.remote_id return '%s#%s/%d' % (base_path, self.status, self.id) @@ -158,12 +150,13 @@ def execute_before_save(sender, instance, *args, **kwargs): return # populate fields for local users + instance.remote_id = 'https://%s/user/%s' % (DOMAIN, instance.username) instance.localname = instance.username instance.username = '%s@%s' % (instance.username, DOMAIN) - instance.actor = instance.absolute_id - instance.inbox = '%s/inbox' % instance.absolute_id + instance.actor = instance.remote_id + instance.inbox = '%s/inbox' % instance.remote_id instance.shared_inbox = 'https://%s/inbox' % DOMAIN - instance.outbox = '%s/outbox' % instance.absolute_id + instance.outbox = '%s/outbox' % instance.remote_id if not instance.private_key: random_generator = Random.new().read key = RSA.generate(1024, random_generator) diff --git a/fedireads/status.py b/fedireads/status.py index b6020ae0..d6dfc564 100644 --- a/fedireads/status.py +++ b/fedireads/status.py @@ -2,7 +2,7 @@ from django.db import IntegrityError from fedireads import models -from fedireads.books_manager import get_or_create_book, get_by_absolute_id +from fedireads.books_manager import get_or_create_book from fedireads.sanitize_html import InputHtmlParser @@ -155,14 +155,11 @@ def create_boost_from_activity(user, activity): return models.Boost.objects.get(status=status, user=user) -def get_status(absolute_id): +def get_status(remote_id): ''' find a status in the database ''' - return get_by_absolute_id(absolute_id, models.Status) - - -def get_favorite(absolute_id): - ''' find a status in the database ''' - return get_by_absolute_id(absolute_id, models.Favorite) + return models.Status.objects.select_subclasses().filter( + remote_id=remote_id + ).first() def create_status(user, content, reply_parent=None, mention_books=None, diff --git a/fedireads/templates/notifications.html b/fedireads/templates/notifications.html index d7dc6a0f..d5c4a70b 100644 --- a/fedireads/templates/notifications.html +++ b/fedireads/templates/notifications.html @@ -18,12 +18,12 @@ {% include 'snippets/username.html' with user=notification.related_user %} {% if notification.notification_type == 'FAVORITE' %} favorited your - status + status {% elif notification.notification_type == 'REPLY' %} - replied + replied to your - status + status {% elif notification.notification_type == 'FOLLOW' %} followed you @@ -35,10 +35,10 @@ {% elif notification.notification_type == 'BOOST' %} - boosted your status + boosted your status {% endif %} {% else %} - your import completed. + your import completed. {% endif %} diff --git a/fedireads/templates/snippets/status_content.html b/fedireads/templates/snippets/status_content.html index fd21b2bb..542d7690 100644 --- a/fedireads/templates/snippets/status_content.html +++ b/fedireads/templates/snippets/status_content.html @@ -60,7 +60,7 @@ {% include 'snippets/status_content.html' with status=status|boosted_status %} {% endif %} - {% if not max_depth and status.reply_parent or status|replies %}

Thread{% endif %} + {% if not max_depth and status.reply_parent or status|replies %}

Thread{% endif %} diff --git a/fedireads/templates/snippets/status_header.html b/fedireads/templates/snippets/status_header.html index 2cc224bd..3a95e295 100644 --- a/fedireads/templates/snippets/status_header.html +++ b/fedireads/templates/snippets/status_header.html @@ -17,9 +17,9 @@ boosted {% elif status.reply_parent %} {% with parent_status=status|parent %} - replied to {% include 'snippets/username.html' with user=parent_status.user possessive=True %} {{ parent_status.status_type | lower }} + replied to {% include 'snippets/username.html' with user=parent_status.user possessive=True %} {{ parent_status.status_type | lower }} {% endwith %} {% endif %} - {{ status.published_date | naturaltime }} + {{ status.published_date | naturaltime }} diff --git a/fedireads/tests/test_base_model.py b/fedireads/tests/test_base_model.py index a2dfbbda..85934f27 100644 --- a/fedireads/tests/test_base_model.py +++ b/fedireads/tests/test_base_model.py @@ -7,25 +7,19 @@ from fedireads.settings import DOMAIN class BaseModel(TestCase): - def test_absolute_id(self): + def test_remote_id(self): instance = FedireadsModel() instance.id = 1 - expected = instance.absolute_id + expected = instance.get_remote_id() self.assertEqual(expected, 'https://%s/fedireadsmodel/1' % DOMAIN) - def test_absolute_id_with_remote(self): - instance = FedireadsModel() - instance.remote_id = 'boop doop' - expected = instance.absolute_id - self.assertEqual(expected, 'boop doop') - - def test_absolute_id_with_user(self): + def test_remote_id_with_user(self): user = models.User.objects.create_user( 'mouse', 'mouse@mouse.com', 'mouseword') instance = FedireadsModel() instance.user = user instance.id = 1 - expected = instance.absolute_id + expected = instance.get_remote_id() self.assertEqual( expected, 'https://%s/user/mouse/fedireadsmodel/1' % DOMAIN) diff --git a/fedireads/tests/test_book_model.py b/fedireads/tests/test_book_model.py index 58cc8fc0..205c9ff8 100644 --- a/fedireads/tests/test_book_model.py +++ b/fedireads/tests/test_book_model.py @@ -7,14 +7,18 @@ from fedireads import models, settings class Book(TestCase): ''' not too much going on in the books model but here we are ''' def setUp(self): - work = models.Work.objects.create(title='Example Work') - models.Edition.objects.create(title='Example Edition', parent_work=work) + self.work = models.Work.objects.create(title='Example Work') + models.Edition.objects.create(title='Example Edition', parent_work=self.work) - def test_absolute_id(self): - ''' editions and works use the same absolute id syntax ''' - book = models.Edition.objects.first() - expected_id = 'https://%s/book/%d' % (settings.DOMAIN, book.id) - self.assertEqual(book.absolute_id, expected_id) + def test_remote_id(self): + ''' editions and works use the same remote_id syntax ''' + expected_id = 'https://%s/book/%d' % (settings.DOMAIN, self.work.id) + self.assertEqual(self.work.get_remote_id(), expected_id) + + def test_local_id(self): + ''' the local_id property for books ''' + expected_id = 'https://%s/book/%d' % (settings.DOMAIN, self.work.id) + self.assertEqual(self.work.local_id, expected_id) def test_create_book(self): ''' you shouldn't be able to create Books (only editions and works) ''' @@ -37,8 +41,8 @@ class Shelf(TestCase): models.Shelf.objects.create( name='Test Shelf', identifier='test-shelf', user=user) - def test_absolute_id(self): + def test_remote_id(self): ''' editions and works use the same absolute id syntax ''' shelf = models.Shelf.objects.get(identifier='test-shelf') expected_id = 'https://%s/user/mouse/shelf/test-shelf' % settings.DOMAIN - self.assertEqual(shelf.absolute_id, expected_id) + self.assertEqual(shelf.get_remote_id(), expected_id) diff --git a/fedireads/tests/test_books_manager.py b/fedireads/tests/test_books_manager.py index 8ec330b1..d09b1eb8 100644 --- a/fedireads/tests/test_books_manager.py +++ b/fedireads/tests/test_books_manager.py @@ -35,33 +35,3 @@ class Book(TestCase): same_connector = books_manager.get_or_create_connector(remote_id) self.assertEqual(connector.identifier, same_connector.identifier) - - - def test_get_by_absolute_id_local(self): - abs_id = 'https://%s/book/%d' % (DOMAIN, self.work.id) - work = books_manager.get_by_absolute_id(abs_id, models.Work) - self.assertEqual(work, self.work) - - work = books_manager.get_by_absolute_id(abs_id, models.Edition) - self.assertIsNone(work) - - - def test_get_by_absolute_id_remote(self): - remote_work = models.Work.objects.create( - title='Example Work', - remote_id='https://example.com/book/123', - ) - - abs_id = 'https://example.com/book/123' - work = books_manager.get_by_absolute_id(abs_id, models.Work) - self.assertEqual(work, remote_work) - - - def test_get_by_absolute_id_invalid(self): - abs_id = 'https://%s/book/34534623' % DOMAIN - result = books_manager.get_by_absolute_id(abs_id, models.Work) - self.assertIsNone(result) - - abs_id = 'httook534623' - result = books_manager.get_by_absolute_id(abs_id, models.Work) - self.assertIsNone(result) diff --git a/fedireads/tests/test_broadcast.py b/fedireads/tests/test_broadcast.py index 2583084a..074a7dfb 100644 --- a/fedireads/tests/test_broadcast.py +++ b/fedireads/tests/test_broadcast.py @@ -11,7 +11,7 @@ class Book(TestCase): follower = models.User.objects.create_user( 'rat', 'rat@mouse.mouse', 'ratword', local=False, - actor='http://example.com/u/1', + remote_id='http://example.com/u/1', outbox='http://example.com/u/1/o', shared_inbox='http://example.com/inbox', inbox='http://example.com/u/1/inbox') @@ -20,14 +20,14 @@ class Book(TestCase): no_inbox_follower = models.User.objects.create_user( 'hamster', 'hamster@mouse.mouse', 'hamword', shared_inbox=None, local=False, - actor='http://example.com/u/2', + remote_id='http://example.com/u/2', outbox='http://example.com/u/2/o', inbox='http://example.com/u/2/inbox') self.user.followers.add(no_inbox_follower) non_fr_follower = models.User.objects.create_user( 'gerbil', 'gerb@mouse.mouse', 'gerbword', - actor='http://example.com/u/3', + remote_id='http://example.com/u/3', outbox='http://example2.com/u/3/o', inbox='http://example2.com/u/3/inbox', shared_inbox='http://example2.com/inbox', @@ -40,7 +40,7 @@ class Book(TestCase): models.User.objects.create_user( 'nutria', 'nutria@mouse.mouse', 'nuword', - actor='http://example.com/u/4', + remote_id='http://example.com/u/4', outbox='http://example.com/u/4/o', shared_inbox='http://example.com/inbox', inbox='http://example.com/u/4/inbox', diff --git a/fedireads/tests/test_comment.py b/fedireads/tests/test_comment.py index 2da16741..5eb28473 100644 --- a/fedireads/tests/test_comment.py +++ b/fedireads/tests/test_comment.py @@ -45,7 +45,7 @@ class Comment(TestCase): "items": [] } }, - "inReplyToBook": self.book.absolute_id, + "inReplyToBook": self.book.remote_id, "fedireadsType": "Comment" } comment = status_builder.create_comment_from_activity( diff --git a/fedireads/tests/test_quotation.py b/fedireads/tests/test_quotation.py index 45f577a8..1687659b 100644 --- a/fedireads/tests/test_quotation.py +++ b/fedireads/tests/test_quotation.py @@ -53,7 +53,7 @@ class Quotation(TestCase): 'items': [] } }, - 'inReplyToBook': self.book.absolute_id, + 'inReplyToBook': self.book.remote_id, 'fedireadsType': 'Quotation', 'quote': 'quote body' } diff --git a/fedireads/tests/test_review.py b/fedireads/tests/test_review.py index 097a2d5e..33c9c2be 100644 --- a/fedireads/tests/test_review.py +++ b/fedireads/tests/test_review.py @@ -66,7 +66,7 @@ class Review(TestCase): 'items': [] } }, - 'inReplyToBook': self.book.absolute_id, + 'inReplyToBook': self.book.remote_id, 'fedireadsType': 'Review', 'name': 'review title', 'rating': 3 diff --git a/fedireads/tests/test_status.py b/fedireads/tests/test_status.py index dbec4b26..186da68d 100644 --- a/fedireads/tests/test_status.py +++ b/fedireads/tests/test_status.py @@ -22,6 +22,7 @@ class Status(TestCase): self.assertEqual(reply.content, content) self.assertEqual(reply.reply_parent, status) + def test_create_status_from_activity(self): book = models.Edition.objects.create(title='Example Edition') review = status_builder.create_review( @@ -29,7 +30,7 @@ class Status(TestCase): activity = { 'id': 'https://example.com/user/mouse/status/12', 'url': 'https://example.com/user/mouse/status/12', - 'inReplyTo': review.absolute_id, + 'inReplyTo': review.remote_id, 'published': '2020-05-10T02:15:59.635557+00:00', 'attributedTo': 'https://example.com/user/mouse', 'to': [ diff --git a/fedireads/tests/test_status_model.py b/fedireads/tests/test_status_model.py index 6d221d62..d40e25c8 100644 --- a/fedireads/tests/test_status_model.py +++ b/fedireads/tests/test_status_model.py @@ -23,7 +23,7 @@ class Status(TestCase): self.assertEqual(status.activity_type, 'Note') expected_id = 'https://%s/user/mouse/status/%d' % \ (settings.DOMAIN, status.id) - self.assertEqual(status.absolute_id, expected_id) + self.assertEqual(status.remote_id, expected_id) def test_comment(self): comment = models.Comment.objects.first() @@ -31,7 +31,7 @@ class Status(TestCase): self.assertEqual(comment.activity_type, 'Note') expected_id = 'https://%s/user/mouse/comment/%d' % \ (settings.DOMAIN, comment.id) - self.assertEqual(comment.absolute_id, expected_id) + self.assertEqual(comment.remote_id, expected_id) def test_quotation(self): quotation = models.Quotation.objects.first() @@ -39,7 +39,7 @@ class Status(TestCase): self.assertEqual(quotation.activity_type, 'Note') expected_id = 'https://%s/user/mouse/quotation/%d' % \ (settings.DOMAIN, quotation.id) - self.assertEqual(quotation.absolute_id, expected_id) + self.assertEqual(quotation.remote_id, expected_id) def test_review(self): review = models.Review.objects.first() @@ -47,7 +47,7 @@ class Status(TestCase): self.assertEqual(review.activity_type, 'Article') expected_id = 'https://%s/user/mouse/review/%d' % \ (settings.DOMAIN, review.id) - self.assertEqual(review.absolute_id, expected_id) + self.assertEqual(review.remote_id, expected_id) class Tag(TestCase): diff --git a/fedireads/tests/test_user_model.py b/fedireads/tests/test_user_model.py index c972bca0..6168ca33 100644 --- a/fedireads/tests/test_user_model.py +++ b/fedireads/tests/test_user_model.py @@ -14,10 +14,9 @@ class User(TestCase): ''' username instead of id here ''' user = models.User.objects.get(localname='mouse') expected_id = 'https://%s/user/mouse' % DOMAIN - self.assertEqual(user.absolute_id, expected_id) + self.assertEqual(user.remote_id, expected_id) self.assertEqual(user.username, 'mouse@%s' % DOMAIN) self.assertEqual(user.localname, 'mouse') - self.assertEqual(user.actor, 'https://%s/user/mouse' % DOMAIN) self.assertEqual(user.shared_inbox, 'https://%s/inbox' % DOMAIN) self.assertEqual(user.inbox, '%s/inbox' % expected_id) self.assertEqual(user.outbox, '%s/outbox' % expected_id) diff --git a/fedireads/wellknown.py b/fedireads/wellknown.py index 4f2da723..49a6210a 100644 --- a/fedireads/wellknown.py +++ b/fedireads/wellknown.py @@ -24,7 +24,7 @@ def webfinger(request): { 'rel': 'self', 'type': 'application/activity+json', - 'href': user.actor + 'href': user.remote_id } ] })