Validation Handler Update

[dblythy]

A bit to unpack here, let me know if you'd like it separated into multiple PRs.

Update Parse.Cloud Validation Handlers

This updates validation handlers to allow async, and removes the requirement for a return value on the validation handler. This is a breaking change to existing validator functionality, as returning 'false' won't fail the validation. Existing return false, will have to be replaced with throw 'error'.

Extend Parse.Cloud Validation Handlers to other Triggers

For completeness, I have added the ability to add a validation function to beforeSave, afterSave, beforeDelete, afterDelete, beforeFind, afterFind, beforeFileSave, afterFileSave, beforeDeleteFile, afterDeleteFile, beforeConnect, beforeSubscribe and afterLiveQueryEvent triggers. I have added test cases for all uses.

Allow validation parameter to be an object

Probably jumped on this one a bit early. This allows for some default inbuilt validation, making Parse.Cloud functions easier, more secure, and more accessible to less experienced developers, for less effort.

Options:

requireUser (Boolean): will throw if no user
requireMaster (Boolean): will throw if no masterKey
fields (Array of Strings or Object): used for validating params for Parse.Cloud.define, or validating fields on request.object. Examples:

fields: {
  data: {
    type: String,
    default: 'default',
    options: val => {
        return val.length > 5; // custom validator function for key "data"
    },
    error: 'Data must be longer than 5 characters'
  },
  data1: {
    type: String, // throws if type is wrong
    required: true, //throws if null
    options: ['must be a','must b b'], // throws if value is not in options
    constant: true, // reverts value to request.original
  }
}

If fields is an array of strings, type will be ignored, and required will be 'true', e.g: fields: ['data','data1']

requireUserKeys (Array of Strings or Object): key required on request.user to make a request. Useful for validating users have completed onboarding before calling further functions.
validateMasterKey: whether requests using {useMasterKey:true} should run the validation. In my opinion, it makes sense for requests with the masterKey to skip the validation by default.

Options for validation object expected to evolve over time.

I haven't completely finished the docs yet for Parse.Cloud.js because I wanted to see your thoughts. If you're happy with the PR, I'll add all the validation options to Parse.Cloud.js.

Allow file triggers to throw Parse.Errors

At the moment, if you throw a parse error from a file trigger (throw new Parse.Error(Parse.Error.SCRIPT_FAILED, 'It should fail'), the error will be converted to a file trigger error, removing the error.code that you've set. This PR passes the error from the trigger through.

Closes #6965.

Let me know what you think

[ghost]

Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page.

Generated by 🚫 dangerJS

[codecov]

Codecov Report

Merging #6968 into master will increase coverage by 0.02%.
The diff coverage is 99.22%.

@@            Coverage Diff             @@
##           master    #6968      +/-   ##
==========================================
+ Coverage   93.83%   93.86%   +0.02%     
==========================================
  Files         169      169              
  Lines       12271    12362      +91     
==========================================
+ Hits        11514    11603      +89     
- Misses        757      759       +2     

Impacted Files	Coverage Δ
src/triggers.js	94.48% <99.09%> (+1.95%)	⬆️
src/Routers/FilesRouter.js	87.06% <100.00%> (+1.00%)	⬆️
src/Routers/FunctionsRouter.js	94.11% <100.00%> (-0.82%)	⬇️
src/cloud-code/Parse.Cloud.js	98.59% <100.00%> (ø)
src/Adapters/Storage/Mongo/MongoStorageAdapter.js	92.92% <0.00%> (-0.89%)	⬇️
src/RestWrite.js	93.98% <0.00%> (ø)
src/vendor/mongodbUrl.js	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68a1b30...014e821. Read the comment docs.

[dblythy]

Failing test isn't related to this PR.

Also: how to handle docs. Should using the inbuilt validation blocks be recommended? I'm thinking the JS in this section of the docs titled "Implementing Validation" should change to something like:

---

"We recommend using the third parameter of a Parse.Cloud function to validate the request, prior to the function being called. Validators should make sure all data referenced in the upcoming trigger is correctly formatted."

const adminValidator = (request) => {
  if (!request.user || !request.user.get('Admin')) {
    throw 'Unauthorised';
  }
}
Parse.Cloud.define('AdminFunction',(request) => {
// do admin function here. only admins will be able to call function.
}, adminValidator);

// ensure that Admin is set to false on signup, and that it cannot be edited.
Parse.Cloud.beforeSave(Parse.User, (request) => {
  // do any additional trigger work here
},{
  fields : {
    Admin : {
      default : false,
      constant: true
    }
  }
};

And for the example function:

Parse.Cloud.beforeSave("Review", (request) => {

},{
  fields: {
    stars : {
      required:true,
      options: stars => {
        return stars >= 1 && stars =< 5;
      },
      error: 'Your review must be between one and five stars'
    }
}
});

[dplewis]

Can you increase code coverage?

[mtrezza]

Thanks for this useful PR!

Update Parse.Cloud Validation Handlers
This updates validation handlers to allow async, and removes the requirement for a return value on the validation handler. Also no breaking changes, as returning 'false' still fails the validation, as per existing behaviour.

Not sure if this has been addressed yet, but returning nothing or false seems unintuitive and inconsistent with other Cloud Code methods. When there is a return value, I would expect to always return one. How about removing the return value and changing it to throwing only, even if that is a breaking change?

[dblythy]

Thank you for your thoughts and taking the time to go through this @mtrezza and @dplewis!

[dblythy]

Is coverage okay, or do we need more tests @dplewis?

Also, my inbuilt validator object is heavily inspired by Vue, and cloud function validation that I find myself often using. I expect this to evolve as more use cases are found, but should anyone suggest any other options for it, please do.

[dplewis]

Some quick changes. The last step is to document the validationHandler.

@param {(Object|Function)} validationHandler

[dblythy]

I can't seem to get the formatting of the API reference correct. Could anyone pls help me / point me in the right direction?

[dplewis]

@dblythy Can you give me access to your fork?

[dplewis]

@dblythy Our JSDoc template needs to be updated to format correctly.

[dblythy]

Thanks again @dplewis, you’re a legend.

[alljinx]

Hello.

Nice work but looks like de index.d.ts was not updated (before save does not accept the third arg).
Issuer opened here : #7016

And sync func dosen't seems to work in 'options' attributes, even with a throw, am I doing wrong ?
fields: { title : { required: true, options: async (title: string): Promise<void> => { throw 'error'; }, error: 'Title is bad' } }

[Cinezaster]

@dblythy would it be correct to say that an async options function doesn't work?
From your experience adding this validation would it be a lot of work to implement.

I'm trying to implement a unique value validation on a column and think a check on validation level would be nicer solution then to throw an error in beforeSave. Since I need to do a db lookup it needs to be async. Since I'm checking on a pointer, I don't think I could do this on DB level.

[dblythy]

It is not currently possible @Cinezaster. I have opened an issue and will work on a fix in a wk or two.