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

[openapi] Adopt enum-driven visibility #5120

Closed
witemple-msft opened this issue Nov 14, 2024 · 4 comments · Fixed by #5416
Closed

[openapi] Adopt enum-driven visibility #5120

witemple-msft opened this issue Nov 14, 2024 · 4 comments · Fixed by #5416
Assignees
Labels
emitter:openapi3 Issues for @typespec/openapi3 emitter feature New feature or request needs-area triaged:core

Comments

@witemple-msft
Copy link
Member

This is a tracking issue for the OpenAPI library to adopt enum-driven visibility analysis APIs and implement any enhancements to the core typespec libraries that are required to support OpenAPI's use case.

See #4825

@markcowl
Copy link
Contributor

est: 8

@markcowl markcowl added emitter:openapi3 Issues for @typespec/openapi3 emitter feature New feature or request labels Nov 18, 2024
@wanlwanl wanlwanl self-assigned this Dec 5, 2024
@wanlwanl
Copy link
Member

Wait for: #5416

@witemple-msft
Copy link
Member Author

Hey @wanlwanl, I ended up taking this as part of the related issue for the HTTP core. There won't be anything left to do here after #5416 is merged, as OpenAPI doesn't do much with visibility outside of what the HTTP core does for it.

@wanlwanl
Copy link
Member

Great! Thanks for the reminder.

github-merge-queue bot pushed a commit that referenced this issue Jan 22, 2025
This PR brings the core enum-driven visibility system to parity with the
existing visibility concepts and uses its new analysis methods in the
HTTP and OpenAPI libraries.

- Added lifecycle visibility modifiers for `Delete` and `Query` phases.
`Delete` applies when a resource is passed as an argument to a DELETE
operation. `Query` applies when a resource is passed as an argument to a
GET or HEAD operation.
- Added lifecycle transform templates for Delete and Query.
- Made the logic of applying a visibility filter a little bit more
robust. It will consider the `any` constraint of the filter to be
satisfied if it is missing OR if some modifier in the constraint is
present. If the `any` constraint is present, but empty, it will be
unsatisfiable.
- Deprecated `getParameterVisibility` and `getReturnTypeVisibility` in
favor of `getParameterVisibilityFilter` and
`getReturnTypeVisibilityFilter` which return VisibilityFilter objects
instead of `string[]`. Like the modifier analysis methods, the filter
versions of these methods are infallible.
- `getParameterVisibilityFilter` and `getReturnTypeVisibilityFilter`
accept a `VisibilityProvider` object that provides default visibility
filters for parameters and return types if the parameter/returnType
visibilities are not set explicitly. TypeSpec core exports an
`EmptyProvider` that always return filters with no constraints, suitable
as a default. HTTP exports `HttpVisibilityProvider`, which must be used
when working in HTTP, as it provides the HTTP-specific default
visibility logic that determines visibility by verb.
- `HttpVisibilityProvider` can be instantiated with either an `HttpVerb`
directly, in which case that exact verb will be used;
`HttpOperationOptions`, in which case the verbSelector will be used if
present, otherwise the fallback logic will be used; or no argument in
which case the fallback logic will always be used. This gives it parity
with `getHttpOperation`.
- HTTP now marshals conversions between core visibility and HTTP
`Visibility` through visibility filters instead of visibility string
arrays. The HTTP Visibility concept is unchanged.
- Used new forms of visibility analysis APIs in openapi to detect
read-only properties.
- ~~Fixed a bug in http-server-csharp that was surfaced by these changes
where a visibility constraint `Visibility.Create & Visibility.Update`
was provided, resulting in Visibility.None and properties being made
invisible, affecting effective type calculation for anonymous models.~~
(invalidated by merge)
- Fixed a bug in which `hasVisibility` and `getVisibilityForClass` were
improperly initializing the visibility state. This didn't affect the new
APIs, but it would cause the state of the legacy APIs to be corrupted if
visibility is queried from the new APIs before the legacy APIs.

Closes #5119
Closes #5120

---------

Co-authored-by: Will Temple <[email protected]>
Co-authored-by: Mark Cowlishaw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:openapi3 Issues for @typespec/openapi3 emitter feature New feature or request needs-area triaged:core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants