Fix creation of duplicate Editions

bookwyrm/activitypub/base_activity.py

@@ -120,6 +120,7 @@ def to_model(
         save: bool = True,
         overwrite: bool = True,
         allow_external_connections: bool = True,
+        trigger=None,
     ) -> Optional[TBookWyrmModel]:
         """convert from an activity to a model instance. Args:
         model: the django model that this object is being converted to
@@ -133,6 +134,9 @@ def to_model(
             only update blank fields if false
         allow_external_connections: look up missing data if true,
             throw an exception if false and an external connection is needed
+        trigger: the object that originally triggered this
+            self.to_model. e.g. if this is a Work being dereferenced from
+            an incoming Edition
         """
         model = model or get_model_from_type(self.type)
 
@@ -223,6 +227,8 @@ def to_model(
             related_field_name = model_field.field.name
 
             for item in values:
+                if trigger and item == trigger.remote_id:
+                    continue
                 set_related_field.delay(
                     related_model.__name__,
                     instance.__class__.__name__,

bookwyrm/activitypub/note.py

@@ -50,6 +50,7 @@ def to_model(
         save=True,
         overwrite=True,
         allow_external_connections=True,
+        trigger=None,
     ):
         instance = super().to_model(
             model, instance, allow_create, save, overwrite, allow_external_connections

bookwyrm/models/activitypub_mixin.py

@@ -129,7 +129,20 @@ def find_existing(cls, data):
 
     def broadcast(self, activity, sender, software=None, queue=BROADCAST):
         """send out an activity"""
+
+        # if we're posting about ShelfBooks, set a delay to give the base activity
+        # time to add the book on remote servers first to avoid race conditions
+        countdown = (
+            10
+            if (
+                isinstance(activity, object)
+                and not isinstance(activity["object"], str)
+                and activity["object"].get("type", None) in ["GeneratedNote", "Comment"]
+            )
+            else 0
+        )
         broadcast_task.apply_async(
+            countdown=countdown,
             args=(
                 sender.id,
                 json.dumps(activity, cls=activitypub.ActivityEncoder),
@@ -227,6 +240,7 @@ def save(
                 return
 
             try:
+                # TODO: here is where we might use an ActivityPub extension instead
                 # do we have a "pure" activitypub version of this for mastodon?
                 if software != "bookwyrm" and hasattr(self, "pure_content"):
                     pure_activity = self.to_create_activity(user, pure=True)

bookwyrm/models/fields.py

@@ -86,7 +86,9 @@ def set_field_from_activity(
                 raise
             value = getattr(data, "actor")
         formatted = self.field_from_activity(
-            value, allow_external_connections=allow_external_connections
+            value,
+            allow_external_connections=allow_external_connections,
+            trigger=instance,
         )
         if formatted is None or formatted is MISSING or formatted == {}:
             return False
@@ -128,7 +130,7 @@ def field_to_activity(self, value):
         return value
 
     # pylint: disable=unused-argument
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         """formatter to convert activitypub into a model value"""
         if value and hasattr(self, "activitypub_wrapper"):
             value = value.get(self.activitypub_wrapper)
@@ -150,7 +152,9 @@ def __init__(self, *args, load_remote=True, **kwargs):
         self.load_remote = load_remote
         super().__init__(*args, **kwargs)
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
+        """trigger: the object that triggered this deserialization.
+        For example the Edition for which self is the parent Work"""
         if not value:
             return None
 
@@ -160,7 +164,7 @@ def field_from_activity(self, value, allow_external_connections=True):
                 # only look in the local database
                 return related_model.find_existing(value.serialize())
             # this is an activitypub object, which we can deserialize
-            return value.to_model(model=related_model)
+            return value.to_model(model=related_model, trigger=trigger)
         try:
             # make sure the value looks like a remote id
             validate_remote_id(value)
@@ -336,7 +340,7 @@ def field_to_activity(self, value):
             return f"{value.instance.remote_id}/{self.name}"
         return [i.remote_id for i in value.all()]
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         if value is None or value is MISSING:
             return None
         if not isinstance(value, list):
@@ -386,7 +390,7 @@ def field_to_activity(self, value):
             )
         return tags
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         if not isinstance(value, list):
             # GoToSocial DMs and single-user mentions are
             # sent as objects, not as an array of objects
@@ -481,7 +485,7 @@ def field_to_activity(self, value, alt=None):
 
         return activitypub.Image(url=url, name=alt)
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         image_slug = value
         # when it's an inline image (User avatar/icon, Book cover), it's a json
         # blob, but when it's an attached image, it's just a url
@@ -538,7 +542,7 @@ def field_to_activity(self, value):
             return None
         return value.isoformat()
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         missing_fields = datetime(1970, 1, 1)  # "2022-10" => "2022-10-01"
         try:
             date_value = dateutil.parser.parse(value, default=missing_fields)
@@ -556,7 +560,7 @@ class PartialDateField(ActivitypubFieldMixin, PartialDateModel):
     def field_to_activity(self, value) -> str:
         return value.partial_isoformat() if value else None
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         # pylint: disable=no-else-return
         try:
             return from_partial_isoformat(value)
@@ -584,7 +588,7 @@ def field_from_activity(self, value, allow_external_connections=True):
 class HtmlField(ActivitypubFieldMixin, models.TextField):
     """a text field for storing html"""
 
-    def field_from_activity(self, value, allow_external_connections=True):
+    def field_from_activity(self, value, allow_external_connections=True, trigger=None):
         if not value or value == MISSING:
             return None
         return clean(value)

bookwyrm/tests/views/test_reading.py

@@ -70,7 +70,9 @@ def test_start_reading(self, *_):
             },
         )
         request.user = self.local_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
             views.ReadingStatus.as_view()(request, "start", self.book.id)
 
         self.assertEqual(shelf.books.get(), self.book)
@@ -86,6 +88,12 @@ def test_start_reading(self, *_):
         self.assertEqual(readthrough.user, self.local_user)
         self.assertEqual(readthrough.book, self.book)
 
+        # Three broadcast tasks:
+        # 1. Create Readthrough
+        # 2. Create post as pure_content (for non-BookWyrm)
+        # 3. Create post with book attached - this should only happen once!
+        self.assertEqual(len(mock.mock_calls), 3)
+
     def test_start_reading_with_comment(self, *_):
         """begin a book"""
         shelf = self.local_user.shelf_set.get(identifier=models.Shelf.READING)

bookwyrm/views/helpers.py

@@ -156,8 +156,7 @@ def handle_reading_status(user, shelf, book, privacy):
         # it's a non-standard shelf, don't worry about it
         return
 
-    status = create_generated_note(user, message, mention_books=[book], privacy=privacy)
-    status.save()
+    create_generated_note(user, message, mention_books=[book], privacy=privacy)
 
 
 def load_date_in_user_tz_as_utc(date_str: str, user: models.User) -> datetime: