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

Name collision on request/response objects #16

Open
leeola opened this issue Feb 29, 2020 · 3 comments
Open

Name collision on request/response objects #16

leeola opened this issue Feb 29, 2020 · 3 comments
Labels
bug Something isn't working rocket-okapi This affects the rocket-okapi crate

Comments

@leeola
Copy link

leeola commented Feb 29, 2020

When two routes have the same name for a Request / Response the namespaces are clobbered. Example:

https://github.com/leeola/example_okapi_name_collision (specifically this file) will incorrectly report that the /two route has a response type of:

{
  "reply": "string"
}

Instead of what it should be,

{
  "different_schema": "string"
}

I'm happy to help developing a solution to this, but I'm unsure how best it can be handled. Ideally, I imagine something resembling idiomatic OpenAPI specs should be generated. Though, looking at the classic PetStore example it looks like that entire bottom area is ripe for having many structure names occupy the same namespace.

Thoughts on how best to handle this?

@leeola leeola changed the title Name collision on response objects Name collision on request/response objects Feb 29, 2020
@leeola
Copy link
Author

leeola commented Feb 29, 2020

Additionally, I should note that there are two possible areas to fix:

  1. In the route section structures can be incorrectly reported. As in the original report.
  2. Even if the Schemas section allowed both Response objects to have their own section, it would not be visually clear what Response object each is. Some type of prefix may be desired, eg: /one::Response, /two::Response.

I suppose another option to fixing this would be to provide some form of openapi_struct(rename = "ResponseOne"). While this would work, it would be quite tedious for large projects.

I suppose it depends on what would produce the most idiomatic OpenAPI sheet.

@leeola
Copy link
Author

leeola commented Mar 1, 2020

Here is the JSON for reference:

{
  "openapi": "3.0.0",
  "info": {
    "title": "",
    "version": ""
  },
  "servers": [
    {
      "url": "/my_resource"
    }
  ],
  "paths": {
    "/one": {
      "get": {
        "operationId": "handler_one_handler_one",
        "responses": {
          "200": {
            "description": "",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Response"
                }
              }
            }
          }
        }
      }
    },
    "/two": {
      "get": {
        "operationId": "handler_two_handler_two",
        "responses": {
          "200": {
            "description": "",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Response"
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Response": {
        "type": "object",
        "required": [
          "reply"
        ],
        "properties": {
          "reply": {
            "type": "string"
          }
        }
      }
    }
  }
}

@GREsau
Copy link
Owner

GREsau commented May 24, 2020

So this is really a shortcoming of schemars, where schemas are given a name which gets appended to #/components/schemas/ (or #/definitions/ for non-openapi schemas). When deriving JsonSchema on a type, the default schema name is simply set to the name of the type, which can indeed cause problems when multiple types have the same name.

This can currently be worked around by adding a #[schemars(rename = "...")] or #[serde(rename = "...")] attribute to one of the types, which will override the schema name:

pub mod handler_two {
    use super::*;

    #[derive(serde::Serialize, schemars::JsonSchema)]
    #[schemars(rename = "Response2")] // <-- add this attribute!
    pub struct Response {
        different_schema: String,
    }

...

It may be possible to improve this in schemars by including the output of module_path!() in the schema name, as long as it doesn't make the names too verbose...I'll have a think about it

@ralpha ralpha added bug Something isn't working rocket-okapi This affects the rocket-okapi crate labels Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rocket-okapi This affects the rocket-okapi crate
Projects
None yet
Development

No branches or pull requests

3 participants