parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

feat: add `ParseQuery.triggerFields` to trigger LiveQuery only on update of specific fields

Open dblythy opened this issue 3 years ago • 4 comments

New Pull Request Checklist

  • [x] I am not disclosing a vulnerability.
  • [x] I am creating this PR in reference to an issue.

Issue Description

Live Query fires when fields are not updated, even when using the fields option. This results in unnecessary overhead.

Related issue: #8027

Approach

Checks if fields have changed, if not do not fire LQ.

TODOs before merging

  • [x] Add tests
  • [x] A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

dblythy avatar Jun 08 '22 01:06 dblythy

Thanks for opening this pull request!

  • ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

Codecov Report

Base: 94.34% // Head: 93.88% // Decreases project coverage by -0.45% :warning:

Coverage data is based on head (8aade83) compared to base (40810b4). Patch coverage: 100.00% of modified lines in pull request are covered.

:exclamation: Current head 8aade83 differs from pull request most recent head 0ae3b3d. Consider uploading reports for the commit 0ae3b3d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #8028      +/-   ##
==========================================
- Coverage   94.34%   93.88%   -0.46%     
==========================================
  Files         181      181              
  Lines       14388    14401      +13     
==========================================
- Hits        13574    13521      -53     
- Misses        814      880      +66     
Impacted Files Coverage Δ
src/LiveQuery/ParseLiveQueryServer.js 95.75% <100.00%> (-0.10%) :arrow_down:
src/Adapters/Cache/RedisCacheAdapter.js 17.39% <0.00%> (-73.92%) :arrow_down:
src/LiveQuery/ParseCloudCodePublisher.js 84.21% <0.00%> (-15.79%) :arrow_down:
src/Routers/AggregateRouter.js 90.00% <0.00%> (-8.00%) :arrow_down:
src/ParseServerRESTController.js 92.42% <0.00%> (-4.55%) :arrow_down:
src/batch.js 89.47% <0.00%> (-3.51%) :arrow_down:
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 89.80% <0.00%> (-2.34%) :arrow_down:
src/RestWrite.js 94.46% <0.00%> (-0.45%) :arrow_down:
src/ParseServer.js 92.20% <0.00%> (-0.44%) :arrow_down:
src/Controllers/DatabaseController.js 93.80% <0.00%> (-0.29%) :arrow_down:
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Jun 08 '22 02:06 codecov[bot]

Could you please rename a fix describing what is broken, instead of how it should behave?

I guess here it would be something like:

fix: setting the fields parameter still triggers LiveQuery update events for unrelated fields

mtrezza avatar Jun 08 '22 08:06 mtrezza

I will reformat the title to use the proper commit message syntax.

I will reformat the title to use the proper commit message syntax.

I see a lot of the tests passing; is this ready for review yet?

mtrezza avatar Dec 31 '22 03:12 mtrezza

@dblythy If I read this correctly, this is a breaking change; could you please suggest a note for the changelog that describes the full scope of change in this PR? Then we should be good to merge.

mtrezza avatar Jan 09 '23 16:01 mtrezza

The label type:feature cannot be used here.

The label block:major cannot be used here.

The label type:feature cannot be used here.

This isn't breaking and does not change the fields behavour (although perhaps fields should be renamed to select for consistency).

fields: behaves as select, defines what fields to return listen: only fires LQ if the props have been mutated

dblythy avatar Jan 15 '23 04:01 dblythy

I think we can

  • add the select method with the behavior of the current fields
  • deprecate the fields method

Since we're preparing a major release we could reduce the work and just rename fields to select, which makes this a breaking change. But what does that mean for the LiveQuery client SDKs? The SDKs are client side and if a breaking change requires to update clients (which usually takes more time) we don't want people be stuck on Parse Server 5 because of that. So I'd suggest to only deprecate fields.

mtrezza avatar Jan 15 '23 10:01 mtrezza

Yes, I agree. The issue of renaming .fields to .select and deprecating .select is a different issue to this PR though - this PR introduces a new feature listen that allows LiveQuery to conform to the spec:

Suppose the ParseObject Player contains three fields name, id and age. If you are only interested in the change of the name field, you can set query.fields to [name]

Although query.fields will need to updated to query.listen

dblythy avatar Jan 15 '23 10:01 dblythy

Got it, so we can consider this as PR as non-breaking and ready for merge?

And the current behavior of fields is purely like select, there is no overlap with the new listen, correct?

mtrezza avatar Jan 15 '23 10:01 mtrezza

Yep! We will just need to update the spec accordingly - describe fields as restricting fields, and listen to the current explanation for fields

dblythy avatar Jan 15 '23 11:01 dblythy

I know it's a last-minute question, but wouldn't the term watch make more sense than listen?

  • "listen" may imply a passive unspecific observation, like listening on a port for any signal to come in
  • "watch" may imply an active specific observation, like watching a Parse Object property for changes

What do you think?

mtrezza avatar Jan 15 '23 23:01 mtrezza

🎉 This change has been released in version 6.0.0-alpha.25

parseplatformorg avatar Jan 16 '23 11:01 parseplatformorg

🎉 This change has been released in version 6.0.0-beta.1

parseplatformorg avatar Jan 31 '23 03:01 parseplatformorg

🎉 This change has been released in version 6.0.0

parseplatformorg avatar Jan 31 '23 03:01 parseplatformorg