fix: cache album ID in Firestore — prevent duplicate Family Faces albums

src/dmaf/__main__.py

@@ -18,7 +18,13 @@
 from dmaf.config import Settings
 from dmaf.database import get_database
 from dmaf.face_recognition import best_match, load_known_faces
-from dmaf.google_photos import create_media_item, ensure_album, get_creds, upload_bytes
+from dmaf.google_photos import (
+    create_media_item,
+    ensure_album,
+    get_creds,
+    get_or_create_album_id,
+    upload_bytes,
+)
 from dmaf.known_refresh import KnownRefreshManager
 from dmaf.watcher import NewImageHandler, run_watch, scan_and_process_once
 
@@ -209,7 +215,16 @@ def main(argv: list[str] | None = None) -> int:
     album_id = None
     if settings.google_photos_album_name:
         try:
-            album_id = ensure_album(creds, settings.google_photos_album_name)
+            fp = settings.dedup.firestore_project
+            if fp:
+                album_id = get_or_create_album_id(
+                    creds,
+                    settings.google_photos_album_name,
+                    firestore_project=fp,
+                )
+            else:
+                # No Firestore configured (local dev) — create without caching
+                album_id = ensure_album(creds, settings.google_photos_album_name)
         except Exception as e:
             logger.warning(f"Album ensure failed - continuing without album: {e}")
             album_id = None

src/dmaf/google_photos/__init__.py

@@ -9,12 +9,14 @@
     create_media_item,
     ensure_album,
     get_creds,
+    get_or_create_album_id,
     upload_bytes,
 )
 
 __all__ = [
     "get_creds",
     "ensure_album",
+    "get_or_create_album_id",
     "upload_bytes",
     "create_media_item",
     "SCOPES",

src/dmaf/google_photos/api.py

@@ -28,63 +28,99 @@ def get_creds(
         else:
             flow = InstalledAppFlow.from_client_secrets_file(client_secret_path, SCOPES)
             creds = flow.run_local_server(port=0)
-        # Try to save refreshed token (gracefully handle read-only filesystems)
         try:
             with open(token_path, "w") as f:
                 f.write(creds.to_json())
         except OSError as e:
-            # Read-only filesystem in cloud deployment - token refresh is in-memory only
             import logging
-
             logging.getLogger(__name__).debug(f"Could not write token file (read-only): {e}")
     return creds
 
 
 def ensure_album(creds: Credentials, album_name: str | None) -> str | None:
+    """Create a new Google Photos album and return its ID.
+
+    NOTE: photoslibrary.appendonly (DMAF's scope) cannot list existing albums —
+    that requires photoslibrary.readonly. This function only creates.
+    Use get_or_create_album_id to avoid creating duplicates across Cloud Run
+    invocations.
+    """
     if not album_name:
         return None
-    # Needs photoslibrary.readonly or photoslibrary scope to list-create albums.
-    # Safer approach: create once manually and paste album ID here.
-    # Example call below requires elevated scopes - comment out if using appendonly-only.
-    headers = {"Authorization": f"Bearer {creds.token}"}
-    # Try to find existing album
-    r = requests.get(
-        "https://photoslibrary.googleapis.com/v1/albums?pageSize=50", headers=headers, timeout=30
-    )
-    if r.status_code == 200:
-        albums_list = r.json().get("albums", [])
-        for album in albums_list:
-            if album.get("title") == album_name:
-                album_id = album.get("id")
-                return album_id if isinstance(album_id, str) else None
-    # Create album
-    body_dict = {"album": {"title": album_name}}
+    headers = {"Authorization": f"Bearer {creds.token}", "Content-Type": "application/json"}
     r = requests.post(
         "https://photoslibrary.googleapis.com/v1/albums",
         headers=headers,
-        json=body_dict,
+        json={"album": {"title": album_name}},
         timeout=30,
     )
     r.raise_for_status()
     result = r.json().get("id")
     return result if isinstance(result, str) else None
 
 
-@with_retry(RetryConfig(max_retries=3, base_delay=2.0))
-def upload_bytes(creds: Credentials, img_bytes: bytes, filename: str) -> str:
-    """
-    Upload raw image bytes to Google Photos.
+def _firestore_client(project: str):
+    """Create a Firestore client (extracted for testability)."""
+    from google.cloud import firestore as _fs  # noqa: PLC0415
+    return _fs.Client(project=project), _fs.SERVER_TIMESTAMP
+
 
-    Automatically retries on network errors and 429/5xx HTTP errors.
+def get_or_create_album_id(
+    creds: Credentials,
+    album_name: str,
+    firestore_project: str,
+    firestore_collection: str = "dmaf_config",
+) -> str | None:
+    """Return a cached Google Photos album ID, creating the album only once.
+
+    The album ID is stored in Firestore under
+    {firestore_collection}/google_photos_album. On subsequent Cloud Run
+    invocations the cached ID is returned immediately — no duplicate albums.
+
+    Root cause this fixes: appendonly scope cannot list albums, so the old
+    ensure_album silently failed the GET, then created a new album on every
+    job run (once per hour = many duplicate "Family Faces" albums).
 
     Args:
-        creds: Google OAuth credentials
-        img_bytes: Image data as bytes
-        filename: Original filename (for metadata)
+        creds: Google OAuth credentials (appendonly scope is sufficient).
+        album_name: Desired album title.
+        firestore_project: GCP project ID for Firestore.
+        firestore_collection: Firestore collection for DMAF config docs.
 
     Returns:
-        Upload token to use with create_media_item
+        Album ID string, or None on failure.
     """
+    import logging
+
+    logger = logging.getLogger(__name__)
+    db, SERVER_TIMESTAMP = _firestore_client(firestore_project)
+    ref = db.collection(firestore_collection).document("google_photos_album")
+
+    doc = ref.get()
+    if doc.exists:
+        data = doc.to_dict() or {}
+        cached_id = data.get("album_id")
+        cached_name = data.get("album_name")
+        if cached_id and cached_name == album_name:
+            cached_id_str = str(cached_id)
+            logger.debug(f"Using cached album ID for '{album_name}': {cached_id_str[:12]}...")
+            return cached_id_str
+        # Album name changed — fall through to create a new one
+
+    album_id = ensure_album(creds, album_name)
+    if album_id:
+        ref.set({
+            "album_name": album_name,
+            "album_id": album_id,
+            "created_at": SERVER_TIMESTAMP,
+        })
+        logger.info(f"Created and cached Google Photos album '{album_name}' -> {album_id[:12]}...")
+    return album_id
+
+
+@with_retry(RetryConfig(max_retries=3, base_delay=2.0))
+def upload_bytes(creds: Credentials, img_bytes: bytes, filename: str) -> str:
+    """Upload raw image bytes to Google Photos."""
     headers = {
         "Authorization": f"Bearer {creds.token}",
         "Content-type": "application/octet-stream",
@@ -105,23 +141,7 @@ def upload_bytes(creds: Credentials, img_bytes: bytes, filename: str) -> str:
 def create_media_item(
     creds: Credentials, upload_token: str, album_id: str | None, description: str | None = None
 ):
-    """
-    Create a media item in Google Photos from an upload token.
-
-    Automatically retries on network errors and 429/5xx HTTP errors.
-
-    Args:
-        creds: Google OAuth credentials
-        upload_token: Token from upload_bytes()
-        album_id: Optional album ID to add the item to
-        description: Optional description for the media item
-
-    Returns:
-        Media item ID
-
-    Raises:
-        RuntimeError: If Google Photos API returns an error status
-    """
+    """Create a media item in Google Photos from an upload token."""
     headers = {"Authorization": f"Bearer {creds.token}", "Content-Type": "application/json"}
     new_item: dict[str, str | dict[str, str]] = {"simpleMediaItem": {"uploadToken": upload_token}}
     if description:

tests/test_google_photos_api.py

@@ -123,85 +123,123 @@ def test_invalid_creds_no_refresh_token(
 
 
 class TestEnsureAlbum:
-    """Test album lookup and creation."""
+    """Test album creation (ensure_album only creates, never lists)."""
 
     def test_none_album_name(self):
         """Test that None album_name returns None."""
         mock_creds = Mock()
         result = ensure_album(mock_creds, None)
         assert result is None
 
-    @patch("dmaf.google_photos.api.requests.get")
-    def test_find_existing_album(self, mock_get):
-        """Test finding an existing album by name."""
+    @patch("dmaf.google_photos.api.requests.post")
+    def test_create_album(self, mock_post):
+        """Test creating a new album via POST."""
         mock_creds = Mock()
         mock_creds.token = "test_token"
 
-        # Mock API response with existing album
-        mock_response = Mock()
-        mock_response.status_code = 200
-        mock_response.json.return_value = {
-            "albums": [
-                {"id": "album_1", "title": "Other Album"},
-                {"id": "album_2", "title": "My Test Album"},
-                {"id": "album_3", "title": "Another Album"},
-            ]
-        }
-        mock_get.return_value = mock_response
+        mock_post_response = Mock()
+        mock_post_response.json.return_value = {"id": "new_album_id"}
+        mock_post.return_value = mock_post_response
 
         album_id = ensure_album(mock_creds, "My Test Album")
 
-        assert album_id == "album_2"
-        mock_get.assert_called_once()
+        assert album_id == "new_album_id"
+        mock_post.assert_called_once()
+        call_kwargs = mock_post.call_args[1]
+        assert call_kwargs["json"] == {"album": {"title": "My Test Album"}}
 
-    @patch("dmaf.google_photos.api.requests.get")
     @patch("dmaf.google_photos.api.requests.post")
-    def test_create_new_album(self, mock_post, mock_get):
-        """Test creating a new album when it doesn't exist."""
+    def test_create_album_missing_id(self, mock_post):
+        """Test handling response without an id field."""
         mock_creds = Mock()
         mock_creds.token = "test_token"
 
-        # Mock GET response (album not found)
-        mock_get_response = Mock()
-        mock_get_response.status_code = 200
-        mock_get_response.json.return_value = {"albums": []}
-        mock_get.return_value = mock_get_response
-
-        # Mock POST response (album created)
         mock_post_response = Mock()
-        mock_post_response.json.return_value = {"id": "new_album_id"}
+        mock_post_response.json.return_value = {}
         mock_post.return_value = mock_post_response
 
-        album_id = ensure_album(mock_creds, "New Album")
+        result = ensure_album(mock_creds, "Album")
+        assert result is None
 
-        assert album_id == "new_album_id"
 
-        # Verify POST was called with correct data
-        mock_post.assert_called_once()
-        call_kwargs = mock_post.call_args[1]
-        assert call_kwargs["json"] == {"album": {"title": "New Album"}}
+class TestGetOrCreateAlbumId:
+    """Test Firestore-cached album ID lookup."""
+
+    def _make_fs_mock(self, exists: bool = True, data: dict | None = None):
+        """Build a mock Firestore db + ref that _firestore_client() returns."""
+        mock_doc = Mock()
+        mock_doc.exists = exists
+        mock_doc.to_dict.return_value = data or {}
+        mock_ref = Mock()
+        mock_ref.get.return_value = mock_doc
+        mock_collection = Mock()
+        mock_collection.document.return_value = mock_ref
+        mock_db = Mock()
+        mock_db.collection.return_value = mock_collection
+        return mock_db, mock_ref
+
+    @patch("dmaf.google_photos.api.requests.post")
+    @patch("dmaf.google_photos.api._firestore_client")
+    def test_returns_cached_id(self, mock_fs, mock_post):
+        """Cached album ID is returned without calling Google Photos API."""
+        from dmaf.google_photos.api import get_or_create_album_id
+
+        mock_creds = Mock()
+        mock_creds.token = "tok"
+        mock_db, _ = self._make_fs_mock(
+            exists=True, data={"album_name": "Family Faces", "album_id": "cached_id_123"}
+        )
+        mock_fs.return_value = (mock_db, "ts")
+
+        result = get_or_create_album_id(mock_creds, "Family Faces", "proj")
+
+        assert result == "cached_id_123"
+        mock_post.assert_not_called()  # No Google Photos API call
 
-    @patch("dmaf.google_photos.api.requests.get")
     @patch("dmaf.google_photos.api.requests.post")
-    def test_api_error_no_albums_returned(self, mock_post, mock_get):
-        """Test handling API response without albums key."""
+    @patch("dmaf.google_photos.api._firestore_client")
+    def test_creates_and_caches_on_first_run(self, mock_fs, mock_post):
+        """On first run (no cache), album is created and ID is cached."""
+        from dmaf.google_photos.api import get_or_create_album_id
+
         mock_creds = Mock()
-        mock_creds.token = "test_token"
+        mock_creds.token = "tok"
+        mock_db, mock_ref = self._make_fs_mock(exists=False)
+        mock_fs.return_value = (mock_db, "ts")
 
-        # Mock API response without albums
-        mock_get_response = Mock()
-        mock_get_response.status_code = 200
-        mock_get_response.json.return_value = {}
-        mock_get.return_value = mock_get_response
+        mock_response = Mock()
+        mock_response.json.return_value = {"id": "brand_new_id"}
+        mock_post.return_value = mock_response
 
-        # Mock album creation
-        mock_post_response = Mock()
-        mock_post_response.json.return_value = {"id": "created_album_id"}
-        mock_post.return_value = mock_post_response
+        result = get_or_create_album_id(mock_creds, "Family Faces", "proj")
+
+        assert result == "brand_new_id"
+        mock_ref.set.assert_called_once()
+        saved = mock_ref.set.call_args[0][0]
+        assert saved["album_id"] == "brand_new_id"
+        assert saved["album_name"] == "Family Faces"
+
+    @patch("dmaf.google_photos.api.requests.post")
+    @patch("dmaf.google_photos.api._firestore_client")
+    def test_recreates_when_album_name_changes(self, mock_fs, mock_post):
+        """If album name changed in config, a new album is created."""
+        from dmaf.google_photos.api import get_or_create_album_id
+
+        mock_creds = Mock()
+        mock_creds.token = "tok"
+        mock_db, mock_ref = self._make_fs_mock(
+            exists=True, data={"album_name": "Old Name", "album_id": "old_id"}
+        )
+        mock_fs.return_value = (mock_db, "ts")
+
+
+        mock_response = Mock()
+        mock_response.json.return_value = {"id": "new_name_id"}
+        mock_post.return_value = mock_response
+
+        result = get_or_create_album_id(mock_creds, "New Name", "proj")
 
-        # Should proceed to create album when not found
-        album_id = ensure_album(mock_creds, "Album")
-        assert album_id == "created_album_id"
+        assert result == "new_name_id"
 
 
 class TestUploadBytes: