Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #7340 by correclty computing function name for push event #7341

Merged
merged 4 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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);
Copy link
Member

@mtrezza mtrezza Apr 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think you could rewrite these timed blocks to use async/await instead of being nested?

I know that goes a bit beyond the scope of this PR, but it would certainly make the tests easier to read. So instead of these nests we would just do:

await new Promise(resolve => setTimeout(resolve, ASYNC_TEST_WAIT_TIME));

or maybe even shorter if you want to use a method:

await timeout(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
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
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