diff --git a/accounts/sync.py b/accounts/sync.py index 38a17d6..69742ff 100644 --- a/accounts/sync.py +++ b/accounts/sync.py @@ -3,7 +3,7 @@ from django.http import JsonResponse from django.utils import timezone from django.views.decorators.http import require_http_methods -from books.models import EBook, EBookProgress, EBookHighlights, EBookBookmarks +from books.models import EBook, EBookProgress, EBookHighlights, EBookBookmarks, book_rows from books.views import _now_ms, progress_state from podcasts.models import PodcastFeed, EpisodeProgress, PodcastQueue from radio.models import SavedStation @@ -30,9 +30,7 @@ def sync_snapshot(request): user = request.user - books = list(EBook.objects.filter(user=user).values('id', 'meta_ct', 'meta_iv', 'uploaded_at', 'is_read')) - for b in books: - b['uploaded_at'] = b['uploaded_at'].isoformat() + books = [{k: b[k] for k in ('id', 'meta_ct', 'meta_iv', 'uploaded_at', 'is_read')} for b in book_rows(user)] book_progress = [ {'book_id': p.book_id, 'updated_at': p.updated_at.isoformat(), **progress_state(p)} diff --git a/books/migrations/0007_list_indexes.py b/books/migrations/0007_list_indexes.py new file mode 100644 index 0000000..054c9b1 --- /dev/null +++ b/books/migrations/0007_list_indexes.py @@ -0,0 +1,23 @@ +# Generated by Django 6.1 on 2026-10-19 13:24 + +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('books', '0006_covers_and_prefs'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddIndex( + model_name='ebook', + index=models.Index(fields=['user', 'uploaded_at', 'is_read'], name='ebook_list_idx'), + ), + migrations.AddIndex( + model_name='ebook', + index=models.Index(fields=['user', 'cover_iv'], name='ebook_cover_idx'), + ), + ] diff --git a/books/models.py b/books/models.py index d50c5eb..ce958d2 100644 --- a/books/models.py +++ b/books/models.py @@ -17,11 +17,34 @@ class EBook(models.Model): class Meta: ordering = ['uploaded_at'] + # The list reads these from indexes, never from the row: the file (data_ct) stands + # before them in it, and SQLite walks through all of a file to reach a column after it + # — 37 s for a library of 400 books (docs/database.md). See book_rows(). + indexes = [ + models.Index(fields=['user', 'uploaded_at', 'is_read'], name='ebook_list_idx'), + models.Index(fields=['user', 'cover_iv'], name='ebook_cover_idx'), + ] def __str__(self): return f"EBook #{self.pk} (user={self.user_id})" +def book_rows(user): + """Every book of the user as {id, meta_ct, meta_iv, uploaded_at (ISO), is_read, has_cover}, + oldest first — read without passing a book's file: meta_ct and meta_iv stand before it in + the row, the rest comes from the covering indexes ebook_list_idx and ebook_cover_idx.""" + books = {b['id']: b for b in EBook.objects.filter(user=user).order_by().values('id', 'meta_ct', 'meta_iv')} + for book_id, at, read in EBook.objects.filter(user=user).order_by().values_list('id', 'uploaded_at', 'is_read'): + if book_id in books: + books[book_id].update(uploaded_at=at, is_read=read) + covered = set(EBook.objects.filter(user=user).exclude(cover_iv='').order_by().values_list('id', flat=True)) + out = sorted((b for b in books.values() if 'uploaded_at' in b), key=lambda b: (b['uploaded_at'], b['id'])) + for b in out: + b['uploaded_at'] = b['uploaded_at'].isoformat() + b['has_cover'] = b['id'] in covered + return out + + class LibraryPrefs(models.Model): """The library's own settings, encrypted with the user's key (the folder order: folder names are private, docs/library.md). One blob, replaced as a whole.""" diff --git a/books/tests.py b/books/tests.py index e98d3d1..0be2825 100644 --- a/books/tests.py +++ b/books/tests.py @@ -199,3 +199,29 @@ class CoverAndPrefsTests(TestCase): self.post('/books/prefs/', {'prefs_ct': 'AAA', 'prefs_iv': 'a1b2c3d4e5f6'}) self.post('/books/prefs/', {'prefs_ct': 'BBB', 'prefs_iv': 'a1b2c3d4e5f7'}) self.assertEqual(self.client.get('/books/prefs/', **self.auth).json(), {'prefs_ct': 'BBB', 'prefs_iv': 'a1b2c3d4e5f7'}) + + +class BookRowsReadPastNoFileTests(TestCase): + """The list must not read a column that stands after the file (data_ct) in a book's row: + SQLite would walk through the whole file to get there (docs/database.md).""" + + def test_the_list_queries_use_covering_indexes(self): + from django.contrib.auth.models import User + from django.db import connection + from django.test.utils import CaptureQueriesContext + from books.models import EBook, book_rows + user = User.objects.create_user('rows', password='x') + EBook.objects.create(user=user, meta_ct='m', meta_iv='i', data_ct='d' * 100000, data_iv='j', cover_iv='c', cover_ct='cc') + EBook.objects.create(user=user, meta_ct='m2', meta_iv='i2', data_ct='d', data_iv='j') + with CaptureQueriesContext(connection) as q: + rows = book_rows(user) + self.assertEqual([r['meta_ct'] for r in rows], ['m', 'm2']) + self.assertEqual([r['has_cover'] for r in rows], [True, False]) + late = ('uploaded_at', 'is_read', 'cover_iv', 'cover_ct', 'data_ct', 'data_iv') + with connection.cursor() as cur: + for query in q.captured_queries: + sql = query['sql'] + if any(col in sql for col in late): + cur.execute('EXPLAIN QUERY PLAN ' + sql.replace('%s', '?'), [user.id] if '?' in sql or '%s' in sql else []) + plan = ' '.join(str(r) for r in cur.fetchall()) + self.assertIn('COVERING INDEX', plan, sql) diff --git a/books/views.py b/books/views.py index 701798b..14a4c47 100644 --- a/books/views.py +++ b/books/views.py @@ -12,7 +12,7 @@ from django.http import JsonResponse from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods -from .models import EBook, EBookProgress, EBookHighlights, EBookBookmarks, LibraryPrefs +from .models import EBook, EBookProgress, EBookHighlights, EBookBookmarks, LibraryPrefs, book_rows def _require_auth(request): @@ -291,13 +291,7 @@ def book_list(request): err = _require_auth(request) if err: return err - with_cover = set(request.user.ebooks.exclude(cover_ct='').values_list('id', flat=True)) - books = list( - request.user.ebooks.values('id', 'meta_ct', 'meta_iv', 'uploaded_at', 'is_read') - ) - for b in books: - b['uploaded_at'] = b['uploaded_at'].isoformat() - b['has_cover'] = b['id'] in with_cover + books = book_rows(request.user) progress_map = {p.book_id: p for p in EBookProgress.objects.filter(user=request.user)} highlighted_ids = set( EBookHighlights.objects.filter(user=request.user).values_list('book_id', flat=True) diff --git a/docs/database.md b/docs/database.md index 096f22f..6b02b09 100644 --- a/docs/database.md +++ b/docs/database.md @@ -22,3 +22,15 @@ database: a copy of `db.sqlite3` alone may miss the latest writes. To copy it sa or stop the container first. Back to the old mode, if ever needed: `PRAGMA journal_mode=DELETE` (with no other connection open) and remove `diora/sqlite.py`'s hook. + +## The book list never reads past the file + +A book's row is `user, meta_ct, meta_iv, data_ct (the file), data_iv, uploaded_at, is_read, +cover_ct, cover_iv`. SQLite stores a big value in a chain of overflow pages, and to reach a +column *after* it, it walks the whole chain: listing 427 books read gigabytes and took 37 s +(the desktop app gives up after 10 s, so it never synced). Since then `books.models.book_rows()` +reads `id, meta_ct, meta_iv` from the rows (they stand before the file) and `uploaded_at, +is_read` / whether there is a cover from the covering indexes `ebook_list_idx` and +`ebook_cover_idx` — no list query touches a file. `books/tests.py` checks the query plans. +Writes still rewrite the whole row (file included); moving the files into a table of their own +would end that too. diff --git a/e2e/navigation.spec.js b/e2e/navigation.spec.js index 3d0a7ee..6df58f6 100644 --- a/e2e/navigation.spec.js +++ b/e2e/navigation.spec.js @@ -111,7 +111,7 @@ test('editions: two books joined show once with their formats, set apart show tw await page.goto('/#books'); await page.waitForFunction(() => typeof _importMany === 'function' && Array.isArray(_lastBookListData)); await page.locator('#book-file-input').setInputFiles(files); - await page.waitForFunction(() => _lastBookListData.filter(b => /^Mehrfach/.test(b.title)).length === 2, null, { timeout: 30000 }); + await page.waitForFunction(() => _lastBookListData.filter(b => /^Mehrfach/.test(b.title) && b.id).length === 2, null, { timeout: 30000 }); const ids = await page.evaluate(() => _lastBookListData.filter(b => /^Mehrfach/.test(b.title)).map(b => b.id)); const list = page.locator('#book-list'); // New books: as cards under "Ungelesen" (not in the "Zuletzt gelesen" area on top).