diff --git a/js/app/src/shell.tsx b/js/app/src/shell.tsx index fedd52d11..de6beb4d0 100644 --- a/js/app/src/shell.tsx +++ b/js/app/src/shell.tsx @@ -447,13 +447,18 @@ export default function Shell() { }, []); const notificationToken = useNotificationToken(); + const did = useStore((state) => state.oauthSession?.did); const hydrated = useHydrated(); + // Re-register when the token changes OR once the logged-in DID resolves, so a + // token acquired before the OAuth session finishes restoring still gets its + // repoDID association registered (otherwise the user is excluded from + // follower livestream notifications). useEffect(() => { if (notificationToken) { registerNotificationToken(); } - }, [notificationToken]); + }, [notificationToken, did]); // Handle incoming push notification routing const notificationDestination = useNotificationDestination(); diff --git a/js/app/store/slices/platformSlice.native.ts b/js/app/store/slices/platformSlice.native.ts index c608b3aa4..abb1ebef3 100644 --- a/js/app/store/slices/platformSlice.native.ts +++ b/js/app/store/slices/platformSlice.native.ts @@ -94,6 +94,14 @@ export const createPlatformSlice: StateCreator< console.warn("Failed to acquire notification token"); } + // FCM rotates tokens periodically. Keep the store in sync so the + // registration effect re-runs and re-registers the new token, otherwise + // a rotated token never reaches the server until the next cold start. + msg.onTokenRefresh((refreshed) => { + console.log("Notification token refreshed"); + set({ notificationToken: refreshed }); + }); + // Subscribe to topic(s) if desired msg .subscribeToTopic("live") @@ -150,21 +158,21 @@ export const createPlatformSlice: StateCreator< return; } - const { platform, bluesky } = get() as AppStore & { - platform: { notificationToken: string | null }; - bluesky: { oauthSession?: { did?: string } }; - }; + // The store is flat (slices are spread, not namespaced), so read these + // directly off get(). The previous get().platform / get().bluesky access + // was always undefined, which meant token was undefined and this threw + // before ever registering anything. + const { notificationToken, oauthSession } = get(); - const token = platform?.notificationToken; + const token = notificationToken; if (!token) { - throw new Error("No notification token to register"); + console.log("No notification token to register yet"); + return; } - const body: any = { - token, - }; + const body: { token: string; repoDID?: string } = { token }; - const did = bluesky?.oauthSession?.did; + const did = oauthSession?.did; if (did) { body.repoDID = did; } diff --git a/pkg/statedb/notification.go b/pkg/statedb/notification.go index 5d80c312f..3c98f74e7 100644 --- a/pkg/statedb/notification.go +++ b/pkg/statedb/notification.go @@ -3,6 +3,8 @@ package statedb import ( "fmt" "time" + + "gorm.io/gorm/clause" ) type Notification struct { @@ -12,18 +14,28 @@ type Notification struct { UpdatedAt time.Time `gorm:"column:updated_at"` } +// CreateNotification registers (or refreshes) a device's push token. When a +// repoDID is supplied we upsert it onto the token's row so livestream blasts +// can target the user's followers. When repoDID is empty we make sure the +// token row exists but never clobber an existing repoDID association. +// +// This deliberately avoids DB.Save(): Save issues a full-row UPDATE including +// zero-value columns, so a re-registration with no repoDID (e.g. the client +// posts before its OAuth session has restored) would blank out repo_did and +// silently drop the user from follower notifications. func (state *StatefulDB) CreateNotification(token string, repoDID string) error { - not := Notification{ - Token: token, - } if repoDID != "" { - not.RepoDID = repoDID - } - err := state.DB.Save(¬).Error - if err != nil { - return err + not := Notification{Token: token, RepoDID: repoDID} + return state.DB.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "token"}}, + DoUpdates: clause.AssignmentColumns([]string{"repo_did", "updated_at"}), + }).Create(¬).Error } - return nil + not := Notification{Token: token} + return state.DB.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "token"}}, + DoNothing: true, + }).Create(¬).Error } func (state *StatefulDB) ListNotifications() ([]Notification, error) { diff --git a/pkg/statedb/notification_test.go b/pkg/statedb/notification_test.go new file mode 100644 index 000000000..1f593df95 --- /dev/null +++ b/pkg/statedb/notification_test.go @@ -0,0 +1,76 @@ +package statedb + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestNotificationRepoDIDPreserved guards against the regression where +// re-registering a push token without a repoDID wiped out the existing +// association. CreateNotification used to DB.Save() the row, which issues a +// full-row UPDATE and blanked repo_did whenever the client re-registered +// before its OAuth session had restored, silently dropping the device from +// follower livestream blasts. +func TestNotificationRepoDIDPreserved(t *testing.T) { + WithAllDatabases(t, func(state *StatefulDB) { + const token = "device-token-1" + const didA = "did:plc:aaaa" + const didB = "did:plc:bbbb" + + // Initial registration while logged in associates the DID. + require.NoError(t, state.CreateNotification(token, didA)) + tokens, err := state.GetManyNotificationTokens([]string{didA}) + require.NoError(t, err) + require.Equal(t, []string{token}, tokens) + + // Re-registration without a DID (e.g. before the OAuth session has + // restored) must NOT clobber the existing association. + require.NoError(t, state.CreateNotification(token, "")) + tokens, err = state.GetManyNotificationTokens([]string{didA}) + require.NoError(t, err) + require.Equal(t, []string{token}, tokens, "repo_did was wiped by a DID-less re-registration") + + // ...and it must not have duplicated the row (token is the primary key). + nots, err := state.ListNotifications() + require.NoError(t, err) + require.Len(t, nots, 1) + + // Re-registering with a different DID replaces the association. + require.NoError(t, state.CreateNotification(token, didB)) + tokens, err = state.GetManyNotificationTokens([]string{didB}) + require.NoError(t, err) + require.Equal(t, []string{token}, tokens) + tokens, err = state.GetManyNotificationTokens([]string{didA}) + require.NoError(t, err) + require.Empty(t, tokens, "old DID association should be replaced") + }) +} + +// TestNotificationAnonymousThenAssociated covers a token that first registers +// with no DID (anonymous) and later gets associated once the user logs in, +// without creating a duplicate row. +func TestNotificationAnonymousThenAssociated(t *testing.T) { + WithAllDatabases(t, func(state *StatefulDB) { + const token = "device-token-2" + const did = "did:plc:cccc" + + // Anonymous registration: the row exists but has no association yet. + require.NoError(t, state.CreateNotification(token, "")) + tokens, err := state.GetManyNotificationTokens([]string{did}) + require.NoError(t, err) + require.Empty(t, tokens) + nots, err := state.ListNotifications() + require.NoError(t, err) + require.Len(t, nots, 1) + + // Once logged in, the association is set without adding a new row. + require.NoError(t, state.CreateNotification(token, did)) + tokens, err = state.GetManyNotificationTokens([]string{did}) + require.NoError(t, err) + require.Equal(t, []string{token}, tokens) + nots, err = state.ListNotifications() + require.NoError(t, err) + require.Len(t, nots, 1) + }) +}