feat(scraper): ZaparooCompanion xml support#839
Merged
Conversation
📝 WalkthroughWalkthroughThis PR extends the gamelist.xml scraper to support ZaparooCompanion parent/child entries and new boxart image variants (3D, side, back). It adds database methods for media lookup by path suffix and title by slug, introduces a JSON-RPC API endpoint to derive slug/title metadata from file paths, and updates image property mapping to prefer XML fields over filesystem scanning. ChangesCompanion Scraper, Image Variants, and Title-from-Path API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
BossRighteous
changed the title
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/api/models/responses.go`:
- Line 456: The SecondarySlug field is a nullable *string but its JSON tag lacks
omitempty so it is emitted as null; update the struct field declaration for
SecondarySlug to include the omitempty directive (change the json tag for
SecondarySlug to "secondarySlug,omitempty") so the key is omitted when unset,
ensuring behavior matches other optional response fields.
In `@pkg/database/scraper/gamelistxml/scraper_test.go`:
- Around line 920-1067: Add unit tests exercising the companion parent/child
ingestion/enrichment branches of GamelistXMLScraper.MapToDB: create a temp
SystemRootPath, construct GamelistRecord entries representing a parent (with
companion metadata and media files in AvailableMediaDirs) and a child (with a
companion reference linking to the parent), invoke
(&GamelistXMLScraper{}).MapToDB(&rec) and assert that the result contains the
parsed parent data, the child is matched to the parent (check TitleProps for
relation/parent tags), and media/title upserts are present for both (verify
expected media property keys from tags.TagProperty* and TitleProps values use
filepath.ToSlash on created files). Ensure tests cover both enrichment-from-file
(explicit Game.Boxart*/paths) and filesystem fallback using AvailableMediaDirs.
In `@pkg/database/scraper/gamelistxml/scraper.go`:
- Around line 1093-1095: The code is using only filepath.Base(c.ResolvedPath)
when calling mdb.FindMediaBySystemAndPathSuffix which can match unrelated files
with the same filename; change this to compute and use a path suffix relative to
the system root instead (e.g., use filepath.Rel(systemRoot, c.ResolvedPath) and
normalize with filepath.ToSlash) and pass that relative suffix to
mdb.FindMediaBySystemAndPathSuffix, falling back to the basename only if Rel
fails; apply the same change to the other occurrences around the block that
currently use filepath.Base (the call sites involving c.ResolvedPath and
mdb.FindMediaBySystemAndPathSuffix).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 915994b1-b73c-4262-9b4a-3e89a4974359
📒 Files selected for processing (16)
pkg/api/methods/media_image.gopkg/api/methods/media_title_from_path.gopkg/api/models/models.gopkg/api/models/params.gopkg/api/models/responses.gopkg/api/server.gopkg/database/database.gopkg/database/mediadb/mediadb.gopkg/database/mediadb/sql_media_titles.gopkg/database/mediadb/sql_scraper.gopkg/database/scraper/gamelistxml/scraper.gopkg/database/scraper/gamelistxml/scraper_test.gopkg/database/tags/tag_values.gopkg/database/tags/tags.gopkg/platforms/shared/esapi/gamelist.gopkg/testing/helpers/db_mocks.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/api/methods/media_title_from_path_test.go`:
- Line 59: Replace the hardcoded POSIX paths in the test params (the []byte JSON
literal that contains "/roms/nes/game.nes") with a platform-correct path built
via filepath.Join and then injected into the JSON for the request; for example,
construct path := filepath.Join("roms", "nes", "game.nes") (and the other
occurrence on line 63), then produce the params payload using json.Marshal or
fmt.Sprintf to include that path string instead of the literal so tests run
cross-platform in media_title_from_path_test.go.
In `@pkg/database/scraper/gamelistxml/scraper.go`:
- Around line 1108-1121: The code currently sets seenTitles[title.DBID] = true
even if UpsertMediaTitleTags or UpsertMediaTitleProperties failed; change the
logic in scraper.go so that seenTitles is only marked true after both required
upserts succeed (treat absent meta.TitleTags or meta.TitleProps as a no-op
success), i.e. track success for the tag upsert and the props upsert (call to
mdb.UpsertMediaTitleTags and mdb.UpsertMediaTitleProperties) and only set
seenTitles[title.DBID] = true when both operations that should run completed
without error; if either returns an error, do not mark seenTitles so later
children can retry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 917cbd9e-1e90-492b-937f-82bb1839c9c4
📒 Files selected for processing (8)
pkg/api/methods/media_title_from_path_test.gopkg/api/models/responses.gopkg/database/database.gopkg/database/mediadb/sql_scraper.gopkg/database/mediadb/sql_scraper_test.gopkg/database/scraper/gamelistxml/scraper.gopkg/database/scraper/gamelistxml/scraper_test.gopkg/testing/helpers/db_mocks.go
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/api/models/responses.go (1)
455-461: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winPlace
MediaTitleParseResponsewith top-level type declarations.This new type is declared after method definitions; move it into the type block before functions/methods to align with file-level Go structure rules.
As per coding guidelines, "Define Go types and consts near the top of the file, before functions and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/models/responses.go` around lines 455 - 461, MediaTitleParseResponse is declared after functions; move this type declaration into the file's top-level type declarations block (or create one if absent) so all types/consts live before any functions/methods; locate the MediaTitleParseResponse type symbol and cut/paste it into the existing top area where other type definitions are declared (before functions/methods) to comply with Go file-structure guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/api/models/params.go`:
- Around line 243-246: Move the MediaTitleParseParams type declaration so type
definitions appear before methods: locate MediaTitleParseParams and cut/paste it
above the ReaderConnection.IsEnabled method declaration (i.e., with other
top-level type/const defs near the top of the file). Ensure the struct tag and
field names remain unchanged and run go vet/go fmt to confirm formatting.
---
Outside diff comments:
In `@pkg/api/models/responses.go`:
- Around line 455-461: MediaTitleParseResponse is declared after functions; move
this type declaration into the file's top-level type declarations block (or
create one if absent) so all types/consts live before any functions/methods;
locate the MediaTitleParseResponse type symbol and cut/paste it into the
existing top area where other type definitions are declared (before
functions/methods) to comply with Go file-structure guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98f7ddf6-5c15-4491-866c-1baddd1bbd4f
📒 Files selected for processing (6)
pkg/api/methods/media_title_parse.gopkg/api/methods/media_title_parse_test.gopkg/api/models/models.gopkg/api/models/params.gopkg/api/models/responses.gopkg/api/server.go
Comment on lines
+243
to
+246
| type MediaTitleParseParams struct { | ||
| SystemID string `json:"systemId" validate:"required,min=1"` | ||
| Path string `json:"path" validate:"required,min=1"` | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move MediaTitleParseParams above method declarations.
MediaTitleParseParams is introduced after ReaderConnection.IsEnabled (Line 112). Please keep new type declarations before functions/methods in this file.
As per coding guidelines, "Define Go types and consts near the top of the file, before functions and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/api/models/params.go` around lines 243 - 246, Move the
MediaTitleParseParams type declaration so type definitions appear before
methods: locate MediaTitleParseParams and cut/paste it above the
ReaderConnection.IsEnabled method declaration (i.e., with other top-level
type/const defs near the top of the file). Ensure the struct tag and field names
remain unchanged and run go vet/go fmt to confirm formatting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Note:
source="ZaparooCompanion"is just a play on words since I jammed with @Anime0t4ku (of MiSTer Companion) on the format expectations and testing. It's not a specific vendor lock-in and any scraper/utility is free to use thesourcevalue and format concision.Adds ZaparooCompanion XML scraping support to the
gamelist.xmlscraper and expands the boxart property model to distinguish 2D front, 3D, side, and back artwork.Also adds an API method to cast a submitted path string to a MediaTitle with name/slugs/etc. This is useful to me, and I presume others in the future who understand and want to optimize for MediaTitle slugs as a parent in scraping.
Path ignorance
Titles are mapped by filename regardless of path, so any routine targeting MediaTitle enrichment can ignore paths.
This applies to scraper ingest, able to dedupe unique filenames regardless of path. It also applies to scraper output, where all paths can be made
./game.extas a flattened mapping.A new
MediaDBImethodFindMediaBySystemAndPathSuffixhandles the suffix-match lookup, with LIKE wildcard escaping for safety. This allows root paths to lookup any matching system Media in the DB on XML scan.ZaparooCompanion parent/child enrichment
gamelist.xmlfiles generated by ZaparooCompanion use a two-record schema: a parent entry (identified bysource="ZaparooCompanion"+idattribute, no path) carries full game metadata, and one or more child entries (parentidattribute + path) link regional ROM releases to that parent.The scraper now processes these entries as a pre-pass before the standard slug-based scrape:
MapToDB(no new DB rows created).Mediarow viaFindMediaBySystemAndPathSuffix. The parent's tags and properties are upserted onto the child's existingMediaTitle. Per-childregionandlangtags are written to theMediarow..slug extension MediaTitle matcher
A .slug extension in a companion style child entry will target the MediaTitle directly without proxy Media lookup. This is the ultimate generic targeting and may be used in conjunction with the
media.titleFromPathmethod for best sharability between romsets and filenames.Boxart property split
TagPropertyImageBoxartpreviously acted as a catch-all for all boxart variants. It is now scoped to 2D front artwork only. Three new tag property constants are introduced:TagPropertyImageBoxartimage-boxart<boxart2d>XML /boxart,boxart2d,boxart2dfrontdirsTagPropertyImageBoxart3Dimage-boxart3d<boxart3d>XML /boxart3ddirTagPropertyImageBoxartSideimage-boxartsideboxart2dsidedir (filesystem only)TagPropertyImageBoxartBackimage-boxartbackboxart2dbackdir (filesystem only)boxart3dis added to themedia.imageAPI default preference order betweenboxartandscreenshot. All four types are exposed inimageTypeTagsfor explicit API requests.esapi.GameadditionsNew XML fields decoded from
gamelist.xml:sourceattribute →SourceAttr(companion source detection)parentidattribute →ParentIDAttr(child→parent link)screenshot,titlescreen,boxart2d,boxart3d,logoelements, custom but clear intent as mapped to tags.MapToDBis updated to use these fields:Logofalls back toWheelfor the wheel property;TitleScreenfalls back toTitleShotfor the titleshot property.New API endpoint
media.titleFromPath— computes aMediaTitleslug and name from a system ID and path without touching the filesystem or database. Used by ZaparooCompanion to preview how the scanner will interpret a ROM path before indexing.Testing Steps
I can provide a small sample SNES system with complex conditions used in both the Companion testing as well as the Core testing. Tested against nested and repeated rom in hierarchy.
Summary by CodeRabbit
Release Notes