Fixes serializer handling default dataclass fields

bookwyrm/activitypub/base_activity.py

@@ -74,7 +74,8 @@ class ActivityObject:
             try:
                 value = kwargs[field.name]
             except KeyError:
-                if field.default == MISSING:
+                if field.default == MISSING and \
+                        field.default_factory == MISSING:
                     raise ActivitySerializerError(\
                             'Missing required field: %s' % field.name)
                 value = field.default
@@ -143,6 +144,8 @@ class ActivityObject:
 
         # add images
         for (model_key, value) in image_fields.items():
+            if not value:
+                continue
             getattr(instance, model_key).save(*value, save=True)
 
         # add one to many fields
@@ -188,6 +191,8 @@ def resolve_foreign_key(model, remote_id):
 
 def tag_formatter(tags, tag_type):
     ''' helper function to extract foreign keys from tag activity json '''
+    if not isinstance(tags, list):
+        return []
     items = []
     types = {
         'Book': models.Book,
@@ -207,9 +212,9 @@ def tag_formatter(tags, tag_type):
 
 def image_formatter(image_json):
     ''' helper function to load images and format them for a model '''
-    url = image_json.get('url')
+    if not image_json or not hasattr(image_json, 'url'):
-    if not url:
         return None
+    url = image_json.get('url')
 
     try:
         response = requests.get(url)

bookwyrm/models/base_model.py

@@ -59,24 +59,31 @@ class ActivitypubMixin:
     def to_activity(self, pure=False):
         ''' convert from a model to an activity '''
         if pure:
+            # works around bookwyrm-specific fields for vanilla AP services
             mappings = self.pure_activity_mappings
         else:
+            # may include custom fields that bookwyrm instances will understand
             mappings = self.activity_mappings
 
         fields = {}
         for mapping in mappings:
             if not hasattr(self, mapping.model_key) or not mapping.activity_key:
+                # this field on the model isn't serialized
                 continue
             value = getattr(self, mapping.model_key)
             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
-            if isinstance(value, datetime):
+            elif isinstance(value, datetime):
                 value = value.isoformat()
+
+            # run the custom formatter function set in the model
             result = mapping.activity_formatter(value)
             if mapping.activity_key in fields and \
                     isinstance(fields[mapping.activity_key], list):
-                # there are two database fields that map to the same AP list
+                # there can be two database fields that map to the same AP list
-                # this happens in status, which combines user and book tags
+                # this happens in status tags, which combines user and book tags
                 fields[mapping.activity_key] += result
             else:
                 fields[mapping.activity_key] = result
@@ -265,7 +272,7 @@ def tag_formatter(items, name_field, activity_type):
 
 def image_formatter(image, default_path=None):
     ''' convert images into activitypub json '''
-    if image:
+    if image and hasattr(image, 'url'):
         url = image.url
     elif default_path:
         url = default_path