From 2d008be1813f278d0bbaa331dc6d105ebca6da47 Mon Sep 17 00:00:00 2001 From: harishchoragudi Date: Thu, 8 Jun 2017 12:41:20 -0500 Subject: [PATCH] Fix for issue#5788 -CPPRest generated models are not using inheritance as specified in contract (#5791) * fixed map to use value instead of mapentry while doing fromJson. * cpprest models now use inheritance properly instead of always extending from ModelBase * cpprest models now use inheritance properly instead of always extending from ModelBase * removed a sysout used for debugging * toJson() and fromJson() now leverages parent class's corresponding methods * virtual is not needed as override essentially does the same thing. * added docstring for getModelByName * corrected the javadoc * fixed @param issue in javadoc * fixed @param uncapitalized P in param in javadoc --- .../languages/CppRestClientCodegen.java | 49 +++++++++++++++++++ .../languages/NancyFXServerCodegen.java | 24 +-------- .../io/swagger/codegen/utils/ModelUtils.java | 36 ++++++++++++++ .../resources/cpprest/api-source.mustache | 2 +- .../resources/cpprest/model-header.mustache | 11 ++++- .../resources/cpprest/model-source.mustache | 20 +++++--- 6 files changed, 109 insertions(+), 33 deletions(-) create mode 100644 modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/ModelUtils.java diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/CppRestClientCodegen.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/CppRestClientCodegen.java index 3c2dac1dca3..6053939fa4e 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/CppRestClientCodegen.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/CppRestClientCodegen.java @@ -1,7 +1,10 @@ package io.swagger.codegen.languages; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.Multimap; import io.swagger.codegen.*; import io.swagger.codegen.examples.ExampleGenerator; +import io.swagger.codegen.utils.ModelUtils; import io.swagger.models.Model; import io.swagger.models.Operation; import io.swagger.models.Response; @@ -11,6 +14,8 @@ import io.swagger.models.properties.*; import java.util.*; import java.io.File; +import static com.google.common.base.Strings.isNullOrEmpty; + public class CppRestClientCodegen extends DefaultCodegen implements CodegenConfig { public static final String DECLSPEC = "declspec"; @@ -20,6 +25,9 @@ public class CppRestClientCodegen extends DefaultCodegen implements CodegenConfi protected String declspec = ""; protected String defaultInclude = ""; + private final Set parentModels = new HashSet<>(); + private final Multimap childrenByParent = ArrayListMultimap.create(); + /** * Configures the type of generator. * @@ -236,6 +244,13 @@ public class CppRestClientCodegen extends DefaultCodegen implements CodegenConfi if (isFileProperty(property)) { property.vendorExtensions.put("x-codegen-file", true); } + + if (!isNullOrEmpty(model.parent)) { + parentModels.add(model.parent); + if (!childrenByParent.containsEntry(model.parent, model)) { + childrenByParent.put(model.parent, model); + } + } } protected boolean isFileProperty(CodegenProperty property) { @@ -397,4 +412,38 @@ public class CppRestClientCodegen extends DefaultCodegen implements CodegenConfi return input.replace("*/", "*_/").replace("/*", "/_*"); } + @Override + public Map postProcessAllModels(final Map models) { + + final Map processed = super.postProcessAllModels(models); + postProcessParentModels(models); + return processed; + } + + private void postProcessParentModels(final Map models) { + for (final String parent : parentModels) { + final CodegenModel parentModel = ModelUtils.getModelByName(parent, models); + final Collection childrenModels = childrenByParent.get(parent); + for (final CodegenModel child : childrenModels) { + processParentPropertiesInChildModel(parentModel, child); + } + } + } + + /** + * Sets the child property's isInherited flag to true if it is an inherited property + */ + private void processParentPropertiesInChildModel(final CodegenModel parent, final CodegenModel child) { + final Map childPropertiesByName = new HashMap<>(child.vars.size()); + for (final CodegenProperty childProperty : child.vars) { + childPropertiesByName.put(childProperty.name, childProperty); + } + for (final CodegenProperty parentProperty : parent.vars) { + final CodegenProperty duplicatedByParent = childPropertiesByName.get(parentProperty.name); + if (duplicatedByParent != null) { + duplicatedByParent.isInherited = true; + } + } + } + } diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/NancyFXServerCodegen.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/NancyFXServerCodegen.java index 6e63dbbedf6..3d21bed182a 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/NancyFXServerCodegen.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/NancyFXServerCodegen.java @@ -12,6 +12,7 @@ import io.swagger.codegen.CodegenOperation; import io.swagger.codegen.CodegenProperty; import io.swagger.codegen.CodegenType; import io.swagger.codegen.SupportingFile; +import io.swagger.codegen.utils.ModelUtils; import io.swagger.models.Swagger; import io.swagger.models.properties.Property; import io.swagger.models.properties.StringProperty; @@ -228,7 +229,7 @@ public class NancyFXServerCodegen extends AbstractCSharpCodegen { private void postProcessParentModels(final Map models) { log.debug("Processing parents: " + parentModels); for (final String parent : parentModels) { - final CodegenModel parentModel = modelByName(parent, models); + final CodegenModel parentModel = ModelUtils.getModelByName(parent, models); parentModel.hasChildren = true; final Collection childrenModels = childrenByParent.get(parent); for (final CodegenModel child : childrenModels) { @@ -237,27 +238,6 @@ public class NancyFXServerCodegen extends AbstractCSharpCodegen { } } - private CodegenModel modelByName(final String name, final Map models) { - final Object data = models.get(name); - if (data instanceof Map) { - final Map dataMap = (Map) data; - final Object dataModels = dataMap.get("models"); - if (dataModels instanceof List) { - final List dataModelsList = (List) dataModels; - for (final Object entry : dataModelsList) { - if (entry instanceof Map) { - final Map entryMap = (Map) entry; - final Object model = entryMap.get("model"); - if (model instanceof CodegenModel) { - return (CodegenModel) model; - } - } - } - } - } - return null; - } - private void processParentPropertiesInChildModel(final CodegenModel parent, final CodegenModel child) { final Map childPropertiesByName = new HashMap<>(child.vars.size()); for (final CodegenProperty property : child.vars) { diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/ModelUtils.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/ModelUtils.java new file mode 100644 index 00000000000..49e40effbff --- /dev/null +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/ModelUtils.java @@ -0,0 +1,36 @@ +package io.swagger.codegen.utils; + +import io.swagger.codegen.CodegenModel; + +import java.util.List; +import java.util.Map; + +public class ModelUtils { + /** + * Searches for the model by name in the map of models and returns it + * + * @param name Name of the model + * @param models Map of models + * @return model + */ + public static CodegenModel getModelByName(final String name, final Map models) { + final Object data = models.get(name); + if (data instanceof Map) { + final Map dataMap = (Map) data; + final Object dataModels = dataMap.get("models"); + if (dataModels instanceof List) { + final List dataModelsList = (List) dataModels; + for (final Object entry : dataModelsList) { + if (entry instanceof Map) { + final Map entryMap = (Map) entry; + final Object model = entryMap.get("model"); + if (model instanceof CodegenModel) { + return (CodegenModel) model; + } + } + } + } + } + return null; + } +} diff --git a/modules/swagger-codegen/src/main/resources/cpprest/api-source.mustache b/modules/swagger-codegen/src/main/resources/cpprest/api-source.mustache index 9899a86b6e5..343a2697817 100644 --- a/modules/swagger-codegen/src/main/resources/cpprest/api-source.mustache +++ b/modules/swagger-codegen/src/main/resources/cpprest/api-source.mustache @@ -325,7 +325,7 @@ pplx::task<{{#returnType}}{{{returnType}}}{{/returnType}}{{^returnType}}void{{/r {{#vendorExtensions.x-codegen-response.items.isPrimitiveType}}result[item.first] = ModelBase::{{vendorExtensions.x-codegen-response.items.datatype}}FromJson(item.second); {{/vendorExtensions.x-codegen-response.items.isPrimitiveType}}{{^vendorExtensions.x-codegen-response.items.isPrimitiveType}}{{#vendorExtensions.x-codegen-response.items.isString}}result[item.first] = ModelBase::stringFromJson(item.second); {{/vendorExtensions.x-codegen-response.items.isString}}{{^vendorExtensions.x-codegen-response.items.isString}}{{{vendorExtensions.x-codegen-response.items.datatype}}} itemObj({{{vendorExtensions.x-codegen-response.items.defaultValue}}}); - itemObj->fromJson(item); + itemObj->fromJson(item.second); result[item.first] = itemObj; {{/vendorExtensions.x-codegen-response.items.isString}}{{/vendorExtensions.x-codegen-response.items.isPrimitiveType}} } diff --git a/modules/swagger-codegen/src/main/resources/cpprest/model-header.mustache b/modules/swagger-codegen/src/main/resources/cpprest/model-header.mustache index 4dc426e76ca..c32084eebc2 100644 --- a/modules/swagger-codegen/src/main/resources/cpprest/model-header.mustache +++ b/modules/swagger-codegen/src/main/resources/cpprest/model-header.mustache @@ -8,8 +8,10 @@ #ifndef {{classname}}_H_ #define {{classname}}_H_ +{{^parent}} {{{defaultInclude}}} #include "ModelBase.h" +{{/parent}} {{#imports}}{{{this}}} {{/imports}} @@ -22,7 +24,7 @@ namespace {{this}} { /// {{description}} /// class {{declspec}} {{classname}} - : public ModelBase + : public {{#parent}}{{{parent}}}{{/parent}}{{^parent}}ModelBase{{/parent}} { public: {{classname}}(); @@ -43,6 +45,7 @@ public: /// {{classname}} members {{#vars}} + {{^isInherited}} /// /// {{description}} /// @@ -52,12 +55,16 @@ public: {{/isNotContainer}}{{^required}}bool {{baseName}}IsSet() const; void unset{{name}}(); {{/required}} + {{/isInherited}} {{/vars}} protected: - {{#vars}}{{{datatype}}} m_{{name}}; + {{#vars}} + {{^isInherited}} + {{{datatype}}} m_{{name}}; {{^required}}bool m_{{name}}IsSet; {{/required}} + {{/isInherited}} {{/vars}} }; diff --git a/modules/swagger-codegen/src/main/resources/cpprest/model-source.mustache b/modules/swagger-codegen/src/main/resources/cpprest/model-source.mustache index 4856a11f5ee..33a35a908a2 100644 --- a/modules/swagger-codegen/src/main/resources/cpprest/model-source.mustache +++ b/modules/swagger-codegen/src/main/resources/cpprest/model-source.mustache @@ -9,11 +9,11 @@ namespace {{this}} { {{classname}}::{{classname}}() { - {{#vars}}{{#isNotContainer}}{{#isPrimitiveType}}m_{{name}} = {{{defaultValue}}}; + {{#vars}}{{^isInherited}}{{#isNotContainer}}{{#isPrimitiveType}}m_{{name}} = {{{defaultValue}}}; {{/isPrimitiveType}}{{^isPrimitiveType}}{{#isString}}m_{{name}} = {{{defaultValue}}}; {{/isString}}{{#isDateTime}}m_{{name}} = {{{defaultValue}}}; {{/isDateTime}}{{/isPrimitiveType}}{{/isNotContainer}}{{^required}}m_{{name}}IsSet = false; - {{/required}}{{/vars}} + {{/required}}{{/isInherited}}{{/vars}} } {{classname}}::~{{classname}}() @@ -27,9 +27,10 @@ void {{classname}}::validate() web::json::value {{classname}}::toJson() const { - web::json::value val = web::json::value::object(); + {{#parent}}web::json::value val = this->{{{parent}}}::toJson(); {{/parent}} + {{^parent}}web::json::value val = web::json::value::object();{{/parent}} - {{#vars}}{{#isPrimitiveType}}{{^isListContainer}}{{^required}}if(m_{{name}}IsSet) + {{#vars}}{{^isInherited}}{{#isPrimitiveType}}{{^isListContainer}}{{^required}}if(m_{{name}}IsSet) { val[U("{{baseName}}")] = ModelBase::toJson(m_{{name}}); } @@ -53,14 +54,16 @@ web::json::value {{classname}}::toJson() const val[U("{{baseName}}")] = ModelBase::toJson(m_{{name}}); } {{/required}}{{#required}}val[U("{{baseName}}")] = ModelBase::toJson(m_{{name}}); - {{/required}}{{/isPrimitiveType}}{{/isListContainer}}{{/vars}} + {{/required}}{{/isPrimitiveType}}{{/isListContainer}}{{/isInherited}}{{/vars}} return val; } void {{classname}}::fromJson(web::json::value& val) { - {{#vars}}{{#isPrimitiveType}}{{^isListContainer}}{{^required}}if(val.has_field(U("{{baseName}}"))) + {{#parent}}this->{{{parent}}}::fromJson(val); {{/parent}} + + {{#vars}}{{^isInherited}}{{#isPrimitiveType}}{{^isListContainer}}{{^required}}if(val.has_field(U("{{baseName}}"))) { {{setter}}(ModelBase::{{baseType}}FromJson(val[U("{{baseName}}")])); } @@ -111,7 +114,7 @@ void {{classname}}::fromJson(web::json::value& val) {{/vendorExtensions.x-codegen-file}}{{^vendorExtensions.x-codegen-file}}{{{datatype}}} new{{name}}({{{defaultValue}}}); new{{name}}->fromJson(val[U("{{baseName}}")]); {{setter}}( new{{name}} ); - {{/vendorExtensions.x-codegen-file}}{{/isDateTime}}{{/isString}}{{/required}}{{/isPrimitiveType}}{{/isListContainer}}{{/vars}} + {{/vendorExtensions.x-codegen-file}}{{/isDateTime}}{{/isString}}{{/required}}{{/isPrimitiveType}}{{/isListContainer}}{{/isInherited}}{{/vars}} } void {{classname}}::toMultipart(std::shared_ptr multipart, const utility::string_t& prefix) const @@ -222,7 +225,7 @@ void {{classname}}::fromMultiPart(std::shared_ptr multipart, } -{{#vars}}{{^isNotContainer}}{{{datatype}}}& {{classname}}::{{getter}}() +{{#vars}}{{^isInherited}}{{^isNotContainer}}{{{datatype}}}& {{classname}}::{{getter}}() { return m_{{name}}; } @@ -245,6 +248,7 @@ void {{classname}}::unset{{name}}() m_{{name}}IsSet = false; } {{/required}} +{{/isInherited}} {{/vars}} {{#modelNamespaceDeclarations}}