Skip to content
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

ISSUE-1131: Added diagnostic reporting recursive concatenation of dia… #1245

Merged
merged 3 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,48 @@ public class OpenApiDiagnostic : IDiagnostic
/// Open API specification version of the document parsed.
/// </summary>
public OpenApiSpecVersion SpecificationVersion { get; set; }

/// <summary>
/// Append another set of diagnostic Errors and Warnings to this one, this may be appended from another external
/// document's parsing and we want to indicate which file it originated from.
/// </summary>
/// <param name="diagnosticToAdd">The diagnostic instance of which the errors and warnings are to be appended to this diagnostic's</param>
/// <param name="fileNameToAdd">The originating file of the diagnostic to be appended, this is prefixed to each error and warning to indicate the originating file</param>
public void AppendDiagnostic(OpenApiDiagnostic diagnosticToAdd, string fileNameToAdd = null)
{
var fileNameIsSupplied = !string.IsNullOrEmpty(fileNameToAdd);
foreach (var err in diagnosticToAdd.Errors)
{
var errMsgWithFileName = fileNameIsSupplied ? $"[File: {fileNameToAdd}] {err.Message}" : err.Message;
Errors.Add(new OpenApiError(err.Pointer, errMsgWithFileName));
}
foreach (var warn in diagnosticToAdd.Warnings)
baywet marked this conversation as resolved.
Show resolved Hide resolved
{
var warnMsgWithFileName = fileNameIsSupplied ? $"[File: {fileNameToAdd}] {warn.Message}" : warn.Message;
Warnings.Add(new OpenApiError(warn.Pointer, warnMsgWithFileName));
}
}
}
}

/// <summary>
/// Extension class for IList to add the Method "AddRange" used above
/// </summary>
internal static class IDiagnosticExtensions
{
/// <summary>
/// Extension method for IList so that another list can be added to the current list.
/// </summary>
/// <param name="collection"></param>
/// <param name="enumerable"></param>
/// <typeparam name="T"></typeparam>
internal static void AddRange<T>(this ICollection<T> collection, IEnumerable<T> enumerable)
{
if (collection is null || enumerable is null) return;

foreach (var cur in enumerable)
darrelmiller marked this conversation as resolved.
Show resolved Hide resolved
{
collection.Add(cur);
}
}
}
12 changes: 9 additions & 3 deletions src/Microsoft.OpenApi.Readers/OpenApiYamlDocumentReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,13 @@ public async Task<ReadResult> ReadAsync(YamlDocument input, CancellationToken ca

if (_settings.LoadExternalRefs)
{
await LoadExternalRefs(document, cancellationToken);
var diagnosticExternalRefs = await LoadExternalRefs(document, cancellationToken);
// Merge diagnostics of external reference
if (diagnosticExternalRefs != null)
{
diagnostic.Errors.AddRange(diagnosticExternalRefs.Errors);
diagnostic.Warnings.AddRange(diagnosticExternalRefs.Warnings);
}
}

ResolveReferences(diagnostic, document);
Expand Down Expand Up @@ -133,15 +139,15 @@ public async Task<ReadResult> ReadAsync(YamlDocument input, CancellationToken ca
};
}

private async Task LoadExternalRefs(OpenApiDocument document, CancellationToken cancellationToken)
private async Task<OpenApiDiagnostic> LoadExternalRefs(OpenApiDocument document, CancellationToken cancellationToken)
{
// Create workspace for all documents to live in.
var openApiWorkSpace = new OpenApiWorkspace();

// Load this root document into the workspace
var streamLoader = new DefaultStreamLoader(_settings.BaseUrl);
var workspaceLoader = new OpenApiWorkspaceLoader(openApiWorkSpace, _settings.CustomExternalLoader ?? streamLoader, _settings);
await workspaceLoader.LoadAsync(new OpenApiReference() { ExternalResource = "/" }, document, cancellationToken);
return await workspaceLoader.LoadAsync(new OpenApiReference() { ExternalResource = "/" }, document, cancellationToken);
}

private void ResolveReferences(OpenApiDiagnostic diagnostic, OpenApiDocument document)
Expand Down
22 changes: 19 additions & 3 deletions src/Microsoft.OpenApi.Readers/Services/OpenApiWorkspaceLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public OpenApiWorkspaceLoader(OpenApiWorkspace workspace, IStreamLoader loader,
_readerSettings = readerSettings;
}

internal async Task LoadAsync(OpenApiReference reference, OpenApiDocument document, CancellationToken cancellationToken)
internal async Task<OpenApiDiagnostic> LoadAsync(OpenApiReference reference, OpenApiDocument document, CancellationToken cancellationToken, OpenApiDiagnostic diagnostic = null)
{
_workspace.AddDocument(reference.ExternalResource, document);
document.Workspace = _workspace;
Expand All @@ -36,17 +36,33 @@ internal async Task LoadAsync(OpenApiReference reference, OpenApiDocument docume

var reader = new OpenApiStreamReader(_readerSettings);

if (diagnostic is null)
{
diagnostic = new OpenApiDiagnostic();
}

// Walk references
foreach (var item in referenceCollector.References)
{
// If not already in workspace, load it and process references
if (!_workspace.Contains(item.ExternalResource))
{
var input = await _loader.LoadAsync(new Uri(item.ExternalResource, UriKind.RelativeOrAbsolute));
var result = await reader.ReadAsync(input, cancellationToken); // TODO merge diagnostics
await LoadAsync(item, result.OpenApiDocument, cancellationToken);
var result = await reader.ReadAsync(input, cancellationToken);
// Merge diagnostics
if (result.OpenApiDiagnostic != null)
{
diagnostic.AppendDiagnostic(result.OpenApiDiagnostic, item.ExternalResource);
}
if (result.OpenApiDocument != null)
{
var loadDiagnostic = await LoadAsync(item, result.OpenApiDocument, cancellationToken, diagnostic);
diagnostic = loadDiagnostic;
}
}
}

return diagnostic;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@
<AssemblyOriginatorKeyFile>..\..\src\Microsoft.OpenApi.snk</AssemblyOriginatorKeyFile>
</PropertyGroup>
<ItemGroup>
<None Remove="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoMain.yaml" />
<None Remove="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoReference.yaml" />
<None Remove="V2Tests\Samples\ComponentRootReference.json" />
<None Remove="V2Tests\Samples\OpenApiPathItem\pathItemWithBodyPathParameter.yaml" />
<None Remove="V2Tests\Samples\OpenApiPathItem\pathItemWithFormDataPathParameter.yaml" />
<None Remove="V3Tests\Samples\OpenApiWorkspace\TodoComponents.yaml" />
<None Remove="V3Tests\Samples\OpenApiWorkspace\TodoMain.yaml" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoMain.yaml" />
<EmbeddedResource Include="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoReference.yaml" />
<EmbeddedResource Include="OpenApiReaderTests\Samples\unsupported.v1.yaml">
<CopyToOutputDirectory>Never</CopyToOutputDirectory>
</EmbeddedResource>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System.Collections.Generic;
using System.Threading.Tasks;
using System;
using FluentAssertions;
using Microsoft.OpenApi.Exceptions;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Readers.Tests.OpenApiWorkspaceTests;
using Xunit;
using Microsoft.OpenApi.Readers.Interface;
using System.IO;

namespace Microsoft.OpenApi.Readers.Tests.OpenApiReaderTests
{
Expand Down Expand Up @@ -32,5 +40,45 @@ public void DetectedSpecificationVersionShouldBeV3_0()
diagnostic.SpecificationVersion.Should().Be(OpenApiSpecVersion.OpenApi3_0);
}
}

[Fact]
public async Task DiagnosticReportMergedForExternalReference()
{
// Create a reader that will resolve all references
var reader = new OpenApiStreamReader(new OpenApiReaderSettings()
{
LoadExternalRefs = true,
CustomExternalLoader = new ResourceLoader(),
BaseUrl = new Uri("fie://c:\\")
});

ReadResult result;
using (var stream = Resources.GetStream("OpenApiReaderTests/Samples/OpenApiDiagnosticReportMerged/TodoMain.yaml"))
{
result = await reader.ReadAsync(stream);
}

Assert.NotNull(result);
Assert.NotNull(result.OpenApiDocument.Workspace);
Assert.True(result.OpenApiDocument.Workspace.Contains("TodoReference.yaml"));
result.OpenApiDiagnostic.Errors.Should().BeEquivalentTo(new List<OpenApiError> {
new OpenApiError( new OpenApiException("[File: ./TodoReference.yaml] Invalid Reference identifier 'object-not-existing'.")) });

}
}

public class ResourceLoader : IStreamLoader
{
public Stream Load(Uri uri)
{
return null;
}

public Task<Stream> LoadAsync(Uri uri)
{
var path = new Uri(new Uri("http://example.org/OpenApiReaderTests/Samples/OpenApiDiagnosticReportMerged/"), uri).AbsolutePath;
path = path.Substring(1); // remove leading slash
return Task.FromResult(Resources.GetStream(path));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
openapi: 3.0.1
info:
title: Example using a remote reference
version: 1.0.0
paths:
"/todos":
get:
parameters:
- $ref: ./TodoReference.yaml#/components/parameters/filter
responses:
200:
description: Ok
content:
application/json:
schema:
$ref: ./TodoReference.yaml#/components/schemas/todo
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
openapi: 3.0.1
info:
title: Components for the todo app
version: 1.0.0
paths: {}
components:
parameters:
filter:
name: filter
in: query
schema:
type: string
schemas:
todo:
type: object
allOf:
- $ref: "#/components/schemas/entity"
- $ref: "#/components/schemas/object-not-existing"
properties:
subject:
type: string
entity:
type: object
properties:
id:
type:string