Skip to content

Commit 3a17904

Browse files
authored
Push: Cleanup invalid device tokens (#4149)
* Adds collecting invalid / expired device tokens from GCM / APNS * Cleanup invalid installations based on responses from the adapters * Adds test for cleanup * Adds tests for removal
1 parent a1554d0 commit 3a17904

File tree

3 files changed

+111
-2
lines changed

3 files changed

+111
-2
lines changed

spec/PushWorker.spec.js

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
var PushWorker = require('../src').PushWorker;
22
var PushUtils = require('../src/Push/utils');
33
var Config = require('../src/Config');
4+
var { pushStatusHandler } = require('../src/StatusHandler');
45

56
describe('PushWorker', () => {
67
it('should run with small batch', (done) => {
@@ -156,4 +157,89 @@ describe('PushWorker', () => {
156157
expect(PushUtils.groupByLocaleIdentifier([])).toEqual({default: []});
157158
});
158159
});
160+
161+
describe('pushStatus', () => {
162+
it('should remove invalid installations', (done) => {
163+
const config = new Config('test');
164+
const handler = pushStatusHandler(config);
165+
const spy = spyOn(config.database, "update").and.callFake(() => {
166+
return Promise.resolve();
167+
});
168+
handler.trackSent([
169+
{
170+
transmitted: false,
171+
device: {
172+
deviceToken: 1,
173+
deviceType: 'ios',
174+
},
175+
response: { error: 'Unregistered' }
176+
},
177+
{
178+
transmitted: true,
179+
device: {
180+
deviceToken: 10,
181+
deviceType: 'ios',
182+
},
183+
},
184+
{
185+
transmitted: false,
186+
device: {
187+
deviceToken: 2,
188+
deviceType: 'ios',
189+
},
190+
response: { error: 'NotRegistered' }
191+
},
192+
{
193+
transmitted: false,
194+
device: {
195+
deviceToken: 3,
196+
deviceType: 'ios',
197+
},
198+
response: { error: 'InvalidRegistration' }
199+
},
200+
{
201+
transmitted: true,
202+
device: {
203+
deviceToken: 11,
204+
deviceType: 'ios',
205+
},
206+
},
207+
{
208+
transmitted: false,
209+
device: {
210+
deviceToken: 4,
211+
deviceType: 'ios',
212+
},
213+
response: { error: 'InvalidRegistration' }
214+
},
215+
{
216+
transmitted: false,
217+
device: {
218+
deviceToken: 5,
219+
deviceType: 'ios',
220+
},
221+
response: { error: 'InvalidRegistration' }
222+
},
223+
{ // should not be deleted
224+
transmitted: false,
225+
device: {
226+
deviceToken: 101,
227+
deviceType: 'ios',
228+
},
229+
response: { error: 'invalid error...' }
230+
}
231+
], true);
232+
expect(spy).toHaveBeenCalled();
233+
expect(spy.calls.count()).toBe(1);
234+
const lastCall = spy.calls.mostRecent();
235+
expect(lastCall.args[0]).toBe('_Installation');
236+
expect(lastCall.args[1]).toEqual({
237+
deviceToken: { '$in': [1,2,3,4,5] }
238+
});
239+
expect(lastCall.args[2]).toEqual({
240+
deviceToken: { '__op': "Delete" }
241+
});
242+
done();
243+
});
244+
});
159245
});

src/Push/PushWorker.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { pushStatusHandler } from '../StatusHandler';
99
import * as utils from './utils';
1010
import { ParseMessageQueue } from '../ParseMessageQueue';
1111
import { PushQueue } from './PushQueue';
12+
import logger from '../logger';
1213

1314
function groupByBadge(installations) {
1415
return installations.reduce((map, installation) => {
@@ -80,6 +81,7 @@ export class PushWorker {
8081
}
8182

8283
if (!utils.isPushIncrementing(body)) {
84+
logger.verbose(`Sending push to ${installations.length}`);
8385
return this.adapter.send(body, installations, pushStatus.objectId).then((results) => {
8486
return pushStatus.trackSent(results);
8587
});

src/StatusHandler.js

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,13 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
162162
{status: "running", updatedAt: new Date(), count });
163163
}
164164

165-
const trackSent = function(results) {
165+
const trackSent = function(results, cleanupInstallations = process.env.PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS) {
166166
const update = {
167167
updatedAt: new Date(),
168168
numSent: 0,
169169
numFailed: 0
170170
};
171+
const devicesToRemove = [];
171172
if (Array.isArray(results)) {
172173
results = flatten(results);
173174
results.reduce((memo, result) => {
@@ -181,6 +182,18 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
181182
if (result.transmitted) {
182183
memo.numSent++;
183184
} else {
185+
if (result && result.response && result.response.error && result.device && result.device.deviceToken) {
186+
const token = result.device.deviceToken;
187+
const error = result.response.error;
188+
// GCM errors
189+
if (error === 'NotRegistered' || error === 'InvalidRegistration') {
190+
devicesToRemove.push(token);
191+
}
192+
// APNS errors
193+
if (error === 'Unregistered') {
194+
devicesToRemove.push(token);
195+
}
196+
}
184197
memo.numFailed++;
185198
}
186199
return memo;
@@ -189,7 +202,7 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
189202
}
190203

191204
logger.verbose(`_PushStatus ${objectId}: sent push! %d success, %d failures`, update.numSent, update.numFailed);
192-
205+
logger.verbose(`_PushStatus ${objectId}: needs cleanup`, { devicesToRemove });
193206
['numSent', 'numFailed'].forEach((key) => {
194207
if (update[key] > 0) {
195208
update[key] = {
@@ -201,6 +214,14 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
201214
}
202215
});
203216

217+
if (devicesToRemove.length > 0 && cleanupInstallations) {
218+
logger.info(`Removing device tokens on ${devicesToRemove.length} _Installations`);
219+
database.update('_Installation', { deviceToken: { '$in': devicesToRemove }}, { deviceToken: {"__op": "Delete"} }, {
220+
acl: undefined,
221+
many: true
222+
});
223+
}
224+
204225
return handler.update({ objectId }, update).then((res) => {
205226
if (res && res.count === 0) {
206227
return this.complete();

0 commit comments

Comments
 (0)