From f284eebecef2b3a98b54a4f4374f4a44f0fd8034 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 23 Aug 2021 07:04:59 -0700 Subject: [PATCH 1/4] Fixes boosts removing statuses from feeds --- bookwyrm/activitystreams.py | 5 +++-- bookwyrm/redis_store.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 0a90c9f4..4b9f6a60 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -273,9 +273,10 @@ def add_status_on_create(sender, instance, created, *args, **kwargs): created_date__lt=instance.created_date, ) for stream in streams.values(): - stream.remove_object_from_related_stores(boosted) + audience = streams.get_stores_for_object(instance) + stream.remove_object_from_related_stores(boosted, stores=audience) for status in old_versions: - stream.remove_object_from_related_stores(status) + stream.remove_object_from_related_stores(status, stores=audience) @receiver(signals.post_delete, sender=models.Boost) diff --git a/bookwyrm/redis_store.py b/bookwyrm/redis_store.py index fa5c73a5..edba5ae6 100644 --- a/bookwyrm/redis_store.py +++ b/bookwyrm/redis_store.py @@ -33,8 +33,9 @@ class RedisStore(ABC): # and go! return pipeline.execute() - def remove_object_from_related_stores(self, obj): + def remove_object_from_related_stores(self, obj, stores=None): """remove an object from all stores""" + stores = stores or self.get_stores_for_object(obj) pipeline = r.pipeline() for store in self.get_stores_for_object(obj): pipeline.zrem(store, -1, obj.id) From 9c21f4d8e673caacda26a2b5e1be3415c90c14b3 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 23 Aug 2021 10:36:23 -0700 Subject: [PATCH 2/4] Adds tests for boosts in activitystreams --- bookwyrm/activitystreams.py | 2 +- bookwyrm/tests/test_activitystreams.py | 64 ++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 4b9f6a60..0e07905b 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -273,7 +273,7 @@ def add_status_on_create(sender, instance, created, *args, **kwargs): created_date__lt=instance.created_date, ) for stream in streams.values(): - audience = streams.get_stores_for_object(instance) + audience = stream.get_stores_for_object(instance) stream.remove_object_from_related_stores(boosted, stores=audience) for status in old_versions: stream.remove_object_from_related_stores(status, stores=audience) diff --git a/bookwyrm/tests/test_activitystreams.py b/bookwyrm/tests/test_activitystreams.py index ac57d8b3..1041b889 100644 --- a/bookwyrm/tests/test_activitystreams.py +++ b/bookwyrm/tests/test_activitystreams.py @@ -286,3 +286,67 @@ class Activitystreams(TestCase): # yes book, yes audience result = activitystreams.BooksStream().get_statuses_for_user(self.local_user) self.assertEqual(list(result), [status]) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + def test_boost_to_another_timeline(self, *_): + """add a boost and deduplicate the boosted status on the timeline""" + status = models.Status.objects.create(user=self.local_user, content="hi") + boost = models.Boost.objects.create( + boosted_status=status, + user=self.another_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.add_status_on_create(models.Boost, boost, True) + self.assertTrue(mock.called) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertEqual( + call_args[1]["stores"], ["{:d}-home".format(self.another_user.id)] + ) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + def test_boost_to_following_timeline(self, *_): + """add a boost and deduplicate the boosted status on the timeline""" + self.local_user.following.add(self.another_user) + status = models.Status.objects.create(user=self.local_user, content="hi") + boost = models.Boost.objects.create( + boosted_status=status, + user=self.another_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.add_status_on_create(models.Boost, boost, True) + self.assertTrue(mock.called) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertTrue( + "{:d}-home".format(self.another_user.id) in call_args[1]["stores"] + ) + self.assertTrue( + "{:d}-home".format(self.local_user.id) in call_args[1]["stores"] + ) + + @patch("bookwyrm.activitystreams.LocalStream.remove_object_from_related_stores") + @patch("bookwyrm.activitystreams.BooksStream.remove_object_from_related_stores") + def test_boost_to_same_timeline(self, *_): + """add a boost and deduplicate the boosted status on the timeline""" + status = models.Status.objects.create(user=self.local_user, content="hi") + boost = models.Boost.objects.create( + boosted_status=status, + user=self.local_user, + ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ) as mock: + activitystreams.add_status_on_create(models.Boost, boost, True) + self.assertTrue(mock.called) + call_args = mock.call_args + self.assertEqual(call_args[0][0], status) + self.assertEqual( + call_args[1]["stores"], ["{:d}-home".format(self.local_user.id)] + ) From 16235d1d904f0fcba22da09fc80c6c9884e83104 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 23 Aug 2021 10:48:23 -0700 Subject: [PATCH 3/4] Actually use provided stoers list --- bookwyrm/redis_store.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/redis_store.py b/bookwyrm/redis_store.py index edba5ae6..521e73b2 100644 --- a/bookwyrm/redis_store.py +++ b/bookwyrm/redis_store.py @@ -37,7 +37,7 @@ class RedisStore(ABC): """remove an object from all stores""" stores = stores or self.get_stores_for_object(obj) pipeline = r.pipeline() - for store in self.get_stores_for_object(obj): + for store in stores: pipeline.zrem(store, -1, obj.id) pipeline.execute() From d3cfceafca52200a97182b5d0607a6da62786b49 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 23 Aug 2021 13:58:19 -0700 Subject: [PATCH 4/4] Mocks creation of boost --- bookwyrm/tests/test_activitystreams.py | 34 +++++++++++++++++--------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/bookwyrm/tests/test_activitystreams.py b/bookwyrm/tests/test_activitystreams.py index 1041b889..56ba844a 100644 --- a/bookwyrm/tests/test_activitystreams.py +++ b/bookwyrm/tests/test_activitystreams.py @@ -8,6 +8,7 @@ from bookwyrm import activitystreams, models @patch("bookwyrm.activitystreams.ActivityStream.add_status") @patch("bookwyrm.activitystreams.BooksStream.add_book_statuses") @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") +# pylint: disable=too-many-public-methods class Activitystreams(TestCase): """using redis to build activity streams""" @@ -292,10 +293,13 @@ class Activitystreams(TestCase): def test_boost_to_another_timeline(self, *_): """add a boost and deduplicate the boosted status on the timeline""" status = models.Status.objects.create(user=self.local_user, content="hi") - boost = models.Boost.objects.create( - boosted_status=status, - user=self.another_user, - ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.another_user, + ) with patch( "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" ) as mock: @@ -313,10 +317,13 @@ class Activitystreams(TestCase): """add a boost and deduplicate the boosted status on the timeline""" self.local_user.following.add(self.another_user) status = models.Status.objects.create(user=self.local_user, content="hi") - boost = models.Boost.objects.create( - boosted_status=status, - user=self.another_user, - ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.another_user, + ) with patch( "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" ) as mock: @@ -336,10 +343,13 @@ class Activitystreams(TestCase): def test_boost_to_same_timeline(self, *_): """add a boost and deduplicate the boosted status on the timeline""" status = models.Status.objects.create(user=self.local_user, content="hi") - boost = models.Boost.objects.create( - boosted_status=status, - user=self.local_user, - ) + with patch( + "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" + ): + boost = models.Boost.objects.create( + boosted_status=status, + user=self.local_user, + ) with patch( "bookwyrm.activitystreams.HomeStream.remove_object_from_related_stores" ) as mock: