Merge pull request #4039 from Spinnich/perf/smart-collection-gallery-query

perf(collections): serve smart and standard collections from a composed query

Author: gantoine

backend/endpoints/collections.py

@@ -14,6 +14,7 @@
     SmartCollectionSchema,
     VirtualCollectionSchema,
 )
+from endpoints.roms import refresh_affected_smart_collections
 from exceptions.endpoint_exceptions import (
     CollectionAlreadyExistsException,
     CollectionNotFoundInDatabaseException,
@@ -201,8 +202,10 @@ async def add_smart_collection(
         SmartCollection(**cleaned_data)
     )
 
-    # Fetch the ROMs to update the database model
-    smart_collection = created_smart_collection.update_properties(request.user.id)
+    smart_collection = (
+        db_collection_handler.refresh_smart_collection(created_smart_collection.id)
+        or created_smart_collection
+    )
 
     return SmartCollectionSchema.model_validate(smart_collection)
 
@@ -557,6 +560,7 @@ async def add_roms_to_collection(
     updated_collection = db_collection_handler.add_roms_to_collection(
         id, payload.rom_ids
     )
+    refresh_affected_smart_collections(payload.rom_ids, membership_only=True)
     return CollectionSchema.model_validate(updated_collection)
 
 
@@ -586,6 +590,7 @@ async def remove_roms_from_collection(
     updated_collection = db_collection_handler.remove_roms_from_collection(
         id, payload.rom_ids
     )
+    refresh_affected_smart_collections(payload.rom_ids, membership_only=True)
     return CollectionSchema.model_validate(updated_collection)
 
 
@@ -642,8 +647,9 @@ async def update_smart_collection(
         id, cleaned_data
     )
 
-    # Fetch the ROMs to update the database model
-    smart_collection = updated_smart_collection.update_properties(request.user.id)
+    smart_collection = (
+        db_collection_handler.refresh_smart_collection(id) or updated_smart_collection
+    )
 
     return SmartCollectionSchema.model_validate(smart_collection)
 

backend/endpoints/roms/__init__.py

@@ -52,7 +52,7 @@
     assert_rom_visible,
     get_permissions,
 )
-from handler.database import db_rom_handler, db_save_handler
+from handler.database import db_collection_handler, db_rom_handler, db_save_handler
 from handler.database.base_handler import sync_session
 from handler.filesystem import fs_resource_handler, fs_rom_handler
 from handler.filesystem.assets_handler import validate_image_upload
@@ -112,13 +112,33 @@
 router.include_router(patch_router)
 
 
+# RomUser fields the statuses filter branches on.
+STATUS_MEMBERSHIP_FIELDS = frozenset({"status", "now_playing", "backlogged", "hidden"})
+
+
 def safe_int_or_none(value: Any) -> int | None:
     if value is None or value == "":
         return None
 
     return safe_int(value)
 
 
+def refresh_affected_smart_collections(
+    rom_ids: Sequence[int], membership_only: bool = False
+) -> None:
+    """Follow a change into the cached smart collection membership.
+
+    The write has already been committed, so a stale count is the worst this
+    can cost, and reporting it back as a failed write would be a lie.
+    """
+    try:
+        db_collection_handler.refresh_smart_collections_for_roms(
+            rom_ids, membership_only=membership_only
+        )
+    except Exception as e:
+        log.error(f"Couldn't refresh smart collections for {rom_ids}: {e}")
+
+
 def build_unscoped_sidecar_cache_key(
     user_id: int,
     order_by: str,
@@ -613,8 +633,8 @@ def get_roms(
     query = db_rom_handler.filter_roms(
         query=unfiltered_query,
         user_id=request.user.id,
-        hidden_platform_ids=perms.hidden_platform_ids,
-        hidden_rom_ids=perms.hidden_rom_ids,
+        hidden_platform_ids=perms.hidden_platform_ids,  # type: ignore
+        hidden_rom_ids=perms.hidden_rom_ids,  # type: ignore
         platform_ids=platform_ids,
         collection_id=collection_id,
         virtual_collection_id=virtual_collection_id,
@@ -1536,6 +1556,7 @@ async def update_rom(
             raise RomNotFoundInDatabaseException(id)
 
         db_rom_handler.invalidate_filter_values_cache()
+        refresh_affected_smart_collections([id])
         return DetailedRomSchema.from_orm_with_request(rom, request)
 
     provided_fields = form_data.model_fields_set
@@ -1915,6 +1936,7 @@ async def update_rom(
         fire_and_forget(meta_playmatch_handler.submit_manual_match_suggestion(rom))
 
     db_rom_handler.invalidate_filter_values_cache()
+    refresh_affected_smart_collections([id])
     return DetailedRomSchema.from_orm_with_request(rom, request)
 
 
@@ -1981,7 +2003,7 @@ async def delete_roms(
     perms = get_permissions(request)
     assert_can(perms, PermEntity.ROMS, PermAction.DELETE)
 
-    successful_items = 0
+    deleted_ids: list[int] = []
     failed_ids = []
     errors = []
 
@@ -2036,16 +2058,19 @@ async def delete_roms(
                     f"Couldn't find resources to delete for {hl(str(rom.name or 'ROM'), color=BLUE)}"
                 )
 
-            successful_items += 1
+            deleted_ids.append(id)
         except Exception as e:
             failed_ids.append(id)
             errors.append(f"Failed to delete ROM {id}: {str(e)}")
 
-    if successful_items:
+    if deleted_ids:
         db_rom_handler.invalidate_filter_values_cache()
+        # Deleted ROMs would otherwise linger in the cached smart collection
+        # membership until the next scan.
+        refresh_affected_smart_collections(deleted_ids)
 
     return {
-        "successful_items": successful_items,
+        "successful_items": len(deleted_ids),
         "failed_ids": failed_ids,
         "errors": errors,
     }
@@ -2098,4 +2123,9 @@ async def update_rom_user(
     if "hidden" in cleaned_data:
         db_rom_handler.invalidate_filter_values_cache()
 
+    # The statuses filter reads all four of these, and `hidden` also drops the
+    # ROM from every user-scoped query, so any of them can move membership.
+    if STATUS_MEMBERSHIP_FIELDS & cleaned_data.keys():
+        refresh_affected_smart_collections([id], membership_only=True)
+
     return RomUserSchema.model_validate(rom_user)

backend/endpoints/saves.py

@@ -11,6 +11,7 @@
 from decorators.auth import protected_route
 from endpoints.responses.assets import SaveSchema, SaveSummarySchema, SlotSummarySchema
 from endpoints.responses.device import DeviceSyncSchema
+from endpoints.roms import refresh_affected_smart_collections
 from exceptions.endpoint_exceptions import RomNotFoundInDatabaseException
 from handler.auth.constants import Scope
 from handler.auth.dependencies import assert_rom_visible
@@ -391,6 +392,8 @@ async def add_save(
         rom_user.id, {"last_played": datetime.now(timezone.utc)}
     )
 
+    refresh_affected_smart_collections([rom.id], membership_only=True)
+
     return _build_save_schema(db_save, _syncs_for_save(db_save.id, device), device)
 
 
@@ -684,6 +687,9 @@ def update_save_visibility(
             save.screenshot.id, {"is_public": is_public}
         )
 
+    # Sharing a save exposes it to every other user's `has_saves` filter.
+    refresh_affected_smart_collections([save.rom_id], membership_only=True)
+
     return _build_save_schema(updated)
 
 
@@ -712,13 +718,16 @@ async def delete_saves(
         log.error(error)
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=error)
 
+    affected_rom_ids: set[int] = set()
+
     for save_id in saves:
         save = db_save_handler.get_save(user_id=request.user.id, id=save_id)
         if not save:
             error = f"Save with ID {save_id} not found"
             log.error(error)
             raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=error)
 
+        affected_rom_ids.add(save.rom_id)
         db_save_handler.delete_save(save_id)
 
         log.info(
@@ -741,6 +750,8 @@ async def delete_saves(
                 error = f"Screenshot file {hl(save.screenshot.file_name)} not found for save {hl(save.file_name)}[{hl(save.rom.platform_slug)}]"
                 log.error(error)
 
+    refresh_affected_smart_collections(list(affected_rom_ids), membership_only=True)
+
     return saves
 
 

backend/endpoints/sockets/scan.py

@@ -26,7 +26,12 @@
 )
 from exceptions.socket_exceptions import ScanStoppedException
 from handler.auth.constants import Scope
-from handler.database import db_firmware_handler, db_platform_handler, db_rom_handler
+from handler.database import (
+    db_collection_handler,
+    db_firmware_handler,
+    db_platform_handler,
+    db_rom_handler,
+)
 from handler.filesystem import (
     fs_firmware_handler,
     fs_platform_handler,
@@ -969,6 +974,14 @@ async def stop_scan():
         # The library changed; drop cached filter values.
         db_rom_handler.invalidate_filter_values_cache()
 
+        # Smart collection membership is derived from the library, and is no
+        # longer recomputed while serving a gallery page. The scan itself is
+        # done, so a failure here must not report it as one.
+        try:
+            db_collection_handler.refresh_smart_collections()
+        except Exception as e:
+            log.error(f"Couldn't refresh smart collections after the scan: {e}")
+
         # Export metadata files if enabled in config
         config = cm.get_config()
 

backend/endpoints/states.py

@@ -6,6 +6,7 @@
 
 from decorators.auth import protected_route
 from endpoints.responses.assets import StateSchema
+from endpoints.roms import refresh_affected_smart_collections
 from exceptions.endpoint_exceptions import RomNotFoundInDatabaseException
 from handler.auth.constants import Scope
 from handler.auth.dependencies import assert_rom_visible
@@ -169,6 +170,8 @@ async def add_state(
     if not rom:
         raise RomNotFoundInDatabaseException(rom_id)
 
+    refresh_affected_smart_collections([rom.id], membership_only=True)
+
     return StateSchema.model_validate(db_state)
 
 
@@ -357,6 +360,9 @@ def update_state_visibility(
             state.screenshot.id, {"is_public": is_public}
         )
 
+    # Sharing a state exposes it to every other user's `has_states` filter.
+    refresh_affected_smart_collections([state.rom_id], membership_only=True)
+
     return StateSchema.model_validate(updated)
 
 
@@ -385,13 +391,16 @@ async def delete_states(
         log.error(error)
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=error)
 
+    affected_rom_ids: set[int] = set()
+
     for state_id in states:
         state = db_state_handler.get_state(user_id=request.user.id, id=state_id)
         if not state:
             error = f"State with ID {state_id} not found"
             log.error(error)
             raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=error)
 
+        affected_rom_ids.add(state.rom_id)
         db_state_handler.delete_state(state_id)
         log.info(
             f"Deleting state {hl(state.file_name)} [{state.rom.platform_slug}] from filesystem"
@@ -414,4 +423,6 @@ async def delete_states(
                 error = f"Screenshot file {hl(state.screenshot.file_name)} not found for state {hl(state.file_name)}[{hl(state.rom.platform_slug)}]"
                 log.error(error)
 
+    refresh_affected_smart_collections(list(affected_rom_ids), membership_only=True)
+
     return states