Skip to content

Commit 5529850

Browse files
mstniymstniy
committed
fix: relation double-add bug (#2056), new approach
1 parent 64fdfb2 commit 5529850

7 files changed

+96
-98
lines changed

‎integration/test/ParseRelationTest.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,26 @@ describe('Parse Relation', () => {
4343
});
4444
});
4545

46+
it('can do consecutive adds (#2056)', async () => {
47+
const ChildObject = Parse.Object.extend('ChildObject');
48+
const childObjects = [];
49+
for (let i = 0; i < 2; i++) {
50+
childObjects.push(new ChildObject({ x: i }));
51+
}
52+
const parent = new Parse.Object('ParentObject');
53+
await parent.save();
54+
await Parse.Object.saveAll(childObjects);
55+
for (const child of childObjects) {
56+
parent.relation('child').add(child);
57+
}
58+
await parent.save();
59+
const results = await parent.relation('child').query().find();
60+
assert.equal(results.length, 2);
61+
const expectedIds = new Set(childObjects.map(r => r.id));
62+
const foundIds = new Set(results.map(r => r.id));
63+
assert.deepEqual(expectedIds, foundIds);
64+
});
65+
4666
it('can query relation without schema', done => {
4767
const ChildObject = Parse.Object.extend('ChildObject');
4868
const childObjects = [];

‎src/ObjectStateMutations.js

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,15 @@ export function mergeFirstPendingState(pendingOps: Array<OpsMap>) {
8282
export function estimateAttribute(
8383
serverData: AttributeMap,
8484
pendingOps: Array<OpsMap>,
85-
className: string,
86-
id: ?string,
85+
object: ParseObject,
8786
attr: string
8887
): mixed {
8988
let value = serverData[attr];
9089
for (let i = 0; i < pendingOps.length; i++) {
9190
if (pendingOps[i][attr]) {
9291
if (pendingOps[i][attr] instanceof RelationOp) {
93-
if (id) {
94-
value = pendingOps[i][attr].applyTo(value, { className: className, id: id }, attr);
92+
if (object.id) {
93+
value = pendingOps[i][attr].applyTo(value, object, attr);
9594
}
9695
} else {
9796
value = pendingOps[i][attr].applyTo(value);
@@ -104,8 +103,7 @@ export function estimateAttribute(
104103
export function estimateAttributes(
105104
serverData: AttributeMap,
106105
pendingOps: Array<OpsMap>,
107-
className: string,
108-
id: ?string
106+
object: ParseObject
109107
): AttributeMap {
110108
const data = {};
111109
let attr;
@@ -115,12 +113,8 @@ export function estimateAttributes(
115113
for (let i = 0; i < pendingOps.length; i++) {
116114
for (attr in pendingOps[i]) {
117115
if (pendingOps[i][attr] instanceof RelationOp) {
118-
if (id) {
119-
data[attr] = pendingOps[i][attr].applyTo(
120-
data[attr],
121-
{ className: className, id: id },
122-
attr
123-
);
116+
if (object.id) {
117+
data[attr] = pendingOps[i][attr].applyTo(data[attr], object, attr);
124118
}
125119
} else {
126120
if (attr.includes('.')) {

‎src/ParseOp.js

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,42 +14,42 @@ export function opFromJSON(json: { [key: string]: any }): ?Op {
1414
return null;
1515
}
1616
switch (json.__op) {
17-
case 'Delete':
18-
return new UnsetOp();
19-
case 'Increment':
20-
return new IncrementOp(json.amount);
21-
case 'Add':
22-
return new AddOp(decode(json.objects));
23-
case 'AddUnique':
24-
return new AddUniqueOp(decode(json.objects));
25-
case 'Remove':
26-
return new RemoveOp(decode(json.objects));
27-
case 'AddRelation': {
28-
const toAdd = decode(json.objects);
29-
if (!Array.isArray(toAdd)) {
30-
return new RelationOp([], []);
31-
}
32-
return new RelationOp(toAdd, []);
33-
}
34-
case 'RemoveRelation': {
35-
const toRemove = decode(json.objects);
36-
if (!Array.isArray(toRemove)) {
37-
return new RelationOp([], []);
17+
case 'Delete':
18+
return new UnsetOp();
19+
case 'Increment':
20+
return new IncrementOp(json.amount);
21+
case 'Add':
22+
return new AddOp(decode(json.objects));
23+
case 'AddUnique':
24+
return new AddUniqueOp(decode(json.objects));
25+
case 'Remove':
26+
return new RemoveOp(decode(json.objects));
27+
case 'AddRelation': {
28+
const toAdd = decode(json.objects);
29+
if (!Array.isArray(toAdd)) {
30+
return new RelationOp([], []);
31+
}
32+
return new RelationOp(toAdd, []);
3833
}
39-
return new RelationOp([], toRemove);
40-
}
41-
case 'Batch': {
42-
let toAdd = [];
43-
let toRemove = [];
44-
for (let i = 0; i < json.ops.length; i++) {
45-
if (json.ops[i].__op === 'AddRelation') {
46-
toAdd = toAdd.concat(decode(json.ops[i].objects));
47-
} else if (json.ops[i].__op === 'RemoveRelation') {
48-
toRemove = toRemove.concat(decode(json.ops[i].objects));
34+
case 'RemoveRelation': {
35+
const toRemove = decode(json.objects);
36+
if (!Array.isArray(toRemove)) {
37+
return new RelationOp([], []);
4938
}
39+
return new RelationOp([], toRemove);
40+
}
41+
case 'Batch': {
42+
let toAdd = [];
43+
let toRemove = [];
44+
for (let i = 0; i < json.ops.length; i++) {
45+
if (json.ops[i].__op === 'AddRelation') {
46+
toAdd = toAdd.concat(decode(json.ops[i].objects));
47+
} else if (json.ops[i].__op === 'RemoveRelation') {
48+
toRemove = toRemove.concat(decode(json.ops[i].objects));
49+
}
50+
}
51+
return new RelationOp(toAdd, toRemove);
5052
}
51-
return new RelationOp(toAdd, toRemove);
52-
}
5353
}
5454
return null;
5555
}
@@ -336,19 +336,13 @@ export class RelationOp extends Op {
336336
return obj.id;
337337
}
338338

339-
applyTo(value: mixed, object?: { className: string, id: ?string }, key?: string): ?ParseRelation {
339+
applyTo(value: mixed, parent: ParseObject, key?: string): ?ParseRelation {
340340
if (!value) {
341-
if (!object || !key) {
341+
if (!parent || !key) {
342342
throw new Error(
343343
'Cannot apply a RelationOp without either a previous value, or an object and a key'
344344
);
345345
}
346-
const parent = new ParseObject(object.className);
347-
if (object.id && object.id.indexOf('local') === 0) {
348-
parent._localId = object.id;
349-
} else if (object.id) {
350-
parent.id = object.id;
351-
}
352346
const relation = new ParseRelation(parent, key);
353347
relation.targetClassName = this._targetClassName;
354348
return relation;

‎src/SingleInstanceStateController.js

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as ObjectStateMutations from './ObjectStateMutations';
66

77
import type { Op } from './ParseOp';
88
import type { AttributeMap, ObjectCache, OpsMap, State } from './ObjectStateMutations';
9+
import ParseObject from './ParseObject';
910

1011
type ObjectIdentifier = {
1112
className: string,
@@ -99,22 +100,16 @@ export function getObjectCache(obj: ObjectIdentifier): ObjectCache {
99100
return {};
100101
}
101102

102-
export function estimateAttribute(obj: ObjectIdentifier, attr: string): mixed {
103+
export function estimateAttribute(obj: ParseObject, attr: string): mixed {
103104
const serverData = getServerData(obj);
104105
const pendingOps = getPendingOps(obj);
105-
return ObjectStateMutations.estimateAttribute(
106-
serverData,
107-
pendingOps,
108-
obj.className,
109-
obj.id,
110-
attr
111-
);
106+
return ObjectStateMutations.estimateAttribute(serverData, pendingOps, obj, attr);
112107
}
113108

114-
export function estimateAttributes(obj: ObjectIdentifier): AttributeMap {
109+
export function estimateAttributes(obj: ParseObject): AttributeMap {
115110
const serverData = getServerData(obj);
116111
const pendingOps = getPendingOps(obj);
117-
return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj.className, obj.id);
112+
return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj);
118113
}
119114

120115
export function commitServerChanges(obj: ObjectIdentifier, changes: AttributeMap) {

‎src/UniqueInstanceStateController.js

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,19 +96,13 @@ export function getObjectCache(obj: ParseObject): ObjectCache {
9696
export function estimateAttribute(obj: ParseObject, attr: string): mixed {
9797
const serverData = getServerData(obj);
9898
const pendingOps = getPendingOps(obj);
99-
return ObjectStateMutations.estimateAttribute(
100-
serverData,
101-
pendingOps,
102-
obj.className,
103-
obj.id,
104-
attr
105-
);
99+
return ObjectStateMutations.estimateAttribute(serverData, pendingOps, obj, attr);
106100
}
107101

108102
export function estimateAttributes(obj: ParseObject): AttributeMap {
109103
const serverData = getServerData(obj);
110104
const pendingOps = getPendingOps(obj);
111-
return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj.className, obj.id);
105+
return ObjectStateMutations.estimateAttributes(serverData, pendingOps, obj);
112106
}
113107

114108
export function commitServerChanges(obj: ParseObject, changes: AttributeMap) {

‎src/__tests__/ObjectStateMutations-test.js

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mockObject.registerSubclass = function () {};
1414
jest.setMock('../ParseObject', mockObject);
1515

1616
const ObjectStateMutations = require('../ObjectStateMutations');
17+
const { default: ParseObject } = require('../ParseObject');
1718
const ParseOps = require('../ParseOp');
1819
const TaskQueue = require('../TaskQueue');
1920

@@ -81,13 +82,17 @@ describe('ObjectStateMutations', () => {
8182
ObjectStateMutations.estimateAttribute(
8283
serverData,
8384
pendingOps,
84-
'someClass',
85-
'someId',
85+
{ className: 'someClass', id: 'someId' },
8686
'counter'
8787
)
8888
).toBe(14);
8989
expect(
90-
ObjectStateMutations.estimateAttribute(serverData, pendingOps, 'someClass', 'someId', 'name')
90+
ObjectStateMutations.estimateAttribute(
91+
serverData,
92+
pendingOps,
93+
{ className: 'someClass', id: 'someId' },
94+
'name'
95+
)
9196
).toBe('foo');
9297

9398
pendingOps.push({
@@ -98,21 +103,24 @@ describe('ObjectStateMutations', () => {
98103
ObjectStateMutations.estimateAttribute(
99104
serverData,
100105
pendingOps,
101-
'someClass',
102-
'someId',
106+
{ className: 'someClass', id: 'someId' },
103107
'counter'
104108
)
105109
).toBe(15);
106110
expect(
107-
ObjectStateMutations.estimateAttribute(serverData, pendingOps, 'someClass', 'someId', 'name')
111+
ObjectStateMutations.estimateAttribute(
112+
serverData,
113+
pendingOps,
114+
{ className: 'someClass', id: 'someId' },
115+
'name'
116+
)
108117
).toBe('override');
109118

110119
pendingOps.push({ likes: new ParseOps.RelationOp([], []) });
111120
const relation = ObjectStateMutations.estimateAttribute(
112121
serverData,
113122
pendingOps,
114-
'someClass',
115-
'someId',
123+
{ className: 'someClass', id: 'someId' },
116124
'likes'
117125
);
118126
expect(relation.parent.id).toBe('someId');
@@ -142,12 +150,10 @@ describe('ObjectStateMutations', () => {
142150
});
143151

144152
pendingOps.push({ likes: new ParseOps.RelationOp([], []) });
145-
const attributes = ObjectStateMutations.estimateAttributes(
146-
serverData,
147-
pendingOps,
148-
'someClass',
149-
'someId'
150-
);
153+
const attributes = ObjectStateMutations.estimateAttributes(serverData, pendingOps, {
154+
className: 'someClass',
155+
id: 'someId',
156+
});
151157
expect(attributes.likes.parent.id).toBe('someId');
152158
expect(attributes.likes.parent.className).toBe('someClass');
153159
expect(attributes.likes.key).toBe('likes');
@@ -206,7 +212,7 @@ describe('ObjectStateMutations', () => {
206212
'name.foo': 'bar',
207213
data: { count: 5 },
208214
});
209-
expect(serverData).toEqual({ name: { foo: 'bar' }, data: { count: 5 } });
215+
expect(serverData).toEqual({ name: { foo: 'bar' }, data: { count: 5 } });
210216
expect(objectCache).toEqual({ data: '{"count":5}' });
211217
});
212218

‎src/__tests__/ParseOp-test.js

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,8 @@ jest.setMock('../ParseRelation', mockRelation);
3030
const ParseRelation = require('../ParseRelation');
3131
const ParseObject = require('../ParseObject');
3232
const ParseOp = require('../ParseOp');
33-
const {
34-
Op,
35-
SetOp,
36-
UnsetOp,
37-
IncrementOp,
38-
AddOp,
39-
AddUniqueOp,
40-
RemoveOp,
41-
RelationOp,
42-
opFromJSON,
43-
} = ParseOp;
33+
const { Op, SetOp, UnsetOp, IncrementOp, AddOp, AddUniqueOp, RemoveOp, RelationOp, opFromJSON } =
34+
ParseOp;
4435

4536
describe('ParseOp', () => {
4637
it('base class', () => {
@@ -398,7 +389,11 @@ describe('ParseOp', () => {
398389
expect(rel.targetClassName).toBe('Item');
399390
expect(r2.applyTo(rel, { className: 'Delivery', id: 'D3' })).toBe(rel);
400391

401-
const relLocal = r.applyTo(undefined, { className: 'Delivery', id: 'localD4' }, 'shipments');
392+
const relLocal = r.applyTo(
393+
undefined,
394+
{ className: 'Delivery', _localId: 'localD4' },
395+
'shipments'
396+
);
402397
expect(relLocal.parent._localId).toBe('localD4');
403398

404399
expect(r.applyTo.bind(r, 'string')).toThrow(

0 commit comments

Comments
 (0)