[BUG][JAVA] Not specifying additionalProperties should generate the Map
Bug Report Checklist
- [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?
- [x] What's the actual output vs expected output?
- [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description
Hello,
Regarding additionalProperties and the specification here: https://json-schema.org/understanding-json-schema/reference/object#additionalproperties, I think there might be a bug in the Java spring generator.
I'm not specifying the additionalProperties property and in this case, because of the specification mentioning:
By default any additional properties are allowed.
I would expect the JsonWebKey generated class to contain the Map<String, Object> additionalProperties (and get/put methods) to be generated. But it is not !
It seems the only way to generate this map is to use either:
additionalProperties: true
Or:
additionalProperties:
type: string
Which I would like to avoid and shouldn't be necessary.
Am I missing something ?
Thank you for your help !
openapi-generator version
7.10.0 (current latest) 7.11.0-SNAPSHOT (2b891f6da9db9cf24fe0186f5635d550c958c657)
OpenAPI declaration file content or url
My src/main/resources/swagger/swagger.yaml file:
swagger: '2.0'
info:
version: '1.0'
title: FooBar
paths:
/jwks:
get:
operationId: getJwks
responses:
200:
description: "Success"
schema:
type: "array"
items:
$ref: "#/definitions/JsonWebKey"
definitions:
JsonWebKey:
title: JSON Web Key
type: object
properties:
kid:
type: string
# No additionalProperties specified
Generation Details
I'm using this pom.xml:
https://gist.github.com/ncelerier/1729b37b6f9d37aefc8af33926fed07b
Steps to reproduce
Running mvn clean install, then looking at the generated file if the Map is generated or not:
target/generated-sources/openapi-server/src/main/java/com/example/models/JsonWebKey.java
Related issues/PRs
Suggest a fix
I can suggest something but I would like to know first if this is really a bug or if I am missing something.
did you try setting disallowAdditionalPropertiesIfNotPresent to false? (e.g. --additional-properties disallowAdditionalPropertiesIfNotPresent=false in CLI)
https://openapi-generator.tech/docs/generators/java/
Hi @wing328, I have this property in the pom.xml :
<configOptions>
<disallowAdditionalPropertiesIfNotPresent>false</disallowAdditionalPropertiesIfNotPresent>
</configOptions>
Could it be that it is not taken into account ?
I think there might be a bug in the Java spring generator.
Ah. I can only assure this option works for java client generator (jersey2, jersey3, okhttp-gson) but not the spring generator. Sorry.
One way to check is to remove the option or set the option to true to see if there's any change in the output
For the java client generator, I tried with this and indeed the Map is generated:
<generatorName>java</generatorName>
<configOptions>
<disallowAdditionalPropertiesIfNotPresent>false</disallowAdditionalPropertiesIfNotPresent>
</configOptions>
However, if I set additionalProperties: false in my schema, the Map is still generated whereas it shouldn't.
@ncelerier Please can you help confirm if this is the case for the default library and the java generator?
Hello, sorry for this late response.
Forget what I said about the java client generator and additionalProperties: false : the Map is not generated which is the expected behaviour. Sorry, I think I mixed things up.
Here is a recap of the current behaviour using version 7.11.0 :
| generatorName | additionalProperties value | additionalProperties field/methods present | Result |
|---|---|---|---|
| spring | Not specified | No | :x: Should be present |
| spring | additionalProperties: false | No | :heavy_check_mark: |
| spring | additionalProperties: true | Yes, Map<String, Object> |
:heavy_check_mark: |
| spring | additionalProperties: type: string | Yes, Map<String, String> |
:heavy_check_mark: |
| java | Not specified | Yes, Map<String, Object> |
:heavy_check_mark: |
| java | additionalProperties: false | No | :heavy_check_mark: |
| java | additionalProperties: true | Yes, Map<String, Object> additionalProperties |
:heavy_check_mark: |
| java | additionalProperties: type: string | Yes, Map<String, Object> additionalProperties |
:x: Should be Map<String, String> additionalProperties, as with the spring generator |
Hey, @wing328, sorry for taking so long, but I think I see the cause of the issue:
https://github.com/OpenAPITools/openapi-generator/blob/de310f6ee16a8d6a44c27e0dca744fee1828b30f/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java#L2868
isDisallowAdditionalPropertiesIfNotPresent is not taken into account at this point, so addAdditionPropertiesToCodeGenModel is never called, which means CodegenModel.additionalPropertiesType is never set here:
https://github.com/OpenAPITools/openapi-generator/blob/de310f6ee16a8d6a44c27e0dca744fee1828b30f/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java#L2333
My idea is to add another if-else branch to this block with isDisallowAdditionalPropertiesIfNotPresent, but since DefaultGenerator is such a fundamental class, I want to know if simply adding a test case like givenAdditionalPropertiesNotDefinedAndIsDisallowAdditionalPropertiesIfNotPresentIsFalseWhenGenerateModelThenAdditionalPropertiesTypeIsObject is enough, or if there's more involved.
Also, for the next part of this issue:
❌ Should be Map<String, String> additionalProperties, as with the spring generator
I see that this is because Object is hardcoded for okhttp-gson which is the default, and it seems to be that way because of the custom adapter:
https://github.com/OpenAPITools/openapi-generator/blob/de310f6ee16a8d6a44c27e0dca744fee1828b30f/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache#L538
Does this mean that not much can be done without a complete rewrite? I'm not sure if it's worth changing if that's the case
@wing328 @martin-mfg , please what are your thoughts?
Hi @ncelerier
to make sure we're all on the same page: all rows of the table in https://github.com/OpenAPITools/openapi-generator/issues/20139#issuecomment-2607479080 were tested with disallowAdditionalPropertiesIfNotPresent=false, correct?
Hi @DavidGrath,
isDisallowAdditionalPropertiesIfNotPresentis not taken into account at this point, soaddAdditionPropertiesToCodeGenModelis never called
This is true for both the Spring generator and the Java generator, right? Yet they behave differently if additionalProperties is not specified. So I think the problem is located elsewhere. The reason could be that java okhttp-gson uses this check: https://github.com/OpenAPITools/openapi-generator/blob/f409fb61848f71cb464f2caa7d15364703d311b7/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/additional_properties.mustache#L1 while spring uses a slightly different check: https://github.com/OpenAPITools/openapi-generator/blob/d27fa00cf55d3d7aae46bb9f8affbf33f057432e/modules/openapi-generator/src/main/resources/JavaSpring/additionalProperties.mustache#L1 I didn't look further into this though.
Does this mean that not much can be done without a complete rewrite? I'm not sure if it's worth changing if that's the case
We could at least improve the situation a bit by adding some code which verifies that incoming data complies with the expected data type.
Alternatively we could completely change the implementation for additionalProperties in okhttp-gson - not by rewriting it but instead by copying over the additionalProperties implementation from a different library or Java-based generator - e.g. spring.
Either way this seems to be independent of the originally discussed problem, so it can be addressed in a separate PR.
https://github.com/OpenAPITools/openapi-generator/pull/21229
@martin-mfg, I was able to confirm. I found out that these libraries all use the same additional properties mustache file:
jersey3, jersey2, feign, resttemplate, webclient, restclient, native
After making the if-else change, I ran a DataProvider test with all the Java libraries, including the ones not listed above, and all the other ones failed the test, except feign-hc5 for some reason, but I reverted back to a single hard-coded value to avoid redundancy
I also tested spring and it worked
@DavidGrath Thanks for mentioning the PR again. I had overlooked it before. And thanks for creating a fix!
@martin-mfg thank you, and you're welcome!
Hello,
@martin-mfg, disallowAdditionalPropertiesIfNotPresent=false was indeed being used.
@DavidGrath, I have tested your branch and I think it is working properly now.
Here is a recap of the current behaviour using your branch :
| generatorName | additionalProperties value | additionalProperties field/methods present | Result |
|---|---|---|---|
| spring | Not specified | Yes, Map<String, Object> |
:heavy_check_mark: (OK thanks to your fix David) |
| spring | additionalProperties: false | No | :heavy_check_mark: |
| spring | additionalProperties: true | Yes, Map<String, Object> |
:heavy_check_mark: |
| spring | additionalProperties: type: string | Yes, Map<String, String> |
:heavy_check_mark: |
| java | Not specified | Yes, Map<String, Object> |
:heavy_check_mark: |
| java | additionalProperties: false | No | :heavy_check_mark: |
| java | additionalProperties: true | Yes, Map<String, Object> additionalProperties |
:heavy_check_mark: |
| java | additionalProperties: type: string | Yes, Map<String, Object> additionalProperties |
:x: Should be Map<String, String> additionalProperties, as with the spring generator |
There is still a difference between spring/java and the additionalProperties: type: string, but this one is less important and is not really related to this ticket.
Thank you for your help and fixes !!