I was using parse.com and had written a cloud code function that worked perfectly. When I moved to a self hosted parse-server backend, some of the cloud code function stopped working.
Parse.Cloud.afterSave("League", function (request) {
if (request.object.get("leaderboard") == null) {
var leaderboard = Parse.Object.extend("Leaderboard");
var newInstance = new leaderboard();
newInstance.save(null , {useMasterKey: true})
.then(function (result) {
request.object.set("leaderboard", result);
request.object.save(null ,{useMasterKey: true});
},
function (error) {
console.log("Error");
});
});
}else{
var membersRelation = request.object.relation("members");
var membersQuery = membersRelation.query();
membersQuery.count(null , {useMasterKey: true})
.then(function (totalNumber) {
request.object.set("memberCount", totalNumber)
request.object.save(null ,{useMasterKey: true});
}, function (error) {
console.log("Error")
})
}
As you can see, I define the afterSave hook for the League class. In my hook, I have to update the same object again when I set a new value (leaderboard and/or membersCount) so save is called save more than once.
The function saves data properly, but it causes an infinite loop too. I understand it happens because I call request.object.save() that will change the League class again so the afterSave event fires again, and so on. I don't know how I can handle this condition. Someone suggested to me to add a timeout but not sure how. Can you please help with a solution to this problem.
Thanks
Your approach has two problems:
There is a race condition on leaderboard. When the promise of the first save resolves, there will be no leaderboard and then it'll magically be there "at some point in the future". Much better to set the initial value in a beforeSave so that the state of league is known and predictable.
There is also a race condition on membersCount. Imagine that two updates adding and/or removing members comes in at the same time. Between when you read the relations and write the count, an other update could occur. You could end up with the wrong count, or even a negative number!
To solve 1, we simply move the creation of the leaderboard into a beforeSave. To solve 2 we move the calculation of the membersCount into the beforeSave, use the supplied information on the dirty object about member adds and subtracts and finally we use increment to make sure that the update is atomic and avoid a race condition.
Below is working code with a unit test. Note that if I were doing my own code review of this, I would a) want to test adding multiple and subtracting multiple members b) split the big first test into multiple tests where only one thing is tested per test. c) test adding and removing in the same save.
I'm using es6 constructs cause I like them ;).
Trying to put in lots of comments, but feel free to ask me if something is confusing.
PS If you don't know how to do and run unit tests on your cloud code, ask another question, cause it is invaluable for figuring out how this stuff works (and looking at the parse-server unit tests is the best documentation you'll find)
Good luck!
const addLeaderboard = function addLeaderboard(league) {
// note the simplified object creation without using extends.
return new Parse.Object('Leaderboard')
// I was surprised to find that I had to save the new leaderboard
// before saving the league. too bad & unit tests ftw.
.save(null, { useMasterKey: true })
// "fat arrow" function declaration. If there's only a single
// line in the function and you don't use {} then the result
// of that line is the return value. cool!
.then(leaderboard => league.set('leaderboard', leaderboard));
}
const leagueBeforeSave = function leagueBeforeSave(request, response) {
// Always prefer immutability to avoid bugs!
const league = request.object;
if (league.op('members')) {
// Using a debugger to see what is available on the league
// is super helpful, cause I have never seen this stuff
// documented, but its obvious in a debugger.
const membersAdded = league.op('members').relationsToAdd.length;
const membersRemoved = league.op('members').relationsToRemove.length;
const membersChange = membersAdded - membersRemoved;
if (membersChange !== 0) {
// by setting increment when the save is done, the
// change in this value will be atomic. By using a change
// in the value rather than an absolute number
// we avoid a race condition when paired with the atomicity of increment
league.increment('membersCount', membersChange);
}
}
if (!league.get('leaderboard')) {
// notice we don't have to save the league, we just
// add the leaderboard. When we call success, the league
// will be saved and the leaderboard will be there....
addLeaderboard(league)
.then(() => response.success(league))
.catch(response.error);
} else {
response.success(league);
}
};
// The rest of this is just to test our beforeSave hook.
describe('league save logic', () => {
beforeEach(() => {
Parse.Cloud.beforeSave('League', leagueBeforeSave);
});
it('should create league and increment properly', (done) => {
Parse.Promise.when([
new Parse.Object('Member').save(),
new Parse.Object('Member').save(),
new Parse.Object('Member').save(),
new Parse.Object('Member').save(),
])
.then((members) => {
const league = new Parse.Object('League');
const memberRelation = league.relation('members');
memberRelation.add(members);
// I want to use members in the next promise block,
// there are a number of ways to do this, but I like
// passing the value this way. See Parse.Promise.when
// doc if this is mysterious.
return Parse.Promise.when(
league.save(null, { useMasterKey: true }),
members);
})
.then((league, members) => {
expect(league.get('leaderboard').className).toBe('Leaderboard');
expect(league.get('membersCount')).toBe(4);
const memberRelation = league.relation('members');
memberRelation.remove(members[0]);
return league.save(null, { useMasterKey: true });
})
.then((league) => {
expect(league.get('membersCount')).toBe(3);
// just do a save with no change to members to make sure
// we don't have something that breaks in that case...
return league
.set('foo', 'bar')
.save(null, { useMasterKey: true })
})
.then(league => {
expect(league.get('foo')).toBe('bar');
done();
})
.catch(done.fail);
});
it('should work to create new without any members too', (done) => {
new Parse.Object('League')
.save() // we don't really need the useMasterKey in unit tests unless we setup `acl`s..:).
.then((league) => {
expect(league.get('leaderboard').className).toBe('Leaderboard');
done();
})
.catch(done.fail);
});
});