-
Notifications
You must be signed in to change notification settings - Fork 9.2k
fix: adds version 3.0.4 and 3.1.1 to supported versions #10247
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge them
@char0n since we've worked together on the spec and since you've contributed a lot on this project, I thought maybe you could help get this merged? |
Would be nice to get this fixed quickly. |
Any news ? |
Any word on what the hold up is? |
@baywet - Maybe merge base branch in so this can continue to progress? |
@richard-collette-precisely done! Let me know if anything else is needed to get this merged |
@baywet - We need a reviewer. Unfortunately I am not one of them. Thank you for the PR though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approve
I am confused, already reviewed, why not merge? |
@char0n would we be able to get a review from you on this? Not sure who all the maintainers are. |
This will continue to be a problem for each new release. They are currently working on 3.2 minor release and they are also considering to move to major.minor only, without the patch release. The entire discussion about major.minor: OAI/OpenAPI-Specification#4233 3.2 release WIP: https://github.com/OAI/OpenAPI-Specification/milestone/12 3.1.2 release WIP: https://github.com/OAI/OpenAPI-Specification/milestone/17 |
But at this time, those things do not exist and the UI cannot purport to support something that it may not, so the idea of refactoring belongs to another issue, not this one. This one addresses the ground truth, today. 6.5 weeks and this hasn't been merged, is mind blowing. This PR is on the verge of just being a documentation update. |
Still waiting... |
My team and I are quite dependent on this PR being merged. What's the hold-up? |
Help, @char0n, you're our only hope |
@alec-petersen I even sent an email to @char0n but he didn't care :D |
@frantuma @ponelat @MiloszTarka Would we be able to get a review for this PR? 🙏 Thank you! |
Signed-off-by: Vincent Biret <[email protected]>
Signed-off-by: Vincent Biret <[email protected]>
Signed-off-by: Vincent Biret <[email protected]>
Shouldn't the patch version be ignored at all by SwaggerUI to be less error prone for future patches, instead of adding new versions one by one to the validation? The OpenAPI Specification is versioned using a major.minor.patch versioning scheme. The major.minor portion of the version string (for example 3.1) SHALL designate the OAS feature set. .patch versions address errors in, or provide clarifications to, this document, not the feature set. Tooling which supports OAS 3.1 SHOULD be compatible with all OAS 3.1.* versions. The patch version SHOULD NOT be considered by tooling, making no distinction between 3.1.0 and 3.1.1 for example. e.g.
instead of
btw, here is a quickfix for those who don't want to wait: internal class FixOpenApiVersionMiddleware {
private readonly RequestDelegate _next;
public FixOpenApiVersionMiddleware(RequestDelegate next) {
_next = next;
}
public async Task InvokeAsync(HttpContext context) {
if (!Regex.IsMatch(context.Request.Path, @".*swagger.*\.(json|ya?ml)")) {
await _next(context);
return;
}
var body = string.Empty;
var responseBody = new MemoryStream();
var originalBodyStream = context.Response.Body;
context.Response.Body = responseBody;
await _next(context);
context.Response.Body = originalBodyStream;
responseBody.Seek(0, SeekOrigin.Begin);
body = await new StreamReader(responseBody).ReadToEndAsync();
await context.Response.WriteAsync(Regex.Replace(body, @"openapi: (\d+)\.(\d+)\.\d+", @"openapi: $1.$2.0"));
}
} and then in Program.cs app.UseMiddleware<FixOpenApiVersionMiddleware>();
[...]
app.UseSwagger();
app.UseSwaggerUI(); |
Signed-off-by: Vincent Biret <[email protected]>
Co-authored-by: Vladimír Gorej <[email protected]>
@char0n thanks for the review, just pushed the changes. Let me know what you think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, let's see if all the tests pass.
I must admit, this is funny ;] |
@baywet thanks for the contribution! |
@bve-wd yes you are right. From historical perspective, OpenAPI editors have stipulated that 3.0.3 was the last version of 3.0 line. Now we have 3.0.4 and they say again 3.0.4 is the last one. We already did changes to other Swagger JavaScript projects to be forward compatible, please see #10247 (comment). I've requested change in this PR to be aligned with this. OpenAPI 3.1 matching is already forward compatible for some time: swagger-ui/src/core/plugins/oas31/fn.js Line 10 in df9aadf
|
Release ETA - imminent. I'll need to follow up with some additional changes. |
Thanks @char0n ❤️ |
NOTE: It was technically possible to resolve this issue with a simple plugin: const OpenAPI304Plugin = () => {
return {
statePlugins: {
spec: {
selectors: {
isOAS30: (jsSpec) => {
const oasVersion = jsSpec.get("json").get("openapi")
return typeof oasVersion === "string" && /^3\.0\.(?:[1-9]\d*|0)$/.test(oasVersion)
}
}
}
}
}
}
const ui = SwaggerUIBundle({
url: "https://gist.githubusercontent.com/char0n/963882a03e2da5981dec6e9675909279/raw/15ac032083312ab0e29f4210f6e602c3a5ee9389/openapi-3-0-4.json",
dom_id: "#swagger-ui",
presets: [
SwaggerUIBundle.presets.apis,
SwaggerUIStandalonePreset
],
plugins: [
SwaggerUIBundle.plugins.DownloadUrl,
OpenAPI304Plugin,
],
// requestSnippetsEnabled: true,
layout: "StandaloneLayout"
}) |
The change has been released as https://github.com/swagger-api/swagger-ui/releases/tag/v5.19.0. Thank you everybody! |
Wow, I just loaded a project that was working just fine last week and no code has changed in that project but VS 2022 was updated from 17.3.2 to 17.3.3 and boom, "Unable to render this definition..." error
This solved the issue. I honestly don't know why |
Something must have updated the version of Microsoft.OpenApi to 1.6.23 in your project so you hit the state that exposes this issue. |
@martincostello Probably... |
Description
adds version 3.0.4 to the list of supported versions
3.0.4 is a maintenance release of the specification that does not change any of the functional aspects.
https://www.openapis.org/blog/2024/10/25/announcing-openapi-specification-patch-releases
Motivation and Context
fixes microsoft/OpenAPI.NET#2030
How Has This Been Tested?
This has NOT been tested beyond the affected unit tests. However the changes are straightforward and minor, I do not expect any side effects.
I'd appreciate if anyone could take the time to pull and test this just for sanity.
Screenshots (if appropriate):
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests