Skip to content
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.

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

[qt5 server] Improvement in response handling #675

Merged
merged 10 commits into from
Jul 30, 2018

Conversation

etherealjoy
Copy link
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

  • Support serialization of response and error body for json and non json types
  • Remove Leading underscore in headers due to clang warnings
  • Support header insertion/retrieval
  • Remove redundant overrides from super
  • Add missing route setup
  • Apply const and const reference where applicable
  • Add placeholder to send custom responses
  • Suppress known warnings in codegen class
  • Implemented improvements mentioned here [cpp] pass arguments as const reference  #272

@stkrwork @MartinDelille

socket->writeHeaders();{{#returnType}}{{^isPrimitiveType}}
QJsonDocument resDoc(::{{cppNamespace}}::toJsonValue(res).to{{^isListContainer}}Object{{/isListContainer}}{{#isListContainer}}Array{{/isListContainer}}());{{/isPrimitiveType}}
socket->writeJson(resDoc);{{#isPrimitiveType}}
socket->write(::{{cppNamespace}}::toStringValue(res).toUtf8());{{/isPrimitiveType}}{{/returnType}}{{^returnType}}
Copy link
Contributor Author

@etherealjoy etherealjoy Jul 28, 2018

Choose a reason for hiding this comment

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

@wing328 @stkrwork
How is the serialization done for the array/map of primitive types?
Are they comma/CRLF/white space separated?

Copy link
Contributor

Choose a reason for hiding this comment

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

For restbed i used json arrays

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for json arrays it is OK, I mean if the response is array or map of primitive types.
Beccause they are arrays/maps of strings , how do we indicate it is an array instead of a long string. How is this defined in spec?

@etherealjoy
Copy link
Contributor Author

etherealjoy commented Jul 28, 2018

@wing328 @stkrwork
https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/cpp-qt5-qhttpengine-server/apirouter.cpp.mustache#L69

  • How do we handle multiple pathparam in other langs.
  • I wish to make this code in the line above somehow indexable so that it becomes more performant in case of multiple paths with pathparams. Suggestions?

@wing328
Copy link
Member

wing328 commented Jul 29, 2018

How do we handle multiple pathparam in other langs.

@etherealjoy can you please give an example to illustrate the problem?

@etherealjoy
Copy link
Contributor Author

etherealjoy commented Jul 30, 2018

swagger: '2.0'
info:
  title: Test
  version: v1.0

basePath: /
schemes:
  - http

paths:
  /endpoints/{pathpar1}/{pathpar2}:
    get:
      produces:
        - text/string
      parameters:
        - name: pathpar1
          required: true
          in: path
          type: string
        - name: pathpar2
          required: true
          in: path
          type: string          
      responses:
        '200':
          description: Success
          schema:
            type: array
            items:
              type: string
        '400':
          description: Bad Request
          schema:
            type: string

I have no idea how the string would look like for this response.
And how are the path params handled in such cases

@wing328
Copy link
Member

wing328 commented Jul 30, 2018

      responses:
        '200':
          description: Success
          schema:
            type: array
            items:
              type: string

It should be something like:

["cat", "dog", "bird"]

@etherealjoy
Copy link
Contributor Author

@wing328
For primitive types I don't think we have Maps right?

@wing328
Copy link
Member

wing328 commented Jul 30, 2018

@etherealjoy right. Map/Dictionary/Hash is "type: object"

@etherealjoy
Copy link
Contributor Author

etherealjoy commented Jul 30, 2018

I will add support for multiple path params in a later PR.

@wing328 wing328 merged commit 926b971 into OpenAPITools:master Jul 30, 2018
@wing328 wing328 added this to the 3.2.0 milestone Jul 30, 2018
@etherealjoy etherealjoy deleted the qt5server-improvement branch July 30, 2018 16:32
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
* Remove warnings and add custom request sending

* Remove duplicate code from subclass and add missing setupRoutes

* Removed redundant override

* Add serialization of responses

* Fix CI failure

* Add inline function to duplicate code.

* Make const reference wherever possible

* Add support for Array of Primitive types.

* Add Array of Primitive support for Error response

* Update for multiple path params
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants