code style cleanup

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',

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')))
 

fedireads/forms.py

@@ -101,7 +101,7 @@ class EditionForm(ModelForm):
             'updated_date',
             'last_sync_date',
 
-            'authors',
+            'authors',# TODO
             'parent_work',
             'shelves',
             'misc_identifiers',

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 = []

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'])
 
 

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)

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):

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,
     })