fix(etl): key subscription identity on entity_type, not just numeric id - #469
Merged
Conversation
subscriptions.user_id is overloaded: it holds the followed user's id for
User subscriptions and the event id for Event subscriptions. User and
event ids are allocated independently, so user N and event N can both
exist — but subscriptions_current_uniq_idx (0030) keyed current rows on
(subscriber_id, user_id) alone, conflating the two:
- Subscribe rejected any subscription whose target collided numerically
with an existing subscription of the other type ("already exists with
a different entity type") — including tombstoned ones, since
unsubscribes keep is_current = true.
- The Follow auto-subscribe upsert had no such guard and could land on a
colliding Event row: following the user flipped that row instead of
creating a User subscription, and unfollowing tombstoned the Event
subscription — silently unsubscribing the user from a remix contest.
Migration 0037 rebuilds the index as (subscriber_id, user_id,
entity_type) WHERE is_current; the old key is strictly tighter, so
existing data cannot violate the new one and no dedupe is needed. Both
upserts now arbitrate on the widened key (the follow path writes
entity_type explicitly instead of relying on the column default), and
the cross-type rejection in validateSubscribe is gone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 7, 2026
…tity-entity-type # Conflicts: # pkg/etl/processors/entity_manager/social_follow.go
This was referenced Aug 8, 2026
rickyrombo
added a commit
to AudiusProject/api
that referenced
this pull request
Aug 8, 2026
…main CI (#1013) ## What Four related fixes around the overloaded `subscriptions.user_id` column (it mirrors the event id for Event rows, and event ids are allocated independently of user ids). Background: #1011 seeded production's `subscriptions_current_uniq_idx` into 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}/subscribers` and the upload-notification fan-outs in `handle_track` / `handle_playlist` match on `user_id` alone. 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 adding `entity_type = 'User'` to all three readers (trigger functions updated in `ddl/functions/`, which `pg_migrate.sh` re-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, so `database.Seed` panics. The legacy row moves to its own subscriber; both exclusion behaviors stay covered. Once go-openaudio#469 widens the index to include `entity_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`.** The `current_user_subscribed_targets` CTE in `get_users.sql` also matched on `user_id` alone, so a viewer following event N showed as subscribed to user N on every user-list surface. Same `entity_type = 'User'` fix (sqlc regenerated). Note: this CTE filters only `is_delete` and not `is_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-go` restores the go-build cache (including test results) keyed on `go.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=1` forces tests to actually run. ## Tests - `TestUsersSubscribers` now 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_DoesCurrentUserSubscribeIgnoresEventSubscriptions` seeds a colliding Event subscription and asserts the flag stays false (fails without the filter). - `TestEventsFollowers_ReturnsOnlyLiveEventSubscribers` passes again (was panicking on main). - Full suite green locally with `-count=1` against a freshly initialized schema volume. ## Not in this PR The `pkg/etl` pin bump + seeded-index update land separately once go-openaudio#469/#470 cut a release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- 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
1 participant
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.
Problem
subscriptions.user_idis overloaded: it holds the followed user's id forUsersubscriptions and the event id forEventsubscriptions. User ids and event ids are allocated independently, so user N and event N can both exist — butsubscriptions_current_uniq_idx(migration 0030) keyed current rows on(subscriber_id, user_id)alone, conflating the two. Two failure modes:Subscriberefused any subscription whose target collided numerically with an existing subscription of the other entity type — including tombstoned ones, since unsubscribes keepis_current = true. Having once followed and unfollowed user N permanently blocked following event N, and vice versa.This surfaced downstream in AudiusProject/api, where seeding the production indexes into the test schema (AudiusProject/api#1011) made an existing events-followers test fixture — a legitimate cross-type collision — unseedable, breaking CI on main there.
Fix
(subscriber_id, user_id, entity_type) WHERE is_current = true. The 0030 key is strictly tighter, so existing data cannot violate the widened key; no dedupe or backfill needed.social_subscription.go: upsert arbitrates on the widened key,entity_typemoves out of theDO UPDATEset (it's identity now), and the cross-type rejection (subscriptionIdentityTypeConflict) is deleted.social_follow.go: the auto-subscribe upsert writesentity_type = 'User'explicitly and arbitrates on the widened key, so it can never land on — or tombstone — a colliding Event subscription.track_contest_subscribe.gois unchanged: itsON CONFLICTtargets the PK, and the widened index makes its insert strictly less conflict-prone.Tests
TestSubscribe_UserAndEventWithSameIdCoexist: subscribe to user N and event N, unsubscribe the user, the event subscription survives.TestFollow_AutoSubscribeIgnoresCollidingEventSubscription: the exact silent-clobber scenario — follow then unfollow a user whose id collides with a followed event.Full
pkg/etlsuite passes with-count=1against Postgres 17.🤖 Generated with Claude Code