Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions backend/handler/database/roms_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,13 @@
40: (RomFile.sha1_hash, RomFile.chd_sha1_hash),
}

# Every column here is indexed on `roms`, so the sort walks the index and stops at the page.
ROM_METADATA_ORDER_COLUMNS: dict[str, QueryableAttribute] = {
"first_release_date": Rom.generated_first_release_date,
"average_rating": Rom.generated_average_rating,
"player_count": Rom.generated_player_count,
}

# Filter dropdowns read the narrow `roms_facets` mirror instead of `roms`,
# whose rows carry the raw metadata blobs. Column order matches the unpacking
# in `_collect_filter_values`.
Expand Down Expand Up @@ -1409,6 +1416,8 @@ def get_roms_query(
if user_id and hasattr(RomUser, order_by) and not hasattr(Rom, order_by):
order_attr = getattr(RomUser, order_by)
query = query.filter(RomUser.user_id == user_id)
elif order_by in ROM_METADATA_ORDER_COLUMNS:
order_attr = ROM_METADATA_ORDER_COLUMNS[order_by]
elif hasattr(RomMetadata, order_by) and not hasattr(Rom, order_by):
order_attr = getattr(RomMetadata, order_by)
query = query.outerjoin(RomMetadata, RomMetadata.rom_id == Rom.id)
Expand Down
14 changes: 14 additions & 0 deletions backend/models/rom.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
BigInteger,
Boolean,
Enum,
FetchedValue,
Float,
ForeignKey,
Index,
Expand Down Expand Up @@ -401,6 +402,19 @@ class Rom(BaseModel):
CustomJSON(), default=dict
)

# Read-only slice of the stored generated columns from the `roms_metadata` view
generated_first_release_date: Mapped[int | None] = mapped_column(
BigInteger(), server_default=FetchedValue(), server_onupdate=FetchedValue()
)
generated_average_rating: Mapped[float | None] = mapped_column(
Float(), server_default=FetchedValue(), server_onupdate=FetchedValue()
)
generated_player_count: Mapped[str | None] = mapped_column(
String(length=100),
server_default=FetchedValue(),
server_onupdate=FetchedValue(),
)

path_cover_s: Mapped[str | None] = mapped_column(Text, default="")
path_cover_l: Mapped[str | None] = mapped_column(Text, default="")
url_cover: Mapped[str | None] = mapped_column(
Expand Down
148 changes: 148 additions & 0 deletions backend/tests/handler/database/test_roms_metadata_sort.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
"""Ordering the gallery by a `roms_metadata` field.

`roms_metadata` is a thin view over STORED generated columns on `roms`
(migration 0098). Resolving one of its columns as the sort key used to join the
view back in, which re-joins `roms` to itself and leaves the sort key on the
joined table: the database cannot read that from an index, so it filesorts the
whole library on every page. The generated columns are indexed on `roms`, so
these tests pin both the ordering results and the query reading them directly.
"""

import pytest

from handler.database import db_rom_handler
from models.platform import Platform
from models.rom import Rom
from models.user import User


def _make_rom(platform: Platform, fs_name: str, **metadata) -> Rom:
rom = db_rom_handler.add_rom(
Rom(
platform_id=platform.id,
name=fs_name,
slug=fs_name,
fs_name=f"{fs_name}.zip",
fs_name_no_tags=fs_name,
fs_name_no_ext=fs_name,
fs_extension="zip",
fs_path=f"{platform.slug}/roms",
)
)
if metadata:
rom = db_rom_handler.update_rom(rom.id, metadata)
return rom


def _ordered_names(**kwargs) -> list[str]:
return [rom.name for rom in db_rom_handler.get_roms_scalar(**kwargs)]


class TestMetadataSortQueryShape:
"""The sort key has to be a `roms` column for its index to be usable."""

@pytest.mark.parametrize(
("order_by", "expected_column"),
[
("first_release_date", "generated_first_release_date"),
("average_rating", "generated_average_rating"),
("player_count", "generated_player_count"),
],
)
def test_orders_by_the_indexed_roms_column(
self, order_by: str, expected_column: str
):
query, order_column = db_rom_handler.get_roms_query(order_by=order_by)
sql = str(query)

assert f"ORDER BY roms.{expected_column} ASC" in sql
assert order_column is getattr(Rom, expected_column)
# `Rom.metadatum` is a `lazy="joined"` eager load, so one join to the
# view is expected; the sort must not add a second one.
assert sql.count("JOIN roms_metadata") == 1

def test_descending_metadata_sort_keeps_the_roms_column(self):
query, _ = db_rom_handler.get_roms_query(
order_by="first_release_date", order_dir="desc"
)

assert "ORDER BY roms.generated_first_release_date DESC" in str(query)

def test_rom_column_sort_is_unchanged(self):
query, order_column = db_rom_handler.get_roms_query(order_by="fs_size_bytes")

assert "ORDER BY roms.fs_size_bytes ASC" in str(query)
assert order_column is Rom.fs_size_bytes

def test_metadata_sort_does_not_join_the_view_for_a_user(
self, admin_user: User, platform: Platform
):
query, _ = db_rom_handler.get_roms_query(
order_by="first_release_date", user_id=admin_user.id
)
sql = str(query)

# The rom_user join still has to be there, only the self-join goes.
assert sql.count("JOIN roms_metadata") == 1
assert "JOIN rom_user" in sql


class TestMetadataSortResults:
"""The values sorted on are the ones the view exposes."""

@pytest.fixture
def dated_roms(self, platform: Platform) -> None:
_make_rom(
platform, "middle", igdb_metadata={"first_release_date": "1000000000"}
)
_make_rom(platform, "oldest", igdb_metadata={"first_release_date": "100000000"})
_make_rom(
platform, "newest", igdb_metadata={"first_release_date": "1700000000"}
)

def test_first_release_date_ascending(self, dated_roms: None):
assert _ordered_names(order_by="first_release_date", order_dir="asc") == [
"oldest",
"middle",
"newest",
]

def test_average_rating_descending(self, platform: Platform):
_make_rom(platform, "mediocre", igdb_metadata={"total_rating": "50"})
_make_rom(platform, "great", igdb_metadata={"total_rating": "95"})
_make_rom(platform, "poor", igdb_metadata={"total_rating": "10"})

assert _ordered_names(order_by="average_rating", order_dir="desc") == [
"great",
"mediocre",
"poor",
]

def test_player_count_ascending(self, platform: Platform):
_make_rom(platform, "four", igdb_metadata={"player_count": "4"})
_make_rom(platform, "two", igdb_metadata={"player_count": "2"})

assert _ordered_names(order_by="player_count", order_dir="asc") == [
"two",
"four",
]

def test_roms_without_metadata_are_still_returned(self, platform: Platform):
"""An unmatched rom has no release date, and must not be filtered out."""
_make_rom(platform, "dated", igdb_metadata={"first_release_date": "100000000"})
_make_rom(platform, "undated")

names = _ordered_names(order_by="first_release_date", order_dir="asc")

# NULL ordering differs per engine, so only membership is asserted.
assert sorted(names) == ["dated", "undated"]

def test_sort_matches_the_values_the_view_exposes(self, platform: Platform):
rom = _make_rom(
platform, "quoted", igdb_metadata={"first_release_date": "1569369600"}
)

reloaded = db_rom_handler.get_rom(rom.id)
assert reloaded is not None
assert reloaded.metadatum.first_release_date == 1569369600000
assert reloaded.generated_first_release_date == 1569369600000
Loading