Bug/server 6899/embedded document increment (#1219)

* Add further complexity to test cases when documents changed

A bug has been identified when we attempt to increment a value in an embedded
document.  This could potentially be missed by the current tests because they
only have a document with a single value and that is the value that is updated.

This commit adds a second property to the embedded document and ensures it is
still present after the various function calls

* Add tests that demonstrate the issue with incrementing embedded doc

Once the object is saved, the response from the save only returns the updated
properties on the embedded document.  This is then used to REPLACE the embedded
document instead of being merged with it.  This is incorrect if the operation
was an Increment/Decrement/Add

* Attempt to resolve embedded document increment bug

This change resolves the system when we use it in our code... however it does
NOT result in the new tests that were created passing.  I am a little confused
by that.

* Adjust handleSaveResponse conditional handling.

Only certain types of objects should be merged using Javascript spread operator.
Previous attempt was including Dates... which fails

* _handleSaveResponse: Merge response value with original attributes for nested objects

Co-authored-by: Diamond Lewis <[email protected]>
Co-authored-by: Robby Helms <[email protected]>

Author: 3 people

integration/test/ParseObjectTest.js

@@ -255,6 +255,27 @@ describe('Parse Object', () => {
     assert.equal(result.get('objectField').number, 20);
   });
 
+  it('can increment nested field and retain full object', async () => {
+    const obj = new TestObject();
+    obj.set('objectField', { number: 5, letter: 'a' });
+    assert.equal(obj.get('objectField').number, 5);
+    assert.equal(obj.get('objectField').letter, 'a');
+    await obj.save();
+
+    obj.increment('objectField.number', 15);
+    assert.equal(obj.get('objectField').number, 20);
+    assert.equal(obj.get('objectField').letter, 'a');
+    await obj.save();
+
+    assert.equal(obj.get('objectField').number, 20);
+    assert.equal(obj.get('objectField').letter, 'a');
+
+    const query = new Parse.Query(TestObject);
+    const result = await query.get(obj.id);
+    assert.equal(result.get('objectField').number, 20);
+    assert.equal(result.get('objectField').letter, 'a');
+  });
+
   it('can increment non existing field', async () => {
     const obj = new TestObject();
     obj.set('objectField', { number: 5 });

src/ParseObject.js

@@ -395,7 +395,7 @@ class ParseObject {
     for (attr in pending) {
       if (pending[attr] instanceof RelationOp) {
         changes[attr] = pending[attr].applyTo(undefined, this, attr);
-      } else if (!(attr in response)) {
+      } else if (!(attr in response) && !attr.includes('.')) {
         // Only SetOps and UnsetOps should not come back with results
         changes[attr] = pending[attr].applyTo(undefined);
       }
@@ -407,7 +407,13 @@ class ParseObject {
       } else if (attr === 'ACL') {
         changes[attr] = new ParseACL(response[attr]);
       } else if (attr !== 'objectId') {
-        changes[attr] = decode(response[attr]);
+        const val = decode(response[attr]);
+        if (val && Object.getPrototypeOf(val) === Object.prototype) {
+          // Update the object by merging in updates w/ old object
+          changes[attr] = { ...this.attributes[attr], ...val }
+        } else {
+          changes[attr] = val
+        }
         if (changes[attr] instanceof UnsetOp) {
           changes[attr] = undefined;
         }

src/__tests__/ObjectStateMutations-test.js

@@ -120,17 +120,19 @@ describe('ObjectStateMutations', () => {
   });
 
   it('can estimate attributes for nested documents', () => {
-    const serverData = { objectField: { counter: 10 } };
+    const serverData = { objectField: { counter: 10, letter: 'a' } };
     let pendingOps = [{ 'objectField.counter': new ParseOps.IncrementOp(2) }];
     expect(ObjectStateMutations.estimateAttributes(serverData, pendingOps, 'someClass', 'someId')).toEqual({
       objectField: {
-        counter: 12
+        counter: 12,
+        letter: 'a'
       },
     });
     pendingOps = [{ 'objectField.counter': new ParseOps.SetOp(20) }];
     expect(ObjectStateMutations.estimateAttributes(serverData, pendingOps, 'someClass', 'someId')).toEqual({
       objectField: {
-        counter: 20
+        counter: 20,
+        letter: 'a'
       },
     });
   });

src/__tests__/ParseObject-test.js

@@ -610,20 +610,21 @@ describe('ParseObject', () => {
     o._finishFetch({
       objectId: 'setNested',
       objectField: {
-        number: 5
+        number: 5,
+        letter: 'a'
       },
       otherField: {},
     });
 
     expect(o.attributes).toEqual({
-      objectField: { number: 5 },
+      objectField: { number: 5, letter: 'a' },
       otherField: {},
     });
     o.set('otherField', { hello: 'world' });
     o.set('objectField.number', 20);
 
     expect(o.attributes).toEqual({
-      objectField: { number: 20 },
+      objectField: { number: 20, letter: 'a' },
       otherField: { hello: 'world' },
     });
     expect(o.op('objectField.number') instanceof SetOp).toBe(true);
@@ -634,6 +635,44 @@ describe('ParseObject', () => {
     });
   });
 
+  it('can increment a nested field', () => {
+    const o = new ParseObject('Person');
+    o._finishFetch({
+      objectId: 'incNested',
+      objectField: {
+        number: 5,
+        letter: 'a'
+      },
+    });
+
+    expect(o.attributes).toEqual({
+      objectField: { number: 5, letter: 'a' },
+    });
+    o.increment('objectField.number');
+
+    expect(o.attributes).toEqual({
+      objectField: { number: 6, letter: 'a' },
+    });
+    expect(o.op('objectField.number') instanceof IncrementOp).toBe(true);
+    expect(o.dirtyKeys()).toEqual(['objectField.number', 'objectField']);
+    expect(o._getSaveJSON()).toEqual({
+      'objectField.number': {
+        "__op": "Increment",
+        "amount": 1
+      },
+    });
+
+    // Nested objects only return values changed
+    o._handleSaveResponse({
+      objectId: 'incNested',
+      objectField: {
+        number: 6
+      }
+    })
+    expect(o.get('objectField').number).toEqual(6)
+    expect(o.get('objectField').letter).toEqual('a')
+  });
+
   it('ignore set nested field on new object', () => {
     const o = new ParseObject('Person');
     o.set('objectField.number', 20);