Diagnostic report of an external reference with an error cannot be obtained

[elize-vdr]

Is your feature request related to a problem? Please describe.
I am using the Readers to parse OpenApi contracts. I have an OpenApi contract that references an external file. It turned out this external file has an error, but the ReturnResult I get has no diagnostics in, I stepped through the code and I can see where it fails, but the diagnostics gets lost, it seems to be in here (see snippet below) in the code, and I noticed that possibly this problem has come up before since there is a comment // TODO merge _diagnositics:

internal class OpenApiWorkspaceLoader
{
.....
        internal async Task LoadAsync(OpenApiReference reference, OpenApiDocument document)
        {
            _workspace.AddDocument(reference.ExternalResource, document);
            document.Workspace = _workspace;

            // Collect remote references by walking document
            var referenceCollector = new OpenApiRemoteReferenceCollector(document);
            var collectorWalker = new OpenApiWalker(referenceCollector);
            collectorWalker.Walk(document);

            var reader = new OpenApiStreamReader(_readerSettings);

            // 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); // TODO merge _diagnositics
                    await LoadAsync(item, result.OpenApiDocument);
                }
            }
        }
}

Describe the solution you'd like
I would love to get the diagnostics. Our Use Case is that we analyze and validate the OpenApi contract for the user and if there is an error we would like to give them a usable report that tells them what they should fix in the main document as well as any external references.

Describe alternatives you've considered
I thought of implementing something here myself, but there seems to be a slight change of structure required because the OpenApiWorkspaceLoader.LoadAsync() seems to be recursive and the diagnostic is thrown away on each recursion because there is nowhere to store it to pass it back up, for example perhaps make it part of the OpenApiDocument, or another result wrapper that can pass both on in the recursion. But I do not want to break things because I am unfamiliar with this library.
So I am wondering what the chances are somebody will be able to do this, or tell me what to do then I can help out and implement, because it will really be great for us to have.

[elize-vdr]

I made a fork and implemented a solution whereby I append the diagnostics and then pass it up from OpenApiWorkspaceLoader.LoadAsync().
I add the ExternalResource in square brackets to the start of the error/warning so that one can see where the error/warning originated from, else I do not know in which of the external referenced files it originated from.
I now get an error report something like this

        "error": [
            "allOf is not a valid property at #/components/responses/Environments_PUT_201",
            "allOf is not a valid property at #/components/responses/PublishedEndpoint_POSTPUT_201",
            "$ref is not a valid property at #/components/securitySchemes/OAuth2",
            "$ref is not a valid property at #/components/securitySchemes/PersonalAccessTokens",
            "[File: common-definitions_v4.yaml] (Lin: 1162, Col: 2, Chr: 45055) - (Lin: 1162, Col: 2, Chr: 45055): While scanning a simple key, could not find expected ':'.",
            "[#/components/responses/Environments_PUT_201/description] The field 'description' in 'response' object is REQUIRED.",
            "[#/components/responses/PublishedEndpoint_POSTPUT_201/description] The field 'description' in 'response' object is REQUIRED."
        ]

The 5th error is the one from the external resource, this line and it indicates the file name which is important:

"[File: common-definitions_v4.yaml] (Lin: 1162, Col: 2, Chr: 45055) - (Lin: 1162, Col: 2, Chr: 45055): While scanning a simple key, could not find expected ':'.",

I am wondering if I can then submit this change?

[elize-vdr]

The changes I made OpenApiWorkspaceLoader.LoadAsync() now looks like this:

internal async Task<OpenApiDiagnostic> LoadAsync(OpenApiReference reference, OpenApiDocument document, OpenApiDiagnostic diagnostic = null)
        {
            _workspace.AddDocument(reference.ExternalResource, document);
            document.Workspace = _workspace;

            // Collect remote references by walking document
            var referenceCollector = new OpenApiRemoteReferenceCollector(document);
            var collectorWalker = new OpenApiWalker(referenceCollector);
            collectorWalker.Walk(document);

            var reader = new OpenApiStreamReader(_readerSettings);

            // Walk references
            if (diagnostic is null)
            {
                diagnostic = new OpenApiDiagnostic();
            }
            
            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); // TODO merge _diagnositics
                    if (result.OpenApiDiagnostic != null)
                    {
                        diagnostic.AppendDiagnostic(result.OpenApiDiagnostic, item.ExternalResource);
                    }
                    if (result.OpenApiDocument != null)
                    {
                        var loadDiagnostic = await LoadAsync(item, result.OpenApiDocument, diagnostic);
                        diagnostic = loadDiagnostic;
                    }
                }
            }

            return diagnostic;
        }

I then added method AppendDiagnostic to the OpenApiDiagnostic class to append another diagnostic to it supplying a file name (or not) it originated from:

/// <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"></param>
        /// <param name="fileNameToAdd"></param>
        public void AppendDiagnostic(OpenApiDiagnostic diagnosticToAdd, string fileNameToAdd)
        {
            foreach (var err in diagnosticToAdd.Errors) {
                string errMsgWithFileName;
                if (!(string.IsNullOrEmpty(fileNameToAdd) || string.IsNullOrWhiteSpace(fileNameToAdd)))
                {
                    errMsgWithFileName = $"[File: {fileNameToAdd}] {err.Message}";
                }
                else
                {
                    errMsgWithFileName = err.Message;
                }
                Errors.Add(new OpenApiError(err.Pointer, errMsgWithFileName));
            }
            foreach (var warn in diagnosticToAdd.Warnings) {
                string warnMsgWithFileName;
                if (!(string.IsNullOrEmpty(fileNameToAdd) || string.IsNullOrWhiteSpace(fileNameToAdd)))
                {
                    warnMsgWithFileName = $"[File: {fileNameToAdd}] {warn.Message}";
                }
                else
                {
                    warnMsgWithFileName = warn.Message;
                }
                Errors.Add(new OpenApiError(warn.Pointer, warnMsgWithFileName));
            }
        }

I also added an extension for IList so that I can call AddRange() to append other Errors and Warnings to that of an OpenApiDiagnostic, this:

/// <summary>
    /// Extension class for IList to add the Method "AddRange"
    /// </summary>
    public 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>
        public static void AddRange<T>(this ICollection<T> collection, IEnumerable<T> enumerable) {
            foreach (var cur in enumerable) {
                collection.Add(cur);
            }
        }
    }

Then in order to pass the Concatenated diagnostics up in the chain had to change

• OpenApiYamlDocumentReader.LoadExternalRefs() to also return Task<OpenApiDiagnostic>
• Then in OpenApiYamlDocumentReader.ReadAsync where it calls LoadExternalRefs() receive the diagnostic from LoadExternalRefs() and append this to the local diagnostic, this code then looks like this:

try
            {
                // Parse the OpenAPI Document
                document = context.Parse(input);

                if (_settings.LoadExternalRefs)
                {
                    var diagnosticExternalRefs = await LoadExternalRefs(document);
                    if (diagnosticExternalRefs != null)
                    {
                        diagnostic.Errors.AddRange(diagnosticExternalRefs.Errors);
                        diagnostic.Warnings.AddRange(diagnosticExternalRefs.Warnings);
                    }
                }

                ResolveReferences(diagnostic, document);
            }
            catch (OpenApiException ex)
            {
                diagnostic.Errors.Add(new OpenApiError(ex));
            }

[elize-vdr]

Any ideas on this please? Can we go ahead and make a branch, make this change and then create a Merge Request?

[MaggieKimani1]

@elize-vdr Please feel free to start a PR then we can review it.

[elize-vdr]

My apologies, did not get around to this, going to do it in the next couple of days

[elize-vdr]

Added the changes and created a Pull request: #1245

[elize-vdr]

Pull request has been maccepted and merged in, this issue can be closed.