diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index ee4225f7..9fedf0de 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -33,6 +33,10 @@ vi.mock('react-native', () => { }, NativeEventEmitter, PermissionsAndroid: { RESULTS: { GRANTED: 'granted' }, request: vi.fn(async () => 'granted') }, + // What the stores the server check drops from need at load. + AppState: { currentState: 'active', addEventListener: () => ({ remove: () => undefined }) }, + Appearance: { getColorScheme: () => 'dark', addChangeListener: () => ({ remove: () => undefined }) }, + Linking: { openURL: async () => undefined }, }; }); @@ -96,6 +100,7 @@ import type { EmailPushConfig } from '../../api/types'; import { jmapClient } from '../../api/jmap-client'; import { NativeModules } from 'react-native'; import { generateAccountId } from '../account-utils'; +import { reconcileServerFingerprint } from '../server-change'; const OUR_DCID = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; const ACCOUNT_ID = generateAccountId('user@example.com', 'https://mail.example.com'); @@ -1118,6 +1123,35 @@ describe('resyncPushNotifications', () => { expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull(); }); + it('re-creates the subscription when this build first sees a server replaced before it ran (0.16.23 to 1.0)', async () => { + // Left by the released build, which recorded the expiry but no server + // fingerprint, against the 0.16 server; the fresh 1.0 install holds no + // subscriptions. The first check can't tell a replacement from none. + await AsyncStorage.setItem(SUB_KEY, 'existing'); + await AsyncStorage.setItem(EXPIRES_KEY, inDays(60)); + await AsyncStorage.setItem('push:accountIds:v1', JSON.stringify([ACCOUNT_ID])); + listMock.mockResolvedValue([]); + const session10 = { + apiUrl: 'https://mail.example.com/jmap/', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' }, + accounts: { + 'jmap-primary': { + name: 'user', isPersonal: true, isReadOnly: false, + accountCapabilities: { 'urn:ietf:params:jmap:mail': {}, 'urn:ietf:params:jmap:principals:owner': {} }, + }, + }, + capabilities: { 'urn:ietf:params:jmap:core': {}, 'urn:ietf:params:jmap:metadata': {} }, + }; + + expect(await reconcileServerFingerprint(ACCOUNT_ID, session10 as never)).toBe(false); + const result = await resyncPushNotifications({ relayBaseUrl: RELAY }); + + expect(result?.subscriptionId).toBe('new-server-id'); + expect(createMock).toHaveBeenCalledTimes(1); + expect(await readPushAccountIds()).toEqual([ACCOUNT_ID]); + expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull(); + }); + it('does not take over another device\'s subscription that reuses the recorded id', async () => { // Stalwart's ids are short and sequential: after a reinstall another // device's subscription can come back under the id this device recorded. diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index fec7f4c2..332c6426 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -1274,6 +1274,16 @@ export async function forgetPushServerState(accountId: string): Promise { ]); } +/** + * Forget the recorded subscription's expiry alone, for an account whose + * server may have been replaced unseen (lib/server-change): the next resync + * then re-creates a subscription the server lacks instead of taking it for + * revoked. + */ +export async function forgetPushSubscriptionExpiry(accountId: string): Promise { + await AsyncStorage.removeItem(subscriptionExpiresKey(accountId)); +} + /** * Tear down push for a single account. Destroys every JMAP subscription the * server holds for this device (assumes the jmapClient is currently @@ -1339,8 +1349,8 @@ export async function disablePushForAccount(accountId: string): Promise { * True when the server no longer has this account's recorded subscription * although it wasn't due to expire - someone revoked it. Without a known * expiry (a registration made by an older build, or one on a server that was - * replaced since) nothing can be told apart, so it counts as not revoked. - * Throws when the server can't be asked. + * replaced since, or may have been) nothing can be told apart, so it counts + * as not revoked. Throws when the server can't be asked. */ async function wasRevokedOnServer(accountId: string): Promise { const storedServerId = await AsyncStorage.getItem(subscriptionIdKey(accountId)); diff --git a/src/lib/server-change.ts b/src/lib/server-change.ts index d9e3bfee..a554e87e 100644 --- a/src/lib/server-change.ts +++ b/src/lib/server-change.ts @@ -12,7 +12,7 @@ import { toast } from '../stores/toast-store'; import { clearBodyHeights } from './body-heights'; import { clearBodyDocuments } from './email-body-document'; import { clearEmailDetailCache } from './email-detail-cache'; -import { forgetPushServerState } from './push-notifications'; +import { forgetPushServerState, forgetPushSubscriptionExpiry } from './push-notifications'; import { primaryAccountId, readServerFingerprint, @@ -97,22 +97,62 @@ export async function forgetServerDerivedState( // Get the emptied caches onto disk before the new fingerprint is. await step(() => flushPersistedWrites()); - if (droppedOps > 0) { - toast.warning(t( - 'offline.discarded_changes', - '{count, plural, one {# offline change was} other {# offline changes were}} discarded because the mail server changed', - { count: droppedOps }, - )); + announceDiscardedChanges(droppedOps); + if (failures.length > 0) throw failures[0]; +} + +function announceDiscardedChanges(count: number): void { + if (count <= 0) return; + toast.warning(t( + 'offline.discarded_changes', + '{count, plural, one {# offline change was} other {# offline changes were}} discarded because the mail server changed', + { count }, + )); +} + +/** + * The first sight of an account: no build before this one recorded its + * server, so nothing tells whether it was replaced before this one ran (an + * update of the app and a reinstall for Stalwart 1.0 can fall together). + * The caches stay, as they do while the server stays the same; dropping them + * would cost everyone who updates a full download. What would act on the + * server by itself goes: + * - the outbox. Its ops replay on their own, and on a replaced server an id + * names another message: a queued delete deletes that one. Nothing proves + * an op was made on this server: the queue records no server, a + * reinstalled Stalwart may accept the old JMAP states, and a queued move + * or delete took its message out of the cached list, so there is no copy + * to compare the server's message under that id with. Offline edits of + * whoever updates with some still unsent are lost, and announced. + * - the push subscription's recorded expiry. A replaced server holds no + * subscription, which the resync would take for a revocation and turn + * push off for good; without the expiry it re-creates it. + * Every step runs; the first failure is thrown afterwards. + */ +async function forgetUnconfirmedServerState(accountId: string): Promise { + const failures: unknown[] = []; + let droppedOps = 0; + try { + droppedOps = await useOutboxStore.getState().clearAccount(accountId); + } catch (err) { + failures.push(err); } + try { + await forgetPushSubscriptionExpiry(accountId); + } catch (err) { + failures.push(err); + } + announceDiscardedChanges(droppedOps); if (failures.length > 0) throw failures[0]; } /** * Compare the session with the fingerprint recorded for the local account * (username@host) and drop what was derived from the server when it changed. - * The first sight of an account only records it. The fingerprint is written - * after the caches are gone, so an interrupted drop is repeated next time. - * Resolves to true when caches were dropped. + * On the first sight of an account only what can't be confirmed to belong + * to its server goes (forgetUnconfirmedServerState). The fingerprint is + * written after that, so an interrupted drop is repeated next time. Resolves + * to true when caches were dropped. */ export async function reconcileServerFingerprint(accountId: string, session: JMAPSession): Promise { const next = sessionFingerprint(session); @@ -131,6 +171,8 @@ export async function reconcileServerFingerprint(accountId: string, session: JMA previous: await readServerPrimaryAccountId(accountId), next: primary || null, }); + } else { + await forgetUnconfirmedServerState(accountId); } await writeServerFingerprint(accountId, next, primary); return recorded !== null; diff --git a/src/stores/__tests__/auth-store-server-change.test.ts b/src/stores/__tests__/auth-store-server-change.test.ts index 2274dcf9..6b0a2e9a 100644 --- a/src/stores/__tests__/auth-store-server-change.test.ts +++ b/src/stores/__tests__/auth-store-server-change.test.ts @@ -78,9 +78,11 @@ vi.mock('../../lib/push-notifications', () => ({ teardownPushNotifications: vi.fn(async () => undefined), teardownPushNotificationsForAccount: vi.fn(async () => undefined), forgetPushServerState: vi.fn(async () => undefined), + forgetPushSubscriptionExpiry: vi.fn(async () => undefined), })); import AsyncStorage from '@react-native-async-storage/async-storage'; +import { forgetPushSubscriptionExpiry } from '../../lib/push-notifications'; import { useAuthStore } from '../auth-store'; import { useAccountStore } from '../account-store'; import { useEmailStore } from '../email-store'; @@ -190,4 +192,19 @@ describe('restoring the session on a replaced server', () => { expect(useOutboxStore.getState().entries.map((e) => e.op.emailId)).toEqual(['b']); expect(await AsyncStorage.getItem(OUTBOX_KEY)).not.toBeNull(); }); + + it('sends none of the queue the build before left, the first time this one sees the server (Stalwart 1.0)', async () => { + // No fingerprint: the released build never recorded one, and the server + // may have been replaced before this one ran. "b" can be another message. + await AsyncStorage.setItem(OUTBOX_KEY, JSON.stringify(QUEUED)); + + expect(await useAuthStore.getState().restoreSession()).toBe(true); + await new Promise((r) => setTimeout(r, 20)); + + expect(emailSets()).toEqual([]); + expect(useOutboxStore.getState().entries).toEqual([]); + expect(await AsyncStorage.getItem(OUTBOX_KEY)).toBeNull(); + expect(forgetPushSubscriptionExpiry).toHaveBeenCalledWith(ACCOUNT); + expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + }); }); diff --git a/src/stores/__tests__/calendar-subscriptions-store.test.ts b/src/stores/__tests__/calendar-subscriptions-store.test.ts index ba609fbd..7d827c1c 100644 --- a/src/stores/__tests__/calendar-subscriptions-store.test.ts +++ b/src/stores/__tests__/calendar-subscriptions-store.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('../../api/calendar', () => ({ createCalendar: vi.fn(), @@ -16,7 +16,7 @@ vi.mock('../../api/jmap-client', () => ({ jmapClient: { accountId: 'acc-1', isConnected: true, username: 'user', serverUrl: 'https://mail.example.org' }, })); const calendarStore = vi.hoisted(() => ({ - importEvents: vi.fn(async () => ({ created: 1, updated: 0 })), + importEvents: vi.fn(async (_events: unknown[], _calendarId: string) => ({ created: 1, updated: 0 })), fetchCalendars: vi.fn(async () => undefined), refresh: vi.fn(async () => undefined), })); @@ -32,6 +32,7 @@ vi.mock('@react-native-async-storage/async-storage', () => ({ })); import * as calendarApi from '../../api/calendar'; +import { useAccountStore } from '../account-store'; import { normalizeFeedUrl, selectAccountSubscriptions, @@ -185,8 +186,9 @@ describe('subscriptions of a server that was replaced (0.16.23 to 1.0)', () => { expect(useCalendarSubscriptionsStore.getState().subscriptions).toEqual([]); }); - it('drops no events from a calendar under the recorded id that carries another name', async () => { - // The reinstalled server gave "c" to the user's own calendar. + it('neither drops events from nor imports the feed into a calendar under the recorded id that carries another name', async () => { + // The reinstalled server gave "c" to the user's own calendar (a server + // replaced before this build first saw it, so nothing unlinked it). vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Personal'); vi.mocked(calendarApi.queryEvents).mockResolvedValue(['p1']); vi.mocked(calendarApi.getEventIdentities).mockResolvedValue([{ id: 'p1', uid: 'dentist', calendarIds: { c: true } }] as never); @@ -198,6 +200,21 @@ describe('subscriptions of a server that was replaced (0.16.23 to 1.0)', () => { expect(calendarApi.queryEvents).not.toHaveBeenCalled(); expect(calendarApi.deleteEvents).not.toHaveBeenCalled(); expect(calendarApi.updateEvent).not.toHaveBeenCalled(); + // The feed goes into a new mirror, as for an unlinked subscription. + expect(calendarApi.createCalendar).toHaveBeenCalledWith('Holidays', '#ff0000'); + expect(calendarStore.importEvents.mock.calls.map(([, calendarId]) => calendarId)).toEqual(['n1']); + expect(only()).toMatchObject({ calendarId: 'n1', accountId: 'acc-1', localAccountId: ME, lastError: null }); + }); + + it('touches nothing when it cannot tell whether the recorded calendar is the mirror', async () => { + vi.mocked(calendarApi.getCalendarName).mockRejectedValue(new Error('Network request failed')); + useCalendarSubscriptionsStore.setState({ subscriptions: [mine()] }); + + await useCalendarSubscriptionsStore.getState().syncSubscription('mine'); + + expect(calendarStore.importEvents).not.toHaveBeenCalled(); + expect(calendarApi.createCalendar).not.toHaveBeenCalled(); + expect(only()).toMatchObject({ calendarId: 'c', lastSyncAt: 5, lastError: 'Network request failed' }); }); it('still drops events that left the feed from its own mirror calendar', async () => { @@ -233,4 +250,113 @@ describe('subscriptions of a server that was replaced (0.16.23 to 1.0)', () => { expect(calendarApi.updateCalendar).not.toHaveBeenCalled(); expect(only().name).toBe('Public holidays'); }); + + it('keeps its name when the mirror could not be renamed, so it still prunes and removes its mirror', async () => { + vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Holidays'); + vi.mocked(calendarApi.updateCalendar).mockRejectedValueOnce(new Error('Network request failed')); + useCalendarSubscriptionsStore.setState({ subscriptions: [mine()] }); + + await expect(useCalendarSubscriptionsStore.getState().updateSubscription('mine', { name: 'Public holidays', color: '#00ff00' })) + .rejects.toThrow('Network request failed'); + expect(only()).toMatchObject({ name: 'Holidays', color: '#ff0000' }); + + // The mirror on the server is still called "Holidays". + vi.mocked(calendarApi.queryEvents).mockResolvedValue(['x1']); + vi.mocked(calendarApi.getEventIdentities).mockResolvedValue([{ id: 'x1', uid: 'gone', calendarIds: { c: true } }] as never); + await useCalendarSubscriptionsStore.getState().syncSubscription('mine'); + expect(calendarApi.deleteEvents).toHaveBeenCalledWith(['x1']); + expect(calendarApi.createCalendar).not.toHaveBeenCalled(); + await useCalendarSubscriptionsStore.getState().removeSubscription('mine'); + expect(calendarApi.deleteCalendar).toHaveBeenCalledWith('c'); + }); + + it('keeps its name when the mirror\'s name cannot be read', async () => { + vi.mocked(calendarApi.getCalendarName).mockRejectedValueOnce(new Error('Network request failed')); + useCalendarSubscriptionsStore.setState({ subscriptions: [mine()] }); + + await expect(useCalendarSubscriptionsStore.getState().updateSubscription('mine', { name: 'Public holidays' })) + .rejects.toThrow('Network request failed'); + + expect(calendarApi.updateCalendar).not.toHaveBeenCalled(); + expect(only().name).toBe('Holidays'); + }); +}); + +describe('subscriptions made before they recorded their account', () => { + // Accounts on two servers can get the same JMAP id for their own data: + // usera on one.example and userb on two.example are both "d" here. + const USERA = 'usera@one.example'; + const USERB = 'userb@two.example'; + const account = (id: string, jmapAccountId?: string) => ({ + id, serverUrl: `https://${id.split('@')[1]}`, username: id.split('@')[0], displayName: id, email: id, + avatarColor: '#000', lastLoginAt: 0, isConnected: true, hasError: false, isDefault: id === USERA, jmapAccountId, + }); + // userb's, made on two.example before subscriptions recorded their account. + const legacy: CalendarSubscription = { + id: 'sports', name: 'Sports', url: 'https://feeds.example/s.ics', calendarId: 'k', accountId: 'd', lastSyncAt: 5, lastError: null, + }; + const only = () => useCalendarSubscriptionsStore.getState().subscriptions[0]; + + beforeEach(() => { + vi.clearAllMocks(); + useCalendarSubscriptionsStore.setState({ syncing: {}, subscriptions: [{ ...legacy }] }); + globalThis.fetch = vi.fn(async () => ({ + ok: true, status: 200, headers: { get: () => null }, text: async () => 'BEGIN:VCALENDAR\r\nEND:VCALENDAR\r\n', + })) as unknown as typeof fetch; + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue([{ uid: 'e1', title: 'Final', start: '2027-01-01T00:00:00' }]); + vi.mocked(calendarApi.queryEvents).mockResolvedValue([]); + vi.mocked(calendarApi.getEventIdentities).mockResolvedValue([]); + vi.mocked(calendarApi.createCalendar).mockResolvedValue({ id: 'n1', name: 'Sports' } as never); + }); + + afterEach(() => { + useAccountStore.setState({ accounts: [] }); + }); + + it('are not taken by a replaced server\'s account when another account has the same JMAP id', async () => { + useAccountStore.setState({ accounts: [account(USERA, 'd'), account(USERB, 'd')] }); + + await useCalendarSubscriptionsStore.getState().unlinkAccount(USERA, { previous: 'd', next: 'd' }); + + expect(only()).toEqual(legacy); + }); + + it('are not taken while another account\'s JMAP id is not known yet', async () => { + useAccountStore.setState({ accounts: [account(USERA, 'd'), account(USERB)] }); + + await useCalendarSubscriptionsStore.getState().unlinkAccount(USERA, { previous: 'd', next: 'd' }); + + expect(only()).toEqual(legacy); + }); + + it('go with the replaced server\'s account when no other account has their JMAP id', async () => { + useAccountStore.setState({ accounts: [account(USERA, 'd'), account(USERB, 'e')] }); + + await useCalendarSubscriptionsStore.getState().unlinkAccount(USERA, { previous: 'd', next: 'h' }); + + expect(only()).toMatchObject({ calendarId: null, localAccountId: USERA, accountId: 'h', lastSyncAt: null }); + }); + + it('become the account\'s once a sync finds their mirror in it', async () => { + // The client serves user@mail.example.org, JMAP id "acc-1". + vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Sports'); + useCalendarSubscriptionsStore.setState({ subscriptions: [{ ...legacy, accountId: 'acc-1' }] }); + + await useCalendarSubscriptionsStore.getState().syncSubscription('sports'); + + expect(only()).toMatchObject({ calendarId: 'k', accountId: 'acc-1', localAccountId: 'user@mail.example.org', lastError: null }); + expect(calendarStore.importEvents).toHaveBeenCalledWith([expect.objectContaining({ uid: 'e1' })], 'k'); + }); + + it('are left alone by a sync that does not find their mirror when another account may own them', async () => { + useAccountStore.setState({ accounts: [account('user@mail.example.org', 'acc-1'), account(USERB, 'acc-1')] }); + vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Personal'); + useCalendarSubscriptionsStore.setState({ subscriptions: [{ ...legacy, accountId: 'acc-1' }] }); + + await useCalendarSubscriptionsStore.getState().syncSubscription('sports'); + + expect(calendarApi.createCalendar).not.toHaveBeenCalled(); + expect(calendarStore.importEvents).not.toHaveBeenCalled(); + expect(only()).toEqual({ ...legacy, accountId: 'acc-1' }); + }); }); diff --git a/src/stores/__tests__/server-change.test.ts b/src/stores/__tests__/server-change.test.ts index 34e2b4c2..cbc85791 100644 --- a/src/stores/__tests__/server-change.test.ts +++ b/src/stores/__tests__/server-change.test.ts @@ -302,15 +302,75 @@ describe('reconcileServerFingerprint', () => { expect(useToastStore.getState().toasts).toEqual([]); }); - it('only records the first server it sees for an account', async () => { - await seed(ACCOUNT); - await AsyncStorage.removeItem(`server-fingerprint:v1:${ACCOUNT}`); + describe('on the first sight of an account (no build before recorded its server)', () => { + // The update may have come after the server was replaced (0.16.23 to 1.0). + const EXPIRES_KEY = `push:subscriptionExpires:v1:${ACCOUNT}`; + const firstSight = async () => { + await seed(ACCOUNT); + await AsyncStorage.removeItem(`server-fingerprint:v1:${ACCOUNT}`); + // Recorded by the released build against the server it knew. + await AsyncStorage.setItem(EXPIRES_KEY, '2026-12-01T00:00:00.000Z'); + }; - expect(await reconcileServerFingerprint(ACCOUNT, session('1.0'))).toBe(false); + it('keeps the caches, and drops the queued changes with a notice and the push expiry (Stalwart 1.0)', async () => { + await firstSight(); - expect(useEmailStore.getState().mailboxState).toBe('s1iaq'); - expect(useOutboxStore.getState().entries).toHaveLength(2); - expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + expect(await reconcileServerFingerprint(ACCOUNT, session('1.0'))).toBe(false); + + // What the app only shows stays. + expect(useEmailStore.getState().mailboxState).toBe('s1iaq'); + expect(useCalendarSubscriptionsStore.getState().subscriptions).toEqual(SUBSCRIPTIONS); + expect(await AsyncStorage.getItem(`push:notifiedIds:v1:${ACCOUNT}`)).not.toBeNull(); + // What would act on the server by itself goes: queued and failed ops... + expect(useOutboxStore.getState().entries).toEqual([]); + expect(useOutboxStore.getState().failed).toEqual([]); + const keys = await storedKeys(); + expect(keys).not.toContain(`webmail:outbox:v1:${ACCOUNT}`); + expect(keys).not.toContain(`webmail:outbox:v1:${ACCOUNT}:failed`); + expect(useToastStore.getState().toasts.map((toast) => toast.title)).toEqual([ + '3 offline changes were discarded because the mail server changed', + ]); + // ... and the expiry that would take a missing subscription for revoked. + expect(await AsyncStorage.getItem(EXPIRES_KEY)).toBeNull(); + expect(await AsyncStorage.getItem(`push:subscriptionId:v2:${ACCOUNT}`)).toBe('a'); + expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + // Another account's queue waits for its own first sight. + expect(JSON.parse((await AsyncStorage.getItem(`webmail:outbox:v1:${OTHER}`))!)).toHaveLength(2); + }); + + it('says nothing when no changes were queued (Stalwart 0.16.23)', async () => { + await firstSight(); + await useOutboxStore.getState().clearAccount(ACCOUNT); + + await reconcileServerFingerprint(ACCOUNT, session('0.16.23')); + + expect(useToastStore.getState().toasts).toEqual([]); + expect(await AsyncStorage.getItem(EXPIRES_KEY)).toBeNull(); + }); + + it('does it again with the next session when it failed part-way', async () => { + await firstSight(); + const removeItem = vi.mocked(AsyncStorage.removeItem); + const remove = removeItem.getMockImplementation()!; + removeItem.mockImplementation(async (key: string) => { + if (key === EXPIRES_KEY) throw new Error('disk full'); + return remove(key); + }); + + try { + await expect(reconcileServerFingerprint(ACCOUNT, session('1.0'))).rejects.toThrow('disk full'); + } finally { + removeItem.mockImplementation(remove); + } + expect(useOutboxStore.getState().entries).toEqual([]); + expect(await readServerFingerprint(ACCOUNT)).toBeNull(); + + expect(await reconcileServerFingerprint(ACCOUNT, session('1.0'))).toBe(false); + expect(await AsyncStorage.getItem(EXPIRES_KEY)).toBeNull(); + expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + // Announced once. + expect(useToastStore.getState().toasts).toHaveLength(1); + }); }); it('tries again with the next session when dropping failed part-way', async () => { diff --git a/src/stores/calendar-subscriptions-store.ts b/src/stores/calendar-subscriptions-store.ts index b1db69a7..113dffb9 100644 --- a/src/stores/calendar-subscriptions-store.ts +++ b/src/stores/calendar-subscriptions-store.ts @@ -16,6 +16,8 @@ import { import { jmapClient } from '../api/jmap-client'; import { uploadBytes } from '../api/blob'; import { generateAccountId } from '../lib/account-utils'; +import { readServerPrimaryAccountId } from '../lib/server-fingerprint'; +import { useAccountStore } from './account-store'; import { useCalendarStore } from './calendar-store'; import { t } from './locale-store'; @@ -34,7 +36,8 @@ export interface CalendarSubscription { accountId?: string; /** * The signed-in account (username@host) it belongs to: JMAP account ids - * repeat across servers. Unset on subscriptions made before it was kept. + * repeat across servers. Unset on subscriptions made before it was kept, + * until a sync finds their mirror calendar in an account. */ localAccountId?: string; /** Minutes between automatic refreshes (default 60). */ @@ -72,8 +75,9 @@ interface SubscriptionsState { * its calendar ids name other calendars now, or none. The account's * subscriptions keep their feed, name, colour and interval, lose their * mirror calendar and get a new one on the next sync. Subscriptions made - * before the account was recorded on them are matched by the JMAP account - * ids the old and the new server gave it. + * before the account was recorded on them go with it when they name a JMAP + * account id the old or the new server gave it, and no other signed-in + * account has that id. */ unlinkAccount: (localAccountId: string, jmapAccountIds: { previous: string | null; next: string | null }) => Promise; } @@ -108,6 +112,7 @@ export function currentLocalAccountId(): string | null { // The mirror calendar is recognised by the name the subscription gave it. // A calendar under the recorded id with another name is not the mirror: the // id may be reused by a replaced server, or the user renamed the calendar. +// False when the server can't be asked. async function isMirrorCalendar(calendarId: string, name: string): Promise { try { return (await getCalendarName(calendarId)) === name; @@ -116,6 +121,29 @@ async function isMirrorCalendar(calendarId: string, name: string): Promise, +): Promise { + if (sub.accountId && !jmapAccountIds.includes(sub.accountId)) return false; + for (const other of useAccountStore.getState().accounts) { + if (other.id === localAccountId) continue; + const otherJmapId = (await readServerPrimaryAccountId(other.id)) ?? other.jmapAccountId ?? null; + // Not known yet (not signed in to since the id was first recorded). + if (otherJmapId === null) return false; + if (!sub.accountId || otherJmapId === sub.accountId) return false; + } + return true; +} + // webcal:// and webcals:// are just iCalendar over HTTP(S) — clients map // them to https. Credentials embedded in the URL (#275) are moved into an // Authorization header because RN's WHATWG fetch rejects userinfo in URLs. @@ -164,17 +192,18 @@ async function fetchAndParseFeed(url: string): Promise[]> return events.filter((e) => !!e.start); } -// Reconcile the local calendar with the remote feed: drop events whose UID +// Reconcile the mirror calendar with the remote feed: drop events whose UID // disappeared upstream (unlinking rather than deleting when the event also // lives in another calendar), then import the current set (importEvents -// dedupes by UID, so unchanged events are left in place). Nothing is dropped -// from a calendar that doesn't carry the subscription's name. -async function syncFeedIntoCalendar(calendarId: string, url: string, name: string): Promise { +// dedupes by UID, so unchanged events are left in place). Callers make sure +// `calendarId` is the mirror: nothing goes into, nor is dropped from, another +// calendar. One made for this sync has nothing to drop. +async function syncFeedIntoCalendar(calendarId: string, url: string, dropStale: boolean): Promise { const parsed = await fetchAndParseFeed(url); - const parsedUids = new Set(parsed.map((e) => e.uid).filter(Boolean) as string[]); - try { - if (await isMirrorCalendar(calendarId, name)) { + if (dropStale) { + const parsedUids = new Set(parsed.map((e) => e.uid).filter(Boolean) as string[]); + try { const ids = await queryEvents([calendarId], '', ''); const existing = await getEventIdentities(ids); const stale = existing.filter((e) => e.uid && !parsedUids.has(e.uid)); @@ -186,9 +215,9 @@ async function syncFeedIntoCalendar(calendarId: string, url: string, name: strin else await updateEvent(e.id, { calendarIds: others }); } if (toDelete.length > 0) await deleteEvents(toDelete); + } catch { + // Non-fatal: if we can't enumerate existing events, still import new ones. } - } catch { - // Non-fatal: if we can't enumerate existing events, still import new ones. } await useCalendarStore.getState().importEvents(parsed, calendarId); @@ -219,7 +248,7 @@ export const useCalendarSubscriptionsStore = create()( await useCalendarStore.getState().fetchCalendars(); try { set({ syncing: { ...get().syncing, [sub.id]: true } }); - await syncFeedIntoCalendar(calendar.id, sub.url, name); + await syncFeedIntoCalendar(calendar.id, sub.url, false); set({ subscriptions: [...get().subscriptions, { ...sub, lastSyncAt: Date.now() }] }); await useCalendarStore.getState().refresh(); return { ...sub, lastSyncAt: Date.now() }; @@ -240,8 +269,7 @@ export const useCalendarSubscriptionsStore = create()( updateSubscription: async (id, updates) => { const sub = get().subscriptions.find((s) => s.id === id); if (!sub) return; - const next: CalendarSubscription = { - ...sub, + const changes: Partial = { ...(updates.name !== undefined ? { name: updates.name.trim() } : {}), ...(updates.url !== undefined ? { url: updates.url.trim() } : {}), ...(updates.color !== undefined ? { color: updates.color } : {}), @@ -249,26 +277,24 @@ export const useCalendarSubscriptionsStore = create()( ? { refreshIntervalMinutes: updates.refreshIntervalMinutes } : {}), }; + const next: CalendarSubscription = { ...sub, ...changes }; // An unlinked subscription has no calendar to rename yet; its next - // sync makes one with the new name and colour. - if ( - sub.calendarId - && (next.name !== sub.name || next.color !== sub.color) - && (await isMirrorCalendar(sub.calendarId, sub.name)) - ) { - try { + // sync makes one with the new name and colour. A mirror is renamed + // first, and when that fails (or its name can't be read) nothing + // changes: a sync recognises the mirror by the subscription's name, + // and would take the old-named calendar for another one. + if (sub.calendarId && (next.name !== sub.name || next.color !== sub.color)) { + if ((await getCalendarName(sub.calendarId)) === sub.name) { // Only what changed: Stalwart 1.0 refuses a colour that is not // #rgb, #rrggbb or a CSS name, which another client may have set. await updateCalendar(sub.calendarId, { ...(next.name !== sub.name ? { name: next.name } : {}), ...(next.color !== sub.color ? { color: next.color } : {}), }); - await useCalendarStore.getState().fetchCalendars(); - } catch { - // The mirror calendar keeps its old name/colour; the sub still updates. + void useCalendarStore.getState().fetchCalendars().catch(() => undefined); } } - set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? next : s)) }); + set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? { ...s, ...changes } : s)) }); if (next.url !== sub.url) await get().syncSubscription(id); }, @@ -292,27 +318,49 @@ export const useCalendarSubscriptionsStore = create()( const sub = get().subscriptions.find((s) => s.id === id); if (!sub) return; if (get().syncing[id]) return; - // An unlinked subscription gets its new mirror in its own account only. + // Only its own account syncs a subscription, and an unlinked one gets + // its new mirror in its own account only. const localAccountId = currentLocalAccountId(); - if (!sub.calendarId && (!localAccountId || (sub.localAccountId && sub.localAccountId !== localAccountId))) return; + if (sub.localAccountId && sub.localAccountId !== localAccountId) return; + if (!sub.calendarId && !localAccountId) return; set({ syncing: { ...get().syncing, [id]: true } }); try { + const jmapAccountId = currentAccountId(); let calendarId = sub.calendarId; + // Throws when the server can't be asked; the next sync tries again. + if (calendarId && (await getCalendarName(calendarId)) !== sub.name) { + // The recorded id names another calendar, or none: the server was + // replaced unseen, or the mirror was renamed or removed elsewhere. + // The feed goes into its own mirror only: a new one, made as for + // an unlinked subscription. One made before subscriptions recorded + // their account may be another account's, with the same JMAP id. + if (!sub.localAccountId + && !(localAccountId && (await ownsLegacySubscription(sub, localAccountId, [jmapAccountId])))) { + return; + } + calendarId = null; + } else if (calendarId && !sub.localAccountId && localAccountId) { + // Its mirror is in this account, so it is this account's alone + // from now on, not every account's with the same JMAP id. + const owner = { localAccountId, accountId: sub.accountId ?? jmapAccountId ?? undefined }; + set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? { ...s, ...owner } : s)) }); + } if (!calendarId) { // As addSubscription does. Linked right away, so a feed that // fails below doesn't get another calendar on the next sync. calendarId = (await createCalendar(sub.name, sub.color)).id; - const linked = { calendarId, accountId: currentAccountId() ?? undefined, localAccountId: localAccountId ?? undefined }; + const linked = { calendarId, accountId: jmapAccountId ?? undefined, localAccountId: localAccountId ?? undefined }; set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? { ...s, ...linked } : s)) }); await useCalendarStore.getState().fetchCalendars(); } - await syncFeedIntoCalendar(calendarId, sub.url, sub.name); + const linkedBefore = calendarId === sub.calendarId; + await syncFeedIntoCalendar(calendarId, sub.url, linkedBefore); set({ subscriptions: get().subscriptions.map((s) => s.id === id ? { ...s, lastSyncAt: Date.now(), lastError: null } : s, ), }); - if (!sub.calendarId) await useCalendarStore.getState().refresh(); + if (!linkedBefore) await useCalendarStore.getState().refresh(); } catch (err) { const message = err instanceof Error ? err.message : t('calendar.subscription.refresh_error', 'Failed to refresh subscription'); set({ @@ -348,11 +396,15 @@ export const useCalendarSubscriptionsStore = create()( if (!useCalendarSubscriptionsStore.persist.hasHydrated()) { await useCalendarSubscriptionsStore.persist.rehydrate(); } - const owned = (s: CalendarSubscription) => (s.localAccountId - ? s.localAccountId === localAccountId - : !s.accountId || s.accountId === previous || s.accountId === next); + const owned = new Set(); + for (const s of get().subscriptions) { + const mine = s.localAccountId + ? s.localAccountId === localAccountId + : await ownsLegacySubscription(s, localAccountId, [previous, next]); + if (mine) owned.add(s.id); + } set({ - subscriptions: get().subscriptions.map((s) => (owned(s) + subscriptions: get().subscriptions.map((s) => (owned.has(s.id) ? { ...s, calendarId: null,