Skip to content

Commit f4ac78c

Browse files
Spinnichclaude
andcommitted
perf(collections): serve smart collections from a composed query
A smart collection re-ran its filters across the whole library on every request, loaded every match into memory with its joined metadata, wrote the membership back, and only then asked for the page being displayed. The cost tracked library size rather than collection size, was paid twice on first load and again on every scroll, and the write bumped updated_at, which the covers embed as a ?ts= cache-buster. Translate the stored filter_criteria into filter_roms keyword arguments and apply them to a bare Rom.id select instead, so membership stays in SQL and the database returns just the rows being rendered. The criteria vocabulary is filter_roms' own, so this composes with the gallery's filters. smart_collection_id is dropped from the criteria: the v2 create dialog records the route it was opened from, so a collection built while viewing another one carries its id, which would nest and could cycle. Bind parameters for the MATCH clauses are now named after the term they carry. A statement can hold two of them (the collection's search term and the gallery's), and SQLAlchemy does not uniquify explicitly named text() parameters, so a fixed name let one term overwrite the other. The cached rom_ids / rom_count / path_covers_* columns still back the collections list and the ROM detail page, so refresh them on write -- smart collection create and update, after a scan, and after ROMs are deleted -- rather than as a side effect of browsing. They are computed for the owner: the row is shared, so a per-user criterion like favorite would otherwise describe whoever opened the collection last. Regular collections and the favorites filter get the smaller half of the same treatment: an indexed subquery over collections_roms instead of an IN list built in Python. An unknown collection or smart collection now returns nothing rather than falling through to the entire library. Fixes #4029 AI assistance: implemented with Claude Code (Opus 5), tests first, then reviewed and verified by me. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 80e58d5 commit f4ac78c

9 files changed

Lines changed: 802 additions & 203 deletions

File tree

backend/endpoints/collections.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,10 @@ async def add_smart_collection(
201201
SmartCollection(**cleaned_data)
202202
)
203203

204-
# Fetch the ROMs to update the database model
205-
smart_collection = created_smart_collection.update_properties(request.user.id)
204+
smart_collection = (
205+
db_collection_handler.refresh_smart_collection(created_smart_collection.id)
206+
or created_smart_collection
207+
)
206208

207209
return SmartCollectionSchema.model_validate(smart_collection)
208210

@@ -642,8 +644,9 @@ async def update_smart_collection(
642644
id, cleaned_data
643645
)
644646

645-
# Fetch the ROMs to update the database model
646-
smart_collection = updated_smart_collection.update_properties(request.user.id)
647+
smart_collection = (
648+
db_collection_handler.refresh_smart_collection(id) or updated_smart_collection
649+
)
647650

648651
return SmartCollectionSchema.model_validate(smart_collection)
649652

backend/endpoints/roms/__init__.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
assert_rom_visible,
5353
get_permissions,
5454
)
55-
from handler.database import db_rom_handler, db_save_handler
55+
from handler.database import db_collection_handler, db_rom_handler, db_save_handler
5656
from handler.database.base_handler import sync_session
5757
from handler.filesystem import fs_resource_handler, fs_rom_handler
5858
from handler.filesystem.assets_handler import validate_image_upload
@@ -2043,6 +2043,9 @@ async def delete_roms(
20432043

20442044
if successful_items:
20452045
db_rom_handler.invalidate_filter_values_cache()
2046+
# Deleted ROMs would otherwise linger in the cached smart collection
2047+
# membership until the next scan.
2048+
db_collection_handler.refresh_smart_collections()
20462049

20472050
return {
20482051
"successful_items": successful_items,

backend/endpoints/sockets/scan.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@
2626
)
2727
from exceptions.socket_exceptions import ScanStoppedException
2828
from handler.auth.constants import Scope
29-
from handler.database import db_firmware_handler, db_platform_handler, db_rom_handler
29+
from handler.database import (
30+
db_collection_handler,
31+
db_firmware_handler,
32+
db_platform_handler,
33+
db_rom_handler,
34+
)
3035
from handler.filesystem import (
3136
fs_firmware_handler,
3237
fs_platform_handler,
@@ -969,6 +974,10 @@ async def stop_scan():
969974
# The library changed; drop cached filter values.
970975
db_rom_handler.invalidate_filter_values_cache()
971976

977+
# Smart collection membership is derived from the library, and is no
978+
# longer recomputed while serving a gallery page.
979+
db_collection_handler.refresh_smart_collections()
980+
972981
# Export metadata files if enabled in config
973982
config = cm.get_config()
974983

backend/handler/database/collections_handler.py

Lines changed: 158 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
union_all,
1414
update,
1515
)
16+
from sqlalchemy.engine import Row
1617
from sqlalchemy.exc import IntegrityError
1718
from sqlalchemy.orm import (
1819
Query,
@@ -23,8 +24,10 @@
2324
selectinload,
2425
)
2526

27+
from config import FRONTEND_RESOURCES_PATH
2628
from decorators.database import begin_session
2729
from models.collection import (
30+
SMART_COLLECTION_MAX_COVERS,
2831
Collection,
2932
CollectionRom,
3033
SmartCollection,
@@ -42,6 +45,10 @@
4245
COVERS_BATCH_SIZE = 100
4346

4447

48+
def _strip_cache_buster(urls: Sequence[str]) -> list[str]:
49+
return [url.split("?", 1)[0] for url in urls]
50+
51+
4552
def with_roms(func):
4653
@functools.wraps(func)
4754
def wrapper(*args, **kwargs):
@@ -466,80 +473,162 @@ def delete_smart_collection(
466473
.execution_options(synchronize_session="evaluate")
467474
)
468475

469-
def get_smart_collection_roms(
470-
self, smart_collection: SmartCollection, user_id: int | None = None
471-
) -> Sequence["Rom"]:
472-
"""Get ROMs that match the smart collection's filter criteria."""
473-
from handler.database import db_rom_handler
476+
def get_smart_collection_criteria(
477+
self, smart_collection: SmartCollection
478+
) -> dict[str, Any]:
479+
"""Translate stored filter criteria into `filter_roms` keyword arguments.
474480
475-
# Extract filter criteria
481+
`smart_collection_id` is dropped: the create dialog records the route it
482+
was opened from, so a smart collection built while viewing another one
483+
carries that id, and following it would nest (and could cycle).
484+
"""
476485
criteria = smart_collection.filter_criteria
477486

478-
# Convert legacy single-value criteria to arrays for backward compatibility
479-
def convert_legacy_filter(new_key: str, old_key: str) -> list[str] | None:
480-
"""Convert legacy single-value filter to array format."""
481-
if new_value := criteria.get(new_key):
482-
return new_value if isinstance(new_value, list) else [new_value]
483-
if old_value := criteria.get(old_key):
484-
return old_value if isinstance(old_value, list) else [old_value]
485-
return None
487+
# Early versions stored single values under `selected_*` keys.
488+
def as_list(new_key: str, old_key: str) -> list[str] | None:
489+
value = criteria.get(new_key) or criteria.get(old_key)
490+
if not value:
491+
return None
492+
return value if isinstance(value, list) else [value]
486493

487-
# Apply conversions
488-
genres = convert_legacy_filter("genres", "selected_genre")
489-
franchises = convert_legacy_filter("franchises", "selected_franchise")
490-
collections = convert_legacy_filter("collections", "selected_collection")
491-
companies = convert_legacy_filter("companies", "selected_company")
492-
age_ratings = convert_legacy_filter("age_ratings", "selected_age_rating")
493-
regions = convert_legacy_filter("regions", "selected_region")
494-
languages = convert_legacy_filter("languages", "selected_language")
495-
tags = convert_legacy_filter("tags", "selected_tag")
496-
statuses = convert_legacy_filter("statuses", "selected_status")
497-
498-
# Use the existing filter_roms method with the stored criteria
499494
platform_ids = criteria.get("platform_ids")
500-
if platform_ids is None:
501-
if platform_id := criteria.get("platform_id"):
502-
platform_ids = [platform_id]
503-
504-
return db_rom_handler.get_roms_scalar(
505-
platform_ids=platform_ids,
506-
collection_id=criteria.get("collection_id"),
507-
virtual_collection_id=criteria.get("virtual_collection_id"),
508-
search_term=criteria.get("search_term"),
509-
matched=criteria.get("matched"),
510-
favorite=criteria.get("favorite"),
511-
duplicate=criteria.get("duplicate"),
512-
playable=criteria.get("playable"),
513-
has_ra=criteria.get("has_ra"),
514-
has_saves=criteria.get("has_saves"),
515-
has_states=criteria.get("has_states"),
516-
has_soundtrack=criteria.get("has_soundtrack"),
517-
missing=criteria.get("missing"),
518-
verified=criteria.get("verified"),
519-
genres=genres,
520-
franchises=franchises,
521-
collections=collections,
522-
companies=companies,
523-
age_ratings=age_ratings,
524-
statuses=statuses,
525-
regions=regions,
526-
languages=languages,
527-
player_counts=criteria.get("player_counts"),
528-
tags=tags,
529-
metadata_providers=criteria.get("metadata_providers"),
530-
# Logic operators for multi-value filters
531-
genres_logic=criteria.get("genres_logic", "any"),
532-
franchises_logic=criteria.get("franchises_logic", "any"),
533-
collections_logic=criteria.get("collections_logic", "any"),
534-
companies_logic=criteria.get("companies_logic", "any"),
535-
age_ratings_logic=criteria.get("age_ratings_logic", "any"),
536-
regions_logic=criteria.get("regions_logic", "any"),
537-
languages_logic=criteria.get("languages_logic", "any"),
538-
player_counts_logic=criteria.get("player_counts_logic", "any"),
539-
statuses_logic=criteria.get("statuses_logic", "any"),
540-
metadata_providers_logic=criteria.get("metadata_providers_logic", "any"),
541-
tags_logic=criteria.get("tags_logic", "any"),
542-
user_id=user_id,
495+
if platform_ids is None and (platform_id := criteria.get("platform_id")):
496+
platform_ids = [platform_id]
497+
498+
return {
499+
"platform_ids": platform_ids,
500+
"collection_id": criteria.get("collection_id"),
501+
"virtual_collection_id": criteria.get("virtual_collection_id"),
502+
"search_term": criteria.get("search_term"),
503+
"matched": criteria.get("matched"),
504+
"favorite": criteria.get("favorite"),
505+
"duplicate": criteria.get("duplicate"),
506+
"playable": criteria.get("playable"),
507+
"has_ra": criteria.get("has_ra"),
508+
"has_saves": criteria.get("has_saves"),
509+
"has_states": criteria.get("has_states"),
510+
"has_soundtrack": criteria.get("has_soundtrack"),
511+
"missing": criteria.get("missing"),
512+
"verified": criteria.get("verified"),
513+
"genres": as_list("genres", "selected_genre"),
514+
"franchises": as_list("franchises", "selected_franchise"),
515+
"collections": as_list("collections", "selected_collection"),
516+
"companies": as_list("companies", "selected_company"),
517+
"age_ratings": as_list("age_ratings", "selected_age_rating"),
518+
"regions": as_list("regions", "selected_region"),
519+
"languages": as_list("languages", "selected_language"),
520+
"tags": as_list("tags", "selected_tag"),
521+
"statuses": as_list("statuses", "selected_status"),
522+
"player_counts": criteria.get("player_counts"),
523+
"metadata_providers": criteria.get("metadata_providers"),
524+
"genres_logic": criteria.get("genres_logic", "any"),
525+
"franchises_logic": criteria.get("franchises_logic", "any"),
526+
"collections_logic": criteria.get("collections_logic", "any"),
527+
"companies_logic": criteria.get("companies_logic", "any"),
528+
"age_ratings_logic": criteria.get("age_ratings_logic", "any"),
529+
"regions_logic": criteria.get("regions_logic", "any"),
530+
"languages_logic": criteria.get("languages_logic", "any"),
531+
"player_counts_logic": criteria.get("player_counts_logic", "any"),
532+
"statuses_logic": criteria.get("statuses_logic", "any"),
533+
"metadata_providers_logic": criteria.get("metadata_providers_logic", "any"),
534+
"tags_logic": criteria.get("tags_logic", "any"),
535+
}
536+
537+
@begin_session
538+
def get_smart_collection_members(
539+
self,
540+
smart_collection: SmartCollection,
541+
user_id: int | None = None,
542+
session: Session = None, # type: ignore
543+
) -> Sequence[Row[tuple[int, str | None, str | None]]]:
544+
"""Every member's id and cover paths, in the collection's own order.
545+
546+
Only the columns the cached membership needs, so refreshing never
547+
hydrates ROM metadata.
548+
"""
549+
from handler.database import db_rom_handler
550+
551+
criteria = smart_collection.filter_criteria
552+
query, _ = db_rom_handler.get_roms_query(
543553
order_by=criteria.get("order_by", "name"),
544554
order_dir=criteria.get("order_dir", "asc"),
555+
search_term=criteria.get("search_term"),
556+
user_id=user_id,
557+
session=session,
545558
)
559+
query = db_rom_handler.build_smart_collection_query(
560+
query=query,
561+
smart_collection=smart_collection,
562+
user_id=user_id,
563+
session=session,
564+
).with_only_columns(Rom.id, Rom.path_cover_s, Rom.path_cover_l)
565+
566+
return session.execute(query).all()
567+
568+
@begin_session
569+
def refresh_smart_collection(
570+
self,
571+
id: int,
572+
session: Session = None, # type: ignore
573+
) -> SmartCollection | None:
574+
"""Recompute a smart collection's cached membership columns.
575+
576+
Those columns back the collections list and the ROM detail page, and are
577+
maintained on write: when the collection changes, and when the library
578+
does. They describe the owner's view, since the row is shared and
579+
criteria like `favorite` or `has_saves` answer differently per user.
580+
"""
581+
smart_collection = session.scalar(
582+
select(SmartCollection).filter_by(id=id).limit(1)
583+
)
584+
if not smart_collection:
585+
return None
586+
587+
members = self.get_smart_collection_members(
588+
smart_collection, user_id=smart_collection.user_id, session=session
589+
)
590+
rom_ids = [member.id for member in members]
591+
covers_small = [
592+
f"{FRONTEND_RESOURCES_PATH}/{member.path_cover_s}"
593+
for member in members
594+
if member.path_cover_s
595+
][:SMART_COLLECTION_MAX_COVERS]
596+
covers_large = [
597+
f"{FRONTEND_RESOURCES_PATH}/{member.path_cover_l}"
598+
for member in members
599+
if member.path_cover_l
600+
][:SMART_COLLECTION_MAX_COVERS]
601+
602+
# Compare without the cache-buster: it is read from `updated_at` before
603+
# the write bumps it, so a stored URL never carries the row's current
604+
# timestamp and comparing whole URLs would rewrite on every refresh.
605+
if (
606+
smart_collection.rom_ids == rom_ids
607+
and _strip_cache_buster(smart_collection.path_covers_small) == covers_small
608+
and _strip_cache_buster(smart_collection.path_covers_large) == covers_large
609+
):
610+
return smart_collection
611+
612+
timestamp = smart_collection.updated_at
613+
return self.update_smart_collection(
614+
id,
615+
{
616+
"rom_count": len(rom_ids),
617+
"rom_ids": rom_ids,
618+
"path_covers_small": [f"{c}?ts={timestamp}" for c in covers_small],
619+
"path_covers_large": [f"{c}?ts={timestamp}" for c in covers_large],
620+
},
621+
session=session,
622+
)
623+
624+
@begin_session
625+
def refresh_smart_collections(
626+
self,
627+
session: Session = None, # type: ignore
628+
) -> int:
629+
"""Refresh every smart collection, e.g. once the library has changed."""
630+
ids = session.scalars(select(SmartCollection.id)).all()
631+
for id in ids:
632+
self.refresh_smart_collection(id, session=session)
633+
634+
return len(ids)

0 commit comments

Comments
 (0)