Failing tests in latest about json-schema-validator

[fge]

Yep, there is a little refactoring going on, but I have been so busy I couldn't work on this project for nearly two weeks.

One of the failing tests is issue #7, which I am currently fixing, and which has triggered this refactoring.

[clarkmccusa]

Looking to contribute some enhancements back ... Will look into tests to see if I might help there too.

Sent from my iPad

On Jun 29, 2012, at 4:19 AM, [email protected] wrote:

Yep, there is a little refactoring going on, but I have been so busy I couldn't work on this project for nearly two weeks.

One of the failing tests is issue #7, which I am currently fixing, and which has triggered this refactoring.

---

Reply to this email directly or view it on GitHub:
#9 (comment)

[fge]

I'd be very happy about that... The problem currently is to be able to pass a cheap enough context back to a validation process, this context being aware of the current schema root.

I am not very good at software architecture and have gone too far in the details so far :/

And my code lacks documentation. Yeah, right now, it sucks.

[clarkmccusa]

I think it has a lot of potential. I am not sure why no one has an
implementation in Java, several in Javascript that seem to be pretty solid.

I have found there does not appear to be support for schema of the form:

"type":["string",null]

Where a null value is allowed. Do you agree with this? I'm currently
working toward a fix, though I have some further work to do to understand
what order the validation takes place in.

Mike

On Fri, Jun 29, 2012 at 8:54 AM, fge <
[email protected]

wrote:

I'd be very happy about that... The problem currently is to be able to
pass a cheap enough context back to a validation process, this context
being aware of the current schema root.

I am not very good at software architecture and have gone too far in the
details so far :/

And my code lacks documentation. Yeah, right now, it sucks.

---

Reply to this email directly or view it on GitHub:
#9 (comment)

[fge]

The real schema would be:

{
"type": [ "string", "null" ]
}

which is currently supported. { "type": [ "string", null ] } is not valid per the current spec.

The type keyword is a viper's nest, but in its simplest form, is a string or an array of strings referencing "atomic" types, of which 6 exist: string, integer, number, null, array, object, boolean. In the current code, those atomic types are "materialized" via the NodeType class.

However, the current problem lies with this particular keyword and its cousins. The full list of problematic keywords is: type, disallow, extends, dependencies (for the latter, if schema dependencies are specified). These are the only 4 keywords in the current draft which can potentially trigger recursive validation, "extends" for an obvious reason, others for not so obvious reasons.

My problem is to keep the context in such cases, and it is the point of issue 7: I need to be able to resolve spawned schemas from the current validation context, and a context bears a root schema. Which is currently not done correctly.

[fge]

To expand further: I think a validation "object" should be implementing the so-called "composite pattern". In this pattern, all keywords would be "leaf" objects save for these 4 blasted keywords which are "type", "disallow", "dependencies" and "extends".

Now, the question is how. And I'm stuck.

There is some test code in the "next" branch, however it is only a proof of concept code. And it is not functional as it stands :/

[fge]

There are no more failing tests now. I'd appreciate any input on the design in particular, it's still flaky.

[clarkmccusa]

Hi Francis,

Wondering if you have an "architecture overview" doc (even 1 page would be
helpful). I am interested in adding two things to your library, which I
would like to contribute back:

1. Support for null values in union types ["string","null"] with an
   enumeration type (currently a null value passed to a type with an enum
   tries to compare the null to the enumeration values, and reports that the
   enum value does not match
   and
2. Adding context to error messages such that the specific data constraints
   are included in the message ("length is greater than max allowed langth"
   becomes "string length of 7 is longer than the allowed value of 5")

I'm trying to follow the entry point from ValidJsonSchema, but I'm having
some difficulty grokking the overall design.

Thanks
Mike

On Wed, Jul 11, 2012 at 10:37 AM, fge <
[email protected]

wrote:

There are no more failing tests now. I'd appreciate any input on the
design in particular, it's still flaky.

---

Reply to this email directly or view it on GitHub:
#9 (comment)

[fge]

Well, that's the missing Javadoc... I have to add more, that's for sure. There is some, but not enough.

I don't understand your first point? Can you give an example?

As to the second: all keyword validators are built by a KeywordFactory, all of them are in the org.eel.kitchen.jsonschema.keyword (which does have a certain amount of Javadoc ;)).

All of them extend the abstract KeywordValidator class, and they add messages to the ValidationContext they are passed to via the context's .addMessage() method.

As you will see, all keyword validators, when instantiated, will retain the value they have to validate against as an instance variable (which is, of course, validator dependent). In all cases, all you have to do it modifying messages appropriately.

Of course, tests for the appropriate content of messages would be appreciated ;) For this, you'll have to modify the AbstractKeywordValidatorTest class (in test sources, of course). Should be easy enough. Test JSON files are all in the resources/keyword directory.

I intended to do that at one point, but postponed for two reasons: 1. there are still architectural problems, and 2. I have been sidetracked by other projects.

As to the main architectural problems:

• the ValidationContext structure has one role too many -- the role of transporting the context from one validator to another along with the role of collecting messages;
• keyword validators themselves are rather simple except for 4 of them: type, disallow, extends and dependencies -- coincidentally, those are the only 4 validators which can spawn subschemas.

This second point is the reason for the first point, I guess I should use a composite pattern somehow but cannot come with a solution :/

Thanks!

[clarkmccusa]

Thanks, helpful stuff for sure.

Here is an example of what is most troubling for me at the moment ...

Snippet from my schema:

"email_addresses":{

"type":"array",

"additionalItems":false,

"optional":false,

"items":{

"type":"object",

"properties":{

"status":{"type":["string","null"],"enum
":["ACTIVE","UNCONFIRMED","OPTOUT","REMOVED","NON_SUBSCRIBER","VISITOR"]},

"confirm_status":{"type":"string","enum
":["CONFIRMED","NO_CONFIRMATION_REQUIRED","UNCONFIRMED"]},

"opt_in_source":{"type":"string","enum
":["ACTION_BY_VISITOR","ACTION_BY_OWNER"]},

"opt_out_source":{"type":"string","enum
":["ACTION_BY_VISITOR","ACTION_BY_OWNER"]},

"opt_in_date":{"type":"string","format":"date-time"},

"opt_out_date":{"type":"string","format":"date-time"},

"email_address":{"type":"string",
"format":"email","maxLength":80,"optional":false}

Most important is the email status field:

"status":{"type":["string","null"],"enum
":["ACTIVE","UNCONFIRMED","OPTOUT","REMOVED","NON_SUBSCRIBER","VISITOR"]},

If my instance document contains the following:

"email_addresses":[

    {

        "status":null,

       "opt_in_source":"ACTION_BY_VISITOR",

        "email_address":"[email protected]"

    }

],

This is the message I get ...

Message:#/email_addresses/0/status: instance does not match any enum value

Which, while a true statement, I don't think it should even check the enum
value, as null values are allowed as part of the union type with "string",
and the value passed was in fact null.

Let me know if I am missing something ...

Thanks

mike

On Wed, Jul 25, 2012 at 3:34 PM, fge <
[email protected]

wrote:

Well, that's the missing Javadoc... I have to add more, that's for sure.
There is some, but not enough.

I don't understand your first point? Can you give an example?

As to the second: all keyword validators are built by a KeywordFactory,
all of them are in the org.eel.kitchen.jsonschema.keyword (which does
have a certain amount of Javadoc ;)).

All of them extend the abstract KeywordValidator class, and they add
messages to the ValidationContext they are passed to via the context's
.addMessage() method.

As you will see, all keyword validators, when instantiated, will retain
the value they have to validate against as an instance variable (which is,
of course, validator dependent). In all cases, all you have to do it
modifying messages appropriately.

Of course, tests for the appropriate content of messages would be
appreciated ;) For this, you'll have to modify the
AbstractKeywordValidatorTest class (in test sources, of course). Should be
easy enough. Test JSON files are all in the resources/keyword directory.

I intended to do that at one point, but postponed for two reasons: 1.
there are still architectural problems, and 2. I have been sidetracked by
other projects.

As to the main architectural problems:

• the ValidationContext structure has one role too many -- the role of
  transporting the context from one validator to another along with the role
  of collecting messages;
• keyword validators themselves are rather simple except for 4 of them:
  type, disallow, extends and dependencies -- coincidentally, those are the
  only 4 validators which can spawn subschemas.

This second point is the reason for the first point, I guess I should use
a composite pattern somehow but cannot come with a solution :/

Thanks!

---

Reply to this email directly or view it on GitHub:
#9 (comment)

[fge]

Most important is the email status field:

"status":{"type":["string","null"],"enum
":["ACTIVE","UNCONFIRMED","OPTOUT","REMOVED","NON_SUBSCRIBER","VISITOR"]},

If my instance document contains the following:

"email_addresses":[

    {

        "status":null,

       "opt_in_source":"ACTION_BY_VISITOR",

        "email_address":"[email protected]"

    }

],

This is the message I get ...

Message:#/email_addresses/0/status: instance does not match any enum value

This is expected: all keywords in a schema must match for the instance to be valid.

You seem to be missing one thing: null is a valid JSON value! So, remove the "type" (you can also remove it from your other similar schemas since all your enum values are strings anyay) and add a null value to the enum:

"enum": [ "ACTIVE", "UNCONFIRMED", "OPTOUT", "REMOVED", "NON_SUBSCRIBER", "VISITOR", null ]

[clarkmccusa]

Ah, you are correct ... that works well, thank you.,

Ok, with regard to #2, how are you accepting work ... just complete in a
separate fork, then issue a pull request for you to look at?

On a different note, have you thought at all about validation of individual
nodes within an instance document? The use case is an API that supports
"partial update on POST" (some call it overloaded POST), where I do not
have to PUT an entire representation of a resource in order to change one
field value within it.

Mike

On Wed, Jul 25, 2012 at 4:06 PM, fge <
[email protected]

wrote:

Most important is the email status field:

"status":{"type":["string","null"],"enum

":["ACTIVE","UNCONFIRMED","OPTOUT","REMOVED","NON_SUBSCRIBER","VISITOR"]},

If my instance document contains the following:

"email_addresses":[

    {

        "status":null,

       "opt_in_source":"ACTION_BY_VISITOR",

        "email_address":"[email protected]"

    }

],

This is the message I get ...

Message:#/email_addresses/0/status: instance does not match any enum
value

This is expected: all keywords in a schema must match for the instance to
be valid.

You seem to be missing one thing: null is a valid JSON value! So, remove
the "type" (you can also remove it from your other similar schemas since
all your enum values are strings anyay) and add a null value to the enum:

"enum": [ "ACTIVE", "UNCONFIRMED", "OPTOUT", "REMOVED",
"NON_SUBSCRIBER", "VISITOR", null ]

---

Reply to this email directly or view it on GitHub:
#9 (comment)

[fge]

That would be ideal, though I don't say that I will eventually merge via github -- I'll probably end up merging "by hand" because github doesn't know how to do merge messages correctly.

If you are so inclined, you can use git format-patch --cover-letter + git send-email. Your choice, really ;)

[fge]

May I close this issue?

[clarkmccusa]

Most definitely, thank you.
On Aug 16, 2012 8:26 PM, "fge" [email protected] wrote:

May I close this issue?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/9#issuecomment-7803884.

[fge]

OK, thanks, have fun! Waiting for more feedback if you feel like it ;)