Merge pull request #1304 from bookwyrm-social/prevent-import-overwrite

Prevent overwriting data on import form outside data source

bookwyrm/activitypub/base_activity.py

@@ -106,8 +106,10 @@ 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):
+    # pylint: disable=too-many-locals,too-many-branches,too-many-arguments
+    def to_model(
+        self, model=None, instance=None, allow_create=True, save=True, overwrite=True
+    ):
         """convert from an activity to a model instance"""
         model = model or get_model_from_type(self.type)
 
@@ -129,9 +131,12 @@ class ActivityObject:
 
         # keep track of what we've changed
         update_fields = []
+        # sets field on the model using the activity value
         for field in instance.simple_fields:
             try:
-                changed = field.set_field_from_activity(instance, self)
+                changed = field.set_field_from_activity(
+                    instance, self, overwrite=overwrite
+                )
                 if changed:
                     update_fields.append(field.name)
             except AttributeError as e:
@@ -140,7 +145,9 @@ class ActivityObject:
         # 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:
-            changed = field.set_field_from_activity(instance, self, save=save)
+            changed = field.set_field_from_activity(
+                instance, self, save=save, overwrite=overwrite
+            )
             if changed:
                 update_fields.append(field.name)
 

bookwyrm/connectors/abstract_connector.py

@@ -139,7 +139,7 @@ class AbstractConnector(AbstractMinimalConnector):
                 **dict_from_mappings(work_data, self.book_mappings)
             )
             # this will dedupe automatically
-            work = work_activity.to_model(model=models.Work)
+            work = work_activity.to_model(model=models.Work, overwrite=False)
             for author in self.get_authors_from_data(work_data):
                 work.authors.add(author)
 
@@ -156,7 +156,7 @@ class AbstractConnector(AbstractMinimalConnector):
         mapped_data = dict_from_mappings(edition_data, self.book_mappings)
         mapped_data["work"] = work.remote_id
         edition_activity = activitypub.Edition(**mapped_data)
-        edition = edition_activity.to_model(model=models.Edition)
+        edition = edition_activity.to_model(model=models.Edition, overwrite=False)
         edition.connector = self.connector
         edition.save()
 
@@ -182,7 +182,7 @@ class AbstractConnector(AbstractMinimalConnector):
             return None
 
         # this will dedupe
-        return activity.to_model(model=models.Author)
+        return activity.to_model(model=models.Author, overwrite=False)
 
     @abstractmethod
     def is_work_data(self, data):

bookwyrm/models/fields.py

@@ -66,7 +66,7 @@ class ActivitypubFieldMixin:
             self.activitypub_field = activitypub_field
         super().__init__(*args, **kwargs)
 
-    def set_field_from_activity(self, instance, data):
+    def set_field_from_activity(self, instance, data, overwrite=True):
         """helper function for assinging a value to the field. Returns if changed"""
         try:
             value = getattr(data, self.get_activitypub_field())
@@ -79,8 +79,15 @@ class ActivitypubFieldMixin:
         if formatted is None or formatted is MISSING or formatted == {}:
             return False
 
+        current_value = (
+            getattr(instance, self.name) if hasattr(instance, self.name) else None
+        )
+        # if we're not in overwrite mode, only continue updating the field if its unset
+        if current_value and not overwrite:
+            return False
+
         # the field is unchanged
-        if hasattr(instance, self.name) and getattr(instance, self.name) == formatted:
+        if current_value == formatted:
             return False
 
         setattr(instance, self.name, formatted)
@@ -210,7 +217,10 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField):
         )
 
     # pylint: disable=invalid-name
-    def set_field_from_activity(self, instance, data):
+    def set_field_from_activity(self, instance, data, overwrite=True):
+        if not overwrite:
+            return False
+
         original = getattr(instance, self.name)
         to = data.to
         cc = data.cc
@@ -273,8 +283,11 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField):
         self.link_only = link_only
         super().__init__(*args, **kwargs)
 
-    def set_field_from_activity(self, instance, data):
+    def set_field_from_activity(self, instance, data, overwrite=True):
         """helper function for assinging a value to the field"""
+        if not overwrite and getattr(instance, self.name).exists():
+            return False
+
         value = getattr(data, self.get_activitypub_field())
         formatted = self.field_from_activity(value)
         if formatted is None or formatted is MISSING:
@@ -377,13 +390,16 @@ class ImageField(ActivitypubFieldMixin, models.ImageField):
         super().__init__(*args, **kwargs)
 
     # pylint: disable=arguments-differ
-    def set_field_from_activity(self, instance, data, save=True):
+    def set_field_from_activity(self, instance, data, save=True, overwrite=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 False
 
+        if not overwrite and hasattr(instance, self.name):
+            return False
+
         getattr(instance, self.name).save(*formatted, save=save)
         return True