openapi-python-client icon indicating copy to clipboard operation
openapi-python-client copied to clipboard

Don't prepend title based property names, use Titles for full model type names

Open rtaycher opened this issue 3 years ago • 3 comments

Is your feature request related to a problem? Please describe. Ideally an openapi description would get us nice not too long types without a lot of futzing with overriding type names via class_overrides.

If you add a title/subtitle to an objects body it seems to append it to the path based name making names very long and often redundant and you have to fix it via class_overrides (or if you control the openapi description by moving everything into components which dont cause long names)

An example to clarify

paths:
  /bob:
    get:
      operationId: get_bob_method
      requestBody:
        content: 
          application/json:
            schema:
              title: Get Bob Body
              type: object
              properties:
                ArrayProp:
                  type: array
                  items:
                    title: Bob Body Item
                    type: object
                    properties:
                      MyProp:
                        type: string
      responses:
        200:
          description: "Response"
          content:
            application/json:
              schema:
                title: The Robert Report
                type: object
                properties:
                  ArrayProp:
                    type: array
                    items:
                      title: Robert Response Item
                      type: object
                      properties:
                        MyPropR:
                          type: string       

results in GetBobMethodGetBobBody / GetBobMethodGetBobBodyBobBodyItem for the json_body and GetBobMethodTheRobertReport / GetBobMethodTheRobertReportRobertResponseItem for the response type instead of GetBobBody, BobBodyItem, and TheRobertReport/RobertResponseItem

Trying out the alternative openapi-generator-cli it returns GetBobBody, BobBodyItem, and TheRobertReport/RobertResponseItem.

While this is not mandated anywhere I think its much more sensible.

Usually with a title you will have named the full type/subtype and not need to prepend it with the method name/outer type.

Describe the solution you'd like We should keep appending to a name when using default property names/item but start from scratch every time we hit a title.

Currently the only way to fix things is to rename via class_overrides or if you have control of the openapi description move everything into components(since types from component only use the component name and not the full path name)

rtaycher avatar Dec 27 '21 12:12 rtaycher

I forgot to add that this could cause a lot of type name changes/churn for anyone generating their library with an older version then regenerating with a newer version of openapi-python-client.

I think the names would be nicer for most people but it would be a significant change(for who's openapi files use titles). I don't think this project grantees that sort of stability but if this is an issue maybe add a config flag in the config file for those who want to preserve old behavior?

rtaycher avatar Dec 27 '21 12:12 rtaycher

We used to just use the title and not prepend a bunch of stuff, but folks ended up getting a bunch of duplicate class names. Detecting those duplicate class names and renaming them in a deterministic manner turned out to be a really hard problem to solve, so we just made the names as unique as possible.

I'm all for generating better names, but we'd have to come up with a very good system for determining those names consistently (so adding a new route/body or reordering properties doesn't end up being a breaking change to the generated code).

If we come up with such a system, we also then have to be very sure it's worth the breaking change or worth the additional maintenance of two different naming systems (to your point about this being jarring after regen). Most users are still on 0.9, I assume because of breaking changes in 0.10 😬.

Maybe an option we can consider is using modules to guard against duplicate class names. We only really need reusable classes (e.g., things in #/components) in models, we could put route-specific classes in their route's module.

dbanty avatar Dec 27 '21 17:12 dbanty

I did think about namespacing models (although I imagined they would be layered in models instead of in api routes IE get_bob_method.get_bob_body.BobBodyItem instead of GetBobMethodGetBobBodyBobBodyItem (that way you don't have to worry about duplicates model names in the same route).

But that seemed more complicated/might need a lot of discussion. Plus I've been using the model re-export

import api_lib.model as m
from api_lib.api. import get_stuff
#...
get_stuff.sync(client=client, json_body=m.StuffGetter(...)

and it seemed like it might not work as nicely with that pattern.


Would it be possible to make fullpaths the default/and have short title names via an override in the config file? That way people who might have duplicate titles/components can avoid it while others can take advantage.

I tried it out in #560 and it doesn't seem like a lot of code.

rtaycher avatar Dec 28 '21 12:12 rtaycher