Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jsonb-dev] [External] : @JsonbRequired for JSONB and JSON Schema
Some comments inline

Romain Manni-Bucau
@rmannibucau |  Blog | Old Blog | Github | LinkedIn | Book


Le mar. 16 nov. 2021 à 12:38, David Kral <david.k.kral@xxxxxxxxxx> a écrit :

Hi Jean-Louis,

 

I don’t think we are getting here into the area of bean validation. This is not about validation of the content in any way.

 

The PR introduced backwards incompatible change to the creators, since the parameters are no longer required by default, some other mechanism for specifying required parameters needed to be introduced. @JsonbRequired annotation has been added mainly to provide users ability to specify which parameters are expected to be present in the JSON upon object creation. It does not care if the value has been null or value. It is simple requirement of some property to be present. If the required property is not present -> fail.


This is a wrong solution by construction and it does not reach its functional goal. If you want to restore previous behavior for existing app you must have a *system property* - or env var - cause otherwise it requires a new compilation and code change which means you don't need this annotation in such cases anyway.
 

 

I do not agree it is messy. The way I see it, is the same behaviour as we used to have, and no extended bean validation was required either. Therefore, I do not think doing any bean validation integration or even schema creation make sense.


There it is also part of the story only.
Most (all actually) users needing validations need more than that so can't use that and use something else like a jsonschema based validator or a custom one - often in interceptors or filters - for the most common cases I saw.
So having a built-in validation nobody uses - or intend to use this way - is likely wrong and in terms of consistency with the ecosystem will make the user life harder.
JSON-P needs a validation (at parser level likely), so once it is there JSON-B will just wire it providing a JsonProvider in JsonbConfig or something like that and voilà.
This is the level JSON-B must integrate with the ecosystem.
Currently the validation is super rude - no way to recover - and so incomplete that it is disabled in most applications.

Also, if we can split in 2 the topics:

1. relax the instantiation - everybody agreed on that and consider current spec as a blocker
2. required handling or not: there, there is no agreement so maybe just postpone it instead of forcing the merge?
 

 

David

 

Od: jsonb-dev <jsonb-dev-bounces@xxxxxxxxxxx> za uživatele Jean-Louis Monteiro via jsonb-dev
Odesláno: úterý 16. listopadu 2021 12:12
Komu: jsonb-dev@xxxxxxxxxxx; jsonp-dev@xxxxxxxxxxx
Kopie: Jean-Louis Monteiro <jlmonteiro@xxxxxxxxxxxxx>; Jakarta specification discussions <jakarta.ee-spec@xxxxxxxxxxx>
Předmět: [External] : [jsonb-dev] @JsonbRequired for JSONB and JSON Schema

 

Hi all,

 

I’d like to follow up on some discussions around the @JsonbRequired introduced.

 

https://github.com/eclipse-ee4j/jsonb-api/pull/285

 

On the feature itself, I clearly see some benefits to have validation happening before the bean is constructed and fields populated with the JSON payload. Good feature.

 

On the other hand, I don’t think @JsonbRequired is a good way to achieve it.

 

Basically I see 2 options for validation 

  • payload validation 
  • bean validation

 

@JsonbRequired in the bean to do payload validation does not seem accurate to me. It will set us for failure in the future.

 

If we want to address bean validation, we should consider having some integration of Bean Validation using Bean Validation annotations. Of course, this means validation will happen after the bean is constructed, but it also is way richer than “is the field required or not”. Users will be able to reuse constraints and implement business logic validation.

 

I agree that validating the payload before the object gets constructed is very interesting. But then, we should use JSON Schema for it. I am ok to create JSON Schema equivalent annotations, at least for the stable subset and the most interesting rules. From there, we can generate the JSON Schema, and feed the JSON Parser with it so the parser can effectively validate the payload before with a valid JSON Schema (same way an XML parser would get the XSD to validate the payload in JAXB).

 

Having @JsonbRequired seems to be in the middle and a hack for a very simple use case. I would not introduce it now and give us more time to discuss and think about the payload validation a bit more.

 

The hack has been introduced to solve an issue with the design of @JsonbCreator. See https://github.com/eclipse-ee4j/jsonb-api/issues/121

 

Again the design for @JsonbCreator and the need for the feature is awesome for immutable objects. The way to solve it with @JsonbRequired is messy.

 

Moreover, if we look at MicroProfile and OpenAPI, we are also half way because OpenAPI has @Schema, for instance

 

@Schema(type = SchemaType.OBJECT, implementation = InventoryList.class)

or 

@Schema(required = true)

private final String hostname;

 

From there, we generate an openapi document with all constraints (not a JSON Schema but pretty close).

 

Let’s say we need business validation or cross fields validations, we’ll use Bean Validation.

Let’s say we want OpenAPI documentation, we’ll use OpenAPI @Schema

And now let’s say we want JSON Payload validations, we’ll use @JsonbRequired?

 

Can we maybe consider the schema validation as a whole?

We could use the same set of annotations to generate the OpenAPI documentation and to generate the JSON Schema for the JSON Parser.

 

Thoughts?

I am happy to create an issue to revert @JsonbRequired if there is a consensus to wait.

_______________________________________________
jsonb-dev mailing list
jsonb-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jsonb-dev

Back to the top