[BUG] [ALL] inconsistent handling of boolean additionalProperties [JAVA] additionalProperties.put("useOptional", "false") means true in mustache template

[jpfinne]

• [x] Have you provided a full/minimal spec to reproduce the issue?
• [ ] 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?
• [ ] What's the actual output vs expected output?
• [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

xxxCodegen.processOpt() uses several techniques to process boolean additionalProperties:

if (additionalProperties.containsKey(CodegenConstants.SORT_MODEL_PROPERTIES_BY_REQUIRED_FLAG)) {
        this.setSortModelPropertiesByRequiredFlag(Boolean.valueOf(additionalProperties
                .get(CodegenConstants.SORT_MODEL_PROPERTIES_BY_REQUIRED_FLAG).toString()));
    }
}

if (additionalProperties.containsKey(USE_RUNTIME_EXCEPTION)) {
    this.setUseRuntimeException(convertPropertyToBooleanAndWriteBack(USE_RUNTIME_EXCEPTION));
}

if (additionalProperties.containsKey(SPRING_CONTROLLER)) {
    this.setUseSpringController(convertPropertyToBoolean(SPRING_CONTROLLER));
}
writePropertyBack(SPRING_CONTROLLER, useSpringController);

if (additionalProperties.containsKey(USE_OPTIONAL)) {
    this.setUseOptional(convertPropertyToBoolean(USE_OPTIONAL));
}
if (useOptional) {
    writePropertyBack(USE_OPTIONAL, useOptional);
}

Each one have slightly different behaviour or even nasty side effects.

Let's take useOptional in SpringCodegen:

additionalProperties.put("useOptional", "false") 

It gives springCodegen.useOptional = false but in templates useOptional is true For example: {{#useOptional}}useOptional:{{useOptional}}{{/useOptional}} generates useOptional:false

openapi-generator version

all (including 7.6.0)

Generation Details

Steps to reproduce

SpringCodegen codegen = new SpringCodegen();
codegen.additionalProperties().put(OPENAPI_NULLABLE, "true");
codegen.additionalProperties().put(USE_OPTIONAL, "false");
...

-> Optional<xxx> are generated in the models

SpringCodegen codegen = new SpringCodegen();
codegen.additionalProperties().put(OPENAPI_NULLABLE, "true");
codegen.additionalProperties().put(USE_OPTIONAL, false);
...

Optional<xxx> are not generated in the models

This is because OPENAPI_NULLABLE is written back in additionalProperties:

if (additionalProperties.containsKey(OPENAPI_NULLABLE)) {
    this.setOpenApiNullable(Boolean.parseBoolean(additionalProperties.get(OPENAPI_NULLABLE).toString()));
}
additionalProperties.put(OPENAPI_NULLABLE, openApiNullable);

but USE_OPTIONAL is only written back if it is equals to true or "true"

Related issues/PRs

Suggest a fix (from most verbose to least verbose)

• use convertPropertyToBooleanAndWriteBack (or equivalent) everywhere
• use Consumer in processOpts

processAdditionalProperty(USE_OPTIONAL, SpringCodeGen::setUseOptional);

public void processAdditionalProperty(String propertyKey, Consumer<Boolean> consumer) {
  if (additionalProperties.contains(propertyKey)) {
     boolean value = convertPropertyToBoolean(propertyKey);
     consumer.accept(value);
     writePropertyBack(propertyKey, value);
  }
}

• use Consumer in cliOptions.add(CliOption.newBoolean(...))

 cliOptions.add(
   CliOption.newBoolean(USE_OPTIONAL,   // property name
     "Use Optional container for optional parameters",  // description
    useOptional,   // default value
    SpringCodeGen::setUseOptional));  // setter when additionalProperty is set

and loop into all boolean cliOptions. Call processAdditionalProperty()

I think the last 2 options are the best. I can create a PR with the prefered option

[wing328]

thanks for opening an issue to track this

for option 2, personally it's not something i prefer.

for option 1, i think convertPropertyToBooleanAndWriteBack is used in many generators already so my take is to go with this function unless it has some limitations that option 3 can overcome.

[jpfinne]

There is an excellent description of the issue in JavaJAXRSCXFExtServerCodegenTest.testSettersForConfigValues by @demonfiddler

    // It's apparent that most of these setters aren't useful to client code, only to the generator itself. The only
    // reliable way to set most features is through the additional properties, since CodegenConfig.processOpts()
    // overrides are typically coded to set config fields from the additional properties, not the other way round.
    

So this issue is a general one for years.

The fix I'm implementing solves that (and several dozen issues of discrepencies betwween the java and the mustache values)

The current situation is the following: The java code sometimes uses the field value, sometimes the additionalProperty mustache uses only the additionalProperties sometimes writePropertyBack fixes the mismatches, sometimes not writePropertyBack cannot systematically be used because some code check for additionalProperties.contains() A good example is AbstractJavaCodegen and the handling of invokerPackage:

if (additionalProperties.containsKey(CodegenConstants.INVOKER_PACKAGE)) {
       this.setInvokerPackage((String) additionalProperties.get(CodegenConstants.INVOKER_PACKAGE));
} else if (additionalProperties.containsKey(CodegenConstants.API_PACKAGE)) {
...
} else if (additionalProperties.containsKey(CodegenConstants.MODEL_PACKAGE)) {        
...

this piece of code is tested dozen of times in all the java generators. It is a sign that it is very weak

My fix

Only write back to additionalProperties if it was present -> this ensures that boolean are correctly converted -> this ensures that there is no mismatch. Do not writeBack to additionalProperties if not present. Use the field value as much as possible in the java code. In mustache, use the additionalProperties as the primary context and the codegen instance in mustache as the parent context.
So mustache retrieve the values in this order:

• additionalProperty
• codegen.getter
• codegen.field

I will first only it for the java generator but it can be generalized.