fix(api): restore album-purchase track access - #1014
Merged
Conversation
Buying an album grants access to its tracks, and that access is meant to
survive a track later leaving the album for anyone whose purchase predates
the removal. That check has never worked.
tracks.playlists_previously_containing_track is a jsonb object keyed by
playlist id:
{"1284768821": {"time": 1725873897}}
Three things were wrong, stacked so that only the first was reachable:
1. The reader unmarshalled that object into a Go slice of
{playlist_id, removal_time}. That always errors, the error was
swallowed by an `err == nil` guard, and the resulting empty map meant
the entitlement loop iterated nothing -- denying access silently, with
no log and no failed request.
2. The query read the same column with jsonb_each_text and cast the value
to numeric. The value is an object, not a scalar, so it raises
`invalid input syntax for type numeric`. Unreachable in practice
because (1) short-circuits first: fixing only the Go side would have
traded a silent denial for a query error.
3. The model was playlist-shaped. Removal times belong to a (track,
album) pair -- two tracks can leave the same album months apart, and a
purchase between those dates covers one but not the other. Collecting
results into a playlist-keyed set cannot express that and would grant
or deny both together.
The column is now parsed as the object it is, flattened to (track_id,
playlist_id, removal_time) triples, and joined with jsonb_to_recordset so
each pair is matched against the purchase date on its own. Anything
unparseable yields no removals, which denies rather than grants.
Verified against Postgres with two tracks leaving one album at different
times: a buyer before both keeps both, a buyer between them keeps only the
track still in the album at purchase, a buyer after both keeps neither.
TestTrackAccessAfterRemovalFromPurchasedAlbum covers all three through the
access-info endpoint, and fails if the old array-shaped reader is restored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Buying an album grants access to its tracks, and that access is meant to survive a track later leaving the album for anyone whose purchase predates the removal. This PR now fixes the full path: the API reader, the ETL writer dependency, and the recoverable historical data.
Reader fixes
tracks.playlists_previously_containing_trackis a jsonb object keyed by playlist id:{"1284768821": {"time": 1725873897}}Three bugs prevented the API from using it:
{playlist_id, removal_time}. The error was swallowed, so access was denied silently.jsonb_each_textand cast the object value ({"time": ...}) directly to numeric, which would fail once the reader bug was fixed.(track, album)pair. If two tracks leave the same album at different times, a purchase between the removals covers only the track that was still in the album.The API now parses the production object shape, flattens it to
(track_id, playlist_id, removal_time)records, and joins those records to purchases withjsonb_to_recordset. Malformed input denies rather than grants.ETL writer
go.modnow consumes the merged OpenAudio writer from OpenAudio/go-openaudio#481:That pseudo-version resolves to merge commit
95f8e2ff0c66573920488e272dc53ca245c57343. The writer updatesplaylist_tracksand both reverse-index columns in the same transaction, using the block timestamp for future removal records.Backfill
Migration
0238_backfill_track_playlist_reverse_index.sqltreatsplaylist_tracksas the authoritative membership table and:tracks.playlists_containing_trackwith every active relation;It compares arrays as sets so ordering alone does not rewrite a track, keeps the search trigger enabled, temporarily suppresses the expensive catalog recount trigger, preserves a pre-disabled trigger state, and is idempotent.
The migration deliberately does not invent missing historical removal records.
playlist_tracks.updated_atwas written withnow(), not block time; using it for entitlement could grant access to a buyer who purchased after the on-chain removal but before a delayed indexer processed it. The merged ETL writer records exact block timestamps going forward.Rollout
API DDL runs in the pre-roll migration Job, while the old indexer can still be active. After the new indexer version is fully rolled out, manually execute
ddl/migrations/0238_backfill_track_playlist_reverse_index.sqlonce more against the writer database. Restartingpg_migratealone will not rerun an already tracked file.Then verify that the current reverse index has no mismatches:
Expected result:
0.Verification
go test -count=1 ./api/dbv1 -run 'TestParseTrackRemovals|TestTrackRemovalMarshalsToRecordsetColumns'go test -count=1 ./api -run TestTrackAccessAfterRemovalFromPurchasedAlbumUPDATE 0on_trackremains disabledmismatched_tracks = 0TestSearchstill requires Elasticsearch when run locally; it was already unrelated to this change.