Create helper function on field for settings values

bookwyrm/activitypub/base_activity.py

@@ -4,8 +4,6 @@ from json import JSONEncoder
 
 from django.apps import apps
 from django.db import transaction
-from django.db.models.fields.files import ImageFileDescriptor
-from django.db.models.fields.related_descriptors import ManyToManyDescriptor
 
 from bookwyrm.connectors import ConnectorException, get_data
 from bookwyrm.tasks import app
@@ -77,55 +75,30 @@ class ActivityObject:
             )
 
         # check for an existing instance, if we're not updating a known obj
-        if not instance:
+        instance = instance or model.find_existing(self.serialize()) or model()
-            instance = model.find_existing(self.serialize()) or model()
 
-        many_to_many_fields = {}
+        for field in instance.simple_fields:
-        image_fields = {}
+            field.set_field_from_activity(instance, self)
-        for field in model._meta.get_fields():
-            # check if it's an activitypub field
-            if not hasattr(field, 'field_to_activity'):
-                continue
-            # 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)
+        # image fields have to be set after other fields because they can save
-
+        # too early and jank up users
-            if isinstance(model_field, ManyToManyDescriptor):
+        for field in instance.image_fields:
-                # status mentions book/users for example, stash this for later
+            field.set_field_from_activity(instance, self, save=save)
-                many_to_many_fields[field.name] = value
-            elif isinstance(model_field, ImageFileDescriptor):
-                # image fields need custom handling
-                image_fields[field.name] = value
-            else:
-                # just a good old fashioned model.field = value
-                setattr(instance, field.name, value)
-
-        # if this isn't here, it messes up saving users. who even knows.
-        for (model_key, value) in image_fields.items():
-            getattr(instance, model_key).save(*value, save=save)
 
         if not save:
-            # we can't set many to many and reverse fields on an unsaved object
             return instance
 
+        # we can't set many to many and reverse fields on an unsaved object
         instance.save()
 
         # add many to many fields, which have to be set post-save
-        for (model_key, values) in many_to_many_fields.items():
+        for field in instance.many_to_many_fields:
             # mention books/users, for example
-            getattr(instance, model_key).set(values)
+            field.set_field_from_activity(instance, self)
-
-        if not save or 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:
+                instance.deserialize_reverse_fields:
             # attachments on Status, for example
             values = getattr(self, activity_field_name)
             if values is None or values is MISSING:

bookwyrm/models/base_model.py

@@ -10,13 +10,11 @@ from Crypto.Hash import SHA256
 from django.core.paginator import Paginator
 from django.db import models
 from django.db.models import Q
-from django.db.models.fields.files import ImageFileDescriptor
-from django.db.models.fields.related_descriptors import ManyToManyDescriptor
 from django.dispatch import receiver
 
 from bookwyrm import activitypub
 from bookwyrm.settings import DOMAIN, PAGE_LENGTH
-from .fields import RemoteIdField
+from .fields import ImageField, ManyToManyField, RemoteIdField
 
 
 PrivacyLevels = models.TextChoices('Privacy', [
@@ -79,9 +77,9 @@ class ActivitypubMixin:
             if not hasattr(field, 'field_to_activity'):
                 continue
 
-            if isinstance(field, ImageFileDescriptor):
+            if isinstance(field, ImageField):
                 self.image_fields.append(field)
-            elif isinstance(field, ManyToManyDescriptor):
+            elif isinstance(field, ManyToManyField):
                 self.many_to_many_fields.append(field)
             else:
                 self.simple_fields.append(field)

bookwyrm/models/fields.py

@@ -1,4 +1,5 @@
 ''' activitypub-aware django model fields '''
+from dataclasses import MISSING
 import re
 from uuid import uuid4
 
@@ -38,6 +39,16 @@ class ActivitypubFieldMixin:
             self.activitypub_field = activitypub_field
         super().__init__(*args, **kwargs)
 
+
+    def set_field_from_activity(self, instance, data):
+        ''' helper function for assinging a value to the field '''
+        value = getattr(data, self.get_activitypub_field())
+        formatted = self.field_from_activity(value)
+        if formatted is None or formatted is MISSING:
+            return
+        setattr(instance, self.name, formatted)
+
+
     def field_to_activity(self, value):
         ''' formatter to convert a model value into activitypub '''
         if hasattr(self, 'activitypub_wrapper'):
@@ -145,6 +156,14 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField):
         self.link_only = link_only
         super().__init__(*args, **kwargs)
 
+    def set_field_from_activity(self, instance, data):
+        ''' helper function for assinging a value to the field '''
+        value = getattr(data, self.get_activitypub_field())
+        formatted = self.field_from_activity(value)
+        if formatted is None or formatted is MISSING:
+            return
+        getattr(instance, self.name).set(formatted)
+
     def field_to_activity(self, value):
         if self.link_only:
             return '%s/%s' % (value.instance.remote_id, self.name)
@@ -210,9 +229,20 @@ def image_serializer(value):
 
 class ImageField(ActivitypubFieldMixin, models.ImageField):
     ''' activitypub-aware image field '''
+    # pylint: disable=arguments-differ
+    def set_field_from_activity(self, instance, data, save=True):
+        ''' helper function for assinging a value to the field '''
+        value = getattr(data, self.get_activitypub_field())
+        formatted = self.field_from_activity(value)
+        if formatted is None or formatted is MISSING:
+            return
+        getattr(instance, self.name).save(*formatted, save=save)
+
+
     def field_to_activity(self, value):
         return image_serializer(value)
 
+
     def field_from_activity(self, value):
         image_slug = value
         # when it's an inline image (User avatar/icon, Book cover), it's a json