From d36bd99bf62fc58cf3ef9a88d5151dcf0540f8a3 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 11:58:09 +0800 Subject: [PATCH 1/7] feat(media): collapse singleton media containers --- docs/api/methods.md | 8 +- docs/scraper.md | 4 +- pkg/api/methods/media_alias.go | 128 +++++++++++++++++++++++ pkg/api/methods/media_batch.go | 9 ++ pkg/api/methods/media_browse.go | 78 ++++++++++++-- pkg/api/methods/media_image.go | 69 +++++++++--- pkg/api/methods/media_meta.go | 101 ++++++++++++++++-- pkg/api/methods/media_resolve.go | 6 ++ pkg/api/methods/media_resolve_test.go | 52 +++++++++ pkg/database/database.go | 3 + pkg/database/mediadb/sql_scraper.go | 48 +++++++++ pkg/database/mediadb/sql_scraper_test.go | 108 +++++++++++++++++++ pkg/testing/helpers/db_mocks.go | 13 +++ 13 files changed, 594 insertions(+), 33 deletions(-) create mode 100644 pkg/api/methods/media_alias.go diff --git a/docs/api/methods.md b/docs/api/methods.md index 753181c0..31c4d8cf 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -559,7 +559,7 @@ All parameters are optional. When called with no parameters, returns root entrie | Key | Type | Required | Description | | :----------- | :------- | :------- | :----------------------------------------------------------------------------------------------- | -| mediaId | number | No | Opaque media database row ID. Present on `media` entries for efficient follow-up `media.meta` and `media.image` requests. | +| mediaId | number | No | Opaque media database row ID. Present on `media` entries, and on `directory` entries that contain exactly one indexed media descendant, for efficient follow-up `media.meta` and `media.image` requests. | | name | string | Yes | Display name of the entry. | | path | string | Yes | Full path to the entry. | | type | string | Yes | Entry type: `root`, `directory`, or `media`. | @@ -567,9 +567,9 @@ All parameters are optional. When called with no parameters, returns root entrie | group | string | No | Launcher group name. Present on virtual scheme `root` entries. | | systemId | string | No | System ID for the media or single-system filtered route (e.g. `SNES`). Present on `media` entries and filtered `root` entries when exactly one system applies. | | systemIds | string[] | No | System IDs represented by a filtered `root` or `directory` entry. | -| zapScript | string | No | ZapScript command to launch this media. Present on `media` entries. | -| relativePath | string | No | Relative path from root directory. Present on `media` entries. | -| tags | object[] | No | Tags attached to the media. Each object has `tag` (string) and `type` (string). Present on `media` entries. | +| zapScript | string | No | ZapScript command to launch this media. Present on `media` entries and singleton media-container `directory` entries. | +| relativePath | string | No | Relative path from root directory. Present on `media` entries and singleton media-container `directory` entries. | +| tags | object[] | No | Tags attached to the media. Each object has `tag` (string) and `type` (string). Present on `media` entries and singleton media-container `directory` entries. | ##### Browse pagination object diff --git a/docs/scraper.md b/docs/scraper.md index 9631a1ca..29166d4a 100644 --- a/docs/scraper.md +++ b/docs/scraper.md @@ -205,9 +205,9 @@ Progress is queryable with `media.scrape.status` and broadcast as `media.scrapin Only one scraper can run at a time, and scraping is mutually exclusive with media indexing. -`media.meta` returns the metadata graph for media rows: media-level tags and properties, title-level tags and properties, and stored system identity. Single requests accept `mediaId` or `system`/`path` and keep the single-response shape; batch requests use `items` and return per-item results. Binary property bytes are not included; clients should use `media.image` for image data. +`media.meta` returns the metadata graph for media rows: media-level tags and properties, title-level tags and properties, and stored system identity. Single requests accept `mediaId` or `system`/`path` and keep the single-response shape; batch requests use `items` and return per-item results. Binary property bytes are not included; clients should use `media.image` for image data. A `system`/`path` request for a folder or zip-as-directory container resolves to its only non-missing indexed media descendant when exactly one exists. -`media.image` accepts one media ref plus image type preferences such as `image`, `boxart`, `boxart3d`, `screenshot`, `wheel`, `titleshot`, `map`, `marquee`, and `fanart`. These resolve to canonical image property tags; for example `boxart` becomes `property:image-boxart` and `image` becomes `property:image-image`. Media-level properties are preferred over title-level properties for the same type. For stale image properties in these canonical tags, such as missing file paths for `property:image-boxart` or `property:image-image`, `media.image` logs the stale property in memory only and does not delete DB rows; lookup falls through to the next available source. +`media.image` accepts one media ref plus image type preferences such as `image`, `boxart`, `boxart3d`, `screenshot`, `wheel`, `titleshot`, `map`, `marquee`, and `fanart`. These resolve to canonical image property tags; for example `boxart` becomes `property:image-boxart` and `image` becomes `property:image-image`. Media-level properties are preferred over title-level properties for the same type. Singleton container aliases are checked as media-level fallbacks, so artwork attached to a sole child or its container can be found from either path. For stale image properties in these canonical tags, such as missing file paths for `property:image-boxart` or `property:image-image`, `media.image` logs the stale property in memory only and does not delete DB rows; lookup falls through to the next available source. ## Useful Focused Tests diff --git a/pkg/api/methods/media_alias.go b/pkg/api/methods/media_alias.go new file mode 100644 index 00000000..04f5342e --- /dev/null +++ b/pkg/api/methods/media_alias.go @@ -0,0 +1,128 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package methods + +import ( + "fmt" + "strings" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" +) + +func resolveSingletonMediaPath( + env *requests.RequestEnv, + system database.System, + mediaPath string, +) (*database.Media, error) { + media, err := env.Database.MediaDB.FindSingleDescendantMedia(env.Context, system.DBID, mediaPath) + if err != nil { + return nil, fmt.Errorf("failed to resolve singleton media path: %w", err) + } + return media, nil +} + +func equivalentMediaIDs(env *requests.RequestEnv, row *database.MediaFullRow) ([]int64, error) { + if env == nil || row == nil || env.Database == nil || env.Database.MediaDB == nil { + return nil, nil + } + + ids := []int64{row.DBID} + seen := map[int64]bool{row.DBID: true} + add := func(media *database.Media) { + if media == nil || seen[media.DBID] { + return + } + seen[media.DBID] = true + ids = append(ids, media.DBID) + } + + child, err := env.Database.MediaDB.FindSingleDescendantMedia(env.Context, row.System.DBID, row.Path) + if err != nil { + return nil, fmt.Errorf("failed to find child alias media: %w", err) + } + add(child) + + parentPath := strings.TrimSuffix(row.ParentDir, "/") + if parentPath == "" || parentPath == row.Path { + return ids, nil + } + + onlyChild, err := env.Database.MediaDB.FindSingleDescendantMedia(env.Context, row.System.DBID, parentPath) + if err != nil { + return nil, fmt.Errorf("failed to verify parent alias media: %w", err) + } + if onlyChild == nil || onlyChild.DBID != row.DBID { + return ids, nil + } + + parent, err := env.Database.MediaDB.FindMediaBySystemAndPath(env.Context, row.System.DBID, parentPath) + if err != nil { + return nil, fmt.Errorf("failed to find parent alias media: %w", err) + } + add(parent) + + return ids, nil +} + +func mergeMediaTags(primary []database.TagInfo, aliases ...[]database.TagInfo) []database.TagInfo { + if len(aliases) == 0 { + return primary + } + merged := make([]database.TagInfo, 0, len(primary)) + seen := make(map[string]bool) + appendUnique := func(tags []database.TagInfo) { + for _, tag := range tags { + key := tag.Type + "\x00" + tag.Tag + if seen[key] { + continue + } + seen[key] = true + merged = append(merged, tag) + } + } + appendUnique(primary) + for _, tags := range aliases { + appendUnique(tags) + } + return merged +} + +func mergeMediaProperties(primary []database.MediaProperty, aliases ...[]database.MediaProperty) []database.MediaProperty { + if len(aliases) == 0 { + return primary + } + merged := make([]database.MediaProperty, 0, len(primary)) + seen := make(map[string]bool) + appendUnique := func(props []database.MediaProperty) { + for _, prop := range props { + if seen[prop.TypeTag] { + continue + } + seen[prop.TypeTag] = true + merged = append(merged, prop) + } + } + appendUnique(primary) + for _, props := range aliases { + appendUnique(props) + } + return merged +} diff --git a/pkg/api/methods/media_batch.go b/pkg/api/methods/media_batch.go index f8525435..71ad2f26 100644 --- a/pkg/api/methods/media_batch.go +++ b/pkg/api/methods/media_batch.go @@ -189,6 +189,15 @@ func resolveMediaRefs(env *requests.RequestEnv, refs []mediaRefParam) ([]resolve } continue } + if fallback == nil { + fallback, fallbackErr = resolveSingletonMediaPath(env, system, path) + if fallbackErr != nil { + for _, index := range pathIndexes[systemID+"\x00"+path] { + resolved[index].Err = fallbackErr + } + continue + } + } if fallback == nil { for _, index := range pathIndexes[systemID+"\x00"+path] { resolved[index].Err = models.ClientErrf("media not found: %s/%s", systemID, path) diff --git a/pkg/api/methods/media_browse.go b/pkg/api/methods/media_browse.go index 575df9c9..51737ee4 100644 --- a/pkg/api/methods/media_browse.go +++ b/pkg/api/methods/media_browse.go @@ -641,22 +641,28 @@ func buildBrowseResponse( entries := make([]models.BrowseEntry, 0, len(dirs)+len(files)) + var rootDirs []string + if env.LauncherCache != nil && env.Platform != nil { + rootDirs = env.Platform.RootDirs(env.Config) + } + // Add directory entries for _, dir := range dirs { - entries = append(entries, models.BrowseEntry{ + dirPath := path + "/" + dir.Name + entry := models.BrowseEntry{ Name: dir.Name, - Path: path + "/" + dir.Name, + Path: dirPath, Type: "directory", FileCount: &dir.FileCount, SystemIDs: dir.SystemIDs, - }) + } + if dir.FileCount == 1 { + annotateSingletonDirectoryEntry(env, &entry, dirPath, dir.SystemIDs, rootDirs) + } + entries = append(entries, entry) } // Build file entries - var rootDirs []string - if env.LauncherCache != nil && env.Platform != nil { - rootDirs = env.Platform.RootDirs(env.Config) - } for i := range files { entry := buildMediaEntry(&files[i], env, rootDirs) entries = append(entries, entry) @@ -722,6 +728,64 @@ func buildMediaEntry( return entry } +func annotateSingletonDirectoryEntry( + env *requests.RequestEnv, + entry *models.BrowseEntry, + dirPath string, + systemIDs []string, + rootDirs []string, +) { + if env == nil || entry == nil || env.Database == nil || len(systemIDs) != 1 { + return + } + + db := env.Database.MediaDB + system, err := db.FindSystemBySystemID(systemIDs[0]) + if err != nil { + log.Debug().Err(err).Str("system", systemIDs[0]).Msg("browse singleton directory system lookup failed") + return + } + media, err := db.FindSingleDescendantMedia(env.Context, system.DBID, dirPath) + if err != nil { + log.Debug().Err(err).Str("path", dirPath).Msg("browse singleton directory lookup failed") + return + } + if media == nil { + return + } + + row, err := db.GetMediaWithTitleAndSystem(env.Context, media.DBID) + if err != nil || row == nil { + log.Debug().Err(err).Int64("mediaDBID", media.DBID).Msg("browse singleton directory media lookup failed") + return + } + tags, err := db.GetMediaTagsByMediaDBID(env.Context, row.DBID) + if err != nil { + log.Debug().Err(err).Int64("mediaDBID", row.DBID).Msg("browse singleton directory tag lookup failed") + return + } + zapTags, err := db.GetZapScriptTagsBySystemAndPath(env.Context, row.System.SystemID, row.Path) + if err != nil { + log.Debug().Err(err).Int64("mediaDBID", row.DBID).Msg("browse singleton directory zapscript tag lookup failed") + return + } + + result := database.SearchResultWithCursor{ + MediaID: row.DBID, + SystemID: row.System.SystemID, + Name: row.Title.Name, + Path: row.Path, + Tags: tags, + ZapScriptTags: zapTags, + } + mediaEntry := buildMediaEntry(&result, env, rootDirs) + entry.MediaID = mediaEntry.MediaID + entry.SystemID = mediaEntry.SystemID + entry.RelPath = mediaEntry.RelPath + entry.ZapScript = mediaEntry.ZapScript + entry.Tags = mediaEntry.Tags +} + // isPathUnderRoots checks if the given path is at or under one of the allowed root directories. func isPathUnderRoots(path string, rootDirs []string) bool { for _, root := range rootDirs { diff --git a/pkg/api/methods/media_image.go b/pkg/api/methods/media_image.go index 287eab4a..d3b5aaef 100644 --- a/pkg/api/methods/media_image.go +++ b/pkg/api/methods/media_image.go @@ -27,6 +27,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "sort" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" @@ -139,9 +140,9 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic db := env.Database.MediaDB row := resolved[0].Row - mediaProps, err := db.GetMediaPropertyMetadata(env.Context, row.DBID) + mediaPropSources, err := mediaImagePropSources(&env, row) if err != nil { - return nil, fmt.Errorf("failed to get media property metadata: %w", err) + return nil, err } titleProps, err := db.GetMediaTitlePropertyMetadata(env.Context, row.Title.DBID) if err != nil { @@ -149,7 +150,7 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic } prefs := imagePrefs(nil, ref.ImageTypes) - return selectMediaImage(env.Context, afero.NewOsFs(), db, row, mediaProps, titleProps, prefs, maxBytes) + return selectMediaImageFromSources(env.Context, afero.NewOsFs(), db, row, mediaPropSources, titleProps, prefs, maxBytes) } func parseMediaImageRequest(raw json.RawMessage) (mediaRefParam, error) { @@ -175,17 +176,17 @@ func handleMediaImageSinglePath(env *requests.RequestEnv, ref mediaRefParam, max return nil, err } - mediaProps, err := db.GetMediaPropertyMetadata(env.Context, row.DBID) + mediaPropSources, err := mediaImagePropSources(env, row) if err != nil { - return nil, fmt.Errorf("failed to get media property metadata: %w", err) + return nil, err } titleProps, err := db.GetMediaTitlePropertyMetadata(env.Context, row.Title.DBID) if err != nil { return nil, fmt.Errorf("failed to get title property metadata: %w", err) } - return selectMediaImage( - env.Context, afero.NewOsFs(), db, row, mediaProps, titleProps, imagePrefs(nil, ref.ImageTypes), maxBytes, + return selectMediaImageFromSources( + env.Context, afero.NewOsFs(), db, row, mediaPropSources, titleProps, imagePrefs(nil, ref.ImageTypes), maxBytes, ) } @@ -236,6 +237,30 @@ func mediaImageBlobTooLargeError(prop *database.MediaProperty, maxBytes int64) e ) } +func mediaImagePropSources(env *requests.RequestEnv, row *database.MediaFullRow) ([][]database.MediaProperty, error) { + ids, err := equivalentMediaIDs(env, row) + if err != nil { + return nil, err + } + if len(ids) == 1 { + props, err := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) + if err != nil { + return nil, fmt.Errorf("failed to get media property metadata: %w", err) + } + return [][]database.MediaProperty{props}, nil + } + + propsByID, err := env.Database.MediaDB.GetMediaPropertyMetadataByMediaDBIDs(env.Context, ids) + if err != nil { + return nil, fmt.Errorf("failed to get media property metadata: %w", err) + } + sources := make([][]database.MediaProperty, 0, len(ids)) + for _, id := range ids { + sources = append(sources, propsByID[id]) + } + return sources, nil +} + func selectMediaImage( ctx context.Context, fs afero.Fs, @@ -246,12 +271,24 @@ func selectMediaImage( prefs []string, maxBytes int64, ) (models.MediaImageResponse, error) { - mediaMap := buildPropsMap(mediaProps) - titleMap := buildPropsMap(titleProps) - sources := []mediaImageSource{ - {mediaMap, true}, - {titleMap, false}, + return selectMediaImageFromSources(ctx, fs, db, row, [][]database.MediaProperty{mediaProps}, titleProps, prefs, maxBytes) +} + +func selectMediaImageFromSources( + ctx context.Context, + fs afero.Fs, + db database.MediaDBI, + row *database.MediaFullRow, + mediaPropSources [][]database.MediaProperty, + titleProps []database.MediaProperty, + prefs []string, + maxBytes int64, +) (models.MediaImageResponse, error) { + sources := make([]mediaImageSource, 0, len(mediaPropSources)+1) + for _, props := range mediaPropSources { + sources = append(sources, mediaImageSource{buildPropsMap(props), true}) } + sources = append(sources, mediaImageSource{buildPropsMap(titleProps), false}) seen := make(map[string]bool, len(prefs)) for _, t := range prefs { @@ -281,18 +318,22 @@ func selectMediaImage( } } + mediaImageProps := make([]string, 0) + for _, props := range mediaPropSources { + mediaImageProps = append(mediaImageProps, imagePropertyTypeTags(props)...) + } log.Debug(). Str("system", row.System.SystemID). Str("path", row.Path). Int64("mediaDBID", row.DBID). Int64("titleDBID", row.Title.DBID). Strs("prefs", prefs). - Strs("mediaImageProps", imagePropertyTypeTags(mediaProps)). + Strs("mediaImageProps", mediaImageProps). Strs("titleImageProps", imagePropertyTypeTags(titleProps)). Msg("media.image: no image found") return models.MediaImageResponse{}, models.ClientErrf( - "no image found for media: system=%q path=%q", row.System.SystemID, row.Path, + "no image found for media: system=%q path=%q", row.System.SystemID, filepath.ToSlash(row.Path), ) } diff --git a/pkg/api/methods/media_meta.go b/pkg/api/methods/media_meta.go index fb12dde2..9edc07d0 100644 --- a/pkg/api/methods/media_meta.go +++ b/pkg/api/methods/media_meta.go @@ -55,7 +55,7 @@ func HandleMediaMeta(env requests.RequestEnv) (any, error) { //nolint:gocritic / } db := env.Database.MediaDB - mediaTags, err := db.GetMediaTagsByMediaDBIDs(env.Context, mediaIDs) + mediaTagsByID, err := db.GetMediaTagsByMediaDBIDs(env.Context, mediaIDs) if err != nil { return nil, fmt.Errorf("failed to get media tags: %w", err) } @@ -63,7 +63,7 @@ func HandleMediaMeta(env requests.RequestEnv) (any, error) { //nolint:gocritic / if err != nil { return nil, fmt.Errorf("failed to get title tags: %w", err) } - mediaProps, err := db.GetMediaPropertyMetadataByMediaDBIDs(env.Context, mediaIDs) + mediaPropsByID, err := db.GetMediaPropertyMetadataByMediaDBIDs(env.Context, mediaIDs) if err != nil { return nil, fmt.Errorf("failed to get media property metadata: %w", err) } @@ -73,10 +73,22 @@ func HandleMediaMeta(env requests.RequestEnv) (any, error) { //nolint:gocritic / } if !params.Batch { + mediaTags := mediaTagsByID[resolved[0].Row.DBID] + mediaProps := mediaPropsByID[resolved[0].Row.DBID] + ids, metaErr := equivalentMediaIDs(&env, resolved[0].Row) + if metaErr != nil { + return nil, metaErr + } + if len(ids) > 1 { + mediaTags, mediaProps, metaErr = mergedMediaMeta(&env, resolved[0].Row) + if metaErr != nil { + return nil, metaErr + } + } return buildMediaMetaResponse( resolved[0].Row, - mediaTags[resolved[0].Row.DBID], titleTags[resolved[0].Row.Title.DBID], - mediaProps[resolved[0].Row.DBID], titleProps[resolved[0].Row.Title.DBID], + mediaTags, titleTags[resolved[0].Row.Title.DBID], + mediaProps, titleProps[resolved[0].Row.Title.DBID], ), nil } @@ -87,10 +99,22 @@ func HandleMediaMeta(env requests.RequestEnv) (any, error) { //nolint:gocritic / items[i].Error = &errText continue } + mediaTags := mediaTagsByID[item.Row.DBID] + mediaProps := mediaPropsByID[item.Row.DBID] + ids, metaErr := equivalentMediaIDs(&env, item.Row) + if metaErr != nil { + return nil, metaErr + } + if len(ids) > 1 { + mediaTags, mediaProps, metaErr = mergedMediaMeta(&env, item.Row) + if metaErr != nil { + return nil, metaErr + } + } response := buildMediaMetaResponse( item.Row, - mediaTags[item.Row.DBID], titleTags[item.Row.Title.DBID], - mediaProps[item.Row.DBID], titleProps[item.Row.Title.DBID], + mediaTags, titleTags[item.Row.Title.DBID], + mediaProps, titleProps[item.Row.Title.DBID], ) items[i].Media = &response.Media } @@ -109,6 +133,45 @@ func buildMediaMetaBatchErrorResponse(resolved []resolvedMediaItem) models.Media return models.MediaMetaBatchResponse{Items: items} } +func mergedMediaMeta( + env *requests.RequestEnv, + row *database.MediaFullRow, +) ([]database.TagInfo, []database.MediaProperty, error) { + ids, err := equivalentMediaIDs(env, row) + if err != nil { + return nil, nil, err + } + if len(ids) == 1 { + mediaTags, err := env.Database.MediaDB.GetMediaTagsByMediaDBID(env.Context, row.DBID) + if err != nil { + return nil, nil, fmt.Errorf("failed to get media tags: %w", err) + } + mediaProps, err := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) + if err != nil { + return nil, nil, fmt.Errorf("failed to get media property metadata: %w", err) + } + return mediaTags, mediaProps, nil + } + + mediaTagsByID, err := env.Database.MediaDB.GetMediaTagsByMediaDBIDs(env.Context, ids) + if err != nil { + return nil, nil, fmt.Errorf("failed to get media tags: %w", err) + } + mediaPropsByID, err := env.Database.MediaDB.GetMediaPropertyMetadataByMediaDBIDs(env.Context, ids) + if err != nil { + return nil, nil, fmt.Errorf("failed to get media property metadata: %w", err) + } + + aliasTags := make([][]database.TagInfo, 0, len(ids)-1) + aliasProps := make([][]database.MediaProperty, 0, len(ids)-1) + for _, id := range ids[1:] { + aliasTags = append(aliasTags, mediaTagsByID[id]) + aliasProps = append(aliasProps, mediaPropsByID[id]) + } + return mergeMediaTags(mediaTagsByID[row.DBID], aliasTags...), + mergeMediaProperties(mediaPropsByID[row.DBID], aliasProps...), nil +} + func handleMediaMetaSinglePath(env *requests.RequestEnv, ref mediaRefParam) (any, error) { db := env.Database.MediaDB row, err := resolveMediaBySystemAndPath(env, ref.System, ref.Path) @@ -116,10 +179,25 @@ func handleMediaMetaSinglePath(env *requests.RequestEnv, ref mediaRefParam) (any return nil, err } + ids, err := equivalentMediaIDs(env, row) + if err != nil { + return nil, err + } mediaTags, err := db.GetMediaTagsByMediaDBID(env.Context, row.DBID) if err != nil { return nil, fmt.Errorf("failed to get media tags: %w", err) } + if len(ids) > 1 { + mediaTagsByID, tagsErr := db.GetMediaTagsByMediaDBIDs(env.Context, ids[1:]) + if tagsErr != nil { + return nil, fmt.Errorf("failed to get media tags: %w", tagsErr) + } + aliasTags := make([][]database.TagInfo, 0, len(ids)-1) + for _, id := range ids[1:] { + aliasTags = append(aliasTags, mediaTagsByID[id]) + } + mediaTags = mergeMediaTags(mediaTags, aliasTags...) + } titleTags, err := db.GetMediaTitleTagsByMediaTitleDBID(env.Context, row.Title.DBID) if err != nil { return nil, fmt.Errorf("failed to get title tags: %w", err) @@ -128,6 +206,17 @@ func handleMediaMetaSinglePath(env *requests.RequestEnv, ref mediaRefParam) (any if err != nil { return nil, fmt.Errorf("failed to get media property metadata: %w", err) } + if len(ids) > 1 { + mediaPropsByID, propsErr := db.GetMediaPropertyMetadataByMediaDBIDs(env.Context, ids[1:]) + if propsErr != nil { + return nil, fmt.Errorf("failed to get media property metadata: %w", propsErr) + } + aliasProps := make([][]database.MediaProperty, 0, len(ids)-1) + for _, id := range ids[1:] { + aliasProps = append(aliasProps, mediaPropsByID[id]) + } + mediaProps = mergeMediaProperties(mediaProps, aliasProps...) + } titleProps, err := db.GetMediaTitlePropertyMetadata(env.Context, row.Title.DBID) if err != nil { return nil, fmt.Errorf("failed to get title property metadata: %w", err) diff --git a/pkg/api/methods/media_resolve.go b/pkg/api/methods/media_resolve.go index 53b1a379..9f172f9e 100644 --- a/pkg/api/methods/media_resolve.go +++ b/pkg/api/methods/media_resolve.go @@ -58,6 +58,12 @@ func resolveMediaBySystemAndPath( if err != nil { return nil, err } + if media == nil { + media, err = resolveSingletonMediaPath(env, system, mediaPath) + if err != nil { + return nil, err + } + } if media == nil { return nil, models.ClientErrf("media not found: %s/%s", systemID, mediaPath) } diff --git a/pkg/api/methods/media_resolve_test.go b/pkg/api/methods/media_resolve_test.go index 78b73513..17584c5c 100644 --- a/pkg/api/methods/media_resolve_test.go +++ b/pkg/api/methods/media_resolve_test.go @@ -128,6 +128,58 @@ func TestResolveMediaBySystemAndPath_RelativeFallbackAmbiguous(t *testing.T) { pl.AssertExpectations(t) } +func TestResolveMediaBySystemAndPath_SingletonContainerFallbackSuccess(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + pl := mocks.NewMockPlatform() + cfg := &config.Instance{} + system := database.System{DBID: 10, SystemID: "NES", Name: "Nintendo Entertainment System"} + containerPath := filepath.ToSlash(filepath.Join("roms", "NES", "Game.zip")) + childPath := containerPath + "/Game.nes" + media := database.Media{DBID: 20, Path: childPath, ParentDir: containerPath + "/"} + row := &database.MediaFullRow{ + Media: media, + Title: database.MediaTitle{DBID: 30, Name: "Game"}, + System: system, + } + + mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) + mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, containerPath). + Return((*database.Media)(nil), nil) + mockDB.On("FindSingleDescendantMedia", mock.Anything, system.DBID, containerPath).Return(&media, nil) + mockDB.On("GetMediaWithTitleAndSystem", mock.Anything, media.DBID).Return(row, nil) + + env := makeResolveMediaEnv(mockDB, pl, nil, cfg) + result, err := resolveMediaBySystemAndPath(&env, "NES", containerPath) + require.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, childPath, result.Path) + mockDB.AssertExpectations(t) +} + +func TestResolveMediaBySystemAndPath_SingletonContainerFallbackMiss(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + pl := mocks.NewMockPlatform() + cfg := &config.Instance{} + system := database.System{DBID: 10, SystemID: "NES", Name: "Nintendo Entertainment System"} + containerPath := filepath.ToSlash(filepath.Join("roms", "NES", "Collection")) + + mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) + mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, containerPath). + Return((*database.Media)(nil), nil) + mockDB.On("FindSingleDescendantMedia", mock.Anything, system.DBID, containerPath). + Return((*database.Media)(nil), nil) + + env := makeResolveMediaEnv(mockDB, pl, nil, cfg) + _, err := resolveMediaBySystemAndPath(&env, "NES", containerPath) + require.Error(t, err) + assert.Contains(t, err.Error(), "media not found") + mockDB.AssertExpectations(t) +} + func TestResolveMediaBySystemAndPath_URIDoesNotUseRelativeFallback(t *testing.T) { t.Parallel() diff --git a/pkg/database/database.go b/pkg/database/database.go index 6f8639f5..02a99ee6 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -692,6 +692,9 @@ type MediaDBI interface { // or nil, nil when no row is found. FindMediaBySystemAndPath(ctx context.Context, systemDBID int64, path string) (*Media, error) FindMediaBySystemAndPaths(ctx context.Context, systemDBID int64, paths []string) (map[string]Media, error) + // FindSingleDescendantMedia returns the only non-missing Media row below dirPath + // for systemDBID, or nil, nil when dirPath has zero or multiple descendants. + FindSingleDescendantMedia(ctx context.Context, systemDBID int64, dirPath string) (*Media, error) // FindMediaBySystemAndPathFold returns the Media row matching systemDBID and // path using a case-insensitive path comparison, or nil, nil when no row is diff --git a/pkg/database/mediadb/sql_scraper.go b/pkg/database/mediadb/sql_scraper.go index 92a24e0d..86ec2f72 100644 --- a/pkg/database/mediadb/sql_scraper.go +++ b/pkg/database/mediadb/sql_scraper.go @@ -121,6 +121,54 @@ func (db *MediaDB) FindMediaBySystemAndPaths( return results, rows.Err() } +func (db *MediaDB) FindSingleDescendantMedia( + ctx context.Context, systemDBID int64, dirPath string, +) (*database.Media, error) { + if db.sql == nil { + return nil, ErrNullSQL + } + + prefix := strings.TrimRight(dirPath, "/") + "/" + rows, err := db.sql.QueryContext(ctx, ` + SELECT DBID, MediaTitleDBID, SystemDBID, Path, ParentDir, IsMissing + FROM Media + WHERE SystemDBID = ? AND IsMissing = 0 AND substr(Path, 1, length(?)) = ? + ORDER BY Path ASC, DBID ASC + LIMIT 2 + `, systemDBID, prefix, prefix) + if err != nil { + return nil, fmt.Errorf("failed to query FindSingleDescendantMedia: %w", err) + } + defer func() { + if closeErr := rows.Close(); closeErr != nil { + log.Warn().Err(closeErr).Msg("failed to close rows") + } + }() + + var matches []database.Media + for rows.Next() { + var row database.Media + if scanErr := rows.Scan( + &row.DBID, + &row.MediaTitleDBID, + &row.SystemDBID, + &row.Path, + &row.ParentDir, + &row.IsMissing, + ); scanErr != nil { + return nil, fmt.Errorf("failed to scan FindSingleDescendantMedia: %w", scanErr) + } + matches = append(matches, row) + } + if rowsErr := rows.Err(); rowsErr != nil { + return nil, fmt.Errorf("failed to iterate FindSingleDescendantMedia: %w", rowsErr) + } + if len(matches) != 1 { + return nil, nil //nolint:nilnil // zero or ambiguous descendants are not singleton aliases + } + return &matches[0], nil +} + // FindMediaBySystemAndPathFold returns the Media row for the given system and // path using a case-insensitive path comparison, or nil, nil when not found. // LOWER() in SQLite covers ASCII only, which is sufficient for filesystem paths. diff --git a/pkg/database/mediadb/sql_scraper_test.go b/pkg/database/mediadb/sql_scraper_test.go index f0724d8d..e1fbe818 100644 --- a/pkg/database/mediadb/sql_scraper_test.go +++ b/pkg/database/mediadb/sql_scraper_test.go @@ -95,6 +95,114 @@ func TestFindMediaBySystemAndPath_WrongSystem(t *testing.T) { assert.Nil(t, m, "path exists but systemDBID doesn't match") } +func TestFindSingleDescendantMedia_ReturnsOnlyChild(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupScraperTestDB(t) + defer cleanup() + ctx := context.Background() + + childPath := filepath.ToSlash(filepath.Join("roms", "Zelda", "zelda.nes")) + _, err := mediaDB.sql.ExecContext(ctx, ` + INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES (2, 1, 'zelda', 'Zelda'); + INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path, ParentDir) VALUES (2, 2, 1, ?, ?); + `, childPath, filepath.ToSlash(filepath.Join("roms", "Zelda"))+"/") + require.NoError(t, err) + + media, err := mediaDB.FindSingleDescendantMedia(ctx, 1, filepath.ToSlash(filepath.Join("roms", "Zelda"))) + require.NoError(t, err) + require.NotNil(t, media) + assert.Equal(t, int64(2), media.DBID) + assert.Equal(t, childPath, media.Path) +} + +func TestFindSingleDescendantMedia_RejectsMultipleChildren(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupScraperTestDB(t) + defer cleanup() + ctx := context.Background() + + parent := filepath.ToSlash(filepath.Join("roms", "Collection")) + _, err := mediaDB.sql.ExecContext(ctx, ` + INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES + (2, 1, 'one', 'One'), + (3, 1, 'two', 'Two'); + INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path, ParentDir) VALUES + (2, 2, 1, ?, ?), + (3, 3, 1, ?, ?); + `, parent+"/one.nes", parent+"/", parent+"/two.nes", parent+"/") + require.NoError(t, err) + + media, err := mediaDB.FindSingleDescendantMedia(ctx, 1, parent) + require.NoError(t, err) + assert.Nil(t, media) +} + +func TestFindSingleDescendantMedia_IgnoresMissingAndOtherSystems(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupScraperTestDB(t) + defer cleanup() + ctx := context.Background() + + parent := filepath.ToSlash(filepath.Join("roms", "Shared")) + _, err := mediaDB.sql.ExecContext(ctx, ` + INSERT INTO Systems (DBID, SystemID, Name) VALUES (2, 'SNES', 'Super Nintendo'); + INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES + (2, 1, 'nes-game', 'NES Game'), + (3, 2, 'snes-game', 'SNES Game'), + (4, 1, 'missing-game', 'Missing Game'); + INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path, ParentDir, IsMissing) VALUES + (2, 2, 1, ?, ?, 0), + (3, 3, 2, ?, ?, 0), + (4, 4, 1, ?, ?, 1); + `, parent+"/nes.nes", parent+"/", parent+"/snes.sfc", parent+"/", parent+"/missing.nes", parent+"/") + require.NoError(t, err) + + media, err := mediaDB.FindSingleDescendantMedia(ctx, 1, parent+"/") + require.NoError(t, err) + require.NotNil(t, media) + assert.Equal(t, int64(2), media.DBID) +} + +func TestFindSingleDescendantMedia_UsesByteExactPrefix(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupScraperTestDB(t) + defer cleanup() + ctx := context.Background() + + _, err := mediaDB.sql.ExecContext(ctx, ` + INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES + (2, 1, 'underscore', 'Underscore'), + (3, 1, 'wildcard', 'Wildcard'), + (4, 1, 'percent', 'Percent'), + (5, 1, 'percent-wildcard', 'Percent Wildcard'), + (6, 1, 'case-upper', 'Case Upper'), + (7, 1, 'case-lower', 'Case Lower'); + INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path, ParentDir, IsMissing) VALUES + (2, 2, 1, 'roms/Game_1/game.nes', 'roms/Game_1/', 0), + (3, 3, 1, 'roms/GameA1/game.nes', 'roms/GameA1/', 0), + (4, 4, 1, 'roms/Game%1/game.nes', 'roms/Game%1/', 0), + (5, 5, 1, 'roms/GameXYZ1/game.nes', 'roms/GameXYZ1/', 0), + (6, 6, 1, 'roms/CaseGame/game.nes', 'roms/CaseGame/', 0), + (7, 7, 1, 'roms/casegame/game.nes', 'roms/casegame/', 0); + `) + require.NoError(t, err) + + underscore, err := mediaDB.FindSingleDescendantMedia(ctx, 1, "roms/Game_1") + require.NoError(t, err) + require.NotNil(t, underscore) + assert.Equal(t, int64(2), underscore.DBID) + + percent, err := mediaDB.FindSingleDescendantMedia(ctx, 1, "roms/Game%1") + require.NoError(t, err) + require.NotNil(t, percent) + assert.Equal(t, int64(4), percent.DBID) + + caseExact, err := mediaDB.FindSingleDescendantMedia(ctx, 1, "roms/CaseGame") + require.NoError(t, err) + require.NotNil(t, caseExact) + assert.Equal(t, int64(6), caseExact.DBID) +} + // --- FindMediaBySystemAndPathFold --- func TestFindMediaBySystemAndPathFold_ExactMatch(t *testing.T) { diff --git a/pkg/testing/helpers/db_mocks.go b/pkg/testing/helpers/db_mocks.go index 1288cca3..38ea979c 100644 --- a/pkg/testing/helpers/db_mocks.go +++ b/pkg/testing/helpers/db_mocks.go @@ -2254,6 +2254,19 @@ func (m *MockMediaDBI) FindMediaBySystemAndPaths( return nil, args.Error(1) //nolint:wrapcheck // mock passes testify errors through unwrapped by design } +func (m *MockMediaDBI) FindSingleDescendantMedia( + ctx context.Context, systemDBID int64, dirPath string, +) (*database.Media, error) { + if !m.hasExpectedCall("FindSingleDescendantMedia") { + return nil, nil //nolint:nilnil // default mock behavior for tests that do not exercise aliasing + } + args := m.Called(ctx, systemDBID, dirPath) + if result, ok := args.Get(0).(*database.Media); ok { + return result, args.Error(1) //nolint:wrapcheck // mock passes testify errors through unwrapped by design + } + return nil, args.Error(1) //nolint:wrapcheck // mock passes testify errors through unwrapped by design +} + func (m *MockMediaDBI) FindMediaBySystemAndPathFold( ctx context.Context, systemDBID int64, path string, ) (*database.Media, error) { From bc2d76392845fe286c81fce5431869a14b33d521 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 12:02:12 +0800 Subject: [PATCH 2/7] fix(media): satisfy lint for singleton aliases --- pkg/api/methods/media_alias.go | 5 ++++- pkg/api/methods/media_image.go | 33 +++++++++++---------------------- pkg/api/methods/media_meta.go | 12 ++++++------ 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/pkg/api/methods/media_alias.go b/pkg/api/methods/media_alias.go index 04f5342e..dceb33fd 100644 --- a/pkg/api/methods/media_alias.go +++ b/pkg/api/methods/media_alias.go @@ -105,7 +105,10 @@ func mergeMediaTags(primary []database.TagInfo, aliases ...[]database.TagInfo) [ return merged } -func mergeMediaProperties(primary []database.MediaProperty, aliases ...[]database.MediaProperty) []database.MediaProperty { +func mergeMediaProperties( + primary []database.MediaProperty, + aliases ...[]database.MediaProperty, +) []database.MediaProperty { if len(aliases) == 0 { return primary } diff --git a/pkg/api/methods/media_image.go b/pkg/api/methods/media_image.go index d3b5aaef..c206c278 100644 --- a/pkg/api/methods/media_image.go +++ b/pkg/api/methods/media_image.go @@ -150,7 +150,9 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic } prefs := imagePrefs(nil, ref.ImageTypes) - return selectMediaImageFromSources(env.Context, afero.NewOsFs(), db, row, mediaPropSources, titleProps, prefs, maxBytes) + return selectMediaImageFromSources( + env.Context, afero.NewOsFs(), db, row, mediaPropSources, titleProps, prefs, maxBytes, + ) } func parseMediaImageRequest(raw json.RawMessage) (mediaRefParam, error) { @@ -238,42 +240,29 @@ func mediaImageBlobTooLargeError(prop *database.MediaProperty, maxBytes int64) e } func mediaImagePropSources(env *requests.RequestEnv, row *database.MediaFullRow) ([][]database.MediaProperty, error) { - ids, err := equivalentMediaIDs(env, row) + mediaIDs, err := equivalentMediaIDs(env, row) if err != nil { return nil, err } - if len(ids) == 1 { - props, err := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) - if err != nil { - return nil, fmt.Errorf("failed to get media property metadata: %w", err) + if len(mediaIDs) == 1 { + props, propErr := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) + if propErr != nil { + return nil, fmt.Errorf("failed to get media property metadata: %w", propErr) } return [][]database.MediaProperty{props}, nil } - propsByID, err := env.Database.MediaDB.GetMediaPropertyMetadataByMediaDBIDs(env.Context, ids) + propsByID, err := env.Database.MediaDB.GetMediaPropertyMetadataByMediaDBIDs(env.Context, mediaIDs) if err != nil { return nil, fmt.Errorf("failed to get media property metadata: %w", err) } - sources := make([][]database.MediaProperty, 0, len(ids)) - for _, id := range ids { + sources := make([][]database.MediaProperty, 0, len(mediaIDs)) + for _, id := range mediaIDs { sources = append(sources, propsByID[id]) } return sources, nil } -func selectMediaImage( - ctx context.Context, - fs afero.Fs, - db database.MediaDBI, - row *database.MediaFullRow, - mediaProps []database.MediaProperty, - titleProps []database.MediaProperty, - prefs []string, - maxBytes int64, -) (models.MediaImageResponse, error) { - return selectMediaImageFromSources(ctx, fs, db, row, [][]database.MediaProperty{mediaProps}, titleProps, prefs, maxBytes) -} - func selectMediaImageFromSources( ctx context.Context, fs afero.Fs, diff --git a/pkg/api/methods/media_meta.go b/pkg/api/methods/media_meta.go index 9edc07d0..d7cb04eb 100644 --- a/pkg/api/methods/media_meta.go +++ b/pkg/api/methods/media_meta.go @@ -142,13 +142,13 @@ func mergedMediaMeta( return nil, nil, err } if len(ids) == 1 { - mediaTags, err := env.Database.MediaDB.GetMediaTagsByMediaDBID(env.Context, row.DBID) - if err != nil { - return nil, nil, fmt.Errorf("failed to get media tags: %w", err) + mediaTags, tagsErr := env.Database.MediaDB.GetMediaTagsByMediaDBID(env.Context, row.DBID) + if tagsErr != nil { + return nil, nil, fmt.Errorf("failed to get media tags: %w", tagsErr) } - mediaProps, err := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) - if err != nil { - return nil, nil, fmt.Errorf("failed to get media property metadata: %w", err) + mediaProps, propsErr := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) + if propsErr != nil { + return nil, nil, fmt.Errorf("failed to get media property metadata: %w", propsErr) } return mediaTags, mediaProps, nil } From d4eb3b08ebf18da505b3c9f65115f8fbd9c20a8f Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 12:24:26 +0800 Subject: [PATCH 3/7] fix(media): gate singleton aliases by platform --- docs/api/methods.md | 8 ++++---- docs/scraper.md | 4 ++-- pkg/api/methods/media_alias.go | 13 ++++++++++++- pkg/api/methods/media_browse.go | 2 +- pkg/api/methods/media_resolve_test.go | 27 +++++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/docs/api/methods.md b/docs/api/methods.md index 31c4d8cf..780e800d 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -559,7 +559,7 @@ All parameters are optional. When called with no parameters, returns root entrie | Key | Type | Required | Description | | :----------- | :------- | :------- | :----------------------------------------------------------------------------------------------- | -| mediaId | number | No | Opaque media database row ID. Present on `media` entries, and on `directory` entries that contain exactly one indexed media descendant, for efficient follow-up `media.meta` and `media.image` requests. | +| mediaId | number | No | Opaque media database row ID. Present on `media` entries, and on zip-as-directory platform `directory` entries that contain exactly one indexed media descendant, for efficient follow-up `media.meta` and `media.image` requests. | | name | string | Yes | Display name of the entry. | | path | string | Yes | Full path to the entry. | | type | string | Yes | Entry type: `root`, `directory`, or `media`. | @@ -567,9 +567,9 @@ All parameters are optional. When called with no parameters, returns root entrie | group | string | No | Launcher group name. Present on virtual scheme `root` entries. | | systemId | string | No | System ID for the media or single-system filtered route (e.g. `SNES`). Present on `media` entries and filtered `root` entries when exactly one system applies. | | systemIds | string[] | No | System IDs represented by a filtered `root` or `directory` entry. | -| zapScript | string | No | ZapScript command to launch this media. Present on `media` entries and singleton media-container `directory` entries. | -| relativePath | string | No | Relative path from root directory. Present on `media` entries and singleton media-container `directory` entries. | -| tags | object[] | No | Tags attached to the media. Each object has `tag` (string) and `type` (string). Present on `media` entries and singleton media-container `directory` entries. | +| zapScript | string | No | ZapScript command to launch this media. Present on `media` entries and singleton media-container `directory` entries on zip-as-directory platforms. | +| relativePath | string | No | Relative path from root directory. Present on `media` entries and singleton media-container `directory` entries on zip-as-directory platforms. | +| tags | object[] | No | Tags attached to the media. Each object has `tag` (string) and `type` (string). Present on `media` entries and singleton media-container `directory` entries on zip-as-directory platforms. | ##### Browse pagination object diff --git a/docs/scraper.md b/docs/scraper.md index 29166d4a..f30af6a5 100644 --- a/docs/scraper.md +++ b/docs/scraper.md @@ -205,9 +205,9 @@ Progress is queryable with `media.scrape.status` and broadcast as `media.scrapin Only one scraper can run at a time, and scraping is mutually exclusive with media indexing. -`media.meta` returns the metadata graph for media rows: media-level tags and properties, title-level tags and properties, and stored system identity. Single requests accept `mediaId` or `system`/`path` and keep the single-response shape; batch requests use `items` and return per-item results. Binary property bytes are not included; clients should use `media.image` for image data. A `system`/`path` request for a folder or zip-as-directory container resolves to its only non-missing indexed media descendant when exactly one exists. +`media.meta` returns the metadata graph for media rows: media-level tags and properties, title-level tags and properties, and stored system identity. Single requests accept `mediaId` or `system`/`path` and keep the single-response shape; batch requests use `items` and return per-item results. Binary property bytes are not included; clients should use `media.image` for image data. On platforms that treat zips as directories, a `system`/`path` request for a folder or zip-as-directory container resolves to its only non-missing indexed media descendant when exactly one exists. -`media.image` accepts one media ref plus image type preferences such as `image`, `boxart`, `boxart3d`, `screenshot`, `wheel`, `titleshot`, `map`, `marquee`, and `fanart`. These resolve to canonical image property tags; for example `boxart` becomes `property:image-boxart` and `image` becomes `property:image-image`. Media-level properties are preferred over title-level properties for the same type. Singleton container aliases are checked as media-level fallbacks, so artwork attached to a sole child or its container can be found from either path. For stale image properties in these canonical tags, such as missing file paths for `property:image-boxart` or `property:image-image`, `media.image` logs the stale property in memory only and does not delete DB rows; lookup falls through to the next available source. +`media.image` accepts one media ref plus image type preferences such as `image`, `boxart`, `boxart3d`, `screenshot`, `wheel`, `titleshot`, `map`, `marquee`, and `fanart`. These resolve to canonical image property tags; for example `boxart` becomes `property:image-boxart` and `image` becomes `property:image-image`. Media-level properties are preferred over title-level properties for the same type. On zip-as-directory platforms, singleton container aliases are checked as media-level fallbacks, so artwork attached to a sole child or its container can be found from either path. For stale image properties in these canonical tags, such as missing file paths for `property:image-boxart` or `property:image-image`, `media.image` logs the stale property in memory only and does not delete DB rows; lookup falls through to the next available source. ## Useful Focused Tests diff --git a/pkg/api/methods/media_alias.go b/pkg/api/methods/media_alias.go index dceb33fd..dcb4a9e4 100644 --- a/pkg/api/methods/media_alias.go +++ b/pkg/api/methods/media_alias.go @@ -27,11 +27,19 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" ) +func singletonMediaAliasesEnabled(env *requests.RequestEnv) bool { + return env != nil && env.Platform != nil && env.Platform.Settings().ZipsAsDirs +} + func resolveSingletonMediaPath( env *requests.RequestEnv, system database.System, mediaPath string, ) (*database.Media, error) { + if !singletonMediaAliasesEnabled(env) { + return nil, nil //nolint:nilnil // disabled aliasing has no singleton fallback + } + media, err := env.Database.MediaDB.FindSingleDescendantMedia(env.Context, system.DBID, mediaPath) if err != nil { return nil, fmt.Errorf("failed to resolve singleton media path: %w", err) @@ -40,11 +48,14 @@ func resolveSingletonMediaPath( } func equivalentMediaIDs(env *requests.RequestEnv, row *database.MediaFullRow) ([]int64, error) { - if env == nil || row == nil || env.Database == nil || env.Database.MediaDB == nil { + if row == nil { return nil, nil } ids := []int64{row.DBID} + if env == nil || env.Database == nil || env.Database.MediaDB == nil || !singletonMediaAliasesEnabled(env) { + return ids, nil + } seen := map[int64]bool{row.DBID: true} add := func(media *database.Media) { if media == nil || seen[media.DBID] { diff --git a/pkg/api/methods/media_browse.go b/pkg/api/methods/media_browse.go index 51737ee4..8f9426e1 100644 --- a/pkg/api/methods/media_browse.go +++ b/pkg/api/methods/media_browse.go @@ -735,7 +735,7 @@ func annotateSingletonDirectoryEntry( systemIDs []string, rootDirs []string, ) { - if env == nil || entry == nil || env.Database == nil || len(systemIDs) != 1 { + if env == nil || entry == nil || env.Database == nil || len(systemIDs) != 1 || !singletonMediaAliasesEnabled(env) { return } diff --git a/pkg/api/methods/media_resolve_test.go b/pkg/api/methods/media_resolve_test.go index 17584c5c..ba3468f3 100644 --- a/pkg/api/methods/media_resolve_test.go +++ b/pkg/api/methods/media_resolve_test.go @@ -144,6 +144,7 @@ func TestResolveMediaBySystemAndPath_SingletonContainerFallbackSuccess(t *testin System: system, } + pl.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}) mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, containerPath). Return((*database.Media)(nil), nil) @@ -167,6 +168,7 @@ func TestResolveMediaBySystemAndPath_SingletonContainerFallbackMiss(t *testing.T system := database.System{DBID: 10, SystemID: "NES", Name: "Nintendo Entertainment System"} containerPath := filepath.ToSlash(filepath.Join("roms", "NES", "Collection")) + pl.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}) mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, containerPath). Return((*database.Media)(nil), nil) @@ -180,6 +182,28 @@ func TestResolveMediaBySystemAndPath_SingletonContainerFallbackMiss(t *testing.T mockDB.AssertExpectations(t) } +func TestResolveMediaBySystemAndPath_SingletonContainerFallbackDisabled(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + pl := mocks.NewMockPlatform() + cfg := &config.Instance{} + system := database.System{DBID: 10, SystemID: "NES", Name: "Nintendo Entertainment System"} + containerPath := filepath.ToSlash(filepath.Join("roms", "NES", "Game.zip")) + + pl.On("Settings").Return(platforms.Settings{ZipsAsDirs: false}) + mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) + mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, containerPath). + Return((*database.Media)(nil), nil) + + env := makeResolveMediaEnv(mockDB, pl, nil, cfg) + _, err := resolveMediaBySystemAndPath(&env, "NES", containerPath) + require.Error(t, err) + assert.Contains(t, err.Error(), "media not found") + mockDB.AssertNotCalled(t, "FindSingleDescendantMedia", mock.Anything, system.DBID, containerPath) + mockDB.AssertExpectations(t) +} + func TestResolveMediaBySystemAndPath_URIDoesNotUseRelativeFallback(t *testing.T) { t.Parallel() @@ -192,6 +216,7 @@ func TestResolveMediaBySystemAndPath_URIDoesNotUseRelativeFallback(t *testing.T) }) mediaPath := "steam://440/Team%20Fortress%202" + pl.On("Settings").Return(platforms.Settings{}) mockDB.On("FindSystemBySystemID", "Steam").Return(system, nil) mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, mediaPath). Return((*database.Media)(nil), nil) @@ -213,6 +238,7 @@ func TestResolveMediaBySystemAndPath_MissingLauncherCacheSkipsRelativeFallback(t system := database.System{DBID: 10, SystemID: "NES", Name: "Nintendo Entertainment System"} mediaPath := filepath.Join("NES", "mario.nes") + pl.On("Settings").Return(platforms.Settings{}) mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, mediaPath). Return((*database.Media)(nil), nil) @@ -237,6 +263,7 @@ func TestResolveMediaBySystemAndPath_MalformedRelativePathSkipsFallback(t *testi }) mediaPath := "mario.nes" + pl.On("Settings").Return(platforms.Settings{}) mockDB.On("FindSystemBySystemID", "NES").Return(system, nil) mockDB.On("FindMediaBySystemAndPath", mock.Anything, system.DBID, mediaPath). Return((*database.Media)(nil), nil) From b4ff54f2fac063019a9e0d7c84e90670ff4e321a Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 12:39:25 +0800 Subject: [PATCH 4/7] fix(media): address singleton alias review --- pkg/api/methods/media_alias_test.go | 162 +++++++++++++++++++++++ pkg/api/methods/media_browse.go | 2 +- pkg/api/methods/media_image.go | 16 +-- pkg/api/methods/media_resolve_test.go | 5 +- pkg/database/mediadb/sql_scraper_test.go | 13 +- 5 files changed, 184 insertions(+), 14 deletions(-) create mode 100644 pkg/api/methods/media_alias_test.go diff --git a/pkg/api/methods/media_alias_test.go b/pkg/api/methods/media_alias_test.go new file mode 100644 index 00000000..cd56a15d --- /dev/null +++ b/pkg/api/methods/media_alias_test.go @@ -0,0 +1,162 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package methods + +import ( + "context" + "testing" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" + testhelpers "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestEquivalentMediaIDsNilAndDisabledGuards(t *testing.T) { + t.Parallel() + + row := &database.MediaFullRow{Media: database.Media{DBID: 10}} + + ids, err := equivalentMediaIDs(nil, nil) + require.NoError(t, err) + assert.Nil(t, ids) + + ids, err = equivalentMediaIDs(nil, row) + require.NoError(t, err) + assert.Equal(t, []int64{10}, ids) + + ids, err = equivalentMediaIDs(&requests.RequestEnv{}, row) + require.NoError(t, err) + assert.Equal(t, []int64{10}, ids) + + ids, err = equivalentMediaIDs(&requests.RequestEnv{Database: &database.Database{}}, row) + require.NoError(t, err) + assert.Equal(t, []int64{10}, ids) +} + +func TestEquivalentMediaIDsParentChildAliases(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + platform := mocks.NewMockPlatform() + platform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}) + + row := &database.MediaFullRow{ + Media: database.Media{ + DBID: 20, + Path: "roms/Game.zip/Game.nes", + ParentDir: "roms/Game.zip/", + }, + System: database.System{DBID: 1, SystemID: "NES"}, + } + parent := &database.Media{DBID: 10, Path: "roms/Game.zip"} + + mockDB.On("FindSingleDescendantMedia", mock.Anything, int64(1), row.Path). + Return((*database.Media)(nil), nil) + mockDB.On("FindSingleDescendantMedia", mock.Anything, int64(1), "roms/Game.zip"). + Return(&database.Media{DBID: 20, Path: row.Path}, nil) + mockDB.On("FindMediaBySystemAndPath", mock.Anything, int64(1), "roms/Game.zip").Return(parent, nil) + + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockDB}, + Platform: platform, + } + ids, err := equivalentMediaIDs(env, row) + require.NoError(t, err) + assert.Equal(t, []int64{20, 10}, ids) + mockDB.AssertExpectations(t) + platform.AssertExpectations(t) +} + +func TestEquivalentMediaIDsSkipsEmptyOrSelfParent(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + platform := mocks.NewMockPlatform() + platform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}).Twice() + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockDB}, + Platform: platform, + } + + for _, row := range []*database.MediaFullRow{ + { + Media: database.Media{DBID: 20, Path: "roms/Game.nes"}, + System: database.System{DBID: 1}, + }, + { + Media: database.Media{DBID: 21, Path: "roms/Game.zip", ParentDir: "roms/Game.zip/"}, + System: database.System{DBID: 1}, + }, + } { + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, row.Path). + Return((*database.Media)(nil), nil).Once() + ids, err := equivalentMediaIDs(env, row) + require.NoError(t, err) + assert.Equal(t, []int64{row.DBID}, ids) + } + + mockDB.AssertExpectations(t) + platform.AssertExpectations(t) +} + +func TestMergeMediaTagsDedupesAndPreservesPrimary(t *testing.T) { + t.Parallel() + + primary := []database.TagInfo{ + {Type: "region", Tag: "us"}, + {Type: "lang", Tag: "en"}, + } + alias := []database.TagInfo{ + {Type: "region", Tag: "us"}, + {Type: "region", Tag: "jp"}, + } + + assert.Equal(t, []database.TagInfo{ + {Type: "region", Tag: "us"}, + {Type: "lang", Tag: "en"}, + {Type: "region", Tag: "jp"}, + }, mergeMediaTags(primary, alias)) +} + +func TestMergeMediaPropertiesDedupesAndPreservesPrimary(t *testing.T) { + t.Parallel() + + primary := []database.MediaProperty{ + {TypeTag: "property:image-boxart", Text: "primary.png"}, + {TypeTag: "property:description", Text: "primary desc"}, + } + alias := []database.MediaProperty{ + {TypeTag: "property:image-boxart", Text: "alias.png"}, + {TypeTag: "property:image-screenshot", Text: "shot.png"}, + } + + assert.Equal(t, []database.MediaProperty{ + {TypeTag: "property:image-boxart", Text: "primary.png"}, + {TypeTag: "property:description", Text: "primary desc"}, + {TypeTag: "property:image-screenshot", Text: "shot.png"}, + }, mergeMediaProperties(primary, alias)) +} diff --git a/pkg/api/methods/media_browse.go b/pkg/api/methods/media_browse.go index 8f9426e1..ba7193ef 100644 --- a/pkg/api/methods/media_browse.go +++ b/pkg/api/methods/media_browse.go @@ -648,7 +648,7 @@ func buildBrowseResponse( // Add directory entries for _, dir := range dirs { - dirPath := path + "/" + dir.Name + dirPath := filepath.ToSlash(filepath.Join(path, dir.Name)) entry := models.BrowseEntry{ Name: dir.Name, Path: dirPath, diff --git a/pkg/api/methods/media_image.go b/pkg/api/methods/media_image.go index c206c278..25a1755a 100644 --- a/pkg/api/methods/media_image.go +++ b/pkg/api/methods/media_image.go @@ -144,9 +144,9 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic if err != nil { return nil, err } - titleProps, err := db.GetMediaTitlePropertyMetadata(env.Context, row.Title.DBID) + titleProps, err := db.GetMediaTitleProperties(env.Context, row.Title.DBID) if err != nil { - return nil, fmt.Errorf("failed to get title property metadata: %w", err) + return nil, fmt.Errorf("failed to get title properties: %w", err) } prefs := imagePrefs(nil, ref.ImageTypes) @@ -182,9 +182,9 @@ func handleMediaImageSinglePath(env *requests.RequestEnv, ref mediaRefParam, max if err != nil { return nil, err } - titleProps, err := db.GetMediaTitlePropertyMetadata(env.Context, row.Title.DBID) + titleProps, err := db.GetMediaTitleProperties(env.Context, row.Title.DBID) if err != nil { - return nil, fmt.Errorf("failed to get title property metadata: %w", err) + return nil, fmt.Errorf("failed to get title properties: %w", err) } return selectMediaImageFromSources( @@ -245,16 +245,16 @@ func mediaImagePropSources(env *requests.RequestEnv, row *database.MediaFullRow) return nil, err } if len(mediaIDs) == 1 { - props, propErr := env.Database.MediaDB.GetMediaPropertyMetadata(env.Context, row.DBID) + props, propErr := env.Database.MediaDB.GetMediaProperties(env.Context, row.DBID) if propErr != nil { - return nil, fmt.Errorf("failed to get media property metadata: %w", propErr) + return nil, fmt.Errorf("failed to get media properties: %w", propErr) } return [][]database.MediaProperty{props}, nil } - propsByID, err := env.Database.MediaDB.GetMediaPropertyMetadataByMediaDBIDs(env.Context, mediaIDs) + propsByID, err := env.Database.MediaDB.GetMediaPropertiesByMediaDBIDs(env.Context, mediaIDs) if err != nil { - return nil, fmt.Errorf("failed to get media property metadata: %w", err) + return nil, fmt.Errorf("failed to get media properties: %w", err) } sources := make([][]database.MediaProperty, 0, len(mediaIDs)) for _, id := range mediaIDs { diff --git a/pkg/api/methods/media_resolve_test.go b/pkg/api/methods/media_resolve_test.go index ba3468f3..fb56a90d 100644 --- a/pkg/api/methods/media_resolve_test.go +++ b/pkg/api/methods/media_resolve_test.go @@ -136,8 +136,9 @@ func TestResolveMediaBySystemAndPath_SingletonContainerFallbackSuccess(t *testin cfg := &config.Instance{} system := database.System{DBID: 10, SystemID: "NES", Name: "Nintendo Entertainment System"} containerPath := filepath.ToSlash(filepath.Join("roms", "NES", "Game.zip")) - childPath := containerPath + "/Game.nes" - media := database.Media{DBID: 20, Path: childPath, ParentDir: containerPath + "/"} + childPath := filepath.ToSlash(filepath.Join(containerPath, "Game.nes")) + parentDir := filepath.ToSlash(containerPath) + "/" + media := database.Media{DBID: 20, Path: childPath, ParentDir: parentDir} row := &database.MediaFullRow{ Media: media, Title: database.MediaTitle{DBID: 30, Name: "Game"}, diff --git a/pkg/database/mediadb/sql_scraper_test.go b/pkg/database/mediadb/sql_scraper_test.go index e1fbe818..af7e7989 100644 --- a/pkg/database/mediadb/sql_scraper_test.go +++ b/pkg/database/mediadb/sql_scraper_test.go @@ -122,6 +122,9 @@ func TestFindSingleDescendantMedia_RejectsMultipleChildren(t *testing.T) { ctx := context.Background() parent := filepath.ToSlash(filepath.Join("roms", "Collection")) + parentDir := filepath.ToSlash(filepath.Join(parent, "")) + "/" + onePath := filepath.ToSlash(filepath.Join(parent, "one.nes")) + twoPath := filepath.ToSlash(filepath.Join(parent, "two.nes")) _, err := mediaDB.sql.ExecContext(ctx, ` INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES (2, 1, 'one', 'One'), @@ -129,7 +132,7 @@ func TestFindSingleDescendantMedia_RejectsMultipleChildren(t *testing.T) { INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path, ParentDir) VALUES (2, 2, 1, ?, ?), (3, 3, 1, ?, ?); - `, parent+"/one.nes", parent+"/", parent+"/two.nes", parent+"/") + `, onePath, parentDir, twoPath, parentDir) require.NoError(t, err) media, err := mediaDB.FindSingleDescendantMedia(ctx, 1, parent) @@ -144,6 +147,10 @@ func TestFindSingleDescendantMedia_IgnoresMissingAndOtherSystems(t *testing.T) { ctx := context.Background() parent := filepath.ToSlash(filepath.Join("roms", "Shared")) + parentDir := filepath.ToSlash(filepath.Join(parent, "")) + "/" + nesPath := filepath.ToSlash(filepath.Join(parent, "nes.nes")) + snesPath := filepath.ToSlash(filepath.Join(parent, "snes.sfc")) + missingPath := filepath.ToSlash(filepath.Join(parent, "missing.nes")) _, err := mediaDB.sql.ExecContext(ctx, ` INSERT INTO Systems (DBID, SystemID, Name) VALUES (2, 'SNES', 'Super Nintendo'); INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES @@ -154,10 +161,10 @@ func TestFindSingleDescendantMedia_IgnoresMissingAndOtherSystems(t *testing.T) { (2, 2, 1, ?, ?, 0), (3, 3, 2, ?, ?, 0), (4, 4, 1, ?, ?, 1); - `, parent+"/nes.nes", parent+"/", parent+"/snes.sfc", parent+"/", parent+"/missing.nes", parent+"/") + `, nesPath, parentDir, snesPath, parentDir, missingPath, parentDir) require.NoError(t, err) - media, err := mediaDB.FindSingleDescendantMedia(ctx, 1, parent+"/") + media, err := mediaDB.FindSingleDescendantMedia(ctx, 1, parentDir) require.NoError(t, err) require.NotNil(t, media) assert.Equal(t, int64(2), media.DBID) From 8632a5b7282677bfef97744bbad3aa0c92516469 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 12:45:12 +0800 Subject: [PATCH 5/7] test(media): cover singleton alias metadata --- pkg/api/methods/media_batch_test.go | 61 +++++++++++++++++++++++ pkg/api/methods/media_browse_test.go | 74 ++++++++++++++++++++++++++++ pkg/api/methods/media_image_test.go | 47 ++++++++++++++++++ pkg/api/methods/media_meta_test.go | 57 +++++++++++++++++++++ 4 files changed, 239 insertions(+) create mode 100644 pkg/api/methods/media_batch_test.go diff --git a/pkg/api/methods/media_batch_test.go b/pkg/api/methods/media_batch_test.go new file mode 100644 index 00000000..bddd0943 --- /dev/null +++ b/pkg/api/methods/media_batch_test.go @@ -0,0 +1,61 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package methods + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseMediaRequest_RejectsMixedBatchAndTopLevelRef(t *testing.T) { + t.Parallel() + + _, err := parseMediaRequest(json.RawMessage(`{ + "mediaId": 1, + "items": [{"mediaId": 2}] + }`), maxMediaMetaBatchItems) + require.Error(t, err) + assert.Contains(t, err.Error(), "items cannot be mixed") +} + +func TestParseMediaRequest_RejectsInvalidBatchItemImageTypes(t *testing.T) { + t.Parallel() + + _, err := parseMediaRequest(json.RawMessage(`{ + "items": [{"mediaId": 2, "imageTypes": [""]}] + }`), maxMediaImageBatchItems) + require.Error(t, err) + assert.Contains(t, err.Error(), "items[0]") + assert.Contains(t, err.Error(), "imageTypes entries must be non-empty") +} + +func TestParseMediaRequest_RejectsTopLevelImageTypesInBatch(t *testing.T) { + t.Parallel() + + _, err := parseMediaRequest(json.RawMessage(`{ + "imageTypes": [""], + "items": [{"mediaId": 2}] + }`), maxMediaImageBatchItems) + require.Error(t, err) + assert.Contains(t, err.Error(), "imageTypes entries must be non-empty") +} diff --git a/pkg/api/methods/media_browse_test.go b/pkg/api/methods/media_browse_test.go index f0ba7da4..de093642 100644 --- a/pkg/api/methods/media_browse_test.go +++ b/pkg/api/methods/media_browse_test.go @@ -431,6 +431,80 @@ func TestHandleMediaBrowse_SystemRootRoutesUsesCachedCandidates(t *testing.T) { mockMediaDB.AssertExpectations(t) } +func TestAnnotateSingletonDirectoryEntry_WhenZipsAsDirsEnabled(t *testing.T) { + t.Parallel() + + mockMediaDB := helpers.NewMockMediaDBI() + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}).Once() + entry := models.BrowseEntry{ + Name: "Game.zip", + Path: filepath.ToSlash(filepath.Join("roms", "Game.zip")), + Type: "directory", + FileCount: intPtr(1), + SystemIDs: []string{"NES"}, + } + row := &database.MediaFullRow{ + Media: database.Media{ + DBID: 20, + Path: filepath.ToSlash(filepath.Join("roms", "Game.zip", "Game.nes")), + ParentDir: filepath.ToSlash(filepath.Join("roms", "Game.zip")) + "/", + }, + Title: database.MediaTitle{DBID: 30, Name: "Game"}, + System: database.System{DBID: 1, SystemID: "NES"}, + } + + mockMediaDB.On("FindSystemBySystemID", "NES").Return(row.System, nil).Once() + mockMediaDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, entry.Path). + Return(&row.Media, nil).Once() + mockMediaDB.On("GetMediaWithTitleAndSystem", mock.Anything, row.DBID).Return(row, nil).Once() + mockMediaDB.On("GetMediaTagsByMediaDBID", mock.Anything, row.DBID). + Return([]database.TagInfo{{Type: "favorite", Tag: "true"}}, nil).Once() + mockMediaDB.On("GetZapScriptTagsBySystemAndPath", mock.Anything, "NES", row.Path). + Return([]database.TagInfo{}, nil).Once() + + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockMediaDB}, + Platform: mockPlatform, + } + annotateSingletonDirectoryEntry(env, &entry, entry.Path, entry.SystemIDs, nil) + + assert.Equal(t, row.DBID, entry.MediaID) + require.NotNil(t, entry.SystemID) + assert.Equal(t, "NES", *entry.SystemID) + require.NotNil(t, entry.ZapScript) + assert.NotEmpty(t, *entry.ZapScript) + assert.Equal(t, []database.TagInfo{{Type: "favorite", Tag: "true"}}, entry.Tags) + mockMediaDB.AssertExpectations(t) + mockPlatform.AssertExpectations(t) +} + +func TestAnnotateSingletonDirectoryEntry_WhenZipsAsDirsDisabledSkipsLookup(t *testing.T) { + t.Parallel() + + mockMediaDB := helpers.NewMockMediaDBI() + mockPlatform := mocks.NewMockPlatform() + mockPlatform.On("Settings").Return(platforms.Settings{ZipsAsDirs: false}).Once() + entry := models.BrowseEntry{ + Path: filepath.ToSlash(filepath.Join("roms", "Game.zip")), + Type: "directory", + FileCount: intPtr(1), + SystemIDs: []string{"NES"}, + } + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockMediaDB}, + Platform: mockPlatform, + } + annotateSingletonDirectoryEntry(env, &entry, entry.Path, entry.SystemIDs, nil) + + assert.Zero(t, entry.MediaID) + assert.Nil(t, entry.ZapScript) + mockMediaDB.AssertNotCalled(t, "FindSystemBySystemID", mock.Anything) + mockPlatform.AssertExpectations(t) +} + func TestDedupeSystemRootEntries(t *testing.T) { t.Parallel() diff --git a/pkg/api/methods/media_image_test.go b/pkg/api/methods/media_image_test.go index bafb04ef..209c9d12 100644 --- a/pkg/api/methods/media_image_test.go +++ b/pkg/api/methods/media_image_test.go @@ -31,6 +31,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" "github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state" testhelpers "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" @@ -345,6 +346,52 @@ func TestHandleMediaImage_FilePathInfersContentType(t *testing.T) { mockDB.AssertExpectations(t) } +func TestMediaImagePropSources_IncludesSingletonAliasSources(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + platform := mocks.NewMockPlatform() + platform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}).Once() + + row := &database.MediaFullRow{ + Media: database.Media{ + DBID: 20, + Path: filepath.ToSlash(filepath.Join("roms", "Game.zip", "Game.nes")), + ParentDir: filepath.ToSlash(filepath.Join("roms", "Game.zip")) + "/", + }, + Title: database.MediaTitle{DBID: 200}, + System: database.System{DBID: 1, SystemID: "NES"}, + } + parentPath := filepath.ToSlash(filepath.Join("roms", "Game.zip")) + parent := &database.Media{DBID: 10, Path: parentPath} + + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, row.Path). + Return((*database.Media)(nil), nil).Once() + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, parentPath). + Return(&row.Media, nil).Once() + mockDB.On("FindMediaBySystemAndPath", mock.Anything, row.System.DBID, parentPath). + Return(parent, nil).Once() + mockDB.On("GetMediaPropertiesByMediaDBIDs", mock.Anything, []int64{20, 10}). + Return(map[int64][]database.MediaProperty{ + 20: {{TypeTag: "property:image-boxart", Text: "child.png"}}, + 10: {{TypeTag: "property:image-boxart", Text: "parent.png"}}, + }, nil).Once() + + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockDB}, + Platform: platform, + } + sources, err := mediaImagePropSources(env, row) + require.NoError(t, err) + + require.Len(t, sources, 2) + assert.Equal(t, "child.png", sources[0][0].Text) + assert.Equal(t, "parent.png", sources[1][0].Text) + mockDB.AssertExpectations(t) + platform.AssertExpectations(t) +} + func TestHandleMediaImage_NoMatchFound(t *testing.T) { t.Parallel() diff --git a/pkg/api/methods/media_meta_test.go b/pkg/api/methods/media_meta_test.go index a071ef85..d4a203ff 100644 --- a/pkg/api/methods/media_meta_test.go +++ b/pkg/api/methods/media_meta_test.go @@ -30,7 +30,9 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" testhelpers "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -402,6 +404,61 @@ func TestHandleMediaMeta_BatchAllMediaIDMissesSkipMetadataFetch(t *testing.T) { mockDB.AssertExpectations(t) } +func TestMergedMediaMeta_MergesSingletonAliasMetadata(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + platform := mocks.NewMockPlatform() + platform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}).Once() + + row := &database.MediaFullRow{ + Media: database.Media{ + DBID: 20, + Path: filepath.ToSlash(filepath.Join("roms", "Game.zip", "Game.nes")), + ParentDir: filepath.ToSlash(filepath.Join("roms", "Game.zip")) + "/", + }, + Title: database.MediaTitle{DBID: 200}, + System: database.System{DBID: 1, SystemID: "NES"}, + } + parentPath := filepath.ToSlash(filepath.Join("roms", "Game.zip")) + parent := &database.Media{DBID: 10, Path: parentPath} + + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, row.Path). + Return((*database.Media)(nil), nil).Once() + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, parentPath). + Return(&row.Media, nil).Once() + mockDB.On("FindMediaBySystemAndPath", mock.Anything, row.System.DBID, parentPath). + Return(parent, nil).Once() + mockDB.On("GetMediaTagsByMediaDBIDs", mock.Anything, []int64{20, 10}).Return(map[int64][]database.TagInfo{ + 20: {{Type: "genre", Tag: "platformer"}}, + 10: {{Type: "genre", Tag: "platformer"}, {Type: "favorite", Tag: "true"}}, + }, nil).Once() + mockDB.On("GetMediaPropertyMetadataByMediaDBIDs", mock.Anything, []int64{20, 10}). + Return(map[int64][]database.MediaProperty{ + 20: {{TypeTag: "property:description", Text: "child"}}, + 10: {{TypeTag: "property:description", Text: "parent"}, {TypeTag: "property:image-boxart", Text: "box.png"}}, + }, nil).Once() + + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockDB}, + Platform: platform, + } + tags, props, err := mergedMediaMeta(env, row) + require.NoError(t, err) + + assert.Equal(t, []database.TagInfo{ + {Type: "genre", Tag: "platformer"}, + {Type: "favorite", Tag: "true"}, + }, tags) + assert.Equal(t, []database.MediaProperty{ + {TypeTag: "property:description", Text: "child"}, + {TypeTag: "property:image-boxart", Text: "box.png"}, + }, props) + mockDB.AssertExpectations(t) + platform.AssertExpectations(t) +} + func TestHandleMediaMeta_InvalidParams(t *testing.T) { t.Parallel() From 0d89680bac2b0208132b7eb1bc465c50d2dbaf7a Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 12:46:21 +0800 Subject: [PATCH 6/7] test(media): satisfy alias coverage lint --- pkg/api/methods/media_meta_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/api/methods/media_meta_test.go b/pkg/api/methods/media_meta_test.go index d4a203ff..0ca0b1fc 100644 --- a/pkg/api/methods/media_meta_test.go +++ b/pkg/api/methods/media_meta_test.go @@ -436,7 +436,10 @@ func TestMergedMediaMeta_MergesSingletonAliasMetadata(t *testing.T) { mockDB.On("GetMediaPropertyMetadataByMediaDBIDs", mock.Anything, []int64{20, 10}). Return(map[int64][]database.MediaProperty{ 20: {{TypeTag: "property:description", Text: "child"}}, - 10: {{TypeTag: "property:description", Text: "parent"}, {TypeTag: "property:image-boxart", Text: "box.png"}}, + 10: { + {TypeTag: "property:description", Text: "parent"}, + {TypeTag: "property:image-boxart", Text: "box.png"}, + }, }, nil).Once() env := &requests.RequestEnv{ From ecc17685b0fef7f46cef76904b64ad04445b340e Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Fri, 29 May 2026 15:46:37 +0800 Subject: [PATCH 7/7] test(media): cover singleton alias lookup paths --- pkg/api/methods/media_batch_test.go | 48 ++++++++++++++++++ pkg/api/methods/media_image_test.go | 27 ++++++++++ pkg/api/methods/media_meta_test.go | 64 ++++++++++++++++++++++++ pkg/database/mediadb/sql_scraper_test.go | 33 ++++++++++++ 4 files changed, 172 insertions(+) diff --git a/pkg/api/methods/media_batch_test.go b/pkg/api/methods/media_batch_test.go index bddd0943..d805c7cb 100644 --- a/pkg/api/methods/media_batch_test.go +++ b/pkg/api/methods/media_batch_test.go @@ -20,10 +20,18 @@ package methods import ( + "context" "encoding/json" + "path/filepath" "testing" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" + testhelpers "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -59,3 +67,43 @@ func TestParseMediaRequest_RejectsTopLevelImageTypesInBatch(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "imageTypes entries must be non-empty") } + +func TestResolveMediaRefs_UsesSingletonFallbackForBatchPath(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + platform := mocks.NewMockPlatform() + platform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}).Once() + + system := database.System{DBID: 1, SystemID: "NES", Name: "NES"} + containerPath := filepath.ToSlash(filepath.Join("roms", "Game.zip")) + childPath := filepath.ToSlash(filepath.Join(containerPath, "Game.nes")) + media := database.Media{DBID: 20, Path: childPath} + row := database.MediaFullRow{ + Media: media, + Title: database.MediaTitle{DBID: 30, Name: "Game"}, + System: system, + } + + mockDB.On("FindSystemBySystemID", "NES").Return(system, nil).Once() + mockDB.On("FindMediaBySystemAndPaths", mock.Anything, system.DBID, []string{containerPath}). + Return(map[string]database.Media{}, nil).Once() + mockDB.On("FindSingleDescendantMedia", mock.Anything, system.DBID, containerPath). + Return(&media, nil).Once() + mockDB.On("GetMediaWithTitleAndSystemByIDs", mock.Anything, []int64{media.DBID}). + Return(map[int64]database.MediaFullRow{media.DBID: row}, nil).Once() + + env := &requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{MediaDB: mockDB}, + Platform: platform, + } + resolved, err := resolveMediaRefs(env, []mediaRefParam{{System: "NES", Path: containerPath}}) + require.NoError(t, err) + require.Len(t, resolved, 1) + require.NoError(t, resolved[0].Err) + require.NotNil(t, resolved[0].Row) + assert.Equal(t, childPath, resolved[0].Row.Path) + mockDB.AssertExpectations(t) + platform.AssertExpectations(t) +} diff --git a/pkg/api/methods/media_image_test.go b/pkg/api/methods/media_image_test.go index 209c9d12..83c2245e 100644 --- a/pkg/api/methods/media_image_test.go +++ b/pkg/api/methods/media_image_test.go @@ -437,6 +437,33 @@ func TestHandleMediaImage_MediaNotFound(t *testing.T) { mockDB.AssertExpectations(t) } +func TestHandleMediaImage_MediaIDSuccess(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + blobData := []byte("media-id-image") + row := makeMediaFullRow(44, 440) + + mockDB.On("GetMediaWithTitleAndSystemByIDs", mock.Anything, []int64{row.DBID}). + Return(map[int64]database.MediaFullRow{row.DBID: *row}, nil).Once() + mockDB.On("GetMediaProperties", mock.Anything, row.DBID). + Return([]database.MediaProperty{ + {TypeTag: "property:image-boxart", ContentType: "image/png", Binary: blobData}, + }, nil).Once() + mockDB.On("GetMediaTitleProperties", mock.Anything, row.Title.DBID). + Return([]database.MediaProperty{}, nil).Once() + + env := makeMediaImageEnv(t, mockDB, json.RawMessage(`{"mediaId":44,"imageTypes":["boxart"]}`)) + result, err := HandleMediaImage(env) + require.NoError(t, err) + + resp, ok := result.(models.MediaImageResponse) + require.True(t, ok) + assert.Equal(t, "property:image-boxart", resp.TypeTag) + assert.Equal(t, base64.StdEncoding.EncodeToString(blobData), resp.Data) + mockDB.AssertExpectations(t) +} + func TestHandleMediaImage_ItemsParamReturnsClientError(t *testing.T) { t.Parallel() diff --git a/pkg/api/methods/media_meta_test.go b/pkg/api/methods/media_meta_test.go index 0ca0b1fc..6e5447cf 100644 --- a/pkg/api/methods/media_meta_test.go +++ b/pkg/api/methods/media_meta_test.go @@ -404,6 +404,70 @@ func TestHandleMediaMeta_BatchAllMediaIDMissesSkipMetadataFetch(t *testing.T) { mockDB.AssertExpectations(t) } +func TestHandleMediaMeta_MediaIDMergesSingletonAliasMetadata(t *testing.T) { + t.Parallel() + + mockDB := testhelpers.NewMockMediaDBI() + platform := mocks.NewMockPlatform() + platform.On("Settings").Return(platforms.Settings{ZipsAsDirs: true}).Twice() + + row := &database.MediaFullRow{ + Media: database.Media{ + DBID: 20, + Path: filepath.ToSlash(filepath.Join("roms", "Game.zip", "Game.nes")), + ParentDir: filepath.ToSlash(filepath.Join("roms", "Game.zip")) + "/", + }, + Title: database.MediaTitle{DBID: 200, Name: "Game"}, + System: database.System{DBID: 1, SystemID: "NES", Name: "NES"}, + } + parentPath := filepath.ToSlash(filepath.Join("roms", "Game.zip")) + parent := &database.Media{DBID: 10, Path: parentPath} + + mockDB.On("GetMediaWithTitleAndSystemByIDs", mock.Anything, []int64{row.DBID}). + Return(map[int64]database.MediaFullRow{row.DBID: *row}, nil).Once() + mockDB.On("GetMediaTagsByMediaDBIDs", mock.Anything, []int64{row.DBID}). + Return(map[int64][]database.TagInfo{row.DBID: {{Type: "genre", Tag: "platformer"}}}, nil).Once() + mockDB.On("GetMediaTitleTagsByMediaTitleDBIDs", mock.Anything, []int64{row.Title.DBID}). + Return(map[int64][]database.TagInfo{}, nil).Once() + mockDB.On("GetMediaPropertyMetadataByMediaDBIDs", mock.Anything, []int64{row.DBID}). + Return(map[int64][]database.MediaProperty{row.DBID: {{TypeTag: "property:description", Text: "child"}}}, nil). + Once() + mockDB.On("GetMediaTitlePropertyMetadataByMediaTitleDBIDs", mock.Anything, []int64{row.Title.DBID}). + Return(map[int64][]database.MediaProperty{}, nil).Once() + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, row.Path). + Return((*database.Media)(nil), nil).Twice() + mockDB.On("FindSingleDescendantMedia", mock.Anything, row.System.DBID, parentPath). + Return(&row.Media, nil).Twice() + mockDB.On("FindMediaBySystemAndPath", mock.Anything, row.System.DBID, parentPath). + Return(parent, nil).Twice() + mockDB.On("GetMediaTagsByMediaDBIDs", mock.Anything, []int64{20, 10}).Return(map[int64][]database.TagInfo{ + 20: {{Type: "genre", Tag: "platformer"}}, + 10: {{Type: "favorite", Tag: "true"}}, + }, nil).Once() + mockDB.On("GetMediaPropertyMetadataByMediaDBIDs", mock.Anything, []int64{20, 10}). + Return(map[int64][]database.MediaProperty{ + 20: {{TypeTag: "property:description", Text: "child"}}, + 10: {{TypeTag: "property:image-boxart", Text: "box.png"}}, + }, nil).Once() + + env := makeMediaMetaEnv(t, mockDB, `{"mediaId":20}`) + env.Platform = platform + result, err := HandleMediaMeta(env) + require.NoError(t, err) + + resp, ok := result.(models.MediaMetaResponse) + require.True(t, ok) + assert.Equal(t, row.Path, resp.Media.Path) + assert.Equal(t, []database.TagInfo{ + {Type: "genre", Tag: "platformer"}, + {Type: "favorite", Tag: "true"}, + }, resp.Media.Tags) + assert.Equal(t, "child", resp.Media.Properties["property:description"].Text) + assert.Equal(t, "box.png", resp.Media.Properties["property:image-boxart"].Text) + mockDB.AssertExpectations(t) + platform.AssertExpectations(t) +} + func TestMergedMediaMeta_MergesSingletonAliasMetadata(t *testing.T) { t.Parallel() diff --git a/pkg/database/mediadb/sql_scraper_test.go b/pkg/database/mediadb/sql_scraper_test.go index af7e7989..9e74ffee 100644 --- a/pkg/database/mediadb/sql_scraper_test.go +++ b/pkg/database/mediadb/sql_scraper_test.go @@ -95,6 +95,39 @@ func TestFindMediaBySystemAndPath_WrongSystem(t *testing.T) { assert.Nil(t, m, "path exists but systemDBID doesn't match") } +func TestFindMediaBySystemAndPaths_ReturnsMatchesByPath(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupScraperTestDB(t) + defer cleanup() + ctx := context.Background() + + marioPath := filepath.ToSlash(filepath.Join("roms", "mario.nes")) + zeldaPath := filepath.ToSlash(filepath.Join("roms", "zelda.nes")) + missingPath := filepath.ToSlash(filepath.Join("roms", "missing.nes")) + _, err := mediaDB.sql.ExecContext(ctx, ` + INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES (2, 1, 'zelda', 'Zelda'); + INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path) VALUES (2, 2, 1, ?); + `, zeldaPath) + require.NoError(t, err) + + results, err := mediaDB.FindMediaBySystemAndPaths(ctx, 1, []string{marioPath, zeldaPath, missingPath}) + require.NoError(t, err) + require.Len(t, results, 2) + assert.Equal(t, int64(1), results[marioPath].DBID) + assert.Equal(t, int64(2), results[zeldaPath].DBID) + assert.NotContains(t, results, missingPath) +} + +func TestFindMediaBySystemAndPaths_EmptyInput(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupScraperTestDB(t) + defer cleanup() + + results, err := mediaDB.FindMediaBySystemAndPaths(context.Background(), 1, nil) + require.NoError(t, err) + assert.Empty(t, results) +} + func TestFindSingleDescendantMedia_ReturnsOnlyChild(t *testing.T) { t.Parallel() mediaDB, cleanup := setupScraperTestDB(t)