-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate classes in package com.fasterxml.jackson.databind.jsonschema
#3745
Conversation
5190ca4
to
fa45a5e
Compare
* @author Ryan Heaton | ||
* @author Tatu Saloranta | ||
* @deprecated Since 2.2, we recommend use of external |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not true; was not deprecated in 2.2; this will be against 2.15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this javadoc from com.fasterxml.jackson.databind.jsonschema.JsonSchema
. I would have thought that JsonSerializableSchema
and SchemaAware
were deprecated since 2.2 along with JsonSchema
but just forgot to mark with deprecated. So It should be against to 2.15?
https://github.com/FasterXML/jackson-databind/blob/2.15/src/main/java/com/fasterxml/jackson/databind/jsonschema/JsonSchema.java#L19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can't just pretend this was deprecated back in v2.2 - you have to say 2.15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, @luozhenyu it was not declared as deprecated and we cannot "backdate" decision -- even if that might have been intent. I suspect it was not deprecated just to reduce noise but either way 2.15
is needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have changed it to 2.15
public @interface JsonSerializableSchema | ||
{ | ||
/** | ||
* Marker value used to indicate that property has "no value"; | ||
* needed because annotations cannot have null as default | ||
* value. | ||
*/ | ||
public final static String NO_VALUE = "##irrelevant"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not make unrelated changes like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got it
|
||
/** | ||
* Property that can be used to indicate id of the type when | ||
* generating JSON Schema; empty String indicates that no id | ||
* is defined. | ||
*/ | ||
public String id() default ""; | ||
|
||
String id() default ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not make such mechanical changes; I know it is redundant but these changes obscure actual diff.
if (ser instanceof SchemaAware) { | ||
schemaNode = ((SchemaAware) ser).getSchema(provider, hint, | ||
isOptional); | ||
if (ser instanceof com.fasterxml.jackson.databind.jsonschema.SchemaAware) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, please leave as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the import com.fasterxml.jackson.databind.jsonschema.SchemaAware;
statement will generate a compiler warning of deprecated usage. So I replace it with a fully qualified class name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@luozhenyu I appreciate your avoiding warnings, but is this the only way? (I forget if there was another mechanism).
Ok; let's please start with a smaller schema just deprecating |
f90d016
to
21a0117
Compare
21a0117
to
13e08e4
Compare
13e08e4
to
0d21baf
Compare
Ok @luozhenyu this looks good: thank you for contributing the PR! The last before I merge this, unless I have asked and received it (apologies if so, just let me know), is Contributor License Agreement (CLA) from https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf . Looking forward to merging this! |
@cowtowncoder I have signed and sent by email~ |
com.fasterxml.jackson.databind.jsonschema
The package
com.fasterxml.jackson.databind.jsonschema
has moved to an external module JSON Schema generator module and all classes in this package should be marked with@Deprecated
.This PR deprecated classes and methods in package jsonschema. Programmers implementing
interface SchemaAware
or override methodpublic JsonNode getSchema(SerializerProvider provider, Type typeHint)
will get a deprecation warning.