[Darrel (darrelmiller)]

Reverts #1491 due to issues identified in #1506

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[Vincent Biret (baywet)]

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.

[dldl-cmd]

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: basic

OpenApiReadingWritingTests.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: [ ]

[Maggie Kimani (MaggieKimani1)]

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.
Ultimately the main issue being addressed is should we support defining external references at the root of the component?
For instance using the example below:

File 1: ExampleSchema.yaml:

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/ExampleSchema

File 2. CommonSchema.yaml

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: string

The CommonSchema.yaml file doesn't get loaded into the OpenApiWorkSpace due to the same issue highlighted on this comment https://github.com/microsoft/OpenAPI.NET/issues/1490#issuecomment-1838428614
We should be able to support visiting remote references that exist directly in a component hence the fix #1491 shouldn't be reverted.

[Darrel (darrelmiller)]

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: string

I am not suggesting this is particularly useful, but this is a scenario that we used to support.

[Darrel (darrelmiller)]

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.