Merge pull request #619 from /issues/603

Fix resolution of external files

Author: darrelmiller

src/Microsoft.OpenApi.Hidi/OpenApiService.cs

@@ -10,8 +10,8 @@
 using System.Net.Http;
 using System.Security;
 using System.Text;
-using System.Text.Json;
 using System.Threading.Tasks;
+using System.Text.Json;
 using Microsoft.Extensions.Logging;
 using System.Xml.Linq;
 using Microsoft.OData.Edm.Csdl;
@@ -40,8 +40,8 @@ public static async Task<int> TransformOpenApiDocument(
             string? version,
             OpenApiFormat? format,
             LogLevel loglevel,
-            bool inline,
-            bool resolveexternal,
+            bool inlineLocal,
+            bool inlineExternal,
             string filterbyoperationids,
             string filterbytags,
             string filterbycollection,
@@ -99,8 +99,9 @@ CancellationToken cancellationToken
                         stopwatch.Restart();
                         var result = await new OpenApiStreamReader(new OpenApiReaderSettings
                         {
-                            ReferenceResolution = resolveexternal ? ReferenceResolutionSetting.ResolveAllReferences : ReferenceResolutionSetting.ResolveLocalReferences,
-                            RuleSet = ValidationRuleSet.GetDefaultRuleSet()
+                            RuleSet = ValidationRuleSet.GetDefaultRuleSet(),
+                            LoadExternalRefs = inlineExternal,
+                            BaseUrl = openapi.StartsWith("http") ? new Uri(openapi) : new Uri("file:" + new FileInfo(openapi).DirectoryName + "\\")
                         }
                         ).ReadAsync(stream);
 
@@ -177,7 +178,8 @@ CancellationToken cancellationToken
 
                     var settings = new OpenApiWriterSettings()
                     {
-                        ReferenceInline = inline ? ReferenceInlineSetting.InlineLocalReferences : ReferenceInlineSetting.DoNotInlineReferences
+                        InlineLocalReferences = inlineLocal,
+                        InlineExternalReferences = inlineExternal
                     };
 
                     IOpenApiWriter writer = openApiFormat switch
@@ -239,7 +241,7 @@ public static async Task<int> ValidateOpenApiDocument(
                         RuleSet = ValidationRuleSet.GetDefaultRuleSet()
                     }
                     ).ReadAsync(stream);
-
+                    
                     logger.LogTrace("{timestamp}ms: Completed parsing.", stopwatch.ElapsedMilliseconds);
 
                     document = result.OpenApiDocument;
@@ -316,6 +318,73 @@ public static async Task<OpenApiDocument> ConvertCsdlToOpenApi(Stream csdl)
             return document;
         }
 
+        /// <summary>
+        /// Fixes the references in the resulting OpenApiDocument.
+        /// </summary>
+        /// <param name="document"> The converted OpenApiDocument.</param>
+        /// <returns> A valid OpenApiDocument instance.</returns>
+        public static OpenApiDocument FixReferences(OpenApiDocument document)
+        {
+            // This method is only needed because the output of ConvertToOpenApi isn't quite a valid OpenApiDocument instance.
+            // So we write it out, and read it back in again to fix it up.
+
+            var sb = new StringBuilder();
+            document.SerializeAsV3(new OpenApiYamlWriter(new StringWriter(sb)));
+            var doc = new OpenApiStringReader().Read(sb.ToString(), out _);
+
+            return doc;
+        }
+
+        private static async Task<Stream> GetStream(string input, ILogger logger)
+        {
+            var stopwatch = new Stopwatch();
+            stopwatch.Start();
+
+            Stream stream;
+            if (input.StartsWith("http"))
+            {
+                try
+                {
+                    var httpClientHandler = new HttpClientHandler()
+                    {
+                        SslProtocols = System.Security.Authentication.SslProtocols.Tls12,
+                    };
+                    using var httpClient = new HttpClient(httpClientHandler)
+                    {
+                      DefaultRequestVersion = HttpVersion.Version20
+                    };
+                    stream = await httpClient.GetStreamAsync(input);
+                }
+                catch (HttpRequestException ex)
+                {
+                    logger.LogError($"Could not download the file at {input}, reason{ex}");
+                    return null;
+                }
+            }
+            else 
+            {
+                try
+                {
+                    var fileInput = new FileInfo(input);
+                    stream = fileInput.OpenRead();
+                }
+                catch (Exception ex) when (ex is FileNotFoundException ||
+                    ex is PathTooLongException ||
+                    ex is DirectoryNotFoundException ||
+                    ex is IOException ||
+                    ex is UnauthorizedAccessException ||
+                    ex is SecurityException ||
+                    ex is NotSupportedException)
+                {
+                    logger.LogError($"Could not open the file at {input}, reason: {ex.Message}");
+                    return null;
+                }
+            }
+            stopwatch.Stop();
+            logger.LogTrace("{timestamp}ms: Read file {input}", stopwatch.ElapsedMilliseconds, input);
+            return stream;
+        }
+
         /// <summary>
         /// Takes in a file stream, parses the stream into a JsonDocument and gets a list of paths and Http methods
         /// </summary>
@@ -353,17 +422,28 @@ public static Dictionary<string, List<string>> ParseJsonCollectionFile(Stream st
         /// </summary>
         /// <param name="document"> The converted OpenApiDocument.</param>
         /// <returns> A valid OpenApiDocument instance.</returns>
-        private static OpenApiDocument FixReferences(OpenApiDocument document)
-        {
-            // This method is only needed because the output of ConvertToOpenApi isn't quite a valid OpenApiDocument instance.
-            // So we write it out, and read it back in again to fix it up.
-
-            var sb = new StringBuilder();
-            document.SerializeAsV3(new OpenApiYamlWriter(new StringWriter(sb)));
-            var doc = new OpenApiStringReader().Read(sb.ToString(), out _);
-
-            return doc;
-        }
+        // private static OpenApiDocument FixReferences2(OpenApiDocument document)
+        // {
+        //     // This method is only needed because the output of ConvertToOpenApi isn't quite a valid OpenApiDocument instance.
+        //     // So we write it out, and read it back in again to fix it up.
+
+        //     OpenApiDocument document;
+        //     logger.LogTrace("Parsing the OpenApi file");
+        //     var result = await new OpenApiStreamReader(new OpenApiReaderSettings
+        //     {
+        //         RuleSet = ValidationRuleSet.GetDefaultRuleSet(),
+        //         BaseUrl = new Uri(openapi)
+        //     }
+        //     ).ReadAsync(stream);
+
+        //     document = result.OpenApiDocument;
+        //     var context = result.OpenApiDiagnostic;
+        //     var sb = new StringBuilder();
+        //     document.SerializeAsV3(new OpenApiYamlWriter(new StringWriter(sb)));
+        //     var doc = new OpenApiStringReader().Read(sb.ToString(), out _);
+
+        //     return doc;
+        // }
 
         /// <summary>
         /// Reads stream from file system or makes HTTP request depending on the input string

src/Microsoft.OpenApi.Hidi/Program.cs

@@ -48,11 +48,11 @@ static async Task Main(string[] args)
             var filterByCollectionOption = new Option<string>("--filter-by-collection", "Filters OpenApiDocument by Postman collection provided");
             filterByCollectionOption.AddAlias("-c");
 
-            var inlineOption = new Option<bool>("--inline", "Inline $ref instances");
-            inlineOption.AddAlias("-i");
+            var inlineLocalOption = new Option<bool>("--inlineLocal", "Inline local $ref instances");
+            inlineLocalOption.AddAlias("-il");
 
-            var resolveExternalOption = new Option<bool>("--resolve-external", "Resolve external $refs");
-            resolveExternalOption.AddAlias("-ex");
+            var inlineExternalOption = new Option<bool>("--inlineExternal", "Inline external $ref instances");
+            inlineExternalOption.AddAlias("-ie");
 
             var validateCommand = new Command("validate")
             {
@@ -74,12 +74,12 @@ static async Task Main(string[] args)
                 filterByOperationIdsOption,
                 filterByTagsOption,
                 filterByCollectionOption,
-                inlineOption,
-                resolveExternalOption,
+                inlineLocalOption,
+                inlineExternalOption
             };
 
             transformCommand.SetHandler<string, string, FileInfo, bool, string?, OpenApiFormat?, LogLevel, bool, bool, string, string, string, CancellationToken> (
-                OpenApiService.TransformOpenApiDocument, descriptionOption, csdlOption, outputOption, cleanOutputOption, versionOption, formatOption, logLevelOption, inlineOption, resolveExternalOption, filterByOperationIdsOption, filterByTagsOption, filterByCollectionOption);
+                OpenApiService.TransformOpenApiDocument, descriptionOption, csdlOption, outputOption, cleanOutputOption, versionOption, formatOption, logLevelOption, inlineLocalOption, inlineExternalOption, filterByOperationIdsOption, filterByTagsOption, filterByCollectionOption);
 
             rootCommand.Add(transformCommand);
             rootCommand.Add(validateCommand);

src/Microsoft.OpenApi.Readers/OpenApiReaderSettings.cs

@@ -4,15 +4,10 @@
 using Microsoft.OpenApi.Any;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Readers.Interface;
-using Microsoft.OpenApi.Readers.ParseNodes;
-using Microsoft.OpenApi.Readers.Services;
 using Microsoft.OpenApi.Validations;
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace Microsoft.OpenApi.Readers
 {
@@ -30,7 +25,7 @@ public enum ReferenceResolutionSetting
         /// </summary>
         ResolveLocalReferences,
         /// <summary>
-        /// Convert all references to references of valid domain objects.
+        /// ResolveAllReferences effectively means load external references. Will be removed in v2. External references are never "resolved".
         /// </summary>
         ResolveAllReferences
     }
@@ -43,8 +38,14 @@ public class OpenApiReaderSettings
         /// <summary>
         /// Indicates how references in the source document should be handled.
         /// </summary>
+        /// <remarks>This setting will be going away in the next major version of this library.  Use GetEffective on model objects to get resolved references.</remarks>
         public ReferenceResolutionSetting ReferenceResolution { get; set; } = ReferenceResolutionSetting.ResolveLocalReferences;
 
+        /// <summary>
+        /// When external references are found, load them into a shared workspace
+        /// </summary>
+        public bool LoadExternalRefs { get; set; } = false;
+
         /// <summary>
         /// Dictionary of parsers for converting extensions into strongly typed classes
         /// </summary>

src/Microsoft.OpenApi.Readers/OpenApiStreamReader.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license.
 
+using System;
 using System.IO;
 using System.Threading.Tasks;
 using Microsoft.OpenApi.Interfaces;
@@ -23,6 +24,12 @@ public class OpenApiStreamReader : IOpenApiReader<Stream, OpenApiDiagnostic>
         public OpenApiStreamReader(OpenApiReaderSettings settings = null)
         {
             _settings = settings ?? new OpenApiReaderSettings();
+
+            if((_settings.ReferenceResolution == ReferenceResolutionSetting.ResolveAllReferences || _settings.LoadExternalRefs)
+                && _settings.BaseUrl == null)
+            {
+                throw new ArgumentException("BaseUrl must be provided to resolve external references.");
+            }
         }
 
         /// <summary>

src/Microsoft.OpenApi.Readers/OpenApiYamlDocumentReader.cs

@@ -53,6 +53,11 @@ public OpenApiDocument Read(YamlDocument input, out OpenApiDiagnostic diagnostic
                 // Parse the OpenAPI Document
                 document = context.Parse(input);
 
+                if (_settings.LoadExternalRefs)
+                {
+                    throw new InvalidOperationException("Cannot load external refs using the synchronous Read, use ReadAsync instead.");
+                }
+
                 ResolveReferences(diagnostic, document);
             }
             catch (OpenApiException ex)
@@ -88,7 +93,12 @@ public async Task<ReadResult> ReadAsync(YamlDocument input)
                 // Parse the OpenAPI Document
                 document = context.Parse(input);
 
-                await ResolveReferencesAsync(diagnostic, document);
+                if (_settings.LoadExternalRefs)
+                {
+                    await LoadExternalRefs(document);
+                }
+
+                ResolveReferences(diagnostic, document);
             }
             catch (OpenApiException ex)
             {
@@ -112,52 +122,28 @@ public async Task<ReadResult> ReadAsync(YamlDocument input)
             };
         }
 
-
-        private void ResolveReferences(OpenApiDiagnostic diagnostic, OpenApiDocument document)
+        private async Task LoadExternalRefs(OpenApiDocument document)
         {
-            // Resolve References if requested
-            switch (_settings.ReferenceResolution)
-            {
-                case ReferenceResolutionSetting.ResolveAllReferences:
-                    throw new ArgumentException("Cannot resolve all references via a synchronous call. Use ReadAsync.");
-                case ReferenceResolutionSetting.ResolveLocalReferences:
-                    var errors = document.ResolveReferences(false);
+            // Create workspace for all documents to live in.
+            var openApiWorkSpace = new OpenApiWorkspace();
 
-                    foreach (var item in errors)
-                    {
-                        diagnostic.Errors.Add(item);
-                    }
-                    break;
-                case ReferenceResolutionSetting.DoNotResolveReferences:
-                    break;
-            }
+            // 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);
         }
 
-        private async Task ResolveReferencesAsync(OpenApiDiagnostic diagnostic, OpenApiDocument document)
+        private void ResolveReferences(OpenApiDiagnostic diagnostic, OpenApiDocument document)
         {
             List<OpenApiError> errors = new List<OpenApiError>();
 
             // Resolve References if requested
             switch (_settings.ReferenceResolution)
             {
                 case ReferenceResolutionSetting.ResolveAllReferences:
-
-                    // Create workspace for all documents to live in.
-                    var openApiWorkSpace = new OpenApiWorkspace();
-
-                    // Load this root document into the workspace
-                    var streamLoader = new DefaultStreamLoader();
-                    var workspaceLoader = new OpenApiWorkspaceLoader(openApiWorkSpace, _settings.CustomExternalLoader ?? streamLoader, _settings);
-                    await workspaceLoader.LoadAsync(new OpenApiReference() { ExternalResource = "/" }, document);
-
-                    // Resolve all references in all the documents loaded into the OpenApiWorkspace
-                    foreach (var doc in openApiWorkSpace.Documents)
-                    {
-                        errors.AddRange(doc.ResolveReferences(true));
-                    }
-                    break;
+                    throw new ArgumentException("Resolving external references is not supported");
                 case ReferenceResolutionSetting.ResolveLocalReferences:
-                    errors.AddRange(document.ResolveReferences(false));
+                    errors.AddRange(document.ResolveReferences());
                     break;
                 case ReferenceResolutionSetting.DoNotResolveReferences:
                     break;

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

@@ -14,32 +14,41 @@ namespace Microsoft.OpenApi.Readers.Services
     /// </summary>
     internal class DefaultStreamLoader : IStreamLoader
     {
+        private readonly Uri baseUrl;
         private HttpClient _httpClient = new HttpClient();
 
+
+        public DefaultStreamLoader(Uri baseUrl)
+        {
+            this.baseUrl = baseUrl;
+        }
+
         public Stream Load(Uri uri)
         {
+            var absoluteUri = new Uri(baseUrl, uri);
             switch (uri.Scheme)
             {
                 case "file":
-                    return File.OpenRead(uri.AbsolutePath);
+                    return File.OpenRead(absoluteUri.AbsolutePath);
                 case "http":
                 case "https":
-                    return _httpClient.GetStreamAsync(uri).GetAwaiter().GetResult();
-
+                    return _httpClient.GetStreamAsync(absoluteUri).GetAwaiter().GetResult();
                 default:
                     throw new ArgumentException("Unsupported scheme");
             }
         }
 
         public async Task<Stream> LoadAsync(Uri uri)
         {
-            switch (uri.Scheme)
+            var absoluteUri = new Uri(baseUrl, uri);
+
+            switch (absoluteUri.Scheme)
             {
                 case "file":
-                    return File.OpenRead(uri.AbsolutePath);
+                    return File.OpenRead(absoluteUri.AbsolutePath);
                 case "http":
                 case "https":
-                    return await _httpClient.GetStreamAsync(uri);
+                    return await _httpClient.GetStreamAsync(absoluteUri);
                 default:
                     throw new ArgumentException("Unsupported scheme");
             }