This is an enjoyable book with great characters.
", + "to": ["https://example.net/user/library_lurker/followers"], + "cc": [], + "replies": { + "id": "https://example.net/user/library_lurker/review/2/replies", + "type": "OrderedCollection", + "totalItems": 0, + "first": "https://example.net/user/library_lurker/review/2/replies?page=1", + "last": "https://example.net/user/library_lurker/review/2/replies?page=1", + "@context": "https://www.w3.org/ns/activitystreams" + }, + "summary": "Spoilers ahead!", + "tag": [], + "attachment": [], + "sensitive": true, + "inReplyToBook": "https://example.net/book/1", + "name": "What a cracking read", + "rating": 4.5, + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +##### Comment + +A `Comment` on a book mentions a book and has a message body, reading status, and progress indicator. + +Example: + +```json +{ + "id": "https://example.net/user/library_lurker/comment/9", + "type": "Comment", + "published": "2023-06-30T21:43:46.013132+00:00", + "attributedTo": "https://example.net/user/library_lurker", + "content": "This is a very enjoyable book so far.
", + "to": ["https://example.net/user/library_lurker/followers"], + "cc": [], + "replies": { + "id": "https://example.net/user/library_lurker/comment/9/replies", + "type": "OrderedCollection", + "totalItems": 0, + "first": "https://example.net/user/library_lurker/comment/9/replies?page=1", + "last": "https://example.net/user/library_lurker/comment/9/replies?page=1", + "@context": "https://www.w3.org/ns/activitystreams" + }, + "summary": "Spoilers ahead!", + "tag": [], + "attachment": [], + "sensitive": true, + "inReplyToBook": "https://example.net/book/1", + "readingStatus": "reading", + "progress": 25, + "progressMode": "PG", + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +##### Quotation + +A quotation (aka "quote") has a message body, an excerpt from a book including position as a page number or percentage indicator, and mentions a book. + +Example: + +```json +{ + "id": "https://example.net/user/mouse/quotation/13", + "url": "https://example.net/user/mouse/quotation/13", + "inReplyTo": null, + "published": "2020-05-10T02:38:31.150343+00:00", + "attributedTo": "https://example.net/user/mouse", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.net/user/mouse/followers" + ], + "sensitive": false, + "content": "I really like this quote", + "type": "Quotation", + "replies": { + "id": "https://example.net/user/mouse/quotation/13/replies", + "type": "Collection", + "first": { + "type": "CollectionPage", + "next": "https://example.net/user/mouse/quotation/13/replies?only_other_accounts=true&page=true", + "partOf": "https://example.net/user/mouse/quotation/13/replies", + "items": [] + } + }, + "inReplyToBook": "https://example.net/book/1", + "quote": "To be or not to be, that is the question.", + "position": 50, + "positionMode": "PCT", + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +### Custom Objects + +##### Work +A particular book, a "work" in the [FRBR](https://en.wikipedia.org/wiki/Functional_Requirements_for_Bibliographic_Records) sense. + +Example: + +```json +{ + "id": "https://bookwyrm.social/book/5988", + "type": "Work", + "authors": [ + "https://bookwyrm.social/author/417" + ], + "first_published_date": null, + "published_date": null, + "title": "Piranesi", + "sort_title": null, + "subtitle": null, + "description": "**From the *New York Times* bestselling author of *Jonathan Strange & Mr. Norrell*, an intoxicating, hypnotic new novel set in a dreamlike alternative reality.", + "languages": [], + "series": null, + "series_number": null, + "subjects": [ + "English literature" + ], + "subject_places": [], + "openlibrary_key": "OL20893680W", + "librarything_key": null, + "goodreads_key": null, + "attachment": [ + { + "url": "https://bookwyrm.social/images/covers/10226290-M.jpg", + "type": "Image" + } + ], + "lccn": null, + "editions": [ + "https://bookwyrm.social/book/5989" + ], + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +##### Edition +A particular _manifestation_ of a Work, in the [FRBR](https://en.wikipedia.org/wiki/Functional_Requirements_for_Bibliographic_Records) sense. + +Example: + +```json +{ + "id": "https://bookwyrm.social/book/5989", + "lastEditedBy": "https://example.net/users/rat", + "type": "Edition", + "authors": [ + "https://bookwyrm.social/author/417" + ], + "first_published_date": null, + "published_date": "2020-09-15T00:00:00+00:00", + "title": "Piranesi", + "sort_title": null, + "subtitle": null, + "description": "Piranesi's house is no ordinary building; its rooms are infinite, its corridors endless, its walls are lined with thousands upon thousands of statues, each one different from all the others.", + "languages": [ + "English" + ], + "series": null, + "series_number": null, + "subjects": [], + "subject_places": [], + "openlibrary_key": "OL29486417M", + "librarything_key": null, + "goodreads_key": null, + "isfdb": null, + "attachment": [ + { + "url": "https://bookwyrm.social/images/covers/50202953._SX318_.jpg", + "type": "Image" + } + ], + "isbn_10": "1526622424", + "isbn_13": "9781526622426", + "oclc_number": null, + "asin": null, + "pages": 272, + "physical_format": null, + "publishers": [ + "Bloomsbury Publishing Plc" + ], + "work": "https://bookwyrm.social/book/5988", + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +#### Shelf + +A user's book collection. By default, every user has a `to-read`, `reading`, `read`, and `stopped-reading` shelf which are used to track reading progress. Users may create an unlimited number of additional shelves with their own ids. + +Example + +```json +{ + "id": "https://example.net/user/avid_reader/books/extraspecialbooks-5", + "type": "Shelf", + "totalItems": 0, + "first": "https://example.net/user/avid_reader/books/extraspecialbooks-5?page=1", + "last": "https://example.net/user/avid_reader/books/extraspecialbooks-5?page=1", + "name": "Extra special books", + "owner": "https://example.net/user/avid_reader", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.net/user/avid_reader/followers" + ], + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +#### List + +A collection of books that may have items contributed by users other than the one who created the list. + +Example: + +```json +{ + "id": "https://example.net/list/1", + "type": "BookList", + "totalItems": 0, + "first": "https://example.net/list/1?page=1", + "last": "https://example.net/list/1?page=1", + "name": "My cool list", + "owner": "https://example.net/user/avid_reader", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.net/user/avid_reader/followers" + ], + "summary": "A list of books I like.", + "curation": "curated", + "@context": "https://www.w3.org/ns/activitystreams" +} +``` + +#### Activities + +- `Create`: Adds a shelf or list to the database. +- `Delete`: Removes a shelf or list. +- `Add`: Adds a book to a shelf or list. +- `Remove`: Removes a book from a shelf or list. + +## Alternative Serialization +Because BookWyrm uses custom object types that aren't listed in [the standard ActivityStreams Vocabulary](https://www.w3.org/TR/activitystreams-vocabulary), some statuses are transformed into standard types when sent to or viewed by non-BookWyrm services. `Review`s are converted into `Article`s, and `Comment`s and `Quotation`s are converted into `Note`s, with a link to the book and the cover image attached. + +In future this may be done with [JSON-LD type arrays](https://www.w3.org/TR/json-ld/#specifying-the-type) instead. + +## Other extensions + +### Webfinger + +Bookwyrm uses the [Webfinger](https://datatracker.ietf.org/doc/html/rfc7033) standard to identify and disambiguate fediverse actors. The [Webfinger documentation on the Mastodon project](https://docs.joinmastodon.org/spec/webfinger/) provides a good overview of how Webfinger is used. + +### HTTP Signatures + +Bookwyrm uses and requires HTTP signatures for all `POST` requests. `GET` requests are not signed by default, but if Bookwyrm receives a `403` response to a `GET` it will re-send the request, signed by the default server user. This usually will have a user id of `https://example.net/user/bookwyrm.instance.actor` + +#### publicKey id + +In older versions of Bookwyrm the `publicKey.id` was incorrectly listed in request headers as `https://example.net/user/username#main-key`. As of v0.6.3 the id is now listed correctly, as `https://example.net/user/username/#main-key`. In most ActivityPub implementations this will make no difference as the URL will usually resolve to the same place. + +### NodeInfo + +Bookwyrm uses the [NodeInfo](http://nodeinfo.diaspora.software/) standard to provide statistics and version information for each instance. + +## Further Documentation + +See [docs.joinbookwyrm.com/](https://docs.joinbookwyrm.com/) for more documentation. \ No newline at end of file diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 9b7897ebaa..615db440a3 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/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, MEDIUM +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) @@ -241,7 +250,7 @@ def serialize(self, **kwargs): return data -@app.task(queue=MEDIUM) +@app.task(queue=MISC) @transaction.atomic def set_related_field( model_name, origin_model_name, related_field_name, related_remote_id, data @@ -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 diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index d3aca4471d..5db0dc3ac1 100644 --- a/bookwyrm/activitypub/book.py +++ b/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" diff --git a/bookwyrm/activitystreams.py b/bookwyrm/activitystreams.py index 5d581d564e..71f9e42d77 100644 --- a/bookwyrm/activitystreams.py +++ b/bookwyrm/activitystreams.py @@ -8,7 +8,7 @@ from bookwyrm import models from bookwyrm.redis_store import RedisStore, r -from bookwyrm.tasks import app, LOW, MEDIUM, HIGH +from bookwyrm.tasks import app, STREAMS, IMPORT_TRIGGERED from bookwyrm.telemetry import open_telemetry @@ -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,7 +342,11 @@ 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""" - priority = HIGH + # 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) if instance.published_date < timezone.now() - timedelta( @@ -353,7 +356,7 @@ def add_status_on_create_command(sender, instance, created): if instance.user.local: return # an out of date remote status is a low priority but should be added - priority = LOW + priority = IMPORT_TRIGGERED add_status_task.apply_async( args=(instance.id,), @@ -497,7 +500,7 @@ def remove_statuses_on_unshelve(sender, instance, *args, **kwargs): # ---- TASKS -@app.task(queue=LOW) +@app.task(queue=STREAMS) def add_book_statuses_task(user_id, book_id): """add statuses related to a book on shelve""" user = models.User.objects.get(id=user_id) @@ -505,7 +508,7 @@ def add_book_statuses_task(user_id, book_id): BooksStream().add_book_statuses(user, book) -@app.task(queue=LOW) +@app.task(queue=STREAMS) def remove_book_statuses_task(user_id, book_id): """remove statuses about a book from a user's books feed""" user = models.User.objects.get(id=user_id) @@ -513,7 +516,7 @@ def remove_book_statuses_task(user_id, book_id): BooksStream().remove_book_statuses(user, book) -@app.task(queue=MEDIUM) +@app.task(queue=STREAMS) def populate_stream_task(stream, user_id): """background task for populating an empty activitystream""" user = models.User.objects.get(id=user_id) @@ -521,7 +524,7 @@ def populate_stream_task(stream, user_id): stream.populate_streams(user) -@app.task(queue=MEDIUM) +@app.task(queue=STREAMS) def remove_status_task(status_ids): """remove a status from any stream it might be in""" # this can take an id or a list of ids @@ -536,7 +539,7 @@ def remove_status_task(status_ids): ) -@app.task(queue=HIGH) +@app.task(queue=STREAMS) def add_status_task(status_id, increment_unread=False): """add a status to any stream it should be in""" status = models.Status.objects.select_subclasses().get(id=status_id) @@ -548,7 +551,7 @@ def add_status_task(status_id, increment_unread=False): stream.add_status(status, increment_unread=increment_unread) -@app.task(queue=MEDIUM) +@app.task(queue=STREAMS) def remove_user_statuses_task(viewer_id, user_id, stream_list=None): """remove all statuses by a user from a viewer's stream""" stream_list = [streams[s] for s in stream_list] if stream_list else streams.values() @@ -558,7 +561,7 @@ def remove_user_statuses_task(viewer_id, user_id, stream_list=None): stream.remove_user_statuses(viewer, user) -@app.task(queue=MEDIUM) +@app.task(queue=STREAMS) def add_user_statuses_task(viewer_id, user_id, stream_list=None): """add all statuses by a user to a viewer's stream""" stream_list = [streams[s] for s in stream_list] if stream_list else streams.values() @@ -568,7 +571,7 @@ def add_user_statuses_task(viewer_id, user_id, stream_list=None): stream.add_user_statuses(viewer, user) -@app.task(queue=MEDIUM) +@app.task(queue=STREAMS) def handle_boost_task(boost_id): """remove the original post and other, earlier boosts""" instance = models.Status.objects.get(id=boost_id) diff --git a/bookwyrm/book_search.py b/bookwyrm/book_search.py index 822c87f016..ceb228f40c 100644 --- a/bookwyrm/book_search.py +++ b/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 diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 950bb11f98..8b6dcb8858 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -1,5 +1,7 @@ """ functionality outline for a book data connector """ +from __future__ import annotations from abc import ABC, abstractmethod +from typing import Optional, TypedDict, Any, Callable, Union, Iterator from urllib.parse import quote_plus import imghdr import logging @@ -16,33 +18,38 @@ from bookwyrm.settings import USER_AGENT from .connector_manager import load_more_data, ConnectorException, raise_not_valid_url from .format_mappings import format_mappings - +from ..book_search import SearchResult logger = logging.getLogger(__name__) +JsonDict = dict[str, Any] + + +class ConnectorResults(TypedDict): + """TypedDict for results returned by connector""" + + connector: AbstractMinimalConnector + results: list[SearchResult] + class AbstractMinimalConnector(ABC): """just the bare bones, for other bookwyrm instances""" - def __init__(self, identifier): + def __init__(self, identifier: str): # load connector settings info = models.Connector.objects.get(identifier=identifier) self.connector = info # the things in the connector model to copy over - self_fields = [ - "base_url", - "books_url", - "covers_url", - "search_url", - "isbn_search_url", - "name", - "identifier", - ] - for field in self_fields: - setattr(self, field, getattr(info, field)) - - def get_search_url(self, query): + self.base_url = info.base_url + self.books_url = info.books_url + self.covers_url = info.covers_url + self.search_url = info.search_url + self.isbn_search_url = info.isbn_search_url + self.name = info.name + self.identifier = info.identifier + + def get_search_url(self, query: str) -> str: """format the query url""" # Check if the query resembles an ISBN if maybe_isbn(query) and self.isbn_search_url and self.isbn_search_url != "": @@ -54,13 +61,21 @@ def get_search_url(self, query): # searched as free text. This, instead, only searches isbn if it's isbn-y return f"{self.search_url}{quote_plus(query)}" - def process_search_response(self, query, data, min_confidence): + def process_search_response( + self, query: str, data: Any, min_confidence: float + ) -> list[SearchResult]: """Format the search results based on the format of the query""" if maybe_isbn(query): return list(self.parse_isbn_search_data(data))[:10] return list(self.parse_search_data(data, min_confidence))[:10] - async def get_results(self, session, url, min_confidence, query): + async def get_results( + self, + session: aiohttp.ClientSession, + url: str, + min_confidence: float, + query: str, + ) -> Optional[ConnectorResults]: """try this specific connector""" # pylint: disable=line-too-long headers = { @@ -74,55 +89,63 @@ async def get_results(self, session, url, min_confidence, query): async with session.get(url, headers=headers, params=params) as response: if not response.ok: logger.info("Unable to connect to %s: %s", url, response.reason) - return + return None try: raw_data = await response.json() except aiohttp.client_exceptions.ContentTypeError as err: logger.exception(err) - return + return None - return { - "connector": self, - "results": self.process_search_response( + return ConnectorResults( + connector=self, + results=self.process_search_response( query, raw_data, min_confidence ), - } + ) except asyncio.TimeoutError: logger.info("Connection timed out for url: %s", url) except aiohttp.ClientError as err: logger.info(err) + return None @abstractmethod - def get_or_create_book(self, remote_id): + def get_or_create_book(self, remote_id: str) -> Optional[models.Book]: """pull up a book record by whatever means possible""" @abstractmethod - def parse_search_data(self, data, min_confidence): + def parse_search_data( + self, data: Any, min_confidence: float + ) -> Iterator[SearchResult]: """turn the result json from a search into a list""" @abstractmethod - def parse_isbn_search_data(self, data): + def parse_isbn_search_data(self, data: Any) -> Iterator[SearchResult]: """turn the result json from a search into a list""" class AbstractConnector(AbstractMinimalConnector): """generic book data connector""" - def __init__(self, identifier): + generated_remote_link_field = "" + + def __init__(self, identifier: str): super().__init__(identifier) # fields we want to look for in book data to copy over # title we handle separately. - self.book_mappings = [] + self.book_mappings: list[Mapping] = [] + self.author_mappings: list[Mapping] = [] - def get_or_create_book(self, remote_id): + def get_or_create_book(self, remote_id: str) -> Optional[models.Book]: """translate arbitrary json into an Activitypub dataclass""" # first, check if we have the origin_id saved existing = models.Edition.find_existing_by_remote_id( remote_id ) or models.Work.find_existing_by_remote_id(remote_id) if existing: - if hasattr(existing, "default_edition"): + if hasattr(existing, "default_edition") and isinstance( + existing.default_edition, models.Edition + ): return existing.default_edition return existing @@ -154,6 +177,9 @@ def get_or_create_book(self, remote_id): ) # this will dedupe automatically work = work_activity.to_model(model=models.Work, overwrite=False) + if not work: + return None + for author in self.get_authors_from_data(work_data): work.authors.add(author) @@ -161,12 +187,21 @@ def get_or_create_book(self, remote_id): load_more_data.delay(self.connector.id, work.id) return edition - def get_book_data(self, remote_id): # pylint: disable=no-self-use + def get_book_data(self, remote_id: str) -> JsonDict: # pylint: disable=no-self-use """this allows connectors to override the default behavior""" return get_data(remote_id) - def create_edition_from_data(self, work, edition_data, instance=None): + def create_edition_from_data( + self, + work: models.Work, + edition_data: Union[str, JsonDict], + instance: Optional[models.Edition] = None, + ) -> Optional[models.Edition]: """if we already have the work, we're ready""" + if isinstance(edition_data, str): + # We don't expect a string here + return None + mapped_data = dict_from_mappings(edition_data, self.book_mappings) mapped_data["work"] = work.remote_id edition_activity = activitypub.Edition(**mapped_data) @@ -174,6 +209,9 @@ def create_edition_from_data(self, work, edition_data, instance=None): model=models.Edition, overwrite=False, instance=instance ) + if not edition: + return None + # if we're updating an existing instance, we don't need to load authors if instance: return edition @@ -190,7 +228,9 @@ def create_edition_from_data(self, work, edition_data, instance=None): return edition - def get_or_create_author(self, remote_id, instance=None): + def get_or_create_author( + self, remote_id: str, instance: Optional[models.Author] = None + ) -> Optional[models.Author]: """load that author""" if not instance: existing = models.Author.find_existing_by_remote_id(remote_id) @@ -210,46 +250,51 @@ def get_or_create_author(self, remote_id, instance=None): model=models.Author, overwrite=False, instance=instance ) - def get_remote_id_from_model(self, obj): + def get_remote_id_from_model(self, obj: models.BookDataModel) -> Optional[str]: """given the data stored, how can we look this up""" - return getattr(obj, getattr(self, "generated_remote_link_field")) + remote_id: Optional[str] = getattr(obj, self.generated_remote_link_field) + return remote_id - def update_author_from_remote(self, obj): + def update_author_from_remote(self, obj: models.Author) -> Optional[models.Author]: """load the remote data from this connector and add it to an existing author""" remote_id = self.get_remote_id_from_model(obj) + if not remote_id: + return None return self.get_or_create_author(remote_id, instance=obj) - def update_book_from_remote(self, obj): + def update_book_from_remote(self, obj: models.Edition) -> Optional[models.Edition]: """load the remote data from this connector and add it to an existing book""" remote_id = self.get_remote_id_from_model(obj) + if not remote_id: + return None data = self.get_book_data(remote_id) return self.create_edition_from_data(obj.parent_work, data, instance=obj) @abstractmethod - def is_work_data(self, data): + def is_work_data(self, data: JsonDict) -> bool: """differentiate works and editions""" @abstractmethod - def get_edition_from_work_data(self, data): + def get_edition_from_work_data(self, data: JsonDict) -> JsonDict: """every work needs at least one edition""" @abstractmethod - def get_work_from_edition_data(self, data): + def get_work_from_edition_data(self, data: JsonDict) -> JsonDict: """every edition needs a work""" @abstractmethod - def get_authors_from_data(self, data): + def get_authors_from_data(self, data: JsonDict) -> Iterator[models.Author]: """load author data""" @abstractmethod - def expand_book_data(self, book): + def expand_book_data(self, book: models.Book) -> None: """get more info on a book""" -def dict_from_mappings(data, mappings): +def dict_from_mappings(data: JsonDict, mappings: list[Mapping]) -> JsonDict: """create a dict in Activitypub format, using mappings supplies by the subclass""" - result = {} + result: JsonDict = {} for mapping in mappings: # sometimes there are multiple mappings for one field, don't # overwrite earlier writes in that case @@ -259,7 +304,11 @@ def dict_from_mappings(data, mappings): return result -def get_data(url, params=None, timeout=settings.QUERY_TIMEOUT): +def get_data( + url: str, + params: Optional[dict[str, str]] = None, + timeout: int = settings.QUERY_TIMEOUT, +) -> JsonDict: """wrapper for request.get""" # check if the url is blocked raise_not_valid_url(url) @@ -292,10 +341,15 @@ def get_data(url, params=None, timeout=settings.QUERY_TIMEOUT): logger.info(err) raise ConnectorException(err) + if not isinstance(data, dict): + raise ConnectorException("Unexpected data format") + return data -def get_image(url, timeout=10): +def get_image( + url: str, timeout: int = 10 +) -> Union[tuple[ContentFile[bytes], str], tuple[None, None]]: """wrapper for requesting an image""" raise_not_valid_url(url) try: @@ -325,14 +379,19 @@ def get_image(url, timeout=10): class Mapping: """associate a local database field with a field in an external dataset""" - def __init__(self, local_field, remote_field=None, formatter=None): + def __init__( + self, + local_field: str, + remote_field: Optional[str] = None, + formatter: Optional[Callable[[Any], Any]] = None, + ): noop = lambda x: x self.local_field = local_field self.remote_field = remote_field or local_field self.formatter = formatter or noop - def get_value(self, data): + def get_value(self, data: JsonDict) -> Optional[Any]: """pull a field from incoming json and return the formatted version""" value = data.get(self.remote_field) if not value: @@ -343,7 +402,7 @@ def get_value(self, data): return None -def infer_physical_format(format_text): +def infer_physical_format(format_text: str) -> Optional[str]: """try to figure out what the standardized format is from the free value""" format_text = format_text.lower() if format_text in format_mappings: @@ -356,7 +415,7 @@ def infer_physical_format(format_text): return matches[0] -def unique_physical_format(format_text): +def unique_physical_format(format_text: str) -> Optional[str]: """only store the format if it isn't directly in the format mappings""" format_text = format_text.lower() if format_text in format_mappings: @@ -365,7 +424,7 @@ def unique_physical_format(format_text): return format_text -def maybe_isbn(query): +def maybe_isbn(query: str) -> bool: """check if a query looks like an isbn""" isbn = re.sub(r"[\W_]", "", query) # removes filler characters # ISBNs must be numeric except an ISBN10 checkdigit can be 'X' diff --git a/bookwyrm/connectors/bookwyrm_connector.py b/bookwyrm/connectors/bookwyrm_connector.py index e07a0b281a..4064f4b4c5 100644 --- a/bookwyrm/connectors/bookwyrm_connector.py +++ b/bookwyrm/connectors/bookwyrm_connector.py @@ -1,4 +1,7 @@ """ using another bookwyrm instance as a source of book data """ +from __future__ import annotations +from typing import Any, Iterator + from bookwyrm import activitypub, models from bookwyrm.book_search import SearchResult from .abstract_connector import AbstractMinimalConnector @@ -7,15 +10,19 @@ class Connector(AbstractMinimalConnector): """this is basically just for search""" - def get_or_create_book(self, remote_id): + def get_or_create_book(self, remote_id: str) -> models.Edition: return activitypub.resolve_remote_id(remote_id, model=models.Edition) - def parse_search_data(self, data, min_confidence): + def parse_search_data( + self, data: list[dict[str, Any]], min_confidence: float + ) -> Iterator[SearchResult]: for search_result in data: search_result["connector"] = self yield SearchResult(**search_result) - def parse_isbn_search_data(self, data): + def parse_isbn_search_data( + self, data: list[dict[str, Any]] + ) -> Iterator[SearchResult]: for search_result in data: search_result["connector"] = self yield SearchResult(**search_result) diff --git a/bookwyrm/connectors/connector_manager.py b/bookwyrm/connectors/connector_manager.py index 7e823c0afa..444a626ba6 100644 --- a/bookwyrm/connectors/connector_manager.py +++ b/bookwyrm/connectors/connector_manager.py @@ -1,8 +1,11 @@ """ interface with whatever connectors the app has """ +from __future__ import annotations import asyncio import importlib import ipaddress import logging +from asyncio import Future +from typing import Iterator, Any, Optional, Union, overload, Literal from urllib.parse import urlparse import aiohttp @@ -12,8 +15,10 @@ from requests import HTTPError from bookwyrm import book_search, models +from bookwyrm.book_search import SearchResult +from bookwyrm.connectors import abstract_connector from bookwyrm.settings import SEARCH_TIMEOUT -from bookwyrm.tasks import app, LOW +from bookwyrm.tasks import app, CONNECTORS logger = logging.getLogger(__name__) @@ -22,11 +27,15 @@ class ConnectorException(HTTPError): """when the connector can't do what was asked""" -async def async_connector_search(query, items, min_confidence): +async def async_connector_search( + query: str, + items: list[tuple[str, abstract_connector.AbstractConnector]], + min_confidence: float, +) -> list[Optional[abstract_connector.ConnectorResults]]: """Try a number of requests simultaneously""" timeout = aiohttp.ClientTimeout(total=SEARCH_TIMEOUT) async with aiohttp.ClientSession(timeout=timeout) as session: - tasks = [] + tasks: list[Future[Optional[abstract_connector.ConnectorResults]]] = [] for url, connector in items: tasks.append( asyncio.ensure_future( @@ -35,14 +44,29 @@ async def async_connector_search(query, items, min_confidence): ) results = await asyncio.gather(*tasks) - return results + return list(results) -def search(query, min_confidence=0.1, return_first=False): +@overload +def search( + query: str, *, min_confidence: float = 0.1, return_first: Literal[False] +) -> list[abstract_connector.ConnectorResults]: + ... + + +@overload +def search( + query: str, *, min_confidence: float = 0.1, return_first: Literal[True] +) -> Optional[SearchResult]: + ... + + +def search( + query: str, *, min_confidence: float = 0.1, return_first: bool = False +) -> Union[list[abstract_connector.ConnectorResults], Optional[SearchResult]]: """find books based on arbitrary keywords""" if not query: - return [] - results = [] + return None if return_first else [] items = [] for connector in get_connectors(): @@ -57,8 +81,12 @@ def search(query, min_confidence=0.1, return_first=False): items.append((url, connector)) # load as many results as we can - results = asyncio.run(async_connector_search(query, items, min_confidence)) - results = [r for r in results if r] + # failed requests will return None, so filter those out + results = [ + r + for r in asyncio.run(async_connector_search(query, items, min_confidence)) + if r + ] if return_first: # find the best result from all the responses and return that @@ -66,11 +94,12 @@ def search(query, min_confidence=0.1, return_first=False): all_results = sorted(all_results, key=lambda r: r.confidence, reverse=True) return all_results[0] if all_results else None - # failed requests will return None, so filter those out return results -def first_search_result(query, min_confidence=0.1): +def first_search_result( + query: str, min_confidence: float = 0.1 +) -> Union[models.Edition, SearchResult, None]: """search until you find a result that fits""" # try local search first result = book_search.search(query, min_confidence=min_confidence, return_first=True) @@ -80,13 +109,13 @@ def first_search_result(query, min_confidence=0.1): return search(query, min_confidence=min_confidence, return_first=True) or None -def get_connectors(): +def get_connectors() -> Iterator[abstract_connector.AbstractConnector]: """load all connectors""" for info in models.Connector.objects.filter(active=True).order_by("priority").all(): yield load_connector(info) -def get_or_create_connector(remote_id): +def get_or_create_connector(remote_id: str) -> abstract_connector.AbstractConnector: """get the connector related to the object's server""" url = urlparse(remote_id) identifier = url.netloc @@ -109,8 +138,8 @@ def get_or_create_connector(remote_id): return load_connector(connector_info) -@app.task(queue=LOW) -def load_more_data(connector_id, book_id): +@app.task(queue=CONNECTORS) +def load_more_data(connector_id: str, book_id: str) -> None: """background the work of getting all 10,000 editions of LoTR""" connector_info = models.Connector.objects.get(id=connector_id) connector = load_connector(connector_info) @@ -118,8 +147,10 @@ def load_more_data(connector_id, book_id): connector.expand_book_data(book) -@app.task(queue=LOW) -def create_edition_task(connector_id, work_id, data): +@app.task(queue=CONNECTORS) +def create_edition_task( + connector_id: int, work_id: int, data: Union[str, abstract_connector.JsonDict] +) -> None: """separate task for each of the 10,000 editions of LoTR""" connector_info = models.Connector.objects.get(id=connector_id) connector = load_connector(connector_info) @@ -127,23 +158,31 @@ def create_edition_task(connector_id, work_id, data): connector.create_edition_from_data(work, data) -def load_connector(connector_info): +def load_connector( + connector_info: models.Connector, +) -> abstract_connector.AbstractConnector: """instantiate the connector class""" connector = importlib.import_module( f"bookwyrm.connectors.{connector_info.connector_file}" ) - return connector.Connector(connector_info.identifier) + return connector.Connector(connector_info.identifier) # type: ignore[no-any-return] @receiver(signals.post_save, sender="bookwyrm.FederatedServer") # pylint: disable=unused-argument -def create_connector(sender, instance, created, *args, **kwargs): +def create_connector( + sender: Any, + instance: models.FederatedServer, + created: Any, + *args: Any, + **kwargs: Any, +) -> None: """create a connector to an external bookwyrm server""" if instance.application_type == "bookwyrm": get_or_create_connector(f"https://{instance.server_name}") -def raise_not_valid_url(url): +def raise_not_valid_url(url: str) -> None: """do some basic reality checks on the url""" parsed = urlparse(url) if not parsed.scheme in ["http", "https"]: diff --git a/bookwyrm/connectors/inventaire.py b/bookwyrm/connectors/inventaire.py index f3e24c0ec5..c08bcdee14 100644 --- a/bookwyrm/connectors/inventaire.py +++ b/bookwyrm/connectors/inventaire.py @@ -1,9 +1,10 @@ """ inventaire data connector """ import re +from typing import Any, Union, Optional, Iterator, Iterable from bookwyrm import models from bookwyrm.book_search import SearchResult -from .abstract_connector import AbstractConnector, Mapping +from .abstract_connector import AbstractConnector, Mapping, JsonDict from .abstract_connector import get_data from .connector_manager import ConnectorException, create_edition_task @@ -13,7 +14,7 @@ class Connector(AbstractConnector): generated_remote_link_field = "inventaire_id" - def __init__(self, identifier): + def __init__(self, identifier: str): super().__init__(identifier) get_first = lambda a: a[0] @@ -60,13 +61,13 @@ def __init__(self, identifier): Mapping("died", remote_field="wdt:P570", formatter=get_first), ] + shared_mappings - def get_remote_id(self, value): + def get_remote_id(self, value: str) -> str: """convert an id/uri into a url""" return f"{self.books_url}?action=by-uris&uris={value}" - def get_book_data(self, remote_id): + def get_book_data(self, remote_id: str) -> JsonDict: data = get_data(remote_id) - extracted = list(data.get("entities").values()) + extracted = list(data.get("entities", {}).values()) try: data = extracted[0] except (KeyError, IndexError): @@ -74,10 +75,16 @@ def get_book_data(self, remote_id): # flatten the data so that images, uri, and claims are on the same level return { **data.get("claims", {}), - **{k: data.get(k) for k in ["uri", "image", "labels", "sitelinks", "type"]}, + **{ + k: data.get(k) + for k in ["uri", "image", "labels", "sitelinks", "type"] + if k in data + }, } - def parse_search_data(self, data, min_confidence): + def parse_search_data( + self, data: JsonDict, min_confidence: float + ) -> Iterator[SearchResult]: for search_result in data.get("results", []): images = search_result.get("image") cover = f"{self.covers_url}/img/entities/{images[0]}" if images else None @@ -96,7 +103,7 @@ def parse_search_data(self, data, min_confidence): connector=self, ) - def parse_isbn_search_data(self, data): + def parse_isbn_search_data(self, data: JsonDict) -> Iterator[SearchResult]: """got some data""" results = data.get("entities") if not results: @@ -114,35 +121,44 @@ def parse_isbn_search_data(self, data): connector=self, ) - def is_work_data(self, data): + def is_work_data(self, data: JsonDict) -> bool: return data.get("type") == "work" - def load_edition_data(self, work_uri): + def load_edition_data(self, work_uri: str) -> JsonDict: """get a list of editions for a work""" # pylint: disable=line-too-long url = f"{self.books_url}?action=reverse-claims&property=wdt:P629&value={work_uri}&sort=true" return get_data(url) - def get_edition_from_work_data(self, data): - data = self.load_edition_data(data.get("uri")) + def get_edition_from_work_data(self, data: JsonDict) -> JsonDict: + work_uri = data.get("uri") + if not work_uri: + raise ConnectorException("Invalid URI") + data = self.load_edition_data(work_uri) try: uri = data.get("uris", [])[0] except IndexError: raise ConnectorException("Invalid book data") return self.get_book_data(self.get_remote_id(uri)) - def get_work_from_edition_data(self, data): - uri = data.get("wdt:P629", [None])[0] + def get_work_from_edition_data(self, data: JsonDict) -> JsonDict: + try: + uri = data.get("wdt:P629", [])[0] + except IndexError: + raise ConnectorException("Invalid book data") + if not uri: raise ConnectorException("Invalid book data") return self.get_book_data(self.get_remote_id(uri)) - def get_authors_from_data(self, data): + def get_authors_from_data(self, data: JsonDict) -> Iterator[models.Author]: authors = data.get("wdt:P50", []) for author in authors: - yield self.get_or_create_author(self.get_remote_id(author)) + model = self.get_or_create_author(self.get_remote_id(author)) + if model: + yield model - def expand_book_data(self, book): + def expand_book_data(self, book: models.Book) -> None: work = book # go from the edition to the work, if necessary if isinstance(book, models.Edition): @@ -154,11 +170,16 @@ def expand_book_data(self, book): # who knows, man return - for edition_uri in edition_options.get("uris"): + for edition_uri in edition_options.get("uris", []): remote_id = self.get_remote_id(edition_uri) create_edition_task.delay(self.connector.id, work.id, remote_id) - def create_edition_from_data(self, work, edition_data, instance=None): + def create_edition_from_data( + self, + work: models.Work, + edition_data: Union[str, JsonDict], + instance: Optional[models.Edition] = None, + ) -> Optional[models.Edition]: """pass in the url as data and then call the version in abstract connector""" if isinstance(edition_data, str): try: @@ -168,22 +189,26 @@ def create_edition_from_data(self, work, edition_data, instance=None): return None return super().create_edition_from_data(work, edition_data, instance=instance) - def get_cover_url(self, cover_blob, *_): + def get_cover_url( + self, cover_blob: Union[list[JsonDict], JsonDict], *_: Any + ) -> Optional[str]: """format the relative cover url into an absolute one: {"url": "/img/entities/e794783f01b9d4f897a1ea9820b96e00d346994f"} """ # covers may or may not be a list - if isinstance(cover_blob, list) and len(cover_blob) > 0: + if isinstance(cover_blob, list): + if len(cover_blob) == 0: + return None cover_blob = cover_blob[0] cover_id = cover_blob.get("url") - if not cover_id: + if not isinstance(cover_id, str): return None # cover may or may not be an absolute url already if re.match(r"^http", cover_id): return cover_id return f"{self.covers_url}{cover_id}" - def resolve_keys(self, keys): + def resolve_keys(self, keys: Iterable[str]) -> list[str]: """cool, it's "wd:Q3156592" now what the heck does that mean""" results = [] for uri in keys: @@ -191,10 +216,10 @@ def resolve_keys(self, keys): data = self.get_book_data(self.get_remote_id(uri)) except ConnectorException: continue - results.append(get_language_code(data.get("labels"))) + results.append(get_language_code(data.get("labels", {}))) return results - def get_description(self, links): + def get_description(self, links: JsonDict) -> str: """grab an extracted excerpt from wikipedia""" link = links.get("enwiki") if not link: @@ -204,15 +229,15 @@ def get_description(self, links): data = get_data(url) except ConnectorException: return "" - return data.get("extract") + return data.get("extract", "") - def get_remote_id_from_model(self, obj): + def get_remote_id_from_model(self, obj: models.BookDataModel) -> str: """use get_remote_id to figure out the link from a model obj""" remote_id_value = obj.inventaire_id return self.get_remote_id(remote_id_value) -def get_language_code(options, code="en"): +def get_language_code(options: JsonDict, code: str = "en") -> Any: """when there are a bunch of translation but we need a single field""" result = options.get(code) if result: diff --git a/bookwyrm/connectors/openlibrary.py b/bookwyrm/connectors/openlibrary.py index 0fd786660c..4dc6d6ac14 100644 --- a/bookwyrm/connectors/openlibrary.py +++ b/bookwyrm/connectors/openlibrary.py @@ -1,9 +1,13 @@ """ openlibrary data connector """ import re +from typing import Any, Optional, Union, Iterator, Iterable + +from markdown import markdown from bookwyrm import models from bookwyrm.book_search import SearchResult -from .abstract_connector import AbstractConnector, Mapping +from bookwyrm.utils.sanitizer import clean +from .abstract_connector import AbstractConnector, Mapping, JsonDict from .abstract_connector import get_data, infer_physical_format, unique_physical_format from .connector_manager import ConnectorException, create_edition_task from .openlibrary_languages import languages @@ -14,7 +18,7 @@ class Connector(AbstractConnector): generated_remote_link_field = "openlibrary_link" - def __init__(self, identifier): + def __init__(self, identifier: str): super().__init__(identifier) get_first = lambda a, *args: a[0] @@ -94,14 +98,14 @@ def __init__(self, identifier): Mapping("inventaire_id", remote_field="links", formatter=get_inventaire_id), ] - def get_book_data(self, remote_id): + def get_book_data(self, remote_id: str) -> JsonDict: data = get_data(remote_id) if data.get("type", {}).get("key") == "/type/redirect": - remote_id = self.base_url + data.get("location") + remote_id = self.base_url + data.get("location", "") return get_data(remote_id) return data - def get_remote_id_from_data(self, data): + def get_remote_id_from_data(self, data: JsonDict) -> str: """format a url from an openlibrary id field""" try: key = data["key"] @@ -109,10 +113,10 @@ def get_remote_id_from_data(self, data): raise ConnectorException("Invalid book data") return f"{self.books_url}{key}" - def is_work_data(self, data): + def is_work_data(self, data: JsonDict) -> bool: return bool(re.match(r"^[\/\w]+OL\d+W$", data["key"])) - def get_edition_from_work_data(self, data): + def get_edition_from_work_data(self, data: JsonDict) -> JsonDict: try: key = data["key"] except KeyError: @@ -124,7 +128,7 @@ def get_edition_from_work_data(self, data): raise ConnectorException("No editions for work") return edition - def get_work_from_edition_data(self, data): + def get_work_from_edition_data(self, data: JsonDict) -> JsonDict: try: key = data["works"][0]["key"] except (IndexError, KeyError): @@ -132,7 +136,7 @@ def get_work_from_edition_data(self, data): url = f"{self.books_url}{key}" return self.get_book_data(url) - def get_authors_from_data(self, data): + def get_authors_from_data(self, data: JsonDict) -> Iterator[models.Author]: """parse author json and load or create authors""" for author_blob in data.get("authors", []): author_blob = author_blob.get("author", author_blob) @@ -144,7 +148,7 @@ def get_authors_from_data(self, data): continue yield author - def get_cover_url(self, cover_blob, size="L"): + def get_cover_url(self, cover_blob: list[str], size: str = "L") -> Optional[str]: """ask openlibrary for the cover""" if not cover_blob: return None @@ -152,8 +156,10 @@ def get_cover_url(self, cover_blob, size="L"): image_name = f"{cover_id}-{size}.jpg" return f"{self.covers_url}/b/id/{image_name}" - def parse_search_data(self, data, min_confidence): - for idx, search_result in enumerate(data.get("docs")): + def parse_search_data( + self, data: JsonDict, min_confidence: float + ) -> Iterator[SearchResult]: + for idx, search_result in enumerate(data.get("docs", [])): # build the remote id from the openlibrary key key = self.books_url + search_result["key"] author = search_result.get("author_name") or ["Unknown"] @@ -174,7 +180,7 @@ def parse_search_data(self, data, min_confidence): confidence=confidence, ) - def parse_isbn_search_data(self, data): + def parse_isbn_search_data(self, data: JsonDict) -> Iterator[SearchResult]: for search_result in list(data.values()): # build the remote id from the openlibrary key key = self.books_url + search_result["key"] @@ -188,12 +194,12 @@ def parse_isbn_search_data(self, data): year=search_result.get("publish_date"), ) - def load_edition_data(self, olkey): + def load_edition_data(self, olkey: str) -> JsonDict: """query openlibrary for editions of a work""" url = f"{self.books_url}/works/{olkey}/editions" return self.get_book_data(url) - def expand_book_data(self, book): + def expand_book_data(self, book: models.Book) -> None: work = book # go from the edition to the work, if necessary if isinstance(book, models.Edition): @@ -206,14 +212,14 @@ def expand_book_data(self, book): # who knows, man return - for edition_data in edition_options.get("entries"): + for edition_data in edition_options.get("entries", []): # does this edition have ANY interesting data? if ignore_edition(edition_data): continue create_edition_task.delay(self.connector.id, work.id, edition_data) -def ignore_edition(edition_data): +def ignore_edition(edition_data: JsonDict) -> bool: """don't load a million editions that have no metadata""" # an isbn, we love to see it if edition_data.get("isbn_13") or edition_data.get("isbn_10"): @@ -232,19 +238,30 @@ def ignore_edition(edition_data): return True -def get_description(description_blob): +def get_description(description_blob: Union[JsonDict, str]) -> str: """descriptions can be a string or a dict""" if isinstance(description_blob, dict): - return description_blob.get("value") - return description_blob + description = markdown(description_blob.get("value", "")) + else: + description = markdown(description_blob) + + if ( + description.startswith("") + and description.endswith("
") + and description.count("") == 1 + ): + # If there is just one
tag and it is around the text remove it + return description[len("
") : -len("
")].strip() + + return clean(description) -def get_openlibrary_key(key): +def get_openlibrary_key(key: str) -> str: """convert /books/OL27320736M into OL27320736M""" return key.split("/")[-1] -def get_languages(language_blob): +def get_languages(language_blob: Iterable[JsonDict]) -> list[Optional[str]]: """/language/eng -> English""" langs = [] for lang in language_blob: @@ -252,14 +269,14 @@ def get_languages(language_blob): return langs -def get_dict_field(blob, field_name): +def get_dict_field(blob: Optional[JsonDict], field_name: str) -> Optional[Any]: """extract the isni from the remote id data for the author""" if not blob or not isinstance(blob, dict): return None return blob.get(field_name) -def get_wikipedia_link(links): +def get_wikipedia_link(links: list[Any]) -> Optional[str]: """extract wikipedia links""" if not isinstance(links, list): return None @@ -272,7 +289,7 @@ def get_wikipedia_link(links): return None -def get_inventaire_id(links): +def get_inventaire_id(links: list[Any]) -> Optional[str]: """extract and format inventaire ids""" if not isinstance(links, list): return None @@ -282,11 +299,13 @@ def get_inventaire_id(links): continue if link.get("title") == "inventaire.io": iv_link = link.get("url") + if not isinstance(iv_link, str): + return None return iv_link.split("/")[-1] return None -def pick_default_edition(options): +def pick_default_edition(options: list[JsonDict]) -> Optional[JsonDict]: """favor physical copies with covers in english""" if not options: return None diff --git a/bookwyrm/emailing.py b/bookwyrm/emailing.py index 2271077b12..5e08ebba13 100644 --- a/bookwyrm/emailing.py +++ b/bookwyrm/emailing.py @@ -3,7 +3,7 @@ from django.template.loader import get_template from bookwyrm import models, settings -from bookwyrm.tasks import app, HIGH +from bookwyrm.tasks import app, EMAIL from bookwyrm.settings import DOMAIN @@ -75,7 +75,7 @@ def format_email(email_name, data): return (subject, html_content, text_content) -@app.task(queue=HIGH) +@app.task(queue=EMAIL) def send_email(recipient, subject, html_content, text_content): """use a task to send the email""" email = EmailMultiAlternatives( diff --git a/bookwyrm/forms/books.py b/bookwyrm/forms/books.py index 623beaa042..4885dc0632 100644 --- a/bookwyrm/forms/books.py +++ b/bookwyrm/forms/books.py @@ -20,6 +20,7 @@ class Meta: model = models.Edition fields = [ "title", + "sort_title", "subtitle", "description", "series", @@ -45,6 +46,9 @@ class Meta: ] widgets = { "title": forms.TextInput(attrs={"aria-describedby": "desc_title"}), + "sort_title": forms.TextInput( + attrs={"aria-describedby": "desc_sort_title"} + ), "subtitle": forms.TextInput(attrs={"aria-describedby": "desc_subtitle"}), "description": forms.Textarea( attrs={"aria-describedby": "desc_description"} @@ -107,6 +111,7 @@ class Meta: model = models.Work fields = [ "title", + "sort_title", "subtitle", "authors", "description", diff --git a/bookwyrm/forms/lists.py b/bookwyrm/forms/lists.py index 647db3bfe9..f5008baa3c 100644 --- a/bookwyrm/forms/lists.py +++ b/bookwyrm/forms/lists.py @@ -24,7 +24,7 @@ class SortListForm(forms.Form): sort_by = ChoiceField( choices=( ("order", _("List Order")), - ("title", _("Book Title")), + ("sort_title", _("Book Title")), ("rating", _("Rating")), ), label=_("Sort By"), diff --git a/bookwyrm/isbn/RangeMessage.xml b/bookwyrm/isbn/RangeMessage.xml new file mode 100644 index 0000000000..619cf1ff7a --- /dev/null +++ b/bookwyrm/isbn/RangeMessage.xml @@ -0,0 +1,7904 @@ + + + + + + + + + + + + + + + +]> +{% blocktrans with name=add_author %}Creating a new author: {{ name }}{% endblocktrans %}
{% endif %} - {% if not book %} + {% if not book.parent_work %}