json-schema-org / json-schema-spec

The JSON Schema specification
http://json-schema.org/
Other
3.76k stars 264 forks source link

"deprecation" property #74

Closed awwright closed 5 years ago

awwright commented 8 years ago

Create a schema keyword that specifies an instance (property in an instance, usually) as deprecated -- it shouldn't be used, but exists only for internal use, or for reverse compatibility with obsolete or old products.

dlax commented 7 years ago

Just to mention that the discussion is getting confusing because, as per the title of this issue, we should be talking about hyper-schema; but #391 touches the validation part, hence addressing the "per-field deprecation". (On the other hand, #173 (closed) concerned hyper-schema; but I agree with @handrews that the targetHints approach should be tried first.)

handrews commented 7 years ago

@dlax this is an annotation keyword like readOnly, so when it was filed (nearly a year ago), Hyper-Schema was the correct place for it. Hyper-Schema was in the title for historical reasons that are now irrelevant- you still see this (or Validation, or Core) as a prefix on some other issues that I haven't gotten around to re-titling. Anyway, I changed the title. Even #173 was really focused more on fields than resources, although it was actually very hard to figure out how it worked in various situations which is why it was rejected.

handrews commented 7 years ago

@levbishop thanks for the feedback and use cases.

There's a lot going on not just with this specific proposal but around how annotation keywords (which have no impact on validation, but instead just provide information about usage) should work.

Stepping back even from that, there is a principle that JSON Schema keywords can be interpreted within a single schema object in isolation. It is not necessary to know whether the schema object is a root schema or subschema. If it is a subschema, it is not necessary to know whether it is a property schema, an array item schema, or a schema describing some aspect of a hyperlink.

However, deprecation is not a stand-alone concept. Deprecation occurs within some context. A specific property member is deprecated within an object. A resource is deprecated within a hypermedia system. I'm posting the following questions for a third time, noting that no one has ever answered them. If you want to advocate for deprecated as a boolean within a property's schema, you need to answer these questions without violating JSON Schema's architectural principles:

In a root schema with a "self" link it means that the resource itself is deprecated? But it's still there currently?

In a root schema without a "self" link, or in a non-Hyper-Schema schema, it means....?

In a property subschema (in any of the property keywords) it means that all matching properties are deprecated but will still be present? [this is the one that is well understood, of course]

In a schema applying to a single position in an array/tuple ("items" is an array) it means...?

In a schema applying to multiple array positions ("items" as one schema, or "additionalItems") it means...?

In the schema for "submissionSchema", "targetSchema", "hrefSchema", "headerSchema", it means...?

In terms of implementation, how can I tell if a property is deprecated? Looking in the immediate schema for properties is not sufficient. I need to look down through any *Of keywords that are present. Your oneOf example shows this, but consider a very complex schema with multiple levels of *Of keywords. It gets more and more difficult. This, on its own, is not a fatal problem: figuring out if a link applies to an instance has the same problem, and there is a section in Hyper-Schema addressing the more complex cases. But it's a factor. And (getting back to my "larger questions about annotation keywords" comment), the same issue has caused quite a bit of confusion over what to do with title, description, and default in such combinatoric cases.

Applying deprecation at the object schema level, where the keyword values indicate which fields are deprecated in the context of that object, puts the keyword at the level where its effects are felt. When looking at an object, I want to know which fields are deprecated. Implementations, for the most part, look at one schema at a time. If I am looking at a single property's schema, in many implementations I am no longer aware that I'm looking at a nested bit of data. This simplifies implementation considerably which is why it is such a desirable property.

This is also why there is a proposal to switch readOnly to a similar approach. It's why required, which was originally a boolean within a subschema, moved out to the object schema level in draft-04.

handrews commented 7 years ago

@levbishop for your example (which I've wrapped in the object-level schema as that's obviously necessary to show the alternative)

{
  "type": "object",
  "properties": {
    "foo": {
      "anyOf": [ 
        { "type": "integer", "minimum": 0}, 
        { "deprecated": true, "type": "number"}
      ]
    }
  }
}

the equivalent would be something like this (the "then": true isn't strictly necessary, but without it one would be forgiven for wondering if the "then" got left off. But it's valid to have "if" and "else" with no "then").

{
  "type": "object",
  "if": {
    "properties": {
      "foo": {"type": "integer", "minimum": 0}
    }
  },
  "then": true,
  "else": {
    "deprecated": {"foo": "2017-10-01"},
    "properties": {
      "foo": {"type": "number"}
    }
  }
}
handrews commented 7 years ago

At the moment, this feels unlikely to be resolved for draft-07. If someone can reconcile all of the above concerns and get to a successful PR, I'll happily include it. But I'm going to put it in the "future" milestone for now.

philsturgeon commented 7 years ago

So long as we're 100% focusing on field deprecation in this issue and not hypermedia stuff (that'll be another issue/PR), I'm confident we can get this thing back on track.

It seems to me now, despite my earlier interest in "deprecated" being a keyword in the property's schema, that approach is a non-starter. The examples from @handrews make that clear to me.

GraphQL Types can get away with that simple annotation next to the field declaration as they don't have any of the if/then/else stuff JSON Schema does.

Is there any agreement on these statements:

  1. The deprecated keyword will need to work very much like required

  2. Trying to get messages involved is overkill, as the field is going away and the why doesn't really matter

  3. Trying to get versions in when JSON Schema has no opinion on versions is going to be a mess

  4. Dates are necessary to avoid panic attacks that a field might go away tomorrow, or to avoid complacency and the warning being ignores. A date provides the right level of urgency for the resolution, even though the date might not be 100% abided to on the server

@Relequestual @handrews @dlax

handrews commented 7 years ago

even though the date might not be 100% abided to on the server

I think this bit from the Sunset header proposal may be relevant

Even though the Sunset header information is made available by the resource itself, there is no guarantee that the resource indeed will become unavailable, and if so, how the response will look like for requests made after that timestamp.

I'd also say there's no guarantee that the field (in our case) won't disappear sooner either, although that would certainly annoy clients/customers and be, um... ill-advised.

dret commented 7 years ago

On 2017-09-19 13:10, Henry Andrews wrote:

I'd also say there's no guarantee that the field (in our case) won't disappear sooner either, although that would certainly annoy clients/customers and be, um... ill-advised.

i think that's the case for all these link hints. it's the same for links being annotated with "this is where to find the english version" or "this is where to find the PDF version". you'll never know for sure until you actually try at runtime. clients need to be prepared for this (i.e., shouldn't crash and burn), so making this as explicit as possible helps the robustness of the landscape.

handrews commented 7 years ago

@dret agreed. In the rewrite I'm doing for #377 (just reworking for clarity, no implementation-impacting changes), I plan to put a section about this in the Target Hints section. Part of that reorganization is to facilitate making this patterns clear rather than just tacking them onto certain individual fields.

philsturgeon commented 7 years ago

@dret just so everyone is all clear, this issue is not about link hints, only field deprecations. Let's make sure from now on we're all only talking about field deprecation. :)

handrews commented 7 years ago

@philsturgeon @dret sorry that I was my fault for bringing in the Sunset header. These things are very separate but analogous in my head so I tend to reference both without thinking that it might be confusing.

Relequestual commented 7 years ago

In agreement with @philsturgeon on all counts for this one. We should make sure the date is in an unambigious format!

adamvoss commented 7 years ago

Trying to get messages involved is overkill, as the field is going away and the why doesn't really matter

It is important to know why in order to know how to update a document to remove the deprecated member.

I think this issue may be running into of intention stemming from different usages of JSON Schema. Many of the vocal users here seem to be ones who just JSON Schema in APIs and I have even seen comments (not in this item) that JSON is always used as part of a larger system. This is not true in my usage where I will use manually maintained JSON in-lieu of a database or spreadsheet for storing information (often to be displayed by Jekyll but always) and I will have a schema for it. Similarly, many tools/applications have user-edited JSON configuration.

Regarding deprecation specifically, Microsoft even added a custom extension for JSON Schema to capture a deprecation message. See: https://github.com/Microsoft/vscode-json-languageservice/blob/a89e995877162b698e338e8301403a4ea49d5d92/src/test/parser.test.ts#L1155-L1171

handrews commented 7 years ago

@adamvoss this is going in the validation spec rather than the hyper-schema spec specifically because it applies to more than just APIs/hypermedia.

adamvoss commented 7 years ago

I'm confused by your response. I was only trying to say a deprecation message is important.

On Sep 20, 2017 8:47 PM, "Henry Andrews" notifications@github.com wrote:

@adamvoss https://github.com/adamvoss this is going in the validation spec rather than the hyper-schema spec specifically because it applies to more than just APIs/hypermedia.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/json-schema-org/json-schema-spec/issues/74#issuecomment-331027671, or mute the thread https://github.com/notifications/unsubscribe-auth/AA67P4Tg2sq-BE-6Qui9h_G1XNYUDHP_ks5skcBBgaJpZM4KRq8u .

handrews commented 7 years ago

I think this issue may be running into of intention stemming from different usages of JSON Schema. Many of the vocal users here seem to be ones who just JSON Schema in APIs and I have even seen comments (not in this item) that JSON is always used as part of a larger system.

I was replying to this part.

I don't have an opinion on the messages at the moment.

philsturgeon commented 7 years ago

@adamvoss fair enough on this:

It is important to know why in order to know how to update a document to remove the deprecated member.

That's a valid point. Would you be ok with deprecationHref or deprecationInfo or something, which would contain a URL to a place where there is more information? It could be a JSON Schema reference which lets folks know they can replace it, or a link to human readable documentation, or a blog post, etc.

This is how sunset is avoiding the need for messages, and maybe we could do the same.

handrews commented 7 years ago

@philsturgeon @adamvoss if people want to include URIs they should just do so using Hyper-Schema keywords (even if the system is not otherwise hypermedia)

{
    "deprecated": ["foo"],
    "properties": {"foo": true, "bar": true},
    "links": [{"rel": "deprecation", "href": "https://example.com/deprecations/foo"}]
}

or something like that. We already have a generic syntax for adding links so I'd rather not just toss in more keywords for specific links.

philsturgeon commented 7 years ago

LGTM. So we dont need message or link. Keeping it easy.

handrews commented 7 years ago

Also, more reasons to use and implement Hyper-Schema :-) For this use case, you wouldn't even need a full implementation, but it would hopefully encourage people to think of what else you could do with hyper-schema.

With this approach, a schema author could choose to link to a single resource that explains all deprecations for the object, or individual resources for each deprecated field, whichever makes more sense for the resource's users.

dret commented 7 years ago

On Sep 21, 2017, at 15:10, Henry Andrews notifications@github.com wrote:

With this approach, a schema author could choose to link to a single resource that explains all deprecations for the object, or individual resources for each deprecated field, whichever makes more sense for the resource's users.

and if you're really good, you'd have a single resource supporting frag ids, so that the individual links still would provide meaningful access to the specific information.

adamvoss commented 7 years ago

It is important to know why in order to know how to update a document to remove the deprecated > member.

That's a valid point. Would you be ok with deprecationHref or deprecationInfo or something, which would contain a URL to a place where there is more information? It could be a JSON Schema reference which lets folks know they can replace it, or a link to human readable documentation, or a blog post, etc.

I don't think this would allow for a very good editor experience and it creates an additional burden on a schema creator to have a place to host such information. Most of the schemas at http://schemastore.org/json/ are for things that would be manually edited by a human rather than tooling and many of the schema there are not hosted by the projects that describe them (where requiring a url/hosting would either more coordination from the target project or create a problem for external schema authors).

handrews commented 7 years ago

@adamvoss it's possible for the link to be an internal one that simply points to a different place within the schema (since any implementation may add extension keywords, and contentMediaType would allow storing, for instance, HTML). That allows for a great deal of flexibility in proving arbitrary additional deprecation information without needing to include every possibility directly in the spec.

handrews commented 7 years ago

I'm not dead-set on the link approach, but I think it is a good extension mechanism, and we will no doubt continue to find areas where some bit of data is important for a specific use case, but not broadly important enough to be worth complicating the primary specification.

It would be good to have an approach for handling these long-tail features.

adamvoss commented 7 years ago

The process you specified would only be useful to an editor if it were standardized. Otherwise the link leads allows for essentially infinite variability which cannot be codified into an algorithm. Even then, it sounds like a complicated and not very discoverable process.

This post is too long for me to just mention him directly, but if you have a concise specific wonder about the usage, it could be valuable to mentions aeschli who might be able to comment on MS's decision to add a custom property. Here is the issue that implemented that feature: https://github.com/Microsoft/vscode/issues/14260

Edit: also possibly worth linking, discussion on VSCode's use of JSON Schema.

handrews commented 7 years ago

@adamvoss if we're making up the link relation we can make up requirements for its target.

But I'm going to step back and let you and @philsturgeon sort this out while I focus on the getting hyper-schema over the finish line.

handrews commented 7 years ago

Earlier, in response to @levbishop, I said:

In terms of implementation, how can I tell if a property is deprecated? Looking in the immediate schema for properties is not sufficient. I need to look down through any Of keywords that are present. Your oneOf example shows this, but consider a very complex schema with multiple levels of Of keywords. It gets more and more difficult. This, on its own, is not a fatal problem: figuring out if a link applies to an instance has the same problem, and there is a section in Hyper-Schema addressing the more complex cases. But it's a factor.

Having thought about this in several contexts a lot more, I no longer think it's a factor, as there is inevitably a need for this sort of search to support annotation keywords in general. See #423 for my full thoughts on the matter. I'm not sure any of it solves the disagreements here, but for people who like consistent underlying theoretical frameworks, well... it's a thing :-P

philsturgeon commented 7 years ago

Because some men want to watch the world burn, here's how PayPal handle this with their own vendor prefixed deprecated OpenAPI extensions: https://github.com/paypal/api-standards/blob/master/api-style-guide.md#annotation-x-deprecated

I'd like anyone interested in this deprecated functionality to have a bit of a think about these keywords, and see if they think that the current suggestions fit.

We don't need to copy PayPal, and they're a company very focused on "API Version" as a planned and published thing, many are not that. They also get into the nitty gritty of deprecating enum values, and I'm not sure I want to live in that world - even though I've 100% had to do that in the past.

Let's all have a little think on these usages. If others could post with thoughts that'd be cool.

handrews commented 7 years ago

Thanks, @philsturgeon, I'll take a look at that.

Also, I invite you, @adamvoss, @levbishop and anyone else interested to take a look at PR #424 which lays out a way of approaching annotations and extended vocabularies, which will include whatever we end up doing for field deprecation. It might help us all think about how this should work a little more thoroughly. This is the PR resulting from issue #423 that I mentioned above. It's probably actually easier to just read the PR and ignore the issue :-P

handrews commented 6 years ago

We've done a lot of work defining what annotations are and how they get collected and used during the later parts of draft-07 and into the main work of draft-08. Which has also made some underlying JSON Schema principles more clear to me. With that in mind, I would now support adopting the single boolean form for a number of reasons:

  1. See #364 for why I'm dropping some other proposals to use arrays instead of booleans.
  2. OpenAPI has a substantial user base setting a precedent. I have no intention of adopting everything OpenAPI adopts (e.g. nullable and discriminator do not fit in JSON Schema for a number of reasons), but this was an option under serious consideration as a proposal anyway, and the one @awwright favored as the person who filed the issue. And it seems nice to keep compatibility if we can.
  3. JSON Schema in general favors annotations that can be interpreted broadly, and allows applications to narrow that interpretation. Having a simple boolean that can be backed up by application-specific behavior or extensions seems to fit that philosophy.
  4. JSON Schema in general prefers to allow harmless nonsense (and if applications come up with a use, great!) rather than trying to prevent keyword combinations that seem useless. This is relevant further down.
  5. If we really need other data (dates, links, etc.) we can add those as more keywords later.
  6. I have answers for the previously unanswered questions:

In a root schema with a "self" link is means that the resource itself is deprecated? But it's still there currently? In a root schema without a "self" link it means....? In the schema for "schema" "submissionSchema" or "targetSchema" it means...?

In all cases it means that the entire representation is somehow deprecated. Applications / APIs can provide a more specific meaning, but I would read it to mean "everything about this document is going away". That may or may not mean the resource itself goes away- we have the Sunset header for that anyway.

We are now much more clear about target and context representations and how Hyper-Schema works in general. If you write a "self" link with a target schema (in targetSchema) that is incompatible with or out of sync with your context schema (the one containing the "self" link), then that's your problem.

In general, if there's no sensible interpretation, it can and should be ignored (JSON Schema allows harmless weird things).

In a property subschema (in any of the property keywords) it means that all matching properties are deprecated but will still be present?

Um... yeah... I'm not sure why I thought this was questionable.

In a schema applying to a single position in an array/tuple ("items" is an array) it means...? In a schema applying to multiple array positions ("items" as one schema, or "additionalItems") it means...?

It's application-specific. I would guess it means something about the array elements (or certain positions of it) being deprecated without the concept of an array being deprecated. What that means I'm not sure, but if anyone actually wants to use it presumably they'll provide some guidance on what they mean. This definitely falls under the "allow harmless apparent nonsense that someone might find a good unanticipated use for anyway" approach.

There are far too many cases that need discussing and a coherent view of hypermedia resources that need to be developed before we can just toss this in.

...and we have now spent a great deal of effort developing that view. I'd say it's safe to just toss it in now :-)

Objections? @awwright ?

lashchev commented 5 years ago

@handrews Is there any consensus/progress on this?

handrews commented 5 years ago

@philsturgeon want to try to wrap this up for draft-08?

philsturgeon commented 5 years ago

This is now part of my sprint. It's got a JIRA issue and everything.

philsturgeon commented 5 years ago

Potential implementation over on #737