From 3f1b62eb98a1acce321f9a0b31e4ee006f48cd65 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 11:04:13 -0800 Subject: [PATCH] Fixes Add activity still janky --- bookwyrm/activitypub/base_activity.py | 51 +++++++++++++++++---------- bookwyrm/activitypub/verbs.py | 11 +++--- bookwyrm/models/fields.py | 13 ++++--- bookwyrm/tests/views/test_inbox.py | 12 ++++--- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index c20ab3b8..db6cdc94 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -80,17 +80,10 @@ class ActivityObject: setattr(self, field.name, value) - def to_model(self, instance=None, allow_create=True, save=True): + def to_model(self, model=None, instance=None, allow_create=True, save=True): ''' convert from an activity to a model instance ''' # figure out the right model -- wish I had a better way for this - models = apps.get_models() - model = [m for m in models if hasattr(m, 'activity_serializer') and \ - hasattr(m.activity_serializer, 'type') and \ - m.activity_serializer.type == self.type] - if not len(model): - raise ActivitySerializerError( - 'No model found for activity type "%s"' % self.type) - model = model[0] + model = model or get_model_from_type(self.type) if hasattr(model, 'ignore_activity') and model.ignore_activity(self): return instance @@ -156,6 +149,14 @@ class ActivityObject: def serialize(self): ''' convert to dictionary with context attr ''' data = self.__dict__ + # recursively serialize + for (k, v) in data.items(): + try: + is_subclass = issubclass(v, ActivityObject) + except TypeError: + is_subclass = False + if is_subclass: + data[k] = v.serialize() data = {k:v for (k, v) in data.items() if v is not None} data['@context'] = 'https://www.w3.org/ns/activitystreams' return data @@ -207,11 +208,23 @@ def set_related_field( item.save() -def resolve_remote_id(model, remote_id, refresh=False, save=True): +def get_model_from_type(activity_type): + ''' given the activity, what type of model ''' + models = apps.get_models() + model = [m for m in models if hasattr(m, 'activity_serializer') and \ + hasattr(m.activity_serializer, 'type') and \ + m.activity_serializer.type == activity_type] + if not len(model): + raise ActivitySerializerError( + 'No model found for activity type "%s"' % activity_type) + return model[0] + +def resolve_remote_id(remote_id, model=None, refresh=False, save=True): ''' take a remote_id and return an instance, creating if necessary ''' - result = model.find_existing_by_remote_id(remote_id) - if result and not refresh: - return result + if model:# a bonus check we can do if we already know the model + result = model.find_existing_by_remote_id(remote_id) + if result and not refresh: + return result # load the data and create the object try: @@ -220,13 +233,15 @@ def resolve_remote_id(model, remote_id, refresh=False, save=True): raise ActivitySerializerError( 'Could not connect to host for remote_id in %s model: %s' % \ (model.__name__, remote_id)) + # determine the model implicitly, if not provided + if not model: + model = get_model_from_type(data.get('type')) # check for existing items with shared unique identifiers - if not result: - result = model.find_existing(data) - if result and not refresh: - return result + result = model.find_existing(data) + if result and not refresh: + return result item = model.activity_serializer(**data) # if we're refreshing, "result" will be set and we'll update it - return item.to_model(instance=result, save=save) + return item.to_model(model=model, instance=result, save=save) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index c3de73f3..300e1bf3 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -2,7 +2,7 @@ from dataclasses import dataclass from typing import List -from .base_activity import ActivityObject, Signature +from .base_activity import ActivityObject, Signature, resolve_remote_id from .book import Edition @@ -113,19 +113,22 @@ class Reject(Verb): class Add(Verb): '''Add activity ''' target: str - object: ActivityObject + object: Edition type: str = 'Add' def action(self): ''' add obj to collection ''' - self.to_model() + target = resolve_remote_id(self.target, refresh=False) + # we want to related field that isn't the book, this is janky af sorry + model = [t for t in type(target)._meta.related_objects \ + if t.name != 'edition'][0].related_model + self.to_model(model=model) @dataclass(init=False) class AddBook(Add): '''Add activity that's aware of the book obj ''' object: Edition - type: str = 'Add' @dataclass(init=False) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 880c7600..f2465fb6 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -122,13 +122,12 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): return None related_model = self.related_model - if isinstance(value, dict) and value.get('id'): + if hasattr(value, 'id') and value.id: if not self.load_remote: # only look in the local database - return related_model.find_existing(value) + return related_model.find_existing(value.serialize()) # this is an activitypub object, which we can deserialize - activity_serializer = related_model.activity_serializer - return activity_serializer(**value).to_model() + return value.to_model(model=related_model) try: # make sure the value looks like a remote id validate_remote_id(value) @@ -139,7 +138,7 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): if not self.load_remote: # only look in the local database return related_model.find_existing_by_remote_id(value) - return activitypub.resolve_remote_id(related_model, value) + return activitypub.resolve_remote_id(value, model=related_model) class RemoteIdField(ActivitypubFieldMixin, models.CharField): @@ -280,7 +279,7 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): except ValidationError: continue items.append( - activitypub.resolve_remote_id(self.related_model, remote_id) + activitypub.resolve_remote_id(remote_id, model=self.related_model) ) return items @@ -317,7 +316,7 @@ class TagField(ManyToManyField): # tags can contain multiple types continue items.append( - activitypub.resolve_remote_id(self.related_model, link.href) + activitypub.resolve_remote_id(link.href, model=self.related_model) ) return items diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index cfd0e8c0..e7ce4c55 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -566,10 +566,12 @@ class Inbox(TestCase): views.inbox.activity_task(activity) - def test_handle_add_book(self): + def test_handle_add_book_to_shelf(self): ''' shelving a book ''' + work = models.Work.objects.create(title='work title') book = models.Edition.objects.create( - title='Test', remote_id='https://bookwyrm.social/book/37292') + title='Test', remote_id='https://bookwyrm.social/book/37292', + parent_work=work) shelf = models.Shelf.objects.create( user=self.remote_user, name='Test Shelf') shelf.remote_id = 'https://bookwyrm.social/user/mouse/shelf/to-read' @@ -581,13 +583,15 @@ class Inbox(TestCase): "actor": "https://example.com/users/rat", "object": { "type": "Edition", + "title": "Test Title", + "work": work.remote_id, "id": "https://bookwyrm.social/book/37292", }, "target": "https://bookwyrm.social/user/mouse/shelf/to-read", "@context": "https://www.w3.org/ns/activitystreams" } - #views.inbox.activity_task(activity) - #self.assertEqual(shelf.books.first(), book) + views.inbox.activity_task(activity) + self.assertEqual(shelf.books.first(), book) def test_handle_update_user(self):