fastest-validator icon indicating copy to clipboard operation
fastest-validator copied to clipboard

Add conditionals

Open alexjab opened this issue 4 years ago • 13 comments

Hi,

thanks again for this project!

I've been working on "conditional" types and wanted to have your opinion on the feature and API. I'm trying to implement something similarly to what's available in JSON-Schema (https://json-schema.org/understanding-json-schema/reference/conditionals.html). I haven't followed the exact same syntax (I added types instead of adding properties to all the current available types), because I feel it's easier to do, and allows for more powerful usage.

Although it looks like it's quite a specific need, I don't think I am the only one having it; in my case, a common pattern that I happen to stumble upon a lot, is having an object that takes different properties according to a single field's value (usually a "type").

It's quite similar to what multi solves, except that multi will show all errors if none of the values are a match, and the order of these errors depends on the order in which the rules are written, sometimes making it for confusing error messages.

Here is an example using fictitious discounts:

const Validator = require("./");

v = new Validator();

const multiSchema = {
  $$root: true,
  type: "multi",
  rules: [
    {
      type: "object",
      props: {
        discount_type: { type: "equal", value: "amount" },
        discount_amount: {
          type: "number",
        },
      },
    },
    {
      type: "object",
      props: {
        discount_type: { type: "equal", value: "percent" },
        discount_percent: {
          type: "number",
          max: 100,
        },
      },
    },
  ],
};

console.log(
  v.validate({ discount_type: "percent", discount_percent: 150 }, multiSchema)
);
// =>
// [
//   {
//     type: 'equalValue',
//     message: "The 'discount_type' field value must be equal to 'amount'.",
//     field: 'discount_type',
//     expected: 'amount',
//     actual: 'percent'
//   },
//   {
//     type: 'required',
//     message: "The 'discount_amount' field is required.",
//     field: 'discount_amount',
//     actual: undefined
//   },
//   {
//     type: 'numberMax',
//     message: "The 'discount_percent' field must be less than or equal to 100.",
//     field: 'discount_percent',
//     expected: 100,
//     actual: 150
//   }
// ]

console.log(
  v.validate({ discount_type: "foobar", discount_amount: 20 }, multiSchema)
);
// =>
//[
//   {
//     type: 'equalValue',
//     message: "The 'discount_type' field value must be equal to 'percent'.",
//     field: 'discount_type',
//     expected: 'percent',
//     actual: 'foobar'
//   },
//   {
//     type: 'equalValue',
//     message: "The 'discount_type' field value must be equal to 'amount'.",
//     field: 'discount_type',
//     expected: 'amount',
//     actual: 'foobar'
//   }
// ]

const ifSchema = {
  $$root: true,
  type: "if",
  if: {
    type: "object",
    props: {
      discount_type: { type: "equal", value: "percent" },
    },
  },
  then: {
    type: "object",
    props: {
      discount_type: { type: "equal", value: "percent" },
      discount_percent: {
        type: "number",
        max: 100,
      },
    },
  },
  else: {
    type: "object",
    props: {
      discount_type: { type: "equal", value: "amount" },
      discount_amount: {
        type: "number",
      },
    },
  },
};

console.log(
  v.validate({ discount_type: "percent", discount_percent: 150 }, ifSchema)
);
// =>
// [
//   {
//     type: 'numberMax',
//     message: "The 'discount_percent' field must be less than or equal to 100.",
//     field: 'discount_percent',
//     expected: 100,
//     actual: 150
//   }
// ]

In this previous example, my "if" type might not be the best choice, but I think it works great for more simple values.

Here is a better approach for more complex scenarios:

const switchSchema = {
  $$root: true,
  type: "switch",
  cases: [
    {
      if: {
        type: "object",
        props: {
          discount_type: { type: "equal", value: "percent" },
        },
        strict: false,
      },
      then: {
        type: "object",
        props: {
          discount_type: { type: "equal", value: "percent" },
          discount_percent: {
            type: "number",
            max: 100,
          },
        },
      },
    },
    {
      if: {
        type: "object",
        props: {
          discount_type: { type: "equal", value: "amount" },
        },
        strict: false,
      },
      then: {
        type: "object",
        props: {
          discount_type: { type: "equal", value: "amount" },
          discount_amount: {
            type: "number",
          },
        },
      },
    },
  ],
  else: {
    type: "object",
    props: {
      discount_type: { type: "string", enum: ["amount", "percent"] },
    },
    strict: false,
  },
};

console.log(
  v.validate(
    { discount_type: "percent", discount_percent: 150 },
    switchSchema
  )
);
// =>
// [
//   {
//     type: 'numberMax',
//     message: "The 'discount_percent' field must be less than or equal to 100.",
//     field: 'discount_percent',
//     expected: 100,
//     actual: 150
//   }
// ]

console.log(
  v.validate({ discount_type: "foobar", discount_amount: 150 }, switchSchema)
);
// =>
//[
//   {
//     type: 'stringEnum',
//     message: "The 'discount_type' field does not match any of the allowed values.",
//     field: 'discount_type',
//     expected: 'amount, percent',
//     actual: 'foobar'
//   }
// ]

I've added some code and some tests, but no types and no doc, because I'd like your input first.

What do you think of these proposals?

Keep up the good work,

Alex

alexjab avatar Nov 29 '20 12:11 alexjab

Coverage Status

Coverage increased (+0.02%) to 99.614% when pulling 846f246bda2a71cb891173466efa78cba6da70dd on alexjab:if_switch_rules into 714566fa7554982d7f1a1967503890a822b56568 on icebob:master.

coveralls avatar Nov 29 '20 12:11 coveralls

Coverage Status

Coverage increased (+0.02%) to 99.614% when pulling 846f246bda2a71cb891173466efa78cba6da70dd on alexjab:if_switch_rules into 714566fa7554982d7f1a1967503890a822b56568 on icebob:master.

coveralls avatar Nov 29 '20 12:11 coveralls

@alexjab Thanks for this proposal! But what problem does this feature solve? I think we already had this feature with custom rules/checkers. You can write any javascript code there without schema limits, And the final schema will be more readable.

Our goal is different from json-schema, json-schema goal is to be language independent, so they can't support custom checker functions in schema like FV, so they introduced this conditional validating feature

erfanium avatar Nov 29 '20 13:11 erfanium

Hi, @erfanium thanks for your reply.

This feature allows to use all the already available schemas, validation methods and paths (conditionally), the same way multi allows us to do that.

The examples of the custom validation functions only show basic types and that you can validate simple values using javascript. They do not really show how you would validate more complex values such as objects or arrays. Usually, I would use a multi type, but as I've explained, I think it also has some limitations.

Is there a way of accessing the "validation" method from within a custom validator (so that I would be able to validate objects and arrays and not just basic types)?

I agree that json-schema's purpose is being portable between languages, but I also think it's convenient to have access to more tools from within the same language / library that you already know.

BR,

Alex

alexjab avatar Nov 29 '20 17:11 alexjab

I've also noticed Joi has conditionals (https://joi.dev/api/?v=17.3.0#alternativesconditionalcondition-options) so it seems to be a rather standard feature.

alexjab avatar Nov 29 '20 17:11 alexjab

@alexjab thank you for your work. If I understand correctly, the multi can cover the conditional feature but it prints the errors from the "wrong" cases, right? What if we only try to fix this?

icebob avatar Nov 30 '20 10:11 icebob

Hi @icebob , thanks for your reply.

The "limitation" with multi (quotes here, because it works great and exactly as advertised) is that it has no way of "knowing" which error is the most meaningful to the user, and in some cases, show conflicting errors.

Consider the following schema:

const schema = {
  $$root: true,
  type: "multi",
  rules: [
    {
      type: "object",
      props: {
        object_type: { type: "equal", value: "red" },
        red_id: "number",
      },
    },
    {
      type: "object",
      props: {
        object_type: { type: "equal", value: "blue" },
        blue_id: "number",
      },
    },
  ],
};

And the following calls:

console.log(v.validate({ object_type: "red", blue_id: 1 }, schema));
// [
//   {
//     type: 'required',
//     message: "The 'red_id' field is required.",
//     field: 'red_id',
//     actual: undefined
//   },
//   {
//     type: 'equalValue',
//     message: "The 'object_type' field value must be equal to 'blue'.",
//     field: 'object_type',
//     expected: 'blue',
//     actual: 'red'
//   }
// ]

console.log(v.validate({ object_type: "blue", red_id: 1 }, schema));
// [
//   {
//     type: 'equalValue',
//     message: "The 'object_type' field value must be equal to 'red'.",
//     field: 'object_type',
//     expected: 'red',
//     actual: 'blue'
//   },
//   {
//     type: 'required',
//     message: "The 'blue_id' field is required.",
//     field: 'blue_id',
//     actual: undefined
//   }
// ]

1/ In the second example, the more "logical" error should be "The 'blue_id' field is required.", at least that's what a human would probably consider better, 2/ Both examples return conflicting errors: fixing both errors in the first example results in both errors in the second example.

The issue comes from the fact that multi has no way of knowing that part of the data is a condition (with its own rules) and the rest of the data is subjected to this condition.

According to my proposal, this would solve this issue:

const schema2 = {
  $$root: true,
  type: "switch",
  cases: [
    {
      if: {
        type: "object",
        props: {
          object_type: { type: "equal", value: "red" },
        },
        strict: false,
      },
      then: {
        type: "object",
        props: {
          object_type: { type: "equal", value: "red" },
          red_id: "number",
        },
      },
    },
    {
      if: {
        type: "object",
        props: {
          object_type: { type: "equal", value: "blue" },
        },
        strict: false,
      },
      then: {
        type: "object",
        props: {
          object_type: { type: "equal", value: "blue" },
          blue_id: "number",
        },
      },
    },
  ],
  else: {
    type: "object",
    props: {
      object_type: { type: "string", enum: ["red", "blue"] },
    },
    strict: false,
  },
};

console.log(v.validate({ object_type: "red", blue_id: 1 }, schema2));
// [
//   {
//     type: 'required',
//     message: "The 'red_id' field is required.",
//     field: 'red_id',
//     actual: undefined
//   }
// ]

console.log(v.validate({ object_type: "blue", red_id: 1 }, schema2));
// [
//   {
//     type: 'required',
//     message: "The 'blue_id' field is required.",
//     field: 'blue_id',
//     actual: undefined
//   }
// ]

console.log(v.validate({ object_type: "foo", red_id: 1 }, schema2));
// [
//   {
//     type: 'stringEnum',
//     message: "The 'object_type' field does not match any of the allowed values.",
//     field: 'object_type',
//     expected: 'red, blue',
//     actual: 'foo'
//   }
// ]

Because I can "tell" the schema (and to readers) which part of the data is the "condition".

I agree, this has some challenges and the syntax is far from perfect; for example for objects, you have to pass strict: false to the "condition" schema, because it will fail if there are additional keys. You can see this in the Joi example (they use unknown()) and in JSON-schema (additionalProperties is true by default).

Maybe there's another way to do that, and I think that it's quite a common use-case.

Thanks,

Alex

alexjab avatar Nov 30 '20 14:11 alexjab

Funny enough just yesterday evening I tought of suggesting a "union" kind behaviour just for the reason mentioned at the very start of this thread. We are using the fastest-validator to validate incoming web request json (mostly over web sockets). In each request there is a type (typically string) telling the client what the server wants. To get proper error messaging we have to first select a schema on the type and then validate against this. So why not integrate this in the fastest-validator.

If I was a bit faster I would have suggested to combine the multi with some property (honestly: not thought to the end!) on the multi itself similiar to "switchField": "type" and then on each multi item something like "switchCase": "deleteOne". Before running a multi with "switchField" set it will read the "type" (in this example) from the data under test and only respect those multi elements with either a "switchCase" of this value or no "switchCase" at all - or all, if the value is undefined/null.

Concerning our use case (which is I think can not be so rare) I would even fail the overall multi if "switchCase" is set but the data can not provide a proper (always string? ok fine for us, but...) value. But there may be a multi rule to handle this.

In most languages there ist a default switch case but for validation I would be more strict: first the filter on multi elements is processed to find all "switchCase" rules with the expected value. If there is less than 1 the validation failes before doing a deep dive into the array of rules.

JMS-1 avatar Nov 30 '20 15:11 JMS-1

I would suggest considering other syntax options. The proposed syntax looks, in my opinion, terribly inconvenient and redundant.

intech avatar Nov 30 '20 17:11 intech

It would be a good solution to describe the basic rules, and one additional to indicate the order of verification by referring in the same scheme. Something like this:

const schema = {
  $$root: true,
  type: "multi",
  order: ["object_type", "red_id", "blue_id"], // order for all properties in rules
  rules: [
    {
      type: "object",
      props: {
        object_type: { type: "equal", value: "red" },
        red_id: "number",
      },
    },
    {
      type: "object",
      props: {
        object_type: { type: "equal", value: "blue" },
        blue_id: "number",
      },
    },
  ],
};
const schema = {
  $$root: true,
  type: "multi",
  order: [1, 0], // index from rules
  rules: [
    {
      type: "object",
      order: ["object_type", "red_id"],
      props: {
        object_type: { type: "equal", value: "red" },
        red_id: "number",
      },
    },
    {
      type: "object",
      order: ["object_type", "blue_id"],
      props: {
        object_type: { type: "equal", value: "blue" },
        blue_id: "number",
      },
    },
  ],
};

or use the order of objects in the schema itself for a sequence of checks

intech avatar Nov 30 '20 17:11 intech

@alexjab

Hi, @erfanium thanks for your reply. Is there a way of accessing the "validation" method from within a custom validator (so that I would be able to validate objects and arrays and not just basic types)?

Yes, you are right, there is no such way, But I usually don't use validators in these situations, In my opinion, validators are not good for complicated and unusual data validations (as ORMs are not good for complicated SQL queries), These tools were made for agility and clean code, but if they are used for the wrong need, it will backfire.

Thanks!

erfanium avatar Nov 30 '20 20:11 erfanium

@alexjab @icebob

And the following calls:

console.log(v.validate({ object_type: "red", blue_id: 1 }, schema));
// [
//   {
//     type: 'required',
//     message: "The 'red_id' field is required.",
//     field: 'red_id',
//     actual: undefined
//   },
//   {
//     type: 'equalValue',
//     message: "The 'object_type' field value must be equal to 'blue'.",
//     field: 'object_type',
//     expected: 'blue',
//     actual: 'red'
//   }
// ]

We can fix this problem with a little change:

[
   {
      type: 'multi',
      message: "The 'red_id' field is required OR The 'object_type' field value must be equal to 'blue'",
      rules: [
         {
           type: 'required',
           message: "The 'red_id' field is required.",
           field: 'red_id',
           actual: undefined
         },
         {
           type: 'equalValue',
           message: "The 'object_type' field value must be equal to 'blue'.",
           field: 'object_type',
           expected: 'blue',
           actual: 'red'
         }
      ]
   },
 ]

erfanium avatar Nov 30 '20 20:11 erfanium

@erfanium

validators are not good for complicated and unusual data validations

I agree of course, maybe the difference is where we set the limit; I think that such conditionals still have their place in a validation library, but maybe it's because it suits the features I need at the moment; in any case, I understand your point and it makes perfect sense.

As you outlined, an out-of-the-box solution would be to use neutral custom error messages for both errors so that any combination would still make sense to the user (although it does not really scale well with the number of fields).

const schema = {
  $$root: true,
  type: 'multi',
  rules: [
    {
      type: 'object',
      props: {
        object_type: {
          type: 'equal',
          value: 'red',
          messages: {
            equalValue:
              "The object_type must be set to 'red' when providing a red_id OR the red_id field is required when using a type 'red'",
          },
        },
        red_id: {
          type: 'number',
          messages: {
            required:
              "The red_id field is required when using a type 'red' OR the object_type must be set to 'red' when providing a red_id",
          },
        },
      },
    },
    {
      type: 'object',
      props: {
        object_type: {
          type: 'equal',
          value: 'blue',
          messages: {
            equalValue: 
              "The object_type must be set to 'blue' when providing a blue_id OR the blue_id field is required when using a type 'blue'",
          },
        },
        blue_id: {
          type: 'number',
          messages: {
            required: 
              "The blue_id field is required when using a type 'blue' OR the object_type must be set to 'blue' when providing a blue_id",
          },
        },
      },
    },
  ],
}

Another possible solution (inspired by @intech 's answer) would be adding a property to the schema to let the user specify the order of the error messages, so that equalValue always pops-up after required, but I'm not sure it's trivial, or that it would really benefit others.

alexjab avatar Dec 01 '20 21:12 alexjab