From ee7c04cc7e442bdba54e5c9d92a135e013315803 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 9 May 2020 14:26:27 -0700 Subject: [PATCH] code style cleanup --- fedireads/activitypub/shelve.py | 2 +- fedireads/broadcast.py | 9 +++- fedireads/forms.py | 2 +- fedireads/goodreads_import.py | 5 +++ fedireads/incoming.py | 73 +++++++++++++++------------------ fedireads/models/import_job.py | 4 +- fedireads/view_actions.py | 1 - fedireads/wellknown.py | 26 ++++++------ 8 files changed, 65 insertions(+), 57 deletions(-) diff --git a/fedireads/activitypub/shelve.py b/fedireads/activitypub/shelve.py index d3a22edf..46cf4132 100644 --- a/fedireads/activitypub/shelve.py +++ b/fedireads/activitypub/shelve.py @@ -12,7 +12,7 @@ def get_remove(*args): def get_add_remove(user, book, shelf, action='Add'): - ''' format an Add or Remove json blob ''' + ''' format a shelve book json blob ''' uuid = uuid4() return { '@context': 'https://www.w3.org/ns/activitystreams', diff --git a/fedireads/broadcast.py b/fedireads/broadcast.py index 995df2cf..6501137e 100644 --- a/fedireads/broadcast.py +++ b/fedireads/broadcast.py @@ -19,10 +19,13 @@ def get_public_recipients(user, software=None): # TODO: eventually we may want to handle particular software differently followers = followers.filter(fedireads_user=(software == 'fedireads')) + # we want shared inboxes when available shared = followers.filter( shared_inbox__isnull=False ).values_list('shared_inbox', flat=True).distinct() + # if a user doesn't have a shared inbox, we need their personal inbox + # iirc pixelfed doesn't have shared inboxes inboxes = followers.filter( shared_inbox__isnull=True ).values_list('inbox', flat=True) @@ -33,7 +36,9 @@ def get_public_recipients(user, software=None): def broadcast(sender, activity, software=None, \ privacy='public', direct_recipients=None): ''' send out an event ''' + # start with parsing the direct recipients recipients = [u.inbox for u in direct_recipients or []] + # and then add any other recipients # TODO: other kinds of privacy if privacy == 'public': recipients += get_public_recipients(sender, software=software) @@ -69,7 +74,9 @@ def sign_and_send(sender, activity, destination): ] message_to_sign = '\n'.join(signature_headers) - # TODO: raise an error if the user doesn't have a private key + if not sender.private_key: + # this shouldn't happen. it would be bad if it happened. + raise ValueError('No private key found for sender') signer = pkcs1_15.new(RSA.import_key(sender.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode('utf8'))) diff --git a/fedireads/forms.py b/fedireads/forms.py index a7501bf5..060658c7 100644 --- a/fedireads/forms.py +++ b/fedireads/forms.py @@ -101,7 +101,7 @@ class EditionForm(ModelForm): 'updated_date', 'last_sync_date', - 'authors', + 'authors',# TODO 'parent_work', 'shelves', 'misc_identifiers', diff --git a/fedireads/goodreads_import.py b/fedireads/goodreads_import.py index fae01d9c..ff7f9429 100644 --- a/fedireads/goodreads_import.py +++ b/fedireads/goodreads_import.py @@ -12,6 +12,7 @@ MAX_ENTRIES = 500 def create_job(user, csv_file): + ''' check over a csv and creates a database entry for the job''' job = ImportJob.objects.create(user=user) for index, entry in enumerate(list(csv.DictReader(csv_file))[:MAX_ENTRIES]): if not all(x in entry for x in ('ISBN13', 'Title', 'Author')): @@ -19,13 +20,17 @@ def create_job(user, csv_file): ImportItem(job=job, index=index, data=entry).save() return job + def start_import(job): + ''' initalizes a csv import job ''' result = import_data.delay(job.id) job.task_id = result.id job.save() + @app.task def import_data(job_id): + ''' does the actual lookup work in a celery task ''' job = ImportJob.objects.get(id=job_id) try: results = [] diff --git a/fedireads/incoming.py b/fedireads/incoming.py index 62fd3d43..206da9d4 100644 --- a/fedireads/incoming.py +++ b/fedireads/incoming.py @@ -42,6 +42,9 @@ def shared_inbox(request): except json.decoder.JSONDecodeError: return HttpResponseBadRequest() + if not activity.get('object'): + return HttpResponseBadRequest() + try: verify_signature(request) except ValueError: @@ -128,9 +131,13 @@ def verify_signature(request): @app.task def handle_follow(activity): ''' someone wants to follow a local user ''' - # figure out who they want to follow - to_follow = models.User.objects.get(actor=activity['object']) - # figure out who they are + # figure out who they want to follow -- not using get_or_create because + # we only allow you to follow local users + try: + to_follow = models.User.objects.get(actor=activity['object']) + except models.User.DoesNotExist: + return False + # figure out who the actor is user = get_or_create_remote_user(activity['actor']) try: request = models.UserFollowRequest.objects.create( @@ -165,14 +172,11 @@ def handle_follow(activity): def handle_unfollow(activity): ''' unfollow a local user ''' obj = activity['object'] - if not obj['type'] == 'Follow': - #idk how to undo other things - return HttpResponseNotFound() try: requester = get_or_create_remote_user(obj['actor']) to_unfollow = models.User.objects.get(actor=obj['object']) except models.User.DoesNotExist: - return HttpResponseNotFound() + return False to_unfollow.followers.remove(requester) @@ -209,7 +213,7 @@ def handle_follow_reject(activity): ) request.delete() except models.UserFollowRequest.DoesNotExist: - pass + return False @app.task @@ -217,46 +221,37 @@ def handle_create(activity): ''' someone did something, good on them ''' user = get_or_create_remote_user(activity['actor']) - if not 'object' in activity: - return False - if user.local: # we really oughtn't even be sending in this case return True if activity['object'].get('fedireadsType') and \ 'inReplyToBook' in activity['object']: - try: - if activity['object']['fedireadsType'] == 'Review': - builder = status_builder.create_review_from_activity - elif activity['object']['fedireadsType'] == 'Quotation': - builder = status_builder.create_quotation_from_activity - else: - builder = status_builder.create_comment_from_activity + if activity['object']['fedireadsType'] == 'Review': + builder = status_builder.create_review_from_activity + elif activity['object']['fedireadsType'] == 'Quotation': + builder = status_builder.create_quotation_from_activity + else: + builder = status_builder.create_comment_from_activity - # create the status, it'll throw a valueerror if anything is missing - builder(user, activity['object']) - except ValueError: - return False + # create the status, it'll throw a ValueError if anything is missing + builder(user, activity['object']) elif activity['object'].get('inReplyTo'): # only create the status if it's in reply to a status we already know if not status_builder.get_status(activity['object']['inReplyTo']): return True - try: - status = status_builder.create_status_from_activity( - user, - activity['object'] + status = status_builder.create_status_from_activity( + user, + activity['object'] + ) + if status and status.reply_parent: + status_builder.create_notification( + status.reply_parent.user, + 'REPLY', + related_user=status.user, + related_status=status, ) - if status and status.reply_parent: - status_builder.create_notification( - status.reply_parent.user, - 'REPLY', - related_user=status.user, - related_status=status, - ) - except ValueError: - return False return True @@ -268,7 +263,7 @@ def handle_favorite(activity): status = models.Status.objects.get(id=status_id) liker = get_or_create_remote_user(activity['actor']) except (models.Status.DoesNotExist, models.User.DoesNotExist): - return + return False if not liker.local: status_builder.create_favorite_from_activity(liker, activity) @@ -287,7 +282,7 @@ def handle_unfavorite(activity): favorite_id = activity['object']['id'] fav = status_builder.get_favorite(favorite_id) if not fav: - return HttpResponseNotFound() + return False fav.delete() @@ -300,7 +295,7 @@ def handle_boost(activity): status = models.Status.objects.get(id=status_id) booster = get_or_create_remote_user(activity['actor']) except (models.Status.DoesNotExist, models.User.DoesNotExist): - return HttpResponseNotFound() + return False if not booster.local: status_builder.create_boost_from_activity(booster, activity) @@ -318,7 +313,7 @@ def handle_tag(activity): ''' someone is tagging or shelving a book ''' user = get_or_create_remote_user(activity['actor']) if not user.local: - book = activity['target']['id'].split('/')[-1] + book = activity['target']['id'] status_builder.create_tag(user, book, activity['object']['name']) diff --git a/fedireads/models/import_job.py b/fedireads/models/import_job.py index 9836bea9..63976af2 100644 --- a/fedireads/models/import_job.py +++ b/fedireads/models/import_job.py @@ -1,3 +1,4 @@ +''' track progress of goodreads imports ''' import re import dateutil.parser @@ -5,7 +6,7 @@ from django.db import models from django.utils import timezone from fedireads import books_manager -from fedireads.models import Edition, ReadThrough, User, Book +from fedireads.models import ReadThrough, User, Book from fedireads.utils.fields import JSONField # Mapping goodreads -> fedireads shelf titles. @@ -32,6 +33,7 @@ def construct_search_term(title, author): return ' '.join([title, author]) + class ImportJob(models.Model): user = models.ForeignKey(User, on_delete=models.CASCADE) created_date = models.DateTimeField(default=timezone.now) diff --git a/fedireads/view_actions.py b/fedireads/view_actions.py index eaa49304..6c33e7c2 100644 --- a/fedireads/view_actions.py +++ b/fedireads/view_actions.py @@ -13,7 +13,6 @@ from fedireads import forms, models, outgoing from fedireads import goodreads_import from fedireads.settings import DOMAIN from fedireads.views import get_user_from_username -from fedireads.books_manager import get_or_create_book def user_login(request): diff --git a/fedireads/wellknown.py b/fedireads/wellknown.py index 9e5f3f31..4f2da723 100644 --- a/fedireads/wellknown.py +++ b/fedireads/wellknown.py @@ -53,23 +53,23 @@ def nodeinfo(request): status_count = models.Status.objects.filter(user__local=True).count() user_count = models.User.objects.count() return JsonResponse({ - "version": "2.0", - "software": { - "name": "fedireads", - "version": "0.0.1" + 'version': '2.0', + 'software': { + 'name': 'fedireads', + 'version': '0.0.1' }, - "protocols": [ - "activitypub" + 'protocols': [ + 'activitypub' ], - "usage": { - "users": { - "total": user_count, - "activeMonth": user_count, # TODO - "activeHalfyear": user_count, # TODO + 'usage': { + 'users': { + 'total': user_count, + 'activeMonth': user_count, # TODO + 'activeHalfyear': user_count, # TODO }, - "localPosts": status_count, + 'localPosts': status_count, }, - "openRegistrations": True, + 'openRegistrations': True, })