From d18f94e28a209f80478553618f4016dfefe44a23 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 05:27:22 +0800 Subject: [PATCH 01/11] feat(api): add local-path image delivery Let clients request Core-owned cached thumbnail paths while preserving inline delivery as the default. Validate returned cache artifacts and fall back to inline data when materialization fails. Inspired by Giancarlo Erra's MiSTer artwork performance work and measurements in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra --- docs/api/methods.md | 44 ++++- pkg/api/methods/media_image.go | 258 ++++++++++++++++++++------- pkg/api/methods/media_image_test.go | 183 +++++++++++++++++-- pkg/api/methods/media_scrape_test.go | 8 +- pkg/api/models/params.go | 1 + pkg/api/models/responses.go | 8 +- 6 files changed, 411 insertions(+), 91 deletions(-) diff --git a/docs/api/methods.md b/docs/api/methods.md index 02c0dbf20..abba4ce89 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -1895,7 +1895,7 @@ An object identifying the media row by `mediaId` or by `system` and canonical `p **Access:** All clients. -Return the best matching image for one indexed media row as base64-encoded data. +Return the best matching image for one indexed media row. Inline base64 delivery remains default. Clients can explicitly request a transient path to a Core-owned cached thumbnail. `media.image` checks the requested image types in order. For each type it tries media-level properties first, then title-level properties. If a stored file path no longer exists, the stale property is removed and lookup continues. @@ -1909,19 +1909,24 @@ An object identifying the media row by `mediaId` or `(system, path)`. Canonical | system | string | No | System ID. Required when `mediaId` is omitted. | | path | string | No | Canonical indexed media path. Required when `mediaId` is omitted. | | imageTypes | string[] | No | Image type preference order. Defaults to `image`, `thumbnail`, `boxart`, `boxart3d`, `screenshot`, `wheel`, `titleshot`, `map`, `marquee`, `fanart`. | -| maxSize | number | No | Longest-edge size hint in pixels. When set, the server resizes the image to fit a `maxSize`×`maxSize` box and caches the result; omit it for the full-size image. | +| maxSize | number | No | Longest-edge size hint in pixels. When set, the server resizes the image to fit a `maxSize`×`maxSize` box and caches the result; omit it for the full-size image. Required for `localPath` delivery. | +| delivery | string | No | `inline` (default) or `localPath`. `localPath` requires a positive `maxSize` and returns a path on the Core host. | Supported image type values are `image`, `thumbnail`, `boxart`, `boxart3d`, `screenshot`, `wheel`, `titleshot`, `map`, `marquee`, and `fanart`. They resolve to canonical property tags such as `property:image-image` and `property:image-boxart`. Resizing is intended for grid and preview views where transferring and holding full-size art is expensive. `maxSize` is snapped up to the nearest of a small set of standard tiers (`32`, `64`, `128`, `256`, `512`, `768`) server-side. The returned image is **never larger than the snapped tier and never larger than the source** — when the source already fits the tier it is returned at its native dimensions, so the result may still be larger than the exact `maxSize` you asked for. Request your true display size (logical size × pixel ratio) and downscale to the final size on the client. The snapped tiers bound how many resized variants are cached per image. Output is re-encoded as WebP (lossy, alpha preserved) regardless of source format — including when the source already fits the box, so even a near-native request still gets the smaller WebP — and cached on disk so repeat requests are cheap. The original bytes are kept only when WebP would not shrink them (already-compact sources), when `maxSize` is omitted/non-positive (full size), or when the source cannot be decoded. +`localPath` never returns an original scraper or media path. Core resolves image semantics, materializes its own bounded thumbnail cache artifact, and returns that path. Path delivery is available to any client that explicitly requests it, regardless of peer locality or Core platform; remote callers are responsible for having an appropriate shared-filesystem view of the Core host path. Treat the path as opaque, transient, and nonportable: read it immediately, never persist it or derive neighboring paths, and retry once with `delivery: "inline"` if the file is inaccessible or disappears before it is opened. If cache materialization fails, Core can safely return `delivery: "inline"` in the same response. + #### Result | Key | Type | Required | Description | | :---------- | :----- | :------- | :------------------------------------------- | +| delivery | string | Yes | Actual delivery used: `inline` or `localPath`. Clients must inspect this field because a requested local path can fall back inline. | | contentType | string | Yes | MIME type of the returned image data. | | extension | string | No | File extension without a dot, derived from MIME type or source path. | -| data | string | Yes | Base64-encoded image bytes. | +| data | string | No | Base64-encoded image bytes. Present for `inline` delivery. | +| localPath | string | No | Absolute, opaque Core-host path to a cached thumbnail. Present for `localPath` delivery. | | typeTag | string | Yes | Canonical property tag that matched. | #### Example @@ -1949,6 +1954,7 @@ Resizing is intended for grid and preview views where transferring and holding f "jsonrpc": "2.0", "id": "e5f6a7b8-7a5d-11ef-9c7b-020304050607", "result": { + "delivery": "inline", "contentType": "image/webp", "extension": "webp", "data": "UklGRiQAAABXRUJQVlA4...", @@ -1957,6 +1963,38 @@ Resizing is intended for grid and preview views where transferring and holding f } ``` +##### Local-path request + +```json +{ + "jsonrpc": "2.0", + "id": "e5f6a7b8-7a5d-11ef-9c7b-020304050607", + "method": "media.image", + "params": { + "mediaId": 123, + "imageTypes": ["boxart"], + "maxSize": 256, + "delivery": "localPath" + } +} +``` + +##### Local-path response + +```json +{ + "jsonrpc": "2.0", + "id": "e5f6a7b8-7a5d-11ef-9c7b-020304050607", + "result": { + "delivery": "localPath", + "contentType": "image/webp", + "extension": "webp", + "localPath": "/media/fat/zaparoo/cache/thumbs/v2/U05FUw/example.webp", + "typeTag": "property:image-boxart" + } +} +``` + ### scrapers **Access:** All clients. diff --git a/pkg/api/methods/media_image.go b/pkg/api/methods/media_image.go index 0933a6545..ff848a2cd 100644 --- a/pkg/api/methods/media_image.go +++ b/pkg/api/methods/media_image.go @@ -59,6 +59,8 @@ const ( misterMediaImageMaxBytes = int64(2 * 1024 * 1024) defaultMediaImageMaxBytes = int64(8 * 1024 * 1024) mediaImageNoImageMax = 4096 + mediaImageDeliveryInline = "inline" + mediaImageDeliveryPath = "localPath" // mediaThumbCacheDirName is the sub-directory under the core cache dir where // resized thumbnail files are persisted across restarts. mediaThumbCacheDirName = "thumbs" @@ -320,43 +322,94 @@ func thumbCacheExtension(contentType string, data []byte) string { return "" } -// get looks up a cached resized image. Returns (data, contentType, true) on -// hit, or (nil, "", false) on miss or any I/O error. -func (c *mediaThumbCache) get( +func thumbCacheFileInfo(fs afero.Fs, path string) (os.FileInfo, error) { + if lstater, ok := fs.(afero.Lstater); ok { + info, _, err := lstater.LstatIfPossible(path) + if err != nil { + return nil, fmt.Errorf("lstat thumbnail cache file: %w", err) + } + return info, nil + } + info, err := fs.Stat(path) + if err != nil { + return nil, fmt.Errorf("stat thumbnail cache file: %w", err) + } + return info, nil +} + +// lookupPath returns a regular cached thumbnail path without reading its bytes. +func (c *mediaThumbCache) lookupPath( ref mediaRefParam, system, typeTag string, maxSize int, -) (data []byte, contentType string, found bool) { +) (path, contentType string, found bool) { hash := hashThumbKey(thumbKey(ref, typeTag, maxSize)) for _, format := range thumbCacheFormats { //nolint:gosec // path is constructed from controlled dirs + SHA-256 hash + fixed extension - b, err := afero.ReadFile(c.fs, filepath.Join(c.systemDir(system), hash+format.ext)) - if err == nil { - return b, format.contentType, true + candidate := filepath.Join(c.systemDir(system), hash+format.ext) + info, err := thumbCacheFileInfo(c.fs, candidate) + if err == nil && info.Mode().IsRegular() { + return candidate, format.contentType, true } } - return nil, "", false + return "", "", false +} + +// read returns cached thumbnail bytes. A deletion between lookup and read is a miss. +func (c *mediaThumbCache) read( + ref mediaRefParam, system, typeTag string, maxSize int, +) (data []byte, contentType string, found bool) { + path, contentType, found := c.lookupPath(ref, system, typeTag, maxSize) + if !found { + return nil, "", false + } + //nolint:gosec // lookupPath only returns controlled cache paths. + data, err := afero.ReadFile(c.fs, path) + if err != nil { + return nil, "", false + } + return data, contentType, true } -// set writes resized image bytes to the disk cache through a same-directory -// temporary file and atomic rename. Failures are logged and ignored — the -// caller always has the bytes in memory and must not fail on a cache write. +// get keeps cache-focused tests concise; production response paths use lookupPath or read explicitly. +func (c *mediaThumbCache) get( + ref mediaRefParam, system, typeTag string, maxSize int, +) (data []byte, contentType string, found bool) { + return c.read(ref, system, typeTag, maxSize) +} + +// isSafeLocalPath proves a returned cache path is absolute, contained by the +// versioned thumbnail cache, and still names a regular non-symlink file. +func (c *mediaThumbCache) isSafeLocalPath(path string) bool { + cleanDir := filepath.Clean(c.dir) + cleanPath := filepath.Clean(path) + if !filepath.IsAbs(cleanDir) || !filepath.IsAbs(cleanPath) { + return false + } + rel, err := filepath.Rel(cleanDir, cleanPath) + if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + return false + } + info, err := thumbCacheFileInfo(c.fs, cleanPath) + return err == nil && info.Mode().IsRegular() +} + +// set writes resized image bytes through a same-directory temporary file and +// atomic rename. The resulting path is returned only after materialization. func (c *mediaThumbCache) set( ref mediaRefParam, system, typeTag string, maxSize int, data []byte, contentType string, -) { +) (string, error) { ext := thumbCacheExtension(contentType, data) if ext == "" { - return + return "", fmt.Errorf("unsupported thumbnail content type %q", contentType) } dir := c.systemDir(system) if err := c.fs.MkdirAll(dir, 0o750); err != nil { //nolint:gosec // cache dir, 0o750 is intentional - log.Debug().Err(err).Str("dir", dir).Msg("media.image: thumb cache: failed to create dir") - return + return "", fmt.Errorf("create thumbnail cache directory: %w", err) } hash := hashThumbKey(thumbKey(ref, typeTag, maxSize)) path := filepath.Join(dir, hash+ext) tmp, err := afero.TempFile(c.fs, dir, ".thumb-*.tmp") if err != nil { - log.Debug().Err(err).Str("dir", dir).Msg("media.image: thumb cache: failed to create temporary file") - return + return "", fmt.Errorf("create thumbnail cache temporary file: %w", err) } tmpPath := tmp.Name() defer func() { @@ -364,20 +417,18 @@ func (c *mediaThumbCache) set( _ = c.fs.Remove(tmpPath) }() if err := c.fs.Chmod(tmpPath, 0o600); err != nil { //nolint:gosec // cache file, 0o600 is intentional - log.Debug().Err(err).Str("path", tmpPath).Msg("media.image: thumb cache: failed to set permissions") - return + return "", fmt.Errorf("set thumbnail cache file permissions: %w", err) } if _, err := tmp.Write(data); err != nil { - log.Debug().Err(err).Str("path", tmpPath).Msg("media.image: thumb cache: failed to write temporary file") - return + return "", fmt.Errorf("write thumbnail cache temporary file: %w", err) } if err := tmp.Close(); err != nil { - log.Debug().Err(err).Str("path", tmpPath).Msg("media.image: thumb cache: failed to close temporary file") - return + return "", fmt.Errorf("close thumbnail cache temporary file: %w", err) } if err := c.fs.Rename(tmpPath, path); err != nil { - log.Debug().Err(err).Str("path", path).Msg("media.image: thumb cache: failed to replace file") + return "", fmt.Errorf("replace thumbnail cache file: %w", err) } + return path, nil } // wipe empties the thumbnail cache and clears the in-memory resolved-type memo. @@ -782,13 +833,82 @@ func imagePropertyTypeTags(props []database.MediaProperty) []string { return result } -// HandleMediaImage returns a single best-match image for a media record as a -// base64-encoded blob. +func inlineMediaImageResponse(data []byte, contentType, sourcePath, typeTag string) models.MediaImageResponse { + return models.MediaImageResponse{ + Extension: mediaContentExtension(contentType, sourcePath), + ContentType: contentType, + Data: base64.StdEncoding.EncodeToString(data), + Delivery: mediaImageDeliveryInline, + TypeTag: typeTag, + } +} + +func localMediaImageResponse( + cache *mediaThumbCache, path, contentType, typeTag string, +) (models.MediaImageResponse, bool) { + if !cache.isSafeLocalPath(path) { + return models.MediaImageResponse{}, false + } + return models.MediaImageResponse{ + Extension: mediaContentExtension(contentType, ""), + ContentType: contentType, + Delivery: mediaImageDeliveryPath, + LocalPath: path, + TypeTag: typeTag, + }, true +} + +func cachedMediaImageResponse( + cache *mediaThumbCache, + ref mediaRefParam, + system string, + typeTag string, + maxSize int, + sourcePath string, + localPath bool, +) (models.MediaImageResponse, bool) { + path, contentType, found := cache.lookupPath(ref, system, typeTag, maxSize) + if !found { + return models.MediaImageResponse{}, false + } + if localPath { + if response, ok := localMediaImageResponse(cache, path, contentType, typeTag); ok { + return response, true + } + } + data, contentType, found := cache.read(ref, system, typeTag, maxSize) + if !found { + return models.MediaImageResponse{}, false + } + return inlineMediaImageResponse(data, contentType, sourcePath, typeTag), true +} + +func validateMediaImageDelivery(delivery string, ref mediaRefParam) error { + switch delivery { + case "", mediaImageDeliveryInline: + return nil + case mediaImageDeliveryPath: + if ref.MaxSize == nil || *ref.MaxSize <= 0 { + return models.ClientErrf("media.image: localPath delivery requires a positive maxSize") + } + return nil + default: + return models.ClientErrf("media.image: unsupported delivery %q", delivery) + } +} + +// HandleMediaImage returns a single best-match image inline or as a transient, +// Core-owned cached thumbnail path when explicitly requested. func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic // single-use parameter in API handler - ref, err := parseMediaImageRequest(env.Params) + ref, delivery, err := parseMediaImageRequest(env.Params) if err != nil { return nil, err } + if deliveryErr := validateMediaImageDelivery(delivery, ref); deliveryErr != nil { + return nil, deliveryErr + } + localPath := delivery == mediaImageDeliveryPath + // Snap the requested size onto a standard tier so every view shares one // cached image per cover and the resize dimension is bounded. All downstream // uses (cache keys, resolved-type memo, resize) see the snapped value. @@ -799,19 +919,15 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic prefs := imagePrefs(nil, ref.ImageTypes) // Pre-semaphore fast path: if this exact request was served before and the - // disk thumbnail is still present, return it without ever acquiring the - // semaphore or loading the original full-size image from disk. + // disk thumbnail is still present, return it without loading the original. if ref.MaxSize != nil && *ref.MaxSize > 0 { maxSize := int(*ref.MaxSize) - if tc := mediaThumbCachePointer.Load(); tc != nil { - if resolved, ok := tc.getResolvedThumb(ref, prefs, maxSize); ok { - if cached, cachedCT, cacheHit := tc.get(ref, resolved.system, resolved.typeTag, maxSize); cacheHit { - return models.MediaImageResponse{ - Extension: mediaContentExtension(cachedCT, ""), - ContentType: cachedCT, - Data: base64.StdEncoding.EncodeToString(cached), - TypeTag: resolved.typeTag, - }, nil + if cache := mediaThumbCachePointer.Load(); cache != nil { + if resolved, ok := cache.getResolvedThumb(ref, prefs, maxSize); ok { + if response, found := cachedMediaImageResponse( + cache, ref, resolved.system, resolved.typeTag, maxSize, "", localPath, + ); found { + return response, nil } } } @@ -853,54 +969,58 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic return nil, err } - binary, ct := raw.binary, raw.contentType + binary, contentType := raw.binary, raw.contentType if ref.MaxSize != nil && *ref.MaxSize > 0 { maxSize := int(*ref.MaxSize) - tc := mediaThumbCachePointer.Load() + cache := mediaThumbCachePointer.Load() // Record the resolved typeTag so the pre-semaphore path can find the // disk file on the next request without loading the original image. - if tc != nil { - tc.setResolvedThumb(ref, prefs, maxSize, raw.system, raw.typeTag) - } - // Check the disk thumb cache before doing the expensive decode+resize. - if tc != nil { - if cached, cachedCT, ok := tc.get(ref, raw.system, raw.typeTag, maxSize); ok { - return models.MediaImageResponse{ - Extension: mediaContentExtension(cachedCT, raw.text), - ContentType: cachedCT, - Data: base64.StdEncoding.EncodeToString(cached), - TypeTag: raw.typeTag, - }, nil + if cache != nil { + cache.setResolvedThumb(ref, prefs, maxSize, raw.system, raw.typeTag) + if response, found := cachedMediaImageResponse( + cache, ref, raw.system, raw.typeTag, maxSize, raw.text, localPath, + ); found { + return response, nil } } - binary, ct = resizeImageIfNeeded(binary, ct, maxSize) - // Write to the disk cache on a miss so future requests skip the resize. - if tc != nil { - tc.set(ref, raw.system, raw.typeTag, maxSize, binary, ct) + + binary, contentType = resizeImageIfNeeded(binary, contentType, maxSize) + if cache != nil { + path, cacheErr := cache.set(ref, raw.system, raw.typeTag, maxSize, binary, contentType) + if cacheErr != nil { + log.Debug().Err(cacheErr).Msg("media.image: failed to materialize thumbnail cache file") + } else if localPath { + if response, ok := localMediaImageResponse(cache, path, contentType, raw.typeTag); ok { + return response, nil + } + log.Debug().Msg("media.image: thumbnail cache path failed local delivery validation") + } } } - return models.MediaImageResponse{ - Extension: mediaContentExtension(ct, raw.text), - ContentType: ct, - Data: base64.StdEncoding.EncodeToString(binary), - TypeTag: raw.typeTag, - }, nil + return inlineMediaImageResponse(binary, contentType, raw.text, raw.typeTag), nil } -func parseMediaImageRequest(raw json.RawMessage) (mediaRefParam, error) { - var ref mediaRefParam +func parseMediaImageRequest(raw json.RawMessage) (mediaRefParam, string, error) { + var params models.MediaImageParams decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() - if err := decoder.Decode(&ref); err != nil { - return mediaRefParam{}, models.ClientErrf("invalid params: %w", err) + if err := decoder.Decode(¶ms); err != nil { + return mediaRefParam{}, "", models.ClientErrf("invalid params: %w", err) + } + ref := mediaRefParam{ + MediaID: params.MediaID, MaxSize: params.MaxSize, System: params.System, + Path: params.Path, ImageTypes: params.ImageTypes, } if err := validateMediaRef(ref); err != nil { - return mediaRefParam{}, models.ClientErrf("invalid params: %w", err) + return mediaRefParam{}, "", models.ClientErrf("invalid params: %w", err) } if err := validateImageTypes(ref.ImageTypes); err != nil { - return mediaRefParam{}, models.ClientErrf("invalid params: %w", err) + return mediaRefParam{}, "", models.ClientErrf("invalid params: %w", err) + } + if ref.MaxSize != nil && (*ref.MaxSize <= 0 || *ref.MaxSize > 8192) { + return mediaRefParam{}, "", models.ClientErrf("invalid params: maxSize must be between 1 and 8192") } - return ref, nil + return ref, params.Delivery, nil } func loadRawMediaImageSinglePath( diff --git a/pkg/api/methods/media_image_test.go b/pkg/api/methods/media_image_test.go index 3b45ef95f..a2b9551e7 100644 --- a/pkg/api/methods/media_image_test.go +++ b/pkg/api/methods/media_image_test.go @@ -93,6 +93,21 @@ func mediaImageParams(row *database.MediaFullRow, extra string) json.RawMessage return json.RawMessage(fmt.Sprintf(`{"system": %q, "path": %q%s}`, row.System.SystemID, row.Path, extra)) } +func setMediaThumbCacheForTest( + t testing.TB, + cache *mediaThumbCache, + ref mediaRefParam, + system string, + typeTag string, + maxSize int, + data []byte, + contentType string, +) { + t.Helper() + _, err := cache.set(ref, system, typeTag, maxSize, data, contentType) + require.NoError(t, err) +} + func TestMediaThumbCache_GetSetAndWipe(t *testing.T) { t.Parallel() @@ -108,7 +123,7 @@ func TestMediaThumbCache_GetSetAndWipe(t *testing.T) { _, _, found := cache.get(ref, "SNES", "property:image-boxart", 100) assert.False(t, found) - cache.set(ref, "SNES", "property:image-boxart", 100, []byte("png-data"), "image/png") + setMediaThumbCacheForTest(t, cache, ref, "SNES", "property:image-boxart", 100, []byte("png-data"), "image/png") data, contentType, found := cache.get(ref, "SNES", "property:image-boxart", 100) require.True(t, found) assert.Equal(t, []byte("png-data"), data) @@ -135,11 +150,37 @@ func TestMediaThumbCache_SkipsUnsupportedContentType(t *testing.T) { mediaID := int64(1) ref := mediaRefParam{MediaID: &mediaID} - cache.set(ref, "SNES", "property:image-boxart", 100, []byte("not an image"), "text/plain") + _, err := cache.set(ref, "SNES", "property:image-boxart", 100, []byte("not an image"), "text/plain") + require.Error(t, err) _, _, found := cache.get(ref, "SNES", "property:image-boxart", 100) assert.False(t, found) } +func TestMediaThumbCache_IsSafeLocalPath(t *testing.T) { + t.Parallel() + + fs := afero.NewOsFs() + cache := &mediaThumbCache{ + fs: fs, dir: filepath.Join(t.TempDir(), mediaThumbCacheVersionDir()), + resolvedTypes: make(map[string]resolvedThumb), + } + require.NoError(t, fs.MkdirAll(cache.dir, 0o750)) + inside := filepath.Join(cache.dir, "inside.webp") + require.NoError(t, afero.WriteFile(fs, inside, []byte("inside"), 0o600)) + assert.True(t, cache.isSafeLocalPath(inside)) + + outside := filepath.Join(t.TempDir(), "outside.webp") + require.NoError(t, afero.WriteFile(fs, outside, []byte("outside"), 0o600)) + assert.False(t, cache.isSafeLocalPath(outside)) + + symlink := filepath.Join(cache.dir, "link.webp") + require.NoError(t, os.Symlink(outside, symlink)) + assert.False(t, cache.isSafeLocalPath(symlink)) + + relativeCache := &mediaThumbCache{fs: afero.NewMemMapFs(), dir: "relative"} + assert.False(t, relativeCache.isSafeLocalPath(filepath.Join("relative", "image.webp"))) +} + func TestWipeMediaThumbCache_EmptiesLiveDirInPlace(t *testing.T) { fs := afero.NewMemMapFs() thumbs := filepath.Join("cache", "thumbs") @@ -150,7 +191,7 @@ func TestWipeMediaThumbCache_EmptiesLiveDirInPlace(t *testing.T) { mediaID := int64(1) ref := mediaRefParam{MediaID: &mediaID} - cache.set(ref, "SNES", "property:image-boxart", 512, []byte("webp-bytes"), "image/webp") + setMediaThumbCacheForTest(t, cache, ref, "SNES", "property:image-boxart", 512, []byte("webp-bytes"), "image/webp") cache.setResolvedThumb(ref, nil, 512, "SNES", "property:image-boxart") _, _, found := cache.get(ref, "SNES", "property:image-boxart", 512) require.True(t, found) @@ -191,8 +232,10 @@ func TestWipeMediaThumbCacheSystems_PreservesOtherSystems(t *testing.T) { snesID, genesisID := int64(1), int64(2) snesRef := mediaRefParam{MediaID: &snesID} genesisRef := mediaRefParam{MediaID: &genesisID} - cache.set(snesRef, "SNES", "property:image-boxart", 512, []byte("snes"), "image/webp") - cache.set(genesisRef, "Genesis", "property:image-boxart", 512, []byte("genesis"), "image/webp") + setMediaThumbCacheForTest(t, cache, snesRef, "SNES", "property:image-boxart", 512, []byte("snes"), "image/webp") + setMediaThumbCacheForTest( + t, cache, genesisRef, "Genesis", "property:image-boxart", 512, []byte("genesis"), "image/webp", + ) cache.setResolvedThumb(snesRef, nil, 512, "SNES", "property:image-boxart") cache.setResolvedThumb(genesisRef, nil, 512, "Genesis", "property:image-boxart") @@ -221,8 +264,10 @@ func TestInvalidateIndexedThumbnails_SelectiveSystems(t *testing.T) { snesID, genesisID := int64(1), int64(2) snesRef := mediaRefParam{MediaID: &snesID} genesisRef := mediaRefParam{MediaID: &genesisID} - cache.set(snesRef, "SNES", "property:image-boxart", 256, []byte("snes"), "image/webp") - cache.set(genesisRef, "Genesis", "property:image-boxart", 256, []byte("genesis"), "image/webp") + setMediaThumbCacheForTest(t, cache, snesRef, "SNES", "property:image-boxart", 256, []byte("snes"), "image/webp") + setMediaThumbCacheForTest( + t, cache, genesisRef, "Genesis", "property:image-boxart", 256, []byte("genesis"), "image/webp", + ) invalidateIndexedThumbnails([]systemdefs.System{{ID: "SNES"}}, false) @@ -256,8 +301,12 @@ func TestInvalidateIndexedThumbnails_FullCache(t *testing.T) { snesID, genesisID := int64(1), int64(2) snesRef := mediaRefParam{MediaID: &snesID} genesisRef := mediaRefParam{MediaID: &genesisID} - cache.set(snesRef, "SNES", "property:image-boxart", 256, []byte("snes"), "image/webp") - cache.set(genesisRef, "Genesis", "property:image-boxart", 256, []byte("genesis"), "image/webp") + setMediaThumbCacheForTest( + t, cache, snesRef, "SNES", "property:image-boxart", 256, []byte("snes"), "image/webp", + ) + setMediaThumbCacheForTest( + t, cache, genesisRef, "Genesis", "property:image-boxart", 256, []byte("genesis"), "image/webp", + ) invalidateIndexedThumbnails(tt.systems, tt.rebuild) @@ -657,6 +706,116 @@ func TestHandleMediaImage_MaxSizeResizesAndCachesThumbnail(t *testing.T) { strictDB.AssertExpectations(t) } +func expectInlineImageProperty(mockDB *testhelpers.MockMediaDBI, row *database.MediaFullRow, data []byte) { + expectMediaImageResolve(mockDB, row) + mockDB.On("GetMediaProperties", mock.Anything, row.DBID). + Return([]database.MediaProperty{}, nil) + mockDB.On("GetMediaTitleProperties", mock.Anything, row.Title.DBID). + Return([]database.MediaProperty{ + {TypeTag: "property:image-boxart", ContentType: "image/png", Binary: data}, + }, nil) +} + +func TestHandleMediaImage_LocalPathColdAndWarm(t *testing.T) { + // Not parallel: installs process-wide thumbnail cache pointer. + cache := &mediaThumbCache{ + fs: afero.NewOsFs(), dir: filepath.Join(t.TempDir(), mediaThumbCacheVersionDir()), + resolvedTypes: make(map[string]resolvedThumb), + } + mediaThumbCachePointer.Store(cache) + t.Cleanup(func() { mediaThumbCachePointer.Store(nil) }) + + row := makeMediaFullRow(9001, 9010) + mockDB := testhelpers.NewMockMediaDBI() + expectInlineImageProperty(mockDB, row, []byte("cached-image")) + env := makeMediaImageEnv(t, mockDB, mediaImageParams( + row, `"maxSize": 256, "delivery": "localPath"`, + )) + // Path delivery is explicit and independent of peer locality or platform. + require.False(t, env.IsLocal) + require.Nil(t, env.Platform) + + result, err := HandleMediaImage(env) + require.NoError(t, err) + resp, ok := result.(models.MediaImageResponse) + require.True(t, ok) + assert.Equal(t, mediaImageDeliveryPath, resp.Delivery) + assert.Empty(t, resp.Data) + assert.True(t, filepath.IsAbs(resp.LocalPath)) + assert.True(t, cache.isSafeLocalPath(resp.LocalPath)) + cached, err := afero.ReadFile(cache.fs, resp.LocalPath) + require.NoError(t, err) + assert.Equal(t, []byte("cached-image"), cached) + + strictDB := testhelpers.NewMockMediaDBI() + warmEnv := makeMediaImageEnv(t, strictDB, mediaImageParams( + row, `"maxSize": 256, "delivery": "localPath"`, + )) + warmResult, err := HandleMediaImage(warmEnv) + require.NoError(t, err) + warmResp, ok := warmResult.(models.MediaImageResponse) + require.True(t, ok) + assert.Equal(t, resp.LocalPath, warmResp.LocalPath) + assert.Equal(t, mediaImageDeliveryPath, warmResp.Delivery) + strictDB.AssertExpectations(t) +} + +func TestHandleMediaImage_LocalPathCacheWriteFailureFallsBackInline(t *testing.T) { + // Not parallel: installs process-wide thumbnail cache pointer. + cache := &mediaThumbCache{ + fs: afero.NewReadOnlyFs(afero.NewMemMapFs()), + dir: filepath.Join(string(filepath.Separator), "cache", mediaThumbCacheVersionDir()), + resolvedTypes: make(map[string]resolvedThumb), + } + mediaThumbCachePointer.Store(cache) + t.Cleanup(func() { mediaThumbCachePointer.Store(nil) }) + + row := makeMediaFullRow(9002, 9020) + mockDB := testhelpers.NewMockMediaDBI() + expectInlineImageProperty(mockDB, row, []byte("inline-fallback")) + env := makeMediaImageEnv(t, mockDB, mediaImageParams( + row, `"maxSize": 256, "delivery": "localPath"`, + )) + + result, err := HandleMediaImage(env) + require.NoError(t, err) + resp, ok := result.(models.MediaImageResponse) + require.True(t, ok) + assert.Equal(t, mediaImageDeliveryInline, resp.Delivery) + assert.Empty(t, resp.LocalPath) + decoded, err := base64.StdEncoding.DecodeString(resp.Data) + require.NoError(t, err) + assert.Equal(t, []byte("inline-fallback"), decoded) +} + +func TestHandleMediaImage_LocalPathValidation(t *testing.T) { + tests := []struct { + name string + extra string + wantError string + }{ + { + name: "missing maxSize", extra: `"delivery": "localPath"`, + wantError: "requires a positive maxSize", + }, + { + name: "unknown delivery", extra: `"delivery": "sharedMemory"`, + wantError: "unsupported delivery", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + row := makeMediaFullRow(9100, 9110) + env := makeMediaImageEnv(t, testhelpers.NewMockMediaDBI(), mediaImageParams(row, tt.extra)) + + _, err := HandleMediaImage(env) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + }) + } +} + // TestHandleMediaImage_DefaultPrefs_TitleBlobFound verifies that when no imageTypes // param is given, the handler uses the default preference order and returns the // first matching title-level property with inline binary data. @@ -1072,7 +1231,7 @@ func TestHandleMediaImage_NoImagePathCacheSkipsMediaDB(t *testing.T) { row := makeMediaFullRow(405, 4050) params := mediaImageParams(row, `"imageTypes": ["boxart"]`) - ref, err := parseMediaImageRequest(params) + ref, _, err := parseMediaImageRequest(params) require.NoError(t, err) prefs := imagePrefs(nil, ref.ImageTypes) mediaImageNoImages.add( @@ -1098,7 +1257,7 @@ func TestHandleMediaImage_NoImageCacheBypassesSemaphore(t *testing.T) { defer func() { <-mediaImageSem }() params := json.RawMessage(`{"mediaId":406,"imageTypes":["boxart"]}`) - ref, err := parseMediaImageRequest(params) + ref, _, err := parseMediaImageRequest(params) require.NoError(t, err) prefs := imagePrefs(nil, ref.ImageTypes) mediaImageNoImages.add( @@ -1121,7 +1280,7 @@ func TestHandleMediaImage_NoImageCacheRecheckedAfterSemaphore(t *testing.T) { mediaImageSem <- struct{}{} params := json.RawMessage(`{"mediaId":407,"imageTypes":["boxart"]}`) - ref, err := parseMediaImageRequest(params) + ref, _, err := parseMediaImageRequest(params) require.NoError(t, err) prefs := imagePrefs(nil, ref.ImageTypes) noImageKey := mediaImageNoImageRequestKey(ref, prefs) diff --git a/pkg/api/methods/media_scrape_test.go b/pkg/api/methods/media_scrape_test.go index 35479d267..d6a4f1e0b 100644 --- a/pkg/api/methods/media_scrape_test.go +++ b/pkg/api/methods/media_scrape_test.go @@ -405,8 +405,8 @@ func TestHandleMediaScrape_WipesThumbCacheOnCompletion(t *testing.T) { resolvedTypes: make(map[string]resolvedThumb), } mediaID := int64(1) - oldCache.set( - mediaRefParam{MediaID: &mediaID}, "SNES", "property:image-boxart", 100, + setMediaThumbCacheForTest( + t, oldCache, mediaRefParam{MediaID: &mediaID}, "SNES", "property:image-boxart", 100, []byte("png-data"), "image/png", ) mediaThumbCachePointer.Store(oldCache) @@ -487,7 +487,7 @@ func TestInvalidateChangedScrapeThumbnails_NoChangesPreservesCache(t *testing.T) } mediaID := int64(1) ref := mediaRefParam{MediaID: &mediaID} - cache.set(ref, "SNES", "property:image-boxart", 256, []byte("cached"), "image/webp") + setMediaThumbCacheForTest(t, cache, ref, "SNES", "property:image-boxart", 256, []byte("cached"), "image/webp") mediaThumbCachePointer.Store(cache) t.Cleanup(func() { mediaThumbCachePointer.Store(nil) }) @@ -592,7 +592,7 @@ func TestHandleMediaScrape_FatalUpdateDoesNotSynthesizeDone(t *testing.T) { } mediaID := int64(1) ref := mediaRefParam{MediaID: &mediaID} - cache.set(ref, "SNES", "property:image-boxart", 256, []byte("cached"), "image/webp") + setMediaThumbCacheForTest(t, cache, ref, "SNES", "property:image-boxart", 256, []byte("cached"), "image/webp") mediaThumbCachePointer.Store(cache) t.Cleanup(func() { mediaThumbCachePointer.Store(nil) }) diff --git a/pkg/api/models/params.go b/pkg/api/models/params.go index 672e43d3a..31723b7c3 100644 --- a/pkg/api/models/params.go +++ b/pkg/api/models/params.go @@ -336,6 +336,7 @@ type MediaImageParams struct { MaxSize *int32 `json:"maxSize,omitempty" validate:"omitempty,gt=0,max=8192"` System string `json:"system" validate:"omitempty,min=1"` Path string `json:"path" validate:"omitempty,min=1"` + Delivery string `json:"delivery,omitempty" validate:"omitempty,oneof=inline localPath"` ImageTypes []string `json:"imageTypes" validate:"omitempty,dive,min=1"` } diff --git a/pkg/api/models/responses.go b/pkg/api/models/responses.go index 07a0872ec..003d12800 100644 --- a/pkg/api/models/responses.go +++ b/pkg/api/models/responses.go @@ -380,12 +380,14 @@ type MediaMetaBatchResponse struct { } // MediaImageResponse is the response for the media.image method. -// It contains the best-match image for a media record, base64-encoded. +// Inline delivery contains Data; local-path delivery contains LocalPath. type MediaImageResponse struct { Extension *string `json:"extension,omitempty"` ContentType string `json:"contentType"` - Data string `json:"data"` // base64-encoded blob - TypeTag string `json:"typeTag"` // e.g. "property:image-boxart" + Data string `json:"data,omitempty"` // base64-encoded blob + Delivery string `json:"delivery"` // "inline" or "localPath" + LocalPath string `json:"localPath,omitempty"` // transient Core-owned thumbnail path + TypeTag string `json:"typeTag"` // e.g. "property:image-boxart" } type ScrapeSystemProgressResponse struct { From ab76569c250f9dae5564f0999ec15a4bcdb48a61 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 05:39:50 +0800 Subject: [PATCH 02/11] feat(api): add distinct media history Let clients request newest-session-per-media history directly from Core while preserving existing paginated history behavior. Group by system and media path before cursor filtering so older sessions cannot reappear on later pages. Inspired by Giancarlo Erra's recents performance work and measurements in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra --- docs/api/methods.md | 18 +-- pkg/api/methods/media_history.go | 19 ++- pkg/api/methods/media_history_test.go | 43 +++++++ pkg/api/models/params.go | 9 +- pkg/database/database.go | 3 + pkg/database/userdb/media_history.go | 111 ++++++++++++++++++ .../userdb/media_history_property_test.go | 74 ++++++++++++ pkg/testing/helpers/db_mocks.go | 14 +++ 8 files changed, 275 insertions(+), 16 deletions(-) diff --git a/docs/api/methods.md b/docs/api/methods.md index abba4ce89..60c51b7d9 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -1419,18 +1419,19 @@ None. Empty params may be omitted or sent as `{}`. **Access:** All clients. -Return paginated media play history. +Return paginated media play history. Set `distinctMedia` to return only the newest session for each `(systemId, mediaPath)` identity, which is useful for recents grids. #### Parameters Optionally, an object: -| Key | Type | Required | Description | -| :---------- | :------- | :------- | :---------------------------------------------------------------------------------------------- | -| limit | number | No | Maximum number of entries to return. Default is 25, maximum is 100. | -| cursor | string | No | Cursor for pagination. Omit for first page, use `nextCursor` from previous response for subsequent pages. | -| systems | string[] | No | Filter to one or more system IDs (e.g., `["SNES", "NES"]`). | -| fuzzySystem | boolean | No | Enable fuzzy matching for system IDs. | +| Key | Type | Required | Description | +| :------------ | :------- | :------- | :---------------------------------------------------------------------------------------------- | +| limit | number | No | Maximum number of entries to return. Default is 25, maximum is 100. | +| cursor | string | No | Cursor for pagination. Omit for first page, use `nextCursor` from previous response for subsequent pages with the same filters and `distinctMedia` value. | +| systems | string[] | No | Filter to one or more system IDs (e.g., `["SNES", "NES"]`). | +| fuzzySystem | boolean | No | Enable fuzzy matching for system IDs. | +| distinctMedia | boolean | No | Return the newest session for each unique `(systemId, mediaPath)` pair. Each page contains up to `limit` unique media entries. Default is `false`. | #### Result @@ -1464,7 +1465,8 @@ Optionally, an object: "id": "a1b2c3d4-7a5d-11ef-9c7b-020304050607", "method": "media.history", "params": { - "limit": 10 + "limit": 10, + "distinctMedia": true } } ``` diff --git a/pkg/api/methods/media_history.go b/pkg/api/methods/media_history.go index 70add2af5..79a4f711c 100644 --- a/pkg/api/methods/media_history.go +++ b/pkg/api/methods/media_history.go @@ -27,6 +27,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/api/validation" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/rs/zerolog/log" ) @@ -36,6 +37,7 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti limit := defaultMediaHistoryLimit var lastID int64 var systemIDs []string + var distinctMedia bool if len(env.Params) > 0 { var params models.MediaHistoryParams @@ -49,7 +51,9 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti if params.Limit != nil { limit = *params.Limit } - + if params.DistinctMedia != nil { + distinctMedia = *params.DistinctMedia + } if params.Cursor != nil { cursor, err := decodeCursor(*params.Cursor) if err != nil { @@ -73,11 +77,17 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti } } - // Fetch one extra to detect next page + // Fetch one extra to detect next page. queryStarted := time.Now() - entries, err := env.Database.UserDB.GetMediaHistory(systemIDs, lastID, limit+1) + var entries []database.MediaHistoryEntry + var err error + if distinctMedia { + entries, err = env.Database.UserDB.GetDistinctMediaHistory(env.Context, systemIDs, lastID, limit+1) + } else { + entries, err = env.Database.UserDB.GetMediaHistory(systemIDs, lastID, limit+1) + } if err != nil { - log.Error().Err(err).Msg("error getting media history") + log.Error().Err(err).Bool("distinctMedia", distinctMedia).Msg("error getting media history") return nil, fmt.Errorf("error getting media history: %w", err) } queryElapsed := time.Since(queryStarted) @@ -126,6 +136,7 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti log.Debug(). Int("entries", len(responseEntries)). + Bool("distinctMedia", distinctMedia). Dur("queryDuration", queryElapsed). Dur("enrichDuration", enrichElapsed). Dur("buildDuration", time.Since(buildStarted)). diff --git a/pkg/api/methods/media_history_test.go b/pkg/api/methods/media_history_test.go index f2c50977e..48b07690e 100644 --- a/pkg/api/methods/media_history_test.go +++ b/pkg/api/methods/media_history_test.go @@ -216,6 +216,49 @@ func TestHandleMediaHistory_WithLimit(t *testing.T) { mockUserDB.AssertExpectations(t) } +func TestHandleMediaHistory_DistinctMedia(t *testing.T) { + t.Parallel() + + mockUserDB := helpers.NewMockUserDBI() + now := time.Now() + firstPath := filepath.Join(string(filepath.Separator), "games", "first.nes") + secondPath := filepath.Join(string(filepath.Separator), "games", "second.nes") + thirdPath := filepath.Join(string(filepath.Separator), "games", "third.nes") + mockUserDB.On( + "GetDistinctMediaHistory", mock.Anything, []string{"NES"}, int64(0), 3, + ).Return([]database.MediaHistoryEntry{ + {DBID: 10, SystemID: "NES", SystemName: "NES", MediaName: "First", MediaPath: firstPath, StartTime: now}, + {DBID: 8, SystemID: "NES", SystemName: "NES", MediaName: "Second", MediaPath: secondPath, StartTime: now}, + {DBID: 5, SystemID: "NES", SystemName: "NES", MediaName: "Third", MediaPath: thirdPath, StartTime: now}, + }, nil) + + env := requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{UserDB: mockUserDB}, + Params: json.RawMessage(`{ + "systems": ["NES"], + "limit": 2, + "distinctMedia": true + }`), + } + + result, err := HandleMediaHistory(env) + require.NoError(t, err) + resp, ok := result.(models.MediaHistoryResponse) + require.True(t, ok) + require.Len(t, resp.Entries, 2) + assert.Equal(t, "First", resp.Entries[0].MediaName) + assert.Equal(t, "Second", resp.Entries[1].MediaName) + require.NotNil(t, resp.Pagination) + assert.True(t, resp.Pagination.HasNextPage) + require.NotNil(t, resp.Pagination.NextCursor) + cursor, err := decodeCursor(*resp.Pagination.NextCursor) + require.NoError(t, err) + require.NotNil(t, cursor) + assert.Equal(t, int64(8), *cursor) + mockUserDB.AssertExpectations(t) +} + func TestHandleMediaHistory_WithCursor(t *testing.T) { t.Parallel() diff --git a/pkg/api/models/params.go b/pkg/api/models/params.go index 31723b7c3..d13ac1d9f 100644 --- a/pkg/api/models/params.go +++ b/pkg/api/models/params.go @@ -292,10 +292,11 @@ type MediaStoppedParams struct { } type MediaHistoryParams struct { - Systems *[]string `json:"systems,omitempty" validate:"omitempty,dive,min=1"` - FuzzySystem *bool `json:"fuzzySystem,omitempty"` - Limit *int `json:"limit,omitempty" validate:"omitempty,gt=0,max=100"` - Cursor *string `json:"cursor,omitempty"` + Systems *[]string `json:"systems,omitempty" validate:"omitempty,dive,min=1"` + FuzzySystem *bool `json:"fuzzySystem,omitempty"` + DistinctMedia *bool `json:"distinctMedia,omitempty"` + Limit *int `json:"limit,omitempty" validate:"omitempty,gt=0,max=100"` + Cursor *string `json:"cursor,omitempty"` } type MediaHistoryTopParams struct { diff --git a/pkg/database/database.go b/pkg/database/database.go index ab7de21cf..0a6354a8f 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -837,6 +837,9 @@ type UserDBI interface { UpdateMediaHistoryIdentity(dbid int64, identity *MediaIdentity) (bool, error) CloseMediaHistory(dbid int64, endTime time.Time, playTime int) error GetMediaHistory(systemIDs []string, lastID int64, limit int) ([]MediaHistoryEntry, error) + GetDistinctMediaHistory( + ctx context.Context, systemIDs []string, lastID int64, limit int, + ) ([]MediaHistoryEntry, error) GetLatestMediaHistory() (MediaHistoryEntry, bool, error) GetMediaHistoryTop(systemIDs []string, since *time.Time, limit int) ([]MediaHistoryTopEntry, error) CloseHangingMediaHistory() error diff --git a/pkg/database/userdb/media_history.go b/pkg/database/userdb/media_history.go index 174a4a627..877c89ca9 100644 --- a/pkg/database/userdb/media_history.go +++ b/pkg/database/userdb/media_history.go @@ -73,6 +73,17 @@ func (db *UserDB) GetMediaHistory(systemIDs []string, lastID int64, limit int) ( return sqlGetMediaHistory(db.ctx, db.sql.Load(), systemIDs, nil, lastID, limit) } +// GetDistinctMediaHistory returns the newest lean history row for each +// (system, media path) identity, ordered for stable cursor pagination. +func (db *UserDB) GetDistinctMediaHistory( + ctx context.Context, systemIDs []string, lastID int64, limit int, +) ([]database.MediaHistoryEntry, error) { + if db.sql.Load() == nil { + return nil, ErrNullSQL + } + return sqlGetDistinctMediaHistory(ctx, db.sql.Load(), systemIDs, lastID, limit) +} + // GetLatestMediaHistory retrieves the most recent media history entry with no enrichment. func (db *UserDB) GetLatestMediaHistory() (database.MediaHistoryEntry, bool, error) { if db.sql.Load() == nil { @@ -454,6 +465,106 @@ func sqlGetMediaHistory( return list, nil } +func sqlGetDistinctMediaHistory( + ctx context.Context, db *sql.DB, systemIDs []string, lastID int64, limit int, +) ([]database.MediaHistoryEntry, error) { + if limit <= 0 { + limit = 25 + } + if limit > 100 { + limit = 100 + } + if lastID == 0 { + lastID = math.MaxInt64 + } + + args := make([]any, 0, len(systemIDs)+2) + latestWhere := "" + switch len(systemIDs) { + case 1: + latestWhere = "WHERE SystemID = ?" + args = append(args, systemIDs[0]) + default: + if len(systemIDs) > 1 { + placeholders := make([]string, len(systemIDs)) + for i, systemID := range systemIDs { + placeholders[i] = "?" + args = append(args, systemID) + } + latestWhere = "WHERE SystemID IN (" + strings.Join(placeholders, ", ") + ")" + } + } + args = append(args, lastID, limit) + + // Group before applying the cursor. Filtering raw history rows first would + // let an older session for a media identity reappear on a later page. + //nolint:gosec // latestWhere contains only fixed SQL and placeholders. + query := fmt.Sprintf(` + WITH LatestMedia AS ( + SELECT MAX(DBID) AS DBID + FROM MediaHistory + %s + GROUP BY SystemID, MediaPath + ) + SELECT + history.DBID, history.StartTime, history.EndTime, + history.SystemID, history.SystemName, history.MediaPath, + history.MediaName, history.LauncherID, history.PlayTime + FROM MediaHistory AS history + INNER JOIN LatestMedia AS latest ON latest.DBID = history.DBID + WHERE history.DBID < ? + ORDER BY history.DBID DESC + LIMIT ?; + `, latestWhere) + + list := make([]database.MediaHistoryEntry, 0, limit) + queryStarted := time.Now() + rows, err := db.QueryContext(ctx, query, args...) + if err != nil { + return list, fmt.Errorf("failed to query distinct media history: %w", err) + } + defer func() { + if closeErr := rows.Close(); closeErr != nil { + log.Warn().Err(closeErr).Msg("failed to close distinct media history rows") + } + }() + + for rows.Next() { + var entry database.MediaHistoryEntry + var startTimeUnix int64 + var endTimeUnix sql.NullInt64 + if scanErr := rows.Scan( + &entry.DBID, + &startTimeUnix, + &endTimeUnix, + &entry.SystemID, + &entry.SystemName, + &entry.MediaPath, + &entry.MediaName, + &entry.LauncherID, + &entry.PlayTime, + ); scanErr != nil { + return list, fmt.Errorf("failed to scan distinct media history row: %w", scanErr) + } + entry.StartTime = time.Unix(startTimeUnix, 0) + if endTimeUnix.Valid { + endTime := time.Unix(endTimeUnix.Int64, 0) + entry.EndTime = &endTime + } + list = append(list, entry) + } + if err = rows.Err(); err != nil { + return list, fmt.Errorf("error iterating distinct media history rows: %w", err) + } + + log.Debug(). + Int("systems", len(systemIDs)). + Int("rows", len(list)). + Dur("queryDuration", time.Since(queryStarted)). + Msg("distinct media history query timing") + return list, nil +} + func sqlGetLatestMediaHistory(ctx context.Context, db *sql.DB) (database.MediaHistoryEntry, bool, error) { stmt, err := db.PrepareContext(ctx, ` SELECT DBID, StartTime, SystemID, SystemName, MediaPath, MediaName, LauncherID diff --git a/pkg/database/userdb/media_history_property_test.go b/pkg/database/userdb/media_history_property_test.go index eacf33bb1..ce6d35a01 100644 --- a/pkg/database/userdb/media_history_property_test.go +++ b/pkg/database/userdb/media_history_property_test.go @@ -22,11 +22,13 @@ package userdb import ( "context" "database/sql" + "path/filepath" "testing" "time" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" _ "github.com/mattn/go-sqlite3" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "pgregory.net/rapid" ) @@ -259,6 +261,78 @@ func TestPropertyGetMediaHistoryLimitClamping(t *testing.T) { }) } +func TestGetDistinctMediaHistory_UniquePaginationAndSystemScope(t *testing.T) { + t.Parallel() + + ctx := context.Background() + db, err := sql.Open("sqlite3", ":memory:") + require.NoError(t, err) + db.SetMaxOpenConns(1) + defer func() { _ = db.Close() }() + + _, err = db.ExecContext(ctx, ` + CREATE TABLE MediaHistory ( + DBID INTEGER PRIMARY KEY AUTOINCREMENT, + StartTime INTEGER NOT NULL, + EndTime INTEGER, + SystemID TEXT NOT NULL, + SystemName TEXT NOT NULL, + MediaPath TEXT NOT NULL, + MediaName TEXT NOT NULL, + LauncherID TEXT NOT NULL, + PlayTime INTEGER NOT NULL + ) + `) + require.NoError(t, err) + + samePath := filepath.Join("games", "shared.rom") + otherPath := filepath.Join("games", "other.rom") + thirdPath := filepath.Join("games", "third.rom") + rows := []struct { + systemID string + path string + name string + }{ + {systemID: "NES", path: samePath, name: "NES old"}, + {systemID: "SNES", path: samePath, name: "SNES shared"}, + {systemID: "NES", path: otherPath, name: "NES other"}, + {systemID: "NES", path: samePath, name: "NES newest"}, + {systemID: "SNES", path: thirdPath, name: "SNES third"}, + } + for i, row := range rows { + _, err = db.ExecContext(ctx, ` + INSERT INTO MediaHistory ( + StartTime, EndTime, SystemID, SystemName, MediaPath, + MediaName, LauncherID, PlayTime + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?) + `, int64(100+i), int64(110+i), row.systemID, row.systemID, row.path, row.name, row.systemID, i) + require.NoError(t, err) + } + + firstPage, err := sqlGetDistinctMediaHistory(ctx, db, nil, 0, 2) + require.NoError(t, err) + require.Len(t, firstPage, 2) + assert.Equal(t, []int64{5, 4}, []int64{firstPage[0].DBID, firstPage[1].DBID}) + assert.Equal(t, "NES newest", firstPage[1].MediaName) + + secondPage, err := sqlGetDistinctMediaHistory(ctx, db, nil, firstPage[1].DBID, 2) + require.NoError(t, err) + require.Len(t, secondPage, 2) + assert.Equal(t, []int64{3, 2}, []int64{secondPage[0].DBID, secondPage[1].DBID}) + assert.Equal(t, "SNES shared", secondPage[1].MediaName, + "same path under another system remains a distinct identity") + + nesOnly, err := sqlGetDistinctMediaHistory(ctx, db, []string{"NES"}, 0, 10) + require.NoError(t, err) + require.Len(t, nesOnly, 2) + assert.Equal(t, []int64{4, 3}, []int64{nesOnly[0].DBID, nesOnly[1].DBID}) + + cancelled, cancel := context.WithCancel(ctx) + cancel() + _, err = sqlGetDistinctMediaHistory(cancelled, db, nil, 0, 10) + require.ErrorIs(t, err, context.Canceled) +} + // TestPropertyGetMediaHistoryLastIDPagination verifies pagination token handling. func TestPropertyGetMediaHistoryLastIDPagination(t *testing.T) { t.Parallel() diff --git a/pkg/testing/helpers/db_mocks.go b/pkg/testing/helpers/db_mocks.go index 02b453aef..380a4f2bd 100644 --- a/pkg/testing/helpers/db_mocks.go +++ b/pkg/testing/helpers/db_mocks.go @@ -390,6 +390,20 @@ func (m *MockUserDBI) GetMediaHistory( return history, nil } +func (m *MockUserDBI) GetDistinctMediaHistory( + ctx context.Context, systemIDs []string, lastID int64, limit int, +) ([]database.MediaHistoryEntry, error) { + args := m.Called(ctx, systemIDs, lastID, limit) + history, ok := args.Get(0).([]database.MediaHistoryEntry) + if !ok { + history = []database.MediaHistoryEntry{} + } + if err := args.Error(1); err != nil { + return history, fmt.Errorf("mock UserDBI get distinct media history failed: %w", err) + } + return history, nil +} + func (m *MockUserDBI) GetLatestMediaHistory() (database.MediaHistoryEntry, bool, error) { args := m.Called() entry, ok := args.Get(0).(database.MediaHistoryEntry) From 963e21b98240cb9d3bd6078e6a9068ebde239966 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 05:48:28 +0800 Subject: [PATCH 03/11] chore(api): add request transport timings Correlate queue, handler, response, marshal, payload, encryption, and HTTP timing logs by request ID. This makes target-device API latency attributable without changing response contracts. Motivated by MiSTer performance measurements from ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 --- pkg/api/request_priority.go | 11 +++- pkg/api/request_priority_test.go | 19 ++++++ pkg/api/server.go | 109 ++++++++++++++++++++++++++----- pkg/api/ws_dispatcher.go | 84 +++++++++++++++++------- 4 files changed, 180 insertions(+), 43 deletions(-) diff --git a/pkg/api/request_priority.go b/pkg/api/request_priority.go index ad0464548..fd6653017 100644 --- a/pkg/api/request_priority.go +++ b/pkg/api/request_priority.go @@ -101,13 +101,18 @@ func classifyAPIMethod(method string) apiRequestPriority { } } -func methodFromAPIRequestPayload(msg []byte) string { +func requestMetadataFromAPIRequestPayload(msg []byte) (string, models.RPCID) { var req models.RequestObject if err := json.Unmarshal(msg, &req); err != nil { log.Debug().Err(err).Msg("failed to unmarshal API request payload") - return "" + return "", models.RPCID{} } - return strings.ToLower(req.Method) + return strings.ToLower(req.Method), req.ID +} + +func methodFromAPIRequestPayload(msg []byte) string { + method, _ := requestMetadataFromAPIRequestPayload(msg) + return method } func isImageAPIMethod(method string) bool { diff --git a/pkg/api/request_priority_test.go b/pkg/api/request_priority_test.go index 56d75663a..f88bcd33e 100644 --- a/pkg/api/request_priority_test.go +++ b/pkg/api/request_priority_test.go @@ -21,6 +21,7 @@ package api import ( "context" + "strings" "testing" "time" @@ -126,6 +127,24 @@ func TestMethodFromAPIRequestPayload(t *testing.T) { } } +func TestRequestMetadataFromAPIRequestPayload(t *testing.T) { + t.Parallel() + + method, requestID := requestMetadataFromAPIRequestPayload( + []byte(`{"jsonrpc":"2.0","method":"Media.Search","id":"request-42"}`), + ) + assert.Equal(t, models.MethodMediaSearch, method) + assert.Equal(t, `"request-42"`, requestID.String()) +} + +func TestRequestIDForLogTruncatesLongIDs(t *testing.T) { + t.Parallel() + + logged := requestIDForLog(models.NewStringID(strings.Repeat("x", maxLoggedRequestIDLen))) + assert.Len(t, logged, maxLoggedRequestIDLen) + assert.True(t, strings.HasSuffix(logged, "...")) +} + func TestIsImageAPIMethod(t *testing.T) { t.Parallel() diff --git a/pkg/api/server.go b/pkg/api/server.go index 4008726e8..73a1afc53 100644 --- a/pkg/api/server.go +++ b/pkg/api/server.go @@ -73,7 +73,10 @@ var allowedOrigins = []string{ "http://localhost", // Fallback/development } -const websocketMaxMessageSize = 4 * 1024 * 1024 +const ( + websocketMaxMessageSize = 4 * 1024 * 1024 + maxLoggedRequestIDLen = 128 +) var JSONRPCErrorParseError = models.ErrorObject{ Code: -32700, @@ -113,9 +116,17 @@ func newWebSocketSession() *melody.Melody { return session } -// logSafeRequest logs a request but avoids logging sensitive or large content +func requestIDForLog(id models.RPCID) string { + value := id.String() + if len(value) <= maxLoggedRequestIDLen { + return value + } + return value[:maxLoggedRequestIDLen-3] + "..." +} + +// logSafeRequest logs a request but avoids logging sensitive or large content. func logSafeRequest(req *models.RequestObject) { - log.Debug().Str("method", req.Method).Interface("id", req.ID).Msg("received request") + log.Debug().Str("method", req.Method).Str("requestId", requestIDForLog(req.ID)).Msg("received request") } // logSafeResponse logs a response but redacts large or binary fields. Any @@ -143,6 +154,7 @@ func logSafeResponse(result any) { log.Debug(). Str("typeTag", resp.TypeTag). Str("contentType", resp.ContentType). + Str("delivery", resp.Delivery). Int("data_len", len(resp.Data)). Msg("sending response") case models.MediaMetaResponse: @@ -454,6 +466,28 @@ func handleRequest( return resp, nil } +func logWebSocketTransportTiming( + id models.RPCID, + responseType string, + encrypted bool, + responseBytes int, + marshalDuration time.Duration, + writeDuration time.Duration, + writeErr error, +) { + event := log.Debug(). + Str("requestId", requestIDForLog(id)). + Str("responseType", responseType). + Bool("encrypted", encrypted). + Int("responseBytes", responseBytes). + Dur("marshalDuration", marshalDuration). + Dur("writeDuration", writeDuration) + if writeErr != nil { + event = event.Err(writeErr) + } + event.Msg("websocket response transport timing") +} + // sendWSResponse marshals a method result and sends it to the client. func sendWSResponse(session *melody.Session, id models.RPCID, result any) error { logSafeResponse(result) @@ -464,13 +498,20 @@ func sendWSResponse(session *melody.Session, id models.RPCID, result any) error Result: result, } + marshalStarted := time.Now() data, err := json.Marshal(resp) + marshalDuration := time.Since(marshalStarted) if err != nil { return fmt.Errorf("error marshalling response: %w", err) } - if err := session.Write(data); err != nil { - return fmt.Errorf("failed to write websocket response: %w", err) + writeStarted := time.Now() + writeErr := session.Write(data) + logWebSocketTransportTiming( + id, "result", false, len(data), marshalDuration, time.Since(writeStarted), writeErr, + ) + if writeErr != nil { + return fmt.Errorf("failed to write websocket response: %w", writeErr) } return nil } @@ -485,14 +526,20 @@ func sendWSError(session *melody.Session, id models.RPCID, errObj models.ErrorOb Error: &errObj, } + marshalStarted := time.Now() data, err := json.Marshal(resp) + marshalDuration := time.Since(marshalStarted) if err != nil { return fmt.Errorf("error marshalling error response: %w", err) } - err = session.Write(data) - if err != nil { - return fmt.Errorf("failed to write to session: %w", err) + writeStarted := time.Now() + writeErr := session.Write(data) + logWebSocketTransportTiming( + id, "error", false, len(data), marshalDuration, time.Since(writeStarted), writeErr, + ) + if writeErr != nil { + return fmt.Errorf("failed to write to session: %w", writeErr) } return nil } @@ -513,7 +560,7 @@ func handleResponse(resp models.ResponseObject) error { // whole map would flood the debug log. Error messages cross the same // 4 MB WS boundary, so cap them too. const maxErrorMessageLen = 200 - ev := log.Debug().Interface("id", resp.ID) + ev := log.Debug().Str("requestId", requestIDForLog(resp.ID)) if resp.Error != nil { ev = ev.Int("errorCode", resp.Error.Code) msg := resp.Error.Message @@ -966,6 +1013,7 @@ func processRequestObject( resp, rpcError := handleRequest(methodMap, env, req) log.Debug(). Str("method", req.Method). + Str("requestId", requestIDForLog(req.ID)). Dur("duration", time.Since(started)). Bool("error", rpcError != nil). Msg("api request handled") @@ -1287,12 +1335,19 @@ func sendWSEncryptedResponse( ID: id, Result: result, } + marshalStarted := time.Now() data, err := json.Marshal(resp) + marshalDuration := time.Since(marshalStarted) if err != nil { return fmt.Errorf("marshal response: %w", err) } - if err := cs.SendEncryptedFrame(data, session.Write); err != nil { - return fmt.Errorf("send encrypted response: %w", err) + writeStarted := time.Now() + writeErr := cs.SendEncryptedFrame(data, session.Write) + logWebSocketTransportTiming( + id, "result", true, len(data), marshalDuration, time.Since(writeStarted), writeErr, + ) + if writeErr != nil { + return fmt.Errorf("send encrypted response: %w", writeErr) } return nil } @@ -1315,12 +1370,19 @@ func sendWSEncryptedError( ID: id, Error: &rpcErr, } + marshalStarted := time.Now() data, err := json.Marshal(resp) + marshalDuration := time.Since(marshalStarted) if err != nil { return fmt.Errorf("marshal error response: %w", err) } - if err := cs.SendEncryptedFrame(data, session.Write); err != nil { - return fmt.Errorf("send encrypted error: %w", err) + writeStarted := time.Now() + writeErr := cs.SendEncryptedFrame(data, session.Write) + logWebSocketTransportTiming( + id, "error", true, len(data), marshalDuration, time.Since(writeStarted), writeErr, + ) + if writeErr != nil { + return fmt.Errorf("send encrypted error: %w", writeErr) } return nil } @@ -1417,7 +1479,10 @@ func handlePostRequest( } var respBody []byte + responseType := "result" + marshalStarted := time.Now() if result.Error != nil { + responseType = "error" errorResp := models.ResponseErrorObject{ JSONRPC: "2.0", ID: result.ID, @@ -1442,16 +1507,28 @@ func handlePostRequest( return } } + marshalDuration := time.Since(marshalStarted) + writeStarted := time.Now() w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - _, err = w.Write(respBody) - if err != nil { - log.Error().Err(err).Msg("failed to write response") + writtenBytes, writeErr := w.Write(respBody) + if writeErr != nil { + log.Error().Err(writeErr).Msg("failed to write response") } if f, ok := w.(http.Flusher); ok { f.Flush() } + log.Debug(). + Str("method", method). + Str("requestId", requestIDForLog(result.ID)). + Str("responseType", responseType). + Int("responseBytes", len(respBody)). + Int("writtenBytes", writtenBytes). + Dur("marshalDuration", marshalDuration). + Dur("writeDuration", time.Since(writeStarted)). + Bool("writeError", writeErr != nil). + Msg("http response transport timing") if result.AfterWrite != nil { result.AfterWrite() } diff --git a/pkg/api/ws_dispatcher.go b/pkg/api/ws_dispatcher.go index a4ee0a602..cf32ec6fd 100644 --- a/pkg/api/ws_dispatcher.go +++ b/pkg/api/ws_dispatcher.go @@ -23,6 +23,7 @@ import ( "context" "errors" "fmt" + "time" apimiddleware "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/middleware" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" @@ -56,23 +57,34 @@ var ( const wsDispatcherSessionKey = "api.ws.dispatcher" +func queueDuration(enqueuedAt time.Time) time.Duration { + if enqueuedAt.IsZero() { + return 0 + } + return time.Since(enqueuedAt) +} + type wsRequestJob struct { - tracker RequestTracker - methodMap *MethodMap - cs *apimiddleware.ClientSession - cancel context.CancelFunc - env *requests.RequestEnv - method string - msg []byte - image bool + tracker RequestTracker + methodMap *MethodMap + cs *apimiddleware.ClientSession + cancel context.CancelFunc + env *requests.RequestEnv + enqueuedAt time.Time + requestID models.RPCID + method string + msg []byte + image bool } type wsResponseJob struct { - tracker RequestTracker - cs *apimiddleware.ClientSession - cancel context.CancelFunc - result requestResult - pong bool + enqueuedAt time.Time + tracker RequestTracker + cs *apimiddleware.ClientSession + cancel context.CancelFunc + method string + result requestResult + pong bool } type wsSessionDispatcher struct { @@ -197,7 +209,9 @@ func (d *wsSessionDispatcher) enqueuePong(cs *apimiddleware.ClientSession, track select { case <-d.ctx.Done(): return d.ctx.Err() - case d.responses <- &wsResponseJob{cs: cs, tracker: tracker, pong: true}: + case d.responses <- &wsResponseJob{ + cs: cs, tracker: tracker, enqueuedAt: time.Now(), method: "ping", pong: true, + }: return nil default: return errors.New("websocket response queue is full") @@ -216,6 +230,12 @@ func (d *wsSessionDispatcher) worker(queue <-chan *wsRequestJob) { } func (d *wsSessionDispatcher) runJob(job *wsRequestJob) { + log.Debug(). + Str("method", job.method). + Str("requestId", requestIDForLog(job.requestID)). + Dur("queueWaitDuration", queueDuration(job.enqueuedAt)). + Msg("websocket request dequeued") + //nolint:gosec // Cancellation is transferred to job and invoked when response handling completes. ctx, cancel := requestContextForAPIMethod(d.ctx, job.method) job.env.Context = ctx @@ -226,7 +246,7 @@ func (d *wsSessionDispatcher) runJob(job *wsRequestJob) { log.Error().Interface("panic", r).Msg("panic in websocket request worker") d.enqueueResponse(&wsResponseJob{ result: requestResult{ID: models.NullRPCID, Error: &JSONRPCErrorInternalError, ShouldReply: true}, - cs: job.cs, tracker: job.tracker, cancel: job.cancel, + cs: job.cs, tracker: job.tracker, cancel: job.cancel, method: job.method, }) } }() @@ -248,7 +268,9 @@ func (d *wsSessionDispatcher) runJob(job *wsRequestJob) { defer unlock() result := processRequestObject(job.methodMap, *job.env, job.msg) - d.enqueueResponse(&wsResponseJob{result: result, cs: job.cs, tracker: job.tracker, cancel: job.cancel}) + d.enqueueResponse(&wsResponseJob{ + result: result, cs: job.cs, tracker: job.tracker, cancel: job.cancel, method: job.method, + }) } func mediaDBLockModeForAPIMethod(method string) mediaDBLockMode { @@ -288,10 +310,12 @@ func (d *wsSessionDispatcher) finishWithoutReply(job *wsRequestJob) { cs: job.cs, tracker: job.tracker, cancel: job.cancel, + method: job.method, }) } func (d *wsSessionDispatcher) enqueueResponse(resp *wsResponseJob) { + resp.enqueuedAt = time.Now() select { case <-d.ctx.Done(): if resp.cancel != nil { @@ -316,6 +340,16 @@ func (d *wsSessionDispatcher) writer() { } func (d *wsSessionDispatcher) writeResponse(resp *wsResponseJob) { + requestID := resp.result.ID + if resp.pong { + requestID = models.RPCID{} + } + log.Debug(). + Str("method", resp.method). + Str("requestId", requestIDForLog(requestID)). + Dur("responseQueueDuration", queueDuration(resp.enqueuedAt)). + Msg("websocket response dequeued") + defer func() { if resp.cancel != nil { resp.cancel() @@ -361,17 +395,19 @@ func enqueueWSRequest( cs *apimiddleware.ClientSession, tracker RequestTracker, ) error { - method := methodFromAPIRequestPayload(msg) + method, requestID := requestMetadataFromAPIRequestPayload(msg) priority := classifyAPIMethod(method) env.Context = d.ctx job := &wsRequestJob{ - methodMap: methodMap, - env: env, - method: method, - msg: append([]byte(nil), msg...), - cs: cs, - tracker: tracker, - image: isImageAPIMethod(method), + methodMap: methodMap, + env: env, + enqueuedAt: time.Now(), + requestID: requestID, + method: method, + msg: append([]byte(nil), msg...), + cs: cs, + tracker: tracker, + image: isImageAPIMethod(method), } if err := d.enqueue(job, priority); err != nil { return fmt.Errorf("enqueue websocket request: %w", err) From 82e4e2fb9ba5841aac9360b7739af3a9c143b1a9 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 06:29:26 +0800 Subject: [PATCH 04/11] perf(database): cache cover availability Build an exact in-memory index of media and title artwork properties while retaining a bounded title-first SQL fallback during cold start. Invalidate the index on image-property and database lifecycle changes so Core can serve cover status without exposing its schema. Inspired by Giancarlo Erra's MiSTer artwork and browse performance work and measurements in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra --- pkg/database/database.go | 21 +- pkg/database/mediadb/mediadb.go | 28 ++ pkg/database/mediadb/sql_browse.go | 402 ++++++++++++++++++++--- pkg/database/mediadb/sql_browse_test.go | 113 ++++++- pkg/database/mediadb/sql_scraper.go | 27 +- pkg/database/mediadb/sql_scraper_test.go | 8 +- pkg/testing/helpers/db_mocks.go | 17 + 7 files changed, 541 insertions(+), 75 deletions(-) diff --git a/pkg/database/database.go b/pkg/database/database.go index 0a6354a8f..802e4b952 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -239,12 +239,18 @@ type MediaUserData struct { IsFavorite bool } -// MediaPathID identifies a Media row by its system ID and path, used for batch -// media-ID resolution of API responses. +// MediaPathID identifies a Media row and its title by system ID and path, used +// for batch API response enrichment. type MediaPathID struct { - SystemID string - Path string - DBID int64 + SystemID string + Path string + DBID int64 + MediaTitleDBID int64 +} + +type MediaCoverRef struct { + MediaDBID int64 + MediaTitleDBID int64 } type TagType struct { @@ -1039,6 +1045,7 @@ type MediaDBI interface { BrowseDirectories(ctx context.Context, opts BrowseDirectoriesOptions) ([]BrowseDirectoryResult, error) BrowseDirCount(ctx context.Context, opts BrowseDirCountOptions) (int, error) BrowseFiles(ctx context.Context, opts *BrowseFilesOptions) ([]SearchResultWithCursor, error) + GetMediaCoverStatus(ctx context.Context, refs []MediaCoverRef) (map[int64]bool, error) BrowseFileCount(ctx context.Context, opts BrowseFileCountOptions) (int, error) BrowseIndex(ctx context.Context, opts BrowseIndexOptions) (BrowseIndexResult, error) BrowseVirtualSchemes(ctx context.Context, opts BrowseVirtualSchemesOptions) ([]BrowseVirtualScheme, error) @@ -1121,8 +1128,8 @@ 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) - // FindMediaIDsByPaths returns the system ID, path, and DBID of every Media - // row whose Path is in paths, in a single query across all systems. + // FindMediaIDsByPaths returns the system ID, path, media DBID, and title DBID + // of every Media row whose Path is in paths, in one query across all systems. FindMediaIDsByPaths(ctx context.Context, paths []string) ([]MediaPathID, error) // FindSingleContainerLaunchMedia returns the one logical launch target in the // direct contents of containerPath for systemDBID, or nil, nil when the diff --git a/pkg/database/mediadb/mediadb.go b/pkg/database/mediadb/mediadb.go index be7121982..c26e36a24 100644 --- a/pkg/database/mediadb/mediadb.go +++ b/pkg/database/mediadb/mediadb.go @@ -202,6 +202,7 @@ type invalidationScope struct { func (db *MediaDB) invalidateCaches(scope invalidationScope) { db.inMemoryTagCache.Store(nil) clearPrefixPolicyCache() + clearCoverAvailabilityCacheFor(db.sql.Load()) if scope.UtilityTagDBIDsChanged { clearUtilityTagCache() clearImagePropertyTagCache() @@ -365,6 +366,7 @@ func (db *MediaDB) Open() error { } } clearUtilityTagCache() + clearCoverAvailabilityCache() clearImagePropertyTagCache() clearPrefixPolicyCache() @@ -376,6 +378,7 @@ func (db *MediaDB) Open() error { } } + registerCoverAvailabilityCacheOwner(sqlInstance, db) return nil } @@ -1320,6 +1323,7 @@ func (db *MediaDB) Close() error { logSQLTraceSummary() clearUtilityTagCacheFor(db.sql.Load()) clearImagePropertyTagCacheFor(db.sql.Load()) + unregisterCoverAvailabilityCacheOwner(db.sql.Load()) clearPrefixPolicyCacheFor(db.sql.Load()) err := db.sql.Load().Close() @@ -1364,6 +1368,7 @@ func (db *MediaDB) cacheInvalidationScopeForCommittedTransaction() invalidationS func (db *MediaDB) SetSQLForTesting(ctx context.Context, sqlDB *sql.DB, platform platforms.Platform) error { db.sql.Store(sqlDB) clearUtilityTagCache() + clearCoverAvailabilityCache() clearImagePropertyTagCache() clearPrefixPolicyCache() db.ctx = ctx @@ -2132,6 +2137,29 @@ func (db *MediaDB) BrowseFiles( return results, err } +// GetMediaCoverStatus reports image-property availability at media or title scope. +func (db *MediaDB) GetMediaCoverStatus( + ctx context.Context, refs []database.MediaCoverRef, +) (map[int64]bool, error) { + if db.sql.Load() == nil { + return nil, ErrNullSQL + } + if coverIndex := cachedCoverAvailabilityIndex(db.sql.Load()); coverIndex != nil { + statuses := make(map[int64]bool, len(refs)) + for _, ref := range refs { + if ref.MediaDBID <= 0 { + continue + } + statuses[ref.MediaDBID] = coverIndex.hasTitle(ref.MediaTitleDBID) || + coverIndex.hasMedia(ref.MediaDBID) + } + return statuses, nil + } + statuses, err := fetchCoverStatuses(ctx, db.sql.Load(), refs) + db.NoteCorruption(err) + return statuses, err +} + // BrowseFileCount returns the total number of immediate child files under a path prefix. func (db *MediaDB) BrowseFileCount( ctx context.Context, opts database.BrowseFileCountOptions, diff --git a/pkg/database/mediadb/sql_browse.go b/pkg/database/mediadb/sql_browse.go index d2339a2b9..b92f156fd 100644 --- a/pkg/database/mediadb/sql_browse.go +++ b/pkg/database/mediadb/sql_browse.go @@ -22,6 +22,7 @@ package mediadb import ( "context" "database/sql" + "encoding/json" "errors" "fmt" "path/filepath" @@ -44,7 +45,23 @@ import ( // TagTypes join on tt.Type = tags.TagTypeProperty. const imagePropertyValuePrefix = "image-" +type coverPropertyScope uint8 + +type coverAvailabilityIndex struct { + mediaIDs []int64 + titleIDs []int64 +} + +type coverAvailabilityCacheEntry struct { + index *coverAvailabilityIndex + generation uint64 + building bool +} + const ( + coverPropertyScopeMedia coverPropertyScope = iota + coverPropertyScopeTitle + browseSortRankPrefixAsc = "rank-prefix-asc" browseSortRankPrefixDesc = "rank-prefix-desc" browseSortDatePrefixAsc = "date-prefix-asc" @@ -148,6 +165,136 @@ func clearImagePropertyTagCacheFor(db sqlQueryable) { } } +// coverAvailabilityCache stores immutable sorted ID sets per database handle. +// Production database handles are registered with an owner so first use can +// build in the background while the request uses the bounded SQL fallback. +var ( + coverAvailabilityCacheMu syncutil.RWMutex + coverAvailabilityCacheMap map[sqlQueryable]*coverAvailabilityCacheEntry + coverAvailabilityOwnerMap map[sqlQueryable]*MediaDB +) + +func clearCoverAvailabilityCache() { + coverAvailabilityCacheMu.Lock() + defer coverAvailabilityCacheMu.Unlock() + coverAvailabilityCacheMap = nil +} + +func clearCoverAvailabilityCacheFor(db sqlQueryable) { + if db == nil { + return + } + + coverAvailabilityCacheMu.Lock() + defer coverAvailabilityCacheMu.Unlock() + entry := coverAvailabilityCacheMap[db] + if entry == nil { + return + } + entry.generation++ + entry.index = nil +} + +func registerCoverAvailabilityCacheOwner(db sqlQueryable, owner *MediaDB) { + if db == nil || owner == nil { + return + } + coverAvailabilityCacheMu.Lock() + defer coverAvailabilityCacheMu.Unlock() + if coverAvailabilityOwnerMap == nil { + coverAvailabilityOwnerMap = make(map[sqlQueryable]*MediaDB) + } + coverAvailabilityOwnerMap[db] = owner +} + +func unregisterCoverAvailabilityCacheOwner(db sqlQueryable) { + if db == nil { + return + } + coverAvailabilityCacheMu.Lock() + defer coverAvailabilityCacheMu.Unlock() + delete(coverAvailabilityOwnerMap, db) + delete(coverAvailabilityCacheMap, db) +} + +func cachedCoverAvailabilityIndex(db sqlQueryable) *coverAvailabilityIndex { + coverAvailabilityCacheMu.RLock() + defer coverAvailabilityCacheMu.RUnlock() + entry := coverAvailabilityCacheMap[db] + if entry == nil { + return nil + } + return entry.index +} + +func ensureCoverAvailabilityIndexBuild(db sqlQueryable, imageTagIDs []int64) { + coverAvailabilityCacheMu.Lock() + owner := coverAvailabilityOwnerMap[db] + if owner == nil { + coverAvailabilityCacheMu.Unlock() + return + } + // Avoid a full property-index scan while indexing or optimization owns the + // database. Requests keep using the bounded SQL fallback and a later request + // starts the build once background work is idle. + if owner.HasBackgroundOperations() { + coverAvailabilityCacheMu.Unlock() + return + } + if coverAvailabilityCacheMap == nil { + coverAvailabilityCacheMap = make(map[sqlQueryable]*coverAvailabilityCacheEntry) + } + entry := coverAvailabilityCacheMap[db] + if entry == nil { + entry = &coverAvailabilityCacheEntry{} + coverAvailabilityCacheMap[db] = entry + } + if entry.index != nil || entry.building { + coverAvailabilityCacheMu.Unlock() + return + } + entry.building = true + generation := entry.generation + coverAvailabilityCacheMu.Unlock() + + tagIDs := append([]int64(nil), imageTagIDs...) + owner.TrackBackgroundOperation() + go func() { + defer owner.BackgroundOperationDone() + index, err := buildCoverAvailabilityIndex(context.WithoutCancel(owner.ctx), db, tagIDs) + + coverAvailabilityCacheMu.Lock() + current := coverAvailabilityCacheMap[db] + if current == entry { + entry.building = false + if err == nil && entry.generation == generation { + entry.index = index + } + } + coverAvailabilityCacheMu.Unlock() + + if err != nil { + log.Debug().Err(err).Msg("cover availability index build failed; retaining SQL fallback") + } + }() +} + +func (index *coverAvailabilityIndex) hasMedia(id int64) bool { + return sortedInt64sContain(index.mediaIDs, id) +} + +func (index *coverAvailabilityIndex) hasTitle(id int64) bool { + return sortedInt64sContain(index.titleIDs, id) +} + +func sortedInt64sContain(ids []int64, id int64) bool { + if id <= 0 { + return false + } + pos := sort.Search(len(ids), func(i int) bool { return ids[i] >= id }) + return pos < len(ids) && ids[pos] == id +} + // prefixPolicyCache memoises detected browse prefix policies per DB handle and // directory. Detection reads every Media.Path in the directory, which costs // 40-70ms per browse on SD-card hardware; the policy only changes when media @@ -1002,15 +1149,22 @@ func fetchAndAttachCoverFlags( return fmt.Errorf("browse cover flags backfill title ids: %w", err) } - mediaIDs := make([]int64, 0, len(results)) + if coverIndex := cachedCoverAvailabilityIndex(db); coverIndex != nil { + for i := range results { + results[i].HasCover = coverIndex.hasTitle(results[i].MediaTitleID) || + coverIndex.hasMedia(results[i].MediaID) + } + return nil + } + ensureCoverAvailabilityIndexBuild(db, imageTagIDs) + + // Scrapers normally store artwork at title scope. Resolve title covers first, + // then query media-scope properties only for entries that remain uncovered. + // This preserves media-level overrides without probing both property tables + // for every result. titleIDs := make([]int64, 0, len(results)) - mediaIndex := make(map[int64][]int, len(results)) titleIndex := make(map[int64][]int, len(results)) for i := range results { - if _, ok := mediaIndex[results[i].MediaID]; !ok { - mediaIDs = append(mediaIDs, results[i].MediaID) - } - mediaIndex[results[i].MediaID] = append(mediaIndex[results[i].MediaID], i) if results[i].MediaTitleID == 0 { continue } @@ -1020,65 +1174,221 @@ func fetchAndAttachCoverFlags( titleIndex[results[i].MediaTitleID] = append(titleIndex[results[i].MediaTitleID], i) } - mediaPlaceholders := prepareVariadic("?", ",", len(mediaIDs)) + if len(titleIDs) > 0 { + coveredTitleIDs, queryErr := queryImagePropertyEntityIDs( + ctx, db, coverPropertyScopeTitle, titleIDs, imageTagIDs, + ) + if queryErr != nil { + return fmt.Errorf("browse cover flags query: %w", queryErr) + } + for id := range coveredTitleIDs { + for _, idx := range titleIndex[id] { + results[idx].HasCover = true + } + } + } + + mediaIDs := make([]int64, 0, len(results)) + mediaIndex := make(map[int64][]int, len(results)) + for i := range results { + if results[i].HasCover || results[i].MediaID <= 0 { + continue + } + if _, ok := mediaIndex[results[i].MediaID]; !ok { + mediaIDs = append(mediaIDs, results[i].MediaID) + } + mediaIndex[results[i].MediaID] = append(mediaIndex[results[i].MediaID], i) + } + if len(mediaIDs) == 0 { + return nil + } + + coveredMediaIDs, err := queryImagePropertyEntityIDs( + ctx, db, coverPropertyScopeMedia, mediaIDs, imageTagIDs, + ) + if err != nil { + return fmt.Errorf("browse cover flags query: %w", err) + } + for id := range coveredMediaIDs { + for _, idx := range mediaIndex[id] { + results[idx].HasCover = true + } + } + return nil +} + +func buildCoverAvailabilityIndex( + ctx context.Context, + db sqlQueryable, + imageTagIDs []int64, +) (*coverAvailabilityIndex, error) { + started := time.Now() + titleIDs, err := queryAllImagePropertyEntityIDs(ctx, db, coverPropertyScopeTitle, imageTagIDs) + if err != nil { + return nil, fmt.Errorf("load title cover IDs: %w", err) + } + mediaIDs, err := queryAllImagePropertyEntityIDs(ctx, db, coverPropertyScopeMedia, imageTagIDs) + if err != nil { + return nil, fmt.Errorf("load media cover IDs: %w", err) + } + index := &coverAvailabilityIndex{ + titleIDs: sortAndCompactInt64s(titleIDs), + mediaIDs: sortAndCompactInt64s(mediaIDs), + } + log.Debug(). + Int("titleIDs", len(index.titleIDs)). + Int("mediaIDs", len(index.mediaIDs)). + Int("bytes", (cap(index.titleIDs)+cap(index.mediaIDs))*8). + Dur("duration", time.Since(started)). + Msg("cover availability index built") + return index, nil +} + +func queryAllImagePropertyEntityIDs( + ctx context.Context, + db sqlQueryable, + scope coverPropertyScope, + imageTagIDs []int64, +) ([]int64, error) { tagPlaceholders := prepareVariadic("?", ",", len(imageTagIDs)) - args := make([]any, 0, len(mediaIDs)+len(titleIDs)+len(imageTagIDs)*2) - queryParts := []string{` - SELECT 'media' AS Scope, mp.MediaDBID AS ID - FROM MediaProperties mp - WHERE mp.MediaDBID IN (` + mediaPlaceholders + `) - AND mp.TypeTagDBID IN (` + tagPlaceholders + `)`} - for _, id := range mediaIDs { + var query string + switch scope { + case coverPropertyScopeMedia: + query = `SELECT json_group_array(MediaDBID) FROM ( + SELECT mp.MediaDBID, mp.TypeTagDBID + FROM MediaProperties mp + WHERE mp.TypeTagDBID IN (` + tagPlaceholders + `) + ORDER BY mp.MediaDBID, mp.TypeTagDBID)` + case coverPropertyScopeTitle: + query = `SELECT json_group_array(MediaTitleDBID) FROM ( + SELECT mtp.MediaTitleDBID, mtp.TypeTagDBID + FROM MediaTitleProperties mtp + WHERE mtp.TypeTagDBID IN (` + tagPlaceholders + `) + ORDER BY mtp.MediaTitleDBID, mtp.TypeTagDBID)` + default: + return nil, fmt.Errorf("unknown cover property scope %d", scope) + } + + args := make([]any, len(imageTagIDs)) + for i, id := range imageTagIDs { + args[i] = id + } + // ORDER BY matches each table's unique covering index. SQLite scans that + // compact index sequentially and returns one aggregate value, avoiding both + // random table reads and one CGO row crossing per stored property. + var rawIDs string + //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?" + if err := db.QueryRowContext(ctx, query, args...).Scan(&rawIDs); err != nil { + return nil, fmt.Errorf("query all cover IDs: %w", err) + } + var ids []int64 + if err := json.Unmarshal([]byte(rawIDs), &ids); err != nil { + return nil, fmt.Errorf("decode cover IDs: %w", err) + } + return ids, nil +} + +func sortAndCompactInt64s(ids []int64) []int64 { + if len(ids) < 2 { + return ids + } + sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] }) + write := 1 + for read := 1; read < len(ids); read++ { + if ids[read] == ids[write-1] { + continue + } + ids[write] = ids[read] + write++ + } + return ids[:write] +} + +func queryImagePropertyEntityIDs( + ctx context.Context, + db sqlQueryable, + scope coverPropertyScope, + entityIDs []int64, + imageTagIDs []int64, +) (map[int64]struct{}, error) { + if len(entityIDs) == 0 || len(imageTagIDs) == 0 { + return map[int64]struct{}{}, nil + } + + entityPlaceholders := prepareVariadic("?", ",", len(entityIDs)) + tagPlaceholders := prepareVariadic("?", ",", len(imageTagIDs)) + var query string + switch scope { + case coverPropertyScopeMedia: + query = `SELECT mp.MediaDBID + FROM MediaProperties mp + WHERE mp.MediaDBID IN (` + entityPlaceholders + `) + AND mp.TypeTagDBID IN (` + tagPlaceholders + `)` + case coverPropertyScopeTitle: + query = `SELECT mtp.MediaTitleDBID + FROM MediaTitleProperties mtp + WHERE mtp.MediaTitleDBID IN (` + entityPlaceholders + `) + AND mtp.TypeTagDBID IN (` + tagPlaceholders + `)` + default: + return nil, fmt.Errorf("unknown cover property scope %d", scope) + } + + args := make([]any, 0, len(entityIDs)+len(imageTagIDs)) + for _, id := range entityIDs { args = append(args, id) } for _, id := range imageTagIDs { args = append(args, id) } - if len(titleIDs) > 0 { - titlePlaceholders := prepareVariadic("?", ",", len(titleIDs)) - queryParts = append(queryParts, ` - SELECT 'title' AS Scope, mtp.MediaTitleDBID AS ID - FROM MediaTitleProperties mtp - WHERE mtp.MediaTitleDBID IN (`+titlePlaceholders+`) - AND mtp.TypeTagDBID IN (`+tagPlaceholders+`)`) - for _, id := range titleIDs { - args = append(args, id) - } - for _, id := range imageTagIDs { - args = append(args, id) - } - } //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?" - query := strings.Join(queryParts, "\n\t\tUNION ALL\n") - rows, err := db.QueryContext(ctx, query, args...) if err != nil { - return fmt.Errorf("browse cover flags query: %w", err) + return nil, fmt.Errorf("query cover entity IDs: %w", err) } defer func() { _ = rows.Close() }() + covered := make(map[int64]struct{}) for rows.Next() { - var scope string var id int64 - if scanErr := rows.Scan(&scope, &id); scanErr != nil { - return fmt.Errorf("browse cover flags scan: %w", scanErr) + if scanErr := rows.Scan(&id); scanErr != nil { + return nil, fmt.Errorf("scan cover entity ID: %w", scanErr) } - switch scope { - case "media": - for _, idx := range mediaIndex[id] { - results[idx].HasCover = true - } - case "title": - for _, idx := range titleIndex[id] { - results[idx].HasCover = true - } + covered[id] = struct{}{} + } + if rowsErr := rows.Err(); rowsErr != nil { + return nil, rowsErr + } + return covered, nil +} + +func fetchCoverStatuses( + ctx context.Context, db sqlQueryable, refs []database.MediaCoverRef, +) (map[int64]bool, error) { + statuses := make(map[int64]bool, len(refs)) + results := make([]database.SearchResultWithCursor, 0, len(refs)) + seen := make(map[int64]struct{}, len(refs)) + for _, ref := range refs { + if ref.MediaDBID <= 0 { + continue } + if _, ok := seen[ref.MediaDBID]; ok { + continue + } + seen[ref.MediaDBID] = struct{}{} + statuses[ref.MediaDBID] = false + results = append(results, database.SearchResultWithCursor{ + MediaID: ref.MediaDBID, + MediaTitleID: ref.MediaTitleDBID, + }) } - if err = rows.Err(); err != nil { - return fmt.Errorf("browse cover flags rows: %w", err) + if err := fetchAndAttachCoverFlags(ctx, db, results); err != nil { + return nil, err } - return nil + for i := range results { + statuses[results[i].MediaID] = results[i].HasCover + } + return statuses, nil } // backfillMissingTitleIDs populates MediaTitleID for any result that has a MediaID diff --git a/pkg/database/mediadb/sql_browse_test.go b/pkg/database/mediadb/sql_browse_test.go index 7a0bc65c3..55dfc8fb7 100644 --- a/pkg/database/mediadb/sql_browse_test.go +++ b/pkg/database/mediadb/sql_browse_test.go @@ -919,13 +919,14 @@ func TestFetchAndAttachCoverFlags_NoCoverEntries(t *testing.T) { {MediaID: 2, MediaTitleID: 102, Name: "AnotherNoCoverGame"}, } - // Query returns no rows — neither media ID has any image property. Title IDs - // are populated so no backfill query runs; the media leg binds mediaIDs and - // the title leg binds titleIDs. + // Neither title has an image, so both media IDs proceed to fallback. expectImagePropertyTagLookup(mock, 901) - mock.ExpectQuery(`SELECT 'media' AS Scope, mp\.MediaDBID AS ID`). - WithArgs(int64(1), int64(2), int64(901), int64(101), int64(102), int64(901)). - WillReturnRows(sqlmock.NewRows([]string{"Scope", "ID"})) + mock.ExpectQuery(`SELECT mtp\.MediaTitleDBID`). + WithArgs(int64(101), int64(102), int64(901)). + WillReturnRows(sqlmock.NewRows([]string{"MediaTitleDBID"})) + mock.ExpectQuery(`SELECT mp\.MediaDBID`). + WithArgs(int64(1), int64(2), int64(901)). + WillReturnRows(sqlmock.NewRows([]string{"MediaDBID"})) err = fetchAndAttachCoverFlags(context.Background(), db, results) require.NoError(t, err) @@ -945,11 +946,14 @@ func TestFetchAndAttachCoverFlags_MediaLevelCover(t *testing.T) { {MediaID: 20, MediaTitleID: 120, Name: "GameWithoutCover"}, } - // Query returns mediaID 10 as having a cover (media-level property). + // Neither title has an image; media fallback finds mediaID 10 only. expectImagePropertyTagLookup(mock, 901) - mock.ExpectQuery(`SELECT 'media' AS Scope, mp\.MediaDBID AS ID`). - WithArgs(int64(10), int64(20), int64(901), int64(110), int64(120), int64(901)). - WillReturnRows(sqlmock.NewRows([]string{"Scope", "ID"}).AddRow("media", int64(10))) + mock.ExpectQuery(`SELECT mtp\.MediaTitleDBID`). + WithArgs(int64(110), int64(120), int64(901)). + WillReturnRows(sqlmock.NewRows([]string{"MediaTitleDBID"})) + mock.ExpectQuery(`SELECT mp\.MediaDBID`). + WithArgs(int64(10), int64(20), int64(901)). + WillReturnRows(sqlmock.NewRows([]string{"MediaDBID"}).AddRow(int64(10))) err = fetchAndAttachCoverFlags(context.Background(), db, results) require.NoError(t, err) @@ -971,11 +975,11 @@ func TestFetchAndAttachCoverFlags_TitleLevelCover(t *testing.T) { {MediaID: 31, MediaTitleID: 100, Name: "GameA (Rev B)"}, } - // Query returns the shared title ID from the title-level UNION ALL leg. + // Shared title cover resolves both entries, so no media fallback runs. expectImagePropertyTagLookup(mock, 901) - mock.ExpectQuery(`SELECT 'media' AS Scope, mp\.MediaDBID AS ID`). - WithArgs(int64(30), int64(31), int64(901), int64(100), int64(901)). - WillReturnRows(sqlmock.NewRows([]string{"Scope", "ID"}).AddRow("title", int64(100))) + mock.ExpectQuery(`SELECT mtp\.MediaTitleDBID`). + WithArgs(int64(100), int64(901)). + WillReturnRows(sqlmock.NewRows([]string{"MediaTitleDBID"}).AddRow(int64(100))) err = fetchAndAttachCoverFlags(context.Background(), db, results) require.NoError(t, err) @@ -995,8 +999,8 @@ func TestFetchAndAttachCoverFlags_QueryError(t *testing.T) { } expectImagePropertyTagLookup(mock, 901) - mock.ExpectQuery(`SELECT 'media' AS Scope, mp\.MediaDBID AS ID`). - WithArgs(int64(5), int64(901), int64(105), int64(901)). + mock.ExpectQuery(`SELECT mtp\.MediaTitleDBID`). + WithArgs(int64(105), int64(901)). WillReturnError(errors.New("db unavailable")) err = fetchAndAttachCoverFlags(context.Background(), db, results) @@ -1079,6 +1083,83 @@ func TestFetchAndAttachCoverFlags_Integration_MediaLevelProperty(t *testing.T) { require.NoError(t, fetchAndAttachCoverFlags(ctx, mediaDB.sql.Load(), results)) assert.True(t, results[0].HasCover, "media with image property should have HasCover=true") assert.False(t, results[1].HasCover, "media without image property should have HasCover=false") + + statuses, err := mediaDB.GetMediaCoverStatus(ctx, []database.MediaCoverRef{ + {MediaDBID: mediaA.DBID, MediaTitleDBID: titleA.DBID}, + {MediaDBID: mediaB.DBID, MediaTitleDBID: titleB.DBID}, + {MediaDBID: mediaA.DBID, MediaTitleDBID: titleA.DBID}, + {}, + }) + require.NoError(t, err) + assert.Equal(t, map[int64]bool{mediaA.DBID: true, mediaB.DBID: false}, statuses) +} + +func TestCoverAvailabilityIndex_AsyncBuildAndInvalidation(t *testing.T) { + mediaDB, cleanup := setupTempMediaDB(t) + defer cleanup() + seedImagePropertyTags(t, mediaDB) + + ctx := context.Background() + sys, err := mediaDB.FindOrInsertSystem(database.System{SystemID: "NES", Name: "NES"}) + require.NoError(t, err) + nesSystem, err := systemdefs.GetSystem("NES") + require.NoError(t, err) + + require.NoError(t, mediaDB.BeginTransaction(false)) + title, err := mediaDB.InsertMediaTitle(&database.MediaTitle{ + SystemDBID: sys.DBID, + Slug: slugs.Slugify(nesSystem.GetMediaType(), "Cached Cover"), + Name: "Cached Cover", + }) + require.NoError(t, err) + media, err := mediaDB.InsertMedia(database.Media{ + SystemDBID: sys.DBID, + MediaTitleDBID: title.DBID, + Path: filepath.Join("roms", "nes", "cached_cover.nes"), + ParentDir: filepath.ToSlash(filepath.Join("roms", "nes")) + "/", + }) + require.NoError(t, err) + require.NoError(t, mediaDB.CommitTransaction()) + require.NoError(t, mediaDB.UpsertMediaTitleProperties(ctx, title.DBID, []database.MediaProperty{ + {TypeTag: tags.PropertyTypeTag(tags.TagPropertyImageBoxart), Text: filepath.Join("art", "cached.png")}, + })) + + mediaDB.TrackBackgroundOperation() + firstPass := []database.SearchResultWithCursor{{MediaID: media.DBID, MediaTitleID: title.DBID}} + require.NoError(t, fetchAndAttachCoverFlags(ctx, mediaDB.sql.Load(), firstPass)) + require.True(t, firstPass[0].HasCover, "SQL fallback must serve first request") + assert.Nil(t, cachedCoverAvailabilityIndex(mediaDB.sql.Load()), + "cache build must not compete with existing background work") + mediaDB.BackgroundOperationDone() + + buildPass := []database.SearchResultWithCursor{{MediaID: media.DBID, MediaTitleID: title.DBID}} + require.NoError(t, fetchAndAttachCoverFlags(ctx, mediaDB.sql.Load(), buildPass)) + require.True(t, buildPass[0].HasCover) + mediaDB.WaitForBackgroundOperations() + + index := cachedCoverAvailabilityIndex(mediaDB.sql.Load()) + require.NotNil(t, index) + assert.True(t, index.hasTitle(title.DBID)) + assert.False(t, index.hasMedia(media.DBID)) + + statuses, err := mediaDB.GetMediaCoverStatus(ctx, []database.MediaCoverRef{ + {MediaDBID: media.DBID, MediaTitleDBID: title.DBID}, + {MediaDBID: media.DBID + 1, MediaTitleDBID: title.DBID + 1}, + }) + require.NoError(t, err) + assert.Equal(t, map[int64]bool{media.DBID: true, media.DBID + 1: false}, statuses) + + require.NoError(t, mediaDB.DeleteMediaTitleProperty(ctx, title.DBID, 901)) + assert.Nil(t, cachedCoverAvailabilityIndex(mediaDB.sql.Load()), "image deletion must invalidate index") + + secondPass := []database.SearchResultWithCursor{{MediaID: media.DBID, MediaTitleID: title.DBID}} + require.NoError(t, fetchAndAttachCoverFlags(ctx, mediaDB.sql.Load(), secondPass)) + assert.False(t, secondPass[0].HasCover, "fallback must observe deleted cover") + mediaDB.WaitForBackgroundOperations() + + index = cachedCoverAvailabilityIndex(mediaDB.sql.Load()) + require.NotNil(t, index) + assert.False(t, index.hasTitle(title.DBID)) } func TestFetchAndAttachCoverFlags_Integration_TitleLevelProperty(t *testing.T) { diff --git a/pkg/database/mediadb/sql_scraper.go b/pkg/database/mediadb/sql_scraper.go index b88d8dd92..0111b24cc 100644 --- a/pkg/database/mediadb/sql_scraper.go +++ b/pkg/database/mediadb/sql_scraper.go @@ -163,7 +163,7 @@ func findMediaIDsByPathBatch(ctx context.Context, db sqlQueryable, paths []strin //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?". rows, err := db.QueryContext(ctx, ` - SELECT s.SystemID, m.Path, m.DBID + SELECT s.SystemID, m.Path, m.DBID, m.MediaTitleDBID FROM Media m INNER JOIN Systems s ON m.SystemDBID = s.DBID WHERE m.Path IN (`+prepareVariadic("?", ",", len(paths))+`) @@ -180,7 +180,7 @@ func findMediaIDsByPathBatch(ctx context.Context, db sqlQueryable, paths []strin results := make([]database.MediaPathID, 0, len(paths)) for rows.Next() { var row database.MediaPathID - if err := rows.Scan(&row.SystemID, &row.Path, &row.DBID); err != nil { + if err := rows.Scan(&row.SystemID, &row.Path, &row.DBID, &row.MediaTitleDBID); err != nil { return nil, fmt.Errorf("failed to scan FindMediaIDsByPaths: %w", err) } results = append(results, row) @@ -1463,6 +1463,11 @@ func (db *MediaDB) UpsertMediaTitleProperties( if db.sql.Load() == nil { return ErrNullSQL } + invalidateCoverIndex := containsImageProperty(props) + if invalidateCoverIndex { + clearCoverAvailabilityCacheFor(db.sql.Load()) + defer clearCoverAvailabilityCacheFor(db.sql.Load()) + } tx, err := db.sql.Load().BeginTx(ctx, nil) if err != nil { return fmt.Errorf("UpsertMediaTitleProperties: begin transaction: %w", err) @@ -1552,6 +1557,11 @@ func (db *MediaDB) UpsertMediaProperties(ctx context.Context, mediaDBID int64, p if db.sql.Load() == nil { return ErrNullSQL } + invalidateCoverIndex := containsImageProperty(props) + if invalidateCoverIndex { + clearCoverAvailabilityCacheFor(db.sql.Load()) + defer clearCoverAvailabilityCacheFor(db.sql.Load()) + } if db.inTransaction { return upsertMediaProperties(ctx, db.conn(), mediaDBID, props) } @@ -2369,6 +2379,8 @@ func (db *MediaDB) DeleteMediaTitleProperty(ctx context.Context, mediaTitleDBID, if db.sql.Load() == nil { return ErrNullSQL } + clearCoverAvailabilityCacheFor(db.sql.Load()) + defer clearCoverAvailabilityCacheFor(db.sql.Load()) _, err := db.sql.Load().ExecContext(ctx, `DELETE FROM MediaTitleProperties WHERE MediaTitleDBID = ? AND TypeTagDBID = ?`, mediaTitleDBID, typeTagDBID, @@ -2387,6 +2399,8 @@ func (db *MediaDB) DeleteMediaProperty(ctx context.Context, mediaDBID, typeTagDB if db.sql.Load() == nil { return ErrNullSQL } + clearCoverAvailabilityCacheFor(db.sql.Load()) + defer clearCoverAvailabilityCacheFor(db.sql.Load()) _, err := db.sql.Load().ExecContext(ctx, `DELETE FROM MediaProperties WHERE MediaDBID = ? AND TypeTagDBID = ?`, mediaDBID, typeTagDBID, @@ -2399,6 +2413,15 @@ func (db *MediaDB) DeleteMediaProperty(ctx context.Context, mediaDBID, typeTagDB return nil } +func containsImageProperty(props []database.MediaProperty) bool { + for i := range props { + if isImageProperty(props[i].TypeTag) { + return true + } + } + return false +} + // resolvePropertyTypeTag looks up the DBID of the Tags row for the given full // tag string (e.g. "property:description"). The tag must already exist in the DB // (seeded by SeedCanonicalTags). Returns an error if not found. diff --git a/pkg/database/mediadb/sql_scraper_test.go b/pkg/database/mediadb/sql_scraper_test.go index dfd98cfdc..b7dfb5d1f 100644 --- a/pkg/database/mediadb/sql_scraper_test.go +++ b/pkg/database/mediadb/sql_scraper_test.go @@ -156,8 +156,8 @@ func TestFindMediaIDsByPaths_ReturnsSamePathAcrossSystems(t *testing.T) { results, err := mediaDB.FindMediaIDsByPaths(ctx, []string{mediaPath}) require.NoError(t, err) assert.ElementsMatch(t, []database.MediaPathID{ - {SystemID: "NES", Path: mediaPath, DBID: 1}, - {SystemID: "SNES", Path: mediaPath, DBID: 2}, + {SystemID: "NES", Path: mediaPath, DBID: 1, MediaTitleDBID: 1}, + {SystemID: "SNES", Path: mediaPath, DBID: 2, MediaTitleDBID: 2}, }, results) } @@ -185,8 +185,8 @@ func TestFindMediaIDsByPaths_ChunksLargeInput(t *testing.T) { results, err := mediaDB.FindMediaIDsByPaths(ctx, paths) require.NoError(t, err) assert.ElementsMatch(t, []database.MediaPathID{ - {SystemID: "NES", Path: marioPath, DBID: 1}, - {SystemID: "NES", Path: zeldaPath, DBID: 2}, + {SystemID: "NES", Path: marioPath, DBID: 1, MediaTitleDBID: 1}, + {SystemID: "NES", Path: zeldaPath, DBID: 2, MediaTitleDBID: 2}, }, results) } diff --git a/pkg/testing/helpers/db_mocks.go b/pkg/testing/helpers/db_mocks.go index 380a4f2bd..6fcf46953 100644 --- a/pkg/testing/helpers/db_mocks.go +++ b/pkg/testing/helpers/db_mocks.go @@ -2697,6 +2697,23 @@ func (m *MockMediaDBI) BrowseFiles( return []database.SearchResultWithCursor{}, nil } +func (m *MockMediaDBI) GetMediaCoverStatus( + ctx context.Context, refs []database.MediaCoverRef, +) (map[int64]bool, error) { + if !m.hasExpectedCall("GetMediaCoverStatus") { + return map[int64]bool{}, nil + } + args := m.Called(ctx, refs) + statuses, ok := args.Get(0).(map[int64]bool) + if !ok { + statuses = map[int64]bool{} + } + if err := args.Error(1); err != nil { + return statuses, fmt.Errorf("mock get media cover status failed: %w", err) + } + return statuses, nil +} + func (m *MockMediaDBI) BrowseFileCount( ctx context.Context, opts database.BrowseFileCountOptions, ) (int, error) { From 24214abfb156d026d8c618e6f2858b544f2293eb Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 06:44:37 +0800 Subject: [PATCH 05/11] feat(api): expose media cover availability Return an always-present hasCover flag from media.search and media.history using Core's batched media/title artwork lookup. This lets clients plan artwork requests without reading Core-owned database tables. Inspired by Giancarlo Erra's artwork, browse, and recents performance work in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra --- docs/api/methods.md | 3 ++ pkg/api/methods/media.go | 16 +++++++++ pkg/api/methods/media_history.go | 35 ++++++++++++++++++-- pkg/api/methods/media_history_test.go | 40 +++++++++++++++++++++++ pkg/api/methods/media_response_helpers.go | 40 ++++++++++++++++++----- pkg/api/methods/media_search_test.go | 40 +++++++++++++++++++++++ pkg/api/models/responses.go | 2 ++ 7 files changed, 166 insertions(+), 10 deletions(-) diff --git a/docs/api/methods.md b/docs/api/methods.md index 60c51b7d9..af52c3563 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -557,6 +557,7 @@ An object: | name | string | Yes | A human-readable version of the result's filename without a file extension. | | path | string | Yes | Canonical indexed media path. Use with `system.id` for `media.meta` and `media.image`. | | relativePath | string | No | Launcher-relative convenience path, when it can be derived. Not a stable media identity. | +| hasCover | boolean | Yes | Whether media-level or title-level image properties are available. | | zapScript | string | Yes | ZapScript command to launch this media item. Includes the disambiguating tags inline (e.g. `@Arcade/X-Men Vs. Street Fighter (region:eu) (builddate:1996-10-04)`) so the written command resolves back to this specific variant. | | tags | [TagInfo](#taginfo-object)[] | Yes | Array of tags associated with this media item. | | disambiguatingTags | [TagInfo](#taginfo-object)[] | No | Subset of `tags` whose values differ across same-named siblings of this title, ordered by display importance. Omitted when the title has nothing to disambiguate. Clients can render these to tell variants apart. | @@ -1450,6 +1451,7 @@ Optionally, an object: | mediaName | string | Yes | Display name of the media. | | mediaPath | string | Yes | Path to the media file. | | relativePath | string | No | Launcher-relative convenience path, when it can be derived. Not a stable media identity. | +| hasCover | boolean | Yes | Whether media-level or title-level image properties are available. | | launcherId | string | Yes | ID of the launcher used. | | startedAt | string | Yes | Timestamp when media started in RFC3339 format. | | endedAt | string | No | Timestamp when media stopped in RFC3339 format. Omitted if media is still active. | @@ -1486,6 +1488,7 @@ Optionally, an object: "mediaName": "Super Mario World", "mediaPath": "/roms/snes/Super Mario World (USA).sfc", "relativePath": "snes/Super Mario World (USA).sfc", + "hasCover": true, "launcherId": "SNES", "startedAt": "2025-01-22T14:30:00Z", "endedAt": "2025-01-22T15:15:30Z", diff --git a/pkg/api/methods/media.go b/pkg/api/methods/media.go index e5db6f368..5a41c3528 100644 --- a/pkg/api/methods/media.go +++ b/pkg/api/methods/media.go @@ -1009,6 +1009,21 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic searchResults = searchResults[:maxResults] } + coverStatuses := make(map[int64]bool) + if len(searchResults) > 0 { + coverRefs := make([]database.MediaCoverRef, len(searchResults)) + for i := range searchResults { + coverRefs[i] = database.MediaCoverRef{ + MediaDBID: searchResults[i].MediaID, + MediaTitleDBID: searchResults[i].MediaTitleID, + } + } + coverStatuses, err = env.Database.MediaDB.GetMediaCoverStatus(ctx, coverRefs) + if err != nil { + return nil, fmt.Errorf("get media search cover status: %w", err) + } + } + // Convert to API models var rootDirs []string if env.LauncherCache != nil && env.Platform != nil { @@ -1040,6 +1055,7 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic results = append(results, models.SearchResultMedia{ MediaID: result.MediaID, RelPath: relPath, + HasCover: coverStatuses[result.MediaID], System: resultSystem, Name: result.Name, Path: result.Path, diff --git a/pkg/api/methods/media_history.go b/pkg/api/methods/media_history.go index 79a4f711c..c19aaaf4e 100644 --- a/pkg/api/methods/media_history.go +++ b/pkg/api/methods/media_history.go @@ -104,7 +104,36 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti }) } enrichStarted := time.Now() - mediaIDs := mediaResponseMediaIDs(&env, mediaRefs) + mediaRows, err := resolveMediaPathIDs(env.Context, env.Database.MediaDB, mediaRefs) + if err != nil { + return nil, fmt.Errorf("resolve media history cover identities: %w", err) + } + mediaIDs := make(map[mediaPathRef]int64, len(mediaRows)) + coverRefs := make([]database.MediaCoverRef, 0, len(mediaRows)) + seenIDs := make(map[int64]struct{}, len(mediaRows)) + for _, ref := range mediaRefs { + row := mediaRows[ref] + if row.DBID <= 0 { + continue + } + mediaIDs[ref] = row.DBID + if _, ok := seenIDs[row.DBID]; ok { + continue + } + seenIDs[row.DBID] = struct{}{} + coverRefs = append(coverRefs, database.MediaCoverRef{ + MediaDBID: row.DBID, + MediaTitleDBID: row.MediaTitleDBID, + }) + } + + coverStatuses := make(map[int64]bool) + if len(coverRefs) > 0 { + coverStatuses, err = env.Database.MediaDB.GetMediaCoverStatus(env.Context, coverRefs) + if err != nil { + return nil, fmt.Errorf("get media history cover status: %w", err) + } + } enrichElapsed := time.Since(enrichStarted) buildStarted := time.Now() @@ -119,10 +148,12 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti formatted := entry.EndTime.Format(time.RFC3339) endedAt = &formatted } + mediaID := mediaIDs[ref] responseEntries = append(responseEntries, models.MediaHistoryResponseEntry{ - MediaID: mediaIDs[ref], + MediaID: mediaID, RelPath: mediaResponseRelativePath(&env, entry.SystemID, entry.MediaPath), + HasCover: coverStatuses[mediaID], SystemID: entry.SystemID, SystemName: entry.SystemName, MediaName: entry.MediaName, diff --git a/pkg/api/methods/media_history_test.go b/pkg/api/methods/media_history_test.go index 48b07690e..3a920fc9a 100644 --- a/pkg/api/methods/media_history_test.go +++ b/pkg/api/methods/media_history_test.go @@ -78,6 +78,7 @@ func TestHandleMediaHistory_NoParams(t *testing.T) { assert.Equal(t, "Super Mario Bros", resp.Entries[0].MediaName) assert.Equal(t, 1800, resp.Entries[0].PlayTime) assert.NotNil(t, resp.Entries[0].EndedAt) + assert.False(t, resp.Entries[0].HasCover) assert.NotNil(t, resp.Pagination) assert.False(t, resp.Pagination.HasNextPage) assert.Equal(t, 25, resp.Pagination.PageSize) @@ -153,6 +154,45 @@ func TestHandleMediaHistory_WithMediaIDAndRelativePath(t *testing.T) { mockMediaDB.AssertExpectations(t) } +func TestHandleMediaHistory_IncludesCoverStatus(t *testing.T) { + t.Parallel() + + mockUserDB := helpers.NewMockUserDBI() + mockMediaDB := helpers.NewMockMediaDBI() + now := time.Now() + coveredPath := filepath.Join(string(filepath.Separator), "games", "covered.nes") + uncoveredPath := filepath.Join(string(filepath.Separator), "games", "uncovered.nes") + mockUserDB.On("GetMediaHistory", []string(nil), int64(0), 26). + Return([]database.MediaHistoryEntry{ + {DBID: 2, SystemID: "NES", MediaPath: coveredPath, MediaName: "Covered", StartTime: now}, + {DBID: 1, SystemID: "NES", MediaPath: uncoveredPath, MediaName: "Uncovered", StartTime: now}, + }, nil) + mockMediaDB.On("FindMediaIDsByPaths", mock.Anything, []string{coveredPath, uncoveredPath}). + Return([]database.MediaPathID{ + {SystemID: "NES", Path: coveredPath, DBID: 20, MediaTitleDBID: 200}, + {SystemID: "NES", Path: uncoveredPath, DBID: 10, MediaTitleDBID: 100}, + }, nil) + mockMediaDB.On("GetMediaCoverStatus", mock.Anything, []database.MediaCoverRef{ + {MediaDBID: 20, MediaTitleDBID: 200}, + {MediaDBID: 10, MediaTitleDBID: 100}, + }).Return(map[int64]bool{20: true, 10: false}, nil) + + env := requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{UserDB: mockUserDB, MediaDB: mockMediaDB}, + Params: json.RawMessage(`{}`), + } + result, err := HandleMediaHistory(env) + require.NoError(t, err) + response, ok := result.(models.MediaHistoryResponse) + require.True(t, ok) + require.Len(t, response.Entries, 2) + assert.True(t, response.Entries[0].HasCover) + assert.False(t, response.Entries[1].HasCover) + mockUserDB.AssertExpectations(t) + mockMediaDB.AssertExpectations(t) +} + func TestMediaResponseMediaIDs_BoundsSlowLookup(t *testing.T) { t.Parallel() diff --git a/pkg/api/methods/media_response_helpers.go b/pkg/api/methods/media_response_helpers.go index 2a738bf8f..cb8b2c51e 100644 --- a/pkg/api/methods/media_response_helpers.go +++ b/pkg/api/methods/media_response_helpers.go @@ -21,6 +21,7 @@ package methods import ( "context" + "fmt" "time" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" @@ -123,9 +124,11 @@ func toPlaylistState(p *playlists.Playlist) models.PlaylistState { } } -func mediaIDsByPath(ctx context.Context, db database.MediaDBI, refs []mediaPathRef) map[mediaPathRef]int64 { +func resolveMediaPathIDs( + ctx context.Context, db database.MediaDBI, refs []mediaPathRef, +) (map[mediaPathRef]database.MediaPathID, error) { if db == nil || len(refs) == 0 { - return nil + return map[mediaPathRef]database.MediaPathID{}, nil } wanted := make(map[mediaPathRef]bool, len(refs)) @@ -142,33 +145,54 @@ func mediaIDsByPath(ctx context.Context, db database.MediaDBI, refs []mediaPathR } } if len(paths) == 0 { - return nil + return map[mediaPathRef]database.MediaPathID{}, nil } started := time.Now() rows, err := db.FindMediaIDsByPaths(ctx, paths) if err != nil { - log.Debug().Err(err).Msg("could not resolve media IDs by path") - return nil + return nil, fmt.Errorf("resolve media IDs by path: %w", err) } - mediaIDs := make(map[mediaPathRef]int64, len(rows)) + resolved := make(map[mediaPathRef]database.MediaPathID, len(rows)) for _, row := range rows { if row.DBID <= 0 { continue } ref := mediaPathRef{SystemID: row.SystemID, Path: row.Path} if wanted[ref] { - mediaIDs[ref] = row.DBID + resolved[ref] = row } } log.Debug(). Int("refs", len(refs)). Int("paths", len(paths)). - Int("resolved", len(mediaIDs)). + Int("resolved", len(resolved)). Dur("duration", time.Since(started)). Msg("media ID enrichment timing") + return resolved, nil +} + +func resolveMediaIDsByPath( + ctx context.Context, db database.MediaDBI, refs []mediaPathRef, +) (map[mediaPathRef]int64, error) { + rows, err := resolveMediaPathIDs(ctx, db, refs) + if err != nil { + return nil, err + } + mediaIDs := make(map[mediaPathRef]int64, len(rows)) + for ref, row := range rows { + mediaIDs[ref] = row.DBID + } + return mediaIDs, nil +} +func mediaIDsByPath(ctx context.Context, db database.MediaDBI, refs []mediaPathRef) map[mediaPathRef]int64 { + mediaIDs, err := resolveMediaIDsByPath(ctx, db, refs) + if err != nil { + log.Debug().Err(err).Msg("could not resolve media IDs by path") + return nil + } return mediaIDs } diff --git a/pkg/api/methods/media_search_test.go b/pkg/api/methods/media_search_test.go index 7f38e0b7e..7f7686d31 100644 --- a/pkg/api/methods/media_search_test.go +++ b/pkg/api/methods/media_search_test.go @@ -253,9 +253,49 @@ func TestHandleMediaSearch_WithoutCursor(t *testing.T) { assert.Equal(t, "NES", searchResults.Results[0].System.ID) assert.Equal(t, "Mario Bros", searchResults.Results[0].Name) assert.Equal(t, "/games/mario.nes", searchResults.Results[0].Path) + assert.False(t, searchResults.Results[0].HasCover) } } +func TestHandleMediaSearch_IncludesCoverStatus(t *testing.T) { + t.Parallel() + + mockMediaDB := helpers.NewMockMediaDBI() + mockMediaDB.On("SearchMediaWithFilters", mock.Anything, mock.Anything). + Return([]database.SearchResultWithCursor{ + { + SystemID: "NES", Name: "Covered", Path: filepath.Join("games", "covered.nes"), + MediaID: 1, MediaTitleID: 11, + }, + { + SystemID: "NES", Name: "Uncovered", Path: filepath.Join("games", "uncovered.nes"), + MediaID: 2, MediaTitleID: 22, + }, + }, nil) + mockMediaDB.On("GetMediaCoverStatus", mock.Anything, []database.MediaCoverRef{ + {MediaDBID: 1, MediaTitleDBID: 11}, + {MediaDBID: 2, MediaTitleDBID: 22}, + }).Return(map[int64]bool{1: true, 2: false}, nil) + + paramsJSON, err := json.Marshal(models.SearchParams{}) + require.NoError(t, err) + result, err := HandleMediaSearch(requests.RequestEnv{ + Context: context.Background(), + Params: paramsJSON, + Database: &database.Database{ + MediaDB: mockMediaDB, + }, + }) + require.NoError(t, err) + + response, ok := result.(models.SearchResults) + require.True(t, ok) + require.Len(t, response.Results, 2) + assert.True(t, response.Results[0].HasCover) + assert.False(t, response.Results[1].HasCover) + mockMediaDB.AssertExpectations(t) +} + func TestHandleMediaSearch_WithExplicitSort(t *testing.T) { t.Parallel() diff --git a/pkg/api/models/responses.go b/pkg/api/models/responses.go index 003d12800..5814d52bb 100644 --- a/pkg/api/models/responses.go +++ b/pkg/api/models/responses.go @@ -66,6 +66,7 @@ type SearchResultMedia struct { Tags []database.TagInfo `json:"tags"` DisambiguatingTags []database.TagInfo `json:"disambiguatingTags,omitempty"` MediaID int64 `json:"mediaId,omitempty"` + HasCover bool `json:"hasCover"` } type PaginationInfo struct { @@ -287,6 +288,7 @@ type MediaHistoryResponseEntry struct { StartedAt string `json:"startedAt"` PlayTime int `json:"playTime"` MediaID int64 `json:"mediaId,omitempty"` + HasCover bool `json:"hasCover"` } type MediaHistoryResponse struct { From ee18f1f0541093b80fe40b0a8edbdf827b5b047b Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 06:49:09 +0800 Subject: [PATCH 06/11] chore(api): add media search stage timings Measure semaphore wait, database search, cover lookup, response construction, system metadata, ZapScript, and relative-path stages independently. This makes remaining target-device search latency attributable without changing API behavior. Motivated by MiSTer performance measurements from ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 --- pkg/api/methods/media.go | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/pkg/api/methods/media.go b/pkg/api/methods/media.go index 5a41c3528..8c421346d 100644 --- a/pkg/api/methods/media.go +++ b/pkg/api/methods/media.go @@ -905,6 +905,8 @@ func searchResultSystem( func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic // single-use parameter in API handler log.Info().Msg("received media search request") + handlerStarted := time.Now() + semaphoreStarted := time.Now() select { case searchSem <- struct{}{}: @@ -912,6 +914,7 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic case <-env.Context.Done(): return nil, env.Context.Err() } + semaphoreDuration := time.Since(semaphoreStarted) var params models.SearchParams if err := validation.ValidateAndUnmarshal(env.Params, ¶ms); err != nil { @@ -923,7 +926,6 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic if params.MaxResults != nil && *params.MaxResults > 0 { maxResults = *params.MaxResults } - ctx := env.Context var sortOrder string @@ -998,7 +1000,9 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic Limit: limit, } + searchStarted := time.Now() searchResults, err = env.Database.MediaDB.SearchMediaWithFilters(ctx, &searchFilters) + searchDuration := time.Since(searchStarted) if err != nil { return nil, fmt.Errorf("error searching media with filters: %w", err) } @@ -1009,6 +1013,7 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic searchResults = searchResults[:maxResults] } + coverStarted := time.Now() coverStatuses := make(map[int64]bool) if len(searchResults) > 0 { coverRefs := make([]database.MediaCoverRef, len(searchResults)) @@ -1023,8 +1028,10 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic return nil, fmt.Errorf("get media search cover status: %w", err) } } + coverDuration := time.Since(coverStarted) // Convert to API models + responseBuildStarted := time.Now() var rootDirs []string if env.LauncherCache != nil && env.Platform != nil { rootDirs = env.Platform.RootDirs(env.Config) @@ -1039,11 +1046,20 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic } results := make([]models.SearchResultMedia, 0, len(searchResults)) + var systemBuildDuration time.Duration + var zapScriptDuration time.Duration + var relativePathDuration time.Duration for i := range searchResults { result := &searchResults[i] + stageStarted := time.Now() resultSystem := searchResultSystem(result.SystemID, launchableSystems) + systemBuildDuration += time.Since(stageStarted) + + stageStarted = time.Now() zapScript := result.ZapScript() + zapScriptDuration += time.Since(stageStarted) + stageStarted = time.Now() var relPath *string if env.LauncherCache != nil { rel := env.LauncherCache.ToRelativePath(rootDirs, result.SystemID, result.Path) @@ -1051,6 +1067,7 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic relPath = &rel } } + relativePathDuration += time.Since(stageStarted) results = append(results, models.SearchResultMedia{ MediaID: result.MediaID, @@ -1091,6 +1108,19 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic PageSize: maxResults, } } + responseBuildDuration := time.Since(responseBuildStarted) + + log.Debug(). + Int("rows", len(results)). + Dur("semaphoreDuration", semaphoreDuration). + Dur("searchDuration", searchDuration). + Dur("coverDuration", coverDuration). + Dur("responseBuildDuration", responseBuildDuration). + Dur("systemBuildDuration", systemBuildDuration). + Dur("zapScriptDuration", zapScriptDuration). + Dur("relativePathDuration", relativePathDuration). + Dur("handlerDuration", time.Since(handlerStarted)). + Msg("media search handler step timing") return models.SearchResults{ Results: results, From 03fe09ab63b51348a5440d429c24f4549319c240 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 07:06:25 +0800 Subject: [PATCH 07/11] perf(database): reuse fetched search tags Carry title IDs through search results and derive disambiguating ZapScript tags from media-scope rows already returned by the full tag query. Keep a small-page tag preflight while avoiding a redundant query for larger tagged result sets. Developed while attributing MiSTer search latency reported through ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 --- pkg/database/mediadb/disambiguation_test.go | 16 +++ pkg/database/mediadb/sql_search.go | 114 +++++++++++++++++--- pkg/database/mediadb/sql_search_test.go | 57 ++++++++-- pkg/database/mediadb/sql_test.go | 87 ++++++++------- 4 files changed, 205 insertions(+), 69 deletions(-) diff --git a/pkg/database/mediadb/disambiguation_test.go b/pkg/database/mediadb/disambiguation_test.go index 53f7013af..7ba935b5d 100644 --- a/pkg/database/mediadb/disambiguation_test.go +++ b/pkg/database/mediadb/disambiguation_test.go @@ -125,6 +125,22 @@ func TestRecomputeSystemDisambiguation_DifferingTagDisambiguates(t *testing.T) { assert.Equal(t, database.TagInfo{Type: "release", Tag: "USA"}, results[0].ZapScriptTags[0]) require.Len(t, results[1].ZapScriptTags, 1) assert.Equal(t, database.TagInfo{Type: "release", Tag: "Europe"}, results[1].ZapScriptTags[0]) + + // Search paths carrying title IDs reuse the already fetched media tags + // instead of issuing a second disambiguation query. + fetchedResults := []database.SearchResultWithCursor{ + { + MediaID: mediaIDs[0], MediaTitleID: titleDBID, Name: "Sonic", SystemID: "NES", + DisambiguationTypes: "release", + }, + { + MediaID: mediaIDs[1], MediaTitleID: titleDBID, Name: "Sonic", SystemID: "NES", + DisambiguationTypes: "release", + }, + } + require.NoError(t, attachTagsAndDisambiguation(ctx, mediaDB.sql.Load(), fetchedResults)) + assert.Equal(t, results[0].ZapScriptTags, fetchedResults[0].ZapScriptTags) + assert.Equal(t, results[1].ZapScriptTags, fetchedResults[1].ZapScriptTags) } // TestDisambiguationBackfill_RecomputesStaleTitlesAndStamps covers the one-time diff --git a/pkg/database/mediadb/sql_search.go b/pkg/database/mediadb/sql_search.go index bd2234fd3..a7e9ee497 100644 --- a/pkg/database/mediadb/sql_search.go +++ b/pkg/database/mediadb/sql_search.go @@ -38,6 +38,8 @@ import ( "github.com/rs/zerolog/log" ) +const tagPreflightMaxResults = 25 + // fetchAndAttachTags fetches tags for a slice of search results and attaches them to the results. // This helper consolidates duplicated tag-fetching logic across multiple search functions. // Combines file-level (MediaTags) and title-level (MediaTitleTags) tags via UNION ALL @@ -155,14 +157,25 @@ func fetchAndAttachTagsByResultIDs( ) error { tagIDs := collectResultTagIDs(results) unionStarted := time.Now() - hasTags, err := resultIDsHaveTags(ctx, db, tagIDs.mediaIDs, tagIDs.titleIDs) - if err != nil { - return err - } - if !hasTags { - finishAttachTags(results, nil) - logFetchAndAttachTagsTiming(results, 0, unionStarted) - return nil + var preflightDuration time.Duration + if len(results) <= tagPreflightMaxResults { + preflightStarted := time.Now() + hasTags, err := resultIDsHaveTags(ctx, db, tagIDs.mediaIDs, tagIDs.titleIDs) + preflightDuration = time.Since(preflightStarted) + if err != nil { + return err + } + if !hasTags { + finishAttachTags(results, nil) + attachZapScriptTagsFromFetchedTags(results, nil) + log.Debug(). + Int("rows", len(results)). + Dur("preflightDuration", preflightDuration). + Dur("duration", time.Since(unionStarted)). + Msg("fetch and attach tags by result IDs timing") + logFetchAndAttachTagsTiming(results, 0, unionStarted) + return nil + } } mediaPlaceholders := prepareVariadic("?", ",", len(tagIDs.mediaIDs)) @@ -201,6 +214,7 @@ func fetchAndAttachTagsByResultIDs( tagsArgs = append(tagsArgs, id) } + tagQueryStarted := time.Now() tagsStmt, err := db.PrepareContext(ctx, tagsQuery) if err != nil { return fmt.Errorf("failed to prepare tags query: %w", err) @@ -223,6 +237,7 @@ func fetchAndAttachTagsByResultIDs( tagsMap := make(map[int64][]database.TagInfo) seen := make(map[int64]map[tagKey]int) + mediaTagKeys := make(map[int64]map[tagKey]struct{}) for tagsRows.Next() { tagRow, scanErr := scanSourceTagRow(tagsRows) if scanErr != nil { @@ -232,6 +247,13 @@ func fetchAndAttachTagsByResultIDs( mediaIDsForTag := []int64{tagRow.sourceID} if tagRow.sourceKind == 1 { mediaIDsForTag = tagIDs.titleToMediaIDs[tagRow.sourceID] + } else { + keys := mediaTagKeys[tagRow.sourceID] + if keys == nil { + keys = make(map[tagKey]struct{}) + mediaTagKeys[tagRow.sourceID] = keys + } + keys[tagKey{typ: tagRow.tagType, tag: dbtags.UnpadTagValue(tagRow.tag)}] = struct{}{} } for _, mediaID := range mediaIDsForTag { appendTagInfo(tagsMap, seen, mediaID, tagRow.tag, tagRow.tagType, tagRow.label) @@ -242,6 +264,14 @@ func fetchAndAttachTagsByResultIDs( } finishAttachTags(results, tagsMap) + attachZapScriptTagsFromFetchedTags(results, mediaTagKeys) + log.Debug(). + Int("rows", len(results)). + Int("tagPairs", len(tagsMap)). + Dur("preflightDuration", preflightDuration). + Dur("tagQueryDuration", time.Since(tagQueryStarted)). + Dur("duration", time.Since(unionStarted)). + Msg("fetch and attach tags by result IDs timing") logFetchAndAttachTagsTiming(results, len(tagsMap), unionStarted) return nil } @@ -432,12 +462,55 @@ func attachTagsAndDisambiguation( db sqlQueryable, results []database.SearchResultWithCursor, ) error { + hasTitleIDs := allResultsHaveTitleIDs(results) if err := fetchAndAttachTags(ctx, db, results); err != nil { return err } + if hasTitleIDs { + return nil + } return attachZapScriptTags(ctx, db, results) } +func attachZapScriptTagsFromFetchedTags( + results []database.SearchResultWithCursor, + mediaTagKeys map[int64]map[tagKey]struct{}, +) { + for i := range results { + result := &results[i] + result.ZapScriptTags = []database.TagInfo{} + if result.DisambiguationTypes == "" { + continue + } + + disambiguatingTypes := make(map[string]struct{}) + for tagType := range strings.SplitSeq(result.DisambiguationTypes, ",") { + disambiguatingTypes[tagType] = struct{}{} + } + mediaKeys := mediaTagKeys[result.MediaID] + for _, tag := range result.Tags { + if _, ok := disambiguatingTypes[tag.Type]; !ok { + continue + } + if _, ok := mediaKeys[tagKey{typ: tag.Type, tag: tag.Tag}]; !ok { + continue + } + result.ZapScriptTags = append(result.ZapScriptTags, tag) + } + sortZapScriptTags(result.ZapScriptTags) + } +} + +func sortZapScriptTags(tags []database.TagInfo) { + sort.SliceStable(tags, func(a, b int) bool { + ra, rb := database.TagTypeDisplayRank(tags[a].Type), database.TagTypeDisplayRank(tags[b].Type) + if ra != rb { + return ra < rb + } + return tags[a].Tag < tags[b].Tag + }) +} + // attachZapScriptTags populates ZapScriptTags on each result with the tags that // disambiguate it from its same-title siblings. Disambiguation is precomputed and // stored per title in MediaTitles.DisambiguationTypes (see RecomputeTitleDisambiguation), @@ -450,6 +523,7 @@ func attachTagsAndDisambiguation( // treated as "title has no variants" and skips the lookup, so a result left empty by // mistake silently yields no ZapScriptTags. func attachZapScriptTags(ctx context.Context, db sqlQueryable, results []database.SearchResultWithCursor) error { + started := time.Now() for i := range results { if results[i].ZapScriptTags == nil { results[i].ZapScriptTags = []database.TagInfo{} @@ -476,6 +550,10 @@ func attachZapScriptTags(ctx context.Context, db sqlQueryable, results []databas mediaIDs = append(mediaIDs, id) } if len(mediaIDs) == 0 { + log.Debug(). + Int("rows", len(results)). + Dur("duration", time.Since(started)). + Msg("attach ZapScript tags timing") return nil } @@ -527,16 +605,16 @@ func attachZapScriptTags(ctx context.Context, db sqlQueryable, results []databas if zapTags, ok := byMedia[results[i].MediaID]; ok { // Order by display importance (variant flags, region, ... credit last) so // clients can render left-to-right and truncate. Within a type, sort by value. - sort.SliceStable(zapTags, func(a, b int) bool { - ra, rb := database.TagTypeDisplayRank(zapTags[a].Type), database.TagTypeDisplayRank(zapTags[b].Type) - if ra != rb { - return ra < rb - } - return zapTags[a].Tag < zapTags[b].Tag - }) + sortZapScriptTags(zapTags) results[i].ZapScriptTags = zapTags } } + log.Debug(). + Int("rows", len(results)). + Int("queriedMedia", len(mediaIDs)). + Int("tagPairs", len(byMedia)). + Dur("duration", time.Since(started)). + Msg("attach ZapScript tags timing") return nil } @@ -925,6 +1003,7 @@ func sqlSearchMediaWithFiltersSorted( MediaTitles.Name, Media.Path, Media.DBID, + MediaTitles.DBID, MediaTitles.DisambiguationTypes` + sortValueSelect + ` FROM Systems INNER JOIN MediaTitles ON Systems.DBID = MediaTitles.SystemDBID @@ -979,6 +1058,7 @@ func sqlSearchMediaWithFiltersSorted( &result.Name, &result.Path, &result.MediaID, + &result.MediaTitleID, &result.DisambiguationTypes, ) } else { @@ -987,6 +1067,7 @@ func sqlSearchMediaWithFiltersSorted( &result.Name, &result.Path, &result.MediaID, + &result.MediaTitleID, &result.DisambiguationTypes, &result.SortValue, ) @@ -1110,6 +1191,7 @@ func sqlSearchMediaByTitleDBIDsSorted( MediaTitles.Name, Media.Path, Media.DBID, + MediaTitles.DBID, MediaTitles.DisambiguationTypes` + sortValueSelect + ` FROM MediaTitles INNER JOIN Systems ON Systems.DBID = MediaTitles.SystemDBID @@ -1140,6 +1222,7 @@ func sqlSearchMediaByTitleDBIDsSorted( &r.Name, &r.Path, &r.MediaID, + &r.MediaTitleID, &r.DisambiguationTypes, ) } else { @@ -1148,6 +1231,7 @@ func sqlSearchMediaByTitleDBIDsSorted( &r.Name, &r.Path, &r.MediaID, + &r.MediaTitleID, &r.DisambiguationTypes, &r.SortValue, ) diff --git a/pkg/database/mediadb/sql_search_test.go b/pkg/database/mediadb/sql_search_test.go index 19fd8c04a..d168528ad 100644 --- a/pkg/database/mediadb/sql_search_test.go +++ b/pkg/database/mediadb/sql_search_test.go @@ -534,6 +534,41 @@ func TestFetchAndAttachTags_MultipleYearTags(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +func TestAttachTagsAndDisambiguation_ReusesFetchedMediaTags(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + + results := []database.SearchResultWithCursor{{ + MediaID: 1, + MediaTitleID: 10, + DisambiguationTypes: "release,region", + }} + mock.ExpectQuery("SELECT EXISTS"). + WithArgs(int64(1), int64(10)). + WillReturnRows(sqlmock.NewRows([]string{"hasTags"}).AddRow(true)) + mock.ExpectPrepare(`SELECT.*SourceKind.*SourceDBID.*Tags\.Tag.*TagTypes\.Type`). + ExpectQuery(). + WithArgs(int64(1), int64(10)). + WillReturnRows(sqlmock.NewRows([]string{ + "SourceKind", "SourceDBID", "Tag", "DisplayName", "Type", + }). + AddRow(0, int64(1), "USA", "USA", "release"). + AddRow(0, int64(1), "us", "United States", "region"). + AddRow(1, int64(10), "Global", "Global", "release"). + AddRow(1, int64(10), "Action", "Action", "genre")) + + require.NoError(t, attachTagsAndDisambiguation(context.Background(), db, results)) + require.Len(t, results[0].Tags, 4) + assert.Equal(t, []database.TagInfo{ + {Type: "region", Tag: "us", Label: "United States"}, + {Type: "release", Tag: "USA", Label: "USA"}, + }, results[0].ZapScriptTags) + assert.NoError(t, mock.ExpectationsWereMet()) +} + func TestSqlSearchMediaWithFilters_IntegrationWithTags(t *testing.T) { t.Parallel() db, mock, err := testsqlmock.NewSQLMock() @@ -548,22 +583,25 @@ func TestSqlSearchMediaWithFilters_IntegrationWithTags(t *testing.T) { includeName := false // Mock the main media query - mediaRows := sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("nes", "Super Mario Bros", "/games/mario.nes", int64(1), "") + mediaRows := sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow("nes", "Super Mario Bros", "/games/mario.nes", int64(1), int64(10), "") mock.ExpectPrepare(`SELECT.*FROM Systems.*WHERE Systems.SystemID IN`). ExpectQuery(). WithArgs("nes", "%mario%", "%mario%", limit). // Slug LIKE, SecondarySlug LIKE WillReturnRows(mediaRows) - // Mock the tags query - tagRows := sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type", "DisplayName"}). - AddRow(int64(1), "Action", "genre", "Action"). - AddRow(int64(1), "1985", "year", "1985") - - mock.ExpectPrepare(`SELECT.*MediaDBID.*Tag.*Type FROM`). + // Mock direct tag lookup using media and title IDs from the result query. + mock.ExpectQuery("SELECT EXISTS"). + WithArgs(int64(1), int64(10)). + WillReturnRows(sqlmock.NewRows([]string{"hasTags"}).AddRow(true)) + tagRows := sqlmock.NewRows([]string{"SourceKind", "SourceDBID", "Tag", "DisplayName", "Type"}). + AddRow(0, int64(1), "Action", "Action", "genre"). + AddRow(1, int64(10), "1985", "1985", "year") + mock.ExpectPrepare(`SELECT.*SourceKind.*SourceDBID.*Tags\.Tag.*TagTypes\.Type`). ExpectQuery(). - WithArgs(int64(1), int64(1)). + WithArgs(int64(1), int64(10)). WillReturnRows(tagRows) results, err := sqlSearchMediaWithFilters( @@ -572,6 +610,7 @@ func TestSqlSearchMediaWithFilters_IntegrationWithTags(t *testing.T) { require.NoError(t, err) require.Len(t, results, 1) assert.Equal(t, "Super Mario Bros", results[0].Name) + assert.Equal(t, int64(10), results[0].MediaTitleID) assert.Len(t, results[0].Tags, 2) assert.Equal(t, "Action", results[0].Tags[0].Tag) assert.Equal(t, "1985", results[0].Tags[1].Tag) diff --git a/pkg/database/mediadb/sql_test.go b/pkg/database/mediadb/sql_test.go index 5487089ef..209bdcb2e 100644 --- a/pkg/database/mediadb/sql_test.go +++ b/pkg/database/mediadb/sql_test.go @@ -637,15 +637,11 @@ func TestSqlSearchMediaWithFilters_WithTags(t *testing.T) { ExpectQuery(). // Slug LIKE, SecondarySlug LIKE, tag args doubled for both tag sources WithArgs("NES", "%mario%", "%mario%", "genre", "Action", "genre", "Action", 10). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("NES", "Mario", "/games/mario.nes", 1, "")) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow("NES", "Mario", "/games/mario.nes", 1, 10, "")) - // Mock second query: get tags for the media items - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(1, 1). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"}). - AddRow(1, "Action", "genre")) + expectSearchTagsQuery(mock, 1, 10) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, variantGroups, rawWords, tags, nil, nil, 10, includeName, @@ -672,11 +668,10 @@ const searchSystemFilterPattern = `(?s)WHERE\s+Systems\.SystemID IN \(\?(,\?)*\) const searchVariantSystemFilterPattern = `(?s)WHERE\s+Systems\.SystemID IN \(\?(,\?)*\) AND\s+` + `Media\.IsMissing = 0.*MediaTitles\.Slug LIKE.*LIMIT \?` -func expectSearchTagsQuery(mock sqlmock.Sqlmock, mediaID int64) { - mock.ExpectPrepare("(?s)SELECT.*MediaDBID.*Tags\\.Tag.*TagTypes\\.Type.*FROM Media"). - ExpectQuery(). - WithArgs(mediaID, mediaID). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "DisplayName", "Type"})) +func expectSearchTagsQuery(mock sqlmock.Sqlmock, mediaID, titleID int64) { + mock.ExpectQuery("SELECT EXISTS"). + WithArgs(mediaID, titleID). + WillReturnRows(sqlmock.NewRows([]string{"hasTags"}).AddRow(false)) // No disambiguation lookup is expected: these fixtures return an empty // DisambiguationTypes, so attachZapScriptTags skips the query entirely. } @@ -694,9 +689,10 @@ func TestSqlSearchMediaWithFilters_AllSystemsTagOnlySkipsSystemFilter(t *testing mock.ExpectPrepare("(?s)WHERE\\s+Media\\.IsMissing = 0.*ORDER BY Media\\.DBID ASC.*LIMIT \\?"). ExpectQuery(). WithArgs("user", "favorite", "user", "favorite", 10). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow(systems[0].ID, "Favorite", filepath.ToSlash(filepath.Join("roms", "favorite.rom")), 7, "")) - expectSearchTagsQuery(mock, 7) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow(systems[0].ID, "Favorite", filepath.ToSlash(filepath.Join("roms", "favorite.rom")), 7, 70, "")) + expectSearchTagsQuery(mock, 7, 70) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, nil, nil, tags, nil, nil, 10, false, @@ -728,9 +724,10 @@ func TestSqlSearchMediaWithFilters_DuplicateSystemsMissingOneKeepsSystemFilter(t mock.ExpectPrepare(searchSystemFilterPattern). ExpectQuery(). WithArgs(args...). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow(allSystems[0].ID, "Favorite", filepath.ToSlash(filepath.Join("roms", "favorite.rom")), 8, "")) - expectSearchTagsQuery(mock, 8) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow(allSystems[0].ID, "Favorite", filepath.ToSlash(filepath.Join("roms", "favorite.rom")), 8, 80, "")) + expectSearchTagsQuery(mock, 8, 80) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, nil, nil, tags, nil, nil, 10, false, @@ -760,9 +757,10 @@ func TestSqlSearchMediaWithFilters_NonTagDrivenKeepsSystemFilter(t *testing.T) { mock.ExpectPrepare(searchSystemFilterPattern). ExpectQuery(). WithArgs(args...). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow(systems[0].ID, "Favorite", filepath.ToSlash(filepath.Join("roms", "favorite.rom")), 9, "")) - expectSearchTagsQuery(mock, 9) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow(systems[0].ID, "Favorite", filepath.ToSlash(filepath.Join("roms", "favorite.rom")), 9, 90, "")) + expectSearchTagsQuery(mock, 9, 90) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, nil, nil, tags, nil, nil, 10, true, @@ -789,9 +787,10 @@ func TestSqlSearchMediaWithFilters_NonTagDrivenKeepsSystemFilter(t *testing.T) { mock.ExpectPrepare(searchVariantSystemFilterPattern). ExpectQuery(). WithArgs(args...). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow(systems[0].ID, "Mario", filepath.ToSlash(filepath.Join("roms", "mario.rom")), 10, "")) - expectSearchTagsQuery(mock, 10) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow(systems[0].ID, "Mario", filepath.ToSlash(filepath.Join("roms", "mario.rom")), 10, 100, "")) + expectSearchTagsQuery(mock, 10, 100) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, [][]string{{"mario"}}, []string{"mario"}, tags, nil, nil, 10, false, @@ -1922,21 +1921,23 @@ func TestSqlSearchMediaByTitleDBIDs_BasicLookup(t *testing.T) { mock.ExpectQuery("SELECT .+ FROM MediaTitles"). WithArgs(int64(10), int64(20), 100). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("NES", "Super Mario Bros", "/games/nes/smb.nes", 100, ""). - AddRow("NES", "Zelda", "/games/nes/zelda.nes", 200, "")) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }). + AddRow("NES", "Super Mario Bros", "/games/nes/smb.nes", 100, 10, ""). + AddRow("NES", "Zelda", "/games/nes/zelda.nes", 200, 20, "")) - // Tag query (fetchAndAttachTags uses PrepareContext) - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(100, 200, 100, 200). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + mock.ExpectQuery("SELECT EXISTS"). + WithArgs(100, 200, 10, 20). + WillReturnRows(sqlmock.NewRows([]string{"hasTags"}).AddRow(false)) results, err := sqlSearchMediaByTitleDBIDs( context.Background(), db, []int64{10, 20}, nil, nil, nil, 100) require.NoError(t, err) assert.Len(t, results, 2) assert.Equal(t, "Super Mario Bros", results[0].Name) + assert.Equal(t, int64(10), results[0].MediaTitleID) + assert.Equal(t, int64(20), results[1].MediaTitleID) assert.NoError(t, mock.ExpectationsWereMet()) } @@ -1957,13 +1958,11 @@ func TestSqlSearchMediaByTitleDBIDs_WithCursor(t *testing.T) { cursor := int64(150) mock.ExpectQuery("SELECT .+ FROM MediaTitles"). WithArgs(int64(10), cursor, 100). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("NES", "Zelda", "/games/nes/zelda.nes", 200, "")) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow("NES", "Zelda", "/games/nes/zelda.nes", 200, 10, "")) - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(200, 200). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + expectSearchTagsQuery(mock, 200, 10) results, err := sqlSearchMediaByTitleDBIDs( context.Background(), db, []int64{10}, nil, nil, &cursor, 100) @@ -1981,13 +1980,11 @@ func TestSqlSearchMediaByTitleDBIDs_WithLetter(t *testing.T) { letter := "S" mock.ExpectQuery("SELECT .+ FROM MediaTitles"). WithArgs(int64(10), letter, 100). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("NES", "Super Mario Bros", "/games/nes/smb.nes", 100, "")) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow("NES", "Super Mario Bros", "/games/nes/smb.nes", 100, 10, "")) - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(100, 100). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + expectSearchTagsQuery(mock, 100, 10) results, err := sqlSearchMediaByTitleDBIDs( context.Background(), db, []int64{10}, nil, &letter, nil, 100) From 287613f4b180e0e4a1e53435c5c5614e3425aeff Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 07:38:35 +0800 Subject: [PATCH 08/11] test(database): update search mocks for title IDs Update existing scoped and multi-variant search mocks for the MediaTitleDBID column and shared tag lookup introduced by search enrichment. --- .../mediadb/media_search_scope_test.go | 31 +++++++------------ .../mediadb/multi_variant_search_test.go | 26 +++++++--------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/pkg/database/mediadb/media_search_scope_test.go b/pkg/database/mediadb/media_search_scope_test.go index e249e01db..bf013aee6 100644 --- a/pkg/database/mediadb/media_search_scope_test.go +++ b/pkg/database/mediadb/media_search_scope_test.go @@ -65,14 +65,13 @@ func TestMediaDB_SearchMediaWithFilters_ScopesCachedVariantsByMediaType(t *testi mock.ExpectQuery("SELECT .+ FROM MediaTitles"). WithArgs(int64(10), int64(20), 10). WillReturnRows(sqlmock.NewRows([]string{ - "SystemID", "Name", "Path", "DBID", "DisambiguationTypes", + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", }). - AddRow(nes.ID, "R-Type", nesPath, int64(100), ""). - AddRow(movie.ID, "R-Type", moviePath, int64(200), "")) - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(100, 200, 100, 200). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + AddRow(nes.ID, "R-Type", nesPath, int64(100), int64(10), ""). + AddRow(movie.ID, "R-Type", moviePath, int64(200), int64(20), "")) + mock.ExpectQuery("SELECT EXISTS"). + WithArgs(100, 200, 10, 20). + WillReturnRows(sqlmock.NewRows([]string{"hasTags"}).AddRow(false)) results, err := mediaDB.SearchMediaWithFilters(context.Background(), &database.SearchFilters{ Systems: []systemdefs.System{*nes, *movie}, @@ -133,22 +132,16 @@ func TestMediaDB_SearchMediaWithFilters_ScopesSQLVariantsByMediaType(t *testing. ExpectQuery(). WithArgs(nes.ID, "%rtype%", "%rtype%", 1). WillReturnRows(sqlmock.NewRows([]string{ - "SystemID", "Name", "Path", "DBID", "DisambiguationTypes", - }).AddRow(nes.ID, "R-Type", nesPath, int64(300), "")) - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(300, 300). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow(nes.ID, "R-Type", nesPath, int64(300), int64(30), "")) + expectSearchTagsQuery(mock, 300, 30) mock.ExpectPrepare("SELECT.*Systems\\.SystemID.*MediaTitles\\.Name.*Media\\.Path.*Media\\.DBID.*"). ExpectQuery(). WithArgs(movie.ID, "%r%", "%r%", 1). WillReturnRows(sqlmock.NewRows([]string{ - "SystemID", "Name", "Path", "DBID", "DisambiguationTypes", - }).AddRow(movie.ID, "R-Type", moviePath, int64(200), "")) - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(200, 200). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow(movie.ID, "R-Type", moviePath, int64(200), int64(20), "")) + expectSearchTagsQuery(mock, 200, 20) results, err := mediaDB.SearchMediaWithFilters(context.Background(), &database.SearchFilters{ Systems: []systemdefs.System{*nes, *movie}, diff --git a/pkg/database/mediadb/multi_variant_search_test.go b/pkg/database/mediadb/multi_variant_search_test.go index 263c25dfe..b8651d187 100644 --- a/pkg/database/mediadb/multi_variant_search_test.go +++ b/pkg/database/mediadb/multi_variant_search_test.go @@ -71,14 +71,11 @@ func TestSearchMediaWithFilters_MultipleSameMediaTypeSystems(t *testing.T) { mock.ExpectPrepare("SELECT.*Systems\\.SystemID.*MediaTitles\\.Name.*Media\\.Path.*Media\\.DBID.*"). ExpectQuery(). WithArgs("NES", "SNES", "%mario%", "%mario%", 10). // Should be 5 args, not 7 - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("NES", "Super Mario Bros", mediaPath, int64(1), "")) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow("NES", "Super Mario Bros", mediaPath, int64(1), int64(10), "")) - // Mock tags query - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(int64(1), int64(1)). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + expectSearchTagsQuery(mock, 1, 10) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, variantGroups, rawWords, nil, nil, nil, 10, false, @@ -121,7 +118,9 @@ func TestSearchMediaWithFilters_DifferentMediaTypes(t *testing.T) { mock.ExpectPrepare("SELECT.*Systems\\.SystemID.*MediaTitles\\.Name.*Media\\.Path.*Media\\.DBID.*"). ExpectQuery(). WithArgs("PS2", "TVEpisode", "%losts01e05%", "%losts01e05%", 10). - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"})) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + })) // No tags query mock needed - no results means fetchAndAttachTags returns early @@ -182,14 +181,11 @@ func TestSearchMediaWithFilters_MultipleWordsMultipleSystems(t *testing.T) { "%mario%", "%mario%", // Word 2: Slug LIKE, SecondarySlug LIKE 10, // Limit ). // Should be 8 args, not 16 - WillReturnRows(sqlmock.NewRows([]string{"SystemID", "Name", "Path", "DBID", "DisambiguationTypes"}). - AddRow("SNES", "Super Mario World", mediaPath, int64(1), "")) + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + }).AddRow("SNES", "Super Mario World", mediaPath, int64(1), int64(10), "")) - // Mock tags query - mock.ExpectPrepare("SELECT.*MediaDBID.*Tag.*Type FROM"). - ExpectQuery(). - WithArgs(int64(1), int64(1)). - WillReturnRows(sqlmock.NewRows([]string{"MediaDBID", "Tag", "Type"})) + expectSearchTagsQuery(mock, 1, 10) results, err := sqlSearchMediaWithFilters( context.Background(), db, systems, variantGroups, rawWords, nil, nil, nil, 10, false, From 6fa2efe3659f464c8c85a76f8993997b35f0538e Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 07:53:28 +0800 Subject: [PATCH 09/11] perf(database): stream dense search candidates Stream default-order media rows by DBID and filter large cached title candidate sets in memory, stopping once the page fills. Cache scoped media bounds, support up to four systems, and fall back to grouped SQL when a bounded window is sparse or the request is unsupported. Developed from target-device latency analysis prompted by ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 --- .../mediadb/media_search_scope_test.go | 364 +++++++++++++++++- pkg/database/mediadb/mediadb.go | 204 +++++++++- pkg/database/mediadb/sql_search.go | 272 ++++++++++++- 3 files changed, 816 insertions(+), 24 deletions(-) diff --git a/pkg/database/mediadb/media_search_scope_test.go b/pkg/database/mediadb/media_search_scope_test.go index bf013aee6..767643afc 100644 --- a/pkg/database/mediadb/media_search_scope_test.go +++ b/pkg/database/mediadb/media_search_scope_test.go @@ -21,7 +21,9 @@ package mediadb import ( "context" + "fmt" "path/filepath" + "strings" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -155,7 +157,7 @@ func TestMediaDB_SearchMediaWithFilters_ScopesSQLVariantsByMediaType(t *testing. assert.NoError(t, mock.ExpectationsWereMet()) } -func TestMediaDB_SearchMediaWithFilters_FallsBackBeforeSQLiteVariableLimit(t *testing.T) { +func TestMediaDB_SearchMediaWithFilters_FallsBackWhenScopedStreamIsSparse(t *testing.T) { t.Parallel() db, mock, err := testsqlmock.NewSQLMock() @@ -186,11 +188,20 @@ func TestMediaDB_SearchMediaWithFilters_FallsBackBeforeSQLiteVariableLimit(t *te mediaDB.sql.Store(db) mediaDB.slugSearchCache.Store(cache) + mock.ExpectQuery("SELECT MIN\\(DBID\\), MAX\\(DBID\\).*FROM Media"). + WithArgs(int64(1)). + WillReturnRows(sqlmock.NewRows([]string{"min", "max"}).AddRow(int64(1), int64(20_000))) + mock.ExpectQuery("SELECT.*MediaTitles\\.Name.*Media\\.Path.*FROM Media NOT INDEXED"). + WithArgs(int64(1), int64(10_000), int64(1)). + WillReturnRows(sqlmock.NewRows([]string{ + "Name", "Path", "DBID", "DisambiguationTypes", "MediaTitleDBID", + })) + mock.ExpectPrepare("SELECT.*Systems\\.SystemID.*MediaTitles\\.Name.*Media\\.Path.*Media\\.DBID.*"). ExpectQuery(). WithArgs(nes.ID, "%rtype%", "%rtype%", 10). WillReturnRows(sqlmock.NewRows([]string{ - "SystemID", "Name", "Path", "DBID", "DisambiguationTypes", + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", })) results, err := mediaDB.SearchMediaWithFilters(context.Background(), &database.SearchFilters{ @@ -203,3 +214,352 @@ func TestMediaDB_SearchMediaWithFilters_FallsBackBeforeSQLiteVariableLimit(t *te assert.Empty(t, results) assert.NoError(t, mock.ExpectationsWereMet()) } + +func TestScopedCandidateStream_MatchesGroupedSQL(t *testing.T) { + t.Parallel() + + mediaDB, cleanup := setupBrowsePlanTestDB(t) + defer cleanup() + ctx := context.Background() + + tx, err := mediaDB.sql.Load().BeginTx(ctx, nil) + require.NoError(t, err) + defer func() { _ = tx.Rollback() }() + _, err = tx.ExecContext(ctx, ` + INSERT INTO Systems (DBID, SystemID, Name) VALUES + (1, 'SNES', 'Super Nintendo'), + (2, 'NES', 'Nintendo');`) + require.NoError(t, err) + + titleStmt, err := tx.PrepareContext(ctx, ` + INSERT INTO MediaTitles (DBID, SystemDBID, Slug, Name) VALUES (?, ?, ?, ?)`) + require.NoError(t, err) + defer func() { require.NoError(t, titleStmt.Close()) }() + mediaStmt, err := tx.PrepareContext(ctx, ` + INSERT INTO Media (DBID, MediaTitleDBID, SystemDBID, Path, IsMissing) VALUES (?, ?, ?, ?, ?)`) + require.NoError(t, err) + defer func() { require.NoError(t, mediaStmt.Close()) }() + + const rowsPerSystem = 600 + snesCandidates := make(map[string][]int64) + allCandidates := make(map[string][]int64) + var firstSNES, lastSNES int64 + for i := 1; i <= rowsPerSystem; i++ { + var slug string + switch i % 6 { + case 0: + slug = fmt.Sprintf("alpha-super-%03d", i) + case 1: + slug = fmt.Sprintf("alpha-%03d", i) + case 2: + slug = fmt.Sprintf("super-%03d", i) + case 3: + slug = fmt.Sprintf("rare-%03d", i) + default: + slug = fmt.Sprintf("plain-%03d", i) + } + + for systemDBID := int64(1); systemDBID <= 2; systemDBID++ { + mediaID := int64((i-1)*2) + systemDBID + titleID := systemDBID*10_000 + int64(i) + systemID := "SNES" + if systemDBID == 2 { + systemID = "NES" + } + _, err = titleStmt.ExecContext(ctx, titleID, systemDBID, slug, fmt.Sprintf("%s Game %03d", systemID, i)) + require.NoError(t, err) + _, err = mediaStmt.ExecContext( + ctx, mediaID, titleID, systemDBID, + filepath.Join("roms", strings.ToLower(systemID), fmt.Sprintf("game-%03d.rom", i)), + i%17 == 0, + ) + require.NoError(t, err) + for _, query := range []string{"alpha", "super", "rare", "plain"} { + if strings.Contains(slug, query) { + allCandidates[query] = append(allCandidates[query], titleID) + } + } + if systemDBID == 1 { + if firstSNES == 0 { + firstSNES = mediaID + } + lastSNES = mediaID + for _, query := range []string{"alpha", "super", "rare", "plain"} { + if strings.Contains(slug, query) { + snesCandidates[query] = append(snesCandidates[query], titleID) + } + } + } + } + } + require.NoError(t, tx.Commit()) + + bounds := mediaDBIDBounds{first: firstSNES, last: lastSNES} + snes := []systemdefs.System{{ID: "SNES"}} + assertParity := func(t *testing.T, query string, limit int, cursor *int64) []database.SearchResultWithCursor { + t.Helper() + expected, queryErr := sqlSearchMediaWithFiltersSorted( + ctx, mediaDB.sql.Load(), snes, [][]string{{query}}, []string{query}, + "", nil, nil, cursor, nil, "", limit, false, + ) + require.NoError(t, queryErr) + actual, streamErr := sqlSearchMediaByLargeTitleDBIDSetInSystems( + ctx, mediaDB.sql.Load(), snesCandidates[query], map[int64]string{1: "SNES"}, bounds, cursor, limit, + ) + require.NoError(t, streamErr) + assert.Equal(t, searchResultIDs(expected), searchResultIDs(actual)) + for i := range actual { + assert.Equal(t, expected[i].MediaTitleID, actual[i].MediaTitleID) + } + return expected + } + + for _, tc := range []struct { + query string + limit int + }{ + {query: "alpha", limit: 25}, + {query: "alpha", limit: 100}, + {query: "super", limit: 50}, + {query: "rare", limit: 100}, + {query: "plain", limit: 300}, + } { + t.Run(fmt.Sprintf("%s-%d", tc.query, tc.limit), func(t *testing.T) { + assertParity(t, tc.query, tc.limit, nil) + }) + } + + firstPage := assertParity(t, "alpha", 25, nil) + require.Len(t, firstPage, 25) + cursor := firstPage[len(firstPage)-1].MediaID + assertParity(t, "alpha", 25, &cursor) + + allSystems := []systemdefs.System{{ID: "SNES"}, {ID: "NES"}} + multiExpected, err := sqlSearchMediaWithFiltersSorted( + ctx, mediaDB.sql.Load(), allSystems, [][]string{{"alpha"}}, []string{"alpha"}, + "", nil, nil, nil, nil, "", 100, false, + ) + require.NoError(t, err) + multiActual, err := sqlSearchMediaByLargeTitleDBIDSetInSystems( + ctx, mediaDB.sql.Load(), allCandidates["alpha"], map[int64]string{1: "SNES", 2: "NES"}, + mediaDBIDBounds{first: 1, last: rowsPerSystem * 2}, nil, 100, + ) + require.NoError(t, err) + assert.Equal(t, searchResultIDs(multiExpected), searchResultIDs(multiActual)) + for i := range multiActual { + assert.Equal(t, multiExpected[i].SystemID, multiActual[i].SystemID) + assert.Equal(t, multiExpected[i].MediaTitleID, multiActual[i].MediaTitleID) + } +} + +func searchResultIDs(results []database.SearchResultWithCursor) []int64 { + ids := make([]int64, len(results)) + for i := range results { + ids[i] = results[i].MediaID + } + return ids +} + +func TestMediaSearchBounds_CachesAndClears(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + + mediaDB := &MediaDB{} + mediaDB.sql.Store(db) + + expectBounds := func(first, last int64) { + mock.ExpectQuery("SELECT MIN\\(DBID\\), MAX\\(DBID\\).*FROM Media"). + WithArgs(int64(7)). + WillReturnRows(sqlmock.NewRows([]string{"min", "max"}).AddRow(first, last)) + } + + expectBounds(100, 200) + bounds, found, err := mediaDB.getMediaSearchBounds(context.Background(), 7) + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, mediaDBIDBounds{first: 100, last: 200}, bounds) + + cached, found, err := mediaDB.getMediaSearchBounds(context.Background(), 7) + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, bounds, cached) + + mediaDB.clearMediaSearchBounds() + expectBounds(300, 400) + refreshed, found, err := mediaDB.getMediaSearchBounds(context.Background(), 7) + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, mediaDBIDBounds{first: 300, last: 400}, refreshed) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestMediaSearchBounds_MetadataInvalidationPreservesCache(t *testing.T) { + t.Parallel() + + mediaDB, cleanup := setupTempMediaDB(t) + defer cleanup() + ctx := context.Background() + + system, err := mediaDB.FindOrInsertSystem(database.System{SystemID: "SNES", Name: "SNES"}) + require.NoError(t, err) + require.NoError(t, mediaDB.BeginTransaction(false)) + title, err := mediaDB.InsertMediaTitle(&database.MediaTitle{ + SystemDBID: system.DBID, + Slug: "bounds-cache", + Name: "Bounds Cache", + }) + require.NoError(t, err) + media, err := mediaDB.InsertMedia(database.Media{ + SystemDBID: system.DBID, + MediaTitleDBID: title.DBID, + Path: filepath.Join("roms", "snes", "bounds-cache.sfc"), + }) + require.NoError(t, err) + require.NoError(t, mediaDB.CommitTransaction()) + + bounds, found, err := mediaDB.getMediaSearchBounds(ctx, system.DBID) + require.NoError(t, err) + assert.True(t, found) + assert.Equal(t, mediaDBIDBounds{first: media.DBID, last: media.DBID}, bounds) + + mediaDB.invalidateCaches(invalidationScope{AllSystems: true}) + mediaDB.mediaSearchBoundsMu.RLock() + cached, ok := mediaDB.mediaSearchBounds[system.DBID] + mediaDB.mediaSearchBoundsMu.RUnlock() + assert.True(t, ok, "metadata-only invalidation must preserve media bounds") + assert.Equal(t, bounds, cached) + + mediaDB.invalidateCaches(invalidationScope{AllSystems: true, MediaRowsChanged: true}) + mediaDB.mediaSearchBoundsMu.RLock() + assert.Empty(t, mediaDB.mediaSearchBounds) + mediaDB.mediaSearchBoundsMu.RUnlock() + + // Metadata-only transactions must not evict bounds; media inserts must. + bounds, found, err = mediaDB.getMediaSearchBounds(ctx, system.DBID) + require.NoError(t, err) + assert.True(t, found) + require.NoError(t, mediaDB.BeginTransaction(false)) + require.NoError(t, mediaDB.CommitTransaction()) + mediaDB.mediaSearchBoundsMu.RLock() + assert.Equal(t, bounds, mediaDB.mediaSearchBounds[system.DBID]) + mediaDB.mediaSearchBoundsMu.RUnlock() + + require.NoError(t, mediaDB.BeginTransaction(false)) + secondTitle, err := mediaDB.InsertMediaTitle(&database.MediaTitle{ + SystemDBID: system.DBID, + Slug: "bounds-cache-second", + Name: "Bounds Cache Second", + }) + require.NoError(t, err) + _, err = mediaDB.InsertMedia(database.Media{ + SystemDBID: system.DBID, + MediaTitleDBID: secondTitle.DBID, + Path: filepath.Join("roms", "snes", "bounds-cache-second.sfc"), + }) + require.NoError(t, err) + require.NoError(t, mediaDB.CommitTransaction()) + mediaDB.mediaSearchBoundsMu.RLock() + assert.Empty(t, mediaDB.mediaSearchBounds) + mediaDB.mediaSearchBoundsMu.RUnlock() +} + +func TestScopedCandidateStreamQueryPlanUsesRowIDOrder(t *testing.T) { + t.Parallel() + + mediaDB, cleanup := setupBrowsePlanTestDB(t) + defer cleanup() + seedBrowsePlanTestDB(t, mediaDB, 100) + + rows, err := mediaDB.sql.Load().QueryContext( + context.Background(), "EXPLAIN QUERY PLAN "+scopedCandidateStreamQuery(1), 1, 100, 1, + ) + require.NoError(t, err) + defer func() { require.NoError(t, rows.Close()) }() + + var planLines []string + for rows.Next() { + var id, parent, notUsed int + var detail string + require.NoError(t, rows.Scan(&id, &parent, ¬Used, &detail)) + planLines = append(planLines, detail) + } + require.NoError(t, rows.Err()) + + plan := strings.Join(planLines, "\n") + assert.Contains(t, plan, "INTEGER PRIMARY KEY") + assert.NotContains(t, plan, "USE TEMP B-TREE FOR ORDER BY") +} + +func TestSQLSearchMediaByLargeTitleDBIDSetInSystems(t *testing.T) { + t.Parallel() + + mediaDB, cleanup := setupBrowsePlanTestDB(t) + defer cleanup() + const pageSize = 10 + seedBrowsePlanTestDB(t, mediaDB, (sqliteMaxParams+pageSize)*2) + + candidateIDs := make([]int64, sqliteMaxParams+pageSize) + for i := range candidateIDs { + candidateIDs[i] = int64((i + 1) * 2) + } + + bounds := mediaDBIDBounds{first: 1, last: (sqliteMaxParams + pageSize) * 2} + results, err := sqlSearchMediaByLargeTitleDBIDSetInSystems( + context.Background(), mediaDB.sql.Load(), candidateIDs, map[int64]string{1: "MiSTer:Arcade"}, + bounds, nil, pageSize, + ) + require.NoError(t, err) + require.Len(t, results, pageSize) + for i := range results { + assert.Equal(t, "MiSTer:Arcade", results[i].SystemID) + assert.Equal(t, int64((i+1)*2), results[i].MediaID) + assert.Equal(t, int64((i+1)*2), results[i].MediaTitleID) + } + + cursor := results[len(results)-1].MediaID + secondPage, err := sqlSearchMediaByLargeTitleDBIDSetInSystems( + context.Background(), mediaDB.sql.Load(), candidateIDs, map[int64]string{1: "MiSTer:Arcade"}, + bounds, &cursor, pageSize, + ) + require.NoError(t, err) + require.Len(t, secondPage, pageSize) + for i := range secondPage { + assert.Equal(t, int64((i+pageSize+1)*2), secondPage[i].MediaID) + } +} + +func TestSQLSearchMediaByLargeTitleDBIDSet(t *testing.T) { + t.Parallel() + + mediaDB, cleanup := setupBrowsePlanTestDB(t) + defer cleanup() + const pageSize = 10 + seedBrowsePlanTestDB(t, mediaDB, (sqliteMaxParams+pageSize)*2) + + candidateIDs := make([]int64, sqliteMaxParams+pageSize) + for i := range candidateIDs { + candidateIDs[i] = int64((i + 1) * 2) + } + + results, err := sqlSearchMediaByLargeTitleDBIDSet( + context.Background(), mediaDB.sql.Load(), candidateIDs, "", nil, nil, nil, pageSize, + ) + require.NoError(t, err) + require.Len(t, results, pageSize) + for i := range results { + assert.Equal(t, int64((i+1)*2), results[i].MediaID) + } + + cursor := results[len(results)-1].MediaID + secondPage, err := sqlSearchMediaByLargeTitleDBIDSet( + context.Background(), mediaDB.sql.Load(), candidateIDs, "", nil, nil, &cursor, pageSize, + ) + require.NoError(t, err) + require.Len(t, secondPage, pageSize) + for i := range secondPage { + assert.Equal(t, int64((i+pageSize+1)*2), secondPage[i].MediaID) + } +} diff --git a/pkg/database/mediadb/mediadb.go b/pkg/database/mediadb/mediadb.go index c26e36a24..a0b871ecf 100644 --- a/pkg/database/mediadb/mediadb.go +++ b/pkg/database/mediadb/mediadb.go @@ -128,12 +128,17 @@ func getSqliteConnParams() string { "&_page_size=8192&_foreign_keys=ON&_txlock=immediate" } +type mediaDBIDBounds struct { + first int64 + last int64 +} + type MediaDB struct { clock clockwork.Clock ctx context.Context pl platforms.Platform - batchInsertScanStage *BatchInserter - batchInsertScanProperty *BatchInserter + scrapeImageSystems map[string]struct{} + batchInsertTagType *BatchInserter stmtInsertMedia *sql.Stmt tx *sql.Tx stmtInsertSystem *sql.Stmt @@ -143,7 +148,7 @@ type MediaDB struct { batchInsertMediaTag *BatchInserter inMemoryTagCache atomic.Pointer[tagCache] batchInsertTag *BatchInserter - batchInsertTagType *BatchInserter + mediaSearchBounds map[int64]mediaDBIDBounds stmtInsertMediaTag *sql.Stmt batchInsertMediaTitle *BatchInserter stmtInsertMediaTitle *sql.Stmt @@ -151,14 +156,16 @@ type MediaDB struct { batchInsertSystem *BatchInserter batchInsertScanTag *BatchInserter slugSearchCache atomic.Pointer[SlugSearchCache] - scrapeImageSystems map[string]struct{} + batchInsertScanStage *BatchInserter + batchInsertScanProperty *BatchInserter dbPath string backgroundOps sync.WaitGroup backgroundOpsCount atomic.Int64 - backgroundOpsMu syncutil.RWMutex vacuumRetryDelay time.Duration analyzeRetryDelay time.Duration batchSize int + backgroundOpsMu syncutil.RWMutex + mediaSearchBoundsMu syncutil.RWMutex sqlMu syncutil.RWMutex scrapeImageChangesMu syncutil.Mutex recreating atomic.Bool @@ -169,6 +176,7 @@ type MediaDB struct { inTransaction bool browseCacheDirty bool utilityTagCacheDirty bool + mediaSearchBoundsDirty bool scrapeImageChangesAll bool } @@ -196,11 +204,15 @@ type invalidationScope struct { AllSystems bool PreserveSlugSearchCache bool UtilityTagDBIDsChanged bool + MediaRowsChanged bool } // invalidateCaches handles all cache invalidation in one place func (db *MediaDB) invalidateCaches(scope invalidationScope) { db.inMemoryTagCache.Store(nil) + if scope.MediaRowsChanged { + db.clearMediaSearchBounds() + } clearPrefixPolicyCache() clearCoverAvailabilityCacheFor(db.sql.Load()) if scope.UtilityTagDBIDsChanged { @@ -257,6 +269,59 @@ func (db *MediaDB) invalidateCaches(scope invalidationScope) { } } +func (db *MediaDB) clearMediaSearchBounds() { + db.mediaSearchBoundsMu.Lock() + defer db.mediaSearchBoundsMu.Unlock() + db.mediaSearchBounds = nil +} + +func (db *MediaDB) getMediaSearchBounds(ctx context.Context, systemDBID int64) (mediaDBIDBounds, bool, error) { + db.mediaSearchBoundsMu.RLock() + bounds, ok := db.mediaSearchBounds[systemDBID] + db.mediaSearchBoundsMu.RUnlock() + if ok { + return bounds, bounds.first > 0, nil + } + + // Serialize misses so concurrent first searches for one system do not repeat + // the covering-index scan. Stable libraries pay this once per system; media + // mutations clear the map through invalidateCaches. + db.mediaSearchBoundsMu.Lock() + defer db.mediaSearchBoundsMu.Unlock() + if bounds, ok = db.mediaSearchBounds[systemDBID]; ok { + return bounds, bounds.first > 0, nil + } + + queryStarted := time.Now() + var firstMediaID, lastMediaID sql.NullInt64 + if err := db.sql.Load().QueryRowContext(ctx, ` + SELECT MIN(DBID), MAX(DBID) + FROM Media + WHERE SystemDBID = ? AND IsMissing = 0`, systemDBID).Scan(&firstMediaID, &lastMediaID); err != nil { + return mediaDBIDBounds{}, false, fmt.Errorf("query media search bounds: %w", err) + } + if firstMediaID.Valid && lastMediaID.Valid { + bounds = mediaDBIDBounds{first: firstMediaID.Int64, last: lastMediaID.Int64} + } + if db.mediaSearchBounds == nil { + db.mediaSearchBounds = make(map[int64]mediaDBIDBounds) + } + db.mediaSearchBounds[systemDBID] = bounds + log.Debug(). + Int64("systemDBID", systemDBID). + Int64("firstMediaID", bounds.first). + Int64("lastMediaID", bounds.last). + Dur("duration", time.Since(queryStarted)). + Msg("media search system bounds cached") + return bounds, bounds.first > 0, nil +} + +func invalidationScopeForMediaSystemIDs(systemIDs []string) invalidationScope { + scope := invalidationScopeForSystemIDs(systemIDs) + scope.MediaRowsChanged = true + return scope +} + func invalidationScopeForSystemIDs(systemIDs []string) invalidationScope { if len(systemIDs) == 0 || len(systemIDs) > maxSelectiveInvalidationSystems { return invalidationScope{AllSystems: true} @@ -369,6 +434,7 @@ func (db *MediaDB) Open() error { clearCoverAvailabilityCache() clearImagePropertyTagCache() clearPrefixPolicyCache() + db.clearMediaSearchBounds() if !exists { log.Debug().Msg("media database is new, allocating schema") @@ -658,9 +724,9 @@ func (db *MediaDB) UpdateLastGenerated() error { case getSystemsErr != nil: log.Warn().Err(getSystemsErr). Msg("failed to load indexing systems for cache invalidation; clearing all caches") - db.invalidateCaches(invalidationScope{AllSystems: true}) + db.invalidateCaches(invalidationScope{AllSystems: true, MediaRowsChanged: true}) default: - scope := invalidationScopeForSystemIDs(systemIDs) + scope := invalidationScopeForMediaSystemIDs(systemIDs) scope.PreserveSlugSearchCache = isIndexing && !scope.AllSystems db.invalidateCaches(scope) } @@ -1165,7 +1231,9 @@ func (db *MediaDB) Truncate() error { } // Invalidate all caches after full truncation - db.invalidateCaches(invalidationScope{AllSystems: true, UtilityTagDBIDsChanged: true}) + db.invalidateCaches(invalidationScope{ + AllSystems: true, UtilityTagDBIDsChanged: true, MediaRowsChanged: true, + }) // Reclaim disk space freed by the truncation if err := sqlVacuum(db.ctx, db.sql.Load()); err != nil { @@ -1188,7 +1256,7 @@ func (db *MediaDB) TruncateSystems(systemIDs []string) error { } // Invalidate caches for the affected systems - scope := invalidationScopeForSystemIDs(systemIDs) + scope := invalidationScopeForMediaSystemIDs(systemIDs) scope.UtilityTagDBIDsChanged = true db.invalidateCaches(scope) return nil @@ -1302,7 +1370,9 @@ func (db *MediaDB) CleanMediaOrphans(ctx context.Context) (int64, error) { log.Warn().Err(cacheErr).Msg("failed to invalidate cached media counts after orphan cleanup") } } - db.invalidateCaches(invalidationScope{AllSystems: true, UtilityTagDBIDsChanged: true}) + db.invalidateCaches(invalidationScope{ + AllSystems: true, UtilityTagDBIDsChanged: true, MediaRowsChanged: true, + }) if err := sqlInvalidateBrowseCache(ctx, db.sql.Load()); err != nil { log.Warn().Err(err).Msg("failed to invalidate browse cache after orphan cleanup") } @@ -1334,32 +1404,35 @@ func (db *MediaDB) Close() error { } func (db *MediaDB) cacheInvalidationScopeForCommittedTransaction() invalidationScope { + allSystemsScope := func() invalidationScope { + return invalidationScope{ + AllSystems: true, + UtilityTagDBIDsChanged: db.utilityTagCacheDirty, + MediaRowsChanged: db.mediaSearchBoundsDirty, + } + } + // CommitTransaction already holds db.sqlMu, so use the SQL helpers directly // instead of getters that would try to take the lock again. status, statusErr := sqlGetIndexingStatus(db.ctx, db.sql.Load()) if statusErr != nil { log.Warn().Err(statusErr).Msg("failed to determine indexing status for cache invalidation") - scope := invalidationScope{AllSystems: true} - scope.UtilityTagDBIDsChanged = db.utilityTagCacheDirty - return scope + return allSystemsScope() } if status != IndexingStatusRunning && status != IndexingStatusPending { - scope := invalidationScope{AllSystems: true} - scope.UtilityTagDBIDsChanged = db.utilityTagCacheDirty - return scope + return allSystemsScope() } systemIDs, getSystemsErr := sqlGetIndexingSystems(db.ctx, db.sql.Load()) if getSystemsErr != nil { log.Warn().Err(getSystemsErr).Msg("failed to load indexing systems for cache invalidation") - scope := invalidationScope{AllSystems: true} - scope.UtilityTagDBIDsChanged = db.utilityTagCacheDirty - return scope + return allSystemsScope() } scope := invalidationScopeForSystemIDs(systemIDs) scope.UtilityTagDBIDsChanged = db.utilityTagCacheDirty + scope.MediaRowsChanged = db.mediaSearchBoundsDirty return scope } @@ -1371,6 +1444,7 @@ func (db *MediaDB) SetSQLForTesting(ctx context.Context, sqlDB *sql.DB, platform clearCoverAvailabilityCache() clearImagePropertyTagCache() clearPrefixPolicyCache() + db.clearMediaSearchBounds() db.ctx = ctx db.pl = platform db.clock = clockwork.NewRealClock() @@ -1591,6 +1665,7 @@ func (db *MediaDB) ReconcileStagedSystem( return stats, err } if stats.MediaUpserted > 0 || stats.MediaMissing > 0 { + db.mediaSearchBoundsDirty = true countKeys := []string{DBConfigMediaMissingCount} if stats.MediaUpserted > 0 { countKeys = append(countKeys, DBConfigMediaTotalCount) @@ -1640,6 +1715,7 @@ func (db *MediaDB) RollbackTransaction() error { db.inTransaction = false // Clear transaction flag (no cache invalidation needed on rollback) db.clearBrowseCacheInvalidation() db.utilityTagCacheDirty = false + db.mediaSearchBoundsDirty = false if err != nil { return fmt.Errorf("failed to rollback transaction: %w", err) } @@ -1667,6 +1743,7 @@ func (db *MediaDB) rollbackAndLogError() { db.inTransaction = false db.clearBrowseCacheInvalidation() db.utilityTagCacheDirty = false + db.mediaSearchBoundsDirty = false } func (db *MediaDB) BeginTransaction(batchEnabled bool) error { @@ -1681,6 +1758,7 @@ func (db *MediaDB) BeginTransaction(batchEnabled bool) error { if db.inTransaction { return errors.New("transaction already in progress") } + db.mediaSearchBoundsDirty = false // Begin a proper transaction tx, err := db.sql.Load().BeginTx(db.ctx, nil) @@ -1916,12 +1994,14 @@ func (db *MediaDB) CommitTransactionWithOptions(options database.TransactionOpti db.inTransaction = false db.clearBrowseCacheInvalidation() db.utilityTagCacheDirty = false + db.mediaSearchBoundsDirty = false return fmt.Errorf("failed to flush batch inserts: %w; rollback also failed: %w", closeErr, rbErr) } db.tx = nil db.inTransaction = false db.clearBrowseCacheInvalidation() db.utilityTagCacheDirty = false + db.mediaSearchBoundsDirty = false return fmt.Errorf("failed to flush batch inserts: %w", closeErr) } } else { @@ -1938,12 +2018,14 @@ func (db *MediaDB) CommitTransactionWithOptions(options database.TransactionOpti db.inTransaction = false db.clearBrowseCacheInvalidation() db.utilityTagCacheDirty = false + db.mediaSearchBoundsDirty = false return fmt.Errorf("commit failed: %w; rollback also failed: %w", err, rbErr) } db.tx = nil db.inTransaction = false db.clearBrowseCacheInvalidation() db.utilityTagCacheDirty = false + db.mediaSearchBoundsDirty = false return fmt.Errorf("failed to commit transaction: %w", err) } @@ -1959,7 +2041,9 @@ func (db *MediaDB) CommitTransactionWithOptions(options database.TransactionOpti switch { case statusErr != nil: log.Warn().Err(statusErr).Msg("failed to determine indexing status for cache invalidation") - db.invalidateCaches(invalidationScope{AllSystems: true}) + db.invalidateCaches(invalidationScope{ + AllSystems: true, MediaRowsChanged: db.mediaSearchBoundsDirty, + }) case indexingStatus == IndexingStatusRunning || indexingStatus == IndexingStatusPending: scope := db.cacheInvalidationScopeForCommittedTransaction() scope.PreserveSlugSearchCache = true @@ -1968,6 +2052,7 @@ func (db *MediaDB) CommitTransactionWithOptions(options database.TransactionOpti default: db.invalidateCaches(db.cacheInvalidationScopeForCommittedTransaction()) } + db.mediaSearchBoundsDirty = false if err := db.flushBrowseCacheInvalidation(); err != nil { return err } @@ -2361,7 +2446,15 @@ func (db *MediaDB) SearchMediaWithFilters( cacheReady := cache != nil && cache.CanServeSystems(systemIDs) cacheableGroups := mediaSearchTypeGroupsCacheable(groups) if cacheReady && cacheableGroups { + candidateStarted := time.Now() candidateIDs := searchMediaTypeGroupsInCache(cache, groups) + candidateDuration := time.Since(candidateStarted) + log.Debug(). + Strs("systems", systemIDs). + Int("mediaTypeGroups", len(groups)). + Int("candidates", len(candidateIDs)). + Dur("duration", candidateDuration). + Msg("media search in-memory candidate timing") if len(candidateIDs) == 0 { return []database.SearchResultWithCursor{}, nil } @@ -2378,10 +2471,77 @@ func (db *MediaDB) SearchMediaWithFilters( filters.Letter, filters.Cursor, filters.SortCursor, filters.Sort, filters.Limit) } + canStreamCandidates := filters.Sort == "" && filters.SortCursor == nil && filters.PathPrefix == "" && + len(filters.Tags) == 0 && filters.Letter == nil && len(candidateIDs) >= filters.Limit + if canStreamCandidates { + var streamResults []database.SearchResultWithCursor + var streamErr error + strategy := "" + switch { + case requestedAllSystems(searchSystems): + strategy = "global" + streamResults, streamErr = sqlSearchMediaByLargeTitleDBIDSet( + ctx, db.sql.Load(), candidateIDs, filters.PathPrefix, filters.Tags, + filters.Letter, filters.Cursor, filters.Limit) + case len(searchSystems) > 0 && len(searchSystems) <= maxScopedStreamSystems: + resolvedDBIDs := cache.ResolveSystemDBIDs(systemIDs) + if len(resolvedDBIDs) == len(systemIDs) { + scopedSystems := make(map[int64]string, len(resolvedDBIDs)) + var bounds mediaDBIDBounds + for i, systemDBID := range resolvedDBIDs { + var systemBounds mediaDBIDBounds + var found bool + systemBounds, found, streamErr = db.getMediaSearchBounds(ctx, systemDBID) + if streamErr != nil { + break + } + if !found { + continue + } + scopedSystems[systemDBID] = systemIDs[i] + if bounds.first == 0 || systemBounds.first < bounds.first { + bounds.first = systemBounds.first + } + bounds.last = max(bounds.last, systemBounds.last) + } + if streamErr == nil && len(scopedSystems) == 0 { + return []database.SearchResultWithCursor{}, nil + } + if streamErr == nil { + strategy = "system-scope" + streamResults, streamErr = sqlSearchMediaByLargeTitleDBIDSetInSystems( + ctx, db.sql.Load(), candidateIDs, scopedSystems, bounds, + filters.Cursor, filters.Limit) + } + } + } + if streamErr != nil && strategy == "" { + return nil, streamErr + } + if strategy != "" { + log.Debug(). + Strs("systems", systemIDs). + Str("strategy", strategy). + Int("mediaTypeGroups", len(groups)). + Int("candidates", len(candidateIDs)). + Msg("media search streaming large in-memory candidate set") + if streamErr == nil { + return streamResults, nil + } + if !errors.Is(streamErr, errSearchCandidateSetTooSparse) { + return nil, streamErr + } + log.Debug(). + Str("strategy", strategy). + Msg("media search candidate stream too sparse; falling back to grouped SQL") + } + } + log.Debug(). Int("candidates", len(candidateIDs)). Int("queryParams", queryParams). Int("maxQueryParams", sqliteMaxParams). + Str("sort", filters.Sort). Msg("media search cache candidates exceed SQLite parameter budget") } @@ -3091,6 +3251,7 @@ func (db *MediaDB) InsertMedia(row database.Media) (database.Media, error) { //n return row, fmt.Errorf("failed to add media to batch: %w", err) } db.markBrowseCacheDirty() + db.mediaSearchBoundsDirty = true // Return row as-is (DBID is already set by caller) return row, nil } @@ -3104,12 +3265,13 @@ func (db *MediaDB) InsertMedia(row database.Media) (database.Media, error) { //n // Only invalidate cache if NOT in a transaction (transactions invalidate once on commit) if err == nil && !db.inTransaction { - db.invalidateCaches(invalidationScope{AllSystems: true}) + db.invalidateCaches(invalidationScope{AllSystems: true, MediaRowsChanged: true}) if invalidateErr := db.invalidateBrowseCacheForMediaChange(); invalidateErr != nil { return result, invalidateErr } } else if err == nil { db.markBrowseCacheDirty() + db.mediaSearchBoundsDirty = true } return result, err diff --git a/pkg/database/mediadb/sql_search.go b/pkg/database/mediadb/sql_search.go index a7e9ee497..075565707 100644 --- a/pkg/database/mediadb/sql_search.go +++ b/pkg/database/mediadb/sql_search.go @@ -38,7 +38,13 @@ import ( "github.com/rs/zerolog/log" ) -const tagPreflightMaxResults = 25 +const ( + largeCandidateScanFloor = 10_000 + maxScopedStreamSystems = 4 + tagPreflightMaxResults = 25 +) + +var errSearchCandidateSetTooSparse = errors.New("large media search candidate set is too sparse to stream") // fetchAndAttachTags fetches tags for a slice of search results and attaches them to the results. // This helper consolidates duplicated tag-fetching logic across multiple search functions. @@ -1264,6 +1270,270 @@ func sqlSearchMediaByTitleDBIDsSorted( return results, nil } +// sqlSearchMediaByLargeTitleDBIDSet streams Media in legacy DBID order and +// filters title candidates in memory. This avoids both SQLite's bind-variable +// limit and a full grouped LIKE scan. Streaming also stops as soon as the page +// is full, which is critical on SD-backed libraries. +func sqlSearchMediaByLargeTitleDBIDSet( + ctx context.Context, + db sqlQueryable, + titleDBIDs []int64, + pathPrefix string, + tags []zapscript.TagFilter, + letter *string, + cursor *int64, + limit int, +) ([]database.SearchResultWithCursor, error) { + if len(titleDBIDs) == 0 || limit <= 0 { + return []database.SearchResultWithCursor{}, nil + } + + candidateSet := make(map[int64]struct{}, len(titleDBIDs)) + for _, titleDBID := range titleDBIDs { + candidateSet[titleDBID] = struct{}{} + } + + conditions := []string{"Media.IsMissing = 0"} + args := make([]any, 0, 10) + if pathPrefix != "" { + pathClause, pathArgs := browsePathPrefixCondition( + "Media.Path", mediaRecursivePathPrefix(pathPrefix)) + conditions = append(conditions, pathClause) + args = append(args, pathArgs...) + } + if cursor != nil { + conditions = append(conditions, "Media.DBID > ?") + args = append(args, *cursor) + } + tagFilterClauses, tagFilterArgs := buildCandidateTagFilterSQL(tags) + conditions = append(conditions, tagFilterClauses...) + args = append(args, tagFilterArgs...) + letterClauses, letterArgs := BuildLetterFilterSQL(letter, "MediaTitles.Name") + conditions = append(conditions, letterClauses...) + args = append(args, letterArgs...) + + //nolint:gosec // Safe: WHERE clause built from sanitized components + query := ` + SELECT + Systems.SystemID, + MediaTitles.Name, + Media.Path, + Media.DBID, + MediaTitles.DisambiguationTypes, + MediaTitles.DBID + FROM Media + CROSS JOIN MediaTitles ON MediaTitles.DBID = Media.MediaTitleDBID + INNER JOIN Systems ON Systems.DBID = MediaTitles.SystemDBID + WHERE ` + strings.Join(conditions, " AND ") + ` + ORDER BY Media.DBID` + + queryStarted := time.Now() + rows, err := db.QueryContext(ctx, query, args...) + if err != nil { + return nil, fmt.Errorf("failed to stream media candidate set: %w", err) + } + defer func() { _ = rows.Close() }() + + results := make([]database.SearchResultWithCursor, 0, min(limit, 100)) + scannedRows := 0 + scanLimit := max(largeCandidateScanFloor, limit*100) + tooSparse := false + for rows.Next() { + scannedRows++ + var result database.SearchResultWithCursor + if scanErr := rows.Scan( + &result.SystemID, + &result.Name, + &result.Path, + &result.MediaID, + &result.DisambiguationTypes, + &result.MediaTitleID, + ); scanErr != nil { + return nil, fmt.Errorf("failed to scan streamed media candidate: %w", scanErr) + } + if _, ok := candidateSet[result.MediaTitleID]; ok { + results = append(results, result) + if len(results) == limit { + break + } + } + if scannedRows >= scanLimit { + tooSparse = true + break + } + } + if err = rows.Err(); err != nil { + return nil, fmt.Errorf("streamed media candidate rows error: %w", err) + } + if err = rows.Close(); err != nil { + return nil, fmt.Errorf("close streamed media candidate rows: %w", err) + } + queryElapsed := time.Since(queryStarted) + if tooSparse { + log.Debug(). + Int("titleDBIDs", len(titleDBIDs)). + Int("scannedRows", scannedRows). + Int("rows", len(results)). + Dur("queryDuration", queryElapsed). + Msg("search media large candidate set exceeded streaming scan limit") + return nil, errSearchCandidateSetTooSparse + } + + tagsStarted := time.Now() + if err := attachTagsAndDisambiguation(ctx, db, results); err != nil { + return nil, err + } + + log.Debug(). + Int("titleDBIDs", len(titleDBIDs)). + Int("scannedRows", scannedRows). + Int("tagFilters", len(tags)). + Int("rows", len(results)). + Dur("queryDuration", queryElapsed). + Dur("tagsDuration", time.Since(tagsStarted)). + Msg("search media large candidate set step timing") + + return results, nil +} + +func scopedCandidateStreamQuery(systemCount int) string { + return ` + SELECT + MediaTitles.Name, + Media.Path, + Media.DBID, + MediaTitles.DisambiguationTypes, + MediaTitles.DBID, + Media.SystemDBID + FROM Media NOT INDEXED + INNER JOIN MediaTitles ON MediaTitles.DBID = Media.MediaTitleDBID + WHERE Media.DBID BETWEEN ? AND ? + AND Media.SystemDBID IN (` + prepareVariadic("?", ",", systemCount) + `) + AND Media.IsMissing = 0 + ORDER BY Media.DBID` +} + +// sqlSearchMediaByLargeTitleDBIDSetInSystems scans one bounded rowid window +// spanning a small requested system set. System rows are normally contiguous +// after indexing, so dense candidate sets fill a page without binding thousands +// of title IDs or sorting the full scoped result. A sparse window returns +// errSearchCandidateSetTooSparse so callers preserve correctness through the +// grouped SQL fallback. +func sqlSearchMediaByLargeTitleDBIDSetInSystems( + ctx context.Context, + db sqlQueryable, + titleDBIDs []int64, + systemIDsByDBID map[int64]string, + bounds mediaDBIDBounds, + cursor *int64, + limit int, +) ([]database.SearchResultWithCursor, error) { + if len(titleDBIDs) == 0 || limit <= 0 { + return []database.SearchResultWithCursor{}, nil + } + + scanStart := bounds.first + if cursor != nil { + if *cursor >= bounds.last { + return []database.SearchResultWithCursor{}, nil + } + scanStart = max(scanStart, *cursor+1) + } + scanEnd := bounds.last + if bounds.last-scanStart >= largeCandidateScanFloor { + scanEnd = scanStart + largeCandidateScanFloor - 1 + } + + candidateSet := make(map[int64]struct{}, len(titleDBIDs)) + for _, titleDBID := range titleDBIDs { + candidateSet[titleDBID] = struct{}{} + } + systemDBIDs := make([]int64, 0, len(systemIDsByDBID)) + for systemDBID := range systemIDsByDBID { + systemDBIDs = append(systemDBIDs, systemDBID) + } + sort.Slice(systemDBIDs, func(i, j int) bool { return systemDBIDs[i] < systemDBIDs[j] }) + + // NOT INDEXED forces the integer primary-key range scan. Without it, + // SQLite may choose a SystemDBID/path index and build a temporary B-tree + // for ORDER BY Media.DBID, recreating the grouped search bottleneck. + args := make([]any, 0, len(systemDBIDs)+2) + args = append(args, scanStart, scanEnd) + for _, systemDBID := range systemDBIDs { + args = append(args, systemDBID) + } + queryStarted := time.Now() + rows, err := db.QueryContext(ctx, scopedCandidateStreamQuery(len(systemDBIDs)), args...) + if err != nil { + return nil, fmt.Errorf("query scoped media candidate stream: %w", err) + } + defer func() { _ = rows.Close() }() + + results := make([]database.SearchResultWithCursor, 0, min(limit, 100)) + scannedRows := 0 + for rows.Next() { + scannedRows++ + var systemDBID int64 + result := database.SearchResultWithCursor{} + if scanErr := rows.Scan( + &result.Name, + &result.Path, + &result.MediaID, + &result.DisambiguationTypes, + &result.MediaTitleID, + &systemDBID, + ); scanErr != nil { + return nil, fmt.Errorf("scan scoped media candidate: %w", scanErr) + } + if _, ok := candidateSet[result.MediaTitleID]; !ok { + continue + } + result.SystemID = systemIDsByDBID[systemDBID] + results = append(results, result) + if len(results) == limit { + break + } + } + if err = rows.Err(); err != nil { + return nil, fmt.Errorf("scoped media candidate rows: %w", err) + } + if err = rows.Close(); err != nil { + return nil, fmt.Errorf("close scoped media candidate rows: %w", err) + } + queryElapsed := time.Since(queryStarted) + + if len(results) < limit && scanEnd < bounds.last { + log.Debug(). + Int("systems", len(systemDBIDs)). + Int("titleDBIDs", len(titleDBIDs)). + Int64("scanStart", scanStart). + Int64("scanEnd", scanEnd). + Int("scannedRows", scannedRows). + Int("rows", len(results)). + Dur("queryDuration", queryElapsed). + Msg("scoped search candidate set exceeded streaming DBID window") + return nil, errSearchCandidateSetTooSparse + } + + tagsStarted := time.Now() + if attachErr := attachTagsAndDisambiguation(ctx, db, results); attachErr != nil { + return nil, attachErr + } + + log.Debug(). + Int("systems", len(systemDBIDs)). + Int("titleDBIDs", len(titleDBIDs)). + Int64("scanStart", scanStart). + Int64("scanEnd", scanEnd). + Int("scannedRows", scannedRows). + Int("rows", len(results)). + Dur("queryDuration", queryElapsed). + Dur("tagsDuration", time.Since(tagsStarted)). + Msg("search media scoped candidate set step timing") + + return results, nil +} + func sqlSearchMediaBySlug( ctx context.Context, db sqlQueryable, From ae4434be2c432322c01f76a37db692087f85c7b9 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 08:02:46 +0800 Subject: [PATCH 10/11] chore(ui): document search parameter lint exceptions --- pkg/ui/tui/mock_api_client_test.go | 2 ++ pkg/ui/tui/settings_service.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pkg/ui/tui/mock_api_client_test.go b/pkg/ui/tui/mock_api_client_test.go index 6780ffca8..471dc8818 100644 --- a/pkg/ui/tui/mock_api_client_test.go +++ b/pkg/ui/tui/mock_api_client_test.go @@ -335,6 +335,8 @@ func (m *MockSettingsService) CancelWriteTag(ctx context.Context) error { } // SearchMedia mocks searching for media. +// +//nolint:gocritic // Value parameter implements the SettingsService interface used by TUI tests. func (m *MockSettingsService) SearchMedia( ctx context.Context, params models.SearchParams, diff --git a/pkg/ui/tui/settings_service.go b/pkg/ui/tui/settings_service.go index 47103d74b..b147462ee 100644 --- a/pkg/ui/tui/settings_service.go +++ b/pkg/ui/tui/settings_service.go @@ -441,6 +441,8 @@ func (s *DefaultSettingsService) CancelWriteTag(ctx context.Context) error { } // SearchMedia searches for media matching the given parameters. +// +//nolint:gocritic // Value parameter preserves the SettingsService interface contract. func (s *DefaultSettingsService) SearchMedia( ctx context.Context, params models.SearchParams, From 077eb7c78d58d29365f07dbbfe4eb1c6ad398457 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Thu, 13 Aug 2026 09:24:53 +0800 Subject: [PATCH 11/11] fix(api): harden media browsing fallbacks --- docs/api/methods.md | 2 + pkg/api/methods/media.go | 10 +- pkg/api/methods/media_history.go | 61 ++--- pkg/api/methods/media_history_test.go | 67 ++++++ pkg/api/methods/media_image.go | 4 +- pkg/api/methods/media_image_test.go | 10 +- pkg/api/methods/media_search_test.go | 39 ++++ pkg/api/transport_timing_test.go | 208 ++++++++++++++++++ pkg/database/database.go | 3 + .../mediadb/media_search_scope_test.go | 171 ++++++++++++++ pkg/database/mediadb/mediadb.go | 106 ++++++--- pkg/database/mediadb/sql_browse.go | 70 +++--- pkg/database/mediadb/sql_browse_test.go | 36 +++ 13 files changed, 694 insertions(+), 93 deletions(-) create mode 100644 pkg/api/transport_timing_test.go diff --git a/docs/api/methods.md b/docs/api/methods.md index af52c3563..9c711c0bc 100644 --- a/docs/api/methods.md +++ b/docs/api/methods.md @@ -616,6 +616,7 @@ An object: "name": "240p Test Suite (PD) v0.03 tepples", "path": "/media/fat/games/Gameboy/240p Test Suite (PD) v0.03 tepples.gb", "relativePath": "Gameboy/240p Test Suite (PD) v0.03 tepples.gb", + "hasCover": false, "zapScript": "@Gameboy/240p Test Suite (PD) v0.03 tepples", "system": { "category": "Handheld", @@ -673,6 +674,7 @@ An object: "name": "Super Mario Bros.", "path": "/media/fat/games/NES/Super Mario Bros.nes", "relativePath": "NES/Super Mario Bros.nes", + "hasCover": true, "zapScript": "@NES/Super Mario Bros. (year:1985)", "system": { "category": "Console", diff --git a/pkg/api/methods/media.go b/pkg/api/methods/media.go index 8c421346d..b60472cf7 100644 --- a/pkg/api/methods/media.go +++ b/pkg/api/methods/media.go @@ -1023,9 +1023,13 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic MediaTitleDBID: searchResults[i].MediaTitleID, } } - coverStatuses, err = env.Database.MediaDB.GetMediaCoverStatus(ctx, coverRefs) - if err != nil { - return nil, fmt.Errorf("get media search cover status: %w", err) + coverCtx, cancelCoverLookup := optionalDBEnrichmentContext(ctx) + resolvedCoverStatuses, coverErr := env.Database.MediaDB.GetMediaCoverStatus(coverCtx, coverRefs) + cancelCoverLookup() + if coverErr != nil { + log.Debug().Err(coverErr).Msg("could not enrich media search cover status") + } else { + coverStatuses = resolvedCoverStatuses } } coverDuration := time.Since(coverStarted) diff --git a/pkg/api/methods/media_history.go b/pkg/api/methods/media_history.go index c19aaaf4e..dc6e6f60a 100644 --- a/pkg/api/methods/media_history.go +++ b/pkg/api/methods/media_history.go @@ -104,34 +104,43 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti }) } enrichStarted := time.Now() - mediaRows, err := resolveMediaPathIDs(env.Context, env.Database.MediaDB, mediaRefs) - if err != nil { - return nil, fmt.Errorf("resolve media history cover identities: %w", err) - } - mediaIDs := make(map[mediaPathRef]int64, len(mediaRows)) - coverRefs := make([]database.MediaCoverRef, 0, len(mediaRows)) - seenIDs := make(map[int64]struct{}, len(mediaRows)) - for _, ref := range mediaRefs { - row := mediaRows[ref] - if row.DBID <= 0 { - continue - } - mediaIDs[ref] = row.DBID - if _, ok := seenIDs[row.DBID]; ok { - continue + mediaIDs := make(map[mediaPathRef]int64) + coverStatuses := make(map[int64]bool) + enrichCtx, cancelEnrichment := optionalDBEnrichmentContext(env.Context) + defer cancelEnrichment() + + mediaRows, enrichErr := resolveMediaPathIDs(enrichCtx, env.Database.MediaDB, mediaRefs) + if enrichErr != nil { + log.Debug().Err(enrichErr).Msg("could not enrich media history from media database") + } else { + resolvedMediaIDs := make(map[mediaPathRef]int64, len(mediaRows)) + coverRefs := make([]database.MediaCoverRef, 0, len(mediaRows)) + seenIDs := make(map[int64]struct{}, len(mediaRows)) + for _, ref := range mediaRefs { + row := mediaRows[ref] + if row.DBID <= 0 { + continue + } + resolvedMediaIDs[ref] = row.DBID + if _, ok := seenIDs[row.DBID]; ok { + continue + } + seenIDs[row.DBID] = struct{}{} + coverRefs = append(coverRefs, database.MediaCoverRef{ + MediaDBID: row.DBID, + MediaTitleDBID: row.MediaTitleDBID, + }) } - seenIDs[row.DBID] = struct{}{} - coverRefs = append(coverRefs, database.MediaCoverRef{ - MediaDBID: row.DBID, - MediaTitleDBID: row.MediaTitleDBID, - }) - } - coverStatuses := make(map[int64]bool) - if len(coverRefs) > 0 { - coverStatuses, err = env.Database.MediaDB.GetMediaCoverStatus(env.Context, coverRefs) - if err != nil { - return nil, fmt.Errorf("get media history cover status: %w", err) + resolvedCoverStatuses := make(map[int64]bool) + if len(coverRefs) > 0 { + resolvedCoverStatuses, enrichErr = env.Database.MediaDB.GetMediaCoverStatus(enrichCtx, coverRefs) + } + if enrichErr != nil { + log.Debug().Err(enrichErr).Msg("could not enrich media history cover status") + } else { + mediaIDs = resolvedMediaIDs + coverStatuses = resolvedCoverStatuses } } enrichElapsed := time.Since(enrichStarted) diff --git a/pkg/api/methods/media_history_test.go b/pkg/api/methods/media_history_test.go index 3a920fc9a..0c95dafae 100644 --- a/pkg/api/methods/media_history_test.go +++ b/pkg/api/methods/media_history_test.go @@ -22,6 +22,7 @@ package methods import ( "context" "encoding/json" + "errors" "fmt" "path/filepath" "testing" @@ -193,6 +194,72 @@ func TestHandleMediaHistory_IncludesCoverStatus(t *testing.T) { mockMediaDB.AssertExpectations(t) } +func TestHandleMediaHistory_EnrichmentFailuresAreNonFatal(t *testing.T) { + t.Parallel() + + mediaPath := filepath.Join(string(filepath.Separator), "games", "history.nes") + entry := database.MediaHistoryEntry{ + DBID: 1, SystemID: "NES", MediaPath: mediaPath, MediaName: "History", StartTime: time.Now(), + } + + tests := []struct { + setup func(*testing.T, *helpers.MockMediaDBI) + name string + }{ + { + name: "media identity lookup", + setup: func(_ *testing.T, mockMediaDB *helpers.MockMediaDBI) { + mockMediaDB.On("FindMediaIDsByPaths", mock.Anything, []string{mediaPath}). + Return(nil, errors.New("identity lookup failed")) + }, + }, + { + name: "cover lookup", + setup: func(t *testing.T, mockMediaDB *helpers.MockMediaDBI) { + t.Helper() + mockMediaDB.On("FindMediaIDsByPaths", mock.Anything, []string{mediaPath}). + Return([]database.MediaPathID{{ + SystemID: "NES", Path: mediaPath, DBID: 42, MediaTitleDBID: 420, + }}, nil) + mockMediaDB.On("GetMediaCoverStatus", mock.Anything, []database.MediaCoverRef{{ + MediaDBID: 42, MediaTitleDBID: 420, + }}).Run(func(args mock.Arguments) { + ctx, ok := args.Get(0).(context.Context) + require.True(t, ok) + deadline, ok := ctx.Deadline() + require.True(t, ok, "optional enrichment must have a deadline") + assert.WithinDuration(t, time.Now().Add(optionalDBEnrichmentTimeout), deadline, time.Second) + }).Return(nil, errors.New("cover lookup failed")) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockUserDB := helpers.NewMockUserDBI() + mockMediaDB := helpers.NewMockMediaDBI() + mockUserDB.On("GetMediaHistory", []string(nil), int64(0), 26). + Return([]database.MediaHistoryEntry{entry}, nil) + tt.setup(t, mockMediaDB) + + result, err := HandleMediaHistory(requests.RequestEnv{ + Context: context.Background(), + Database: &database.Database{ + UserDB: mockUserDB, MediaDB: mockMediaDB, + }, + }) + require.NoError(t, err) + response, ok := result.(models.MediaHistoryResponse) + require.True(t, ok) + require.Len(t, response.Entries, 1) + assert.Zero(t, response.Entries[0].MediaID) + assert.False(t, response.Entries[0].HasCover) + mockUserDB.AssertExpectations(t) + mockMediaDB.AssertExpectations(t) + }) + } +} + func TestMediaResponseMediaIDs_BoundsSlowLookup(t *testing.T) { t.Parallel() diff --git a/pkg/api/methods/media_image.go b/pkg/api/methods/media_image.go index ff848a2cd..4554070ee 100644 --- a/pkg/api/methods/media_image.go +++ b/pkg/api/methods/media_image.go @@ -876,8 +876,8 @@ func cachedMediaImageResponse( return response, true } } - data, contentType, found := cache.read(ref, system, typeTag, maxSize) - if !found { + data, err := afero.ReadFile(cache.fs, path) + if err != nil { return models.MediaImageResponse{}, false } return inlineMediaImageResponse(data, contentType, sourcePath, typeTag), true diff --git a/pkg/api/methods/media_image_test.go b/pkg/api/methods/media_image_test.go index a2b9551e7..622bbb1cd 100644 --- a/pkg/api/methods/media_image_test.go +++ b/pkg/api/methods/media_image_test.go @@ -24,6 +24,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "image" "image/color" @@ -31,6 +32,7 @@ import ( "image/png" "os" "path/filepath" + "syscall" "testing" "time" @@ -174,7 +176,13 @@ func TestMediaThumbCache_IsSafeLocalPath(t *testing.T) { assert.False(t, cache.isSafeLocalPath(outside)) symlink := filepath.Join(cache.dir, "link.webp") - require.NoError(t, os.Symlink(outside, symlink)) + if err := os.Symlink(outside, symlink); err != nil { + if errors.Is(err, os.ErrPermission) || errors.Is(err, syscall.ENOSYS) || + errors.Is(err, syscall.EOPNOTSUPP) { + t.Skipf("symlinks unavailable: %v", err) + } + require.NoError(t, err) + } assert.False(t, cache.isSafeLocalPath(symlink)) relativeCache := &mediaThumbCache{fs: afero.NewMemMapFs(), dir: "relative"} diff --git a/pkg/api/methods/media_search_test.go b/pkg/api/methods/media_search_test.go index 7f7686d31..6d9120a79 100644 --- a/pkg/api/methods/media_search_test.go +++ b/pkg/api/methods/media_search_test.go @@ -22,6 +22,7 @@ package methods import ( "context" "encoding/json" + "errors" "fmt" "path/filepath" "testing" @@ -296,6 +297,44 @@ func TestHandleMediaSearch_IncludesCoverStatus(t *testing.T) { mockMediaDB.AssertExpectations(t) } +func TestHandleMediaSearch_CoverFailureIsNonFatal(t *testing.T) { + t.Parallel() + + mockMediaDB := helpers.NewMockMediaDBI() + mockMediaDB.On("SearchMediaWithFilters", mock.Anything, mock.Anything). + Return([]database.SearchResultWithCursor{{ + SystemID: "NES", Name: "Game", Path: filepath.Join("games", "game.nes"), + MediaID: 1, MediaTitleID: 11, + }}, nil) + mockMediaDB.On("GetMediaCoverStatus", mock.Anything, []database.MediaCoverRef{{ + MediaDBID: 1, MediaTitleDBID: 11, + }}).Run(func(args mock.Arguments) { + ctx, ok := args.Get(0).(context.Context) + require.True(t, ok) + deadline, ok := ctx.Deadline() + require.True(t, ok, "optional enrichment must have a deadline") + assert.WithinDuration(t, time.Now().Add(optionalDBEnrichmentTimeout), deadline, time.Second) + }).Return(nil, errors.New("cover lookup failed")) + + paramsJSON, err := json.Marshal(models.SearchParams{}) + require.NoError(t, err) + result, err := HandleMediaSearch(requests.RequestEnv{ + Context: context.Background(), + Params: paramsJSON, + Database: &database.Database{ + MediaDB: mockMediaDB, + }, + }) + require.NoError(t, err) + + response, ok := result.(models.SearchResults) + require.True(t, ok) + require.Len(t, response.Results, 1) + assert.Equal(t, int64(1), response.Results[0].MediaID) + assert.False(t, response.Results[0].HasCover) + mockMediaDB.AssertExpectations(t) +} + func TestHandleMediaSearch_WithExplicitSort(t *testing.T) { t.Parallel() diff --git a/pkg/api/transport_timing_test.go b/pkg/api/transport_timing_test.go new file mode 100644 index 000000000..b3b3ca74e --- /dev/null +++ b/pkg/api/transport_timing_test.go @@ -0,0 +1,208 @@ +// 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 api + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func findTransportLogEvent(t *testing.T, output, message string) map[string]any { + t.Helper() + for line := range strings.SplitSeq(strings.TrimSpace(output), "\n") { + var event map[string]any + if err := json.Unmarshal([]byte(line), &event); err != nil { + continue + } + if event["message"] == message { + return event + } + } + t.Fatalf("log event %q not found in %s", message, output) + return nil +} + +func TestLogWebSocketTransportTimingFields(t *testing.T) { + tests := []struct { + writeErr error + name string + responseType string + encrypted bool + }{ + {name: "plaintext result success", responseType: "result"}, + {name: "plaintext error failure", responseType: "error", writeErr: errors.New("write failed")}, + {name: "encrypted result success", responseType: "result", encrypted: true}, + { + name: "encrypted error failure", responseType: "error", encrypted: true, + writeErr: errors.New("write failed"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + originalLogger := log.Logger + log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) + defer func() { log.Logger = originalLogger }() + + logWebSocketTransportTiming( + models.NewStringID("request-1"), tt.responseType, tt.encrypted, 321, + 2*time.Millisecond, 3*time.Millisecond, tt.writeErr, + ) + + event := findTransportLogEvent(t, buf.String(), "websocket response transport timing") + assert.Equal(t, requestIDForLog(models.NewStringID("request-1")), event["requestId"]) + assert.Equal(t, tt.responseType, event["responseType"]) + assert.Equal(t, tt.encrypted, event["encrypted"]) + assert.InDelta(t, 321, event["responseBytes"], 0) + assert.Contains(t, event, "marshalDuration") + assert.Contains(t, event, "writeDuration") + if tt.writeErr != nil { + assert.Equal(t, tt.writeErr.Error(), event["error"]) + } else { + assert.NotContains(t, event, "error") + } + }) + } +} + +func TestHTTPResponseTransportTimingFields(t *testing.T) { + tests := []struct { + name string + method string + responseType string + }{ + {name: "result", method: "test.echo", responseType: "result"}, + {name: "error", method: "test.error", responseType: "error"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler, _, _ := createTestPostHandler(t) + var buf bytes.Buffer + originalLogger := log.Logger + log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) + defer func() { log.Logger = originalLogger }() + + body := `{"jsonrpc":"2.0","id":"transport-id","method":"` + tt.method + `"}` + req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/api", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + recorder := httptest.NewRecorder() + handler(recorder, req) + require.Equal(t, http.StatusOK, recorder.Code) + + event := findTransportLogEvent(t, buf.String(), "http response transport timing") + assert.Equal(t, tt.method, event["method"]) + assert.Equal(t, requestIDForLog(models.NewStringID("transport-id")), event["requestId"]) + assert.Equal(t, tt.responseType, event["responseType"]) + assert.Positive(t, event["responseBytes"]) + assert.Equal(t, event["responseBytes"], event["writtenBytes"]) + assert.Contains(t, event, "marshalDuration") + assert.Contains(t, event, "writeDuration") + assert.Equal(t, false, event["writeError"]) + }) + } +} + +func TestWebSocketDispatcherQueueMetadata(t *testing.T) { + assert.Zero(t, queueDuration(time.Time{})) + assert.Positive(t, queueDuration(time.Now().Add(-time.Millisecond))) + + d := &wsSessionDispatcher{ + ctx: t.Context(), + high: make(chan *wsRequestJob, 1), + normal: make(chan *wsRequestJob, 1), + low: make(chan *wsRequestJob, 1), + responses: make(chan *wsResponseJob, 1), + } + var methodMap MethodMap + env := &requests.RequestEnv{Context: context.Background()} + require.NoError(t, enqueueWSRequest( + d, &methodMap, env, + []byte(`{"jsonrpc":"2.0","method":"media.meta","id":"queue-id"}`), + nil, nil, + )) + requestJob := <-d.normal + assert.Equal(t, models.MethodMediaMeta, requestJob.method) + assert.Equal(t, models.NewStringID("queue-id"), requestJob.requestID) + assert.False(t, requestJob.enqueuedAt.IsZero()) + + responseJob := &wsResponseJob{method: requestJob.method, result: requestResult{ShouldReply: false}} + d.enqueueResponse(responseJob) + queuedResponse := <-d.responses + assert.Equal(t, models.MethodMediaMeta, queuedResponse.method) + assert.False(t, queuedResponse.enqueuedAt.IsZero()) + + require.NoError(t, d.enqueuePong(nil, nil)) + pong := <-d.responses + assert.True(t, pong.pong) + assert.Equal(t, "ping", pong.method) + assert.False(t, pong.enqueuedAt.IsZero()) +} + +func TestWebSocketDispatcherQueueTimingLogs(t *testing.T) { + var buf bytes.Buffer + originalLogger := log.Logger + log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) + defer func() { log.Logger = originalLogger }() + + var methodMap MethodMap + require.NoError(t, methodMap.AddMethod("test.queue", func(requests.RequestEnv) (any, error) { + return map[string]bool{"ok": true}, nil + })) + d := &wsSessionDispatcher{ctx: t.Context(), responses: make(chan *wsResponseJob, 1)} + job := &wsRequestJob{ + methodMap: &methodMap, + env: &requests.RequestEnv{Context: t.Context()}, + enqueuedAt: time.Now().Add(-time.Millisecond), + requestID: models.NewStringID("queue-log-id"), + method: "test.queue", + msg: []byte(`{"jsonrpc":"2.0","method":"test.queue","id":"queue-log-id"}`), + } + d.runJob(job) + response := <-d.responses + assert.False(t, response.enqueuedAt.IsZero()) + + requestEvent := findTransportLogEvent(t, buf.String(), "websocket request dequeued") + assert.Equal(t, "test.queue", requestEvent["method"]) + assert.Equal(t, requestIDForLog(models.NewStringID("queue-log-id")), requestEvent["requestId"]) + assert.Contains(t, requestEvent, "queueWaitDuration") + + response.result.ShouldReply = false + d.writeResponse(response) + responseEvent := findTransportLogEvent(t, buf.String(), "websocket response dequeued") + assert.Equal(t, "test.queue", responseEvent["method"]) + assert.Equal(t, requestIDForLog(models.NewStringID("queue-log-id")), responseEvent["requestId"]) + assert.Contains(t, responseEvent, "responseQueueDuration") +} diff --git a/pkg/database/database.go b/pkg/database/database.go index 802e4b952..98ae0508e 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -248,6 +248,7 @@ type MediaPathID struct { MediaTitleDBID int64 } +// MediaCoverRef identifies media and title rows for a cover-status lookup. type MediaCoverRef struct { MediaDBID int64 MediaTitleDBID int64 @@ -1045,6 +1046,8 @@ type MediaDBI interface { BrowseDirectories(ctx context.Context, opts BrowseDirectoriesOptions) ([]BrowseDirectoryResult, error) BrowseDirCount(ctx context.Context, opts BrowseDirCountOptions) (int, error) BrowseFiles(ctx context.Context, opts *BrowseFilesOptions) ([]SearchResultWithCursor, error) + // GetMediaCoverStatus returns statuses keyed by MediaDBID. True means a media- + // or title-level image exists; absent keys mean no cover. GetMediaCoverStatus(ctx context.Context, refs []MediaCoverRef) (map[int64]bool, error) BrowseFileCount(ctx context.Context, opts BrowseFileCountOptions) (int, error) BrowseIndex(ctx context.Context, opts BrowseIndexOptions) (BrowseIndexResult, error) diff --git a/pkg/database/mediadb/media_search_scope_test.go b/pkg/database/mediadb/media_search_scope_test.go index 767643afc..7f701d80f 100644 --- a/pkg/database/mediadb/media_search_scope_test.go +++ b/pkg/database/mediadb/media_search_scope_test.go @@ -21,10 +21,12 @@ package mediadb import ( "context" + "errors" "fmt" "path/filepath" "strings" "testing" + "time" "github.com/DATA-DOG/go-sqlmock" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" @@ -215,6 +217,53 @@ func TestMediaDB_SearchMediaWithFilters_FallsBackWhenScopedStreamIsSparse(t *tes assert.NoError(t, mock.ExpectationsWereMet()) } +func TestMediaDB_SearchMediaWithFilters_FallsBackWhenBoundsLookupFails(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + + nes, err := systemdefs.GetSystem(systemdefs.SystemNES) + require.NoError(t, err) + entries := make([]struct { + slug string + secSlug string + titleDBID int64 + systemDBID int64 + }, sqliteMaxParams) + for i := range entries { + entries[i] = struct { + slug string + secSlug string + titleDBID int64 + systemDBID int64 + }{slug: "rtype", titleDBID: int64(i + 1), systemDBID: 1} + } + cache := buildTestCache(entries, map[int64]string{1: nes.ID}) + cache.complete = true + + mediaDB := &MediaDB{} + mediaDB.sql.Store(db) + mediaDB.slugSearchCache.Store(cache) + mock.ExpectQuery("SELECT MIN\\(DBID\\), MAX\\(DBID\\).*FROM Media"). + WithArgs(int64(1)). + WillReturnError(errors.New("bounds unavailable")) + mock.ExpectPrepare("SELECT.*Systems\\.SystemID.*MediaTitles\\.Name.*Media\\.Path.*Media\\.DBID.*"). + ExpectQuery(). + WithArgs(nes.ID, "%rtype%", "%rtype%", 10). + WillReturnRows(sqlmock.NewRows([]string{ + "SystemID", "Name", "Path", "DBID", "MediaTitleDBID", "DisambiguationTypes", + })) + + results, err := mediaDB.SearchMediaWithFilters(context.Background(), &database.SearchFilters{ + Systems: []systemdefs.System{*nes}, Query: "R-Type", Limit: 10, + }) + require.NoError(t, err) + assert.Empty(t, results) + assert.NoError(t, mock.ExpectationsWereMet()) +} + func TestScopedCandidateStream_MatchesGroupedSQL(t *testing.T) { t.Parallel() @@ -307,6 +356,7 @@ func TestScopedCandidateStream_MatchesGroupedSQL(t *testing.T) { ctx, mediaDB.sql.Load(), snesCandidates[query], map[int64]string{1: "SNES"}, bounds, cursor, limit, ) require.NoError(t, streamErr) + require.Len(t, actual, len(expected)) assert.Equal(t, searchResultIDs(expected), searchResultIDs(actual)) for i := range actual { assert.Equal(t, expected[i].MediaTitleID, actual[i].MediaTitleID) @@ -345,6 +395,7 @@ func TestScopedCandidateStream_MatchesGroupedSQL(t *testing.T) { mediaDBIDBounds{first: 1, last: rowsPerSystem * 2}, nil, 100, ) require.NoError(t, err) + require.Len(t, multiActual, len(multiExpected)) assert.Equal(t, searchResultIDs(multiExpected), searchResultIDs(multiActual)) for i := range multiActual { assert.Equal(t, multiExpected[i].SystemID, multiActual[i].SystemID) @@ -396,6 +447,126 @@ func TestMediaSearchBounds_CachesAndClears(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +func TestMediaSearchBounds_CoalescesSameSystem(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + mock.ExpectQuery("SELECT MIN\\(DBID\\), MAX\\(DBID\\).*FROM Media"). + WithArgs(int64(7)). + WillDelayFor(100 * time.Millisecond). + WillReturnRows(sqlmock.NewRows([]string{"min", "max"}).AddRow(100, 200)) + + mediaDB := &MediaDB{} + mediaDB.sql.Store(db) + type boundsResult struct { + err error + bounds mediaDBIDBounds + found bool + } + results := make(chan boundsResult, 2) + load := func() { + bounds, found, queryErr := mediaDB.getMediaSearchBounds(context.Background(), 7) + results <- boundsResult{err: queryErr, bounds: bounds, found: found} + } + go load() + require.Eventually(t, func() bool { + mediaDB.mediaSearchBoundsMu.RLock() + defer mediaDB.mediaSearchBoundsMu.RUnlock() + return mediaDB.mediaSearchBoundsLoads[7] != nil + }, time.Second, time.Millisecond) + go load() + + for range 2 { + result := <-results + require.NoError(t, result.err) + assert.True(t, result.found) + assert.Equal(t, mediaDBIDBounds{first: 100, last: 200}, result.bounds) + } + assert.NoError(t, mock.ExpectationsWereMet(), "same-system misses should share one SQL query") +} + +func TestMediaSearchBounds_CoalescesPerSystem(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + db.SetMaxOpenConns(4) + mock.MatchExpectationsInOrder(false) + + for _, systemDBID := range []int64{7, 8} { + mock.ExpectQuery("SELECT MIN\\(DBID\\), MAX\\(DBID\\).*FROM Media"). + WithArgs(systemDBID). + WillDelayFor(100 * time.Millisecond). + WillReturnRows(sqlmock.NewRows([]string{"min", "max"}).AddRow(systemDBID*10, systemDBID*10+9)) + } + + mediaDB := &MediaDB{} + mediaDB.sql.Store(db) + type boundsResult struct { + err error + bounds mediaDBIDBounds + found bool + } + results := make(chan boundsResult, 2) + for _, systemDBID := range []int64{7, 8} { + go func() { + bounds, found, queryErr := mediaDB.getMediaSearchBounds(context.Background(), systemDBID) + results <- boundsResult{err: queryErr, bounds: bounds, found: found} + }() + } + + require.Eventually(t, func() bool { + mediaDB.mediaSearchBoundsMu.RLock() + defer mediaDB.mediaSearchBoundsMu.RUnlock() + return len(mediaDB.mediaSearchBoundsLoads) == 2 + }, time.Second, time.Millisecond, "different systems should load concurrently") + + for range 2 { + result := <-results + require.NoError(t, result.err) + assert.True(t, result.found) + assert.Positive(t, result.bounds.first) + } + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestMediaSearchBounds_InvalidatedLoadIsNotCached(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + mock.ExpectQuery("SELECT MIN\\(DBID\\), MAX\\(DBID\\).*FROM Media"). + WithArgs(int64(7)). + WillDelayFor(100 * time.Millisecond). + WillReturnRows(sqlmock.NewRows([]string{"min", "max"}).AddRow(100, 200)) + + mediaDB := &MediaDB{} + mediaDB.sql.Store(db) + result := make(chan mediaDBIDBounds, 1) + go func() { + bounds, _, _ := mediaDB.getMediaSearchBounds(context.Background(), 7) + result <- bounds + }() + + require.Eventually(t, func() bool { + mediaDB.mediaSearchBoundsMu.RLock() + defer mediaDB.mediaSearchBoundsMu.RUnlock() + return mediaDB.mediaSearchBoundsLoads[7] != nil + }, time.Second, time.Millisecond) + mediaDB.clearMediaSearchBounds() + + assert.Equal(t, mediaDBIDBounds{first: 100, last: 200}, <-result) + mediaDB.mediaSearchBoundsMu.RLock() + _, cached := mediaDB.mediaSearchBounds[7] + mediaDB.mediaSearchBoundsMu.RUnlock() + assert.False(t, cached, "an invalidated in-flight result must not be cached") + assert.NoError(t, mock.ExpectationsWereMet()) +} + func TestMediaSearchBounds_MetadataInvalidationPreservesCache(t *testing.T) { t.Parallel() diff --git a/pkg/database/mediadb/mediadb.go b/pkg/database/mediadb/mediadb.go index a0b871ecf..a3450bdeb 100644 --- a/pkg/database/mediadb/mediadb.go +++ b/pkg/database/mediadb/mediadb.go @@ -133,6 +133,14 @@ type mediaDBIDBounds struct { last int64 } +type mediaSearchBoundsLoad struct { + done chan struct{} + err error + bounds mediaDBIDBounds + generation uint64 + found bool +} + type MediaDB struct { clock clockwork.Clock ctx context.Context @@ -149,6 +157,7 @@ type MediaDB struct { inMemoryTagCache atomic.Pointer[tagCache] batchInsertTag *BatchInserter mediaSearchBounds map[int64]mediaDBIDBounds + mediaSearchBoundsLoads map[int64]*mediaSearchBoundsLoad stmtInsertMediaTag *sql.Stmt batchInsertMediaTitle *BatchInserter stmtInsertMediaTitle *sql.Stmt @@ -163,6 +172,7 @@ type MediaDB struct { backgroundOpsCount atomic.Int64 vacuumRetryDelay time.Duration analyzeRetryDelay time.Duration + mediaSearchBoundsGen uint64 batchSize int backgroundOpsMu syncutil.RWMutex mediaSearchBoundsMu syncutil.RWMutex @@ -273,47 +283,71 @@ func (db *MediaDB) clearMediaSearchBounds() { db.mediaSearchBoundsMu.Lock() defer db.mediaSearchBoundsMu.Unlock() db.mediaSearchBounds = nil + db.mediaSearchBoundsLoads = nil + db.mediaSearchBoundsGen++ } func (db *MediaDB) getMediaSearchBounds(ctx context.Context, systemDBID int64) (mediaDBIDBounds, bool, error) { - db.mediaSearchBoundsMu.RLock() - bounds, ok := db.mediaSearchBounds[systemDBID] - db.mediaSearchBoundsMu.RUnlock() - if ok { - return bounds, bounds.first > 0, nil - } - - // Serialize misses so concurrent first searches for one system do not repeat - // the covering-index scan. Stable libraries pay this once per system; media - // mutations clear the map through invalidateCaches. db.mediaSearchBoundsMu.Lock() - defer db.mediaSearchBoundsMu.Unlock() - if bounds, ok = db.mediaSearchBounds[systemDBID]; ok { + if bounds, ok := db.mediaSearchBounds[systemDBID]; ok { + db.mediaSearchBoundsMu.Unlock() return bounds, bounds.first > 0, nil } + if load := db.mediaSearchBoundsLoads[systemDBID]; load != nil { + db.mediaSearchBoundsMu.Unlock() + select { + case <-load.done: + return load.bounds, load.found, load.err + case <-ctx.Done(): + return mediaDBIDBounds{}, false, fmt.Errorf("query media search bounds: %w", ctx.Err()) + } + } + if db.mediaSearchBoundsLoads == nil { + db.mediaSearchBoundsLoads = make(map[int64]*mediaSearchBoundsLoad) + } + load := &mediaSearchBoundsLoad{ + done: make(chan struct{}), + generation: db.mediaSearchBoundsGen, + } + db.mediaSearchBoundsLoads[systemDBID] = load + db.mediaSearchBoundsMu.Unlock() queryStarted := time.Now() var firstMediaID, lastMediaID sql.NullInt64 - if err := db.sql.Load().QueryRowContext(ctx, ` + queryErr := db.sql.Load().QueryRowContext(ctx, ` SELECT MIN(DBID), MAX(DBID) FROM Media - WHERE SystemDBID = ? AND IsMissing = 0`, systemDBID).Scan(&firstMediaID, &lastMediaID); err != nil { - return mediaDBIDBounds{}, false, fmt.Errorf("query media search bounds: %w", err) + WHERE SystemDBID = ? AND IsMissing = 0`, systemDBID).Scan(&firstMediaID, &lastMediaID) + if queryErr != nil { + queryErr = fmt.Errorf("query media search bounds: %w", queryErr) + } else if firstMediaID.Valid && lastMediaID.Valid { + load.bounds = mediaDBIDBounds{first: firstMediaID.Int64, last: lastMediaID.Int64} } - if firstMediaID.Valid && lastMediaID.Valid { - bounds = mediaDBIDBounds{first: firstMediaID.Int64, last: lastMediaID.Int64} + load.found = load.bounds.first > 0 + load.err = queryErr + + db.mediaSearchBoundsMu.Lock() + if current := db.mediaSearchBoundsLoads[systemDBID]; current == load { + delete(db.mediaSearchBoundsLoads, systemDBID) } - if db.mediaSearchBounds == nil { - db.mediaSearchBounds = make(map[int64]mediaDBIDBounds) + if queryErr == nil && load.generation == db.mediaSearchBoundsGen { + if db.mediaSearchBounds == nil { + db.mediaSearchBounds = make(map[int64]mediaDBIDBounds) + } + db.mediaSearchBounds[systemDBID] = load.bounds } - db.mediaSearchBounds[systemDBID] = bounds - log.Debug(). - Int64("systemDBID", systemDBID). - Int64("firstMediaID", bounds.first). - Int64("lastMediaID", bounds.last). - Dur("duration", time.Since(queryStarted)). - Msg("media search system bounds cached") - return bounds, bounds.first > 0, nil + close(load.done) + db.mediaSearchBoundsMu.Unlock() + + if queryErr == nil { + log.Debug(). + Int64("systemDBID", systemDBID). + Int64("firstMediaID", load.bounds.first). + Int64("lastMediaID", load.bounds.last). + Dur("duration", time.Since(queryStarted)). + Msg("media search system bounds cached") + } + return load.bounds, load.found, load.err } func invalidationScopeForMediaSystemIDs(systemIDs []string) invalidationScope { @@ -2488,11 +2522,14 @@ func (db *MediaDB) SearchMediaWithFilters( if len(resolvedDBIDs) == len(systemIDs) { scopedSystems := make(map[int64]string, len(resolvedDBIDs)) var bounds mediaDBIDBounds + boundsReady := true for i, systemDBID := range resolvedDBIDs { - var systemBounds mediaDBIDBounds - var found bool - systemBounds, found, streamErr = db.getMediaSearchBounds(ctx, systemDBID) - if streamErr != nil { + systemBounds, found, boundsErr := db.getMediaSearchBounds(ctx, systemDBID) + if boundsErr != nil { + log.Debug().Err(boundsErr). + Int64("systemDBID", systemDBID). + Msg("media search bounds unavailable; using grouped SQL") + boundsReady = false break } if !found { @@ -2504,10 +2541,10 @@ func (db *MediaDB) SearchMediaWithFilters( } bounds.last = max(bounds.last, systemBounds.last) } - if streamErr == nil && len(scopedSystems) == 0 { + if boundsReady && len(scopedSystems) == 0 { return []database.SearchResultWithCursor{}, nil } - if streamErr == nil { + if boundsReady { strategy = "system-scope" streamResults, streamErr = sqlSearchMediaByLargeTitleDBIDSetInSystems( ctx, db.sql.Load(), candidateIDs, scopedSystems, bounds, @@ -2515,9 +2552,6 @@ func (db *MediaDB) SearchMediaWithFilters( } } } - if streamErr != nil && strategy == "" { - return nil, streamErr - } if strategy != "" { log.Debug(). Strs("systems", systemIDs). diff --git a/pkg/database/mediadb/sql_browse.go b/pkg/database/mediadb/sql_browse.go index b92f156fd..26423682b 100644 --- a/pkg/database/mediadb/sql_browse.go +++ b/pkg/database/mediadb/sql_browse.go @@ -1315,51 +1315,71 @@ func queryImagePropertyEntityIDs( return map[int64]struct{}{}, nil } - entityPlaceholders := prepareVariadic("?", ",", len(entityIDs)) - tagPlaceholders := prepareVariadic("?", ",", len(imageTagIDs)) - var query string + var table, alias, entityColumn string switch scope { case coverPropertyScopeMedia: - query = `SELECT mp.MediaDBID - FROM MediaProperties mp - WHERE mp.MediaDBID IN (` + entityPlaceholders + `) - AND mp.TypeTagDBID IN (` + tagPlaceholders + `)` + table, alias, entityColumn = "MediaProperties", "mp", "MediaDBID" case coverPropertyScopeTitle: - query = `SELECT mtp.MediaTitleDBID - FROM MediaTitleProperties mtp - WHERE mtp.MediaTitleDBID IN (` + entityPlaceholders + `) - AND mtp.TypeTagDBID IN (` + tagPlaceholders + `)` + table, alias, entityColumn = "MediaTitleProperties", "mtp", "MediaTitleDBID" default: return nil, fmt.Errorf("unknown cover property scope %d", scope) } - args := make([]any, 0, len(entityIDs)+len(imageTagIDs)) - for _, id := range entityIDs { - args = append(args, id) + chunkSize := sqliteMaxParams - len(imageTagIDs) + if chunkSize <= 0 { + return nil, fmt.Errorf("too many image property tag IDs: %d", len(imageTagIDs)) } - for _, id := range imageTagIDs { - args = append(args, id) + tagPlaceholders := prepareVariadic("?", ",", len(imageTagIDs)) + covered := make(map[int64]struct{}) + for start := 0; start < len(entityIDs); start += chunkSize { + chunk := entityIDs[start:min(start+chunkSize, len(entityIDs))] + entityPlaceholders := prepareVariadic("?", ",", len(chunk)) + query := `SELECT ` + alias + `.` + entityColumn + ` + FROM ` + table + ` ` + alias + ` + WHERE ` + alias + `.` + entityColumn + ` IN (` + entityPlaceholders + `) + AND ` + alias + `.TypeTagDBID IN (` + tagPlaceholders + `)` + + args := make([]any, 0, len(chunk)+len(imageTagIDs)) + for _, id := range chunk { + args = append(args, id) + } + for _, id := range imageTagIDs { + args = append(args, id) + } + + if err := queryImagePropertyEntityIDChunk(ctx, db, query, args, covered); err != nil { + return nil, err + } } + return covered, nil +} - //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?" +func queryImagePropertyEntityIDChunk( + ctx context.Context, + db sqlQueryable, + query string, + args []any, + covered map[int64]struct{}, +) (err error) { + //nolint:gosec // Caller selects table identifiers from fixed constants and generates placeholders internally. rows, err := db.QueryContext(ctx, query, args...) if err != nil { - return nil, fmt.Errorf("query cover entity IDs: %w", err) + return fmt.Errorf("query cover entity IDs: %w", err) } - defer func() { _ = rows.Close() }() + defer func() { + if closeErr := rows.Close(); err == nil && closeErr != nil { + err = fmt.Errorf("close cover entity ID rows: %w", closeErr) + } + }() - covered := make(map[int64]struct{}) for rows.Next() { var id int64 if scanErr := rows.Scan(&id); scanErr != nil { - return nil, fmt.Errorf("scan cover entity ID: %w", scanErr) + return fmt.Errorf("scan cover entity ID: %w", scanErr) } covered[id] = struct{}{} } - if rowsErr := rows.Err(); rowsErr != nil { - return nil, rowsErr - } - return covered, nil + return rows.Err() } func fetchCoverStatuses( diff --git a/pkg/database/mediadb/sql_browse_test.go b/pkg/database/mediadb/sql_browse_test.go index 55dfc8fb7..f88a8912e 100644 --- a/pkg/database/mediadb/sql_browse_test.go +++ b/pkg/database/mediadb/sql_browse_test.go @@ -22,6 +22,7 @@ package mediadb import ( "context" "database/sql" + "database/sql/driver" "errors" "path/filepath" "strings" @@ -988,6 +989,41 @@ func TestFetchAndAttachCoverFlags_TitleLevelCover(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +func TestQueryImagePropertyEntityIDs_ChunksMaximumPage(t *testing.T) { + t.Parallel() + + db, mock, err := testsqlmock.NewSQLMock() + require.NoError(t, err) + defer func() { _ = db.Close() }() + + const maxPageSize = 1000 + entityIDs := make([]int64, maxPageSize) + for i := range entityIDs { + entityIDs[i] = int64(i + 1) + } + firstArgs := make([]driver.Value, 0, sqliteMaxParams) + for _, id := range entityIDs[:sqliteMaxParams-1] { + firstArgs = append(firstArgs, id) + } + firstArgs = append(firstArgs, int64(901)) + secondArgs := []driver.Value{int64(999), int64(1000), int64(901)} + + queryPattern := `SELECT mtp\.MediaTitleDBID\s+FROM MediaTitleProperties mtp` + mock.ExpectQuery(queryPattern). + WithArgs(firstArgs...). + WillReturnRows(sqlmock.NewRows([]string{"MediaTitleDBID"}).AddRow(int64(1))) + mock.ExpectQuery(queryPattern). + WithArgs(secondArgs...). + WillReturnRows(sqlmock.NewRows([]string{"MediaTitleDBID"}).AddRow(int64(1000))) + + covered, err := queryImagePropertyEntityIDs( + context.Background(), db, coverPropertyScopeTitle, entityIDs, []int64{901}, + ) + require.NoError(t, err) + assert.Equal(t, map[int64]struct{}{1: {}, 1000: {}}, covered) + assert.NoError(t, mock.ExpectationsWereMet()) +} + func TestFetchAndAttachCoverFlags_QueryError(t *testing.T) { t.Parallel() db, mock, err := testsqlmock.NewSQLMock()