Skip to content

Commit c080552

Browse files
authored
Merge pull request #1245 from elize-vdr/ISSUE-1131
ISSUE-1131: Added diagnostic reporting recursive concatenation of dia…
2 parents 050dd51 + 88a0220 commit c080552

File tree

7 files changed

+165
-6
lines changed

7 files changed

+165
-6
lines changed

src/Microsoft.OpenApi.Readers/OpenApiDiagnostic.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,48 @@ public class OpenApiDiagnostic : IDiagnostic
2626
/// Open API specification version of the document parsed.
2727
/// </summary>
2828
public OpenApiSpecVersion SpecificationVersion { get; set; }
29+
30+
/// <summary>
31+
/// Append another set of diagnostic Errors and Warnings to this one, this may be appended from another external
32+
/// document's parsing and we want to indicate which file it originated from.
33+
/// </summary>
34+
/// <param name="diagnosticToAdd">The diagnostic instance of which the errors and warnings are to be appended to this diagnostic's</param>
35+
/// <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>
36+
public void AppendDiagnostic(OpenApiDiagnostic diagnosticToAdd, string fileNameToAdd = null)
37+
{
38+
var fileNameIsSupplied = !string.IsNullOrEmpty(fileNameToAdd);
39+
foreach (var err in diagnosticToAdd.Errors)
40+
{
41+
var errMsgWithFileName = fileNameIsSupplied ? $"[File: {fileNameToAdd}] {err.Message}" : err.Message;
42+
Errors.Add(new OpenApiError(err.Pointer, errMsgWithFileName));
43+
}
44+
foreach (var warn in diagnosticToAdd.Warnings)
45+
{
46+
var warnMsgWithFileName = fileNameIsSupplied ? $"[File: {fileNameToAdd}] {warn.Message}" : warn.Message;
47+
Warnings.Add(new OpenApiError(warn.Pointer, warnMsgWithFileName));
48+
}
49+
}
50+
}
51+
}
52+
53+
/// <summary>
54+
/// Extension class for IList to add the Method "AddRange" used above
55+
/// </summary>
56+
internal static class IDiagnosticExtensions
57+
{
58+
/// <summary>
59+
/// Extension method for IList so that another list can be added to the current list.
60+
/// </summary>
61+
/// <param name="collection"></param>
62+
/// <param name="enumerable"></param>
63+
/// <typeparam name="T"></typeparam>
64+
internal static void AddRange<T>(this ICollection<T> collection, IEnumerable<T> enumerable)
65+
{
66+
if (collection is null || enumerable is null) return;
67+
68+
foreach (var cur in enumerable)
69+
{
70+
collection.Add(cur);
71+
}
2972
}
3073
}

src/Microsoft.OpenApi.Readers/OpenApiYamlDocumentReader.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,13 @@ public async Task<ReadResult> ReadAsync(YamlDocument input, CancellationToken ca
102102

103103
if (_settings.LoadExternalRefs)
104104
{
105-
await LoadExternalRefs(document, cancellationToken);
105+
var diagnosticExternalRefs = await LoadExternalRefs(document, cancellationToken);
106+
// Merge diagnostics of external reference
107+
if (diagnosticExternalRefs != null)
108+
{
109+
diagnostic.Errors.AddRange(diagnosticExternalRefs.Errors);
110+
diagnostic.Warnings.AddRange(diagnosticExternalRefs.Warnings);
111+
}
106112
}
107113

108114
ResolveReferences(diagnostic, document);
@@ -133,15 +139,15 @@ public async Task<ReadResult> ReadAsync(YamlDocument input, CancellationToken ca
133139
};
134140
}
135141

136-
private async Task LoadExternalRefs(OpenApiDocument document, CancellationToken cancellationToken)
142+
private async Task<OpenApiDiagnostic> LoadExternalRefs(OpenApiDocument document, CancellationToken cancellationToken)
137143
{
138144
// Create workspace for all documents to live in.
139145
var openApiWorkSpace = new OpenApiWorkspace();
140146

141147
// Load this root document into the workspace
142148
var streamLoader = new DefaultStreamLoader(_settings.BaseUrl);
143149
var workspaceLoader = new OpenApiWorkspaceLoader(openApiWorkSpace, _settings.CustomExternalLoader ?? streamLoader, _settings);
144-
await workspaceLoader.LoadAsync(new OpenApiReference() { ExternalResource = "/" }, document, cancellationToken);
150+
return await workspaceLoader.LoadAsync(new OpenApiReference() { ExternalResource = "/" }, document, cancellationToken);
145151
}
146152

147153
private void ResolveReferences(OpenApiDiagnostic diagnostic, OpenApiDocument document)

src/Microsoft.OpenApi.Readers/Services/OpenApiWorkspaceLoader.cs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public OpenApiWorkspaceLoader(OpenApiWorkspace workspace, IStreamLoader loader,
2424
_readerSettings = readerSettings;
2525
}
2626

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

3737
var reader = new OpenApiStreamReader(_readerSettings);
3838

39+
if (diagnostic is null)
40+
{
41+
diagnostic = new OpenApiDiagnostic();
42+
}
43+
3944
// Walk references
4045
foreach (var item in referenceCollector.References)
4146
{
4247
// If not already in workspace, load it and process references
4348
if (!_workspace.Contains(item.ExternalResource))
4449
{
4550
var input = await _loader.LoadAsync(new Uri(item.ExternalResource, UriKind.RelativeOrAbsolute));
46-
var result = await reader.ReadAsync(input, cancellationToken); // TODO merge diagnostics
47-
await LoadAsync(item, result.OpenApiDocument, cancellationToken);
51+
var result = await reader.ReadAsync(input, cancellationToken);
52+
// Merge diagnostics
53+
if (result.OpenApiDiagnostic != null)
54+
{
55+
diagnostic.AppendDiagnostic(result.OpenApiDiagnostic, item.ExternalResource);
56+
}
57+
if (result.OpenApiDocument != null)
58+
{
59+
var loadDiagnostic = await LoadAsync(item, result.OpenApiDocument, cancellationToken, diagnostic);
60+
diagnostic = loadDiagnostic;
61+
}
4862
}
4963
}
64+
65+
return diagnostic;
5066
}
5167
}
5268
}

test/Microsoft.OpenApi.Readers.Tests/Microsoft.OpenApi.Readers.Tests.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,17 @@
1313
<AssemblyOriginatorKeyFile>..\..\src\Microsoft.OpenApi.snk</AssemblyOriginatorKeyFile>
1414
</PropertyGroup>
1515
<ItemGroup>
16+
<None Remove="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoMain.yaml" />
17+
<None Remove="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoReference.yaml" />
1618
<None Remove="V2Tests\Samples\ComponentRootReference.json" />
1719
<None Remove="V2Tests\Samples\OpenApiPathItem\pathItemWithBodyPathParameter.yaml" />
1820
<None Remove="V2Tests\Samples\OpenApiPathItem\pathItemWithFormDataPathParameter.yaml" />
1921
<None Remove="V3Tests\Samples\OpenApiWorkspace\TodoComponents.yaml" />
2022
<None Remove="V3Tests\Samples\OpenApiWorkspace\TodoMain.yaml" />
2123
</ItemGroup>
2224
<ItemGroup>
25+
<EmbeddedResource Include="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoMain.yaml" />
26+
<EmbeddedResource Include="OpenApiReaderTests\Samples\OpenApiDiagnosticReportMerged\TodoReference.yaml" />
2327
<EmbeddedResource Include="OpenApiReaderTests\Samples\unsupported.v1.yaml">
2428
<CopyToOutputDirectory>Never</CopyToOutputDirectory>
2529
</EmbeddedResource>

test/Microsoft.OpenApi.Readers.Tests/OpenApiReaderTests/OpenApiDiagnosticTests.cs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
33

4+
using System.Collections.Generic;
5+
using System.Threading.Tasks;
6+
using System;
47
using FluentAssertions;
8+
using Microsoft.OpenApi.Exceptions;
9+
using Microsoft.OpenApi.Models;
10+
using Microsoft.OpenApi.Readers.Tests.OpenApiWorkspaceTests;
511
using Xunit;
12+
using Microsoft.OpenApi.Readers.Interface;
13+
using System.IO;
614

715
namespace Microsoft.OpenApi.Readers.Tests.OpenApiReaderTests
816
{
@@ -32,5 +40,45 @@ public void DetectedSpecificationVersionShouldBeV3_0()
3240
diagnostic.SpecificationVersion.Should().Be(OpenApiSpecVersion.OpenApi3_0);
3341
}
3442
}
43+
44+
[Fact]
45+
public async Task DiagnosticReportMergedForExternalReference()
46+
{
47+
// Create a reader that will resolve all references
48+
var reader = new OpenApiStreamReader(new OpenApiReaderSettings()
49+
{
50+
LoadExternalRefs = true,
51+
CustomExternalLoader = new ResourceLoader(),
52+
BaseUrl = new Uri("fie://c:\\")
53+
});
54+
55+
ReadResult result;
56+
using (var stream = Resources.GetStream("OpenApiReaderTests/Samples/OpenApiDiagnosticReportMerged/TodoMain.yaml"))
57+
{
58+
result = await reader.ReadAsync(stream);
59+
}
60+
61+
Assert.NotNull(result);
62+
Assert.NotNull(result.OpenApiDocument.Workspace);
63+
Assert.True(result.OpenApiDocument.Workspace.Contains("TodoReference.yaml"));
64+
result.OpenApiDiagnostic.Errors.Should().BeEquivalentTo(new List<OpenApiError> {
65+
new OpenApiError( new OpenApiException("[File: ./TodoReference.yaml] Invalid Reference identifier 'object-not-existing'.")) });
66+
67+
}
68+
}
69+
70+
public class ResourceLoader : IStreamLoader
71+
{
72+
public Stream Load(Uri uri)
73+
{
74+
return null;
75+
}
76+
77+
public Task<Stream> LoadAsync(Uri uri)
78+
{
79+
var path = new Uri(new Uri("http://example.org/OpenApiReaderTests/Samples/OpenApiDiagnosticReportMerged/"), uri).AbsolutePath;
80+
path = path.Substring(1); // remove leading slash
81+
return Task.FromResult(Resources.GetStream(path));
82+
}
3583
}
3684
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
openapi: 3.0.1
2+
info:
3+
title: Example using a remote reference
4+
version: 1.0.0
5+
paths:
6+
"/todos":
7+
get:
8+
parameters:
9+
- $ref: ./TodoReference.yaml#/components/parameters/filter
10+
responses:
11+
200:
12+
description: Ok
13+
content:
14+
application/json:
15+
schema:
16+
$ref: ./TodoReference.yaml#/components/schemas/todo
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
openapi: 3.0.1
2+
info:
3+
title: Components for the todo app
4+
version: 1.0.0
5+
paths: {}
6+
components:
7+
parameters:
8+
filter:
9+
name: filter
10+
in: query
11+
schema:
12+
type: string
13+
schemas:
14+
todo:
15+
type: object
16+
allOf:
17+
- $ref: "#/components/schemas/entity"
18+
- $ref: "#/components/schemas/object-not-existing"
19+
properties:
20+
subject:
21+
type: string
22+
entity:
23+
type: object
24+
properties:
25+
id:
26+
type:string

0 commit comments

Comments
 (0)