diff --git a/docs/scraper.md b/docs/scraper.md index e9c4485da..2ea14ec8c 100644 --- a/docs/scraper.md +++ b/docs/scraper.md @@ -186,11 +186,11 @@ Game manuals can be selected through Update All's **Game Manuals (EN)** settings ### Discovery -Core derives `docs` roots from MiSTer's configured SD, USB, network/CIFS, and custom index roots. It recognizes content by installed format rather than repository name: +Core derives `docs` roots from MiSTer's configured SD, USB, network/CIFS, and custom index roots, and also probes `/media/usb6` and `/media/usb7`, which artwork packs may be installed to but MiSTer's games-folder list does not reach. It recognizes content by installed format rather than repository name, following the [MiSTer Artwork Pack format](https://github.com/chipster6502/MiSTer_artwork_pack/blob/main/PACK_FORMAT.md): -- Artwork: `docs//Artwork/index.tsv` plus image files in the same directory. -- Optional title metadata: `gameinfo.tsv` beside the artwork index. -- Optional description: `synopsis_en.tsv` beside the artwork index. +- Artwork: `docs//Artwork/` holding one `.jpg` per game, normally with an `index.tsv` beside them that maps every known dump to its key. `` is the MiSTer `games/` folder name. A directory with images and no index still resolves games filed under their exact key. +- Optional title metadata: `gameinfo.tsv` beside the images. Games it lists without an image still receive their metadata. +- Optional description: `synopsis_.tsv` beside the images. Which languages a pack ships varies per system, so Core reads whichever files exist and picks the first match from `media.default_langs`, then English, then the first available language. - Manuals: direct PDF files in a child directory whose name contains `manual`, for example `docs/SNES/Manuals/` or `docs/NES/Famicom Disk System Manuals/`. This format-based discovery means future compatible databases need no Core update. Run `mister-docs` again after Downloader installs or updates content. Normal runs rescan installed records idempotently; force runs additionally delete stale box-art/manual properties whose old paths are proven to belong to a discovered MiSTer docs convention. @@ -199,7 +199,14 @@ Metadata files are treated as untrusted input. Core bounds their size and record ### Matching And Fields -`index.tsv` maps ROM or MRA basenames to canonical artwork keys. Core prefers an exact media basename match. When no exact media match exists, a unique title-slug match may receive title-level artwork; ambiguous matches are skipped. CRC and size columns are not used because hashing every installed ROM would impose substantial MiSTer I/O. +`index.tsv` maps catalogued dump names to artwork keys: No-Intro names for cartridges, Redump names for CD systems, and MAME parent setnames for arcade. Core resolves each pack entry to installed media in the pack format's order, stopping at the first hit: + +1. The catalogued name as a media basename, at media scope. +2. For arcade, the `` inside each installed `.mra`, at media scope. MRA filenames are titles, so the setname is the only handle an arcade key has; Core reads it from the MRA only for systems that have an arcade artwork source. +3. A media basename whose trailing parenthesised tag is itself a pack key, such as `Shock Troopers (set 1) (shocktro)`, at media scope. +4. A unique bare-title match, at title scope. This step is skipped when the stripped title is not unique among the pack's keys or among the library's titles, and it is only available to index rows: images the index does not mention resolve by exact name alone. + +CRC and size columns are not used because hashing every installed ROM would impose substantial MiSTer I/O; the pack format treats that step as optional. | Source | Destination | |---|---| @@ -208,10 +215,12 @@ Metadata files are treated as untrusted input. Core bounds their size and record | `gameinfo.tsv` genre | title tag `genre` | | `gameinfo.tsv` developer | title tag `developer` | | `gameinfo.tsv` players | title tag `players` using highest numeric value | -| `synopsis_en.tsv` synopsis | title property `property:description` | +| `synopsis_.tsv` synopsis | title property `property:description` | | Manual PDF | title property `property:manual` | -Manual filenames are matched with the same game-title slug normalization used by MediaDB, including leading/trailing article handling. Basenames with no matching title, or whose slug collision remains ambiguous after normalized-name matching, are left unmatched. Category-like names such as system manuals, overlays, or charts are not filtered separately. Base-system sources can enrich compatible fallback systems such as SNES MSU and Genesis variants. +Manual filenames are matched with the same game-title slug normalization used by MediaDB, including leading/trailing article handling. Basenames with no matching title, or whose slug collision remains ambiguous after normalized-name matching, are left unmatched. Category-like names such as system manuals, overlays, or charts are not filtered separately. + +Base-system sources enrich their variants, such as SNES MSU-1, Genesis MSU, and the granular arcade systems. On top of that, Core applies the pack format's shared-catalogue rules: Game Boy and Game Boy Color each fall back to the other, Super Game Boy reads both, and FDS falls back to NES but never the reverse. Systems the pack catalogues separately do not fill each other's gaps, so SG-1000 never receives ColecoVision art and Neo Geo Pocket Color never receives Neo Geo Pocket art, even though the general system fallbacks allow it. If multiple docs roots provide the same property, MiSTer root order decides which source wins. As with other scrapers, running a different scraper later may replace exclusive tags or same-type properties. diff --git a/pkg/database/mediadb/sql_property_plan_test.go b/pkg/database/mediadb/sql_property_plan_test.go new file mode 100644 index 000000000..deff6ca1b --- /dev/null +++ b/pkg/database/mediadb/sql_property_plan_test.go @@ -0,0 +1,97 @@ +// 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 mediadb + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPropertyLookupsDriveFromPropertyTable pins the join order of the bulk +// property lookups. Scrapers fill the property tables after indexing has run +// ANALYZE, so the planner usually has no statistics for them; without a fixed +// order it drove a many-ID lookup from TagTypes outward and probed the +// property index once per tag per requested ID, which took minutes on a +// MiSTer for a few thousand IDs. +func TestPropertyLookupsDriveFromPropertyTable(t *testing.T) { + t.Parallel() + mediaDB, cleanup := setupTempMediaDB(t) + defer cleanup() + ctx := context.Background() + + const ids = 2000 + args := make([]any, ids) + for i := range args { + args[i] = int64(i + 1) + } + inList := prepareVariadic("?", ",", ids) + + tests := []struct { + name string + query string + wantFirst string + }{ + { + name: "media properties", + query: mediaPropertyQuery("WHERE mp.MediaDBID IN ("+inList+")", propertyGroupInclude), + wantFirst: "SEARCH mp USING INDEX mediaproperties_media_idx (MediaDBID=?)", + }, + { + name: "media property metadata", + query: mediaPropertyMetadataQuery("WHERE mp.MediaDBID IN ("+inList+")", propertyGroupInclude), + wantFirst: "SEARCH mp USING INDEX mediaproperties_media_idx (MediaDBID=?)", + }, + { + name: "title properties", + query: mediaTitlePropertyQuery("WHERE mtp.MediaTitleDBID IN ("+inList+")", propertyGroupInclude), + wantFirst: "SEARCH mtp USING INDEX mediatitleproperties_title_idx (MediaTitleDBID=?)", + }, + { + name: "title property metadata", + query: mediaTitlePropertyMetadataQuery( + "WHERE mtp.MediaTitleDBID IN ("+inList+")", propertyGroupInclude, + ), + wantFirst: "SEARCH mtp USING INDEX mediatitleproperties_title_idx (MediaTitleDBID=?)", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rows, err := mediaDB.sql.Load().QueryContext(ctx, "EXPLAIN QUERY PLAN "+tt.query, args...) + require.NoError(t, err) + defer func() { _ = rows.Close() }() + var plan []string + for rows.Next() { + var id, parent, notUsed int + var detail string + require.NoError(t, rows.Scan(&id, &parent, ¬Used, &detail)) + plan = append(plan, detail) + } + require.NoError(t, rows.Err()) + require.NotEmpty(t, plan) + assert.Equal(t, tt.wantFirst, plan[0], "plan: %v", plan) + for _, step := range plan { + assert.NotContains(t, step, "SCAN tt", "plan drives from TagTypes: %v", plan) + } + }) + } +} diff --git a/pkg/database/mediadb/sql_scraper.go b/pkg/database/mediadb/sql_scraper.go index 0de589990..334f2a917 100644 --- a/pkg/database/mediadb/sql_scraper.go +++ b/pkg/database/mediadb/sql_scraper.go @@ -2609,7 +2609,9 @@ func (db *MediaDB) loadMediaTitlePropertiesByMediaTitleDBIDs( args := int64Args(mediaTitleDBIDs) where := `WHERE mtp.MediaTitleDBID IN (` + prepareVariadic("?", ",", len(mediaTitleDBIDs)) + `)` //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?". - rows, err := db.sql.Load().QueryContext(ctx, mediaTitlePropertyQuery(where, propertyGroupInclude), args...) + rows, err := db.sql.Load().QueryContext( + ctx, mediaTitlePropertyQuery(where, propertyGroupInclude), args..., + ) if err != nil { return nil, fmt.Errorf("failed to query GetMediaTitlePropertiesByMediaTitleDBIDs: %w", err) } @@ -2667,7 +2669,9 @@ func (db *MediaDB) GetMediaTitlePropertyMetadataByMediaTitleDBIDs( args := int64Args(mediaTitleDBIDs) where := `WHERE mtp.MediaTitleDBID IN (` + prepareVariadic("?", ",", len(mediaTitleDBIDs)) + `)` //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?". - rows, err := db.sql.Load().QueryContext(ctx, mediaTitlePropertyMetadataQuery(where, propertyGroupInclude), args...) + rows, err := db.sql.Load().QueryContext( + ctx, mediaTitlePropertyMetadataQuery(where, propertyGroupInclude), args..., + ) if err != nil { return nil, fmt.Errorf("failed to query GetMediaTitlePropertyMetadataByMediaTitleDBIDs: %w", err) } @@ -2737,7 +2741,9 @@ func (db *MediaDB) loadMediaPropertiesByMediaDBIDs( args := int64Args(mediaDBIDs) where := `WHERE mp.MediaDBID IN (` + prepareVariadic("?", ",", len(mediaDBIDs)) + `)` //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?". - rows, err := db.sql.Load().QueryContext(ctx, mediaPropertyQuery(where, propertyGroupInclude), args...) + rows, err := db.sql.Load().QueryContext( + ctx, mediaPropertyQuery(where, propertyGroupInclude), args..., + ) if err != nil { return nil, fmt.Errorf("failed to query GetMediaPropertiesByMediaDBIDs: %w", err) } @@ -2792,7 +2798,9 @@ func (db *MediaDB) GetMediaPropertyMetadataByMediaDBIDs( args := int64Args(mediaDBIDs) where := `WHERE mp.MediaDBID IN (` + prepareVariadic("?", ",", len(mediaDBIDs)) + `)` //nolint:gosec // Safe: prepareVariadic only generates SQL placeholders like "?, ?, ?". - rows, err := db.sql.Load().QueryContext(ctx, mediaPropertyMetadataQuery(where, propertyGroupInclude), args...) + rows, err := db.sql.Load().QueryContext( + ctx, mediaPropertyMetadataQuery(where, propertyGroupInclude), args..., + ) if err != nil { return nil, fmt.Errorf("failed to query GetMediaPropertyMetadataByMediaDBIDs: %w", err) } @@ -3148,12 +3156,19 @@ func propertyMetadataSelectColumns(entityIDColumn string, groupMode propertyGrou return strings.Join(parts, ", ") } +// The property query builders below join with CROSS JOIN, which SQLite +// treats as an inner join whose nesting order is fixed left to right. Without +// it, and without fresh statistics on the property tables, the planner drives +// a lookup of many IDs from TagTypes outward and probes the property index +// once per tag per requested ID - a few thousand IDs then take minutes on a +// MiSTer. Starting from the property table keeps every lookup at one index +// search per requested ID whatever the statistics say. func mediaTitlePropertyQuery(where string, groupMode propertyGroupMode) string { return ` SELECT ` + propertySelectColumns("mtp.MediaTitleDBID", groupMode) + ` FROM MediaTitleProperties mtp - JOIN Tags t ON mtp.TypeTagDBID = t.DBID - JOIN TagTypes tt ON t.TypeDBID = tt.DBID + CROSS JOIN Tags t ON mtp.TypeTagDBID = t.DBID + CROSS JOIN TagTypes tt ON t.TypeDBID = tt.DBID LEFT JOIN MediaBlobs mb ON mtp.BlobDBID = mb.DBID ` + where } @@ -3162,8 +3177,8 @@ func mediaTitlePropertyMetadataQuery(where string, groupMode propertyGroupMode) return ` SELECT ` + propertyMetadataSelectColumns("mtp.MediaTitleDBID", groupMode) + ` FROM MediaTitleProperties mtp - JOIN Tags t ON mtp.TypeTagDBID = t.DBID - JOIN TagTypes tt ON t.TypeDBID = tt.DBID + CROSS JOIN Tags t ON mtp.TypeTagDBID = t.DBID + CROSS JOIN TagTypes tt ON t.TypeDBID = tt.DBID LEFT JOIN MediaBlobs mb ON mtp.BlobDBID = mb.DBID ` + where } @@ -3172,8 +3187,8 @@ func mediaPropertyQuery(where string, groupMode propertyGroupMode) string { return ` SELECT ` + propertySelectColumns("mp.MediaDBID", groupMode) + ` FROM MediaProperties mp - JOIN Tags t ON mp.TypeTagDBID = t.DBID - JOIN TagTypes tt ON t.TypeDBID = tt.DBID + CROSS JOIN Tags t ON mp.TypeTagDBID = t.DBID + CROSS JOIN TagTypes tt ON t.TypeDBID = tt.DBID LEFT JOIN MediaBlobs mb ON mp.BlobDBID = mb.DBID ` + where } @@ -3182,8 +3197,8 @@ func mediaPropertyMetadataQuery(where string, groupMode propertyGroupMode) strin return ` SELECT ` + propertyMetadataSelectColumns("mp.MediaDBID", groupMode) + ` FROM MediaProperties mp - JOIN Tags t ON mp.TypeTagDBID = t.DBID - JOIN TagTypes tt ON t.TypeDBID = tt.DBID + CROSS JOIN Tags t ON mp.TypeTagDBID = t.DBID + CROSS JOIN TagTypes tt ON t.TypeDBID = tt.DBID LEFT JOIN MediaBlobs mb ON mp.BlobDBID = mb.DBID ` + where } diff --git a/pkg/database/scraper/misterdocs/discovery.go b/pkg/database/scraper/misterdocs/discovery.go index 72ac3764c..0e04688fd 100644 --- a/pkg/database/scraper/misterdocs/discovery.go +++ b/pkg/database/scraper/misterdocs/discovery.go @@ -69,9 +69,21 @@ func candidateDocsRoots(roots []string) []string { } appendRoot(filepath.Join(root, "docs")) } + for _, root := range extraDocsRoots { + appendRoot(root) + } return result } +// extraDocsRoots are mount points artwork packs tell consumers to probe that +// MiSTer's own games-folder list does not reach. Packs install with the +// Downloader's "pext" path, so docs can land on any USB drive. Roots that do +// not exist are skipped during discovery, so probing costs one stat each. +var extraDocsRoots = []string{ //nolint:gochecknoglobals // Fixed MiSTer mount points. + "/media/usb6/docs", + "/media/usb7/docs", +} + func discoverSources(fs afero.Fs, roots []string) ([]sourceDir, error) { var result []sourceDir seen := make(map[string]struct{}) @@ -100,8 +112,7 @@ func discoverSources(fs afero.Fs, roots []string) ([]sourceDir, error) { var kind sourceKind var systemID string switch { - case strings.EqualFold(child.Name(), artworkDirName) && - isRegularFile(fs, filepath.Join(path, indexFileName)): + case strings.EqualFold(child.Name(), artworkDirName) && hasArtworkContent(fs, path): kind = sourceArtwork systemID = resolveSourceSystem(systemEntry.Name(), "") case strings.Contains(strings.ToLower(child.Name()), "manual"): @@ -172,6 +183,33 @@ func pathWithin(path, root string) bool { return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) } +// hasArtworkContent reports whether an Artwork directory holds anything worth +// loading. The index resolves every dump not filed under its own key, but a +// pack shipped without one still serves exact-key images, so a directory with +// images and no index is a valid source rather than a directory to ignore. +func hasArtworkContent(fs afero.Fs, dir string) bool { + if isRegularFile(fs, filepath.Join(dir, indexFileName)) { + return true + } + directory, err := fs.Open(dir) + if err != nil { + return false + } + defer func() { _ = directory.Close() }() + for { + entries, readErr := directory.Readdir(directoryReadBatch) + for _, entry := range entries { + if !entry.IsDir() && entry.Mode()&os.ModeSymlink == 0 && + supportedImageExt(filepath.Ext(entry.Name())) { + return true + } + } + if readErr != nil { + return false + } + } +} + func isRegularDir(fs afero.Fs, path string) bool { info, err := lstat(fs, path) return err == nil && info.IsDir() && info.Mode()&os.ModeSymlink == 0 @@ -205,6 +243,29 @@ func sourcesBySystem(sources []sourceDir) map[string][]sourceDir { return result } +// artworkSiblings adds the shared-catalogue fallbacks the artwork pack format +// specifies and the general system definitions do not model. ScreenScraper +// splits dual-mode cartridges between the Game Boy and Game Boy Color +// catalogues on its own criteria, so each has to try the other, and Super Game +// Boy ships no pack of its own. Famicom Disk System is asymmetric on purpose: +// a disk release may borrow the cartridge box, but a cartridge must never +// receive the disk release's. +var artworkSiblings = map[string][]string{ //nolint:gochecknoglobals // Fixed artwork pack rules. + systemdefs.SystemGameboy: {systemdefs.SystemGameboyColor}, + systemdefs.SystemGameboyColor: {systemdefs.SystemGameboy}, + systemdefs.SystemSuperGameboy: {systemdefs.SystemGameboy, systemdefs.SystemGameboyColor}, + systemdefs.SystemFDS: {systemdefs.SystemNES}, +} + +// artworkFallbackBlocks drops general system fallbacks between systems the +// artwork pack catalogues separately. Filling an SG-1000 gap with a +// ColecoVision box serves art for a different release, and the pack's own rule +// is that an absent image beats a wrong one. +var artworkFallbackBlocks = map[string]map[string]struct{}{ //nolint:gochecknoglobals // Fixed artwork pack rules. + systemdefs.SystemSG1000: {systemdefs.SystemColecoVision: {}}, + systemdefs.SystemNeoGeoPocketColor: {systemdefs.SystemNeoGeoPocket: {}}, +} + func sourceIDsForTarget(targetID string) []string { seen := make(map[string]struct{}) var result []string @@ -215,12 +276,17 @@ func sourceIDsForTarget(targetID string) []string { } seen[id] = struct{}{} result = append(result, id) - sys, err := systemdefs.GetSystem(id) - if err != nil { - return + blocked := artworkFallbackBlocks[id] + if sys, err := systemdefs.GetSystem(id); err == nil { + for _, fallback := range sys.Fallbacks { + if _, ok := blocked[fallback]; ok { + continue + } + visit(fallback) + } } - for _, fallback := range sys.Fallbacks { - visit(fallback) + for _, sibling := range artworkSiblings[id] { + visit(sibling) } } visit(targetID) diff --git a/pkg/database/scraper/misterdocs/discovery_test.go b/pkg/database/scraper/misterdocs/discovery_test.go index 7bd086200..c8b11abfc 100644 --- a/pkg/database/scraper/misterdocs/discovery_test.go +++ b/pkg/database/scraper/misterdocs/discovery_test.go @@ -59,6 +59,8 @@ func TestCandidateDocsRoots_PreservesOrderAndDeduplicates(t *testing.T) { filepath.Join("media", "usb0", "docs"), filepath.Join("media", "usb0", "games", "docs"), filepath.Join("media", "fat", "docs"), + filepath.Clean("/media/usb6/docs"), + filepath.Clean("/media/usb7/docs"), }, got) } @@ -139,3 +141,102 @@ func TestOrderedTargetSystems_ResolvesAliasesAndFiltersUnindexed(t *testing.T) { ) assert.Equal(t, []string{systemdefs.SystemGenesis, systemdefs.SystemSNES}, got) } + +// TestResolveSourceSystem_ResolvesEveryPublishedPackFolder pins the folder +// names the MiSTer Artwork Pack publishes (PACK_FORMAT.md, "Published +// systems"), so a rename on either side is caught here rather than as a pack +// that silently imports nothing. Satellaview is published too but has no +// system in systemdefs, so it is deliberately absent. +func TestResolveSourceSystem_ResolvesEveryPublishedPackFolder(t *testing.T) { + t.Parallel() + + folders := map[string]string{ + "3DO": systemdefs.System3DO, + "ATARI5200": systemdefs.SystemAtari5200, + "ATARI7800": systemdefs.SystemAtari7800, + "AmigaCD32": systemdefs.SystemAmigaCD32, + "Arcade": systemdefs.SystemArcade, + "Atari2600": systemdefs.SystemAtari2600, + "AtariLynx": systemdefs.SystemAtariLynx, + "CD-i": systemdefs.SystemCDI, + "Coleco": systemdefs.SystemColecoVision, + "FDS": systemdefs.SystemFDS, + "GAMEBOY": systemdefs.SystemGameboy, + "GBA": systemdefs.SystemGBA, + "GBC": systemdefs.SystemGameboyColor, + "GameGear": systemdefs.SystemGameGear, + "Genesis": systemdefs.SystemGenesis, + "Intellivision": systemdefs.SystemIntellivision, + "Jaguar": systemdefs.SystemJaguar, + "MegaCD": systemdefs.SystemMegaCD, + "N64": systemdefs.SystemNintendo64, + "NEOGEO": systemdefs.SystemNeoGeo, + "NES": systemdefs.SystemNES, + "NeoGeo-CD": systemdefs.SystemNeoGeoCD, + "NeoGeoPocket": systemdefs.SystemNeoGeoPocket, + "NeoGeoPocket-Color": systemdefs.SystemNeoGeoPocketColor, + "ODYSSEY2": systemdefs.SystemOdyssey2, + "PSX": systemdefs.SystemPSX, + "S32X": systemdefs.SystemSega32X, + "SG-1000": systemdefs.SystemSG1000, + "SMS": systemdefs.SystemMasterSystem, + "SNES": systemdefs.SystemSNES, + "Saturn": systemdefs.SystemSaturn, + "SuperGrafx": systemdefs.SystemSuperGrafx, + "TGFX16": systemdefs.SystemTurboGrafx16, + "TGFX16-CD": systemdefs.SystemTurboGrafx16CD, + "VECTREX": systemdefs.SystemVectrex, + "VirtualBoy": systemdefs.SystemVirtualBoy, + "WonderSwan": systemdefs.SystemWonderSwan, + "WonderSwanColor": systemdefs.SystemWonderSwanColor, + } + for folder, want := range folders { + assert.Equal(t, want, resolveSourceSystem(folder, ""), "docs/%s/Artwork", folder) + } +} + +func TestSourceIDsForTarget_FollowsArtworkPackSiblingRules(t *testing.T) { + t.Parallel() + + tests := []struct { + target string + want []string + }{ + // Game Boy and Game Boy Color each try the other; Super Game Boy has + // no pack of its own and reads both. + {target: systemdefs.SystemGameboy, want: []string{systemdefs.SystemGameboy, systemdefs.SystemGameboyColor}}, + {target: systemdefs.SystemGameboyColor, want: []string{ + systemdefs.SystemGameboyColor, systemdefs.SystemGameboy, + }}, + {target: systemdefs.SystemSuperGameboy, want: []string{ + systemdefs.SystemSuperGameboy, systemdefs.SystemGameboy, systemdefs.SystemGameboyColor, + }}, + // A disk release may borrow the cartridge box; a cartridge must never + // receive the disk release's. + {target: systemdefs.SystemFDS, want: []string{systemdefs.SystemFDS, systemdefs.SystemNES}}, + {target: systemdefs.SystemNES, want: []string{systemdefs.SystemNES}}, + // Separately catalogued systems do not fill each other's gaps, even + // where the general system fallbacks say they may. + {target: systemdefs.SystemSG1000, want: []string{systemdefs.SystemSG1000}}, + {target: systemdefs.SystemNeoGeoPocketColor, want: []string{systemdefs.SystemNeoGeoPocketColor}}, + // Variants of one catalogue still inherit from it. + {target: systemdefs.SystemCPS2, want: []string{systemdefs.SystemCPS2, systemdefs.SystemArcade}}, + } + for _, tt := range tests { + assert.Equal(t, tt.want, sourceIDsForTarget(tt.target), tt.target) + } +} + +func TestDiscoverSources_AcceptsArtworkDirectoryWithoutIndex(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + root := filepath.Join("media", "fat") + artwork := filepath.Join(root, "docs", "Genesis", artworkDirName) + require.NoError(t, fs.MkdirAll(artwork, 0o750)) + require.NoError(t, afero.WriteFile(fs, filepath.Join(artwork, "Sonic (USA).jpg"), []byte("image"), 0o600)) + + sources, err := discoverSources(fs, []string{root}) + require.NoError(t, err) + assert.Equal(t, []sourceDir{{Path: artwork, SystemID: systemdefs.SystemGenesis, Kind: sourceArtwork}}, sources) +} diff --git a/pkg/database/scraper/misterdocs/matching.go b/pkg/database/scraper/misterdocs/matching.go index a11bade35..728993561 100644 --- a/pkg/database/scraper/misterdocs/matching.go +++ b/pkg/database/scraper/misterdocs/matching.go @@ -37,6 +37,14 @@ type systemIndex struct { titlesByID map[int64]database.TitleWithSystem mediaByBase map[string][]database.MediaWithFullPath mediaByTitle map[int64][]database.MediaWithFullPath + // mediaByTrailingTag indexes media on the parenthesised tag their name ends + // with, so a ROM pack that prefixes an identifier with a title of its own + // invention still resolves: "Shock Troopers (set 1) (shocktro)". + mediaByTrailingTag map[string][]database.MediaWithFullPath + // mediaBySetName indexes arcade media on the setname inside their MRA. It + // is populated only for systems with an arcade artwork source, because + // filling it means reading every MRA. + mediaBySetName map[string][]database.MediaWithFullPath } type pendingWrite struct { @@ -46,6 +54,9 @@ type pendingWrite struct { titleProp map[string]database.MediaProperty mediaID int64 titleID int64 + // records counts the pack records this write resolves, so write progress + // can be reported in the same unit the step's totals use. + records int } type matchStats struct { @@ -56,10 +67,12 @@ type matchStats struct { func newSystemIndex(titles []database.TitleWithSystem, media []database.MediaWithFullPath) systemIndex { idx := systemIndex{ - titlesBySlug: make(map[string][]database.TitleWithSystem, len(titles)), - titlesByID: make(map[int64]database.TitleWithSystem, len(titles)), - mediaByBase: make(map[string][]database.MediaWithFullPath, len(media)), - mediaByTitle: make(map[int64][]database.MediaWithFullPath, len(titles)), + titlesBySlug: make(map[string][]database.TitleWithSystem, len(titles)), + titlesByID: make(map[int64]database.TitleWithSystem, len(titles)), + mediaByBase: make(map[string][]database.MediaWithFullPath, len(media)), + mediaByTitle: make(map[int64][]database.MediaWithFullPath, len(titles)), + mediaByTrailingTag: make(map[string][]database.MediaWithFullPath), + mediaBySetName: make(map[string][]database.MediaWithFullPath), } for _, title := range titles { idx.titlesBySlug[title.Slug] = append(idx.titlesBySlug[title.Slug], title) @@ -69,6 +82,9 @@ func newSystemIndex(titles []database.TitleWithSystem, media []database.MediaWit base := normalizedMediaBase(item.Path) if base != "" { idx.mediaByBase[base] = append(idx.mediaByBase[base], item) + if tag := trailingParenTag(base); tag != "" { + idx.mediaByTrailingTag[tag] = append(idx.mediaByTrailingTag[tag], item) + } } idx.mediaByTitle[item.MediaTitleDBID] = append(idx.mediaByTitle[item.MediaTitleDBID], item) } @@ -84,39 +100,37 @@ func newSystemIndex(titles []database.TitleWithSystem, media []database.MediaWit return idx } +// matchResult is what one step's matching produced: the write targets, how +// many pack records each target resolves (index-aligned, for progress in the +// same unit as the totals), the match counters, and every image path a +// record referenced, which force runs use to recognise stale properties. +type matchResult struct { + Found map[string]struct{} + Targets []database.ScrapeWriteTarget + RecordsPerTarget []int + Stats matchStats +} + +// buildPendingWrites resolves every record to a write target. func buildPendingWrites( idx systemIndex, records []sourceRecords, runID string, -) ([]database.ScrapeWriteTarget, matchStats, map[string]struct{}) { +) matchResult { pending := make(map[int64]*pendingWrite) foundPaths := make(map[string]struct{}) stats := matchStats{} for _, source := range records { for _, record := range source.Artwork { stats.Processed++ - foundPaths[filepath.Clean(record.ImagePath)] = struct{}{} - media, title, exact := matchArtwork(idx, record) - if media == nil || title == nil { - stats.Skipped++ - continue - } - write := getPending(pending, media.DBID, title.DBID) - prop := database.MediaProperty{ - TypeTag: tags.PropertyTypeTag(tags.TagPropertyImageBoxart), - Text: filepath.ToSlash(record.ImagePath), + if record.ImagePath != "" { + foundPaths[filepath.Clean(record.ImagePath)] = struct{}{} } - props := write.titleProp - if exact { - props = write.mediaProp - } - if _, exists := props[prop.TypeTag]; exists { + if applyArtworkRecord(idx, pending, source, record) { + stats.Matched++ + } else { stats.Skipped++ - continue } - props[prop.TypeTag] = prop - applyGameMetadata(write, source, record.Key) - stats.Matched++ } for _, manualPath := range source.Manuals { stats.Processed++ @@ -141,6 +155,7 @@ func buildPendingWrites( continue } write.titleProp[prop.TypeTag] = prop + write.records++ stats.Matched++ } stats.Skipped += source.RowErrors @@ -153,8 +168,10 @@ func buildPendingWrites( } sort.Slice(mediaIDs, func(i, j int) bool { return mediaIDs[i] < mediaIDs[j] }) targets := make([]database.ScrapeWriteTarget, 0, len(mediaIDs)) + recordsPerTarget := make([]int, 0, len(mediaIDs)) for _, mediaID := range mediaIDs { p := pending[mediaID] + recordsPerTarget = append(recordsPerTarget, p.records) write := &database.ScrapeWrite{ Sentinel: scraper.SentinelTagInfo(scraperID), MediaTags: sortedTags(p.mediaTags), @@ -169,39 +186,65 @@ func buildPendingWrites( MediaDBID: p.mediaID, MediaTitleDBID: p.titleID, Write: write, }) } - return targets, stats, foundPaths + return matchResult{Targets: targets, RecordsPerTarget: recordsPerTarget, Stats: stats, Found: foundPaths} } -func matchArtwork( +// applyArtworkRecord resolves one pack record to a media row and stages its +// image and metadata. A record with no image still carries title metadata, for +// games the pack catalogues without artwork. +func applyArtworkRecord( idx systemIndex, + pending map[int64]*pendingWrite, + source sourceRecords, record artworkRecord, -) (*database.MediaWithFullPath, *database.TitleWithSystem, bool) { - base := strings.ToLower(strings.TrimSpace(record.Name)) - if candidates := idx.mediaByBase[base]; len(candidates) == 1 { - media := candidates[0] - if title := titleByID(idx, media.MediaTitleDBID); title != nil { - return &media, title, true +) bool { + media, title, exact := matchArtwork(idx, record) + if media == nil || title == nil { + return false + } + write := getPending(pending, media.DBID, title.DBID) + if record.ImagePath != "" { + prop := database.MediaProperty{ + TypeTag: tags.PropertyTypeTag(tags.TagPropertyImageBoxart), + Text: filepath.ToSlash(record.ImagePath), } - } else if len(candidates) > 1 { - slug := slugs.Slugify(slugs.MediaTypeGame, record.Name) - var selected *database.MediaWithFullPath - for i := range candidates { - title := titleByID(idx, candidates[i].MediaTitleDBID) - if title == nil || title.Slug != slug { - continue - } - if selected != nil { - selected = nil - break - } - candidate := candidates[i] - selected = &candidate + props := write.titleProp + if exact { + props = write.mediaProp } - if selected != nil { - return selected, titleByID(idx, selected.MediaTitleDBID), true + if _, exists := props[prop.TypeTag]; exists { + return false } + props[prop.TypeTag] = prop } + applyGameMetadata(write, source, record.Key) + write.records++ + return true +} +// matchArtwork resolves a pack record to installed media, cheapest step first +// and stopping at the first hit: the catalogued name as a filename, the arcade +// setname inside an MRA, the name's trailing tag when it is itself a pack key, +// and finally the bare title. +func matchArtwork( + idx systemIndex, + record artworkRecord, +) (*database.MediaWithFullPath, *database.TitleWithSystem, bool) { + name := strings.ToLower(strings.TrimSpace(record.Name)) + if media, title := uniqueMedia(idx, idx.mediaByBase[name], record.Name); media != nil { + return media, title, true + } + if media, title := uniqueMedia(idx, idx.mediaBySetName[name], record.Name); media != nil { + return media, title, true + } + key := strings.ToLower(strings.TrimSpace(record.Key)) + if media, title := uniqueMedia(idx, idx.mediaByTrailingTag[key], record.Name); media != nil { + return media, title, true + } + + if !record.SlugUnique { + return nil, nil, false + } slug := slugs.Slugify(slugs.MediaTypeGame, record.Name) titles := idx.titlesBySlug[slug] if len(titles) != 1 { @@ -215,6 +258,58 @@ func matchArtwork( return media, &title, false } +// uniqueMedia picks the single media a candidate list points at. When several +// share a name, only a candidate whose title also matches the record resolves; +// anything still ambiguous is left for a later step. +func uniqueMedia( + idx systemIndex, + candidates []database.MediaWithFullPath, + recordName string, +) (*database.MediaWithFullPath, *database.TitleWithSystem) { + if len(candidates) == 0 { + return nil, nil + } + if len(candidates) == 1 { + media := candidates[0] + if title := titleByID(idx, media.MediaTitleDBID); title != nil { + return &media, title + } + return nil, nil + } + slug := slugs.Slugify(slugs.MediaTypeGame, recordName) + var selected *database.MediaWithFullPath + for i := range candidates { + title := titleByID(idx, candidates[i].MediaTitleDBID) + if title == nil || title.Slug != slug { + continue + } + if selected != nil { + return nil, nil + } + candidate := candidates[i] + selected = &candidate + } + if selected == nil { + return nil, nil + } + return selected, titleByID(idx, selected.MediaTitleDBID) +} + +// trailingParenTag returns the content of the parenthesised tag a name ends +// with. Matching on it is only safe against an existing pack key, so callers +// look the result up rather than trusting it. +func trailingParenTag(base string) string { + trimmed := strings.TrimSpace(base) + if !strings.HasSuffix(trimmed, ")") { + return "" + } + open := strings.LastIndex(trimmed, "(") + if open < 0 { + return "" + } + return strings.ToLower(strings.TrimSpace(trimmed[open+1 : len(trimmed)-1])) +} + func matchManualTitle(idx systemIndex, path string) *database.TitleWithSystem { name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) slug := slugs.Slugify(slugs.MediaTypeGame, name) @@ -244,24 +339,36 @@ func matchManualTitle(idx systemIndex, path string) *database.TitleWithSystem { return selected } +// applyGameMetadata stages title metadata for a key. The first record to +// reach a title wins each field: index rows are processed before the records +// synthesised from images and gameinfo, so the representative dump's details +// are not replaced by a demo or regional variant that resolves to the same +// title later. func applyGameMetadata(write *pendingWrite, source sourceRecords, key string) { info, ok := source.GameInfo[key] if ok { if year := normalizedYear(info.Year); year != "" { - write.titleTags[string(tags.TagTypeYear)] = database.TagInfo{Type: string(tags.TagTypeYear), Tag: year} + setTitleTag(write, database.TagInfo{Type: string(tags.TagTypeYear), Tag: year}) } appendNormalizedTitleTag(write, tags.TagTypeGenre, info.Genre) appendNormalizedTitleTag(write, tags.TagTypeDeveloper, info.Developer) if players := normalizePlayers(info.Players); players != "" { - write.titleTags[string(tags.TagTypePlayers)] = database.TagInfo{ - Type: string(tags.TagTypePlayers), Tag: players, - } + setTitleTag(write, database.TagInfo{Type: string(tags.TagTypePlayers), Tag: players}) } } - if synopsis := cleanText(source.Synopsis[key]); synopsis != "" { - write.titleProp[tags.PropertyTypeTag(tags.TagPropertyDescription)] = database.MediaProperty{ - TypeTag: tags.PropertyTypeTag(tags.TagPropertyDescription), Text: synopsis, - } + synopsis := cleanText(source.Synopsis[key]) + if synopsis == "" { + return + } + description := tags.PropertyTypeTag(tags.TagPropertyDescription) + if _, exists := write.titleProp[description]; !exists { + write.titleProp[description] = database.MediaProperty{TypeTag: description, Text: synopsis} + } +} + +func setTitleTag(write *pendingWrite, tag database.TagInfo) { + if _, exists := write.titleTags[tag.Type]; !exists { + write.titleTags[tag.Type] = tag } } @@ -274,7 +381,7 @@ func appendNormalizedTitleTag(write *pendingWrite, tagType tags.TagType, raw str if normalized == "" { return } - write.titleTags[string(tagType)] = database.TagInfo{Type: string(tagType), Tag: normalized, Label: raw} + setTitleTag(write, database.TagInfo{Type: string(tagType), Tag: normalized, Label: raw}) } func normalizedYear(value string) string { diff --git a/pkg/database/scraper/misterdocs/matching_test.go b/pkg/database/scraper/misterdocs/matching_test.go index bda1a2180..93d9629cd 100644 --- a/pkg/database/scraper/misterdocs/matching_test.go +++ b/pkg/database/scraper/misterdocs/matching_test.go @@ -57,7 +57,8 @@ func TestBuildPendingWrites_MapsExactArtworkMetadataAndManual(t *testing.T) { Manuals: []string{manualPath}, }} - targets, stats, found := buildPendingWrites(testSystemIndex(), records, "run-1") + matched := buildPendingWrites(testSystemIndex(), records, "run-1") + targets, stats, found := matched.Targets, matched.Stats, matched.Found require.Len(t, targets, 1) assert.Equal(t, matchStats{Processed: 2, Matched: 2}, stats) assert.Contains(t, found, filepath.Clean(artPath)) @@ -96,7 +97,8 @@ func TestBuildPendingWrites_SkipsDuplicateArtworkForMedia(t *testing.T) { {Name: "Game (USA)", Key: "Game", ImagePath: secondPath}, }}} - targets, stats, _ := buildPendingWrites(testSystemIndex(), records, "") + matched := buildPendingWrites(testSystemIndex(), records, "") + targets, stats := matched.Targets, matched.Stats require.Len(t, targets, 1) assert.Equal(t, matchStats{Processed: 2, Matched: 1, Skipped: 1}, stats) require.Len(t, targets[0].Write.MediaProps, 1) @@ -106,7 +108,7 @@ func TestBuildPendingWrites_SkipsDuplicateArtworkForMedia(t *testing.T) { func TestMatchArtwork_UsesUniqueTitleFallbackAtTitleScope(t *testing.T) { t.Parallel() - record := artworkRecord{Name: "The Game", Key: "Game", ImagePath: "Game.jpg"} + record := artworkRecord{Name: "The Game", Key: "Game", ImagePath: "Game.jpg", SlugUnique: true} media, title, exact := matchArtwork(testSystemIndex(), record) require.NotNil(t, media) require.NotNil(t, title) @@ -167,7 +169,7 @@ func TestMatchArtwork_TitleFallbackPrefersPresentMedia(t *testing.T) { }, ) - media, title, exact := matchArtwork(idx, artworkRecord{Name: "Game"}) + media, title, exact := matchArtwork(idx, artworkRecord{Name: "Game", SlugUnique: true}) require.NotNil(t, media) require.NotNil(t, title) assert.Equal(t, int64(200), media.DBID) @@ -204,3 +206,126 @@ func TestNormalizedYear(t *testing.T) { assert.Equal(t, "1994", normalizedYear("1994-01-01")) assert.Empty(t, normalizedYear("unknown")) } + +func TestMatchArtwork_ResolvesArcadeBySetName(t *testing.T) { + t.Parallel() + + idx := newSystemIndex( + []database.TitleWithSystem{{DBID: 10, Slug: "streetfighteralpha3", Name: "Street Fighter Alpha 3"}}, + []database.MediaWithFullPath{{ + DBID: 100, MediaTitleDBID: 10, Path: "/games/_Arcade/Street Fighter Alpha 3.mra", + }}, + ) + idx.mediaBySetName["sfa3"] = []database.MediaWithFullPath{{ + DBID: 100, MediaTitleDBID: 10, Path: "/games/_Arcade/Street Fighter Alpha 3.mra", + }} + + media, title, exact := matchArtwork(idx, artworkRecord{Name: "SFA3", Key: "sfa3", ImagePath: "sfa3.jpg"}) + require.NotNil(t, media) + require.NotNil(t, title) + assert.True(t, exact) + assert.Equal(t, int64(100), media.DBID) +} + +func TestMatchArtwork_ResolvesTrailingTagOnlyWhenItIsAKey(t *testing.T) { + t.Parallel() + + idx := newSystemIndex( + []database.TitleWithSystem{ + {DBID: 10, Slug: "shocktroopers", Name: "Shock Troopers"}, + {DBID: 20, Slug: "sonicthehedgehog", Name: "Sonic The Hedgehog"}, + }, + []database.MediaWithFullPath{ + {DBID: 100, MediaTitleDBID: 10, Path: "/games/NEOGEO/Shock Troopers (set 1) (shocktro).zip"}, + {DBID: 200, MediaTitleDBID: 20, Path: "/games/Genesis/Sonic The Hedgehog (USA, Europe).md"}, + }, + ) + + media, title, exact := matchArtwork(idx, artworkRecord{Name: "shocktro", Key: "shocktro", ImagePath: "x.jpg"}) + require.NotNil(t, media) + require.NotNil(t, title) + assert.True(t, exact) + assert.Equal(t, int64(100), media.DBID) + + // A region tag is a trailing tag too, but no pack key is a region, so it + // must never fire; with no unique bare title either, nothing matches. + media, title, _ = matchArtwork(idx, artworkRecord{ + Name: "Sonic The Hedgehog 2 (USA, Europe)", Key: "Sonic The Hedgehog 2 (USA, Europe)", ImagePath: "y.jpg", + }) + assert.Nil(t, media) + assert.Nil(t, title) +} + +func TestMatchArtwork_RefusesTitleFallbackForNonUniqueSlug(t *testing.T) { + t.Parallel() + + record := artworkRecord{Name: "The Game", Key: "Game", ImagePath: "Game.jpg", SlugUnique: false} + media, title, exact := matchArtwork(testSystemIndex(), record) + assert.Nil(t, media) + assert.Nil(t, title) + assert.False(t, exact) +} + +func TestBuildPendingWrites_WritesMetadataWithoutImage(t *testing.T) { + t.Parallel() + + records := []sourceRecords{{ + Artwork: []artworkRecord{{Name: "Game (USA)", Key: "Game (USA)"}}, + GameInfo: map[string]gameInfoRecord{"Game (USA)": {Year: "1994", Genre: "Platform"}}, + Synopsis: map[string]string{"Game (USA)": "Details without a box."}, + }} + + matched := buildPendingWrites(testSystemIndex(), records, "") + targets, stats, found := matched.Targets, matched.Stats, matched.Found + require.Len(t, targets, 1) + assert.Equal(t, matchStats{Processed: 1, Matched: 1}, stats) + assert.Empty(t, found) + write := targets[0].Write + assert.Empty(t, write.MediaProps) + require.Len(t, write.TitleProps, 1) + assert.Equal(t, tags.PropertyTypeTag(tags.TagPropertyDescription), write.TitleProps[0].TypeTag) + tagValues := make(map[string]string, len(write.TitleTags)) + for _, tag := range write.TitleTags { + tagValues[tag.Type] = tag.Tag + } + assert.Equal(t, "1994", tagValues[string(tags.TagTypeYear)]) +} + +func TestTrailingParenTag(t *testing.T) { + t.Parallel() + + assert.Equal(t, "shocktro", trailingParenTag("shock troopers (set 1) (shocktro)")) + assert.Equal(t, "usa, europe", trailingParenTag("Sonic (USA, Europe)")) + assert.Empty(t, trailingParenTag("plain name")) + assert.Empty(t, trailingParenTag("(unbalanced")) + assert.Empty(t, trailingParenTag("unbalanced)")) +} + +func TestBuildPendingWrites_FirstRecordWinsTitleMetadata(t *testing.T) { + t.Parallel() + + records := []sourceRecords{{ + Artwork: []artworkRecord{ + {Name: "Game (USA)", Key: "Game", ImagePath: filepath.Join("docs", "SNES", "Artwork", "Game.jpg")}, + {Name: "Game (USA) (Demo)", Key: "Game (USA) (Demo)", SlugUnique: true}, + }, + GameInfo: map[string]gameInfoRecord{ + "Game": {Year: "1994", Genre: "Platform"}, + "Game (USA) (Demo)": {Year: "1993", Genre: "Demo"}, + }, + Synopsis: map[string]string{"Game": "Full release.", "Game (USA) (Demo)": "Demo disc."}, + }} + + matched := buildPendingWrites(testSystemIndex(), records, "") + targets, stats := matched.Targets, matched.Stats + require.Len(t, targets, 1) + assert.Equal(t, matchStats{Processed: 2, Matched: 2}, stats) + tagValues := make(map[string]string, len(targets[0].Write.TitleTags)) + for _, tag := range targets[0].Write.TitleTags { + tagValues[tag.Type] = tag.Tag + } + assert.Equal(t, "1994", tagValues[string(tags.TagTypeYear)]) + assert.Equal(t, "platform", tagValues[string(tags.TagTypeGenre)]) + require.Len(t, targets[0].Write.TitleProps, 1) + assert.Equal(t, "Full release.", targets[0].Write.TitleProps[0].Text) +} diff --git a/pkg/database/scraper/misterdocs/mra.go b/pkg/database/scraper/misterdocs/mra.go new file mode 100644 index 000000000..39e92e5d5 --- /dev/null +++ b/pkg/database/scraper/misterdocs/mra.go @@ -0,0 +1,82 @@ +// 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 misterdocs + +import ( + "encoding/xml" + "io" + "os" + "strings" + + "github.com/spf13/afero" +) + +// maxMRABytes caps how much of an MRA is parsed. Real MRAs are a few kilobytes; +// the cap bounds a corrupt or hostile file without rejecting a legitimate one. +const maxMRABytes = int64(256 * 1024) + +// mraSetNameElement is the element arcade artwork is keyed on. The pack files +// arcade games under the MAME parent setname, which lives inside the MRA and +// not in its filename, so the setname has to be read out of the XML. +const mraSetNameElement = "setname" + +// mraExt is the MiSTer arcade descriptor extension. +const mraExt = ".mra" + +// readMRASetName extracts from an MRA. It streams the document and +// stops at the first match, so a malformed tail costs nothing. ok is false when +// the file is unreadable, is not XML, or carries no setname. +func readMRASetName(fs afero.Fs, path string) (setName string, ok bool) { + info, err := lstat(fs, path) + if err != nil || !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 { + return "", false + } + if info.Size() > maxMRABytes { + return "", false + } + file, err := fs.Open(path) + if err != nil { + return "", false + } + defer func() { _ = file.Close() }() + + decoder := xml.NewDecoder(io.LimitReader(file, maxMRABytes)) + // MRAs in the wild declare legacy encodings and leave tags unclosed. The + // setname is ASCII either way, so read the bytes as they are rather than + // rejecting the file over its header. + decoder.Strict = false + decoder.CharsetReader = func(_ string, input io.Reader) (io.Reader, error) { return input, nil } + for { + token, tokenErr := decoder.Token() + if tokenErr != nil { + return "", false + } + start, isStart := token.(xml.StartElement) + if !isStart || !strings.EqualFold(start.Name.Local, mraSetNameElement) { + continue + } + var value string + if decodeErr := decoder.DecodeElement(&value, &start); decodeErr != nil { + return "", false + } + value = strings.TrimSpace(value) + return value, value != "" + } +} diff --git a/pkg/database/scraper/misterdocs/mra_test.go b/pkg/database/scraper/misterdocs/mra_test.go new file mode 100644 index 000000000..4b81c0654 --- /dev/null +++ b/pkg/database/scraper/misterdocs/mra_test.go @@ -0,0 +1,127 @@ +// 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 misterdocs + +import ( + "bytes" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReadMRASetName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + wantName string + wantOK bool + }{ + { + name: "setname element", + content: ` + Street Fighter Alpha 3 + sfa3 + cps2 +`, + wantName: "sfa3", + wantOK: true, + }, + { + name: "surrounding whitespace is trimmed", + content: "\n shocktro \n", + wantName: "shocktro", + wantOK: true, + }, + { + name: "case is preserved for the caller to fold", + content: "CPS1GAME", + wantName: "CPS1GAME", + wantOK: true, + }, + { + name: "legacy encoding declaration is not fatal", + content: ` +pacman`, + wantName: "pacman", + wantOK: true, + }, + { + name: "unclosed tags before the setname are tolerated", + content: "cps1ffight", + wantName: "ffight", + wantOK: true, + }, + { + name: "no setname", + content: "Game", + }, + { + name: "empty setname", + content: "", + }, + { + name: "not xml", + content: "this is not an mra", + }, + { + name: "empty file", + content: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() + path := filepath.Join("games", "_Arcade", "Game.mra") + require.NoError(t, fs.MkdirAll(filepath.Dir(path), 0o750)) + require.NoError(t, afero.WriteFile(fs, path, []byte(tt.content), 0o600)) + + gotName, gotOK := readMRASetName(fs, path) + assert.Equal(t, tt.wantOK, gotOK) + assert.Equal(t, tt.wantName, gotName) + }) + } +} + +func TestReadMRASetName_RejectsMissingDirectoryAndOversized(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + dir := filepath.Join("games", "_Arcade") + require.NoError(t, fs.MkdirAll(filepath.Join(dir, "Folder.mra"), 0o750)) + + _, ok := readMRASetName(fs, filepath.Join(dir, "Missing.mra")) + assert.False(t, ok) + _, ok = readMRASetName(fs, filepath.Join(dir, "Folder.mra")) + assert.False(t, ok) + + oversized := append( + []byte("big"), + bytes.Repeat([]byte(" "), int(maxMRABytes))..., + ) + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, "Big.mra"), oversized, 0o600)) + _, ok = readMRASetName(fs, filepath.Join(dir, "Big.mra")) + assert.False(t, ok) +} diff --git a/pkg/database/scraper/misterdocs/parse.go b/pkg/database/scraper/misterdocs/parse.go index faac6495c..54324f4fe 100644 --- a/pkg/database/scraper/misterdocs/parse.go +++ b/pkg/database/scraper/misterdocs/parse.go @@ -32,6 +32,7 @@ import ( "strings" "unicode/utf8" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/slugs" "github.com/rs/zerolog/log" "github.com/spf13/afero" ) @@ -40,12 +41,26 @@ const ( maxMetadataBytes = int64(8 * 1024 * 1024) maxMetadataRecords = 100_000 directoryReadBatch = 256 + + gameInfoFileName = "gameinfo.tsv" + // Synopsis files are named per language, and which languages a pack ships + // varies by system, so they are globbed rather than probed by name. + synopsisFilePrefix = "synopsis_" + synopsisFileSuffix = ".tsv" + defaultSynopsisLang = "en" ) +// artworkRecord is one resolvable entry from a pack. ImagePath is empty for a +// game the pack holds metadata but no image for; those still carry year, genre +// and synopsis so a title can show details without artwork. type artworkRecord struct { Name string Key string ImagePath string + // SlugUnique is false when another key in the same pack reduces to the + // same bare title. Falling back to a title match then serves a coin-flip + // image, which the format treats as worse than serving none. + SlugUnique bool } type gameInfoRecord struct { @@ -64,10 +79,10 @@ type sourceRecords struct { RowErrors int } -func loadSourceRecords(ctx context.Context, fs afero.Fs, source sourceDir) (sourceRecords, error) { +func loadSourceRecords(ctx context.Context, fs afero.Fs, source sourceDir, langs []string) (sourceRecords, error) { switch source.Kind { case sourceArtwork: - return loadArtworkRecords(ctx, fs, source.Path) + return loadArtworkRecords(ctx, fs, source.Path, langs) case sourceManuals: manuals, err := loadManualRecords(ctx, fs, source.Path) return sourceRecords{Manuals: manuals}, err @@ -76,31 +91,57 @@ func loadSourceRecords(ctx context.Context, fs afero.Fs, source sourceDir) (sour } } -func loadArtworkRecords(ctx context.Context, fs afero.Fs, dir string) (sourceRecords, error) { +func loadArtworkRecords(ctx context.Context, fs afero.Fs, dir string, langs []string) (sourceRecords, error) { images, err := imageFilesByStem(ctx, fs, dir) if err != nil { return sourceRecords{}, err } - rows, err := readTSV(ctx, fs, filepath.Join(dir, indexFileName)) - if err != nil { - return sourceRecords{}, fmt.Errorf("misterdocs: parse artwork index: %w", err) - } result := sourceRecords{ - Artwork: make([]artworkRecord, 0, len(rows.records)), + Artwork: make([]artworkRecord, 0, len(images)), GameInfo: make(map[string]gameInfoRecord), Synopsis: make(map[string]string), } + if err := loadArtworkIndex(ctx, fs, dir, images, &result); err != nil { + return sourceRecords{}, err + } + // Only index rows may fall back to a bare-title match. Records added + // after this point resolve by exact name alone, which is all the format + // promises for an image the index does not mention. + markUniqueSlugs(result.Artwork) + loadArtworkMetadata(ctx, fs, dir, langs, &result) + appendUnindexedRecords(images, &result) + return result, nil +} + +// loadArtworkIndex resolves every catalogued dump to the image that represents +// it. A pack with no index still serves images filed under their own key, so a +// missing index is not an error; appendUnindexedRecords covers those images. +func loadArtworkIndex( + ctx context.Context, + fs afero.Fs, + dir string, + images map[string]string, + result *sourceRecords, +) error { + indexPath := filepath.Join(dir, indexFileName) + if !isRegularFile(fs, indexPath) { + return nil + } + rows, err := readTSV(ctx, fs, indexPath) + if err != nil { + return fmt.Errorf("misterdocs: parse artwork index: %w", err) + } nameCol, nameOK := rows.columns["name"] keyCol, keyOK := rows.columns["key"] if !nameOK || !keyOK { - return sourceRecords{}, errors.New("misterdocs: index.tsv requires name and key columns") + return errors.New("misterdocs: index.tsv requires name and key columns") } recordsByName := make(map[string]artworkRecord) recordOrder := make([]string, 0, len(rows.records)) ambiguousNames := make(map[string]struct{}) for _, row := range rows.records { if err := ctx.Err(); err != nil { - return sourceRecords{}, err + return err } name, key, ok := rowValues(row, nameCol, keyCol) if !ok || name == "" || key == "" { @@ -120,6 +161,8 @@ func loadArtworkRecords(ctx context.Context, fs afero.Fs, dir string) (sourceRec if existing, duplicate := recordsByName[nameKey]; duplicate { result.RowErrors++ if !strings.EqualFold(existing.Key, key) { + // Two keys claim one name, so the row accepted earlier is + // unusable too and is retracted along with this one. delete(recordsByName, nameKey) ambiguousNames[nameKey] = struct{}{} result.RowErrors++ @@ -134,9 +177,14 @@ func loadArtworkRecords(ctx context.Context, fs afero.Fs, dir string) (sourceRec result.Artwork = append(result.Artwork, record) } } + return nil +} - if isRegularFile(fs, filepath.Join(dir, "gameinfo.tsv")) { - parsed, parseErr := loadGameInfo(ctx, fs, filepath.Join(dir, "gameinfo.tsv")) +func loadArtworkMetadata(ctx context.Context, fs afero.Fs, dir string, langs []string, result *sourceRecords) { + gameInfoPath := filepath.Join(dir, gameInfoFileName) + if isRegularFile(fs, gameInfoPath) { + parsed, rowErrors, parseErr := loadGameInfo(ctx, fs, gameInfoPath) + result.RowErrors += rowErrors if parseErr != nil { result.RowErrors++ log.Warn().Err(parseErr).Str("dir", dir).Msg("misterdocs: skipped optional game metadata") @@ -144,38 +192,166 @@ func loadArtworkRecords(ctx context.Context, fs afero.Fs, dir string) (sourceRec result.GameInfo = parsed } } - if isRegularFile(fs, filepath.Join(dir, "synopsis_en.tsv")) { - parsed, parseErr := loadSynopsis(ctx, fs, filepath.Join(dir, "synopsis_en.tsv")) - if parseErr != nil { - result.RowErrors++ - log.Warn().Err(parseErr).Str("dir", dir).Msg("misterdocs: skipped optional synopsis") - } else { - result.Synopsis = parsed + synopsisPath := synopsisFileForLangs(ctx, fs, dir, langs) + if synopsisPath == "" { + return + } + parsed, rowErrors, parseErr := loadSynopsis(ctx, fs, synopsisPath) + result.RowErrors += rowErrors + if parseErr != nil { + result.RowErrors++ + log.Warn().Err(parseErr).Str("path", synopsisPath).Msg("misterdocs: skipped optional synopsis") + return + } + result.Synopsis = parsed +} + +// appendUnindexedRecords adds what the index does not cover. Every image no +// row names is filed under its own key, because a key used as a filename +// resolves whether or not the index mentions it - and it is the only step left +// when a pack ships with no index at all. Every gameinfo key with neither an +// image nor a row becomes a metadata-only record, so a game the pack holds +// details but no artwork for still gets them; the key itself, a catalogue name +// or a setname, is the only handle such a game has. Neither kind is an index +// row, so neither is eligible for the bare-title fallback. +func appendUnindexedRecords(images map[string]string, result *sourceRecords) { + names := make(map[string]struct{}, len(result.Artwork)) + keys := make(map[string]struct{}, len(result.Artwork)) + for i := range result.Artwork { + names[strings.ToLower(result.Artwork[i].Name)] = struct{}{} + keys[strings.ToLower(result.Artwork[i].Key)] = struct{}{} + } + stems := make([]string, 0, len(images)) + for stem := range images { + if _, ok := names[stem]; !ok { + stems = append(stems, stem) } } - return result, nil + sort.Strings(stems) + for _, stem := range stems { + path := images[stem] + name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)) + result.Artwork = append(result.Artwork, artworkRecord{Name: name, Key: name, ImagePath: path}) + keys[stem] = struct{}{} + } + infoKeys := make([]string, 0, len(result.GameInfo)) + for key := range result.GameInfo { + lowered := strings.ToLower(key) + if _, ok := keys[lowered]; ok { + continue + } + // A gameinfo key that is already an index name resolves through that + // row to its representative image and metadata; a second record for + // it would only re-match the same media. + if _, ok := names[lowered]; ok { + continue + } + infoKeys = append(infoKeys, key) + } + sort.Strings(infoKeys) + for _, key := range infoKeys { + result.Artwork = append(result.Artwork, artworkRecord{Name: key, Key: key}) + } } -func loadGameInfo(ctx context.Context, fs afero.Fs, path string) (map[string]gameInfoRecord, error) { - rows, err := readTSV(ctx, fs, path) +// markUniqueSlugs flags the records whose bare title identifies exactly one +// key, which are the only ones allowed to match a title rather than a dump. +func markUniqueSlugs(records []artworkRecord) { + recordSlugs := make([]string, len(records)) + keysBySlug := make(map[string]map[string]struct{}, len(records)) + for i := range records { + slug := slugs.Slugify(slugs.MediaTypeGame, records[i].Name) + recordSlugs[i] = slug + if slug == "" { + continue + } + if keysBySlug[slug] == nil { + keysBySlug[slug] = make(map[string]struct{}, 1) + } + keysBySlug[slug][strings.ToLower(records[i].Key)] = struct{}{} + } + for i := range records { + records[i].SlugUnique = recordSlugs[i] != "" && len(keysBySlug[recordSlugs[i]]) == 1 + } +} + +// synopsisFileForLangs picks a synopsis file by preferred language. A pack +// ships a language file only where the source held text in it, so the set +// varies per system and has to be globbed rather than assumed. +func synopsisFileForLangs(ctx context.Context, fs afero.Fs, dir string, langs []string) string { + entries, err := afero.ReadDir(fs, dir) if err != nil { - return nil, fmt.Errorf("misterdocs: parse gameinfo: %w", err) + return "" + } + byLang := make(map[string]string, len(entries)) + available := make([]string, 0, len(entries)) + for _, entry := range entries { + if ctx.Err() != nil { + return "" + } + name := strings.ToLower(entry.Name()) + if entry.IsDir() || !strings.HasPrefix(name, synopsisFilePrefix) || + !strings.HasSuffix(name, synopsisFileSuffix) { + continue + } + path := filepath.Join(dir, entry.Name()) + if !isRegularFile(fs, path) { + continue + } + lang := strings.TrimSuffix(strings.TrimPrefix(name, synopsisFilePrefix), synopsisFileSuffix) + if lang == "" { + continue + } + byLang[lang] = path + available = append(available, lang) + } + if len(available) == 0 { + return "" + } + for _, lang := range append(append([]string{}, langs...), defaultSynopsisLang) { + lang = strings.ToLower(strings.TrimSpace(lang)) + if path := byLang[lang]; path != "" { + return path + } + if sep := strings.IndexAny(lang, "-_"); sep > 0 { + if path := byLang[lang[:sep]]; path != "" { + return path + } + } + } + sort.Strings(available) + return byLang[available[0]] +} + +// loadGameInfo returns the metadata rows keyed by pack key. rowErrors counts +// rows dropped for repeating a key; the first row for a key is kept. +func loadGameInfo( + ctx context.Context, + fs afero.Fs, + path string, +) (records map[string]gameInfoRecord, rowErrors int, err error) { + rows, readErr := readTSV(ctx, fs, path) + if readErr != nil { + return nil, 0, fmt.Errorf("misterdocs: parse gameinfo: %w", readErr) } keyCol, ok := rows.columns["key"] if !ok { - return nil, errors.New("misterdocs: gameinfo.tsv requires key column") + return nil, 0, errors.New("misterdocs: gameinfo.tsv requires key column") } result := make(map[string]gameInfoRecord, len(rows.records)) for _, row := range rows.records { - if err := ctx.Err(); err != nil { - return nil, err + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, rowErrors, ctxErr } key := field(row, keyCol) if key == "" { continue } if _, duplicate := result[key]; duplicate { - return nil, fmt.Errorf("misterdocs: gameinfo.tsv contains duplicate key %q", key) + // Keep the first row. Dropping the whole file over one repeated + // key would cost every other game its metadata. + rowErrors++ + continue } result[key] = gameInfoRecord{ Name: fieldByName(row, rows.columns, "name"), @@ -185,34 +361,41 @@ func loadGameInfo(ctx context.Context, fs afero.Fs, path string) (map[string]gam Players: fieldByName(row, rows.columns, "players"), } } - return result, nil + return result, rowErrors, nil } -func loadSynopsis(ctx context.Context, fs afero.Fs, path string) (map[string]string, error) { - rows, err := readTSV(ctx, fs, path) - if err != nil { - return nil, fmt.Errorf("misterdocs: parse synopsis: %w", err) +// loadSynopsis returns descriptions keyed by pack key. rowErrors counts rows +// dropped for repeating a key; the first row for a key is kept. +func loadSynopsis( + ctx context.Context, + fs afero.Fs, + path string, +) (records map[string]string, rowErrors int, err error) { + rows, readErr := readTSV(ctx, fs, path) + if readErr != nil { + return nil, 0, fmt.Errorf("misterdocs: parse synopsis: %w", readErr) } keyCol, keyOK := rows.columns["key"] synopsisCol, synopsisOK := rows.columns["synopsis"] if !keyOK || !synopsisOK { - return nil, errors.New("misterdocs: synopsis_en.tsv requires key and synopsis columns") + return nil, 0, errors.New("misterdocs: synopsis file requires key and synopsis columns") } result := make(map[string]string, len(rows.records)) for _, row := range rows.records { - if err := ctx.Err(); err != nil { - return nil, err + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, rowErrors, ctxErr } key, synopsis, ok := rowValues(row, keyCol, synopsisCol) if !ok || key == "" || synopsis == "" { continue } if _, duplicate := result[key]; duplicate { - return nil, fmt.Errorf("misterdocs: synopsis_en.tsv contains duplicate key %q", key) + rowErrors++ + continue } result[key] = synopsis } - return result, nil + return result, rowErrors, nil } func loadManualRecords(ctx context.Context, fs afero.Fs, dir string) ([]string, error) { diff --git a/pkg/database/scraper/misterdocs/parse_fuzz_test.go b/pkg/database/scraper/misterdocs/parse_fuzz_test.go index ef7232a6f..16b598c2d 100644 --- a/pkg/database/scraper/misterdocs/parse_fuzz_test.go +++ b/pkg/database/scraper/misterdocs/parse_fuzz_test.go @@ -90,3 +90,31 @@ func FuzzPathWithin(f *testing.F) { } }) } + +func FuzzReadMRASetName(f *testing.F) { + f.Add([]byte("sfa3")) + f.Add([]byte(`x`)) + f.Add([]byte("")) + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + if int64(len(data)) > maxMRABytes { + t.Skip() + } + fs := afero.NewMemMapFs() + path := filepath.Join("games", "_Arcade", "Game.mra") + if err := fs.MkdirAll(filepath.Dir(path), 0o750); err != nil { + t.Fatal(err) + } + if err := afero.WriteFile(fs, path, data, 0o600); err != nil { + t.Fatal(err) + } + setName, ok := readMRASetName(fs, path) + if ok && strings.TrimSpace(setName) == "" { + t.Fatalf("readMRASetName reported ok with a blank setname for %q", data) + } + if !ok && setName != "" { + t.Fatalf("readMRASetName returned %q without ok", setName) + } + }) +} diff --git a/pkg/database/scraper/misterdocs/parse_test.go b/pkg/database/scraper/misterdocs/parse_test.go index b0548dc24..04af32e4c 100644 --- a/pkg/database/scraper/misterdocs/parse_test.go +++ b/pkg/database/scraper/misterdocs/parse_test.go @@ -82,11 +82,17 @@ func TestLoadArtworkRecords_ImportsIndexAndOptionalMetadata(t *testing.T) { require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) } - got, err := loadArtworkRecords(context.Background(), fs, dir) + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) require.NoError(t, err) - require.Len(t, got.Artwork, 1) + require.Len(t, got.Artwork, 2) assert.Equal(t, "Game (USA)", got.Artwork[0].Name) assert.Equal(t, filepath.Join(dir, "Canonical Game.jpg"), got.Artwork[0].ImagePath) + assert.True(t, got.Artwork[0].SlugUnique) + // The image the index points at is also reachable under its own key, but + // only by exact name: it is not an index row, so no bare-title fallback. + assert.Equal(t, artworkRecord{ + Name: "Canonical Game", Key: "Canonical Game", ImagePath: filepath.Join(dir, "Canonical Game.jpg"), + }, got.Artwork[1]) assert.Equal(t, "1994", got.GameInfo["Canonical Game"].Year) assert.Equal(t, "A & B", got.Synopsis["Canonical Game"]) assert.Equal(t, 1, got.RowErrors) @@ -107,9 +113,13 @@ func TestLoadArtworkRecords_OmitsAmbiguousDuplicateNames(t *testing.T) { require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), content, 0o600)) } - got, err := loadArtworkRecords(context.Background(), fs, dir) + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) require.NoError(t, err) - assert.Empty(t, got.Artwork) + // Neither key may claim the shared name, but each image still resolves + // under its own key. + require.Len(t, got.Artwork, 2) + assert.Equal(t, "First", got.Artwork[0].Name) + assert.Equal(t, "Second", got.Artwork[1].Name) assert.Equal(t, 2, got.RowErrors) } @@ -129,7 +139,7 @@ func TestLoadArtworkRecords_SkipsMalformedOptionalMetadata(t *testing.T) { require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), content, 0o600)) } - got, err := loadArtworkRecords(context.Background(), fs, dir) + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) require.NoError(t, err) require.Len(t, got.Artwork, 1) assert.Empty(t, got.GameInfo) @@ -137,7 +147,7 @@ func TestLoadArtworkRecords_SkipsMalformedOptionalMetadata(t *testing.T) { assert.Equal(t, 2, got.RowErrors) } -func TestLoadArtworkRecords_SkipsDuplicateOptionalMetadataKeys(t *testing.T) { +func TestLoadArtworkRecords_KeepsFirstOfDuplicateOptionalMetadataKeys(t *testing.T) { t.Parallel() fs := afero.NewMemMapFs() @@ -155,11 +165,11 @@ func TestLoadArtworkRecords_SkipsDuplicateOptionalMetadataKeys(t *testing.T) { require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), content, 0o600)) } - got, err := loadArtworkRecords(context.Background(), fs, dir) + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) require.NoError(t, err) require.Len(t, got.Artwork, 1) - assert.Empty(t, got.GameInfo) - assert.Empty(t, got.Synopsis) + assert.Equal(t, "1994", got.GameInfo["Game"].Year) + assert.Equal(t, "First", got.Synopsis["Game"]) assert.Equal(t, 2, got.RowErrors) } @@ -171,7 +181,7 @@ func TestLoadArtworkRecords_RequiresColumns(t *testing.T) { require.NoError(t, fs.MkdirAll(dir, 0o750)) require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, "index.tsv"), []byte("#name\nGame\n"), 0o600)) - _, err := loadArtworkRecords(context.Background(), fs, dir) + _, err := loadArtworkRecords(context.Background(), fs, dir, nil) require.Error(t, err) assert.Contains(t, err.Error(), "requires name and key") } @@ -185,11 +195,11 @@ func TestLoadSourceRecords_HandlesManualsAndRejectsUnknownKinds(t *testing.T) { manualPath := filepath.Join(dir, "Game.pdf") require.NoError(t, afero.WriteFile(fs, manualPath, []byte("pdf"), 0o600)) - got, err := loadSourceRecords(context.Background(), fs, sourceDir{Path: dir, Kind: sourceManuals}) + got, err := loadSourceRecords(context.Background(), fs, sourceDir{Path: dir, Kind: sourceManuals}, nil) require.NoError(t, err) assert.Equal(t, []string{manualPath}, got.Manuals) - _, err = loadSourceRecords(context.Background(), fs, sourceDir{Path: dir, Kind: sourceKind(255)}) + _, err = loadSourceRecords(context.Background(), fs, sourceDir{Path: dir, Kind: sourceKind(255)}, nil) require.ErrorContains(t, err, "unknown source kind") } @@ -297,3 +307,190 @@ func TestNormalizePlayers(t *testing.T) { assert.Equal(t, "8", normalizePlayers("1, 2 / 8")) assert.Empty(t, normalizePlayers("unknown")) } + +func TestLoadArtworkRecords_PicksSynopsisByPreferredLanguage(t *testing.T) { + t.Parallel() + + writeSynopsisPack := func(t *testing.T) (afero.Fs, string) { + t.Helper() + fs := afero.NewMemMapFs() + dir := filepath.Join("docs", "Genesis", "Artwork") + require.NoError(t, fs.MkdirAll(dir, 0o750)) + files := map[string]string{ + "index.tsv": "#name\tkey\nGame\tGame\n", + "Game.jpg": "image", + "synopsis_de.tsv": "#key\tsynopsis\nGame\tDeutsch\n", + "synopsis_fr.tsv": "#key\tsynopsis\nGame\tFrançais\n", + } + for name, content := range files { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) + } + return fs, dir + } + + tests := []struct { + name string + want string + langs []string + }{ + {name: "first preferred language present", langs: []string{"fr", "en"}, want: "Français"}, + {name: "later preferred language present", langs: []string{"es", "de"}, want: "Deutsch"}, + {name: "regional tag falls back to its base", langs: []string{"fr-CA"}, want: "Français"}, + {name: "underscore regional tag falls back to its base", langs: []string{"de_DE"}, want: "Deutsch"}, + {name: "case is ignored", langs: []string{"FR"}, want: "Français"}, + {name: "no preference and no english picks deterministically", langs: nil, want: "Deutsch"}, + {name: "unavailable preference picks deterministically", langs: []string{"es"}, want: "Deutsch"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + fs, dir := writeSynopsisPack(t) + got, err := loadArtworkRecords(context.Background(), fs, dir, tt.langs) + require.NoError(t, err) + assert.Equal(t, tt.want, got.Synopsis["Game"]) + }) + } +} + +func TestLoadArtworkRecords_PrefersEnglishOverArbitraryLanguage(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + dir := filepath.Join("docs", "Genesis", "Artwork") + require.NoError(t, fs.MkdirAll(dir, 0o750)) + files := map[string]string{ + "index.tsv": "#name\tkey\nGame\tGame\n", + "Game.jpg": "image", + "synopsis_de.tsv": "#key\tsynopsis\nGame\tDeutsch\n", + "synopsis_en.tsv": "#key\tsynopsis\nGame\tEnglish\n", + "synopsis_it.tsv": "#key\tsynopsis\nGame\tItaliano\n", + } + for name, content := range files { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) + } + + got, err := loadArtworkRecords(context.Background(), fs, dir, []string{"es"}) + require.NoError(t, err) + assert.Equal(t, "English", got.Synopsis["Game"]) +} + +func TestLoadArtworkRecords_AddsMetadataOnlyRecordsForImagelessGames(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + dir := filepath.Join("docs", "SNES", "Artwork") + require.NoError(t, fs.MkdirAll(dir, 0o750)) + files := map[string]string{ + "index.tsv": "#name\tkey\nShown (USA)\tShown (USA)\n", + "gameinfo.tsv": "#key\tname\tyear\n" + + "Shown (USA)\tShown\t1994\n" + + "Unseen (USA)\tUnseen\t1995\n", + "Shown (USA).jpg": "image", + } + for name, content := range files { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) + } + + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) + require.NoError(t, err) + require.Len(t, got.Artwork, 2) + assert.Equal(t, "Shown (USA)", got.Artwork[0].Key) + assert.NotEmpty(t, got.Artwork[0].ImagePath) + assert.Equal(t, artworkRecord{Name: "Unseen (USA)", Key: "Unseen (USA)"}, got.Artwork[1]) + assert.Equal(t, "1995", got.GameInfo["Unseen (USA)"].Year) + assert.Zero(t, got.RowErrors) +} + +func TestLoadArtworkRecords_DegradesToExactKeysWithoutIndex(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + dir := filepath.Join("docs", "SNES", "Artwork") + require.NoError(t, fs.MkdirAll(dir, 0o750)) + files := map[string]string{ + "Zelda (USA).jpg": "image", + "Mario (USA).jpg": "image", + "gameinfo.tsv": "#key\tyear\nMario (USA)\t1991\n", + } + for name, content := range files { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) + } + + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) + require.NoError(t, err) + assert.Equal(t, []artworkRecord{ + {Name: "Mario (USA)", Key: "Mario (USA)", ImagePath: filepath.Join(dir, "Mario (USA).jpg")}, + {Name: "Zelda (USA)", Key: "Zelda (USA)", ImagePath: filepath.Join(dir, "Zelda (USA).jpg")}, + }, got.Artwork) + assert.Equal(t, "1991", got.GameInfo["Mario (USA)"].Year) + assert.Zero(t, got.RowErrors) +} + +func TestLoadArtworkRecords_MarksSlugUniquenessPerKey(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + dir := filepath.Join("docs", "GAMEBOY", "Artwork") + require.NoError(t, fs.MkdirAll(dir, 0o750)) + files := map[string]string{ + // Two dumps of one game share a bare title and a key: still unique. + // Two games whose bare titles collide across keys are not. + "index.tsv": "#name\tkey\n" + + "Blaster Master Boy (USA)\tBlaster Master Boy (USA)\n" + + "Blaster Master Boy (USA) (Beta)\tBlaster Master Boy (USA)\n" + + "Tetris (World)\tTetris (World)\n" + + "Tetris (Japan)\tTetris (Japan)\n", + "Blaster Master Boy (USA).jpg": "image", + "Tetris (World).jpg": "image", + "Tetris (Japan).jpg": "image", + } + for name, content := range files { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) + } + + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) + require.NoError(t, err) + unique := make(map[string]bool, len(got.Artwork)) + for _, record := range got.Artwork { + unique[record.Name] = record.SlugUnique + } + assert.True(t, unique["Blaster Master Boy (USA)"]) + assert.True(t, unique["Blaster Master Boy (USA) (Beta)"]) + assert.False(t, unique["Tetris (World)"]) + assert.False(t, unique["Tetris (Japan)"]) +} + +func TestLoadArtworkRecords_DoesNotDuplicateDumpsAlreadyNamedInIndex(t *testing.T) { + t.Parallel() + + // Real PSX pack shape: a demo dump is an index name resolving to the + // representative key, and also a gameinfo key with no image of its own. + fs := afero.NewMemMapFs() + dir := filepath.Join("docs", "PSX", "Artwork") + require.NoError(t, fs.MkdirAll(dir, 0o750)) + files := map[string]string{ + "index.tsv": "#name\tcrc\tsize\tkey\n" + + "'98 Koushien (Japan) (Demo)\t6ab3e4ce\t93\t'98 Koushien - Koukou Yakyuu Simulation (Japan)\n" + + "'98 Koushien - Koukou Yakyuu Simulation (Japan)\tda95e43a\t113\t" + + "'98 Koushien - Koukou Yakyuu Simulation (Japan)\n", + "gameinfo.tsv": "#key\tname\tyear\n" + + "'98 Koushien (Japan) (Demo)\t'98 Koushien\t1998\n" + + "'98 Koushien - Koukou Yakyuu Simulation (Japan)\t'98 Koushien\t1998\n", + "'98 Koushien - Koukou Yakyuu Simulation (Japan).jpg": "image", + } + for name, content := range files { + require.NoError(t, afero.WriteFile(fs, filepath.Join(dir, name), []byte(content), 0o600)) + } + + got, err := loadArtworkRecords(context.Background(), fs, dir, nil) + require.NoError(t, err) + names := make([]string, 0, len(got.Artwork)) + for _, record := range got.Artwork { + names = append(names, record.Name) + } + assert.Equal(t, []string{ + "'98 Koushien (Japan) (Demo)", + "'98 Koushien - Koukou Yakyuu Simulation (Japan)", + }, names) + assert.Zero(t, got.RowErrors) +} diff --git a/pkg/database/scraper/misterdocs/scraper.go b/pkg/database/scraper/misterdocs/scraper.go index b1eb83ee4..f03eddd90 100644 --- a/pkg/database/scraper/misterdocs/scraper.go +++ b/pkg/database/scraper/misterdocs/scraper.go @@ -27,10 +27,12 @@ import ( "fmt" "path/filepath" "strings" + "time" "github.com/ZaparooProject/zaparoo-core/v2/pkg/config" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/scraper" + "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/systemdefs" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/tags" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers/bgpriority" "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" @@ -74,9 +76,13 @@ func NewPlatformScraper() platforms.Scraper { return fmt.Errorf("misterdocs: list indexed systems: %w", err) } targets := orderedTargetSystems(indexed, opts.Systems) + var langs []string + if cfg != nil { + langs = cfg.DefaultLangs() + } impl := &scraperImpl{ fs: fs, db: db.MediaDB, docsRoots: candidateDocsRoots(rootDirs), - sources: sourcesBySystem(sources), + sources: sourcesBySystem(sources), langs: langs, } go impl.scrapeLoop(ctx, opts, targets, ch) return nil @@ -89,6 +95,7 @@ type scraperImpl struct { fs afero.Fs db database.MediaDBI docsRoots []string + langs []string } func (s *scraperImpl) scrapeLoop( @@ -124,8 +131,20 @@ func (s *scraperImpl) scrapeLoop( return } + stepStart := time.Now() + report := func(processed, total, matched, skipped int) { + select { + case ch <- scraper.ScrapeUpdate{ + SystemID: targetID, Processed: processed, Total: total, Matched: matched, Skipped: skipped, + TotalSteps: len(steps), CurrentStep: step + 1, + }: + case <-ctx.Done(): + } + } + var records []sourceRecords var sourceError error + arcadeSource := false successfulRoots := make(map[string]struct{}) for _, sourceID := range sourceIDsForTarget(targetID) { for _, source := range s.sources[sourceID] { @@ -135,7 +154,7 @@ func (s *scraperImpl) scrapeLoop( } return } - loaded, loadErr := loadSourceRecords(ctx, s.fs, source) + loaded, loadErr := loadSourceRecords(ctx, s.fs, source, s.langs) if loadErr != nil { sourceError = errors.Join( sourceError, @@ -144,33 +163,90 @@ func (s *scraperImpl) scrapeLoop( continue } records = append(records, loaded) + if source.Kind == sourceArtwork && sourceID == systemdefs.SystemArcade { + arcadeSource = true + } if root := s.docsRootForSource(source.Path); root != "" { successfulRoots[root] = struct{}{} } } } + loadDuration := time.Since(stepStart) + totalRecords := 0 + for i := range records { + totalRecords += len(records[i].Artwork) + len(records[i].Manuals) + records[i].RowErrors + } + // A large pack takes minutes to match and write, so tell the API how + // big the step is before any of that starts. + if totalRecords > 0 { + report(0, totalRecords, 0, 0) + } + cleanupRoots := make([]string, 0, len(successfulRoots)) for _, root := range s.docsRoots { if _, ok := successfulRoots[root]; ok { cleanupRoots = append(cleanupRoots, root) } } + scanStart := time.Now() idx := newSystemIndex(titles, media) - writeTargets, stats, foundPaths := buildPendingWrites(idx, records, opts.RunID) + if arcadeSource { + if err := s.indexArcadeSetNames(ctx, opts, &idx, media); err != nil { + ch <- scraper.ScrapeUpdate{ + Done: true, Processed: totalProcessed, Matched: totalMatched, Skipped: totalSkipped, + } + return + } + } + scanDuration := time.Since(scanStart) + matchStart := time.Now() + matched := buildPendingWrites(idx, records, opts.RunID) + writeTargets, stats := matched.Targets, matched.Stats + matchDuration := time.Since(matchStart) + cleanupStart := time.Now() if opts.Force && sourceError == nil && len(cleanupRoots) > 0 { if _, cleanupErr := s.deleteStaleProperties( - ctx, opts, media, titles, foundPaths, cleanupRoots, + ctx, opts, media, titles, matched.Found, cleanupRoots, ); cleanupErr != nil { sourceError = cleanupErr } } + cleanupDuration := time.Since(cleanupStart) + // Skipped records are finished once matching is; matched ones finish + // as their rows commit, so progress advances with each write batch. + if len(writeTargets) > 0 && stats.Skipped > 0 { + report(stats.Skipped, stats.Processed, 0, stats.Skipped) + } + writeStart := time.Now() + matchedWritten := 0 stepError := sourceError - if err := s.applyTargets(ctx, opts, writeTargets); err != nil { + if err := s.applyTargets(ctx, opts, writeTargets, func(from, to int) { + for i := from; i < to; i++ { + matchedWritten += matched.RecordsPerTarget[i] + } + if to < len(writeTargets) { + report(stats.Skipped+matchedWritten, stats.Processed, matchedWritten, stats.Skipped) + } + }); err != nil { stepError = errors.Join(stepError, err) stats.Skipped++ } + writeDuration := time.Since(writeStart) + log.Debug(). + Str("system", targetID). + Int("records", stats.Processed). + Int("matched", stats.Matched). + Int("skipped", stats.Skipped). + Int("targets", len(writeTargets)). + Dur("load", loadDuration). + Dur("scan", scanDuration). + Dur("match", matchDuration). + Dur("cleanup", cleanupDuration). + Dur("write", writeDuration). + Dur("total", time.Since(stepStart)). + Msg("misterdocs: step complete") totalProcessed += stats.Processed totalMatched += stats.Matched totalSkipped += stats.Skipped @@ -185,6 +261,44 @@ func (s *scraperImpl) scrapeLoop( } } +// indexArcadeSetNames resolves installed MRAs to the setname they declare. +// Arcade artwork is filed under the MAME parent setname, which lives inside the +// MRA and never in its filename, so without this pass an arcade pack matches +// nothing. It runs only for systems that actually have an arcade source. +func (s *scraperImpl) indexArcadeSetNames( + ctx context.Context, + opts scraper.ScrapeOptions, + idx *systemIndex, + media []database.MediaWithFullPath, +) error { + started := time.Now() + scanned, resolved := 0, 0 + for i := range media { + if !strings.EqualFold(filepath.Ext(media[i].Path), mraExt) { + continue + } + if err := waitForScrape(ctx, opts); err != nil { + return err + } + scanned++ + setName, ok := readMRASetName(s.fs, media[i].Path) + if !ok { + continue + } + resolved++ + key := strings.ToLower(setName) + idx.mediaBySetName[key] = append(idx.mediaBySetName[key], media[i]) + } + if scanned > 0 { + log.Debug(). + Int("mraFiles", scanned). + Int("resolved", resolved). + Dur("elapsed", time.Since(started)). + Msg("misterdocs: resolved arcade setnames") + } + return nil +} + func (s *scraperImpl) eligibleTargets(targets []string, force bool) []string { result := make([]string, 0, len(targets)) for _, target := range targets { @@ -202,10 +316,13 @@ func (s *scraperImpl) eligibleTargets(targets []string, force bool) []string { return result } +// applyTargets writes targets in batches, calling onBatch with the half-open +// index range of each batch once it has committed. func (s *scraperImpl) applyTargets( ctx context.Context, opts scraper.ScrapeOptions, targets []database.ScrapeWriteTarget, + onBatch func(from, to int), ) error { batcher, canBatch := s.db.(database.ScrapeResultBatchApplier) for start := 0; start < len(targets); start += writeBatchSize { @@ -214,19 +331,34 @@ func (s *scraperImpl) applyTargets( } end := min(start+writeBatchSize, len(targets)) batch := targets[start:end] - if canBatch { - batchErr := batcher.ApplyScrapeResults(ctx, batch) - if batchErr == nil { - continue - } - log.Warn().Err(batchErr). - Int("targets", len(batch)). - Msg("misterdocs: batch write failed, falling back to per-record writes") + if err := s.applyBatch(ctx, batcher, canBatch, batch); err != nil { + return err } - for _, target := range batch { - if err := s.db.ApplyScrapeResult(ctx, target.MediaDBID, target.MediaTitleDBID, target.Write); err != nil { - return fmt.Errorf("misterdocs: write media %d: %w", target.MediaDBID, err) - } + if onBatch != nil { + onBatch(start, end) + } + } + return nil +} + +func (s *scraperImpl) applyBatch( + ctx context.Context, + batcher database.ScrapeResultBatchApplier, + canBatch bool, + batch []database.ScrapeWriteTarget, +) error { + if canBatch { + batchErr := batcher.ApplyScrapeResults(ctx, batch) + if batchErr == nil { + return nil + } + log.Warn().Err(batchErr). + Int("targets", len(batch)). + Msg("misterdocs: batch write failed, falling back to per-record writes") + } + for _, target := range batch { + if err := s.db.ApplyScrapeResult(ctx, target.MediaDBID, target.MediaTitleDBID, target.Write); err != nil { + return fmt.Errorf("misterdocs: write media %d: %w", target.MediaDBID, err) } } return nil diff --git a/pkg/database/scraper/misterdocs/scraper_test.go b/pkg/database/scraper/misterdocs/scraper_test.go index 68bd513e3..a55d8e930 100644 --- a/pkg/database/scraper/misterdocs/scraper_test.go +++ b/pkg/database/scraper/misterdocs/scraper_test.go @@ -22,7 +22,9 @@ package misterdocs import ( "context" "errors" + "fmt" "path/filepath" + "strings" "testing" "github.com/ZaparooProject/zaparoo-core/v2/pkg/database" @@ -176,6 +178,9 @@ func TestScrapeLoop_ForceSkipsCleanupAfterSourceLoadFailure(t *testing.T) { docsRoot := filepath.Join("media", "fat", "docs") sourcePath := filepath.Join(docsRoot, "SNES", artworkDirName) require.NoError(t, fs.MkdirAll(sourcePath, 0o750)) + require.NoError(t, afero.WriteFile( + fs, filepath.Join(sourcePath, indexFileName), []byte("#title\tartwork\n"), 0o600, + )) mediaDB := testhelpers.NewMockMediaDBI() mediaDB.On("GetTitlesBySystemID", systemdefs.SystemSNES).Return([]database.TitleWithSystem{ @@ -202,7 +207,7 @@ func TestScrapeLoop_ForceSkipsCleanupAfterSourceLoadFailure(t *testing.T) { } require.Len(t, updates, 2) require.Error(t, updates[0].Err) - require.ErrorContains(t, updates[0].Err, "parse artwork index") + require.ErrorContains(t, updates[0].Err, "requires name and key columns") assert.True(t, updates[1].Done) mediaDB.AssertNotCalled(t, "GetMediaPropertyMetadataByMediaDBIDs", assertmock.Anything, assertmock.Anything) mediaDB.AssertNotCalled( @@ -220,6 +225,11 @@ func TestScrapeLoop_AccumulatesSourceLoadFailures(t *testing.T) { secondSource := filepath.Join(docsRoot, "SNES", "Artwork Two") require.NoError(t, fs.MkdirAll(firstSource, 0o750)) require.NoError(t, fs.MkdirAll(secondSource, 0o750)) + for _, source := range []string{firstSource, secondSource} { + require.NoError(t, afero.WriteFile( + fs, filepath.Join(source, indexFileName), []byte("#title\tartwork\n"), 0o600, + )) + } mediaDB := testhelpers.NewMockMediaDBI() mediaDB.On("GetTitlesBySystemID", systemdefs.SystemSNES).Return([]database.TitleWithSystem{}, nil) @@ -531,7 +541,9 @@ func TestScrapeLoop_BatchWritesAndFallback(t *testing.T) { for update := range ch { updates = append(updates, update) } - require.Len(t, updates, 2) + // A progress update announcing the step size precedes the final one. + require.Len(t, updates, 3) + updates = updates[1:] require.Len(t, mediaDB.batches, 1) require.Len(t, mediaDB.batches[0], 1) if writeErr == nil { @@ -550,3 +562,138 @@ func TestScrapeLoop_BatchWritesAndFallback(t *testing.T) { }) } } + +func TestScrapeLoop_ResolvesArcadeArtworkThroughMRASetNames(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + docsRoot := filepath.Join("media", "fat", "docs") + artwork := filepath.Join(docsRoot, "Arcade", artworkDirName) + arcade := filepath.Join("media", "fat", "_Arcade") + require.NoError(t, fs.MkdirAll(artwork, 0o750)) + require.NoError(t, fs.MkdirAll(arcade, 0o750)) + // Keys and index names are MAME setnames; a clone row points at its parent. + files := map[string]string{ + filepath.Join(artwork, indexFileName): "#name\tcrc\tsize\tkey\n" + + "sfa3\t\t\tsfa3\n" + + "sfa3u\t\t\tsfa3\n" + + "shocktro\t\t\tshocktro\n", + filepath.Join(artwork, "sfa3.jpg"): "image", + filepath.Join(artwork, "shocktro.jpg"): "image", + filepath.Join(arcade, "Street Fighter Alpha 3 (USA).mra"): "" + + "Street Fighter Alpha 3sfa3u", + filepath.Join(arcade, "Shock Troopers.mra"): "" + + "SHOCKTRO", + filepath.Join(arcade, "Broken.mra"): "not xml at all", + } + for path, content := range files { + require.NoError(t, afero.WriteFile(fs, path, []byte(content), 0o600)) + } + + mediaDB := testhelpers.NewMockMediaDBI() + mediaDB.On("GetTitlesBySystemID", systemdefs.SystemArcade).Return([]database.TitleWithSystem{ + {DBID: 10, Slug: "streetfighteralpha3", Name: "Street Fighter Alpha 3", SystemID: systemdefs.SystemArcade}, + {DBID: 20, Slug: "shocktroopers", Name: "Shock Troopers", SystemID: systemdefs.SystemArcade}, + {DBID: 30, Slug: "broken", Name: "Broken", SystemID: systemdefs.SystemArcade}, + }, nil) + mediaDB.On("GetMediaBySystemID", systemdefs.SystemArcade).Return([]database.MediaWithFullPath{ + {DBID: 100, MediaTitleDBID: 10, Path: filepath.Join(arcade, "Street Fighter Alpha 3 (USA).mra")}, + {DBID: 200, MediaTitleDBID: 20, Path: filepath.Join(arcade, "Shock Troopers.mra")}, + {DBID: 300, MediaTitleDBID: 30, Path: filepath.Join(arcade, "Broken.mra")}, + }, nil) + written := make(map[int64]string, 2) + mediaDB.On("ApplyScrapeResult", assertmock.Anything, assertmock.Anything, assertmock.Anything, assertmock.Anything). + Run(func(args assertmock.Arguments) { + write, ok := args.Get(3).(*database.ScrapeWrite) + require.True(t, ok) + require.Len(t, write.MediaProps, 1) + mediaID, ok := args.Get(1).(int64) + require.True(t, ok) + written[mediaID] = write.MediaProps[0].Text + }).Return(nil) + + impl := &scraperImpl{ + fs: fs, db: mediaDB, docsRoots: []string{docsRoot}, + sources: map[string][]sourceDir{systemdefs.SystemArcade: { + {Path: artwork, SystemID: systemdefs.SystemArcade, Kind: sourceArtwork}, + }}, + } + ch := make(chan scraper.ScrapeUpdate, 4) + impl.scrapeLoop(context.Background(), scraper.ScrapeOptions{}, []string{systemdefs.SystemArcade}, ch) + + var updates []scraper.ScrapeUpdate + for update := range ch { + updates = append(updates, update) + } + // Live progress updates precede the step's final update; the final one + // for the system is the last before Done. + require.GreaterOrEqual(t, len(updates), 3) + final := updates[len(updates)-2] + assert.Equal(t, systemdefs.SystemArcade, final.SystemID) + require.NoError(t, final.Err) + assert.Equal(t, 2, final.Matched) + assert.Equal(t, final.Processed, final.Total) + assert.True(t, updates[len(updates)-1].Done) + first := updates[0] + assert.Equal(t, 0, first.Processed, "first update announces the step size before matching") + assert.Equal(t, final.Total, first.Total) + assert.Equal(t, map[int64]string{ + 100: filepath.ToSlash(filepath.Join(artwork, "sfa3.jpg")), + 200: filepath.ToSlash(filepath.Join(artwork, "shocktro.jpg")), + }, written) + mediaDB.AssertExpectations(t) +} + +func TestScrapeLoop_SkipsMRAScanWithoutArcadeSource(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + docsRoot := filepath.Join("media", "fat", "docs") + artwork := filepath.Join(docsRoot, "SNES", artworkDirName) + require.NoError(t, fs.MkdirAll(artwork, 0o750)) + require.NoError(t, afero.WriteFile(fs, filepath.Join(artwork, indexFileName), []byte("#name\tkey\n"), 0o600)) + + mediaDB := testhelpers.NewMockMediaDBI() + mediaDB.On("GetTitlesBySystemID", systemdefs.SystemSNES).Return([]database.TitleWithSystem{}, nil) + // An MRA path that does not exist on the filesystem: reading it would + // fail, and the scan must not even be attempted for a console system. + mediaDB.On("GetMediaBySystemID", systemdefs.SystemSNES).Return([]database.MediaWithFullPath{ + {DBID: 100, MediaTitleDBID: 10, Path: "/games/SNES/Stray.mra"}, + }, nil) + + opened := 0 + countingFS := &openCountingFS{Fs: fs, count: &opened} + impl := &scraperImpl{ + fs: countingFS, db: mediaDB, docsRoots: []string{docsRoot}, + sources: map[string][]sourceDir{systemdefs.SystemSNES: { + {Path: artwork, SystemID: systemdefs.SystemSNES, Kind: sourceArtwork}, + }}, + } + ch := make(chan scraper.ScrapeUpdate, 4) + impl.scrapeLoop(context.Background(), scraper.ScrapeOptions{}, []string{systemdefs.SystemSNES}, ch) + var updates []scraper.ScrapeUpdate + for update := range ch { + updates = append(updates, update) + } + require.NotEmpty(t, updates) + assert.True(t, updates[len(updates)-1].Done) + assert.Zero(t, opened, "no MRA should be opened for a system without an arcade source") + mediaDB.AssertExpectations(t) +} + +// openCountingFS counts opens of .mra files. +type openCountingFS struct { + afero.Fs + count *int +} + +func (fs *openCountingFS) Open(name string) (afero.File, error) { + if strings.EqualFold(filepath.Ext(name), mraExt) { + *fs.count++ + } + file, err := fs.Fs.Open(name) + if err != nil { + return nil, fmt.Errorf("open %q: %w", name, err) + } + return file, nil +}