Track changed fields in activity to model code

bookwyrm/activitypub/base_activity.py

@@ -106,6 +106,7 @@ class ActivityObject:
                 value = field.default
             setattr(self, field.name, value)
 
+    # pylint: disable=too-many-locals,too-many-branches
     def to_model(self, model=None, instance=None, allow_create=True, save=True):
         """convert from an activity to a model instance"""
         model = model or get_model_from_type(self.type)
@@ -126,27 +127,36 @@ class ActivityObject:
             return None
         instance = instance or model()
 
+        # keep track of what we've changed
+        update_fields = []
         for field in instance.simple_fields:
             try:
-                field.set_field_from_activity(instance, self)
+                changed = field.set_field_from_activity(instance, self)
+                if changed:
+                    update_fields.append(field.name)
             except AttributeError as e:
                 raise ActivitySerializerError(e)
 
         # image fields have to be set after other fields because they can save
         # too early and jank up users
         for field in instance.image_fields:
-            field.set_field_from_activity(instance, self, save=save)
+            changed = field.set_field_from_activity(instance, self, save=save)
+            if changed:
+                update_fields.append(field.name)
 
         if not save:
             return instance
 
         with transaction.atomic():
+            # can't force an update on fields unless the object already exists in the db
+            if not instance.id:
+                update_fields = None
             # we can't set many to many and reverse fields on an unsaved object
             try:
                 try:
-                    instance.save(broadcast=False)
+                    instance.save(broadcast=False, update_fields=update_fields)
                 except TypeError:
-                    instance.save()
+                    instance.save(update_fields=update_fields)
             except IntegrityError as e:
                 raise ActivitySerializerError(e)
 

bookwyrm/models/fields.py

@@ -67,7 +67,7 @@ class ActivitypubFieldMixin:
         super().__init__(*args, **kwargs)
 
     def set_field_from_activity(self, instance, data):
-        """helper function for assinging a value to the field"""
+        """helper function for assinging a value to the field. Returns if changed"""
         try:
             value = getattr(data, self.get_activitypub_field())
         except AttributeError:
@@ -77,8 +77,14 @@ class ActivitypubFieldMixin:
             value = getattr(data, "actor")
         formatted = self.field_from_activity(value)
         if formatted is None or formatted is MISSING or formatted == {}:
-            return
+            return False
+
+        # the field is unchanged
+        if getattr(instance, self.name) == formatted:
+            return False
+
         setattr(instance, self.name, formatted)
+        return True
 
     def set_activity_from_field(self, activity, instance):
         """update the json object"""
@@ -205,6 +211,7 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField):
 
     # pylint: disable=invalid-name
     def set_field_from_activity(self, instance, data):
+        original = getattr(instance, self.name)
         to = data.to
         cc = data.cc
         if to == [self.public]:
@@ -215,6 +222,7 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField):
             setattr(instance, self.name, "unlisted")
         else:
             setattr(instance, self.name, "followers")
+        return original == getattr(instance, self.name)
 
     def set_activity_from_field(self, activity, instance):
         # explicitly to anyone mentioned (statuses only)
@@ -270,9 +278,10 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField):
         value = getattr(data, self.get_activitypub_field())
         formatted = self.field_from_activity(value)
         if formatted is None or formatted is MISSING:
-            return
+            return False
         getattr(instance, self.name).set(formatted)
         instance.save(broadcast=False)
+        return True
 
     def field_to_activity(self, value):
         if self.link_only:
@@ -373,8 +382,10 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
         value = getattr(data, self.get_activitypub_field())
         formatted = self.field_from_activity(value)
         if formatted is None or formatted is MISSING:
-            return
+            return False
+
         getattr(instance, self.name).save(*formatted, save=save)
+        return True
 
     def set_activity_from_field(self, activity, instance):
         value = getattr(instance, self.name)

bookwyrm/tests/activitypub/test_author.py

@@ -1,6 +1,4 @@
-import datetime
-
-from unittest.mock import patch
+"""test author serializer"""
 from django.test import TestCase
 from bookwyrm import models
 

bookwyrm/tests/activitypub/test_base_activity.py

@@ -20,16 +20,18 @@ from bookwyrm import models
 
 
 @patch("bookwyrm.activitystreams.ActivityStream.add_status")
+@patch("bookwyrm.suggested_users.rerank_user_task.delay")
 class BaseActivity(TestCase):
     """the super class for model-linked activitypub dataclasses"""
 
     def setUp(self):
         """we're probably going to re-use this so why copy/paste"""
-        self.user = models.User.objects.create_user(
-            "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse"
-        )
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"):
+            self.user = models.User.objects.create_user(
+                "mouse", "mouse@mouse.mouse", "mouseword", local=True, localname="mouse"
+            )
         self.user.remote_id = "http://example.com/a/b"
-        self.user.save(broadcast=False)
+        self.user.save(broadcast=False, update_fields=["remote_id"])
 
         datafile = pathlib.Path(__file__).parent.joinpath("../data/ap_user.json")
         self.userdata = json.loads(datafile.read_bytes())
@@ -44,24 +46,24 @@ class BaseActivity(TestCase):
         image.save(output, format=image.format)
         self.image_data = output.getvalue()
 
-    def test_init(self, _):
+    def test_init(self, *_):
         """simple successfuly init"""
         instance = ActivityObject(id="a", type="b")
         self.assertTrue(hasattr(instance, "id"))
         self.assertTrue(hasattr(instance, "type"))
 
-    def test_init_missing(self, _):
+    def test_init_missing(self, *_):
         """init with missing required params"""
         with self.assertRaises(ActivitySerializerError):
             ActivityObject()
 
-    def test_init_extra_fields(self, _):
+    def test_init_extra_fields(self, *_):
         """init ignoring additional fields"""
         instance = ActivityObject(id="a", type="b", fish="c")
         self.assertTrue(hasattr(instance, "id"))
         self.assertTrue(hasattr(instance, "type"))
 
-    def test_init_default_field(self, _):
+    def test_init_default_field(self, *_):
         """replace an existing required field with a default field"""
 
         @dataclass(init=False)
@@ -74,7 +76,7 @@ class BaseActivity(TestCase):
         self.assertEqual(instance.id, "a")
         self.assertEqual(instance.type, "TestObject")
 
-    def test_serialize(self, _):
+    def test_serialize(self, *_):
         """simple function for converting dataclass to dict"""
         instance = ActivityObject(id="a", type="b")
         serialized = instance.serialize()
@@ -83,7 +85,7 @@ class BaseActivity(TestCase):
         self.assertEqual(serialized["type"], "b")
 
     @responses.activate
-    def test_resolve_remote_id(self, _):
+    def test_resolve_remote_id(self, *_):
         """look up or load remote data"""
         # existing item
         result = resolve_remote_id("http://example.com/a/b", model=models.User)
@@ -105,14 +107,14 @@ class BaseActivity(TestCase):
         self.assertEqual(result.remote_id, "https://example.com/user/mouse")
         self.assertEqual(result.name, "MOUSE?? MOUSE!!")
 
-    def test_to_model_invalid_model(self, _):
+    def test_to_model_invalid_model(self, *_):
         """catch mismatch between activity type and model type"""
         instance = ActivityObject(id="a", type="b")
         with self.assertRaises(ActivitySerializerError):
             instance.to_model(model=models.User)
 
     @responses.activate
-    def test_to_model_image(self, _):
+    def test_to_model_image(self, *_):
         """update an image field"""
         activity = activitypub.Person(
             id=self.user.remote_id,
@@ -145,7 +147,7 @@ class BaseActivity(TestCase):
         self.assertEqual(self.user.name, "New Name")
         self.assertEqual(self.user.key_pair.public_key, "hi")
 
-    def test_to_model_many_to_many(self, _):
+    def test_to_model_many_to_many(self, *_):
         """annoying that these all need special handling"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             status = models.Status.objects.create(
@@ -176,7 +178,7 @@ class BaseActivity(TestCase):
         self.assertEqual(status.mention_books.first(), book)
 
     @responses.activate
-    def test_to_model_one_to_many(self, _):
+    def test_to_model_one_to_many(self, *_):
         """these are reversed relationships, where the secondary object
         keys the primary object but not vice versa"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
@@ -215,7 +217,7 @@ class BaseActivity(TestCase):
         self.assertIsNone(status.attachments.first())
 
     @responses.activate
-    def test_set_related_field(self, _):
+    def test_set_related_field(self, *_):
         """celery task to add back-references to created objects"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             status = models.Status.objects.create(

bookwyrm/tests/test_emailing.py

@@ -14,13 +14,14 @@ class Emailing(TestCase):
     def setUp(self):
         """we need basic test data and mocks"""
         self.factory = RequestFactory()
-        self.local_user = models.User.objects.create_user(
-            "mouse@local.com",
-            "mouse@mouse.mouse",
-            "password",
-            local=True,
-            localname="mouse",
-        )
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@local.com",
+                "mouse@mouse.mouse",
+                "password",
+                local=True,
+                localname="mouse",
+            )
         models.SiteSettings.objects.create()
 
     def test_invite_email(self, email_mock):

bookwyrm/tests/test_preview_images.py

@@ -1,5 +1,6 @@
 """ test generating preview images """
 import pathlib
+from unittest.mock import patch
 from PIL import Image
 
 from django.test import TestCase
@@ -8,7 +9,6 @@ from django.core.files.uploadedfile import SimpleUploadedFile
 from django.db.models.fields.files import ImageFieldFile
 
 from bookwyrm import models, settings
-
 from bookwyrm.preview_images import (
     generate_site_preview_image_task,
     generate_edition_preview_image_task,
@@ -29,18 +29,19 @@ class PreviewImages(TestCase):
         avatar_file = pathlib.Path(__file__).parent.joinpath(
             "../static/images/no_cover.jpg"
         )
-        self.local_user = models.User.objects.create_user(
-            "possum@local.com",
-            "possum@possum.possum",
-            "password",
-            local=True,
-            localname="possum",
-            avatar=SimpleUploadedFile(
-                avatar_file,
-                open(avatar_file, "rb").read(),
-                content_type="image/jpeg",
-            ),
-        )
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "possum@local.com",
+                "possum@possum.possum",
+                "password",
+                local=True,
+                localname="possum",
+                avatar=SimpleUploadedFile(
+                    avatar_file,
+                    open(avatar_file, "rb").read(),
+                    content_type="image/jpeg",
+                ),
+            )
 
         self.work = models.Work.objects.create(title="Test Work")
         self.edition = models.Edition.objects.create(

bookwyrm/tests/test_signing.py

@@ -37,19 +37,20 @@ class Signature(TestCase):
 
     def setUp(self):
         """create users and test data"""
-        self.mouse = models.User.objects.create_user(
-            "mouse@%s" % DOMAIN,
-            "mouse@example.com",
-            "",
-            local=True,
-            localname="mouse",
-        )
-        self.rat = models.User.objects.create_user(
-            "rat@%s" % DOMAIN, "rat@example.com", "", local=True, localname="rat"
-        )
-        self.cat = models.User.objects.create_user(
-            "cat@%s" % DOMAIN, "cat@example.com", "", local=True, localname="cat"
-        )
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"):
+            self.mouse = models.User.objects.create_user(
+                "mouse@%s" % DOMAIN,
+                "mouse@example.com",
+                "",
+                local=True,
+                localname="mouse",
+            )
+            self.rat = models.User.objects.create_user(
+                "rat@%s" % DOMAIN, "rat@example.com", "", local=True, localname="rat"
+            )
+            self.cat = models.User.objects.create_user(
+                "cat@%s" % DOMAIN, "cat@example.com", "", local=True, localname="cat"
+            )
 
         private_key, public_key = create_key_pair()
 

bookwyrm/tests/test_templatetags.py

@@ -22,13 +22,14 @@ class TemplateTags(TestCase):
 
     def setUp(self):
         """create some filler objects"""
-        self.user = models.User.objects.create_user(
-            "mouse@example.com",
-            "mouse@mouse.mouse",
-            "mouseword",
-            local=True,
-            localname="mouse",
-        )
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"):
+            self.user = models.User.objects.create_user(
+                "mouse@example.com",
+                "mouse@mouse.mouse",
+                "mouseword",
+                local=True,
+                localname="mouse",
+            )
         with patch("bookwyrm.models.user.set_remote_server.delay"):
             self.remote_user = models.User.objects.create_user(
                 "rat",