Code Monkey home page Code Monkey logo

Comments (100)

handrews avatar handrews commented on May 16, 2024 10

VOTE-A-RAMA!!!

It's time to gauge community support for all re-use/extension/additionalProperties proposals and actually make some decisions for the next draft.

Please use emoji reactions ON THIS COMMENT to indicate your support.

  • You do not need to vote on every proposal
  • If you have no opinion, don't vote- that is also useful data
  • If you've already commented on this issue, please still vote so we know your current thoughts
  • Not all proposals solve exactly the same problem, so we may end up implementing more than one

This is not a binding majority-rule vote, but it will be a very significant input into discussions.

Here are the meanings for the emojis:

  • Celebration / Hooray / Tada!: I support this so strongly that I want to be the primary advocate for it
  • Heart: I think this is an ideal solution
  • Smiley face: I'd be happy with this solution
  • Thumbs up: I'd tolerate this solution
  • Thumbs down: I'd rather we not do this, but it wouldn't be the end of the world
  • Frowny face: I'd be actively unhappy, and may even consider other technologies instead

If you want to explain in more detail, feel free to add another comment, but please also vote on this comment.

The vote should stay open for several weeks- I'll update this comment with specifics once we see how much feedback we are getting and whether any really obvious patterns emerge.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024 2

[I'm coming back to JSON Schema after some time away from working with it, so I apologize if this comment misses something from the last year and a half or so or just exposes how rusty I am with this stuff]

A good use of $merge is with a field like readOnly, where combining multiple values using allOf doesn't make sense. What would it mean to have an allOf where readOnly is false on one side and true on the other? Using $merge (or $patch) you can easily express "just like this other thing except you can't modify it."

Another useful situation is with a re-usable type that has a serviceable default value for "description", but in many cases that description should be overridden with a description specific to the use of the type. Using allOf for this results in multiple descriptions- there are ways around the problem, but none of them are as clear and readable as $merge.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024 2

My only real concern about how the specifications are arranged or combined is that if the json-schema spec reads in a way that causes implementors to generally not support $merge and $patch, that's a big usability problem.

For interoperability, it is good for json-schema to indicate preferred preprocessing mechanisms. Otherwise we'll have a proliferation of options and tools.

What we need is a guarantee that, given a set of tools that follows the json-schema specification, we can be sure that any site that also follows the json-schema specification will work with those tools. If essential pre-processing is left out, then there is an extra API-specific step of figuring out which preprocessing you need to do. That will be a barrier to broad, consistent adoption.

It won't matter how well json-schema validates anything if it's too awkward to use as specified, and too non-standard to use effectively with additional specifications that make it usable.

Ideally, $merge and $patch could (like $ref) be their own RFC (or RFCs), with json-schema specifying that a conforming implementation should support them, and implement pre-processing and lazy evaluation in such-and-such a manner.

from json-schema-spec.

fge avatar fge commented on May 16, 2024 1

@brettz9 yes, I am fully aware of it; however, no JSON Schema keyword currently allows null as a value, so this remains a viable option.

Maybe this will not be the case in v5 though.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024 1

"type": "null" ?

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024 1

I agree with @fge that $merge covers literally all real use cases. It doesn't allow removing nulls and adding/removing array items, but I think we can live without it. Drop $patch maybe?

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024 1

I think using $merge/$patch is not only helping to maintain the schema - it allows to easily see what is the difference between the source and the resulting schema. If, for example, I need to extend a meta-schema to include schemas for some custom properties, they will be clearly visible with $merge/$patch. Same if I add/remove requirements that I want to impose on the schemas.

Programs must be written for people to read, and only incidentally for machines to execute.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024 1

@seagreen

Ehrrrm. I think correctness of a specification is more important than the readability of programs using the specification.

In my experience, if you want to sell an approach to several hundred engineers, it needs to be readable. Tooling can compensate to a degree, but at some point people always end up having to work with the actual source.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024 1

I dumped the "inherits" name a while back (in my own notes, not anywhere you would have seen :-), as it gives the wrong idea. I've been calling it $combine as I've worked on it.

And it actually does not violate any such principles, although this is not obvious. It can in fact be implemented as a preprocessor step by applying schema algebra. The resulting schema will have the correct behavior (although if there are "oneOf" and "not" keywords involved, the result is quite verbose and I would not want to read it, which is why the keyword is useful). I would assume that implementations would apply it when the schema is loaded to avoid the performance hit of applying it for each validation.

I will file it in a basic form, without the full explanation of how "oneOf" and "not" are handled inside of combined schemas- we can dig into that if it gets traction.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024 1

Alternatively, as the full problem being considered in #314 is rather complex, another option would be to add support for feature declaration:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "$requires": ["$merge"],
  "type": "..."
}

This is a more straightforward solution to allow schema authors and implementations to negotiate behavior. By declaring that the schema requires "$merge" support, the schema author ensures that their schema will either be interpreted correctly, or rejected with a clear error (unsupported optional keyword). "$patch" would be another optional requirement.

This is different from "format" support, as an implementation that does not validate based on "format" will still work with schemas that use "format", and since "format" is useful for conveying semantics to applications, it is still a useful keyword even when validation support is not present.

On the other hand "$merge" and "$patch" are structural keywords. Attempting to interpret a schema that uses either without supporting them will not end well :-P

But requiring everyone to implement them, particularly "$patch', is burdensome. Especially when many people have been quite clear that they/we neither need nor want this functionality.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

I'd personally prefer requiring the more expressive one, JSON Patch by default (and either also requiring JSON Merge Patch or making it optional). Even though JSON Patch is more cumbersome, null is, imo, not that obscure of a feature to forego if support is being added for one or the other.

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

You'll have to clarify what you mean by "And again" because this is an entirely new issue in the tracker. Can you provide some use-cases of what problems this solves?

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@ACubed I have mentioned it countless times on the forums, but anyway, here it goes again...

Schema at uri1 reads:

{
    "description": "base schema",
    "type": "object",
    "properties": { "p1": { "type": "string" } },
    "additionalProperties": false
}

Schema at uri2 wants to extend that schema so that p1 has a minimum length of 4, and add a property p2 of type integer:

{
    "$patch": {
        "source": { "$ref": "uri1" },
        "with": [
            { "op": "add", "path": "/properties/p1/minLength", "value": 4 },
            { "op": "add", "path": "/properties/p2", "value": { "type": "integer" } }
        ]
    }
}

That is with JSON Patch.

With JSON Merge Patch that would be:

{
    "$merge": {
        "source": { "$ref": "uri1" },
        "with": {
            "properties": { 
                "p1": { "minLength": 4 },
                "p2": { "type": "integer" }
            }
        }
    }
}

This means there is no need for the hacks of "limiting only to defined properties" etc.

Unambiguous and it Just Works(tm).

And yes, I said "again" since I have proposed this idea for more than a year.

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

@fge Please understand it needs to get posted here, too, for the record. Thanks!

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

Note that with JSON Merge Patch, null is used to indicate deletion of a value, but there is no mechanism in that specification for adding the value null. So while JSON Merge Patch is a nice option to have around since it is more compact and easier to see at a glance what the changes are, it has this deficiency regarding null values.

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@epoberezkin that is the JSON String "null", not JSON null

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

Ah. I now understand what you mean - there is no null in JSON-schema itself indeed.

from json-schema-spec.

erosb avatar erosb commented on May 16, 2024

@fge "enum" does permit null, doesn't it?

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

null can be used in enums, but it is ok for $merge, as it replaces the whole array. Only constant will be the issue, but it's an edge-case really.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

@fge, just to clarify. The result of both $merge and $patch is the patched schema, there is no assumption that the original schema gets modified, correct?

So I can create schemas like this:

{
  "id": "my-meta-schema.json",
  "$merge": {
    "source": "http://json-schema.org/draft-04/schema#",
    "with": {
      "properties": {
        "myCustomKeyword": { "type": "string" }
      }
    }
  }
}

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@epoberezkin yes, the result is the patched schema; the original schema is not touched in any way, shape or form.

Of course, this means requiring a preprocessing of some sort, but then so does $ref, and it still retains priority.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

With the current standard it's relatively easy to extend a schema that is not recursive without merge/patch - allOf does good enough job.

But for recursive schemas this approach simply doesn't work, so there is no mechanism to extend meta-schemas and any other recursive schemas, which is a shame... So $merge is really important.

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@erosb only as a member in an array.

But then JSON Merge Patch doesn't allow modifications of array elements individually. If you specify an array, the whole array is replaced.

JSON Patch on the other hand allows patching of arrays without a problem.

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@epoberezkin that is not correct. Your schema would be:

{
  "id": "my-meta-schema.json",
  "$merge": {
    "source": { "$ref": "http://json-schema.org/draft-04/schema#" },
    "with": {
      "properties": {
        "myCustomKeyword": { "type": "string" }
      }
    }
  }
}

The argument to source is a JSON Schema, not a URI to a JSON Schema.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

Got it. Yes, makes sense. This way you can both patch some third-party schema and also use the same patch to patch multiple schemas without the need to put them in separate files.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

FWIW, JSON Patch also has copy and move, especially useful when one wishes to keep in sync with the referenced document, however it changes... JSON Merge Patch not only can't do this but it is ambiguous as to whether a change was a result of a move or just a deletion+addition

(And incidentally, if there is ever a JSON Merge Patch 2.0, I hope it can avoid the null problem by using something like "\u0000" in place of null (and then require strings to begin with an actual NUL to be preceded by one extra NUL) as well as to allow modification of strings by allowing for inclusion of the previous string, e.g., "Prepending some text to the old value: ${current}", with \${ allowing the literal.)

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

@brettz9 (Or just introduce a "delete" operation.)

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

@ACubed: JSON Patch has the "remove" operation, but I'm speaking of JSON Merge Patch, which doesn't have--and wouldn't work with--operations per se--it just mimics the appearance (of the altered portions) of the document (as in @fge's example)--with the exception of null which it overrides. Overriding a specific string (and providing an escape option for it, as I mentioned) would allow one to avoid the ambiguity.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

The following is my ideal JSON patch/merge approach which I'll call "Power Patch". It has the full expressivity of JSON Patch, but in a more succinct form, and it can optionally include JSON Merge Patch patches and/or "Whole document" patches (explained below).

The paths mentioned below are, as in JSON Patch, JSON Pointers.

value can be any JSON value.

Order is significant, even for the keys of an object (which, incidentally, ECMAScript now assures).

mergePatchValue would behave as patch objects in JSON Merge Patch, with the exception (discussed earlier) that null could be used as an actual value to add to a document, and a single "\u0000" would trigger deletions instead (with other strings starting with NUL requiring an additional NUL to be added at the beginning). The advantage of this approach is that it allows a clear indication of where modifications are being made without needing to redefine the whole document and without needing to specify the operations (though limited to JSON Patch's add, replace, and remove).

docValue would be shaped similarly to mergePatchValue, but instead of using NUL for deletions, this object would cause any properties which had been present before and which were not specified now to be deleted. Although this might technically not be what we think of as a patch, its value lies in ensuring that each snapshot reflects the whole structure at a given moment, and without explicitly needing to delete unused items. And with its integration into "Power Patch", users have the option of switching to or from this approach as desired.

The rest of the properties (which are inspired by, and expanding on, JSON Patch), offer the advantage of allowing for moving, copying, and testing. Unlike the other approaches, by looking at any given patch, you are able to tell what parts were assumed to be already present (the paths) and what parts are new (the values). In order to allow the JSON Patch properties to work with less redundancy (as in JSON Merge Patch or the "Whole document" method), I've added basePaths which is an object whose key is a base path and whose value is itself a Power Patch object (which can therefore have its own "basePaths" for nesting paths further).

The following is a single patch showing all properties, though each property is optional. These patches could also be put within an array (or an object whose keys are versions) to indicate a sequence of patches to be applied.

{
    merge: mergePatchValue,
    whole: docValue,
    basePaths: {
        path1: {
            // Operations here would use paths (e.g., path2) relative to the base path (i.e., path1)
            add: {
                path2: value
            }
            basePaths: {
                path2: {
                    replace: {
                        path3: value
                    }
                }
            }
        }
    },
    test: {
        "path1": value
    },
    add: {
        "path1": value
    },
    replace: {
        "path1": value
    },
    move: {
        "fromPath1": "destinationPath1"
    },
    copy: {
        "fromPath1": "destinationPath1"
    },
    remove: ["path1"]
}

The only disadvantage I see to this approach (as compared to JSON Patch) is that if you want to allow some kind of custom options to a given operation, you'd have to add them as a property outside of the operation object (e.g., copyOptions). But I think the succinctness is worth it.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

One other consideration: as pertains to JSON Schema and if this format were to also be used for versioning (especially for offline storage like IndexedDB), move and copy might also be used as a signal to the application that the content itself ought to be moved or copied when there is a schema change (since the other operations would not provide enough information, e.g., whether a remove+add was deleting the schema property and its associated content or just moving it).

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@brettz9 there is a reason why JSON Patch uses arrays, and your proposal demonstrates it: you cannot rely on keys order.

In your example it is requested that basePaths be evaluated before anything else... And that won't happen!

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

Actually, ECMAScript is being redefined to allow for predictable iteration order: http://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-ownpropertykeys . (And FWIW, in my example, it is not basePaths alone that needs to work first--that is just an optional item, but yes, iteration order does need to be reliable, but thankfully, it appears that with IE no longer a sticking point, iteration order is becoming reliable again...)

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@brettz9 ECMAScript maybe. JSON? Won't happen, for backwards compatibility reasons.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

Yeah, thanks, @fge, I do see the JSON spec mentions it being "unordered".

(And FWIW, I may have mistakenly repeated this comment since for-in order states at https://tc39.github.io/ecma262/#sec-enumerate-object-properties (which is indicated by https://tc39.github.io/ecma262/#sec-runtime-semantics-forin-div-ofheadevaluation-tdznames-expr-iterationkind ) that , "The mechanics and order of enumerating the properties is not specified but must conform to the rules specified below" and the only rule relevant to OwnPropertyKeys is "EnumerateObjectProperties must obtain the own property keys of the target object by calling its [[OwnPropertyKeys]] internal method"--which doesn't suggest for-in must iterate in the order of those keys. However, one could apparently use Object.getOwnPropertyNames safely as that refers to https://tc39.github.io/ecma262/#sec-getownpropertykeys which relies on OwnPropertyKeys which does abide by the first-in order, more or less.)

I hope there will be a JSON 2.0 which fixes this and the separator problem.

But regardless, the format proposed here can still be readily modified to be JSON-friendly (and even preferable to my earlier version since it allows multiple operations of the same type at the same level, e.g., there could be one add at the beginning and one at the end--and it also allows easier access to the operation type, and also if custom options were needed, they could be added as a third argument):

[
    ["basePaths", [
        ["path1", [
            // Operations here would use paths (e.g., path2) relative to the base path (i.e., path1)
            ["add", [
                ["path2", value]
            ]],
            ["basePaths", [
                ["path2", [
                    ["replace", [
                        ["path3", value]
                    ]]
                ]]
            ]]
        ]]
    ]],
    ["test", [
        ["/path1", value]
    ]],
    ["add", [
        ["/path1", value]
    ]],
    ["replace", [
        ["/path1", value]
    ]],
    ["move", [
        ["/fromPath1", "/destinationPath1"]
    ]],
    ["copy", [
        ["/fromPath1", "/destinationPath1"]
    ]],
    ["remove", ["/path1"]]
]

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@brettz9 but why?

JSON Patch is an RFC, and implementations exist. Why go to the trouble of defining another alternative?

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

Because it is much less ugly and verbose, especially without basePaths...I also like the hybrid option since one can use the pseudo-JSON Patch for move and copy, and the improved JSON Merge Patch for everything else.

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@brettz9 beauty is in the eye of the beholder... I myself find JSON Patch to be terse, short and to the point, and your proposal to be needlessly complicated :p

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

Beauty is in the eye of the beholder (and the above could use a sequence of objects within the array instead if preferred) but terseness is not... Ignoring the merge/whole options, here's what the equivalent in JSON Patch would require:

[
    {"op": "add", "path": "/path1/path2", "value": value},
    {"op": "replace", "path": "/path1/path2/path3", "value": value},
    {"op": "test", "path": "/path1", "value": value},
    {"op": "add", "path": "/path1", "value": value},
    {"op": "replace", "path": "/path1", "value": value},
    {"op": "move", "from": "fromPath1", "path": "/destinationPath1"},
    {"op": "copy", "from": "fromPath1", "path": "/destinationPath1"},
    {"op": "remove", "path": "/path1"}
]

Using value as "value", the stringified length is 405, whereas with my version, it is only 323. And that is using basePaths in an example where the paths aren't long, and also not taking advantage of the fact that each operation can have child operations which my example only hinted at which would avoid further duplication.

from json-schema-spec.

fge avatar fge commented on May 16, 2024

@brettz9 maybe, but the JSON Patch is imnsho more readable.

Also, your paths are wrong. JSON Pointers are always absolute.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

Ah yes, thank you, fixed, including the counts (I didn't change basePaths since it is designed to avoid the need for absolute paths.)

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

FWIW, it could instead be expressed as the following (also length 323):

[
    {basePaths: [
        {path1: [
            // Operations here would use paths (e.g., path2) relative to the base path (i.e., path1)
            {add: [
                {path2: value}
            ]},
            {basePaths: [
                {path2: [
                    {replace: [
                        {path3: value}
                    ]}
                ]}
            ]}
        ]}
    ]},
    {test: [
        {"/path1": value}
    ]},
    {add: [
        {"/path1": value}
    ]},
    {replace: [
        {"/path1": value}
    ]},
    {move: [
        {"/fromPath1": "/destinationPath1"}
    ]},
    {copy: [
        {"/fromPath1": "/destinationPath1"}
    ]},
    {remove: ["/path1"]}
]

or if one did not need child arrays (length 303):

[
    {basePaths: [
        {path1: [
            // Operations here would use paths (e.g., path2) relative to the base path (i.e., path1)
            {add: {path2: value}},
            {basePaths: {
                path2: {
                    replace: {path3: value}
                }
            }}
        ]}
    ]},
    {test: {"/path1": value}},
    {add: {"/path1": value}},
    {replace: {"/path1": value}},
    {move: {"/fromPath1": "/destinationPath1"}},
    {copy: {"/fromPath1": "/destinationPath1"}},
    {remove: "/path1"}
]

Note that basePaths is not required--I'm just trying to show the full expressivity here. I think the latter part of the latter example ought to be clear in its purpose--and I think superiority--without all the extra cruft.

from json-schema-spec.

JanTvrdik avatar JanTvrdik commented on May 16, 2024

Inventing new syntax is bad idea. We should stick to relying on existing RFCs.

from json-schema-spec.

robbie-mac avatar robbie-mac commented on May 16, 2024

I'd like to derail this conversation a bit, and point out, just because we can do something, doesnt mean we should.
Ive seen a couple of examples to support patch and merge, all are based on the fact that a schema author closed their schema to additions.
Shouldnt we respect that? If we want to extend a closed schema why dont we copy the schema, make your edits, then rename it and publish it under a new uri?
We dont add a crap load of complexity to our validators. Keeping them fast and free from bloat. Yes I understand that programmers are inherently lazy SOB's. Ill put my habd up for that category. Thats why I learned to program in the first place. I wanted to automate tasks that I didnt want to do. So instead of modifying the spec to add/merge/delete/modify, why not create a toolset?

from json-schema-spec.

robbie-mac avatar robbie-mac commented on May 16, 2024

And as an aside, its interesting that people are referencing RFC's that build apon JSONSchema, but the schema itself is not an RFC. Its an internet-draft, and one that has expired at that. Unless I am completely mistaken, which if I am, the spec needs to be updated to reflect this.

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

@robbie-mac : the problem as I see it is that sometimes we want to stay in sync with an existing schema. It's not just about initial effort, but ongoing maintenance...

from json-schema-spec.

robbie-mac avatar robbie-mac commented on May 16, 2024

@brettz9
A good point.
However I agree with @JanTvrdik , I don't think this has a place in this spec, specifically because this spec defines a data format for describing other data. Its not, nor should it, include methodology for manipulating that data.
Besides, if either the merge or patch specs get approved, then you can use them on the data. You would just have to validate your source json objects by their respective schema.
As it sits now, I think that a simple change to the spec, with huge implications to validators, is to modify the definition of $schema, instead of a single string that is a uri, make it an array of strings that are uri. That way you can contain a chain of schema, the last (or first) entry is the terminus by a schema that is self describing.
Also, change the application of references to be able to reference ANY of the schema in that chain.

Now you can have a listing for the progression of changes. One that can be $ref'd for different values.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

@fge I implemented $merge and $patch in Ajv validator. Thanks.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

This keyword is a bit more complex to use and to implement than it seems on the surface because of the $ref resolution in source and in the patch (in $merge keyword the patch can be a schema).

  1. Resolve before or after merging? Ajv doesn't resolve $refs before merging at the moment but it means that you cannot extend them, you have to provide local alternatives to the same $refs (that can use $merge inside them though), which is only possible if they are relative to the current schema...
  2. What is the scope for $ref resolution? The source schema (particularly when source doesn't have it's own ID - should it take parent into account?) or the current schema?
  3. Recursive $refs (e.g. source in $merge has $ref pointing to the current schema)?

from json-schema-spec.

seagreen avatar seagreen commented on May 16, 2024

I'd like to derail this conversation a bit, and point out, just because we can do something, doesnt mean we should.
Ive seen a couple of examples to support patch and merge, all are based on the fact that a schema author closed their schema to additions.
Shouldnt we respect that? If we want to extend a closed schema why dont we copy the schema, make your edits, then rename it and publish it under a new uri?

My initial feelings lean entirely this way as well.

the problem as I see it is that sometimes we want to stay in sync with an existing schema. It's not just about initial effort, but ongoing maintenance...

Interesting. I think if that can be addressed outside of JSON schema that would be best. Perhaps one could set up a server that watches a particular schema at one URI, patches it, and then serves it at another URI? Would that cover your use case @brettz9?

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

If it doesn't make it into JSON Schema, @seagreen , I would probably be more interested in a client-side library that does the resolving/patching first based on my own custom merge/patch meta-language (like JSON References) with the remote schema having been made available to the client via a git module or npm bringing the remote schema to my server. I prefer to avoid server scripts when possible.

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

I'd like to see some use cases where (1) copying an existing schema and modifying that is preferable, and (2) where this proposal is preferable to copy-and-modify

from json-schema-spec.

brettz9 avatar brettz9 commented on May 16, 2024

The first set you presented, @awwright , is fine for:

  1. Where you are just using an original schema as a base of inspiration but wish to go in your own independent direction
  2. Where you wish to avoid any potential dangers in live-resolving from a schema not under your control.

The second set is far preferable for:

  1. Keeping one's modifications in sync with a target spec with a minimum of modifications, making for potentially easier maintenance if the target spec gets updated.
  2. Ensuring one's data remains compliant in all regards to a target spec (including the latest version), except for the tightening or loosing of restrictions one wishes to impose. Copy-and-modify might foster laziness in staying up to date with the latest spec since it requires merging changes rather than stand-off markup which may keep on working.

from json-schema-spec.

seagreen avatar seagreen commented on May 16, 2024

@epoberezkin:

I think using $merge/$patch is not only helping to maintain the schema - it allows to easily see what is the difference between the source and the resulting schema.

Great point.

"Programs must be written for people to read, and only incidentally for machines to execute."

Ehrrrm. I think correctness of a specification is more important than the readability of programs using the specification. We're still trying to figure out how $ref works in JSON Schema draft 4 (take a look through the issues and PRs here), so adding more features that will interact with $ref scares me. This doesn't mean that we shouldn't (the advantages could outweigh the disadvantages), just trying to explain my feeling here.

@handrews:

Another useful situation is with a re-usable type that has a serviceable default value for "description", but in many cases that description should be overridden with a description specific to the use of the type. Using allOf for this results in multiple descriptions- there are ways around the problem, but none of them are as clear and readable as $merge.

Nobody's saying you shouldn't have $merge, just that it shouldn't be part of the JSON Schema spec. I obviously agree that $merge is really useful. At the same time it adds no actual new validation capabilities. (This is different than $ref, which lets us express infinitely recursive schemas, which couldn't be done without $ref).

You guys are making great arguments for the expressive power of $merge and $patch though. I feel like the desire to express "this URL is just like this other one, except for $patch such-and-such" will be a commonly occurring situation even outside of JSON Schema. Would it be possible to address that with a separate protocol? Then it wouldn't have to be custom baked into many different specifications. (Also I think in general we're bad at expressing "this information can be purely derived from that information" and would love to hear more thoughts on this. It's an important problem)

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

My feelings on $merge and usability come from my experience as one of the architects of Riverbed Technology's APIs (although I left Riverbed about a year and a half ago and am speaking only for myself here). You can see $merge used extensively in the service definitions for the SteelHead and SteelCentral Controller products.

from json-schema-spec.

seagreen avatar seagreen commented on May 16, 2024

(Going to talk straight here, please interpret this as nicely as possible=) )

Specifying "this data is just like this data except for such-and-such changes" is an extremely fundamental ability. It also has no direct relation to schema-ing. As a schema specification JSON Schema should remain agnostic about it. Instead of being hardcoded in JSON Schema it should be provided by a separate specification that allows one URI to declare "this URI is just like that URI except for changes X and Y".

This has the additional advantage that is sidesteps the "incompatible preprocessors" problem you describe entirely (which I agree is a serious issue -- incompatible schema implementation are not acceptable).

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@seagreen : No worries about interpretation, directness is preferred :-) I don't take this sort of discussion personally.

I may be misremembering something, but I seem to recall the order of resolving $ref, $merge, $patch, and $data (assuming it makes the cut) as being fairly important. Do we think that if $merge and $patch were specified together in one standard that addressed how they interact with $ref, that that would be sufficient? That feels plausible but I'm still getting back up to speed here. $data is IIRC integral to json-schema and would be addressed here.

I would be entirely willing to support a multi-RFC approach if the incompatible preprocessors / usability / readability concerns are clearly solved in the process. I just don't want to have to decide on a trade-off of having readability with $merge/$patch but being a second-class citizen in the json-schema world.

from json-schema-spec.

seagreen avatar seagreen commented on May 16, 2024

No worries about interpretation, directness is preferred :-) I don't take this sort of discussion personally.

What a great attitude. That comment wasn't totally directed at you though, it was also to make it less embarrassing if I proceeded to say something totally wrong, which definitely happens:)

Do we think that if $merge and $patch were specified together in one standard that addressed how they interact with $ref, that that would be sufficient?

IMO it would help, but the unsuitability of mixing "a standard for patching data" into this spec would still not be worth it.

I would be entirely willing to support a multi-RFC approach if the incompatible preprocessors / usability / readability concerns are clearly solved in the process.

To argue against my own points: this would definitely take time and might never happen, and in the meantime people would be having to make URIs for patched data without a nice protocol to describe what they're doing.

I just don't want to have to decide on a trade-off of having readability with $merge/$patch but being a second-class citizen in the json-schema world.

I don't like the second-class citizen thing either. I would prefer no MAYs in this part of the spec -- either REQUIRED for $merge and $patch or nothing.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@seagreen : I think I'm missing something here

in the meantime people would be having to make URIs for patched data without a nice protocol to describe what they're doing.

Why is making URIs the key concern?

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@robbie-mac : With respect to your point

Ive seen a couple of examples to support patch and merge, all are based on the fact that a schema author closed their schema to additions.

I just skimmed through all of Riverbed's publicly available JSON Schema-based service definitions and could not find a single example of that pattern (adding properties to an object that set additionalProperties: false, even in a system where nearly every object has that setting).

But $merge is used very frequently. $merge (with JSON Relative Pointer) was considered a requirement in order to adopt JSON Schema, as without it, adding usage-specific information to types is either impossible or very awkward.

The complete set of $merge use cases that I saw:

  • Usage-specific "description" for documentation generation. This just doesn't work with anyOf, as you end up with multiple competing description strings. Or you have to forbid default generic descriptions entirely.
  • Further restricting validation for a more specific re-usable type or for a specific use of a type. We incorporated the "readOnly" flag from v5, so making a type read-only was common. As was imposing a "min" and/or "max" on a numeric field.
  • Merging links onto a type in order to make a resource- a specific usage of that type that related to other resources in distinct ways.

from json-schema-spec.

tadhgpearson avatar tadhgpearson commented on May 16, 2024

Indeed, the need to overwrite the description element of a $ref that @handrews mentioned is something we find in our models all the time. ( Here's an example.) So, as a user, for me this is a welcome suggestion.

The only change I would suggest is: to me the words merge and patch imply some choice in how the two elements are merged together to create an output. But this is not the case; the specification provides a precedence, and you're overwriting or replacing some element of the source reference with your provided value.

So I would choose a word like replace, overwrite or substitute instead of merge or patch.

from json-schema-spec.

seagreen avatar seagreen commented on May 16, 2024

Why is making URIs the key concern?

IMO a $merge'd document should be considered a new document, and get its own URI.

Doing things this way means we don't even need to mention $merge/$patch (or any of the other millions of ways we can think of to modify data) in the JSON Schema spec. So making URIs is a concern because it lets us use $merge/$patch (which you guys are making a convincing argument for needing) orthogonally to this spec.

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

IMO a $merge'd document should be considered a new document, and get its own URI.

This is a good point. The patch and the result are different resources that a person might want to individually address.

Also, I'm concerned about making changing schemas far cheaper than it should be. A schema that you're validating against shouldn't typically change without the knowledge of the party performing the validation.

I'm also curious as to which particular times this function would be preferred over anyOf/allOf, and if we can introduce new keywords to fix the bulk of the remaining problems.

Does anyone have some particular, concrete examples?

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

IMO a $merge'd document should be considered a new document, and get its own URI.

There is id keyword to achieve it already. There was a proposal to make it $id for consistency. So yes, the schema that uses $merge keyword will either have a new id or no id, it shouldn't inherit the id from source in any case (although source id would change resolution scope unless it's removed by the patch in with).

I'm also curious as to which particular times this function would be preferred over anyOf/allOf, and if we can introduce new keywords to fix the bulk of the remaining problems.

anyOf/allOf does not address adding allowed properties to the schemas when additionalProperties: false is used. It either difficult or impossible (in recursive schemas) to refactor the schema to address this issue.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

The example to the above would be this

Source schema:

{
  "id": "foo",
  "properties": {
    "foo": {}
  },
  "additionalProperties": false
}

This schema

{
  "anyOf": [
    { "$ref": "foo" },
    {
      "properties": {
        "bar": {}
      },
      "additionalProperties": false 
    }
  ]
}

allows one of properties but not both. No elegant solution seem to exist to this problem apart from $merge. In recursive cases there is no solution at all...

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@awwright :

I'm also curious as to which particular times this function would be preferred over anyOf/allOf, and if we can introduce new keywords to fix the bulk of the remaining problems.

Does anyone have some particular, concrete examples?

I think I provided a rather substantial set of use cases a few comments up. You can see the actual files containing the schemas demonstrating those use cases at https://support.riverbed.com/apis/ (anything with the cmc, mgmt, rios, or sh prefixes- the other services used an earlier format). $merge is used 243 times across those services.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

IMO a $merge'd document should be considered a new document, and get its own URI.

Do you mean that a document with un-evaluated $merges and the result of evaluating the $merges in that document should have different URIs? If so I do not follow the reasoning. $merge and $patch need to be evaluated lazily, IIRC from our library implementation. The code is at

https://github.com/riverbed/reschema/blob/master/reschema/jsonschema.py (schema processing)
https://github.com/riverbed/sleepwalker/blob/master/sleepwalker/datarep.py (generic client library)

I can dig through and figure out exactly what we did if there is interest (I haven't looked at this code in well over a year). I do know that we maintained a URI with JSON-pointer fragments for both the current position within the data representation and the corresponding point in the schema document against which that data fragment is validated.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@awwright :

I'm concerned about making changing schemas far cheaper than it should be. A schema that you're validating against shouldn't typically change without the knowledge of the party performing the validation.

I'm not sure how this applies. In a large-scale multi-product REST effort, the point (for me) is to apply local usage information to re-usable types at the point of use. Not changing the validation structure of the actual data instance.

If the source of the merge/patch changes, we want to pick up those changes, because we want the current definition of the type. Of course, type changes affect API versioning whether they are merged in or done directly, so some care must be taken. But this is true no matter how you incorporate $ref'd shared types.

Everyone else seems to be focused on the "I'd like to override additionalProperties: false" use case, but as I've said we did not do that at Riverbed (as best I can remember and tell from the published schemas). I don't want to dismiss the additionalProperties: false use case, as I think it is valid. JSON Schema only allows for either completely closing off the possibilities of additional properties, or leaving yourself open to missing typo'd property names during validation.

Just like choosing a strongly or weakly typed language depending on what you are programming, some APIs will decide it is better to "close off" types to get strict validation even though they do not consider the types closed to extension.

While I've generally not been a fan of an "$extends" keyword, I can see it being more clear if it means "the extension can add or override properties that are part of the validation structure." That to me is orthogonal to "this bit of metadata is not relevant to this usage and should be replaced" which is my use case for merging and patching.

Again, not necessarily advocating for $extends, but I feel like none of the current options properly address the notion of adding more properties to a type that requires strict validation of property names.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@awwright : Looking at your new site that you referenced in another issue, I noticed that you have the JSON Schema keywords divided up into validation vs annotation vs hypermedia properties.

For my use cases, it may be sufficient to allow $merge/$patch to only modify annotation properties. The only use case I have that is not covered by that, which is adding links to a schema, can be done (I think) with anyOf.

As a general rule I think it would be bad form to use $merge/$patch to change the validation in a contradictory way. The readOnly case would need to be addressed, but that property (as we were using it with $merge) is not part of the JSON Schema specification anyway.

@seagreen : In this approach, the role of the JSON Schema standard would not necessarily be to specify how $merge/$patch work in general, but to specify where and how they are legal to use with JSON Schema.

I think this might also mitigate the "new URI" problem, as the use cases for accessing annotation properties feels a bit different than for accessing validation properties. I'm still a little fuzzy on this concern so I'm not at all certain about this last point.

Obviously, restricting $merge/$patch to this usage would not help the people who want to use it to get around things like additionalProperties: false, but as mentioned in the last comment, I really think that is a distinctly different use case that, if we want to solve it, should be solved in a way that clearly addresses the semantics involved. A $merge/$patch would be guaranteed to not change the validation semantics, while some other approach ($extends?) would clearly indicate that the semantics are being modified, but in some well-specified way rather than the endless possibilities offered by $merge/$patch.

from json-schema-spec.

seagreen avatar seagreen commented on May 16, 2024

@handrews:

I'm still a little fuzzy on this concern so I'm not at all certain about this last point.

I should have done this in the first place, but I'll write out my use case now so you can see things from my perspective (whether that use case is worth changing JSON Schema is another issue, but at least you'll understand it).

I'm writing a content-addressable, schema'd datastore as an experiment in seeing what features can be built on top of the simplest building blocks possible. Building block (1) is JSON, for reasons we can probably all appreciate. Building block (2) will be a schema format for structural JSON validation.

My issue with having "$merge" and "$patch" in the core JSON Schema specification is (1) they don't actually add any new structural validation capability, and (2) they precommit me to supporting a certain way of specifying derived data.

I'm excited about making a way to specify derived data for my project, but it should happen at a layer higher than the schema layer. I shouldn't be forced to support a certain way of deriving data just because I want to use JSON Schema's structural validation features.

This is where the URI thing comes in. We're all good programmers with the skills to make a lazy-loading map of URIs to schemas. If we want to modify existing schemas and use them in our own JSON Schemas, why don't we give that derived schema a URI in the map and then link to it? There's no need for this to get mixed in with the core of the JSON Schema specification, especially as that commits us to only a certain way of deriving schemas ($merge and $patch).

@epoberezkin:

Hopefully that info dump will explain why "id" is not a solution here. By "URI" I meant a normal URI that doesn't rely on extra features specified in JSON Schema. The URI https://github.com/ can be interpreted by any tool, a link to a JSON Schema "$id" that contains a $merge to some other schema can only be interpreted by a tool that understands both JSON Schema id-URIs and the Merge spec. My suggestion to use URIs was a means to get away from pulling more specifications into JSON Schema, this isn't a feature provided by "id" URIs.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@seagreen You should look at this discussion: https://groups.google.com/forum/#!topic/json-schema/dDkozAUTxRk

TL;DR: I broke up the various use cases, and either made targeted proposals for each or argued that they are supported by existing features or should intentionally not be supported.

I note both $merge/$patch and ban-unknown-properties-mode but ultimately dismiss both options for two more specifically targeted options - one for adding properties while still not allowing unknown properties (which affects validation), and one for setting annotation properties (like "description") for each point of use (which does not affect validation at all).

Both proposals are backwards-compatible and do not rely on behavior set outside of the schema document itself. I'll be filing them on this GitHub project within the next few days- waiting for a bit more initial feedback.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@seagreen and everyone else- please give feedback to the new proposals on the mailing list or wait for the separate issues rather than overloading this issue.

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

@handrews The current draft-04 splits up the properties largely the same way. Actually, I copied the groups mostly from that.

Don't you mean "validation properties", though? What if I want to copy a schema, but with additionalProperties: true set?

In any event, I'm very cautious about this proposal. It's too powerful. You might laugh, but this is serious business! The more generic we make a solution, the less useful things we can deploy it for.

For instance, suppose I generate a SQL database using JSON Schema (a real-world use-case, too). I have two schemas: one for blog posts, and one for podcast posts that "extends" blog posts by attaching some additional fields. Because this is a SQL table, we can't store additional data, so both schemas need "additionalProperties":false, but we also need a clear way of saying "the properties body/id/publishDate/author in the 'podcast' schema are the 'same' properties found in the 'blog' schema"

With patch/merge, you lose the ability to do this.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@awwright if I am reading your question right, the proposal I called "expands"/"expandable" in the last email that I sent does apply to validation properties. Depending on exactly how we want to set it up, it may only apply to expanding the set of valid properties while otherwise preserving "additionalProperties": false Your SQL example falls in this case.

I increasingly like the idea of targeting "expands" specifically at this case because it is easily describable, has clear real-world examples, and is the most specific (and therefore least powerful) solution that addresses the use case.

The "uses" proposal is narrowly targeted at annotation (non-validation) properties.

Just be clear, it's $merge/$patch that you are asserting is too powerful? At this point I agree even though I was originally a fan. That's why I am pushing for these two much more narrowly targeted use cases to include in JSON Schema.

If other people want to do arbitrary transformations on JSON as part of constructing the schema that they want, as @seagreen indicates, that can be totally outside of JSON Schema. And therefore it wouldn't matter if they implement something like $merge/$patch or just write some Perl script or whatever. I've come to agree that JSON Schema should not be in the business of supporting arbitrary JSON transformations.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@awwright I missed your "what if I want to set additionalProperties: true?" question earlier. Good question, I can kinda see it either way. I am working on the write-up for that proposal and I will address it there. Basically, it comes down to whether the combination of properties should be symmetric (in which case you'd be either AND-ing or OR-ing boolean fields, and for additionalProperties only AND-ing makes sense, so it can never change to true) or whether one overrides the other (in which case you can override false with true as easily as true with false).

I need to think through the implications of each choice a bit more, and also about how to handle objects within objects, etc. The overall result needs to have some sort of clear and consistent intuition behind it.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

This is one of the huge controversies that apparently derailed the original post-Draft 04 discussions. I'd like to get some closure on it as it keeps popping up and throwing other discussions into limbo because it's not clear whether this will be accepted or not. The original proposer and primary advocate has long since left the project.

There is substantial opposition from @awwright, @seagreen, @robbie-mac, @JanTvrdik, and me all centered around the ideas of this either being too powerful or just not belonging within the JSON Schema specification (I was initially in favor, but found this argument against convincing- eventually :-).

@brettz9 seems to be OK or even prefer being able to implement his own outside pre-processor for whatever patching language is most useful for him.

The use case that @tadhgpearson and I need to solve is more narrowly addressed by the $use proposal in issue #98. I have sketched out some solutions for other use cases in an email to the Google group (I haven't filed the other major idea from it because the write-up is complex and I don't personally need it- I can work on it if someone wants to advocate it).

I believe that the only person in this thread who may still be advocating for this is @epoberezkin .

This seems like it qualifies as "rough consensus to reject", so I'd like to make a last call for arguments (with, say, a two week window?) and/or figure out what needs to be done to resolve this.

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

@handrews the issues that this proposal is aiming to address are real, relevant to the core of validation functionality (unlike $use you proposed that addresses less central use cases, that are covered by $merge anyway), and still not resolved.

I think @fge may have got a bit tired of the discussion but I doubt his opinion on the issue has changed. So the argument "it is too powerful" or "not belonging to the standard" are a bit strange given that it addresses the issues that definitely belong to the core of the standard. I will reiterate:

  1. Extending schemas that have additionalProperties keyword (without extremely verbose approaches of redefining all properties) - there is no alternative proposal that would solve the issue.
  2. Extending recursive schemas (without repeating all the cases where recursion can happen).

Schema extension, from what I can see, is a very common need and it makes schemas more maintainable and less verbose. The alternatives (such as using programmatic build process to create multiple schemas from a single template) make it difficult to reason about schemas and to implement in a cross platform way. On the other hand $merge keyword is very simple to implement and to understand - how can it be seen as too powerful? Beats me...

So I would suggest not closing it until some other ideas to solve the issues above are proposed.

If the issue is syntax, it can be simplified. For example to this:

{
  "$merge": "ref",
  "$with": {}
}

where $with can also be ref (string) and optional, so $merge can be used as a literal inclusion (and changing resolution scope in the included fragment, unlike $ref).

It makes the standard more complete and useful. And If we don't want $merge why would we have $use? It is defined in a more complex way (by limiting the properties it applies to) and the same arguments that are against $merge (namely, that it can be replaced by a separate pre-processing step, outside of the standard) also apply to $use.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

there is no alternative proposal that would solve the issue.

Yes there is, it's in the email thread that I linked. I'm just not going to spend time writing up all the details as an issue unless it's clearly necessary, since many of the larger proposals I've written up have been ignored so far (although $use has not), and I do not personally have any need to circumvent the current behavior of "additionalProperties": false, even having worked with ~20 services with upwards of 200 schemas.

Restating the same arguments is not going to change anyone's mind, and I don't think any one person's objections should be able to keep an issue open indefinitely (and yes, I expect at some point to have to have one or more of my proposals closed because I am that one person).

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

Yes there is, it's in the email thread that I linked.

If you refer to "inherits" I suggest to write some formal non-contradictory rules about how it works. At the moment, unlike $merge, inherits is just a vague idea, unless I am missing some issue here.

One of the problem with this keyword that it violates several design principles:

  • that schema keywords are shallow: not affected by anything but sibling keywords. Inherits will be making them dependent on other schemas
  • that validation has no context: validator will have to keep track of the properties

Also there will be some non obvious unexpected behaviours.
In any case, before we can say it is there please create an issue with the detailed description how it is going to work so we can compare.

from json-schema-spec.

awwright avatar awwright commented on May 16, 2024

I think a lot of the problems described here, and then some, are covered by #151.

It appears most of the Riverbed $merge uses are just pulling in a definition, like "TCP port number", and its constraints; and annotating it with a use-specific "description" for documentation purposes. But this could easily be done with "allOf", or maybe a special keyword. All the uses I've seen seem to be exactly described by #151.

I'm looking for something relational, so I can discern which schema/profile that an instance within a document (like a property) "belongs" to, in terms of semantic meaning. See that issue for a better description.

Are there any uses that #151 doesn't seem to describe, that this issue would solve?

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

But this could easily be done with "allOf", or maybe a special keyword.

The special keyword is proposed in #98 , $use. And yes, we only ever used $merge for that purpose, which is part of why I changed my mind to oppose the more general $merge. My current project also only needs the $use functionality.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

(I think we also might have used $merge to combine a validation definition and a LDO-ish thing, but that really should have been done with allOf)

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

As of #160 (comment) and #158 (comment) I think @epoberezkin I are coming to an agreement that both $merge/$patch and $use belong outside of JSON Schema.

I will be happy to close #98 ($use) as out of scope once this issue is similarly closed.

from json-schema-spec.

kiloquad avatar kiloquad commented on May 16, 2024

Moved to #348. Feel free to delete this comment if you like.

from json-schema-spec.

Relequestual avatar Relequestual commented on May 16, 2024

@kiloquad Currently, you can't. $merge isn't a thing yet.
You CAN work round this and do what you want by using allOf and $ref. Is that enough to work it out from here or would you like an example? =] Happy to provide one if it's helpful.

Also, would you mind moving your comment to a new issue as a question please? Trying to keep issues tidy.

from json-schema-spec.

erayd avatar erayd commented on May 16, 2024

@handrews Per your request here:

The reason $use is unsuitable for my purposes is because I want some way of achieving full inheritance, including overriding of validation keywords.

For example, $use can't do this:

{
  "definitions": {
    "parentSchema": {
      "description": "lots of complex validation rules that I don't want to copy / paste in a zillion places",
      "properties": {
        "propertyOne": {"type": "integer"},
        "propertyTwo": {"type": "string"},
        "propertyFiveHundred": {"type":"string", "pattern": "^complexRegex$"}
      }
    }
  },
  "properties": {
    "childOne": {
      "description": "same as /definitions/parentSchema, but without propertyOne, and with a different propertyTwentyFive"
    },
    "childTwo": {
      "description": "same as /definitions/parentSchema, but without propertyEightyOne and a different regex for propertyFiveHundred"
    }
  }
}

There's nothing wrong with $use if all you want to do is validate a ton of objects against a single schema, but give them all e.g. different descriptions for UI generation. However, it's patently unsuitable if you want to reuse an existing schema with minor changes to the validation logic. $use only solves a small subset of the issues that $merge / $patch can address completely.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@erayd $use is definitely not attempting to solve those problems (intentionally).

Looking at your "but without..." are you trying to add, change, and remove properties? That's a surprising definition of "inheritance" to me. Usually it should be possible to treat a child as a parent: it should validate against the parent's schema, and behave the same way semantically to the extent that the behavior is defined by the parent.

How would you describe your desired parent/child behavior?

from json-schema-spec.

erayd avatar erayd commented on May 16, 2024

@handrews

Yes, I know that $use isn't attempting to solve inheritance. However, based on previous discussion that I've read here, and please correct me if I'm wrong, there seems to be a big push to select either $use, or $merge / $patch - i.e. only one of those options will be implemented, and if $use is the chosen path, then inheritance will never happen.

...it should validate against the parent's schema...

I strongly disagree with this statement. If you override something in a child schema to behave differently (e.g. change the value of pattern, or change a required type) then something that validates against the child may well be invalid against the parent.

That said, arguing the semantics of how inheritance is defined is IMO irrelevant, the important thing is that the functionality is desirable, and $use can't do it.

How would you describe your desired parent/child behavior?

Removal is a nice-to-have, but I can do without it, it just saves a bit of duplication if it's available, and I know that $merge / $patch can do it. Changing stuff is essential, as is adding things.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@erayd My apologies, I kind of intentionally left the outcome vague, which I realized (at the time, but forgot momentarily) was likely to produce either/or arguments. In a lot of cases I want that. But it's definitely possible that we will do more than one thing, or nothing at all (decisively, as in close all of it and declare things out-of-scope of validation, but perhaps in scope for code generation).

As for the definition of inheritance... in most languages, an instance of a child class can be treated as an instance of the parent class. That's what inheritance means to me.

What you describe is what I would call monkey-patching (that's the term in Python, not sure how common it is elsewhere). You take a thing, perform arbitrary edits on its data and methods, and use the resulting thing. It's a very important technique for many test frameworks, but I would not consider it inheritance. That's not to dismiss it as a use case, I'm just trying to sort out terminology here.

One of the main objections to $merge/$patch from many is that it is overly powerful (monkey-patching vs classical inheritance, for lack of a better term).

from json-schema-spec.

erayd avatar erayd commented on May 16, 2024

@handrews

I see what you mean. For what it's worth, implementing both $use and $merge / $patch feels messy to me, noting that $use is essentially just a restricted subset of $merge. However, I'd far rather see both of them than be stuck with $use only.

As for the definition of inheritance... in most languages, an instance of a child class can be treated as an instance of the parent class. That's what inheritance means to me.

Are you able to provide some examples of what you mean? Your definition of inheritance doesn't sound like one I've ever encountered, unless I'm badly misunderstanding you.

Most implementations of inheritance I've come across don't force children to be treatable as an instance of the parent class, they only insist that the API be the same (or a superset). For example, a method that is overridden in a child class must have a declaration and return type that is compatible with the parent, but the actual implementation of that method can be anything you want -- and when called with the same arguments may return a very different result than the parent would. C++, Java and PHP are some examples of languages that work this way.

JSON schema doesn't exactly fit the classical definition of a class very well, as it has no API to speak of - perhaps that's why we're getting muddled over definitions?

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@erayd it's mostly that we're using different definitions of "behave the same", I think.

I'm not talking about the implementation of methods at all, those are irrelevant. But the semantics should be compatible, meaning that the child should not violate the parent's semantics (although the parent need not entirely satisfy the child's). Of course that's not enforced by the languages, but violating that would get thrown out in code review if not sooner in any reasonable environment.

JSON Schemas are constraint systems, so a "child" set of constraints should be the same as or more restrictive than a parent. Otherwise you're just splicing stuff together and it cannot reasonably be called inheritance in my view. That doesn't mean you can't want that splicing feature, and that sort of splicing is exactly what this issue proposes. Which is probably why it's the most controversial option.

from json-schema-spec.

erayd avatar erayd commented on May 16, 2024

@handrews I think I see what you're getting at now :-).

JSON schema already allows for making more restrictive subsets via allOf, but I do feel there's a place for something that allows creating derived schemas that are not purely a subset of the parent. $merge / $patch feels like an ideal solution to this for me, but I'd be open to other ways of achieving the same outcome.

Would having a keyword to disable this kind of derivation resolve some of the controversy? Perhaps something like only allowing derivation from schemas which do not have "final": true set.

from json-schema-spec.

Relequestual avatar Relequestual commented on May 16, 2024

If we accept this proposal, we should focus on making sure we have the required tests, and note the instnaces where we expect implementers to not support and thow errors. @epoberezkin
already has tests in his implementaion.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@erayd I would be more likely to support this if there is a provision for opting out. I think I'd prefer something like "$atomic": true, which implies that this is not something that is split-able (short of a supercollider ;-)

I don't like final because it implies inheritance which I obviously think is the wrong conceptual model for these keywords.

Fundamentally, I don't need this feature (particularly not if "readOnly" becomes an array, which solves my biggest problem), so my main concern is being able to prevent people from abusing it.

If we have it anyway, it would remove the need for a specific solution to re-using partial Link Description Objects, I suppose.

@Relequestual agreed on the need for really good tests.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

I'm trying to think about how to balance this, and the best thing I can come up with is to:

  1. Implement "$schema" as an array, which is discussed (with other options) in #314.
  2. Allow using the array form to independently signal support for plain validation or hyper-schema, plus support for "$merge"/"$patch"

This would provide a mechanism for schema authors to declare whether monkeypatching is allowed, and allow implementations to choose whether to support "$merge"/"$patch" independent of everything else. We could even allow declaring support for "$merge" and "$patch" separately. I confess that while I would like to define "$patch" if we define "$merge", in practice I only used "$merge" when I used anything like this at all. Which I have not for several years.

from json-schema-spec.

erayd avatar erayd commented on May 16, 2024

@handrews I really like that idea. Seems to me that feature declaration is a good middle ground, and it allows for adding things like $merge to the standard without requiring universal support. It also makes custom extensions easier to manage.

I do feel that the feature list needs to be namespaced somehow, to ensure that custom stuff can't conflict with 'official' names for things. Possibly using something like 'org.json-schema.$merge', which is both easily understood and is already a common namespacing format.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@erayd yeah, that would make sense. I was just thinking of the $ being "our" namespace, but this would be more flexible.

This could also easily allow someone to declare support for something like "$use" if they want a less powerful tool. "$use" is actually more complex to implement, as it needs to detect when it's being, er... mis-used :-) But it would give us (or a 3rd-party) the ability to add it in a way that is compatible with what we have so far.

I'm going to file the feature flag thing as it's own issue as I'm sure a lot of people ignore this issue as it's approaching triple-digit numbers of comments. I'm tempted to throw in a few more just to hit 100...

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

see ajv-validator/ajv-merge-patch#14,

TL;DR: allow an array of schemas to be merged as well as one schema. Users seem to need it.

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

@epoberezkin since $merge and $patch are defined in terms of media types, their behavior should not be changed. However, it should be possible to define behavior in terms of a further media type (for instance, Kubernetes defines their own patch media type which is like merge-patch+json but merges arrays based on some sort of internal code annotations).

One option might be to drop the dual keywords and instead just use one of them, and add a third field that indicates the media type of the with clause. That would be flexible and extensible (implementations could register additional patch media types like registering additional formats).

from json-schema-spec.

epoberezkin avatar epoberezkin commented on May 16, 2024

yes, the idea is to allow array in with, should not require media type change I think

from json-schema-spec.

handrews avatar handrews commented on May 16, 2024

At the end of the vote-a-rama, I said that I would consolidate these issues to focus the discussion for draft-08. I've filed #515 which outlines the two possible competing approaches. It's pretty abstract, but once we choose an approach, deciding which exact keyword and behavior we want should be less controversial. Therefore I'm closing this in favor of #515.

from json-schema-spec.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.