Starts getting reverse fields working for deserialization

also fixes the fields on the image model and runs a long overdue
migration

bookwyrm/activitypub/base_activity.py

@@ -76,77 +76,68 @@ class ActivityObject:
 
         # check for an existing instance, if we're not updating a known obj
         if not instance:
-            instance = find_existing_by_remote_id(model, self.id)
+            instance = find_existing_by_remote_id(model, self.id) or model()
 	# TODO: deduplicate books by identifiers
 
-        mapped_fields = {}
         many_to_many_fields = {}
-        one_to_many_fields = {}
-        image_fields = {}
-
         for field in model._meta.get_fields():
             if not hasattr(field, 'field_to_activity'):
                 continue
-            activitypub_field = field.get_activitypub_field()
-            value = field.field_from_activity(getattr(self, activitypub_field))
-            if value is None:
+            # call the formatter associated with the model field class
+            value = field.field_from_activity(
+                getattr(self, field.get_activitypub_field())
+            )
+            if value is None or value is MISSING:
                 continue
 
             model_field = getattr(model, field.name)
 
-            if isinstance(model_field, ForwardManyToOneDescriptor):
-                mapped_fields[field.name] = value
             if isinstance(model_field, ManyToManyDescriptor):
-                # status mentions book/users
+                # status mentions book/users for example, stash this for later
                 many_to_many_fields[field.name] = value
-            elif isinstance(model_field, ReverseManyToOneDescriptor):
-                # attachments on Status, for example
-                one_to_many_fields[field.name] = value
             elif isinstance(model_field, ImageFileDescriptor):
                 # image fields need custom handling
-                image_fields[field.name] = value
+                getattr(instance, field.name).save(*value)
             else:
-                if value == MISSING:
-                    value = None
-                mapped_fields[field.name] = value
+                # just a good old fashioned model.field = value
+                setattr(instance, field.name, value)
 
-        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)
+        instance.save()
 
-        # --- 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():
-            if not value:
-                continue
-            getattr(instance, model_key).save(*value, save=True)
-
-        # add many to many fields
+        # add many to many fields, which have to be set post-save
         for (model_key, values) in many_to_many_fields.items():
             # mention books, mention users, followers
             getattr(instance, model_key).set(values)
 
-        # add one to many fields
-        for (model_key, values) in one_to_many_fields.items():
-            if values == MISSING:
+        if not hasattr(model, 'deserialize_reverse_fields'):
+            return instance
+
+        # reversed relationships in the models
+        for (model_field_name, activity_field_name) in \
+                model.deserialize_reverse_fields:
+            if not activity_field_name:
                 continue
-            model_field = getattr(instance, model_key)
-            related_model = model_field.model
+            # attachments on Status, for example
+            values = getattr(self, activity_field_name)
+            if values is None or values is MISSING:
+                continue
+            try:
+                # this is for one to many
+                related_model = getattr(model, model_field_name).field.model
+            except AttributeError:
+                # it's a one to one or foreign key
+                related_model = getattr(model, model_field_name)\
+                        .related.related_model
+                values = [values]
+
             for item in values:
                 if isinstance(item, str):
                     item = resolve_remote_id(related_model, item)
                 else:
                     item = related_model.activity_serializer(**item)
                     item = item.to_model(related_model)
-                field_name = instance.__class__.__name__.lower()
-                setattr(item, field_name, instance)
+                related_name = instance.__class__.__name__.lower()
+                setattr(item, related_name, instance)
                 item.save()
 
         return instance

bookwyrm/migrations/0020_auto_20201208_0213.py

@@ -0,0 +1,353 @@
+# Generated by Django 3.0.7 on 2020-12-08 02:13
+
+import bookwyrm.models.fields
+from django.conf import settings
+import django.core.validators
+from django.db import migrations, models
+import django.db.models.deletion
+import django.utils.timezone
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('bookwyrm', '0019_auto_20201130_1939'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='author',
+            name='aliases',
+            field=bookwyrm.models.fields.ArrayField(base_field=models.CharField(max_length=255), blank=True, default=list, size=None),
+        ),
+        migrations.AlterField(
+            model_name='author',
+            name='bio',
+            field=bookwyrm.models.fields.TextField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='author',
+            name='born',
+            field=bookwyrm.models.fields.DateTimeField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='author',
+            name='died',
+            field=bookwyrm.models.fields.DateTimeField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='author',
+            name='name',
+            field=bookwyrm.models.fields.CharField(max_length=255),
+        ),
+        migrations.AlterField(
+            model_name='author',
+            name='openlibrary_key',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='author',
+            name='wikipedia_link',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='authors',
+            field=bookwyrm.models.fields.ManyToManyField(to='bookwyrm.Author'),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='cover',
+            field=bookwyrm.models.fields.ImageField(blank=True, null=True, upload_to='covers/'),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='description',
+            field=bookwyrm.models.fields.TextField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='first_published_date',
+            field=bookwyrm.models.fields.DateTimeField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='goodreads_key',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='languages',
+            field=bookwyrm.models.fields.ArrayField(base_field=models.CharField(max_length=255), blank=True, default=list, size=None),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='librarything_key',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='openlibrary_key',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='published_date',
+            field=bookwyrm.models.fields.DateTimeField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='series',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='series_number',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='sort_title',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='subject_places',
+            field=bookwyrm.models.fields.ArrayField(base_field=models.CharField(max_length=255), blank=True, default=list, null=True, size=None),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='subjects',
+            field=bookwyrm.models.fields.ArrayField(base_field=models.CharField(max_length=255), blank=True, default=list, null=True, size=None),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='subtitle',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='book',
+            name='title',
+            field=bookwyrm.models.fields.CharField(max_length=255),
+        ),
+        migrations.AlterField(
+            model_name='boost',
+            name='boosted_status',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='boosters', to='bookwyrm.Status'),
+        ),
+        migrations.AlterField(
+            model_name='comment',
+            name='book',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='asin',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='isbn_10',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='isbn_13',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='oclc_number',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='pages',
+            field=bookwyrm.models.fields.IntegerField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='parent_work',
+            field=bookwyrm.models.fields.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, related_name='editions', to='bookwyrm.Work'),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='physical_format',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='edition',
+            name='publishers',
+            field=bookwyrm.models.fields.ArrayField(base_field=models.CharField(max_length=255), blank=True, default=list, size=None),
+        ),
+        migrations.AlterField(
+            model_name='favorite',
+            name='status',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Status'),
+        ),
+        migrations.AlterField(
+            model_name='favorite',
+            name='user',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='image',
+            name='caption',
+            field=bookwyrm.models.fields.TextField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='image',
+            name='image',
+            field=bookwyrm.models.fields.ImageField(blank=True, null=True, upload_to='status/'),
+        ),
+        migrations.AlterField(
+            model_name='quotation',
+            name='book',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='quotation',
+            name='quote',
+            field=bookwyrm.models.fields.TextField(),
+        ),
+        migrations.AlterField(
+            model_name='review',
+            name='book',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='review',
+            name='name',
+            field=bookwyrm.models.fields.CharField(max_length=255, null=True),
+        ),
+        migrations.AlterField(
+            model_name='review',
+            name='rating',
+            field=bookwyrm.models.fields.IntegerField(blank=True, default=None, null=True, validators=[django.core.validators.MinValueValidator(1), django.core.validators.MaxValueValidator(5)]),
+        ),
+        migrations.AlterField(
+            model_name='shelf',
+            name='name',
+            field=bookwyrm.models.fields.CharField(max_length=100),
+        ),
+        migrations.AlterField(
+            model_name='shelf',
+            name='privacy',
+            field=bookwyrm.models.fields.CharField(choices=[('public', 'Public'), ('unlisted', 'Unlisted'), ('followers', 'Followers'), ('direct', 'Direct')], default='public', max_length=255),
+        ),
+        migrations.AlterField(
+            model_name='shelf',
+            name='user',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='shelfbook',
+            name='added_by',
+            field=bookwyrm.models.fields.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='shelfbook',
+            name='book',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='shelfbook',
+            name='shelf',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Shelf'),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='content',
+            field=bookwyrm.models.fields.TextField(blank=True, null=True),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='mention_books',
+            field=bookwyrm.models.fields.TagField(related_name='mention_book', to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='mention_users',
+            field=bookwyrm.models.fields.TagField(related_name='mention_user', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='published_date',
+            field=bookwyrm.models.fields.DateTimeField(default=django.utils.timezone.now),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='reply_parent',
+            field=bookwyrm.models.fields.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Status'),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='sensitive',
+            field=bookwyrm.models.fields.BooleanField(default=False),
+        ),
+        migrations.AlterField(
+            model_name='status',
+            name='user',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='tag',
+            name='name',
+            field=bookwyrm.models.fields.CharField(max_length=100, unique=True),
+        ),
+        migrations.AlterField(
+            model_name='userblocks',
+            name='user_object',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='userblocks_user_object', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='userblocks',
+            name='user_subject',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='userblocks_user_subject', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='userfollowrequest',
+            name='user_object',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='userfollowrequest_user_object', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='userfollowrequest',
+            name='user_subject',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='userfollowrequest_user_subject', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='userfollows',
+            name='user_object',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='userfollows_user_object', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='userfollows',
+            name='user_subject',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='userfollows_user_subject', to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='usertag',
+            name='book',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='usertag',
+            name='tag',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Tag'),
+        ),
+        migrations.AlterField(
+            model_name='usertag',
+            name='user',
+            field=bookwyrm.models.fields.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL),
+        ),
+        migrations.AlterField(
+            model_name='work',
+            name='default_edition',
+            field=bookwyrm.models.fields.ForeignKey(null=True, on_delete=django.db.models.deletion.PROTECT, to='bookwyrm.Edition'),
+        ),
+        migrations.AlterField(
+            model_name='work',
+            name='lccn',
+            field=bookwyrm.models.fields.CharField(blank=True, max_length=255, null=True),
+        ),
+    ]

bookwyrm/models/attachment.py

@@ -9,7 +9,7 @@ from . import fields
 
 class Attachment(ActivitypubMixin, BookWyrmModel):
     ''' an image (or, in the future, video etc) associated with a status '''
-    status = fields.ForeignKey(
+    status = models.ForeignKey(
         'Status',
         on_delete=models.CASCADE,
         related_name='attachments',
@@ -23,7 +23,8 @@ class Attachment(ActivitypubMixin, BookWyrmModel):
 
 class Image(Attachment):
     ''' an image attachment '''
-    image = fields.ImageField(upload_to='status/', null=True, blank=True)
-    caption = fields.TextField(null=True, blank=True)
+    image = fields.ImageField(
+        upload_to='status/', null=True, blank=True, activitypub_field='url')
+    caption = fields.TextField(null=True, blank=True, activitypub_field='name')
 
     activity_serializer = activitypub.Image

bookwyrm/models/base_model.py

@@ -83,9 +83,11 @@ class ActivitypubMixin:
 
         if hasattr(self, 'serialize_reverse_fields'):
             # for example, editions of a work
-            for field_name in self.serialize_reverse_fields:
-                related_field = getattr(self, field_name)
-                activity[field_name] = unfurl_related_field(related_field)
+            for model_field_name, activity_field_name in \
+                    self.serialize_reverse_fields:
+                related_field = getattr(self, model_field_name)
+                activity[activity_field_name] = \
+                        unfurl_related_field(related_field)
 
         if not activity.get('id'):
             activity['id'] = self.get_remote_id()

bookwyrm/models/book.py

@@ -96,7 +96,8 @@ class Work(OrderedCollectionPageMixin, Book):
         return self.default_edition or self.editions.first()
 
     activity_serializer = activitypub.Work
-    serialize_reverse_fields = ['editions']
+    serialize_reverse_fields = [('editions', 'editions')]
+    deserialize_reverse_fields = [('editions', 'editions')]
 
 
 class Edition(Book):

bookwyrm/models/status.py

@@ -45,7 +45,8 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
     objects = InheritanceManager()
 
     activity_serializer = activitypub.Note
-    serialize_reverse_fields = ['attachments']
+    serialize_reverse_fields = [('attachments', 'attachment')]
+    deserialize_reverse_fields = [('attachments', 'attachment')]
 
     #----- replies collection activitypub ----#
     @classmethod

bookwyrm/models/user.py

@@ -166,7 +166,7 @@ class KeyPair(ActivitypubMixin, BookWyrmModel):
         blank=True, null=True, activitypub_field='publicKeyPem')
 
     activity_serializer = activitypub.PublicKey
-    serialize_reverse_fields = ['owner']
+    serialize_reverse_fields = [('owner', 'owner')]
 
     def get_remote_id(self):
         # self.owner is set by the OneToOneField on User

bookwyrm/tests/activitypub/test_base_activity.py

@@ -34,6 +34,13 @@ class BaseActivity(TestCase):
         self.book = models.Edition.objects.create(
             title='Test Edition', remote_id='http://book.com/book')
 
+        image_file = pathlib.Path(__file__).parent.joinpath(
+            '../../static/images/default_avi.jpg')
+        image = Image.open(image_file)
+        output = BytesIO()
+        image.save(output, format=image.format)
+        self.image_data = output.getvalue()
+
     def test_init(self):
         ''' simple successfuly init '''
         instance = ActivityObject(id='a', type='b')
@@ -147,16 +154,10 @@ class BaseActivity(TestCase):
         ''' update an image field '''
         update_data = activitypub.Person(**self.user.to_activity())
         update_data.icon = {'url': 'http://www.example.com/image.jpg'}
-        image_file = pathlib.Path(__file__).parent.joinpath(
-            '../../static/images/default_avi.jpg')
-        image = Image.open(image_file)
-        output = BytesIO()
-        image.save(output, format=image.format)
-        image_data = output.getvalue()
         responses.add(
             responses.GET,
             'http://www.example.com/image.jpg',
-            body=image_data,
+            body=self.image_data,
             status=200)
 
         self.assertIsNone(self.user.avatar.name)
@@ -189,3 +190,28 @@ class BaseActivity(TestCase):
         update_data.to_model(models.Status, instance=status)
         self.assertEqual(status.mention_users.first(), self.user)
         self.assertEqual(status.mention_books.first(), self.book)
+
+
+    @responses.activate
+    def test_to_model_one_to_many(self):
+        ''' these are reversed relationships, where the secondary object
+        keys the primary object but not vice versa '''
+        status = models.Status.objects.create(
+            content='test status',
+            user=self.user,
+        )
+        update_data = activitypub.Note(**status.to_activity())
+        update_data.attachment = [{
+            'url': 'http://www.example.com/image.jpg',
+            'name': 'alt text',
+            'type': 'Image',
+        }]
+
+        responses.add(
+            responses.GET,
+            'http://www.example.com/image.jpg',
+            body=self.image_data,
+            status=200)
+
+        update_data.to_model(models.Status, instance=status)
+        self.assertIsInstance(status.attachments.first(), models.Image)