Enhance server creation logic and add unit tests#2524
Closed
Mahdigln wants to merge 1 commit intomicrosoft:mainfrom
Closed
Enhance server creation logic and add unit tests#2524Mahdigln wants to merge 1 commit intomicrosoft:mainfrom
Mahdigln wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Updated the `MakeServers` method to include a check for `defaultUrl` when creating servers. The method now returns early if all relevant parameters are null, and the host assignment from `defaultUrl` now preserves the port. Added new unit tests in `OpenApiServerTests.cs` to ensure that base URLs with ports are correctly handled. Tests cover scenarios for base URLs with a port, with a port and path, and with a non-standard port.
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix port omission from BaseUrl in V2 server deserialization
Problem
When setting a
BaseUrlwith a non-standard port and the OpenAPI V2 document doesn't contain any server definitions, the port number was being omitted from the default server URL.Example:
BaseUrl = "http://demo.testfire.net:8080""http://demo.testfire.net"❌"http://demo.testfire.net:8080"✅Root Cause
The
MakeServersmethod inOpenApiDocumentDeserializer.cswas usingUriComponents.NormalizedHostwhich only extracts the hostname without the port number.Solution
Changed
UriComponents.NormalizedHosttoUriComponents.Host | UriComponents.Portto preserve port information when extracting host details from the BaseUrl. This approach is consistent with the existing pattern used inOpenApiDocument.WriteHostInfoV2.Additionally, fixed the early return logic to allow server creation from BaseUrl even when no host, basePath, or schemes are defined in the document.
Changes
src/Microsoft.OpenApi.Readers/V2/OpenApiDocumentDeserializer.cs:UriComponents.NormalizedHosttoUriComponents.Host | UriComponents.PortdefaultUrl != nulltest/Microsoft.OpenApi.Readers.Tests/V2Tests/OpenApiServerTests.cs:Testing
Fixes #1294