Skip to content

Commit

Permalink
[http-client-csharp] fix: gen. unknown discriminator model when discr…
Browse files Browse the repository at this point in the history 
…iminator prop exists (#4936)

fixes: #4935
  • Loading branch information
@jorgerangel-msft
jorgerangel-msft authored Nov 1, 2024
1 parent 220769e commit b7d351b
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 33 deletions.
27 changes: 23 additions & 4 deletions ...or/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs
View file
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)
{
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
55 changes: 29 additions & 26 deletions ...client-csharp/generator/Microsoft.Generator.CSharp.Input/src/InputTypes/InputModelType.cs
View file
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
4 changes: 4 additions & 0 deletions ...ft.Generator.CSharp.Input/src/InputTypes/Serialization/TypeSpecInputModelTypeConverter.cs
View file
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
2 changes: 1 addition & 1 deletion ...tor/Microsoft.Generator.CSharp/test/Providers/ModelFactories/ModelFactoryProviderTests.cs
View file
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
78 changes: 76 additions & 2 deletions .../generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/DiscriminatorTests.cs
View file
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

0 comments on commit b7d351b

Please sign in to comment.