Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customisable validation #79

Closed
geraintluff opened this issue Dec 4, 2013 · 10 comments
Closed

Customisable validation #79

geraintluff opened this issue Dec 4, 2013 · 10 comments

Comments

@geraintluff
Copy link
Owner

I think this could be done with a combination of extra types, and simple "validation extension" functions to be called each time.

From @Bartvds:


Using json-schema on JS objects could use a few more schema rules/flags. Like

  • ownProperty, enumerable
  • non-JSON data types (like function, date, regex, maybe nan, undefined etc)
  • from ES5 frozen / sealed / extensible

So maybe we should look at the schema-rule part of this in a slightly wider scope and see if tv4 + json-schema can support these kind of JS specific 'extensions' (I guess it can if the implementation complexity is not too much).

@Bartvds
Copy link
Collaborator

Bartvds commented Dec 4, 2013

My first reaction is that the type check probably should result in an validation error if the validator for that type could not be found. On the other hand some other errors like missing schema URI's get ignored too.

Many of the extra JS properties are non-exclusive additions to regular rules so they might still need their normal types for the regular validation rules, so I'm not sure if this should be toggled by the type value. In a way it these ideas are a little like the "format" of the string values.

I'll have to brood on this some more.

@geraintluff
Copy link
Owner Author

OK, I've put some stuff in a new branch - I'd appreciate any feedback you have.

Custom types are not handled, but custom keywords can be defined:

tv4.defineKeyword('frozen', function (data, shouldBeFrozen, schema) {
    if (typeof data !== 'object') return null;
    var isFrozen = Object.isFrozen(data);
    if (shouldBeFrozen&& !isFrozen) return "must be frozen";
    if (!shouldBeFrozen && isFrozen) return "must not be frozen";
    return null;
});

The new methods are defined in the branch's README.

@vweevers
Copy link

vweevers commented Jul 8, 2014

Thanks for that feature. Considering a use case for a unique keyword, a couple of things would be nice, given this schema:

{
  properties: {
    title: { type: 'string', unique: true } 
  },

  // Or in root (the same as having a parent array schema with `uniqueItems: true`):
  unique: true,

  // Or as a list of unique properties:
  unique: ['title']
}

1) Asynchronous validation

tv4.defineKeyword('unique', function (data, value, schema, done) {
  // do some lookups, then
  done({code: .., message: ..})
})

2) Providing a context during validation, perhaps as a custom option.

tv4.defineKeyword('unique', function (data, value, schema, opts) {
  if (Array.isArray(value)) {
    // value is an array of properties that should be unique
    opts.context.find(..)
  } else if (value === true) {
    // Either a property should be unique, or the entity as a whole
    opts.context.find(..)
  }
})

tv4.validateMultiple(someEntity, schema, {context: someCollection})

3) Knowing the keyword's position by providing a path or parent argument. There should at least be a way to differentiate between keywords defined on properties and keywords defined on the schema root. That should make the previous example possible.

@geraintluff
Copy link
Owner Author

The issue with (1) is that it would break the existing API - to allow asynchronous custom validation, then we'd have to make validation asynchronous through and through, and end up removing the asynchronous API (or doubling code size by having two versions).

I don't quite understand (2) - what are you finding? What data/methods are encapsulated in opts?

I also don't understand (3) - are you saying that if you took your example and embedded it in another schema like {"properties": {"data": {...}}}, then the behaviour of unique would change? That's against how JSON Schema works at the moment, so I'm uncertain if that's what you mean.

@vweevers
Copy link

vweevers commented Jul 8, 2014

  1. Okay, I misread Asynchronous validation in the readme as "tv4 has async support". Now I understand that it just fetches schemas and then does synchronous validation.

  2. The example was unclear, and so was my head. The problem was that the custom validation needed data that was not available in the validated object. I found a better way to accomplish what I wanted and explaining the example would serve no purpose. Disregard it, I need a break.

  3. Yes, the behavior would depend on where the keyword was embedded. Let's say a custom keyword is published as a "plugin" for tv4. As long it clearly states in its readme that it does non-standard things, I think it should have that freedom. Wouldn't break anything, only offer flexibility to the user.

@geraintluff
Copy link
Owner Author

OK - with (3), I think that a keyword that depends on document position for its behaviour is intensely problematic. Currently, for all possible values for Schema A, the following two are equivalent:

{"id": "/schemas/A", ... Schema A ...}

{
    "id": "/schemas/B",
    "properties": {
        "a": {"$ref": "/schemas/A"}
    }
}

and:

{
    "id": "/schemas/B",
    "properties": {
        "a": {... Schema A ...}
    }
}

Having a keyword that behaved differently just because it was in the root of a document would violate that principle. So, I'm afraid I'm not particularly keen to support positional information based on this use-case, because I think it would be better to redefine the unique keyword to be more predictable. Does that make sense? :S

@vweevers
Copy link

vweevers commented Jul 9, 2014

Yes, very much. Thanks for helping me understand that principle. I see the value in it. It's better to just do (similar to required):

{
  "unique": ["title", ..],
  "properties": {
    "title": {..}
  }
}

@geraintluff
Copy link
Owner Author

Cool - so is it OK to close this issue?

@vweevers
Copy link

vweevers commented Jul 9, 2014

👍

@blackmiaool
Copy link

blackmiaool commented Aug 12, 2018

I implemented an async format validation in my fork. blackmiaool@7f3c69d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants