[cpp-restbed] Added "out-of-the-box" functionality #4759

AIexG · 2019-12-10T13:44:05Z

With those changes the cpp-restbed Codegen should generate a server which automatically stores data received through a request in objects and also creates necessary instances of objects.
The goal was to have the server compile out of the box, to offer a quick start with projects.

Fixed missing objects created from request bodies in api class files.
Fixed member variable names starting with an underscore ("_") generating the same getters and setters as members without the underscore, which led to duplicate generation of the same functions.
Added default value make_shared<Object>() to shared pointers to allow automated object generation
Added classes getting derived from their respective interfaces
Added override of updateAllModels() to clean interfaces of ambiguity
Improves ENUM handling by getting only saved if value is present in spezification

Requires the changes from my PRs #4753 and #4758.

PR checklist

Read the contribution guidelines.
If contributing template-only or documentation-only changes which will change sample output, build the project before.
Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

Additional context

Not sure if I should commit the generated server files for the samples as well, please tell me or remove them if necessary.

Requires the changes from my PRs #4753 to compile.

First pull request(s), hopefully doing everything right!

technical committee

@ravinikam (2017/07) @stkrwork (2017/07) @etherealjoy (2018/02) @MartinDelille (2018/03) @muttleyxd (2019/08)

* handling class dependencies * added method override to clean interfaces of ambiguity * added default values for shared pointers * fixed _name and name parameters generating the same getters and setters * updated enum handling * updated Petstore samples * updated templates for automated object generation

wing328 · 2019-12-10T13:55:17Z

[ERROR] COMPILATION ERROR : 
[ERROR] /home/travis/build/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CppRestbedServerCodegen.java:[107,47] cannot find symbol
  symbol:   method getAllModels(java.util.Map<java.lang.String,java.lang.Object>)
  location: class org.openapitools.codegen.languages.CppRestbedServerCodegen
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project openapi-generator: Compilation failure
[ERROR] /home/travis/build/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CppRestbedServerCodegen.java:[107,47] cannot find symbol
[ERROR]   symbol:   method getAllModels(java.util.Map<java.lang.String,java.lang.Object>)
[ERROR]   location: class org.openapitools.codegen.languages.CppRestbedServerCodegen
[ERROR] -> [Help 1]
[ERROR]

@AIexG thanks for the PR. Please have a look at the compilation errors.

AIexG · 2019-12-10T14:25:24Z

@AIexG thanks for the PR. Please have a look at the compilation errors.

@wing328 as mentioned this PR requires the changes from my PR #4753, I've split them up as the PR does not contain not cpp-restbed specific changes. Edit: Should I instead delete the other PR and push it into this one?

I think I had seen a checkbox to wait untill approved somewhere when I did the first PR, can't find it anywhere anymore, sorry for the inconvenience.

...napi-generator/src/main/java/org/openapitools/codegen/languages/CppRestbedServerCodegen.java

wing328 · 2019-12-19T03:32:30Z

CIs report the compilation errors:

[ERROR] COMPILATION ERROR : 
[ERROR] /home/travis/build/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CppRestbedServerCodegen.java:[107,47] cannot find symbol
  symbol:   method getAllModels(java.util.Map<java.lang.String,java.lang.Object>)
  location: class org.openapitools.codegen.languages.CppRestbedServerCodegen
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project openapi-generator: Compilation failure
[ERROR] /home/travis/build/OpenAPITools/openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/CppRestbedServerCodegen.java:[107,47] cannot find symbol
[ERROR]   symbol:   method getAllModels(java.util.Map<java.lang.String,java.lang.Object>)
[ERROR]   location: class org.openapitools.codegen.languages.CppRestbedServerCodegen
[ERROR] -> [Help 1]
[ERROR]

Let us know if you need help fixing it.

muttleyxd

Fine changes, but I'd change few things regarding model's members and their initialization

muttleyxd · 2019-12-19T07:31:35Z

modules/openapi-generator/src/main/resources/cpp-restbed-server/api-header.mustache

@@ -76,6 +76,10 @@ private:
 		{{#allParams}}{{{dataType}}} const &{{#hasMore}}, {{/hasMore}}{{/allParams}}
 	)> handler_{{httpMethod}}_;
 	{{/vendorExtensions.x-codegen-otherMethods}}
+
+	{{#allParams}}
+	{{{dataType}}} {{paramName}};


You could initialize paramName with braces, so it would be

{{{dataType}}} {{paramName}}{};

which would result in:

int someParam{}

which results in initializing int as zero. This works for any type unless there are some special requirements when initializing it.

You could initialize paramName with braces, so it would be {{{dataType}}} {{paramName}}{};

Edit: confused the api-header and model-header for a sec

By default there should be no special requirements on initialisation. Added in
feddd15

muttleyxd · 2019-12-19T07:37:09Z

modules/openapi-generator/src/main/resources/cpp-restbed-server/model-header.mustache

 protected:
    {{#vars}}
    {{{dataType}}} m_{{name}};
    {{/vars}}
+    {{#vars}}
+    {{#isEnum}}
+    std::vector<{{{dataType}}}> m_{{enumName}};


This could possibly be static const std::vector, but that's just a suggestion (if our uses are rogue and do const_cast's then it could be better to have copies of our enums).

Yup, makes sense.

In hindsight I could have created a member to save the value that is an actual ENUM instead, instead of keeping it as a std::string, but that seemed just a tad easier.

muttleyxd · 2019-12-19T07:48:00Z

modules/openapi-generator/src/main/resources/cpp-restbed-server/model-header.mustache

+    {{#isEnum}}
+    std::vector<{{{dataType}}}> m_{{enumName}};
+    {{/isEnum}}
+    {{/vars}}


This whole block could be simplified to:

{{#vars}} {{{dataType}}} m_{{name}}{}; {{#isEnum}} static const std::vector<{{{dataType}}}> m_{{enumName}} { {{#allowableValues}}{{#enumVars}}{{^-first}}, {{/-first}}{{{value}}}{{/enumVars}}{{/allowableValues}} }; {{/isEnum}} {{/vars}}

and with it we could remove all initialization from model's constructor.

Note: I moved {{#isEnum}} for shorter notation, but I'm completely fine with it being outside in a separate block.

muttleyxd · 2019-12-19T07:49:11Z

modules/openapi-generator/src/main/resources/cpp-restbed-server/model-source.mustache

+    {{/isEnum}}
+	{{/isPrimitiveType}}
+	{{/isContainer}}
+	{{/vars}}


All of this could be removed if we use brace initialization, explained in my previous comments.

Some schemas might contain self, cross or circular references. Which could initialize infinite objects when parsing the JSON, as objects inside members would also be initialized upon initialisation. For that reason I've decided to initialize models as obejcts when actually required in fromPropertyTree(). Also most members will receive their {{{defaultValue}}} if not supplied in the JSON.

Some schemas might contain self, cross or circular references.

You're right, I forgot about that. I wonder if there is a requirement that self/cross/circular property cannot be set as required. Then some kind of optional class would help, but I guess we don't have such constraint.

Then I'm fine with it, I'm thinking if it's viable to implement copy/move constructors to avoid this problem with reference-copying.

I wonder if there is a requirement that self/cross/circular property cannot be set as required.

At least in the OpenAPI-Specification or in the Generator there is none. Then again the restbed generator currently doesn't validate if a passed value is writeable/readable/required.

muttleyxd · 2019-12-19T08:34:52Z

modules/openapi-generator/src/main/resources/cpp-restbed-server/api-source.mustache

@@ -48,6 +48,10 @@ void {{classname}}::stopService() {
 		std::bind(&{{classname}}{{vendorExtensions.x-codegen-resourceName}}Resource::{{httpMethod}}_method_handler, this,
 			std::placeholders::_1));
 	{{/vendorExtensions.x-codegen-otherMethods}}
+
+	{{#allParams}}{{#defaultValue}}{{^isListContainer}}{{paramName}} = {{{defaultValue}}};
+	{{/isListContainer}}{{/defaultValue}}{{^defaultValue}}{{#isModel}}{{paramName}} = std::make_shared<{{baseType}}>();


Is there a reason to make models std::shared_ptr? This will cause problems with using it with no observable benefit (at least I don't see any).

Consider following yaml:

openapi: 3.0.0 info: description: Some description version: 0.0.1 title: Some title tags: - name: hello paths: "/there": get: operationId: helloThereGet tags: - hello summary: Do something responses: 200: description: Successful operation content: application/json: schema: $ref: "#/components/schemas/OurModel" post: requestBody: description: json to deserialize then serialize required: true content: application/json: schema: $ref: "#/components/schemas/OurModel" operationId: helloTherePost tags: - hello summary: Do something (receive) responses: 200: description: Successful operation content: application/json: schema: $ref: "#/components/schemas/OurModel" servers: - url: http://localhost:8080 components: schemas: OurModel: type: object properties: someints: type: array items: type: integer member: $ref: '#/components/schemas/NestedMember' strmem: type: string NestedMember: type: object properties: number: type: integer text: type: string required: - number - text

test file:

#include <gtest/gtest.h> #include "model/OurModel.h" TEST(OurModel, SimpleTest) { using namespace org::openapitools::server::model; OurModel model; model.setSomeints({1, 2, 3}); auto nm = std::make_shared<NestedMember>(); nm->setNumber(10); nm->setText("some text"); model.setMember(nm); EXPECT_EQ((std::vector<int32_t>{1, 2, 3}), model.getSomeints()); // success EXPECT_EQ(10, model.getMember()->getNumber()); // success EXPECT_EQ("some text", model.getMember()->getText()); // success OurModel copy = model; copy.setSomeints({4, 5, 6}); copy.getMember()->setText("this modifies model->member->text too"); EXPECT_EQ((std::vector<int32_t>{1, 2, 3}), model.getSomeints()); // success EXPECT_EQ(10, model.getMember()->getNumber()); // success EXPECT_EQ("some text", model.getMember()->getText()); // fail, even though it got modified in other object EXPECT_EQ((std::vector<int32_t>{4, 5, 6}), copy.getSomeints()); // success EXPECT_EQ(10, copy.getMember()->getNumber()); // success EXPECT_EQ("this modifies model->member->text too", copy.getMember()->getText()); // success }

Output:

main.cpp:27: Failure Expected equality of these values: "some text" model.getMember()->getText() Which is: "this modifies model->member->text too"

I'd be fine with using shared_ptr if every property would use it. But it's inconsistent, you can copy your model's object, modify any property except for nested models and it won't affect original object. Then you would modify nested object and it will modify any number of other objects, which probably isn't what we want.

Might just be due to lack of experience. I had this intentionally after it would not compile with my testing changes, seems to work just fine without. I've removed the lines in the commit ff780fa !

muttleyxd

Sorry for not responding for 2 weeks, but I've been not using my PC recently.

I'm not exactly satisfied with this shared_ptr situation and inconsistent copying, but I guess we have to live it (since it was like that before this PR too).

Looks good to me!

wing328 · 2020-01-02T12:36:09Z

CircleCI failure already fixed in the master.

* master: (275 commits) Initial CODEOWNERS (OpenAPITools#4924) [scala] Support for Set when array has uniqueItems=true (OpenAPITools#4926) remove nodejs server samples, scripts (OpenAPITools#4919) Added ability to work with `defaultHeaders` and fixed authentication for code generated by openapi-generator for typescript-node (OpenAPITools#4896) replace petstore_api with packageName (OpenAPITools#4921) remove base_object_spec.mustache from ruby client (OpenAPITools#4918) Add an link to Ada article (OpenAPITools#4920) avoid using hardcode prefix in example (OpenAPITools#4917) [dart-dio] Fix basepath (OpenAPITools#4911) [java][client] jackson update (OpenAPITools#4907) [Swift] Minor improvements to swift 5 generator (OpenAPITools#4910) [cpp-restbed] Added "out-of-the-box" functionality (OpenAPITools#4759) New generator swift5 (OpenAPITools#4086) [dart-dio] Adds support for multipart form data post body (OpenAPITools#4797) [go][client] fix when schema have multiple servers (OpenAPITools#4901) Unables CI tests of python-flask-python2 (OpenAPITools#4889) [C-libcurl] The JSON key name in request/response body should not be escaped even though it is a C key word. (OpenAPITools#4893) upgrade to JUnit 4.13 (OpenAPITools#4899) [r] Ignoring README.md in Rbuildignore (OpenAPITools#4898) update samples ...

fix whitespace

00156bc

AIexG commented Dec 13, 2019

View reviewed changes

...napi-generator/src/main/java/org/openapitools/codegen/languages/CppRestbedServerCodegen.java Show resolved Hide resolved

Merge branch 'master' into improve_cpp_restbed

e3d118a

muttleyxd suggested changes Dec 19, 2019

View reviewed changes

AIexG added 2 commits December 19, 2019 12:56

removed model initialisation

ff780fa

added model brace initialisation

feddd15

muttleyxd approved these changes Jan 2, 2020

View reviewed changes

wing328 merged commit 1cb99e3 into OpenAPITools:master Jan 2, 2020

wing328 added this to the 4.2.3 milestone Jan 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cpp-restbed] Added "out-of-the-box" functionality #4759

[cpp-restbed] Added "out-of-the-box" functionality #4759

AIexG commented Dec 10, 2019 •

edited

Loading

wing328 commented Dec 10, 2019

AIexG commented Dec 10, 2019 •

edited

Loading

wing328 commented Dec 19, 2019

muttleyxd left a comment

muttleyxd Dec 19, 2019 •

edited

Loading

AIexG Dec 19, 2019 •

edited

Loading

muttleyxd Dec 19, 2019

AIexG Dec 19, 2019

muttleyxd Dec 19, 2019

muttleyxd Dec 19, 2019

AIexG Dec 19, 2019

muttleyxd Dec 19, 2019

AIexG Dec 19, 2019

muttleyxd Dec 19, 2019

AIexG Dec 19, 2019 •

edited

Loading

muttleyxd left a comment

wing328 commented Jan 2, 2020

[cpp-restbed] Added "out-of-the-box" functionality #4759

[cpp-restbed] Added "out-of-the-box" functionality #4759

Conversation

AIexG commented Dec 10, 2019 • edited Loading

PR checklist

Additional context

technical committee

wing328 commented Dec 10, 2019

AIexG commented Dec 10, 2019 • edited Loading

wing328 commented Dec 19, 2019

muttleyxd left a comment

Choose a reason for hiding this comment

muttleyxd Dec 19, 2019 • edited Loading

Choose a reason for hiding this comment

AIexG Dec 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AIexG Dec 19, 2019 • edited Loading

Choose a reason for hiding this comment

muttleyxd left a comment

Choose a reason for hiding this comment

wing328 commented Jan 2, 2020

AIexG commented Dec 10, 2019 •

edited

Loading

AIexG commented Dec 10, 2019 •

edited

Loading

muttleyxd Dec 19, 2019 •

edited

Loading

AIexG Dec 19, 2019 •

edited

Loading

AIexG Dec 19, 2019 •

edited

Loading