diff --git a/docs/exif_mapping.md b/docs/exif_mapping.md index d62ac8c..28e4596 100644 --- a/docs/exif_mapping.md +++ b/docs/exif_mapping.md @@ -102,9 +102,14 @@ if color is a numeric label (e.g. "0 (normal)", "+2 (high)"): → saturation adjustment; decode using table below elif color is a non-numeric string (e.g. "Acros", "None (B&W)", "Film Simulation"): - → film simulation name or special case; recipe returns "N/A" for the Color field + → film simulation name or special case; recipe returns None for the Color field ``` +Note that Color returns `None`, not the string `"N/A"` that Sharpness uses for its +equivalent case. For a monochromatic simulation that absence is correct and the +recipe is stored. For a colour simulation it is not: a colour recipe with no Color +value fails `validate_recipe_data`, and the image is skipped at ingest. + ### Numeric saturation mapping table | Camera display value | EXIF stored value | Recipe output | @@ -136,8 +141,9 @@ elif color is a non-numeric string (e.g. "Acros", "None (B&W)", "Film Simulation ### Key observations -- `Film Simulation` appears for some film simulations (e.g. Eterna, Astia, Pro Neg. Std); it means the film profile controls saturation internally and the user cannot override it. -- For B&W/Acros/Sepia simulations the `Film Mode` EXIF field is absent; `Saturation` encodes the simulation name instead. In those cases the Color recipe field is `"N/A"` — there is no separate saturation adjustment. +- `Film Simulation` means the camera, not the user, drove saturation for that shot. It is not a property of the film simulation: it has been observed on Eterna, Astia and Pro Neg. Std images, but the same simulations carry numeric values on the great majority of shots. Nothing else in the EXIF flags the condition, so this value is the only signal. +- A colour simulation whose `Saturation` is `Film Simulation` records no recipe of the user's, so the image is skipped at ingest rather than stored with an empty Color. See [management_commands.md](management_commands.md) for how skips are reported. +- For B&W/Acros/Sepia simulations the `Film Mode` EXIF field is absent; `Saturation` encodes the simulation name instead. In those cases the Color recipe field is `None` — there is no separate saturation adjustment, and the recipe is stored normally. - Recipe output for numeric values is a signed integer string: `"-2"`, `"+3"`, `"0"`. --- diff --git a/docs/management_commands.md b/docs/management_commands.md index cd4b284..a521846 100644 --- a/docs/management_commands.md +++ b/docs/management_commands.md @@ -63,6 +63,13 @@ Behaviour depends on your install mode: - **Full install** (`USE_ASYNC_TASKS=True`): one Celery task is enqueued per image and processed in parallel by the worker (start it first with `make worker`). +Files that cannot produce a recipe are skipped, never aborting the run. That covers +images carrying no Fujifilm metadata, and images whose EXIF fails recipe validation, +such as a colour simulation whose Color the camera set rather than the user. In lite +mode the command reports how many were skipped; add `--verbosity 2` to list their +paths. In full install mode each skip is recorded as an `image.import.skipped` event +in the worker log, carrying the reason and the offending recipe field. + --- ## Rating images in bulk diff --git a/src/application/usecases/images/process_images.py b/src/application/usecases/images/process_images.py index b38949b..71e2dca 100644 --- a/src/application/usecases/images/process_images.py +++ b/src/application/usecases/images/process_images.py @@ -1,19 +1,32 @@ +import attrs from django.conf import settings from src.domain.images import events, operations, queries +from src.domain.recipes import validation as recipe_validation from src.services import workertasks -def import_images_from_folder(*, folder: str) -> int: +@attrs.frozen +class FolderImportSummary: """ - Process all JPG images in *folder*, dispatching async or sync based on settings. + Counts produced by an :func:`import_images_from_folder` run. + + ``skipped`` holds the files that cannot produce a recipe. It is always empty + in async mode, where each file's outcome is only known inside the worker. + """ + + total: int + processed: int = 0 + skipped: tuple[str, ...] = () - Returns the total number of images found. + +def import_images_from_folder(*, folder: str) -> FolderImportSummary: + """ + Process all JPG images in *folder*, dispatching async or sync based on settings. """ if settings.USE_ASYNC_TASKS: - return _enqueue_images_in_folder(folder=folder) - total, _ = _process_images_in_folder(folder=folder) - return total + return FolderImportSummary(total=_enqueue_images_in_folder(folder=folder)) + return _process_images_in_folder(folder=folder) def _enqueue_images_in_folder(*, folder: str) -> int: @@ -33,18 +46,38 @@ def _enqueue_images_in_folder(*, folder: str) -> int: return len(paths) -def _process_images_in_folder(*, folder: str) -> tuple[int, list[str]]: +def _process_images_in_folder(*, folder: str) -> FolderImportSummary: """ - Process all JPG images in *folder* sequentially, skipping those without Fujifilm metadata. + Process all JPG images in *folder* sequentially, one file at a time. - Returns: - A tuple of (total_found, skipped_paths). + A file that carries no Fujifilm metadata, or whose EXIF cannot produce a + valid recipe, is recorded as skipped so it never aborts the rest of the run. """ paths = queries.collect_image_paths(folder=folder) - skipped = [] + processed = 0 + skipped: list[str] = [] for path in paths: try: operations.process_image(image_path=path) except operations.NoFilmSimulationError: skipped.append(path) - return len(paths), skipped + events.publish_event( + event_type=events.IMAGE_IMPORT_SKIPPED, + image_path=path, + reason=events.SKIP_REASON_NO_FILM_SIMULATION, + ) + except recipe_validation.InvalidFujifilmRecipeData as exc: + skipped.append(path) + events.publish_event( + event_type=events.IMAGE_IMPORT_SKIPPED, + image_path=path, + reason=events.SKIP_REASON_INVALID_RECIPE_DATA, + recipe_field=exc.field, + ) + else: + processed += 1 + return FolderImportSummary( + total=len(paths), + processed=processed, + skipped=tuple(skipped), + ) diff --git a/src/application/usecases/library/process_synced_image.py b/src/application/usecases/library/process_synced_image.py index 0531e8a..397c39d 100644 --- a/src/application/usecases/library/process_synced_image.py +++ b/src/application/usecases/library/process_synced_image.py @@ -1,9 +1,11 @@ import structlog +from src.domain.images import events as image_events from src.domain.images import operations as image_operations from src.domain.images.queries import NoFilmSimulationError from src.domain.library import operations as library_operations from src.domain.library import queries as library_queries +from src.domain.recipes import validation as recipe_validation logger = structlog.get_logger("application.library.process_synced_image") @@ -13,10 +15,11 @@ def process_synced_image(*, image_path: str, sync_run_id: int) -> None: Process a single image as part of sync run *sync_run_id* and record progress. Composes the pure image-processing operation with sync-run bookkeeping so the - processing operation stays unaware of sync: non-Fujifilm files count as - skipped, unexpected failures are logged and counted as errors (the run - continues), and successful imports count as processed. When every image in the - run has a terminal outcome, the run is completed. + processing operation stays unaware of sync: files that cannot produce a recipe + count as skipped, whether because they carry no Fujifilm metadata or because + their EXIF fails recipe validation; unexpected failures are logged and counted + as errors (the run continues); and successful imports count as processed. When + every image in the run has a terminal outcome, the run is completed. If the run no longer exists (its folder was removed while this work was queued), the call returns without processing. @@ -30,6 +33,19 @@ def process_synced_image(*, image_path: str, sync_run_id: int) -> None: image_operations.process_image(image_path=image_path) except NoFilmSimulationError: run.record_skipped() + image_events.publish_event( + event_type=image_events.IMAGE_IMPORT_SKIPPED, + image_path=image_path, + reason=image_events.SKIP_REASON_NO_FILM_SIMULATION, + ) + except recipe_validation.InvalidFujifilmRecipeData as exc: + run.record_skipped() + image_events.publish_event( + event_type=image_events.IMAGE_IMPORT_SKIPPED, + image_path=image_path, + reason=image_events.SKIP_REASON_INVALID_RECIPE_DATA, + recipe_field=exc.field, + ) except Exception: logger.exception("Failed to process image during sync") run.record_error() diff --git a/src/application/usecases/recipes/import_recipes_from_uploaded_files.py b/src/application/usecases/recipes/import_recipes_from_uploaded_files.py index 4d5b3f4..112fa44 100644 --- a/src/application/usecases/recipes/import_recipes_from_uploaded_files.py +++ b/src/application/usecases/recipes/import_recipes_from_uploaded_files.py @@ -5,6 +5,7 @@ from src.domain.images.queries import NoFilmSimulationError from src.domain.recipes import dataclasses as recipe_dataclasses from src.domain.recipes import operations +from src.domain.recipes import validation as recipe_validation def import_recipes_from_uploaded_files( @@ -17,8 +18,9 @@ def import_recipes_from_uploaded_files( the recipe is extracted, and the temporary file is deleted immediately afterwards — whether the operation succeeds or fails. - Files that do not contain Fujifilm recipe EXIF data are recorded as - failures; processing continues with the remaining files. + Files that do not contain Fujifilm recipe EXIF data, and files whose EXIF + cannot produce a valid recipe, are recorded as failures; processing + continues with the remaining files. Returns an ImportRecipesResult describing which files were imported and which failed. @@ -34,7 +36,7 @@ def import_recipes_from_uploaded_files( tmp_path = tmp.name recipe, _ = operations.get_or_create_recipe_from_filepath(filepath=tmp_path) imported.append(recipe) - except NoFilmSimulationError: + except (NoFilmSimulationError, recipe_validation.InvalidFujifilmRecipeData): failed.append(file.name) finally: if tmp_path is not None: diff --git a/src/domain/images/events.py b/src/domain/images/events.py index 62510e4..2b4d730 100644 --- a/src/domain/images/events.py +++ b/src/domain/images/events.py @@ -22,10 +22,15 @@ IMAGE_RATING_SET = "image.rating.set" IMAGE_RATING_FAILED = "image.rating.failed" IMAGE_DEDUP_FILE_MISSING = "image.dedup.file.missing" +IMAGE_IMPORT_SKIPPED = "image.import.skipped" TASK_IMAGE_ENQUEUED = "task.image.enqueued" TASK_IMAGE_STARTED = "task.image.started" TASK_IMAGE_COMPLETED = "task.image.completed" +# Values carried on the `reason` field of IMAGE_IMPORT_SKIPPED. +SKIP_REASON_NO_FILM_SIMULATION = "no_film_simulation" +SKIP_REASON_INVALID_RECIPE_DATA = "invalid_recipe_data" + def publish_event(*, event_type: str, **kwargs: object) -> None: """ diff --git a/src/domain/images/operations.py b/src/domain/images/operations.py index a183c0a..b5d5aa0 100644 --- a/src/domain/images/operations.py +++ b/src/domain/images/operations.py @@ -126,6 +126,8 @@ def process_image(*, image_path: str) -> models.Image: Raises: NoFilmSimulationError: If the image has no film simulation EXIF data. + InvalidFujifilmRecipeData: If the image's EXIF cannot produce a valid + recipe. Nothing is persisted: the whole call is one transaction. """ metadata = queries.read_image_exif(image_path=image_path) diff --git a/src/domain/recipes/operations.py b/src/domain/recipes/operations.py index 8a4276d..fd31052 100644 --- a/src/domain/recipes/operations.py +++ b/src/domain/recipes/operations.py @@ -387,6 +387,9 @@ def get_or_create_recipe_from_metadata( Create or retrieve a FujifilmRecipe for the given parsed EXIF data. :raises NoFilmSimulationError: If the EXIF data contains no known film simulation. + :raises InvalidFujifilmRecipeData: If the EXIF data cannot produce a valid + recipe, e.g. a colour simulation whose Color the camera set rather than + the user, leaving no value to store. """ try: recipe_data = image_queries.exif_to_recipe(exif=metadata) @@ -402,6 +405,7 @@ def get_or_create_recipe_from_filepath( Read EXIF from *filepath* and return the matching FujifilmRecipe, creating it if needed. :raises NoFilmSimulationError: If the file is not a Fujifilm image or has no film simulation. + :raises InvalidFujifilmRecipeData: If the file's EXIF cannot produce a valid recipe. """ metadata = image_queries.read_image_exif(image_path=filepath) if metadata.camera_make.upper() != "FUJIFILM": diff --git a/src/interfaces/management/commands/process_images.py b/src/interfaces/management/commands/process_images.py index 02e2429..692ee24 100644 --- a/src/interfaces/management/commands/process_images.py +++ b/src/interfaces/management/commands/process_images.py @@ -16,9 +16,18 @@ def handle(self, *args: object, **options: Any) -> None: folder = options["folder"] self.stdout.write(f"Scanning {folder} for JPG files…") - total = process_images.import_images_from_folder(folder=folder) + summary = process_images.import_images_from_folder(folder=folder) if settings.USE_ASYNC_TASKS: - self.stdout.write(self.style.SUCCESS(f"Successfully enqueued {total} tasks.")) - else: - self.stdout.write(self.style.SUCCESS(f"Successfully processed {total} images.")) + self.stdout.write(self.style.SUCCESS(f"Successfully enqueued {summary.total} tasks.")) + return + + self.stdout.write(self.style.SUCCESS( + f"Successfully processed {summary.processed} of {summary.total} images." + )) + if summary.skipped: + self.stdout.write(f"Skipped {len(summary.skipped)} image(s) that cannot produce a recipe.") + # The full list is behind -v 2 so a large skip run does not swamp the output. + if options["verbosity"] >= 2: + for path in summary.skipped: + self.stdout.write(f" skipped: {path}") diff --git a/src/interfaces/tasks.py b/src/interfaces/tasks.py index b85db14..3666de6 100644 --- a/src/interfaces/tasks.py +++ b/src/interfaces/tasks.py @@ -7,6 +7,7 @@ from src.application.usecases.library.process_synced_image import process_synced_image from src.domain.images import events, operations from src.domain.images.thumbnails import operations as thumbnail_operations +from src.domain.recipes import validation as recipe_validation @shared_task(name="domain.process_image", bind=True, queue=settings.PROCESS_IMAGE_QUEUE) @@ -22,7 +23,20 @@ def process_image_task(self: Any, /, *, image_path: str, **kwargs: object) -> st try: recipe = operations.process_image(image_path=image_path) except operations.NoFilmSimulationError: + events.publish_event( + event_type=events.IMAGE_IMPORT_SKIPPED, + image_path=image_path, + reason=events.SKIP_REASON_NO_FILM_SIMULATION, + ) return f"Skipped {image_path} (no film simulation)" + except recipe_validation.InvalidFujifilmRecipeData as exc: + events.publish_event( + event_type=events.IMAGE_IMPORT_SKIPPED, + image_path=image_path, + reason=events.SKIP_REASON_INVALID_RECIPE_DATA, + recipe_field=exc.field, + ) + return f"Skipped {image_path} (invalid recipe data)" events.publish_event( event_type=events.TASK_IMAGE_COMPLETED, image_path=image_path, diff --git a/tests/functional/test_process_image_task.py b/tests/functional/test_process_image_task.py new file mode 100644 index 0000000..3a8b4dd --- /dev/null +++ b/tests/functional/test_process_image_task.py @@ -0,0 +1,64 @@ +import shutil +from pathlib import Path + +import pytest + +from src.data import models +from src.domain.images import events +from src.interfaces.tasks import process_image_task + +FIXTURES_DIR = Path(__file__).resolve().parent.parent / "fixtures" / "images" +FUJIFILM_FIXTURE = FIXTURES_DIR / "XS107114.JPG" +NON_FUJIFILM_FIXTURE = FIXTURES_DIR / "sub-folder" / "img_4968_dng_embedded.jpg" +# The camera set this file's Saturation, so its EXIF cannot produce a valid recipe. +CAMERA_CONTROLLED_FIXTURE = ( + Path(__file__).resolve().parent.parent / "fixtures" / "recipe" / "film_simulation_eterna.jpg" +) + + +def _skip_events(captured_logs): + return [e for e in captured_logs if e.get("event_type") == events.IMAGE_IMPORT_SKIPPED] + + +def _run_task(fixture: Path, tmp_path: Path) -> str: + image_path = tmp_path / fixture.name + shutil.copy(fixture, image_path) + return process_image_task.apply(kwargs={"image_path": str(image_path)}).get() + + +@pytest.mark.django_db +class TestProcessImageTask: + def test_processes_a_valid_image(self, tmp_path): + result = _run_task(FUJIFILM_FIXTURE, tmp_path) + + assert "Processed" in result + assert models.Image.objects.count() == 1 + + def test_skips_an_image_whose_recipe_data_is_invalid(self, tmp_path): + result = _run_task(CAMERA_CONTROLLED_FIXTURE, tmp_path) + + assert "Skipped" in result + assert "invalid recipe data" in result + assert models.Image.objects.count() == 0 + + def test_skips_an_image_with_no_film_simulation(self, tmp_path): + result = _run_task(NON_FUJIFILM_FIXTURE, tmp_path) + + assert "Skipped" in result + assert "no film simulation" in result + assert models.Image.objects.count() == 0 + + def test_publishes_an_import_skipped_event_for_invalid_recipe_data(self, tmp_path, captured_logs): + _run_task(CAMERA_CONTROLLED_FIXTURE, tmp_path) + + skipped = _skip_events(captured_logs) + assert len(skipped) == 1 + assert skipped[0]["reason"] == events.SKIP_REASON_INVALID_RECIPE_DATA + assert skipped[0]["recipe_field"] == "color" + + def test_publishes_an_import_skipped_event_for_no_film_simulation(self, tmp_path, captured_logs): + _run_task(NON_FUJIFILM_FIXTURE, tmp_path) + + skipped = _skip_events(captured_logs) + assert len(skipped) == 1 + assert skipped[0]["reason"] == events.SKIP_REASON_NO_FILM_SIMULATION diff --git a/tests/functional/test_process_images.py b/tests/functional/test_process_images.py index b8fd6dc..43b44d8 100644 --- a/tests/functional/test_process_images.py +++ b/tests/functional/test_process_images.py @@ -1,3 +1,4 @@ +import shutil from pathlib import Path import pytest @@ -7,13 +8,17 @@ from src.data import models from src.domain.images import events -FIXTURES_DIR = str(Path(__file__).resolve().parent.parent / "fixtures" / "images") +FIXTURES_DIR = Path(__file__).resolve().parent.parent / "fixtures" / "images" +# The camera set this file's Saturation, so its EXIF cannot produce a valid recipe. +CAMERA_CONTROLLED_FIXTURE = ( + Path(__file__).resolve().parent.parent / "fixtures" / "recipe" / "film_simulation_eterna.jpg" +) @pytest.mark.django_db class TestProcessImagesCommand: def test_processes_all_images_in_folder(self, capsys, captured_logs): - call_command("process_images", FIXTURES_DIR) + call_command("process_images", str(FIXTURES_DIR)) assert models.Image.objects.count() == 6 @@ -36,8 +41,47 @@ def test_processes_all_images_in_folder(self, capsys, captured_logs): class TestProcessImagesCommandSync: def test_processes_images_sequentially(self, capsys): with override_settings(USE_ASYNC_TASKS=False): - call_command("process_images", FIXTURES_DIR) + call_command("process_images", str(FIXTURES_DIR)) assert models.Image.objects.count() == 6 captured = capsys.readouterr() - assert "Successfully processed 7 images." in captured.out + assert "Successfully processed 6 of 7 images." in captured.out + + +@pytest.mark.django_db +class TestProcessImagesCommandSkips: + def test_reports_skipped_images_without_aborting_the_run(self, capsys, tmp_path): + shutil.copy(FIXTURES_DIR / "XS107114.JPG", tmp_path / "good.jpg") + shutil.copy(CAMERA_CONTROLLED_FIXTURE, tmp_path / "camera_controlled.jpg") + + with override_settings(USE_ASYNC_TASKS=False): + call_command("process_images", str(tmp_path)) + + # The valid image is still imported: the unusable one must not abort the run. + assert models.Image.objects.count() == 1 + captured = capsys.readouterr() + assert "Successfully processed 1 of 2 images." in captured.out + assert "Skipped 1 image(s) that cannot produce a recipe." in captured.out + assert "camera_controlled.jpg" not in captured.out + + def test_lists_skipped_paths_at_higher_verbosity(self, capsys, tmp_path): + shutil.copy(CAMERA_CONTROLLED_FIXTURE, tmp_path / "camera_controlled.jpg") + + with override_settings(USE_ASYNC_TASKS=False): + call_command("process_images", str(tmp_path), verbosity=2) + + assert models.Image.objects.count() == 0 + captured = capsys.readouterr() + assert "skipped: " in captured.out + assert "camera_controlled.jpg" in captured.out + + def test_publishes_an_import_skipped_event(self, tmp_path, captured_logs): + shutil.copy(CAMERA_CONTROLLED_FIXTURE, tmp_path / "camera_controlled.jpg") + + with override_settings(USE_ASYNC_TASKS=False): + call_command("process_images", str(tmp_path)) + + skipped = [e for e in captured_logs if e.get("event_type") == events.IMAGE_IMPORT_SKIPPED] + assert len(skipped) == 1 + assert skipped[0]["reason"] == events.SKIP_REASON_INVALID_RECIPE_DATA + assert skipped[0]["recipe_field"] == "color" diff --git a/tests/integration/application/library/test_process_synced_image.py b/tests/integration/application/library/test_process_synced_image.py index 638eefe..62d1ef0 100644 --- a/tests/integration/application/library/test_process_synced_image.py +++ b/tests/integration/application/library/test_process_synced_image.py @@ -6,11 +6,17 @@ from src.application.usecases.library.process_synced_image import process_synced_image from src.data import models +from src.domain.images import events from tests.factories import SyncRunFactory FIXTURES_DIR = Path(__file__).resolve().parent.parent.parent.parent / "fixtures" / "images" FUJIFILM_FIXTURE = FIXTURES_DIR / "XS107114.JPG" NON_FUJIFILM_FIXTURE = FIXTURES_DIR / "sub-folder" / "img_4968_dng_embedded.jpg" +# The camera set this file's Saturation, so its EXIF cannot produce a valid recipe. +CAMERA_CONTROLLED_FIXTURE = ( + Path(__file__).resolve().parent.parent.parent.parent + / "fixtures" / "recipe" / "film_simulation_eterna.jpg" +) @pytest.mark.django_db @@ -39,6 +45,30 @@ def test_records_skipped_for_non_fujifilm_image(self, tmp_path): assert run.skipped == 1 assert run.processed == 0 + def test_records_skipped_for_an_image_that_fails_recipe_validation(self, tmp_path): + image_path = tmp_path / CAMERA_CONTROLLED_FIXTURE.name + shutil.copy(CAMERA_CONTROLLED_FIXTURE, image_path) + run = SyncRunFactory(state=models.SyncRun.STATE_PROCESSING, total=1) + + process_synced_image(image_path=str(image_path), sync_run_id=run.pk) + + run.refresh_from_db() + assert run.skipped == 1 + assert run.errors == 0 + assert run.processed == 0 + + def test_publishes_an_import_skipped_event_for_invalid_recipe_data(self, tmp_path, captured_logs): + image_path = tmp_path / CAMERA_CONTROLLED_FIXTURE.name + shutil.copy(CAMERA_CONTROLLED_FIXTURE, image_path) + run = SyncRunFactory(state=models.SyncRun.STATE_PROCESSING, total=1) + + process_synced_image(image_path=str(image_path), sync_run_id=run.pk) + + skipped = [e for e in captured_logs if e.get("event_type") == events.IMAGE_IMPORT_SKIPPED] + assert len(skipped) == 1 + assert skipped[0]["reason"] == events.SKIP_REASON_INVALID_RECIPE_DATA + assert skipped[0]["recipe_field"] == "color" + def test_records_error_and_continues_on_unexpected_failure(self): run = SyncRunFactory(state=models.SyncRun.STATE_PROCESSING, total=1) diff --git a/tests/integration/application/recipes/test_import_recipes_from_uploaded_files.py b/tests/integration/application/recipes/test_import_recipes_from_uploaded_files.py index 98f348e..b72af67 100644 --- a/tests/integration/application/recipes/test_import_recipes_from_uploaded_files.py +++ b/tests/integration/application/recipes/test_import_recipes_from_uploaded_files.py @@ -9,6 +9,11 @@ from src.domain.recipes.dataclasses import ImportRecipesResult, UploadedFile FIXTURES_DIR = Path(__file__).resolve().parent.parent.parent.parent / "fixtures" / "images" +# The camera set this file's Saturation, so its EXIF cannot produce a valid recipe. +CAMERA_CONTROLLED_FIXTURE = ( + Path(__file__).resolve().parent.parent.parent.parent + / "fixtures" / "recipe" / "film_simulation_eterna.jpg" +) def uploaded_file_from_fixture(filename: str) -> UploadedFile: @@ -62,6 +67,33 @@ def test_records_failure_for_non_fujifilm_file(self): assert result.imported == () assert result.failed == ("canon.jpg",) + def test_records_failure_for_a_file_that_fails_recipe_validation(self): + files = [ + UploadedFile( + name="camera_controlled.jpg", + content=CAMERA_CONTROLLED_FIXTURE.read_bytes(), + ) + ] + + result = import_recipes_from_uploaded_files(files=files) + + assert result.imported == () + assert result.failed == ("camera_controlled.jpg",) + + def test_continues_after_a_file_that_fails_recipe_validation(self): + files = [ + UploadedFile( + name="camera_controlled.jpg", + content=CAMERA_CONTROLLED_FIXTURE.read_bytes(), + ), + uploaded_file_from_fixture("XS107114.JPG"), + ] + + result = import_recipes_from_uploaded_files(files=files) + + assert len(result.imported) == 1 + assert result.failed == ("camera_controlled.jpg",) + def test_continues_after_failure_and_processes_remaining_files(self): non_fujifilm = UploadedFile(name="bad.jpg", content=b"\xff\xd8\xff\xd9") files = [ diff --git a/tests/integration/domain/images/test_operations.py b/tests/integration/domain/images/test_operations.py index 469dd54..35c2cbf 100644 --- a/tests/integration/domain/images/test_operations.py +++ b/tests/integration/domain/images/test_operations.py @@ -9,6 +9,7 @@ from src.domain.images import events from src.domain.images.dataclasses import ImageExifData from src.domain.images.operations import NoFilmSimulationError, process_image +from src.domain.recipes.validation import InvalidFujifilmRecipeData FIXTURES_DIR = Path(__file__).resolve().parent.parent.parent.parent / "fixtures" / "images" FIXTURE_IMAGE = str(FIXTURES_DIR / "XS107114.JPG") @@ -185,3 +186,30 @@ def test_raises_when_film_simulation_and_color_are_empty(self): process_image(image_path="any/path.jpg") assert models.Image.objects.count() == 0 + + +@pytest.mark.django_db +class TestProcessImageInvalidRecipeData: + def test_raises_for_an_image_whose_color_the_camera_set(self): + """The camera writes the sentinel 'Film Simulation' into Color when it, not + the user, drives saturation. A colour simulation with no Color value cannot + produce a valid recipe.""" + image_path = str(RECIPE_FIXTURES_DIR / "film_simulation_eterna.jpg") + + with pytest.raises(InvalidFujifilmRecipeData) as exc_info: + process_image(image_path=image_path) + + assert exc_info.value.field == "color" + assert exc_info.value.value is None + + def test_leaves_no_partial_rows_behind(self): + """process_image is one transaction, so a rejected recipe must not leave the + FujifilmExif row it creates before validation runs.""" + image_path = str(RECIPE_FIXTURES_DIR / "film_simulation_eterna.jpg") + + with pytest.raises(InvalidFujifilmRecipeData): + process_image(image_path=image_path) + + assert models.Image.objects.count() == 0 + assert models.FujifilmExif.objects.count() == 0 + assert models.FujifilmRecipe.objects.count() == 0 diff --git a/tests/unit/application/images/test_process_images.py b/tests/unit/application/images/test_process_images.py new file mode 100644 index 0000000..9c5e797 --- /dev/null +++ b/tests/unit/application/images/test_process_images.py @@ -0,0 +1,108 @@ +from unittest.mock import patch + +from django.test import override_settings + +from src.application.usecases.images import process_images as uc +from src.domain.images import events +from src.domain.images.queries import NoFilmSimulationError +from src.domain.recipes.validation import InvalidFujifilmRecipeData + +_COLLECT = "src.application.usecases.images.process_images.queries.collect_image_paths" +_PROCESS = "src.application.usecases.images.process_images.operations.process_image" + + +def _skip_events(captured_logs: list[dict[str, object]]) -> list[dict[str, object]]: + return [e for e in captured_logs if e.get("event_type") == events.IMAGE_IMPORT_SKIPPED] + + +class TestProcessImagesInFolder: + def test_continues_after_an_image_that_fails_recipe_validation(self) -> None: + paths = ["a.jpg", "bad.jpg", "c.jpg"] + side_effect = [None, InvalidFujifilmRecipeData("color", None), None] + + with patch(_COLLECT, return_value=paths): + with patch(_PROCESS, side_effect=side_effect) as mock_process: + summary = uc._process_images_in_folder(folder="/photos") + + assert mock_process.call_count == 3 + assert summary.processed == 2 + assert summary.skipped == ("bad.jpg",) + + def test_continues_after_an_image_with_no_film_simulation(self) -> None: + paths = ["a.jpg", "canon.jpg", "c.jpg"] + side_effect = [None, NoFilmSimulationError("canon.jpg"), None] + + with patch(_COLLECT, return_value=paths): + with patch(_PROCESS, side_effect=side_effect) as mock_process: + summary = uc._process_images_in_folder(folder="/photos") + + assert mock_process.call_count == 3 + assert summary.processed == 2 + assert summary.skipped == ("canon.jpg",) + + def test_reports_processed_and_skipped_counts(self) -> None: + paths = ["a.jpg", "canon.jpg", "bad.jpg"] + side_effect = [ + None, + NoFilmSimulationError("canon.jpg"), + InvalidFujifilmRecipeData("color", None), + ] + + with patch(_COLLECT, return_value=paths): + with patch(_PROCESS, side_effect=side_effect): + summary = uc._process_images_in_folder(folder="/photos") + + assert summary.total == 3 + assert summary.processed == 1 + assert summary.skipped == ("canon.jpg", "bad.jpg") + + def test_publishes_an_import_skipped_event_per_skipped_image(self, captured_logs) -> None: + paths = ["canon.jpg", "bad.jpg"] + side_effect = [ + NoFilmSimulationError("canon.jpg"), + InvalidFujifilmRecipeData("color", None), + ] + + with patch(_COLLECT, return_value=paths): + with patch(_PROCESS, side_effect=side_effect): + uc._process_images_in_folder(folder="/photos") + + skipped = _skip_events(captured_logs) + assert len(skipped) == 2 + assert skipped[0]["image_path"] == "canon.jpg" + assert skipped[0]["reason"] == events.SKIP_REASON_NO_FILM_SIMULATION + assert skipped[1]["image_path"] == "bad.jpg" + assert skipped[1]["reason"] == events.SKIP_REASON_INVALID_RECIPE_DATA + assert skipped[1]["recipe_field"] == "color" + + def test_publishes_no_event_when_every_image_is_processed(self, captured_logs) -> None: + with patch(_COLLECT, return_value=["a.jpg", "b.jpg"]): + with patch(_PROCESS, return_value=None): + summary = uc._process_images_in_folder(folder="/photos") + + assert summary.skipped == () + assert _skip_events(captured_logs) == [] + + +class TestImportImagesFromFolder: + def test_reports_only_the_enqueued_total_in_async_mode(self) -> None: + with override_settings(USE_ASYNC_TASKS=True): + with patch(_COLLECT, return_value=["a.jpg", "b.jpg"]): + with patch("src.application.usecases.images.process_images.workertasks.enqueue_task"): + summary = uc.import_images_from_folder(folder="/photos") + + assert summary.total == 2 + assert summary.processed == 0 + assert summary.skipped == () + + def test_reports_per_file_outcomes_in_sync_mode(self) -> None: + side_effect = [None, InvalidFujifilmRecipeData("color", None)] + + with override_settings(USE_ASYNC_TASKS=False): + with patch(_COLLECT, return_value=["a.jpg", "bad.jpg"]): + with patch(_PROCESS, side_effect=side_effect): + summary = uc.import_images_from_folder(folder="/photos") + + assert summary.total == 2 + assert summary.processed == 1 + assert summary.skipped == ("bad.jpg",)