Merge branch 'main' into production

.github/workflows/mypy.yml

@@ -0,0 +1,50 @@
+name: Mypy
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
+    - name: Set up Python 3.9
+      uses: actions/setup-python@v4
+      with:
+        python-version: 3.9
+    - name: Install Dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements.txt
+    - name: Analysing the code with mypy
+      env:
+        SECRET_KEY: beepbeep
+        DEBUG: false
+        USE_HTTPS: true
+        DOMAIN: your.domain.here
+        BOOKWYRM_DATABASE_BACKEND: postgres
+        MEDIA_ROOT: images/
+        POSTGRES_PASSWORD: hunter2
+        POSTGRES_USER: postgres
+        POSTGRES_DB: github_actions
+        POSTGRES_HOST: 127.0.0.1
+        CELERY_BROKER: ""
+        REDIS_BROKER_PORT: 6379
+        REDIS_BROKER_PASSWORD: beep
+        USE_DUMMY_CACHE: true
+        FLOWER_PORT: 8888
+        EMAIL_HOST: "smtp.mailgun.org"
+        EMAIL_PORT: 587
+        EMAIL_HOST_USER: ""
+        EMAIL_HOST_PASSWORD: ""
+        EMAIL_USE_TLS: true
+        ENABLE_PREVIEW_IMAGES: false
+        ENABLE_THUMBNAIL_GENERATION: true
+        HTTP_X_FORWARDED_PROTO: false
+      run: |
+        mypy bookwyrm celerywyrm

bookwyrm/activitypub/base_activity.py

@@ -2,6 +2,8 @@
 from dataclasses import dataclass, fields, MISSING
 from json import JSONEncoder
 import logging
+from typing import Optional, Union, TypeVar, overload, Any
+
 import requests
 
 from django.apps import apps
@@ -10,12 +12,15 @@
 
 from bookwyrm import models
 from bookwyrm.connectors import ConnectorException, get_data
+from bookwyrm.models import base_model
 from bookwyrm.signatures import make_signature
 from bookwyrm.settings import DOMAIN, INSTANCE_ACTOR_USERNAME
 from bookwyrm.tasks import app, MISC
 
 logger = logging.getLogger(__name__)
 
+TBookWyrmModel = TypeVar("TBookWyrmModel", bound=base_model.BookWyrmModel)
+
 
 class ActivitySerializerError(ValueError):
     """routine problems serializing activitypub json"""
@@ -65,7 +70,11 @@ class ActivityObject:
     id: str
     type: str
 
-    def __init__(self, activity_objects=None, **kwargs):
+    def __init__(
+        self,
+        activity_objects: Optional[list[str, base_model.BookWyrmModel]] = None,
+        **kwargs: dict[str, Any],
+    ):
         """this lets you pass in an object with fields that aren't in the
         dataclass, which it ignores. Any field in the dataclass is required or
         has a default value"""
@@ -101,13 +110,13 @@ def __init__(self, activity_objects=None, **kwargs):
     # pylint: disable=too-many-locals,too-many-branches,too-many-arguments
     def to_model(
         self,
-        model=None,
-        instance=None,
-        allow_create=True,
-        save=True,
-        overwrite=True,
-        allow_external_connections=True,
-    ):
+        model: Optional[type[TBookWyrmModel]] = None,
+        instance: Optional[TBookWyrmModel] = None,
+        allow_create: bool = True,
+        save: bool = True,
+        overwrite: bool = True,
+        allow_external_connections: bool = True,
+    ) -> Optional[TBookWyrmModel]:
         """convert from an activity to a model instance. Args:
         model: the django model that this object is being converted to
             (will guess if not known)
@@ -296,14 +305,40 @@ def get_model_from_type(activity_type):
 
 
 # pylint: disable=too-many-arguments
+@overload
 def resolve_remote_id(
-    remote_id,
-    model=None,
-    refresh=False,
-    save=True,
-    get_activity=False,
-    allow_external_connections=True,
-):
+    remote_id: str,
+    model: type[TBookWyrmModel],
+    refresh: bool = False,
+    save: bool = True,
+    get_activity: bool = False,
+    allow_external_connections: bool = True,
+) -> TBookWyrmModel:
+    ...
+
+
+# pylint: disable=too-many-arguments
+@overload
+def resolve_remote_id(
+    remote_id: str,
+    model: Optional[str] = None,
+    refresh: bool = False,
+    save: bool = True,
+    get_activity: bool = False,
+    allow_external_connections: bool = True,
+) -> base_model.BookWyrmModel:
+    ...
+
+
+# pylint: disable=too-many-arguments
+def resolve_remote_id(
+    remote_id: str,
+    model: Optional[Union[str, type[base_model.BookWyrmModel]]] = None,
+    refresh: bool = False,
+    save: bool = True,
+    get_activity: bool = False,
+    allow_external_connections: bool = True,
+) -> base_model.BookWyrmModel:
     """take a remote_id and return an instance, creating if necessary. Args:
     remote_id: the unique url for looking up the object in the db or by http
     model: a string or object representing the model that corresponds to the object

bookwyrm/activitypub/book.py

@@ -1,6 +1,6 @@
 """ book and author data """
 from dataclasses import dataclass, field
-from typing import List
+from typing import Optional
 
 from .base_activity import ActivityObject
 from .image import Document
@@ -11,19 +11,19 @@
 class BookData(ActivityObject):
     """shared fields for all book data and authors"""
 
-    openlibraryKey: str = None
-    inventaireId: str = None
-    librarythingKey: str = None
-    goodreadsKey: str = None
-    bnfId: str = None
-    viaf: str = None
-    wikidata: str = None
-    asin: str = None
-    aasin: str = None
-    isfdb: str = None
-    lastEditedBy: str = None
-    links: List[str] = field(default_factory=lambda: [])
-    fileLinks: List[str] = field(default_factory=lambda: [])
+    openlibraryKey: Optional[str] = None
+    inventaireId: Optional[str] = None
+    librarythingKey: Optional[str] = None
+    goodreadsKey: Optional[str] = None
+    bnfId: Optional[str] = None
+    viaf: Optional[str] = None
+    wikidata: Optional[str] = None
+    asin: Optional[str] = None
+    aasin: Optional[str] = None
+    isfdb: Optional[str] = None
+    lastEditedBy: Optional[str] = None
+    links: list[str] = field(default_factory=list)
+    fileLinks: list[str] = field(default_factory=list)
 
 
 # pylint: disable=invalid-name
@@ -35,17 +35,17 @@ class Book(BookData):
     sortTitle: str = None
     subtitle: str = None
     description: str = ""
-    languages: List[str] = field(default_factory=lambda: [])
+    languages: list[str] = field(default_factory=list)
     series: str = ""
     seriesNumber: str = ""
-    subjects: List[str] = field(default_factory=lambda: [])
-    subjectPlaces: List[str] = field(default_factory=lambda: [])
+    subjects: list[str] = field(default_factory=list)
+    subjectPlaces: list[str] = field(default_factory=list)
 
-    authors: List[str] = field(default_factory=lambda: [])
+    authors: list[str] = field(default_factory=list)
     firstPublishedDate: str = ""
     publishedDate: str = ""
 
-    cover: Document = None
+    cover: Optional[Document] = None
     type: str = "Book"
 
 
@@ -58,10 +58,10 @@ class Edition(Book):
     isbn10: str = ""
     isbn13: str = ""
     oclcNumber: str = ""
-    pages: int = None
+    pages: Optional[int] = None
     physicalFormat: str = ""
     physicalFormatDetail: str = ""
-    publishers: List[str] = field(default_factory=lambda: [])
+    publishers: list[str] = field(default_factory=list)
     editionRank: int = 0
 
     type: str = "Edition"
@@ -73,7 +73,7 @@ class Work(Book):
     """work instance of a book object"""
 
     lccn: str = ""
-    editions: List[str] = field(default_factory=lambda: [])
+    editions: list[str] = field(default_factory=list)
     type: str = "Work"
 
 
@@ -83,12 +83,12 @@ class Author(BookData):
     """author of a book"""
 
     name: str
-    isni: str = None
-    viafId: str = None
-    gutenbergId: str = None
-    born: str = None
-    died: str = None
-    aliases: List[str] = field(default_factory=lambda: [])
+    isni: Optional[str] = None
+    viafId: Optional[str] = None
+    gutenbergId: Optional[str] = None
+    born: Optional[str] = None
+    died: Optional[str] = None
+    aliases: list[str] = field(default_factory=list)
     bio: str = ""
     wikipediaLink: str = ""
     type: str = "Author"

bookwyrm/activitystreams.py

@@ -329,10 +329,9 @@ def add_status_on_create(sender, instance, created, *args, **kwargs):
         remove_status_task.delay(instance.id)
         return
 
-    # To avoid creating a zillion unnecessary tasks caused by re-saving the model,
-    # check if it's actually ready to send before we go. We're trusting this was
-    # set correctly by the inbox or view
-    if not instance.ready:
+    # We don't want to create multiple add_status_tasks for each status, and because
+    # the transactions are atomic, on_commit won't run until the status is ready to add.
+    if not created:
         return
 
     # when creating new things, gotta wait on the transaction
@@ -343,6 +342,10 @@ def add_status_on_create(sender, instance, created, *args, **kwargs):
 
 def add_status_on_create_command(sender, instance, created):
     """runs this code only after the database commit completes"""
+    # boosts trigger 'saves" twice, so don't bother duplicating the task
+    if sender == models.Boost and not created:
+        return
+
     priority = STREAMS
     # check if this is an old status, de-prioritize if so
     # (this will happen if federation is very slow, or, more expectedly, on csv import)

bookwyrm/book_search.py

@@ -1,22 +1,53 @@
 """ using a bookwyrm instance as a source of book data """
+from __future__ import annotations
 from dataclasses import asdict, dataclass
 from functools import reduce
 import operator
+from typing import Optional, Union, Any, Literal, overload
 
 from django.contrib.postgres.search import SearchRank, SearchQuery
 from django.db.models import F, Q
+from django.db.models.query import QuerySet
 
 from bookwyrm import models
 from bookwyrm import connectors
 from bookwyrm.settings import MEDIA_FULL_URL
 
 
+@overload
+def search(
+    query: str,
+    *,
+    min_confidence: float = 0,
+    filters: Optional[list[Any]] = None,
+    return_first: Literal[False],
+) -> QuerySet[models.Edition]:
+    ...
+
+
+@overload
+def search(
+    query: str,
+    *,
+    min_confidence: float = 0,
+    filters: Optional[list[Any]] = None,
+    return_first: Literal[True],
+) -> Optional[models.Edition]:
+    ...
+
+
 # pylint: disable=arguments-differ
-def search(query, min_confidence=0, filters=None, return_first=False):
+def search(
+    query: str,
+    *,
+    min_confidence: float = 0,
+    filters: Optional[list[Any]] = None,
+    return_first: bool = False,
+) -> Union[Optional[models.Edition], QuerySet[models.Edition]]:
     """search your local database"""
     filters = filters or []
     if not query:
-        return []
+        return None if return_first else []
     query = query.strip()
 
     results = None
@@ -66,7 +97,9 @@ def format_search_result(search_result):
     ).json()
 
 
-def search_identifiers(query, *filters, return_first=False):
+def search_identifiers(
+    query, *filters, return_first=False
+) -> Union[Optional[models.Edition], QuerySet[models.Edition]]:
     """tries remote_id, isbn; defined as dedupe fields on the model"""
     if connectors.maybe_isbn(query):
         # Oh did you think the 'S' in ISBN stood for 'standard'?
@@ -87,7 +120,9 @@ def search_identifiers(query, *filters, return_first=False):
     return results
 
 
-def search_title_author(query, min_confidence, *filters, return_first=False):
+def search_title_author(
+    query, min_confidence, *filters, return_first=False
+) -> QuerySet[models.Edition]:
     """searches for title and author"""
     query = SearchQuery(query, config="simple") | SearchQuery(query, config="english")
     results = (
@@ -122,11 +157,11 @@ class SearchResult:
     title: str
     key: str
     connector: object
-    view_link: str = None
-    author: str = None
-    year: str = None
-    cover: str = None
-    confidence: int = 1
+    view_link: Optional[str] = None
+    author: Optional[str] = None
+    year: Optional[str] = None
+    cover: Optional[str] = None
+    confidence: float = 1.0
 
     def __repr__(self):
         # pylint: disable=consider-using-f-string