Add OpenAPI 3.2 stream emission support with itemSchema#8888
Conversation
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
|
@copilot cleanup the console logs you've added in this pull request |
|
All changed packages have been documented.
Show changes
|
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
Removed all debug console.log statements from openapi.ts and test files in commit 559641d. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
SSEStream accepted unions without `@events` decorator, generating
incorrect schemas. This validation ensures proper event stream
definitions at compile time.
## Changes
- **Added validation** in `packages/sse/src/validate.ts`:
- Traverse all models during validation phase
- Check streams with `text/event-stream` content-type
- Verify streamOf type is a union with `@events` decorator
- Report `sse-stream-union-not-events` diagnostic on failure
- **Added diagnostic** `sse-stream-union-not-events` in
`packages/sse/src/lib.ts`
- **Added tests** covering invalid/valid SSEStream usage and HttpStream
with SSE content-type
## Example
This now fails validation:
```typespec
union BasicUnion {
userconnect: UserConnect,
}
op subscribe(): SSEStream<BasicUnion>; // Error: union must have @events
```
This compiles:
```typespec
@events
union BasicUnion {
userconnect: UserConnect,
}
op subscribe(): SSEStream<BasicUnion>; // OK
```
Fixes #8888
> [!WARNING]
>
> <details>
> <summary>Firewall rules blocked me from connecting to one or more
addresses (expand for details)</summary>
>
> #### I tried to connect to the following addresses, but was blocked by
firewall rules:
>
> - `telemetry.astro.build`
> - Triggering command: `node
/home/REDACTED/work/typespec/typespec/website/node_modules/.bin/../astro/astro.js
build` (dns block)
>
> If you need me to access, download, or install something from one of
these locations, you can either:
>
> - Configure [Actions setup
steps](https://gh.io/copilot/actions-setup-steps) to set up my
environment, which run before the firewall is enabled
> - Add the appropriate URLs or hosts to the custom allowlist in this
repository's [Copilot coding agent
settings](https://github.com/microsoft/typespec/settings/copilot/coding_agent)
(admins only)
>
> </details>
<!-- START COPILOT CODING AGENT SUFFIX -->
<details>
<summary>Original prompt</summary>
>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>SSEStream should NOT accept a union model without the
events decorator</issue_title>
> <issue_description>This definition
>
> ```ts
> import "@typespec/streams";
> import "@typespec/sse";
> import "@typespec/events";
>
> using SSE;
>
>
> model UserConnect {
> name: string;
> }
>
> union BasicUnion {
> userconnect: UserConnect,
> }
> op subscribe(): SSEStream<BasicUnion>;
> ```
>
> Builds but generates the wrong schema (instead of item schema with all
the right properties)
>
> ```ts
> import "@typespec/streams";
> import "@typespec/sse";
> import "@typespec/events";
>
> using SSE;
>
>
> model UserConnect {
> name: string;
> }
>
> @TypeSpec.Events.events
> union BasicUnion {
> userconnect: UserConnect,
> }
> op subscribe(): SSEStream<BasicUnion>;
> ```
>
> Generates the correct itemSchema.
>
> Ultimately this is not a responsibility of the OpenAPI3 emitter to
check for this, but rather the SSE library should fail building when
being passed a union model as a generic type parameter value when that
union model DOES NOT have the required decorator.
> This will be handled in a separate issue/pull request
>
> _Originally posted by @baywet in
#8888 (comment)
> </issue_description>
>
> <agent_instructions>before committing any file, run `npx prettier
--write filePath` to ensure they are formatted correctly.
> once you're done with the changes, run `npx @chronus/chronus add` and
select bugfix to add an entry in the changelog</agent_instructions>
>
> ## Comments on the Issue (you are @copilot in this section)
>
> <comments>
> <comment_new><author>@timotheeguerin</author><body>
> Also note that as `SSEStream` is just defined as `HttpStream<Type,
"text/event-stream">;`
>
> that also might need to validate when `text/event-stream` is
used</body></comment_new>
> </comments>
>
</details>
- Fixes #8950
<!-- START COPILOT CODING AGENT TIPS -->
---
💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.
---------
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: baywet <7905502+baywet@users.noreply.github.com>
Co-authored-by: Vincent Biret <vibiret@microsoft.com>
Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
baywet
left a comment
There was a problem hiding this comment.
leaving some comments after testing things locally
| } else if (content.schema && "$ref" in content.schema) { | ||
| // Fallback: use schema directly if no itemSchema | ||
| const eventUnionType = context.generateTypeFromRefableSchema( | ||
| content.schema, | ||
| operationScope, | ||
| ); | ||
| return `SSEStream<${eventUnionType}>`; |
There was a problem hiding this comment.
this is wrong, we shouldn't fallback on the main schema for streaming as schema is bounded and that does not work with streams which don't have collections bounds
| return contents.map(([mediaType, content]) => { | ||
| // Special handling for Server-Sent Events | ||
| if (mediaType === "text/event-stream") { | ||
| context.markSSEUsage(); |
There was a problem hiding this comment.
this should be moved down to the condition when we detected a valid case
| path: OpenAPI3PathItem | OpenAPIPathItem3_2, | ||
| context: Context, | ||
| ): void { | ||
| if (!("get" in path && path.get && path.get.responses)) { |
There was a problem hiding this comment.
this is wrong, a streaming response could come from any operation that MAY return a response body
follow up to microsoft#8888 --------- Signed-off-by: Vincent Biret <vibiret@microsoft.com> Co-authored-by: Timothee Guerin <tiguerin@microsoft.com>
follow up to #8888 --------- Signed-off-by: Vincent Biret <vibiret@microsoft.com>
follow up to #8888 --------- Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Implementation Plan for Stream Support in OpenAPI 3.2
This PR implements support for streaming responses (including Server-Sent Events) in OpenAPI 3.2 emission and import using the
itemSchemafield.Checklist
itemSchemasupport to OpenAPI 3.2 MediaType definition in types.tsChanges Made
Type Definitions:
itemSchemaproperty toOpenAPIMediaType3_2for streaming event schema definitionsSSE Module (
sse-module.ts):xml-module.tspattern@typespec/streams.getStreamOfitemSchemawithoneOfvariants for@eventsunion membersOpenAPI Emitter Integration:
getBodyContentEntryto detect all stream responses viagetStreamMetadataitemSchemaHttpStream<ContentType, Type>, not just SSE streamsTesting:
Diagnostics:
Stream Support
This implementation supports any stream type in OpenAPI 3.2.0:
text/event-streamapplication/jsonlHttpStream<ContentType, Type>For OpenAPI 3.0/3.1, streams are emitted without
itemSchemaand a warning is generated.Original prompt
This section details on the original issue you should resolve
<issue_title>[Feat] OpenAPI - add support for SSE import and emission</issue_title>
<issue_description>### Clear and concise description of the problem
Now that we have support for OpenAPI 3.2.0 (#8828 ) both in emissions and in importing descriptions to TypeSpec, it'd be nice to have support for importing SSE (server-sent events).
@timotheeguerin and I spent the better part of an hour combing through multiple specifications to understand what needs to happen here, so strap in, this is going to be a long issue.
Relevant resources
Import
Scenario 1 : no terminal event
Given the following OpenAPI Response object.
I'd expect aresulting TypeSpec description looking like this.
Note that the terminal event is NOT present, this is ok because it's an invention of some APIs, and is not part of the WHATWG spec. It should remain optional.
Here a couple of things are worth noting:
@TypeSpec.Events.eventsdecorator is added to the union typeSSEStream<ChannelEvents>(instead of ChannelEvents)Scenario 2: with terminal events
Given the following OpenAPI Response object.