Skip to content

[ALL][CLIENT] Fix serialization for query params specified by ref #11864

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
sorin-florea wants to merge 5 commits into from
Closed

[ALL][CLIENT] Fix serialization for query params specified by ref #11864

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ef29bd6
Fix ref query param serialization
sorin-florea Mar 14, 2022
04aeef7
Add comments
sorin-florea May 10, 2022
9b3be0e
Fix test
sorin-florea May 10, 2022
e4cd1a5
Merge remote-tracking branch 'origin/master' into sorin/fix-ref-query…
sorin-florea May 10, 2022
f7d3c0e
Fix test
sorin-florea May 10, 2022
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
Next Next commit
Fix ref query param serialization
  • Loading branch information
@sorin-florea
sorin-florea committed May 10, 2022
commit ef29bd6338dbe53e76987f23f36f59454d6183fd
32 changes: 15 additions & 17 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 @@ -6876,23 +6876,21 @@ protected void addVarsRequiredVarsAdditionalProps(Schema schema, IJsonSchemaVali
if (!"object".equals(schema.getType())) {
return;
}
if (schema instanceof ObjectSchema) {
ObjectSchema objSchema = (ObjectSchema) schema;
HashSet<String> requiredVars = new HashSet<>();
if (objSchema.getRequired() != null) {
requiredVars.addAll(objSchema.getRequired());
}
if (objSchema.getProperties() != null && objSchema.getProperties().size() > 0) {
property.setHasVars(true);
}
addVars(property, property.getVars(), objSchema.getProperties(), requiredVars);
List<CodegenProperty> requireCpVars = property.getVars()
.stream()
.filter(p -> Boolean.TRUE.equals(p.required)).collect(Collectors.toList());
property.setRequiredVars(requireCpVars);
if (property.getRequiredVars() != null && property.getRequiredVars().size() > 0) {
property.setHasRequired(true);
}

HashSet<String> requiredVars = new HashSet<>();
if (schema.getRequired() != null) {
requiredVars.addAll(schema.getRequired());
}
if (schema.getProperties() != null && schema.getProperties().size() > 0) {
property.setHasVars(true);
}
addVars(property, property.getVars(), schema.getProperties(), requiredVars);
List<CodegenProperty> requireCpVars = property.getVars()
.stream()
.filter(p -> Boolean.TRUE.equals(p.required)).collect(Collectors.toList());
property.setRequiredVars(requireCpVars);
if (property.getRequiredVars() != null && property.getRequiredVars().size() > 0) {
property.setHasRequired(true);
}
}

Expand Down
147 changes: 129 additions & 18 deletions modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,33 @@
import io.swagger.v3.oas.models.info.Contact;
import io.swagger.v3.oas.models.info.Info;
import io.swagger.v3.oas.models.info.License;
import io.swagger.v3.oas.models.media.ArraySchema;
import io.swagger.v3.oas.models.media.BooleanSchema;
import io.swagger.v3.oas.models.media.DateSchema;
import io.swagger.v3.oas.models.media.DateTimeSchema;
import io.swagger.v3.oas.models.media.IntegerSchema;
import io.swagger.v3.oas.models.media.NumberSchema;
import io.swagger.v3.oas.models.media.ObjectSchema;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.media.StringSchema;
import io.swagger.v3.oas.models.parameters.Parameter;
import io.swagger.v3.oas.models.security.*;
import io.swagger.v3.oas.models.parameters.QueryParameter;
import io.swagger.v3.oas.models.security.OAuthFlow;
import io.swagger.v3.oas.models.security.OAuthFlows;
import io.swagger.v3.oas.models.security.Scopes;
import io.swagger.v3.oas.models.security.SecurityRequirement;
import io.swagger.v3.oas.models.security.SecurityScheme;
import io.swagger.v3.oas.models.tags.Tag;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.comparator.PathFileComparator;
import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.openapitools.codegen.api.TemplateDefinition;
import org.openapitools.codegen.api.TemplateFileType;
import org.openapitools.codegen.api.TemplatePathLocator;
import org.openapitools.codegen.api.TemplateProcessor;
import org.openapitools.codegen.config.GlobalSettings;
import org.openapitools.codegen.api.TemplatingEngineAdapter;
import org.openapitools.codegen.api.TemplateFileType;
import org.openapitools.codegen.config.GlobalSettings;
import org.openapitools.codegen.ignore.CodegenIgnoreProcessor;
import org.openapitools.codegen.languages.PythonClientCodegen;
import org.openapitools.codegen.languages.PythonExperimentalClientCodegen;
Expand All @@ -61,12 +74,28 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.*;
import java.io.File;
import java.io.IOException;
import java.math.BigDecimal;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.time.ZonedDateTime;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentSkipListSet;
import java.util.function.Function;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -550,7 +579,7 @@ void generateApis(List<File> files, List<OperationsMap> allOperations, List<Mode
LOGGER.info("Skipping generation of APIs.");
return;
}
Map<String, List<CodegenOperation>> paths = processPaths(this.openAPI.getPaths());
Map<String, List<CodegenOperation>> paths = processPaths(this.openAPI.getPaths(), allModels);
Set<String> apisToGenerate = null;
String apiNames = GlobalSettings.getProperty("apis");
if (apiNames != null && !apiNames.isEmpty()) {
Expand Down Expand Up @@ -808,9 +837,9 @@ Map<String, Object> buildSupportFileBundle(List<OperationsMap> allOperations, Li
* <p>
* Examples:
* <p>
* boolean hasOAuthMethods
* boolean hasOAuthMethods
* <p>
* List&lt;CodegenSecurity&gt; oauthMethods
* List&lt;CodegenSecurity&gt; oauthMethods
*
* @param bundle the map which the booleans and collections will be added
*/
Expand Down Expand Up @@ -1046,7 +1075,7 @@ private File processTemplateToFile(Map<String, Object> templateData, String temp
}
}

public Map<String, List<CodegenOperation>> processPaths(Paths paths) {
public Map<String, List<CodegenOperation>> processPaths(Paths paths, List<ModelMap> allModels) {
Map<String, List<CodegenOperation>> ops = new TreeMap<>();
// when input file is not valid and doesn't contain any paths
if (paths == null) {
Expand All @@ -1055,19 +1084,19 @@ public Map<String, List<CodegenOperation>> processPaths(Paths paths) {
for (Map.Entry<String, PathItem> pathsEntry : paths.entrySet()) {
String resourcePath = pathsEntry.getKey();
PathItem path = pathsEntry.getValue();
processOperation(resourcePath, "get", path.getGet(), ops, path);
processOperation(resourcePath, "head", path.getHead(), ops, path);
processOperation(resourcePath, "put", path.getPut(), ops, path);
processOperation(resourcePath, "post", path.getPost(), ops, path);
processOperation(resourcePath, "delete", path.getDelete(), ops, path);
processOperation(resourcePath, "patch", path.getPatch(), ops, path);
processOperation(resourcePath, "options", path.getOptions(), ops, path);
processOperation(resourcePath, "trace", path.getTrace(), ops, path);
processOperation(resourcePath, "get", path.getGet(), ops, path, allModels);
processOperation(resourcePath, "head", path.getHead(), ops, path, allModels);
processOperation(resourcePath, "put", path.getPut(), ops, path, allModels);
processOperation(resourcePath, "post", path.getPost(), ops, path, allModels);
processOperation(resourcePath, "delete", path.getDelete(), ops, path, allModels);
processOperation(resourcePath, "patch", path.getPatch(), ops, path, allModels);
processOperation(resourcePath, "options", path.getOptions(), ops, path, allModels);
processOperation(resourcePath, "trace", path.getTrace(), ops, path, allModels);
}
return ops;
}

private void processOperation(String resourcePath, String httpMethod, Operation operation, Map<String, List<CodegenOperation>> operations, PathItem path) {
private void processOperation(String resourcePath, String httpMethod, Operation operation, Map<String, List<CodegenOperation>> operations, PathItem path, List<ModelMap> allModels) {
if (operation == null) {
return;
}
Expand Down Expand Up @@ -1115,6 +1144,31 @@ private void processOperation(String resourcePath, String httpMethod, Operation
if (operation.getParameters() != null) {
for (Parameter parameter : operation.getParameters()) {
operationParameters.add(generateParameterId(parameter));

if ((parameter instanceof QueryParameter || "query".equalsIgnoreCase(parameter.getIn()))
&& parameter.getSchema() != null && parameter.getSchema().get$ref() != null
&& Parameter.StyleEnum.DEEPOBJECT != parameter.getStyle()) {

Optional<CodegenModel> matchedModel = allModels.stream()
.map(ModelMap::getModel)
.filter(codegenModel -> codegenModel != null
&& codegenModel.getName() != null
&& codegenModel.getName().equalsIgnoreCase(parameter.getName())).findFirst();

if (matchedModel.isPresent()) {
CodegenModel model = matchedModel.get();

Schema schema = parameter.getSchema();
parameter.set$ref(schema.get$ref());
schema.set$ref(null);
schema.setType(model.getDataType().toLowerCase(Locale.ROOT));

Map<String, Schema> properties = mapProperties(model);
if(!properties.isEmpty()) {
schema.setProperties(properties);
}
}
}
}
}

Expand Down Expand Up @@ -1170,6 +1224,63 @@ private void processOperation(String resourcePath, String httpMethod, Operation

}

private Map<String, Schema> mapProperties(CodegenModel model) {
Map<String, Schema> properties = new LinkedHashMap<>();

for (CodegenProperty property : model.getVars()) {
Schema mappedSchema = mapToSchema(property);

if (mappedSchema != null) {
mappedSchema.setDescription(model.getDescription());
properties.put(property.name, mappedSchema);
}
}
return properties;
}

private Schema mapToSchema(CodegenProperty property) {
switch (property.openApiType) {
case "string":
StringSchema stringSchema = new StringSchema();
stringSchema.setMinLength(property.getMinLength());
stringSchema.setMaxLength(property.getMaxLength());
stringSchema.setPattern(property.getPattern());

return stringSchema;
case "integer":
IntegerSchema integerSchema = new IntegerSchema();
integerSchema.setMinimum(toBigDecimal(property.getMinimum()));
integerSchema.setMaximum(toBigDecimal(property.getMaximum()));

return integerSchema;
case "number":
NumberSchema floatSchema = new NumberSchema();
floatSchema.setMinimum(toBigDecimal(property.getMinimum()));
floatSchema.setMaximum(toBigDecimal(property.getMaximum()));
return floatSchema;
case "boolean":
return new BooleanSchema();
case "object":
return new ObjectSchema();
case "array":
ArraySchema arraySchema = new ArraySchema();
arraySchema.setMinItems(property.getMinItems());
arraySchema.setMaxItems(property.getMaxItems());

if (property.getItems() != null) {
arraySchema.setItems(mapToSchema(property.getItems()));
}

return arraySchema;
}

return null;
}

private BigDecimal toBigDecimal(String value) {
return value == null ? null : new BigDecimal(value);
}

private static String generateParameterId(Parameter parameter) {
return parameter.getName() + ":" + parameter.getIn();
}
Expand Down
10 changes: 5 additions & 5 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 @@ -3380,14 +3380,14 @@ public void testHasRequiredInProperties() {
"ComposedHasAllofOptPropHasPropertiesNoRequired",
"ComposedHasAllofOptPropNoPropertiesHasRequired", // TODO: hasRequired should be true, fix this
"ObjectHasPropertiesHasRequired", // False because this is extracted into another component and is a ref
"ComposedNoAllofPropsHasPropertiesHasRequired", // False because this is extracted into another component and is a ref
"ComposedHasAllofOptPropHasPropertiesHasRequired", // TODO: hasRequired should be true, fix this
"ComposedHasAllofReqPropNoPropertiesNoRequired",
"ComposedHasAllofReqPropHasPropertiesNoRequired",
"ComposedHasAllofReqPropNoPropertiesHasRequired", // TODO: hasRequired should be true, fix this
"ComposedHasAllofReqPropHasPropertiesHasRequired" // TODO: hasRequired should be true, fix this
"ComposedHasAllofReqPropNoPropertiesHasRequired" // TODO: hasRequired should be true, fix this
));
HashSet<String> modelNamesWithRequired = new HashSet(Arrays.asList(
"ComposedNoAllofPropsHasPropertiesHasRequired",
"ComposedHasAllofOptPropHasPropertiesHasRequired",
"ComposedHasAllofReqPropHasPropertiesHasRequired"
));
for (CodegenProperty var : cm.getVars()) {
boolean hasRequired = var.getHasRequired();
Expand Down Expand Up @@ -3702,7 +3702,7 @@ public void testComposedPropertyTypes() {
modelName = "ObjectWithComposedProperties";
CodegenModel m = codegen.fromModel(modelName, openAPI.getComponents().getSchemas().get(modelName));
/* TODO inline allOf schema are created as separate models and the following assumptions that
the properties are non-model are no longer valid and need to be revised
the properties are non-model are no longer valid and need to be revised
assertTrue(m.vars.get(0).getIsMap());
assertTrue(m.vars.get(1).getIsNumber());
assertTrue(m.vars.get(2).getIsUnboundedInteger());
Expand Down
5 changes: 3 additions & 2 deletions modules/openapi-generator/src/test/java/org/openapitools/codegen/DefaultGeneratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.openapitools.codegen;

import com.google.common.collect.Lists;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.Operation;
import io.swagger.v3.oas.models.PathItem;
Expand Down Expand Up @@ -354,7 +355,7 @@ public void testNonStrictProcessPaths() throws Exception {

DefaultGenerator generator = new DefaultGenerator();
generator.opts(opts);
Map<String, List<CodegenOperation>> result = generator.processPaths(openAPI.getPaths());
Map<String, List<CodegenOperation>> result = generator.processPaths(openAPI.getPaths(), Lists.newArrayList());
Assert.assertEquals(result.size(), 1);
List<CodegenOperation> defaultList = result.get("Default");
Assert.assertEquals(defaultList.size(), 2);
Expand All @@ -379,7 +380,7 @@ public void testProcessPaths() throws Exception {

DefaultGenerator generator = new DefaultGenerator();
generator.opts(opts);
Map<String, List<CodegenOperation>> result = generator.processPaths(openAPI.getPaths());
Map<String, List<CodegenOperation>> result = generator.processPaths(openAPI.getPaths(), Lists.newArrayList());
Assert.assertEquals(result.size(), 1);
List<CodegenOperation> defaultList = result.get("Default");
Assert.assertEquals(defaultList.size(), 4);
Expand Down
Loading