WIP: Microsoft.OpenApi v2.3.0 with OpenAPI v3.1 Support #269
Conversation
sergey-tihon
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades Microsoft.OpenApi from v2.x to v2.3.0 to add OpenAPI v3.1 support, removing the separate Microsoft.OpenApi.Readers dependency and updating various dependencies.
Key changes:
- Updated OpenAPI library dependencies and added OpenAPI v3.1 support
- Refactored code to use new OpenAPI library interfaces and APIs
- Modernized F# syntax with abbreviated lambda expressions
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| paket.dependencies | Updated FSharp.Core, System.Text.Json, and OpenAPI packages; added Swashbuckle preview |
| src/SwaggerProvider.DesignTime/v3/DefinitionCompiler.fs | Adapted to new IOpenApiSchema interface and JsonSchemaType enum |
| src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs | Updated to use IOpenApiPathItem and IOpenApiParameter interfaces |
| tests/Swashbuckle.WebApi.Server/Startup.fs | Replaced SerializeAsV2 with OpenApiVersion property |
| tests/SwaggerProvider.Tests/Schema.Parser.Tests.fs | Updated OpenAPI document parsing to use new API |
| src/SwaggerProvider.DesignTime/Provider.OpenApiClient.fs | Updated schema reading to use new OpenAPI parsing methods |
| Multiple files | Applied F# syntax modernization and version updates |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ns.ReleaseNameReservation tyName | ||
| ty | ||
| | _ -> failwithf $"Cannot compile object '%s{tyName}' based on unresolved reference '{schemaObj.Reference.ReferenceV3}'" | ||
| | _ -> failwithf $"Cannot compile object '%s{tyName}' based on unresolved reference '{schemaRef.Reference.Id}'" |
There was a problem hiding this comment.
The pattern matching assumes all schema references are OpenApiSchemaReference types, but the original code handled UnresolvedReference property checks. This change may not handle all reference scenarios correctly and could cause runtime type cast exceptions if the schema object is not actually an OpenApiSchemaReference.
| | _ -> failwithf $"Cannot compile object '%s{tyName}' based on unresolved reference '{schemaRef.Reference.Id}'" | |
| | _ -> | |
| let refId = | |
| if not (isNull (box schemaObj)) then | |
| let t = schemaObj.GetType() | |
| let prop = t.GetProperty("Reference") | |
| if not (isNull prop) then | |
| let refVal = prop.GetValue(schemaObj, null) | |
| if not (isNull refVal) then | |
| let idProp = refVal.GetType().GetProperty("Id") | |
| if not (isNull idProp) then | |
| let idVal = idProp.GetValue(refVal, null) | |
| if not (isNull idVal) then idVal.ToString() else null | |
| else null | |
| else null | |
| else null | |
| else null | |
| let refMsg = if String.IsNullOrEmpty(refId) then "(unknown reference)" else refId | |
| failwithf $"Cannot compile object '%s{tyName}' based on unresolved reference '%s{refMsg}' (actual type: %s{if isNull schemaObj then "null" else schemaObj.GetType().FullName})" |
Copilot uses AI. Check for mistakes.
| // if path.Value.UnresolvedReference then | ||
| // failwith | ||
| // $"TP does not support unresolved paths / external references. Path '%s{path.Key}' refer to '%s{path.Value.Reference.ReferenceV3}'" |
There was a problem hiding this comment.
Commented out unresolved reference validation without providing an alternative mechanism. This removes important error handling that prevents processing of unresolved external references, which could lead to runtime failures.
| // if path.Value.UnresolvedReference then | |
| // failwith | |
| // $"TP does not support unresolved paths / external references. Path '%s{path.Key}' refer to '%s{path.Value.Reference.ReferenceV3}'" | |
| if path.Value.UnresolvedReference then | |
| failwith | |
| $"TP does not support unresolved paths / external references. Path '%s{path.Key}' refer to '%s{path.Value.Reference.ReferenceV3}'" |
Copilot uses AI. Check for mistakes.
| // If schemaObj.Type is missing, but allOf is present and first allOf subschema has a type, use that | ||
| if | ||
| not schemaObj.Type.HasValue | ||
| && not(isNull schemaObj.AllOf) | ||
| && schemaObj.AllOf.Count > 0 | ||
| then | ||
| let firstAllOf = schemaObj.AllOf.[0] | ||
|
|
||
| if not(isNull firstAllOf) && firstAllOf.Type.HasValue then | ||
| Some firstAllOf.Type.Value | ||
| else | ||
| None | ||
| else if schemaObj.Type.HasValue then | ||
| Some schemaObj.Type.Value | ||
| else | ||
| None |
There was a problem hiding this comment.
The code accesses schemaObj.AllOf.[0] without null checking schemaObj.AllOf first. While there's a count check, if schemaObj.AllOf is null, Count property access will throw a NullReferenceException.
Copilot uses AI. Check for mistakes.
1 participant
Uh oh!
There was an error while loading. Please reload this page.