Merge pull request #2032 from microsoft/fix/tag-references

fix: side effects in tag references

Author: baywet

src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs

@@ -86,6 +86,7 @@ public static ReadResult Read(JsonNode jsonNode, OpenApiReaderSettings settings,
         /// <inheritdoc/>
         public T ReadFragment<T>(MemoryStream input,
                                  OpenApiSpecVersion version,
+                                 OpenApiDocument openApiDocument,
                                  out OpenApiDiagnostic diagnostic,
                                  OpenApiReaderSettings settings = null) where T : IOpenApiElement
         {
@@ -105,13 +106,13 @@ public T ReadFragment<T>(MemoryStream input,
                 return default;
             }
 
-            return ReadFragment<T>(jsonNode, version, out diagnostic, settings);
+            return ReadFragment<T>(jsonNode, version, openApiDocument, out diagnostic, settings);
         }
 
         /// <inheritdoc/>
-        public static T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
+        public static T ReadFragment<T>(JsonNode input, OpenApiSpecVersion version, OpenApiDocument openApiDocument, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
         {
-            return _jsonReader.ReadFragment<T>(input, version, out diagnostic, settings);
+            return _jsonReader.ReadFragment<T>(input, version, openApiDocument, out diagnostic, settings);
         }
 
         /// <summary>

src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs

@@ -5,6 +5,7 @@
 using System.Text.Json.Nodes;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.OpenApi.Models;
 using Microsoft.OpenApi.Reader;
 
 namespace Microsoft.OpenApi.Interfaces
@@ -36,9 +37,10 @@ public interface IOpenApiReader
         /// </summary>
         /// <param name="input">Memory stream containing OpenAPI description to parse.</param>
         /// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
+        /// <param name="openApiDocument">The OpenApiDocument object to which the fragment belongs, used to lookup references.</param>
         /// <param name="diagnostic">Returns diagnostic object containing errors detected during parsing.</param>
         /// <param name="settings">The OpenApiReader settings.</param>
         /// <returns>Instance of newly created IOpenApiElement.</returns>
-        T ReadFragment<T>(MemoryStream input, OpenApiSpecVersion version, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
+        T ReadFragment<T>(MemoryStream input, OpenApiSpecVersion version, OpenApiDocument openApiDocument, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement;
     }
 }

src/Microsoft.OpenApi/Interfaces/IOpenApiVersionService.cs

@@ -28,7 +28,7 @@ internal interface IOpenApiVersionService
         /// <param name="node">document fragment node</param>
         /// <param name="doc">A host document instance.</param>
         /// <returns>Instance of OpenAPIElement</returns>
-        T LoadElement<T>(ParseNode node, OpenApiDocument doc = null) where T : IOpenApiElement;
+        T LoadElement<T>(ParseNode node, OpenApiDocument doc) where T : IOpenApiElement;
 
         /// <summary>
         /// Converts a generic RootNode instance into a strongly typed OpenApiDocument

src/Microsoft.OpenApi/Models/OpenApiDocument.cs

@@ -501,21 +501,6 @@ private static string ConvertByteArrayToString(byte[] hash)
                 throw new ArgumentException(Properties.SRResource.LocalReferenceRequiresType);
             }
 
-            // Special case for Tag
-            if (reference.Type == ReferenceType.Tag)
-            {
-                foreach (var tag in this.Tags ?? Enumerable.Empty<OpenApiTag>())
-                {
-                    if (tag.Name == reference.Id)
-                    {
-                        tag.Reference = reference;
-                        return tag;
-                    }
-                }
-
-                return null;
-            }
-
             string uriLocation;
             if (reference.Id.Contains("/")) // this means its a URL reference
             {

src/Microsoft.OpenApi/Models/OpenApiOperation.cs

@@ -26,7 +26,7 @@ public class OpenApiOperation : IOpenApiSerializable, IOpenApiExtensible, IOpenA
         /// A list of tags for API documentation control.
         /// Tags can be used for logical grouping of operations by resources or any other qualifier.
         /// </summary>
-        public IList<OpenApiTag>? Tags { get; set; } = new List<OpenApiTag>();
+        public IList<OpenApiTagReference>? Tags { get; set; } = [];
 
         /// <summary>
         /// A short summary of what the operation does.
@@ -121,7 +121,7 @@ public OpenApiOperation() { }
         /// </summary>
         public OpenApiOperation(OpenApiOperation? operation)
         {
-            Tags = operation?.Tags != null ? new List<OpenApiTag>(operation.Tags) : null;
+            Tags = operation?.Tags != null ? new List<OpenApiTagReference>(operation.Tags) : null;
             Summary = operation?.Summary ?? Summary;
             Description = operation?.Description ?? Description;
             ExternalDocs = operation?.ExternalDocs != null ? new(operation?.ExternalDocs) : null;

src/Microsoft.OpenApi/Models/OpenApiTag.cs

@@ -11,7 +11,7 @@ namespace Microsoft.OpenApi.Models
     /// <summary>
     /// Tag Object.
     /// </summary>
-    public class OpenApiTag : IOpenApiReferenceable, IOpenApiExtensible
+    public class OpenApiTag : IOpenApiSerializable, IOpenApiExtensible
     {
         /// <summary>
         /// The name of the tag.
@@ -38,11 +38,6 @@ public class OpenApiTag : IOpenApiReferenceable, IOpenApiExtensible
         /// </summary>
         public bool UnresolvedReference { get; set; }
 
-        /// <summary>
-        /// Reference.
-        /// </summary>
-        public OpenApiReference Reference { get; set; }
-
         /// <summary>
         /// Parameterless constructor
         /// </summary>
@@ -58,7 +53,6 @@ public OpenApiTag(OpenApiTag tag)
             ExternalDocs = tag?.ExternalDocs != null ? new(tag.ExternalDocs) : null;
             Extensions = tag?.Extensions != null ? new Dictionary<string, IOpenApiExtension>(tag.Extensions) : null;
             UnresolvedReference = tag?.UnresolvedReference ?? UnresolvedReference;
-            Reference = tag?.Reference != null ? new(tag.Reference) : null;
         }
 
         /// <summary>

src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs

@@ -1,7 +1,9 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. 
 
+using System;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.OpenApi.Interfaces;
 using Microsoft.OpenApi.Writers;
 
@@ -10,21 +12,24 @@ namespace Microsoft.OpenApi.Models.References
     /// <summary>
     /// Tag Object Reference
     /// </summary>
-    public class OpenApiTagReference : OpenApiTag
+    public class OpenApiTagReference : OpenApiTag, IOpenApiReferenceable
     {
         internal OpenApiTag _target;
-        private readonly OpenApiReference _reference;
-        private string _description;
 
-        private OpenApiTag Target
+        /// <summary>
+        /// Reference.
+        /// </summary>
+        public OpenApiReference Reference { get; set; }
+
+        /// <summary>
+        /// Resolved target of the reference.
+        /// </summary>
+        public OpenApiTag Target
         {
             get
             {
-                _target ??= Reference.HostDocument?.ResolveReferenceTo<OpenApiTag>(_reference);
-                _target ??= new OpenApiTag() { Name = _reference.Id };
-                OpenApiTag resolved = new OpenApiTag(_target);
-                if (!string.IsNullOrEmpty(_description)) resolved.Description = _description;
-                return resolved;
+                _target ??= Reference.HostDocument?.Tags.FirstOrDefault(t => StringComparer.Ordinal.Equals(t.Name, Reference.Id));
+                return _target;
             }
         }
 
@@ -37,49 +42,43 @@ public OpenApiTagReference(string referenceId, OpenApiDocument hostDocument)
         {
             Utils.CheckArgumentNullOrEmpty(referenceId);
 
-            _reference = new OpenApiReference()
+            Reference = new OpenApiReference()
             {
                 Id = referenceId,
                 HostDocument = hostDocument,
                 Type = ReferenceType.Tag
             };
-
-            Reference = _reference;
         }
 
-        internal OpenApiTagReference(OpenApiTag target, string referenceId)
+        /// <summary>
+        /// Copy Constructor
+        /// </summary>
+        /// <param name="source">The source to copy information from.</param>
+        public OpenApiTagReference(OpenApiTagReference source):base()
         {
-            _target = target;
-
-            _reference = new OpenApiReference()
-            {
-                Id = referenceId,
-                Type = ReferenceType.Tag,
-            };
+            Reference = source?.Reference != null ? new(source.Reference) : null;
+            _target = source?._target;
         }
 
+        private const string ReferenceErrorMessage = "Setting the value from the reference is not supported, use the target property instead.";
         /// <inheritdoc/>
-        public override string Description
-        {
-            get => string.IsNullOrEmpty(_description) ? Target?.Description : _description;
-            set => _description = value;
-        }
+        public override string Description { get => Target.Description; set => throw new InvalidOperationException(ReferenceErrorMessage); }
 
         /// <inheritdoc/>
-        public override OpenApiExternalDocs ExternalDocs { get => Target?.ExternalDocs; set => Target.ExternalDocs = value; }
+        public override OpenApiExternalDocs ExternalDocs { get => Target.ExternalDocs; set => throw new InvalidOperationException(ReferenceErrorMessage); }
 
         /// <inheritdoc/>
-        public override IDictionary<string, IOpenApiExtension> Extensions { get => Target?.Extensions; set => Target.Extensions = value; }
+        public override IDictionary<string, IOpenApiExtension> Extensions { get => Target.Extensions; set => throw new InvalidOperationException(ReferenceErrorMessage); }
 
         /// <inheritdoc/>
-        public override string Name { get => Target?.Name; set => Target.Name = value; }
+        public override string Name { get => Target.Name; set => throw new InvalidOperationException(ReferenceErrorMessage); }
 
         /// <inheritdoc/>
         public override void SerializeAsV3(IOpenApiWriter writer)
         {
-            if (!writer.GetSettings().ShouldInlineReference(_reference))
+            if (!writer.GetSettings().ShouldInlineReference(Reference))
             {
-                _reference.SerializeAsV3(writer);
+                Reference.SerializeAsV3(writer);
             }
             else
             {
@@ -90,9 +89,9 @@ public override void SerializeAsV3(IOpenApiWriter writer)
         /// <inheritdoc/>
         public override void SerializeAsV31(IOpenApiWriter writer)
         {
-            if (!writer.GetSettings().ShouldInlineReference(_reference))
+            if (!writer.GetSettings().ShouldInlineReference(Reference))
             {
-                _reference.SerializeAsV31(writer);
+                Reference.SerializeAsV31(writer);
             }
             else
             {
@@ -103,9 +102,9 @@ public override void SerializeAsV31(IOpenApiWriter writer)
         /// <inheritdoc/>
         public override void SerializeAsV2(IOpenApiWriter writer)
         {
-            if (!writer.GetSettings().ShouldInlineReference(_reference))
+            if (!writer.GetSettings().ShouldInlineReference(Reference))
             {
-                _reference.SerializeAsV2(writer);
+                Reference.SerializeAsV2(writer);
             }
             else
             {

src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs

@@ -148,10 +148,12 @@ public async Task<ReadResult> ReadAsync(Stream input,
         /// <inheritdoc/>
         public T ReadFragment<T>(MemoryStream input,
                                  OpenApiSpecVersion version,
+                                 OpenApiDocument openApiDocument,
                                  out OpenApiDiagnostic diagnostic,
                                  OpenApiReaderSettings settings = null) where T : IOpenApiElement
         {
-            if (input is null) throw new ArgumentNullException(nameof(input));
+            Utils.CheckArgumentNull(input);
+            Utils.CheckArgumentNull(openApiDocument);
 
             JsonNode jsonNode;
 
@@ -167,12 +169,13 @@ public T ReadFragment<T>(MemoryStream input,
                 return default;
             }
 
-            return ReadFragment<T>(jsonNode, version, out diagnostic);
+            return ReadFragment<T>(jsonNode, version, openApiDocument, out diagnostic);
         }
 
         /// <inheritdoc/>
         public T ReadFragment<T>(JsonNode input,
                          OpenApiSpecVersion version,
+                         OpenApiDocument openApiDocument,
                          out OpenApiDiagnostic diagnostic,
                          OpenApiReaderSettings settings = null) where T : IOpenApiElement
         {
@@ -187,7 +190,7 @@ public T ReadFragment<T>(JsonNode input,
             try
             {
                 // Parse the OpenAPI element
-                element = context.ParseFragment<T>(input, version);
+                element = context.ParseFragment<T>(input, version, openApiDocument);
             }
             catch (OpenApiException ex)
             {

src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs

@@ -61,14 +61,15 @@ public static ReadResult Load(MemoryStream stream,
         /// <param name="input">Stream containing OpenAPI description to parse.</param>
         /// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
         /// <param name="format"></param>
+        /// <param name="openApiDocument">The OpenApiDocument object to which the fragment belongs, used to lookup references.</param>
         /// <param name="diagnostic">Returns diagnostic object containing errors detected during parsing.</param>
         /// <param name="settings">The OpenApiReader settings.</param>
         /// <returns>Instance of newly created IOpenApiElement.</returns>
         /// <returns>The OpenAPI element.</returns>
-        public static T Load<T>(MemoryStream input, OpenApiSpecVersion version, string format, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
+        public static T Load<T>(MemoryStream input, OpenApiSpecVersion version, string format, OpenApiDocument openApiDocument, out OpenApiDiagnostic diagnostic, OpenApiReaderSettings settings = null) where T : IOpenApiElement
         {
             format ??= InspectStreamFormat(input);
-            return OpenApiReaderRegistry.GetReader(format).ReadFragment<T>(input, version, out diagnostic, settings);
+            return OpenApiReaderRegistry.GetReader(format).ReadFragment<T>(input, version, openApiDocument, out diagnostic, settings);
         }
 
         /// <summary>
@@ -91,13 +92,14 @@ public static async Task<ReadResult> LoadAsync(string url, OpenApiReaderSettings
         /// <param name="url">The path to the OpenAPI file</param>
         /// <param name="version">Version of the OpenAPI specification that the fragment conforms to.</param>
         /// <param name="settings">The OpenApiReader settings.</param>
+        /// <param name="openApiDocument">The OpenApiDocument object to which the fragment belongs, used to lookup references.</param>
         /// <param name="token"></param>
         /// <returns>Instance of newly created IOpenApiElement.</returns>
         /// <returns>The OpenAPI element.</returns>
-        public static async Task<T> LoadAsync<T>(string url, OpenApiSpecVersion version, OpenApiReaderSettings settings = null, CancellationToken token = default) where T : IOpenApiElement
+        public static async Task<T> LoadAsync<T>(string url, OpenApiSpecVersion version, OpenApiDocument openApiDocument, OpenApiReaderSettings settings = null, CancellationToken token = default) where T : IOpenApiElement
         {
             var (stream, format) = await RetrieveStreamAndFormatAsync(url, token).ConfigureAwait(false);
-            return await LoadAsync<T>(stream, version, format, settings, token);
+            return await LoadAsync<T>(stream, version, openApiDocument, format, settings, token);
         }
 
         /// <summary>
@@ -145,27 +147,30 @@ public static async Task<ReadResult> LoadAsync(Stream input, string format = nul
         /// <typeparam name="T"></typeparam>
         /// <param name="input"></param>
         /// <param name="version"></param>
+        /// <param name="openApiDocument">The document used to lookup tag or schema references.</param>
         /// <param name="format"></param>
         /// <param name="settings"></param>
         /// <param name="token"></param>
         /// <returns></returns>
         public static async Task<T> LoadAsync<T>(Stream input,
                                                  OpenApiSpecVersion version,
+                                                 OpenApiDocument openApiDocument,
                                                  string format = null,
                                                  OpenApiReaderSettings settings = null,
                                                  CancellationToken token = default) where T : IOpenApiElement
         {
+            Utils.CheckArgumentNull(openApiDocument);
             if (input is null) throw new ArgumentNullException(nameof(input));
             if (input is MemoryStream memoryStream)
             {
-                return Load<T>(memoryStream, version, format, out var _, settings);
+                return Load<T>(memoryStream, version, format, openApiDocument, out var _, settings);
             }
             else
             {
                 memoryStream = new MemoryStream();
                 await input.CopyToAsync(memoryStream, 81920, token).ConfigureAwait(false);
                 memoryStream.Position = 0;
-                return Load<T>(memoryStream, version, format, out var _, settings);
+                return Load<T>(memoryStream, version, format, openApiDocument, out var _, settings);
             }
         }
 
@@ -195,12 +200,14 @@ public static ReadResult Parse(string input,
         /// </summary>
         /// <param name="input">The input string.</param>
         /// <param name="version"></param>
+        /// <param name="openApiDocument">The OpenApiDocument object to which the fragment belongs, used to lookup references.</param>
         /// <param name="diagnostic">The diagnostic entity containing information from the reading process.</param>
         /// <param name="format">The Open API format</param>
         /// <param name="settings">The OpenApi reader settings.</param>
         /// <returns>An OpenAPI document instance.</returns>
         public static T Parse<T>(string input,
                                  OpenApiSpecVersion version,
+                                 OpenApiDocument openApiDocument,
                                  out OpenApiDiagnostic diagnostic,
                                  string format = null,
                                  OpenApiReaderSettings settings = null) where T : IOpenApiElement
@@ -209,7 +216,7 @@ public static T Parse<T>(string input,
             format ??= InspectInputFormat(input);
             settings ??= new OpenApiReaderSettings();
             using var stream = new MemoryStream(Encoding.UTF8.GetBytes(input));
-            return Load<T>(stream, version, format, out diagnostic, settings);
+            return Load<T>(stream, version, format, openApiDocument, out diagnostic, settings);
         }
 
         private static readonly OpenApiReaderSettings DefaultReaderSettings = new();