Skip to content

Bugfix/3248 #3857

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.

Sign up for GitHub

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

Closed
peyerroger wants to merge 2 commits into from
Closed

Bugfix/3248 #3857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91651db
Fixes RequestParam regressions in Spring generator #3248
peyerroger Sep 7, 2019
4aa83f6
Merge branch 'master' of https://github.com/peyerroger/openapi-generator
peyerroger Sep 7, 2019
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
9 changes: 0 additions & 9 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.openapitools.codegen;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.google.common.base.CaseFormat;
import com.google.common.collect.ImmutableMap;
import com.samskivert.mustache.Mustache;
Expand Down Expand Up @@ -2303,7 +2302,6 @@ public CodegenProperty fromProperty(String name, Schema p) {
// property.baseType = getSimpleRef(p.get$ref());
//}
// --END of revision
property.isModel = ModelUtils.isModel(p);
Copy link
Member

@jimschubert jimschubert Sep 7, 2019

Choose a reason for hiding this comment

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

If you're changing the template to not use isModel, we should keep this as the change affects all generators. See my resolution in #3855, which sets isModel as expected (after the naive attempt done in setNonArrayMapProperty.

setNonArrayMapProperty(property, type);
}

Expand Down Expand Up @@ -3129,9 +3127,6 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports)
} else {
codegenParameter.dataType = codegenProperty.dataType;
}
if (ModelUtils.isObjectSchema(parameterSchema)) {
codegenProperty.complexType = codegenParameter.dataType;
}
codegenParameter.dataFormat = codegenProperty.dataFormat;
codegenParameter.required = codegenProperty.required;

Expand Down Expand Up @@ -3318,10 +3313,6 @@ public CodegenParameter fromParameter(Parameter parameter, Set<String> imports)
* @return data type
*/
protected String getParameterDataType(Parameter parameter, Schema schema) {
if (parameter.get$ref() != null) {
String refName = ModelUtils.getSimpleRef(parameter.get$ref());
return toModelName(refName);
}
return null;
}

Expand Down
25 changes: 0 additions & 25 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,31 +525,6 @@ public static boolean isEmailSchema(Schema schema) {
return false;
}

/**
* Check to see if the schema is a model with at least one properties
*
* @param schema potentially containing a '$ref'
* @return true if it's a model with at least one properties
*/
public static boolean isModel(Schema schema) {
Copy link
Member

@jimschubert jimschubert Sep 7, 2019

Choose a reason for hiding this comment

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

Removing a public helper is a breaking change (these are used by programmatic consumers, and this would therefore need to be deprecated before removal).

if (schema == null) {
LOGGER.error("Schema cannot be null in isModel check");
return false;
}

// has at least one property
if (schema.getProperties() != null && !schema.getProperties().isEmpty()) {
return true;
}

// composed schema is a model
if (schema instanceof ComposedSchema) {
return true;
}

return false;
}

/**
* Check to see if the schema is a free form object
*
Expand Down
2 changes: 1 addition & 1 deletion modules/openapi-generator/src/main/resources/JavaSpring/queryParams.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{#isQueryParam}}{{#useBeanValidation}}{{>beanValidationQueryParams}}{{/useBeanValidation}}@ApiParam(value = "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}}{{^isContainer}}{{#defaultValue}}, defaultValue = {{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{/defaultValue}}{{/isContainer}}) {{#useBeanValidation}}@Valid{{/useBeanValidation}}{{^isModel}} @RequestParam(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}{{^isContainer}}{{#defaultValue}}, defaultValue={{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{/defaultValue}}{{/isContainer}}){{/isModel}} {{>optionalDataType}} {{paramName}}{{/isQueryParam}}
{{#isQueryParam}}{{#useBeanValidation}}{{>beanValidationQueryParams}}{{/useBeanValidation}}@ApiParam(value = "{{{description}}}"{{#required}}, required = true{{/required}}{{#allowableValues}}, allowableValues = "{{#values}}{{{.}}}{{^-last}}, {{/-last}}{{#-last}}{{/-last}}{{/values}}"{{/allowableValues}}{{^isContainer}}{{#defaultValue}}, defaultValue = {{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{/defaultValue}}{{/isContainer}}) {{#useBeanValidation}}@Valid{{/useBeanValidation}}{{#isPrimitiveType}} @RequestParam(value = "{{baseName}}"{{#required}}, required = true{{/required}}{{^required}}, required = false{{/required}}{{^isContainer}}{{#defaultValue}}, defaultValue={{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{{defaultValue}}}{{^isString}}"{{/isString}}{{#isString}}{{#isEnum}}"{{/isEnum}}{{/isString}}{{/defaultValue}}{{/isContainer}}){{/isPrimitiveType}} {{>optionalDataType}} {{paramName}}{{/isQueryParam}}
15 changes: 0 additions & 15 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,21 +836,6 @@ private Map<String, Object> codegenModelWithXEnumVarName() {
return objs;
}

@Test
public void objectQueryParamIdentifyAsObject() {
Copy link
Member

@jimschubert jimschubert Sep 7, 2019

Choose a reason for hiding this comment

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

NOTE: Removing this test/functionality would be considered a breaking change and would need to be documented as a regression.

final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/objectQueryParam.yaml");
new InlineModelResolver().flatten(openAPI);
final DefaultCodegen codegen = new DefaultCodegen();
codegen.setOpenAPI(openAPI);

Set<String> imports = new HashSet<>();
CodegenParameter parameter = codegen.fromParameter(openAPI.getPaths().get("/pony").getGet().getParameters().get(0), imports);

Assert.assertEquals(parameter.dataType, "PageQuery");
Assert.assertEquals(imports.size(), 1);
Assert.assertEquals(imports.iterator().next(), "PageQuery");
}

@Test
public void mapParamImportInnerObject() {
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/2_0/mapArgs.yaml");
Expand Down
44 changes: 41 additions & 3 deletions ...enapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ public void doNotGenerateRequestParamForObjectQueryParam() throws IOException {
String outputPath = output.getAbsolutePath().replace('\\', '/');

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/3_0/objectQueryParam.yaml", null, new ParseOptions()).getOpenAPI();
.readLocation("src/test/resources/3_0/objectQueryParam.yaml", null, new ParseOptions()).getOpenAPI();

SpringCodegen codegen = new SpringCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put(CXFServerFeatures.LOAD_TEST_DATA_FROM_FILE, "true");

ClientOptInput input = new ClientOptInput();
input.setOpenAPI(openAPI);
input.setConfig(codegen);
input.openAPI(openAPI);
input.config(codegen);

MockDefaultGenerator generator = new MockDefaultGenerator();
generator.opts(input).generate();
Expand All @@ -174,6 +174,44 @@ private void checkFileNotContains(MockDefaultGenerator generator, String path, S
assertFalse(file.contains(line));
}

@Test
public void doGenerateRequestParamForSimpleParam() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();
String outputPath = output.getAbsolutePath().replace('\\', '/');

OpenAPI openAPI = new OpenAPIParser()
.readLocation("src/test/resources/3_0/issue_3248.yaml", null, new ParseOptions()).getOpenAPI();

SpringCodegen codegen = new SpringCodegen();
codegen.setOutputDir(output.getAbsolutePath());
codegen.additionalProperties().put(CXFServerFeatures.LOAD_TEST_DATA_FROM_FILE, "true");

ClientOptInput input = new ClientOptInput();
input.openAPI(openAPI);
input.config(codegen);

MockDefaultGenerator generator = new MockDefaultGenerator();
generator.opts(input).generate();

checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/MonkeysApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/ElephantsApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/ZebrasApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/BearsApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/CamelsApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/PandasApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/CrocodilesApi.java", "@RequestParam");
checkFileContains(generator, outputPath + "/src/main/java/org/openapitools/api/PolarBearsApi.java", "@RequestParam");

}

private void checkFileContains(MockDefaultGenerator generator, String path, String... lines) {
String file = generator.getFiles().get(path);
assertNotNull(file);
for (String line : lines)
assertTrue(file.contains(line));
}

@Test
public void clientOptsUnicity() {
SpringCodegen codegen = new SpringCodegen();
Expand Down
121 changes: 121 additions & 0 deletions modules/openapi-generator/src/test/resources/3_0/issue_3248.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
openapi: 3.0.0
servers:
- url: 'localhost:8080'
info:
version: 1.0.0
title: OpenAPI Petstore
license:
name: Apache-2.0
url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
paths:
/monkeys:
get:
operationId: getMonkeys
parameters:
- $ref: '#/components/parameters/refDate'
/elephants:
get:
operationId: getElephants
parameters:
- in: query
name: someDate
required: true
schema:
type: string
format: date
/girafes:
get:
operationId: getGirafes
parameters:
- $ref: '#/components/parameters/refStatus'
/zebras:
get:
operationId: getZebras
parameters:
- in: query
name: status
required: true
schema:
type: integer
enum: [0,1]
default: 0
/bears:
get:
operationId: getBears
parameters:
- $ref: '#/components/parameters/refCondition'
/camels:
get:
operationId: getCamels
parameters:
- in: query
name: condition
required: true
schema:
type: string
enum:
- sleeping
- awake
/pandas:
get:
operationId: getPandas
parameters:
- $ref: '#/components/parameters/refName'
/crocodiles:
get:
operationId: getCrocodiles
parameters:
- in: query
name: name
required: true
schema:
type: string
/polarBears:
get:
operationId: getPolarBears
parameters:
- $ref: '#/components/parameters/refAge'
/birds:
get:
operationId: getBirds
parameters:
- in: query
name: age
required: true
schema:
type: integer
components:
parameters:
refDate:
in: query
name: refDate
required: true
schema:
type: string
format: date
refStatus:
in: query
name: refStatus
required: true
schema:
type: integer
enum: [0,1]
default: 0
refCondition:
in: query
name: refCondition
schema:
type: string
enum:
- sleeping
- awake
refName:
in: query
name: refName
schema:
type: string
refAge:
in: query
name: refAge
schema:
type: integer