Skip to content

CMRT 21: Worker: Retrieve all four MBID fields from AcoustID in one request - #27

Open
Swarnadip-Kar wants to merge 3 commits into
cmrt-feature-fingerprint-clear-reanalyzefrom
cmrt-worker-mbid-fields
Open

CMRT 21: Worker: Retrieve all four MBID fields from AcoustID in one request#27
Swarnadip-Kar wants to merge 3 commits into
cmrt-feature-fingerprint-clear-reanalyzefrom
cmrt-worker-mbid-fields

Conversation

@Swarnadip-Kar

Copy link
Copy Markdown
Owner

Resolves the three hardcoded QString() placeholders in acoustidResultReady
and the "requires a separate MusicBrainz call" TODO. All four MBID fields
now come from the single AcoustID /v2/lookup response with no extra
network calls.

Changes

acoustidworker.h

  • LookupResult gains releaseId, artistId, trackId string members.
    All default to QString(). Comments document the JSON path each
    field is extracted from.

acoustidworker.cpp — doLookup()

  • meta parameter: "recordingids" → "recordings releases tracks"
  • JSON parsing: existing recording UUID loop unchanged. New block
    after it descends into recordings[0].releases[0] for releaseId and
    artistId, and into releases[0].mediums[0].tracks[0] for trackId.
    Every array level is guarded with isEmpty() — the API omits arrays
    entirely when empty rather than returning [].

acoustidworker.cpp — processJob() Step 5a

  • cacheEntry.musicbrainzReleaseId populated from result.releaseId so
    cache hits also carry the release field for audio-duplicate tracks.
  • emit acoustidResultReady: three QString() replaced with
    result.releaseId, result.trackId, result.artistId.

Notes for reviewers

  • The guard at every array level (recordings, releases, artists,
    mediums, tracks) is required. The AcoustID API contract omits arrays
    entirely when they are empty — a missing key returns QJsonValue::Undefined
    and .toArray() returns an empty QJsonArray, which isEmpty() catches.
    Skipping any guard would crash on tracks with partial metadata coverage.
  • Only the first recording, first release, first artist, and first track
    are used — consistent with how we already take the first recording UUID
    for the recording ID.
  • The cache write now includes musicbrainzReleaseId. This is safe because
    AcoustIdCacheEntry.musicbrainzReleaseId was already nullable (empty
    string maps to NULL in the schema from day one).

Depends on: cmrt-feature-fingerprint-clear-reanalyze (#26 )

@coveralls

coveralls commented Jun 8, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 27123028384

Coverage decreased (-0.007%) to 30.459%

Details

  • Coverage decreased (-0.007%) from the base build.
  • Patch coverage: 34 uncovered changes across 1 file (0 of 34 lines covered, 0.0%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
src/musicbrainz/acoustidworker.cpp 34 0 0.0%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/musicbrainz/acoustidworker.cpp 1 0.0%

Coverage Stats

Coverage Status
Relevant Lines: 116171
Covered Lines: 35385
Line Coverage: 30.46%
Coverage Strength: 59292.24 hits per line

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants