allOf fails if it references a type that also uses allOf with just single item #1091
Comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 25, 2024
…llOf reference (#1103) Fixes #1091 Example of a valid spec that triggered this bug: https://gist.github.com/eli-bl/8f5c7d1d872d9fda5379fa6370dab6a8 In this spec, CreateCat contains only an `allOf` with a single reference to CreateAnimal. The current behavior of `openapi-python-client` in such a case is that it treats CreateCat as simply an alias for CreateAnimal; any references to it are treated as references to CreateAnimal, and it doesn't bother creating a model class for CreateCat. And if the spec contained only those two types, then this would be successful. (Note, the term "alias" doesn't exist in OpenAPI/JSON Schema, but I'm using it here to mean "a type that extends one other type with `allOf`, with no changes." Whether that should be treated as a separate thing in any way is not really a concern of OpenAPI; it's an issue for us only because we are generating code for model classes. See also: #1104) Anyway, in this case the spec also contains UpdateCat, which extends CreateCat with an additional property. This _should_ be exactly the same as extending CreateAnimal... but, prior to this fix, it resulted in a parsing error. The problem happened like this: 1. In `_create_schemas`, we create a ModelProperty for each of the three schemas. * The one for CreateCat is handled slightly differently: its `data` attribute points to the exact same schema as CreateAnimal, and we do not add it into `schemas.classes_by_name` because we don't want to generate a separate Python class for it. 2. In `_process_models`, we're attempting to finish filling in the property list for each model. * That might not be possible right away because there might be a reference to another model that hasn't been fully processed yet. So we iterate as many times as necessary until they're all fully resolved. However... * What we are iterating over is `schemas.classes_by_name`. There's an incorrect assumption that every named model is included in that dict; in this case, CreateCat is not in it. * Therefore, CreateCat remains in an invalid state, and the reference from CreateAnimal to CreateCat causes an error. My solution is to use `classes_by_name` only for the purpose of determining what Python classes to generate, and add a new collection, `models_to_process`, which includes _every_ ModelProperty including ones that are aliases. After the fix, generating a client from the example spec succeeds. The only Python model classes created are CreateAnimal and UpdateCat; the `post` endpoint that referenced CreateCat uses the CreateAnimal class. Again, that's consistent with how `openapi-python-client` currently handles these type aliases; the difference is just that it no longer fails when it sees a reference _to_ the alias. --------- Co-authored-by: Dylan Anthony <[email protected]>
Merged
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 25, 2024
> [!IMPORTANT] > Merging this pull request will create this release ## Fixes ### Allow OpenAPI 3.1-style `exclusiveMinimum` and `exclusiveMaximum` Fixed by PR #1092. Thanks @mikkelam! ### Add missing `cast` import when using `const` Fixed by PR #1072. Thanks @dorcohe! ### Correctly resolve references to a type that is itself just a single allOf reference PR #1103 fixed issue #1091. Thanks @eli-bl! ### Support `const` booleans and floats Fixed in PR #1086. Thanks @flxdot! Co-authored-by: knope-bot[bot] <152252888+knope-bot[bot]@users.noreply.github.com>
micha91
pushed a commit
to micha91/openapi-python-client
that referenced
this issue
May 13, 2025
…llOf reference (openapi-generators#1103) Fixes openapi-generators#1091 Example of a valid spec that triggered this bug: https://gist.github.com/eli-bl/8f5c7d1d872d9fda5379fa6370dab6a8 In this spec, CreateCat contains only an `allOf` with a single reference to CreateAnimal. The current behavior of `openapi-python-client` in such a case is that it treats CreateCat as simply an alias for CreateAnimal; any references to it are treated as references to CreateAnimal, and it doesn't bother creating a model class for CreateCat. And if the spec contained only those two types, then this would be successful. (Note, the term "alias" doesn't exist in OpenAPI/JSON Schema, but I'm using it here to mean "a type that extends one other type with `allOf`, with no changes." Whether that should be treated as a separate thing in any way is not really a concern of OpenAPI; it's an issue for us only because we are generating code for model classes. See also: openapi-generators#1104) Anyway, in this case the spec also contains UpdateCat, which extends CreateCat with an additional property. This _should_ be exactly the same as extending CreateAnimal... but, prior to this fix, it resulted in a parsing error. The problem happened like this: 1. In `_create_schemas`, we create a ModelProperty for each of the three schemas. * The one for CreateCat is handled slightly differently: its `data` attribute points to the exact same schema as CreateAnimal, and we do not add it into `schemas.classes_by_name` because we don't want to generate a separate Python class for it. 2. In `_process_models`, we're attempting to finish filling in the property list for each model. * That might not be possible right away because there might be a reference to another model that hasn't been fully processed yet. So we iterate as many times as necessary until they're all fully resolved. However... * What we are iterating over is `schemas.classes_by_name`. There's an incorrect assumption that every named model is included in that dict; in this case, CreateCat is not in it. * Therefore, CreateCat remains in an invalid state, and the reference from CreateAnimal to CreateCat causes an error. My solution is to use `classes_by_name` only for the purpose of determining what Python classes to generate, and add a new collection, `models_to_process`, which includes _every_ ModelProperty including ones that are aliases. After the fix, generating a client from the example spec succeeds. The only Python model classes created are CreateAnimal and UpdateCat; the `post` endpoint that referenced CreateCat uses the CreateAnimal class. Again, that's consistent with how `openapi-python-client` currently handles these type aliases; the difference is just that it no longer fails when it sees a reference _to_ the alias. --------- Co-authored-by: Dylan Anthony <[email protected]>
micha91
pushed a commit
to micha91/openapi-python-client
that referenced
this issue
May 13, 2025
> [!IMPORTANT] > Merging this pull request will create this release ## Fixes ### Allow OpenAPI 3.1-style `exclusiveMinimum` and `exclusiveMaximum` Fixed by PR openapi-generators#1092. Thanks @mikkelam! ### Add missing `cast` import when using `const` Fixed by PR openapi-generators#1072. Thanks @dorcohe! ### Correctly resolve references to a type that is itself just a single allOf reference PR openapi-generators#1103 fixed issue openapi-generators#1091. Thanks @eli-bl! ### Support `const` booleans and floats Fixed in PR openapi-generators#1086. Thanks @flxdot! Co-authored-by: knope-bot[bot] <152252888+knope-bot[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Conditions:
allOfwith a single element referencing Schema A.allOfthat 1. references Schema B and 2. adds a property.Expected behavior:
Observed behavior:
OpenAPI Spec File
https://gist.github.com/eli-bl/8f5c7d1d872d9fda5379fa6370dab6a8
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: