From 72c7829bab4a997fec24be2b9d1925a21bd6e3bb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 28 Nov 2020 17:29:03 -0800 Subject: [PATCH] Preserve remote_id syntax for authors and books --- bookwyrm/activitypub/base_activity.py | 114 +++++++++++++------------- bookwyrm/models/author.py | 12 +-- bookwyrm/models/base_model.py | 10 ++- bookwyrm/models/book.py | 23 ++---- bookwyrm/views.py | 2 +- 5 files changed, 72 insertions(+), 89 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index b9b6b9b4..4c73ab97 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -6,9 +6,8 @@ from uuid import uuid4 import dateutil.parser from dateutil.parser import ParserError from django.core.files.base import ContentFile -from django.db import transaction from django.db.models.fields.related_descriptors \ - import ForwardManyToOneDescriptor, ManyToManyDescriptor, \ + import ForwardManyToOneDescriptor, ManyToManyDescriptor, \ ReverseManyToOneDescriptor from django.db.models.fields import DateTimeField from django.db.models.fields.files import ImageFileDescriptor @@ -16,8 +15,6 @@ from django.db.models.query_utils import DeferredAttribute from django.utils import timezone import requests -from bookwyrm import models - class ActivitySerializerError(ValueError): ''' routine problems serializing activitypub json ''' @@ -135,7 +132,7 @@ class ActivityObject: # if the AP field is a serialized object (as in Add) remote_id = formatted_value['id'] else: - # if the AP field is just a remote_id (as in every other case) + # if the field is just a remote_id (as in every other case) remote_id = formatted_value reference = resolve_remote_id(fk_model, remote_id) mapped_fields[mapping.model_key] = reference @@ -153,62 +150,67 @@ class ActivityObject: formatted_value = None mapped_fields[mapping.model_key] = formatted_value - with transaction.atomic(): - if instance: - # updating an existing model instance - 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) + if instance: + # updating an existing model instance + 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) + print('CREATING') + print(instance) + print(instance.id) - # --- these are all fields that can't be saved until after the - # instance has an id (after it's been saved). ---------------# + # --- these are all fields that can't be saved until after the + # instance has an id (after it's been saved). ---------------# - # add images - for (model_key, value) in image_fields.items(): - formatted_value = image_formatter(value) - if not formatted_value: - continue - getattr(instance, model_key).save(*formatted_value, save=True) + # add images + for (model_key, value) in image_fields.items(): + formatted_value = image_formatter(value) + if not formatted_value: + continue + getattr(instance, model_key).save(*formatted_value, save=True) - # add many to many fields - for (model_key, values) in many_to_many_fields.items(): - # mention books, mention users - if values == MISSING: - continue - model_field = getattr(instance, model_key) - model = model_field.model - items = [] - for link in values: - if isinstance(link, dict): - # check that the Type matches the model (Status - # tags contain both user mentions and book tags) - if not model.activity_serializer.type == \ - link.get('type'): - continue - remote_id = link.get('href') - else: - remote_id = link - items.append( - resolve_remote_id(model, remote_id) - ) - getattr(instance, model_key).set(items) + # add many to many fields + for (model_key, values) in many_to_many_fields.items(): + # mention books, mention users + if values == MISSING: + continue + model_field = getattr(instance, model_key) + model = model_field.model + items = [] + for link in values: + if isinstance(link, dict): + # check that the Type matches the model (Status + # tags contain both user mentions and book tags) + if not model.activity_serializer.type == \ + link.get('type'): + continue + remote_id = link.get('href') + else: + remote_id = link + items.append( + resolve_remote_id(model, remote_id) + ) + getattr(instance, model_key).set(items) - # 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: + # 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: + if isinstance(item, str): + print(model) + item = resolve_remote_id(model, item) + else: 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() + item = item.to_model(model) + field_name = instance.__class__.__name__.lower() + setattr(item, field_name, instance) + item.save() return instance diff --git a/bookwyrm/models/author.py b/bookwyrm/models/author.py index 0a934e45..fdaf73d9 100644 --- a/bookwyrm/models/author.py +++ b/bookwyrm/models/author.py @@ -1,7 +1,4 @@ ''' database schema for info about authors ''' -from uuid import uuid4 -import re - from django.db import models from django.utils import timezone @@ -37,18 +34,13 @@ class Author(ActivitypubMixin, BookWyrmModel): self.remote_id = self.get_remote_id() if not self.id: - # force set the remote id to a local version self.origin_id = self.remote_id - self.remote_id = self.get_remote_id() + self.remote_id = None return super().save(*args, **kwargs) def get_remote_id(self): ''' editions and works both use "book" instead of model_name ''' - uuid = str(uuid4())[:8] - # in Book, the title is used to make the url more readable, but - # since an author's name can change, I didn't want to lock in a - # potential deadname (or maiden name) in the urk. - return 'https://%s/author/%s' % (DOMAIN, uuid) + return 'https://%s/author/%s' % (DOMAIN, self.id) @property def display_name(self): diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index 877f094e..71f3c7b7 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -12,7 +12,7 @@ from Crypto.Hash import SHA256 from django.db import models from django.db.models.fields.files import ImageFileDescriptor from django.db.models.fields.related_descriptors \ - import ManyToManyDescriptor + import ManyToManyDescriptor, ReverseManyToOneDescriptor from django.dispatch import receiver from bookwyrm import activitypub @@ -74,16 +74,18 @@ class ActivitypubMixin: # this field on the model isn't serialized continue value = getattr(self, mapping.model_key) - model_field_type = getattr(self.__class__, mapping.model_key) + model_field = getattr(self.__class__, mapping.model_key) + print(mapping.model_key, type(model_field)) 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 - elif isinstance(model_field_type, ManyToManyDescriptor): + elif isinstance(model_field, ManyToManyDescriptor) or \ + isinstance(model_field, ReverseManyToOneDescriptor): value = [i.remote_id for i in value.all()] elif isinstance(value, datetime): value = value.isoformat() - elif isinstance(model_field_type, ImageFileDescriptor): + elif isinstance(model_field, ImageFileDescriptor): value = image_formatter(value) # run the custom formatter function set in the model diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index d3460aac..cc377d21 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -1,5 +1,4 @@ ''' database schema for books and shelves ''' -from uuid import uuid4 import re from django.db import models @@ -99,16 +98,13 @@ class Book(ActivitypubMixin, BookWyrmModel): self.remote_id = self.get_remote_id() if not self.id: - # force set the remote id to a local version self.origin_id = self.remote_id - self.remote_id = self.get_remote_id() + self.remote_id = None return super().save(*args, **kwargs) def get_remote_id(self): ''' editions and works both use "book" instead of model_name ''' - uuid = str(uuid4())[:8] - clean_title = re.sub(r'[\W-]', '', self.title.replace(' ', '-')).lower() - return 'https://%s/author/%s-%s' % (DOMAIN, clean_title, uuid) + return 'https://%s/book/%d' % (DOMAIN, self.id) def __repr__(self): return "<{} key={!r} title={!r}>".format( @@ -129,17 +125,6 @@ class Work(OrderedCollectionPageMixin, Book): null=True ) - - def to_edition_list(self, **kwargs): - ''' activitypub serialization for this work's editions ''' - remote_id = self.remote_id + '/editions' - return self.to_ordered_collection( - self.edition_set, - remote_id=remote_id, - **kwargs - ) - - activity_serializer = activitypub.Work @@ -161,7 +146,8 @@ class Edition(Book): through='ShelfBook', through_fields=('book', 'shelf') ) - parent_work = models.ForeignKey('Work', on_delete=models.PROTECT, null=True) + parent_work = models.ForeignKey( + 'Work', on_delete=models.PROTECT, null=True, related_name='editions') activity_serializer = activitypub.Edition @@ -175,6 +161,7 @@ class Edition(Book): return super().save(*args, **kwargs) + def isbn_10_to_13(isbn_10): ''' convert an isbn 10 into an isbn 13 ''' isbn_10 = re.sub(r'[^0-9X]', '', isbn_10) diff --git a/bookwyrm/views.py b/bookwyrm/views.py index c6992e6b..bd62902c 100644 --- a/bookwyrm/views.py +++ b/bookwyrm/views.py @@ -540,7 +540,7 @@ def book_page(request, book_id): return HttpResponseNotFound() reviews = models.Review.objects.filter( - book__in=work.edition_set.all(), + book__in=work.editions.all(), ) # all reviews for the book reviews = get_activity_feed(request.user, 'federated', model=reviews)