Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Setting and unsetting property without saving Parse.Object causes server error #2117

Open
wants to merge 10 commits into
base: alpha
Choose a base branch
from
59 changes: 59 additions & 0 deletions integration/test/ParseObjectTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,44 @@ describe('Parse Object', () => {
assert.equal(result.get('objectField').string, 'hello');
});

it('can set and unset without save', async () => {
const obj = new TestObject({
objectField: {
number: 5,
string: 'hello',
},
});
obj.unset('objectField.number');
assert.equal(obj.get('objectField').number, undefined);
assert.equal(obj.get('objectField').string, 'hello');
await obj.save();

const query = new Parse.Query(TestObject);
const result = await query.get(obj.id);
assert.equal(result.get('objectField').number, undefined);
assert.equal(result.get('objectField').string, 'hello');
});

it('can set and set sub-property without save', async () => {
const obj = new TestObject({
objectField: {
number: 5,
string: 'hello',
},
});
obj.set('objectField.numberb', 4);
assert.equal(obj.get('objectField').number, 5);
assert.equal(obj.get('objectField').numberb, 4);
assert.equal(obj.get('objectField').string, 'hello');
await obj.save();

const query = new Parse.Query(TestObject);
const result = await query.get(obj.id);
assert.equal(result.get('objectField').number, 5);
assert.equal(result.get('objectField').numberb, 4);
assert.equal(result.get('objectField').string, 'hello');
});

it('can unset nested fields two levels', async () => {
const obj = new TestObject({
objectField: {
Expand All @@ -497,6 +535,27 @@ describe('Parse Object', () => {
assert.equal(result.get('objectField').string, 'hello');
});

it('can unset nested fields two levels - without save between', async () => {
const obj = new TestObject({
objectField: {
foo: {
bar: 5,
},
string: 'hello',
},
});

obj.unset('objectField.foo.bar');
assert.equal(obj.get('objectField').foo.bar, undefined);
assert.equal(obj.get('objectField').string, 'hello');
await obj.save();

const query = new Parse.Query(TestObject);
const result = await query.get(obj.id);
assert.equal(result.get('objectField').foo.bar, undefined);
assert.equal(result.get('objectField').string, 'hello');
});

it('can unset non existing fields', async () => {
const obj = new TestObject();
obj.set('objectField', { number: 5 });
Expand Down
12 changes: 10 additions & 2 deletions src/ParseObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
AddUniqueOp,
RemoveOp,
RelationOp,
validPendingParentOp,
applyOpToParent,
} from './ParseOp';
import ParseRelation from './ParseRelation';
import * as SingleInstanceStateController from './SingleInstanceStateController';
Expand Down Expand Up @@ -104,7 +106,7 @@
* @param {string} className The class name for the object
* @param {object} attributes The initial set of data to store in the object.
* @param {object} options The options for this object instance.
* @param {boolean} [options.ignoreValidation=false] Set to `true` ignore any attribute validation errors.

Check warning on line 109 in src/ParseObject.ts

View workflow job for this annotation

GitHub Actions / Lint

Defaults are not permitted on @param
*/
constructor(
className?: string | { className: string, [attr: string]: any },
Expand Down Expand Up @@ -779,8 +781,14 @@
const last = pendingOps.length - 1;
const stateController = CoreManager.getObjectStateController();
for (const attr in newOps) {
const nextOp = newOps[attr].mergeWith(pendingOps[last][attr]);
stateController.setPendingOp(this._getStateIdentifier(), attr, nextOp);
const parentAttr = validPendingParentOp(attr, pendingOps[last]);
if (parentAttr) {
const parentOp = pendingOps[last][parentAttr];
applyOpToParent(parentAttr, parentOp, attr, newOps[attr]);
} else {
const nextOp = newOps[attr].mergeWith(pendingOps[last][attr]);
stateController.setPendingOp(this._getStateIdentifier(), attr, nextOp);
}
}

return this;
Expand Down
50 changes: 50 additions & 0 deletions src/ParseOp.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,56 @@ export function opFromJSON(json: { [key: string]: any }): ?Op {
return null;
}

export function validPendingParentOp(attr, pendingOps) {
if (!pendingOps || pendingOps[attr]) {
return null;
}

const lastDot = attr.lastIndexOf('.');
if (lastDot === -1) {
return null;
}

// This is an object with dot notation. So need to also match "parents"
const parentString = attr.substring(0, lastDot);
for (const pendingAttr in pendingOps) {
if (parentString.startsWith(pendingAttr)) {
return pendingAttr;
}
}
}

export function applyOpToParent(parentAttr: string, parent: Op, attr: string, op: Op) {
const subAttr = attr.substring(parentAttr.length + 1);

if (!(parent instanceof SetOp) || !subAttr) {
throw new TypeError(`Trying to set sub property on a invalid property (${parentAttr} -> ${subAttr})`);
}

let object = parent._value;
const fields = subAttr.split('.');
const last = fields[fields.length - 1];
for (let i = 0; i < fields.length - 1; i++) {
const key = fields[i];
if (!(key in object)) {
if (op instanceof UnsetOp) {
// property already doesn't exist, we don't have to do anytihng
return;
}
object[key] = {};
}
object = object[key];
}

if (op instanceof UnsetOp) {
delete object[last];
} else {
object[last] = op.applyTo(object[last]);
}

return parent;
}

export class Op {
// Empty parent class
applyTo(value: any): any {} /* eslint-disable-line @typescript-eslint/no-unused-vars */
Expand Down
92 changes: 92 additions & 0 deletions src/__tests__/ParseObject-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,98 @@ describe('ParseObject', () => {
expect(o2.attributes).toEqual({});
});

it('can set sub property of a local changed object without creating an op', () => {
const o = new ParseObject('Person');
o.set('data', { a: 2 });
expect(Object.keys(o._getSaveJSON()).length).toBe(1);

o.set('datab', {v: 2});
expect(Object.keys(o._getSaveJSON()).length).toBe(2);

o.set('data.b', 3);
expect(Object.keys(o._getSaveJSON()).length).toBe(2);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2, b: 3});

o.set({"data.c" : 5, "data.d.a": 4});
expect(Object.keys(o._getSaveJSON()).length).toBe(2);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2, b: 3, c: 5, d: { a: 4 }});
});

it('can unset sub property of a local changed object without creating an op', () => {
const o = new ParseObject('Person');
o.set('data', { a: 2, b: 4 });
expect(Object.keys(o._getSaveJSON()).length).toBe(1);

o.unset('data.b');
expect(Object.keys(o._getSaveJSON()).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2});

o.unset('data.c');
expect(Object.keys(o._getPendingOps()[0]).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2});

o.unset('data.c.d');
expect(Object.keys(o._getSaveJSON()).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2});

o.set('data.b.c', 3);
expect(Object.keys(o._getSaveJSON()).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2, b: { c: 3 }});

o.unset('data.b.c');
expect(Object.keys(o._getSaveJSON()).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2, b: {}});

o.unset('data.b');
expect(Object.keys(o._getSaveJSON()).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 2});
});

it('can increment sub property of a local changed object without creating an op', () => {
const o = new ParseObject('Person');
o.set('data', {a: 2, b: 4});
expect(Object.keys(o._getSaveJSON()).length).toBe(1);

o.increment('data.a', 3);
expect(Object.keys(o._getSaveJSON()).length).toBe(1);
expect(o._getSaveJSON()['data']).toStrictEqual({ a: 5, b: 4});
});

it('collapse sub-property sets with parents as well', () => {
const o = new ParseObject('Person');
o._finishFetch({
objectId: 'o12312',
data: { a: 3 }
});
expect(o.dirty()).toBe(false);
expect(Object.keys(o._getSaveJSON()).length).toBe(0);

o.set('data.b', { c: 1 });
expect(Object.keys(o._getSaveJSON()).length).toBe(1);

o.set('data.boo', 4);
expect(Object.keys(o._getSaveJSON()).length).toBe(2);
expect(o._getSaveJSON()['data.boo']).toStrictEqual(4);

o.set('data.b.c', 2);
expect(Object.keys(o._getSaveJSON()).length).toBe(2);
expect(o._getSaveJSON()['data.b']).toStrictEqual({ c: 2 });
});

it('throw exception on non-sensical parent (not set)', async () => {
const o = new ParseObject('Person');
o.increment('data', 2);
expect(Object.keys(o._getSaveJSON()).length).toBe(1);

try {
o.set('data.a', 3);
expect(true).toBe(false);
} catch (e) {
expect(e.message).toBe('Trying to set sub property on a invalid property (data -> a)');
}

});

it('can clear all fields', () => {
const o = new ParseObject('Person');
o._finishFetch({
Expand Down