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

Add support for primitive data types in responses #902

Open
pablomg92z opened this issue Jun 6, 2023 · 5 comments · May be fixed by #945
Open

Add support for primitive data types in responses #902

pablomg92z opened this issue Jun 6, 2023 · 5 comments · May be fixed by #945

Comments

@pablomg92z
Copy link

pablomg92z commented Jun 6, 2023

Hi,

As far as I can see, grape-swagger supports object and file schema types grape-swagger#response-documentation but it does not support primitive data types (unless wrapped in a grape entity) as swagger 2.0 specification does Swagger 2.0#responsesDefinitionsObject.

This causes that the next response specification which is valid for swagger 2.0 cannot be generated using grape swagger:

{
  "description": "A simple string response",
  "schema": {
    "type": "string"
  }
}

So ideally I would like grape-swagger to support the schema type key to the responses, allowing us to specify responses using primitive data types like swagger 2.0 specifies.

As an example, adding the schema type key would allow us to describe primitive data type in responses:

success [
  { message: 'A simple string response' , type: String, as: :user_name },
]

which should generate the next specification:

{
  "description": "A simple string response",
  "schema": {
    "type": "string"
  }
}

Also the type schema should allow us to describe primitive data types directly into object schema types without the need to wrap them in a grape entity, as an example:

success [
  { model: API::V1::Entities::Roles::User, as: :users, is_array: true },
  { type: String, as: :page_token },
  { type: Integer, as: :count }
]

which should generate the next specification:

        "responses": {
          "200": {
            "description": "Return a list of users for given user role",
            "schema": {
              "type": "object",
              "properties": {
                "users": {
                  "type": "array",
                  "items": {
                    "$ref": "#/defs/API_V1_Entities_Roles_User"
                  }
                },
                "page_token": {
                  "type": "string"
                },
                "count": {
                  "type": "integer"
                }
              }
            }
          },

Would it be possible to add support for primitive data types as described in this issue? I can work on a PR if you agree. Thanks!

@LeFnord
Copy link
Member

LeFnord commented Jun 6, 2023

Hi @pablomg92z … in general I agree with a PR, but have one question.
For me the last example looks perfect, but the in the first and second one, there tha :as keyword exist, but will not used in the spec, is it wanted?

@pablomg92z
Copy link
Author

pablomg92z commented Jun 7, 2023

Hi @LeFnord , thanks for your quick reply! and good catch, I should have not included the :as keyword in the first example since it adds no value when defining primitive data type schemas (which is why it was not present in the output spec), as far as I know the :as keyword only makes sense for object type schemas.

I will proceed to create the PR then where I add support for the type keyword to describe primitive data types and arrays.

PS: What I can include in a future PR as well is the addition of the :format keyword as specified in Swagger 2.0#DataTypes which is an optional modifier property for primitives.

@Sashasugar
Copy link

Is there any progress on introducing these primitive types into a successful response?
I have an endpoint that return a list of string and I would love the FE api to have the docs generated properly.

Or if there's a way to make this happen now I would love to know.... 🙏

@mopp
Copy link

mopp commented Feb 28, 2024

@LeFnord @Sashasugar Hi, I also need this feature. Any update?

@gregg-platogo
Copy link

@LeFnord @Sashasugar Hi, I also need this feature. Any update?

@mopp I opened a PR to address that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants