Skip to content

[C++][Pistache] Fixes for struct model#12006

Merged
wing328 merged 3 commits intoOpenAPITools:masterfrom
ppngn:pistache-fix-struct-model
May 1, 2022
Merged

[C++][Pistache] Fixes for struct model#12006
wing328 merged 3 commits intoOpenAPITools:masterfrom
ppngn:pistache-fix-struct-model

Conversation

@ppngn
Copy link
Copy Markdown
Contributor

@ppngn ppngn commented Mar 30, 2022

Hi, I have tried to use struct models with newest Pistache version but it does not compile:

  • bunch of conversion warnings (truncating from 32-bit int to 16-bit uint)
  • Pistache::Optional was used but it got removed from Pistache some time ago
  • validation was missing, I have ported the validation to struct model, I think it looks okay - I have generated samples with useStructModel: "true" and the part related to validation did not change in PetStore sample

I have run bin/generate-samples.sh and it modified a bunch of files in .NET Core generator, but I didn't commit these files, apart from that everything else is committed.

$ git status
On branch pistache-fix-struct-model
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   samples/client/petstore/csharp-netcore/OpenAPIClient-generichost-net6.0-nrt/.openapi-generator/FILES
	modified:   samples/client/petstore/csharp-netcore/OpenAPIClient-generichost-net6.0/.openapi-generator/FILES
	modified:   samples/client/petstore/csharp-netcore/OpenAPIClient-generichost-netstandard2.0/.openapi-generator/FILES

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

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples: 
    ./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

muttleyxd
muttleyxd approved these changes Mar 30, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@muttleyxd muttleyxd left a comment

Choose a reason for hiding this comment

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

looks good to me

@wing328 wing328 added this to the 6.0.0 milestone Apr 3, 2022
@ppngn
Copy link
Copy Markdown
Contributor Author

ppngn commented Apr 26, 2022

@muttleyxd is anything blocking the merge?

@muttleyxd
Copy link
Copy Markdown
Contributor

muttleyxd commented Apr 26, 2022

Hi @ppngn, no idea to be honest.

I think @wing328 might be able to tell us when it can be merged

@wing328
Copy link
Copy Markdown
Member

wing328 commented May 1, 2022

Sorry for the delay in reviewing this PR.

I tested with update samples (useStructModel: "true") but got the following errors:

/usr/bin/cmake: /usr/local/lib/libcurl.so.4: no version information available (required by /usr/bin/cmake)
[ 51%] Building CXX object CMakeFiles/api-server.dir/api/PetApi.cpp.o
In file included from /home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/api/PetApi.h:30,
                 from /home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/api/PetApi.cpp:13:
/home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/model/Pet.h:35:29: error: declaration of 'std::optional<org::openapitools::server::model::Category> org::openapitools::server::model::Pet::Category' changes meaning of 'Category' [-fpermissive]
   35 |     std::optional<Category> Category;
      |                             ^~~~~~~~
In file included from /home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/model/Pet.h:24,
                 from /home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/api/PetApi.h:30,
                 from /home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/api/PetApi.cpp:13:
/home/wing328/Code/openapi-generator/samples/server/petstore/cpp-pistache/model/Category.h:29:8: note: 'Category' declared here as 'struct org::openapitools::server::model::Category'
   29 | struct Category
      |        ^~~~~~~~
make[2]: *** [CMakeFiles/api-server.dir/build.make:63: CMakeFiles/api-server.dir/api/PetApi.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:81: CMakeFiles/api-server.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

Can you please take a look when you've time?

The updated samples can be found in https://github.com/OpenAPITools/openapi-generator/tree/ppngn-pistache-fix-struct-model2

@wing328
Copy link
Copy Markdown
Member

wing328 commented May 1, 2022

Merged #12221 into master and then your branch, tested again and the result is good.

Thanks again for the PR.

@wing328 wing328 merged commit 153e1db into OpenAPITools:master May 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@muttleyxd muttleyxd muttleyxd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Issue: Bug Server: C++

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@ppngn @muttleyxd @wing328