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

tsp-openapi3 - scope top-level parameters to Parameters namespace #4216

Merged
merged 6 commits into from
Aug 29, 2024

Conversation

chrisradek
Copy link
Member

Fixes #4151

This PR updates tsp-openapi3's model generation so that all top-level parameters (#/components/parameters) are nested in a Parameters block namespace.

Prior to this change, if top-level parameter had the same name as a top-level schema, we would attempt to merge the two. This worked OK if the schema was an object type, but led to broken results if the schema was anything else.

Note:
In the linked issue, it was suggested that top-level schemas not be scoped to their own namespace, so if a schema is referenced by a parameter, it will now qualify it with the file-level namespace. This PR introduces a context object that contains some state that can be passed around. This is useful for keeping track of the file-level namespace and using it when necessary, but the context will also be useful in cases where we need to look at the definition of a referenced schema from another schema.

@chrisradek chrisradek added the openapi3:converter Issues for @typespec/openapi3 openapi to typespec converter label Aug 20, 2024
@azure-sdk
Copy link
Collaborator

azure-sdk commented Aug 20, 2024

All changed packages have been documented.

  • @typespec/openapi3
Show changes

@typespec/openapi3 - fix ✏️

Fixes issue in tsp-openapi3 that resulted in component schemas and parameters with the same name being merged into a single TypeSpec data type.

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

Copy link
Contributor

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

A few questions and comments.

@chrisradek chrisradek added this pull request to the merge queue Aug 29, 2024
Merged via the queue into microsoft:main with commit 5a75506 Aug 29, 2024
22 checks passed
@chrisradek chrisradek deleted the tsp-openapi3-add-context branch August 29, 2024 19:14
github-merge-queue bot pushed a commit that referenced this pull request Aug 29, 2024
Fixes #4152
Depends on #4216

This PR updates how tsp-openapi3 handles generating models for schemas
that use `allOf`.

Currently `allOf` is ignored unless there is only 1 member and that
member is a schema reference. In this scenario, the model extends the
single member.

This update now takes all of the schema `allOf` members into
consideration when generating a model.
- inline-schemas have their properties merged into the model's
properties
- schema references without a discriminator defined are spread into the
model
- if only 1 schema reference contains a discriminator, then the model
extends it, otherwise these schema references are spread as well.

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
weidongxu-microsoft pushed a commit to weidongxu-microsoft/typespec that referenced this pull request Sep 3, 2024
…crosoft#4216)

Fixes microsoft#4151 

This PR updates tsp-openapi3's model generation so that all top-level
parameters (`#/components/parameters`) are nested in a `Parameters`
block namespace.

Prior to this change, if top-level parameter had the same name as a
top-level schema, we would attempt to merge the two. This worked OK if
the schema was an object type, but led to broken results if the schema
was anything else.

Note:
In the linked issue, it was suggested that top-level schemas not be
scoped to their own namespace, so if a schema is referenced by a
parameter, it will now qualify it with the file-level namespace. This PR
introduces a `context` object that contains some state that can be
passed around. This is useful for keeping track of the file-level
namespace and using it when necessary, but the context will also be
useful in cases where we need to look at the definition of a referenced
schema from another schema.

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
weidongxu-microsoft pushed a commit to weidongxu-microsoft/typespec that referenced this pull request Sep 3, 2024
…soft#4232)

Fixes microsoft#4152
Depends on microsoft#4216

This PR updates how tsp-openapi3 handles generating models for schemas
that use `allOf`.

Currently `allOf` is ignored unless there is only 1 member and that
member is a schema reference. In this scenario, the model extends the
single member.

This update now takes all of the schema `allOf` members into
consideration when generating a model.
- inline-schemas have their properties merged into the model's
properties
- schema references without a discriminator defined are spread into the
model
- if only 1 schema reference contains a discriminator, then the model
extends it, otherwise these schema references are spread as well.

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
sarangan12 pushed a commit to sarangan12/typespec that referenced this pull request Sep 16, 2024
…crosoft#4216)

Fixes microsoft#4151 

This PR updates tsp-openapi3's model generation so that all top-level
parameters (`#/components/parameters`) are nested in a `Parameters`
block namespace.

Prior to this change, if top-level parameter had the same name as a
top-level schema, we would attempt to merge the two. This worked OK if
the schema was an object type, but led to broken results if the schema
was anything else.

Note:
In the linked issue, it was suggested that top-level schemas not be
scoped to their own namespace, so if a schema is referenced by a
parameter, it will now qualify it with the file-level namespace. This PR
introduces a `context` object that contains some state that can be
passed around. This is useful for keeping track of the file-level
namespace and using it when necessary, but the context will also be
useful in cases where we need to look at the definition of a referenced
schema from another schema.

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
sarangan12 pushed a commit to sarangan12/typespec that referenced this pull request Sep 16, 2024
…soft#4232)

Fixes microsoft#4152
Depends on microsoft#4216

This PR updates how tsp-openapi3 handles generating models for schemas
that use `allOf`.

Currently `allOf` is ignored unless there is only 1 member and that
member is a schema reference. In this scenario, the model extends the
single member.

This update now takes all of the schema `allOf` members into
consideration when generating a model.
- inline-schemas have their properties merged into the model's
properties
- schema references without a discriminator defined are spread into the
model
- if only 1 schema reference contains a discriminator, then the model
extends it, otherwise these schema references are spread as well.

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openapi3:converter Issues for @typespec/openapi3 openapi to typespec converter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsp-openapi3 - prevent collisions when same name is used in Open API 3 components categories.
4 participants