Revert "Fix unresolved references in OpenApiWalker"#1507
Revert "Fix unresolved references in OpenApiWalker"#1507darrelmiller wants to merge 1 commit intovnextmicrosoft/OpenAPI.NET:vnextfrom revert-1491-openapiwalker_referencesmicrosoft/OpenAPI.NET:revert-1491-openapiwalker_referencesCopy head branch name to clipboard
Conversation
|
|
I'd like to see the unit tests outlined in #1506 added as part of the revert if possible. This way if the author of the PR wants to come back with an updated PR, it'll prevent any regression. |
|
I tried to write a test for the found issue, but it seems, that it also does not work when the fix was not applied. I tried the following based of commit d1cf89f. Therefore I think reverting the fix will not be beneficial. securitySchemeReference.yaml openapi: 3.0
info:
version: 2.0.0
title: hello world
paths:
/:
get:
responses:
'200':
description: OK
security:
- ReferenceObject: []
components:
securitySchemes:
ReferenceObject:
$ref: 'external.yaml#/components/securitySchemes/RealObject'
RealObject:
type: http
scheme: basicOpenApiReadingWritingTests.cs using System.Globalization;
using System.IO;
using System.Text;
using FluentAssertions;
using Microsoft.OpenApi.Writers;
using Xunit;
namespace Microsoft.OpenApi.Readers.Tests.V3Tests
{
[Collection("DefaultSettings")]
public class OpenApiReadingWritingTests
{
private const string SampleFolderPath = "V3Tests/Samples/OpenApiReadingWriting/";
[Fact]
public void ReadingAndWritingShouldResultInTheSameDocument()
{
using var stream = Resources.GetStream(Path.Combine(SampleFolderPath, "securitySchemeReference.yaml"));
var streamReader = new StreamReader(stream, Encoding.UTF8);
var sampleDocument = streamReader.ReadToEnd();
stream.Seek(0, SeekOrigin.Begin);
var openApiDoc = new OpenApiStreamReader().Read(stream, out var diagnostic);
var memoryStream = new MemoryStream();
var streamWriter = new FormattingStreamWriter(memoryStream, CultureInfo.InvariantCulture);
var writer = new OpenApiYamlWriter(streamWriter);
openApiDoc.SerializeAsV3(writer);
writer.Flush();
var writtenDocument = Encoding.UTF8.GetString(memoryStream.ToArray());
// Assert
diagnostic.Should().BeEquivalentTo(new OpenApiDiagnostic
{
SpecificationVersion = OpenApiSpecVersion.OpenApi3_0
});
sampleDocument.Should().BeEquivalentTo(writtenDocument);
}
}
}Results in a file with the following content: openapi: 3.0.1
info:
title: hello world
version: 2.0.0
paths:
/:
get:
responses:
'200':
description: OK
components:
securitySchemes:
ReferenceObject:
external.yaml#/components/securitySchemes/RealObject:
RealObject:
type: http
scheme: basic
security:
- external.yaml#/components/securitySchemes/RealObject: [ ] |
|
I have also tested the concerns highlighted on #1506 and found that they're unrelated to the fix done by #1491 and therefore, can be addressed separately. File 1: openapi: '3.0.2'
info:
title: Example API
description: Example for OpenApiWalker reference problem
version: 1.0.0
servers:
- url: 'https://localhost'
paths:
/example:
get:
summary: "Test"
responses:
'200':
description: Example response
content:
application/json:
schema:
$ref: '#/components/schemas/ExampleSchema'
components:
schemas:
ExampleSchema:
$ref: CommonSchema.yaml#/components/schemas/ExampleSchemaFile 2. openapi: '3.0.2'
info:
title: Common definitions
description: Common definitions for OpenApiWalker reference problem
version: 1.0.0
paths: {}
components:
schemas:
ExampleSchema:
type: object
properties:
PropA:
type: string
PropB:
type: stringThe |
|
I do not believe it is possible to properly support reference objects at the root of a component with the current design. I believe this PR reintroduces the condition that allows the following to get rendered properly. openapi: '3.0.2'
info:
title: Example API
description: Example for OpenApiWalker reference problem
version: 1.0.0
components:
schemas:
schemaA:
$ref: "#/components/schemas/schemaB"
schemaB:
type: stringI am not suggesting this is particularly useful, but this is a scenario that we used to support. |
|
After review, we decided that the bug that this PR introduces where we can't accurately write out a document that has a double reference is a lesser bug than not resolving a reference at the root of the component. Therefore we will close this PR to revert the change. |
Reverts #1491 due to issues identified in #1506