Add a failing test case to demonstrate a bug where inherited properties ...

[kiall]

Add a failing test case to demonstrate a bug where inherited properties are not taken into account by validate_additionalProperties.

[kiall]

I'm unsure how to go about fixing this issue in a way that will continue to work once $ref support is merged - so I've added a test case that should pass, but currently fails.

[Julian]

Ugh extends is just the worst :(.

I haven't looked at this carefully, but ultimately I think the thing that will happen is that as the first step in validation we'd resolve and merge all the (recursive) extends. That'd have some other benefits too.

[gazpachoking]

I'm not actually sure this is a bug. I think the schema in this test should be invalid no matter what. The main bit of the schema only allows one property, but the extends requires a different one.

[Julian]

I think you're right, but I wasn't sure enough to say so without investigating. I know there are a few posts on extends recently on the ML.

[kiall]

We actually do this in our JS implementation, and I'm currently adding
something similar as a pre-processing step in our usage of this lib.

It probably won't be suitable for merging back in, but ill see if I can
push code up anyway.
On Oct 22, 2012 1:48 PM, "Julian Berman" notifications@github.com wrote:

Ugh extends is just the worst :(.

I haven't looked at this carefully, but ultimately I think the thing that
will happen is that as the first step in validation we'd resolve and merge
all the (recursive) extends. That'd have some other benefits too.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/38#issuecomment-9662317.

[gazpachoking]

Yeah, it's getting replaced in draft 4 with allOf, which they said should work the same. When thought of like that, nothing can match both of the schemas specified at the same time.

[gazpachoking]

Here's a link to discussion of this topic: https://groups.google.com/d/msg/json-schema/Ru-tCCUlDUU/4j3mvndg2xMJ

[Julian]

There we go :), think that's the one I caught a glimpse of. That looks convincing but I'll take another look when I get home from work. @kiall please let me know what you think as well after you've glanced at that post.

[kiall]

I'm just trying to wrap my head around how "extends" actually means "allOf" (Or "alsoValidateAgainst" as I would have called it!).

One piece that still leaves me confused is the current wording in draft-03 (and I think is unchanged in draft-04 - Do you have a authoritative link to the draft-04 spec?):

Conceptually, the behavior of extends can be seen as validating an instance against all constraints in the extending schema as well as the extended schema(s). More optimized implementations that merge schemas are possible, but are not required. [...] 

This suggests to me that merging of the schemas is perfectly acceptable - but that the behavior of the merged schema should be similar to running the validation against each separately.

I guess our pre-processing step will always be required so!

[gazpachoking]

Yeah, to merge the schemas such that the result is the same as it would be validating them separately is quite tough though. In the instance of your test case the resulting merged schema would have to be invalid for all possible json instances. There is no authoritative draft 4 yet, as it has still not been finalized. Here is the current working copy: https://github.com/json-schema/json-schema/tree/next/proposals allOf specifically is here: https://github.com/json-schema/json-schema/blob/next/proposals/json-schema-validation.txt#L827 It's not a 1:1 translation from extends, rather instead of having a main schema, and an extended schema referenced within it, you'd move to having a mostly empty schema which references both:

{ "allOf": [{mainschema}, {extendsschema}] }