-
-
Notifications
You must be signed in to change notification settings - Fork 669
XMP Sync #1502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rohan-pandeyy
wants to merge
8
commits into
AOSSIE-Org:main
Choose a base branch
from
rohan-pandeyy:feat/xmp-sync
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
XMP Sync #1502
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
13c9dda
feat: stop PictoPy's own file writes from triggering a folder resync
rohan-pandeyy e3d08b1
test: assert the path guarantee that holds on every platform
rohan-pandeyy ec8c1ba
style: enable foreign keys in the self-writes connection
rohan-pandeyy 0b66413
feat: add the XMP packet and segment layer
rohan-pandeyy 8bc3b66
Merge branch 'fix/watcher-self-write-echo' into feat/xmp-sync
rohan-pandeyy eb85e27
fix: only replace the XMP properties a write actually sets
rohan-pandeyy f3c0b5d
Merge branch 'feat/xmp-packet' into feat/xmp-sync
rohan-pandeyy 1c89252
feat: write photo metadata into the files themselves
rohan-pandeyy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,246 @@ | ||
| """ | ||
| Which photos still need their metadata written into the file, and what to write. | ||
|
|
||
| The database stays the fast index; the file is what survives PictoPy. These | ||
| queries pull together the pieces the packet is built from, none of which live | ||
| in one table. | ||
| """ | ||
|
|
||
| import json | ||
| import sqlite3 | ||
| from typing import Any, Dict, List, Mapping, Optional, Tuple, TypedDict | ||
|
|
||
| from app.config.settings import DATABASE_PATH | ||
| from app.logging.setup_logging import get_logger | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| ImageId = str | ||
|
|
||
|
|
||
| class SyncCandidate(TypedDict): | ||
| """Everything the packet for one photo is built from.""" | ||
|
|
||
| id: ImageId | ||
| path: str | ||
| metadata: Mapping[str, Any] | ||
| is_favourite: bool | ||
| keywords: List[str] | ||
| faces: List[Dict[str, Any]] | ||
|
|
||
|
|
||
| def _connect() -> sqlite3.Connection: | ||
| conn = sqlite3.connect(DATABASE_PATH) | ||
| conn.execute("PRAGMA foreign_keys = ON") | ||
| return conn | ||
|
|
||
|
|
||
| def _parse_json(raw: Optional[str], fallback: Any) -> Any: | ||
| if not raw: | ||
| return fallback | ||
| try: | ||
| return json.loads(raw) | ||
| except (json.JSONDecodeError, TypeError): | ||
| return fallback | ||
|
|
||
|
|
||
| def db_get_images_pending_metadata_sync(limit: int = 200) -> List[SyncCandidate]: | ||
| """ | ||
| Photos whose file does not yet carry what the database knows. | ||
|
|
||
| Only tagged photos are returned: writing before the AI pass has run would | ||
| put an empty keyword list into the file and need a second write anyway. | ||
| """ | ||
| conn = _connect() | ||
| cursor = conn.cursor() | ||
|
|
||
| try: | ||
| cursor.execute( | ||
| """ | ||
| SELECT id, path, metadata, isFavourite | ||
| FROM images | ||
| WHERE isMetadataSynced = 0 AND isTagged = 1 | ||
| ORDER BY id | ||
| LIMIT ? | ||
| """, | ||
| (limit,), | ||
| ) | ||
| rows = cursor.fetchall() | ||
| if not rows: | ||
| return [] | ||
|
|
||
| candidates: Dict[ImageId, SyncCandidate] = {} | ||
| for image_id, path, metadata, is_favourite in rows: | ||
| candidates[image_id] = SyncCandidate( | ||
| id=image_id, | ||
| path=path, | ||
| metadata=_parse_json(metadata, {}), | ||
| is_favourite=bool(is_favourite), | ||
| keywords=[], | ||
| faces=[], | ||
| ) | ||
|
|
||
| placeholders = ",".join("?" for _ in candidates) | ||
| identifiers = list(candidates) | ||
|
|
||
| # Tags come through the display view so the file gets the same labels | ||
| # the gallery shows, rather than every low-scoring semantic match. | ||
| cursor.execute( | ||
| f""" | ||
| SELECT d.image_id, m.name | ||
| FROM image_classes_display d | ||
| JOIN mappings m ON m.class_id = d.class_id | ||
| WHERE d.image_id IN ({placeholders}) | ||
| ORDER BY m.name | ||
| """, | ||
| identifiers, | ||
| ) | ||
| for image_id, name in cursor.fetchall(): | ||
| if name: | ||
| candidates[image_id]["keywords"].append(name) | ||
|
|
||
| # Only named clusters: an unnamed one carries no information a person | ||
| # reading the file in another application could use. | ||
| cursor.execute( | ||
| f""" | ||
| SELECT f.image_id, f.bbox, c.cluster_name | ||
| FROM faces f | ||
| JOIN face_clusters c ON c.cluster_id = f.cluster_id | ||
| WHERE f.image_id IN ({placeholders}) | ||
| AND c.cluster_name IS NOT NULL | ||
| AND TRIM(c.cluster_name) != '' | ||
| ORDER BY c.cluster_name | ||
| """, | ||
| identifiers, | ||
| ) | ||
| for image_id, bbox, cluster_name in cursor.fetchall(): | ||
| box = _parse_json(bbox, None) | ||
| if isinstance(box, dict): | ||
| candidates[image_id]["faces"].append( | ||
| {"name": cluster_name, "bbox": box} | ||
| ) | ||
|
|
||
| return list(candidates.values()) | ||
| except sqlite3.Error as e: | ||
| logger.error(f"Error collecting images pending metadata sync: {e}") | ||
| return [] | ||
| finally: | ||
| conn.close() | ||
|
|
||
|
|
||
| def db_count_images_pending_metadata_sync() -> int: | ||
| """How many photos are waiting for their file to be brought up to date.""" | ||
| conn = _connect() | ||
| cursor = conn.cursor() | ||
|
|
||
| try: | ||
| cursor.execute( | ||
| "SELECT COUNT(*) FROM images WHERE isMetadataSynced = 0 AND isTagged = 1" | ||
| ) | ||
| return cursor.fetchone()[0] | ||
| except sqlite3.Error as e: | ||
| logger.error(f"Error counting images pending metadata sync: {e}") | ||
| return 0 | ||
| finally: | ||
| conn.close() | ||
|
|
||
|
|
||
| def db_mark_metadata_synced(written: List[Tuple[ImageId, int, int]]) -> int: | ||
| """ | ||
| Record that a photo's file now carries its metadata, and how it now looks. | ||
|
|
||
| The size and mtime go back into the metadata blob on purpose. Our own write | ||
| changes both, and without this the next folder scan would see the file as | ||
| user-modified, re-read it, and mark it for another write -- a loop the file | ||
| itself keeps feeding. | ||
| """ | ||
| if not written: | ||
| return 0 | ||
|
|
||
| conn = _connect() | ||
| cursor = conn.cursor() | ||
|
|
||
| try: | ||
| updated = 0 | ||
| for image_id, file_size, file_mtime in written: | ||
| cursor.execute("SELECT metadata FROM images WHERE id = ?", (image_id,)) | ||
| row = cursor.fetchone() | ||
| if row is None: | ||
| continue | ||
|
|
||
| metadata = _parse_json(row[0], {}) | ||
| if not isinstance(metadata, dict): | ||
| metadata = {} | ||
| metadata["file_size"] = file_size | ||
| metadata["file_mtime"] = file_mtime | ||
|
|
||
| cursor.execute( | ||
| """ | ||
| UPDATE images | ||
| SET isMetadataSynced = 1, metadata = ? | ||
| WHERE id = ? | ||
| """, | ||
| (json.dumps(metadata), image_id), | ||
| ) | ||
| updated += cursor.rowcount | ||
|
|
||
| conn.commit() | ||
| return updated | ||
| except sqlite3.Error as e: | ||
| logger.error(f"Error marking metadata synced: {e}") | ||
| conn.rollback() | ||
| return 0 | ||
| finally: | ||
| conn.close() | ||
|
|
||
|
|
||
| def db_mark_metadata_dirty(image_ids: List[ImageId]) -> int: | ||
| """Flag photos whose file no longer matches what the database holds.""" | ||
| if not image_ids: | ||
| return 0 | ||
|
|
||
| conn = _connect() | ||
| cursor = conn.cursor() | ||
|
|
||
| try: | ||
| placeholders = ",".join("?" for _ in image_ids) | ||
| cursor.execute( | ||
| f"UPDATE images SET isMetadataSynced = 0 WHERE id IN ({placeholders})", | ||
| image_ids, | ||
| ) | ||
| conn.commit() | ||
| return cursor.rowcount | ||
| except sqlite3.Error as e: | ||
| logger.error(f"Error marking metadata dirty: {e}") | ||
| conn.rollback() | ||
| return 0 | ||
| finally: | ||
| conn.close() | ||
|
|
||
|
|
||
| def db_mark_metadata_dirty_for_cluster(cluster_id: str) -> int: | ||
| """ | ||
| Flag every photo showing a given person. | ||
|
|
||
| Renaming a cluster changes one row but invalidates the region name written | ||
| into every file that person appears in. | ||
| """ | ||
| conn = _connect() | ||
| cursor = conn.cursor() | ||
|
|
||
| try: | ||
| cursor.execute( | ||
| """ | ||
| UPDATE images SET isMetadataSynced = 0 | ||
| WHERE id IN (SELECT image_id FROM faces WHERE cluster_id = ?) | ||
| """, | ||
| (cluster_id,), | ||
| ) | ||
| conn.commit() | ||
| return cursor.rowcount | ||
| except sqlite3.Error as e: | ||
| logger.error(f"Error marking cluster {cluster_id} dirty: {e}") | ||
| conn.rollback() | ||
| return 0 | ||
| finally: | ||
| conn.close() | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
A concurrent metadata change during a pass is lost.
db_get_images_pending_metadata_syncreads the candidates, the writer builds and writes the packet, and thendb_mark_metadata_syncedsetsisMetadataSynced = 1unconditionally. Any change that clears the flag between the read and the mark is overwritten. Example: the user toggles a favourite while the pass runs.db_toggle_image_favourite_statussetsisMetadataSynced = 0, then this function sets it back to 1. The file keeps the old rating, and no later pass picks the photo up until another edit happens.Guard the update with the state observed at read time. A monotonic revision column is one option; a simpler one is to record the flag-clearing timestamp and only mark synced when it has not moved.
🧰 Tools
🪛 ast-grep (0.45.1)
[info] 182-182: use jsonify instead of json.dumps for JSON output
Context: json.dumps(metadata)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 Prompt for AI Agents