joi icon indicating copy to clipboard operation
joi copied to clipboard

Add more functionality to external validators

Open asologor opened this issue 2 years ago • 10 comments

No breaking changes here. No existing tests were modified.

I know that many people want to see these features in Joi and I think this functionality is a must-have for any backend validator. If you do not merge this PR, consider implementing the same or similar interface yourself.

Added a bunch of helpers for externals:

  • prefs - this one existed before.
  • path - ordered array where each element is the accessor to the value where the error happened.
  • label - label of the value. If you are validating an object's property, it will contain the name of that property.
  • root - the root object or primitive value under validation.
  • context - same as root, but contains only the closest parent object in case of nested objects validation.
  • error - a function with signature function(message). You can use it in a return statement (return error('Oops!')) or you can call it multiple times if you want to push more than one error message in a single external validator.

Added a new setting alwaysExecuteExternals (default is false) which in pair with abortEarly: false will make externals execute even after some synchronous validator failed. Chains of external validators will abort early anyway (e.g. Joi.any().external(...).external(...) - second external won't execute if the first one failed).

Now externals throw the same ValidationError instance as all other validators.

A basic example from the API.md doc:

const data = {
    foo: {
        bar: 'baz'
    }
};

await Joi.object({
    foo: {
        bar: Joi.any().external((value, { prefs, path, label, root, context, error }) => {
            // "prefs" object contains current validation settings
            // value === 'baz'
            // path === ['foo', 'bar']
            // label === 'foo.bar'
            // root === { foo: { bar: 'baz' } }
            // context === { bar: 'baz' }

            if (value !== 'hello') {
                return error(`"${value}" is not a valid value for prop ${label}`);
            }
        })
    }
}).validateAsync(data);

See API.md changes and test suites for more details.

asologor avatar Apr 26 '22 15:04 asologor

Thanks, this is great, I just wanted to implement this too. The current implementation is lacking quite a bit for the external methods.

kammerjaeger avatar Apr 27 '22 19:04 kammerjaeger

Could this add (also part of custom):

message(messages, [local]) - a method to generate an error with an internal 'custom' error code and the provided messages object to use as override. Note that this is much slower than using the preferences messages option but is much simpler to write when performance is not important.

jonyeezs avatar Jul 28 '22 00:07 jonyeezs

This is a great improvement over the current .external() implementation.

@jonyeezs I think the current error() helper is acting similar to the messages() function of custom (but with only one message and without local param).

So I'd propose to rename the error() function to message() to be more consistent and to add a new error() helper function that takes a code/type and replaces the default "external" in the created error to be more consistent with

error(code, [local]) - a method to generate error codes using a message code and optional local context.

baumerdev avatar Aug 10 '22 10:08 baumerdev

Is there any real need for multiple calls to error? I feel like this breaks away from the convention in every other place where it's allowed (extensions and custom). I'd argue that there's no need for alwaysExecuteExternals, abortEarly should probably have behaved with this rule the same way it does with others.

Marsup avatar Aug 20 '22 17:08 Marsup

I'd argue that there's no need for alwaysExecuteExternals, abortEarly should probably have behaved with this rule the same way it does with others.

With the current (main branch) version .external() ist only called if and after the other validations pass. Even when you set abortEarly to false, external() isn't called if any other validation fails. See docs: "If schema validation failed, no external validation rules are called"

You may change this this behavior to mimic something like .custom() by always executing it if either there is no other previous error or abortEarly is set to false. I would appreciate it and think it's a good idea to not having different behavior for both functions. But I guess that'd be a breaking change for some relying on external() only being called on successfully validated data.

baumerdev avatar Aug 20 '22 17:08 baumerdev

Is there any real need for multiple calls to error? I feel like this breaks away from the convention in every other place where it's allowed (extensions and custom).

It might be handy sometimes. A few example cases:

  1. Validating an array
  2. Validating several (or all) fields of a form in a single external valdator

I'd argue that there's no need for alwaysExecuteExternals, abortEarly should probably have behaved with this rule the same way it does with others.

Even if abortEarly = false and some non-external field has an error, externals won't be executed. alwaysExecuteExternals makes externals run regardless of other validator outcomes.

Currently, I don't have time to work on this PR. Maintainers are free to make any changes though.

asologor avatar Oct 03 '22 15:10 asologor

Hi! Someone know when will this PR will be merge? I need this fix ^^'

kolpitor avatar Dec 21 '22 15:12 kolpitor

First, I'd like to thank @asologor a lot for the efforts put into this PR, it helped me kick off this feature.

I have implemented it in #2931, this is released as of 17.9.0. I chose to stick to the same API as custom not to confuse people with a similar API but incompatible helpers, it was a bit more work, but I think it's worth it.

For now, alwaysExecuteExternals is not part of it because my mind is not made up yet, I'm not sure whether it should be a global flag, a local flag, or both, or if it should behave accordingly to abortEarly. I don't personally have use for this, so any input the community could provide would help me drive this.

Marsup avatar Mar 20 '23 08:03 Marsup

I would love to see alwaysExecuteExternals implemented so that I could show users all of the errors that exist on a single submission of a form.

zRogersFlexion avatar May 31 '23 17:05 zRogersFlexion

The ability to validate all parts of a schema, including properties that rely on externals, at the same time would be hugely impactful to my project. Please consider adopting this feature. Thanks!

rachelschneiderman avatar May 31 '23 21:05 rachelschneiderman