fix(file): deduplicate book files and protect source library root in FileMoveService by dsmouse · Pull Request #294 · grimmory-tools/grimmory

dsmouse · 2026-03-30T23:21:09Z

Fixes #293

Changes
FileMoveService

Two bugs fixed in both processSingleMove and moveBookToMatchLibraryPattern:

Duplicate BookFileEntity entries — getBookFiles() can return duplicates from the Hibernate lazy collection. Added .stream().distinct().toList() before iterating, so validation, staging, commit, and DB update loops each see each file exactly once.

libraryRoots missing source library — libraryRoots was built only from the target library's paths. On a cross-library move the source library root was absent, giving deleteEmptyParentDirsUpToLibraryFolders no stop boundary on the source side. Fixed by adding the source library path to the set before cleanup.

Summary by CodeRabbit

Bug Fixes
- Improved file move operations to correctly handle duplicate files through deduplication during validation, staging, and database updates.
- Enhanced directory cleanup safety to prevent accidental deletion of directories outside the source library by strengthening protected path verification.

…ileMoveService

coderabbitai · 2026-03-30T23:22:12Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 402c1d95-97b2-4f3b-b0ac-474458145da7

📥 Commits

Reviewing files that changed from the base of the PR and between f1cbcf3 and bc873d4.

📒 Files selected for processing (1)

booklore-api/src/main/java/org/booklore/service/file/FileMoveService.java

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (1)

booklore-api/src/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce @Autowired field injection
Use MapStruct for entity/DTO mapping

Files:

booklore-api/src/main/java/org/booklore/service/file/FileMoveService.java

🧠 Learnings (2)

📚 Learning: 2026-03-26T01:46:48.863Z

Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to booklore-api/src/**/*.java : Use MapStruct for entity/DTO mapping

Applied to files:

booklore-api/src/main/java/org/booklore/service/file/FileMoveService.java

📚 Learning: 2026-03-23T18:36:05.843Z

Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 146
File: booklore-api/src/main/java/org/booklore/util/FileService.java:151-168
Timestamp: 2026-03-23T18:36:05.843Z
Learning: In `booklore-api/src/main/java/org/booklore/util/FileService.java`, the `"bin"` search path in `getSystemSearchPath()` is intentionally left as a relative path (not derived from `appProperties.getPathConfig()`), so binary lookup resolves relative to the process working directory. Do not flag this as an issue.

Applied to files:

booklore-api/src/main/java/org/booklore/service/file/FileMoveService.java

🔇 Additional comments (5)

booklore-api/src/main/java/org/booklore/service/file/FileMoveService.java (5)

13-13: LGTM!

Import added to support the explicit List<BookFileEntity> type declaration for the deduplicated list.

128-129: LGTM — deduplication correctly prevents duplicate processing.

The .stream().distinct().toList() approach properly handles the Hibernate lazy collection duplicates since they are reference-equal objects. All validation, staging, and database update loops now consistently iterate over the deduplicated bookFiles list, preventing the "source not found" errors on second pass.

Also applies to: 147-154, 156-174, 192-201

217-228: LGTM — essential fix for cross-library move safety.

This correctly protects the source library root during cleanup:

Changed to HashSet::new to allow mutation

Added source library path from bookEntity.getLibraryPath() (which still holds the original source before DB update)

Path normalization matches deleteEmptyParentDirsUpToLibraryFolders implementation (per FileMoveHelper.java:179-195)

This prevents deleteEmptyParentDirsUpToLibraryFolders from traversing above the source library root on cross-library moves.

282-283: LGTM — consistent deduplication applied to moveSingleFile.

Same pattern as processSingleMove: the deduplicated bookFiles list is used across all validation, staging, and database update loops.

Also applies to: 303-310, 319-337, 356-365

380-391: LGTM — consistent pattern, though technically redundant in this method.

Since moveSingleFile operates within the same library (no target library parameter), bookWithFiles.getLibraryPath() is already included in bookWithFiles.getLibraryPath().getLibrary().getLibraryPaths(). The explicit addition at line 387 is redundant here but harmless — it maintains consistency with processSingleMove and acts as defensive coding.

📝 Walkthrough

Walkthrough

FileMoveService is updated to fix two bugs: deduplicating BookFileEntity lists before iteration to prevent duplicate processing failures, and adding the source library root path to the cleanup boundary set to prevent directory traversal above intended limits during cross-library moves.

Changes

Cohort / File(s)	Summary
Bug Fix - Book File Deduplication and Directory Cleanup `booklore-api/src/main/java/org/booklore/service/file/FileMoveService.java`	Deduplicates `bookFiles` list using `.stream().distinct().toList()` to prevent duplicate processing in validation and transactional loops. Extends `libraryRoots` to include source library root path alongside target roots, preventing unwanted traversal above the source library during empty-directory cleanup in both `processSingleMove()` and `moveSingleFile()` methods.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

backend, enhancement

Suggested reviewers

balazs-szucs

Poem

🐰 Hops of joy through libraries bright,
Files dance without duplication's plight,
Roots are guarded, paths stay clean,
Cross-library moves, a smoother scene!
The bugs are hopped away tonight! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title follows the conventional commit format with type(scope) prefix and clearly describes the main fixes: deduplicating book files and protecting source library root in FileMoveService.
Description check	✅ Passed	The PR description includes all required template sections with clear, detailed explanations of the two bugs fixed and their solutions.
Linked Issues check	✅ Passed	The code changes fully address both objectives from issue `#293`: deduplicating BookFileEntity entries via .stream().distinct().toList() and adding source library root to libraryRoots set to prevent over-traversal during cleanup.
Out of Scope Changes check	✅ Passed	All changes are directly related to the two bugs described in issue `#293`, with no extraneous modifications or unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

✨ Simplify code

Create PR with simplified code

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

balazs-szucs · 2026-03-31T11:04:21Z

Thank you for this, both changes are very welcome from me!

…ileMoveService (grimmory-tools#294) Co-authored-by: msmouse <git@dsmouse.com>

…ileMoveService (#294) Co-authored-by: msmouse <git@dsmouse.com>

…ileMoveService (grimmory-tools#294) Co-authored-by: msmouse <git@dsmouse.com>

fix(file): deduplicate bookFiles and protect source library root in F…

bc873d4

…ileMoveService

coderabbitai Bot added backend enhancement labels Mar 30, 2026

balazs-szucs merged commit 4992297 into grimmory-tools:develop Mar 31, 2026
14 checks passed

This was referenced Apr 4, 2026

fix(file): allow physical books to be moved between libraries #296

Merged

fix(dto): add DailyListeningDto and update session-related DTOs to use LocalDate #403

Merged

Bookdrop finalizer fails with "Book ID missing after import" #427

Closed

coderabbitai Bot mentioned this pull request Apr 12, 2026

Empty folders don't get cleaned up on metadata update, when sidecar files exist #482

Closed

1 task

zachyale pushed a commit to zachyale/grimmory that referenced this pull request Apr 17, 2026

fix(file): deduplicate bookFiles and protect source library root in F…

26079f6

…ileMoveService (grimmory-tools#294) Co-authored-by: msmouse <git@dsmouse.com>

zachyale pushed a commit to zachyale/grimmory that referenced this pull request Apr 17, 2026

fix(file): deduplicate bookFiles and protect source library root in F…

20b91a1

…ileMoveService (grimmory-tools#294) Co-authored-by: msmouse <git@dsmouse.com>

zachyale pushed a commit that referenced this pull request Apr 17, 2026

fix(file): deduplicate bookFiles and protect source library root in F…

ec2642f

…ileMoveService (#294) Co-authored-by: msmouse <git@dsmouse.com>

coderabbitai Bot mentioned this pull request Apr 18, 2026

fix(file-mover): update file previews and loading state to use signals #650

Closed

zachyale pushed a commit that referenced this pull request Apr 22, 2026

fix(file): deduplicate bookFiles and protect source library root in F…

1625055

…ileMoveService (#294) Co-authored-by: msmouse <git@dsmouse.com>

This was referenced Apr 23, 2026

Organize files leaves empty source folders when sidecar is enabled #820

Closed

Empty folders don't get cleaned up on metadata update, when sidecar files exist #821

Closed

fix(file-move): clean up emptied source folders after sidecar move #822

Merged

coderabbitai Bot mentioned this pull request May 6, 2026

Metadata Manager crashes with "An unexpected error occurred" during background file moves — race condition in writeMetadataToFile() #1162

Closed

dsmouse added a commit to dsmouse/grimmory that referenced this pull request May 11, 2026

fix(file): deduplicate bookFiles and protect source library root in F…

346f1a1

…ileMoveService (grimmory-tools#294) Co-authored-by: msmouse <git@dsmouse.com>

coderabbitai Bot mentioned this pull request May 21, 2026

Orphaned DB entries for manually relocated audiobook files #1442

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(file): deduplicate book files and protect source library root in FileMoveService#294

fix(file): deduplicate book files and protect source library root in FileMoveService#294
balazs-szucs merged 1 commit into
grimmory-tools:developfrom
dsmouse:fix/file-move-duplicate-bookfiles

dsmouse commented Mar 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 30, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Uh oh!

Uh oh!

balazs-szucs commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

dsmouse commented Mar 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Uh oh!

Uh oh!

balazs-szucs commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dsmouse commented Mar 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 30, 2026 •

edited

Loading