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

Create Cloud function afterLiveQueryEvent #6859

Merged
merged 34 commits into from
Oct 19, 2020

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Aug 12, 2020

This is a new Cloud function that allows mutation of the original and current objects that get sent to the LiveQuery client. This is a LiveQuery equivalent of afterFind.

Implementation:

// This will change the values for "foo" and "yolo" only for LQ events
Parse.Cloud.afterLiveQueryEvent('TestObject', req => {
      const current = req.object;
      current.set('foo', 'yolo');

      const original = req.original;
      original.set('yolo', 'foo');
});

subscription.on('update', (object, original) => {
// without the trigger, object.get("foo") and original.get("yolo") would be empty
});
// This will prevent LiveQuery messages being passed to the client if foo is changed
Parse.Cloud.afterLiveQueryEvent('TestObject', req => {

      const current = req.object;
      const original = req.original;

      if (current.get('foo') != original.get('foo')) {
        throw "Don't pass an update trigger, or message";
      }
});

subscription.on('update', (object, original) => {
// this won't fire because the "throw" in the trigger, even if an object was updated.
});

This could also be a solution to #2946, by allowing secondary querying in the afterLiveQueryEvent block.

    const object = new TestObject();
    const child = new Parse.Object('Foo');
    child.set('foo','bar');
    await child.save();

    const relation = object.relation('children');
    relation.add(child);
    const query = new Parse.Query('TestObject');


    Parse.Cloud.afterLiveQueryEvent('TestObject', async req => {
      if (req.event != 'Create') {
        return;
      }
      let query = req.object.relation('children').query();
      query.equalTo('foo','bart');
      let first = await query.first();
      if (!first)  {
        throw "Doesn't match secondary query."
      }
    });


    const subscription = await query.subscribe();
    subscription.on('create', object => {
      // this won't be triggered now as children relation doesn't contain any objects with 'foo' equal to 'bar'.
    });
    await object.save();

Using async will obviously slow down the speed of LiveQuery events, and will require proper indexing / query design. It will also increase the required resources.

I've added a number of test cases to ensure it works as expected for "Create", "Update", "Enter", "Leave", and "Delete" events.

This feature is inspired by a few different threads.

#1686 - include in LiveQuery. This is not a direct solution, however it allows custom mutations and that could emulate .include functionality.
And this issue in the JS SDK, which restricts LiveQuery events to specific events.

Let me know what you think!

@dblythy
Copy link
Member Author

dblythy commented Aug 12, 2020

This is a branch that I am using myself nonetheless. I'd be curious to see if this is something that we could possibly add to the master. Failing tests are not related to this PR.

dplewis and others added 16 commits August 20, 2020 10:09
* fix: upgrade winston from 3.2.1 to 3.3.2

Snyk has created this PR to upgrade winston from 3.2.1 to 3.3.2.

See this package in NPM:
https://www.npmjs.com/package/winston

See this project in Snyk:
https://app.snyk.io/org/acinader/project/8c1a9edb-c8f5-4dc1-b221-4d6030a323eb?utm_source=github&utm_medium=upgrade-pr

* fix tests

Co-authored-by: Diamond Lewis <[email protected]>
Copy link

@facuparedes facuparedes left a comment

Choose a reason for hiding this comment

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

I forked it because I needed it in my project. From what I tested, everything works perfectly. Also, thanks @dblythy, because LiveQuery MatchesQuery is working like a charm.

@dblythy
Copy link
Member Author

dblythy commented Aug 21, 2020

Thank you @FNPCMDs! I would say be careful performance-wise as every LiveQuery event will trigger the secondary query unless the cloud function is restrictive. Also make sure you have your secondary query indexed and efficient as possible 😊

@codecov
Copy link

codecov bot commented Aug 21, 2020

Codecov Report

Merging #6859 into master will decrease coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6859      +/-   ##
==========================================
- Coverage   93.92%   93.78%   -0.14%     
==========================================
  Files         169      169              
  Lines       12210    12252      +42     
==========================================
+ Hits        11468    11491      +23     
- Misses        742      761      +19     
Impacted Files Coverage Δ
src/Adapters/Auth/index.js 93.54% <100.00%> (ø)
src/LiveQuery/ParseLiveQueryServer.js 95.11% <100.00%> (+0.55%) ⬆️
src/cloud-code/Parse.Cloud.js 98.59% <100.00%> (+0.06%) ⬆️
src/triggers.js 92.43% <100.00%> (+0.33%) ⬆️
src/GraphQL/parseGraphQLUtils.js 91.66% <0.00%> (-8.34%) ⬇️
src/GraphQL/transformers/query.js 83.14% <0.00%> (-6.75%) ⬇️
src/GraphQL/transformers/mutation.js 93.54% <0.00%> (-3.23%) ⬇️
src/GraphQL/loaders/usersQueries.js 95.23% <0.00%> (-2.39%) ⬇️
src/GraphQL/loaders/parseClassQueries.js 97.95% <0.00%> (-2.05%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cec333...16fbca8. Read the comment docs.

@dblythy
Copy link
Member Author

dblythy commented Oct 5, 2020

@dplewis @mtrezza do you guys have any feedback / suggestions for implementing this feature so I can close this PR? Or would this feature put too much pressure on the LQ server? Happy to close and move discussions over to the community forum if need be. Cheers

@dplewis
Copy link
Member

dplewis commented Oct 14, 2020

@dblythy Sorry for the late reply. The PR looks good. My question is should this hook return an object? Similar logic to beforeSubscribe that you wrote #6868 (comment)

@dblythy
Copy link
Member Author

dblythy commented Oct 14, 2020

Thanks for the feedback @dplewis. I've removed the option to return an object, and also made sure that mutating req.object / req.original changes the class name 😊

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

LGTM! I look forward to using this in production

@dblythy
Copy link
Member Author

dblythy commented Oct 19, 2020

Let me know if I've made a mistake in the docs formatting @dplewis. I've also added masterKey, installationId, clients, and subscription to the cloud function request for completeness. Thank you 🙏

@dplewis dplewis merged commit bf39cd6 into parse-community:master Oct 19, 2020
@ghost
Copy link

ghost commented Oct 19, 2020

Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page.

Generated by 🚫 dangerJS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants