[java] Fix issue #17472 when using schemaMapping for models in collections, not compilable code with @Valid is generated (#19093)

* Fix issue 17472

* Avoid cast exception
This commit is contained in:
jpfinne 2024-09-02 11:03:08 +02:00 committed by GitHub
parent 1776c000ed
commit b228133a20
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 250 additions and 2 deletions

View File

@ -63,6 +63,7 @@ import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import static org.openapitools.codegen.utils.CamelizeOption.*;
import static org.openapitools.codegen.utils.ModelUtils.getSchemaItems;
import static org.openapitools.codegen.utils.OnceLogger.once;
import static org.openapitools.codegen.utils.StringUtils.*;
@ -1008,8 +1009,9 @@ public abstract class AbstractJavaCodegen extends DefaultCodegen implements Code
Schema<?> schema = unaliasSchema(p);
Schema<?> target = ModelUtils.isGenerateAliasAsModel() ? p : schema;
if (ModelUtils.isArraySchema(target)) {
Schema<?> items = ModelUtils.getSchemaItems(schema);
return getSchemaType(target) + "<" + getBeanValidation(items) + getTypeDeclaration(items) + ">";
Schema<?> items = getSchemaItems(schema);
String typeDeclaration = getTypeDeclarationForArray(items);
return getSchemaType(target) + "<" + typeDeclaration + ">";
} else if (ModelUtils.isMapSchema(target)) {
// Note: ModelUtils.isMapSchema(p) returns true when p is a composed schema that also defines
// additionalproperties: true
@ -1024,6 +1026,29 @@ public abstract class AbstractJavaCodegen extends DefaultCodegen implements Code
return super.getTypeDeclaration(target);
}
private String getTypeDeclarationForArray(Schema<?> items) {
String typeDeclaration = getTypeDeclaration(items);
String beanValidation = getBeanValidation(items);
if (StringUtils.isEmpty(beanValidation)) {
return typeDeclaration;
}
int idxLt = typeDeclaration.indexOf('<');
int idx = idxLt < 0 ?
typeDeclaration.lastIndexOf('.'):
// last dot before the generic like in List<com.mycompany.Container<java.lang.Object>
typeDeclaration.substring(0, idxLt).lastIndexOf('.');
if (idx > 0) {
// fix full qualified name, we need List<java.lang.@Valid String>
// or List<com.mycompany.@Valid Container<java.lang.Object>
return typeDeclaration.substring(0, idx + 1) + beanValidation
+ typeDeclaration.substring(idx + 1);
} else {
return beanValidation + typeDeclaration;
}
}
/**
* This method stand for resolve bean validation for container(array, set).
* Return empty if there's no bean validation for requested type or prop useBeanValidation false or missed.

View File

@ -22,6 +22,7 @@ import io.swagger.v3.parser.core.models.ParseOptions;
import org.openapitools.codegen.ClientOptInput;
import org.openapitools.codegen.CodegenConstants;
import org.openapitools.codegen.DefaultGenerator;
import org.openapitools.codegen.config.GeneratorSettings;
import org.openapitools.codegen.java.assertions.JavaFileAssert;
import org.openapitools.codegen.languages.*;
import org.testng.annotations.DataProvider;
@ -30,6 +31,7 @@ import org.testng.annotations.Test;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.Collections;
import java.util.Map;
import java.util.function.Consumer;
import java.util.function.Function;
@ -410,4 +412,73 @@ public class JavaValidationArrayPrimitivesTest {
asserts.accept(files);
}
@DataProvider(name = "typeMappings")
public Object[] typeMappings(){
return new Object[][]{
{Collections.emptyMap(), "@Valid MyItem" },
{ Map.of("array", "List"), "@Valid MyItem" },
{ Map.of("array", "Set"), "@Valid MyItem" },
{ Collections.emptyMap(), "@Valid MyItem" },
{ Map.of( "MyItem", "com.mycompany.MyItem"), "com.mycompany.@Valid MyItem"},
{ Map.of( "MyItem", "com.mycompany.MyContainer<java.lang.String>"), "com.mycompany.@Valid MyContainer<java.lang.String>"}
};
}
@Test(dataProvider = "typeMappings")
public void typeMappingsForCollections(Map<String,String> typeMappings, String expectedMyItemArgument) throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
final OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/bugs/issue_17472.yaml", null, new ParseOptions()).getOpenAPI();
final SpringCodegen codegen = new SpringCodegen();
codegen.setUseTags(true);
codegen.setOutputDir(output.getAbsolutePath());
codegen.setOpenAPI(openAPI);
final GeneratorSettings generatorSettings = GeneratorSettings.newBuilder()
.withTypeMappings(typeMappings)
.build();
ClientOptInput input = new ClientOptInput();
input.generatorSettings(generatorSettings);
input.openAPI(openAPI);
input.config(codegen);
final DefaultGenerator generator = new DefaultGenerator();
Map<String, File> files = generator.opts(input).generate().stream()
.collect(Collectors.toMap(File::getName, Function.identity()));
String arrayMapping= typeMappings.getOrDefault("array", "List");
// @Valid@Size(min = 5) is not nice, but not related to this fix
// adding a space would probably break many other tests
JavaFileAssert.assertThat(files.get("ListOfPatternsApi.java"))
.fileContains("ResponseEntity<" + arrayMapping + "<String>>",
arrayMapping + "<@Pattern(regexp = \"([a-z]+)\")String> requestBody")
.fileContainsPattern("@Valid\\s*@Size\\(min = 5\\)\\s*@RequestBody");
JavaFileAssert.assertThat(files.get("ListOfStringsApi.java"))
.fileContains(
"ResponseEntity<" + arrayMapping + "<String>>",
arrayMapping + "<@Size(min = 2, max = 2)String> requestBody")
.fileContainsPattern("@Valid\\s*@Size\\(min = 5\\)\\s*@RequestBody");
JavaFileAssert.assertThat(files.get("ListOfObjectsApi.java"))
.fileContains(
"ResponseEntity<" + arrayMapping + "<ListOfObjectsInner>>",
arrayMapping + "<@Valid ListOfObjectsInner> listOfObjectsInner")
.fileContainsPattern("@Valid\\s*@Size\\(min = 5\\)\\s*@RequestBody");
String myItem = typeMappings.getOrDefault("MyItem", "MyItem");
JavaFileAssert.assertThat(files.get("ListOfQualifiedItemApi.java"))
.fileContains(
"ResponseEntity<" + arrayMapping + "<" + myItem + ">>",
arrayMapping + "<"+ expectedMyItemArgument + ">");
if (!typeMappings.containsKey("array")) {
// the mapping to Set is done automatically with uniqueItems: true
JavaFileAssert.assertThat(files.get("ListOfUniqueItemApi.java"))
.fileContains(
"ResponseEntity<Set<" + myItem + ">>",
"Set<" + expectedMyItemArgument + "> ");
}
}
}

View File

@ -187,4 +187,18 @@ public class JavaFileAssert extends AbstractAssert<JavaFileAssert, CompilationUn
public TypeAnnotationsAssert assertTypeAnnotations() {
return new TypeAnnotationsAssert(this, actual.getType(0).getAnnotations());
}
public JavaFileAssert fileContainsPattern(final String pattern) {
final String actualBody = actual.getTokenRange()
.orElseThrow(() -> new IllegalStateException("Empty file"))
.toString();
Assertions.assertThat(actualBody)
.withFailMessage(
"File should contains pattern\n====\n%s\n====\nbut actually was\n====\n%s\n====",
pattern, actualBody
)
.containsPattern(pattern);
return this;
}
}

View File

@ -0,0 +1,138 @@
openapi: 3.0.1
info:
title: TEST 17472
description: TEST 17472
version: 1.0.0
tags:
- name: listOfStrings
- name: listOfObjects
- name: listOfPatterns
- name: listOfQualifiedItem
- name: listOfUniqueItem
paths:
/testListOfStrings:
post:
description: test list of string in argument and response
tags:
- listOfStrings
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfStrings'
responses:
200:
description: 'response'
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfStrings'
/testListOfObjects:
post:
description: test list of string in argument and response
tags:
- listOfObjects
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfObjects'
responses:
200:
description: 'response'
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfObjects'
/testListOfPatterns:
post:
description: test list of string in argument and response
tags:
- listOfPatterns
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfPatterns'
responses:
200:
description: 'response'
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfPatterns'
/testListOfUniqueItems:
post:
description: test list of string in argument and response
tags:
- listOfUniqueItem
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfUniqueItem'
responses:
200:
description: 'response'
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfUniqueItem'
/testListQualifiedItem:
post:
description: test list of string in argument and response
tags:
- listOfQualifiedItem
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfMyItem'
responses:
200:
description: 'response'
content:
application/json:
schema:
$ref: '#/components/schemas/ListOfMyItem'
components:
schemas:
ListOfStrings:
type: array
minItems: 5
items:
maxLength: 2
minLength: 2
type: string
ListOfObjects:
type: array
minItems: 5
items:
type: object
properties:
text:
maxLength: 2
minLength: 2
type: string
ListOfPatterns:
type: array
minItems: 5
items:
pattern: '([a-z]+)'
type: string
ListOfMyItem:
type: array
items:
$ref: '#/components/schemas/MyItem'
MyItem:
type: object
properties:
data:
type: string
ListOfUniqueItem:
uniqueItems: true
type: array
default: []
items:
$ref: '#/components/schemas/MyItem'