[BUG][Java][Spring] openapiNullable - JsonNullable usage is based on nullable property instead of required property

[daberni]

Bug Report Checklist

• [x] Have you provided a full/minimal spec to reproduce the issue?
• [x] Have you validated the input using an OpenAPI validator (example)?
• [x] Have you tested with the latest master to confirm the issue still exists?
• [x] Have you searched for related issues/PRs?
• [x] What's the actual output vs expected output?
• [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

In Java CodeGeneration openapiNullable is used to determine if the special type JsonNullable should be used or not. The description of this type from the original documentation is as follows:

The JsonNullable wrapper shall be used to wrap Java bean fields for which it is important to distinguish between an explicit "null" and the field not being present. A typical usage is when implementing Json Merge Patch where an explicit "null"has the meaning "set this field to null / remove this field" whereas a non-present field has the meaning "don't change the value of this field".

So, in a request body (especially in PATCH requests) it is utilized to signalize, if a specifc field was specified in the body or not. This can be used to just ignore any change to the property and keep the old value or to apply a new value to it.

BUT. The current implementation is using the nullable property of a field instead of the required property. So nullable should actually tell about if a field can be null or not IF it is specified. nullable tells nothing about if a field must be present or can be omitted. This is what required is for. required declares, if a field as to be present or not, independent of if it may be nullable or not.

The fix would be to swap nullable and required for the usage of JsonNullable.

openapi-generator version

6.4.0, but goes back until 5.6.0 minimum (probably since openapiNullable was implemented first).

OpenAPI declaration file content or url

Existing /3_0/petstore-with-nullable-required.yaml can be used as an example

Taking the following Pet model from the referency specification:

    Pet:
      title: a Pet
      description: A pet for sale in the pet store
      type: object
      required:
        - name
        - photoUrls
      properties:
        id:
          type: integer
          format: int64
        category:
          $ref: '#/components/schemas/Category'
        name:
          type: string
          example: doggie
          nullable: true
        photoUrls:
          type: array
          xml:
            name: photoUrl
            wrapped: true
          items:
            type: string

The Pet class would be generated like:

public class Pet {

  @JsonProperty("id")
  private Long id;

  @JsonProperty("category")
  private Category category;

  @JsonProperty("name")
  private String name;

  @JsonProperty("photoUrls")
  @Valid
  private List<String> photoUrls = new ArrayList<>();
}

Although, because only name and photoUrls are declared as required, all other properties should be wrapped in JsonNullable and name should be initialized with null as it is declared as nullable additionally.

public class Pet {

  @JsonProperty("id")
  private JsonNullable<Long> id = JsonNullable.undefined();

  @JsonProperty("category")
  private JsonNullable<Category> category = JsonNullable.undefined();

  @JsonProperty("name")
  private String name = null;

  @JsonProperty("photoUrls")
  @Valid
  private List<String> photoUrls = new ArrayList<>();
}

Generation Details

./bin/generate-samples.sh bin/configs/spring-openapi-nullable-required.yaml

Steps to reproduce

I already created a pull request: #14766

Related issues/PRs

Didn't find any existing issue to this (which is quite suprising).

Suggest a fix

I already created a pull request: #14766

I am totally aware that this is a huge breaking change so I would like to discuss this issue openly. Maybe we need some kind of flag for the new behaviour. Any critics to this behaviour are welcome.

[daberni]

Just stumbled around an additional mustache template >beanValidation where required and isNullable are also mixed up.

[welshm]

I disagree with the assessment that JsonNullable should be set based on required. For the purpose of this discussion, null is considered a value.

nullable indicates that the field may accept the null value.

required indicates that a field must be set and have a value.

If a field is both nullable and required - that value may be null or any other valid value for that field.

JsonNullable is used to determine whether a null value was explicitly set or not. If a field is not nullable - then there is no need to use JsonNullable; a null value is not permitted and can be ignored. Whether it is required or not is irrelevant.

If a field is required and nullable it does not need JsonNullable because there must be a value set. There cannot be an absence of a value.

So I think JsonNullable should be used only when a field is both nullable and not required (which is what the generator is currently doing). In your example, id and category are not declared nullable and so do not need JsonNullable.

[borsch]

@daberni in general there are 4 possible scenarion for nullable + required

1. nullable=true & required=true - means field itself is required to be present in payload, but can be set to null. No need to add JsonNullable
2. nullable=true & required=false - means that payload for this field should match to one of the below case a) can have valid value b) can be set to null with the meaning "let's set value (ex. in DB) explicitly to null" c) can be absent which mean "do nothing with this field"
3. nullable=false & required=true - same as point 1, but null does not allowed. No need to use JsonNullable
4. nullable=false & required=false - same as point 2, but b) does not applicable here since can not be set to null. No need to use JsonNullable

So based on all these cases only for point 2 we can use JsonNullabl, because that's the only case where valid value, null & undefined(not present) can happen

[daberni]

@welshm Interesting approach, I didn't see it this way so far but I understand the idea.

Probably this would be a somehow working approach, the problem is, that it is not implemented this way right now.

Currently, usage of JsonNullable is only determined by the definition of nullable. You can find this in the nullableDataType.mustache template. As far as I can see, there is no further differentiation.

If this would be combined with required this might be one way.

But additionally, which is quite confusing, the beanValidation.mustache depends on required instead of nullable.

So that this would work as you described, one would have to combine this together with openapiNullable too.

One problem which currently also arises, probably because of the combination of the above incomplete implementations, that Spring Model Binding Validation doesn't work properly either. @NotNull annotations are missing on nullable=false annotations, etc.

Also after generating the code it is no longer clear which combination of flags resulted in the current code. So as a developer I would have to write different validation and mapping logic depending on required/nullable attributes, instead of coupling my mapping logic to the present types directly, which would be way more straight forward.

I see that optimizing away the one case required=false & nullable=false where theoretically JsonNullable could be determined by if the value is null or not might be an idea. But this kinda doesn't work with real life scenarios, where only because the openapi-spec tells it this way, clients of the API wouldn't sent null although it is defined nullable=false. So as a developer one must still be able to determine, if a client did send null for a nullable=false & required=false property to throw proper errors. This is currently not possible and the source of why we started to discuss this topic.

For better understanding, It would be more verbose to couple JsonNullable to the required attribute, and @NotNull to the nullable attribute.

[welshm]

openApiNullable is respected - it's used in both nullableDataType as well as pojo - and likely should be used in beanValidation as well.

I think if the @NotNull annotations are missing or incorrect, that is likely a separate issue that should also be resolved. I would also argue that beanValidation should be updated to account for nullability, since you cannot mark a field as @NotNull if is required and nullable (since it may receive a value of null).

Also after generating the code it is no longer clear which combination of flags resulted in the current code. So as a developer I would have to write different validation and mapping logic depending on required/nullable attributes, instead of coupling my mapping logic to the present types directly, which would be way more straight forward.

I think that is likely a matter of perspective - but I agree it could definitely be more clear.

So as a developer one must still be able to determine, if a client did send null for a nullable=false & required=false property to throw proper errors

That's more of a discussion on whether the generated code should be done to allow determining violations of the API contract (explicitly sending a null when it's not nullable and not required) or whether the generated code should enforce API conformance (if you do send null, the value is ignored because it's not a valid value in the contract). Are you able to determine the error if nullable=false & required=true currently? Since that would also be a violation to send an explicit null.

In my opinion, JsonNullable and @NotNull are both connected to the relationship between required and nullable and not connected to just one of the properties, but always both.

One way to resolve this would be to introduce a flag in the generator that could produce JsonNullable in the pattern you desire - and then the user of the generator can decide what pattern they prefer. This would have the added benefit of also being backwards compatible for users of the generator. generateJsonNullableForAllNonRequiredFields - what do you think?

[daberni]

Usage of openApiNullable is absolutely fine, and is set to true by default anyway.

The issue is, that @NotNul is bound to required, and JsonNullable is bound to nullable only. This is not implemented as highlighted previously ("not required and nullable"). So these two topics would need to be addressed anyway. These changes would also be backwards incompatible, as maybe many developers now already rely on the unintuitive implementation or adopted their specifications in a way to meet the desired result (as we did so far).

Regarding validation, because it is implemented incomplete right now it is not possible to validate correctly at the moment.

Having a property which is nullable = false andrequired = false (e.g. a PATCH request for non-nullable property), a client can send null for this without any issues. In the backend, one would have to assume that the property might not be passed at all, because there is no differentiation possible because of missing JsonNullable, and also javax bean validation doesn't work because of missing @NotNull annotation.

I don't see why JsonNullable and @NotNull would require do be connected to both required and nullable at the same time, just for the sake of optimizing one case where JsonNullable may be not absolutely necessary. But I get the idea behind this.

The PR was just to highlight a radical change on how I would see the correct implementation of required and nullable. It's nothing which would ever could be merged that way because it would break every existing code generation. Some configuration for a backwards compatible change is definitely necessary.

The question is, how do we want to specify the behaviour.

1) How should beanValidation work. Imho this is independent from JsonNullable and just stumbled onto it when debugging it. The current beanValidation.mustache doesn't work right now for some cases. I would prefer to bind this only to the nullable property, because I don't see why the simple validation of nullability should be bound to the JsonNullable type. Even in the desired behaviour described above, this should not be necessary.

This would be independent from the openapiNullable config but would also need some backwards compatible migration I guess (extend useBeanValidation option)

2) Should the existing JsonNullable usage be fixed in a way to represent the desired behaviour above? Because currently JsonNullable is only bound to nullable. Then a combination would be necessary with fixes to nullableDataType.mustache and pojo.mustache

3) Should there be the option to configure the generator to use a more explicit and verbose version, where nullable and required do not influence each other and have their well defined associations to @NotNull and JsonNullable.

I would definitely vote for 3) from our side (obviously :D) because 1) and 2) wouldn't matter in that case. But I would also see a clear definition and implementation for 1) and 2).

Maybe utilizing the existing openapiNullable option would be better, because all the rest is depending on it anyway. Extending it with custom values instead of true/false would imho work best.

So keeping openapiNullable with true/false for backwards compatibility but adding new values like:

• true (deprecated)
• false (deprecated)
• none (= false)
• combined (= true, default) = required & nullable will be respected
• requiredOnly

[Djerem-]

@borsch Am I correct that the behavior you describe, if validation is also considered, should be:

• Case 1: required=true & nullable=true We can accept omission as a null value and consider it to be always provided. -> No use of JsonNullable and no validation
• Case 2: required=true & nullable=false The absence of a value can be considered equivalent to a provided null value. -> No use of JsonNullable but use of @NotNull for validation.
• Case 3: required=false & nullable=true We need to know if a value is provided (e.g. for patch) but we can accept a null value. -> Use JsonNullable but no validation
• Case 4: required=false & nullable=false We need to know if a value is provided (e.g. for patch) and it cannot be a null value. -> Use JsonNullable and use of a @NotNullIfPresent** annotation

** I do not know the validation behavior of JsonNullable but, in this case, we want it to be valid on: isPresent() == false || get() != null

[ddnyer]

@Djerem- To perform correctly all validations you need to use JsonNullable for required fields as well. Else you can not detect when a required field is missing. Consider that a missing required field is equivalent to this field being null is not correct. But for those required fields, the getter/setter can use the raw type and hide the JsonNullable to the user, it's only needed for bean validation and does not need to be exposed.

• Case 1: required=true & nullable=true We need to validate that the field is present but the developers should directly use the raw type -> Use JsonNullable to store the property, use the raw type on getter/setter and use a @Present annotation on the property (not on the getter) for validation
• Case 2: required=true & nullable=false We need to validate that the field is present but the developers should directly use the raw type -> Use JsonNullable to store the property, use the raw type on getter/setter, use a @Present annotation on the property (not on the getter) and a @NotNull on the getter for validation

Obviously the @Present annotation checks that the JsonNullable has a value (null or not).

That said for retrocompatibility this strict validation for required fields should probably be optional with a parameter to enable it.

[MelleD]

From my point of view, you/we should basically think again about the JsonNullable. In my use cases it would make sense to return Optional everywhere except for the PATCH endpoints and I think that's exactly what the JsonNullable is for. IMHO the TriState only makes sense for PATCH otherwise it's not needed.

See https://github.com/OpenAPITools/jackson-databind-nullable. "typical usage is when implementing Json Merge Patch where an explicit "null"has the meaning "set this field to null / remove this field" whereas a non-present field has the meaning "don't change the value of this field"."

Currently i habe no other good RESTful usecase for JsonNullable

[welshm]

I think if it's valid for PATCH it's valid for POST as well. Some projects just use POST instead of PATCH - even it it's not "correct"

[daberni]

I absolutely agree that it is primarily relevant in a PATCH request where one can define, it is important to know if the value was passed or not.

But you can't decide on this based on the HTTP method but you have to define something different for that, and for me this is the required attribute of the properties (or the required-list).

If it is "not required" this implies that it is "optional". And in an optional manner I must be able to check if it was present or not.

If it is "not optional", it would be okay to me that the Spring implementation then uses the default value of the property if it is not present. And then an additional validation like "not null"/"nullable" would kick in. But this way you could clearly differentiate between these two configurations

[daberni]

As a sidenote: we have already adopted the mustache templates in this manner and are very happy with this approach. It would be interesting if someone else would have different requirements to this. It would be nice if we wouldn't have to maintain these templates again all the time (reintegrate with upstream changes) and in my opinion others may would benefit from this approach too.

I think the main aspect would be, when one doesn't define the "required-list" completely, every property would be JsonNullable and break a lot of stuff immediatelly. It is a bit cumbersome that everything has to be specified again in the required list, but imho that's the more correct way anyway. I don't know if there would be any simpler solution to this.

[MelleD]

I think if it's valid for PATCH it's valid for POST as well. Some projects just use POST instead of PATCH - even it it's not "correct"

Yeah you are right, that why I said in good RESTful Api(s) ;)

@daberni I think you need the nullable + required field, if you would like to have a general solution for every operation. If you say it's only for PATCH (maybe POST) then you have other solutions.

See discussion here @welshm have a good conclusion

How I said in our use cases, I would like to have everywhere Optional and for PATCH JsonNullable. There you could decide if you like to use required or nullable.

[daberni]

We are already using the suggested approach for every operation, not only PATCH.

What is the benefit of using Optional by default everywhere and JsonNullable for PATCH only? Functional wise they reflect the same thing, saying if the property is present or not. I don't see how it would improve the behaviour when we have 2 options for the same thing? Maybe I don't understand it yet and you could elaborate this?

Or would you couple Optional to nullable, and JsonNullable to required? Or what would your approach be?

[MelleD]

2 options for the same thing?

Thats not correct. @daberni Optional and JsonNullable are complete different things. One is two state the other is a tri state object. Optional present/not present JsonNullable present/not present / present with null value

Also with Collections Optional make no sense, because you can use a empty List for that. But a JsonNullable with Collection can make sense if you use it for patch, because it has a different semantic.

Look into the JSON Merge Patch specification. So how the name is calling its for PATCH :)... Again link to docs: See https://github.com/OpenAPITools/jackson-databind-nullable. "typical usage is when implementing Json Merge Patch where an explicit "null"has the meaning "set this field to null / remove this field" whereas a non-present field has the meaning "don't change the value of this field"."

Easy and quick solution: use it just for PATCH (maybe POST) operation. More general solution see @welshm closed PR. You need nullable + required to create all cases

e.g. required: true --> no Optional and no JsonNullable nullable: true required: false: --> JsonNullable nullable: false required: false: --> Optional

@welshm correct?

But how I said for a GET/DELETE/PUT JsonNullable make not so much sense :)

[MelleD]

@daberni more clear?

[jspetrak]

Persisting in 7.0.0 beta

[gnuemacscoin]

I would like to offer another perspective. To me it makes sense for the outermost null to be tied to the object's required list, the way it is now, since required precedes nullable in the schema graph. The way it is, it integrates well with MapStruct's NullValuePropertyMappingStrategy.IGNORE precisely for this PATCH use-case. All that's missing is an Optional-equivalent wrapper for nullable schemas. The current JsonNullable awkwardly ties up required with nullable instead of aiming for something more orthogonal.

[MelleD]

I don't understand how NullValuePropertyMappingStrategy.IGNORE can help you with a tristate for a PATCH use case.

[gnuemacscoin]

When merging one object into another (using void mapper methods and a @MappingTarget parameter), you tell MapStruct to skip all properties that are null in the source object. So when those null properties correspond to missing JSON keys - you get RFC 7386 semantics. The actual JSON null value needs to be represented somehow, which is what this Optional-like wrapper would be for.

[MelleD]

I don’t get what you would like to say. Please make an example with code. I don’t see how you can achieve a tristate semantic.

But also a little bit strange that we should solve a issue with extra big dependencies after a mapping step. IMHO the semantic is totally clear how to handle JsonNullable and Optional.

[jspetrak]

@gnuemacscoin When the property is null in the source object, it means that it should be set to null. When it is not present, than it should be not modified. That is why JsonNullable is used. To provide isPresent() function and distinguish the third state.

[MelleD]

Yes and the third state (reset state) make just sense for PATCH ( maybe sometimes for POST not sure).

[gnuemacscoin]

public class Opt<T> {
    public @Nullable T value;
}

public class Target {
    public @Nullable String prop;
    public @Nonnull String nonnullProp = ".*";
}

public class Source {
    public @Nullable Opt<String> prop;
    public @Nullable String nonnullProp;
}

@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE)
public interface OptMapper {
    default <T> @Nullable T from(@Nonnull Opt<T> opt) {
        return opt.value;
    }
    void patch(@MappingTarget @Nonnull Target target, Source source);
}

produces the following

public class OptMapperImpl implements OptMapper {

    @Override
    public void patch(Target target, Source source) {
        if ( source == null ) {
            return;
        }

        if ( source.nonnullProp != null ) {
            target.nonnullProp = source.nonnullProp;
        }
        if ( source.prop != null ) {
            target.prop = from( source.prop );
        }
    }

}

[MelleD]

@gnuemacscoin now I just see two states. Where is the reset state?

And again I don’t like to have a mapping step from dto to dto.

[jspetrak]

@MelleD As I see it, the if(source.prop != null) is the same logic as JsonNullable.isPresent but expressed with null value instead of method call. The from() method extracts value from Opt only if the Opt object is present and can be null. In fact, the Opt class is actually JsonNullable equivalent here.

[gnuemacscoin]

In fact, the Opt class is actually JsonNullable equivalent here.

That's what I at first thought the role of JsonNullable was, but JsonNullable wraps both null and undefined (missing key), making the outermost java null unused, and making @Nullable JsonNullable<T> illegal.

[gnuemacscoin]

And again I don’t like to have a mapping step from dto to dto.

In my example Source would be a patch DTO, and Target would be an @Entity class.

[MelleD]

I need a closer look, but I don't recognize the reset case when it is set to null.

But in the end, you create another wrapper class like jsonnullable. Instead of jackson, mapstruct does the translation. The semantic is a little different and reminds me more of a bad practice from optional wrapper. Even with an optional you can get a tristate with null, as I said, bad practice. The sense of JsonNullable and optional is that you no longer want to have null states but rather work with objects. See good article from Parlog https://medium.com/97-things/using-optional-nowhere-somewhere-or-everywhere-b1eb5645eab5

making the outermost java null unused, and making

That’s perfectly the sense of JsonNullable and Optionals not to handle nulls. If you like to handle nulls you and work with nullable you can disable JsonNullable and also Optionals. Then you don’t need it.

For this reason I see no advantage in the suggestion here with mapstruct. We can do the same things with Optional/JsonNullable when we handle required and nullable flags.

PS: I have a jsonnullable mapper with mapstruct to map the status into the domain objects. It's perfect for that. And that’s a perfect mapping use case.

What is the issue with the proposal?

[gnuemacscoin]

Anyway here's a MapStruct configuration grounded in reality of the current JsonNullable.

@Mapper(nullValuePropertyMappingStrategy = NullValuePropertyMappingStrategy.IGNORE)
public interface OptMapper {
    @Condition
    default <T> boolean isPresent(@Nonnull JsonNullable<T> nullable) {
        return nullable.isPresent();
    }
    default <T> @Nullable T from(@Nonnull JsonNullable<T> nullable) {
        return nullable.get();
    }
    default <T> @Nonnull JsonNullable<T> toJsonNullableFrom(@Nullable T value) {
        return JsonNullable.of(value);
    }
    void patch(@MappingTarget SomeEntity entity, SomeDto dto);
    SomeDto from(SomeEntity entity);
}

public class OptMapperImpl implements OptMapper {
    @Override
    public void patch(SomeEntity entity, SomeDto dto) {
        if ( dto == null ) {
            return;
        }
        if ( dto.notNullable != null ) {
            entity.notNullable = dto.notNullable;
        }
        if ( isPresent( dto.nullable ) ) {
            entity.nullable = from( dto.nullable );
        }
    }
    @Override
    public SomeDto from(SomeEntity entity) {
        if ( entity == null ) {
            return null;
        }
        SomeDto someDto = new SomeDto();
        someDto.notNullable = entity.notNullable;
        someDto.nullable = toJsonNullableFrom( entity.nullable );
        return someDto;
    }
}

[MelleD]

I would do it different, but ok that’s one valid mapper why not. It works!

But I don‘t get your point for this discussion. If you don‘t like Optional and JsonNullable and you would like handle more with nulls, you can do it right now.

This discussion here is more to avoid null and therefore use Optionals. And where you need a tristate use JsonNullable

[gnuemacscoin]

@MelleD I disagree with your proposal for

required: true --> no Optional and no JsonNullable nullable: true required: false: --> JsonNullable nullable: false required: false: --> Optional

because it's even less orthogonally designed than what we have now. If I were designing this, the most composable thing I can think of is

• @Nullable Present<@Nullable T> for optional, nullable properties,
• @Nullable Present<T> for optional, non-null properties,
• @Nullable T for required, nullable properties
• T for required, non-null properties

where

public class Present<T> {
       public T value;
}

or equivalent. Or maybe using vavr's Option<T> (or equivalent), which can contain null (unlike Optional<T>)

• Option<@Nullable T> for optional, nullable properties,
• Option<T> for optional, non-null properties,
• @Nullable T for required, nullable properties
• T for required, non-null properties

Now that I know about MapStruct's @Condition I should probably drop the previous argument about the "outermost null". Unfortunately, OpenAPI's missing object keys were at some point mistranslated to null in java, and ? in kotlin, and it probably didn't help the conversation that the question marks in C# and kotlin logically correspond to | null in typescript, as opposed to its own question mark, which is for missing keys.

[MelleD]

I completely disagree. In this case I cannot agree to abuse a wrapper class like Optional as null. For me it's not a question of Mapstruct or not. We have the concepts of Optional its plain Java so there is no need of other types.

We already have JsonNullable and they fit perfectly with the open api spec without any additional dependencies. This are two types which fits perfect for the need Optional for two states, JsonNullable for tristate. Very clear and very easy to understand.

I see no really benefit for this, because I want just 1 (special) case for null. This is the reset state in a PATCH. I want to avoid null in my code!!!

• Option<@Nullable T> for optional, nullable properties,
• Option<T> for optional, non-null properties,
• @Nullable T for required, nullable properties
• T for required, non-null properties

Also I see no sense for a required, nullable properties. What is the use case for this?

How i said my goal is to avoid null values see Parlog. Just 1 null value for the special case reset. That's it...

JsonNullable<@Nullable T> for optional, nullable properties Optional<T> for optional T for required @Nullable T for required, nullable properties --> I would use Optional cause avoiding nulls see no need for this ;)

Here every use case fits and you can decide what you need. I have no need to solve every use case with 1 wrapper class this not my goal. I just need a tristate for the json merge patch and therefore we have a special data type. So it's easy to understand. You see directly in code if you have a JsonNullable you know it's a patch case.

[gnuemacscoin]

You see what you want to see, you brush away edge cases you see no need for, you eschew composability, you're fine with making invalid states representable, and for some reason you're really pushing for this Optional, which is a dead-end if kotlin (which embraces null) and future java's null-restricted types (from Valhalla) are anything to go by.

[MelleD]

you're fine with making invalid states representable

Aha, which state is invalid?

and for some reason you're really pushing for this Optional

Better than invent new own wrapper classes.

Here is a flag https://openapi-generator.tech/docs/generators/spring/ useOptional = false. Then you have no Optional in your code.

[gnuemacscoin]

So I made an attempt at implementing

• @Nullable Present<@Nullable T> for optional, nullable properties
• @Nullable Present<T> for optional, non-null properties
• @Nullable T for required, nullable properties
• T for required, non-null properties

except I named it OptionalProperty, you may find it at https://github.com/gnuemacscoin/jackson-databind-optional-property

[robertop87]

Just a small question, I could not get the JsonNullable generated for a simple project. I just created a sample repository to show the issue.

https://github.com/robertop87/opengen

Basically the yaml definition has a non required 'tag' property defined like this:

tag:
  type: string
  nullable: true 

But the generated java code is:

private String tag;

And expected is:

private JsonNullable<String> tag = JsonNullable.undefined();

am I doing something wrong?

[tofi86]

@robertop87 I think that's not related to this issue here. You also opened an issue and it should be handled there...

[tofi86]

While I'm here, I'd like to get back to the actual discussion after it drifted off a bit recently... Let me jump back a year as I want to focus on a bug which has been mentioned, but never resolved...

On Feb 20 2023 @welshm and @borsch enlightend us about the different scenarios for JsonNullable which I absolutely agree with.

@welshm even mentioned

So I think JsonNullable should be used only when a field is both nullable and not required (which is what the generator is currently doing).

@daberni replied, that...

… it is not implemented this way right now.

Currently, usage of JsonNullable is only determined by the definition of nullable.

and @welshm replied:

openApiNullable is respected - it's used in both nullableDataType as well as pojo

and @daberni replied:

Usage of openApiNullable is absolutely fine, and is set to true by default anyway.

The issue is, that @NotNul is bound to required, and JsonNullable is bound to nullable only. This is not implemented as highlighted previously ("not required and nullable").

Then discussion drifted off...

Long story short: I think @daberni is still right with his statement - even with version 7.3.0 this is not generated as @welshm stated and it is still based exclusively(!) on the nullable property.

This is my example which I just ran with v7.3.0:

Config:

generator: spring
useOptional: false
openApiNullable: true

Schema:

JsonNullableAndRequiredTestDto:
  type: object
  properties:
    nullableTrueRequiredTrueExpectString:
      type: string
      nullable: true
      description: |
        A field which MUST be included in the JSON and whose value MAY be null.
        -> Ergo, a POST/PATCH request can set the field to null in the entity/database.
        -> Expected code generation type: String
    nullableTrueRequiredFalseExpectJsonNullable:
      type: string
      nullable: true
      description: |
        A field which MAY be included in the JSON and whose value MAY be null.
        -> Ergo, a POST/PATCH request can set the field to null in the entity/database.
        -> Omitting the property in the JSON should neither touch nor clear the target DB field!
        -> Sending the property in the JSON with a (value) should update (String) or clear (null) the target DB field!
        -> Expected code generation type: JsonNullable<String>
    nullableFalseRequiredTrueExpectString:
      type: string
      description: |
        A field which MUST be included in the JSON and whose value MUST NOT be null.
        -> Ergo, a POST/PATCH request is not able to null the field in the entity/database.
        -> Expected code generation type: String
    nullableFalseRequiredFalseExpectString:
      type: string
      description: |
        A field which MAY be included in the JSON and whose value MUST NOT be null.
        -> Ergo, a POST/PATCH request is not able to null the field in the entity/database.
        -> Omitting the property in the JSON should neither touch nor clear the target DB field!
        -> Sending the property in the JSON should update the target DB field!
        -> Expected code generation type: String
  required:
    - nullableTrueRequiredTrueExpectString
    - nullableFalseRequiredTrueExpectString

which results in the following Java code:

public class JsonNullableAndRequiredTestDto {

  private JsonNullable<String> nullableTrueRequiredTrueExpectString = JsonNullable.<String>undefined();

  private JsonNullable<String> nullableTrueRequiredFalseExpectJsonNullable = JsonNullable.<String>undefined();

  private String nullableFalseRequiredTrueExpectString;

  private String nullableFalseRequiredFalseExpectString;

 ...
}

For nullableTrueRequiredTrueExpectString as tri-state JsonNullable is generated but technically isn't needed!

So it looks like v7.3.0 does not adhere to the description of JsonNullable generation @welshm and @borsch gave a year ago.

Does anyone have a mustache workaround for this? Or does this need a proper bugfix in the code generator?

---

Addendum: Since v7.2.0 i guess useOptional: true also has an effect on the models:

public class JsonNullableAndRequiredTestDto {

  private JsonNullable<String> nullableTrueRequiredTrueExpectString = JsonNullable.<String>undefined();

  private JsonNullable<String> nullableTrueRequiredFalseExpectJsonNullable = JsonNullable.<String>undefined();

  private String nullableFalseRequiredTrueExpectString;

  private Optional<String> nullableFalseRequiredFalseExpectString = Optional.empty();

 ...
}

but this doesn't solve the above issue and doesn't make things better in my eyes...

It's just making your models worse if you previously used useOptional: true for your controller methods...

[MelleD]

A technically correct answer (there are also other opinions) has been given a long time ago.

in general there are 4 possible scenarion for nullable + required nullable=true & required=true - means field itself is required to be present in payload, but can be set to null. No need to add JsonNullable nullable=true & required=false - means that payload for this field should match to one of the below case a) can have valid value b) can be set to null with the meaning "let's set value (ex. in DB) explicitly to null" c) can be absent which mean "do nothing with this field" nullable=false & required=true - same as point 1, but null does not allowed. No need to use JsonNullable nullable=false & required=false - same as point 2, but b) does not applicable here since can not be set to null. No need to use JsonNullable So based on all these cases only for point 2 we can use JsonNullabl, because that's the only case where valid value, null & undefined(not present) can happen

The question, however, is what is your need? What is your use case? What does it mean semantically? So far no one has been able to describe a use case for this case. What does it mean if field has to be in the payload but can be null. What is the semantic difference between null in the payload and the attribute does not exist?

And yes off course you can customize and override the existing template: https://openapi-generator.tech/docs/templating/

here is the template: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/JavaSpring/nullableDataType.mustache

By the way, I think that not only JsonNullable is a problem but also beanvalidation because it sets a @NotNull for required fields

[jspetrak]

I can say that openapi beginners have quite a hard time to understand the correct semantics of required and nullable. And, based on actual generator implementations, tend to bend the actual api definitions to product the source code they desire.

What is the semantic difference between null in the payload and the attribute does not exist?

This would be usable for partial patch. Attribute missing means "do not touch the current value and I am not stating what the current value is". For POST, PUT, I would expect a default value so there can be clearing substitute for property not available in the payload.

[MelleD]

This would be usable for partial patch. Attribute missing means "do not touch the current value and I am not stating what the current value is".

Correct thats the reason why there is the JsonNullable. If you don't want to use a Container class which handles this then you can disable it. And now you have arguments why it is like it is currently ;) and not to change it. You can do it right now your case no change necessary

For POST, PUT, I would expect a default value so there can be clearing substitute for property not available in the payload.

But there is in open api no semantic for that. That why I use different DTO for POST and PATCH POST without json nullable and PATCH for JSON Merge PATCH with JsonNullable. But also currently complete possible without changes. Thats why I ask, for case 1 required true and nullable true I know no use case.

I can say that openapi beginners have quite a hard time to understand the correct semantics of required and nullable. And, based on actual generator implementations, tend to bend the actual api definitions to product the source code they desire.

That's true but another issue ;)

[jgosmann]

@MelleD

The question, however, is what is your need? What is your use case? What does it mean semantically? So far no one has been able to describe a use case for this case. What does it mean if field has to be in the payload but can be null. What is the semantic difference between null in the payload and the attribute does not exist?

Our use case would be to generate consistent types for our TypeScript client and Java/Spring backend and ensure that certain parameters must be passed. In TypeScript/JavaScript the distinction of nullable and required is represented with undefined and nullable. In particular, a required, but nullable field will allow the assignment of null, but not of undefined. In TypeScript this enforces that the field must be set either to a value or explicitly null. This can be quite useful to prevent that a developer forgets to set a parameter when making a request with the client.

While the TypeScript types are generated in exactly the way I expect, the Java/Spring types are actually generated in an incompatible way and just seem plainly wrong to me, no matter if there is a use case or not (which there is as described above). In fact, it is not even working properly. When I send null for a nullable required property in the JSON request body (the Java type being JsonNullable), the endpoint will return a 400/bad request because the validation of the request body fails; complaining that the property must not be null (though, I'm not quite sure why because it deserializes fine). Bottom line, we would like to use nullable required properties, but we cannot because it breaks the handling in the Java code, effectively making client and server incompatible.

[jgosmann]

The problem I'm describing might be in part due to #11969, but even when I set openApiNullable to false (so that no JsonNullable type is used), the validation fails when sending an null for a nullable required field.

Edit: Ah well, it has been mentioned before:

By the way, I think that not only JsonNullable is a problem but also beanvalidation because it sets a @NotNull for required fields

[MelleD]

But you're talking about symptoms with different technologies and maybe problems there. I'am talking more about good API design and if the request really make sense.

From a purely technical perspective, I don't understand what the difference is between setting a field in the JSON to null or simply leaving it out. What's the difference for you? Why is the request valid if you put null in it and invalid if the field is missing? What use is this information and distinction to the backend? I think this is a simple question :)

I've never had to distinguish between them unless you use JSON Merge Patch. This is the only reason why JsonNullable exists, when you really need a tri state. https://github.com/OpenAPITools/jackson-databind-nullable

A typical usage is when implementing Json Merge Patch where an explicit "null"has the meaning "set this field to null / remove this field" whereas a non-present field has the meaning "don't change the value of this field".

we would like to use nullable required properties

What is stopping you?

[jspetrak]

@MelleD How do you apply default value other then omitting the property and having the default used? Passing property: null means "the value is null" not "the value is default". This is other use case than merge patch. And please, don't tell me that properties with default cannot be null.

[MelleD]

And please, don't tell me that properties with default cannot be null.

I'm not saying anything, I haven't had the case yet. Of course we can now conjure up a case artificially. But no one was able to describe a real use case. There is no question that there is a lot that can be done technically. But no one can describe a single use case to me without getting technical.

Just to make it clear, this doesn't concern me at all, I'm just interested in how you design APIs and why. Even if the case is really "somehow" fixed, it won't affect me because, as I said, I've never designed an API like this before and no one can describe it.

Passing property: null means "the value is null" not "the value is default"

But anyway see technical no issue here and also no need for a required attribute

attribute foo
nullable: true
required:false

JsonNullable absent --> default value
JsonNullable present null value --> null
JsonNullable present value --> value

@jspetrak solved or not?