perf(kobo): defer kepub conversion and optimize sync queries (#1344)

* Fix slow KoboReadingState handling in the sync handler:

Saves up to ~11s on Kobo sync requests.

The prevoius implementation was executing a new query per book returned by the first. In addition, it inserted a new KoboReadingState entry when none existed. On the 1st sync request of a 5000 book unread library, the previous implementation spent 7.7 ms in get_or_create_reading_state + 3.5 ms in get_kobo_reading_state_response

* Eager load book metadata tables in Kobo sync

Saves another 3s on Kobo sync requests.

The previous implementation resulted in 5 additional db queries per book

* Fix bad log line in kobo

* Defer kepub book conversion to book download if supported

Saves another 1.25s on the library/sync request

Note that this only returns a single downloadUrl. If kepub conversion fails, the download handler will fallback to the epub file, which the Android Kobo app at least handles fines.

LOCAL: Disable kobo_auth epub conversion

Author: shavitmichael

cps/helper.py

@@ -5,6 +5,7 @@
 # SPDX-License-Identifier: GPL-3.0-or-later
 # See CONTRIBUTORS for full list of authors.
 
+import glob
 import os
 import random
 import io
@@ -48,15 +49,15 @@
 from . import gdriveutils as gd
 from .constants import (STATIC_DIR as _STATIC_DIR, CACHE_TYPE_THUMBNAILS, THUMBNAIL_TYPE_COVER, THUMBNAIL_TYPE_SERIES,
                         SUPPORTED_CALIBRE_BINARIES)
-from .subproc_wrapper import process_wait
+from .subproc_wrapper import process_wait, process_open
 
 # Track books with pending thumbnail generation to prevent duplicate tasks
 _pending_thumbnail_books = set()
 
 import sys
 sys.path.insert(1, '/app/calibre-web-automated/scripts/')
 from cwa_db import CWA_DB
-from .services.worker import WorkerThread
+from .services.worker import WorkerThread, STAT_FINISH_SUCCESS
 from .tasks.mail import TaskEmail
 from .tasks.thumbnail import TaskClearCoverThumbnailCache, TaskGenerateCoverThumbnails
 from .tasks.metadata_backup import TaskBackupMetadata
@@ -85,7 +86,8 @@ def _directory_contains_only_nfs_placeholders(path):
 
 
 # Convert existing book entry to new format
-def convert_book_format(book_id, calibre_path, old_book_format, new_book_format, user_id, ereader_mail=None, subject=None):
+def convert_book_format(book_id, calibre_path, old_book_format, new_book_format, user_id,
+                        ereader_mail=None, subject=None, blocking=False):
     book = calibre_db.get_book(book_id)
     data = calibre_db.get_book_format(book.id, old_book_format)
     if not data:
@@ -119,7 +121,14 @@ def convert_book_format(book_id, calibre_path, old_book_format, new_book_format,
            link)
     settings['old_book_format'] = old_book_format
     settings['new_book_format'] = new_book_format
-    WorkerThread.add(user_id, TaskConvert(file_path, book.id, txt, settings, ereader_mail, user_id))
+    task = TaskConvert(file_path, book.id, txt, settings, ereader_mail, user_id)
+    WorkerThread.add(user_id, task)
+    if blocking:
+        finished = task.done_event.wait(timeout=120)
+        if not finished:
+            return _("Conversion timed out for book id: %(book)d", book=book_id)
+        if task.stat != STAT_FINISH_SUCCESS:
+            return task.error or _("Conversion failed for book id: %(book)d", book=book_id)
     return None
 
 
@@ -1444,6 +1453,16 @@ def get_download_link(book_id, book_format, client):
         abort(404)
 
     data1 = calibre_db.get_book_format(book.id, book_format.upper())
+    if not data1 and book_format == "kepub" and config.config_kepubifypath:
+        data1 = calibre_db.get_book_format(book.id, "EPUB")
+        if data1:
+            log.info("KEPUB not found for book %d; converting on demand", book.id)
+            err = convert_book_format(book.id, config.get_book_path(), 'EPUB', 'KEPUB', None, blocking=True)
+            if not err:
+                data1 = calibre_db.get_book_format(book.id, "KEPUB")
+            else:
+                log.error("On-demand KEPUB conversion failed for book %d: %s", book.id, err)
+                book_format = "epub"
     if not data1:
         log.error("Requested format %s for book id %s not found in database", book_format.upper(), book_id)
         abort(404)

cps/kobo.py

@@ -33,6 +33,7 @@
 from sqlalchemy import func
 from sqlalchemy.sql.expression import and_, or_
 from sqlalchemy.exc import StatementError
+from sqlalchemy.orm import joinedload
 from sqlalchemy.sql import select
 import requests
 
@@ -232,14 +233,20 @@ def HandleSyncRequest():
 
     log.debug("Kobo Sync: books last modified: {}".format(sync_token.books_last_modified))
 
+    rstate_join = and_(
+        db.Books.id == ub.KoboReadingState.book_id,
+        ub.KoboReadingState.user_id == current_user.id,
+    )
     if only_kobo_shelves:
         changed_entries = calibre_db.session.query(db.Books,
                                                    ub.ArchivedBook.last_modified,
                                                    ub.BookShelf.date_added,
-                                                   ub.ArchivedBook.is_archived)
+                                                   ub.ArchivedBook.is_archived,
+                                                   ub.KoboReadingState)
         changed_entries = (changed_entries
                            .join(db.Data).outerjoin(ub.ArchivedBook, and_(db.Books.id == ub.ArchivedBook.book_id,
                                                                           ub.ArchivedBook.user_id == current_user.id))
+                           .outerjoin(ub.KoboReadingState, rstate_join)
                            .filter(db.Books.id.notin_(calibre_db.session.query(ub.KoboSyncedBooks.book_id)
                                                       .filter(ub.KoboSyncedBooks.user_id == current_user.id)))
                           .filter(or_(
@@ -257,37 +264,50 @@ def HandleSyncRequest():
                                and_(ub.Shelf.user_id == current_user.id, ub.Shelf.kobo_sync == True),
                                db.Books.id.in_(magic_shelf_book_ids) if magic_shelf_book_ids else False
                            ))
+                           .options(joinedload(db.Books.authors),
+                                    joinedload(db.Books.publishers),
+                                    joinedload(db.Books.series),
+                                    joinedload(db.Books.languages),
+                                    joinedload(db.Books.comments),
+                                    joinedload(db.Books.data))
                            .distinct())
     else:
         changed_entries = calibre_db.session.query(db.Books,
                                                    ub.ArchivedBook.last_modified,
-                                                   ub.ArchivedBook.is_archived)
+                                                   ub.ArchivedBook.is_archived,
+                                                   ub.KoboReadingState)
         changed_entries = (changed_entries
                            .join(db.Data).outerjoin(ub.ArchivedBook, and_(db.Books.id == ub.ArchivedBook.book_id,
                                                                           ub.ArchivedBook.user_id == current_user.id))
+                           .outerjoin(ub.KoboReadingState, rstate_join)
                            .filter(db.Books.id.notin_(calibre_db.session.query(ub.KoboSyncedBooks.book_id)
                                                       .filter(ub.KoboSyncedBooks.user_id == current_user.id)))
                            .filter(calibre_db.common_filters(allow_show_archived=True))
                            .filter(db.Data.format.in_(KOBO_FORMATS))
                            .order_by(db.Books.last_modified)
-                           .order_by(db.Books.id))
+                           .order_by(db.Books.id)
+                           .options(joinedload(db.Books.authors),
+                                    joinedload(db.Books.publishers),
+                                    joinedload(db.Books.series),
+                                    joinedload(db.Books.languages),
+                                    joinedload(db.Books.comments),
+                                    joinedload(db.Books.data)))
     log.debug("Kobo Sync: changed entries: {}".format(changed_entries.count()))
 
     reading_states_in_new_entitlements = []
     books = changed_entries.limit(SYNC_ITEM_LIMIT)
     log.debug("Kobo Sync: selected to sync: {}".format(len(books.all())))
     for book in books:
         formats = [data.format for data in book.Books.data]
-        if 'KEPUB' not in formats and config.config_kepubifypath and 'EPUB' in formats:
-            helper.convert_book_format(book.Books.id, config.get_book_path(), 'EPUB', 'KEPUB', current_user.name)
 
-        kobo_reading_state = get_or_create_reading_state(book.Books.id)
+        kobo_reading_state = book.KoboReadingState  # None when no record exists yet
         entitlement = {
             "BookEntitlement": create_book_entitlement(book.Books, archived=(book.is_archived==True)),
             "BookMetadata": get_metadata(book.Books),
         }
 
-        if kobo_reading_state.last_modified > sync_token.reading_state_last_modified:
+        if (kobo_reading_state is not None
+                and kobo_reading_state.last_modified > sync_token.reading_state_last_modified):
             entitlement["ReadingState"] = get_kobo_reading_state_response(book.Books, kobo_reading_state)
             new_reading_state_last_modified = max(new_reading_state_last_modified, kobo_reading_state.last_modified)
             reading_states_in_new_entitlements.append(book.Books.id)
@@ -581,28 +601,32 @@ def _get_cover_image_id(book):
 
 def get_metadata(book):
     download_urls = []
-    kepub = [data for data in book.data if data.format == 'KEPUB']
 
-    for book_data in kepub if len(kepub) > 0 else book.data:
-        if book_data.format not in KOBO_FORMATS:
-            continue
-        for kobo_format in KOBO_FORMATS[book_data.format]:
-            # log.debug('Id: %s, Format: %s' % (book.id, kobo_format))
-            try:
-                if get_epub_layout(book, book_data) == 'pre-paginated':
-                    kobo_format = 'EPUB3FL'
-                download_urls.append(
-                    {
-                        "Format": kobo_format,
-                        "Size": book_data.uncompressed_size,
-                        "Url": get_download_url_for_book(book.id, book_data.format),
-                        # The Kobo forma accepts platforms: (Generic, Android)
-                        "Platform": "Generic",
-                        # "DrmType": "None", # Not required
-                    }
-                )
-            except (zipfile.BadZipfile, FileNotFoundError) as e:
-                log.error(e)
+    kepub_data = next((d for d in book.data if d.format == 'KEPUB'), None)
+    epub_data  = next((d for d in book.data if d.format == 'EPUB'),  None)
+
+    if kepub_data:
+        book_data, dl_format, published_format = kepub_data, 'kepub', 'KEPUB'
+    elif epub_data and config.config_kepubifypath:
+        book_data, dl_format, published_format = epub_data, 'kepub', 'KEPUB'
+    elif epub_data:
+        book_data, dl_format, published_format = epub_data, 'epub', 'EPUB3'
+    else:
+        book_data = None
+
+    if book_data:
+        try:
+            if get_epub_layout(book, book_data) == 'pre-paginated':
+                published_format = 'EPUB3FL'
+        except (zipfile.BadZipfile, FileNotFoundError) as e:
+            log.error(e)
+        download_urls.append({
+            "Format": published_format,
+            "Size": book_data.uncompressed_size,
+            "Url": get_download_url_for_book(book.id, dl_format),
+            "Platform": "Generic",
+            "DrmType": "None",
+        })
 
     book_uuid = book.uuid
     cover_image_id = _get_cover_image_id(book)
@@ -634,11 +658,12 @@ def get_metadata(book):
     }
     metadata.update(get_author(book))
 
-    if get_series(book):
-        name = get_series(book)
+    series_name = get_series(book)
+    if series_name:
+        name = series_name
         try:
             metadata["Series"] = {
-                "Name": get_series(book),
+                "Name": series_name,
                 "Number": get_seriesindex(book),        # ToDo Check int() ?
                 "NumberFloat": float(get_seriesindex(book)),
                 # Get a deterministic id based on the series name.
@@ -1044,8 +1069,10 @@ def get_ub_read_status(kobo_read_status):
 
 
 def get_or_create_reading_state(book_id):
-    book_read = ub.session.query(ub.ReadBook).filter(ub.ReadBook.book_id == book_id,
-                                                     ub.ReadBook.user_id == int(current_user.id)).one_or_none()
+    book_read = ub.session.query(ub.ReadBook).filter(
+        ub.ReadBook.book_id == book_id,
+        ub.ReadBook.user_id == int(current_user.id),
+    ).one_or_none()
     if not book_read:
         book_read = ub.ReadBook(user_id=current_user.id, book_id=book_id)
     if not book_read.kobo_reading_state:
@@ -1123,7 +1150,7 @@ def HandleCoverImageRequest(book_uuid, width, height, Quality, isGreyscale):
         else:
             resolution = COVER_THUMBNAIL_SMALL
     except ValueError:
-        log.error("Requested height %s of book %s is invalid" % (book_uuid, height))
+        log.error("Requested height %s of book %s is invalid" % (height, book_uuid))
         resolution = COVER_THUMBNAIL_SMALL
     book_cover = helper.get_book_cover_with_uuid(book_uuid, resolution=resolution)
     if book_cover:

cps/kobo_auth.py

@@ -94,13 +94,6 @@ def generate_auth_token(user_id):
         ub.session.add(auth_token)
         ub.session_commit()
 
-    books = calibre_db.session.query(db.Books).join(db.Data).all()
-
-    for book in books:
-        formats = [data.format for data in book.data]
-        if 'KEPUB' not in formats and config.config_kepubifypath and 'EPUB' in formats:
-            helper.convert_book_format(book.id, config.config_calibre_dir, 'EPUB', 'KEPUB', current_user.name)
-
     return render_title_template(
         "generate_kobo_auth_url.html",
         title=_("Kobo Setup"),

cps/services/worker.py

@@ -232,6 +232,7 @@ def __init__(self, message):
         self.id = uuid.uuid4()
         self.self_cleanup = False
         self._scheduled = False
+        self.done_event = threading.Event()
 
     @abc.abstractmethod
     def run(self, worker_thread):
@@ -320,10 +321,12 @@ def _handleError(self, error_message):
         self.stat = STAT_FAIL
         self.progress = 1
         self.error = error_message
+        self.done_event.set()
 
     def _handleSuccess(self):
         self.stat = STAT_FINISH_SUCCESS
         self.progress = 1
+        self.done_event.set()
 
     def __str__(self):
         return self.name