diff --git a/locales/rn/en.json b/locales/rn/en.json index 790433fb..9f81ff11 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -137,7 +137,9 @@ "account_changed": "The account changed while the subscription was syncing. It syncs again in its own account.", "add_feed": "Add a feed", "feed_too_large": "Feed is too large", + "feed_truncated": "The feed was cut off before its end. Nothing was changed.", "fetch_failed": "Could not fetch feed (HTTP {status})", + "mass_removal": "The feed left out {count} of the {total} events at once. They were kept in case the feed is broken.", "name_and_url_required": "Enter a name and a feed URL.", "not_a_feed": "That URL did not return an iCalendar feed", "not_synced": "Not synced yet", diff --git a/src/components/__tests__/calendar-default-alert-ids.test.ts b/src/components/__tests__/calendar-default-alert-ids.test.ts new file mode 100644 index 00000000..bc56bad6 --- /dev/null +++ b/src/components/__tests__/calendar-default-alert-ids.test.ts @@ -0,0 +1,30 @@ +import { describe, it, expect, vi } from 'vitest'; + +// The calendar sheet's default reminders. Alert ids must be unique in the +// account on Stalwart 1.0: an alert the lists can't show kept the id +// another client gave it, and when another calendar used that id too, the +// whole Calendar/set was refused, the new name and colour with it. + +vi.mock('../icons', () => ({ Check: () => null, Plus: () => null, X: () => null })); +vi.mock('../Button', () => ({ default: () => null })); +vi.mock('../../lib/use-layout', () => ({ useSheetInsets: () => ({}) })); +vi.mock('../../theme/colors', () => ({ useColors: () => ({}) })); + +import type { Alert } from '../../api/types'; +import { defaultAlertsToSave } from '../calendar/CalendarEditSheet'; + +// An email alert: the reminder lists show display alerts only. +const email: Alert = { '@type': 'Alert', trigger: { '@type': 'OffsetTrigger', offset: '-PT30M', relativeTo: 'start' }, action: 'email' }; + +describe('the default alerts a calendar saves', () => { + it('puts the alerts it keeps under the calendar\'s own ids too', () => { + const timed = defaultAlertsToSave({ id: 'acc-2:b', originalId: 'b' }, false, [{ minutesBefore: 10 }], { 'default-1': email }); + const allDay = defaultAlertsToSave({ id: 'b' }, true, [], { x1: email }); + + expect(timed).toEqual({ + 'cal-b-timed-1': email, + 'cal-b-timed-2': expect.objectContaining({ action: 'display' }), + }); + expect(allDay).toEqual({ 'cal-b-allday-1': email }); + }); +}); diff --git a/src/components/calendar/CalendarEditSheet.tsx b/src/components/calendar/CalendarEditSheet.tsx index 60b65a0b..eb435147 100644 --- a/src/components/calendar/CalendarEditSheet.tsx +++ b/src/components/calendar/CalendarEditSheet.tsx @@ -110,6 +110,27 @@ function DefaultRemindersEditor({ ); } +/** + * The default alerts saved for `calendar`'s timed or all-day events: the + * reminders, and the alerts the lists can't show (`kept`), every one under + * the calendar's own id prefix. Ids must be unique in the account, and a kept + * alert had the id another client gave it: when another calendar used that + * id too, Stalwart 1.0 refused the whole Calendar/set, its name and colour + * with it. + */ +export function defaultAlertsToSave( + calendar: Pick, + allDay: boolean, + reminders: Reminder[], + kept: Record, +): Record { + const prefix = defaultAlertIdPrefix(calendar, allDay); + const rekeyed: Record = {}; + let n = 0; + for (const alert of Object.values(kept)) rekeyed[`${prefix}-${++n}`] = alert; + return remindersToDefaultAlerts(reminders, rekeyed, prefix); +} + interface CalendarEditSheetProps { visible: boolean; // Existing calendar to edit; null creates a new one. @@ -161,10 +182,10 @@ export function CalendarEditSheet({ visible, calendar, onSave, onClose }: Calend const values: CalendarEditValues = { name: trimmed, color, description: description.trim() }; // Ids unique in the account: Stalwart 1.0 refuses one another calendar uses. if (calendar && editsDefaults && !sameReminders(timedReminders, initialTimed)) { - values.defaultAlertsWithTime = remindersToDefaultAlerts(timedReminders, keptTimed, defaultAlertIdPrefix(calendar, false)); + values.defaultAlertsWithTime = defaultAlertsToSave(calendar, false, timedReminders, keptTimed); } if (calendar && editsDefaults && !sameReminders(allDayReminders, initialAllDay)) { - values.defaultAlertsWithoutTime = remindersToDefaultAlerts(allDayReminders, keptAllDay, defaultAlertIdPrefix(calendar, true)); + values.defaultAlertsWithoutTime = defaultAlertsToSave(calendar, true, allDayReminders, keptAllDay); } await onSave(values); onClose(); diff --git a/src/components/calendar/EventModal.tsx b/src/components/calendar/EventModal.tsx index bce7fbba..f3041203 100644 --- a/src/components/calendar/EventModal.tsx +++ b/src/components/calendar/EventModal.tsx @@ -45,6 +45,7 @@ import { import { displayNow } from '../../lib/calendar-timezone'; import { editorTimesChanged, editorTimesFromEvent, eventTimeFieldsToSave } from '../../lib/event-editor-times'; import { useSettingsStore } from '../../stores/settings-store'; +import { CalendarAccountChangedError } from '../../stores/calendar-store'; import { canCreateEventsIn } from '../../lib/calendar-editability'; import { useCalendarLocale } from '../../lib/calendar-locale'; import { formatCalendarDate } from '../../lib/date-format'; @@ -417,6 +418,12 @@ export function EventModal({ }); onClose(); } catch (err) { + // The account changed under the save: the event is the previous + // account's, and the calendar shows another one now. + if (err instanceof CalendarAccountChangedError) { + onClose(); + return; + } // The editor stays open with everything typed, and says why. NativeAlert.alert( t('calendar.notifications.event_error', 'Failed to save event'), diff --git a/src/lib/__tests__/calendar-invitation.test.ts b/src/lib/__tests__/calendar-invitation.test.ts index c3cffb2e..a1f6520f 100644 --- a/src/lib/__tests__/calendar-invitation.test.ts +++ b/src/lib/__tests__/calendar-invitation.test.ts @@ -197,6 +197,23 @@ describe('getInvitationTrustAssessment', () => { .toBe('trusted'); }); + // A mailing list or a forwarder that rewrote the message broke the first + // signature; the sender's own one passed, and so did DMARC. + it('trusts a DMARC pass of the receiving server over a broken signature listed first', () => { + expect(getInvitationTrustAssessment(request, forged( + 'dkim=fail header.d=lists.example.net header.s=l1; dkim=pass header.d=example.com header.s=k1; spf=softfail smtp.mailfrom=lists.example.net; dmarc=pass header.from=example.com', + ), 'request')).toMatchObject({ level: 'trusted', reason: null }); + // A pass only a lower header claims is the sender's word, not the server's. + const below = email({ + from: [{ email: 'alice@example.com' }], + headers: [ + { name: 'Authentication-Results', value: 'mx.example.org; dkim=fail header.d=example.com; dmarc=fail header.from=example.com' }, + { name: 'Authentication-Results', value: 'relay.example; dmarc=pass header.from=example.com' }, + ], + }); + expect(getInvitationTrustAssessment(request, below, 'request').reason).toBe('authentication_failed'); + }); + it('warns about a forged organizer whose mail passes only for the forger\'s domain', () => { const e = email({ from: [{ email: 'mallory@forger.example' }], diff --git a/src/lib/__tests__/recurrence-scope-edit.test.ts b/src/lib/__tests__/recurrence-scope-edit.test.ts index b1c6227e..0075ec60 100644 --- a/src/lib/__tests__/recurrence-scope-edit.test.ts +++ b/src/lib/__tests__/recurrence-scope-edit.test.ts @@ -125,6 +125,25 @@ function actions(master: CalendarEvent | null) { } satisfies OccurrenceEditActions & { deleteEvent: (id: string) => Promise }; } +/** + * `deps` with the account the operation starts in, as the store's + * calendarOperationScope gives it: after `switchAccount` its check throws. + */ +function scoped(deps: D) { + let changed = false; + return { + deps: { + ...deps, + operationScope: () => ({ + assert: () => { + if (changed) throw new Error('The account changed'); + }, + }), + }, + switchAccount: () => { changed = true; }, + }; +} + beforeEach(() => useSettingsStore.setState({ calendarTimeZone: 'Europe/Berlin' })); afterEach(() => useSettingsStore.setState({ calendarTimeZone: 'auto' })); @@ -864,6 +883,39 @@ describe.each(occurrences)('saving this and the following occurrences on Stalwar expect(statusOf(created.participants, ORGANIZER)).toBe('accepted'); expect(statusOf(created.participants, ATTENDEE)).toBe('needs-action'); }); + + // The store writes to the account the client serves when a write begins: + // the rest of the series went to the next account's calendar of the same + // id, and invited its attendees. + it('makes no rest of the series once the account changed after ending it', async () => { + const occurrence = make(); + const { deps, switchAccount } = scoped(actions(series(version))); + deps.updateEvent.mockImplementationOnce(async () => { switchAccount(); return undefined; }); + + await expect(saveOccurrenceEdit('this_and_future', { + event: occurrence, updates: editorUpdates(occurrence), editor: editorOn(occurrence), + calendarId: 'cal', sendScheduling: true, userEmails: [ORGANIZER], + }, deps)).rejects.toThrow('account changed'); + + expect(deps.updateEvent).toHaveBeenCalledTimes(1); + expect(deps.createEvent).not.toHaveBeenCalled(); + }); + + it('puts nothing back in the next account when the account changed while making the rest', async () => { + const occurrence = make(); + const { deps, switchAccount } = scoped(actions(series(version))); + deps.createEvent.mockImplementationOnce(async () => { + switchAccount(); + throw new Error('The account changed while the calendar was saving.'); + }); + + await expect(saveOccurrenceEdit('this_and_future', { + event: occurrence, updates: editorUpdates(occurrence), editor: editorOn(occurrence), sendScheduling: true, userEmails: [ORGANIZER], + }, deps)).rejects.toThrow('account changed'); + + // The truncation alone, no rollback. + expect(deps.updateEvent).toHaveBeenCalledTimes(1); + }); }); describe('the new series from this and the following occurrences', () => { @@ -1156,6 +1208,31 @@ describe.each<[string, Version]>([['1.0', '1.0'], ['0.16', '0.16']])('the series expect(statusOf(change['recurrenceOverrides/2026-10-05T10:00:00'].participants as CalendarEvent['participants'], ATTENDEE)).toBe('accepted'); }); + it('sends no second step once the account changed after the first', async () => { + const master = base(); + const { deps, switchAccount } = scoped(actions(master)); + deps.updateEvent.mockImplementationOnce(async () => { switchAccount(); return undefined; }); + + await expect(saveOccurrenceEdit('this', { + event: master, updates: kickOff(master), sendScheduling: true, userEmails: [ORGANIZER], + }, deps)).rejects.toThrow('account changed'); + + expect(deps.updateEvent).toHaveBeenCalledTimes(1); + expect(deps.updateEvent.mock.calls[0][2]).toEqual({ sendSchedulingMessages: false }); + }); + + it('excludes nothing once the account changed while the series was read', async () => { + const master = base(); + const { deps, switchAccount } = scoped(actions(master)); + deps.getMasterEvent.mockImplementationOnce(async () => { switchAccount(); return master; }); + + await expect(deleteOccurrence('this', { event: master, userEmails: [ORGANIZER] }, deps)).rejects.toThrow('account changed'); + await expect(deleteOccurrence('this_and_future', { event: master, userEmails: [ORGANIZER] }, deps)).rejects.toThrow('account changed'); + + expect(deps.updateEvent).not.toHaveBeenCalled(); + expect(deps.deleteEvent).not.toHaveBeenCalled(); + }); + it('changes it in one write when nobody has answered, or nothing is sent', async () => { const unanswered = base({ participants: { org: participant(version, ORGANIZER, true, 'accepted'), b: participant(version, ATTENDEE, false, 'needs-action') }, diff --git a/src/lib/calendar-invitation.ts b/src/lib/calendar-invitation.ts index 41241a7c..c800ed2d 100644 --- a/src/lib/calendar-invitation.ts +++ b/src/lib/calendar-invitation.ts @@ -483,6 +483,10 @@ export function hasAlignedAuthenticationPass( } function hasAuthenticationFailure(auth?: AuthenticationResults | null): boolean { + // DMARC passed on the receiving server's own header: one aligned method + // passed, and that is the verdict. A mailing list's or a forwarder's + // signature that broke on the way, listed first, flagged such a message. + if (auth?.dmarc?.result === 'pass') return false; return Boolean( auth?.dmarc?.result === 'fail' || auth?.dkim?.result === 'fail' diff --git a/src/lib/recurrence-scope-edit.ts b/src/lib/recurrence-scope-edit.ts index 42433cb0..4bab7c73 100644 --- a/src/lib/recurrence-scope-edit.ts +++ b/src/lib/recurrence-scope-edit.ts @@ -581,6 +581,39 @@ export interface OccurrenceEditActions { ) => Promise; /** The base event of a series, from one of its occurrences. */ getMasterEvent: (occurrence: CalendarEvent) => Promise; + /** + * The account the edit is made in, taken once when it starts (the store's + * calendarOperationScope); `assert` throws once the client serves another. + * Each step is a write of its own: after an account switch the next one + * went to the next account with this one's ids - the rest of a split + * series made in its calendar of the same id, mailed to its attendees. + */ + operationScope?: () => { assert: () => void }; +} + +/** + * `actions` with the account checked before every step, against one scope + * for the whole operation: the store writes to the account the client serves + * when a write begins, and a rollback after the switch would undo nothing of + * this account. Steps handed on (a this-and-future edit from the first + * occurrence saves as 'all') keep that scope. + */ +function inOneScope>(actions: A): A { + const scope = actions.operationScope?.(); + if (!scope) return actions; + const checked = Promise>(step: F | undefined) => + step && (((...args: Parameters) => { + scope.assert(); + return step(...args); + }) as unknown as F); + return { + ...actions, + operationScope: () => scope, + updateEvent: checked(actions.updateEvent), + createEvent: checked(actions.createEvent), + deleteEvent: checked(actions.deleteEvent), + getMasterEvent: checked(actions.getMasterEvent), + }; } export interface OccurrenceEdit { @@ -923,8 +956,9 @@ export function followingSeriesData( export async function saveOccurrenceEdit( scope: RecurrenceEditScope, edit: OccurrenceEdit, - actions: OccurrenceEditActions, + unscoped: OccurrenceEditActions, ): Promise { + const actions = inOneScope(unscoped); const { updateEvent, createEvent } = actions; const { event, updates, sendScheduling } = edit; const scheduling = { sendSchedulingMessages: sendScheduling }; @@ -1009,7 +1043,7 @@ export async function saveOccurrenceEdit( } /** What deleting part of a series works with: the calendar store's actions. */ -export interface OccurrenceDeleteActions extends Pick { +export interface OccurrenceDeleteActions extends Pick { deleteEvent: (id: string) => Promise; } @@ -1022,8 +1056,9 @@ export interface OccurrenceDeleteActions extends Pick { + const actions = inOneScope(unscoped); const { event } = deletion; if (scope === 'this') { if (isServerRecurrenceInstance(event) || isBrowserExpandedOccurrence(event)) { diff --git a/src/screens/CalendarScreen.tsx b/src/screens/CalendarScreen.tsx index 6ed8d41b..51c5c672 100644 --- a/src/screens/CalendarScreen.tsx +++ b/src/screens/CalendarScreen.tsx @@ -40,7 +40,7 @@ import { useCalendarLocale } from '../lib/calendar-locale'; import { displayNow } from '../lib/calendar-timezone'; import { spacing, radius, typography, type ThemePalette } from '../theme/tokens'; import { useColors } from '../theme/colors'; -import { useCalendarStore } from '../stores/calendar-store'; +import { CalendarAccountChangedError, calendarOperationScope, useCalendarStore } from '../stores/calendar-store'; import { useSettingsStore } from '../stores/settings-store'; import { MonthView } from '../components/calendar/MonthView'; import { MonthScrollView } from '../components/calendar/MonthScrollView'; @@ -635,6 +635,9 @@ export default function CalendarScreen() { const reportError = React.useCallback( (err: unknown) => { + // The account changed under the write: nothing failed in the account + // shown now, and the previous one shows what got in when it is back. + if (err instanceof CalendarAccountChangedError) return; Alert.alert( t('calendar.notifications.event_error', 'Something went wrong'), err instanceof Error ? err.message : undefined, @@ -724,12 +727,14 @@ export default function CalendarScreen() { sendScheduling: action.sendScheduling, userEmails: currentUserEmails, resolveName: resolveContactName, - }, { updateEvent, createEvent, getMasterEvent }) + }, { updateEvent, createEvent, getMasterEvent, operationScope: calendarOperationScope }) : await deleteOccurrence(scope, { event, userEmails: currentUserEmails }, { - updateEvent, deleteEvent, getMasterEvent, + updateEvent, deleteEvent, getMasterEvent, operationScope: calendarOperationScope, }); if (!done) throw new Error('Master event not found'); } catch (err) { + // The account changed under the edit: it stopped there (see reportError). + if (err instanceof CalendarAccountChangedError) return; Alert.alert( t('calendar.notifications.event_error', 'Something went wrong'), participantLimitMessage(err, t) ?? (err instanceof Error ? err.message : undefined), diff --git a/src/screens/__tests__/calendar-account-change-silent.test.ts b/src/screens/__tests__/calendar-account-change-silent.test.ts new file mode 100644 index 00000000..c1280b62 --- /dev/null +++ b/src/screens/__tests__/calendar-account-change-silent.test.ts @@ -0,0 +1,41 @@ +// A calendar write the account switch overtook throws +// CalendarAccountChangedError: the write stopped so that nothing reaches the +// next account. Nothing failed in the account shown now, and the alert +// "Something went wrong" came up over it. The places that show a calendar +// write's error pass it over in silence. +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const SRC = join(__dirname, '../..'); + +/** The text of `file` from `marker` to the first `Alert.alert(` after it. */ +function beforeAlert(file: string, marker: string): string { + const src = readFileSync(join(SRC, file), 'utf8'); + const start = src.indexOf(marker); + expect(start).toBeGreaterThanOrEqual(0); + const alert = src.indexOf('Alert.alert(', start); + expect(alert).toBeGreaterThan(start); + return src.slice(start, alert); +} + +const SILENT = /if \(err instanceof CalendarAccountChangedError\)/; + +describe('a calendar write the account switch stopped shows no error', () => { + it('CalendarScreen reportError', () => { + expect(beforeAlert('screens/CalendarScreen.tsx', 'const reportError = React.useCallback(')).toMatch(SILENT); + }); + + it('CalendarScreen saving or deleting part of a series', () => { + expect(beforeAlert('screens/CalendarScreen.tsx', "if (!done) throw new Error('Master event not found');")).toMatch(SILENT); + }); + + it('EventModal save', () => { + expect(beforeAlert('components/calendar/EventModal.tsx', 'await onSave(data, calendarId, {')).toMatch(SILENT); + }); + + it('CalendarScreen gives the scope edits one account for the whole operation', () => { + const src = readFileSync(join(SRC, 'screens/CalendarScreen.tsx'), 'utf8'); + expect(src.match(/operationScope: calendarOperationScope/g)).toHaveLength(2); + }); +}); diff --git a/src/stores/__tests__/calendar-store-account-switch.test.ts b/src/stores/__tests__/calendar-store-account-switch.test.ts index 9d46bc84..9881d8ef 100644 --- a/src/stores/__tests__/calendar-store-account-switch.test.ts +++ b/src/stores/__tests__/calendar-store-account-switch.test.ts @@ -99,7 +99,10 @@ describe('a write the account switch overtook', () => { it('names the account it started in and leaves the next account\'s event alone (updateEvent)', async () => { mockUpdate.mockImplementationOnce(async () => switchAccount()); - await useCalendarStore.getState().updateEvent('c', { title: 'Renamed' }); + // Fails, so that a caller with more steps (lib/recurrence-scope-edit) + // sends no next one to the next account. + await expect(useCalendarStore.getState().updateEvent('c', { title: 'Renamed' })) + .rejects.toBeInstanceOf(CalendarAccountChangedError); expect(mockUpdate).toHaveBeenCalledWith('c', { title: 'Renamed' }, undefined, 'acc-1'); expect(useCalendarStore.getState().events).toEqual([THEIRS]); @@ -108,7 +111,7 @@ describe('a write the account switch overtook', () => { it('leaves the next account\'s event of the same id in the store (deleteEvent)', async () => { mockDelete.mockImplementationOnce(async () => switchAccount()); - await useCalendarStore.getState().deleteEvent('c'); + await expect(useCalendarStore.getState().deleteEvent('c')).rejects.toBeInstanceOf(CalendarAccountChangedError); expect(mockDelete).toHaveBeenCalledWith(['c'], undefined, 'acc-1'); expect(useCalendarStore.getState().events).toEqual([THEIRS]); @@ -120,7 +123,7 @@ describe('a write the account switch overtook', () => { return event('n', 'New'); }); - await useCalendarStore.getState().createEvent({ title: 'New' }, 'b'); + await expect(useCalendarStore.getState().createEvent({ title: 'New' }, 'b')).rejects.toBeInstanceOf(CalendarAccountChangedError); expect(mockCreate).toHaveBeenCalledWith({ title: 'New' }, 'b', undefined, 'acc-1'); expect(mockGetEvents).not.toHaveBeenCalled(); diff --git a/src/stores/__tests__/calendar-store-series-edits.test.ts b/src/stores/__tests__/calendar-store-series-edits.test.ts index 32746a22..af775e66 100644 --- a/src/stores/__tests__/calendar-store-series-edits.test.ts +++ b/src/stores/__tests__/calendar-store-series-edits.test.ts @@ -36,7 +36,8 @@ vi.mock('../../api/jmap-client', () => ({ import * as calendarApi from '../../api/calendar'; import type { CalendarEvent } from '../../api/types'; -import { useCalendarStore } from '../calendar-store'; +import { jmapClient } from '../../api/jmap-client'; +import { CalendarAccountChangedError, calendarOperationScope, useCalendarStore } from '../calendar-store'; import { saveOccurrenceEdit } from '../../lib/recurrence-scope-edit'; import { editorTimesFromEvent } from '../../lib/event-editor-times'; @@ -98,6 +99,39 @@ describe('this and the following occurrences on the calendar store (Stalwart 1.0 expect([calendarId, schedule]).toEqual(['cal-1', true]); expect(created).toMatchObject({ start: '2026-10-19T10:00:00', recurrenceRules: [{ '@type': 'RecurrenceRule', frequency: 'weekly', count: 3 }] }); }); + + // The account changes while the store reloads after ending the series: + // that write is done, and the next one took the account the client served + // by then - the rest of the series went to acc-2's calendar of the same id. + it('makes no rest of the series in the account the client serves after a switch', async () => { + const store = useCalendarStore.getState(); + const refresh = store.refresh; + useCalendarStore.setState({ + refresh: async () => { + useCalendarStore.getState().reset(); + (jmapClient as { accountId: string }).accountId = 'acc-2'; + }, + }); + try { + await expect(saveOccurrenceEdit('this_and_future', { + event: occurrence, + updates: { title: 'Planning, room 2', start: occurrence.start, duration: 'PT1H', timeZone: 'Europe/Berlin', participants }, + editor: { attendees: [{ name: '', email: 'userb@example.org' }], when: editorTimesFromEvent(occurrence) }, + calendarId: 'cal-1', sendScheduling: true, userEmails: [ORGANIZER], + }, { + updateEvent: store.updateEvent, createEvent: store.createEvent, getMasterEvent: store.getMasterEvent, + operationScope: calendarOperationScope, + })).rejects.toBeInstanceOf(CalendarAccountChangedError); + } finally { + (jmapClient as { accountId: string }).accountId = 'acc-1'; + useCalendarStore.setState({ refresh }); + } + + // The truncation in acc-1, and nothing after it: no new series, no rollback. + expect(mockUpdate).toHaveBeenCalledTimes(1); + expect(mockUpdate.mock.calls[0][3]).toBe('acc-1'); + expect(mockCreate).not.toHaveBeenCalled(); + }); }); // The series itself, opened from a reminder or a link: not in the grid, and diff --git a/src/stores/__tests__/calendar-subscriptions-store.test.ts b/src/stores/__tests__/calendar-subscriptions-store.test.ts index 6eb7f5e3..3d6b8dac 100644 --- a/src/stores/__tests__/calendar-subscriptions-store.test.ts +++ b/src/stores/__tests__/calendar-subscriptions-store.test.ts @@ -23,9 +23,17 @@ const calendarStore = vi.hoisted(() => ({ fetchCalendars: vi.fn(async () => undefined), refresh: vi.fn(async () => undefined), })); +const { CalendarAccountChangedError } = vi.hoisted(() => ({ + CalendarAccountChangedError: class CalendarAccountChangedError extends Error { + constructor() { + super('The account changed while the calendar was loading or saving.'); + } + }, +})); vi.mock('../calendar-store', () => ({ useCalendarStore: { getState: () => calendarStore }, calendarStoreGeneration: () => calendarStore.generation, + CalendarAccountChangedError, })); vi.mock('@react-native-async-storage/async-storage', () => ({ default: { @@ -447,4 +455,86 @@ describe('refreshing a subscription', () => { expect(calendarApi.updateEvent).not.toHaveBeenCalled(); expect(calendarStore.importEvents).not.toHaveBeenCalled(); }); + + it('counts the calendar store stopping for an account switch as no failure of the subscription', async () => { + calendarStore.importEvents.mockRejectedValueOnce(new CalendarAccountChangedError()); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarStore.importEvents).toHaveBeenCalledTimes(1); + expect(only()).toMatchObject({ lastSyncAt: 5, lastError: null }); + }); + + // A body cut off mid-VEVENT parses up to the cut: every mirror event after + // it looked gone upstream and was deleted. + it('changes nothing when the feed was cut off before its end', async () => { + vi.mocked(globalThis.fetch).mockResolvedValueOnce({ + ok: true, status: 200, headers: { get: () => null }, + text: async () => 'BEGIN:VCALENDAR\r\nBEGIN:VEVENT\r\nUID:kept\r\nDTSTART:20270102T100000Z\r\nEND:VEVENT\r\nBEGIN:VEVENT\r\nUID:sa', + } as unknown as Response); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarApi.parseCalendarBlob).not.toHaveBeenCalled(); + expect(calendarApi.deleteEvents).not.toHaveBeenCalled(); + expect(calendarStore.importEvents).not.toHaveBeenCalled(); + expect(only().lastError).toMatch(/cut off/); + expect(only().lastSyncAt).toBe(5); + }); + + describe('a feed that leaves out much of a large mirror at once', () => { + const mirror = Array.from({ length: 10 }, (_, i) => ({ id: `m${i}`, uid: `u${i}`, calendarIds: { b: true } })); + const feedOf = (count: number) => mirror.slice(0, count).map((e) => ({ uid: e.uid, title: e.uid, start: '2027-01-02T10:00:00' })); + + beforeEach(() => { + vi.mocked(calendarApi.queryEvents).mockResolvedValue(mirror.map((e) => e.id)); + vi.mocked(calendarApi.getEventIdentities).mockResolvedValue(mirror as never); + }); + + it('keeps the events and says so when more than half of them left the feed', async () => { + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue(feedOf(4) as never); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarApi.deleteEvents).not.toHaveBeenCalled(); + expect(calendarApi.updateEvent).not.toHaveBeenCalled(); + expect(only().lastError).toMatch(/6 of the 10/); + }); + + it('drops them when half of them left', async () => { + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue(feedOf(5) as never); + + await useCalendarSubscriptionsStore.getState().syncSubscription('feed'); + + expect(calendarApi.deleteEvents).toHaveBeenCalledWith(['m5', 'm6', 'm7', 'm8', 'm9'], undefined, 'acc-1'); + expect(only().lastError).toBeNull(); + }); + }); +}); + +describe('adding a subscription', () => { + beforeEach(() => { + vi.clearAllMocks(); + client.accountId = 'acc-1'; + calendarStore.generation = 0; + useCalendarSubscriptionsStore.setState({ syncing: {}, subscriptions: [] }); + 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.createCalendar).mockResolvedValue({ id: 'n1', name: 'Holidays' } as never); + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue([{ uid: 'e1', title: 'New year', start: '2027-01-01T00:00:00' }] as never); + }); + + // The mirror is in the account the add started in: removing it then would + // reach the next account's calendar of the same id. + it('keeps the subscription and its mirror when the calendar store stopped for an account switch', async () => { + calendarStore.importEvents.mockRejectedValueOnce(new CalendarAccountChangedError()); + + await expect(useCalendarSubscriptionsStore.getState().addSubscription({ + name: 'Holidays', url: 'https://feeds.example/holidays.ics', + })).rejects.toBeInstanceOf(CalendarAccountChangedError); + + expect(calendarApi.deleteCalendar).not.toHaveBeenCalled(); + expect(useCalendarSubscriptionsStore.getState().subscriptions).toMatchObject([{ calendarId: 'n1', accountId: 'acc-1', lastError: null }]); + }); }); diff --git a/src/stores/calendar-store.ts b/src/stores/calendar-store.ts index 4be8ade0..cfedee56 100644 --- a/src/stores/calendar-store.ts +++ b/src/stores/calendar-store.ts @@ -361,6 +361,19 @@ function assertInScope(scope: WriteScope): void { if (!inScope(scope)) throw new CalendarAccountChangedError(); } +/** + * The account an operation of several store writes runs in (the scope edits + * in lib/recurrence-scope-edit), taken once when it starts. Each write takes + * the account the client serves when it begins, so a step after an account + * switch went to the next account with the ids of this one. `assert` throws + * CalendarAccountChangedError once the store was reset or the client serves + * another account. + */ +export function calendarOperationScope(): { assert: () => void } { + const scope = writeScope(); + return { assert: () => assertInScope(scope) }; +} + /** * The JMAP account a write goes to, named on every request: a shared * calendar's owner, else the primary account the write started in. @@ -1072,8 +1085,9 @@ export const useCalendarStore = create()( schedule, writeAccount(scope, accountId), ); - // Made; the store shows another account by now. - if (!inScope(scope)) return created; + // Made; the store shows another account by now. Throws so that a caller + // with more to do stops here, and opens nothing of the previous account. + assertInScope(scope); // The /set echo lacks server-computed properties (utcStart/utcEnd, the // normalised recurrence rule); re-read the event so it renders at the // right instant, then expand a recurring series across the loaded range @@ -1085,7 +1099,7 @@ export const useCalendarStore = create()( } catch { // Keep the echo; the next refresh reconciles. } - if (!inScope(scope)) return full; + assertInScope(scope); // Map the same way fetchEvents does so the optimistic insert doesn't // collide with the user's own events and stays visible under the right // calendar filter. @@ -1179,8 +1193,9 @@ export const useCalendarStore = create()( } else { await updateBaseEvent(realId, patch, schedule, account, scope); } - // Saved; the store shows another account by now, whose events can carry the same ids. - if (!inScope(scope)) return; + // Saved; the store shows another account by now, whose events can carry + // the same ids. Throws so that a caller with more steps sends no next one. + assertInScope(scope); set({ events: get().events.map((e) => (e.id === id ? { ...e, ...(changes as Partial) } : e)), }); @@ -1206,16 +1221,17 @@ export const useCalendarStore = create()( const schedule = hasSchedulingParticipants(storeEvent) ? true : undefined; if (target.isOccurrence && storeEvent) { await destroyOccurrence(storeEvent, realId, schedule, writeAccount(scope, accountId), scope); - if (!inScope(scope)) return; + assertInScope(scope); set({ events: get().events.filter((e) => e.id !== id) }); } else if (target.isBrowserOccurrence) { await destroyBrowserOccurrence(target, schedule, scope); - if (!inScope(scope)) return; + assertInScope(scope); set({ events: get().events.filter((e) => e.id !== id) }); } else { await apiDeleteEvents([realId], schedule, writeAccount(scope, accountId)); - // Deleted; the store shows another account by now, whose events can carry the same ids. - if (!inScope(scope)) return; + // Deleted; the store shows another account by now, whose events can + // carry the same ids. Throws as updateEvent does. + assertInScope(scope); knownMasters.delete(id); // Destroying a master removes every expanded occurrence of it, not // just the tapped one. diff --git a/src/stores/calendar-subscriptions-store.ts b/src/stores/calendar-subscriptions-store.ts index fad85615..f5cc47b3 100644 --- a/src/stores/calendar-subscriptions-store.ts +++ b/src/stores/calendar-subscriptions-store.ts @@ -20,7 +20,7 @@ import { generateAccountId } from '../lib/account-utils'; import { feedChanged, feedFields } from '../lib/calendar-feed-diff'; import { readServerPrimaryAccountId } from '../lib/server-fingerprint'; import { useAccountStore } from './account-store'; -import { calendarStoreGeneration, useCalendarStore } from './calendar-store'; +import { CalendarAccountChangedError, calendarStoreGeneration, useCalendarStore } from './calendar-store'; import { t } from './locale-store'; export interface CalendarSubscription { @@ -53,6 +53,9 @@ export const DEFAULT_REFRESH_INTERVAL_MINUTES = 60; // 10 MB is far beyond any real feed; a bigger response is not a calendar. const MAX_FEED_BYTES = 10 * 1024 * 1024; +// A sync removes at most half of a mirror this size or larger at once. +const MASS_REMOVAL_MIN_EVENTS = 10; + interface SubscriptionsState { subscriptions: CalendarSubscription[]; syncing: Record; @@ -205,6 +208,14 @@ class SubscriptionAccountChangedError extends Error { } } +/** + * Whether `err` says the account changed under the sync: its own check, or + * the calendar store's (importEvents stops so after an account switch). + */ +function accountChanged(err: unknown): boolean { + return err instanceof SubscriptionAccountChangedError || err instanceof CalendarAccountChangedError; +} + function assertScope(scope: SyncScope): void { if (currentAccountId() !== scope.accountId || calendarStoreGeneration() !== scope.generation) { throw new SubscriptionAccountChangedError(); @@ -225,6 +236,11 @@ async function fetchAndParseFeed(url: string, scope: SyncScope): Promise { +): Promise { const { accountId } = scope; const parsed = await fetchAndParseFeed(url, scope); let toImport = parsed; let wrote = false; + let warning: string | null = null; if (linkedBefore) { const parsedUids = new Set(parsed.map((e) => e.uid).filter(Boolean) as string[]); @@ -269,7 +288,18 @@ async function syncFeedIntoCalendar( const inMirror = new Set(existing.map((e) => e.uid).filter(Boolean) as string[]); toImport = parsed.filter((e) => !e.uid || !inMirror.has(e.uid)); - const stale = existing.filter((e) => e.uid && !parsedUids.has(e.uid)); + let stale = existing.filter((e) => e.uid && !parsedUids.has(e.uid)); + // A feed that drops most of a sizeable mirror at once is more likely + // broken (an error page that still parses, a server's partial answer) + // than emptied: the events stay, and the sync says so. + if (existing.length >= MASS_REMOVAL_MIN_EVENTS && stale.length > existing.length / 2) { + warning = t( + 'calendar.subscription.mass_removal', + 'The feed left out {count} of the {total} events at once. They were kept in case the feed is broken.', + { count: stale.length, total: existing.length }, + ); + stale = []; + } const toDelete: string[] = []; for (const e of stale) { const others = { ...(e.calendarIds || {}) }; @@ -308,7 +338,7 @@ async function syncFeedIntoCalendar( } } } catch (err) { - if (err instanceof SubscriptionAccountChangedError) throw err; + if (accountChanged(err)) throw err; // Non-fatal: if we can't enumerate existing events, still import new ones. } } @@ -317,6 +347,7 @@ async function syncFeedIntoCalendar( // importEvents shows what it brought in; deletions and changes alone need a reload. if (toImport.length > 0) await useCalendarStore.getState().importEvents(toImport, calendarId); else if (wrote) await useCalendarStore.getState().refresh(); + return warning; } export const useCalendarSubscriptionsStore = create()( @@ -353,7 +384,7 @@ export const useCalendarSubscriptionsStore = create()( await useCalendarStore.getState().refresh(); return { ...sub, lastSyncAt: Date.now() }; } catch (err) { - if (err instanceof SubscriptionAccountChangedError) { + if (accountChanged(err)) { // The mirror is in the account the sync started in, which the // client no longer serves: a request to remove it would reach // the next account. Kept as that account's subscription, it @@ -473,17 +504,17 @@ export const useCalendarSubscriptionsStore = create()( await useCalendarStore.getState().fetchCalendars(); } const linkedBefore = calendarId === sub.calendarId; - await syncFeedIntoCalendar(calendarId, sub.url, linkedBefore, scope); + const warning = await syncFeedIntoCalendar(calendarId, sub.url, linkedBefore, scope); set({ subscriptions: get().subscriptions.map((s) => - s.id === id ? { ...s, lastSyncAt: Date.now(), lastError: null } : s, + s.id === id ? { ...s, lastSyncAt: Date.now(), lastError: warning } : s, ), }); if (!linkedBefore) await useCalendarStore.getState().refresh(); } catch (err) { // Not this subscription's failure: the client serves another // account now, and the sync runs again in its own. - if (err instanceof SubscriptionAccountChangedError) return; + if (accountChanged(err)) return; const message = err instanceof Error ? err.message : t('calendar.subscription.refresh_error', 'Failed to refresh subscription'); set({ subscriptions: get().subscriptions.map((s) =>