Skip to content

Commit

Permalink
[keyserver] Convert ids in rescinds
Browse files Browse the repository at this point in the history
Summary:
[ENG-4569](https://linear.app/comm/issue/ENG-4569)

Rescinds also need to be converted. To do that we need to pass `stateVersion` in the delivery, alongside the `codeVersion` and using it convert the rescinds.

Test Plan:
Checked that notification were rescinded on macOS and Android, after displaying them on web app. Checked if the `rescinded` column in the `notification` table was set to 1, and a new rescind row was created.

Done this for both `stateVersion < 43` (the ids aren't converted) and `stateVersion >= 43` (the ids were converted).
Tested variant when the notification was sent to `stateVersion < 43` but rescind was sent to `stateVersion >= 43` (in this case I also checked if the group summary notification was removed).
Also tested if the notifications were rescinded if they sent from before these changes (current master) and so wouldn't have the `stateVersion` in delivery.

Reviewers: marcin, tomek

Reviewed By: marcin, tomek

Subscribers: ashoat

Differential Revision: https://phab.comm.dev/D8706
  • Loading branch information
MichalGniadek committed Aug 7, 2023
1 parent 4b37840 commit c8cac19
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 10 deletions.
35 changes: 30 additions & 5 deletions keyserver/src/push/rescind.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import apn from '@parse/node-apn';
import invariant from 'invariant';

import type { PlatformDetails } from 'lib/types/device-types.js';
import { threadSubscriptions } from 'lib/types/subscription-types.js';
import { threadPermissions } from 'lib/types/thread-permission-types.js';
import { promiseAll } from 'lib/utils/promises.js';
import { tID } from 'lib/utils/validation-utils.js';

import {
prepareEncryptedAndroidNotificationRescinds,
Expand All @@ -21,6 +23,7 @@ import { apnPush, fcmPush } from './utils.js';
import createIDs from '../creators/id-creator.js';
import { dbQuery, SQL } from '../database/database.js';
import type { SQLStatementType } from '../database/types.js';
import { validateOutput } from '../utils/validation-utils.js';

type ParsedDelivery = {
+platform: 'ios' | 'macos' | 'android',
Expand Down Expand Up @@ -75,6 +78,7 @@ async function rescindPushNotifs(
rowParsedDeliveries.push({
notificationID: delivery.iosID,
codeVersion: delivery.codeVersion,
stateVersion: delivery.stateVersion,
platform: delivery.deviceType ?? 'ios',
deviceTokens,
});
Expand All @@ -85,6 +89,7 @@ async function rescindPushNotifs(
rowParsedDeliveries.push({
notificationID: row.collapse_key ? row.collapse_key : id,
codeVersion: delivery.codeVersion,
stateVersion: delivery.stateVersion,
platform: 'android',
deviceTokens,
});
Expand All @@ -110,6 +115,20 @@ async function rescindPushNotifs(
};

for (const delivery of parsedDeliveries[id]) {
let platformDetails: PlatformDetails = { platform: delivery.platform };
if (delivery.codeVersion) {
platformDetails = {
...platformDetails,
codeVersion: delivery.codeVersion,
};
}
if (delivery.stateVersion) {
platformDetails = {
...platformDetails,
stateVersion: delivery.stateVersion,
};
}

if (delivery.platform === 'ios') {
const devices = delivery.deviceTokens.map(deviceToken => ({
deviceToken,
Expand All @@ -120,7 +139,7 @@ async function rescindPushNotifs(
delivery.notificationID,
row.unread_count,
threadID,
delivery.codeVersion,
platformDetails,
devices,
);
return await apnPush({
Expand All @@ -142,7 +161,7 @@ async function rescindPushNotifs(
delivery.notificationID,
row.unread_count,
threadID,
delivery.codeVersion,
platformDetails,
devices,
);
return await fcmPush({
Expand Down Expand Up @@ -253,13 +272,16 @@ async function prepareIOSNotification(
iosID: string,
unreadCount: number,
threadID: string,
codeVersion: ?number,
platformDetails: PlatformDetails,
devices: $ReadOnlyArray<NotificationTargetDevice>,
): Promise<$ReadOnlyArray<TargetedAPNsNotification>> {
threadID = validateOutput(platformDetails, tID, threadID);
const { codeVersion } = platformDetails;

const notification = new apn.Notification();
notification.topic = getAPNsNotificationTopic({
platform: 'ios',
codeVersion: codeVersion ?? undefined,
codeVersion,
});

if (codeVersion && codeVersion > 198) {
Expand Down Expand Up @@ -297,9 +319,12 @@ async function prepareAndroidNotification(
notifID: string,
unreadCount: number,
threadID: string,
codeVersion: ?number,
platformDetails: PlatformDetails,
devices: $ReadOnlyArray<NotificationTargetDevice>,
): Promise<$ReadOnlyArray<TargetedAndroidNotification>> {
threadID = validateOutput(platformDetails, tID, threadID);
const { codeVersion } = platformDetails;

const notification = {
data: {
badge: unreadCount.toString(),
Expand Down
28 changes: 23 additions & 5 deletions keyserver/src/push/send.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ async function sendPushNotifs(pushInfo: PushInfo) {
return await sendAPNsNotification('ios', targetedNotifications, {
...notificationInfo,
codeVersion,
stateVersion,
});
})();
deliveryPromises.push(deliveryPromise);
Expand Down Expand Up @@ -270,6 +271,7 @@ async function sendPushNotifs(pushInfo: PushInfo) {
return await sendAndroidNotification(targetedNotifications, {
...notificationInfo,
codeVersion,
stateVersion,
});
})();
deliveryPromises.push(deliveryPromise);
Expand All @@ -296,6 +298,7 @@ async function sendPushNotifs(pushInfo: PushInfo) {
return await sendWebNotification(notification, deviceTokens, {
...notificationInfo,
codeVersion,
stateVersion,
});
})();
deliveryPromises.push(deliveryPromise);
Expand Down Expand Up @@ -330,6 +333,7 @@ async function sendPushNotifs(pushInfo: PushInfo) {
return await sendAPNsNotification('macos', targetedNotifications, {
...notificationInfo,
codeVersion,
stateVersion,
});
})();
deliveryPromises.push(deliveryPromise);
Expand All @@ -356,6 +360,7 @@ async function sendPushNotifs(pushInfo: PushInfo) {
return await sendWNSNotification(notification, deviceTokens, {
...notificationInfo,
codeVersion,
stateVersion,
});
})();
deliveryPromises.push(deliveryPromise);
Expand Down Expand Up @@ -982,12 +987,14 @@ type NotificationInfo =
+messageID: string,
+collapseKey: ?string,
+codeVersion: number,
+stateVersion: number,
}
| {
+source: 'mark_as_unread' | 'mark_as_read' | 'activity_update',
+dbID: string,
+userID: string,
+codeVersion: number,
+stateVersion: number,
};

type APNsDelivery = {
Expand All @@ -996,6 +1003,7 @@ type APNsDelivery = {
iosID: string,
deviceTokens: $ReadOnlyArray<string>,
codeVersion: number,
stateVersion: number,
errors?: $ReadOnlyArray<ResponseFailure>,
};
type APNsResult = {
Expand All @@ -1008,7 +1016,7 @@ async function sendAPNsNotification(
targetedNotifications: $ReadOnlyArray<TargetedAPNsNotification>,
notificationInfo: NotificationInfo,
): Promise<APNsResult> {
const { source, codeVersion } = notificationInfo;
const { source, codeVersion, stateVersion } = notificationInfo;

const response = await apnPush({
targetedNotifications,
Expand All @@ -1029,6 +1037,7 @@ async function sendAPNsNotification(
iosID,
deviceTokens,
codeVersion,
stateVersion,
};
if (response.errors) {
delivery.errors = response.errors;
Expand All @@ -1051,6 +1060,7 @@ type AndroidDelivery = {
androidIDs: $ReadOnlyArray<string>,
deviceTokens: $ReadOnlyArray<string>,
codeVersion: number,
stateVersion: number,
errors?: $ReadOnlyArray<Object>,
};
type AndroidResult = {
Expand All @@ -1065,7 +1075,7 @@ async function sendAndroidNotification(
const collapseKey = notificationInfo.collapseKey
? notificationInfo.collapseKey
: null; // for Flow...
const { source, codeVersion } = notificationInfo;
const { source, codeVersion, stateVersion } = notificationInfo;
const response = await fcmPush({
targetedNotifications,
collapseKey,
Expand All @@ -1081,6 +1091,7 @@ async function sendAndroidNotification(
androidIDs,
deviceTokens,
codeVersion,
stateVersion,
};
if (response.errors) {
delivery.errors = response.errors;
Expand All @@ -1100,6 +1111,7 @@ type WebDelivery = {
+deviceType: 'web',
+deviceTokens: $ReadOnlyArray<string>,
+codeVersion?: number,
+stateVersion: number,
+errors?: $ReadOnlyArray<WebPushError>,
};
type WebResult = {
Expand All @@ -1112,7 +1124,7 @@ async function sendWebNotification(
deviceTokens: $ReadOnlyArray<string>,
notificationInfo: NotificationInfo,
): Promise<WebResult> {
const { source, codeVersion } = notificationInfo;
const { source, codeVersion, stateVersion } = notificationInfo;

const response = await webPush({
notification,
Expand All @@ -1125,6 +1137,7 @@ async function sendWebNotification(
deviceTokens,
codeVersion,
errors: response.errors,
stateVersion,
};
const result: WebResult = {
info: notificationInfo,
Expand All @@ -1140,6 +1153,7 @@ type WNSDelivery = {
+wnsIDs: $ReadOnlyArray<string>,
+deviceTokens: $ReadOnlyArray<string>,
+codeVersion?: number,
+stateVersion: number,
+errors?: $ReadOnlyArray<WNSPushError>,
};
type WNSResult = {
Expand All @@ -1152,7 +1166,7 @@ async function sendWNSNotification(
deviceTokens: $ReadOnlyArray<string>,
notificationInfo: NotificationInfo,
): Promise<WNSResult> {
const { source, codeVersion } = notificationInfo;
const { source, codeVersion, stateVersion } = notificationInfo;

const response = await wnsPush({
notification,
Expand All @@ -1167,6 +1181,7 @@ async function sendWNSNotification(
deviceTokens,
codeVersion,
errors: response.errors,
stateVersion,
};
const result: WNSResult = {
info: notificationInfo,
Expand Down Expand Up @@ -1309,6 +1324,7 @@ async function updateBadgeCount(
dbID,
userID,
codeVersion,
stateVersion,
});
})();

Expand All @@ -1319,7 +1335,7 @@ async function updateBadgeCount(
const androidVersionsToTokens = byPlatform.get('android');
if (androidVersionsToTokens) {
for (const [versionKey, deviceInfos] of androidVersionsToTokens) {
const { codeVersion } = stringToVersionKey(versionKey);
const { codeVersion, stateVersion } = stringToVersionKey(versionKey);
const notificationData =
codeVersion < 69
? { badge: unreadCount.toString() }
Expand Down Expand Up @@ -1347,6 +1363,7 @@ async function updateBadgeCount(
dbID,
userID,
codeVersion,
stateVersion,
});
})();
deliveryPromises.push(deliveryPromise);
Expand Down Expand Up @@ -1375,6 +1392,7 @@ async function updateBadgeCount(
dbID,
userID,
codeVersion,
stateVersion,
}),
);
}
Expand Down

0 comments on commit c8cac19

Please sign in to comment.