fix: Calling Parse.Object.relation.add multiple times adds only the first object (#2078)

Author: mstniy

integration/test/ParseRelationTest.js

@@ -43,6 +43,26 @@ describe('Parse Relation', () => {
       });
   });
 
+  it('can do consecutive adds (#2056)', async () => {
+    const ChildObject = Parse.Object.extend('ChildObject');
+    const childObjects = [];
+    for (let i = 0; i < 2; i++) {
+      childObjects.push(new ChildObject({ x: i }));
+    }
+    const parent = new Parse.Object('ParentObject');
+    await parent.save();
+    await Parse.Object.saveAll(childObjects);
+    for (const child of childObjects) {
+      parent.relation('child').add(child);
+    }
+    await parent.save();
+    const results = await parent.relation('child').query().find();
+    assert.equal(results.length, 2);
+    const expectedIds = new Set(childObjects.map(r => r.id));
+    const foundIds = new Set(results.map(r => r.id));
+    assert.deepEqual(expectedIds, foundIds);
+  });
+
   it('can query relation without schema', done => {
     const ChildObject = Parse.Object.extend('ChildObject');
     const childObjects = [];

src/ObjectStateMutations.js

@@ -82,16 +82,15 @@ export function mergeFirstPendingState(pendingOps: Array<OpsMap>) {
 export function estimateAttribute(
   serverData: AttributeMap,
   pendingOps: Array<OpsMap>,
-  className: string,
-  id: ?string,
+  object: ParseObject,
   attr: string
 ): mixed {
   let value = serverData[attr];
   for (let i = 0; i < pendingOps.length; i++) {
     if (pendingOps[i][attr]) {
       if (pendingOps[i][attr] instanceof RelationOp) {
-        if (id) {
-          value = pendingOps[i][attr].applyTo(value, { className: className, id: id }, attr);
+        if (object.id) {
+          value = pendingOps[i][attr].applyTo(value, object, attr);
         }
       } else {
         value = pendingOps[i][attr].applyTo(value);
@@ -104,8 +103,7 @@ export function estimateAttribute(
 export function estimateAttributes(
   serverData: AttributeMap,
   pendingOps: Array<OpsMap>,
-  className: string,
-  id: ?string
+  object: ParseObject
 ): AttributeMap {
   const data = {};
   let attr;
@@ -115,12 +113,8 @@ export function estimateAttributes(
   for (let i = 0; i < pendingOps.length; i++) {
     for (attr in pendingOps[i]) {
       if (pendingOps[i][attr] instanceof RelationOp) {
-        if (id) {
-          data[attr] = pendingOps[i][attr].applyTo(
-            data[attr],
-            { className: className, id: id },
-            attr
-          );
+        if (object.id) {
+          data[attr] = pendingOps[i][attr].applyTo(data[attr], object, attr);
         }
       } else {
         if (attr.includes('.')) {

src/ParseOp.js

@@ -336,19 +336,13 @@ export class RelationOp extends Op {
     return obj.id;
   }
 
-  applyTo(value: mixed, object?: { className: string, id: ?string }, key?: string): ?ParseRelation {
+  applyTo(value: mixed, parent: ParseObject, key?: string): ?ParseRelation {
     if (!value) {
-      if (!object || !key) {
+      if (!parent || !key) {
         throw new Error(
           'Cannot apply a RelationOp without either a previous value, or an object and a key'
         );
       }
-      const parent = new ParseObject(object.className);
-      if (object.id && object.id.indexOf('local') === 0) {
-        parent._localId = object.id;
-      } else if (object.id) {
-        parent.id = object.id;
-      }
       const relation = new ParseRelation(parent, key);
       relation.targetClassName = this._targetClassName;
       return relation;

src/SingleInstanceStateController.js

@@ -6,6 +6,7 @@ import * as ObjectStateMutations from './ObjectStateMutations';
 
 import type { Op } from './ParseOp';
 import type { AttributeMap, ObjectCache, OpsMap, State } from './ObjectStateMutations';
+import ParseObject from './ParseObject';
 
 type ObjectIdentifier = {
   className: string,
@@ -99,22 +100,16 @@ export function getObjectCache(obj: ObjectIdentifier): ObjectCache {
   return {};
 }
 
-export function estimateAttribute(obj: ObjectIdentifier, attr: string): mixed {
+export function estimateAttribute(obj: ParseObject, attr: string): mixed {
   const serverData = getServerData(obj);
   const pendingOps = getPendingOps(obj);
-  return ObjectStateMutations.estimateAttribute(
-    serverData,
-    pendingOps,
-    obj.className,
-    obj.id,
-    attr
-  );
+  return ObjectStateMutations.estimateAttribute(serverData, pendingOps, obj, attr);
 }
 
-export function estimateAttributes(obj: ObjectIdentifier): AttributeMap {
+export function estimateAttributes(obj: ParseObject): AttributeMap {
   const serverData = getServerData(obj);
   const pendingOps = getPendingOps(obj);
-  return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj.className, obj.id);
+  return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj);
 }
 
 export function commitServerChanges(obj: ObjectIdentifier, changes: AttributeMap) {

src/UniqueInstanceStateController.js

@@ -96,19 +96,13 @@ export function getObjectCache(obj: ParseObject): ObjectCache {
 export function estimateAttribute(obj: ParseObject, attr: string): mixed {
   const serverData = getServerData(obj);
   const pendingOps = getPendingOps(obj);
-  return ObjectStateMutations.estimateAttribute(
-    serverData,
-    pendingOps,
-    obj.className,
-    obj.id,
-    attr
-  );
+  return ObjectStateMutations.estimateAttribute(serverData, pendingOps, obj, attr);
 }
 
 export function estimateAttributes(obj: ParseObject): AttributeMap {
   const serverData = getServerData(obj);
   const pendingOps = getPendingOps(obj);
-  return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj.className, obj.id);
+  return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj);
 }
 
 export function commitServerChanges(obj: ParseObject, changes: AttributeMap) {

src/__tests__/ObjectStateMutations-test.js

@@ -81,13 +81,17 @@ describe('ObjectStateMutations', () => {
       ObjectStateMutations.estimateAttribute(
         serverData,
         pendingOps,
-        'someClass',
-        'someId',
+        { className: 'someClass', id: 'someId' },
         'counter'
       )
     ).toBe(14);
     expect(
-      ObjectStateMutations.estimateAttribute(serverData, pendingOps, 'someClass', 'someId', 'name')
+      ObjectStateMutations.estimateAttribute(
+        serverData,
+        pendingOps,
+        { className: 'someClass', id: 'someId' },
+        'name'
+      )
     ).toBe('foo');
 
     pendingOps.push({
@@ -98,21 +102,24 @@ describe('ObjectStateMutations', () => {
       ObjectStateMutations.estimateAttribute(
         serverData,
         pendingOps,
-        'someClass',
-        'someId',
+        { className: 'someClass', id: 'someId' },
         'counter'
       )
     ).toBe(15);
     expect(
-      ObjectStateMutations.estimateAttribute(serverData, pendingOps, 'someClass', 'someId', 'name')
+      ObjectStateMutations.estimateAttribute(
+        serverData,
+        pendingOps,
+        { className: 'someClass', id: 'someId' },
+        'name'
+      )
     ).toBe('override');
 
     pendingOps.push({ likes: new ParseOps.RelationOp([], []) });
     const relation = ObjectStateMutations.estimateAttribute(
       serverData,
       pendingOps,
-      'someClass',
-      'someId',
+      { className: 'someClass', id: 'someId' },
       'likes'
     );
     expect(relation.parent.id).toBe('someId');
@@ -142,12 +149,10 @@ describe('ObjectStateMutations', () => {
     });
 
     pendingOps.push({ likes: new ParseOps.RelationOp([], []) });
-    const attributes = ObjectStateMutations.estimateAttributes(
-      serverData,
-      pendingOps,
-      'someClass',
-      'someId'
-    );
+    const attributes = ObjectStateMutations.estimateAttributes(serverData, pendingOps, {
+      className: 'someClass',
+      id: 'someId',
+    });
     expect(attributes.likes.parent.id).toBe('someId');
     expect(attributes.likes.parent.className).toBe('someClass');
     expect(attributes.likes.key).toBe('likes');
@@ -206,7 +211,7 @@ describe('ObjectStateMutations', () => {
       'name.foo': 'bar',
       data: { count: 5 },
     });
-    expect(serverData).toEqual({ name: { foo: 'bar' }, data: { count: 5 }  });
+    expect(serverData).toEqual({ name: { foo: 'bar' }, data: { count: 5 } });
     expect(objectCache).toEqual({ data: '{"count":5}' });
   });
 

src/__tests__/ParseOp-test.js

@@ -30,17 +30,8 @@ jest.setMock('../ParseRelation', mockRelation);
 const ParseRelation = require('../ParseRelation');
 const ParseObject = require('../ParseObject');
 const ParseOp = require('../ParseOp');
-const {
-  Op,
-  SetOp,
-  UnsetOp,
-  IncrementOp,
-  AddOp,
-  AddUniqueOp,
-  RemoveOp,
-  RelationOp,
-  opFromJSON,
-} = ParseOp;
+const { Op, SetOp, UnsetOp, IncrementOp, AddOp, AddUniqueOp, RemoveOp, RelationOp, opFromJSON } =
+  ParseOp;
 
 describe('ParseOp', () => {
   it('base class', () => {
@@ -398,7 +389,11 @@ describe('ParseOp', () => {
     expect(rel.targetClassName).toBe('Item');
     expect(r2.applyTo(rel, { className: 'Delivery', id: 'D3' })).toBe(rel);
 
-    const relLocal = r.applyTo(undefined, { className: 'Delivery', id: 'localD4' }, 'shipments');
+    const relLocal = r.applyTo(
+      undefined,
+      { className: 'Delivery', _localId: 'localD4' },
+      'shipments'
+    );
     expect(relLocal.parent._localId).toBe('localD4');
 
     expect(r.applyTo.bind(r, 'string')).toThrow(