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

[zachyale]

Description

This fixes an organize file bug where empty source folders could be left behind after a move. The cleanup step ran before sidecar files were moved, so the source directory still technically had content in it during cleanup, and was left behind once sidecars were relocated.

Linked Issue: #821

Changes

• move sidecar relocation ahead of empty directory cleanup in single file/bulk file flow

Summary by CodeRabbit

• Bug Fixes

  • Fixed file operation sequencing to ensure metadata files are processed before empty parent directory cleanup, improving data consistency.

• Tests

  • Enhanced test coverage to validate the correct sequencing of file operations with metadata handling during bulk file moves.

[coderabbitai]

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: 5be669cd-c357-4ae0-959e-8b01f0f4f57f

📥 Commits

Reviewing files that changed from the base of the PR and between 03389d7 and 709e2dc.

📒 Files selected for processing (3)

• backend/src/main/java/org/booklore/service/file/FileMoveService.java
• backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java
• backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java

📜 Recent review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: Test Suite / Frontend Tests
• GitHub Check: Test Suite / Backend Tests
• GitHub Check: Analyze (java-kotlin)
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Frontend Lint Threshold Check

🧰 Additional context used

📓 Path-based instructions (2)

backend/src/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

backend/src/**/*.java: Use 4-space indentation and match surrounding Java style in backend code
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce @Autowired field injection in backend code
Use MapStruct for entity/DTO mapping in backend code
Keep JPA entities on the *Entity suffix in backend code

Files:

• backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java
• backend/src/main/java/org/booklore/service/file/FileMoveService.java
• backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java

backend/src/test/**/*.java

📄 CodeRabbit inference engine (AGENTS.md)

Prefer focused unit tests; use @SpringBootTest only when the Spring context is required in backend code

Files:

• backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java
• backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java

🧠 Learnings (2)

📚 Learning: 2026-04-10T08:15:37.436Z

Learnt from: imnotjames
Repo: grimmory-tools/grimmory PR: 449
File: booklore-api/src/main/java/org/booklore/service/book/BookDownloadService.java:139-145
Timestamp: 2026-04-10T08:15:37.436Z
Learning: When using Spring `ContentDisposition.builder(...).filename(name, StandardCharsets.UTF_8).build()` (i.e., explicitly providing UTF-8), the resulting header value should include both the quoted `filename="=?UTF-8?..."` and the RFC 5987 `filename*=` parameters. In this case, any extra ASCII fallback computation (e.g., deriving an ASCII `fallbackFilename` via `NON_ASCII_PATTERN` and calling `.filename(fallbackFilename)`) is likely redundant—prefer calling only `.filename(fallbackName?, StandardCharsets.UTF_8)` as appropriate and let Spring handle the UTF-8 header parameters. Verify by comparing the emitted header for `filename` and `filename*` before deciding to keep an ASCII fallback.

Applied to files:

• backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java
• backend/src/main/java/org/booklore/service/file/FileMoveService.java
• backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java

📚 Learning: 2026-04-14T12:43:08.698Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 502
File: booklore-api/src/main/java/org/booklore/service/reader/ChapterCacheService.java:0-0
Timestamp: 2026-04-14T12:43:08.698Z
Learning: For this codebase (booklore-api), target Java 25 with `--enable-preview`, so `_` is intentionally used as an unnamed/ignored variable (e.g., lambda parameter or pattern variable) per Java’s preview feature JEP 456. Do not flag `_` in those contexts as an invalid/reserved identifier; only flag it if it’s used in a non-supported position (e.g., where an unnamed variable is not applicable for the Java preview rules).

Applied to files:

• backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java
• backend/src/main/java/org/booklore/service/file/FileMoveService.java
• backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java

🔇 Additional comments (3)

backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java (1)

917-938: LGTM — ordered assertion cleanly pins the regression.

InOrder(sidecarMetadataWriter, fileMoveHelper) with verifications on moveSidecarFiles then deleteEmptyParentDirsUpToLibraryFolders precisely captures the invariant from issue #821 for the bulkMoveFiles path. Symmetric with the coverage added in FileMoveServiceOrderingTest for moveSingleFile.

backend/src/main/java/org/booklore/service/file/FileMoveService.java (1)

239-247: Reorder is correct — cleanup now runs with sidecars already relocated.

deleteEmptyParentDirsUpToLibraryFolders treats only .DS_Store/Thumbs.db as ignorable, so any residual *.metadata.json or cover file in the old parent would previously block deletion. Moving sidecarMetadataWriter.moveSidecarFiles(...) ahead of the cleanup loop in both processSingleMove and moveSingleFile (L420–428) fixes the leftover-folder bug from #821 without altering rollback semantics — the sidecar call remains inside its own try/catch and is only reached after commitMove/DB update succeed.

One minor observation (non-blocking): if sidecarMetadataWriter.moveSidecarFiles(...) fails and logs a warning, the subsequent cleanup will refuse to delete the dir (since the sidecar is still there), leaving the pre-fix state for that edge case. This matches prior behavior, so no action needed, but it may be worth noting in the PR description / issue as a known remaining edge case.

backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java (1)

191-199: LGTM — ordering assertions faithfully mirror the production reorder.

Both the monitored-flow chain (unregister → drain → stage → commit → moveSidecarFiles → cleanup → sleep → register) and the db-cleanup chain (updateFileNameAndSubPath → moveSidecarFiles → deleteEmptyParentDirsUpToLibraryFolders) exactly match the new sequence in FileMoveService.moveSingleFile. Good regression fence.

Also applies to: 232-235

---

📝 Walkthrough

Walkthrough

The PR reorders operations in FileMoveService so that empty parent directory cleanup executes after sidecar metadata files are moved, rather than before. Test assertions are updated to verify this new operation sequence across multiple move scenarios.

Changes

Cohort / File(s)	Summary
Core Service Logic backend/src/main/java/org/booklore/service/file/FileMoveService.java	Relocates the deleteEmptyParentDirsUpToLibraryFolders invocation to occur after sidecarMetadataWriter.moveSidecarFiles in both processSingleMove and moveSingleFile methods.
Ordering Test Assertions backend/src/test/java/org/booklore/service/file/FileMoveServiceOrderingTest.java	Extends InOrder verifier to include sidecarMetadataWriter and asserts moveSidecarFiles is called between file move commit and directory cleanup in both monitored and database-cleanup test flows.
Bulk Move Test backend/src/test/java/org/booklore/service/file/FileMoveServiceTest.java	Adds new Mockito-ordered assertion test for bulkMoveFiles validating that sidecar metadata moves precede empty parent directory deletion.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• Organize files leaves empty source folders when sidecar is enabled #820: Directly addresses the same ordering issue preventing empty source folders from being left behind when sidecar metadata is enabled.

Possibly related PRs

• fix(file): deduplicate book files and protect source library root in FileMoveService #294: Also modifies FileMoveService's move methods and affects the empty-parent-dir cleanup behavior.

Suggested labels

backend, enhancement

Poem

🐰 Reorder, reorder, the files must align,
Sidecars first, then cleanup—now that's divine!
Empty dirs begone, in the proper new way,
This rabbit hops faster with changes today! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title follows conventional commit format with a 'fix' type and descriptive subject clearly reflecting the main change.
Description check	✅ Passed	The PR description includes all required sections with clear explanation of the bug fix, linked issue, and specific changes made.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/organize-empty-folder-cleanup

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/organize-empty-folder-cleanup

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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