diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/PushRouting.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/PushRouting.kt index 3bdcca1b..70c0b97d 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/PushRouting.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/sync/PushRouting.kt @@ -13,7 +13,7 @@ import org.json.JSONObject * The relay forwards the JMAP StateChange's `changed` map verbatim (FCM data * key `changed`: JMAP account id → type → state; only its first account is * repeated as `accountId`), so every account key is looked at. The mail task - * runs only for mail: when some account's map has `EmailDelivery`, for an + * runs only for mail: when some account's map has a mail type, for an * EmailPush, or when `changed` is missing or unreadable (older relays), so a * contact change never ends in a mail notification. */ @@ -28,7 +28,15 @@ object PushRouting { ) private const val EMAIL_PUSH_KIND = "jmap-email-push" - private const val EMAIL_DELIVERY = "EmailDelivery" + + /** + * Types that may mean new mail: `EmailDelivery`, and `Email`/`Mailbox`, + * which new mail changes too. A subscription made by an older build + * listens to them until the launch-time resync replaces its types, and a + * server without `EmailDelivery` has only them. Keep in sync with + * MAIL_TYPES in src/lib/push-background-task.ts. + */ + private val MAIL_TYPES = setOf("EmailDelivery", "Email", "Mailbox") private val CONTACT_TYPES = setOf("ContactCard", "AddressBook") private val CALENDAR_TYPES = setOf("CalendarEvent", "Calendar") @@ -42,7 +50,7 @@ object PushRouting { mail = true continue } - if (EMAIL_DELIVERY in types) mail = true + if (types.any { it in MAIL_TYPES }) mail = true val contacts = types.any { it in CONTACT_TYPES } val calendar = types.any { it in CALENDAR_TYPES } for (route in routes[jmapAccountId].orEmpty()) { diff --git a/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/PushRoutingTest.kt b/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/PushRoutingTest.kt index 1a13656d..8e608022 100644 --- a/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/PushRoutingTest.kt +++ b/android/app/src/test/java/com/anonymous/bulwarkmobile/sync/PushRoutingTest.kt @@ -64,6 +64,26 @@ class PushRoutingTest { assertEquals(setOf("alice-work" to calendar), decision.syncs) } + @Test + fun `an Email or Mailbox change is possible mail, as a subscription from an older build sends it`() { + for (changed in listOf( + """{"c":{"Email":"s1","Mailbox":"s2"}}""", + """{"c":{"Email":"s1"}}""", + """{"c":{"Mailbox":"s1"}}""", + )) { + val decision = PushRouting.decide(stateChange(changed), routes) + assertTrue("changed=$changed", decision.startMailTask) + assertTrue("changed=$changed", decision.syncs.isEmpty()) + } + val mixed = PushRouting.decide(stateChange("""{"team":{"CalendarEvent":"e1"},"c":{"Email":"s1"}}"""), routes) + assertTrue(mixed.startMailTask) + assertEquals(setOf("alice-work" to calendar), mixed.syncs) + // Types that are neither mail nor device sync say nothing about mail. + val other = PushRouting.decide(stateChange("""{"c":{"Thread":"t1","ContactCard":"s1"}}"""), routes) + assertFalse(other.startMailTask) + assertEquals(setOf("alice@example.org" to contacts), other.syncs) + } + @Test fun `an email push always starts the mail task`() { val data = mapOf("kind" to "jmap-email-push", "accountId" to "c", "changed" to """{"c":{"EmailDelivery":"s"}}""") diff --git a/src/lib/__tests__/push-background-task.test.ts b/src/lib/__tests__/push-background-task.test.ts index c8984ad9..8f1f0dae 100644 --- a/src/lib/__tests__/push-background-task.test.ts +++ b/src/lib/__tests__/push-background-task.test.ts @@ -237,6 +237,42 @@ describe('pushes for device sync (#34)', () => { await pushBackgroundTask(stateChange({ 'jmap-primary': { EmailDelivery: 'm1', ContactCard: 's1' } })); expect(jmapClient.getStoredCredentials).toHaveBeenCalledWith(LOCAL); }); + + it('takes an Email or Mailbox change for possible mail, as a subscription from an older build sends it', () => { + expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Email: 's1', Mailbox: 's2' } })))).toBe(false); + expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Email: 's1' } })))).toBe(false); + expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Mailbox: 's1' } })))).toBe(false); + expect(carriesNoMail(parseRelayPushData(stateChange({ a: { CalendarEvent: 'e1' }, b: { Email: 's1' } })))).toBe(false); + // Types that are neither mail nor device sync say nothing about mail. + expect(carriesNoMail(parseRelayPushData(stateChange({ a: { Thread: 't1', ContactCard: 's1' } })))).toBe(true); + }); + + it('notifies new mail from a subscription that listens to Email and Mailbox', async () => { + (secureFetch as ReturnType).mockImplementation(async (url: string, init?: { body?: string }) => { + if (url.endsWith('/.well-known/jmap')) { + return { + ok: true, + json: async () => ({ + apiUrl: 'https://mail.example.com/jmap/', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' }, + accounts: { 'jmap-primary': {} }, + }), + }; + } + // The legacy path: the inbox, its newest unread ids, then the messages. + const [[name]] = JSON.parse(init?.body ?? '{}').methodCalls; + const body = name === 'Mailbox/get' + ? { list: [{ id: 'inbox', role: 'inbox' }] } + : name === 'Email/query' + ? { ids: ['m1'] } + : { list: [{ id: 'm1', threadId: 't1', keywords: {}, subject: 'Hi', from: [{ email: 'bob@example.com' }] }] }; + return { ok: true, json: async () => ({ methodResponses: [[name, body, '0']] }) }; + }); + + await pushBackgroundTask(stateChange({ 'jmap-primary': { Email: 's1', Mailbox: 's2' } })); + + expect(showNotification).toHaveBeenCalledWith(expect.objectContaining({ emailId: 'm1', accountId: LOCAL })); + }); }); describe('selectNotifiableEmails', () => { diff --git a/src/lib/push-background-task.ts b/src/lib/push-background-task.ts index 776b39c4..2c703f8e 100644 --- a/src/lib/push-background-task.ts +++ b/src/lib/push-background-task.ts @@ -378,18 +378,25 @@ async function detachedNewestUnreadInboxIds( return (qBody.ids as string[]) ?? []; } +// Types that may mean new mail: `EmailDelivery`, and `Email`/`Mailbox`, which +// new mail changes too. A subscription made by an older build listens to them +// until the launch-time resync replaces its types, and a server without +// `EmailDelivery` has only them. Keep in sync with MAIL_TYPES in PushRouting.kt. +const MAIL_TYPES = ['EmailDelivery', 'Email', 'Mailbox']; + /** * A StateChange push in which no account got new mail: only contact or * calendar changes, which the native push router already turned into device - * syncs (#34, docs/device-sync.md "Triggers"). Without `EmailDelivery` there - * is nothing to notify, and the legacy path (the newest unread mail) must - * not run for it. An empty or missing `changed` map keeps the old behaviour. + * syncs (#34, docs/device-sync.md "Triggers"). Without a mail type there is + * nothing to notify, and the legacy path (the newest unread mail) must not + * run for it. An empty or missing `changed` map keeps the old behaviour. */ export function carriesNoMail(payload: RelayPushData): boolean { if (payload.kind !== 'jmap-state-change' || !payload.changed) return false; const accounts = Object.values(payload.changed); if (accounts.length === 0) return false; - return !accounts.some((types) => !!types && typeof types === 'object' && 'EmailDelivery' in types); + return !accounts.some((types) => !!types && typeof types === 'object' + && MAIL_TYPES.some((type) => type in types)); } // Fired by BulwarkPushTaskService when a data FCM message arrives. Runs in a