Generate fewer add_status_tasks

Previously, every time a status was saved, a task would start to add it
to people's timelines. This meant there were a ton of duplicate tasks
that were potentially heavy to run. Now, the Status model has a "ready"
field which indicates that it's worth updating the timelines. It
defaults to True, which prevents statuses from accidentally not being
added due to ready state.

The ready state is explicitly set to false in the view, which is the
source of most of the noise for that task.

bookwyrm/activitystreams.py

@@ -298,6 +298,12 @@ def add_status_on_create(sender, instance, created, *args, **kwargs):
         remove_status_task.delay(instance.id)
         return
 
+    # To avoid creating a zillion unnecessary tasks caused by re-saving the model,
+    # check if it's actually ready to send before we go. We're trusting this was
+    # set correctly by the inbox or view
+    if not instance.ready:
+        return
+
     # when creating new things, gotta wait on the transaction
     transaction.on_commit(
         lambda: add_status_on_create_command(sender, instance, created)

bookwyrm/migrations/0164_status_ready.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.2.16 on 2022-11-15 21:40
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0163_merge_0160_auto_20221101_2251_0162_importjob_task_id"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="status",
+            name="ready",
+            field=models.BooleanField(default=True),
+        ),
+    ]

bookwyrm/models/status.py

@@ -63,6 +63,9 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
         activitypub_field="inReplyTo",
     )
     thread_id = models.IntegerField(blank=True, null=True)
+    # statuses get saved a few times, this indicates if they're set
+    ready = models.BooleanField(default=True)
+
     objects = InheritanceManager()
 
     activity_serializer = activitypub.Note
@@ -83,8 +86,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
 
         if not self.reply_parent:
             self.thread_id = self.id
-
-        super().save(broadcast=False, update_fields=["thread_id"])
+            super().save(broadcast=False, update_fields=["thread_id"])
 
     def delete(self, *args, **kwargs):  # pylint: disable=unused-argument
         """ "delete" a status"""
@@ -399,7 +401,7 @@ class ReviewRating(Review):
     def save(self, *args, **kwargs):
         if not self.rating:
             raise ValueError("ReviewRating object must include a numerical rating")
-        return super().save(*args, **kwargs)
+        super().save(*args, **kwargs)
 
     @property
     def pure_content(self):

bookwyrm/tests/activitystreams/test_tasks.py

@@ -7,6 +7,7 @@ from bookwyrm import activitystreams, models
 class Activitystreams(TestCase):
     """using redis to build activity streams"""
 
+    # pylint: disable=invalid-name
     def setUp(self):
         """use a test csv"""
         with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(

bookwyrm/tests/views/inbox/test_inbox_create.py

@@ -3,13 +3,69 @@ import json
 import pathlib
 from unittest.mock import patch
 
-from django.test import TestCase
+from django.test import TestCase, TransactionTestCase
 
 from bookwyrm import models, views
 from bookwyrm.activitypub import ActivitySerializerError
 
 
-# pylint: disable=too-many-public-methods
+# pylint: disable=too-many-public-methods, invalid-name
+class TransactionInboxCreate(TransactionTestCase):
+    """readthrough tests"""
+
+    def setUp(self):
+        """basic user and book data"""
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
+            "bookwyrm.activitystreams.populate_stream_task.delay"
+        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@example.com",
+                "mouse@mouse.com",
+                "mouseword",
+                local=True,
+                localname="mouse",
+            )
+        self.local_user.remote_id = "https://example.com/user/mouse"
+        self.local_user.save(broadcast=False, update_fields=["remote_id"])
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
+            self.remote_user = models.User.objects.create_user(
+                "rat",
+                "rat@rat.com",
+                "ratword",
+                local=False,
+                remote_id="https://example.com/users/rat",
+                inbox="https://example.com/users/rat/inbox",
+                outbox="https://example.com/users/rat/outbox",
+            )
+
+        self.create_json = {
+            "id": "hi",
+            "type": "Create",
+            "actor": "hi",
+            "to": ["https://www.w3.org/ns/activitystreams#public"],
+            "cc": ["https://example.com/user/mouse/followers"],
+            "object": {},
+        }
+        models.SiteSettings.objects.create()
+
+    def test_create_status_transaction(self, *_):
+        """the "it justs works" mode"""
+        datafile = pathlib.Path(__file__).parent.joinpath(
+            "../../data/ap_quotation.json"
+        )
+        status_data = json.loads(datafile.read_bytes())
+
+        models.Edition.objects.create(
+            title="Test Book", remote_id="https://example.com/book/1"
+        )
+        activity = self.create_json
+        activity["object"] = status_data
+
+        with patch("bookwyrm.activitystreams.add_status_task.apply_async") as mock:
+            views.inbox.activity_task(activity)
+        self.assertEqual(mock.call_count, 2)
+
+
 @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
 @patch("bookwyrm.activitystreams.add_book_statuses_task.delay")
 class InboxCreate(TestCase):

bookwyrm/tests/views/test_status.py

@@ -2,21 +2,74 @@
 import json
 from unittest.mock import patch
 from django.core.exceptions import PermissionDenied
-from django.test import TestCase
+from django.test import TestCase, TransactionTestCase
 from django.test.client import RequestFactory
 
 from bookwyrm import forms, models, views
 from bookwyrm.views.status import find_mentions
 from bookwyrm.settings import DOMAIN
+
 from bookwyrm.tests.validate_html import validate_html
 
+# pylint: disable=invalid-name
+@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
+class StatusTransactions(TransactionTestCase):
+    """Test full database transactions"""
+
+    def setUp(self):
+        """we need basic test data and mocks"""
+        self.factory = RequestFactory()
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
+            "bookwyrm.activitystreams.populate_stream_task.delay"
+        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@local.com",
+                "mouse@mouse.com",
+                "mouseword",
+                local=True,
+                localname="mouse",
+                remote_id="https://example.com/users/mouse",
+            )
+            self.another_user = models.User.objects.create_user(
+                f"nutria@{DOMAIN}",
+                "nutria@nutria.com",
+                "password",
+                local=True,
+                localname="nutria",
+            )
+
+        work = models.Work.objects.create(title="Test Work")
+        self.book = models.Edition.objects.create(
+            title="Example Edition",
+            remote_id="https://example.com/book/1",
+            parent_work=work,
+        )
+
+    def test_create_status_saves(self, *_):
+        """This view calls save multiple times"""
+        view = views.CreateStatus.as_view()
+        form = forms.CommentForm(
+            {
+                "content": "hi",
+                "user": self.local_user.id,
+                "book": self.book.id,
+                "privacy": "public",
+            }
+        )
+        request = self.factory.post("", form.data)
+        request.user = self.local_user
+
+        with patch("bookwyrm.activitystreams.add_status_task.apply_async") as mock:
+            view(request, "comment")
+
+        self.assertEqual(mock.call_count, 2)
+
 
 @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay")
 @patch("bookwyrm.activitystreams.populate_stream_task.delay")
 @patch("bookwyrm.lists_stream.populate_lists_task.delay")
 @patch("bookwyrm.activitystreams.remove_status_task.delay")
 @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
-# pylint: disable=invalid-name
 # pylint: disable=too-many-public-methods
 class StatusViews(TestCase):
     """viewing and creating statuses"""

bookwyrm/views/status.py

@@ -82,13 +82,16 @@ class CreateStatus(View):
                 return HttpResponseBadRequest()
             return redirect("/")
 
-        status = form.save(request)
+        status = form.save(request, commit=False)
+        status.ready = False
         # save the plain, unformatted version of the status for future editing
         status.raw_content = status.content
         if hasattr(status, "quote"):
             status.raw_quote = status.quote
 
         status.sensitive = status.content_warning not in [None, ""]
+        # the status has to be saved now before we can add many to many fields
+        # like mentions
         status.save(broadcast=False)
 
         # inspect the text for user tags
@@ -119,6 +122,7 @@ class CreateStatus(View):
         if hasattr(status, "quote"):
             status.quote = to_markdown(status.quote)
 
+        status.ready = True
         status.save(created=created)
 
         # update a readthrough, if needed