Repository: parse-community/Parse-SDK-JS
Pull request #409 (2017-02-11 00:25:46)

Select queries should not erase cached data from unselected fields
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.


Patch:
diff --git a/LICENSE b/LICENSE
index 6ab9a0a..5c19751 100644
--- a/LICENSE
+++ b/LICENSE
@@ -27,4 +27,8 @@ ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
 ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
\ No newline at end of file
+SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+-----
+
+As of April 5, 2017, Parse, LLC has transferred this code to the parse-community organization, and will no longer be contributing to or distributing this code. 
diff --git a/PATENTS b/PATENTS
index 520556d..45e1681 100644
--- a/PATENTS
+++ b/PATENTS
@@ -30,4 +30,8 @@ necessarily infringed by the Software standing alone.
 
 A "Patent Assertion" is any lawsuit or other action alleging direct, indirect,
 or contributory infringement or inducement to infringe any patent, including a
-cross-claim or counterclaim.
\ No newline at end of file
+cross-claim or counterclaim.
+
+-----
+
+As of April 5, 2017, Parse, LLC has transferred this code to the parse-community organization, and will no longer be contributing to or distributing this code. 
diff --git a/README.md b/README.md
index 9019029..b8cc8d5 100644
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@
 [![Npm Version][npm-svg]][npm-link]
 [![License][license-svg]][license-link]
 
-A library that gives you access to the powerful Parse cloud platform from your JavaScript app. For more information on Parse and its features, see [the website](https://parse.com) or [the JavaScript guide](https://parseplatform.github.io/docs/js/guide/).
+A library that gives you access to the powerful Parse cloud platform from your JavaScript app. For more information on Parse and its features, see [the website](http://parseplatform.org) or [the JavaScript guide](https://docs.parseplatform.org/js/guide/).
 
 ## Getting Started
 
@@ -45,11 +45,13 @@ LICENSE file in the root directory of this source tree. An additional grant
 of patent rights can be found in the PATENTS file in the same directory.
 ```
 
- [build-status-svg]: https://travis-ci.org/ParsePlatform/Parse-SDK-JS.svg?branch=master
- [build-status-link]: https://travis-ci.org/ParsePlatform/Parse-SDK-JS
- [coverage-status-svg]: http://codecov.io/github/ParsePlatform/Parse-SDK-JS/coverage.svg?branch=master
- [coverage-status-link]: http://codecov.io/github/ParsePlatform/Parse-SDK-JS?branch=master
+As of April 5, 2017, Parse, LLC has transferred this code to the parse-community organization, and will no longer be contributing to or distributing this code. 
+
+ [build-status-svg]: https://travis-ci.org/parse-community/Parse-SDK-JS.svg?branch=master
+ [build-status-link]: https://travis-ci.org/parse-community/Parse-SDK-JS
+ [coverage-status-svg]: http://codecov.io/github/parse-community/Parse-SDK-JS/coverage.svg?branch=master
+ [coverage-status-link]: http://codecov.io/github/parse-community/Parse-SDK-JS?branch=master
  [npm-svg]: https://badge.fury.io/js/parse.svg
  [npm-link]: https://npmjs.org/parse
  [license-svg]: https://img.shields.io/badge/license-BSD-lightgrey.svg
- [license-link]: https://github.com/ParsePlatform/Parse-SDK-JS/blob/master/LICENSE
+ [license-link]: https://github.com/parse-community/Parse-SDK-JS/blob/master/LICENSE
diff --git a/src/ParseQuery.js b/src/ParseQuery.js
index 9904cd2..78c1488 100644
--- a/src/ParseQuery.js
+++ b/src/ParseQuery.js
@@ -44,6 +44,70 @@ function quote(s: string) {
   return '\\Q' + s.replace('\\E', '\\E\\\\E\\Q') + '\\E';
 }
 
+/**
+ * Handles pre-populating the result data of a query with select fields,
+ * making sure that the data object contains keys for all objects that have
+ * been requested with a select, so that our cached state updates correctly.
+ */
+function handleSelectResult(data: any, select: Array<string>){
+  var serverDataMask = {};
+
+  select.forEach((field) => {
+    let hasSubObjectSelect = field.indexOf(".") !== -1;
+    if (!hasSubObjectSelect && !data.hasOwnProperty(field)){
+      // this field was selected, but is missing from the retrieved data
+      data[field] = undefined
+    } else if (hasSubObjectSelect) {
+      // this field references a sub-object,
+      // so we need to walk down the path components 
+      let pathComponents = field.split(".");
+      var obj = data;
+      var serverMask = serverDataMask;
+
+      pathComponents.forEach((component, index, arr) => {
+        // add keys if the expected data is missing
+        if (!obj[component]) {
+          obj[component] = (index == arr.length-1) ? undefined : {};
+        }
+        obj = obj[component];
+
+        //add this path component to the server mask so we can fill it in later if needed
+        if (index < arr.length-1) {
+          if (!serverMask[component]) {
+            serverMask[component] = {};
+          }
+        }
+      });
+    }
+  });
+
+  if (Object.keys(serverDataMask).length > 0) {
+    // When selecting from sub-objects, we don't want to blow away the missing
+    // information that we may have retrieved before. We've already added any
+    // missing selected keys to sub-objects, but we still need to add in the
+    // data for any previously retrieved sub-objects that were not selected.
+
+    let serverData = CoreManager.getObjectStateController().getServerData({id:data.objectId, className:data.className});
+
+    function copyMissingDataWithMask(src, dest, mask, copyThisLevel){
+      //copy missing elements at this level
+      if (copyThisLevel) {
+        for (var key in src) {
+          if (src.hasOwnProperty(key) && !dest.hasOwnProperty(key)) {
+            dest[key] = src[key]
+          }
+        }
+      }
+      for (var key in mask) {
+        //traverse into objects as needed
+        copyMissingDataWithMask(src[key], dest[key], mask[key], true);
+      }
+    }
+
+    copyMissingDataWithMask(serverData, data, serverDataMask, false);
+  }
+}
+
 /**
  * Creates a new parse Parse.Query for the given Parse.Object subclass.
  * @class Parse.Query
@@ -273,6 +337,8 @@ export default class ParseQuery {
 
     let controller = CoreManager.getQueryController();
 
+    let select = this._select;
+
     return controller.find(
       this.className,
       this.toJSON(),
@@ -285,7 +351,15 @@ export default class ParseQuery {
         if (!data.className) {
           data.className = override;
         }
-        return ParseObject.fromJSON(data, true);
+
+        // Make sure the data object contains keys for all objects that
+        // have been requested with a select, so that our cached state
+        // updates correctly.
+        if (select) {
+          handleSelectResult(data, select);
+        }
+        
+        return ParseObject.fromJSON(data, !select);
       });
     })._thenRunCallbacks(options);
   }
@@ -371,6 +445,8 @@ export default class ParseQuery {
     var params = this.toJSON();
     params.limit = 1;
 
+    var select = this._select;
+
     return controller.find(
       this.className,
       params,
@@ -383,7 +459,15 @@ export default class ParseQuery {
       if (!objects[0].className) {
         objects[0].className = this.className;
       }
-      return ParseObject.fromJSON(objects[0], true);
+
+      // Make sure the data object contains keys for all objects that
+      // have been requested with a select, so that our cached state
+      // updates correctly.
+      if (select) {
+        handleSelectResult(objects[0], select);
+      }
+
+      return ParseObject.fromJSON(objects[0], !select);
     })._thenRunCallbacks(options);
   }
 


Test patch:
diff --git a/src/__tests__/ParseQuery-test.js b/src/__tests__/ParseQuery-test.js
index 070dc85..c33458f 100644
--- a/src/__tests__/ParseQuery-test.js
+++ b/src/__tests__/ParseQuery-test.js
@@ -9,10 +9,14 @@
 
 jest.dontMock('../CoreManager');
 jest.dontMock('../encode');
+jest.dontMock('../decode');
 jest.dontMock('../ParseError');
 jest.dontMock('../ParseGeoPoint');
 jest.dontMock('../ParsePromise');
 jest.dontMock('../ParseQuery');
+jest.dontMock('../SingleInstanceStateController');
+jest.dontMock('../UniqueInstanceStateController');
+jest.dontMock('../ObjectStateMutations');
 
 var mockObject = function(className) {
   this.className = className;
@@ -1332,4 +1336,223 @@ describe('ParseQuery', () => {
       done();
     });
   });
+
+
+  
+  it('overrides cached object with query results', (done) => {
+    jest.dontMock("../ParseObject");
+    jest.resetModules();
+    ParseObject = require('../ParseObject').default;
+    CoreManager = require('../CoreManager');
+    ParseQuery = require('../ParseQuery').default;
+
+    ParseObject.enableSingleInstance();
+    
+    var objectToReturn = { 
+      objectId: 'T01', 
+      name: 'Name', 
+      other: 'other', 
+      className:"Thing", 
+      createdAt: '2017-01-10T10:00:00Z'
+    };
+
+    CoreManager.setQueryController({
+      find(className, params, options) {
+        return ParsePromise.as({
+          results: [objectToReturn]
+        });
+      }
+    });
+
+    var q = new ParseQuery("Thing");
+    var testObject;
+    q.find().then((results) => {
+      testObject = results[0];
+      
+      expect(testObject.get("name")).toBe("Name");
+      expect(testObject.get("other")).toBe("other");
+      
+      objectToReturn = { objectId: 'T01', name: 'Name2'};
+      var q2 = new ParseQuery("Thing");
+      return q2.find();
+    }).then((results) => {
+      expect(results[0].get("name")).toBe("Name2");
+      expect(results[0].has("other")).toBe(false);
+    }).then(() => {
+      expect(testObject.get("name")).toBe("Name2");
+      expect(testObject.has("other")).toBe(false);
+      done();
+     });
+  });
+
+  it('does not override unselected fields with select query results', (done) => {
+    jest.dontMock("../ParseObject");
+    jest.resetModules();
+    ParseObject = require('../ParseObject').default;
+    CoreManager = require('../CoreManager');
+    ParseQuery = require('../ParseQuery').default;
+
+    ParseObject.enableSingleInstance();
+    
+    var objectToReturn = { 
+      objectId: 'T01', 
+      name: 'Name', 
+      other: 'other', 
+      tbd: 'exists', 
+      className:"Thing", 
+      createdAt: '2017-01-10T10:00:00Z',
+      subObject: {key1:"value", key2:"value2", key3:"thisWillGoAway"}
+    };
+
+    CoreManager.setQueryController({
+      find(className, params, options) {
+        return ParsePromise.as({
+          results: [objectToReturn]
+        });
+      }
+    });
+
+    var q = new ParseQuery("Thing");
+    var testObject;
+    return q.find().then((results) => {
+      testObject = results[0];
+      
+      expect(testObject.get("name")).toBe("Name");
+      expect(testObject.get("other")).toBe("other");
+      expect(testObject.has("tbd")).toBe(true);
+      expect(testObject.get("subObject").key1).toBe("value");
+      expect(testObject.get("subObject").key2).toBe("value2");
+      expect(testObject.get("subObject").key3).toBe("thisWillGoAway");
+      
+      var q2 = new ParseQuery("Thing");
+      q2.select("other", "tbd", "subObject.key1", "subObject.key3");
+      objectToReturn = { objectId: 'T01', other: 'other2', subObject:{key1:"updatedValue"}};
+      return q2.find();
+    }).then((results) => {
+      expect(results[0].get("name")).toBe("Name");    //query didn't select this
+      expect(results[0].get("other")).toBe("other2"); //query selected and updated this
+      expect(results[0].has("tbd")).toBe(false);      //query selected this and it wasn't returned
+      //sub-objects should work similarly
+      expect(results[0].get("subObject").key1).toBe("updatedValue");
+      expect(results[0].get("subObject").key2).toBe("value2");
+      expect(results[0].get("subObject").key3).toBeUndefined();
+    }).then(() => {
+      expect(testObject.get("name")).toBe("Name");
+      expect(testObject.get("other")).toBe("other2");
+      expect(testObject.has("tbd")).toBe(false);
+      expect(testObject.get("subObject").key1).toBe("updatedValue");
+      expect(testObject.get("subObject").key2).toBe("value2");
+      expect(testObject.get("subObject").key3).toBeUndefined();  
+      done();
+    }, (error) => {
+      done.fail(error);
+    });
+  });
+
+  it('overrides cached object with first() results', (done) => {
+    jest.dontMock("../ParseObject");
+    jest.resetModules();
+    ParseObject = require('../ParseObject').default;
+    CoreManager = require('../CoreManager');
+    ParseQuery = require('../ParseQuery').default;
+
+    ParseObject.enableSingleInstance();
+    
+    var objectToReturn = { 
+      objectId: 'T01', 
+      name: 'Name', 
+      other: 'other', 
+      className:"Thing", 
+      createdAt: '2017-01-10T10:00:00Z'
+    };
+
+    CoreManager.setQueryController({
+      find(className, params, options) {
+        return ParsePromise.as({
+          results: [objectToReturn]
+        });
+      }
+    });
+
+    var q = new ParseQuery("Thing");
+    var testObject;
+    q.first().then((result) => {
+      testObject = result;
+      
+      expect(testObject.get("name")).toBe("Name");
+      expect(testObject.get("other")).toBe("other");
+      
+      objectToReturn = { objectId: 'T01', name: 'Name2'};
+      var q2 = new ParseQuery("Thing");
+      return q2.first();
+    }).then((result) => {
+      expect(result.get("name")).toBe("Name2");
+      expect(result.has("other")).toBe(false);
+    }).then(() => {
+      expect(testObject.get("name")).toBe("Name2");
+      expect(testObject.has("other")).toBe(false);
+      done();
+     });
+  });
+
+  it('does not override unselected fields for first() on select query', (done) => {
+    jest.dontMock("../ParseObject");
+    jest.resetModules();
+    ParseObject = require('../ParseObject').default;
+    CoreManager = require('../CoreManager');
+    ParseQuery = require('../ParseQuery').default;
+
+    ParseObject.enableSingleInstance();
+    
+    var objectToReturn = { 
+      objectId: 'T01', 
+      name: 'Name', 
+      other: 'other', 
+      tbd: 'exists', 
+      className:"Thing", 
+      subObject: {key1:"value", key2:"value2", key3:"thisWillGoAway"},
+      createdAt: '2017-01-10T10:00:00Z',
+    };
+
+    CoreManager.setQueryController({
+      find(className, params, options) {
+        return ParsePromise.as({
+          results: [objectToReturn]
+        });
+      }
+    });
+
+    var q = new ParseQuery("Thing");
+    var testObject;
+    return q.first().then((result) => {
+      testObject = result;
+      
+      expect(testObject.get("name")).toBe("Name");
+      expect(testObject.get("other")).toBe("other");
+      expect(testObject.has("tbd")).toBe(true);
+      
+      var q2 = new ParseQuery("Thing");
+      q2.select("other", "tbd", "subObject.key1", "subObject.key3");
+      objectToReturn = { objectId: 'T01', other: 'other2', subObject:{key1:"updatedValue"}};
+      return q2.first();
+    }).then((result) => {
+      expect(result.get("name")).toBe("Name");    //query didn't select this
+      expect(result.get("other")).toBe("other2"); //query selected and updated this
+      expect(result.has("tbd")).toBe(false);      //query selected this and it wasn't returned
+      //sub-objects should work similarly
+      expect(result.get("subObject").key1).toBe("updatedValue");
+      expect(result.get("subObject").key2).toBe("value2");
+      expect(result.get("subObject").key3).toBeUndefined();
+    }).then(() => {
+      expect(testObject.get("name")).toBe("Name");
+      expect(testObject.get("other")).toBe("other2");
+      expect(testObject.has("tbd")).toBe(false);  
+      expect(testObject.get("subObject").key1).toBe("updatedValue");
+      expect(testObject.get("subObject").key2).toBe("value2");
+      expect(testObject.get("subObject").key3).toBeUndefined(); 
+      done();
+    }, (error) => {
+      done.fail(error);
+    });
+  });
 });