diff --git a/api/v1_notifications.go b/api/v1_notifications.go index 99089a46..b4a97b03 100644 --- a/api/v1_notifications.go +++ b/api/v1_notifications.go @@ -74,7 +74,7 @@ WITH latest_seen AS ( -- Equivalent to ARRAY[@user_id] && n.user_ids, split so single-recipient rows -- can use notification_single_recipient_user_timestamp_idx while -- multi-recipient arrays keep the existing overlap semantics. -matched_notifications AS ( +single_recipient_groups AS MATERIALIZED ( SELECT n.type, n.group_id FROM notification n CROSS JOIN latest_seen @@ -84,22 +84,40 @@ matched_notifications AS ( AND n.timestamp > COALESCE(latest_seen.seen_at, '-infinity'::timestamp) AND (n.type = ANY(@types) OR @types IS NULL) AND (n.type != ALL(@unsupported_types)) - UNION ALL + GROUP BY n.type, n.group_id + LIMIT @limit +), +single_recipient_count AS MATERIALIZED ( + SELECT COUNT(*)::int AS unread_count + FROM single_recipient_groups +), +-- Only touch the broader multi-recipient GIN path if the fast single-recipient +-- path did not already satisfy the capped unread count. +multi_recipient_groups AS MATERIALIZED ( SELECT n.type, n.group_id FROM notification n CROSS JOIN latest_seen - WHERE COALESCE(array_length(n.user_ids, 1), 0) != 1 + WHERE (SELECT unread_count FROM single_recipient_count) < @limit + AND COALESCE(array_length(n.user_ids, 1), 0) != 1 AND ARRAY[@user_id] && n.user_ids AND n.timestamp > (now()::timestamp - interval '90 days') AND n.timestamp > COALESCE(latest_seen.seen_at, '-infinity'::timestamp) AND (n.type = ANY(@types) OR @types IS NULL) AND (n.type != ALL(@unsupported_types)) + AND NOT EXISTS ( + SELECT 1 + FROM single_recipient_groups sg + WHERE sg.type = n.type + AND sg.group_id = n.group_id + ) + GROUP BY n.type, n.group_id + LIMIT GREATEST(@limit - (SELECT unread_count FROM single_recipient_count), 0) ) SELECT COUNT(*) FROM ( - SELECT 1 - FROM matched_notifications - GROUP BY type, group_id + SELECT type, group_id FROM single_recipient_groups + UNION ALL + SELECT type, group_id FROM multi_recipient_groups LIMIT @limit ) unread_notifications; ` diff --git a/api/v1_notifications_test.go b/api/v1_notifications_test.go index 3685fe14..8fb43dee 100644 --- a/api/v1_notifications_test.go +++ b/api/v1_notifications_test.go @@ -152,6 +152,72 @@ func TestV1Notifications_ReturnsMultiRecipientRows(t *testing.T) { }) } +func TestV1Notifications_LimitZeroDedupesSingleAndMultiRecipientGroups(t *testing.T) { + app := emptyTestApp(t) + + fixtures := database.FixtureMap{ + "notification": []map[string]any{ + { + "id": 1, + "specifier": "single", + "group_id": "milestone:shared", + "type": "milestone", + "user_ids": []int{1}, + "timestamp": time.Now().Add(-1 * time.Minute), + "data": []byte(`{"type": "TRACK_REPOST_COUNT", "threshold": 10, "track_id": 101}`), + }, + { + "id": 2, + "specifier": "multi", + "group_id": "milestone:shared", + "type": "milestone", + "user_ids": []int{1, 2}, + "timestamp": time.Now(), + "data": []byte(`{"type": "TRACK_SAVE_COUNT", "threshold": 10, "track_id": 102}`), + }, + }, + } + + database.Seed(app.pool.Replicas[0], fixtures) + + status, body := testGet(t, app, "/v1/notifications/"+trashid.MustEncodeHashID(1)+"?limit=0") + assert.Equal(t, 200, status) + + jsonAssert(t, body, map[string]any{ + "data.notifications.#": 0, + "data.unread_count": 1, + }) +} + +func TestV1Notifications_LimitZeroCapsUnreadCount(t *testing.T) { + app := emptyTestApp(t) + + notifs := make([]map[string]any, 0, notificationUnreadPollLimit+1) + for i := range notificationUnreadPollLimit + 1 { + notifs = append(notifs, map[string]any{ + "id": i + 1, + "specifier": strconv.Itoa(i + 1), + "group_id": "milestone:" + strconv.Itoa(i+1), + "type": "milestone", + "user_ids": []int{1}, + "timestamp": time.Now().Add(-1 * time.Duration(i) * time.Minute), + "data": []byte(`{"type": "TRACK_REPOST_COUNT", "threshold": 10, "track_id": 101}`), + }) + } + + database.Seed(app.pool.Replicas[0], database.FixtureMap{ + "notification": notifs, + }) + + status, body := testGet(t, app, "/v1/notifications/"+trashid.MustEncodeHashID(1)+"?limit=0") + assert.Equal(t, 200, status) + + jsonAssert(t, body, map[string]any{ + "data.notifications.#": 0, + "data.unread_count": notificationUnreadPollLimit, + }) +} + func TestV1Notifications_NotDeletedTrack(t *testing.T) { app := emptyTestApp(t) diff --git a/ddl/migrations/0228_notification_multi_recipient_user_ids_idx.sql b/ddl/migrations/0228_notification_multi_recipient_user_ids_idx.sql new file mode 100644 index 00000000..094227c5 --- /dev/null +++ b/ddl/migrations/0228_notification_multi_recipient_user_ids_idx.sql @@ -0,0 +1,14 @@ +-- The notification API handles the overwhelmingly common single-recipient path +-- with notification_single_recipient_user_timestamp_idx. Keep the fallback +-- multi-recipient overlap scan off the all-row GIN index so high-notification +-- users do not recheck large numbers of single-recipient rows. +-- +-- NOTE: intentionally NOT wrapped in BEGIN/COMMIT so CREATE INDEX +-- CONCURRENTLY can run without holding an ACCESS EXCLUSIVE lock on notification. + +CREATE INDEX CONCURRENTLY IF NOT EXISTS notification_multi_recipient_user_ids_idx + ON public.notification USING gin (user_ids) + WHERE COALESCE(array_length(user_ids, 1), 0) != 1; + +COMMENT ON INDEX public.notification_multi_recipient_user_ids_idx IS + 'Covers notification reads for the uncommon multi-recipient user_ids array path.';