Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions api/v1_notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
`
Expand Down
66 changes: 66 additions & 0 deletions api/v1_notifications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
14 changes: 14 additions & 0 deletions ddl/migrations/0228_notification_multi_recipient_user_ids_idx.sql
Original file line number Diff line number Diff line change
@@ -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.';
Loading