"additionalProperties" and "allOf", here we go again!

[Francis Galiegue]

OK, I do not count anymore the number of issues open on my project
with regards to this, but this problem has made it on StackOverflow:

http://stackoverflow.com/q/22689900/1093528

And my answer is this:

http://stackoverflow.com/a/23001194/1093528

And my proposals of "strictProperties" and "merge" answer this problem
in what I view the best possible way:

* it relies on an established draft (merge-patch),
* it Just Works(tm),
* it is easy to implement _and_ use.

Now, given all this, does anyone have objections to these two keywords
making it into draft v5?

--
Francis Galiegue, fgal...@gmail.com
JSON Schema in Java: http://json-schema-validator.herokuapp.com

[Geraint]

As you are completely aware, I do have objections.

Mainly, that this kind of pre-processing causes problems with addressing, as well as making it harder to visualise (because when authoring, you don't actually see the schema you're making).

[Francis Galiegue]

On Fri, Apr 11, 2014 at 12:10 PM, Geraint <gerai...@gmail.com> wrote:
> As you are completely aware, I do have objections.
>
> Mainly, that this kind of pre-processing causes problems with addressing,

As I said on github, I don't see why at all. The URI of the schema
doesn't change because of this. This process should be completely
"blind" as far as addressing is concerned -- apply then you have the
result.

You can change "id" in a merge, yes. So what? That doesn't change the
fundamentals of addressing at all.

> as
> well as making it harder to visualise (because when authoring, you don't
> actually see the schema you're making).
>

Well, you don't see a $ref-ed schema either, so I fail to see a
problem here as well.

And look at the benefits!

[Geraint]

On Friday, 11 April 2014 11:21:53 UTC+1, fge wrote:

You can change "id" in a merge, yes. So what? That doesn't change the
fundamentals of addressing at all.

Changing "id" wasn't what I was talking about.  If the merge definition is located at "#/definitions/some-merge", then is there a JSON Pointer fragment that points to some sub-sub-schema inside the merge result?  If so, what does that JSON Pointer fragment resolve to when pre-processing has not happened?

I'm not sure I like an addressing mechanism that starts with "first, fetch this other document and perform a pre-processing step".

[Francis Galiegue]

Can you give an example? I really don't understand what you are
talking about. And again, this is a separate step from addressing. In
the same sense that expanding $data is. If you botch a JSON Reference
because you "mispatch" it, well, that's your fault.

[Geraint]

On Friday, April 11, 2014 2:58:03 PM UTC+1, fge wrote:

Can you give an example? I really don't understand what you are
talking about. 

OK, so my understanding of your proposal is that you have some starting schema you're patching, like:

// starting-schema.json

{

    "type": "object",

    "properties": {

        "foo": {

            "type": "object",

            "properties": {"bar": {}, "baz": {}}

        }

    }

}

And you patch it somehow, like:

// patch-schema.json

{

    "merge": {

        "source": {"$ref": "starting-schema.json"},

        "with": {

            "properties": {

                "foo": {

                    "properties": {

                        "baz": null

                    },

                    "additionalProperties": false

                }

            }

        }

    }

}

So the end result is a schema where the data must be an object, and the "foo" property (if defined) can only contain the single property "bar".  Is this more or less correct?

So in this scenario, I now wish to reference (from elsewhere) the schema for "foo" (not the parent object).  What URI should I use to reference it?

I could use "patch-schema.json#/merge/with/properties/foo" - but if I resolve that (without pre-processing first) then I end up with an invalid schema.  What I wanted was to reference the *merged* schema, but that doesn't actually exist until I've performed pre-processing.  In fact, whether or not I actually want to reference it, it still unsettles me to consider using a schema that doesn't have an actual URI.  Does that help explain my concerns at all?

To be fair, if I did something similar with "allOf" then I couldn't reference the composite schema for "foo" either - but the constraints would just be the intersection of two schemas, so I can still do {"allOf": [{"$ref": "starting-schema.json#/properties/foo"}, {"$ref": "extended-schema.json#/properties/foo"}]}.

[Christopher White]

I'd like to throw my weight behind a merge concept.  We have come up with a number of use cases where lack of a merge capability is causing problems.  It basically comes down to trying to define some common types and then allow changes/additions to that type.

A classic example is defining a common type, but then allowing users of that type to change some of the default values:

{
  "types": {
    "item": {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "color": {
          "type": "string",
          "enum": [ "red", "blue", "green" ],
          "default": "red"
        },
        "size": {
          "type": "integer",
          "minimum": 1,
          "maximum": 100
        }
      }
    }
}

Someone wants to use this type, but change the default to green, or tweak the limits.  This is not something that additionalProperties true or anyOf/allOf can handle because it's often a change.

The syntax I've been playng with is basically a "$patch" next to "$ref":

{
  "$ref": <target>
  "$patch": <jsonpatch>
}

For example:

{
  "types": {
    "green_item": {
      "$ref": "#/types/item",
      "$patch": [
        {
          "op": "replace",
          "path": "/properites/color/default",
          "value": "green"
        }
      ]
    },
    "small_items": {
      "$ref": "#/types/item",
      "$patch": [
        {
          "op": "add",
          "path": "/properites/size/default",
          "value": 10
        },
        {
          "op": "replace",
          "path": "/properites/size/maximum",
          "value": 50
        }
      ]
    }
  }
}

I hear the concern about inability to build direct references to properties within the resulting "green_item" because it requires a processing step.  But for our use case, this is an acceptable tradeoff.  We are looking at defining some complex common types and the only other alternative is to replicate the type definitions in multiple places with minor differences.

Now it may be that we use this syntax interanlly only, such that the final schema that gets published is actually the post-processed version that applies the patch.  But I would actually prefer to have the original references maintained, as it denotes a clear relationship to referenced types and preserves that relationship in the final schema.

In my opinion, this is really not much different than the preprocessing required to handle "$ref" in the first place.  If some compositie type had multiple $refs to other simpler schemas, then there is the same referencing problem of not being able to properly follow full references that span a $ref. 

The beauty of this is that is covers any kind of use case for merge / extend based on existing RFCs with a very simple extension to the existing spec.

[Francis Galiegue]

On Fri, Apr 25, 2014 at 3:45 PM, Christopher White
<chris...@grierwhite.com> wrote:
[...]

>
> The beauty of this is that is covers any kind of use case for merge / extend
> based on existing RFCs with a very simple extension to the existing spec.
>

Exactly, however your proposal has a problem: it contradicts JSON
Reference. You are supposed to ignore other fields other than "$ref"
when processing one ;)

My proposal is this:

{
"merge": {
"source": { "schema": "here" },
"with": { "json": "mergePatchHere" }
}
}

Along with another with "patch" instead of "merge", and the argument
to "with" would then be a RFC 6902 JSON Patch.

The advantage is that you don't "need" the source to be a JSON
Reference; one coud wonder the use of "merge" in this case but hey,
why not; among other things it allows to test merge/patch and JSON
Reference separately ;)


One last thing is that I am a little reluctant to mandate RFC 6902
support. It is not very simple... Unlike JSON Merge Patch (which has
the drawback of being MUCH less powerful, of course).

Regards,

[Christopher White]

On Friday, April 25, 2014 10:52:21 AM UTC-4, fge wrote:

On Fri, Apr 25, 2014 at 3:45 PM, Christopher White
<chris...@grierwhite.com> wrote:
[...]
>
> The beauty of this is that is covers any kind of use case for merge / extend
> based on existing RFCs with a very simple extension to the existing spec.
>

Exactly, however your proposal has a problem: it contradicts JSON
Reference. You are supposed to ignore other fields other than "$ref"
when processing one ;)

Arg -- I just came across that tidbit elsewhere.  You are right.
 

My proposal is this:

{
    "merge": {
        "source": { "schema": "here" },
        "with": { "json": "mergePatchHere" }
    }
}

Along with another with "patch" instead of "merge", and the argument
to "with" would then be a RFC 6902 JSON Patch.

Makes sense -- now I have a better understanding of why you went this route.  What do you think of "$merge" and "$patch"?  That seems to be more inline with this type of operation.

The advantage is that you don't "need" the source to be a JSON
Reference; one coud wonder the use of "merge" in this case but hey,
why not; among other things it allows to test merge/patch and JSON
Reference separately ;)

Yes -- I like that option. 
 

One last thing is that I am a little reluctant to mandate RFC 6902
support. It is not very simple... Unlike JSON Merge Patch (which has
the drawback of being MUCH less powerful, of course).

True, but there are libraries available that support this, so it's not like folks have to implement from scratch.   I personally would find merge-patch a little to weak in many scenarios.

Thanks
...cj

[Francis Galiegue]

Hello again,

On Sat, Apr 26, 2014 at 5:01 PM, Christopher White
<chris...@grierwhite.com> wrote:
[...]
>>

>> Along with another with "patch" instead of "merge", and the argument
>> to "with" would then be a RFC 6902 JSON Patch.
>>
>
> Makes sense -- now I have a better understanding of why you went this route.
> What do you think of "$merge" and "$patch"? That seems to be more inline
> with this type of operation.
>

Yep, I believe you are right here. They are indeed not "regular" keywords.

[...]

>>
>> One last thing is that I am a little reluctant to mandate RFC 6902
>> support. It is not very simple... Unlike JSON Merge Patch (which has
>> the drawback of being MUCH less powerful, of course).
>
>
> True, but there are libraries available that support this, so it's not like
> folks have to implement from scratch. I personally would find merge-patch
> a little to weak in many scenarios.
>

As to available libraries I didn't find that many actually. In Java in
particular (since I do Java), I know of no other library than mine...

I guess JSON Patch support can be mandated as well, after all. But
merge-patch is "good enough" in the vast majority of scenarios;
consider adding a property definition for instance:

// JSON Merge Patch:
{
"$merge": {
"source": { "$ref": "somewhere" },
"with": {
"properties": {
"new": { "schema": "here" }
}
}
}
}

// JSON Patch:
{
"$patch": {
"source": { "$ref": "somewhere" },
"with": [
{ "op": "add", "path": "/properties/new", "value": {
"schema": "here" }
]
}
}

In this case the merge patch is more simple to use. But yeah, if it
weren't mandated, I would certainly miss JSON Patch too.

Maybe "implementations MUST implement $merge and SHOULD implement $patch"?

[Peter Hug]

.On Saturday, 26 April 2014 02:52:21 UTC+12, fge wrote:

On Fri, Apr 25, 2014 at 3:45 PM, Christopher White
<chris...@grierwhite.com> wrote:
[...]
>
> The beauty of this is that is covers any kind of use case for merge / extend
> based on existing RFCs with a very simple extension to the existing spec.
>

Exactly, however your proposal has a problem: it contradicts JSON
Reference. You are supposed to ignore other fields other than "$ref"
when processing one ;)

If we added a new keyword we could also change some rules :). While I see some good things in your "merge" and Christopher's $patch keyword, they don't give me what I'm after.

{

  $inherits: {$ref: #/definitions/recipe},

  title: baking,

  descriptions: recipes for items you'd typically find in a bakery,

  properties: {

    preheat: {},

    backing: {},

    fan: {},

    heating: { type: string, enum: [upper, lower, both] }

  }

}

(Note that a $ref keyword, if specified, would still take precedence and all else in here would be ignored.)

$inherits could be a schema or an array of schemas (allowing to inherit from multiple sources).

I think the tricky bit is to define the rules. Some that spring to my mind are:

1. the type of schema referenced in $inherits must be "object"

2. specify all schema keywords which are not inherited (e.g. additionalProperties, ...)

3. specify all schema keywords which are prohibited in association with $inherits (e.g. type, ...)

4. specify properties which can be overridden (e.g.: title, description, ...)

5. specify properties which are merged or combined (e.g. properties, requires, ...) 

6. properties must be unique, i.e. you can neither $inherits from A and B when both specify property X nor can you specify a property in the schema's properties property which matches an inherited property

I can't see how we could add such a feature without defining strict rules. Admittedly it will make the process of writing these specifications a whole lot more difficult and time consuming, but simply leaving these out altogether is not the answer, is it?

[Francis Galiegue]

Hello,

On Mon, Apr 28, 2014 at 4:00 AM, Peter Hug
<pete...@warehouseoptimization.com> wrote:
[...]

>>
>> Exactly, however your proposal has a problem: it contradicts JSON
>> Reference. You are supposed to ignore other fields other than "$ref"
>> when processing one ;)
>
> If we added a new keyword we could also change some rules :).
>

Not that one! JSON Reference is not defined by JSON Schema; if we use
it, we must obey its rules ;)

> While I see
> some good things in your "merge" and Christopher's $patch keyword, they
> don't give me what I'm after.
>

And what exactly are you after?

> {

> $inherits: {$ref: #/definitions/recipe},
> title: baking,
> descriptions: recipes for items you'd typically find in a bakery,
> properties: {
> preheat: {},
> backing: {},
> fan: {},
> heating: { type: string, enum: [upper, lower, both] }
> }
> }
>
> (Note that a $ref keyword, if specified, would still take precedence and all
> else in here would be ignored.)
>
> $inherits could be a schema or an array of schemas (allowing to inherit from
> multiple sources).
>

I fail to see how you can _not_ do this with "$patch"?

> I think the tricky bit is to define the rules. Some that spring to my mind
> are:
>
> 1. the type of schema referenced in $inherits must be "object"
> 2. specify all schema keywords which are not inherited (e.g.
> additionalProperties, ...)
> 3. specify all schema keywords which are prohibited in association with
> $inherits (e.g. type, ...)
> 4. specify properties which can be overridden (e.g.: title, description,
> ...)
> 5. specify properties which are merged or combined (e.g. properties,
> requires, ...)
> 6. properties must be unique, i.e. you can neither $inherits from A and B
> when both specify property X nor can you specify a property in the schema's
> properties property which matches an inherited property
>
> I can't see how we could add such a feature without defining strict rules.
> Admittedly it will make the process of writing these specifications a whole
> lot more difficult and time consuming, but simply leaving these out
> altogether is not the answer, is it?
>

As you say, it is too complex. And this will lead people to thinking
that JSON Schema is just about JSON Objects whereas it isn't. JSON-LD
and friends all share that common pitfall. Let us not go there!

At least, "$patch" and "$merge" rules are simple ;)

[Peter Hug]

On Monday, 28 April 2014 20:19:15 UTC+12, fge wrote:

Hello,

On Mon, Apr 28, 2014 at 4:00 AM, Peter Hug
<pete...@warehouseoptimization.com> wrote:
[...]
>>
>> Exactly, however your proposal has a problem: it contradicts JSON
>> Reference. You are supposed to ignore other fields other than "$ref"
>> when processing one ;)
>
> If we added a new keyword we could also change some rules :).
>

Not that one! JSON Reference is not defined by JSON Schema; if we use
it, we must obey its rules ;)

> While I see
> some good things in your "merge" and Christopher's $patch keyword, they
> don't give me what I'm after.
>

And what exactly are you after?

Rules!

Consider your merge example:

{

  "merge": {

    "source": {

      "type": "object",

      "properties": { "p": { "type": "string" } },

      "additionalProperties": false

    },

    "with": {

      "properties": { "q": { "enum": [ null ] } }

    }

  }

}

We all can take a fairly educated guess what the intention is here. But consider the following example:

 {

  "merge": {

    "source": {

      "title": "Month",

      "type": "integer"

    },

    "with": {

      "title": "Mon",

      "type": "string",

      "enum": ["Jan", "Feb", ..., "Dec" ]

    }

  }

}

Please tell where this example is in violation of your proposed syntax or the rules (https://github.com/fge/json-schema-validator/wiki/v5%3a-merge). AFAIK it is fully compliant. Yet, at best this example leaves a lot of ambiguities and at worst, is totally useless (I vote for the latter).

The only way to get around these problems is to expand on the syntax and rules and once you've done that, the "$merge" keyword is no longer as simple as you claim.

[Christopher White]

The only way to get around these problems is to expand on the syntax and rules and once you've done that, the "$merge" keyword is no longer as simple as you claim.

I am coming at it from the other side of the problem -- which is I have a team of developers that are trying to write some several schemas leveraging the same basic common types, but change some basic parameters about this type.   The lack of a merge capability means that they are duplicating the same schema in multiple places with only minor changes and trying to keep them in sync.  This is a maintenance and testing nightmare.

One of my original internal proposals was very similar to what your wrote -- devise a set of possible merge options, define how the various properties fall into those categories and so on.  But as you noted it gets complicated fast, and becomes practically unusable.

As such, I'm now favoring the simple approach - give the users the power to wield with caveats about how to use it wisely, but otherwise place few restrictions on it.   As with any tool, you can certainly subvert it in horrible ways, but that doesn't necessarily mean you shouldn't build the tool.

[Francis Galiegue]

On Tue, Apr 29, 2014 at 1:37 AM, Peter Hug
<pete...@warehouseoptimization.com> wrote:
[...]
>>

Sorry but I do not see your point at all here.

Of course you can write crap like this, nothing prevents you to do
that, but you just have to have a modicum of common sense...

So, again, what is your point?

[Peter Hug]

On Tuesday, 29 April 2014 13:33:11 UTC+12, fge wrote:

Sorry but I do not see your point at all here.

Sure, that was deliberate crap, but you're missing the point. Consider this example:

{

  "merge": {

    "source": {

      "title": "foo",

      "type": "object",

      "properties": {

        "p": { "type": "string" },

        "q": { "enum": ["yes", "no"] }

      },

      "additionalProperties": false

    },

    "with": {

      "title": "bar",

      "properties": { "q": { "enum": [ null ] } }

    }

  }

}

This is not crap, but it certainly does raise some questions what the merge result will be:

• is the "#/title" "foo" or "bar"
• is the "#/additionalProperties":false or undefined
• is #/properties/q/enum [ "yes","no",null], ["yes","no"] or [null] 
  

If such issues aren't addressed, each implementation will make its own implementation decision and each author can use his own common sense (haven't you noticed that no two are the same?) of how this ought to work and in due course loose a lot of hair because it neither works as expected nor consistently with different implementation. Now where is the standard we all want?

[Peter Hug]

Sure, if you have a small number of people and only use such interfaces internally you can agree on simple customisations and implement this and get everyone to use them as expected without spelling out any detail. But as soon as you hand out your data and schema to 3rd parties you are in trouble.

Also, I never suggested that defining the rules would be complicated or make the feature practically unusable. Detailed specifications will take time to evolve, but without them, any such feature will be practically unusable.

[Francis Galiegue]

On Tue, Apr 29, 2014 at 4:33 AM, Peter Hug
<pete...@warehouseoptimization.com> wrote:
>
>
> On Tuesday, 29 April 2014 13:33:11 UTC+12, fge wrote:
>>
>>
>> Sorry but I do not see your point at all here.
>
>
> Sure, that was deliberate crap, but you're missing the point. Consider this
> example:
>
> {
> "merge": {
> "source": {
> "title": "foo",
> "type": "object",
> "properties": {
> "p": { "type": "string" },
> "q": { "enum": ["yes", "no"] }
> },
> "additionalProperties": false
> },
> "with": {
> "title": "bar",
> "properties": { "q": { "enum": [ null ] } }
> }
> }
> }
>
> This is not crap, but it certainly does raise some questions what the merge
> result will be:
>
> is the "#/title" "foo" or "bar"
> is the "#/additionalProperties":false or undefined
> is #/properties/q/enum [ "yes","no",null], ["yes","no"] or [null]
>

Well, the JSON merge patch draft gives the answer, no need to go
further than that, really:

{
"title": "bar",

"type": "object",
"properties": {
"p": { "type": "string" },

"q": { "enum": [ ] }
},
"additionalProperties": false
}

ie, an illegal schema. Yes; the only problem of JSON merge patch is
its poor null handling.

Which is why JSON Patch support is desirable.