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

fix: side effects in tag references #2032

Merged
merged 4 commits into from
Jan 15, 2025
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
7 changes: 4 additions & 3 deletions src/Microsoft.OpenApi.Readers/OpenApiYamlReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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>
Expand Down
4 changes: 3 additions & 1 deletion src/Microsoft.OpenApi/Interfaces/IOpenApiReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 0 additions & 15 deletions src/Microsoft.OpenApi/Models/OpenApiDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.OpenApi/Models/OpenApiOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 1 addition & 7 deletions src/Microsoft.OpenApi/Models/OpenApiTag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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>
Expand All @@ -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>
Expand Down
67 changes: 33 additions & 34 deletions src/Microsoft.OpenApi/Models/References/OpenApiTagReference.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -10,21 +12,24 @@ namespace Microsoft.OpenApi.Models.References
/// <summary>
/// Tag Object Reference
/// </summary>
public class OpenApiTagReference : OpenApiTag
public class OpenApiTagReference : OpenApiTag, IOpenApiReferenceable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to implement IOpenApiReferenceable? None of the other proxy objects do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently it is a bug in every other reference class because the walker expects all references to implement IOpenApiReferencable /cc @MaggieKimani1

{
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;
}
}

Expand All @@ -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
{
Expand All @@ -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
{
Expand All @@ -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
{
Expand Down
9 changes: 6 additions & 3 deletions src/Microsoft.OpenApi/Reader/OpenApiJsonReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
{
Expand All @@ -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)
{
Expand Down
21 changes: 14 additions & 7 deletions src/Microsoft.OpenApi/Reader/OpenApiModelFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand Down
Loading
Loading