From bd8858830ac89c497235f2389bf0f8ea8b63afee Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 11:05:37 -0700 Subject: [PATCH 01/18] Created generic redis class for activitstreams --- bookwyrm/activitystreams.py | 114 +++++++++++++----------------------- bookwyrm/redis_store.py | 91 ++++++++++++++++++++++++++++ bookwyrm/views/feed.py | 1 - 3 files changed, 133 insertions(+), 73 deletions(-) create mode 100644 bookwyrm/redis_store.py diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 279079c81..8d1e83018 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -1,18 +1,13 @@ """ access the activity streams stored in redis """ -from abc import ABC from django.dispatch import receiver from django.db.models import signals, Q -import redis -from bookwyrm import models, settings +from bookwyrm import models +from bookwyrm.redis_store import RedisStore, r from bookwyrm.views.helpers import privacy_filter -r = redis.Redis( - host=settings.REDIS_ACTIVITY_HOST, port=settings.REDIS_ACTIVITY_PORT, db=0 -) - -class ActivityStream(ABC): +class ActivityStream(RedisStore): """ a category of activity stream (like home, local, federated) """ def stream_id(self, user): @@ -23,58 +18,38 @@ class ActivityStream(ABC): """ the redis key for this user's unread count for this stream """ return "{}-unread".format(self.stream_id(user)) - def get_value(self, status): # pylint: disable=no-self-use - """ the status id and the rank (ie, published date) """ - return {status.id: status.published_date.timestamp()} + def get_rank(self, obj): # pylint: disable=no-self-use + """ the sort rank of a status, which is published date """ + return obj.published_date.timestamp() def add_status(self, status): """ add a status to users' feeds """ - value = self.get_value(status) - # we want to do this as a bulk operation, hence "pipeline" - pipeline = r.pipeline() - for user in self.stream_users(status): - # add the status to the feed - pipeline.zadd(self.stream_id(user), value) - pipeline.zremrangebyrank( - self.stream_id(user), 0, -1 * settings.MAX_STREAM_LENGTH - ) + # the pipeline contains all the addp-to-stream activities + pipeline = self.add_object_to_related_stores(status, execute=False) + + for user in self.get_audience(status): # add to the unread status count pipeline.incr(self.unread_id(user)) - # and go! - pipeline.execute() - def remove_status(self, status): - """ remove a status from all feeds """ - pipeline = r.pipeline() - for user in self.stream_users(status): - pipeline.zrem(self.stream_id(user), -1, status.id) + # and go! pipeline.execute() def add_user_statuses(self, viewer, user): """ add a user's statuses to another user's feed """ - pipeline = r.pipeline() - statuses = user.status_set.all()[: settings.MAX_STREAM_LENGTH] - for status in statuses: - pipeline.zadd(self.stream_id(viewer), self.get_value(status)) - if statuses: - pipeline.zremrangebyrank( - self.stream_id(user), 0, -1 * settings.MAX_STREAM_LENGTH - ) - pipeline.execute() + statuses = privacy_filter(viewer, user.status_set.all()) + self.bulk_add_objects_to_store(statuses, self.stream_id(viewer)) def remove_user_statuses(self, viewer, user): """ remove a user's status from another user's feed """ - pipeline = r.pipeline() - for status in user.status_set.all()[: settings.MAX_STREAM_LENGTH]: - pipeline.lrem(self.stream_id(viewer), -1, status.id) - pipeline.execute() + statuses = user.status_set.all() + self.bulk_remove_objects_from_store(statuses, self.stream_id(viewer)) def get_activity_stream(self, user): - """ load the ids for statuses to be displayed """ + """ load the statuses to be displayed """ # clear unreads for this feed r.set(self.unread_id(user), 0) - statuses = r.zrevrange(self.stream_id(user), 0, -1) + statuses = super().get_store(self.stream_id(user)) return ( models.Status.objects.select_subclasses() .filter(id__in=statuses) @@ -85,23 +60,11 @@ class ActivityStream(ABC): """ get the unread status count for this user's feed """ return int(r.get(self.unread_id(user)) or 0) - def populate_stream(self, user): + def populate_streamse(self, user): """ go from zero to a timeline """ - pipeline = r.pipeline() - statuses = self.stream_statuses(user) + super().populate_store(self.stream_id(user)) - stream_id = self.stream_id(user) - for status in statuses.all()[: settings.MAX_STREAM_LENGTH]: - pipeline.zadd(stream_id, self.get_value(status)) - - # only trim the stream if statuses were added - if statuses.exists(): - pipeline.zremrangebyrank( - self.stream_id(user), 0, -1 * settings.MAX_STREAM_LENGTH - ) - pipeline.execute() - - def stream_users(self, status): # pylint: disable=no-self-use + def get_audience(self, status): # pylint: disable=no-self-use """ given a status, what users should see it """ # direct messages don't appeard in feeds, direct comments/reviews/etc do if status.privacy == "direct" and status.status_type == "Note": @@ -129,7 +92,10 @@ class ActivityStream(ABC): ) return audience.distinct() - def stream_statuses(self, user): # pylint: disable=no-self-use + def get_stores_for_object(self, obj): + return [self.stream_id(u) for u in self.get_audience(obj)] + + def get_statuses_for_user(self, user): # pylint: disable=no-self-use """ given a user, what statuses should they see on this stream """ return privacy_filter( user, @@ -137,14 +103,18 @@ class ActivityStream(ABC): privacy_levels=["public", "unlisted", "followers"], ) + def get_objects_for_store(self, store): + user = models.User.objects.get(id=store.split('-')[0]) + return self.get_statuses_for_user(user) + class HomeStream(ActivityStream): """ users you follow """ key = "home" - def stream_users(self, status): - audience = super().stream_users(status) + def get_audience(self, status): + audience = super().get_audience(status) if not audience: return [] return audience.filter( @@ -152,7 +122,7 @@ class HomeStream(ActivityStream): | Q(following=status.user) # if the user is following the author ).distinct() - def stream_statuses(self, user): + def get_statuses_for_user(self, user): return privacy_filter( user, models.Status.objects.select_subclasses(), @@ -166,13 +136,13 @@ class LocalStream(ActivityStream): key = "local" - def stream_users(self, status): + def get_audience(self, status): # this stream wants no part in non-public statuses if status.privacy != "public" or not status.user.local: return [] - return super().stream_users(status) + return super().get_audience(status) - def stream_statuses(self, user): + def get_statuses_for_user(self, user): # all public statuses by a local user return privacy_filter( user, @@ -186,13 +156,13 @@ class FederatedStream(ActivityStream): key = "federated" - def stream_users(self, status): + def get_audience(self, status): # this stream wants no part in non-public statuses if status.privacy != "public": return [] - return super().stream_users(status) + return super().get_audience(status) - def stream_statuses(self, user): + def get_statuses_for_user(self, user): return privacy_filter( user, models.Status.objects.select_subclasses(), @@ -217,7 +187,7 @@ def add_status_on_create(sender, instance, created, *args, **kwargs): if instance.deleted: for stream in streams.values(): - stream.remove_status(instance) + stream.remove_object_from_related_stores(instance) return if not created: @@ -234,7 +204,7 @@ def remove_boost_on_delete(sender, instance, *args, **kwargs): """ boosts are deleted """ # we're only interested in new statuses for stream in streams.values(): - stream.remove_status(instance) + stream.remove_object_from_related_stores(instance) @receiver(signals.post_save, sender=models.UserFollows) @@ -248,7 +218,7 @@ def add_statuses_on_follow(sender, instance, created, *args, **kwargs): @receiver(signals.post_delete, sender=models.UserFollows) # pylint: disable=unused-argument -def remove_statuses_on_unfollow(sender, instance, *args, **kwargs): +def remove_objectes_on_unfollow(sender, instance, *args, **kwargs): """ remove statuses from a feed on unfollow """ if not instance.user_subject.local: return @@ -257,7 +227,7 @@ def remove_statuses_on_unfollow(sender, instance, *args, **kwargs): @receiver(signals.post_save, sender=models.UserBlocks) # pylint: disable=unused-argument -def remove_statuses_on_block(sender, instance, *args, **kwargs): +def remove_objectes_on_block(sender, instance, *args, **kwargs): """ remove statuses from all feeds on block """ # blocks apply ot all feeds if instance.user_subject.local: @@ -294,4 +264,4 @@ def populate_streams_on_account_create(sender, instance, created, *args, **kwarg return for stream in streams.values(): - stream.populate_stream(instance) + stream.populate_streams(instance) diff --git a/bookwyrm/redis_store.py b/bookwyrm/redis_store.py new file mode 100644 index 000000000..8255fcbda --- /dev/null +++ b/bookwyrm/redis_store.py @@ -0,0 +1,91 @@ +""" access the activity stores stored in redis """ +from abc import ABC, abstractmethod +import redis + +from bookwyrm import settings + +r = redis.Redis( + host=settings.REDIS_ACTIVITY_HOST, port=settings.REDIS_ACTIVITY_PORT, db=0 +) + + +class RedisStore(ABC): + """ sets of ranked, related objects, like statuses for a user's feed """ + max_length = settings.MAX_STREAM_LENGTH + + def get_value(self, obj): + """ the object and rank """ + return {obj.id: self.get_rank(obj)} + + def add_object_to_related_stores(self, obj, execute=True): + """ add an object to all suitable stores """ + value = self.get_value(obj) + # we want to do this as a bulk operation, hence "pipeline" + pipeline = r.pipeline() + for store in self.get_stores_for_object(obj): + # add the status to the feed + pipeline.zadd(store, value) + # trim the store + pipeline.zremrangebyrank( + store, 0, -1 * self.max_length + ) + if not execute: + return pipeline + # and go! + return pipeline.execute() + + def remove_object_from_related_stores(self, obj): + """ remove an object from all stores """ + pipeline = r.pipeline() + for store in self.get_stores_for_object(obj): + pipeline.zrem(store, -1, obj.id) + pipeline.execute() + + def bulk_add_objects_to_store(self, objs, store): + """ add a list of objects to a given store """ + pipeline = r.pipeline() + for obj in objs[:self.max_length]: + pipeline.zadd(store, self.get_value(obj)) + if objs: + pipeline.zremrangebyrank( + store, 0, -1 * self.max_length + ) + pipeline.execute() + + def bulk_remove_objects_from_store(self, objs, store): + """ remoev a list of objects from a given store """ + pipeline = r.pipeline() + for obj in objs[:self.max_length]: + pipeline.zrem(store, -1, obj.id) + pipeline.execute() + + def get_store(self, store): # pylint: disable=no-self-use + """ load the values in a store """ + return r.zrevrange(store, 0, -1) + + def populate_store(self, store): + """ go from zero to a store """ + pipeline = r.pipeline() + queryset = self.get_objects_for_store(store) + + for obj in queryset[:self.max_length]: + pipeline.zadd(store, self.get_value(obj)) + + # only trim the store if objects were added + if queryset.exists(): + pipeline.zremrangebyrank( + store, 0, -1 * self.max_length + ) + pipeline.execute() + + @abstractmethod + def get_objects_for_store(self, store): + """ a queryset of what should go in a store, used for populating it """ + + @abstractmethod + def get_stores_for_object(self, obj): + """ the stores that an object belongs in """ + + @abstractmethod + def get_rank(self, obj): + """ how to rank an object """ diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index e4be50e33..cda115867 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -31,7 +31,6 @@ class Feed(View): tab = "home" activities = activitystreams.streams[tab].get_activity_stream(request.user) - paginated = Paginator(activities, PAGE_LENGTH) suggested_users = get_suggested_users(request.user) From 0bbaf0a562b9e2f3521a1e096ade9f509659fb8f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 11:10:26 -0700 Subject: [PATCH 02/18] Python formatting --- bookwyrm/activitystreams.py | 2 +- bookwyrm/redis_store.py | 19 +++++++------------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 8d1e83018..3a815322b 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -104,7 +104,7 @@ class ActivityStream(RedisStore): ) def get_objects_for_store(self, store): - user = models.User.objects.get(id=store.split('-')[0]) + user = models.User.objects.get(id=store.split("-")[0]) return self.get_statuses_for_user(user) diff --git a/bookwyrm/redis_store.py b/bookwyrm/redis_store.py index 8255fcbda..4236d6df2 100644 --- a/bookwyrm/redis_store.py +++ b/bookwyrm/redis_store.py @@ -11,6 +11,7 @@ r = redis.Redis( class RedisStore(ABC): """ sets of ranked, related objects, like statuses for a user's feed """ + max_length = settings.MAX_STREAM_LENGTH def get_value(self, obj): @@ -26,9 +27,7 @@ class RedisStore(ABC): # add the status to the feed pipeline.zadd(store, value) # trim the store - pipeline.zremrangebyrank( - store, 0, -1 * self.max_length - ) + pipeline.zremrangebyrank(store, 0, -1 * self.max_length) if not execute: return pipeline # and go! @@ -44,18 +43,16 @@ class RedisStore(ABC): def bulk_add_objects_to_store(self, objs, store): """ add a list of objects to a given store """ pipeline = r.pipeline() - for obj in objs[:self.max_length]: + for obj in objs[: self.max_length]: pipeline.zadd(store, self.get_value(obj)) if objs: - pipeline.zremrangebyrank( - store, 0, -1 * self.max_length - ) + pipeline.zremrangebyrank(store, 0, -1 * self.max_length) pipeline.execute() def bulk_remove_objects_from_store(self, objs, store): """ remoev a list of objects from a given store """ pipeline = r.pipeline() - for obj in objs[:self.max_length]: + for obj in objs[: self.max_length]: pipeline.zrem(store, -1, obj.id) pipeline.execute() @@ -68,14 +65,12 @@ class RedisStore(ABC): pipeline = r.pipeline() queryset = self.get_objects_for_store(store) - for obj in queryset[:self.max_length]: + for obj in queryset[: self.max_length]: pipeline.zadd(store, self.get_value(obj)) # only trim the store if objects were added if queryset.exists(): - pipeline.zremrangebyrank( - store, 0, -1 * self.max_length - ) + pipeline.zremrangebyrank(store, 0, -1 * self.max_length) pipeline.execute() @abstractmethod From cd56abcb0833986885ec063585f7203eaaa00b21 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 12:11:49 -0700 Subject: [PATCH 03/18] Maintain signal names --- bookwyrm/activitystreams.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 3a815322b..fc50ef7b6 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -218,7 +218,7 @@ def add_statuses_on_follow(sender, instance, created, *args, **kwargs): @receiver(signals.post_delete, sender=models.UserFollows) # pylint: disable=unused-argument -def remove_objectes_on_unfollow(sender, instance, *args, **kwargs): +def remove_statuses_on_unfollow(sender, instance, *args, **kwargs): """ remove statuses from a feed on unfollow """ if not instance.user_subject.local: return @@ -227,7 +227,7 @@ def remove_objectes_on_unfollow(sender, instance, *args, **kwargs): @receiver(signals.post_save, sender=models.UserBlocks) # pylint: disable=unused-argument -def remove_objectes_on_block(sender, instance, *args, **kwargs): +def remove_statuses_on_block(sender, instance, *args, **kwargs): """ remove statuses from all feeds on block """ # blocks apply ot all feeds if instance.user_subject.local: From 04b9704187d4a46885cc5a09baf31165282ea1da Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 13:13:56 -0700 Subject: [PATCH 04/18] typo fix --- bookwyrm/activitystreams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index fc50ef7b6..2db6cc20a 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -60,7 +60,7 @@ class ActivityStream(RedisStore): """ get the unread status count for this user's feed """ return int(r.get(self.unread_id(user)) or 0) - def populate_streamse(self, user): + def populate_streams(self, user): """ go from zero to a timeline """ super().populate_store(self.stream_id(user)) From 56330d448b12d7a029b51019e5766050a7b476bb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 14:08:24 -0700 Subject: [PATCH 05/18] Changes remove status redis mock --- bookwyrm/tests/models/test_status_model.py | 4 +++- bookwyrm/tests/test_templatetags.py | 4 +++- bookwyrm/tests/views/test_inbox.py | 6 +++--- bookwyrm/tests/views/test_interaction.py | 2 +- bookwyrm/tests/views/test_status.py | 16 +++++++++++----- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 208bf3ab4..4263c4572 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -116,7 +116,9 @@ class Status(TestCase): def test_status_to_activity_tombstone(self, *_): """ subclass of the base model version with a "pure" serializer """ - with patch("bookwyrm.activitystreams.ActivityStream.remove_status"): + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ): status = models.Status.objects.create( content="test content", user=self.local_user, diff --git a/bookwyrm/tests/test_templatetags.py b/bookwyrm/tests/test_templatetags.py index 61136c2eb..b4dc517f1 100644 --- a/bookwyrm/tests/test_templatetags.py +++ b/bookwyrm/tests/test_templatetags.py @@ -85,7 +85,9 @@ class TemplateTags(TestCase): second_child = models.Status.objects.create( reply_parent=parent, user=self.user, content="hi" ) - with patch("bookwyrm.activitystreams.ActivityStream.remove_status"): + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ): third_child = models.Status.objects.create( reply_parent=parent, user=self.user, diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 37cf00ddc..935909e10 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -444,7 +444,7 @@ class Inbox(TestCase): "object": {"id": self.status.remote_id, "type": "Tombstone"}, } with patch( - "bookwyrm.activitystreams.ActivityStream.remove_status" + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" ) as redis_mock: views.inbox.activity_task(activity) self.assertTrue(redis_mock.called) @@ -477,7 +477,7 @@ class Inbox(TestCase): "object": {"id": self.status.remote_id, "type": "Tombstone"}, } with patch( - "bookwyrm.activitystreams.ActivityStream.remove_status" + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" ) as redis_mock: views.inbox.activity_task(activity) self.assertTrue(redis_mock.called) @@ -616,7 +616,7 @@ class Inbox(TestCase): }, } with patch( - "bookwyrm.activitystreams.ActivityStream.remove_status" + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" ) as redis_mock: views.inbox.activity_task(activity) self.assertTrue(redis_mock.called) diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index 297eeb73d..8d2c63ffc 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -164,7 +164,7 @@ class InteractionViews(TestCase): self.assertEqual(models.Boost.objects.count(), 1) self.assertEqual(models.Notification.objects.count(), 1) with patch( - "bookwyrm.activitystreams.ActivityStream.remove_status" + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" ) as redis_mock: view(request, status.id) self.assertTrue(redis_mock.called) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index e7fc62d56..5eb13b6b2 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -177,7 +177,9 @@ class StatusViews(TestCase): content="hi", book=self.book, user=self.local_user ) - with patch("bookwyrm.activitystreams.ActivityStream.remove_status") as mock: + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ) as mock: result = view(request, status.id) self.assertTrue(mock.called) result.render() @@ -196,7 +198,9 @@ class StatusViews(TestCase): book=self.book, rating=2.0, user=self.local_user ) - with patch("bookwyrm.activitystreams.ActivityStream.remove_status") as mock: + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ) as mock: result = view(request, status.id) self.assertFalse(mock.called) self.assertEqual(result.status_code, 400) @@ -214,7 +218,9 @@ class StatusViews(TestCase): content="hi", user=self.local_user ) - with patch("bookwyrm.activitystreams.ActivityStream.remove_status") as mock: + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" + ) as mock: result = view(request, status.id) self.assertFalse(mock.called) self.assertEqual(result.status_code, 400) @@ -316,7 +322,7 @@ class StatusViews(TestCase): request.user = self.local_user with patch( - "bookwyrm.activitystreams.ActivityStream.remove_status" + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" ) as redis_mock: view(request, status.id) self.assertTrue(redis_mock.called) @@ -351,7 +357,7 @@ class StatusViews(TestCase): request.user.is_superuser = True with patch( - "bookwyrm.activitystreams.ActivityStream.remove_status" + "bookwyrm.activitystreams.ActivityStream.remove_object_from_related_stores" ) as redis_mock: view(request, status.id) self.assertTrue(redis_mock.called) From 6a3c01a67f3fc7fd932469bc1bf82377952043be Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 14:17:45 -0700 Subject: [PATCH 06/18] stream_users function has been renamed --- bookwyrm/tests/test_activitystreams.py | 58 +++++++++++++------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/bookwyrm/tests/test_activitystreams.py b/bookwyrm/tests/test_activitystreams.py index 88ca4693b..b4efeba3f 100644 --- a/bookwyrm/tests/test_activitystreams.py +++ b/bookwyrm/tests/test_activitystreams.py @@ -47,18 +47,18 @@ class Activitystreams(TestCase): "{}-test-unread".format(self.local_user.id), ) - def test_abstractstream_stream_users(self, *_): + def test_abstractstream_get_audience(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" ) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) # remote users don't have feeds self.assertFalse(self.remote_user in users) self.assertTrue(self.local_user in users) self.assertTrue(self.another_user in users) - def test_abstractstream_stream_users_direct(self, *_): + def test_abstractstream_get_audience_direct(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, @@ -66,7 +66,7 @@ class Activitystreams(TestCase): privacy="direct", ) status.mention_users.add(self.local_user) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) self.assertEqual(users, []) status = models.Comment.objects.create( @@ -76,22 +76,22 @@ class Activitystreams(TestCase): book=self.book, ) status.mention_users.add(self.local_user) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) self.assertTrue(self.local_user in users) self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - def test_abstractstream_stream_users_followers_remote_user(self, *_): + def test_abstractstream_get_audience_followers_remote_user(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="followers", ) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) self.assertFalse(users.exists()) - def test_abstractstream_stream_users_followers_self(self, *_): + def test_abstractstream_get_audience_followers_self(self, *_): """ get a list of users that should see a status """ status = models.Comment.objects.create( user=self.local_user, @@ -99,12 +99,12 @@ class Activitystreams(TestCase): privacy="direct", book=self.book, ) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) self.assertTrue(self.local_user in users) self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - def test_abstractstream_stream_users_followers_with_mention(self, *_): + def test_abstractstream_get_audience_followers_with_mention(self, *_): """ get a list of users that should see a status """ status = models.Comment.objects.create( user=self.remote_user, @@ -114,12 +114,12 @@ class Activitystreams(TestCase): ) status.mention_users.add(self.local_user) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) self.assertTrue(self.local_user in users) self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - def test_abstractstream_stream_users_followers_with_relationship(self, *_): + def test_abstractstream_get_audience_followers_with_relationship(self, *_): """ get a list of users that should see a status """ self.remote_user.followers.add(self.local_user) status = models.Comment.objects.create( @@ -128,77 +128,77 @@ class Activitystreams(TestCase): privacy="direct", book=self.book, ) - users = self.test_stream.stream_users(status) + users = self.test_stream.get_audience(status) self.assertFalse(self.local_user in users) self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - def test_homestream_stream_users(self, *_): + def test_homestream_get_audience(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" ) - users = activitystreams.HomeStream().stream_users(status) + users = activitystreams.HomeStream().get_audience(status) self.assertFalse(users.exists()) - def test_homestream_stream_users_with_mentions(self, *_): + def test_homestream_get_audience_with_mentions(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" ) status.mention_users.add(self.local_user) - users = activitystreams.HomeStream().stream_users(status) + users = activitystreams.HomeStream().get_audience(status) self.assertFalse(self.local_user in users) self.assertFalse(self.another_user in users) - def test_homestream_stream_users_with_relationship(self, *_): + def test_homestream_get_audience_with_relationship(self, *_): """ get a list of users that should see a status """ self.remote_user.followers.add(self.local_user) status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" ) - users = activitystreams.HomeStream().stream_users(status) + users = activitystreams.HomeStream().get_audience(status) self.assertTrue(self.local_user in users) self.assertFalse(self.another_user in users) - def test_localstream_stream_users_remote_status(self, *_): + def test_localstream_get_audience_remote_status(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" ) - users = activitystreams.LocalStream().stream_users(status) + users = activitystreams.LocalStream().get_audience(status) self.assertEqual(users, []) - def test_localstream_stream_users_local_status(self, *_): + def test_localstream_get_audience_local_status(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.local_user, content="hi", privacy="public" ) - users = activitystreams.LocalStream().stream_users(status) + users = activitystreams.LocalStream().get_audience(status) self.assertTrue(self.local_user in users) self.assertTrue(self.another_user in users) - def test_localstream_stream_users_unlisted(self, *_): + def test_localstream_get_audience_unlisted(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.local_user, content="hi", privacy="unlisted" ) - users = activitystreams.LocalStream().stream_users(status) + users = activitystreams.LocalStream().get_audience(status) self.assertEqual(users, []) - def test_federatedstream_stream_users(self, *_): + def test_federatedstream_get_audience(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" ) - users = activitystreams.FederatedStream().stream_users(status) + users = activitystreams.FederatedStream().get_audience(status) self.assertTrue(self.local_user in users) self.assertTrue(self.another_user in users) - def test_federatedstream_stream_users_unlisted(self, *_): + def test_federatedstream_get_audience_unlisted(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="unlisted" ) - users = activitystreams.FederatedStream().stream_users(status) + users = activitystreams.FederatedStream().get_audience(status) self.assertEqual(users, []) From 2e245f84be846e967a1488aeb7f8b4ffe66408e0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 18:02:24 -0700 Subject: [PATCH 07/18] Adds test for loading remote boosted statuses --- bookwyrm/tests/views/test_inbox.py | 50 ++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 37cf00ddc..6973f9814 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -572,6 +572,56 @@ class Inbox(TestCase): self.assertEqual(notification.user, self.local_user) self.assertEqual(notification.related_status, self.status) + @responses.activate + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_boost_remote_status(self, redis_mock): + """ boost a status """ + work = models.Work.objects.create(title="work title") + book = models.Edition.objects.create( + title="Test", + remote_id="https://bookwyrm.social/book/37292", + parent_work=work, + ) + self.assertEqual(models.Notification.objects.count(), 0) + activity = { + "type": "Announce", + "id": "%s/boost" % self.status.remote_id, + "actor": self.remote_user.remote_id, + "object": "https://remote.com/status/1", + "to": ["https://www.w3.org/ns/activitystreams#public"], + "cc": ["https://example.com/user/mouse/followers"], + "published": "Mon, 25 May 2020 19:31:20 GMT", + } + responses.add( + responses.GET, + "https://remote.com/status/1", + json={ + "id": "https://remote.com/status/1", + "type": "Comment", + "published": "2021-04-05T18:04:59.735190+00:00", + "attributedTo": self.remote_user.remote_id, + "content": "

a comment

", + "to": ["https://www.w3.org/ns/activitystreams#Public"], + "cc": ["https://b875df3d118b.ngrok.io/user/mouse/followers"], + "inReplyTo": "", + "inReplyToBook": book.remote_id, + "summary": "", + "tag": [], + "sensitive": False, + "@context": "https://www.w3.org/ns/activitystreams" + } + ) + + with patch("bookwyrm.models.status.Status.ignore_activity") as discarder: + discarder.return_value = False + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) + + boost = models.Boost.objects.get() + self.assertEqual(boost.boosted_status.remote_id, "https://remote.com/status/1") + self.assertEqual(boost.boosted_status.comment.status_type, "Comment") + self.assertEqual(boost.boosted_status.comment.book, book) + @responses.activate def test_handle_discarded_boost(self): """ test a boost of a mastodon status that will be discarded """ From a39cd670eff52d27a4145cb134a63d9726d7405d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 5 Apr 2021 18:05:06 -0700 Subject: [PATCH 08/18] Fixes boosted remote statuses coming in as Notes --- bookwyrm/activitypub/base_activity.py | 3 ++- bookwyrm/tests/views/test_inbox.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 768eb2084..452f61e03 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -265,7 +265,8 @@ def resolve_remote_id( "Could not connect to host for remote_id in: %s" % (remote_id) ) # determine the model implicitly, if not provided - if not model: + # or if it's a model with subclasses like Status, check again + if not model or hasattr(model.objects, "select_subclasses"): model = get_model_from_type(data.get("type")) # check for existing items with shared unique identifiers diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 6973f9814..f44a79c69 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -608,8 +608,8 @@ class Inbox(TestCase): "summary": "", "tag": [], "sensitive": False, - "@context": "https://www.w3.org/ns/activitystreams" - } + "@context": "https://www.w3.org/ns/activitystreams", + }, ) with patch("bookwyrm.models.status.Status.ignore_activity") as discarder: From fd66ff1861fbaaef572122c5d2636cb1e9ba067f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 6 Apr 2021 07:53:34 -0700 Subject: [PATCH 09/18] Small tweaks to commends and super() calls --- bookwyrm/activitystreams.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 2db6cc20a..949ae9dad 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -19,12 +19,12 @@ class ActivityStream(RedisStore): return "{}-unread".format(self.stream_id(user)) def get_rank(self, obj): # pylint: disable=no-self-use - """ the sort rank of a status, which is published date """ + """ statuses are sorted by date published """ return obj.published_date.timestamp() def add_status(self, status): """ add a status to users' feeds """ - # the pipeline contains all the addp-to-stream activities + # the pipeline contains all the add-to-stream activities pipeline = self.add_object_to_related_stores(status, execute=False) for user in self.get_audience(status): @@ -36,11 +36,13 @@ class ActivityStream(RedisStore): def add_user_statuses(self, viewer, user): """ add a user's statuses to another user's feed """ + # only add the statuses that the viewer should be able to see (ie, not dms) statuses = privacy_filter(viewer, user.status_set.all()) self.bulk_add_objects_to_store(statuses, self.stream_id(viewer)) def remove_user_statuses(self, viewer, user): """ remove a user's status from another user's feed """ + # remove all so that followers only statuses are removed statuses = user.status_set.all() self.bulk_remove_objects_from_store(statuses, self.stream_id(viewer)) @@ -49,7 +51,7 @@ class ActivityStream(RedisStore): # clear unreads for this feed r.set(self.unread_id(user), 0) - statuses = super().get_store(self.stream_id(user)) + statuses = self.get_store(self.stream_id(user)) return ( models.Status.objects.select_subclasses() .filter(id__in=statuses) @@ -62,7 +64,7 @@ class ActivityStream(RedisStore): def populate_streams(self, user): """ go from zero to a timeline """ - super().populate_store(self.stream_id(user)) + self.populate_store(self.stream_id(user)) def get_audience(self, status): # pylint: disable=no-self-use """ given a status, what users should see it """ From 1f99710dcdc17b5f7fb42ecfa8676da3d9e2c3e7 Mon Sep 17 00:00:00 2001 From: tofuwabohu Date: Tue, 6 Apr 2021 22:36:24 +0200 Subject: [PATCH 10/18] Links to own user in menu --- bookwyrm/templates/layout.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templates/layout.html b/bookwyrm/templates/layout.html index 80eb386a5..0f100f2cd 100644 --- a/bookwyrm/templates/layout.html +++ b/bookwyrm/templates/layout.html @@ -70,7 +70,7 @@ {% if request.user.is_authenticated %}