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

[http-client-csharp] fix: gen. unknown discriminator model when discriminator prop exists #4936

Original file line number Diff line number Diff line change
Expand Up @@ -442,17 +442,36 @@ internal MethodProvider BuildDeserializationMethod()
private MethodBodyStatement[] BuildAbstractDeserializationMethodBody()
{
var unknownVariant = _model.DerivedModels.First(m => m.IsUnknownDiscriminatorModel);
bool onlyContainsUnknownDerivedModel = _model.DerivedModels.Count == 1;
var deserializeDiscriminatedModelsConditions = BuildDiscriminatedModelsCondition(
GetAbstractSwitchCases(unknownVariant),
onlyContainsUnknownDerivedModel,
_jsonElementParameterSnippet);

return
[
new IfStatement(_jsonElementParameterSnippet.ValueKindEqualsNull()) { Return(Null) },
new IfStatement(_jsonElementParameterSnippet.TryGetProperty("kind", out var discriminator))
{
new SwitchStatement(discriminator.GetString(), GetAbstractSwitchCases(unknownVariant))
},
deserializeDiscriminatedModelsConditions,
Return(unknownVariant.Type.Deserialize(_jsonElementParameterSnippet, _serializationOptionsParameter))
];
}

private static MethodBodyStatement BuildDiscriminatedModelsCondition(
SwitchCaseStatement[] abstractSwitchCases,
bool onlyContainsUnknownDerivedModel,
ScopedApi<JsonElement> jsonElementParameterSnippet)
{
if (!onlyContainsUnknownDerivedModel)
jorgerangel-msft marked this conversation as resolved.
Show resolved Hide resolved
{
return new IfStatement(jsonElementParameterSnippet.TryGetProperty("kind", out var discriminator))
{
new SwitchStatement(discriminator.GetString(), abstractSwitchCases)
};
}

return MethodBodyStatement.Empty;
}

private SwitchCaseStatement[] GetAbstractSwitchCases(ModelProvider unknownVariant)
{
SwitchCaseStatement[] cases = new SwitchCaseStatement[_model.DerivedModels.Count - 1];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Diagnostics;

Expand All @@ -9,6 +10,7 @@ namespace Microsoft.Generator.CSharp.Input
[DebuggerDisplay("{GetDebuggerDisplay(),nq}")]
public class InputModelType : InputType
{
private const string UnknownDiscriminatorValue = "unknown";
private IReadOnlyList<InputModelProperty> _properties = [];
private IList<InputModelType> _derivedModels = [];

Expand Down Expand Up @@ -38,7 +40,7 @@ public InputModelType(string name, string crossLanguageDefinitionId, string? acc
DiscriminatorProperty = discriminatorProperty;
DiscriminatedSubtypes = discriminatedSubtypes!;
AdditionalProperties = additionalProperties;
IsUnknownDiscriminatorModel = DiscriminatorValue == "unknown";
IsUnknownDiscriminatorModel = DiscriminatorValue == UnknownDiscriminatorValue;
IsPropertyBag = false;
ModelAsStruct = modelAsStruct;
}
Expand Down Expand Up @@ -79,35 +81,36 @@ public IReadOnlyDictionary<string, InputModelType> DiscriminatedSubtypes
get => _discriminatedSubtypes ??= new Dictionary<string, InputModelType>();
internal set
{
if (value is null || value.Count == 0)
if (value is null || DiscriminatorProperty == null || DiscriminatorValue == UnknownDiscriminatorValue)
return;

_discriminatedSubtypes = new Dictionary<string, InputModelType>(value);

var cleanBaseName = Name.ToCleanName();
_discriminatedSubtypes.Add("unknown",
new InputModelType(
$"Unknown{cleanBaseName}",
$"Unknown{cleanBaseName}",
"internal",
null,
$"Unknown variant of {cleanBaseName}",
Usage | InputModelTypeUsage.Json,
[],
this,
[],
"unknown",
new InputModelProperty(
DiscriminatorProperty!.Name,
DiscriminatorProperty.SerializedName,
DiscriminatorProperty.Description,
DiscriminatorProperty.Type,
DiscriminatorProperty.IsRequired,
DiscriminatorProperty.IsReadOnly,
DiscriminatorProperty.IsDiscriminator),
new Dictionary<string, InputModelType>(),
null,
false)
);
_discriminatedSubtypes.Add(UnknownDiscriminatorValue,
new InputModelType(
$"Unknown{cleanBaseName}",
$"Unknown{cleanBaseName}",
"internal",
null,
$"Unknown variant of {cleanBaseName}",
Usage | InputModelTypeUsage.Json,
[],
this,
[],
UnknownDiscriminatorValue,
new InputModelProperty(
DiscriminatorProperty!.Name,
DiscriminatorProperty.SerializedName,
DiscriminatorProperty.Description,
DiscriminatorProperty.Type,
DiscriminatorProperty.IsRequired,
DiscriminatorProperty.IsReadOnly,
DiscriminatorProperty.IsDiscriminator),
new Dictionary<string, InputModelType>(),
null,
false)
);
}
}
public InputType? AdditionalProperties { get; internal set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ public static InputModelType CreateModelType(ref Utf8JsonReader reader, string?
{
model.DiscriminatedSubtypes = discriminatedSubtypes;
}
else if (model.DiscriminatorProperty != null)
{
model.DiscriminatedSubtypes = new Dictionary<string, InputModelType>();
}
model.ModelAsStruct = modelAsStruct;
if (decorators != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private static InputModelType[] GetTestModels()
values: [InputFactory.EnumMember.String("foo", "bar")]), isDiscriminator: true)
}).ToArray();

var derivedModel = InputFactory.Model("DerivedModel", properties: inheritanceProperties, discriminatedKind: "unknown");
var derivedModel = InputFactory.Model("DerivedModel", properties: inheritanceProperties, discriminatedKind: "foo");
return
[
InputFactory.Model("InternalModel", "internal", properties: properties),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
Expand All @@ -16,6 +17,24 @@ namespace Microsoft.Generator.CSharp.Tests.Providers.ModelProviders
{
public class DiscriminatorTests
{
private static readonly InputModelType _dinosaurModel = InputFactory.Model("dinosaur", discriminatedKind: "dinosaur", properties:
[
InputFactory.Property("type", InputPrimitiveType.String, isRequired: true),
InputFactory.Property("dinosaurKind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true)
]);

private static readonly InputModelType _animalModel = InputFactory.Model(
"animal",
properties:
[
InputFactory.Property("type", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
InputFactory.Property("name", InputPrimitiveType.Boolean, isRequired: true)
],
discriminatedModels: new Dictionary<string, InputModelType>()
{
{ "dinosaur", _dinosaurModel }
});

private static readonly InputModelType _catModel = InputFactory.Model("cat", discriminatedKind: "cat", properties:
[
InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
Expand Down Expand Up @@ -149,9 +168,38 @@ public void CreateUnknownVariant()
MockHelpers.LoadMockPlugin(inputModelTypes: [_baseModel, _catModel, _dogModel]);
var outputLibrary = CodeModelPlugin.Instance.OutputLibrary;
var models = outputLibrary.TypeProviders.OfType<ModelProvider>();
Assert.AreEqual(6, models.Count());
// since each model has a discriminator, there should be 3 additional models for their unknown variants
var unknownPet = models.FirstOrDefault(t => t.Name == "UnknownPet");
Assert.IsNotNull(unknownPet);
var unknownCat = models.FirstOrDefault(t => t.Name == "UnknownCat");
Assert.IsNotNull(unknownCat);
var unknownDog = models.FirstOrDefault(t => t.Name == "UnknownDog");
Assert.IsNotNull(unknownDog);
}

// This test validates that a nested discriminated model with its' own discriminator property will have an unknown variant
[Test]
public void DiscriminatedModelWithNoSubTypesHasUnknownVariant()
{
MockHelpers.LoadMockPlugin(inputModelTypes: [_animalModel, _dinosaurModel]);
var outputLibrary = CodeModelPlugin.Instance.OutputLibrary;
var models = outputLibrary.TypeProviders.OfType<ModelProvider>();
Assert.AreEqual(4, models.Count());
var unknownModel = models.FirstOrDefault(t => t.Name == "UnknownPet");
Assert.IsNotNull(unknownModel);

var animalModel = models.FirstOrDefault(t => t.Name == "Animal");
Assert.IsNotNull(animalModel);
Assert.IsNull(animalModel!.BaseModelProvider);
var unknownAnimal = models.FirstOrDefault(t => t.Name == "UnknownAnimal");
Assert.IsNotNull(unknownAnimal);
Assert.AreEqual(animalModel, unknownAnimal!.BaseModelProvider);

var dinosaurModel = models.FirstOrDefault(t => t.Name == "Dinosaur");
Assert.IsNotNull(dinosaurModel);
Assert.AreEqual(animalModel, dinosaurModel!.BaseModelProvider);
var unknownDinosaur = models.FirstOrDefault(t => t.Name == "UnknownDinosaur");
Assert.IsNotNull(unknownDinosaur);
Assert.AreEqual(dinosaurModel, unknownDinosaur!.BaseModelProvider);
}

[Test]
Expand All @@ -174,6 +222,32 @@ public void UnknownVariantHasPetBase()
Assert.AreEqual("Pet", unknownModel!.Type.BaseType!.Name);
}

// This test validates that a discriminator model whose discriminator value is "unknown" will throw
[Test]
public void DiscriminatedModelWithUnknownValueThrows()
{
Assert.Throws<ArgumentException>(() =>
{
var unknownPlantModel = InputFactory.Model(
"unknownPlant", discriminatedKind: "unknown", properties:
[
InputFactory.Property("type", InputPrimitiveType.String, isRequired: true),
]);

var plantModel = InputFactory.Model(
"plant",
properties:
[
InputFactory.Property("type", InputPrimitiveType.String, isRequired: true, isDiscriminator: true),
InputFactory.Property("name", InputPrimitiveType.Boolean, isRequired: true)
],
discriminatedModels: new Dictionary<string, InputModelType>()
{
{ "unknown", unknownPlantModel }
});
});
}

[Test]
public void SerializationCtorForUnknownTakesDiscriminator()
{
Expand Down
Loading