Skip to content

Commit a5ce9fc

Browse files
flovilmartflovilmart
authored
Refactor pushStatusHandler to use Parse instead of direct access (#4173)
* Refactors pushStatusHandler to use HTTP interface so we can bind CloudCode hooks * Handle correctly nested dot atomic operations * Better handling of restricted class names, add support for afterSave _PushStatus * Adds simple testing for afterSave(PushStatus) * Reverts jobStatusHandler * Addresses fixes * adds delays to all methods
1 parent a39d045 commit a5ce9fc

File tree

9 files changed

+161
-58
lines changed

9 files changed

+161
-58
lines changed

‎spec/CloudCode.spec.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,4 +1627,19 @@ describe('afterFind hooks', () => {
16271627
})
16281628
.then(() => done());
16291629
});
1630+
1631+
it('should validate triggers correctly', () => {
1632+
expect(() => {
1633+
Parse.Cloud.beforeSave('_Session', () => {});
1634+
}).toThrow('Triggers are not supported for _Session class.');
1635+
expect(() => {
1636+
Parse.Cloud.afterSave('_Session', () => {});
1637+
}).toThrow('Triggers are not supported for _Session class.');
1638+
expect(() => {
1639+
Parse.Cloud.beforeSave('_PushStatus', () => {});
1640+
}).toThrow('Only afterSave is allowed on _PushStatus');
1641+
expect(() => {
1642+
Parse.Cloud.afterSave('_PushStatus', () => {});
1643+
}).not.toThrow();
1644+
});
16301645
});

‎spec/Parse.Push.spec.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ describe('Parse.Push', () => {
127127
alert: 'Hello world!'
128128
}
129129
}, {useMasterKey: true}))
130+
.then(() => delayPromise(500))
130131
.then(() => {
131132
request.get({
132133
url: 'http://localhost:8378/1/classes/_PushStatus',
@@ -155,6 +156,7 @@ describe('Parse.Push', () => {
155156
alert: 'Hello world!'
156157
}
157158
}, {useMasterKey: true}))
159+
.then(() => delayPromise(500)) // put a delay as we keep writing
158160
.then(() => {
159161
request.get({
160162
url: 'http://localhost:8378/1/classes/_PushStatus',

‎spec/PushController.spec.js‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,11 @@ describe('PushController', () => {
388388
});
389389

390390
it('properly creates _PushStatus', (done) => {
391+
const pushStatusAfterSave = {
392+
handler: function() {}
393+
};
394+
const spy = spyOn(pushStatusAfterSave, 'handler').and.callThrough();
395+
Parse.Cloud.afterSave('_PushStatus', pushStatusAfterSave.handler);
391396
var installations = [];
392397
while(installations.length != 10) {
393398
const installation = new Parse.Object("_Installation");
@@ -466,8 +471,36 @@ describe('PushController', () => {
466471
return query.find();
467472
}).catch((error) => {
468473
expect(error.code).toBe(119);
469-
done();
470-
});
474+
})
475+
.then(() => {
476+
function getPushStatus(callIndex) {
477+
return spy.calls.all()[callIndex].args[0].object;
478+
}
479+
expect(spy).toHaveBeenCalled();
480+
expect(spy.calls.count()).toBe(4);
481+
const allCalls = spy.calls.all();
482+
allCalls.forEach((call) => {
483+
expect(call.args.length).toBe(2);
484+
const object = call.args[0].object;
485+
expect(object instanceof Parse.Object).toBe(true);
486+
});
487+
expect(getPushStatus(0).get('status')).toBe('pending');
488+
expect(getPushStatus(1).get('status')).toBe('running');
489+
expect(getPushStatus(1).get('numSent')).toBe(0);
490+
expect(getPushStatus(2).get('status')).toBe('running');
491+
expect(getPushStatus(2).get('numSent')).toBe(10);
492+
expect(getPushStatus(2).get('numFailed')).toBe(5);
493+
// Those are updated from a nested . operation, this would
494+
// not render correctly before
495+
expect(getPushStatus(2).get('failedPerType')).toEqual({
496+
android: 5
497+
});
498+
expect(getPushStatus(2).get('sentPerType')).toEqual({
499+
ios: 10
500+
});
501+
expect(getPushStatus(3).get('status')).toBe('succeeded');
502+
})
503+
.then(done).catch(done.fail);
471504
});
472505

473506
it('should properly report failures in _PushStatus', (done) => {
@@ -710,7 +743,7 @@ describe('PushController', () => {
710743
}).then(() => {
711744
return Parse.Object.saveAll(installations).then(() => {
712745
return pushController.sendPush(payload, {}, config, auth);
713-
});
746+
}).then(() => new Promise(resolve => setTimeout(resolve, 300)));
714747
}).then(() => {
715748
const query = new Parse.Query('_PushStatus');
716749
return query.find({useMasterKey: true}).then((results) => {
@@ -776,20 +809,15 @@ describe('PushController', () => {
776809
var config = new Config(Parse.applicationId);
777810
return Parse.Object.saveAll(installations).then(() => {
778811
return pushController.sendPush(payload, {}, config, auth);
779-
}).then(() => new Promise(resolve => setTimeout(resolve, 100)));
812+
}).then(() => new Promise(resolve => setTimeout(resolve, 300)));
780813
}).then(() => {
781814
const query = new Parse.Query('_PushStatus');
782815
return query.find({useMasterKey: true}).then((results) => {
783816
expect(results.length).toBe(1);
784817
const pushStatus = results[0];
785818
expect(pushStatus.get('status')).toBe('scheduled');
786-
done();
787819
});
788-
}).catch((err) => {
789-
console.error(err);
790-
fail('should not fail');
791-
done();
792-
});
820+
}).then(done).catch(done.err);
793821
});
794822

795823
it('should not enqueue push when device token is not set', (done) => {

‎spec/PushWorker.spec.js‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ describe('PushWorker', () => {
165165
const spy = spyOn(config.database, "update").and.callFake(() => {
166166
return Promise.resolve();
167167
});
168-
handler.trackSent([
168+
const toAwait = handler.trackSent([
169169
{
170170
transmitted: false,
171171
device: {
@@ -239,13 +239,13 @@ describe('PushWorker', () => {
239239
expect(lastCall.args[2]).toEqual({
240240
deviceToken: { '__op': "Delete" }
241241
});
242-
done();
242+
toAwait.then(done).catch(done);
243243
});
244244

245245
it('tracks push status per UTC offsets', (done) => {
246246
const config = new Config('test');
247-
const handler = pushStatusHandler(config, 'ABCDEF1234');
248-
const spy = spyOn(config.database, "update").and.callThrough();
247+
const handler = pushStatusHandler(config);
248+
const spy = spyOn(Parse, "_request").and.callThrough();
249249
const UTCOffset = 1;
250250
handler.setInitial().then(() => {
251251
return handler.trackSent([
@@ -266,14 +266,9 @@ describe('PushWorker', () => {
266266
], UTCOffset)
267267
}).then(() => {
268268
expect(spy).toHaveBeenCalled();
269-
expect(spy.calls.count()).toBe(1);
270269
const lastCall = spy.calls.mostRecent();
271-
expect(lastCall.args[0]).toBe('_PushStatus');
272-
const updatePayload = lastCall.args[2];
273-
expect(updatePayload.updatedAt instanceof Date).toBeTruthy();
274-
// remove the updatedAt as not testable
275-
delete updatePayload.updatedAt;
276-
270+
expect(lastCall.args[0]).toBe('PUT');
271+
expect(lastCall.args[1]).toBe(`classes/_PushStatus/${handler.objectId}`);
277272
expect(lastCall.args[2]).toEqual({
278273
numSent: { __op: 'Increment', amount: 1 },
279274
numFailed: { __op: 'Increment', amount: 1 },
@@ -284,7 +279,7 @@ describe('PushWorker', () => {
284279
count: { __op: 'Increment', amount: -2 },
285280
});
286281
const query = new Parse.Query('_PushStatus');
287-
return query.get('ABCDEF1234', { useMasterKey: true });
282+
return query.get(handler.objectId, { useMasterKey: true });
288283
}).then((pushStatus) => {
289284
const sentPerUTCOffset = pushStatus.get('sentPerUTCOffset');
290285
expect(sentPerUTCOffset['1']).toBe(1);
@@ -315,7 +310,7 @@ describe('PushWorker', () => {
315310
], UTCOffset)
316311
}).then(() => {
317312
const query = new Parse.Query('_PushStatus');
318-
return query.get('ABCDEF1234', { useMasterKey: true });
313+
return query.get(handler.objectId, { useMasterKey: true });
319314
}).then((pushStatus) => {
320315
const sentPerUTCOffset = pushStatus.get('sentPerUTCOffset');
321316
expect(sentPerUTCOffset['1']).toBe(3);
@@ -330,7 +325,7 @@ describe('PushWorker', () => {
330325
spyOn(config.database, "create").and.callFake(() => {
331326
return Promise.resolve();
332327
});
333-
const spy = spyOn(config.database, "update").and.callFake(() => {
328+
const spy = spyOn(Parse, "_request").and.callFake(() => {
334329
return Promise.resolve();
335330
});
336331
const UTCOffset = -6;
@@ -353,14 +348,8 @@ describe('PushWorker', () => {
353348
},
354349
], UTCOffset).then(() => {
355350
expect(spy).toHaveBeenCalled();
356-
expect(spy.calls.count()).toBe(1);
357351
const lastCall = spy.calls.mostRecent();
358-
expect(lastCall.args[0]).toBe('_PushStatus');
359-
const updatePayload = lastCall.args[2];
360-
expect(updatePayload.updatedAt instanceof Date).toBeTruthy();
361-
// remove the updatedAt as not testable
362-
delete updatePayload.updatedAt;
363-
352+
expect(lastCall.args[1]).toBe(`classes/_PushStatus/${handler.objectId}`);
364353
expect(lastCall.args[2]).toEqual({
365354
numSent: { __op: 'Increment', amount: 1 },
366355
numFailed: { __op: 'Increment', amount: 1 },

‎src/Controllers/DatabaseController.js‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,19 @@ DatabaseController.prototype.update = function(className, query, update, {
308308
});
309309
};
310310

311+
function expandResultOnKeyPath(object, key, value) {
312+
if (key.indexOf('.') < 0) {
313+
object[key] = value[key];
314+
return object;
315+
}
316+
const path = key.split('.');
317+
const firstKey = path[0];
318+
const nextPath = path.slice(1).join('.');
319+
object[firstKey] = expandResultOnKeyPath(object[firstKey] || {}, nextPath, value[firstKey]);
320+
delete object[key];
321+
return object;
322+
}
323+
311324
function sanitizeDatabaseResult(originalObject, result) {
312325
const response = {};
313326
if (!result) {
@@ -319,7 +332,8 @@ function sanitizeDatabaseResult(originalObject, result) {
319332
if (keyUpdate && typeof keyUpdate === 'object' && keyUpdate.__op
320333
&& ['Add', 'AddUnique', 'Remove', 'Increment'].indexOf(keyUpdate.__op) > -1) {
321334
// only valid ops that produce an actionable result
322-
response[key] = result[key];
335+
// the op may have happend on a keypath
336+
expandResultOnKeyPath(response, key, result);
323337
}
324338
});
325339
return Promise.resolve(response);

‎src/Push/PushWorker.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export class PushWorker {
5252
const auth = master(config);
5353
const where = utils.applyDeviceTokenExists(query.where);
5454
delete query.where;
55+
pushStatus = pushStatusHandler(config, pushStatus.objectId);
5556
return rest.find(config, auth, '_Installation', where, query).then(({results}) => {
5657
if (results.length == 0) {
5758
return;
@@ -63,7 +64,6 @@ export class PushWorker {
6364
}
6465

6566
sendToAdapter(body: any, installations: any[], pushStatus: any, config: Config, UTCOffset: ?any): Promise<*> {
66-
pushStatus = pushStatusHandler(config, pushStatus.objectId);
6767
// Check if we have locales in the push body
6868
const locales = utils.getLocalesFromPush(body);
6969
if (locales.length > 0) {

‎src/StatusHandler.js‎

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { md5Hash, newObjectId } from './cryptoUtils';
22
import { logger } from './logger';
3+
import Parse from 'parse/node';
34

45
const PUSH_STATUS_COLLECTION = '_PushStatus';
56
const JOB_STATUS_COLLECTION = '_JobStatus';
@@ -50,6 +51,47 @@ function statusHandler(className, database) {
5051
})
5152
}
5253

54+
function restStatusHandler(className) {
55+
let lastPromise = Promise.resolve();
56+
57+
function create(object) {
58+
lastPromise = lastPromise.then(() => {
59+
return Parse._request(
60+
'POST',
61+
`classes/${className}`,
62+
object,
63+
{ useMasterKey: true }
64+
).then((result) => {
65+
// merge the objects
66+
const response = Object.assign({}, object, result);
67+
return Promise.resolve(response);
68+
});
69+
});
70+
return lastPromise;
71+
}
72+
73+
function update(where, object) {
74+
// TODO: when we have updateWhere, use that for proper interfacing
75+
lastPromise = lastPromise.then(() => {
76+
return Parse._request(
77+
'PUT',
78+
`classes/${className}/${where.objectId}`,
79+
object,
80+
{ useMasterKey: true }).then((result) => {
81+
// merge the objects
82+
const response = Object.assign({}, object, result);
83+
return Promise.resolve(response);
84+
});
85+
});
86+
return lastPromise;
87+
}
88+
89+
return Object.freeze({
90+
create,
91+
update
92+
})
93+
}
94+
5395
export function jobStatusHandler(config) {
5496
let jobStatus;
5597
const objectId = newObjectId(config.objectIdSize);
@@ -103,11 +145,12 @@ export function jobStatusHandler(config) {
103145
});
104146
}
105147

106-
export function pushStatusHandler(config, objectId = newObjectId(config.objectIdSize)) {
148+
export function pushStatusHandler(config, existingObjectId) {
107149

108150
let pushStatus;
109151
const database = config.database;
110-
const handler = statusHandler(PUSH_STATUS_COLLECTION, database);
152+
const handler = restStatusHandler(PUSH_STATUS_COLLECTION);
153+
let objectId = existingObjectId;
111154
const setInitial = function(body = {}, where, options = {source: 'rest'}) {
112155
const now = new Date();
113156
let pushTime = now.toISOString();
@@ -133,8 +176,6 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
133176
pushHash = 'd41d8cd98f00b204e9800998ecf8427e';
134177
}
135178
const object = {
136-
objectId,
137-
createdAt: now,
138179
pushTime,
139180
query: JSON.stringify(where),
140181
payload: payloadString,
@@ -148,7 +189,8 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
148189
ACL: {}
149190
}
150191

151-
return handler.create(object).then(() => {
192+
return handler.create(object).then((result) => {
193+
objectId = result.objectId;
152194
pushStatus = {
153195
objectId
154196
};
@@ -159,12 +201,11 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
159201
const setRunning = function(count) {
160202
logger.verbose(`_PushStatus ${objectId}: sending push to %d installations`, count);
161203
return handler.update({status:"pending", objectId: objectId},
162-
{status: "running", updatedAt: new Date(), count });
204+
{status: "running", count });
163205
}
164206

165207
const trackSent = function(results, UTCOffset, cleanupInstallations = process.env.PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS) {
166208
const update = {
167-
updatedAt: new Date(),
168209
numSent: 0,
169210
numFailed: 0
170211
};
@@ -236,26 +277,33 @@ export function pushStatusHandler(config, objectId = newObjectId(config.objectId
236277
const complete = function() {
237278
return handler.update({ objectId }, {
238279
status: 'succeeded',
239-
count: {__op: 'Delete'},
240-
updatedAt: new Date()
280+
count: {__op: 'Delete'}
241281
});
242282
}
243283

244284
const fail = function(err) {
285+
if (typeof err === 'string') {
286+
err = { message: err };
287+
}
245288
const update = {
246-
errorMessage: JSON.stringify(err),
247-
status: 'failed',
248-
updatedAt: new Date()
289+
errorMessage: err,
290+
status: 'failed'
249291
}
250292
return handler.update({ objectId }, update);
251293
}
252294

253-
return Object.freeze({
254-
objectId,
295+
const rval = {
255296
setInitial,
256297
setRunning,
257298
trackSent,
258299
complete,
259300
fail
260-
})
301+
};
302+
303+
// define objectId to be dynamic
304+
Object.defineProperty(rval, "objectId", {
305+
get: () => objectId
306+
});
307+
308+
return Object.freeze(rval);
261309
}

0 commit comments

Comments
 (0)