zedfrigg/bookwyrm
1
0
Fork 0
Code Activity

Merge pull request #2723 from WesleyAC/get-audience-perf

Browse source 
Improve `HomeStream.get_audience` performance
This commit is contained in:
Mouse Reeve 2023-03-12 16:08:54 -07:00 • committed by GitHub
commit 600340771a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 70 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

77
bookwyrm/activitystreams.py
View file

@ -13,18 +13,18 @@ from bookwyrm.tasks import app, LOW, MEDIUM, HIGH
class ActivityStream(RedisStore): class ActivityStream(RedisStore):
"""a category of activity stream (like home, local, books)""" """a category of activity stream (like home, local, books)"""
def stream_id(self, user): def stream_id(self, user_id):
"""the redis key for this user's instance of this stream""" """the redis key for this user's instance of this stream"""
return f"{user.id}-{self.key}" return f"{user_id}-{self.key}"
def unread_id(self, user): def unread_id(self, user_id):
"""the redis key for this user's unread count for this stream""" """the redis key for this user's unread count for this stream"""
stream_id = self.stream_id(user) stream_id = self.stream_id(user_id)
return f"{stream_id}-unread" return f"{stream_id}-unread"
def unread_by_status_type_id(self, user): def unread_by_status_type_id(self, user_id):
"""the redis key for this user's unread count for this stream""" """the redis key for this user's unread count for this stream"""
stream_id = self.stream_id(user) stream_id = self.stream_id(user_id)
return f"{stream_id}-unread-by-type" return f"{stream_id}-unread-by-type"
def get_rank(self, obj): # pylint: disable=no-self-use def get_rank(self, obj): # pylint: disable=no-self-use
@ -37,12 +37,12 @@ class ActivityStream(RedisStore):
pipeline = self.add_object_to_related_stores(status, execute=False) pipeline = self.add_object_to_related_stores(status, execute=False)
if increment_unread: if increment_unread:
for user in self.get_audience(status): for user_id in self.get_audience(status):
# add to the unread status count # add to the unread status count
pipeline.incr(self.unread_id(user)) pipeline.incr(self.unread_id(user_id))
# add to the unread status count for status type # add to the unread status count for status type
pipeline.hincrby( pipeline.hincrby(
self.unread_by_status_type_id(user), get_status_type(status), 1 self.unread_by_status_type_id(user_id), get_status_type(status), 1
) )
# and go! # and go!
@ -52,21 +52,21 @@ class ActivityStream(RedisStore):
"""add a user's statuses to another user's feed""" """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) # only add the statuses that the viewer should be able to see (ie, not dms)
statuses = models.Status.privacy_filter(viewer).filter(user=user) statuses = models.Status.privacy_filter(viewer).filter(user=user)
self.bulk_add_objects_to_store(statuses, self.stream_id(viewer)) self.bulk_add_objects_to_store(statuses, self.stream_id(viewer.id))
def remove_user_statuses(self, viewer, user): def remove_user_statuses(self, viewer, user):
"""remove a user's status from another user's feed""" """remove a user's status from another user's feed"""
# remove all so that followers only statuses are removed # remove all so that followers only statuses are removed
statuses = user.status_set.all() statuses = user.status_set.all()
self.bulk_remove_objects_from_store(statuses, self.stream_id(viewer)) self.bulk_remove_objects_from_store(statuses, self.stream_id(viewer.id))
def get_activity_stream(self, user): def get_activity_stream(self, user):
"""load the statuses to be displayed""" """load the statuses to be displayed"""
# clear unreads for this feed # clear unreads for this feed
r.set(self.unread_id(user), 0) r.set(self.unread_id(user.id), 0)
r.delete(self.unread_by_status_type_id(user)) r.delete(self.unread_by_status_type_id(user.id))
statuses = self.get_store(self.stream_id(user)) statuses = self.get_store(self.stream_id(user.id))
return ( return (
models.Status.objects.select_subclasses() models.Status.objects.select_subclasses()
.filter(id__in=statuses) .filter(id__in=statuses)
@ -83,11 +83,11 @@ class ActivityStream(RedisStore):
def get_unread_count(self, user): def get_unread_count(self, user):
"""get the unread status count for this user's feed""" """get the unread status count for this user's feed"""
return int(r.get(self.unread_id(user)) or 0) return int(r.get(self.unread_id(user.id)) or 0)
def get_unread_count_by_status_type(self, user): def get_unread_count_by_status_type(self, user):
"""get the unread status count for this user's feed's status types""" """get the unread status count for this user's feed's status types"""
status_types = r.hgetall(self.unread_by_status_type_id(user)) status_types = r.hgetall(self.unread_by_status_type_id(user.id))
return { return {
str(key.decode("utf-8")): int(value) or 0 str(key.decode("utf-8")): int(value) or 0
for key, value in status_types.items() for key, value in status_types.items()
@ -95,9 +95,9 @@ class ActivityStream(RedisStore):
def populate_streams(self, user): def populate_streams(self, user):
"""go from zero to a timeline""" """go from zero to a timeline"""
self.populate_store(self.stream_id(user)) self.populate_store(self.stream_id(user.id))
def get_audience(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""" """given a status, what users should see it"""
# direct messages don't appeard in feeds, direct comments/reviews/etc do # direct messages don't appeard in feeds, direct comments/reviews/etc do
if status.privacy == "direct" and status.status_type == "Note": if status.privacy == "direct" and status.status_type == "Note":
@ -136,8 +136,12 @@ class ActivityStream(RedisStore):
) )
return audience.distinct() return audience.distinct()
def get_audience(self, status): # pylint: disable=no-self-use
"""given a status, what users should see it"""
return [user.id for user in self._get_audience(status)]
def get_stores_for_object(self, obj): def get_stores_for_object(self, obj):
return [self.stream_id(u) for u in self.get_audience(obj)] return [self.stream_id(user_id) for user_id in self.get_audience(obj)]
def get_statuses_for_user(self, user): # pylint: disable=no-self-use def get_statuses_for_user(self, user): # pylint: disable=no-self-use
"""given a user, what statuses should they see on this stream""" """given a user, what statuses should they see on this stream"""
@ -157,13 +161,14 @@ class HomeStream(ActivityStream):
key = "home" key = "home"
def get_audience(self, status): def get_audience(self, status):
audience = super().get_audience(status) audience = super()._get_audience(status)
if not audience: if not audience:
return [] return []
return audience.filter( # if the user is the post's author
Q(id=status.user.id) # if the user is the post's author ids_self = [user.id for user in audience.filter(Q(id=status.user.id))]
| Q(following=status.user) # if the user is following the author # if the user is following the author
).distinct() ids_following = [user.id for user in audience.filter(Q(following=status.user))]
return ids_self + ids_following
def get_statuses_for_user(self, user): def get_statuses_for_user(self, user):
return models.Status.privacy_filter( return models.Status.privacy_filter(
@ -183,11 +188,11 @@ class LocalStream(ActivityStream):
key = "local" key = "local"
def get_audience(self, status): def _get_audience(self, status):
# this stream wants no part in non-public statuses # this stream wants no part in non-public statuses
if status.privacy != "public" or not status.user.local: if status.privacy != "public" or not status.user.local:
return [] return []
return super().get_audience(status) return super()._get_audience(status)
def get_statuses_for_user(self, user): def get_statuses_for_user(self, user):
# all public statuses by a local user # all public statuses by a local user
@ -202,7 +207,7 @@ class BooksStream(ActivityStream):
key = "books" key = "books"
def get_audience(self, status): def _get_audience(self, status):
"""anyone with the mentioned book on their shelves""" """anyone with the mentioned book on their shelves"""
# only show public statuses on the books feed, # only show public statuses on the books feed,
# and only statuses that mention books # and only statuses that mention books
@ -217,7 +222,7 @@ class BooksStream(ActivityStream):
else status.mention_books.first().parent_work else status.mention_books.first().parent_work
) )
audience = super().get_audience(status) audience = super()._get_audience(status)
if not audience: if not audience:
return [] return []
return audience.filter(shelfbook__book__parent_work=work).distinct() return audience.filter(shelfbook__book__parent_work=work).distinct()
@ -254,10 +259,10 @@ class BooksStream(ActivityStream):
book_reviews = statuses.filter(Q(review__book__parent_work=work)) book_reviews = statuses.filter(Q(review__book__parent_work=work))
book_mentions = statuses.filter(Q(mention_books__parent_work=work)) book_mentions = statuses.filter(Q(mention_books__parent_work=work))
self.bulk_add_objects_to_store(book_comments, self.stream_id(user)) self.bulk_add_objects_to_store(book_comments, self.stream_id(user.id))
self.bulk_add_objects_to_store(book_quotations, self.stream_id(user)) self.bulk_add_objects_to_store(book_quotations, self.stream_id(user.id))
self.bulk_add_objects_to_store(book_reviews, self.stream_id(user)) self.bulk_add_objects_to_store(book_reviews, self.stream_id(user.id))
self.bulk_add_objects_to_store(book_mentions, self.stream_id(user)) self.bulk_add_objects_to_store(book_mentions, self.stream_id(user.id))
def remove_book_statuses(self, user, book): def remove_book_statuses(self, user, book):
"""add statuses about a book to a user's feed""" """add statuses about a book to a user's feed"""
@ -272,10 +277,10 @@ class BooksStream(ActivityStream):
book_reviews = statuses.filter(Q(review__book__parent_work=work)) book_reviews = statuses.filter(Q(review__book__parent_work=work))
book_mentions = statuses.filter(Q(mention_books__parent_work=work)) book_mentions = statuses.filter(Q(mention_books__parent_work=work))
self.bulk_remove_objects_from_store(book_comments, self.stream_id(user)) self.bulk_remove_objects_from_store(book_comments, self.stream_id(user.id))
self.bulk_remove_objects_from_store(book_quotations, self.stream_id(user)) self.bulk_remove_objects_from_store(book_quotations, self.stream_id(user.id))
self.bulk_remove_objects_from_store(book_reviews, self.stream_id(user)) self.bulk_remove_objects_from_store(book_reviews, self.stream_id(user.id))
self.bulk_remove_objects_from_store(book_mentions, self.stream_id(user)) self.bulk_remove_objects_from_store(book_mentions, self.stream_id(user.id))
# determine which streams are enabled in settings.py # determine which streams are enabled in settings.py

38
bookwyrm/tests/activitystreams/test_abstractstream.py
View file

@ -53,18 +53,18 @@ class Activitystreams(TestCase):
def test_activitystream_class_ids(self, *_): def test_activitystream_class_ids(self, *_):
"""the abstract base class for stream objects""" """the abstract base class for stream objects"""
self.assertEqual( self.assertEqual(
self.test_stream.stream_id(self.local_user), self.test_stream.stream_id(self.local_user.id),
f"{self.local_user.id}-test", f"{self.local_user.id}-test",
) )
self.assertEqual( self.assertEqual(
self.test_stream.unread_id(self.local_user), self.test_stream.unread_id(self.local_user.id),
f"{self.local_user.id}-test-unread", f"{self.local_user.id}-test-unread",
) )
def test_unread_by_status_type_id(self, *_): def test_unread_by_status_type_id(self, *_):
"""stream for status type""" """stream for status type"""
self.assertEqual( self.assertEqual(
self.test_stream.unread_by_status_type_id(self.local_user), self.test_stream.unread_by_status_type_id(self.local_user.id),
f"{self.local_user.id}-test-unread-by-type", f"{self.local_user.id}-test-unread-by-type",
) )
@ -118,9 +118,9 @@ class Activitystreams(TestCase):
) )
users = self.test_stream.get_audience(status) users = self.test_stream.get_audience(status)
# remote users don't have feeds # remote users don't have feeds
self.assertFalse(self.remote_user in users) self.assertFalse(self.remote_user.id in users)
self.assertTrue(self.local_user in users) self.assertTrue(self.local_user.id in users)
self.assertTrue(self.another_user in users) self.assertTrue(self.another_user.id in users)
def test_abstractstream_get_audience_direct(self, *_): def test_abstractstream_get_audience_direct(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -141,9 +141,9 @@ class Activitystreams(TestCase):
) )
status.mention_users.add(self.local_user) status.mention_users.add(self.local_user)
users = self.test_stream.get_audience(status) users = self.test_stream.get_audience(status)
self.assertTrue(self.local_user in users) self.assertTrue(self.local_user.id in users)
self.assertFalse(self.another_user in users) self.assertFalse(self.another_user.id in users)
self.assertFalse(self.remote_user in users) self.assertFalse(self.remote_user.id in users)
def test_abstractstream_get_audience_followers_remote_user(self, *_): def test_abstractstream_get_audience_followers_remote_user(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -153,7 +153,7 @@ class Activitystreams(TestCase):
privacy="followers", privacy="followers",
) )
users = self.test_stream.get_audience(status) users = self.test_stream.get_audience(status)
self.assertFalse(users.exists()) self.assertEqual(users, [])
def test_abstractstream_get_audience_followers_self(self, *_): def test_abstractstream_get_audience_followers_self(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -164,9 +164,9 @@ class Activitystreams(TestCase):
book=self.book, book=self.book,
) )
users = self.test_stream.get_audience(status) users = self.test_stream.get_audience(status)
self.assertTrue(self.local_user in users) self.assertTrue(self.local_user.id in users)
self.assertFalse(self.another_user in users) self.assertFalse(self.another_user.id in users)
self.assertFalse(self.remote_user in users) self.assertFalse(self.remote_user.id in users)
def test_abstractstream_get_audience_followers_with_mention(self, *_): def test_abstractstream_get_audience_followers_with_mention(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -179,9 +179,9 @@ class Activitystreams(TestCase):
status.mention_users.add(self.local_user) status.mention_users.add(self.local_user)
users = self.test_stream.get_audience(status) users = self.test_stream.get_audience(status)
self.assertTrue(self.local_user in users) self.assertTrue(self.local_user.id in users)
self.assertFalse(self.another_user in users) self.assertFalse(self.another_user.id in users)
self.assertFalse(self.remote_user in users) self.assertFalse(self.remote_user.id in users)
def test_abstractstream_get_audience_followers_with_relationship(self, *_): def test_abstractstream_get_audience_followers_with_relationship(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -193,6 +193,6 @@ class Activitystreams(TestCase):
book=self.book, book=self.book,
) )
users = self.test_stream.get_audience(status) users = self.test_stream.get_audience(status)
self.assertFalse(self.local_user in users) self.assertFalse(self.local_user.id in users)
self.assertFalse(self.another_user in users) self.assertFalse(self.another_user.id in users)
self.assertFalse(self.remote_user in users) self.assertFalse(self.remote_user.id in users)

10
bookwyrm/tests/activitystreams/test_homestream.py
View file

@ -44,7 +44,7 @@ class Activitystreams(TestCase):
user=self.remote_user, content="hi", privacy="public" user=self.remote_user, content="hi", privacy="public"
) )
users = activitystreams.HomeStream().get_audience(status) users = activitystreams.HomeStream().get_audience(status)
self.assertFalse(users.exists()) self.assertEqual(users, [])
def test_homestream_get_audience_with_mentions(self, *_): def test_homestream_get_audience_with_mentions(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -53,8 +53,8 @@ class Activitystreams(TestCase):
) )
status.mention_users.add(self.local_user) status.mention_users.add(self.local_user)
users = activitystreams.HomeStream().get_audience(status) users = activitystreams.HomeStream().get_audience(status)
self.assertFalse(self.local_user in users) self.assertFalse(self.local_user.id in users)
self.assertFalse(self.another_user in users) self.assertFalse(self.another_user.id in users)
def test_homestream_get_audience_with_relationship(self, *_): def test_homestream_get_audience_with_relationship(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -63,5 +63,5 @@ class Activitystreams(TestCase):
user=self.remote_user, content="hi", privacy="public" user=self.remote_user, content="hi", privacy="public"
) )
users = activitystreams.HomeStream().get_audience(status) users = activitystreams.HomeStream().get_audience(status)
self.assertTrue(self.local_user in users) self.assertTrue(self.local_user.id in users)
self.assertFalse(self.another_user in users) self.assertFalse(self.another_user.id in users)

10
bookwyrm/tests/activitystreams/test_localstream.py
View file

@ -54,8 +54,8 @@ class Activitystreams(TestCase):
user=self.local_user, content="hi", privacy="public" user=self.local_user, content="hi", privacy="public"
) )
users = activitystreams.LocalStream().get_audience(status) users = activitystreams.LocalStream().get_audience(status)
self.assertTrue(self.local_user in users) self.assertTrue(self.local_user.id in users)
self.assertTrue(self.another_user in users) self.assertTrue(self.another_user.id in users)
def test_localstream_get_audience_unlisted(self, *_): def test_localstream_get_audience_unlisted(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -88,7 +88,7 @@ class Activitystreams(TestCase):
) )
# yes book, yes audience # yes book, yes audience
audience = activitystreams.BooksStream().get_audience(status) audience = activitystreams.BooksStream().get_audience(status)
self.assertTrue(self.local_user in audience) self.assertTrue(self.local_user.id in audience)
def test_localstream_get_audience_books_book_field(self, *_): def test_localstream_get_audience_books_book_field(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -102,7 +102,7 @@ class Activitystreams(TestCase):
) )
# yes book, yes audience # yes book, yes audience
audience = activitystreams.BooksStream().get_audience(status) audience = activitystreams.BooksStream().get_audience(status)
self.assertTrue(self.local_user in audience) self.assertTrue(self.local_user.id in audience)
def test_localstream_get_audience_books_alternate_edition(self, *_): def test_localstream_get_audience_books_alternate_edition(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""
@ -119,7 +119,7 @@ class Activitystreams(TestCase):
) )
# yes book, yes audience # yes book, yes audience
audience = activitystreams.BooksStream().get_audience(status) audience = activitystreams.BooksStream().get_audience(status)
self.assertTrue(self.local_user in audience) self.assertTrue(self.local_user.id in audience)
def test_localstream_get_audience_books_non_public(self, *_): def test_localstream_get_audience_books_non_public(self, *_):
"""get a list of users that should see a status""" """get a list of users that should see a status"""