Fix: Ensure Database Consistency on External Image Deletion Events

[Hemil36]

Fixes : #517

This pull request improves the logic for handling file change events in the watcher_util_handle_file_changes function. The main enhancement is to ensure that deletions of image files within watched folders are correctly processed, and that deletions of watched folders themselves are handled distinctly. This results in more accurate syncing behavior when files or folders are deleted.

File change handling improvements:

• Added a check to distinguish between deletions of watched folders and other file changes, ensuring that deleted watched folders are processed separately and preventing duplicate handling.
• Updated the logic to allow deletions of image files within watched folders to trigger a sync of their parent folders, improving the reliability of folder sync on file deletions.

Summary by CodeRabbit

• Bug Fixes

  • Correctly handles deletion of watched folders, ensuring they are removed cleanly and preventing unnecessary sync operations.
  • Improves reliability after folder deletions by restarting the watcher when required.
  • Maintains normal sync behavior for additions and modifications.

• Documentation

  • Clarified internal comments to distinguish deletion handling from other change types.

[coderabbitai]

Walkthrough

Adds explicit handling for deletions of watched folders in watcher_util_handle_file_changes: flags such deletions, queues their IDs for a delete-folders API call, skips closest-parent calculation and normal sync, and may trigger a watcher restart. Non-deleted changes continue using closest watched parent resolution and sync API calls. Comments were clarified.

Changes

Cohort / File(s)	Summary
Watcher deletion handling sync-microservice/app/utils/watcher.py	Introduced is_deleted_watched_folder flag; routed Change.deleted events for watched folders to delete-folders API and optional watcher restart; preserved existing sync path for additions/modifications and non-deleted paths; added clarifying comments and minor control-flow reorganization.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant FS as File System
  participant W as watcher_util_handle_file_changes
  participant S as Sync API
  participant D as Delete-Folders API
  participant R as Watcher Controller

  FS->>W: Change event (add/modify/delete, path)
  alt Deleted AND path is a watched folder
    note over W: Set is_deleted_watched_folder = true<br/>Collect folder ID
    W->>D: deleteFolders(folderIds)
    alt Requires restart
      D-->>W: Deleted
      W->>R: restartWatcher()
    else No restart
      D-->>W: Deleted
    end
  else Not a watched-folder deletion
    note over W: Resolve closest watched parent folder
    W->>S: syncFolder(parentFolderId, path)
    S-->>W: Sync triggered
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I twitch my ears at folders gone,
A hush, a hop—then carry on.
Deleted burrows marked with care,
I call the brooms to sweep the lair.
For living paths, I drum and sync—
Thump-thump! The warren’s back in sync. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The current title refers to “database consistency” and “external image deletion events,” neither of which are addressed by this patch, which instead refines the file‐watcher logic to distinguish deletions of watched folders and to trigger parent‐folder sync on image file deletions; as a result, the title is misleading and does not summarize the main changes.	Please update the title to reflect the actual changes to watcher_util_handle_file_changes, for example: “Distinguish watched folder deletions and trigger parent‐folder sync on image file deletions in watcher.”

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

sync-microservice/app/utils/watcher.py (1)

78-83: Don’t restart the watcher from inside the watcher thread (self-join hazard).

watcher_util_restart_folder_watcher() calls watcher_util_stop_folder_watcher(), which .join()s watcher_thread. If invoked from the watcher thread, this can raise a RuntimeError and/or deadlock. Restart asynchronously from a separate thread, and (bonus) dedupe folder IDs before calling the delete API.

Apply this diff:

-    if deleted_folder_ids:
-        print(f"Calling delete API for {len(deleted_folder_ids)} deleted folders")
-        watcher_util_call_delete_folders_api(deleted_folder_ids)
-        watcher_util_restart_folder_watcher()
+    if deleted_folder_ids:
+        unique_ids = list(dict.fromkeys(deleted_folder_ids))
+        print(f"Calling delete API for {len(unique_ids)} deleted folders")
+        watcher_util_call_delete_folders_api(unique_ids)
+        # Restart asynchronously to avoid joining the current watcher thread.
+        threading.Thread(
+            target=watcher_util_restart_folder_watcher, daemon=True
+        ).start()

🧹 Nitpick comments (3)

sync-microservice/app/utils/watcher.py (3)

63-65: Comment/behavior mismatch: this syncs on any file deletion, not just images.

If the intent is to limit sync-on-deletion to image files, gate it explicitly; otherwise, update the comment.

Example gating:

def _is_image_file(path: str) -> bool:
    return os.path.splitext(path)[1].lower() in {
        ".jpg", ".jpeg", ".png", ".gif", ".webp", ".bmp", ".tiff", ".svg", ".heic"
    }

# ...
if not is_deleted_watched_folder and (change != Change.deleted or _is_image_file(file_path)):
    # proceed to find closest parent + sync

---

116-116: Remove noisy debug print in hot path.

print("best match: ", best_match) will spam logs on every event. Drop it or lower to a debug logger.

---

19-37: Use the existing folder_id_map for O(1) lookups.

watcher_util_get_folder_id_if_watched currently linearly scans watched_folders. Prefer folder_id_map.get(normalized_path) (ensure keys are normalized consistently when populating).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3779796 and 1dd1584.

📒 Files selected for processing (1)

• sync-microservice/app/utils/watcher.py (1 hunks)

🔇 Additional comments (1)

sync-microservice/app/utils/watcher.py (1)

185-185: Confirm recursive=False meets the deletion-sync requirement.

If images can live in subfolders of watched folders, you likely want recursive=True to see those deletions.

[rahulharpal1603]

LGTM!