Skip to content

Correct generation of additionalProperties Map for numerous Java libraries (issue #20139) #21229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2880,6 +2880,8 @@ protected void updateModelForObject(CodegenModel m, Schema schema) {
if (ModelUtils.isMapSchema(schema)) {
// an object or anyType composed schema that has additionalProperties set
addAdditionPropertiesToCodeGenModel(m, schema);
} else if(!ModelUtils.isDisallowAdditionalPropertiesIfNotPresent()) {
addAdditionPropertiesToCodeGenModel(m, schema);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @DavidGrath thanks again for the PR.

My first question is it looks to me the if block is checking the schema type but your additional is not checking scheme type but rather an option (disallowAdditionalPropertiesIfNotPresent) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wing328 , yes, that is correct, my reasoning being that the property wasn't already considered in the if block already, so addAdditionPropertiesToCodeGenModel wouldn't have been called at all if it was false

} else if (ModelUtils.isFreeFormObject(schema, openAPI)) {
// non-composed object type with no properties + additionalProperties
// additionalProperties must be null, ObjectSchema, or empty Schema
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.openapitools.codegen.java.assertions.JavaFileAssert;
import org.openapitools.codegen.languages.AbstractJavaCodegen;
import org.openapitools.codegen.languages.JavaClientCodegen;
import org.openapitools.codegen.languages.KotlinServerCodegen;
import org.openapitools.codegen.languages.features.BeanValidationFeatures;
import org.openapitools.codegen.languages.features.CXFServerFeatures;
import org.openapitools.codegen.meta.features.SecurityFeature;
Expand All @@ -62,14 +63,17 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.entry;
import static org.assertj.core.api.InstanceOfAssertFactories.FILE;
import static org.openapitools.codegen.CodegenConstants.*;
import static org.openapitools.codegen.TestUtils.newTempFolder;
import static org.openapitools.codegen.TestUtils.validateJavaSourceFiles;
import static org.openapitools.codegen.TestUtils.*;
import static org.openapitools.codegen.languages.AbstractKotlinCodegen.USE_JAKARTA_EE;
import static org.openapitools.codegen.languages.JavaClientCodegen.*;
import static org.openapitools.codegen.languages.KotlinServerCodegen.Constants.INTERFACE_ONLY;
import static org.openapitools.codegen.languages.KotlinServerCodegen.Constants.JAXRS_SPEC;
import static org.testng.Assert.*;

public class JavaClientCodegenTest {
Expand Down Expand Up @@ -3600,6 +3604,31 @@ public void testClassesAreValidJavaOkHttpGson() {
);
}

@Test(description = "Issue #20139")
public void givenAdditionalPropertiesNotDefinedAndIsDisallowAdditionalPropertiesIfNotPresentIsFalseWhenGenerateModelThenAdditionalPropertiesTypeIsObject() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

JavaClientCodegen codegen = new JavaClientCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.setUseBeanValidation(true);
codegen.additionalProperties().put(DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, false);

// Only works for these libraries: jersey3, jersey2, feign, feign-hc5, resttemplate, webclient, restclient native
// The default, okhttp-gson, relies on isAdditionalPropertiesTrue and doesn't use additionalPropertiesType
codegen.additionalProperties().put(LIBRARY, JERSEY2);
new DefaultGenerator().opts(new ClientOptInput()
.openAPI(TestUtils.parseSpec("src/test/resources/2_0/issue_20139.yaml"))
.config(codegen))
.generate();

String outputPath = output.getAbsolutePath() + "/src/main/java/org/openapitools/client";
Path jsonWebKey = Paths.get(outputPath + "/model/JsonWebKey.java");
JavaFileAssert.assertThat(jsonWebKey)
.assertProperty("additionalProperties")
.withType("Map<String, Object>");
}

@Test(description = "Issue #21051")
public void givenComplexObjectHasDefaultValueWhenGenerateThenDefaultAssignmentsAreValid() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@

import static java.util.stream.Collectors.groupingBy;
import static org.assertj.core.api.Assertions.assertThat;
import static org.openapitools.codegen.CodegenConstants.DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT;
import static org.openapitools.codegen.CodegenConstants.LIBRARY;
import static org.openapitools.codegen.TestUtils.*;
import static org.openapitools.codegen.languages.AbstractJavaCodegen.GENERATE_BUILDERS;
import static org.openapitools.codegen.languages.AbstractJavaCodegen.GENERATE_CONSTRUCTOR_WITH_ALL_ARGS;
import static org.openapitools.codegen.languages.JavaClientCodegen.JERSEY2;
import static org.openapitools.codegen.languages.SpringCodegen.*;
import static org.openapitools.codegen.languages.features.DocumentationProviderFeatures.ANNOTATION_LIBRARY;
import static org.openapitools.codegen.languages.features.DocumentationProviderFeatures.DOCUMENTATION_PROVIDER;
Expand Down Expand Up @@ -5502,6 +5505,27 @@ public void testEnumFieldShouldBeFinal_issue21018() throws IOException {
.fileContains("private final String value");
}

@Test(description = "Issue #20139")
public void givenAdditionalPropertiesNotDefinedAndIsDisallowAdditionalPropertiesIfNotPresentIsFalseWhenGenerateModelThenAdditionalPropertiesTypeIsObject() throws Exception {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

SpringCodegen codegen = new SpringCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.setUseBeanValidation(true);
codegen.additionalProperties().put(DISALLOW_ADDITIONAL_PROPERTIES_IF_NOT_PRESENT, false);
new DefaultGenerator().opts(new ClientOptInput()
.openAPI(TestUtils.parseSpec("src/test/resources/2_0/issue_20139.yaml"))
.config(codegen))
.generate();

String outputPath = output.getAbsolutePath() + "/src/main/java/org/openapitools";
Path jsonWebKey = Paths.get(outputPath + "/model/JsonWebKey.java");
JavaFileAssert.assertThat(jsonWebKey)
.assertProperty("additionalProperties")
.withType("Map<String, Object>");
}

@Test
public void testCollectionTypesWithDefaults_issue_collection() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
Expand Down
23 changes: 23 additions & 0 deletions modules/openapi-generator/src/test/resources/2_0/issue_20139.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
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
Original file line number Diff line number Diff line change
Expand Up @@ -1969,11 +1969,13 @@ components:
type: string
pattern: /^[A-Z\s]*$/i
nullable: true
additionalProperties: false
banana:
type: object
properties:
lengthCm:
type: number
additionalProperties: false
mammal:
oneOf:
- $ref: '#/components/schemas/whale'
Expand Down
2 changes: 2 additions & 0 deletions samples/client/petstore/java/jersey3/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2038,6 +2038,7 @@ components:
color:
type: string
apple:
additionalProperties: false
nullable: true
properties:
cultivar:
Expand All @@ -2048,6 +2049,7 @@ components:
type: string
type: object
banana:
additionalProperties: false
properties:
lengthCm:
type: number
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import java.util.Map;
import java.util.HashMap;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import java.util.Objects;
import java.util.Map;
import java.util.HashMap;
Expand Down Expand Up @@ -333,6 +337,43 @@ public void setMapWithUndeclaredPropertiesString(@jakarta.annotation.Nullable Ma
this.mapWithUndeclaredPropertiesString = mapWithUndeclaredPropertiesString;
}

/**
* A container for additional, undeclared properties.
* This is a holder for any undeclared properties as specified with
* the 'additionalProperties' keyword in the OAS document.
*/
private Map<String, Object> additionalProperties;

/**
* Set the additional (undeclared) property with the specified name and value.
* If the property does not already exist, create it otherwise replace it.
*/
@JsonAnySetter
public AdditionalPropertiesClass putAdditionalProperty(String key, Object value) {
if (this.additionalProperties == null) {
this.additionalProperties = new HashMap<>();
}
this.additionalProperties.put(key, value);
return this;
}

/**
* Return the additional (undeclared) property.
*/
@JsonAnyGetter
public Map<String, Object> getAdditionalProperties() {
return additionalProperties;
}

/**
* Return the additional (undeclared) property with the specified name.
*/
public Object getAdditionalProperty(String key) {
if (this.additionalProperties == null) {
return null;
}
return this.additionalProperties.get(key);
}

/**
* Return true if this AdditionalPropertiesClass object is equal to o.
Expand Down Expand Up @@ -370,6 +411,7 @@ public String toString() {
sb.append(" mapWithUndeclaredPropertiesAnytype3: ").append(toIndentedString(mapWithUndeclaredPropertiesAnytype3)).append("\n");
sb.append(" emptyMap: ").append(toIndentedString(emptyMap)).append("\n");
sb.append(" mapWithUndeclaredPropertiesString: ").append(toIndentedString(mapWithUndeclaredPropertiesString)).append("\n");
sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
sb.append("}");
return sb.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import java.util.Map;
import java.util.HashMap;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import java.util.Objects;
import java.util.Map;
import java.util.HashMap;
Expand Down Expand Up @@ -115,6 +119,43 @@ public void setColor(@jakarta.annotation.Nullable String color) {
this.color = color;
}

/**
* A container for additional, undeclared properties.
* This is a holder for any undeclared properties as specified with
* the 'additionalProperties' keyword in the OAS document.
*/
private Map<String, Object> additionalProperties;

/**
* Set the additional (undeclared) property with the specified name and value.
* If the property does not already exist, create it otherwise replace it.
*/
@JsonAnySetter
public Animal putAdditionalProperty(String key, Object value) {
if (this.additionalProperties == null) {
this.additionalProperties = new HashMap<>();
}
this.additionalProperties.put(key, value);
return this;
}

/**
* Return the additional (undeclared) property.
*/
@JsonAnyGetter
public Map<String, Object> getAdditionalProperties() {
return additionalProperties;
}

/**
* Return the additional (undeclared) property with the specified name.
*/
public Object getAdditionalProperty(String key) {
if (this.additionalProperties == null) {
return null;
}
return this.additionalProperties.get(key);
}

/**
* Return true if this Animal object is equal to o.
Expand All @@ -135,6 +176,7 @@ public String toString() {
sb.append("class Animal {\n");
sb.append(" className: ").append(toIndentedString(className)).append("\n");
sb.append(" color: ").append(toIndentedString(color)).append("\n");
sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
sb.append("}");
return sb.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

import org.apache.commons.lang3.builder.EqualsBuilder;
import org.apache.commons.lang3.builder.HashCodeBuilder;
import java.util.Map;
import java.util.HashMap;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import java.util.Objects;
import java.util.Map;
import java.util.HashMap;
Expand Down Expand Up @@ -82,6 +86,43 @@ public void setArrayArrayNumber(@jakarta.annotation.Nullable List<List<BigDecima
this.arrayArrayNumber = arrayArrayNumber;
}

/**
* A container for additional, undeclared properties.
* This is a holder for any undeclared properties as specified with
* the 'additionalProperties' keyword in the OAS document.
*/
private Map<String, Object> additionalProperties;

/**
* Set the additional (undeclared) property with the specified name and value.
* If the property does not already exist, create it otherwise replace it.
*/
@JsonAnySetter
public ArrayOfArrayOfNumberOnly putAdditionalProperty(String key, Object value) {
if (this.additionalProperties == null) {
this.additionalProperties = new HashMap<>();
}
this.additionalProperties.put(key, value);
return this;
}

/**
* Return the additional (undeclared) property.
*/
@JsonAnyGetter
public Map<String, Object> getAdditionalProperties() {
return additionalProperties;
}

/**
* Return the additional (undeclared) property with the specified name.
*/
public Object getAdditionalProperty(String key) {
if (this.additionalProperties == null) {
return null;
}
return this.additionalProperties.get(key);
}

/**
* Return true if this ArrayOfArrayOfNumberOnly object is equal to o.
Expand All @@ -101,6 +142,7 @@ public String toString() {
StringBuilder sb = new StringBuilder();
sb.append("class ArrayOfArrayOfNumberOnly {\n");
sb.append(" arrayArrayNumber: ").append(toIndentedString(arrayArrayNumber)).append("\n");
sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
sb.append("}");
return sb.toString();
}
Expand Down
Loading
Loading