Select queries should not erase cached data from unselected fields #409
Conversation
|
Just curious, is anyone going to merge any of those pending pull requests ? |
|
@hramos can you get me access here? |
|
@dstarke what a beautifully tested PR. I'll jump into it later today |
There was a problem hiding this comment.
@dstarke sorry, I've been under the water a bit lately. Just a small recommendation on the code formatting for that PR (spaces mostly). Can you address those?
like
if(hasSubObjectSelect){
should be
if (hasSubObjectSelect) {
Codecov Report
@@ Coverage Diff @@
## master #409 +/- ##
==========================================
+ Coverage 84.35% 84.49% +0.13%
==========================================
Files 46 46
Lines 3663 3696 +33
Branches 835 846 +11
==========================================
+ Hits 3090 3123 +33
Misses 573 573
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #409 +/- ##
==========================================
+ Coverage 84.35% 84.49% +0.13%
==========================================
Files 46 46
Lines 3663 3696 +33
Branches 835 846 +11
==========================================
+ Hits 3090 3123 +33
Misses 573 573
Continue to review full report at Codecov.
|
|
Thanks for the fixes! This reminds me I need to setup a linter on that repo as well. |
|
@dstarke it seems that this PR has introduced an issue, can you have a look shortly please? |
|
The tests for ParseQuery do specifically test for that condition, and they pass (I just checked this as well), so I think the problem may be somewhere else. The tests for ParseQuery, though, directly supply the objects returned by queries, since they come from another unit. I'm trying to figure out how to reproduce it. |
|
I didn’t managed to repro either, but I believe @dplewis had a hunch |
|
I did find some problems with how sub-objects were handled. Added tests and fixes for those problems in the PR I just created. I still can't quite reproduce the issue as described in #475, but this should fix at least one of the issues that is discussed in: parse-community/parse-server#3999. |
|
@dstarke thanks for the quick response! |
When a query is issued with a select, we know that the object returned will only be part of the truth from the server. The current implementation, on retrieving the results of a query with a select, erases all the information in the shared object except the fields included in the select, which are updated. That's not the best possible estimate of the state of truth on the server.
Ideally, for a query with a select, the cached object should only be updated or erased for the fields involved in the query.
This PR introduces test cases for retrieving previously retrieved objects with a more restrictive select clause in accordance with the above goal, and proposes a code change that fixes the issue and allows these new test cases to pass.
Fixes #371.