Conversation
|
Guys, when are you planning to merge this PR? |
wistefan
added
the
patch
|
|
||
| private void checkForExplicitFieldOverrides(JsonSchema schema) { | ||
| var schemaNode = schema.getSchemaNode(); | ||
| var propertiesNode = schemaNode.get("definitions"); |
There was a problem hiding this comment.
shouldn't the propertiesNode be under "properties", instead of "definitions"? Please do not use magic constants, make them actual constants
|
|
||
| log.debug("Getting all fields for class: {}", clazz.getName()); | ||
|
|
||
| // Traverse the class hierarchy |
There was a problem hiding this comment.
You should add a test for this. I'm pretty sure that this wont give you the fieldnames used in the json-schema, since the json property names are defined via Jackson-Annotation, rather than the raw-property names.
| existingProperties.addAll(getAllFieldNames(beanDescription.getBeanClass())); | ||
|
|
||
| // Add standard TMForum properties that should never be overridden by schemas | ||
| existingProperties.add("id"); |
There was a problem hiding this comment.
Not every extendable entity has an id or href defined. With this, none such entity could be extended with an id or href.
wistefan
left a comment
wistefan
left a comment
There was a problem hiding this comment.
Please make this functionality configurable. It should be deactivated by default and just activated by configuration, since its a quite expensive solution for such an edge case.
| String className = voClass.getName(); | ||
| String baseClassName = null; | ||
|
|
||
| if (className.endsWith("CreateVO")) { |
There was a problem hiding this comment.
If you want to check if something is overwritten by a schema, check the class, not some other connected class. We will not hardcode semantic into the class-names.
| } | ||
|
|
||
| SimpleModule deserializerModule = new SimpleModule(); | ||
| boolean schemaValidationEnabled = Boolean.TRUE.equals(generalProperties.getValidateSchemaOverride()); |
There was a problem hiding this comment.
Why do you use an equals here? Just use the value. It would also be more readable to keep the name. "schemaValidationEnabled" is not the correct description of the functionality, while validateSchemaOverride is
Resolved conflict in ValidatingDeserializer: combined hotfix's schemaValidationEnabled/checkForExplicitFieldOverrides feature with main's change to validate only unknown properties against @schemaLocation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 participants
Greetings @wistefan
I got a ticket because apparently it is possible to change base fields in TMForum using @schemalocation, I dont think this is intended since in the documentation @schemalocation is described for adding additional attributes rather than changing existing ones. Although I might be wrong...
In case it is not intented, here is a PR that tries to solve that problem by checking with beanDescription the fields in the target class as well as the commons
Here is a curl that can trigger this substitution:
Best regards