From a7c660d78bff1353dbeb30977bf294074899d716 Mon Sep 17 00:00:00 2001 From: Christophe Lallement Date: Sat, 22 Aug 2026 17:51:48 -0400 Subject: [PATCH] chore(clean-arc): Add nullaway. --- .mvn/jvm.config | 10 + pom.xml | 143 +++++++++++- src/lombok.config | 10 + .../org/icroco/pholio/cli/PholioCommand.java | 1 + .../org/icroco/pholio/cli/ScanCommand.java | 6 +- .../pholio/domain/library/LibraryFolder.java | 19 ++ .../domain/library/LibrarySubfolder.java | 19 ++ .../pholio/domain/media/GeoLocation.java | 4 +- .../pholio/domain/media/ImageFormat.java | 4 +- .../pholio/domain/media/MediaMetadata.java | 38 +-- .../pholio/domain/media/Orientation.java | 4 +- .../pholio/infra/library/LibraryCatalog.java | 5 +- .../library/LibraryFolderAddedEvent.java | 11 + .../infra/library/LibraryFolderScanner.java | 74 ++++++ .../infra/library/LibraryFolderService.java | 143 ++++++++++++ .../pholio/infra/library/LibraryRouter.java | 4 +- .../pholio/infra/library/LibraryService.java | 7 +- .../infra/media/ExifThumbnailAccessor.java | 5 +- .../infra/media/MediaFormatRegistry.java | 5 +- .../infra/media/MetadataExtractorReader.java | 17 +- .../persistence/LibraryRoutingDataSource.java | 9 +- .../persistence/PersistenceConfiguration.java | 18 +- .../folder/LibraryFolderEntity.java | 31 +++ .../folder/LibraryFolderMapper.java | 18 ++ .../folder/LibraryFolderRepository.java | 12 + .../folder/LibrarySubfolderEntity.java | 33 +++ .../folder/LibrarySubfolderMapper.java | 18 ++ .../folder/LibrarySubfolderRepository.java | 12 + .../infra/preferences/AppPreferences.java | 7 +- .../infra/preferences/PreferenceItem.java | 15 +- .../infra/preferences/PreferenceService.java | 7 +- .../infra/preferences/PreferenceType.java | 4 +- .../infra/preferences/WindowGeometry.java | 17 +- .../pholio/infra/support/AppDirectories.java | 4 +- .../pholio/infra/support/Debouncer.java | 3 +- .../icroco/pholio/infra/task/TaskBody.java | 4 +- .../pholio/infra/task/TaskProgressEvent.java | 4 +- .../icroco/pholio/infra/task/TaskService.java | 46 ++-- .../java/org/icroco/pholio/package-info.java | 4 + .../icroco/pholio/ui/PholioFxApplication.java | 8 +- .../org/icroco/pholio/ui/ViewSwitcher.java | 31 ++- .../org/icroco/pholio/ui/debug/DevTools.java | 36 +-- .../pholio/ui/event/NavigateToViewEvent.java | 4 +- .../icroco/pholio/ui/i18n/I18nService.java | 13 +- .../ui/selection/ViewportSelection.java | 3 +- .../pholio/ui/shell/LibraryFolderTree.java | 218 ++++++++++++++---- .../icroco/pholio/ui/shell/ModalService.java | 12 +- .../pholio/ui/shell/NavigationDrawer.java | 22 +- .../ui/shell/NavigationDrawerSection.java | 12 + .../pholio/ui/shell/NavigationRail.java | 7 +- .../icroco/pholio/ui/shell/SettingsView.java | 8 +- .../icroco/pholio/ui/shell/ToastLayer.java | 3 +- .../org/icroco/pholio/ui/theme/AppTheme.java | 5 +- .../icroco/pholio/ui/view/GalleryView.java | 41 ++-- .../ui/window/ScreenBoundsValidator.java | 4 +- .../pholio/ui/window/WindowStateManager.java | 60 +++-- src/main/resources/application.yaml | 12 +- src/main/resources/css/pholio.css | 24 ++ .../migration/V1__create_library_folder.sql | 21 ++ src/main/resources/messages.properties | 13 +- src/main/resources/messages_fr.properties | 13 +- .../library/LibraryFolderScannerTest.java | 68 ++++++ .../library/LibraryFolderServiceTest.java | 161 +++++++++++++ .../infra/library/LibraryServiceTest.java | 7 +- .../infra/media/MediaFormatRegistryTest.java | 3 + .../folder/LibraryFolderMapperTest.java | 50 ++++ .../folder/LibrarySubfolderMapperTest.java | 37 +++ .../infra/preferences/PreferenceItemTest.java | 6 +- .../org/icroco/pholio/ui/FxTestToolkit.java | 9 +- .../ui/PreferenceSchemaCouplingTest.java | 3 +- .../icroco/pholio/ui/debug/DevToolsTest.java | 4 +- .../ui/shell/LibraryFolderTreeTest.java | 156 +++++++++---- .../pholio/ui/shell/NavigationDrawerTest.java | 63 +++-- .../ui/shell/NavigationRailResizeTest.java | 3 +- .../pholio/ui/shell/NavigationRailTest.java | 10 +- src/test/resources/application.yaml | 4 +- 76 files changed, 1624 insertions(+), 325 deletions(-) create mode 100644 .mvn/jvm.config create mode 100644 src/lombok.config create mode 100644 src/main/java/org/icroco/pholio/domain/library/LibraryFolder.java create mode 100644 src/main/java/org/icroco/pholio/domain/library/LibrarySubfolder.java create mode 100644 src/main/java/org/icroco/pholio/infra/library/LibraryFolderAddedEvent.java create mode 100644 src/main/java/org/icroco/pholio/infra/library/LibraryFolderScanner.java create mode 100644 src/main/java/org/icroco/pholio/infra/library/LibraryFolderService.java create mode 100644 src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderEntity.java create mode 100644 src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapper.java create mode 100644 src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderRepository.java create mode 100644 src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderEntity.java create mode 100644 src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapper.java create mode 100644 src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderRepository.java create mode 100644 src/main/java/org/icroco/pholio/package-info.java create mode 100644 src/main/resources/db/migration/V1__create_library_folder.sql create mode 100644 src/test/java/org/icroco/pholio/infra/library/LibraryFolderScannerTest.java create mode 100644 src/test/java/org/icroco/pholio/infra/library/LibraryFolderServiceTest.java create mode 100644 src/test/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapperTest.java create mode 100644 src/test/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapperTest.java diff --git a/.mvn/jvm.config b/.mvn/jvm.config new file mode 100644 index 0000000..504456f --- /dev/null +++ b/.mvn/jvm.config @@ -0,0 +1,10 @@ +--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED +--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED +--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED +--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED diff --git a/pom.xml b/pom.xml index dccc1da..8ce98f4 100644 --- a/pom.xml +++ b/pom.xml @@ -1,6 +1,6 @@ + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 @@ -22,7 +22,7 @@ Extra themes for AtlantaFx, from DLSC. Pinned to a release present in the local repository so an offline build keeps working; Maven Central carries newer ones. --> - 1.5.0 + 1.9.0 2.1.0 1.0.1 + 2.50.0 12.4.0 org.icroco.pholio.PholioApplication + + 1.7.0.Beta2 + 3.15.0 + 3.6.3 2.21.0 + 0.14.0 4.7.7 UTF-8 4.0.0 @@ -107,6 +113,16 @@ org.flywaydb flyway-core + + + org.springframework.boot + spring-boot-flyway + @@ -230,6 +246,12 @@ lombok provided + + + org.mapstruct + mapstruct + ${mapstruct.version} + org.threeten threeten-extra @@ -293,9 +315,30 @@ + + org.apache.maven.plugins + maven-enforcer-plugin + ${maven-enforcer-plugin.version} + + + enforce-maven + + enforce + + + + + 3.9 + + + + + + org.apache.maven.plugins maven-compiler-plugin + ${maven-compiler-plugin.version} + + org.projectlombok + lombok-mapstruct-binding + 0.2.0 + + + org.mapstruct + mapstruct-processor + ${mapstruct.version} + org.springframework.boot spring-boot-configuration-processor + + com.google.errorprone + error_prone_core + ${errorprone.version} + + + com.uber.nullaway + nullaway + ${nullaway.version} + -Xlint:all,-serial,-processing,-this-escape + -XDcompilePolicy=simple + -XDshould-stop.ifError=FLOW + -Xplugin:ErrorProne + -Xep:NullAway:ERROR + -XepOpt:NullAway:AnnotatedPackages=org.icroco.pholio + -XepOpt:NullAway:CustomNullableAnnotations=org.jspecify.annotations.Nullable + -XepOpt:NullAway:CheckOptionalEmptiness=true + -XepOpt:NullAway:AcknowledgeRestrictiveAnnotations=true + -XepExcludedPaths:.*/generated-sources/.* + + + -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + org.apache.maven.plugins maven-surefire-plugin diff --git a/src/lombok.config b/src/lombok.config new file mode 100644 index 0000000..f6975f1 --- /dev/null +++ b/src/lombok.config @@ -0,0 +1,10 @@ +config.stopBubbling = true + +# 1. Copy JSpecify annotations to generated fields, getters, setters, and builder parameters +lombok.copyableAnnotations += org.jspecify.annotations.Nullable +lombok.copyableAnnotations += org.jspecify.annotations.NonNull +lombok.copyableAnnotations += org.jspecify.annotations.NullMarked + +# 2. Tell Lombok to explicitly mark generated builder methods/classes as @NullMarked +# so JSpecify rules apply inside the generated builder code +lombok.addNullAnnotations = jspecify diff --git a/src/main/java/org/icroco/pholio/cli/PholioCommand.java b/src/main/java/org/icroco/pholio/cli/PholioCommand.java index 90554bf..c86e0bd 100644 --- a/src/main/java/org/icroco/pholio/cli/PholioCommand.java +++ b/src/main/java/org/icroco/pholio/cli/PholioCommand.java @@ -34,6 +34,7 @@ public class PholioCommand implements Callable { private UiModeOptions uiMode = new UiModeOptions(); @Spec + @SuppressWarnings("NullAway.Init") // picocli injects this after construction, before call() private CommandSpec spec; @Override diff --git a/src/main/java/org/icroco/pholio/cli/ScanCommand.java b/src/main/java/org/icroco/pholio/cli/ScanCommand.java index 126f80c..47c0167 100644 --- a/src/main/java/org/icroco/pholio/cli/ScanCommand.java +++ b/src/main/java/org/icroco/pholio/cli/ScanCommand.java @@ -2,6 +2,7 @@ package org.icroco.pholio.cli; import org.icroco.pholio.infra.media.MediaFormatRegistry; import org.icroco.pholio.infra.preferences.AppPreferences; +import org.jspecify.annotations.Nullable; import org.springframework.stereotype.Component; import picocli.CommandLine.Command; import picocli.CommandLine.Option; @@ -43,7 +44,8 @@ public class ScanCommand implements Callable { arity = "0..1", paramLabel = "", description = "Directory to scan. Defaults to the configured library root.") - private Path directory; + @SuppressWarnings("NullAway.Init") // picocli injects this after construction, before call(); absent when arity 0 + private @Nullable Path directory; /** * A plain opt-out flag rather than a {@code negatable = true} {@code --recursive}. @@ -100,7 +102,7 @@ public class ScanCommand implements Callable { return 0; } - private Path resolveTarget() { + private @Nullable Path resolveTarget() { if (directory != null) { return directory; } diff --git a/src/main/java/org/icroco/pholio/domain/library/LibraryFolder.java b/src/main/java/org/icroco/pholio/domain/library/LibraryFolder.java new file mode 100644 index 0000000..364c9a8 --- /dev/null +++ b/src/main/java/org/icroco/pholio/domain/library/LibraryFolder.java @@ -0,0 +1,19 @@ +package org.icroco.pholio.domain.library; + +import lombok.Builder; +import org.jspecify.annotations.Nullable; + +import java.nio.file.Path; +import java.time.Instant; + +/** + * One user-chosen root directory of the currently open library, persisted so it survives a restart. + * + *

Not to be confused with a "library" elsewhere in this codebase ({@code LibraryCatalog}), which names a + * whole switchable database. This is a folder inside the one currently open. + * + * @param id {@code null} before the row is persisted; the database assigns it on insert + */ +@Builder(toBuilder = true) +public record LibraryFolder(@Nullable Long id, Path path, Instant addedAt) { +} diff --git a/src/main/java/org/icroco/pholio/domain/library/LibrarySubfolder.java b/src/main/java/org/icroco/pholio/domain/library/LibrarySubfolder.java new file mode 100644 index 0000000..a155a56 --- /dev/null +++ b/src/main/java/org/icroco/pholio/domain/library/LibrarySubfolder.java @@ -0,0 +1,19 @@ +package org.icroco.pholio.domain.library; + +import lombok.Builder; +import org.jspecify.annotations.Nullable; + +import java.nio.file.Path; +import java.time.Instant; + +/** + * One directory found under a {@link LibraryFolder} root at the time it was last scanned. + * + *

Not read by anything yet — it is the foundation for a future task that diffs the filesystem against + * this table to detect added or removed subfolders, and for filtering the gallery by folder. + * + * @param id {@code null} before the row is persisted; the database assigns it on insert + */ +@Builder(toBuilder = true) +public record LibrarySubfolder(@Nullable Long id, Long libraryFolderId, Path path, Instant lastScannedAt) { +} diff --git a/src/main/java/org/icroco/pholio/domain/media/GeoLocation.java b/src/main/java/org/icroco/pholio/domain/media/GeoLocation.java index 5d9ea04..4dcdfe2 100644 --- a/src/main/java/org/icroco/pholio/domain/media/GeoLocation.java +++ b/src/main/java/org/icroco/pholio/domain/media/GeoLocation.java @@ -1,5 +1,7 @@ package org.icroco.pholio.domain.media; +import org.jspecify.annotations.Nullable; + /** * Where a photo was taken, in decimal degrees. * @@ -8,7 +10,7 @@ package org.icroco.pholio.domain.media; * @param altitude metres above sea level, {@code null} when the file records none — which is far more * common than a missing position, since many cameras log GPS without a barometer */ -public record GeoLocation(double latitude, double longitude, Double altitude) { +public record GeoLocation(double latitude, double longitude, @Nullable Double altitude) { public GeoLocation { if (latitude < -90 || latitude > 90) { diff --git a/src/main/java/org/icroco/pholio/domain/media/ImageFormat.java b/src/main/java/org/icroco/pholio/domain/media/ImageFormat.java index 0fe49ef..8d1c63a 100644 --- a/src/main/java/org/icroco/pholio/domain/media/ImageFormat.java +++ b/src/main/java/org/icroco/pholio/domain/media/ImageFormat.java @@ -1,5 +1,7 @@ package org.icroco.pholio.domain.media; +import org.jspecify.annotations.Nullable; + import java.util.*; import java.util.stream.Collectors; @@ -91,7 +93,7 @@ public enum ImageFormat { *

Answers what a name means, not whether the application can handle it. Ask * {@code MediaFormatRegistry} for that. */ - public static Optional ofExtension(String extension) { + public static Optional ofExtension(@Nullable String extension) { if (extension == null || extension.isBlank()) { return Optional.empty(); } diff --git a/src/main/java/org/icroco/pholio/domain/media/MediaMetadata.java b/src/main/java/org/icroco/pholio/domain/media/MediaMetadata.java index b5478f6..ca0634a 100644 --- a/src/main/java/org/icroco/pholio/domain/media/MediaMetadata.java +++ b/src/main/java/org/icroco/pholio/domain/media/MediaMetadata.java @@ -1,9 +1,11 @@ package org.icroco.pholio.domain.media; import lombok.Builder; +import org.jspecify.annotations.Nullable; import java.time.LocalDateTime; import java.util.Map; +import java.util.Objects; import java.util.Optional; /** @@ -27,15 +29,15 @@ public record MediaMetadata(ImageFormat format, int width, int height, Orientation orientation, - LocalDateTime capturedAt, - String cameraMake, - String cameraModel, - String lens, - Integer isoSpeed, - Double aperture, - Double shutterSeconds, - Double focalLength, - GeoLocation location, + @Nullable LocalDateTime capturedAt, + @Nullable String cameraMake, + @Nullable String cameraModel, + @Nullable String lens, + @Nullable Integer isoSpeed, + @Nullable Double aperture, + @Nullable Double shutterSeconds, + @Nullable Double focalLength, + @Nullable GeoLocation location, Map raw) { public MediaMetadata { @@ -70,23 +72,25 @@ public record MediaMetadata(ImageFormat format, * Make and model joined for display, empty when the file names neither. */ public Optional camera() { - if (isBlank(cameraMake) && isBlank(cameraModel)) { + boolean makeBlank = isBlank(cameraMake); + boolean modelBlank = isBlank(cameraModel); + if (makeBlank && modelBlank) { return Optional.empty(); } - if (isBlank(cameraMake)) { - return Optional.of(cameraModel.trim()); + if (makeBlank) { + return Optional.of(Objects.requireNonNull(cameraModel).trim()); } - if (isBlank(cameraModel)) { - return Optional.of(cameraMake.trim()); + if (modelBlank) { + return Optional.of(Objects.requireNonNull(cameraMake).trim()); } - String make = cameraMake.trim(); - String model = cameraModel.trim(); + String make = Objects.requireNonNull(cameraMake).trim(); + String model = Objects.requireNonNull(cameraModel).trim(); // Many bodies repeat the make inside the model ("NIKON" / "NIKON D850"); joining blindly would // render "NIKON NIKON D850". return Optional.of(model.toLowerCase().startsWith(make.toLowerCase()) ? model : make + " " + model); } - private static boolean isBlank(String value) { + private static boolean isBlank(@Nullable String value) { return value == null || value.isBlank(); } } diff --git a/src/main/java/org/icroco/pholio/domain/media/Orientation.java b/src/main/java/org/icroco/pholio/domain/media/Orientation.java index dc9fc5e..2980a4b 100644 --- a/src/main/java/org/icroco/pholio/domain/media/Orientation.java +++ b/src/main/java/org/icroco/pholio/domain/media/Orientation.java @@ -1,5 +1,7 @@ package org.icroco.pholio.domain.media; +import org.jspecify.annotations.Nullable; + /** * How a stored image must be transformed before it is displayed, as recorded by EXIF tag 0x0112. * @@ -62,7 +64,7 @@ public enum Orientation { *

Absent and out-of-range are treated the same on purpose: a camera writing garbage here should give * an upright photo, not an exception in the middle of a library scan. */ - public static Orientation ofExifValue(Integer exifValue) { + public static Orientation ofExifValue(@Nullable Integer exifValue) { if (exifValue == null) { return NORMAL; } diff --git a/src/main/java/org/icroco/pholio/infra/library/LibraryCatalog.java b/src/main/java/org/icroco/pholio/infra/library/LibraryCatalog.java index ca339ff..07e9f54 100644 --- a/src/main/java/org/icroco/pholio/infra/library/LibraryCatalog.java +++ b/src/main/java/org/icroco/pholio/infra/library/LibraryCatalog.java @@ -1,6 +1,7 @@ package org.icroco.pholio.infra.library; import org.icroco.pholio.infra.support.AppDirectories; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,7 +81,7 @@ public final class LibraryCatalog { * append parameters to the JDBC URL the name is interpolated into. Leading dots are stripped too, so a * name can never produce a hidden file or a {@code ..} traversal. */ - public static String sanitize(String name) { + public static String sanitize(@Nullable String name) { if (name == null || name.isBlank()) { return FALLBACK_NAME; } @@ -110,7 +111,7 @@ public final class LibraryCatalog { *

Guards the case where the user deleted the database file that {@code library.last-opened} still * points at: without this the application would silently recreate an empty database under the old name. */ - public static String resolve(String candidate, List available) { + public static String resolve(@Nullable String candidate, List available) { String name = candidate == null ? "" : sanitize(candidate); if (!name.isBlank() && available.contains(name)) { return name; diff --git a/src/main/java/org/icroco/pholio/infra/library/LibraryFolderAddedEvent.java b/src/main/java/org/icroco/pholio/infra/library/LibraryFolderAddedEvent.java new file mode 100644 index 0000000..2aa704e --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/library/LibraryFolderAddedEvent.java @@ -0,0 +1,11 @@ +package org.icroco.pholio.infra.library; + +import org.icroco.pholio.domain.library.LibraryFolder; + +/** + * Published once a new root folder has been persisted, before its subfolders have been scanned. + * + *

Carries only the added folder, not the whole list, so a listener can append rather than rebuild. + */ +public record LibraryFolderAddedEvent(LibraryFolder folder) { +} diff --git a/src/main/java/org/icroco/pholio/infra/library/LibraryFolderScanner.java b/src/main/java/org/icroco/pholio/infra/library/LibraryFolderScanner.java new file mode 100644 index 0000000..68072e6 --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/library/LibraryFolderScanner.java @@ -0,0 +1,74 @@ +package org.icroco.pholio.infra.library; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.List; +import java.util.stream.Stream; + +/** + * Recursively lists every subdirectory under a {@link org.icroco.pholio.domain.library.LibraryFolder} root, + * for {@link LibraryFolderService} to persist into {@code library_subfolder}. + * + *

Unlike the UI's lazy, one-level-at-a-time {@code FolderTreeItem}, this walks the whole tree in one + * call — it runs off the FX thread, in the background, once per added root. The per-directory error + * handling and hidden-folder filter mirror {@code FolderTreeItem} exactly: an unreadable directory partway + * down the tree must not abort the rest of the scan, and dot-prefixed or OS-hidden directories are noise in + * a photo library. + * + *

An explicit stack, not recursion, so a library nested tens of thousands of directories deep cannot + * overflow the call stack. Directories are never followed through symlinks ({@link Files#list} does not + * traverse them), so there is no cycle to guard against. + */ +@Component +public class LibraryFolderScanner { + + private static final Logger log = LoggerFactory.getLogger(LibraryFolderScanner.class); + + public List scan(Path root) { + List found = new ArrayList<>(); + Deque pending = new ArrayDeque<>(); + pending.push(root); + + while (!pending.isEmpty()) { + for (Path child : subdirectoriesOf(pending.pop())) { + found.add(child); + pending.push(child); + } + } + return found; + } + + private static List subdirectoriesOf(Path directory) { + try (Stream entries = Files.list(directory)) { + return entries.filter(LibraryFolderScanner::isVisible) + .filter(Files::isDirectory) + .toList(); + } + catch (IOException | UncheckedIOException e) { + log.debug("Cannot list '{}': {}", directory, e.toString()); + return List.of(); + } + } + + private static boolean isVisible(Path path) { + Path name = path.getFileName(); + if (name == null || name.toString().startsWith(".")) { + return false; + } + try { + return !Files.isHidden(path); + } + catch (IOException e) { + return false; + } + } +} diff --git a/src/main/java/org/icroco/pholio/infra/library/LibraryFolderService.java b/src/main/java/org/icroco/pholio/infra/library/LibraryFolderService.java new file mode 100644 index 0000000..cc0b393 --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/library/LibraryFolderService.java @@ -0,0 +1,143 @@ +package org.icroco.pholio.infra.library; + +import jakarta.annotation.PostConstruct; +import org.icroco.pholio.domain.library.LibraryFolder; +import org.icroco.pholio.domain.library.LibrarySubfolder; +import org.icroco.pholio.infra.persistence.folder.*; +import org.icroco.pholio.infra.preferences.AppPreferences; +import org.icroco.pholio.infra.task.TaskService; +import org.icroco.pholio.infra.task.TaskType; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.annotation.DependsOn; +import org.springframework.stereotype.Service; + +import java.nio.file.Path; +import java.time.Instant; +import java.util.List; +import java.util.Objects; + +/** + * Owns the folders that make up the currently open library — the database-backed replacement for the old + * single {@code library.root-path} preference. + * + *

Adding a folder is two steps, deliberately not one transaction: the folder itself is saved — and + * that {@code repository.save} call commits on its own, since nothing here wraps it in a wider + * transaction — before the event fires and the background scan starts, so neither ever sees the root row + * as uncommitted. The recursive scan of its subdirectories (into {@code library_subfolder}, unused by + * anything yet — see {@link LibraryFolderScanner}) then runs on a background pool, since a library can be + * tens of thousands of directories deep. + * + *

{@code @DependsOn(LibraryService)}: without it, this bean's own {@code @PostConstruct} can run before + * {@code LibraryService.start()} has opened a library — the two have no other relationship Spring's + * bean-creation order would honour, and when it loses that race the routing datasource throws (no library + * is open yet) the first time this class touches a repository. + */ +@Service +@DependsOn("libraryService") +public class LibraryFolderService { + + private static final String LIBRARY_GROUP = "library"; + private static final String LEGACY_ROOT_PATH_KEY = "root-path"; + + private static final Logger log = LoggerFactory.getLogger(LibraryFolderService.class); + + private final LibraryFolderRepository repository; + private final LibraryFolderMapper mapper; + private final LibrarySubfolderRepository subfolderRepository; + private final LibrarySubfolderMapper subfolderMapper; + private final LibraryFolderScanner scanner; + private final AppPreferences preferences; + private final ApplicationEventPublisher publisher; + private final TaskService taskService; + + public LibraryFolderService(LibraryFolderRepository repository, + LibraryFolderMapper mapper, + LibrarySubfolderRepository subfolderRepository, + LibrarySubfolderMapper subfolderMapper, + LibraryFolderScanner scanner, + AppPreferences preferences, + ApplicationEventPublisher publisher, + TaskService taskService) { + this.repository = repository; + this.mapper = mapper; + this.subfolderRepository = subfolderRepository; + this.subfolderMapper = subfolderMapper; + this.scanner = scanner; + this.preferences = preferences; + this.publisher = publisher; + this.taskService = taskService; + } + + /** + * One-time migration of the pre-database single root, if any. Runs only while the table is still + * empty, so it never re-adds a folder the user has since removed. + */ + @PostConstruct + void migrateLegacyRootPathIfPresent() { + if (repository.count() > 0) { + return; + } + preferences.text(LIBRARY_GROUP, LEGACY_ROOT_PATH_KEY) + .filter(root -> !root.isBlank()) + .ifPresent(root -> { + log.info("Migrating legacy '{}.{}' preference into library_folder: {}", + LIBRARY_GROUP, LEGACY_ROOT_PATH_KEY, root); + add(Path.of(root)); + }); + } + + public List list() { + return repository.findAll().stream().map(mapper::toDomain).toList(); + } + + /** + * Persists {@code directory} as a new root, or returns the existing one if it was already added. + * Publishes {@link LibraryFolderAddedEvent} before the subfolder scan starts, not after it finishes. + */ + public LibraryFolder add(Path directory) { + String absolute = directory.toAbsolutePath().toString(); + + LibraryFolder folder = repository.findByPath(absolute) + .map(mapper::toDomain) + .orElseGet(() -> { + LibraryFolderEntity saved = repository.save( + LibraryFolderEntity.builder() + .path(absolute) + .addedAt(Instant.now()) + .build()); + LibraryFolder created = mapper.toDomain(saved); + log.info("Added library folder '{}'", absolute); + publisher.publishEvent(new LibraryFolderAddedEvent(created)); + scanSubfoldersInBackground(created); + return created; + }); + return folder; + } + + /** + * Removes a root folder by path. The database cascades the delete to its {@code library_subfolder} + * rows in the same statement. + */ + public void remove(Path directory) { + String absolute = directory.toAbsolutePath().toString(); + repository.deleteByPath(absolute); + log.info("Removed library folder '{}'", absolute); + } + + private void scanSubfoldersInBackground(LibraryFolder folder) { + // folder is always already persisted by the time it reaches here (via add()), so it has a real id. + Long libraryFolderId = Objects.requireNonNull(folder.id()); + taskService.execute(TaskType.BACKGROUND_SYNC, () -> { + Instant now = Instant.now(); + List subfolders = scanner.scan(folder.path()).stream() + .map(path -> subfolderMapper.toEntity(new LibrarySubfolder(null, libraryFolderId, path, now))) + .toList(); + if (!subfolders.isEmpty()) { + subfolderRepository.saveAll(subfolders); + } + log.debug("Scanned {} subfolder(s) of '{}'", subfolders.size(), folder.path()); + }); + } +} diff --git a/src/main/java/org/icroco/pholio/infra/library/LibraryRouter.java b/src/main/java/org/icroco/pholio/infra/library/LibraryRouter.java index c6b1834..d5566a9 100644 --- a/src/main/java/org/icroco/pholio/infra/library/LibraryRouter.java +++ b/src/main/java/org/icroco/pholio/infra/library/LibraryRouter.java @@ -1,5 +1,7 @@ package org.icroco.pholio.infra.library; +import org.jspecify.annotations.Nullable; + /** * Re-points the application's {@code DataSource} at another library. * @@ -12,7 +14,7 @@ public interface LibraryRouter { /** * The library currently served, or {@code null} before the first {@link #switchTo}. */ - String current(); + @Nullable String current(); /** * Serves {@code library} from now on, creating its database file if it does not exist. diff --git a/src/main/java/org/icroco/pholio/infra/library/LibraryService.java b/src/main/java/org/icroco/pholio/infra/library/LibraryService.java index 2a4d727..c0635e1 100644 --- a/src/main/java/org/icroco/pholio/infra/library/LibraryService.java +++ b/src/main/java/org/icroco/pholio/infra/library/LibraryService.java @@ -5,6 +5,7 @@ import jakarta.annotation.PreDestroy; import javafx.util.Subscription; import org.icroco.pholio.infra.config.ExecutorConfig; import org.icroco.pholio.infra.preferences.AppPreferences; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; @@ -12,6 +13,7 @@ import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Service; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; @@ -47,7 +49,7 @@ public class LibraryService { private final Executor databaseExecutor; private final ApplicationEventPublisher publisher; - private Subscription subscription; + private @Nullable Subscription subscription; public LibraryService(AppPreferences preferences, Optional router, @@ -99,7 +101,8 @@ public class LibraryService { * The library currently open, as recorded in the preference tree. */ public String currentLibrary() { - return preferences.getValue(LIBRARY_GROUP, LAST_OPENED_KEY, String.class); + // Always non-null after start() has run: it seeds LAST_OPENED_KEY before anything can read it. + return Objects.requireNonNull(preferences.getValue(LIBRARY_GROUP, LAST_OPENED_KEY, String.class)); } /** diff --git a/src/main/java/org/icroco/pholio/infra/media/ExifThumbnailAccessor.java b/src/main/java/org/icroco/pholio/infra/media/ExifThumbnailAccessor.java index f98c344..7975605 100644 --- a/src/main/java/org/icroco/pholio/infra/media/ExifThumbnailAccessor.java +++ b/src/main/java/org/icroco/pholio/infra/media/ExifThumbnailAccessor.java @@ -9,6 +9,7 @@ import org.icroco.pholio.domain.media.EmbeddedThumbnail; import org.icroco.pholio.domain.media.ImageFormat; import org.icroco.pholio.domain.media.MediaAccessException; import org.icroco.pholio.domain.media.ThumbnailAccessor; +import org.jspecify.annotations.Nullable; import java.io.IOException; import java.nio.ByteBuffer; @@ -95,7 +96,7 @@ final class ExifThumbnailAccessor implements ThumbnailAccessor { + "container is beyond what metadata-extractor can do. Check canEmbed(format) first."); } - private static ExifThumbnailDirectory thumbnailDirectoryOf(Path file) { + private static @Nullable ExifThumbnailDirectory thumbnailDirectoryOf(Path file) { if (!Files.isRegularFile(file)) { throw new MediaAccessException(file, "Not a readable file"); } @@ -114,7 +115,7 @@ final class ExifThumbnailAccessor implements ThumbnailAccessor { /** * The requested range, or {@code null} when the file is shorter than the offsets claim. */ - private static byte[] readRange(Path file, int offset, int length) { + private static byte @Nullable [] readRange(Path file, int offset, int length) { try (SeekableByteChannel channel = Files.newByteChannel(file)) { if (channel.size() < (long) offset + length) { return null; diff --git a/src/main/java/org/icroco/pholio/infra/media/MediaFormatRegistry.java b/src/main/java/org/icroco/pholio/infra/media/MediaFormatRegistry.java index 39b3efc..bfc793f 100644 --- a/src/main/java/org/icroco/pholio/infra/media/MediaFormatRegistry.java +++ b/src/main/java/org/icroco/pholio/infra/media/MediaFormatRegistry.java @@ -3,6 +3,7 @@ package org.icroco.pholio.infra.media; import lombok.extern.slf4j.Slf4j; import org.icroco.pholio.domain.media.ImageFormat; import org.icroco.pholio.domain.media.MediaMetaFactory; +import org.jspecify.annotations.Nullable; import org.springframework.stereotype.Service; import java.nio.file.Path; @@ -92,7 +93,7 @@ public class MediaFormatRegistry { /** * As {@link #formatOf(Path)}, from an extension with or without a leading dot, in any case. */ - public Optional formatOfExtension(String extension) { + public Optional formatOfExtension(@Nullable String extension) { if (extension == null || extension.isBlank()) { return Optional.empty(); } @@ -107,7 +108,7 @@ public class MediaFormatRegistry { /** * The factory handling {@code format}, empty when no bean claims it. */ - public Optional factoryFor(ImageFormat format) { + public Optional factoryFor(@Nullable ImageFormat format) { return format == null ? Optional.empty() : Optional.ofNullable(factoriesByFormat.get(format)); } diff --git a/src/main/java/org/icroco/pholio/infra/media/MetadataExtractorReader.java b/src/main/java/org/icroco/pholio/infra/media/MetadataExtractorReader.java index 4ef83ef..6aa1876 100644 --- a/src/main/java/org/icroco/pholio/infra/media/MetadataExtractorReader.java +++ b/src/main/java/org/icroco/pholio/infra/media/MetadataExtractorReader.java @@ -17,6 +17,7 @@ import com.drew.metadata.jpeg.JpegDirectory; import com.drew.metadata.png.PngDirectory; import com.drew.metadata.webp.WebpDirectory; import org.icroco.pholio.domain.media.*; +import org.jspecify.annotations.Nullable; import java.io.IOException; import java.nio.file.Files; @@ -128,7 +129,7 @@ final class MetadataExtractorReader implements MetadataReader { return new int[]{ 0, 0 }; } - private static Orientation orientationOf(ExifIFD0Directory ifd0) { + private static Orientation orientationOf(@Nullable ExifIFD0Directory ifd0) { return Orientation.ofExifValue(integer(ifd0, ExifDirectoryBase.TAG_ORIENTATION)); } @@ -139,7 +140,7 @@ final class MetadataExtractorReader implements MetadataReader { * deliberately not a third fallback, because it changes whenever anything touches the file and would * quietly re-date a whole library. */ - private static LocalDateTime captureTimeOf(ExifSubIFDDirectory subIfd, ExifIFD0Directory ifd0) { + private static @Nullable LocalDateTime captureTimeOf(@Nullable ExifSubIFDDirectory subIfd, @Nullable ExifIFD0Directory ifd0) { LocalDateTime original = dateTime(subIfd, ExifDirectoryBase.TAG_DATETIME_ORIGINAL); if (original != null) { return original; @@ -151,12 +152,12 @@ final class MetadataExtractorReader implements MetadataReader { /** * {@code LensModel} where the body writes it, the older free-text {@code Lens} otherwise. */ - private static String lensOf(ExifSubIFDDirectory subIfd) { + private static @Nullable String lensOf(@Nullable ExifSubIFDDirectory subIfd) { String model = string(subIfd, ExifDirectoryBase.TAG_LENS_MODEL); return model != null ? model : string(subIfd, ExifDirectoryBase.TAG_LENS); } - private static GeoLocation locationOf(Metadata metadata) { + private static @Nullable GeoLocation locationOf(Metadata metadata) { GpsDirectory gps = metadata.getFirstDirectoryOfType(GpsDirectory.class); if (gps == null) { return null; @@ -195,7 +196,7 @@ final class MetadataExtractorReader implements MetadataReader { return raw; } - private static String string(Directory directory, int tag) { + private static @Nullable String string(@Nullable Directory directory, int tag) { if (directory == null) { return null; } @@ -203,11 +204,11 @@ final class MetadataExtractorReader implements MetadataReader { return value == null || value.isBlank() ? null : value.trim(); } - private static Integer integer(Directory directory, int tag) { + private static @Nullable Integer integer(@Nullable Directory directory, int tag) { return directory == null ? null : directory.getInteger(tag); } - private static Double rational(Directory directory, int tag) { + private static @Nullable Double rational(@Nullable Directory directory, int tag) { if (directory == null) { return null; } @@ -215,7 +216,7 @@ final class MetadataExtractorReader implements MetadataReader { return value == null ? null : value.doubleValue(); } - private static LocalDateTime dateTime(Directory directory, int tag) { + private static @Nullable LocalDateTime dateTime(@Nullable Directory directory, int tag) { if (directory == null) { return null; } diff --git a/src/main/java/org/icroco/pholio/infra/persistence/LibraryRoutingDataSource.java b/src/main/java/org/icroco/pholio/infra/persistence/LibraryRoutingDataSource.java index 5d52ea5..d340f3b 100644 --- a/src/main/java/org/icroco/pholio/infra/persistence/LibraryRoutingDataSource.java +++ b/src/main/java/org/icroco/pholio/infra/persistence/LibraryRoutingDataSource.java @@ -4,6 +4,7 @@ import com.zaxxer.hikari.HikariDataSource; import jakarta.annotation.PreDestroy; import org.icroco.pholio.infra.library.LibraryCatalog; import org.icroco.pholio.infra.library.LibraryRouter; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.jdbc.datasource.lookup.AbstractRoutingDataSource; @@ -50,7 +51,7 @@ public class LibraryRoutingDataSource extends AbstractRoutingDataSource implemen private final Map pools = new ConcurrentHashMap<>(); private final Function poolFactory; - private volatile String currentLibrary; + private volatile @Nullable String currentLibrary; LibraryRoutingDataSource(Function poolFactory) { this.poolFactory = poolFactory; @@ -61,7 +62,7 @@ public class LibraryRoutingDataSource extends AbstractRoutingDataSource implemen } @Override - public String current() { + public @Nullable String current() { return currentLibrary; } @@ -91,7 +92,7 @@ public class LibraryRoutingDataSource extends AbstractRoutingDataSource implemen } @Override - protected Object determineCurrentLookupKey() { + protected @Nullable Object determineCurrentLookupKey() { // A per-call ScopedValue override, if one is ever needed, is consulted here first. return currentLibrary; } @@ -120,7 +121,7 @@ public class LibraryRoutingDataSource extends AbstractRoutingDataSource implemen currentLibrary = null; } - private static void close(String library, HikariDataSource pool) { + private static void close(String library, @Nullable HikariDataSource pool) { if (pool == null) { return; } diff --git a/src/main/java/org/icroco/pholio/infra/persistence/PersistenceConfiguration.java b/src/main/java/org/icroco/pholio/infra/persistence/PersistenceConfiguration.java index 78de273..3dca055 100644 --- a/src/main/java/org/icroco/pholio/infra/persistence/PersistenceConfiguration.java +++ b/src/main/java/org/icroco/pholio/infra/persistence/PersistenceConfiguration.java @@ -2,6 +2,7 @@ package org.icroco.pholio.infra.persistence; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; +import org.flywaydb.core.Flyway; import org.icroco.pholio.infra.library.LibraryCatalog; import org.icroco.pholio.infra.support.AppDirectories; import org.slf4j.Logger; @@ -15,6 +16,7 @@ import org.springframework.context.annotation.Primary; import org.springframework.core.env.Environment; import java.nio.file.Path; +import java.util.Objects; /** * Replaces Spring Boot's autoconfigured {@code DataSource} with the library-routing one. @@ -60,7 +62,8 @@ public class PersistenceConfiguration { */ private static HikariDataSource pool(Environment environment, String library) { Path database = LibraryCatalog.databaseFile(library).toAbsolutePath(); - AppDirectories.ensureExists(database.getParent()); + // An absolute path always has a parent; toAbsolutePath() above is what guarantees it here. + AppDirectories.ensureExists(Objects.requireNonNull(database.getParent())); HikariConfig config = new HikariConfig(); Binder.get(environment).bind(HIKARI_PREFIX, Bindable.ofInstance(config)); @@ -76,6 +79,17 @@ public class PersistenceConfiguration { log.info("Opening library '{}' at {}", library, database); // The constructor starts the pool and validates one connection, so a database that cannot be // opened fails here — early enough for the caller to keep serving the previous library. - return new HikariDataSource(config); + HikariDataSource dataSource = new HikariDataSource(config); + + // Migrated here, once per pool, rather than through spring.flyway.enabled: see the exclusion of + // FlywayAutoConfiguration in application.yaml. Nothing publishes this pool until this method + // returns, so no caller can observe an unmigrated schema. + Flyway.configure() + .dataSource(dataSource) + .locations(environment.getProperty("spring.flyway.locations", "classpath:db/migration")) + .load() + .migrate(); + + return dataSource; } } diff --git a/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderEntity.java b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderEntity.java new file mode 100644 index 0000000..eab6db4 --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderEntity.java @@ -0,0 +1,31 @@ +package org.icroco.pholio.infra.persistence.folder; + +import lombok.*; +import org.jspecify.annotations.Nullable; +import org.springframework.data.annotation.Id; +import org.springframework.data.relational.core.mapping.Column; +import org.springframework.data.relational.core.mapping.Table; + +import java.time.Instant; + +// Columns are named explicitly, matching the migration exactly: H2's dialect otherwise upper-cases a +// quoted identifier it derives itself, while the quoted table name above (given literally) is left as +// written — the mismatch makes an unannotated field invisible to a query at runtime, not at compile time. +@Table("library_folder") +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@Builder +public class LibraryFolderEntity { + + @Id + @Column("id") + private @Nullable Long id; + + @Column("path") + private String path; + + @Column("added_at") + private Instant addedAt; +} diff --git a/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapper.java b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapper.java new file mode 100644 index 0000000..9676a19 --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapper.java @@ -0,0 +1,18 @@ +package org.icroco.pholio.infra.persistence.folder; + +import org.icroco.pholio.domain.library.LibraryFolder; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +import java.nio.file.Path; + +@Mapper(componentModel = "spring", imports = Path.class) +public interface LibraryFolderMapper { + + @Mapping(target = "path", expression = "java(Path.of(entity.getPath()))") + LibraryFolder toDomain(LibraryFolderEntity entity); + + @Mapping(target = "id", ignore = true) + @Mapping(target = "path", expression = "java(domain.path().toAbsolutePath().toString())") + LibraryFolderEntity toEntity(LibraryFolder domain); +} diff --git a/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderRepository.java b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderRepository.java new file mode 100644 index 0000000..e67bfbb --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderRepository.java @@ -0,0 +1,12 @@ +package org.icroco.pholio.infra.persistence.folder; + +import org.springframework.data.repository.ListCrudRepository; + +import java.util.Optional; + +public interface LibraryFolderRepository extends ListCrudRepository { + + Optional findByPath(String path); + + void deleteByPath(String path); +} diff --git a/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderEntity.java b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderEntity.java new file mode 100644 index 0000000..a336d9f --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderEntity.java @@ -0,0 +1,33 @@ +package org.icroco.pholio.infra.persistence.folder; + +import lombok.*; +import org.jspecify.annotations.Nullable; +import org.springframework.data.annotation.Id; +import org.springframework.data.relational.core.mapping.Column; +import org.springframework.data.relational.core.mapping.Table; + +import java.time.Instant; + +// Columns are named explicitly — see LibraryFolderEntity for why an unannotated field would silently +// resolve to an upper-cased identifier the migration's quoted, lower-case columns do not match. +@Table("library_subfolder") +@Getter +@Setter +@NoArgsConstructor +@AllArgsConstructor +@Builder +public class LibrarySubfolderEntity { + + @Id + @Column("id") + private @Nullable Long id; + + @Column("library_folder_id") + private Long libraryFolderId; + + @Column("path") + private String path; + + @Column("last_scanned_at") + private Instant lastScannedAt; +} diff --git a/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapper.java b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapper.java new file mode 100644 index 0000000..683e92c --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapper.java @@ -0,0 +1,18 @@ +package org.icroco.pholio.infra.persistence.folder; + +import org.icroco.pholio.domain.library.LibrarySubfolder; +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; + +import java.nio.file.Path; + +@Mapper(componentModel = "spring", imports = Path.class) +public interface LibrarySubfolderMapper { + + @Mapping(target = "path", expression = "java(Path.of(entity.getPath()))") + LibrarySubfolder toDomain(LibrarySubfolderEntity entity); + + @Mapping(target = "id", ignore = true) + @Mapping(target = "path", expression = "java(domain.path().toAbsolutePath().toString())") + LibrarySubfolderEntity toEntity(LibrarySubfolder domain); +} diff --git a/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderRepository.java b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderRepository.java new file mode 100644 index 0000000..88acc64 --- /dev/null +++ b/src/main/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderRepository.java @@ -0,0 +1,12 @@ +package org.icroco.pholio.infra.persistence.folder; + +import org.springframework.data.repository.ListCrudRepository; + +import java.util.List; + +public interface LibrarySubfolderRepository extends ListCrudRepository { + + List findByLibraryFolderId(Long libraryFolderId); + + void deleteByLibraryFolderId(Long libraryFolderId); +} diff --git a/src/main/java/org/icroco/pholio/infra/preferences/AppPreferences.java b/src/main/java/org/icroco/pholio/infra/preferences/AppPreferences.java index 43d0481..74fbfc2 100644 --- a/src/main/java/org/icroco/pholio/infra/preferences/AppPreferences.java +++ b/src/main/java/org/icroco/pholio/infra/preferences/AppPreferences.java @@ -2,6 +2,7 @@ package org.icroco.pholio.infra.preferences; import javafx.beans.property.ObjectProperty; import lombok.Getter; +import org.jspecify.annotations.Nullable; import java.util.*; @@ -81,7 +82,7 @@ public class AppPreferences { * @throws IllegalArgumentException if the preference is not declared, or {@code type} disagrees with * the declared {@link PreferenceType} */ - public T getValue(String group, String key, Class type) { + public @Nullable T getValue(String group, String key, Class type) { return type.cast(require(group, key, type).getValue()); } @@ -117,7 +118,7 @@ public class AppPreferences { * and is written on every window move. */ @SuppressWarnings("unchecked") - public void setValue(String group, String key, T value) { + public void setValue(String group, String key, @Nullable T value) { PreferenceItem item = (PreferenceItem) require(group, key, value == null ? null : value.getClass()); item.setValue(value); } @@ -199,7 +200,7 @@ public class AppPreferences { return copy; } - private PreferenceItem require(String group, String key, Class requested) { + private PreferenceItem require(String group, String key, @Nullable Class requested) { PreferenceItem item = groups.getOrDefault(group, Map.of()).get(key); if (item == null) { throw new IllegalArgumentException("No preference declared for '" + group + "." + key + "'"); diff --git a/src/main/java/org/icroco/pholio/infra/preferences/PreferenceItem.java b/src/main/java/org/icroco/pholio/infra/preferences/PreferenceItem.java index 514b196..d56f6fd 100644 --- a/src/main/java/org/icroco/pholio/infra/preferences/PreferenceItem.java +++ b/src/main/java/org/icroco/pholio/infra/preferences/PreferenceItem.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.annotation.JsonPropertyOrder; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; import lombok.*; +import org.jspecify.annotations.Nullable; import java.util.List; @@ -50,12 +51,12 @@ public class PreferenceItem { * Message key for the displayed label, resolved through {@code I18nService}. A string that matches no * key is shown verbatim, so a quick throwaway preference can carry its label inline. */ - private String label; + private @Nullable String label; /** * Value restored by "Reset". {@code null} is legitimate and means "unset". */ - private T defaultValue; + private @Nullable T defaultValue; /** * When {@code false} the settings view builds no row for this item at all. @@ -70,7 +71,7 @@ public class PreferenceItem { /** * Allowed values for a {@code STRING}; present turns the control into a {@code ComboBox}. */ - private List options; + private @Nullable List options; /** * Bounds for a numeric spinner. Only {@code INT} consumes them today. @@ -78,9 +79,9 @@ public class PreferenceItem { *

{@link Number} rather than {@link Double} so an integer bound round-trips as one: users hand-edit * this file, and a thumbnail size capped at {@code 512.0} pixels reads like a mistake. */ - private Number min; + private @Nullable Number min; - private Number max; + private @Nullable Number max; /** * The live value. Held in a property rather than a plain field so the settings view, the theme manager @@ -101,12 +102,12 @@ public class PreferenceItem { @JsonProperty("value") @ToString.Include(name = "value") @EqualsAndHashCode.Include - public T getValue() { + public @Nullable T getValue() { return property.get(); } @JsonProperty("value") - public void setValue(T value) { + public void setValue(@Nullable T value) { property.set(value); } diff --git a/src/main/java/org/icroco/pholio/infra/preferences/PreferenceService.java b/src/main/java/org/icroco/pholio/infra/preferences/PreferenceService.java index 2f54344..954c5fa 100644 --- a/src/main/java/org/icroco/pholio/infra/preferences/PreferenceService.java +++ b/src/main/java/org/icroco/pholio/infra/preferences/PreferenceService.java @@ -4,6 +4,7 @@ import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import org.icroco.pholio.infra.support.AppDirectories; import org.icroco.pholio.infra.support.Debouncer; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -25,6 +26,7 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.time.Duration; import java.util.Map; +import java.util.Objects; import java.util.function.Supplier; /** @@ -242,7 +244,8 @@ public class PreferenceService { private void write() { try { - AppDirectories.ensureExists(file.getParent()); + // file is always resolved from a directory (see the constructor), so it always has a parent. + AppDirectories.ensureExists(Objects.requireNonNull(file.getParent())); Path temp = Files.createTempFile(file.getParent(), FILE_NAME, ".tmp"); try (OutputStream out = Files.newOutputStream(temp)) { yaml.writeValue(out, preferences.getGroups()); @@ -270,7 +273,7 @@ public class PreferenceService { * replaces it with a value of the declared type. */ @SuppressWarnings("unchecked") - private static void setRaw(PreferenceItem item, Object value) { + private static void setRaw(PreferenceItem item, @Nullable Object value) { ((PreferenceItem) item).setValue(value); } diff --git a/src/main/java/org/icroco/pholio/infra/preferences/PreferenceType.java b/src/main/java/org/icroco/pholio/infra/preferences/PreferenceType.java index 0737505..3a184aa 100644 --- a/src/main/java/org/icroco/pholio/infra/preferences/PreferenceType.java +++ b/src/main/java/org/icroco/pholio/infra/preferences/PreferenceType.java @@ -1,5 +1,7 @@ package org.icroco.pholio.infra.preferences; +import org.jspecify.annotations.Nullable; + import java.util.Collection; import java.util.List; import java.util.Locale; @@ -50,7 +52,7 @@ public enum PreferenceType { * @throws IllegalArgumentException if {@code raw} carries a value that cannot be read as this type, * so a typo in the schema fails at startup rather than silently becoming a default */ - public Object coerce(Object raw) { + public @Nullable Object coerce(@Nullable Object raw) { if (raw == null) { return null; } diff --git a/src/main/java/org/icroco/pholio/infra/preferences/WindowGeometry.java b/src/main/java/org/icroco/pholio/infra/preferences/WindowGeometry.java index e67053e..ec8a303 100644 --- a/src/main/java/org/icroco/pholio/infra/preferences/WindowGeometry.java +++ b/src/main/java/org/icroco/pholio/infra/preferences/WindowGeometry.java @@ -1,5 +1,7 @@ package org.icroco.pholio.infra.preferences; +import org.jspecify.annotations.Nullable; + /** * Persisted position, size and maximised state of a single window. * @@ -9,12 +11,11 @@ package org.icroco.pholio.infra.preferences; *

Per the project rules this is edited only by hand in {@code preferences.yaml} — the settings UI * deliberately exposes no geometry fields. */ -public record WindowGeometry( - Double x, - Double y, - Double width, - Double height, - boolean maximized) { +public record WindowGeometry(@Nullable Double x, + @Nullable Double y, + @Nullable Double width, + @Nullable Double height, + boolean maximized) { public static WindowGeometry unset() { return new WindowGeometry(null, null, null, null, false); @@ -25,14 +26,14 @@ public record WindowGeometry( } public boolean hasSize() { - return isUsable(width) && width > 0 && isUsable(height) && height > 0; + return isUsable(width) && width != null && width > 0 && isUsable(height) && height != null && height > 0; } public WindowGeometry withMaximized(boolean value) { return new WindowGeometry(x, y, width, height, value); } - private static boolean isUsable(Double value) { + private static boolean isUsable(@Nullable Double value) { return value != null && !value.isNaN() && !value.isInfinite(); } } diff --git a/src/main/java/org/icroco/pholio/infra/support/AppDirectories.java b/src/main/java/org/icroco/pholio/infra/support/AppDirectories.java index 6280c15..ceab20f 100644 --- a/src/main/java/org/icroco/pholio/infra/support/AppDirectories.java +++ b/src/main/java/org/icroco/pholio/infra/support/AppDirectories.java @@ -1,5 +1,7 @@ package org.icroco.pholio.infra.support; +import org.jspecify.annotations.Nullable; + import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; @@ -84,7 +86,7 @@ public final class AppDirectories { return Os.LINUX; } - private static Path overrideRoot() { + private static @Nullable Path overrideRoot() { String override = System.getProperty(HOME_OVERRIDE_PROPERTY); return (override == null || override.isBlank()) ? null : Paths.get(override); } diff --git a/src/main/java/org/icroco/pholio/infra/support/Debouncer.java b/src/main/java/org/icroco/pholio/infra/support/Debouncer.java index 2b8dfe9..73c8849 100644 --- a/src/main/java/org/icroco/pholio/infra/support/Debouncer.java +++ b/src/main/java/org/icroco/pholio/infra/support/Debouncer.java @@ -1,5 +1,6 @@ package org.icroco.pholio.infra.support; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -98,7 +99,7 @@ public final class Debouncer implements AutoCloseable { private static final class Pending { private final Runnable action; - private volatile ScheduledFuture future; + private volatile @Nullable ScheduledFuture future; private Pending(Runnable action) { this.action = action; diff --git a/src/main/java/org/icroco/pholio/infra/task/TaskBody.java b/src/main/java/org/icroco/pholio/infra/task/TaskBody.java index 0d01de6..cac1185 100644 --- a/src/main/java/org/icroco/pholio/infra/task/TaskBody.java +++ b/src/main/java/org/icroco/pholio/infra/task/TaskBody.java @@ -1,7 +1,9 @@ package org.icroco.pholio.infra.task; +import org.jspecify.annotations.Nullable; + /** The work behind a tracked, result-bearing submission to {@link TaskService}. */ @FunctionalInterface public interface TaskBody { - T run(ProgressReporter progress) throws Exception; + @Nullable T run(ProgressReporter progress) throws Exception; } diff --git a/src/main/java/org/icroco/pholio/infra/task/TaskProgressEvent.java b/src/main/java/org/icroco/pholio/infra/task/TaskProgressEvent.java index 28adb3f..c9a23df 100644 --- a/src/main/java/org/icroco/pholio/infra/task/TaskProgressEvent.java +++ b/src/main/java/org/icroco/pholio/infra/task/TaskProgressEvent.java @@ -1,5 +1,7 @@ package org.icroco.pholio.infra.task; +import org.jspecify.annotations.Nullable; + import java.util.UUID; /** @@ -8,5 +10,5 @@ import java.util.UUID; * shared sentinel value for "unchanged" would have made a legitimate {@code 0.0} or an empty message * ambiguous with "nothing to report". */ -public record TaskProgressEvent(UUID taskId, Double progress, String message) { +public record TaskProgressEvent(UUID taskId, @Nullable Double progress, @Nullable String message) { } diff --git a/src/main/java/org/icroco/pholio/infra/task/TaskService.java b/src/main/java/org/icroco/pholio/infra/task/TaskService.java index 60d8abc..75cd97a 100644 --- a/src/main/java/org/icroco/pholio/infra/task/TaskService.java +++ b/src/main/java/org/icroco/pholio/infra/task/TaskService.java @@ -1,6 +1,7 @@ package org.icroco.pholio.infra.task; import org.icroco.pholio.infra.config.ExecutorConfig; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Qualifier; @@ -10,13 +11,7 @@ import org.springframework.stereotype.Service; import java.util.EnumMap; import java.util.Map; import java.util.UUID; -import java.util.concurrent.CancellationException; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executor; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.FutureTask; -import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.*; /** * The one place background work gets submitted from, so neither the UI nor the CLI ever injects one of @@ -47,9 +42,9 @@ public class TaskService { private static final Logger log = LoggerFactory.getLogger(TaskService.class); - private final Map executors = new EnumMap<>(TaskType.class); - private final Map> running = new ConcurrentHashMap<>(); - private final ApplicationEventPublisher publisher; + private final Map executors = new EnumMap<>(TaskType.class); + private final Map> running = new ConcurrentHashMap<>(); + private final ApplicationEventPublisher publisher; public TaskService(@Qualifier(ExecutorConfig.DATABASE_EXECUTOR) Executor database, @Qualifier(ExecutorConfig.THUMBNAIL_EXECUTOR) Executor thumbnail, @@ -69,11 +64,11 @@ public class TaskService { * Submits result-bearing work, tracked from the moment this returns. * * @return a future completed with {@code body}'s result, exceptionally on failure, or cancelled if - * {@link #cancel} — or the future's own {@code cancel(true)} — is called before it finishes + * {@link #cancel} — or the future's own {@code cancel(true)} — is called before it finishes */ public CompletableFuture submit(TaskType type, String title, TaskBody body) { - UUID id = UUID.randomUUID(); - TrackedFuture result = new TrackedFuture<>(); + UUID id = UUID.randomUUID(); + TrackedFuture result = new TrackedFuture<>(); ProgressReporter reporter = new PublishingProgressReporter(id); FutureTask futureTask = new FutureTask<>(() -> body.run(reporter)) { @@ -88,7 +83,7 @@ public class TaskService { publisher.publishEvent(new TaskSubmittedEvent(id, title, type)); try { - executors.get(type).execute(futureTask); + executorFor(type).execute(futureTask); } catch (RejectedExecutionException e) { running.remove(id); @@ -99,7 +94,9 @@ public class TaskService { return result; } - /** {@link #submit(TaskType, String, TaskBody)} for work with nothing to hand back. */ + /** + * {@link #submit(TaskType, String, TaskBody)} for work with nothing to hand back. + */ public CompletableFuture submit(TaskType type, String title, TaskAction action) { return submit(type, title, progress -> { action.run(progress); @@ -116,7 +113,18 @@ public class TaskService { * they would a direct {@code Executor.execute} rejection */ public void execute(TaskType type, Runnable work) { - executors.get(type).execute(work); + executorFor(type).execute(work); + } + + /** + * Every {@link TaskType} is registered in the constructor, so a missing entry is a wiring bug. + */ + private Executor executorFor(TaskType type) { + Executor executor = executors.get(type); + if (executor == null) { + throw new IllegalStateException("No executor configured for " + type); + } + return executor; } /** @@ -129,7 +137,9 @@ public class TaskService { return future != null && future.cancel(true); } - /** Cancels every tracked task still running. Called when the window closes, before the context shuts down. */ + /** + * Cancels every tracked task still running. Called when the window closes, before the context shuts down. + */ public void cancelAll() { running.values().forEach(future -> future.cancel(true)); } @@ -162,7 +172,7 @@ public class TaskService { */ private static final class TrackedFuture extends CompletableFuture { - private FutureTask delegate; + private @Nullable FutureTask delegate; void attach(FutureTask delegate) { this.delegate = delegate; diff --git a/src/main/java/org/icroco/pholio/package-info.java b/src/main/java/org/icroco/pholio/package-info.java new file mode 100644 index 0000000..9758267 --- /dev/null +++ b/src/main/java/org/icroco/pholio/package-info.java @@ -0,0 +1,4 @@ +@NullMarked +package org.icroco.pholio; + +import org.jspecify.annotations.NullMarked; \ No newline at end of file diff --git a/src/main/java/org/icroco/pholio/ui/PholioFxApplication.java b/src/main/java/org/icroco/pholio/ui/PholioFxApplication.java index 2073474..37025a1 100644 --- a/src/main/java/org/icroco/pholio/ui/PholioFxApplication.java +++ b/src/main/java/org/icroco/pholio/ui/PholioFxApplication.java @@ -5,11 +5,14 @@ import javafx.application.HostServices; import javafx.stage.Stage; import org.icroco.pholio.LaunchMode; import org.icroco.pholio.PholioBootstrap; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; +import java.util.Objects; + /** * Bridges the JavaFX and Spring lifecycles. * @@ -28,7 +31,7 @@ public class PholioFxApplication extends Application { private static final Logger log = LoggerFactory.getLogger(PholioFxApplication.class); - private ConfigurableApplicationContext context; + private @Nullable ConfigurableApplicationContext context; @Override public void init() { @@ -52,7 +55,8 @@ public class PholioFxApplication extends Application { @Override public void start(Stage primaryStage) { - context.getBean(StageManager.class).showMainWindow(primaryStage); + // JavaFX always calls init() before start(), so context is set by the time this runs. + Objects.requireNonNull(context).getBean(StageManager.class).showMainWindow(primaryStage); } /** diff --git a/src/main/java/org/icroco/pholio/ui/ViewSwitcher.java b/src/main/java/org/icroco/pholio/ui/ViewSwitcher.java index 7997f44..a689b2e 100644 --- a/src/main/java/org/icroco/pholio/ui/ViewSwitcher.java +++ b/src/main/java/org/icroco/pholio/ui/ViewSwitcher.java @@ -17,6 +17,7 @@ import org.icroco.pholio.ui.event.ViewType; import org.icroco.pholio.ui.selection.ViewportSelection; import org.icroco.pholio.ui.view.GalleryView; import org.icroco.pholio.ui.view.ModulePlaceholderView; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; @@ -24,6 +25,7 @@ import org.springframework.context.event.EventListener; import java.util.EnumMap; import java.util.Map; +import java.util.Objects; /** * Owns the main viewport and is the only component that mounts or unmounts a view. @@ -41,15 +43,15 @@ public class ViewSwitcher { private static final Logger log = LoggerFactory.getLogger(ViewSwitcher.class); - private final ApplicationContext context; - private final ViewportSelection viewportSelection; - private final StackPane container = new StackPane(); - private final Map> viewTypes = new EnumMap<>(ViewType.class); - private final ObjectProperty currentType = new SimpleObjectProperty<>(this, "currentType"); + private final @Nullable ApplicationContext context; + private final ViewportSelection viewportSelection; + private final StackPane container = new StackPane(); + private final Map> viewTypes = new EnumMap<>(ViewType.class); + private final ObjectProperty currentType = new SimpleObjectProperty<>(this, "currentType"); - private Node currentView; + private @Nullable Node currentView; - public ViewSwitcher(ApplicationContext context, ViewportSelection viewportSelection) { + public ViewSwitcher(@Nullable ApplicationContext context, ViewportSelection viewportSelection) { this.context = context; this.viewportSelection = viewportSelection; container.getStyleClass().add("main-viewport"); @@ -65,7 +67,9 @@ public class ViewSwitcher { viewTypes.put(ViewType.EXPORT, ModulePlaceholderView.class); } - /** The node to place in the centre of the shell. */ + /** + * The node to place in the centre of the shell. + */ public Pane container() { return container; } @@ -74,7 +78,9 @@ public class ViewSwitcher { return currentType; } - /** Convenience for the initial navigation at startup. */ + /** + * Convenience for the initial navigation at startup. + */ public void navigate(ViewType target) { onNavigate(new NavigateToViewEvent(target)); } @@ -97,7 +103,8 @@ public class ViewSwitcher { disposeCurrent(); - Node view = context.getBean(viewType); + // Only reached when a navigation actually mounts a view; only tests that never navigate pass a null context. + Node view = Objects.requireNonNull(context).getBean(viewType); if (view instanceof NavigationAware aware) { aware.onNavigate(event); } @@ -122,7 +129,9 @@ public class ViewSwitcher { currentView = null; } - /** Disposes whatever is mounted. Called when the window closes. */ + /** + * Disposes whatever is mounted. Called when the window closes. + */ public void dispose() { FxUtils.onFxThread(this::disposeCurrent); } diff --git a/src/main/java/org/icroco/pholio/ui/debug/DevTools.java b/src/main/java/org/icroco/pholio/ui/debug/DevTools.java index e1951c6..cc79702 100644 --- a/src/main/java/org/icroco/pholio/ui/debug/DevTools.java +++ b/src/main/java/org/icroco/pholio/ui/debug/DevTools.java @@ -13,10 +13,13 @@ import javafx.scene.input.KeyCombination; import javafx.stage.Stage; import org.icroco.pholio.ui.common.Disposable; import org.icroco.pholio.ui.common.UiComponent; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Value; +import java.util.Objects; + /** * A scene graph inspector, opened on F12. * @@ -42,13 +45,14 @@ public class DevTools implements Disposable { private static final KeyCombination SHORTCUT = new KeyCodeCombination(KeyCode.F12); private static final String APPLICATION_NAME = "Pholio"; - private final boolean enabled; - private final HostServices hostServices; + private final boolean enabled; + private final @Nullable HostServices hostServices; - private Connector connector; - private Stage toolStage; + private @Nullable Connector connector; + private @Nullable Stage toolStage; - public DevTools(@Value("${pholio.dev-tools.enabled:false}") boolean enabled, HostServices hostServices) { + public DevTools(@Value("${pholio.dev-tools.enabled:false}") boolean enabled, + @Nullable HostServices hostServices) { this.enabled = enabled; this.hostServices = hostServices; } @@ -103,24 +107,28 @@ public class DevTools implements Disposable { // reattach, and WindowMonitor then re-adds its highlight nodes to a root that already holds them — // "duplicate children added", thrown out of the shutdown sequence before the window geometry is saved. // Remove this note if the upstream typo is fixed and the version is raised. - connector = new LocalConnector(primaryStage, APPLICATION_NAME); - ToolPane pane = new ToolPane(connector, new Preferences(hostServices)); + Connector localConnector = new LocalConnector(primaryStage, APPLICATION_NAME); + connector = localConnector; + // Only reached once the inspector is actually opened; real callers always inject real HostServices, + // and only tests that never open the inspector pass null. + ToolPane pane = new ToolPane(localConnector, new Preferences(Objects.requireNonNull(hostServices))); Scene scene = new Scene(pane, GUI.DEFAULT_STAGE_WIDTH, GUI.DEFAULT_STAGE_HEIGHT); // Its own user agent stylesheet, so the inspector keeps its appearance whichever theme the // application is wearing — and so it is not itself restyled by the CSS being debugged. scene.setUserAgentStylesheet(GUI.USER_AGENT_STYLESHEET); - toolStage = new Stage(); - toolStage.setTitle(APPLICATION_NAME + " — scene graph"); - toolStage.setScene(scene); + Stage localToolStage = new Stage(); + toolStage = localToolStage; + localToolStage.setTitle(APPLICATION_NAME + " — scene graph"); + localToolStage.setScene(scene); // The connector walks the live scene graph, so it can only start once the tool window exists. Cleared // after the first run: hiding and reshowing the window must not attach a second set of listeners. - toolStage.setOnShown(event -> { - toolStage.setOnShown(null); - connector.start(); + localToolStage.setOnShown(event -> { + localToolStage.setOnShown(null); + localConnector.start(); }); - toolStage.show(); + localToolStage.show(); } /** diff --git a/src/main/java/org/icroco/pholio/ui/event/NavigateToViewEvent.java b/src/main/java/org/icroco/pholio/ui/event/NavigateToViewEvent.java index dd143c2..4530e29 100644 --- a/src/main/java/org/icroco/pholio/ui/event/NavigateToViewEvent.java +++ b/src/main/java/org/icroco/pholio/ui/event/NavigateToViewEvent.java @@ -1,5 +1,7 @@ package org.icroco.pholio.ui.event; +import org.jspecify.annotations.Nullable; + /** * Request to show another view. * @@ -15,7 +17,7 @@ package org.icroco.pholio.ui.event; * @param payload optional context for the destination (a photo id, a filter, an import batch); may be * {@code null} */ -public record NavigateToViewEvent(ViewType target, Object payload) { +public record NavigateToViewEvent(ViewType target, @Nullable Object payload) { public NavigateToViewEvent(ViewType target) { this(target, null); diff --git a/src/main/java/org/icroco/pholio/ui/i18n/I18nService.java b/src/main/java/org/icroco/pholio/ui/i18n/I18nService.java index e7a67c8..9d5909e 100644 --- a/src/main/java/org/icroco/pholio/ui/i18n/I18nService.java +++ b/src/main/java/org/icroco/pholio/ui/i18n/I18nService.java @@ -7,6 +7,7 @@ import javafx.beans.property.ReadOnlyObjectProperty; import javafx.beans.property.SimpleObjectProperty; import org.icroco.pholio.infra.preferences.AppPreferences; import org.icroco.pholio.ui.common.UiComponent; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.MessageSource; @@ -18,6 +19,7 @@ import java.time.format.DateTimeFormatter; import java.time.format.FormatStyle; import java.util.List; import java.util.Locale; +import java.util.Objects; /** * Single source of translated text, with live language switching. @@ -75,7 +77,7 @@ public class I18nService { * Switches language and persists the choice. Every existing binding updates; no restart, no view * rebuild. */ - public void setLocale(Locale newLocale) { + public void setLocale(@Nullable Locale newLocale) { Locale resolved = supportedOrDefault(newLocale); if (resolved.equals(locale.get())) { return; @@ -126,8 +128,9 @@ public class I18nService { return fallback; } // The four-argument overload returns the default instead of throwing, so no exception is used for - // control flow on a path that runs for every row of the settings view. - return messageSource.getMessage(code, null, fallback, locale.get()); + // control flow on a path that runs for every row of the settings view. The null only happens when + // the default message itself is null, which fallback never is. + return Objects.requireNonNullElse(messageSource.getMessage(code, null, fallback, locale.get()), fallback); } /** Localised human-readable file size, e.g. {@code 4,2 MB} in French. */ @@ -163,14 +166,14 @@ public class I18nService { return candidate.getDisplayLanguage(candidate); } - private Locale resolveInitialLocale(String persisted) { + private Locale resolveInitialLocale(@Nullable String persisted) { if (persisted != null && !persisted.isBlank()) { return supportedOrDefault(Locale.forLanguageTag(persisted)); } return supportedOrDefault(Locale.getDefault()); } - private static Locale supportedOrDefault(Locale candidate) { + private static Locale supportedOrDefault(@Nullable Locale candidate) { if (candidate == null) { return SUPPORTED_LOCALES.getFirst(); } diff --git a/src/main/java/org/icroco/pholio/ui/selection/ViewportSelection.java b/src/main/java/org/icroco/pholio/ui/selection/ViewportSelection.java index 92cb870..09b71c3 100644 --- a/src/main/java/org/icroco/pholio/ui/selection/ViewportSelection.java +++ b/src/main/java/org/icroco/pholio/ui/selection/ViewportSelection.java @@ -4,6 +4,7 @@ import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.ReadOnlyBooleanWrapper; import javafx.scene.Node; import org.icroco.pholio.ui.common.UiComponent; +import org.jspecify.annotations.Nullable; /** * Whether the view currently mounted in the viewport has a selection. @@ -39,7 +40,7 @@ public class ViewportSelection { * * @param view the newly mounted view, or {@code null} when the viewport is emptied */ - public void follow(Node view) { + public void follow(@Nullable Node view) { hasSelection.unbind(); if (view instanceof SelectionSource source) { hasSelection.bind(source.hasSelection()); diff --git a/src/main/java/org/icroco/pholio/ui/shell/LibraryFolderTree.java b/src/main/java/org/icroco/pholio/ui/shell/LibraryFolderTree.java index c54fadb..de9be73 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/LibraryFolderTree.java +++ b/src/main/java/org/icroco/pholio/ui/shell/LibraryFolderTree.java @@ -4,13 +4,18 @@ import atlantafx.base.theme.Styles; import atlantafx.base.theme.Tweaks; import javafx.geometry.Pos; import javafx.scene.Node; -import javafx.scene.control.Label; -import javafx.scene.control.TreeCell; -import javafx.scene.control.TreeItem; -import javafx.scene.control.TreeView; +import javafx.scene.control.*; +import javafx.scene.layout.HBox; +import javafx.scene.layout.Priority; +import javafx.scene.layout.Region; import javafx.scene.layout.StackPane; -import org.icroco.pholio.infra.preferences.AppPreferences; +import javafx.stage.DirectoryChooser; +import org.icroco.pholio.domain.library.LibraryFolder; +import org.icroco.pholio.infra.library.LibraryChangedEvent; +import org.icroco.pholio.infra.library.LibraryFolderAddedEvent; +import org.icroco.pholio.infra.library.LibraryFolderService; import org.icroco.pholio.infra.task.TaskService; +import org.icroco.pholio.infra.task.TaskType; import org.icroco.pholio.ui.common.FxUtils; import org.icroco.pholio.ui.common.SubscriptionScope; import org.icroco.pholio.ui.common.UiComponent; @@ -19,18 +24,25 @@ import org.kordamp.ikonli.feather.Feather; import org.kordamp.ikonli.javafx.FontIcon; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.context.event.EventListener; +import java.io.File; import java.nio.file.Path; +import java.util.List; +import java.util.Optional; +import java.util.function.Consumer; /** - * The Photothèque drawer: every folder and subfolder of the library. + * The Photothèque drawer: every root folder the user has added to the library, and their subfolders. * - *

The root is the configured library directory and the branches are loaded as they are opened — see - * {@link FolderTreeItem}. Nothing is walked ahead of time, so opening the drawer costs one directory - * listing whatever the size of the library. + *

Roots are loaded from {@link LibraryFolderService} and shown under an invisible super-root — several + * folders, not one — with each real root's branches loaded lazily as they are opened, exactly as before; + * see {@link FolderTreeItem}. Nothing is walked ahead of time, so opening the drawer costs one directory + * listing per expanded root, whatever the size of the library. * - *

The root follows {@code library.root-path} live: choosing a directory from the gallery's picker - * repopulates the tree with no restart, because both read the same observable preference. + *

Adding a root is the "+" icon next to the drawer title ({@link #titleAccessory()}), revealed on hover + * by CSS. Removing one is the "-" icon on the row itself, revealed the same way, and only offered on a + * root's own row — a subfolder cannot be removed independently of its root. * *

Selecting a folder does nothing yet. There is no index to filter, so wiring an event * would mean publishing something nobody listens to; the selection is logged and that is all. Filtering the @@ -41,29 +53,27 @@ public class LibraryFolderTree extends StackPane implements NavigationDrawerSect private static final Logger log = LoggerFactory.getLogger(LibraryFolderTree.class); - /** - * Group and key of the library root, as declared in {@code preferences.yaml}. - */ - private static final String LIBRARY_GROUP = "library"; - private static final String ROOT_PATH_KEY = "root-path"; - - private final AppPreferences preferences; - private final TaskService taskService; + private final I18nService i18n; + private final LibraryFolderService libraryFolderService; + private final TaskService taskService; private final SubscriptionScope scope = new SubscriptionScope(); private final TreeView tree = new TreeView<>(); private final Label notConfigured = new Label(); + private final Button addFolder = new Button(); public LibraryFolderTree(I18nService i18n, - AppPreferences preferences, + LibraryFolderService libraryFolderService, TaskService taskService) { - this.preferences = preferences; + this.i18n = i18n; + this.libraryFolderService = libraryFolderService; this.taskService = taskService; getStyleClass().add("library-folder-tree"); - tree.setShowRoot(true); - tree.setCellFactory(view -> new FolderCell()); + tree.setShowRoot(false); + tree.setRoot(new TreeItem<>()); + tree.setCellFactory(view -> new FolderCell(this::removeFolder)); // The tree fills the drawer, which already has its own border; EDGE_TO_EDGE drops the control's // so the two do not stack into a double line. tree.getStyleClass().add(Tweaks.EDGE_TO_EDGE); @@ -75,12 +85,15 @@ public class LibraryFolderTree extends StackPane implements NavigationDrawerSect StackPane.setAlignment(notConfigured, Pos.CENTER); getChildren().addAll(tree, notConfigured); + updateEmptyState(); - // subscribe(Consumer) delivers the current value immediately, so this both renders the initial tree - // and follows a root chosen later from the gallery. Nothing is written back here, which is what - // makes the immediate delivery harmless. - scope.add(preferences.property(LIBRARY_GROUP, ROOT_PATH_KEY, String.class) - .subscribe(root -> FxUtils.onFxThread(() -> applyRoot(root)))); + addFolder.getStyleClass().addAll("drawer-title-accessory", Styles.BUTTON_ICON, Styles.FLAT, Styles.SMALL); + addFolder.setGraphic(FontIcon.of(Feather.PLUS, 14)); + addFolder.setTooltip(new Tooltip()); + addFolder.getTooltip().textProperty().bind(i18n.binding("library.addFolder.tooltip")); + addFolder.setOnAction(event -> chooseFolder()); + + reload(); } @Override @@ -93,23 +106,81 @@ public class LibraryFolderTree extends StackPane implements NavigationDrawerSect return this; } + @Override + public Optional titleAccessory() { + return Optional.of(addFolder); + } + /** - * Points the tree at {@code root}, or shows the "no library" notice when there is none. - * - *

A configured directory is not checked for existence: that is a system call on the FX thread for a - * case the tree already handles — a missing root simply lists nothing. Only a blank preference means - * "no library", and that is the one the notice is about. + * Picks a new root folder and hands it to {@link LibraryFolderService}; the row itself is added once + * {@link #onFolderAdded} hears the folder was actually persisted, not synchronously here. */ - private void applyRoot(String root) { - boolean configured = root != null && !root.isBlank(); + private void chooseFolder() { + DirectoryChooser chooser = new DirectoryChooser(); + chooser.setTitle(i18n.get("library.chooseFolder.dialogTitle")); - tree.setRoot(null); - if (configured) { - FolderTreeItem rootItem = new FolderTreeItem(Path.of(root), taskService); - rootItem.setExpanded(true); - tree.setRoot(rootItem); + File selected = chooser.showDialog(getScene() != null ? getScene().getWindow() : null); + if (selected == null) { + return; } + Path chosen = selected.toPath(); + taskService.execute(TaskType.BACKGROUND_SYNC, () -> libraryFolderService.add(chosen)); + } + @EventListener + public void onFolderAdded(LibraryFolderAddedEvent event) { + FxUtils.onFxThread(() -> appendFolder(event.folder())); + } + + /** + * The database now behind {@link #tree} belongs to a different library — its folders are gone, and + * whatever this drawer was showing is for a library that is no longer open. Reload from scratch rather + * than trying to diff against what a single event can carry. + */ + @EventListener + public void onLibraryChanged(LibraryChangedEvent event) { + reload(); + } + + /** + * Clears the tree and repopulates it from {@link LibraryFolderService}. The clear happens in the same + * FX-thread callback as the repopulation, not a separate one dispatched from wherever the caller runs + * — two independently scheduled {@code Platform.runLater} calls have no ordering guarantee between + * them, and a clear arriving after the folders it was meant to precede would flash the empty state. + */ + private void reload() { + taskService.execute(TaskType.BACKGROUND_SYNC, () -> { + List folders = libraryFolderService.list(); + FxUtils.onFxThread(() -> { + tree.getRoot().getChildren().clear(); + folders.forEach(this::appendFolder); + }); + }); + } + + private void appendFolder(LibraryFolder folder) { + tree.getRoot().getChildren().add(new FolderTreeItem(folder.path(), taskService)); + updateEmptyState(); + } + + /** + * Removes a root's row once {@link LibraryFolderService} confirms the deletion. Triggered only from + * this row's own "-" button, so there is nothing else that needs telling — unlike an add, which can + * arrive from elsewhere, a remove always originates here. + */ + private void removeFolder(TreeItem item) { + Path path = item.getValue(); + taskService.execute(TaskType.BACKGROUND_SYNC, () -> { + libraryFolderService.remove(path); + FxUtils.onFxThread(() -> { + tree.getRoot().getChildren().remove(item); + updateEmptyState(); + }); + }); + } + + private void updateEmptyState() { + boolean configured = !tree.getRoot().getChildren().isEmpty(); tree.setVisible(configured); tree.setManaged(configured); notConfigured.setVisible(!configured); @@ -123,6 +194,14 @@ public class LibraryFolderTree extends StackPane implements NavigationDrawerSect return tree; } + /** + * Removes {@code item} the same way its own "-" button would. For tests; the button is inside a + * {@code TreeCell} graphic, not reachable without a fully rendered scene. + */ + void removeForTest(TreeItem item) { + removeFolder(item); + } + private void onFolderSelected(TreeItem item) { if (item != null) { log.debug("Library folder selected: {}", item.getValue()); @@ -133,32 +212,71 @@ public class LibraryFolderTree extends StackPane implements NavigationDrawerSect public void dispose() { scope.close(); notConfigured.textProperty().unbind(); + addFolder.getTooltip().textProperty().unbind(); + addFolder.setOnAction(null); tree.setRoot(null); tree.setCellFactory(null); } /** - * One folder row: an icon and the folder's name. - * - *

The root shows its full path instead — it is the only row whose parent directory is not on screen, - * so its name alone would not say which library is being displayed. + * One folder row: an icon, the folder's name, and — for a root only — a right-aligned actions area + * ("label .... -") holding the remove button. The actions box exists on every row so future icons + * (rescan, reveal in file manager, …) can be added at any depth without touching the layout again; a + * subfolder's box just stays empty for now. */ private static final class FolderCell extends TreeCell { - private final FontIcon icon = FontIcon.of(Feather.FOLDER, 14); + private final FontIcon icon = FontIcon.of(Feather.FOLDER, 14); + private final Label label = new Label(); + private final HBox actions = new HBox(4); + private final HBox row; + + private final Consumer> onRemove; + + private FolderCell(Consumer> onRemove) { + this.onRemove = onRemove; + + actions.getStyleClass().add("folder-actions"); + actions.setAlignment(Pos.CENTER_RIGHT); + + Region spacer = new Region(); + HBox.setHgrow(spacer, Priority.ALWAYS); + + row = new HBox(6, icon, label, spacer, actions); + row.setAlignment(Pos.CENTER_LEFT); + } @Override protected void updateItem(Path item, boolean empty) { super.updateItem(item, empty); + actions.getChildren().clear(); + if (empty || item == null) { - setText(null); setGraphic(null); + setTooltip(null); return; } - Path name = item.getFileName(); - boolean isRoot = getTreeItem() != null && getTreeItem().getParent() == null; - setText(isRoot || name == null ? item.toString() : name.toString()); - setGraphic(icon); + + Path name = item.getFileName(); + label.setText(name == null ? item.toString() : name.toString()); + setGraphic(row); + + boolean isRoot = getTreeItem() != null && getTreeItem().getParent() != null + && getTreeItem().getParent().getParent() == null; + if (isRoot) { + setTooltip(new Tooltip(item.toString())); + actions.getChildren().add(removeButton()); + } else { + setTooltip(null); + } + } + + private Button removeButton() { + Button remove = new Button(); + remove.getStyleClass().addAll(Styles.BUTTON_ICON, Styles.FLAT, Styles.SMALL); + remove.setGraphic(FontIcon.of(Feather.MINUS, 12)); + remove.setOnAction(event -> onRemove.accept(getTreeItem())); + return remove; } } } diff --git a/src/main/java/org/icroco/pholio/ui/shell/ModalService.java b/src/main/java/org/icroco/pholio/ui/shell/ModalService.java index 34b6270..4456b63 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/ModalService.java +++ b/src/main/java/org/icroco/pholio/ui/shell/ModalService.java @@ -6,10 +6,13 @@ import javafx.scene.Node; import org.icroco.pholio.ui.common.Disposable; import org.icroco.pholio.ui.common.FxUtils; import org.icroco.pholio.ui.common.UiComponent; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.ApplicationContext; +import java.util.Objects; + /** * Shows dialogs inside the scenegraph instead of in OS windows. * @@ -25,12 +28,12 @@ public class ModalService { private static final Logger log = LoggerFactory.getLogger(ModalService.class); - private final ApplicationContext context; + private final @Nullable ApplicationContext context; private final ModalPane modalPane = new ModalPane(); - private Node currentContent; + private @Nullable Node currentContent; - public ModalService(ApplicationContext context) { + public ModalService(@Nullable ApplicationContext context) { this.context = context; modalPane.setAlignment(Pos.CENTER); // Non-persistent: clicking the scrim or pressing Escape dismisses, which is what users expect of @@ -60,7 +63,8 @@ public class ModalService { public void show(Class contentType, boolean persistent) { FxUtils.onFxThread(() -> { releaseContent(); - Node content = context.getBean(contentType); + // Only reached when a modal is actually shown; only tests that never show one pass a null context. + Node content = Objects.requireNonNull(context).getBean(contentType); currentContent = content; modalPane.setPersistent(persistent); modalPane.show(content); diff --git a/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawer.java b/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawer.java index b1c5d9e..f088837 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawer.java +++ b/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawer.java @@ -5,18 +5,21 @@ import javafx.geometry.Insets; import javafx.geometry.Pos; import javafx.scene.Node; import javafx.scene.control.Label; +import javafx.scene.layout.HBox; import javafx.scene.layout.Priority; import javafx.scene.layout.StackPane; import javafx.scene.layout.VBox; import org.icroco.pholio.ui.common.Disposable; import org.icroco.pholio.ui.common.UiComponent; import org.icroco.pholio.ui.i18n.I18nService; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.EnumMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; /** * The panel that opens to the right of the icon rail, titled with the destination it is open on. @@ -42,6 +45,7 @@ public class NavigationDrawer extends VBox implements Disposable { private final Map sections; private final Label title = new Label(); + private final HBox header = new HBox(title); private final Label placeholder = new Label(); private final StackPane body = new StackPane(); @@ -52,13 +56,14 @@ public class NavigationDrawer extends VBox implements Disposable { getStyleClass().add("navigation-drawer"); title.getStyleClass().add(Styles.TITLE_3); - title.setMaxWidth(Double.MAX_VALUE); + HBox.setHgrow(title, Priority.ALWAYS); placeholder.getStyleClass().add(Styles.TEXT_MUTED); placeholder.textProperty().bind(i18n.binding("nav.drawer.empty")); placeholder.setWrapText(true); - VBox header = new VBox(title); + header.getStyleClass().add("drawer-header"); + header.setAlignment(Pos.CENTER_LEFT); header.setPadding(new Insets(2, 10, 2, 10)); body.setAlignment(Pos.TOP_LEFT); @@ -78,6 +83,9 @@ public class NavigationDrawer extends VBox implements Disposable { title.textProperty().bind(i18n.binding(destination.messageKey())); NavigationDrawerSection section = sections.get(destination); + header.getChildren().setAll(section == null + ? List.of(title) + : Stream.concat(Stream.of(title), section.titleAccessory().stream()).toList()); body.getChildren().setAll(section == null ? placeholder : section.content()); } @@ -91,10 +99,18 @@ public class NavigationDrawer extends VBox implements Disposable { /** * The node currently under the title. For tests. */ - Node bodyContent() { + @Nullable Node bodyContent() { return body.getChildren().isEmpty() ? null : body.getChildren().getFirst(); } + /** + * Everything currently in the header row — the title, plus the active section's accessory if it has + * one. For tests. + */ + List headerChildren() { + return List.copyOf(header.getChildren()); + } + /** * Indexes the discovered sections, refusing two claims on one destination. * diff --git a/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawerSection.java b/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawerSection.java index cab5627..c44474c 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawerSection.java +++ b/src/main/java/org/icroco/pholio/ui/shell/NavigationDrawerSection.java @@ -3,6 +3,8 @@ package org.icroco.pholio.ui.shell; import javafx.scene.Node; import org.icroco.pholio.ui.common.Disposable; +import java.util.Optional; + /** * The widget shown in the navigation drawer for one destination. * @@ -29,4 +31,14 @@ public interface NavigationDrawerSection extends Disposable { * position — survives leaving the section and coming back. */ Node content(); + + /** + * An optional small control shown next to the drawer's shared title while this section is active — + * e.g. an "add" button. Rendered inside the same header row as the title, so a purely-CSS + * {@code :hover} rule on the header can reveal it. Absent by default: only sections that need one + * override this. + */ + default Optional titleAccessory() { + return Optional.empty(); + } } diff --git a/src/main/java/org/icroco/pholio/ui/shell/NavigationRail.java b/src/main/java/org/icroco/pholio/ui/shell/NavigationRail.java index cab5849..035a0f4 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/NavigationRail.java +++ b/src/main/java/org/icroco/pholio/ui/shell/NavigationRail.java @@ -21,6 +21,7 @@ import org.icroco.pholio.ui.common.SubscriptionScope; import org.icroco.pholio.ui.common.UiComponent; import org.icroco.pholio.ui.event.ViewType; import org.icroco.pholio.ui.i18n.I18nService; +import org.jspecify.annotations.Nullable; import org.kordamp.ikonli.javafx.FontIcon; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -100,13 +101,13 @@ public class NavigationRail extends BorderPane implements Disposable { * The destination the drawer is open on, kept across a close. Null only for a module with no * destinations, where there is nothing to be active. */ - private Entry active; + private @Nullable Entry active; /** * The module {@link #entries} was last built for. Guards {@link #rebuild} against redoing the same * work if the property fires again with an unchanged value. */ - private ViewType currentModule; + private @Nullable ViewType currentModule; public NavigationRail(I18nService i18n, AppPreferences preferences, @@ -156,7 +157,7 @@ public class NavigationRail extends BorderPane implements Disposable { * the same choice {@link InspectorPanel} makes for an empty selection, and for the same reason. An * invisible-but-managed rail would keep reserving 44px of a viewport that has nothing to put there. */ - private void rebuild(ViewType module) { + private void rebuild(@Nullable ViewType module) { ViewType effective = module == null ? ViewType.GALLERY : module; if (effective == currentModule) { return; diff --git a/src/main/java/org/icroco/pholio/ui/shell/SettingsView.java b/src/main/java/org/icroco/pholio/ui/shell/SettingsView.java index 5443054..48e29fd 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/SettingsView.java +++ b/src/main/java/org/icroco/pholio/ui/shell/SettingsView.java @@ -194,7 +194,7 @@ public class SettingsView extends BorderPane implements Disposable { private Row addRow(GridPane grid, int index, String group, String key, PreferenceItem item) { Label label = new Label(); - label.textProperty().bind(i18n.bindingOr(item.getLabel(), key)); + label.textProperty().bind(i18n.bindingOr(Objects.requireNonNullElse(item.getLabel(), key), key)); scope.addTeardown(() -> label.textProperty().unbind()); Region control = controlFor(group, key, item); @@ -279,7 +279,7 @@ public class SettingsView extends BorderPane implements Disposable { // caps the popup and scrolls it. ComboBox choice = new ComboBox<>(); choice.getItems().setAll(item.getOptions()); - choice.setConverter(optionConverter(item.getLabel())); + choice.setConverter(optionConverter(Objects.requireNonNullElse(item.getLabel(), key))); choice.setPrefWidth(200); choice.setVisibleRowCount(12); bind(choice.valueProperty(), value); @@ -433,7 +433,7 @@ public class SettingsView extends BorderPane implements Disposable { @SuppressWarnings("unchecked") ComboBox choice = (ComboBox) box; String selected = choice.getValue(); - choice.setConverter(optionConverter(row.item.getLabel())); + choice.setConverter(optionConverter(Objects.requireNonNullElse(row.item.getLabel(), row.key))); // Re-setting the value is what forces the button cell to re-render; replacing the converter // alone leaves the previous text in place. choice.setValue(selected); @@ -494,7 +494,7 @@ public class SettingsView extends BorderPane implements Disposable { } private boolean matches(String needle, I18nService i18n) { - return contains(i18n.resolveOr(item.getLabel(), key), needle) + return contains(i18n.resolveOr(Objects.requireNonNullElse(item.getLabel(), key), key), needle) || contains(key, needle) || contains(group, needle); } diff --git a/src/main/java/org/icroco/pholio/ui/shell/ToastLayer.java b/src/main/java/org/icroco/pholio/ui/shell/ToastLayer.java index 79f576c..ade4f68 100644 --- a/src/main/java/org/icroco/pholio/ui/shell/ToastLayer.java +++ b/src/main/java/org/icroco/pholio/ui/shell/ToastLayer.java @@ -15,6 +15,7 @@ import org.icroco.pholio.ui.common.FxUtils; import org.icroco.pholio.ui.common.UiComponent; import org.icroco.pholio.ui.event.TaskFinishedEvent; import org.icroco.pholio.ui.i18n.I18nService; +import org.jspecify.annotations.Nullable; import org.kordamp.ikonli.feather.Feather; import org.kordamp.ikonli.javafx.FontIcon; import org.slf4j.Logger; @@ -116,7 +117,7 @@ public class ToastLayer extends VBox implements Disposable { : event.title(); } - private void dismiss(Notification toast, PauseTransition dwell) { + private void dismiss(Notification toast, @Nullable PauseTransition dwell) { getChildren().remove(toast); if (dwell != null) { pending.remove(dwell); diff --git a/src/main/java/org/icroco/pholio/ui/theme/AppTheme.java b/src/main/java/org/icroco/pholio/ui/theme/AppTheme.java index 9adad96..8d9e9e6 100644 --- a/src/main/java/org/icroco/pholio/ui/theme/AppTheme.java +++ b/src/main/java/org/icroco/pholio/ui/theme/AppTheme.java @@ -2,6 +2,7 @@ package org.icroco.pholio.ui.theme; import atlantafx.base.theme.*; import com.dlsc.atlantafx.themes.*; +import org.jspecify.annotations.Nullable; import java.util.Locale; import java.util.function.Supplier; @@ -73,7 +74,7 @@ public enum AppTheme { * {@code atlantafx-base}; were that ever to break, an eager field would turn one unusable theme into * an {@link ExceptionInInitializerError} that takes the whole enum — and the application — with it. */ - private volatile Theme resolved; + private volatile @Nullable Theme resolved; AppTheme(Supplier factory) { this.factory = factory; @@ -127,7 +128,7 @@ public enum AppTheme { /** * Parses a persisted value, falling back to {@link #DEFAULT} for anything unrecognised. */ - public static AppTheme parse(String value) { + public static AppTheme parse(@Nullable String value) { if (value != null) { String name = value.trim().toUpperCase(Locale.ROOT); for (AppTheme theme : values()) { diff --git a/src/main/java/org/icroco/pholio/ui/view/GalleryView.java b/src/main/java/org/icroco/pholio/ui/view/GalleryView.java index 63529b7..4d703b5 100644 --- a/src/main/java/org/icroco/pholio/ui/view/GalleryView.java +++ b/src/main/java/org/icroco/pholio/ui/view/GalleryView.java @@ -8,7 +8,9 @@ import javafx.scene.control.Label; import javafx.scene.layout.StackPane; import javafx.scene.layout.VBox; import javafx.stage.DirectoryChooser; -import org.icroco.pholio.infra.preferences.AppPreferences; +import org.icroco.pholio.infra.library.LibraryFolderService; +import org.icroco.pholio.infra.task.TaskService; +import org.icroco.pholio.infra.task.TaskType; import org.icroco.pholio.ui.common.Disposable; import org.icroco.pholio.ui.common.SubscriptionScope; import org.icroco.pholio.ui.common.UiView; @@ -19,6 +21,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.File; +import java.nio.file.Path; /** * Main viewport: the thumbnail grid over the library. @@ -32,21 +35,19 @@ public class GalleryView extends StackPane implements Disposable { private static final Logger log = LoggerFactory.getLogger(GalleryView.class); - /** Group and key of the library root, as declared in {@code preferences.yaml}. */ - private static final String LIBRARY_GROUP = "library"; - private static final String ROOT_PATH_KEY = "root-path"; + private final I18nService i18n; + private final LibraryFolderService libraryFolderService; + private final TaskService taskService; - private final I18nService i18n; - private final AppPreferences preferences; + private final SubscriptionScope scope = new SubscriptionScope(); + private final Label title = new Label(); + private final Label subtitle = new Label(); + private final Button chooseRoot = new Button(); - private final SubscriptionScope scope = new SubscriptionScope(); - private final Label title = new Label(); - private final Label subtitle = new Label(); - private final Button chooseRoot = new Button(); - - public GalleryView(I18nService i18n, AppPreferences preferences) { + public GalleryView(I18nService i18n, LibraryFolderService libraryFolderService, TaskService taskService) { this.i18n = i18n; - this.preferences = preferences; + this.libraryFolderService = libraryFolderService; + this.taskService = taskService; getStyleClass().add("empty-state"); @@ -68,7 +69,7 @@ public class GalleryView extends StackPane implements Disposable { } /** - * Picks the library root. + * Picks a library folder to add. * *

A native {@link DirectoryChooser} is used deliberately, and is not a violation of the * "no OS dialogs" rule: that rule is about application dialogs, which must stay themed and in-scene. @@ -77,19 +78,15 @@ public class GalleryView extends StackPane implements Disposable { */ private void chooseLibraryRoot() { DirectoryChooser chooser = new DirectoryChooser(); - chooser.setTitle(i18n.get("settings.library.rootPath")); - - preferences.text(LIBRARY_GROUP, ROOT_PATH_KEY) - .map(File::new) - .filter(File::isDirectory) - .ifPresent(chooser::setInitialDirectory); + chooser.setTitle(i18n.get("library.chooseFolder.dialogTitle")); File selected = chooser.showDialog(getScene() != null ? getScene().getWindow() : null); if (selected == null) { return; } - log.info("Library root set to {}", selected); - preferences.setValue(LIBRARY_GROUP, ROOT_PATH_KEY, selected.getAbsolutePath()); + Path chosen = selected.toPath(); + log.info("Adding library folder {}", chosen); + taskService.execute(TaskType.BACKGROUND_SYNC, () -> libraryFolderService.add(chosen)); } @Override diff --git a/src/main/java/org/icroco/pholio/ui/window/ScreenBoundsValidator.java b/src/main/java/org/icroco/pholio/ui/window/ScreenBoundsValidator.java index ca3c460..662e2dc 100644 --- a/src/main/java/org/icroco/pholio/ui/window/ScreenBoundsValidator.java +++ b/src/main/java/org/icroco/pholio/ui/window/ScreenBoundsValidator.java @@ -1,6 +1,7 @@ package org.icroco.pholio.ui.window; import javafx.geometry.Rectangle2D; +import org.jspecify.annotations.Nullable; import java.util.List; @@ -26,7 +27,8 @@ public final class ScreenBoundsValidator { * * @param minVisibleFraction fraction of the window's area that must be visible, in {@code (0, 1]} */ - public static boolean isVisibleEnough(Rectangle2D window, List screens, double minVisibleFraction) { + public static boolean isVisibleEnough(@Nullable Rectangle2D window, @Nullable List screens, + double minVisibleFraction) { if (window == null || screens == null || screens.isEmpty()) { return false; } diff --git a/src/main/java/org/icroco/pholio/ui/window/WindowStateManager.java b/src/main/java/org/icroco/pholio/ui/window/WindowStateManager.java index 4ed32b2..1972bdc 100644 --- a/src/main/java/org/icroco/pholio/ui/window/WindowStateManager.java +++ b/src/main/java/org/icroco/pholio/ui/window/WindowStateManager.java @@ -9,14 +9,12 @@ import org.icroco.pholio.infra.preferences.WindowGeometry; import org.icroco.pholio.infra.preferences.WindowGeometryPreferences; import org.icroco.pholio.infra.support.Debouncer; import org.icroco.pholio.ui.common.UiComponent; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.time.Duration; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; /** @@ -42,7 +40,9 @@ import java.util.concurrent.ConcurrentHashMap; @UiComponent public class WindowStateManager { - /** Window id of the primary application window. */ + /** + * Window id of the primary application window. + */ public static final String MAIN_WINDOW = "main"; private static final Logger log = LoggerFactory.getLogger(WindowStateManager.class); @@ -55,8 +55,8 @@ public class WindowStateManager { private static final Duration CAPTURE_DELAY = Duration.ofMillis(250); private final WindowGeometryPreferences geometries; - private final PholioProperties properties; - private final Map trackers = new ConcurrentHashMap<>(); + private final PholioProperties properties; + private final Map trackers = new ConcurrentHashMap<>(); public WindowStateManager(WindowGeometryPreferences geometries, PholioProperties properties) { this.geometries = geometries; @@ -74,7 +74,9 @@ public class WindowStateManager { tracker.attach(); } - /** Writes {@code windowId}'s current geometry immediately. Used from the close handler. */ + /** + * Writes {@code windowId}'s current geometry immediately. Used from the close handler. + */ public void captureNow(String windowId) { Tracker tracker = trackers.get(windowId); if (tracker != null) { @@ -82,7 +84,9 @@ public class WindowStateManager { } } - /** Stops tracking a window and flushes its pending geometry. For secondary windows being closed. */ + /** + * Stops tracking a window and flushes its pending geometry. For secondary windows being closed. + */ public void unregister(String windowId) { Tracker tracker = trackers.remove(windowId); if (tracker != null) { @@ -104,17 +108,23 @@ public class WindowStateManager { return bounds; } - /** Per-window state: its own debouncer, listeners, and remembered non-maximised bounds. */ + /** + * Per-window state: its own debouncer, listeners, and remembered non-maximised bounds. + */ private final class Tracker implements AutoCloseable { - private final String windowId; - private final Stage stage; + private final String windowId; + private final Stage stage; private final Debouncer debouncer; - /** Bounds to persist. Updated only while the window is neither maximised nor iconified. */ - private Rectangle2D restoredBounds; + /** + * Bounds to persist. Updated only while the window is neither maximised nor iconified. + */ + private @Nullable Rectangle2D restoredBounds; - /** Suppresses capture while {@link #restore()} is mutating the stage. */ + /** + * Suppresses capture while {@link #restore()} is mutating the stage. + */ private boolean restoring; private boolean attached; @@ -133,8 +143,9 @@ public class WindowStateManager { // hasSize()/hasPosition() are there to test. WindowGeometry saved = geometries.of(windowId); - double width = saved.hasSize() ? saved.width() : defaults.defaultWidth(); - double height = saved.hasSize() ? saved.height() : defaults.defaultHeight(); + // hasSize() guarantees width and height are non-null. + double width = saved.hasSize() ? Objects.requireNonNull(saved.width()) : defaults.defaultWidth(); + double height = saved.hasSize() ? Objects.requireNonNull(saved.height()) : defaults.defaultHeight(); stage.setMinWidth(defaults.minWidth()); stage.setMinHeight(defaults.minHeight()); @@ -142,10 +153,12 @@ public class WindowStateManager { stage.setHeight(height); Optional usablePosition = Optional.of(saved) - .filter(WindowGeometry::hasPosition) - .map(geometry -> new Rectangle2D(geometry.x(), geometry.y(), width, height)) - .filter(rect -> ScreenBoundsValidator.isVisibleEnough( - rect, connectedScreenBounds(), defaults.minVisibleFraction())); + .filter(WindowGeometry::hasPosition) + // hasPosition() guarantees x and y are non-null. + .map(geometry -> new Rectangle2D(Objects.requireNonNull(geometry.x()), + Objects.requireNonNull(geometry.y()), width, height)) + .filter(rect -> ScreenBoundsValidator.isVisibleEnough( + rect, connectedScreenBounds(), defaults.minVisibleFraction())); if (usablePosition.isPresent()) { stage.setX(usablePosition.get().getMinX()); @@ -158,7 +171,7 @@ public class WindowStateManager { stage.setY(centred.getMinY()); if (saved.hasPosition()) { log.info("Saved position for window '{}' is not on any connected screen; " - + "centring on the primary display", windowId); + + "centring on the primary display", windowId); } } @@ -167,7 +180,8 @@ public class WindowStateManager { if (saved.maximized()) { stage.setMaximized(true); } - } finally { + } + finally { restoring = false; } } diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml index 200e268..056c155 100644 --- a/src/main/resources/application.yaml +++ b/src/main/resources/application.yaml @@ -21,9 +21,17 @@ spring: sql: init: mode: never + autoconfigure: + exclude: + # Boot's FlywayAutoConfiguration would build one Flyway bean against the single routing DataSource + # at context-refresh time, before any library is open — LibraryRoutingDataSource.determineTargetDataSource + # requires a library to already be selected. Each library is its own H2 file needing its own schema, so + # migration instead runs once per pool, inside PersistenceConfiguration.pool(...). + # + # Package as of Boot 4.1, where Flyway's autoconfiguration moved out of spring-boot-autoconfigure into + # its own spring-boot-flyway module. + - org.springframework.boot.flyway.autoconfigure.FlywayAutoConfiguration flyway: - # Enabled from the persistence phase onwards, once db/migration contains the schema. - enabled: false locations: classpath:db/migration messages: basename: messages diff --git a/src/main/resources/css/pholio.css b/src/main/resources/css/pholio.css index d4aeb65..d2a2bb5 100644 --- a/src/main/resources/css/pholio.css +++ b/src/main/resources/css/pholio.css @@ -123,6 +123,30 @@ -fx-padding: 0 0 6 0; } +/* + * A section's title accessory (e.g. the "+" that adds a library folder): invisible until the header row + * itself is hovered, so it does not compete with the title for attention on every visit to the drawer. + */ +.navigation-drawer .drawer-header .drawer-title-accessory { + -fx-opacity: 0; +} + +.navigation-drawer .drawer-header:hover .drawer-title-accessory { + -fx-opacity: 1; +} + +/* + * Per-row action icons (currently just "remove a root folder"): same hover convention as the title + * accessory above, scoped to the row instead of the header. + */ +.navigation-drawer .tree-cell .folder-actions { + -fx-opacity: 0; +} + +.navigation-drawer .tree-cell:hover .folder-actions { + -fx-opacity: 1; +} + /* * Tweaks.EDGE_TO_EDGE drops the tree's border, but not its opaque background: without this the tree * paints a lighter rectangle over the drawer surface. diff --git a/src/main/resources/db/migration/V1__create_library_folder.sql b/src/main/resources/db/migration/V1__create_library_folder.sql new file mode 100644 index 0000000..092568e --- /dev/null +++ b/src/main/resources/db/migration/V1__create_library_folder.sql @@ -0,0 +1,21 @@ +-- Identifiers are quoted so H2 stores them exactly as written (lower snake_case); left unquoted, H2 folds +-- them to upper case by default, and Spring Data JDBC's generated SQL quotes lower snake_case identifiers, +-- so an unquoted table here would be invisible to it ("library_folder" != LIBRARY_FOLDER). +CREATE TABLE "library_folder" +( + "id" BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + "path" VARCHAR(2048) NOT NULL, + "added_at" TIMESTAMP NOT NULL, + CONSTRAINT "uk_library_folder_path" UNIQUE ("path") +); + +CREATE TABLE "library_subfolder" +( + "id" BIGINT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, + "library_folder_id" BIGINT NOT NULL REFERENCES "library_folder" ("id") ON DELETE CASCADE, + "path" VARCHAR(2048) NOT NULL, + "last_scanned_at" TIMESTAMP NOT NULL, + CONSTRAINT "uk_library_subfolder_path" UNIQUE ("library_folder_id", "path") +); + +CREATE INDEX "ix_library_subfolder_folder" ON "library_subfolder" ("library_folder_id"); diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties index 0857807..dfd1530 100644 --- a/src/main/resources/messages.properties +++ b/src/main/resources/messages.properties @@ -2,14 +2,12 @@ # and for Locale.ENGLISH itself. French lives in messages_fr.properties. app.name=Pholio app.title=Pholio — Photo library manager - # --- Modules (header bar) --- module.import=Import module.cull=Cull module.reorganize=Reorganize module.maintenance=Maintenance module.export=Export - # --- Navigation rail --- nav.library=Library nav.albums=Albums @@ -20,7 +18,10 @@ nav.rejected=Rejected nav.filters=Quick filters # Shown in the drawer for destinations that have no widget yet. nav.drawer.empty=Nothing here yet - +# --- Library folders (Photothèque drawer) --- +library.addFolder.tooltip=Add a folder… +library.chooseFolder.dialogTitle=Choose a folder to add +library.removeFolder.tooltip=Remove this folder # --- Header bar actions --- action.search.prompt=Search the library… action.settings=Preferences @@ -33,30 +34,25 @@ action.cancel=Cancel action.apply=Apply action.reset=Reset action.applyAndSave=Apply & save - # --- Inspector --- inspector.details.title=Details # --- Background tasks (popover anchored on the status bar) --- tasks.title=Background tasks tasks.empty=No running task tasks.cancel=Cancel this task - # --- Toasts --- toast.task.succeeded={0} finished toast.task.failed={0} failed toast.task.untitled=Background task - # --- Status bar --- status.library.photos={0} photos status.library.notConfigured=No library configured status.tasks.idle=Idle status.tasks.running={0} task(s) running - # --- Gallery --- gallery.empty.title=Your library is empty gallery.empty.subtitle=Set a root directory, then import or scan your photos. gallery.chooseRoot=Choose a root directory… - # --- Settings window --- # Group titles follow the settings.group. convention, where is a top-level key of # preferences.yaml. Row labels are whatever `label` that file declares; a label matching no key here is @@ -94,7 +90,6 @@ settings.window.height=Window height settings.window.maximized=Window maximised settings.note.windowGeometry=Window position and size are saved automatically and can only be changed \ by editing preferences.yaml. - # Option labels for enumerated preferences, keyed

The service under test is built by hand, not autowired, so every {@code TaskService} dispatch runs + * with {@code Runnable::run} — the background subfolder scan then finishes before {@code add} returns, + * with nothing to await. + */ +@SpringBootTest +@ActiveProfiles(Profiles.CLI) +class LibraryFolderServiceTest { + + @Autowired + private LibraryFolderRepository repository; + @Autowired + private LibraryFolderMapper mapper; + @Autowired + private LibrarySubfolderRepository subfolderRepository; + @Autowired + private LibrarySubfolderMapper subfolderMapper; + @Autowired + private LibraryFolderScanner scanner; + + private AppPreferences preferences; + private List events; + private LibraryFolderService service; + + @BeforeEach + void setUp() { + preferences = PreferencesFixture.fromBundledSchema(); + events = new ArrayList<>(); + + Executor inert = Runnable::run; + TaskService taskService = new TaskService(inert, inert, inert, inert, inert, events::add); + + service = new LibraryFolderService(repository, mapper, subfolderRepository, subfolderMapper, scanner, + preferences, events::add, taskService); + } + + @AfterEach + void cleanUp() { + subfolderRepository.deleteAll(); + repository.deleteAll(); + } + + @Test + void addPersistsANewFolderAndPublishesTheEvent(@TempDir Path folder) { + LibraryFolder added = service.add(folder); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(added.id()).isNotNull(); + softly.assertThat(added.path()).isEqualTo(folder.toAbsolutePath()); + softly.assertThat(repository.findByPath(folder.toAbsolutePath().toString())).isPresent(); + softly.assertThat(events).containsExactly(new LibraryFolderAddedEvent(added)); + }); + } + + @Test + void addIsIdempotentForTheSamePath(@TempDir Path folder) { + LibraryFolder first = service.add(folder); + LibraryFolder second = service.add(folder); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(second.id()).isEqualTo(first.id()); + softly.assertThat(repository.count()).isEqualTo(1); + softly.assertThat(events).as("no second event for an already-known folder").hasSize(1); + }); + } + + @Test + void listReturnsEveryPersistedFolder(@TempDir Path first, @TempDir Path second) { + service.add(first); + service.add(second); + + assertThat(service.list()).extracting(LibraryFolder::path) + .containsExactlyInAnyOrder(first.toAbsolutePath(), second.toAbsolutePath()); + } + + @Test + void addScansEverySubdirectoryRecursivelyExcludingHidden(@TempDir Path root) throws IOException { + Files.createDirectories(root.resolve("2024/summer")); + Files.createDirectories(root.resolve("2025")); + Files.createDirectories(root.resolve(".cache")); + + LibraryFolder added = service.add(root); + + List scanned = subfolderRepository.findByLibraryFolderId(Objects.requireNonNull(added.id())).stream() + .map(subfolderMapper::toDomain) + .map(LibrarySubfolder::path) + .toList(); + + assertThat(scanned).containsExactlyInAnyOrder( + root.resolve("2024").toAbsolutePath(), + root.resolve("2024/summer").toAbsolutePath(), + root.resolve("2025").toAbsolutePath()); + } + + @Test + void removeDeletesTheFolderAndCascadesItsSubfolders(@TempDir Path root) throws IOException { + Files.createDirectories(root.resolve("2024")); + LibraryFolder added = service.add(root); + + service.remove(root); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(repository.findAll()).isEmpty(); + softly.assertThat(subfolderRepository.findByLibraryFolderId(Objects.requireNonNull(added.id()))).isEmpty(); + }); + } + + @Test + void migrateLegacyRootPathIfPresentSeedsExactlyOnceWhenTableIsEmpty(@TempDir Path legacyRoot) { + preferences.setValue("library", "root-path", legacyRoot.toString()); + + service.migrateLegacyRootPathIfPresent(); + service.migrateLegacyRootPathIfPresent(); + + assertThat(repository.findAll()).hasSize(1); + assertThat(repository.findByPath(legacyRoot.toAbsolutePath().toString())).isPresent(); + } + + @Test + void migrateLegacyRootPathIfPresentDoesNothingWhenThePreferenceIsBlank() { + service.migrateLegacyRootPathIfPresent(); + + assertThat(repository.findAll()).isEmpty(); + } +} diff --git a/src/test/java/org/icroco/pholio/infra/library/LibraryServiceTest.java b/src/test/java/org/icroco/pholio/infra/library/LibraryServiceTest.java index 4b9d257..397ea95 100644 --- a/src/test/java/org/icroco/pholio/infra/library/LibraryServiceTest.java +++ b/src/test/java/org/icroco/pholio/infra/library/LibraryServiceTest.java @@ -4,6 +4,7 @@ import org.assertj.core.api.SoftAssertions; import org.icroco.pholio.infra.preferences.AppPreferences; import org.icroco.pholio.infra.preferences.PreferencesFixture; import org.icroco.pholio.infra.support.PholioHome; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -184,11 +185,11 @@ class LibraryServiceTest { private final List opened = new ArrayList<>(); - private String failOn; - private String current; + private @Nullable String failOn; + private @Nullable String current; @Override - public String current() { + public @Nullable String current() { return current; } diff --git a/src/test/java/org/icroco/pholio/infra/media/MediaFormatRegistryTest.java b/src/test/java/org/icroco/pholio/infra/media/MediaFormatRegistryTest.java index e8ae37f..d8c89b7 100644 --- a/src/test/java/org/icroco/pholio/infra/media/MediaFormatRegistryTest.java +++ b/src/test/java/org/icroco/pholio/infra/media/MediaFormatRegistryTest.java @@ -177,16 +177,19 @@ class MediaFormatRegistryTest { } @Override + @SuppressWarnings("NullAway") // deliberate: the registry is not supposed to touch these public MetadataReader reader() { return null; } @Override + @SuppressWarnings("NullAway") // deliberate: the registry is not supposed to touch these public MetadataWriter writer() { return null; } @Override + @SuppressWarnings("NullAway") // deliberate: the registry is not supposed to touch these public ThumbnailAccessor thumbnails() { return null; } diff --git a/src/test/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapperTest.java b/src/test/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapperTest.java new file mode 100644 index 0000000..6b8ac2e --- /dev/null +++ b/src/test/java/org/icroco/pholio/infra/persistence/folder/LibraryFolderMapperTest.java @@ -0,0 +1,50 @@ +package org.icroco.pholio.infra.persistence.folder; + +import org.assertj.core.api.SoftAssertions; +import org.icroco.pholio.domain.library.LibraryFolder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Path; +import java.time.Instant; + +class LibraryFolderMapperTest { + + private final LibraryFolderMapper mapper = new LibraryFolderMapperImpl(); + + @Test + void roundTripsBetweenDomainAndEntity(@TempDir Path folder) { + Instant addedAt = Instant.now(); + LibraryFolder domain = LibraryFolder.builder().id(7L).path(folder).addedAt(addedAt).build(); + + LibraryFolderEntity entity = mapper.toEntity(domain); + LibraryFolder back = mapper.toDomain(LibraryFolderEntity.builder() + .id(9L) + .path(entity.getPath()) + .addedAt(entity.getAddedAt()) + .build()); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(entity.getId()).as("the entity id is assigned by the database, not copied in").isNull(); + softly.assertThat(entity.getPath()).isEqualTo(folder.toAbsolutePath().toString()); + softly.assertThat(back.path()).isEqualTo(folder.toAbsolutePath()); + softly.assertThat(back.addedAt()).isEqualTo(addedAt); + }); + } + + @Test + void aRelativePathBecomesAbsoluteOnTheEntity() { + LibraryFolder domain = LibraryFolder.builder() + .id(1L) + .path(Path.of("relative", "folder")) + .addedAt(Instant.now()) + .build(); + + LibraryFolderEntity entity = mapper.toEntity(domain); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(Path.of(entity.getPath())).isAbsolute(); + softly.assertThat(entity.getPath()).endsWith(Path.of("relative", "folder").toString()); + }); + } +} diff --git a/src/test/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapperTest.java b/src/test/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapperTest.java new file mode 100644 index 0000000..0b3f2e5 --- /dev/null +++ b/src/test/java/org/icroco/pholio/infra/persistence/folder/LibrarySubfolderMapperTest.java @@ -0,0 +1,37 @@ +package org.icroco.pholio.infra.persistence.folder; + +import org.assertj.core.api.SoftAssertions; +import org.icroco.pholio.domain.library.LibrarySubfolder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Path; +import java.time.Instant; + +class LibrarySubfolderMapperTest { + + private final LibrarySubfolderMapper mapper = new LibrarySubfolderMapperImpl(); + + @Test + void roundTripsBetweenDomainAndEntity(@TempDir Path folder) { + Instant scannedAt = Instant.now(); + LibrarySubfolder domain = new LibrarySubfolder(3L, 42L, folder, scannedAt); + + LibrarySubfolderEntity entity = mapper.toEntity(domain); + LibrarySubfolder back = mapper.toDomain(LibrarySubfolderEntity.builder() + .id(9L) + .libraryFolderId(entity.getLibraryFolderId()) + .path(entity.getPath()) + .lastScannedAt(entity.getLastScannedAt()) + .build()); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(entity.getId()).as("the entity id is assigned by the database, not copied in").isNull(); + softly.assertThat(entity.getLibraryFolderId()).isEqualTo(42L); + softly.assertThat(entity.getPath()).isEqualTo(folder.toAbsolutePath().toString()); + softly.assertThat(back.path()).isEqualTo(folder.toAbsolutePath()); + softly.assertThat(back.libraryFolderId()).isEqualTo(42L); + softly.assertThat(back.lastScannedAt()).isEqualTo(scannedAt); + }); + } +} diff --git a/src/test/java/org/icroco/pholio/infra/preferences/PreferenceItemTest.java b/src/test/java/org/icroco/pholio/infra/preferences/PreferenceItemTest.java index c4015a9..d09f7c0 100644 --- a/src/test/java/org/icroco/pholio/infra/preferences/PreferenceItemTest.java +++ b/src/test/java/org/icroco/pholio/infra/preferences/PreferenceItemTest.java @@ -1,10 +1,12 @@ package org.icroco.pholio.infra.preferences; import org.assertj.core.api.SoftAssertions; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -76,7 +78,7 @@ class PreferenceItemTest { softly.assertThat(PreferenceType.STRING_LIST.coerce(List.of())).isEqualTo(List.of()); softly.assertThat(PreferenceType.STRING_LIST.coerce(null)).isNull(); @SuppressWarnings("unchecked") - List coerced = (List) PreferenceType.STRING_LIST.coerce(List.of("a")); + List coerced = (List) Objects.requireNonNull(PreferenceType.STRING_LIST.coerce(List.of("a"))); softly.assertThatThrownBy(() -> coerced.add("b")) .isInstanceOf(UnsupportedOperationException.class); }); @@ -197,7 +199,7 @@ class PreferenceItemTest { assertThatThrownBy(item::normalise).isInstanceOf(IllegalArgumentException.class); } - private static PreferenceItem item(PreferenceType type, Object defaultValue) { + private static PreferenceItem item(PreferenceType type, @Nullable Object defaultValue) { PreferenceItem item = new PreferenceItem<>(); item.setType(type); item.setLabel("settings.test"); diff --git a/src/test/java/org/icroco/pholio/ui/FxTestToolkit.java b/src/test/java/org/icroco/pholio/ui/FxTestToolkit.java index 3c34e68..8f4034c 100644 --- a/src/test/java/org/icroco/pholio/ui/FxTestToolkit.java +++ b/src/test/java/org/icroco/pholio/ui/FxTestToolkit.java @@ -3,9 +3,11 @@ package org.icroco.pholio.ui; import javafx.application.Platform; import org.icroco.pholio.infra.preferences.AppPreferences; import org.icroco.pholio.ui.i18n.I18nService; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Assumptions; import org.springframework.context.support.ResourceBundleMessageSource; +import java.util.Objects; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -26,7 +28,7 @@ public final class FxTestToolkit { private static final long TIMEOUT_SECONDS = 20; - private static Boolean available; + private static @Nullable Boolean available; private FxTestToolkit() { } @@ -62,7 +64,8 @@ public final class FxTestToolkit { if (failure.get() != null) { throw failure.get(); } - return result.get(); + // work.get() ran and populated result before the latch counted down, unless it threw (handled above). + return Objects.requireNonNull(result.get()); } /** @@ -71,7 +74,7 @@ public final class FxTestToolkit { public static void runOnFxThread(Runnable work) { onFxThread(() -> { work.run(); - return null; + return true; // dummy non-null result, discarded by the caller }); } diff --git a/src/test/java/org/icroco/pholio/ui/PreferenceSchemaCouplingTest.java b/src/test/java/org/icroco/pholio/ui/PreferenceSchemaCouplingTest.java index e07216c..b4c4050 100644 --- a/src/test/java/org/icroco/pholio/ui/PreferenceSchemaCouplingTest.java +++ b/src/test/java/org/icroco/pholio/ui/PreferenceSchemaCouplingTest.java @@ -12,6 +12,7 @@ import org.springframework.context.support.ResourceBundleMessageSource; import java.util.List; import java.util.Locale; +import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; @@ -105,7 +106,7 @@ class PreferenceSchemaCouplingTest { // Row labels, in contrast, are checked for every group: an invisible item still has to carry // a resolvable label for the day it is switched on. preferences.getGroups().forEach((group, items) -> items.forEach((key, item) -> softly - .assertThat(messages.getMessage(item.getLabel(), null, item.getLabel(), locale)) + .assertThat(messages.getMessage(Objects.requireNonNull(item.getLabel()), null, item.getLabel(), locale)) .as("label of %s.%s in %s", group, key, locale) .isNotEqualTo(item.getLabel()))); } diff --git a/src/test/java/org/icroco/pholio/ui/debug/DevToolsTest.java b/src/test/java/org/icroco/pholio/ui/debug/DevToolsTest.java index 5961381..68b9f5b 100644 --- a/src/test/java/org/icroco/pholio/ui/debug/DevToolsTest.java +++ b/src/test/java/org/icroco/pholio/ui/debug/DevToolsTest.java @@ -14,6 +14,8 @@ import org.icroco.pholio.ui.FxTestToolkit; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.util.Objects; + import static org.assertj.core.api.Assertions.*; import static org.icroco.pholio.ui.FxTestToolkit.onFxThread; import static org.mockito.Mockito.mock; @@ -115,7 +117,7 @@ class DevToolsTest { devTools.install(stage); stage.show(); }); - FxTestToolkit.runOnFxThread(stage.getScene().getAccelerators().get(F12)); + FxTestToolkit.runOnFxThread(Objects.requireNonNull(stage.getScene().getAccelerators().get(F12))); assertThatCode(() -> FxTestToolkit.runOnFxThread(devTools::dispose)).doesNotThrowAnyException(); diff --git a/src/test/java/org/icroco/pholio/ui/shell/LibraryFolderTreeTest.java b/src/test/java/org/icroco/pholio/ui/shell/LibraryFolderTreeTest.java index 3c382f7..bbcbc6b 100644 --- a/src/test/java/org/icroco/pholio/ui/shell/LibraryFolderTreeTest.java +++ b/src/test/java/org/icroco/pholio/ui/shell/LibraryFolderTreeTest.java @@ -2,10 +2,15 @@ package org.icroco.pholio.ui.shell; import javafx.scene.control.TreeItem; import org.assertj.core.api.SoftAssertions; +import org.icroco.pholio.domain.library.LibraryFolder; +import org.icroco.pholio.infra.library.LibraryChangedEvent; +import org.icroco.pholio.infra.library.LibraryFolderAddedEvent; +import org.icroco.pholio.infra.library.LibraryFolderService; import org.icroco.pholio.infra.preferences.AppPreferences; import org.icroco.pholio.infra.preferences.PreferencesFixture; import org.icroco.pholio.infra.task.TaskService; import org.icroco.pholio.ui.FxTestToolkit; +import org.icroco.pholio.ui.i18n.I18nService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -13,35 +18,37 @@ import org.junit.jupiter.api.io.TempDir; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Instant; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicInteger; import static org.assertj.core.api.Assertions.assertThat; import static org.icroco.pholio.ui.FxTestToolkit.onFxThread; +import static org.icroco.pholio.ui.FxTestToolkit.runOnFxThread; +import static org.mockito.Mockito.*; /** - * The library drawer's folder tree, over a real directory. + * The library drawer's folder tree, over real directories. + * + *

{@code LibraryFolderService} is mocked — persistence is {@link LibraryFolderService}'s own concern, + * covered by {@code LibraryFolderServiceTest} — but the directories it reports are real, so + * {@link FolderTreeItem}'s lazy subfolder loading still runs unmodified underneath. * *

The {@code TaskService} handed in runs its {@code BACKGROUND_SYNC} work with {@code Runnable::run}, so - * listings happen inline and the assertions need no waiting. That indirection is why {@code TaskService} is - * a constructor parameter at all: with the production pools the test would have to poll, and a lazy-loading - * tree is exactly the kind of thing whose timing makes flaky tests. - * - *

It also counts calls, which is how laziness is proven — not by observing what is displayed, but by - * observing that the directories nobody opened were never read. + * every dispatch — the initial folder load, a directory listing, an add or a remove — happens inline and the + * assertions need no waiting. */ class LibraryFolderTreeTest { - private static final String LIBRARY_GROUP = "library"; - private static final String ROOT_PATH_KEY = "root-path"; - @TempDir private Path library; - private AppPreferences preferences; - private AtomicInteger listings; - private LibraryFolderTree section; + private I18nService i18n; + private LibraryFolderService libraryFolderService; + private AtomicInteger backgroundRuns; + private TaskService taskService; + private LibraryFolderTree section; @BeforeEach void buildLibrary() throws IOException { @@ -52,17 +59,20 @@ class LibraryFolderTreeTest { Files.createDirectories(library.resolve(".cache")); Files.writeString(library.resolve("photo.jpg"), "not really"); - preferences = PreferencesFixture.fromBundledSchema(); - preferences.setValue(LIBRARY_GROUP, ROOT_PATH_KEY, library.toString()); + AppPreferences preferences = PreferencesFixture.fromBundledSchema(); + i18n = FxTestToolkit.i18n(preferences); - listings = new AtomicInteger(); + libraryFolderService = mock(LibraryFolderService.class); + when(libraryFolderService.list()).thenReturn(List.of(folder(1L, library))); + + backgroundRuns = new AtomicInteger(); Executor counting = command -> { - listings.incrementAndGet(); + backgroundRuns.incrementAndGet(); command.run(); }; Executor inert = Runnable::run; - TaskService taskService = new TaskService(inert, inert, inert, inert, counting, event -> { }); - section = onFxThread(() -> new LibraryFolderTree(FxTestToolkit.i18n(preferences), preferences, taskService)); + taskService = new TaskService(inert, inert, inert, inert, counting, event -> {}); + section = onFxThread(() -> new LibraryFolderTree(i18n, libraryFolderService, taskService)); } @Test @@ -73,18 +83,32 @@ class LibraryFolderTreeTest { }); } + @Test + void offersTheAddFolderButtonAsATitleAccessory() { + assertThat(section.titleAccessory()).isPresent(); + } + @Test void rootsTheTreeAtTheConfiguredDirectory() { SoftAssertions.assertSoftly(softly -> { - softly.assertThat(root().getValue()).isEqualTo(library); - softly.assertThat(root().isExpanded()).as("the first level is worth showing straight away").isTrue(); + softly.assertThat(firstRoot().getValue()).isEqualTo(library); softly.assertThat(section.tree().isVisible()).isTrue(); + softly.assertThat(section.tree().isShowRoot()).as("only real folders are shown, not the sentinel").isFalse(); }); } + @Test + void showsOneTopLevelItemPerConfiguredFolder(@TempDir Path other) { + LibraryFolderTree multiRoot = buildSection(List.of(folder(1L, library), folder(2L, other))); + + List> roots = childrenOf(invisibleRootOf(multiRoot)); + + assertThat(roots).extracting(TreeItem::getValue).containsExactly(library, other); + } + @Test void showsSubdirectoriesSortedAndNothingElse() { - assertThat(namesOf(childrenOf(root()))).containsExactly("2024", "2025"); + assertThat(namesOf(childrenOf(firstRoot()))).containsExactly("2024", "2025"); } /** @@ -92,12 +116,16 @@ class LibraryFolderTreeTest { */ @Test void readsOnlyTheDirectoriesThatWereOpened() { + int baseline = backgroundRuns.get(); // the initial folder-list load has already run, during setup + SoftAssertions.assertSoftly(softly -> { - List> years = childrenOf(root()); - softly.assertThat(listings.get()).as("only the root was read").isEqualTo(1); + List> years = childrenOf(firstRoot()); + softly.assertThat(backgroundRuns.get() - baseline).as("only the root was read").isEqualTo(1); List> months = childrenOf(years.getFirst()); - softly.assertThat(listings.get()).as("one more, for the folder just opened").isEqualTo(2); + softly.assertThat(backgroundRuns.get() - baseline) + .as("one more, for the folder just opened") + .isEqualTo(2); softly.assertThat(namesOf(months)).containsExactly("été"); }); } @@ -107,7 +135,7 @@ class LibraryFolderTreeTest { */ @Test void leafStatusIsKnownOnlyOnceAFolderHasBeenRead() { - List> years = childrenOf(root()); + List> years = childrenOf(firstRoot()); TreeItem empty = years.get(1); SoftAssertions.assertSoftly(softly -> { @@ -118,43 +146,89 @@ class LibraryFolderTreeTest { } @Test - void withoutARootItSaysSoInsteadOfShowingAnEmptyTree() { - FxTestToolkit.runOnFxThread(() -> preferences.setValue(LIBRARY_GROUP, ROOT_PATH_KEY, "")); + void withoutAnyFolderItSaysSoInsteadOfShowingAnEmptyTree() { + LibraryFolderTree empty = buildSection(List.of()); SoftAssertions.assertSoftly(softly -> { - softly.assertThat(section.tree().getRoot()).isNull(); - softly.assertThat(section.tree().isVisible()).isFalse(); - softly.assertThat(section.tree().isManaged()).isFalse(); - softly.assertThat(section.getChildren().stream().anyMatch(node -> node.isVisible() && node != section.tree())) + softly.assertThat(childrenOf(invisibleRootOf(empty))).isEmpty(); + softly.assertThat(empty.tree().isVisible()).isFalse(); + softly.assertThat(empty.tree().isManaged()).isFalse(); + softly.assertThat(empty.getChildren().stream().anyMatch(node -> node.isVisible() && node != empty.tree())) .as("the notice is shown in its place") .isTrue(); }); } /** - * Choosing a directory from the gallery must repopulate the tree, not require a restart. + * The "+" button dispatches through the service and does not touch the tree itself — the row only + * appears once {@link LibraryFolderAddedEvent} confirms the folder was actually persisted. */ @Test - void followsANewRootChosenElsewhere(@TempDir Path other) throws IOException { - Files.createDirectories(other.resolve("archive")); + void appendsARowOnceTheServiceConfirmsAnAddedFolder(@TempDir Path other) { + LibraryFolder added = folder(2L, other); - FxTestToolkit.runOnFxThread(() -> preferences.setValue(LIBRARY_GROUP, ROOT_PATH_KEY, other.toString())); + runOnFxThread(() -> section.onFolderAdded(new LibraryFolderAddedEvent(added))); + + List> roots = childrenOf(invisibleRootOf(section)); + assertThat(roots).extracting(TreeItem::getValue).containsExactly(library, other); + } + + /** + * The "-" button (only ever offered on a root's own row) removes through the service and drops the + * row — {@link LibraryFolderTree#removeForTest} stands in for the button click, which needs a rendered + * scene this test suite does not build. + */ + @Test + void removingARootCallsTheServiceAndDropsItsRow() { + TreeItem root = firstRoot(); + + runOnFxThread(() -> section.removeForTest(root)); SoftAssertions.assertSoftly(softly -> { - softly.assertThat(root().getValue()).isEqualTo(other); - softly.assertThat(namesOf(childrenOf(root()))).containsExactly("archive"); + softly.assertThat(childrenOf(invisibleRootOf(section))).isEmpty(); + softly.assertThat(section.tree().isVisible()).isFalse(); }); + verify(libraryFolderService).remove(library); + } + + /** + * A different library means a different database — whatever this drawer was showing belongs to a + * library that is no longer open, so a switch must reload from {@link LibraryFolderService}, not just + * append to what was already there. + */ + @Test + void switchingLibraryReplacesTheTreeWithTheNewLibrarysFolders(@TempDir Path other) { + when(libraryFolderService.list()).thenReturn(List.of(folder(9L, other))); + + runOnFxThread(() -> section.onLibraryChanged(new LibraryChangedEvent("christophe", "holidays"))); + + List> roots = childrenOf(invisibleRootOf(section)); + assertThat(roots).extracting(TreeItem::getValue).containsExactly(other); } @Test void disposeReleasesTheTree() { - FxTestToolkit.runOnFxThread(section::dispose); + runOnFxThread(section::dispose); assertThat(section.tree().getRoot()).isNull(); } - private TreeItem root() { - return onFxThread(() -> section.tree().getRoot()); + private LibraryFolderTree buildSection(List folders) { + LibraryFolderService service = mock(LibraryFolderService.class); + when(service.list()).thenReturn(folders); + return onFxThread(() -> new LibraryFolderTree(i18n, service, taskService)); + } + + private static LibraryFolder folder(long id, Path path) { + return LibraryFolder.builder().id(id).path(path).addedAt(Instant.now()).build(); + } + + private TreeItem firstRoot() { + return onFxThread(() -> section.tree().getRoot().getChildren().getFirst()); + } + + private static TreeItem invisibleRootOf(LibraryFolderTree tree) { + return onFxThread(() -> tree.tree().getRoot()); } /** diff --git a/src/test/java/org/icroco/pholio/ui/shell/NavigationDrawerTest.java b/src/test/java/org/icroco/pholio/ui/shell/NavigationDrawerTest.java index 91f3120..1fefe16 100644 --- a/src/test/java/org/icroco/pholio/ui/shell/NavigationDrawerTest.java +++ b/src/test/java/org/icroco/pholio/ui/shell/NavigationDrawerTest.java @@ -1,6 +1,7 @@ package org.icroco.pholio.ui.shell; import javafx.scene.Node; +import javafx.scene.control.Button; import javafx.scene.control.Label; import org.assertj.core.api.SoftAssertions; import org.icroco.pholio.infra.preferences.AppPreferences; @@ -11,6 +12,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.List; +import java.util.Objects; +import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -40,16 +43,10 @@ class NavigationDrawerTest { NavigationDrawer drawer = drawerOf(); SoftAssertions.assertSoftly(softly -> { - onFxThread(() -> { - drawer.show(NavigationDestination.LIBRARY); - return null; - }); + FxTestToolkit.runOnFxThread(() -> drawer.show(NavigationDestination.LIBRARY)); softly.assertThat(drawer.titleText()).isEqualTo(i18n.get("nav.library")); - onFxThread(() -> { - drawer.show(NavigationDestination.MAP); - return null; - }); + FxTestToolkit.runOnFxThread(() -> drawer.show(NavigationDestination.MAP)); softly.assertThat(drawer.titleText()).isEqualTo(i18n.get("nav.map")); }); } @@ -75,7 +72,8 @@ class NavigationDrawerTest { SoftAssertions.assertSoftly(softly -> { softly.assertThat(drawer.bodyContent()).isInstanceOf(Label.class); - softly.assertThat(((Label) drawer.bodyContent()).getText()).isEqualTo(i18n.get("nav.drawer.empty")); + softly.assertThat(((Label) Objects.requireNonNull(drawer.bodyContent())).getText()) + .isEqualTo(i18n.get("nav.drawer.empty")); }); } @@ -108,16 +106,36 @@ class NavigationDrawerTest { .hasMessageContaining(SecondStub.class.getName()); } + /** + * The "+" button next to "Phototèque" is this mechanism in production; a section with no accessory + * (every other destination today) must not leave a stale one behind when it takes over the header. + */ + @Test + void showsASectionsTitleAccessoryNextToTheTitleAndDropsItWhenGone() { + Button accessory = new Button(); + StubSection library = new StubSectionWithAccessory(NavigationDestination.LIBRARY, accessory); + StubSection tags = new StubSection(NavigationDestination.TAGS); + NavigationDrawer drawer = drawerOf(library, tags); + + show(drawer, NavigationDestination.LIBRARY); + List withAccessory = onFxThread(drawer::headerChildren); + + show(drawer, NavigationDestination.TAGS); + List withoutAccessory = onFxThread(drawer::headerChildren); + + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(withAccessory).contains(accessory); + softly.assertThat(withoutAccessory).doesNotContain(accessory); + }); + } + @Test void disposingTheDrawerDisposesItsSections() { StubSection albums = new StubSection(NavigationDestination.ALBUMS); StubSection tags = new StubSection(NavigationDestination.TAGS); NavigationDrawer drawer = drawerOf(albums, tags); - onFxThread(() -> { - drawer.dispose(); - return null; - }); + FxTestToolkit.runOnFxThread(drawer::dispose); SoftAssertions.assertSoftly(softly -> { softly.assertThat(albums.disposed).isTrue(); @@ -163,6 +181,25 @@ class NavigationDrawerTest { } } + /** + * A {@link StubSection} that also claims a title accessory, for {@link + * #showsASectionsTitleAccessoryNextToTheTitleAndDropsItWhenGone()}. + */ + private static final class StubSectionWithAccessory extends StubSection { + + private final Node accessory; + + StubSectionWithAccessory(NavigationDestination destination, Node accessory) { + super(destination); + this.accessory = accessory; + } + + @Override + public Optional titleAccessory() { + return Optional.of(accessory); + } + } + /** * Named subclasses, so the conflict message can be asserted to name both culprits. */ diff --git a/src/test/java/org/icroco/pholio/ui/shell/NavigationRailResizeTest.java b/src/test/java/org/icroco/pholio/ui/shell/NavigationRailResizeTest.java index 7b49d62..14f7f08 100644 --- a/src/test/java/org/icroco/pholio/ui/shell/NavigationRailResizeTest.java +++ b/src/test/java/org/icroco/pholio/ui/shell/NavigationRailResizeTest.java @@ -16,6 +16,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.List; +import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; import static org.icroco.pholio.ui.FxTestToolkit.onFxThread; @@ -156,7 +157,7 @@ class NavigationRailResizeTest { } private double persistedWidth() { - return preferences.getValue("ui", "left-panel-width", Double.class); + return Objects.requireNonNull(preferences.getValue("ui", "left-panel-width", Double.class)); } private void press(double x) { diff --git a/src/test/java/org/icroco/pholio/ui/shell/NavigationRailTest.java b/src/test/java/org/icroco/pholio/ui/shell/NavigationRailTest.java index dc2f032..57f415d 100644 --- a/src/test/java/org/icroco/pholio/ui/shell/NavigationRailTest.java +++ b/src/test/java/org/icroco/pholio/ui/shell/NavigationRailTest.java @@ -3,8 +3,10 @@ package org.icroco.pholio.ui.shell; import javafx.scene.control.Button; import javafx.scene.layout.Pane; import org.assertj.core.api.SoftAssertions; +import org.icroco.pholio.infra.library.LibraryFolderService; import org.icroco.pholio.infra.preferences.AppPreferences; import org.icroco.pholio.infra.preferences.PreferencesFixture; +import org.icroco.pholio.infra.task.TaskService; import org.icroco.pholio.ui.FxTestToolkit; import org.icroco.pholio.ui.ViewSwitcher; import org.icroco.pholio.ui.event.ViewType; @@ -18,6 +20,7 @@ import org.junit.jupiter.api.Test; import org.springframework.context.ApplicationContext; import java.util.List; +import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; import static org.icroco.pholio.ui.FxTestToolkit.onFxThread; @@ -44,7 +47,9 @@ class NavigationRailTest { private AppPreferences preferences; private I18nService i18n; + @SuppressWarnings("NullAway.Init") // set inside the onFxThread lambda in buildRail(), which runs synchronously private NavigationDrawer drawer; + @SuppressWarnings("NullAway.Init") // set inside the onFxThread lambda in buildRail(), which runs synchronously private ViewSwitcher viewSwitcher; private NavigationRail rail; private List