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: return correct response when revert is used in beforeSave #7839

Merged
merged 18 commits into from
Mar 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 134 additions & 0 deletions spec/CloudCode.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,110 @@ describe('Cloud Code', () => {
});
});

it('before save can revert fields', async () => {
Parse.Cloud.beforeSave('TestObject', ({ object }) => {
object.revert('foo');
return object;
});

Parse.Cloud.afterSave('TestObject', ({ object }) => {
expect(object.get('foo')).toBeUndefined();
return object;
});

const obj = new TestObject();
obj.set('foo', 'bar');
await obj.save();

expect(obj.get('foo')).toBeUndefined();
await obj.fetch();

expect(obj.get('foo')).toBeUndefined();
});

it('before save can revert fields with existing object', async () => {
Parse.Cloud.beforeSave(
'TestObject',
({ object }) => {
object.revert('foo');
return object;
},
{
skipWithMasterKey: true,
}
);

Parse.Cloud.afterSave(
'TestObject',
({ object }) => {
expect(object.get('foo')).toBe('bar');
return object;
},
{
skipWithMasterKey: true,
}
);

const obj = new TestObject();
obj.set('foo', 'bar');
await obj.save(null, { useMasterKey: true });

expect(obj.get('foo')).toBe('bar');
obj.set('foo', 'yolo');
await obj.save();
expect(obj.get('foo')).toBe('bar');
});

it('can unset in afterSave', async () => {
Parse.Cloud.beforeSave('TestObject', ({ object }) => {
if (!object.existed()) {
object.set('secret', true);
return object;
}
object.revert('secret');
});

Parse.Cloud.afterSave('TestObject', ({ object }) => {
object.unset('secret');
});

Parse.Cloud.beforeFind(
'TestObject',
({ query }) => {
query.exclude('secret');
},
{
skipWithMasterKey: true,
}
);

const obj = new TestObject();
await obj.save();
expect(obj.get('secret')).toBeUndefined();
await obj.fetch();
expect(obj.get('secret')).toBeUndefined();
await obj.fetch({ useMasterKey: true });
expect(obj.get('secret')).toBe(true);
});

it('should revert in beforeSave', async () => {
Parse.Cloud.beforeSave('MyObject', ({ object }) => {
if (!object.existed()) {
object.set('count', 0);
return object;
}
object.revert('count');
return object;
});
const obj = await new Parse.Object('MyObject').save();
expect(obj.get('count')).toBe(0);
obj.set('count', 10);
await obj.save();
expect(obj.get('count')).toBe(0);
await obj.fetch();
expect(obj.get('count')).toBe(0);
});

it('beforeSave should not sanitize database', async done => {
const { adapter } = Config.get(Parse.applicationId).database;
const spy = spyOn(adapter, 'findOneAndUpdate').and.callThrough();
Expand Down Expand Up @@ -1860,6 +1964,36 @@ describe('afterSave hooks', () => {
const myObject = new MyObject();
myObject.save().then(() => done());
});

it('should unset in afterSave', async () => {
Parse.Cloud.afterSave(
'MyObject',
({ object }) => {
object.unset('secret');
},
{
skipWithMasterKey: true,
}
);
const obj = new Parse.Object('MyObject');
obj.set('secret', 'bar');
await obj.save();
expect(obj.get('secret')).toBeUndefined();
await obj.fetch();
expect(obj.get('secret')).toBe('bar');
});

it('should unset', async () => {
Parse.Cloud.beforeSave('MyObject', ({ object }) => {
object.set('secret', 'hidden');
});

Parse.Cloud.afterSave('MyObject', ({ object }) => {
object.unset('secret');
});
const obj = await new Parse.Object('MyObject').save();
expect(obj.get('secret')).toBeUndefined();
});
});

describe('beforeDelete hooks', () => {
Expand Down
59 changes: 31 additions & 28 deletions src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ function RestWrite(config, auth, className, query, data, originalData, clientSDK
// Shared SchemaController to be reused to reduce the number of loadSchema() calls per request
// Once set the schemaData should be immutable
this.validSchemaController = null;
this.pendingOps = {};
}

// A convenient method to perform all the steps of processing the
Expand Down Expand Up @@ -225,18 +226,11 @@ RestWrite.prototype.runBeforeSaveTrigger = function () {
return Promise.resolve();
}

// Cloud code gets a bit of extra data for its objects
var extraData = { className: this.className };
if (this.query && this.query.objectId) {
extraData.objectId = this.query.objectId;
}
const { originalObject, updatedObject } = this.buildParseObjects();

let originalObject = null;
const updatedObject = this.buildUpdatedObject(extraData);
if (this.query && this.query.objectId) {
// This is an update for existing object.
originalObject = triggers.inflate(extraData, this.originalData);
}
const stateController = Parse.CoreManager.getObjectStateController();
const [pending] = stateController.getPendingOps(updatedObject._getStateIdentifier());
this.pendingOps = { ...pending };

return Promise.resolve()
.then(() => {
Expand Down Expand Up @@ -1531,20 +1525,7 @@ RestWrite.prototype.runAfterSaveTrigger = function () {
return Promise.resolve();
}

var extraData = { className: this.className };
if (this.query && this.query.objectId) {
extraData.objectId = this.query.objectId;
}

// Build the original object, we only do this for a update write.
let originalObject;
if (this.query && this.query.objectId) {
originalObject = triggers.inflate(extraData, this.originalData);
}

// Build the inflated object, different from beforeSave, originalData is not empty
// since developers can change data in the beforeSave.
const updatedObject = this.buildUpdatedObject(extraData);
const { originalObject, updatedObject } = this.buildParseObjects();
updatedObject._handleSaveResponse(this.response.response, this.response.status || 200);

this.config.database.loadSchema().then(schemaController => {
Expand All @@ -1569,8 +1550,15 @@ RestWrite.prototype.runAfterSaveTrigger = function () {
this.context
)
.then(result => {
if (result && typeof result === 'object') {
const jsonReturned = result && !result._toFullJSON;
if (jsonReturned) {
this.pendingOps = {};
this.response.response = result;
} else {
this.response.response = this._updateResponseWithData(
(result || updatedObject)._toFullJSON(),
this.data
);
}
})
.catch(function (err) {
Expand Down Expand Up @@ -1604,7 +1592,13 @@ RestWrite.prototype.sanitizedData = function () {
};

// Returns an updated copy of the object
RestWrite.prototype.buildUpdatedObject = function (extraData) {
RestWrite.prototype.buildParseObjects = function () {
const extraData = { className: this.className, objectId: this.query?.objectId };
let originalObject;
if (this.query && this.query.objectId) {
originalObject = triggers.inflate(extraData, this.originalData);
}

const className = Parse.Object.fromJSON(extraData);
const readOnlyAttributes = className.constructor.readOnlyAttributes
? className.constructor.readOnlyAttributes()
Expand Down Expand Up @@ -1642,7 +1636,7 @@ RestWrite.prototype.buildUpdatedObject = function (extraData) {
delete sanitized[attribute];
}
updatedObject.set(sanitized);
return updatedObject;
return { updatedObject, originalObject };
};

RestWrite.prototype.cleanUserAuthData = function () {
Expand All @@ -1662,6 +1656,15 @@ RestWrite.prototype.cleanUserAuthData = function () {
};

RestWrite.prototype._updateResponseWithData = function (response, data) {
const { updatedObject } = this.buildParseObjects();
const stateController = Parse.CoreManager.getObjectStateController();
const [pending] = stateController.getPendingOps(updatedObject._getStateIdentifier());
for (const key in this.pendingOps) {
if (!pending[key]) {
data[key] = this.originalData ? this.originalData[key] : { __op: 'Delete' };
this.storage.fieldsChangedByTrigger.push(key);
}
}
if (_.isEmpty(this.storage.fieldsChangedByTrigger)) {
return response;
}
Expand Down