beforeSave request object changes ignored

[blastoy]

Issue Description

When defining a beforeSave hook, any changes done to the request object will not apply if the hook function returns true. For example:

Parse.Cloud.beforeSave('People', (request) => {
    request.object.set('name', 'John');
    return true;
});

If the return is omitted, then the beforeSave hook works just fine.

If returning a Promise that returns true, the beforeSave does not apply any changes either. For example:

Parse.Cloud.beforeSave('People', (request) => {
    request.object.set('name', 'John');
    return new Promise.resolve(true);
});

I haven't tried with returning other things like strings, but I'm assuming it would also cause the same issue.

An argument could be made that beforeSaves should not return any values anyway and that this kind of bug is a developer error. However, some of the code samples on the migration guide to parse-server 3.0 have beforeSave examples were returns are used. This may lead developers to think returning in beforeSave hooks is safe.

Personally I believe this bug should be fixed so that the beforeSaves returning do not affect the ability to make changes to the request object. If that is deemed to be unnecessary, then a proper warning / changes to the migration guide could be helpful. This kind of bug is pretty difficult to find if your beforeSaves have considerable amounts of logic in them (such as data validation and formatting), and I find it strange that cloud jobs / functions can return just fine while hooks cannot.

Steps to reproduce

1. Start a parse server 3.1.2.
2. Create a beforeSave hook on a random class: Foo.
3. Have the beforeSave hook modify the request object in some way: request.object.set('name', 'bar');.
4. At the end of the hook: return true.
5. Create a new parse object of the class through CURL or the dashboard, giving it a different name than bar such as: replace_me.

Expected Results

Modifications should go through and the Parse object. The object's name field should be bar if following the example above.

Actual Outcome

Modifications do not go through. The object's name stays as 'replace_me' if following the example above.

Environment Setup

• Server

  • parse-server version: 3.1.2
  • Operating System: Windows 10 Education
  • Hardware: i5-7500 3.40 GHz, 64 bit, 16 GB RAM
  • Localhost or remote server? Localhost server

• Database

  • MongoDB version: v3.6.5
  • Storage engine: GridFSBucketAdapter
  • Hardware: i5-7500 3.40 GHz, 64 bit, 16 GB RAM
  • Localhost or remote server? Localhost server

Logs/Trace

[10288] parse-server running on http://localhost:1337/parse

verbose: REQUEST for [POST] /parse/classes/Foo: {
  "name": "replace_me"
} method=POST, url=/parse/classes/Foo, host=localhost:1337, connection=keep-alive, content-length=165, origin=http://localhost:4040, user-agent=Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.80 Safari/537.36, content-type=text/plain, accept=*/*, referer=http://localhost:4040/apps/BluApple/browser/Foo, accept-encoding=gzip, deflate, br, accept-language=en-US,en;q=0.9, name=replace_me

info: beforeSave triggered for Foo for user undefined:
  Input: {"name":"bar"}
  Result: true className=Foo, triggerType=beforeSave, user=undefined

verbose: RESPONSE from [POST] /parse/classes/Foo: {
  "status": 201,
  "response": {
    "objectId": "Nqm02uklhY",
    "createdAt": "2018-12-06T14:54:00.051Z"
  },
  "location": "http://localhost:1337/parse/classes/Foo/Nqm02uklhY"
} status=201, objectId=Nqm02uklhY, createdAt=2018-12-06T14:54:00.051Z, location=http://localhost:1337/parse/classes/Foo/Nqm02uklhY

[flovilmart]

Would you be able to write a failing test?

[blastoy]

Hey @flovilmart,

@dplewis instructed me to copy/paste this test and add a return to the hook:

https://github.com/parse-community/parse-server/blob/master/spec/CloudCode.spec.js#L165

I did that and ran the tests and the test failed.

I don't know where to take it from there though, since I've never submitted a failing test. Do I branch and do a pull request?

[acinader]

NICE!!

Yes, fork parse-server, create a branch, add your test, push to your fork and make a pull request. here's step by step directions: https://git-scm.com/book/en/v2/GitHub-Contributing-to-a-Project

WELCOME!

[blastoy]

I just submitted the pull request. I wasn't too sure on the branch name (idk if there's conventions) but hopefully everything is in order.

@acinader thanks for your help!

[mtrezza]

Just want a moment to recognize @blastoy's example of a good issue description 👍

[parse-github-assistant]

The label type:feature cannot be used in combination with type:improvement.