Skip to content

Commit

Permalink
Fix #7340 by correclty computing function name for push event (#7341)
Browse files Browse the repository at this point in the history 
* Add a failing test for issue #7340

If any delay occurs after "message.event" assignation in
LiveQueryServer._onAfterSave, the next subscription or request with a different
event might overwrite it, and by that using the wrong "push" function name.

* Remove updade of message and use res.event instead

This prevent computing function name from a
incorrect event if multiple subscriptions override
one by one the message.event.

* Update CHANGELOG.md

* Replace setTimeout by async/await expressions
  • Loading branch information
Perceval Archimbaud authored Apr 13, 2021
1 parent 45d00ce commit 87dcd23
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ ___
- Fix file upload issue for S3 compatible storage (Linode, DigitalOcean) by avoiding empty tags property when creating a file (Ali Oguzhan Yildiz) [#7300](https://github.com/parse-community/parse-server/pull/7300)
- Add building Docker image as CI check (Manuel Trezza) [#7332](https://github.com/parse-community/parse-server/pull/7332)
- Add NPM package-lock version check to CI (Manuel Trezza) [#7333](https://github.com/parse-community/parse-server/pull/7333)
- Fix incorrect LiveQuery events triggered for multiple subscriptions on the same class with different events [#7341](https://github.com/parse-community/parse-server/pull/7341)
___
## 4.5.0
[Full Changelog](https://github.com/parse-community/parse-server/compare/4.4.0...4.5.0)
Expand Down
196 changes: 133 additions & 63 deletions spec/ParseLiveQueryServer.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ const queryHashValue = 'hash';
const testUserId = 'userId';
const testClassName = 'TestObject';

const timeout = () => jasmine.timeout(100);

describe('ParseLiveQueryServer', function () {
beforeEach(function (done) {
// Mock ParseWebSocketServer
Expand Down Expand Up @@ -750,10 +752,10 @@ describe('ParseLiveQueryServer', function () {

// Make sure we send command to client, since _matchesACL is async, we have to
// wait and check
setTimeout(function () {
expect(client.pushDelete).toHaveBeenCalled();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushDelete).toHaveBeenCalled();
done();
});

it('has no subscription and can handle object save command', async () => {
Expand Down Expand Up @@ -785,14 +787,14 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we do not send command to client
setTimeout(function () {
expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
});

it('can handle object enter command which matches some subscriptions', async done => {
Expand Down Expand Up @@ -822,14 +824,14 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we send enter command to client
setTimeout(function () {
expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
});

it('can handle object update command which matches some subscriptions', async done => {
Expand All @@ -855,14 +857,14 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we send update command to client
setTimeout(function () {
expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
});

it('can handle object leave command which matches some subscriptions', async done => {
Expand Down Expand Up @@ -892,14 +894,81 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we send leave command to client
setTimeout(function () {
expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).toHaveBeenCalled();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).toHaveBeenCalled();
done();
});

it('sends correct events for object with multiple subscriptions', async done => {
const parseLiveQueryServer = new ParseLiveQueryServer({});

Parse.Cloud.afterLiveQueryEvent('TestObject', () => {
// Simulate delay due to trigger, auth, etc.
return jasmine.timeout(10);
});

// Make mock request message
const message = generateMockMessage(true);
// Add mock client
const clientId = 1;
const client = addMockClient(parseLiveQueryServer, clientId);
client.sessionToken = 'sessionToken';

// Mock queryHash for this special test
const mockQueryHash = jasmine.createSpy('matchesQuery').and.returnValue('hash1');
jasmine.mockLibrary('../lib/LiveQuery/QueryTools', 'queryHash', mockQueryHash);
// Add mock subscription 1
const requestId2 = 2;
await addMockSubscription(parseLiveQueryServer, clientId, requestId2, null, null, 'hash1');

// Mock queryHash for this special test
const mockQueryHash2 = jasmine.createSpy('matchesQuery').and.returnValue('hash2');
jasmine.mockLibrary('../lib/LiveQuery/QueryTools', 'queryHash', mockQueryHash2);
// Add mock subscription 2
const requestId3 = 3;
await addMockSubscription(parseLiveQueryServer, clientId, requestId3, null, null, 'hash2');
// Mock _matchesSubscription to return matching
// In order to mimic a leave, then enter, we need original match return true
// and the current match return false, then the other way around
let counter = 0;
parseLiveQueryServer._matchesSubscription = function (parseObject) {
if (!parseObject) {
return false;
}
counter += 1;
// true, false, false, true
return counter < 2 || counter > 3;
};
parseLiveQueryServer._matchesACL = function () {
// Simulate call
return jasmine.timeout(10).then(() => true);
};
parseLiveQueryServer._onAfterSave(message);

// Make sure we send leave and enter command to client
await timeout();

expect(client.pushCreate).not.toHaveBeenCalled();
expect(client.pushEnter).toHaveBeenCalledTimes(1);
expect(client.pushEnter).toHaveBeenCalledWith(
requestId3,
{ key: 'value', className: 'TestObject' },
{ key: 'originalValue', className: 'TestObject' }
);
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).toHaveBeenCalledTimes(1);
expect(client.pushLeave).toHaveBeenCalledWith(
requestId2,
{ key: 'value', className: 'TestObject' },
{ key: 'originalValue', className: 'TestObject' }
);
done();
});

it('can handle update command with original object', async done => {
Expand Down Expand Up @@ -936,15 +1005,15 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we send update command to client
setTimeout(function () {
expect(client.pushUpdate).toHaveBeenCalled();
const args = parseWebSocket.send.calls.mostRecent().args;
const toSend = JSON.parse(args[0]);
await timeout();

expect(toSend.object).toBeDefined();
expect(toSend.original).toBeDefined();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
expect(client.pushUpdate).toHaveBeenCalled();
const args = parseWebSocket.send.calls.mostRecent().args;
const toSend = JSON.parse(args[0]);

expect(toSend.object).toBeDefined();
expect(toSend.original).toBeDefined();
done();
});

it('can handle object create command which matches some subscriptions', async done => {
Expand All @@ -970,14 +1039,14 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we send create command to client
setTimeout(function () {
expect(client.pushCreate).toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushCreate).toHaveBeenCalled();
expect(client.pushEnter).not.toHaveBeenCalled();
expect(client.pushUpdate).not.toHaveBeenCalled();
expect(client.pushDelete).not.toHaveBeenCalled();
expect(client.pushLeave).not.toHaveBeenCalled();
done();
});

it('can handle create command with fields', async done => {
Expand Down Expand Up @@ -1020,14 +1089,14 @@ describe('ParseLiveQueryServer', function () {
parseLiveQueryServer._onAfterSave(message);

// Make sure we send create command to client
setTimeout(function () {
expect(client.pushCreate).toHaveBeenCalled();
const args = parseWebSocket.send.calls.mostRecent().args;
const toSend = JSON.parse(args[0]);
expect(toSend.object).toBeDefined();
expect(toSend.original).toBeUndefined();
done();
}, jasmine.ASYNC_TEST_WAIT_TIME);
await timeout();

expect(client.pushCreate).toHaveBeenCalled();
const args = parseWebSocket.send.calls.mostRecent().args;
const toSend = JSON.parse(args[0]);
expect(toSend.object).toBeDefined();
expect(toSend.original).toBeUndefined();
done();
});

it('can match subscription for null or undefined parse object', function () {
Expand Down Expand Up @@ -1737,7 +1806,8 @@ describe('ParseLiveQueryServer', function () {
clientId,
requestId,
parseWebSocket,
query
query,
customQueryHashValue
) {
// If parseWebSocket is null, we use the default one
if (!parseWebSocket) {
Expand Down Expand Up @@ -1765,12 +1835,12 @@ describe('ParseLiveQueryServer', function () {
// Make mock subscription
const subscription = parseLiveQueryServer.subscriptions
.get(query.className)
.get(queryHashValue);
.get(customQueryHashValue || queryHashValue);
subscription.hasSubscribingClient = function () {
return false;
};
subscription.className = query.className;
subscription.hash = queryHashValue;
subscription.hash = customQueryHashValue || queryHashValue;
if (subscription.clientRequestIds && subscription.clientRequestIds.has(clientId)) {
subscription.clientRequestIds.get(clientId).push(requestId);
} else {
Expand Down
2 changes: 2 additions & 0 deletions spec/helper.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,5 @@ jasmine.restoreLibrary = function (library, name) {
}
require(library)[name] = libraryCache[library][name];
};

jasmine.timeout = t => new Promise(resolve => setTimeout(resolve, t));
4 changes: 1 addition & 3 deletions src/LiveQuery/ParseLiveQueryServer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ class ParseLiveQueryServer {
} else {
return null;
}
message.event = type;
res = {
event: type,
sessionToken: client.sessionToken,
Expand Down Expand Up @@ -334,8 +333,7 @@ class ParseLiveQueryServer {
originalParseObject = res.original.toJSON();
originalParseObject.className = res.original.className || className;
}
const functionName =
'push' + message.event.charAt(0).toUpperCase() + message.event.slice(1);
const functionName = 'push' + res.event.charAt(0).toUpperCase() + res.event.slice(1);
if (client[functionName]) {
client[functionName](requestId, currentParseObject, originalParseObject);
}
Expand Down

0 comments on commit 87dcd23

Please sign in to comment.