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

Allow user to override checkValidity #25

Merged
merged 1 commit into from
Oct 23, 2014
Merged

Allow user to override checkValidity #25

merged 1 commit into from
Oct 23, 2014

Conversation

silas
Copy link
Contributor

@silas silas commented Jun 14, 2014

What do you think about letting the user override checkValidity?

var env = jjv();

var checkValidity = env.checkValidity;

env.checkValidity = function(env, schema_stack, object_stack, options) {
  var result = checkValidity(env, schema_stack, object_stack, options);
  // do something interesting
  return result;
};

env.validate(schema, data);

This would make it a lot easier to write error formatting code. Right now I have to re-walk the JSON schema (along with the data object) if I want to get access to stuff like enum lists (and the invalid data).

Also, the current validation object doesn't give you enough information re-walk the JSON schema where the path includes stuff like anyOf (without re-validating the data).

Just for reference, I'm trying to get errors like the following:

{
  "code": "VALIDATION_ENUM_MISMATCH",
  "message": "Not a valid option (three), expects: one, two",
  "data": "three",
  "path": "$.str"
}

For something like

var schema = {
  type: 'object',
  properties: {
    str: { type: 'string', enum: ['one', 'two'] },
  }
};

var data = {
  str: 'three',
};

@acornejo
Copy link
Owner

Let me think about this, not entirely sure this is the right way to go about it. Mainly because if you don't use checkValidity, then JJV isn't being used for much, or is it? checkValidity is were all the magic happens, so if you have to reimplement it, then what is the point of using JJV. Or perhaps I am missing something? I could be convinced, just need to understand your use case a bit more.

One alternative I can think of, would be to register an error callback. So when JJV is about to return an error, instead of returning it immediately we call a callback with the offending part of the object/schema. However, I am not sure if that is good enough for your purposes.

@silas
Copy link
Contributor Author

silas commented Jun 14, 2014

Yeah, I wasn't thinking about re-implementing checkValidity, rather just getting access to schema_stack and object_stack as the object is evaluated.

Might be a better way to do it, it's just what I came up with first.

@mackwic
Copy link

mackwic commented Jun 20, 2014

I also think an error callback would be great.
Maybe wrapping the result in a promise ?

@silas
Copy link
Contributor Author

silas commented Oct 7, 2014

@acornejo any thoughts on this?

@whitlockjc
Copy link

Do you minute taking a look at jjve/issues/6 and weighing in on this?

@acornejo
Copy link
Owner

Hi guys,

Time is limited right now, if someone can confirm that all tests pass when this patch is applied, I don't mind merging it. Personally I don't feel this is a good solution, but if it helps anyone (and given that it is not intrusive) I don't mind applying it.

@silas
Copy link
Contributor Author

silas commented Oct 23, 2014

I just rebased against master and confirmed both npm pretest and npm test pass without complaint.

Thanks @acornejo, I know it's not a classy solution, but it should help me solve a bunch of issues. If you come up with an interface you like better in the future I'm happy to update my code to use whatever.

acornejo added a commit that referenced this pull request Oct 23, 2014
Allow user to override checkValidity
@acornejo acornejo merged commit 6eb481e into acornejo:master Oct 23, 2014
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

Successfully merging this pull request may close these issues.

4 participants