fix(api): scope user-subscriber reads to entity_type 'User'; unbreak main CI - #1013
Merged
Merged
Conversation
subscriptions.user_id is overloaded: for Event rows it mirrors the event id, and event ids are allocated independently of user ids. The subscribers endpoint and the upload-notification fan-outs in handle_track/handle_playlist matched on user_id alone, so a follower of event N counts as a subscriber of user N whenever the ids collide — they appear in /v1/users/N/subscribers and get notified about uploads from an artist they never subscribed to. This is reachable today with a single Event row, and OpenAudio/go-openaudio#469 (which legalizes cross-type coexistence) widens the exposure. The trigger functions are updated in ddl/functions/ (pg_migrate.sh re-applies them on md5 change) with the schema dump edited to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy User-type row shared (subscriber_id, user_id) with the deleted-event row. Production's subscriptions_current_uniq_idx — seeded into the test schema by #1011 — keys current rows on exactly that pair, so seeding panics with a unique violation and the test fails before exercising the endpoint. Move the legacy row to its own subscriber; both exclusion behaviors (entity_type filter, is_delete filter) stay covered. Once OpenAudio/go-openaudio#469 widens the index to include entity_type, the same-subscriber collision becomes legal again and is worth re-adding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
setup-go restores the go-build cache (which includes test results) keyed on go.sum, but the test schema lives in a dockerized Postgres that Go's cache invalidation cannot see. A sql/-only change therefore rides a stale green: #1011 merged with a test that fails against its own schema because the api package's results were served from cache. The first PR to touch a Go file afterwards inherited the failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TestEventFollowState_CountsOnlyLiveEventSubscriptions seeds the same (subscriber_id, user_id) pair for an Event row and a legacy User row, tripping the same unique-index panic the previous commit fixed in the followers test. Move the User row to its own subscriber; swept every other subscriptions fixture in the repo for current-row pair collisions and this was the last one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same event-id collision as the subscriber readers: the current_user_subscribed_targets CTE in get_users.sql matched on subscriptions.user_id alone, so a viewer following event N showed as subscribed to user N. Test seeds a colliding Event subscription and fails without the filter. Co-Authored-By: Claude Fable 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.
What
Four related fixes around the overloaded
subscriptions.user_idcolumn (it mirrors the event id for Event rows, and event ids are allocated independently of user ids). Background: #1011 seeded production'ssubscriptions_current_uniq_idxinto the test schema, which both broke main's CI and led to the discovery of the bugs below (see OpenAudio/go-openaudio#469 for the upstream identity fix).1. Live bug: event followers counted as user subscribers.
/v1/users/{id}/subscribersand the upload-notification fan-outs inhandle_track/handle_playlistmatch onuser_idalone. A follower of event N therefore counts as a subscriber of user N whenever the ids collide — they appear in the subscriber list and get "new upload" notifications for an artist they never subscribed to. Reachable today with a single Event row; go-openaudio#469 (which legalizes cross-type coexistence) widens the exposure. Fixed by addingentity_type = 'User'to all three readers (trigger functions updated inddl/functions/, whichpg_migrate.shre-applies on md5 change, with the schema dump edited to match). The Event-side readers were already correctly scoped since #977; this is the mirror image nobody did.2. Unbreak main CI. The events-followers fixture seeds a legacy User-type row sharing
(subscriber_id, user_id)with a deleted-event row — illegal under the index #1011 seeded, sodatabase.Seedpanics. The legacy row moves to its own subscriber; both exclusion behaviors stay covered. Once go-openaudio#469 widens the index to includeentity_type, the same-subscriber collision becomes legal again and is worth re-adding (noted in a comment).2b. Same bug in
does_current_user_subscribe. Thecurrent_user_subscribed_targetsCTE inget_users.sqlalso matched onuser_idalone, so a viewer following event N showed as subscribed to user N on every user-list surface. Sameentity_type = 'User'fix (sqlc regenerated). Note: this CTE filters onlyis_deleteand notis_current, unlike the other readers — that predates the #892 refactor and is left as-is here.3. Close the cache hole that let this merge green.
setup-gorestores the go-build cache (including test results) keyed ongo.sum, but the test schema lives in dockerized Postgres, invisible to Go's cache invalidation — so #1011's sql/-only commits rode a stale green while its own schema change broke a test.-count=1forces tests to actually run.Tests
TestUsersSubscribersnow seeds an Event subscription whose event id collides with the artist's user id and asserts the follower is not listed (fails without the filter).TestUserQuery_DoesCurrentUserSubscribeIgnoresEventSubscriptionsseeds a colliding Event subscription and asserts the flag stays false (fails without the filter).TestEventsFollowers_ReturnsOnlyLiveEventSubscriberspasses again (was panicking on main).-count=1against a freshly initialized schema volume.Not in this PR
The
pkg/etlpin bump + seeded-index update land separately once go-openaudio#469/#470 cut a release.🤖 Generated with Claude Code