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

chore: update-discriminator-mapping #2280

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
13 changes: 10 additions & 3 deletions src/Microsoft.OpenApi/Models/OpenApiDiscriminator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using Microsoft.OpenApi.Interfaces;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Writers;

namespace Microsoft.OpenApi.Models
Expand All @@ -20,7 +21,7 @@ public class OpenApiDiscriminator : IOpenApiSerializable, IOpenApiExtensible
/// <summary>
/// An object to hold mappings between payload values and schema names or references.
/// </summary>
public IDictionary<string, string>? Mapping { get; set; } = new Dictionary<string, string>();
public IDictionary<string, OpenApiSchemaReference>? Mapping { get; set; } = new Dictionary<string, OpenApiSchemaReference>();

/// <summary>
/// This object MAY be extended with Specification Extensions.
Expand All @@ -38,7 +39,7 @@ public OpenApiDiscriminator() { }
public OpenApiDiscriminator(OpenApiDiscriminator discriminator)
{
PropertyName = discriminator?.PropertyName ?? PropertyName;
Mapping = discriminator?.Mapping != null ? new Dictionary<string, string>(discriminator.Mapping) : null;
Mapping = discriminator?.Mapping != null ? new Dictionary<string, OpenApiSchemaReference>(discriminator.Mapping) : null;
Extensions = discriminator?.Extensions != null ? new Dictionary<string, IOpenApiExtension>(discriminator.Extensions) : null;
}

Expand Down Expand Up @@ -80,7 +81,13 @@ private void SerializeInternal(IOpenApiWriter writer)
writer.WriteProperty(OpenApiConstants.PropertyName, PropertyName);

// mapping
writer.WriteOptionalMap(OpenApiConstants.Mapping, Mapping, (w, s) => w.WriteValue(s));
writer.WriteOptionalMap(OpenApiConstants.Mapping, Mapping, (w, s) =>
{
if (!string.IsNullOrEmpty(s.Reference.ReferenceV3) && s.Reference.ReferenceV3 is not null)
{
w.WriteValue(s.Reference.ReferenceV3);
}
});
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System;
using System.Linq;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Reader.ParseNodes;

namespace Microsoft.OpenApi.Reader.V3
Expand All @@ -22,7 +24,7 @@ internal static partial class OpenApiV3Deserializer
},
{
"mapping",
(o, n, _) => o.Mapping = n.CreateSimpleMap(LoadString).Where(kv => kv.Value is not null).ToDictionary(kv => kv.Key, kv => kv.Value!)
(o, n, doc) => o.Mapping = n.CreateSimpleMap((node) => LoadMapping(node, doc))
}
};

Expand All @@ -40,5 +42,11 @@ public static OpenApiDiscriminator LoadDiscriminator(ParseNode node, OpenApiDocu

return discriminator;
}
public static OpenApiSchemaReference LoadMapping(ParseNode node, OpenApiDocument hostDocument)
{
var pointer = node.GetScalarValue() ?? throw new InvalidOperationException("Could not get a pointer reference");
var reference = GetReferenceIdAndExternalResource(pointer);
return new OpenApiSchemaReference(reference.Item1, hostDocument, reference.Item2);
}
}
}
7 changes: 3 additions & 4 deletions src/Microsoft.OpenApi/Reader/V3/OpenApiV3VersionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.OpenApi.Any;
using Microsoft.OpenApi.Exceptions;
using Microsoft.OpenApi.Extensions;
using Microsoft.OpenApi.Interfaces;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Properties;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Reader.ParseNodes;

namespace Microsoft.OpenApi.Reader.V3
Expand All @@ -21,7 +19,7 @@
{
public OpenApiDiagnostic Diagnostic { get; }

private static readonly char[] _pathSeparator = new char[] { '/' };

Check warning on line 22 in src/Microsoft.OpenApi/Reader/V3/OpenApiV3VersionService.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field '_pathSeparator'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 22 in src/Microsoft.OpenApi/Reader/V3/OpenApiV3VersionService.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field '_pathSeparator'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 22 in src/Microsoft.OpenApi/Reader/V3/OpenApiV3VersionService.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field '_pathSeparator'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 22 in src/Microsoft.OpenApi/Reader/V3/OpenApiV3VersionService.cs

View workflow job for this annotation

GitHub Actions / Build

Remove the unused private field '_pathSeparator'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

/// <summary>
/// Create Parsing Context
Expand Down Expand Up @@ -62,7 +60,8 @@
[typeof(OpenApiServer)] = OpenApiV3Deserializer.LoadServer,
[typeof(OpenApiServerVariable)] = OpenApiV3Deserializer.LoadServerVariable,
[typeof(OpenApiTag)] = OpenApiV3Deserializer.LoadTag,
[typeof(OpenApiXml)] = OpenApiV3Deserializer.LoadXml
[typeof(OpenApiXml)] = OpenApiV3Deserializer.LoadXml,
[typeof(OpenApiSchemaReference)] = OpenApiV3Deserializer.LoadMapping
};

public OpenApiDocument LoadDocument(RootNode rootNode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Linq;
using Microsoft.OpenApi.Extensions;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Reader.ParseNodes;

namespace Microsoft.OpenApi.Reader.V31
Expand All @@ -22,9 +23,9 @@ internal static partial class OpenApiV31Deserializer
}
},
{
"mapping", (o, n, _) =>
"mapping", (o, n, doc) =>
{
o.Mapping = n.CreateSimpleMap(LoadString).Where(kv => kv.Value is not null).ToDictionary(kv => kv.Key, kv => kv.Value!);
o.Mapping = n.CreateSimpleMap((node) => LoadMapping(node, doc));
}
}
};
Expand All @@ -47,5 +48,12 @@ public static OpenApiDiscriminator LoadDiscriminator(ParseNode node, OpenApiDocu

return discriminator;
}

public static OpenApiSchemaReference LoadMapping(ParseNode node, OpenApiDocument hostDocument)
{
var pointer = node.GetScalarValue() ?? throw new InvalidOperationException("Could not get a pointer reference");
var reference = GetReferenceIdAndExternalResource(pointer);
return new OpenApiSchemaReference(reference.Item1, hostDocument, reference.Item2);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.OpenApi.Any;
using Microsoft.OpenApi.Exceptions;
using Microsoft.OpenApi.Extensions;
using Microsoft.OpenApi.Interfaces;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Properties;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Reader.ParseNodes;
using Microsoft.OpenApi.Reader.V3;

Expand Down Expand Up @@ -61,7 +59,8 @@ public OpenApiV31VersionService(OpenApiDiagnostic diagnostic)
[typeof(OpenApiServer)] = OpenApiV31Deserializer.LoadServer,
[typeof(OpenApiServerVariable)] = OpenApiV31Deserializer.LoadServerVariable,
[typeof(OpenApiTag)] = OpenApiV31Deserializer.LoadTag,
[typeof(OpenApiXml)] = OpenApiV31Deserializer.LoadXml
[typeof(OpenApiXml)] = OpenApiV31Deserializer.LoadXml,
[typeof(OpenApiSchemaReference)] = OpenApiV31Deserializer.LoadMapping
};

public OpenApiDocument LoadDocument(RootNode rootNode)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System;
using System.IO;
using System.Threading.Tasks;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Models.References;
using Microsoft.OpenApi.Reader;
using Xunit;

Expand All @@ -25,19 +27,21 @@ public async Task ParseBasicDiscriminatorShouldSucceed()
memoryStream.Position = 0;

// Act
var discriminator = OpenApiModelFactory.Load<OpenApiDiscriminator>(memoryStream, OpenApiSpecVersion.OpenApi3_0, OpenApiConstants.Yaml, new(), out var diagnostic, SettingsFixture.ReaderSettings);
var openApiDocument = new OpenApiDocument();
var discriminator = OpenApiModelFactory.Load<OpenApiDiscriminator>(memoryStream, OpenApiSpecVersion.OpenApi3_0, OpenApiConstants.Yaml, openApiDocument, out var diagnostic, SettingsFixture.ReaderSettings);

// Assert
Assert.Equivalent(
new OpenApiDiscriminator
{
PropertyName = "pet_type",
Mapping =
new OpenApiDiscriminator
{
PropertyName = "pet_type",
Mapping =
{
["puppy"] = "#/components/schemas/Dog",
["kitten"] = "Cat"
["puppy"] = new OpenApiSchemaReference("Dog", openApiDocument),
["kitten"] = new OpenApiSchemaReference("Cat" , openApiDocument, "Cat"),
["monster"] = new OpenApiSchemaReference("schema.json" , openApiDocument, "https://gigantic-server.com/schemas/Monster/schema.json")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
["monster"] = new OpenApiSchemaReference("schema.json" , openApiDocument, "https://gigantic-server.com/schemas/Monster/schema.json")
["monster"] = new OpenApiSchemaReference("Sully" , openApiDocument, "https://gigantic-server.com/schemas/Monster/schema.json")

Just for sanity, so we differentiate the schema name from the file. I suspect this is going to impact your test file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, wouldn't the test fall under ExternalSchemaReference Tests where we add this scenario [InlineData("https://gigantic-server.com/schemas/Monster/schema.json#/components/schemas/Sully", "Sully", "https://gigantic-server.com/schemas/Monster/schema.json")]

Copy link
Member

Choose a reason for hiding this comment

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

let's keep the original then and ALSO add this test case, this way we're covered for:

  • internal
  • external single component files
  • external multiple component files
    what do you think?

}
}, discriminator);
}, discriminator);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
propertyName: pet_type
mapping:
puppy: '#/components/schemas/Dog'
kitten: Cat
kitten: Cat
monster: https://gigantic-server.com/schemas/Monster/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ namespace Microsoft.OpenApi.Models
public OpenApiDiscriminator() { }
public OpenApiDiscriminator(Microsoft.OpenApi.Models.OpenApiDiscriminator discriminator) { }
public System.Collections.Generic.IDictionary<string, Microsoft.OpenApi.Interfaces.IOpenApiExtension>? Extensions { get; set; }
public System.Collections.Generic.IDictionary<string, string>? Mapping { get; set; }
public System.Collections.Generic.IDictionary<string, Microsoft.OpenApi.Models.References.OpenApiSchemaReference>? Mapping { get; set; }
public string? PropertyName { get; set; }
public void SerializeAsV2(Microsoft.OpenApi.Writers.IOpenApiWriter writer) { }
public void SerializeAsV3(Microsoft.OpenApi.Writers.IOpenApiWriter writer) { }
Expand Down