**Refactor directory scanning and add scan task support**

- Added a new `scan_directory` task to enable folder scans as asynchronous tasks while maintaining compatibility with the immediate backend.
- Introduced the `reconcile` function, replacing `generate_directory` for improved separation of concerns between database writes and read operations.
- Established a unique constraint on `Directory` (`one_directory_name_per_parent`) to prevent duplicate directories under the same parent.
- Added a "Scan" button to the frontend for re-reading folders from disk, along with new task invocation endpoints in the API.
- Updated tests across modules to validate the new `scan_directory` task, unique constraints, and database indexing logic.
- Improved cleanup logic for stale files, directories, and thumbnails to handle concurrent operations efficiently.
- Simplified `scan_comics` management command by removing user dependency and aligning it with the new `reconcile` function.
- Enhanced frontend components (`TheComicReader`, `TheComicList`, and others) to include lifecycle method cleanups, prevent memory leaks, and handle task-based scans.
This commit is contained in:
ajurna committed 2026-10-13 15:58:06 +01:00
1 parent 4d0ba750ad
commit ce809ed1a9
18 files changed
+364 -81

No files matched your search

+12 -14
View File
@@ -1,11 +1,11 @@
from pathlib import Path
from django.contrib.auth import get_user_model
from django.contrib.auth.base_user import AbstractBaseUser
from django.conf import settings
from django.core.management.base import BaseCommand, CommandParser
from loguru import logger
from comic.models import Directory
from comic.processing import generate_directory
from comic.processing import reconcile
class Command(BaseCommand):
@@ -26,14 +26,12 @@ class Command(BaseCommand):
self.OUTPUT = options.get('out', False)
self.scan_directory()
def scan_directory(self, user: AbstractBaseUser | None = None, directory: Directory | None = None) -> None:
if not user:
user_model = get_user_model()
user = user_model.objects.first()
if not user:
logger.error("No user found, cannot scan comics.")
return
for item in generate_directory(user, directory):
if isinstance(item, Directory):
logger.info(item)
self.scan_directory(user, item)
def scan_directory(self, directory: Directory | None = None) -> None:
reconcile(directory)
path = Path(settings.COMIC_BOOK_VOLUME, directory.path) if directory else settings.COMIC_BOOK_VOLUME
if self.OUTPUT:
self.stdout.write(str(path))
else:
logger.info(path)
for child in Directory.objects.filter(parent=directory):
self.scan_directory(child)
@@ -0,0 +1,38 @@
# Generated by Django 6.1.2 on 2026-10-13 12:51
from collections import defaultdict
from django.db import migrations, models
def merge_duplicate_directories(apps, schema_editor) -> None:
"""Scans could insert the same directory twice, so fold duplicates together
before the unique constraint is added, otherwise the migration fails on data
the bug has already created."""
Directory = apps.get_model('comic', 'Directory')
ComicBook = apps.get_model('comic', 'ComicBook')
groups = defaultdict(list)
for directory in Directory.objects.order_by('id'):
groups[(directory.parent_id, directory.name)].append(directory)
for group in groups.values():
keeper = group[0]
for duplicate in group[1:]:
ComicBook.objects.filter(directory=duplicate).update(directory=keeper)
duplicate.delete()
class Migration(migrations.Migration):
dependencies = [
('comic', '0050_delete_comicpage'),
]
operations = [
migrations.RunPython(merge_duplicate_directories, migrations.RunPython.noop),
migrations.AddConstraint(
model_name='directory',
constraint=models.UniqueConstraint(fields=('parent', 'name'), name='one_directory_name_per_parent'),
),
]
+3
View File
@@ -50,6 +50,9 @@ class Directory(models.Model):
class Meta:
ordering = ['name']
constraints = [
models.UniqueConstraint(fields=['parent', 'name'], name='one_directory_name_per_parent'),
]
def __str__(self) -> str:
+41 -37
View File
@@ -1,9 +1,10 @@
from itertools import chain
from collections.abc import Iterable
from pathlib import Path
from django.conf import settings
from django.contrib.auth.base_user import AbstractBaseUser
from django.db.models import (
BooleanField,
Case,
Count,
ExpressionWrapper,
@@ -12,44 +13,47 @@ from django.db.models import (
PositiveSmallIntegerField,
Q,
QuerySet,
Value,
When,
)
from django.db.models.functions import Coalesce
from comic import models
from comic.archives import get_archive_files
from comic.errors import NotCompatibleArchive
def generate_directory(user: AbstractBaseUser, directory: models.Directory | None = None) \
-> list[models.Directory | models.ComicBook]:
def reconcile(directory: models.Directory | None = None) -> None:
"""Bring the database in line with the filesystem for one directory.
This is the only place the scan writes, so it runs from the scan task and the
management command - never from a browse request.
"""
dir_path = Path(settings.COMIC_BOOK_VOLUME, directory.path) if directory else settings.COMIC_BOOK_VOLUME
files = []
dir_db_query = models.Directory.objects.filter(parent=directory)
clean_directories(dir_db_query, dir_path, directory)
clean_directories(models.Directory.objects.filter(parent=directory), dir_path, directory)
clean_files(models.ComicBook.objects.filter(directory=directory), dir_path, directory)
clean_thumbnails(models.Directory.objects.filter(parent=directory))
clean_thumbnails(models.ComicBook.objects.filter(directory=directory))
file_db_query = models.ComicBook.objects.filter(directory=directory)
clean_files(file_db_query, user, dir_path, directory)
dir_db_query = dir_db_query.annotate(
def list_directory(user: AbstractBaseUser, directory: models.Directory | None = None) \
-> list[models.Directory | models.ComicBook]:
"""Return the contents of one directory for one user, reading only."""
dir_db_query = models.Directory.objects.filter(parent=directory).annotate(
total=Count('comicbook', distinct=True),
progress=Count('comicbook__comicstatus', Q(comicbook__comicstatus__finished=True,
comicbook__comicstatus__user=user), distinct=True),
finished=ExpressionWrapper(Q(total=F('progress')), output_field=IntegerField()),
unread=ExpressionWrapper(Q(total__gt=F('progress')), output_field=IntegerField()),
)
files.extend(dir_db_query)
# Create Missing Status
new_status = [models.ComicStatus(comic=file, user=user) for file in
file_db_query.exclude(comicstatus__in=models.ComicStatus.objects.filter(
comic__in=file_db_query, user=user))]
models.ComicStatus.objects.bulk_create(new_status)
file_db_query = file_db_query.annotate(
progress=F('comicstatus__last_read_page') + 1,
finished=F('comicstatus__finished'),
unread=F('comicstatus__unread'),
# A book with no status for this user is simply unread, so the annotations have to
# stand in for the missing row instead of assuming one exists.
file_db_query = models.ComicBook.objects.filter(directory=directory).annotate(
progress=Coalesce(F('comicstatus__last_read_page') + 1, Value(0, output_field=IntegerField())),
finished=Coalesce(F('comicstatus__finished'), Value(False, output_field=BooleanField())),
unread=Coalesce(F('comicstatus__unread'), Value(True, output_field=BooleanField())),
user=F('comicstatus__user'),
classification=Case(
When(directory__isnull=True, then=models.Directory.Classification.C_G),
@@ -58,12 +62,7 @@ def generate_directory(user: AbstractBaseUser, directory: models.Directory | Non
)
).filter(Q(user__isnull=True) | Q(user=user.id))
files.extend(file_db_query)
for file in chain(file_db_query, dir_db_query):
if file.thumbnail and not Path(file.thumbnail.path).exists():
file.thumbnail.delete()
file.save()
files = list(dir_db_query) + list(file_db_query)
files.sort(key=lambda x: x.title)
files.sort(key=lambda x: x.type, reverse=True)
return files
@@ -74,15 +73,16 @@ def clean_directories(directories: QuerySet, dir_path: Path, directory: models.D
dir_list = {x for x in sorted(dir_path.glob('*')) if x.is_dir()}
# Create new directories db instances
for new_directory in dir_list - dir_db_set:
models.Directory(name=new_directory.name, parent=directory).save()
# get_or_create, not save(): another request may have inserted the same directory
# between the query above and this line, which would leave duplicate rows.
models.Directory.objects.get_or_create(name=new_directory.name, parent=directory)
# Remove stale db instances
for stale_directory in dir_db_set - dir_list:
models.Directory.objects.get(name=stale_directory.name, parent=directory).delete()
models.Directory.objects.filter(name=stale_directory.name, parent=directory).delete()
def clean_files(files: QuerySet, user: AbstractBaseUser, dir_path: Path, directory: models.Directory | None = None) \
-> None:
def clean_files(files: QuerySet, dir_path: Path, directory: models.Directory | None = None) -> None:
file_list = {x for x in sorted(dir_path.glob('*')) if x.is_file()}
files_db_set = {Path(dir_path, x.file_name) for x in files}
@@ -101,14 +101,18 @@ def clean_files(files: QuerySet, user: AbstractBaseUser, dir_path: Path, directo
# Unreadable/corrupt archive - index the file with no page count
pass
books_to_add.append(new_book)
models.ComicBook.objects.bulk_create(books_to_add)
status_to_add = []
for book in books_to_add:
status_to_add.append(models.ComicStatus(user=user, comic=book))
models.ComicStatus.objects.bulk_create(status_to_add)
# ignore_conflicts: a concurrent scan may have indexed the same file, and
# (directory, file_name) is unique.
models.ComicBook.objects.bulk_create(books_to_add, ignore_conflicts=True)
# Remove stale comic instances
for stale_comic in files_db_set - file_list:
models.ComicBook.objects.get(file_name=stale_comic.name, directory=directory).delete()
models.ComicBook.objects.filter(file_name=stale_comic.name, directory=directory).delete()
def clean_thumbnails(items: Iterable[models.Directory | models.ComicBook]) -> None:
for item in items:
if item.thumbnail and not Path(item.thumbnail.path).exists():
item.thumbnail.delete()
item.save()
+21 -5
View File
@@ -19,8 +19,7 @@ from rest_framework.pagination import PageNumberPagination
from rest_framework.request import Request
from rest_framework.response import Response
from comic import models
from comic.processing import generate_directory
from comic import models, processing, tasks
from comic.util import generate_breadcrumbs_from_path
@@ -124,6 +123,10 @@ class BreadcrumbSerializer(serializers.Serializer):
name = serializers.CharField()
class ScanSerializer(serializers.Serializer):
selector = serializers.UUIDField(required=False, allow_null=True)
class BrowseViewSet(viewsets.GenericViewSet):
serializer_class = BrowseSerializer
permission_classes = [permissions.IsAuthenticated]
@@ -133,15 +136,28 @@ class BrowseViewSet(viewsets.GenericViewSet):
return
def list(self, request: Request) -> Response:
serializer = self.get_serializer(generate_directory(request.user), many=True)
serializer = self.get_serializer(processing.list_directory(request.user), many=True)
return Response(serializer.data)
@swagger_auto_schema(responses={status.HTTP_200_OK: BrowseSerializer(many=True)})
def retrieve(self, request: Request, selector: UUID) -> Response:
directory = get_object_or_404(models.Directory, selector=selector)
serializer = self.get_serializer(generate_directory(request.user, directory), many=True)
serializer = self.get_serializer(processing.list_directory(request.user, directory), many=True)
return Response(serializer.data)
@swagger_auto_schema(request_body=ScanSerializer, responses={status.HTTP_200_OK: BrowseSerializer(many=True)})
@action(methods=['post'], detail=False, serializer_class=ScanSerializer)
def scan(self, request: Request) -> Response:
"""Read one directory from disk, then return it. Null selector means the root."""
serializer = self.get_serializer(data=request.data)
if not serializer.is_valid():
return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
selector = serializer.validated_data.get('selector')
tasks.scan_directory.enqueue(str(selector) if selector else None)
directory = models.Directory.objects.filter(selector=selector).first() if selector else None
data = processing.list_directory(request.user, directory)
return Response(BrowseSerializer(data, many=True).data)
@swagger_auto_schema(responses={status.HTTP_200_OK: BreadcrumbSerializer(many=True)})
@action(methods=['get'], detail=True, serializer_class=BreadcrumbSerializer)
def breadcrumbs(self, _request: Request, selector: UUID) -> Response:
@@ -264,7 +280,7 @@ class ReadViewSet(viewsets.GenericViewSet):
@swagger_auto_schema(responses={status.HTTP_200_OK: TypeSerializer()})
@action(methods=['get'], detail=True)
def type(self, _request: Request, selector: UUID) -> Response:
book = models.ComicBook.objects.get(selector=selector)
book = get_object_or_404(models.ComicBook, selector=selector)
return Response({'type': book.file_name.split('.')[-1].lower()})
@swagger_auto_schema(responses={status.HTTP_200_OK: ReadPageSerializer()}, request_body=ReadPageSerializer)
+18
View File
@@ -0,0 +1,18 @@
"""The folder scan as a task.
The default backend runs tasks immediately, so this behaves like a function call today.
Writing it as a task is what lets the scan move to a worker later without changing any
of the call sites in comic/rest.py or the management command.
"""
from django.tasks import task
from comic import models
from comic.processing import reconcile
@task
def scan_directory(selector: str | None = None) -> None:
"""Reconcile one directory, or the root of the comic volume when no selector is given."""
directory = models.Directory.objects.filter(selector=selector).first() if selector else None
reconcile(directory)
+53
View File
@@ -1,8 +1,11 @@
"""Tests for the API behaviour of the comic viewsets."""
import json
import pytest
from comic import models
from comic.tests.factories import make_cbz
UNKNOWN = '00000000-0000-0000-0000-000000000000'
@@ -25,6 +28,12 @@ def test_unknown_book_returns_404_for_an_image(client, reader_user) -> None:
assert client.get(f'/api/read/{UNKNOWN}/image/1/').status_code == 404
@pytest.mark.django_db
def test_unknown_book_returns_404_for_the_type_action(client, reader_user) -> None:
client.force_login(reader_user)
assert client.get(f'/api/read/{UNKNOWN}/type/').status_code == 404
@pytest.mark.django_db
def test_image_requires_authentication(client, book) -> None:
assert client.get(f'/api/read/{book.selector}/image/1/').status_code == 401
@@ -148,3 +157,47 @@ def test_directory_partial_update_rejects_an_unknown_classification(client, admi
content_type='application/json',
)
assert response.status_code == 400
@pytest.mark.django_db
def test_browse_listing_does_not_write_to_the_database(client, reader_user, comic_volume) -> None:
"""Listing is read only, so a browse request cannot race with the scan."""
client.force_login(reader_user)
target = comic_volume / 'Series'
target.mkdir()
make_cbz(target / 'issue.cbz', pages=2)
assert client.get('/api/browse/').status_code == 200
assert models.ComicBook.objects.count() == 0
assert models.ComicStatus.objects.count() == 0
@pytest.mark.django_db
def test_scan_endpoint_indexes_files_that_are_only_on_disk(client, reader_user, comic_volume) -> None:
client.force_login(reader_user)
target = comic_volume / 'Series'
target.mkdir()
make_cbz(target / 'issue.cbz', pages=2)
assert [entry['title'] for entry in client.get('/api/browse/').json()] == []
response = client.post('/api/browse/scan/', data='{}', content_type='application/json')
assert response.status_code == 200
assert [entry['title'] for entry in response.json()] == ['Series']
directory = models.Directory.objects.get(name='Series')
response = client.post('/api/browse/scan/',
data=json.dumps({'selector': str(directory.selector)}),
content_type='application/json')
assert response.status_code == 200
entries = response.json()
assert [entry['title'] for entry in entries] == ['issue.cbz']
assert entries[0]['type'] == 'ComicBook'
assert entries[0]['total'] == 2
assert entries[0]['progress'] == 0
assert entries[0]['unread'] is True
@pytest.mark.django_db
def test_scan_endpoint_requires_authentication(client) -> None:
assert client.post('/api/browse/scan/', data='{}', content_type='application/json').status_code == 401
+11
View File
@@ -3,6 +3,7 @@
from pathlib import Path
import pytest
from django.db import IntegrityError
from comic import models
from comic.tests.factories import make_cbz, make_pdf
@@ -69,3 +70,13 @@ def test_classification_comparison() -> None:
allowed_to_read=models.Directory.Classification.C_PG,
)
assert directory.classification > misc.allowed_to_read
@pytest.mark.django_db
def test_duplicate_directory_names_are_rejected() -> None:
"""The scan depends on this constraint: without it two concurrent requests can create
the same directory twice and the browse listing shows it twice."""
parent = models.Directory.objects.create(name='Series')
models.Directory.objects.create(name='Volume 1', parent=parent)
with pytest.raises(IntegrityError):
models.Directory.objects.create(name='Volume 1', parent=parent)
+94 -7
View File
@@ -3,7 +3,7 @@
import pytest
from comic import models
from comic.processing import clean_directories, clean_files, generate_directory
from comic.processing import clean_directories, clean_files, list_directory, reconcile
from comic.tests.factories import make_cbz
@@ -27,22 +27,109 @@ def test_clean_files_ignores_unsupported_extensions(comic_volume, reader_user) -
make_cbz(target / 'issue.cbz', pages=2)
(target / 'notes.txt').write_text('not a comic')
clean_files(models.ComicBook.objects.filter(directory=directory), reader_user, target, directory)
clean_files(models.ComicBook.objects.filter(directory=directory), target, directory)
books = list(models.ComicBook.objects.filter(directory=directory))
assert [book.file_name for book in books] == ['issue.cbz']
assert books[0].page_count == 2
assert models.ComicStatus.objects.filter(user=reader_user).count() == 1
@pytest.mark.django_db
def test_generate_directory_creates_missing_statuses(comic_volume, reader_user) -> None:
def test_reconcile_indexes_the_directory_without_touching_the_database_on_a_second_run(
comic_volume, reader_user) -> None:
directory = models.Directory.objects.create(name='Series')
target = comic_volume / 'Series'
target.mkdir(parents=True)
make_cbz(target / 'issue.cbz', pages=1)
make_cbz(target / 'issue.cbz', pages=2)
entries = generate_directory(reader_user, directory)
reconcile(directory)
first = list(models.ComicBook.objects.filter(directory=directory))
assert [book.file_name for book in first] == ['issue.cbz']
reconcile(directory)
assert list(models.ComicBook.objects.filter(directory=directory)) == first
@pytest.mark.django_db
def test_list_directory_reports_an_unread_book_without_creating_a_status(comic_volume,
reader_user) -> None:
"""Listing is read only: a book nobody has opened has no ComicStatus row at all."""
directory = models.Directory.objects.create(name='Series')
target = comic_volume / 'Series'
target.mkdir(parents=True)
make_cbz(target / 'issue.cbz', pages=3)
reconcile(directory)
entries = list_directory(reader_user, directory)
assert [entry.title for entry in entries] == ['issue.cbz']
assert models.ComicStatus.objects.filter(user=reader_user, comic=entries[0]).exists()
assert entries[0].progress == 0
assert entries[0].unread is True
assert entries[0].finished is False
assert models.ComicStatus.objects.count() == 0
@pytest.mark.django_db
def test_list_directory_shows_progress_for_a_book_that_has_been_read(comic_volume,
reader_user) -> None:
directory = models.Directory.objects.create(name='Series')
target = comic_volume / 'Series'
target.mkdir(parents=True)
make_cbz(target / 'issue.cbz', pages=3)
reconcile(directory)
book = models.ComicBook.objects.get(directory=directory)
models.ComicStatus.objects.create(user=reader_user, comic=book, last_read_page=1, unread=False)
entries = list_directory(reader_user, directory)
assert entries[0].progress == 2
assert entries[0].unread is False
assert entries[0].finished is False
@pytest.mark.django_db
def test_clean_files_survives_a_concurrent_scan_of_the_same_directory(comic_volume, reader_user,
rated_directory) -> None:
"""Two requests can build the same 'new file' list; the second insert must not clash."""
target = comic_volume / rated_directory.name
target.mkdir(parents=True, exist_ok=True)
make_cbz(target / 'issue.cbz', pages=2)
stale = models.ComicBook.objects.filter(directory=rated_directory)
list(stale) # force evaluation so the cached snapshot is empty
models.ComicBook.objects.create(file_name='issue.cbz', directory=rated_directory, page_count=2)
clean_files(stale, target, rated_directory)
assert models.ComicBook.objects.filter(directory=rated_directory).count() == 1
@pytest.mark.django_db
def test_clean_files_ignores_rows_another_request_already_removed(comic_volume, reader_user,
rated_directory) -> None:
target = comic_volume / rated_directory.name
target.mkdir(parents=True, exist_ok=True)
stale_book = models.ComicBook.objects.create(file_name='gone.cbz', directory=rated_directory)
stale = models.ComicBook.objects.filter(directory=rated_directory)
list(stale) # cache the row, then let another request delete it
stale_book.delete()
clean_files(stale, target, rated_directory)
assert models.ComicBook.objects.filter(directory=rated_directory).count() == 0
@pytest.mark.django_db
def test_clean_directories_does_not_duplicate_rows_when_another_request_inserts_first(comic_volume,
reader_user) -> None:
parent = models.Directory.objects.create(name='Series')
(comic_volume / 'Series' / 'New').mkdir(parents=True)
stale = models.Directory.objects.filter(parent=parent)
list(stale) # cached snapshot is empty
models.Directory.objects.create(name='New', parent=parent)
clean_directories(stale, comic_volume / 'Series', parent)
assert models.Directory.objects.filter(parent=parent).count() == 1
+4 -2
View File
@@ -23,9 +23,11 @@ def test_scan_indexes_nested_directories(comic_volume: Path, reader_user) -> Non
@pytest.mark.django_db
def test_scan_without_a_user_stops_before_reading_the_volume(comic_volume: Path) -> None:
def test_scan_runs_without_any_user(comic_volume: Path) -> None:
"""The command no longer needs a user, because statuses are created when a book is read."""
make_cbz(comic_volume / 'issue.cbz', pages=1)
call_command('scan_comics')
assert models.ComicBook.objects.count() == 0
assert models.ComicBook.objects.count() == 1
assert models.ComicStatus.objects.count() == 0
+17
View File
@@ -0,0 +1,17 @@
"""Tests for the scan task."""
import pytest
from comic import models, tasks
from comic.tests.factories import make_cbz
@pytest.mark.django_db
def test_scan_task_runs_through_the_task_backend(comic_volume) -> None:
"""The immediate backend executes the task in the caller, so this is the whole path."""
make_cbz(comic_volume / 'issue.cbz', pages=2)
tasks.scan_directory.enqueue()
assert models.ComicBook.objects.count() == 1
assert models.ComicBook.objects.get().page_count == 2