From ebc01362e6884157746b031d2974459d58cf91cd Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 10:44:42 -0700 Subject: [PATCH 01/33] Adds redis image for activity streams --- .env.example | 9 +++++++-- bookwyrm/settings.py | 3 +++ docker-compose.yml | 15 ++++++++++++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.env.example b/.env.example index 7a67045cd..7f80a72ce 100644 --- a/.env.example +++ b/.env.example @@ -22,8 +22,13 @@ POSTGRES_USER=fedireads POSTGRES_DB=fedireads POSTGRES_HOST=db -CELERY_BROKER=redis://redis:6379/0 -CELERY_RESULT_BACKEND=redis://redis:6379/0 +# Redis activity stream manager +REDIS_ACTIVITY_HOST=redis_activity +REDIS_ACTIVITY_PORT=6379 + +# Celery config with redis broker +CELERY_BROKER=redis://redis_broker:6379/0 +CELERY_RESULT_BACKEND=redis://redis_broker:6379/0 EMAIL_HOST="smtp.mailgun.org" EMAIL_PORT=587 diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index bcff58287..cd8448503 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -92,6 +92,9 @@ TEMPLATES = [ WSGI_APPLICATION = "bookwyrm.wsgi.application" +# redis +REDIS_ACTIVITY_HOST = env("REDIS_ACTIVITY_HOST", "localhost") +REDIS_ACTIVITY_PORT = env("REDIS_ACTIVITY_PORT", 6379) # Database # https://docs.djangoproject.com/en/2.0/ref/settings/#databases diff --git a/docker-compose.yml b/docker-compose.yml index 5d9cbf1be..3ee9037f9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -31,11 +31,20 @@ services: depends_on: - db - celery_worker + - redis_activity networks: - main ports: - 8000:8000 - redis: + redis_activity: + image: redis + env_file: .env + ports: + - 6378:6378 + networks: + - main + restart: on-failure + redis_broker: image: redis env_file: .env ports: @@ -55,7 +64,7 @@ services: - media_volume:/app/images depends_on: - db - - redis + - redis_broker restart: on-failure flower: build: . @@ -67,7 +76,7 @@ services: - main depends_on: - db - - redis + - redis_broker restart: on-failure ports: - 8888:8888 From 459479db43feb217854f4e272893086c895f3790 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 14:11:23 -0700 Subject: [PATCH 02/33] Add statuses to timelines --- bookwyrm/models/base_model.py | 2 +- bookwyrm/models/status.py | 77 ++++++++++++++++++++++++++++++++++- bookwyrm/settings.py | 4 +- bookwyrm/views/feed.py | 27 ++++++------ 4 files changed, 95 insertions(+), 15 deletions(-) diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index 60e5da0ad..cb2fc851e 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -34,7 +34,7 @@ class BookWyrmModel(models.Model): @receiver(models.signals.post_save) # pylint: disable=unused-argument -def execute_after_save(sender, instance, created, *args, **kwargs): +def set_remote_id(sender, instance, created, *args, **kwargs): """ set the remote_id after save (when the id is available) """ if not created or not hasattr(instance, "get_remote_id"): return diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 904ce461d..6d93ff0a2 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -5,11 +5,14 @@ import re from django.apps import apps from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models +from django.db.models import Q +from django.dispatch import receiver from django.template.loader import get_template from django.utils import timezone from model_utils.managers import InheritanceManager +import redis -from bookwyrm import activitypub +from bookwyrm import activitypub, settings from .activitypub_mixin import ActivitypubMixin, ActivityMixin from .activitypub_mixin import OrderedCollectionPageMixin from .base_model import BookWyrmModel @@ -17,6 +20,10 @@ from .fields import image_serializer from .readthrough import ProgressMode from . import fields +r = redis.Redis( + host=settings.REDIS_ACTIVITY_HOST, port=settings.REDIS_ACTIVITY_PORT, db=0 +) + class Status(OrderedCollectionPageMixin, BookWyrmModel): """ any post, like a reply to a review, etc """ @@ -383,3 +390,71 @@ class Boost(ActivityMixin, Status): # This constraint can't work as it would cross tables. # class Meta: # unique_together = ('user', 'boosted_status') + + +@receiver(models.signals.post_save) +# pylint: disable=unused-argument +def update_feeds(sender, instance, created, *args, **kwargs): + """ add statuses to activity feeds """ + # we're only interested in new statuses that aren't dms + if not created or not issubclass(sender, Status) or instance.privacy == 'direct': + return + + user = instance.user + + community = user.__class__.objects.filter( + local=True # we only manage timelines for local users + ).exclude( + Q(id__in=user.blocks.all()) | Q(blocks=user) # not blocked + ) + + # ------ home timeline: users you follow and yourself + friends = community.filter( + Q(id=user.id) | Q(following=user) + ) + add_status(friends, instance, 'home') + + # local and federated timelines only get public statuses + if instance.privacy != 'public': + return + + # ------ federated timeline: to anyone, anywhere + add_status(community, instance, 'federated') + + # if the author is a remote user, it doesn't go on the local timeline + if not user.local: + return + + # ------ local timeline: to anyone, anywhere + add_status(community, instance, 'local') + + +def add_status(users, status, feed_name): + """ add a status to users' feeds """ + # we want to do this as a bulk operation + pipeline = r.pipeline() + value = {status.id: status.published_date.timestamp()} + for user in users: + feed_id = '{}-{}'.format(user.id, feed_name) + unread_feed_id = '{}-unread'.format(feed_id) + + # add the status to the feed + pipeline.zadd(feed_id, value) + + # add to the unread status count + pipeline.incr(unread_feed_id) + pipeline.execute() + + +def get_activity_stream(user, feed_name, start, end): + """ load the ids for statuses to be displayed """ + feed_id = '{}-{}'.format(user.id, feed_name) + unread_feed_id = '{}-unread'.format(feed_id) + + # clear unreads for this feed + r.set(unread_feed_id, 0) + + statuses = r.zrange(feed_id, start, end) + return Status.objects.select_subclasses().filter( + id__in=statuses + ).order_by('-published_date') diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index cd8448503..bc210dacc 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -92,10 +92,12 @@ TEMPLATES = [ WSGI_APPLICATION = "bookwyrm.wsgi.application" -# redis +# redis/activity streams settings REDIS_ACTIVITY_HOST = env("REDIS_ACTIVITY_HOST", "localhost") REDIS_ACTIVITY_PORT = env("REDIS_ACTIVITY_PORT", 6379) +MAX_STREAM_LENGTH = env("MAX_STREAM_LENGTH", 200) + # Database # https://docs.djangoproject.com/en/2.0/ref/settings/#databases diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index d08c9a42c..a19c2738a 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -28,19 +28,22 @@ class Feed(View): except ValueError: page = 1 - if tab == "home": - activities = get_activity_feed(request.user, following_only=True) + try: + tab_title = { + 'home': _("Home"), + "local": _("Local"), + "federated": _("Federated") + }[tab] + except KeyError: + tab = 'home' tab_title = _("Home") - elif tab == "local": - activities = get_activity_feed( - request.user, privacy=["public", "followers"], local_only=True - ) - tab_title = _("Local") - else: - activities = get_activity_feed( - request.user, privacy=["public", "followers"] - ) - tab_title = _("Federated") + + activities = models.status.get_activity_stream( + request.user, tab, + (1 - page) * PAGE_LENGTH, + page * PAGE_LENGTH + ) + paginated = Paginator(activities, PAGE_LENGTH) data = { From 3efabf1da371a27de04fcd8f4a67ae51afd6562b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 18:39:16 -0700 Subject: [PATCH 03/33] Creates activity stream class --- bookwyrm/activitystreams.py | 176 ++++++++++++++++++++++++++++++++++++ bookwyrm/models/status.py | 77 +--------------- bookwyrm/settings.py | 1 + bookwyrm/views/feed.py | 25 ++--- 4 files changed, 189 insertions(+), 90 deletions(-) create mode 100644 bookwyrm/activitystreams.py diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py new file mode 100644 index 000000000..bceb1cad4 --- /dev/null +++ b/bookwyrm/activitystreams.py @@ -0,0 +1,176 @@ +""" access the activity streams stored in redis """ +from abc import ABC +from django.dispatch import receiver +from django.db.models import signals +from django.db.models import Q +import redis + +from bookwyrm import models, settings +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): + """ a category of activity stream (like home, local, federated) """ + + def stream_id(self, user): + """ the redis key for this user's instance of this stream """ + return '{}-{}'.format(user.id, self.key) + + def unread_id(self, user): + """ the redis key for this user's unread count for this stream """ + return '{}-unread'.format(self.stream_id(user)) + + 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) + + # add to the unread status count + pipeline.incr(self.unread_id(user)) + # and go! + pipeline.execute() + + + 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_activity_stream(self, user): + """ load the ids for 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) + return models.Status.objects.select_subclasses().filter( + id__in=statuses + ).order_by('-published_date') + + + def populate_stream(self, user): + ''' go from zero to a timeline ''' + pipeline = r.pipeline() + statuses = self.stream_statuses(user) + + stream_id = self.stream_id(user) + for status in statuses.all()[:settings.MAX_STREAM_LENGTH]: + pipeline.zadd(stream_id, self.get_value(status)) + pipeline.execute() + + + def stream_users(self, status): # pylint: disable=no-self-use + """ given a status, what users should see it """ + # direct messages don't appeard in feeds. + if status.privacy == 'direct': + return None + + # everybody who could plausibly see this status + audience = models.User.objects.filter( + is_active=True, + local=True # we only create feeds for users of this instance + ).exclude( + Q(id__in=status.user.blocks.all()) | Q(blocks=status.user) # not blocked + ) + + # only visible to the poster's followers and tagged users + if status.privacy == 'followers': + audience = audience.filter( + Q(id=status.user.id) # if the user is the post's author + | Q(following=status.user) # if the user is following the author + ) + return audience + + + def stream_statuses(self, user): # pylint: disable=no-self-use + """ given a user, what statuses should they see on this stream """ + return privacy_filter( + user, + models.Status.objects.select_subclasses(), + privacy_levels=["public", 'unlisted', 'followers'], + ) + + +class HomeStream(ActivityStream): + """ users you follow """ + key = 'home' + + def stream_users(self, status): + audience = super().stream_users(status) + return audience.filter( + Q(id=status.user.id) # if the user is the post's author + | Q(following=status.user) # if the user is following the author + | Q(id__in=status.mention_users.all()) # or the user is mentioned + ) + + def stream_statuses(self, user): + return privacy_filter( + user, + models.Status.objects.select_subclasses(), + privacy_levels=["public", 'unlisted', 'followers'], + following_only=True + ) + + +class LocalStream(ActivityStream): + """ users you follow """ + key = 'local' + + def stream_users(self, status): + # this stream wants no part in non-public statuses + if status.privacy != 'public': + return None + return super().stream_users(status) + + def stream_statuses(self, user): + # all public statuses by a local user + return privacy_filter( + user, + models.Status.objects.select_subclasses().filter(user__local=True), + privacy_levels=["public"], + ) + + +class FederatedStream(ActivityStream): + """ users you follow """ + key = 'federated' + + def stream_users(self, status): + # this stream wants no part in non-public statuses + if status.privacy != 'public': + return None + return super().stream_users(status) + + def stream_statuses(self, user): + return privacy_filter( + user, + models.Status.objects.select_subclasses(), + privacy_levels=["public"], + ) + + + +streams = { + 'home': HomeStream(), + 'local': LocalStream(), + 'federated': FederatedStream(), +} + +@receiver(signals.post_save) +# pylint: disable=unused-argument +def update_feeds(sender, instance, created, *args, **kwargs): + """ add statuses to activity feeds """ + # we're only interested in new statuses that aren't dms + if not created or not issubclass(sender, models.Status) or \ + instance.privacy == 'direct': + return + + for stream in streams.values(): + stream.add_status(instance) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 6d93ff0a2..b313ac862 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -10,9 +10,8 @@ from django.dispatch import receiver from django.template.loader import get_template from django.utils import timezone from model_utils.managers import InheritanceManager -import redis -from bookwyrm import activitypub, settings +from bookwyrm import activitypub from .activitypub_mixin import ActivitypubMixin, ActivityMixin from .activitypub_mixin import OrderedCollectionPageMixin from .base_model import BookWyrmModel @@ -20,10 +19,6 @@ from .fields import image_serializer from .readthrough import ProgressMode from . import fields -r = redis.Redis( - host=settings.REDIS_ACTIVITY_HOST, port=settings.REDIS_ACTIVITY_PORT, db=0 -) - class Status(OrderedCollectionPageMixin, BookWyrmModel): """ any post, like a reply to a review, etc """ @@ -121,7 +116,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): return list(set(mentions)) @classmethod - def ignore_activity(cls, activity): + def ignore_activity(cls, activity): # pylint: disable=too-many-return-statements """ keep notes if they are replies to existing statuses """ if activity.type == "Announce": try: @@ -390,71 +385,3 @@ class Boost(ActivityMixin, Status): # This constraint can't work as it would cross tables. # class Meta: # unique_together = ('user', 'boosted_status') - - -@receiver(models.signals.post_save) -# pylint: disable=unused-argument -def update_feeds(sender, instance, created, *args, **kwargs): - """ add statuses to activity feeds """ - # we're only interested in new statuses that aren't dms - if not created or not issubclass(sender, Status) or instance.privacy == 'direct': - return - - user = instance.user - - community = user.__class__.objects.filter( - local=True # we only manage timelines for local users - ).exclude( - Q(id__in=user.blocks.all()) | Q(blocks=user) # not blocked - ) - - # ------ home timeline: users you follow and yourself - friends = community.filter( - Q(id=user.id) | Q(following=user) - ) - add_status(friends, instance, 'home') - - # local and federated timelines only get public statuses - if instance.privacy != 'public': - return - - # ------ federated timeline: to anyone, anywhere - add_status(community, instance, 'federated') - - # if the author is a remote user, it doesn't go on the local timeline - if not user.local: - return - - # ------ local timeline: to anyone, anywhere - add_status(community, instance, 'local') - - -def add_status(users, status, feed_name): - """ add a status to users' feeds """ - # we want to do this as a bulk operation - pipeline = r.pipeline() - value = {status.id: status.published_date.timestamp()} - for user in users: - feed_id = '{}-{}'.format(user.id, feed_name) - unread_feed_id = '{}-unread'.format(feed_id) - - # add the status to the feed - pipeline.zadd(feed_id, value) - - # add to the unread status count - pipeline.incr(unread_feed_id) - pipeline.execute() - - -def get_activity_stream(user, feed_name, start, end): - """ load the ids for statuses to be displayed """ - feed_id = '{}-{}'.format(user.id, feed_name) - unread_feed_id = '{}-unread'.format(feed_id) - - # clear unreads for this feed - r.set(unread_feed_id, 0) - - statuses = r.zrange(feed_id, start, end) - return Status.objects.select_subclasses().filter( - id__in=statuses - ).order_by('-published_date') diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index bc210dacc..368064355 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -97,6 +97,7 @@ REDIS_ACTIVITY_HOST = env("REDIS_ACTIVITY_HOST", "localhost") REDIS_ACTIVITY_PORT = env("REDIS_ACTIVITY_PORT", 6379) MAX_STREAM_LENGTH = env("MAX_STREAM_LENGTH", 200) +STREAMS = ["home", "local", "federated"] # Database # https://docs.djangoproject.com/en/2.0/ref/settings/#databases diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index a19c2738a..4c003f390 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -9,9 +9,9 @@ from django.utils.decorators import method_decorator from django.utils.translation import gettext as _ from django.views import View -from bookwyrm import forms, models +from bookwyrm import activitystreams, forms, models from bookwyrm.activitypub import ActivitypubResponse -from bookwyrm.settings import PAGE_LENGTH +from bookwyrm.settings import PAGE_LENGTH, STREAMS from .helpers import get_activity_feed, get_user_from_username from .helpers import is_api_request, is_bookwyrm_request, object_visible_to_user @@ -28,21 +28,16 @@ class Feed(View): except ValueError: page = 1 - try: - tab_title = { - 'home': _("Home"), - "local": _("Local"), - "federated": _("Federated") - }[tab] - except KeyError: + if not tab in STREAMS: tab = 'home' - tab_title = _("Home") - activities = models.status.get_activity_stream( - request.user, tab, - (1 - page) * PAGE_LENGTH, - page * PAGE_LENGTH - ) + tab_title = { + 'home': _("Home"), + "local": _("Local"), + "federated": _("Federated") + }[tab] + + activities = activitystreams.streams[tab].get_activity_stream(request.user) paginated = Paginator(activities, PAGE_LENGTH) From 2fe9d1044a7651fd9cd59061ec77ac412c58be4a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 18:42:12 -0700 Subject: [PATCH 04/33] Moves tab title translations into templates --- bookwyrm/templates/feed/feed.html | 10 +++++++++- bookwyrm/views/feed.py | 8 -------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bookwyrm/templates/feed/feed.html b/bookwyrm/templates/feed/feed.html index b7ff6e253..5ad18db66 100644 --- a/bookwyrm/templates/feed/feed.html +++ b/bookwyrm/templates/feed/feed.html @@ -3,7 +3,15 @@ {% load bookwyrm_tags %} {% block panel %} -

{% blocktrans %}{{ tab_title }} Timeline{% endblocktrans %}

+

+ {% if tab == 'home' %} + {% trans "Home Timeline" %} + {% elif tab == 'local' %} + {% trans "Local Timeline" %} + {% else %} + {% trans "Federated Timeline" %} + {% endif %} +

  • diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index 4c003f390..a07ea5240 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -6,7 +6,6 @@ from django.http import HttpResponseNotFound from django.template.response import TemplateResponse from django.utils import timezone from django.utils.decorators import method_decorator -from django.utils.translation import gettext as _ from django.views import View from bookwyrm import activitystreams, forms, models @@ -31,12 +30,6 @@ class Feed(View): if not tab in STREAMS: tab = 'home' - tab_title = { - 'home': _("Home"), - "local": _("Local"), - "federated": _("Federated") - }[tab] - activities = activitystreams.streams[tab].get_activity_stream(request.user) paginated = Paginator(activities, PAGE_LENGTH) @@ -47,7 +40,6 @@ class Feed(View): "user": request.user, "activities": paginated.page(page), "tab": tab, - "tab_title": tab_title, "goal_form": forms.GoalForm(), "path": "/%s" % tab, }, From 0caea7e9ffb2a84f582157b1891778fa5ceec5b1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 18:54:17 -0700 Subject: [PATCH 05/33] Python formatting --- bookwyrm/activitystreams.py | 67 +++++++++++++++++++------------------ bookwyrm/views/feed.py | 2 +- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index bceb1cad4..6910d7bc2 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -18,11 +18,11 @@ class ActivityStream(ABC): def stream_id(self, user): """ the redis key for this user's instance of this stream """ - return '{}-{}'.format(user.id, self.key) + return "{}-{}".format(user.id, self.key) def unread_id(self, user): """ the redis key for this user's unread count for this stream """ - return '{}-unread'.format(self.stream_id(user)) + return "{}-unread".format(self.stream_id(user)) def add_status(self, status): """ add a status to users' feeds """ @@ -38,94 +38,93 @@ class ActivityStream(ABC): # and go! pipeline.execute() - 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_activity_stream(self, user): """ load the ids for 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) - return models.Status.objects.select_subclasses().filter( - id__in=statuses - ).order_by('-published_date') - + return ( + models.Status.objects.select_subclasses() + .filter(id__in=statuses) + .order_by("-published_date") + ) def populate_stream(self, user): - ''' go from zero to a timeline ''' + """ go from zero to a timeline """ pipeline = r.pipeline() statuses = self.stream_statuses(user) stream_id = self.stream_id(user) - for status in statuses.all()[:settings.MAX_STREAM_LENGTH]: + for status in statuses.all()[: settings.MAX_STREAM_LENGTH]: pipeline.zadd(stream_id, self.get_value(status)) pipeline.execute() - def stream_users(self, status): # pylint: disable=no-self-use """ given a status, what users should see it """ # direct messages don't appeard in feeds. - if status.privacy == 'direct': + if status.privacy == "direct": return None # everybody who could plausibly see this status audience = models.User.objects.filter( is_active=True, - local=True # we only create feeds for users of this instance + local=True, # we only create feeds for users of this instance ).exclude( Q(id__in=status.user.blocks.all()) | Q(blocks=status.user) # not blocked ) # only visible to the poster's followers and tagged users - if status.privacy == 'followers': + if status.privacy == "followers": audience = audience.filter( Q(id=status.user.id) # if the user is the post's author - | Q(following=status.user) # if the user is following the author + | Q(following=status.user) # if the user is following the author ) return audience - def stream_statuses(self, user): # pylint: disable=no-self-use """ given a user, what statuses should they see on this stream """ return privacy_filter( user, models.Status.objects.select_subclasses(), - privacy_levels=["public", 'unlisted', 'followers'], + privacy_levels=["public", "unlisted", "followers"], ) class HomeStream(ActivityStream): """ users you follow """ - key = 'home' + + key = "home" def stream_users(self, status): audience = super().stream_users(status) return audience.filter( Q(id=status.user.id) # if the user is the post's author - | Q(following=status.user) # if the user is following the author - | Q(id__in=status.mention_users.all()) # or the user is mentioned + | Q(following=status.user) # if the user is following the author + | Q(id__in=status.mention_users.all()) # or the user is mentioned ) def stream_statuses(self, user): return privacy_filter( user, models.Status.objects.select_subclasses(), - privacy_levels=["public", 'unlisted', 'followers'], - following_only=True + privacy_levels=["public", "unlisted", "followers"], + following_only=True, ) class LocalStream(ActivityStream): """ users you follow """ - key = 'local' + + key = "local" def stream_users(self, status): # this stream wants no part in non-public statuses - if status.privacy != 'public': + if status.privacy != "public": return None return super().stream_users(status) @@ -140,11 +139,12 @@ class LocalStream(ActivityStream): class FederatedStream(ActivityStream): """ users you follow """ - key = 'federated' + + key = "federated" def stream_users(self, status): # this stream wants no part in non-public statuses - if status.privacy != 'public': + if status.privacy != "public": return None return super().stream_users(status) @@ -156,20 +156,23 @@ class FederatedStream(ActivityStream): ) - streams = { - 'home': HomeStream(), - 'local': LocalStream(), - 'federated': FederatedStream(), + "home": HomeStream(), + "local": LocalStream(), + "federated": FederatedStream(), } + @receiver(signals.post_save) # pylint: disable=unused-argument def update_feeds(sender, instance, created, *args, **kwargs): """ add statuses to activity feeds """ # we're only interested in new statuses that aren't dms - if not created or not issubclass(sender, models.Status) or \ - instance.privacy == 'direct': + if ( + not created + or not issubclass(sender, models.Status) + or instance.privacy == "direct" + ): return for stream in streams.values(): diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index a07ea5240..b7ef9eb8c 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -28,7 +28,7 @@ class Feed(View): page = 1 if not tab in STREAMS: - tab = 'home' + tab = "home" activities = activitystreams.streams[tab].get_activity_stream(request.user) From 7eb494b4abc44ae2a2c4025f420fb82e6d99c7e9 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 19:17:46 -0700 Subject: [PATCH 06/33] Removes get activity feed function --- bookwyrm/activitystreams.py | 4 +- bookwyrm/models/status.py | 2 - bookwyrm/tests/views/test_helpers.py | 107 --------------------------- bookwyrm/views/books.py | 5 +- bookwyrm/views/feed.py | 14 +++- bookwyrm/views/helpers.py | 53 ++----------- bookwyrm/views/rss_feed.py | 8 +- bookwyrm/views/user.py | 8 +- 8 files changed, 27 insertions(+), 174 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 6910d7bc2..3ece05035 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -66,8 +66,8 @@ class ActivityStream(ABC): def stream_users(self, status): # pylint: disable=no-self-use """ given a status, what users should see it """ - # direct messages don't appeard in feeds. - if status.privacy == "direct": + # direct messages don't appeard in feeds, direct comments/reviews/etc do + if status.privacy == "direct" and status.status_type == 'Note': return None # everybody who could plausibly see this status diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index b313ac862..77ac4b582 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -5,8 +5,6 @@ import re from django.apps import apps from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models -from django.db.models import Q -from django.dispatch import receiver from django.template.loader import get_template from django.utils import timezone from model_utils.managers import InheritanceManager diff --git a/bookwyrm/tests/views/test_helpers.py b/bookwyrm/tests/views/test_helpers.py index bb4cf69c4..926f96222 100644 --- a/bookwyrm/tests/views/test_helpers.py +++ b/bookwyrm/tests/views/test_helpers.py @@ -80,113 +80,6 @@ class ViewsHelpers(TestCase): request.headers = {"Accept": "Praise"} self.assertFalse(views.helpers.is_api_request(request)) - def test_get_activity_feed(self): - """ loads statuses """ - rat = models.User.objects.create_user( - "rat", "rat@rat.rat", "password", local=True - ) - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - public_status = models.Comment.objects.create( - content="public status", book=self.book, user=self.local_user - ) - direct_status = models.Status.objects.create( - content="direct", user=self.local_user, privacy="direct" - ) - - rat_public = models.Status.objects.create(content="blah blah", user=rat) - rat_unlisted = models.Status.objects.create( - content="blah blah", user=rat, privacy="unlisted" - ) - remote_status = models.Status.objects.create( - content="blah blah", user=self.remote_user - ) - followers_status = models.Status.objects.create( - content="blah", user=rat, privacy="followers" - ) - rat_mention = models.Status.objects.create( - content="blah blah blah", user=rat, privacy="followers" - ) - rat_mention.mention_users.set([self.local_user]) - - statuses = views.helpers.get_activity_feed( - self.local_user, - privacy=["public", "unlisted", "followers"], - following_only=True, - queryset=models.Comment.objects, - ) - self.assertEqual(len(statuses), 1) - self.assertEqual(statuses[0], public_status) - - statuses = views.helpers.get_activity_feed( - self.local_user, privacy=["public", "followers"], local_only=True - ) - self.assertEqual(len(statuses), 2) - self.assertEqual(statuses[1], public_status) - self.assertEqual(statuses[0], rat_public) - - statuses = views.helpers.get_activity_feed(self.local_user, privacy=["direct"]) - self.assertEqual(len(statuses), 1) - self.assertEqual(statuses[0], direct_status) - - statuses = views.helpers.get_activity_feed( - self.local_user, - privacy=["public", "followers"], - ) - self.assertEqual(len(statuses), 3) - self.assertEqual(statuses[2], public_status) - self.assertEqual(statuses[1], rat_public) - self.assertEqual(statuses[0], remote_status) - - statuses = views.helpers.get_activity_feed( - self.local_user, - privacy=["public", "unlisted", "followers"], - following_only=True, - ) - self.assertEqual(len(statuses), 2) - self.assertEqual(statuses[1], public_status) - self.assertEqual(statuses[0], rat_mention) - - rat.followers.add(self.local_user) - statuses = views.helpers.get_activity_feed( - self.local_user, - privacy=["public", "unlisted", "followers"], - following_only=True, - ) - self.assertEqual(len(statuses), 5) - self.assertEqual(statuses[4], public_status) - self.assertEqual(statuses[3], rat_public) - self.assertEqual(statuses[2], rat_unlisted) - self.assertEqual(statuses[1], followers_status) - self.assertEqual(statuses[0], rat_mention) - - def test_get_activity_feed_blocks(self): - """ feed generation with blocked users """ - rat = models.User.objects.create_user( - "rat", "rat@rat.rat", "password", local=True - ) - - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - public_status = models.Comment.objects.create( - content="public status", book=self.book, user=self.local_user - ) - rat_public = models.Status.objects.create(content="blah blah", user=rat) - - statuses = views.helpers.get_activity_feed( - self.local_user, privacy=["public"] - ) - self.assertEqual(len(statuses), 2) - - # block relationship - rat.blocks.add(self.local_user) - statuses = views.helpers.get_activity_feed(self.local_user, privacy=["public"]) - self.assertEqual(len(statuses), 1) - self.assertEqual(statuses[0], public_status) - - statuses = views.helpers.get_activity_feed(rat, privacy=["public"]) - self.assertEqual(len(statuses), 1) - self.assertEqual(statuses[0], rat_public) - def test_is_bookwyrm_request(self): """ checks if a request came from a bookwyrm instance """ request = self.factory.get("", {"q": "Test Book"}) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index f2aa76d79..cabc3223c 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -19,8 +19,7 @@ from bookwyrm.activitypub import ActivitypubResponse from bookwyrm.connectors import connector_manager from bookwyrm.connectors.abstract_connector import get_image from bookwyrm.settings import PAGE_LENGTH -from .helpers import is_api_request, get_activity_feed, get_edition -from .helpers import privacy_filter +from .helpers import is_api_request, get_edition, privacy_filter # pylint: disable= no-self-use @@ -53,7 +52,7 @@ class Book(View): # all reviews for the book reviews = models.Review.objects.filter(book__in=work.editions.all()) - reviews = get_activity_feed(request.user, queryset=reviews) + reviews = privacy_filter(request.user, reviews) # the reviews to show paginated = Paginator( diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index b7ef9eb8c..f2f3c2bd5 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -11,7 +11,7 @@ from django.views import View from bookwyrm import activitystreams, forms, models from bookwyrm.activitypub import ActivitypubResponse from bookwyrm.settings import PAGE_LENGTH, STREAMS -from .helpers import get_activity_feed, get_user_from_username +from .helpers import get_user_from_username, privacy_filter from .helpers import is_api_request, is_bookwyrm_request, object_visible_to_user @@ -58,7 +58,13 @@ class DirectMessage(View): except ValueError: page = 1 - queryset = models.Status.objects + # remove fancy subclasses of status, keep just good ol' notes + queryset = models.Status.objects.filter( + review__isnull=True, + comment__isnull=True, + quotation__isnull=True, + generatednote__isnull=True, + ) user = None if username: @@ -69,8 +75,8 @@ class DirectMessage(View): if user: queryset = queryset.filter(Q(user=user) | Q(mention_users=user)) - activities = get_activity_feed( - request.user, privacy=["direct"], queryset=queryset + activities = privacy_filter( + request.user, queryset, privacy_levels=["direct"] ) paginated = Paginator(activities, PAGE_LENGTH) diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index c62856bab..41822da2c 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -59,6 +59,11 @@ def object_visible_to_user(viewer, obj): def privacy_filter(viewer, queryset, privacy_levels=None, following_only=False): """ filter objects that have "user" and "privacy" fields """ privacy_levels = privacy_levels or ["public", "unlisted", "followers", "direct"] + # if there'd a deleted field, exclude deleted items + try: + queryset = queryset.filter(deleted=False) + except FieldError: + pass # exclude blocks from both directions if not viewer.is_anonymous: @@ -102,54 +107,6 @@ def privacy_filter(viewer, queryset, privacy_levels=None, following_only=False): return queryset -def get_activity_feed( - user, privacy=None, local_only=False, following_only=False, queryset=None -): - """ get a filtered queryset of statuses """ - if queryset is None: - queryset = models.Status.objects.select_subclasses() - - # exclude deleted - queryset = queryset.exclude(deleted=True).order_by("-published_date") - - # apply privacy filters - queryset = privacy_filter(user, queryset, privacy, following_only=following_only) - - # only show dms if we only want dms - if privacy == ["direct"]: - # dms are direct statuses not related to books - queryset = queryset.filter( - review__isnull=True, - comment__isnull=True, - quotation__isnull=True, - generatednote__isnull=True, - ) - else: - try: - queryset = queryset.exclude( - review__isnull=True, - comment__isnull=True, - quotation__isnull=True, - generatednote__isnull=True, - privacy="direct", - ) - except FieldError: - # if we're looking at a subtype of Status (like Review) - pass - - # filter for only local status - if local_only: - queryset = queryset.filter(user__local=True) - - # remove statuses that have boosts in the same queryset - try: - queryset = queryset.filter(~Q(boosters__in=queryset)) - except ValueError: - pass - - return queryset - - def handle_remote_webfinger(query): """ webfingerin' other servers """ user = None diff --git a/bookwyrm/views/rss_feed.py b/bookwyrm/views/rss_feed.py index 57821af4e..ed3e84f47 100644 --- a/bookwyrm/views/rss_feed.py +++ b/bookwyrm/views/rss_feed.py @@ -1,7 +1,7 @@ """ serialize user's posts in rss feed """ from django.contrib.syndication.views import Feed -from .helpers import get_activity_feed, get_user_from_username +from .helpers import get_user_from_username, privacy_filter # pylint: disable=no-self-use, unused-argument class RssFeed(Feed): @@ -24,10 +24,10 @@ class RssFeed(Feed): def items(self, obj): """ the user's activity feed """ - return get_activity_feed( + return privacy_filter( obj, - privacy=["public", "unlisted"], - queryset=obj.status_set.select_subclasses(), + obj.status_set.select_subclasses(), + privacy_levels=["public", "unlisted"], ) def item_link(self, item): diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index 690bf158c..b8e3a86d2 100644 --- a/bookwyrm/views/user.py +++ b/bookwyrm/views/user.py @@ -16,8 +16,8 @@ from django.views import View from bookwyrm import forms, models from bookwyrm.activitypub import ActivitypubResponse from bookwyrm.settings import PAGE_LENGTH -from .helpers import get_activity_feed, get_user_from_username, is_api_request -from .helpers import is_blocked, object_visible_to_user +from .helpers import get_user_from_username, is_api_request +from .helpers import is_blocked, privacy_filter, object_visible_to_user # pylint: disable= no-self-use @@ -72,9 +72,9 @@ class User(View): break # user's posts - activities = get_activity_feed( + activities = privacy_filter( request.user, - queryset=user.status_set.select_subclasses(), + user.status_set.select_subclasses(), ) paginated = Paginator(activities, PAGE_LENGTH) goal = models.AnnualGoal.objects.filter( From 73185c6e4016c11ab9618546aa906f8f60385d46 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 19:19:21 -0700 Subject: [PATCH 07/33] Python formatting --- bookwyrm/activitystreams.py | 2 +- bookwyrm/views/feed.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 3ece05035..dc5323f66 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -67,7 +67,7 @@ class ActivityStream(ABC): def stream_users(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': + if status.privacy == "direct" and status.status_type == "Note": return None # everybody who could plausibly see this status diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index f2f3c2bd5..e0de932d2 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -75,9 +75,7 @@ class DirectMessage(View): if user: queryset = queryset.filter(Q(user=user) | Q(mention_users=user)) - activities = privacy_filter( - request.user, queryset, privacy_levels=["direct"] - ) + activities = privacy_filter(request.user, queryset, privacy_levels=["direct"]) paginated = Paginator(activities, PAGE_LENGTH) activity_page = paginated.page(page) From 26fa81f19be4fe31d1ba4c1656989f685455e662 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 19:53:26 -0700 Subject: [PATCH 08/33] Use redis lists instead of ordered sets Django unsorts it so there's no point --- bookwyrm/activitystreams.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index dc5323f66..aecad727f 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -26,28 +26,24 @@ class ActivityStream(ABC): 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.lpush(self.stream_id(user), status.id) + pipeline.ltrim(self.stream_id(user), 0, settings.MAX_STREAM_LENGTH) # add to the unread status count pipeline.incr(self.unread_id(user)) # and go! pipeline.execute() - 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_activity_stream(self, user): """ load the ids for 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 = r.lrange(self.stream_id(user), 0, -1) return ( models.Status.objects.select_subclasses() .filter(id__in=statuses) @@ -61,7 +57,7 @@ class ActivityStream(ABC): stream_id = self.stream_id(user) for status in statuses.all()[: settings.MAX_STREAM_LENGTH]: - pipeline.zadd(stream_id, self.get_value(status)) + pipeline.lpush(stream_id, status.id) pipeline.execute() def stream_users(self, status): # pylint: disable=no-self-use From 39f34bc6e6f273f48376b8a049c66486631bfa11 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 20:32:59 -0700 Subject: [PATCH 09/33] Adds activity stream utility for adding and removing statuses --- bookwyrm/activitystreams.py | 21 +++++++++++++++++++++ bookwyrm/models/relationship.py | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index aecad727f..7a959df94 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -38,6 +38,27 @@ class ActivityStream(ABC): # 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.lrem(self.stream_id(user), -1, status.id) + pipeline.execute() + + def add_user_statuses(self, user, viewer): + """ add a user's statuses to another user's feed """ + pipeline = r.pipeline() + for status in user.status_set.objects.all()[: settings.MAX_STREAM_LENGTH]: + pipeline.lpush(self.stream_id(viewer), status.id) + pipeline.execute() + + def remove_user_statuses(self, user, viewer): + """ remove a user's status from another user's feed """ + pipeline = r.pipeline() + for status in user.status_set.objects.all()[: settings.MAX_STREAM_LENGTH]: + pipeline.lrem(self.stream_id(viewer), status.id) + pipeline.execute() + def get_activity_stream(self, user): """ load the ids for statuses to be displayed """ # clear unreads for this feed diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index df99d2165..998d7bed5 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -62,7 +62,7 @@ class UserFollows(ActivityMixin, UserRelationship): status = "follows" - def to_activity(self): + def to_activity(self): # pylint: disable=arguments-differ """ overrides default to manually set serializer """ return activitypub.Follow(**generate_activity(self)) From 371e908e8a174a59c148e50346b18eea0a402210 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Mar 2021 21:11:23 -0700 Subject: [PATCH 10/33] Adds handlers for user relationship state changes --- bookwyrm/activitystreams.py | 53 ++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 7a959df94..dfffd4905 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -1,8 +1,7 @@ """ access the activity streams stored in redis """ from abc import ABC from django.dispatch import receiver -from django.db.models import signals -from django.db.models import Q +from django.db.models import signals, Q import redis from bookwyrm import models, settings @@ -45,18 +44,18 @@ class ActivityStream(ABC): pipeline.lrem(self.stream_id(user), -1, status.id) pipeline.execute() - def add_user_statuses(self, user, viewer): + def add_user_statuses(self, viewer, user): """ add a user's statuses to another user's feed """ pipeline = r.pipeline() - for status in user.status_set.objects.all()[: settings.MAX_STREAM_LENGTH]: + for status in user.status_set.all()[: settings.MAX_STREAM_LENGTH]: pipeline.lpush(self.stream_id(viewer), status.id) pipeline.execute() - def remove_user_statuses(self, user, 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.objects.all()[: settings.MAX_STREAM_LENGTH]: - pipeline.lrem(self.stream_id(viewer), status.id) + for status in user.status_set.all()[: settings.MAX_STREAM_LENGTH]: + pipeline.lrem(self.stream_id(viewer), -1, status.id) pipeline.execute() def get_activity_stream(self, user): @@ -182,15 +181,39 @@ streams = { @receiver(signals.post_save) # pylint: disable=unused-argument -def update_feeds(sender, instance, created, *args, **kwargs): - """ add statuses to activity feeds """ - # we're only interested in new statuses that aren't dms - if ( - not created - or not issubclass(sender, models.Status) - or instance.privacy == "direct" - ): +def add_status_on_create(sender, instance, created, *args, **kwargs): + """ add newly created statuses to activity feeds """ + # we're only interested in new statuses + if not created or not issubclass(sender, models.Status): return + # iterates through Home, Local, Federated for stream in streams.values(): stream.add_status(instance) + + +@receiver(signals.post_save, sender=models.UserFollows) +# pylint: disable=unused-argument +def add_statuses_on_follow(sender, instance, created, *args, **kwargs): + """ add a newly followed user's statuses to feeds """ + if not created: + return # idk when this would ever happen though + HomeStream().add_user_statuses(instance.user_subject, instance.user_object) + + +@receiver(signals.post_delete, sender=models.UserFollows) +# pylint: disable=unused-argument +def remove_statuses_on_unfollow(sender, instance, *args, **kwargs): + """ remove statuses from a feed on unfollow """ + HomeStream().remove_user_statuses(instance.user_subject, instance.user_object) + + +@receiver(signals.post_save, sender=models.UserBlocks) +# pylint: disable=unused-argument +def remove_statuses_on_block(sender, instance, *args, **kwargs): + """ remove statuses from all feeds on block """ + # blocks apply ot all feeds + for stream in streams.values(): + # and in both directions + stream.remove_user_statuses(instance.user_subject, instance.user_object) + stream.remove_user_statuses(instance.user_object, instance.user_subject) From 04d20852765f238204e3a5434872b9cd24a4377c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 07:01:49 -0700 Subject: [PATCH 11/33] Create feeds on user registration --- bookwyrm/activitystreams.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index dfffd4905..6182f510b 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -217,3 +217,14 @@ def remove_statuses_on_block(sender, instance, *args, **kwargs): # and in both directions stream.remove_user_statuses(instance.user_subject, instance.user_object) stream.remove_user_statuses(instance.user_object, instance.user_subject) + + +@receiver(signals.post_save, sender=models.User) +# pylint: disable=unused-argument +def populate_feed_on_account_create(sender, instance, created, *args, **kwargs): + """ build a user's feeds when they join """ + if not created or not instance.local: + return + + for stream in streams.values(): + stream.populate_stream(instance) From e17ee408002f19a2dd8594a44486aafb838427cc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 07:28:44 -0700 Subject: [PATCH 12/33] Generalizes broadcast mock in status view tests --- bookwyrm/tests/views/test_status.py | 76 +++++++++++++---------------- 1 file changed, 35 insertions(+), 41 deletions(-) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index 32080a5a8..665fd6277 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -8,6 +8,7 @@ from bookwyrm import forms, models, views from bookwyrm.settings import DOMAIN +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") class StatusViews(TestCase): """ viewing and creating statuses """ @@ -40,7 +41,7 @@ class StatusViews(TestCase): parent_work=work, ) - def test_handle_status(self): + def test_handle_status(self, _): """ create a status """ view = views.CreateStatus.as_view() form = forms.CommentForm( @@ -53,23 +54,22 @@ class StatusViews(TestCase): ) request = self.factory.post("", form.data) request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, "comment") + + view(request, "comment") status = models.Comment.objects.get() self.assertEqual(status.content, "

    hi

    ") self.assertEqual(status.user, self.local_user) self.assertEqual(status.book, self.book) - def test_handle_status_reply(self): + def test_handle_status_reply(self, _): """ create a status in reply to an existing status """ view = views.CreateStatus.as_view() user = models.User.objects.create_user( "rat", "rat@rat.com", "password", local=True ) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - parent = models.Status.objects.create( - content="parent status", user=self.local_user - ) + parent = models.Status.objects.create( + content="parent status", user=self.local_user + ) form = forms.ReplyForm( { "content": "hi", @@ -80,14 +80,14 @@ class StatusViews(TestCase): ) request = self.factory.post("", form.data) request.user = user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, "reply") + + view(request, "reply") status = models.Status.objects.get(user=user) self.assertEqual(status.content, "

    hi

    ") self.assertEqual(status.user, user) self.assertEqual(models.Notification.objects.get().user, self.local_user) - def test_handle_status_mentions(self): + def test_handle_status_mentions(self, _): """ @mention a user in a post """ view = views.CreateStatus.as_view() user = models.User.objects.create_user( @@ -104,8 +104,7 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, "comment") + view(request, "comment") status = models.Status.objects.get() self.assertEqual(list(status.mention_users.all()), [user]) self.assertEqual(models.Notification.objects.get().user, user) @@ -113,7 +112,7 @@ class StatusViews(TestCase): status.content, '

    hi @rat

    ' % user.remote_id ) - def test_handle_status_reply_with_mentions(self): + def test_handle_status_reply_with_mentions(self, _): """ reply to a post with an @mention'ed user """ view = views.CreateStatus.as_view() user = models.User.objects.create_user( @@ -130,8 +129,7 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, "comment") + view(request, "comment") status = models.Status.objects.get() form = forms.ReplyForm( @@ -144,8 +142,8 @@ class StatusViews(TestCase): ) request = self.factory.post("", form.data) request.user = user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - view(request, "reply") + + view(request, "reply") reply = models.Status.replies(status).first() self.assertEqual(reply.content, "

    right

    ") @@ -154,7 +152,7 @@ class StatusViews(TestCase): self.assertFalse(self.remote_user in reply.mention_users.all()) self.assertTrue(self.local_user in reply.mention_users.all()) - def test_find_mentions(self): + def test_find_mentions(self, _): """ detect and look up @ mentions of users """ user = models.User.objects.create_user( "nutria@%s" % DOMAIN, @@ -200,7 +198,7 @@ class StatusViews(TestCase): ("@nutria@%s" % DOMAIN, user), ) - def test_format_links(self): + def test_format_links(self, _): """ find and format urls into a tags """ url = "http://www.fish.com/" self.assertEqual( @@ -223,7 +221,7 @@ class StatusViews(TestCase): "?q=arkady+strugatsky&mode=everything" % url, ) - def test_to_markdown(self): + def test_to_markdown(self, _): """ this is mostly handled in other places, but nonetheless """ text = "_hi_ and http://fish.com is rad" result = views.status.to_markdown(text) @@ -232,33 +230,31 @@ class StatusViews(TestCase): '

    hi and fish.com ' "is rad

    ", ) - def test_to_markdown_link(self): + def test_to_markdown_link(self, _): """ this is mostly handled in other places, but nonetheless """ text = "[hi](http://fish.com) is rad" result = views.status.to_markdown(text) self.assertEqual(result, '

    hi ' "is rad

    ") - def test_handle_delete_status(self): + def test_handle_delete_status(self, mock): """ marks a status as deleted """ view = views.DeleteStatus.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - status = models.Status.objects.create(user=self.local_user, content="hi") + status = models.Status.objects.create(user=self.local_user, content="hi") self.assertFalse(status.deleted) request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - view(request, status.id) - activity = json.loads(mock.call_args_list[0][0][1]) - self.assertEqual(activity["type"], "Delete") - self.assertEqual(activity["object"]["type"], "Tombstone") + + view(request, status.id) + activity = json.loads(mock.call_args_list[0][0][1]) + self.assertEqual(activity["type"], "Delete") + self.assertEqual(activity["object"]["type"], "Tombstone") status.refresh_from_db() self.assertTrue(status.deleted) - def test_handle_delete_status_permission_denied(self): + def test_handle_delete_status_permission_denied(self, _): """ marks a status as deleted """ view = views.DeleteStatus.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - status = models.Status.objects.create(user=self.local_user, content="hi") + status = models.Status.objects.create(user=self.local_user, content="hi") self.assertFalse(status.deleted) request = self.factory.post("") request.user = self.remote_user @@ -268,20 +264,18 @@ class StatusViews(TestCase): status.refresh_from_db() self.assertFalse(status.deleted) - def test_handle_delete_status_moderator(self): + def test_handle_delete_status_moderator(self, mock): """ marks a status as deleted """ view = views.DeleteStatus.as_view() - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - status = models.Status.objects.create(user=self.local_user, content="hi") + status = models.Status.objects.create(user=self.local_user, content="hi") self.assertFalse(status.deleted) request = self.factory.post("") request.user = self.remote_user request.user.is_superuser = True - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: - view(request, status.id) - activity = json.loads(mock.call_args_list[0][0][1]) - self.assertEqual(activity["type"], "Delete") - self.assertEqual(activity["object"]["type"], "Tombstone") + view(request, status.id) + activity = json.loads(mock.call_args_list[0][0][1]) + self.assertEqual(activity["type"], "Delete") + self.assertEqual(activity["object"]["type"], "Tombstone") status.refresh_from_db() self.assertTrue(status.deleted) From ea71c2aa88f50906f57b9704c8ef84289f2ca569 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 08:13:57 -0700 Subject: [PATCH 13/33] Fixes status views tests --- bookwyrm/activitystreams.py | 7 +++- bookwyrm/tests/views/test_status.py | 56 +++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 6182f510b..53eb756ad 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -184,7 +184,12 @@ streams = { def add_status_on_create(sender, instance, created, *args, **kwargs): """ add newly created statuses to activity feeds """ # we're only interested in new statuses - if not created or not issubclass(sender, models.Status): + if not issubclass(sender, models.Status): + return + + if not created and instance.deleted: + for stream in streams.values(): + stream.remove_status(instance) return # iterates through Home, Local, Federated diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index 665fd6277..2a6128504 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -55,7 +55,10 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = self.local_user - view(request, "comment") + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + view(request, "comment") + self.assertTrue(redis_mock.called) + status = models.Comment.objects.get() self.assertEqual(status.content, "

    hi

    ") self.assertEqual(status.user, self.local_user) @@ -67,9 +70,10 @@ class StatusViews(TestCase): user = models.User.objects.create_user( "rat", "rat@rat.com", "password", local=True ) - parent = models.Status.objects.create( - content="parent status", user=self.local_user - ) + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + parent = models.Status.objects.create( + content="parent status", user=self.local_user + ) form = forms.ReplyForm( { "content": "hi", @@ -81,7 +85,10 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = user - view(request, "reply") + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + view(request, "reply") + self.assertTrue(redis_mock.called) + status = models.Status.objects.get(user=user) self.assertEqual(status.content, "

    hi

    ") self.assertEqual(status.user, user) @@ -104,7 +111,10 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = self.local_user - view(request, "comment") + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + view(request, "comment") + self.assertTrue(redis_mock.called) + status = models.Status.objects.get() self.assertEqual(list(status.mention_users.all()), [user]) self.assertEqual(models.Notification.objects.get().user, user) @@ -129,7 +139,9 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = self.local_user - view(request, "comment") + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + view(request, "comment") + self.assertTrue(redis_mock.called) status = models.Status.objects.get() form = forms.ReplyForm( @@ -143,7 +155,10 @@ class StatusViews(TestCase): request = self.factory.post("", form.data) request.user = user - view(request, "reply") + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + view(request, "reply") + self.assertTrue(redis_mock.called) + reply = models.Status.replies(status).first() self.assertEqual(reply.content, "

    right

    ") @@ -239,13 +254,18 @@ class StatusViews(TestCase): def test_handle_delete_status(self, mock): """ marks a status as deleted """ view = views.DeleteStatus.as_view() - status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + status = models.Status.objects.create(user=self.local_user, content="hi") self.assertFalse(status.deleted) request = self.factory.post("") request.user = self.local_user - view(request, status.id) - activity = json.loads(mock.call_args_list[0][0][1]) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_status" + ) as redis_mock: + view(request, status.id) + self.assertTrue(redis_mock.called) + activity = json.loads(mock.call_args_list[1][0][1]) self.assertEqual(activity["type"], "Delete") self.assertEqual(activity["object"]["type"], "Tombstone") status.refresh_from_db() @@ -254,7 +274,8 @@ class StatusViews(TestCase): def test_handle_delete_status_permission_denied(self, _): """ marks a status as deleted """ view = views.DeleteStatus.as_view() - status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + status = models.Status.objects.create(user=self.local_user, content="hi") self.assertFalse(status.deleted) request = self.factory.post("") request.user = self.remote_user @@ -267,14 +288,19 @@ class StatusViews(TestCase): def test_handle_delete_status_moderator(self, mock): """ marks a status as deleted """ view = views.DeleteStatus.as_view() - status = models.Status.objects.create(user=self.local_user, content="hi") + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + status = models.Status.objects.create(user=self.local_user, content="hi") self.assertFalse(status.deleted) request = self.factory.post("") request.user = self.remote_user request.user.is_superuser = True - view(request, status.id) - activity = json.loads(mock.call_args_list[0][0][1]) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_status" + ) as redis_mock: + view(request, status.id) + self.assertTrue(redis_mock.called) + activity = json.loads(mock.call_args_list[1][0][1]) self.assertEqual(activity["type"], "Delete") self.assertEqual(activity["object"]["type"], "Tombstone") status.refresh_from_db() From 73661b9472134c83a884770f92756df7511ce51f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 08:19:25 -0700 Subject: [PATCH 14/33] Mocks redis in outbox tests --- bookwyrm/tests/views/test_outbox.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/bookwyrm/tests/views/test_outbox.py b/bookwyrm/tests/views/test_outbox.py index 5934eb7c7..0bcfde693 100644 --- a/bookwyrm/tests/views/test_outbox.py +++ b/bookwyrm/tests/views/test_outbox.py @@ -11,6 +11,7 @@ from bookwyrm.settings import USER_AGENT # pylint: disable=too-many-public-methods +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") class OutboxView(TestCase): """ sends out activities """ @@ -32,19 +33,19 @@ class OutboxView(TestCase): parent_work=work, ) - def test_outbox(self): + def test_outbox(self, _): """ returns user's statuses """ request = self.factory.get("") result = views.Outbox.as_view()(request, "mouse") self.assertIsInstance(result, JsonResponse) - def test_outbox_bad_method(self): + def test_outbox_bad_method(self, _): """ can't POST to outbox """ request = self.factory.post("") result = views.Outbox.as_view()(request, "mouse") self.assertEqual(result.status_code, 405) - def test_outbox_unknown_user(self): + def test_outbox_unknown_user(self, _): """ should 404 for unknown and remote users """ request = self.factory.post("") result = views.Outbox.as_view()(request, "beepboop") @@ -52,9 +53,9 @@ class OutboxView(TestCase): result = views.Outbox.as_view()(request, "rat") self.assertEqual(result.status_code, 405) - def test_outbox_privacy(self): + def test_outbox_privacy(self, _): """ don't show dms et cetera in outbox """ - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): models.Status.objects.create( content="PRIVATE!!", user=self.local_user, privacy="direct" ) @@ -75,9 +76,9 @@ class OutboxView(TestCase): self.assertEqual(data["type"], "OrderedCollection") self.assertEqual(data["totalItems"], 2) - def test_outbox_filter(self): + def test_outbox_filter(self, _): """ if we only care about reviews, only get reviews """ - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): models.Review.objects.create( content="look at this", name="hi", @@ -101,9 +102,9 @@ class OutboxView(TestCase): self.assertEqual(data["type"], "OrderedCollection") self.assertEqual(data["totalItems"], 1) - def test_outbox_bookwyrm_request_true(self): + def test_outbox_bookwyrm_request_true(self, _): """ should differentiate between bookwyrm and outside requests """ - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): models.Review.objects.create( name="hi", content="look at this", @@ -119,9 +120,9 @@ class OutboxView(TestCase): self.assertEqual(len(data["orderedItems"]), 1) self.assertEqual(data["orderedItems"][0]["type"], "Review") - def test_outbox_bookwyrm_request_false(self): + def test_outbox_bookwyrm_request_false(self, _): """ should differentiate between bookwyrm and outside requests """ - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): models.Review.objects.create( name="hi", content="look at this", From 1cf4c0d5cc632918cbb479b272ec11700a09facc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 08:27:24 -0700 Subject: [PATCH 15/33] Mocks redis in interaction tests --- bookwyrm/tests/views/test_interaction.py | 35 ++++++++++++------------ bookwyrm/tests/views/test_isbn.py | 2 -- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index 857f7061f..b52e9a667 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -5,7 +5,7 @@ from django.test.client import RequestFactory from bookwyrm import models, views - +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") class InteractionViews(TestCase): """ viewing and creating statuses """ @@ -38,12 +38,12 @@ class InteractionViews(TestCase): parent_work=work, ) - def test_handle_favorite(self): + def test_handle_favorite(self, _): """ create and broadcast faving a status """ view = views.Favorite.as_view() request = self.factory.post("") request.user = self.remote_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") view(request, status.id) @@ -56,12 +56,12 @@ class InteractionViews(TestCase): self.assertEqual(notification.user, self.local_user) self.assertEqual(notification.related_user, self.remote_user) - def test_handle_unfavorite(self): + def test_handle_unfavorite(self, _): """ unfav a status """ view = views.Unfavorite.as_view() request = self.factory.post("") request.user = self.remote_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") views.Favorite.as_view()(request, status.id) @@ -73,12 +73,12 @@ class InteractionViews(TestCase): self.assertEqual(models.Favorite.objects.count(), 0) self.assertEqual(models.Notification.objects.count(), 0) - def test_handle_boost(self): + def test_handle_boost(self, _): """ boost a status """ view = views.Boost.as_view() request = self.factory.post("") request.user = self.remote_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") view(request, status.id) @@ -94,12 +94,12 @@ class InteractionViews(TestCase): self.assertEqual(notification.related_user, self.remote_user) self.assertEqual(notification.related_status, status) - def test_handle_boost_unlisted(self): + def test_handle_boost_unlisted(self, _): """ boost a status """ view = views.Boost.as_view() request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create( user=self.local_user, content="hi", privacy="unlisted" ) @@ -109,12 +109,12 @@ class InteractionViews(TestCase): boost = models.Boost.objects.get() self.assertEqual(boost.privacy, "unlisted") - def test_handle_boost_private(self): + def test_handle_boost_private(self, _): """ boost a status """ view = views.Boost.as_view() request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create( user=self.local_user, content="hi", privacy="followers" ) @@ -122,31 +122,32 @@ class InteractionViews(TestCase): view(request, status.id) self.assertFalse(models.Boost.objects.exists()) - def test_handle_boost_twice(self): + def test_handle_boost_twice(self, _): """ boost a status """ view = views.Boost.as_view() request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") view(request, status.id) view(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) - def test_handle_unboost(self): + def test_handle_unboost(self, broadcast_mock): """ undo a boost """ view = views.Unboost.as_view() request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): status = models.Status.objects.create(user=self.local_user, content="hi") views.Boost.as_view()(request, status.id) self.assertEqual(models.Boost.objects.count(), 1) self.assertEqual(models.Notification.objects.count(), 1) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock: + broadcast_mock.call_count = 0 + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): view(request, status.id) - self.assertEqual(mock.call_count, 1) + self.assertEqual(broadcast_mock.call_count, 1) self.assertEqual(models.Boost.objects.count(), 0) self.assertEqual(models.Notification.objects.count(), 0) diff --git a/bookwyrm/tests/views/test_isbn.py b/bookwyrm/tests/views/test_isbn.py index c7ae1f39f..7f03a6109 100644 --- a/bookwyrm/tests/views/test_isbn.py +++ b/bookwyrm/tests/views/test_isbn.py @@ -3,12 +3,10 @@ import json from unittest.mock import patch from django.http import JsonResponse -from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory from bookwyrm import models, views -from bookwyrm.connectors import abstract_connector from bookwyrm.settings import DOMAIN From f290e60b852d15923cc02632a60fef32db140518 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 08:53:28 -0700 Subject: [PATCH 16/33] Improves signal handling and updates inbox tests --- bookwyrm/activitystreams.py | 48 +++++++++++++++++---- bookwyrm/tests/views/test_inbox.py | 67 ++++++++++++++++++++++-------- 2 files changed, 90 insertions(+), 25 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 53eb756ad..c1c548d29 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -187,22 +187,34 @@ def add_status_on_create(sender, instance, created, *args, **kwargs): if not issubclass(sender, models.Status): return - if not created and instance.deleted: + if instance.deleted: for stream in streams.values(): stream.remove_status(instance) return + if not created: + return + # iterates through Home, Local, Federated for stream in streams.values(): stream.add_status(instance) +@receiver(signals.post_delete, sender=models.Boost) +# pylint: disable=unused-argument +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) + + @receiver(signals.post_save, sender=models.UserFollows) # pylint: disable=unused-argument def add_statuses_on_follow(sender, instance, created, *args, **kwargs): """ add a newly followed user's statuses to feeds """ - if not created: - return # idk when this would ever happen though + if not created or not instance.user_subject.local: + return HomeStream().add_user_statuses(instance.user_subject, instance.user_object) @@ -210,6 +222,8 @@ def add_statuses_on_follow(sender, instance, created, *args, **kwargs): # pylint: disable=unused-argument def remove_statuses_on_unfollow(sender, instance, *args, **kwargs): """ remove statuses from a feed on unfollow """ + if not instance.user_subject.local: + return HomeStream().remove_user_statuses(instance.user_subject, instance.user_object) @@ -218,10 +232,30 @@ def remove_statuses_on_unfollow(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 - for stream in streams.values(): - # and in both directions - stream.remove_user_statuses(instance.user_subject, instance.user_object) - stream.remove_user_statuses(instance.user_object, instance.user_subject) + if instance.user_subject.local: + for stream in streams.values(): + stream.remove_user_statuses(instance.user_subject, instance.user_object) + + # and in both directions + if instance.user_object.local: + for stream in streams.values(): + stream.remove_user_statuses(instance.user_object, instance.user_subject) + + +@receiver(signals.post_delete, sender=models.UserBlocks) +# pylint: disable=unused-argument +def add_statuses_on_unblock(sender, instance, *args, **kwargs): + """ remove statuses from all feeds on block """ + public_streams = [LocalStream(), FederatedStream()] + # add statuses back to streams with statuses from anyone + if instance.user_subject.local: + for stream in public_streams: + stream.add_user_statuses(instance.user_subject, instance.user_object) + + # add statuses back to streams with statuses from anyone + if instance.user_object.local: + for stream in public_streams: + stream.add_user_statuses(instance.user_object, instance.user_subject) @receiver(signals.post_save, sender=models.User) diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 10f55f89b..7d08a1e49 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -38,11 +38,12 @@ class Inbox(TestCase): outbox="https://example.com/users/rat/outbox", ) with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - self.status = models.Status.objects.create( - user=self.local_user, - content="Test status", - remote_id="https://example.com/status/1", - ) + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + self.status = models.Status.objects.create( + user=self.local_user, + content="Test status", + remote_id="https://example.com/status/1", + ) self.create_json = { "id": "hi", @@ -139,7 +140,9 @@ class Inbox(TestCase): activity = self.create_json activity["object"] = status_data - views.inbox.activity_task(activity) + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) status = models.Quotation.objects.get() self.assertEqual( @@ -166,7 +169,9 @@ class Inbox(TestCase): activity = self.create_json activity["object"] = status_data - views.inbox.activity_task(activity) + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) status = models.Status.objects.last() self.assertEqual(status.content, "test content in note") self.assertEqual(status.mention_users.first(), self.local_user) @@ -187,7 +192,9 @@ class Inbox(TestCase): activity = self.create_json activity["object"] = status_data - views.inbox.activity_task(activity) + with patch("bookwyrm.activitystreams.ActivityStream.add_status") as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) status = models.Status.objects.last() self.assertEqual(status.content, "test content in note") self.assertEqual(status.reply_parent, self.status) @@ -218,7 +225,7 @@ class Inbox(TestCase): self.assertEqual(book_list.description, "summary text") self.assertEqual(book_list.remote_id, "https://example.com/list/22") - def test_handle_follow_x(self): + def test_handle_follow(self): """ remote user wants to follow local user """ activity = { "@context": "https://www.w3.org/ns/activitystreams", @@ -436,7 +443,11 @@ class Inbox(TestCase): "actor": self.remote_user.remote_id, "object": {"id": self.status.remote_id, "type": "Tombstone"}, } - views.inbox.activity_task(activity) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_status" + ) as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) # deletion doens't remove the status, it turns it into a tombstone status = models.Status.objects.get() self.assertTrue(status.deleted) @@ -465,7 +476,11 @@ class Inbox(TestCase): "actor": self.remote_user.remote_id, "object": {"id": self.status.remote_id, "type": "Tombstone"}, } - views.inbox.activity_task(activity) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_status" + ) as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) # deletion doens't remove the status, it turns it into a tombstone status = models.Status.objects.get() self.assertTrue(status.deleted) @@ -535,7 +550,8 @@ class Inbox(TestCase): views.inbox.activity_task(activity) self.assertEqual(models.Favorite.objects.count(), 0) - def test_handle_boost(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_boost(self, _): """ boost a status """ self.assertEqual(models.Notification.objects.count(), 0) activity = { @@ -560,7 +576,8 @@ class Inbox(TestCase): content="hi", user=self.remote_user, ) - status.save(broadcast=False) + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + status.save(broadcast=False) activity = { "type": "Announce", "id": "http://www.faraway.com/boost/12", @@ -575,9 +592,10 @@ class Inbox(TestCase): def test_handle_unboost(self): """ undo a boost """ - boost = models.Boost.objects.create( - boosted_status=self.status, user=self.remote_user - ) + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + boost = models.Boost.objects.create( + boosted_status=self.status, user=self.remote_user + ) activity = { "type": "Undo", "actor": "hi", @@ -591,7 +609,11 @@ class Inbox(TestCase): "object": self.status.remote_id, }, } - views.inbox.activity_task(activity) + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_status" + ) as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) def test_handle_unboost_unknown_boost(self): """ undo a boost """ @@ -863,6 +885,11 @@ class Inbox(TestCase): "object": "https://example.com/user/mouse", } + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_user_statuses" + ) as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) views.inbox.activity_task(activity) block = models.UserBlocks.objects.get() self.assertEqual(block.user_subject, self.remote_user) @@ -896,5 +923,9 @@ class Inbox(TestCase): "object": "https://example.com/user/mouse", }, } - views.inbox.activity_task(activity) + with patch( + "bookwyrm.activitystreams.ActivityStream.add_user_statuses" + ) as redis_mock: + views.inbox.activity_task(activity) + self.assertTrue(redis_mock.called) self.assertFalse(models.UserBlocks.objects.exists()) From b9ec6a1e2b42d39017f1885d897d7ef4df987e71 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 09:00:04 -0700 Subject: [PATCH 17/33] Mocks redis for helpers tests --- bookwyrm/tests/views/test_helpers.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/bookwyrm/tests/views/test_helpers.py b/bookwyrm/tests/views/test_helpers.py index 926f96222..4fd7ce66d 100644 --- a/bookwyrm/tests/views/test_helpers.py +++ b/bookwyrm/tests/views/test_helpers.py @@ -134,7 +134,8 @@ class ViewsHelpers(TestCase): self.assertIsInstance(result, models.User) self.assertEqual(result.username, "mouse@example.com") - def test_handle_reading_status_to_read(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_reading_status_to_read(self, _): """ posts shelve activities """ shelf = self.local_user.shelf_set.get(identifier="to-read") with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -146,7 +147,8 @@ class ViewsHelpers(TestCase): self.assertEqual(status.mention_books.first(), self.book) self.assertEqual(status.content, "wants to read") - def test_handle_reading_status_reading(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_reading_status_reading(self, _): """ posts shelve activities """ shelf = self.local_user.shelf_set.get(identifier="reading") with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -158,7 +160,8 @@ class ViewsHelpers(TestCase): self.assertEqual(status.mention_books.first(), self.book) self.assertEqual(status.content, "started reading") - def test_handle_reading_status_read(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_reading_status_read(self, _): """ posts shelve activities """ shelf = self.local_user.shelf_set.get(identifier="read") with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -170,7 +173,8 @@ class ViewsHelpers(TestCase): self.assertEqual(status.mention_books.first(), self.book) self.assertEqual(status.content, "finished reading") - def test_handle_reading_status_other(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_reading_status_other(self, _): """ posts shelve activities """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): views.helpers.handle_reading_status( @@ -178,7 +182,8 @@ class ViewsHelpers(TestCase): ) self.assertFalse(models.GeneratedNote.objects.exists()) - def test_object_visible_to_user(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_object_visible_to_user(self, _): """ does a user have permission to view an object """ obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="public" @@ -206,7 +211,8 @@ class ViewsHelpers(TestCase): obj.mention_users.add(self.local_user) self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj)) - def test_object_visible_to_user_follower(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_object_visible_to_user_follower(self, _): """ what you can see if you follow a user """ self.remote_user.followers.add(self.local_user) obj = models.Status.objects.create( @@ -225,7 +231,8 @@ class ViewsHelpers(TestCase): obj.mention_users.add(self.local_user) self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj)) - def test_object_visible_to_user_blocked(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_object_visible_to_user_blocked(self, _): """ you can't see it if they block you """ self.remote_user.blocks.add(self.local_user) obj = models.Status.objects.create( From 2d350474bf4c036f591415be7fcd303f43699aa7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 09:09:30 -0700 Subject: [PATCH 18/33] Mocks redis in feed view tests --- bookwyrm/tests/views/test_feed.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/bookwyrm/tests/views/test_feed.py b/bookwyrm/tests/views/test_feed.py index 426684676..1ff995732 100644 --- a/bookwyrm/tests/views/test_feed.py +++ b/bookwyrm/tests/views/test_feed.py @@ -14,6 +14,8 @@ from bookwyrm import views from bookwyrm.activitypub import ActivitypubResponse +@patch("bookwyrm.activitystreams.ActivityStream.get_activity_stream") +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class FeedViews(TestCase): """ activity feed, statuses, dms """ @@ -34,7 +36,7 @@ class FeedViews(TestCase): ) models.SiteSettings.objects.create() - def test_feed(self): + def test_feed(self, *_): """ there are so many views, this just makes sure it LOADS """ view = views.Feed.as_view() request = self.factory.get("") @@ -44,7 +46,7 @@ class FeedViews(TestCase): result.render() self.assertEqual(result.status_code, 200) - def test_status_page(self): + def test_status_page(self, *_): """ there are so many views, this just makes sure it LOADS """ view = views.Status.as_view() with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -64,7 +66,7 @@ class FeedViews(TestCase): self.assertIsInstance(result, ActivitypubResponse) self.assertEqual(result.status_code, 200) - def test_status_page_with_image(self): + def test_status_page_with_image(self, *_): """ there are so many views, this just makes sure it LOADS """ view = views.Status.as_view() @@ -100,7 +102,7 @@ class FeedViews(TestCase): self.assertIsInstance(result, ActivitypubResponse) self.assertEqual(result.status_code, 200) - def test_replies_page(self): + def test_replies_page(self, *_): """ there are so many views, this just makes sure it LOADS """ view = views.Replies.as_view() with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -120,7 +122,7 @@ class FeedViews(TestCase): self.assertIsInstance(result, ActivitypubResponse) self.assertEqual(result.status_code, 200) - def test_direct_messages_page(self): + def test_direct_messages_page(self, *_): """ there are so many views, this just makes sure it LOADS """ view = views.DirectMessage.as_view() request = self.factory.get("") @@ -130,7 +132,7 @@ class FeedViews(TestCase): result.render() self.assertEqual(result.status_code, 200) - def test_get_suggested_book(self): + def test_get_suggested_book(self, *_): """ gets books the ~*~ algorithm ~*~ thinks you want to post about """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): models.ShelfBook.objects.create( From de2cea5ff20a0c9ea4bf86357eb7b8d47ded1860 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 09:14:31 -0700 Subject: [PATCH 19/33] Adds mocks for block tests --- bookwyrm/tests/views/test_block.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bookwyrm/tests/views/test_block.py b/bookwyrm/tests/views/test_block.py index 60920e38b..71583d708 100644 --- a/bookwyrm/tests/views/test_block.py +++ b/bookwyrm/tests/views/test_block.py @@ -7,6 +7,7 @@ from django.test.client import RequestFactory from bookwyrm import models, views +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") class BlockViews(TestCase): """ view user and edit profile """ @@ -32,7 +33,7 @@ class BlockViews(TestCase): ) models.SiteSettings.objects.create() - def test_block_get(self): + def test_block_get(self, _): """ there are so many views, this just makes sure it LOADS """ view = views.Block.as_view() request = self.factory.get("") @@ -42,20 +43,19 @@ class BlockViews(TestCase): result.render() self.assertEqual(result.status_code, 200) - def test_block_post(self): + def test_block_post(self, _): """ create a "block" database entry from an activity """ view = views.Block.as_view() self.local_user.followers.add(self.remote_user) - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - models.UserFollowRequest.objects.create( - user_subject=self.local_user, user_object=self.remote_user - ) + models.UserFollowRequest.objects.create( + user_subject=self.local_user, user_object=self.remote_user + ) self.assertTrue(models.UserFollows.objects.exists()) self.assertTrue(models.UserFollowRequest.objects.exists()) request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.remove_user_statuses"): view(request, self.remote_user.id) block = models.UserBlocks.objects.get() self.assertEqual(block.user_subject, self.local_user) @@ -64,13 +64,13 @@ class BlockViews(TestCase): self.assertFalse(models.UserFollows.objects.exists()) self.assertFalse(models.UserFollowRequest.objects.exists()) - def test_unblock(self): + def test_unblock(self, _): """ undo a block """ self.local_user.blocks.add(self.remote_user) request = self.factory.post("") request.user = self.local_user - with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): + with patch("bookwyrm.activitystreams.ActivityStream.add_user_statuses"): views.block.unblock(request, self.remote_user.id) self.assertFalse(models.UserBlocks.objects.exists()) From 59deb1cd052d154d745ac45d162a871fabf16f98 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 09:19:11 -0700 Subject: [PATCH 20/33] More mocks for more tests --- bookwyrm/tests/test_goodreads_import.py | 3 +- bookwyrm/tests/test_librarything_import.py | 6 +-- bookwyrm/tests/test_templatetags.py | 52 +++++++++++----------- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/bookwyrm/tests/test_goodreads_import.py b/bookwyrm/tests/test_goodreads_import.py index a62cfdd28..c06b49fc3 100644 --- a/bookwyrm/tests/test_goodreads_import.py +++ b/bookwyrm/tests/test_goodreads_import.py @@ -203,7 +203,8 @@ class GoodreadsImport(TestCase): self.assertEqual(readthrough.finish_date.month, 10) self.assertEqual(readthrough.finish_date.day, 25) - def test_handle_imported_book_review(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_imported_book_review(self, _): """ goodreads review import """ import_job = models.ImportJob.objects.create(user=self.user) datafile = pathlib.Path(__file__).parent.joinpath("data/goodreads.csv") diff --git a/bookwyrm/tests/test_librarything_import.py b/bookwyrm/tests/test_librarything_import.py index a8e4cfe4f..54b8b4226 100644 --- a/bookwyrm/tests/test_librarything_import.py +++ b/bookwyrm/tests/test_librarything_import.py @@ -1,5 +1,4 @@ """ testing import """ -from collections import namedtuple import csv import pathlib from unittest.mock import patch @@ -16,8 +15,8 @@ class LibrarythingImport(TestCase): """ importing from librarything tsv """ def setUp(self): - self.importer = LibrarythingImporter() """ use a test tsv """ + self.importer = LibrarythingImporter() datafile = pathlib.Path(__file__).parent.joinpath("data/librarything.tsv") # Librarything generates latin encoded exports... @@ -200,7 +199,8 @@ class LibrarythingImport(TestCase): self.assertEqual(readthrough.finish_date.month, 5) self.assertEqual(readthrough.finish_date.day, 8) - def test_handle_imported_book_review(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_handle_imported_book_review(self, _): """ librarything review import """ import_job = models.ImportJob.objects.create(user=self.user) datafile = pathlib.Path(__file__).parent.joinpath("data/librarything.tsv") diff --git a/bookwyrm/tests/test_templatetags.py b/bookwyrm/tests/test_templatetags.py index 6bdbd04c0..61136c2eb 100644 --- a/bookwyrm/tests/test_templatetags.py +++ b/bookwyrm/tests/test_templatetags.py @@ -10,6 +10,7 @@ from bookwyrm import models from bookwyrm.templatetags import bookwyrm_tags +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class TemplateTags(TestCase): """ lotta different things here """ @@ -32,34 +33,34 @@ class TemplateTags(TestCase): ) self.book = models.Edition.objects.create(title="Test Book") - def test_dict_key(self): + def test_dict_key(self, _): """ just getting a value out of a dict """ test_dict = {"a": 1, "b": 3} self.assertEqual(bookwyrm_tags.dict_key(test_dict, "a"), 1) self.assertEqual(bookwyrm_tags.dict_key(test_dict, "c"), 0) - def test_get_user_rating(self): + def test_get_user_rating(self, _): """ get a user's most recent rating of a book """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): models.Review.objects.create(user=self.user, book=self.book, rating=3) self.assertEqual(bookwyrm_tags.get_user_rating(self.book, self.user), 3) - def test_get_user_rating_doesnt_exist(self): + def test_get_user_rating_doesnt_exist(self, _): """ there is no rating available """ self.assertEqual(bookwyrm_tags.get_user_rating(self.book, self.user), 0) - def test_get_user_identifer_local(self): + def test_get_user_identifer_local(self, _): """ fall back to the simplest uid available """ self.assertNotEqual(self.user.username, self.user.localname) self.assertEqual(bookwyrm_tags.get_user_identifier(self.user), "mouse") - def test_get_user_identifer_remote(self): + def test_get_user_identifer_remote(self, _): """ for a remote user, should be their full username """ self.assertEqual( bookwyrm_tags.get_user_identifier(self.remote_user), "rat@example.com" ) - def test_get_notification_count(self): + def test_get_notification_count(self, _): """ just countin' """ self.assertEqual(bookwyrm_tags.get_notification_count(self.user), 0) @@ -72,7 +73,7 @@ class TemplateTags(TestCase): self.assertEqual(bookwyrm_tags.get_notification_count(self.user), 2) - def test_get_replies(self): + def test_get_replies(self, _): """ direct replies to a status """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): parent = models.Review.objects.create( @@ -84,12 +85,13 @@ class TemplateTags(TestCase): second_child = models.Status.objects.create( reply_parent=parent, user=self.user, content="hi" ) - third_child = models.Status.objects.create( - reply_parent=parent, - user=self.user, - deleted=True, - deleted_date=timezone.now(), - ) + with patch("bookwyrm.activitystreams.ActivityStream.remove_status"): + third_child = models.Status.objects.create( + reply_parent=parent, + user=self.user, + deleted=True, + deleted_date=timezone.now(), + ) replies = bookwyrm_tags.get_replies(parent) self.assertEqual(len(replies), 2) @@ -97,7 +99,7 @@ class TemplateTags(TestCase): self.assertTrue(second_child in replies) self.assertFalse(third_child in replies) - def test_get_parent(self): + def test_get_parent(self, _): """ get the reply parent of a status """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): parent = models.Review.objects.create( @@ -111,7 +113,7 @@ class TemplateTags(TestCase): self.assertEqual(result, parent) self.assertIsInstance(result, models.Review) - def test_get_user_liked(self): + def test_get_user_liked(self, _): """ did a user like a status """ status = models.Review.objects.create(user=self.remote_user, book=self.book) @@ -120,7 +122,7 @@ class TemplateTags(TestCase): models.Favorite.objects.create(user=self.user, status=status) self.assertTrue(bookwyrm_tags.get_user_liked(self.user, status)) - def test_get_user_boosted(self): + def test_get_user_boosted(self, _): """ did a user boost a status """ status = models.Review.objects.create(user=self.remote_user, book=self.book) @@ -129,7 +131,7 @@ class TemplateTags(TestCase): models.Boost.objects.create(user=self.user, boosted_status=status) self.assertTrue(bookwyrm_tags.get_user_boosted(self.user, status)) - def test_follow_request_exists(self): + def test_follow_request_exists(self, _): """ does a user want to follow """ self.assertFalse( bookwyrm_tags.follow_request_exists(self.user, self.remote_user) @@ -147,7 +149,7 @@ class TemplateTags(TestCase): bookwyrm_tags.follow_request_exists(self.remote_user, self.user) ) - def test_get_boosted(self): + def test_get_boosted(self, _): """ load a boosted status """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): status = models.Review.objects.create(user=self.remote_user, book=self.book) @@ -156,7 +158,7 @@ class TemplateTags(TestCase): self.assertIsInstance(boosted, models.Review) self.assertEqual(boosted, status) - def test_get_book_description(self): + def test_get_book_description(self, _): """ grab it from the edition or the parent """ work = models.Work.objects.create(title="Test Work") self.book.parent_work = work @@ -172,12 +174,12 @@ class TemplateTags(TestCase): self.book.save() self.assertEqual(bookwyrm_tags.get_book_description(self.book), "hello") - def test_get_uuid(self): + def test_get_uuid(self, _): """ uuid functionality """ uuid = bookwyrm_tags.get_uuid("hi") self.assertTrue(re.match(r"hi[A-Za-z0-9\-]", uuid)) - def test_time_since(self): + def test_time_since(self, _): """ ultraconcise timestamps """ self.assertEqual(bookwyrm_tags.time_since("bleh"), "") @@ -207,7 +209,7 @@ class TemplateTags(TestCase): re.match(r"[A-Z][a-z]{2} \d?\d \d{4}", bookwyrm_tags.time_since(years_ago)) ) - def test_get_markdown(self): + def test_get_markdown(self, _): """ mardown format data """ result = bookwyrm_tags.get_markdown("_hi_") self.assertEqual(result, "

    hi

    ") @@ -215,13 +217,13 @@ class TemplateTags(TestCase): result = bookwyrm_tags.get_markdown("_hi_") self.assertEqual(result, "

    hi

    ") - def test_get_mentions(self): + def test_get_mentions(self, _): """ list of people mentioned """ status = models.Status.objects.create(content="hi", user=self.remote_user) result = bookwyrm_tags.get_mentions(status, self.user) self.assertEqual(result, "@rat@example.com ") - def test_get_status_preview_name(self): + def test_get_status_preview_name(self, _): """ status context string """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): status = models.Status.objects.create(content="hi", user=self.user) @@ -246,7 +248,7 @@ class TemplateTags(TestCase): result = bookwyrm_tags.get_status_preview_name(status) self.assertEqual(result, "quotation from Test Book") - def test_related_status(self): + def test_related_status(self, _): """ gets the subclass model for a notification status """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): status = models.Status.objects.create(content="hi", user=self.user) From 9e23bfe7c0677efebebf87868f66ff3ece39d6e8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 10:25:52 -0700 Subject: [PATCH 21/33] Updates activitypub tests --- .../tests/activitypub/test_base_activity.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/bookwyrm/tests/activitypub/test_base_activity.py b/bookwyrm/tests/activitypub/test_base_activity.py index f3d3decde..8ec0b703f 100644 --- a/bookwyrm/tests/activitypub/test_base_activity.py +++ b/bookwyrm/tests/activitypub/test_base_activity.py @@ -19,6 +19,7 @@ from bookwyrm.activitypub import ActivitySerializerError from bookwyrm import models +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class BaseActivity(TestCase): """ the super class for model-linked activitypub dataclasses """ @@ -43,24 +44,24 @@ class BaseActivity(TestCase): image.save(output, format=image.format) self.image_data = output.getvalue() - def test_init(self): + def test_init(self, _): """ simple successfuly init """ instance = ActivityObject(id="a", type="b") self.assertTrue(hasattr(instance, "id")) self.assertTrue(hasattr(instance, "type")) - def test_init_missing(self): + def test_init_missing(self, _): """ init with missing required params """ with self.assertRaises(ActivitySerializerError): ActivityObject() - def test_init_extra_fields(self): + def test_init_extra_fields(self, _): """ init ignoring additional fields """ instance = ActivityObject(id="a", type="b", fish="c") self.assertTrue(hasattr(instance, "id")) self.assertTrue(hasattr(instance, "type")) - def test_init_default_field(self): + def test_init_default_field(self, _): """ replace an existing required field with a default field """ @dataclass(init=False) @@ -73,7 +74,7 @@ class BaseActivity(TestCase): self.assertEqual(instance.id, "a") self.assertEqual(instance.type, "TestObject") - def test_serialize(self): + def test_serialize(self, _): """ simple function for converting dataclass to dict """ instance = ActivityObject(id="a", type="b") serialized = instance.serialize() @@ -82,7 +83,7 @@ class BaseActivity(TestCase): self.assertEqual(serialized["type"], "b") @responses.activate - def test_resolve_remote_id(self): + def test_resolve_remote_id(self, _): """ look up or load remote data """ # existing item result = resolve_remote_id("http://example.com/a/b", model=models.User) @@ -104,14 +105,14 @@ class BaseActivity(TestCase): self.assertEqual(result.remote_id, "https://example.com/user/mouse") self.assertEqual(result.name, "MOUSE?? MOUSE!!") - def test_to_model_invalid_model(self): + def test_to_model_invalid_model(self, _): """ catch mismatch between activity type and model type """ instance = ActivityObject(id="a", type="b") with self.assertRaises(ActivitySerializerError): instance.to_model(model=models.User) @responses.activate - def test_to_model_image(self): + def test_to_model_image(self, _): """ update an image field """ activity = activitypub.Person( id=self.user.remote_id, @@ -144,7 +145,7 @@ class BaseActivity(TestCase): self.assertEqual(self.user.name, "New Name") self.assertEqual(self.user.key_pair.public_key, "hi") - def test_to_model_many_to_many(self): + def test_to_model_many_to_many(self, _): """ annoying that these all need special handling """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): status = models.Status.objects.create( @@ -175,7 +176,7 @@ class BaseActivity(TestCase): self.assertEqual(status.mention_books.first(), book) @responses.activate - def test_to_model_one_to_many(self): + def test_to_model_one_to_many(self, _): """these are reversed relationships, where the secondary object keys the primary object but not vice versa""" with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -214,7 +215,7 @@ class BaseActivity(TestCase): self.assertIsNone(status.attachments.first()) @responses.activate - def test_set_related_field(self): + def test_set_related_field(self, _): """ celery task to add back-references to created objects """ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): status = models.Status.objects.create( From 7f271dbde712fe2ce76d14fc2f48000e5a3cd140 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 10:41:18 -0700 Subject: [PATCH 22/33] Fixes model tests --- .../tests/models/test_activitypub_mixin.py | 33 +++++----- bookwyrm/tests/models/test_base_model.py | 6 +- bookwyrm/tests/models/test_fields.py | 3 +- bookwyrm/tests/models/test_status_model.py | 62 ++++++++++--------- 4 files changed, 54 insertions(+), 50 deletions(-) diff --git a/bookwyrm/tests/models/test_activitypub_mixin.py b/bookwyrm/tests/models/test_activitypub_mixin.py index 930f3a533..0d1acd978 100644 --- a/bookwyrm/tests/models/test_activitypub_mixin.py +++ b/bookwyrm/tests/models/test_activitypub_mixin.py @@ -13,6 +13,7 @@ from bookwyrm.models.activitypub_mixin import ActivitypubMixin from bookwyrm.models.activitypub_mixin import ActivityMixin, ObjectMixin +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class ActivitypubMixins(TestCase): """ functionality shared across models """ @@ -44,7 +45,7 @@ class ActivitypubMixins(TestCase): } # ActivitypubMixin - def test_to_activity(self): + def test_to_activity(self, _): """ model to ActivityPub json """ @dataclass(init=False) @@ -65,7 +66,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(activity["id"], "https://www.example.com/test") self.assertEqual(activity["type"], "Test") - def test_find_existing_by_remote_id(self): + def test_find_existing_by_remote_id(self, _): """ attempt to match a remote id to an object in the db """ # uses a different remote id scheme # this isn't really part of this test directly but it's helpful to state @@ -98,7 +99,7 @@ class ActivitypubMixins(TestCase): # test subclass match result = models.Status.find_existing_by_remote_id("https://comment.net") - def test_find_existing(self): + def test_find_existing(self, _): """ match a blob of data to a model """ book = models.Edition.objects.create( title="Test edition", @@ -108,7 +109,7 @@ class ActivitypubMixins(TestCase): result = models.Edition.find_existing({"openlibraryKey": "OL1234"}) self.assertEqual(result, book) - def test_get_recipients_public_object(self): + def test_get_recipients_public_object(self, _): """ determines the recipients for an object's broadcast """ MockSelf = namedtuple("Self", ("privacy")) mock_self = MockSelf("public") @@ -116,7 +117,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(len(recipients), 1) self.assertEqual(recipients[0], self.remote_user.inbox) - def test_get_recipients_public_user_object_no_followers(self): + def test_get_recipients_public_user_object_no_followers(self, _): """ determines the recipients for a user's object broadcast """ MockSelf = namedtuple("Self", ("privacy", "user")) mock_self = MockSelf("public", self.local_user) @@ -124,7 +125,7 @@ class ActivitypubMixins(TestCase): recipients = ActivitypubMixin.get_recipients(mock_self) self.assertEqual(len(recipients), 0) - def test_get_recipients_public_user_object(self): + def test_get_recipients_public_user_object(self, _): """ determines the recipients for a user's object broadcast """ MockSelf = namedtuple("Self", ("privacy", "user")) mock_self = MockSelf("public", self.local_user) @@ -134,7 +135,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(len(recipients), 1) self.assertEqual(recipients[0], self.remote_user.inbox) - def test_get_recipients_public_user_object_with_mention(self): + def test_get_recipients_public_user_object_with_mention(self, _): """ determines the recipients for a user's object broadcast """ MockSelf = namedtuple("Self", ("privacy", "user")) mock_self = MockSelf("public", self.local_user) @@ -157,7 +158,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(recipients[0], another_remote_user.inbox) self.assertEqual(recipients[1], self.remote_user.inbox) - def test_get_recipients_direct(self): + def test_get_recipients_direct(self, _): """ determines the recipients for a user's object broadcast """ MockSelf = namedtuple("Self", ("privacy", "user")) mock_self = MockSelf("public", self.local_user) @@ -179,7 +180,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(len(recipients), 1) self.assertEqual(recipients[0], another_remote_user.inbox) - def test_get_recipients_combine_inboxes(self): + def test_get_recipients_combine_inboxes(self, _): """ should combine users with the same shared_inbox """ self.remote_user.shared_inbox = "http://example.com/inbox" self.remote_user.save(broadcast=False) @@ -203,7 +204,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(len(recipients), 1) self.assertEqual(recipients[0], "http://example.com/inbox") - def test_get_recipients_software(self): + def test_get_recipients_software(self, _): """ should differentiate between bookwyrm and other remote users """ with patch("bookwyrm.models.user.set_remote_server.delay"): another_remote_user = models.User.objects.create_user( @@ -233,7 +234,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(recipients[0], another_remote_user.inbox) # ObjectMixin - def test_object_save_create(self): + def test_object_save_create(self, _): """ should save uneventufully when broadcast is disabled """ class Success(Exception): @@ -264,7 +265,7 @@ class ActivitypubMixins(TestCase): ObjectModel(user=self.local_user).save(broadcast=False) ObjectModel(user=None).save() - def test_object_save_update(self): + def test_object_save_update(self, _): """ should save uneventufully when broadcast is disabled """ class Success(Exception): @@ -290,7 +291,7 @@ class ActivitypubMixins(TestCase): with self.assertRaises(Success): UpdateObjectModel(id=1, last_edited_by=self.local_user).save() - def test_object_save_delete(self): + def test_object_save_delete(self, _): """ should create delete activities when objects are deleted by flag """ class ActivitySuccess(Exception): @@ -312,7 +313,7 @@ class ActivitypubMixins(TestCase): with self.assertRaises(ActivitySuccess): DeletableObjectModel(id=1, user=self.local_user, deleted=True).save() - def test_to_delete_activity(self): + def test_to_delete_activity(self, _): """ wrapper for Delete activity """ MockSelf = namedtuple("Self", ("remote_id", "to_activity")) mock_self = MockSelf( @@ -327,7 +328,7 @@ class ActivitypubMixins(TestCase): activity["cc"], ["https://www.w3.org/ns/activitystreams#Public"] ) - def test_to_update_activity(self): + def test_to_update_activity(self, _): """ ditto above but for Update """ MockSelf = namedtuple("Self", ("remote_id", "to_activity")) mock_self = MockSelf( @@ -345,7 +346,7 @@ class ActivitypubMixins(TestCase): self.assertIsInstance(activity["object"], dict) # Activity mixin - def test_to_undo_activity(self): + def test_to_undo_activity(self, _): """ and again, for Undo """ MockSelf = namedtuple("Self", ("remote_id", "to_activity", "user")) mock_self = MockSelf( diff --git a/bookwyrm/tests/models/test_base_model.py b/bookwyrm/tests/models/test_base_model.py index 4479d1560..25a2e7ee6 100644 --- a/bookwyrm/tests/models/test_base_model.py +++ b/bookwyrm/tests/models/test_base_model.py @@ -27,18 +27,18 @@ class BaseModel(TestCase): expected = instance.get_remote_id() self.assertEqual(expected, "https://%s/user/mouse/bookwyrmmodel/1" % DOMAIN) - def test_execute_after_save(self): + def test_set_remote_id(self): """ this function sets remote ids after creation """ # using Work because it BookWrymModel is abstract and this requires save # Work is a relatively not-fancy model. instance = models.Work.objects.create(title="work title") instance.remote_id = None - base_model.execute_after_save(None, instance, True) + base_model.set_remote_id(None, instance, True) self.assertEqual( instance.remote_id, "https://%s/book/%d" % (DOMAIN, instance.id) ) # shouldn't set remote_id if it's not created instance.remote_id = None - base_model.execute_after_save(None, instance, False) + base_model.set_remote_id(None, instance, False) self.assertIsNone(instance.remote_id) diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 28faf52c3..18bb028ff 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -185,7 +185,8 @@ class ActivitypubFields(TestCase): self.assertEqual(model_instance.privacy_field, "unlisted") @patch("bookwyrm.models.activitypub_mixin.ObjectMixin.broadcast") - def test_privacy_field_set_activity_from_field(self, _): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_privacy_field_set_activity_from_field(self, *_): """ translate between to/cc fields and privacy """ user = User.objects.create_user( "rat", "rat@rat.rat", "ratword", local=True, localname="rat" diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 1dcf56339..54fe7fee9 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -15,6 +15,7 @@ from bookwyrm import activitypub, models, settings # pylint: disable=too-many-public-methods @patch("bookwyrm.models.Status.broadcast") +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class Status(TestCase): """ lotta types of statuses """ @@ -44,14 +45,14 @@ class Status(TestCase): image.save(output, format=image.format) self.book.cover.save("test.jpg", ContentFile(output.getvalue())) - def test_status_generated_fields(self, _): + def test_status_generated_fields(self, *_): """ setting remote id """ status = models.Status.objects.create(content="bleh", user=self.local_user) expected_id = "https://%s/user/mouse/status/%d" % (settings.DOMAIN, status.id) self.assertEqual(status.remote_id, expected_id) self.assertEqual(status.privacy, "public") - def test_replies(self, _): + def test_replies(self, *_): """ get a list of replies """ parent = models.Status.objects.create(content="hi", user=self.local_user) child = models.Status.objects.create( @@ -70,7 +71,7 @@ class Status(TestCase): # should select subclasses self.assertIsInstance(replies.last(), models.Review) - def test_status_type(self, _): + def test_status_type(self, *_): """ class name """ self.assertEqual(models.Status().status_type, "Note") self.assertEqual(models.Review().status_type, "Review") @@ -78,14 +79,14 @@ class Status(TestCase): self.assertEqual(models.Comment().status_type, "Comment") self.assertEqual(models.Boost().status_type, "Announce") - def test_boostable(self, _): + def test_boostable(self, *_): """ can a status be boosted, based on privacy """ self.assertTrue(models.Status(privacy="public").boostable) self.assertTrue(models.Status(privacy="unlisted").boostable) self.assertFalse(models.Status(privacy="followers").boostable) self.assertFalse(models.Status(privacy="direct").boostable) - def test_to_replies(self, _): + def test_to_replies(self, *_): """ activitypub replies collection """ parent = models.Status.objects.create(content="hi", user=self.local_user) child = models.Status.objects.create( @@ -102,7 +103,7 @@ class Status(TestCase): self.assertEqual(replies["id"], "%s/replies" % parent.remote_id) self.assertEqual(replies["totalItems"], 2) - def test_status_to_activity(self, _): + def test_status_to_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Status.objects.create( content="test content", user=self.local_user @@ -113,20 +114,21 @@ class Status(TestCase): self.assertEqual(activity["content"], "test content") self.assertEqual(activity["sensitive"], False) - def test_status_to_activity_tombstone(self, _): + def test_status_to_activity_tombstone(self, *_): """ subclass of the base model version with a "pure" serializer """ - status = models.Status.objects.create( - content="test content", - user=self.local_user, - deleted=True, - deleted_date=timezone.now(), - ) + with patch("bookwyrm.activitystreams.ActivityStream.remove_status"): + status = models.Status.objects.create( + content="test content", + user=self.local_user, + deleted=True, + deleted_date=timezone.now(), + ) activity = status.to_activity() self.assertEqual(activity["id"], status.remote_id) self.assertEqual(activity["type"], "Tombstone") self.assertFalse(hasattr(activity, "content")) - def test_status_to_pure_activity(self, _): + def test_status_to_pure_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Status.objects.create( content="test content", user=self.local_user @@ -138,7 +140,7 @@ class Status(TestCase): self.assertEqual(activity["sensitive"], False) self.assertEqual(activity["attachment"], []) - def test_generated_note_to_activity(self, _): + def test_generated_note_to_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.GeneratedNote.objects.create( content="test content", user=self.local_user @@ -152,7 +154,7 @@ class Status(TestCase): self.assertEqual(activity["sensitive"], False) self.assertEqual(len(activity["tag"]), 2) - def test_generated_note_to_pure_activity(self, _): + def test_generated_note_to_pure_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.GeneratedNote.objects.create( content="test content", user=self.local_user @@ -176,7 +178,7 @@ class Status(TestCase): ) self.assertEqual(activity["attachment"][0].name, "Test Edition") - def test_comment_to_activity(self, _): + def test_comment_to_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Comment.objects.create( content="test content", user=self.local_user, book=self.book @@ -187,7 +189,7 @@ class Status(TestCase): self.assertEqual(activity["content"], "test content") self.assertEqual(activity["inReplyToBook"], self.book.remote_id) - def test_comment_to_pure_activity(self, _): + def test_comment_to_pure_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Comment.objects.create( content="test content", user=self.local_user, book=self.book @@ -207,7 +209,7 @@ class Status(TestCase): ) self.assertEqual(activity["attachment"][0].name, "Test Edition") - def test_quotation_to_activity(self, _): + def test_quotation_to_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Quotation.objects.create( quote="a sickening sense", @@ -222,7 +224,7 @@ class Status(TestCase): self.assertEqual(activity["content"], "test content") self.assertEqual(activity["inReplyToBook"], self.book.remote_id) - def test_quotation_to_pure_activity(self, _): + def test_quotation_to_pure_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Quotation.objects.create( quote="a sickening sense", @@ -245,7 +247,7 @@ class Status(TestCase): ) self.assertEqual(activity["attachment"][0].name, "Test Edition") - def test_review_to_activity(self, _): + def test_review_to_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Review.objects.create( name="Review name", @@ -262,7 +264,7 @@ class Status(TestCase): self.assertEqual(activity["content"], "test content") self.assertEqual(activity["inReplyToBook"], self.book.remote_id) - def test_review_to_pure_activity(self, _): + def test_review_to_pure_activity(self, *_): """ subclass of the base model version with a "pure" serializer """ status = models.Review.objects.create( name="Review name", @@ -285,7 +287,7 @@ class Status(TestCase): ) self.assertEqual(activity["attachment"][0].name, "Test Edition") - def test_favorite(self, _): + def test_favorite(self, *_): """ fav a status """ real_broadcast = models.Favorite.broadcast @@ -311,7 +313,7 @@ class Status(TestCase): self.assertEqual(activity["object"], status.remote_id) models.Favorite.broadcast = real_broadcast - def test_boost(self, _): + def test_boost(self, *_): """ boosting, this one's a bit fussy """ status = models.Status.objects.create( content="test content", user=self.local_user @@ -323,7 +325,7 @@ class Status(TestCase): self.assertEqual(activity["type"], "Announce") self.assertEqual(activity, boost.to_activity(pure=True)) - def test_notification(self, _): + def test_notification(self, *_): """ a simple model """ notification = models.Notification.objects.create( user=self.local_user, notification_type="FAVORITE" @@ -335,7 +337,7 @@ class Status(TestCase): user=self.local_user, notification_type="GLORB" ) - def test_create_broadcast(self, broadcast_mock): + def test_create_broadcast(self, _, broadcast_mock): """ should send out two verions of a status on create """ models.Comment.objects.create( content="hi", user=self.local_user, book=self.book @@ -355,7 +357,7 @@ class Status(TestCase): self.assertEqual(args["type"], "Create") self.assertEqual(args["object"]["type"], "Comment") - def test_recipients_with_mentions(self, _): + def test_recipients_with_mentions(self, *_): """ get recipients to broadcast a status """ status = models.GeneratedNote.objects.create( content="test content", user=self.local_user @@ -364,7 +366,7 @@ class Status(TestCase): self.assertEqual(status.recipients, [self.remote_user]) - def test_recipients_with_reply_parent(self, _): + def test_recipients_with_reply_parent(self, *_): """ get recipients to broadcast a status """ parent_status = models.GeneratedNote.objects.create( content="test content", user=self.remote_user @@ -375,7 +377,7 @@ class Status(TestCase): self.assertEqual(status.recipients, [self.remote_user]) - def test_recipients_with_reply_parent_and_mentions(self, _): + def test_recipients_with_reply_parent_and_mentions(self, *_): """ get recipients to broadcast a status """ parent_status = models.GeneratedNote.objects.create( content="test content", user=self.remote_user @@ -388,7 +390,7 @@ class Status(TestCase): self.assertEqual(status.recipients, [self.remote_user]) @responses.activate - def test_ignore_activity_boost(self, _): + def test_ignore_activity_boost(self, *_): """ don't bother with most remote statuses """ activity = activitypub.Announce( id="http://www.faraway.com/boost/12", From 28651bd8042883118bfa61d9aea64f1073950697 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 11:27:00 -0700 Subject: [PATCH 23/33] Fixes views tests --- bookwyrm/tests/views/test_goal.py | 3 +- bookwyrm/tests/views/test_interaction.py | 6 +++- bookwyrm/tests/views/test_landing.py | 4 ++- bookwyrm/tests/views/test_reading.py | 13 +++++---- bookwyrm/tests/views/test_rss_feed.py | 36 +++++++++++++----------- bookwyrm/tests/views/test_status.py | 1 - 6 files changed, 36 insertions(+), 27 deletions(-) diff --git a/bookwyrm/tests/views/test_goal.py b/bookwyrm/tests/views/test_goal.py index 990bb5c2b..cbe4fe015 100644 --- a/bookwyrm/tests/views/test_goal.py +++ b/bookwyrm/tests/views/test_goal.py @@ -102,7 +102,8 @@ class GoalViews(TestCase): result = view(request, self.local_user.localname, 2020) self.assertEqual(result.status_code, 404) - def test_create_goal(self): + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_create_goal(self, _): """ create a new goal """ view = views.Goal.as_view() request = self.factory.post( diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index b52e9a667..40152c0fb 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -5,6 +5,7 @@ from django.test.client import RequestFactory from bookwyrm import models, views + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") class InteractionViews(TestCase): """ viewing and creating statuses """ @@ -146,8 +147,11 @@ class InteractionViews(TestCase): self.assertEqual(models.Boost.objects.count(), 1) self.assertEqual(models.Notification.objects.count(), 1) broadcast_mock.call_count = 0 - with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + with patch( + "bookwyrm.activitystreams.ActivityStream.remove_status" + ) as redis_mock: view(request, status.id) self.assertEqual(broadcast_mock.call_count, 1) + self.assertTrue(redis_mock.called) self.assertEqual(models.Boost.objects.count(), 0) self.assertEqual(models.Notification.objects.count(), 0) diff --git a/bookwyrm/tests/views/test_landing.py b/bookwyrm/tests/views/test_landing.py index 910e4a851..2513fecc9 100644 --- a/bookwyrm/tests/views/test_landing.py +++ b/bookwyrm/tests/views/test_landing.py @@ -1,4 +1,5 @@ """ test for app action functionality """ +from unittest.mock import patch from django.contrib.auth.models import AnonymousUser from django.template.response import TemplateResponse from django.test import TestCase @@ -30,7 +31,8 @@ class LandingViews(TestCase): view = views.Home.as_view() request = self.factory.get("") request.user = self.local_user - result = view(request) + with patch("bookwyrm.activitystreams.ActivityStream.get_activity_stream"): + result = view(request) self.assertEqual(result.status_code, 200) result.render() diff --git a/bookwyrm/tests/views/test_reading.py b/bookwyrm/tests/views/test_reading.py index 96d1f1f4c..4661e487e 100644 --- a/bookwyrm/tests/views/test_reading.py +++ b/bookwyrm/tests/views/test_reading.py @@ -8,6 +8,7 @@ from django.utils import timezone from bookwyrm import models, views +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class ReadingViews(TestCase): """ viewing and creating statuses """ @@ -39,7 +40,7 @@ class ReadingViews(TestCase): outbox="https://example.com/users/rat/outbox", ) - def test_start_reading(self): + def test_start_reading(self, _): """ begin a book """ shelf = self.local_user.shelf_set.get(identifier="reading") self.assertFalse(shelf.books.exists()) @@ -70,7 +71,7 @@ class ReadingViews(TestCase): self.assertEqual(readthrough.user, self.local_user) self.assertEqual(readthrough.book, self.book) - def test_start_reading_reshelf(self): + def test_start_reading_reshelf(self, _): """ begin a book """ to_read_shelf = self.local_user.shelf_set.get(identifier="to-read") with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): @@ -90,7 +91,7 @@ class ReadingViews(TestCase): self.assertFalse(to_read_shelf.books.exists()) self.assertEqual(shelf.books.get(), self.book) - def test_finish_reading(self): + def test_finish_reading(self, _): """ begin a book """ shelf = self.local_user.shelf_set.get(identifier="read") self.assertFalse(shelf.books.exists()) @@ -126,7 +127,7 @@ class ReadingViews(TestCase): self.assertEqual(readthrough.user, self.local_user) self.assertEqual(readthrough.book, self.book) - def test_edit_readthrough(self): + def test_edit_readthrough(self, _): """ adding dates to an ongoing readthrough """ start = timezone.make_aware(dateutil.parser.parse("2021-01-03")) readthrough = models.ReadThrough.objects.create( @@ -153,7 +154,7 @@ class ReadingViews(TestCase): self.assertEqual(readthrough.finish_date.day, 7) self.assertEqual(readthrough.book, self.book) - def test_delete_readthrough(self): + def test_delete_readthrough(self, _): """ remove a readthrough """ readthrough = models.ReadThrough.objects.create( book=self.book, user=self.local_user @@ -170,7 +171,7 @@ class ReadingViews(TestCase): views.delete_readthrough(request) self.assertFalse(models.ReadThrough.objects.filter(id=readthrough.id).exists()) - def test_create_readthrough(self): + def test_create_readthrough(self, _): """ adding new read dates """ request = self.factory.post( "", diff --git a/bookwyrm/tests/views/test_rss_feed.py b/bookwyrm/tests/views/test_rss_feed.py index c7fef2a94..0230b4a96 100644 --- a/bookwyrm/tests/views/test_rss_feed.py +++ b/bookwyrm/tests/views/test_rss_feed.py @@ -26,28 +26,30 @@ class RssFeedView(TestCase): ) with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"): - self.review = models.Review.objects.create( - name="Review name", - content="test content", - rating=3, - user=self.user, - book=self.book, - ) + with patch("bookwyrm.activitystreams.ActivityStream.add_status"): + self.review = models.Review.objects.create( + name="Review name", + content="test content", + rating=3, + user=self.user, + book=self.book, + ) - self.quote = models.Quotation.objects.create( - quote="a sickening sense", - content="test content", - user=self.user, - book=self.book, - ) + self.quote = models.Quotation.objects.create( + quote="a sickening sense", + content="test content", + user=self.user, + book=self.book, + ) - self.generatednote = models.GeneratedNote.objects.create( - content="test content", user=self.user - ) + self.generatednote = models.GeneratedNote.objects.create( + content="test content", user=self.user + ) self.factory = RequestFactory() - def test_rss_feed(self): + @patch("bookwyrm.activitystreams.ActivityStream.get_activity_stream") + def test_rss_feed(self, _): """ load an rss feed """ view = rss_feed.RssFeed() request = self.factory.get("/user/rss_user/rss") diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index 2a6128504..5dc0e49a7 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -159,7 +159,6 @@ class StatusViews(TestCase): view(request, "reply") self.assertTrue(redis_mock.called) - reply = models.Status.replies(status).first() self.assertEqual(reply.content, "

    right

    ") self.assertEqual(reply.user, user) From b8cd1d5bce861f1f85725323fbce34620461b8c9 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 12:52:38 -0700 Subject: [PATCH 24/33] Show unread status reload link --- bookwyrm/activitystreams.py | 4 ++++ bookwyrm/static/js/shared.js | 4 ++-- bookwyrm/templates/feed/feed.html | 4 ++++ bookwyrm/templates/layout.html | 4 ++-- bookwyrm/urls.py | 3 ++- bookwyrm/views/__init__.py | 2 +- bookwyrm/views/updates.py | 34 +++++++++++++++++-------------- 7 files changed, 34 insertions(+), 21 deletions(-) diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index c1c548d29..1c8f78be0 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -70,6 +70,10 @@ class ActivityStream(ABC): .order_by("-published_date") ) + def get_unread_count(self, user): + """ get the unread status count for this user's feed """ + return int(r.get(self.unread_id(user))) + def populate_stream(self, user): """ go from zero to a timeline """ pipeline = r.pipeline() diff --git a/bookwyrm/static/js/shared.js b/bookwyrm/static/js/shared.js index d390f482f..1f26aba63 100644 --- a/bookwyrm/static/js/shared.js +++ b/bookwyrm/static/js/shared.js @@ -61,9 +61,9 @@ function polling(el, delay) { function updateCountElement(el, data) { const currentCount = el.innerText; - const count = data[el.getAttribute('data-poll')]; + const count = data.count; if (count != currentCount) { - addRemoveClass(el, 'hidden', count < 1); + addRemoveClass(el.closest('[data-poll-wrapper]'), 'hidden', count < 1); el.innerText = count; } } diff --git a/bookwyrm/templates/feed/feed.html b/bookwyrm/templates/feed/feed.html index 5ad18db66..77b593fe9 100644 --- a/bookwyrm/templates/feed/feed.html +++ b/bookwyrm/templates/feed/feed.html @@ -26,6 +26,10 @@
+ + {# announcements and system messages #} {% if request.user.show_goal and not goal and tab == 'home' %} {% now 'Y' as year %} diff --git a/bookwyrm/templates/layout.html b/bookwyrm/templates/layout.html index e57f61520..d08b18202 100644 --- a/bookwyrm/templates/layout.html +++ b/bookwyrm/templates/layout.html @@ -139,8 +139,8 @@ {% trans "Notifications" %} - - {{ request.user | notification_count }} + + {{ request.user | notification_count }} diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 766227999..44db68928 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -37,7 +37,8 @@ urlpatterns = [ re_path(r"^api/v1/instance/?$", views.instance_info), re_path(r"^api/v1/instance/peers/?$", views.peers), # polling updates - re_path("^api/updates/notifications/?$", views.Updates.as_view()), + re_path("^api/updates/notifications/?$", views.get_notification_count), + re_path("^api/updates/stream/(?P[a-z]+)/?$", views.get_unread_status_count), # authentication re_path(r"^login/?$", views.Login.as_view()), re_path(r"^register/?$", views.Register.as_view()), diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index f1ebfc4ca..3439304f0 100644 --- a/bookwyrm/views/__init__.py +++ b/bookwyrm/views/__init__.py @@ -33,6 +33,6 @@ from .shelf import shelve, unshelve from .site import Site from .status import CreateStatus, DeleteStatus from .tag import Tag, AddTag, RemoveTag -from .updates import Updates +from .updates import get_notification_count, get_unread_status_count from .user import User, EditUser, Followers, Following from .wellknown import webfinger, nodeinfo_pointer, nodeinfo, instance_info, peers diff --git a/bookwyrm/views/updates.py b/bookwyrm/views/updates.py index 83b680c0b..5bcb26b92 100644 --- a/bookwyrm/views/updates.py +++ b/bookwyrm/views/updates.py @@ -1,20 +1,24 @@ """ endpoints for getting updates about activity """ from django.contrib.auth.decorators import login_required from django.http import JsonResponse -from django.utils.decorators import method_decorator -from django.views import View -# pylint: disable= no-self-use -@method_decorator(login_required, name="dispatch") -class Updates(View): - """ so the app can poll """ +from bookwyrm import activitystreams - def get(self, request): - """ any notifications waiting? """ - return JsonResponse( - { - "notifications": request.user.notification_set.filter( - read=False - ).count(), - } - ) +@login_required +def get_notification_count(request): + """ any notifications waiting? """ + return JsonResponse({ + "count": request.user.notification_set.filter( + read=False + ).count(), + }) + +@login_required +def get_unread_status_count(request, stream): + """ any unread statuses for this feed? """ + stream = activitystreams.streams.get(stream) + if not stream: + return JsonResponse({}) + return JsonResponse({ + "count": stream.get_unread_count(request.user) + }) From 9e9fd5c5a66f6a289e025d2aec9fa2a30d7de68e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 12:54:49 -0700 Subject: [PATCH 25/33] Only show feed additions on the first page --- bookwyrm/templates/feed/feed.html | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/feed/feed.html b/bookwyrm/templates/feed/feed.html index 88fffbc74..379bf9272 100644 --- a/bookwyrm/templates/feed/feed.html +++ b/bookwyrm/templates/feed/feed.html @@ -26,12 +26,13 @@ +{# announcements and system messages #} +{% if not activities.number > 1 %} -{# announcements and system messages #} -{% if request.user.show_goal and not goal and tab == 'home' and not activities.number > 1 %} +{% if request.user.show_goal and not goal and tab == 'home' %} {% now 'Y' as year %}
{% include 'snippets/goal_card.html' with year=year %} @@ -39,6 +40,8 @@
{% endif %} +{% endif %} + {# activity feed #} {% if not activities %}

{% trans "There aren't any activities right now! Try following a user to get started" %}

From 5caac46c319201d5fd14a75e7938b7a56653702d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 13:02:45 -0700 Subject: [PATCH 26/33] Updates updates tests --- bookwyrm/tests/views/test_updates.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/bookwyrm/tests/views/test_updates.py b/bookwyrm/tests/views/test_updates.py index b41b20148..3fe5b944c 100644 --- a/bookwyrm/tests/views/test_updates.py +++ b/bookwyrm/tests/views/test_updates.py @@ -1,5 +1,7 @@ """ test for app action functionality """ import json +from unittest.mock import patch + from django.http import JsonResponse from django.test import TestCase from django.test.client import RequestFactory @@ -22,21 +24,33 @@ class UpdateViews(TestCase): ) models.SiteSettings.objects.create() - def test_get_updates(self): + def test_get_notification_count(self): """ there are so many views, this just makes sure it LOADS """ - view = views.Updates.as_view() request = self.factory.get("") request.user = self.local_user - result = view(request) + result = views.get_notification_count(request) self.assertIsInstance(result, JsonResponse) data = json.loads(result.getvalue()) - self.assertEqual(data["notifications"], 0) + self.assertEqual(data["count"], 0) models.Notification.objects.create( notification_type="BOOST", user=self.local_user ) - result = view(request) + result = views.get_notification_count(request) self.assertIsInstance(result, JsonResponse) data = json.loads(result.getvalue()) - self.assertEqual(data["notifications"], 1) + self.assertEqual(data["count"], 1) + + def test_get_unread_status_count(self): + """ there are so many views, this just makes sure it LOADS """ + request = self.factory.get("") + request.user = self.local_user + + with patch('bookwyrm.activitystreams.ActivityStream.get_unread_count') as mock: + mock.return_value = 3 + result = views.get_unread_status_count(request, 'home') + + self.assertIsInstance(result, JsonResponse) + data = json.loads(result.getvalue()) + self.assertEqual(data["count"], 3) From 0d88794f46633cf6b8d3729b20260ca6f3795974 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 13:07:29 -0700 Subject: [PATCH 27/33] Python formatting --- bookwyrm/tests/views/test_updates.py | 4 ++-- bookwyrm/views/updates.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bookwyrm/tests/views/test_updates.py b/bookwyrm/tests/views/test_updates.py index 3fe5b944c..dff730e6d 100644 --- a/bookwyrm/tests/views/test_updates.py +++ b/bookwyrm/tests/views/test_updates.py @@ -47,9 +47,9 @@ class UpdateViews(TestCase): request = self.factory.get("") request.user = self.local_user - with patch('bookwyrm.activitystreams.ActivityStream.get_unread_count') as mock: + with patch("bookwyrm.activitystreams.ActivityStream.get_unread_count") as mock: mock.return_value = 3 - result = views.get_unread_status_count(request, 'home') + result = views.get_unread_status_count(request, "home") self.assertIsInstance(result, JsonResponse) data = json.loads(result.getvalue()) diff --git a/bookwyrm/views/updates.py b/bookwyrm/views/updates.py index 5bcb26b92..b095c965a 100644 --- a/bookwyrm/views/updates.py +++ b/bookwyrm/views/updates.py @@ -4,14 +4,16 @@ from django.http import JsonResponse from bookwyrm import activitystreams + @login_required def get_notification_count(request): """ any notifications waiting? """ - return JsonResponse({ - "count": request.user.notification_set.filter( - read=False - ).count(), - }) + return JsonResponse( + { + "count": request.user.notification_set.filter(read=False).count(), + } + ) + @login_required def get_unread_status_count(request, stream): @@ -19,6 +21,4 @@ def get_unread_status_count(request, stream): stream = activitystreams.streams.get(stream) if not stream: return JsonResponse({}) - return JsonResponse({ - "count": stream.get_unread_count(request.user) - }) + return JsonResponse({"count": stream.get_unread_count(request.user)}) From 02bd94fdc85c7e1ad0cb021fc0b60db734d2f4c0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 13:21:14 -0700 Subject: [PATCH 28/33] Handle '/' view in updates --- bookwyrm/views/updates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/views/updates.py b/bookwyrm/views/updates.py index b095c965a..13ad8513a 100644 --- a/bookwyrm/views/updates.py +++ b/bookwyrm/views/updates.py @@ -16,7 +16,7 @@ def get_notification_count(request): @login_required -def get_unread_status_count(request, stream): +def get_unread_status_count(request, stream='home'): """ any unread statuses for this feed? """ stream = activitystreams.streams.get(stream) if not stream: From 1fa7330595730a9896dc72b02ff8124cafe7b1e8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 13:23:35 -0700 Subject: [PATCH 29/33] Management command for rebuilding ALL feeds --- bookwyrm/management/commands/rebuild_feeds.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 bookwyrm/management/commands/rebuild_feeds.py diff --git a/bookwyrm/management/commands/rebuild_feeds.py b/bookwyrm/management/commands/rebuild_feeds.py new file mode 100644 index 000000000..ce7ead0f3 --- /dev/null +++ b/bookwyrm/management/commands/rebuild_feeds.py @@ -0,0 +1,34 @@ +""" Delete and re-create user feeds """ +from django.core.management.base import BaseCommand +import redis + +from bookwyrm import activitystreams, models, settings + +r = redis.Redis( + host=settings.REDIS_ACTIVITY_HOST, port=settings.REDIS_ACTIVITY_PORT, db=0 +) + +def erase_feeds(): + """ throw the whole redis away """ + r.flushall() + +def create_feeds(): + """ build all the fields for all the users """ + users = models.User.objects.filter( + local=True, + is_active=True, + ) + for user in users: + for stream in activitystreams.streams.values(): + stream.populate_stream(user) + + +class Command(BaseCommand): + """ start all over with user feeds """ + + help = "Delete and re-create all the user feeds" + # pylint: disable=no-self-use,unused-argument + def handle(self, *args, **options): + """ run feed builder """ + erase_feeds() + create_feeds() From 3a0025b105cc4f200b8dd09c840f4e745d2e2d9d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 13:28:05 -0700 Subject: [PATCH 30/33] Python formatting --- bookwyrm/management/commands/rebuild_feeds.py | 2 ++ bookwyrm/views/updates.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/management/commands/rebuild_feeds.py b/bookwyrm/management/commands/rebuild_feeds.py index ce7ead0f3..70d4dd981 100644 --- a/bookwyrm/management/commands/rebuild_feeds.py +++ b/bookwyrm/management/commands/rebuild_feeds.py @@ -8,10 +8,12 @@ r = redis.Redis( host=settings.REDIS_ACTIVITY_HOST, port=settings.REDIS_ACTIVITY_PORT, db=0 ) + def erase_feeds(): """ throw the whole redis away """ r.flushall() + def create_feeds(): """ build all the fields for all the users """ users = models.User.objects.filter( diff --git a/bookwyrm/views/updates.py b/bookwyrm/views/updates.py index 13ad8513a..cc5fc4199 100644 --- a/bookwyrm/views/updates.py +++ b/bookwyrm/views/updates.py @@ -16,7 +16,7 @@ def get_notification_count(request): @login_required -def get_unread_status_count(request, stream='home'): +def get_unread_status_count(request, stream="home"): """ any unread statuses for this feed? """ stream = activitystreams.streams.get(stream) if not stream: From 7b8bb6bb21923e6b0cee74e4ecef7d3af04e023f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 13:38:03 -0700 Subject: [PATCH 31/33] Updates readme --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index e6a75375d..e798fedf5 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ Web backend - [ActivityPub](http://activitypub.rocks/) federation - [Celery](http://celeryproject.org/) task queuing - [Redis](https://redis.io/) task backend +- [Redis (again)](https://redis.io/) activity stream manager Front end - Django templates @@ -236,6 +237,11 @@ When there are changes available in the production branch, you can install and g - `docker-compose exec web python manage.py collectstatic --no-input` loads any updated static files (such as the JavaScript and CSS) - `docker-compose restart` reloads the docker containers +### Re-building activity streams + +If something goes awry with user timelines, and you want to re-create them en mass, there's a management command for that: +`docker-compose run --rm web python manage.py rebuild_feeds` + ### Port Conflicts BookWyrm has multiple services that run on their default ports. From 3e888c17dd1ac01f14f1e711bc98644c09f7d041 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 14:59:51 -0700 Subject: [PATCH 32/33] Fixes audience filters with tests --- bookwyrm/activitystreams.py | 11 +- bookwyrm/tests/test_activitystreams.py | 216 +++++++++++++++++++++++++ 2 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 bookwyrm/tests/test_activitystreams.py diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 1c8f78be0..88f1f0114 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -98,8 +98,14 @@ class ActivityStream(ABC): Q(id__in=status.user.blocks.all()) | Q(blocks=status.user) # not blocked ) + # only visible to the poster and mentioned users + if status.privacy == "direct": + audience = audience.filter( + Q(id=status.user.id) # if the user is the post's author + | Q(id__in=status.mention_users.all()) # if the user is mentioned + ) # only visible to the poster's followers and tagged users - if status.privacy == "followers": + elif status.privacy == "followers": audience = audience.filter( Q(id=status.user.id) # if the user is the post's author | Q(following=status.user) # if the user is following the author @@ -125,7 +131,6 @@ class HomeStream(ActivityStream): return audience.filter( Q(id=status.user.id) # if the user is the post's author | Q(following=status.user) # if the user is following the author - | Q(id__in=status.mention_users.all()) # or the user is mentioned ) def stream_statuses(self, user): @@ -144,7 +149,7 @@ class LocalStream(ActivityStream): def stream_users(self, status): # this stream wants no part in non-public statuses - if status.privacy != "public": + if status.privacy != "public" or not status.user.local: return None return super().stream_users(status) diff --git a/bookwyrm/tests/test_activitystreams.py b/bookwyrm/tests/test_activitystreams.py new file mode 100644 index 000000000..a8daea84a --- /dev/null +++ b/bookwyrm/tests/test_activitystreams.py @@ -0,0 +1,216 @@ +""" testing activitystreams """ +from unittest.mock import patch +from django.test import TestCase +from bookwyrm import activitystreams, models + + +class Activitystreams(TestCase): + """ using redis to build activity streams """ + + def setUp(self): + """ use a test csv """ + self.local_user = models.User.objects.create_user( + "mouse", "mouse@mouse.mouse", "password", local=True, localname="mouse" + ) + self.another_user = models.User.objects.create_user( + "nutria", "nutria@nutria.nutria", "password", local=True, localname="nutria" + ) + 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.book = models.Edition.objects.create(title="test book") + + class TestStream(activitystreams.ActivityStream): + """ test stream, don't have to do anything here """ + + key = "test" + + self.test_stream = TestStream() + + def test_activitystream_class_ids(self): + """ the abstract base class for stream objects """ + self.assertEqual( + self.test_stream.stream_id(self.local_user), + "{}-test".format(self.local_user.id), + ) + self.assertEqual( + self.test_stream.unread_id(self.local_user), + "{}-test-unread".format(self.local_user.id), + ) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_abstractstream_stream_users(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) + # 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) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_abstractstream_stream_users_direct(self, _): + """ get a list of users that should see a status """ + status = models.Status.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + ) + status.mention_users.add(self.local_user) + users = self.test_stream.stream_users(status) + self.assertIsNone(users) + + status = models.Comment.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + book=self.book, + ) + status.mention_users.add(self.local_user) + users = self.test_stream.stream_users(status) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_abstractstream_stream_users_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) + self.assertFalse(users.exists()) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_abstractstream_stream_users_followers_self(self, _): + """ get a list of users that should see a status """ + status = models.Comment.objects.create( + user=self.local_user, + content="hi", + privacy="direct", + book=self.book, + ) + users = self.test_stream.stream_users(status) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_abstractstream_stream_users_followers_with_mention(self, _): + """ get a list of users that should see a status """ + status = models.Comment.objects.create( + user=self.remote_user, + content="hi", + privacy="direct", + book=self.book, + ) + status.mention_users.add(self.local_user) + + users = self.test_stream.stream_users(status) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_abstractstream_stream_users_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( + user=self.remote_user, + content="hi", + privacy="direct", + book=self.book, + ) + users = self.test_stream.stream_users(status) + self.assertFalse(self.local_user in users) + self.assertFalse(self.another_user in users) + self.assertFalse(self.remote_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_homestream_stream_users(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) + self.assertFalse(users.exists()) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_homestream_stream_users_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) + self.assertFalse(self.local_user in users) + self.assertFalse(self.another_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_homestream_stream_users_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) + self.assertTrue(self.local_user in users) + self.assertFalse(self.another_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_localstream_stream_users_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) + self.assertIsNone(users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_localstream_stream_users_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) + self.assertTrue(self.local_user in users) + self.assertTrue(self.another_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_localstream_stream_users_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) + self.assertIsNone(users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_federatedstream_stream_users(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) + self.assertTrue(self.local_user in users) + self.assertTrue(self.another_user in users) + + @patch("bookwyrm.activitystreams.ActivityStream.add_status") + def test_federatedstream_stream_users_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) + self.assertIsNone(users) From 701487c526bdcfdd46aa1cb1171b2bcf52604a2a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Mar 2021 15:03:05 -0700 Subject: [PATCH 33/33] Fixes mocks on activitystreams tests --- bookwyrm/tests/test_activitystreams.py | 46 ++++++++++---------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/bookwyrm/tests/test_activitystreams.py b/bookwyrm/tests/test_activitystreams.py index a8daea84a..d7a3d4eb6 100644 --- a/bookwyrm/tests/test_activitystreams.py +++ b/bookwyrm/tests/test_activitystreams.py @@ -4,6 +4,8 @@ from django.test import TestCase from bookwyrm import activitystreams, models +@patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") +@patch("bookwyrm.activitystreams.ActivityStream.add_status") class Activitystreams(TestCase): """ using redis to build activity streams """ @@ -34,7 +36,7 @@ class Activitystreams(TestCase): self.test_stream = TestStream() - def test_activitystream_class_ids(self): + def test_activitystream_class_ids(self, *_): """ the abstract base class for stream objects """ self.assertEqual( self.test_stream.stream_id(self.local_user), @@ -45,8 +47,7 @@ class Activitystreams(TestCase): "{}-test-unread".format(self.local_user.id), ) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_abstractstream_stream_users(self, _): + def test_abstractstream_stream_users(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" @@ -57,8 +58,7 @@ class Activitystreams(TestCase): self.assertTrue(self.local_user in users) self.assertTrue(self.another_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_abstractstream_stream_users_direct(self, _): + def test_abstractstream_stream_users_direct(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, @@ -81,8 +81,7 @@ class Activitystreams(TestCase): self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_abstractstream_stream_users_followers_remote_user(self, _): + def test_abstractstream_stream_users_followers_remote_user(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, @@ -92,8 +91,7 @@ class Activitystreams(TestCase): users = self.test_stream.stream_users(status) self.assertFalse(users.exists()) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_abstractstream_stream_users_followers_self(self, _): + def test_abstractstream_stream_users_followers_self(self, *_): """ get a list of users that should see a status """ status = models.Comment.objects.create( user=self.local_user, @@ -106,8 +104,7 @@ class Activitystreams(TestCase): self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_abstractstream_stream_users_followers_with_mention(self, _): + def test_abstractstream_stream_users_followers_with_mention(self, *_): """ get a list of users that should see a status """ status = models.Comment.objects.create( user=self.remote_user, @@ -122,8 +119,7 @@ class Activitystreams(TestCase): self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_abstractstream_stream_users_followers_with_relationship(self, _): + def test_abstractstream_stream_users_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( @@ -137,8 +133,7 @@ class Activitystreams(TestCase): self.assertFalse(self.another_user in users) self.assertFalse(self.remote_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_homestream_stream_users(self, _): + def test_homestream_stream_users(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" @@ -146,8 +141,7 @@ class Activitystreams(TestCase): users = activitystreams.HomeStream().stream_users(status) self.assertFalse(users.exists()) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_homestream_stream_users_with_mentions(self, _): + def test_homestream_stream_users_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" @@ -157,8 +151,7 @@ class Activitystreams(TestCase): self.assertFalse(self.local_user in users) self.assertFalse(self.another_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_homestream_stream_users_with_relationship(self, _): + def test_homestream_stream_users_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( @@ -168,8 +161,7 @@ class Activitystreams(TestCase): self.assertTrue(self.local_user in users) self.assertFalse(self.another_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_localstream_stream_users_remote_status(self, _): + def test_localstream_stream_users_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" @@ -177,8 +169,7 @@ class Activitystreams(TestCase): users = activitystreams.LocalStream().stream_users(status) self.assertIsNone(users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_localstream_stream_users_local_status(self, _): + def test_localstream_stream_users_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" @@ -187,8 +178,7 @@ class Activitystreams(TestCase): self.assertTrue(self.local_user in users) self.assertTrue(self.another_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_localstream_stream_users_unlisted(self, _): + def test_localstream_stream_users_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" @@ -196,8 +186,7 @@ class Activitystreams(TestCase): users = activitystreams.LocalStream().stream_users(status) self.assertIsNone(users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_federatedstream_stream_users(self, _): + def test_federatedstream_stream_users(self, *_): """ get a list of users that should see a status """ status = models.Status.objects.create( user=self.remote_user, content="hi", privacy="public" @@ -206,8 +195,7 @@ class Activitystreams(TestCase): self.assertTrue(self.local_user in users) self.assertTrue(self.another_user in users) - @patch("bookwyrm.activitystreams.ActivityStream.add_status") - def test_federatedstream_stream_users_unlisted(self, _): + def test_federatedstream_stream_users_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"