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 the required + nullable issue #3373

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
1dc3b5a
start over
ArcturusZhang Jun 8, 2023
fcfdbc1
regen
ArcturusZhang Jun 8, 2023
4e0f335
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jun 8, 2023
a850392
fix the property bag issue
ArcturusZhang Jun 9, 2023
af053b5
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jun 9, 2023
fd36ade
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jun 12, 2023
b750ffc
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jun 14, 2023
8b134cd
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jun 14, 2023
a78427e
add test cases and fix a minor issue
ArcturusZhang Jun 14, 2023
6b03de5
refactor test cases
ArcturusZhang Jun 14, 2023
c86c9e2
apply the change michael requested
ArcturusZhang Jun 15, 2023
c1e13ff
fix issues in test cases
ArcturusZhang Jun 15, 2023
f4b676e
fix the issue in property bag to keep them unchanged
ArcturusZhang Jun 15, 2023
058ead2
fixed failed test cases
ArcturusZhang Jun 15, 2023
2556315
regen
ArcturusZhang Jun 15, 2023
05a6ca4
fixing more test cases
ArcturusZhang Jun 15, 2023
c2f1c99
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jun 15, 2023
6040299
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jun 26, 2023
7ec472a
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jun 28, 2023
97ab064
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jun 29, 2023
ee61d1d
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jul 3, 2023
b8eff43
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jul 10, 2023
f15fe71
add a configuration
ArcturusZhang Jul 10, 2023
f523844
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jul 11, 2023
9ebf0fb
fix after merge
ArcturusZhang Jul 12, 2023
e9ccc43
make the configuration working
ArcturusZhang Jul 12, 2023
9cd370f
add a test case for this
ArcturusZhang Jul 12, 2023
336fb74
format
ArcturusZhang Jul 12, 2023
f45fc4c
fix test cases
ArcturusZhang Jul 12, 2023
715b28c
change the default value to undefined in order not to pollute the con…
ArcturusZhang Jul 12, 2023
99bc10b
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jul 13, 2023
f97cd53
resolve comments
ArcturusZhang Jul 14, 2023
10ac934
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jul 14, 2023
86ba9a5
Merge remote-tracking branch 'origin/feature/v3' into fix-required-nu…
ArcturusZhang Jul 17, 2023
af4690e
add a test case for spread collections
ArcturusZhang Jul 18, 2023
279abdd
Merge branch 'feature/v3' into fix-required-nullable-property-issue
ArcturusZhang Jul 21, 2023
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: 13 additions & 0 deletions src/AutoRest.CSharp/Common/AutoRest/Plugins/Configuration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public static class Options
public const string PublicDiscriminatorProperty = "public-discriminator-property";
public const string ShouldTreatBase64AsBinaryData = "should-treat-base64-as-binary-data";
public const string MethodsToKeepClientDefaultValue = "methods-to-keep-client-default-value";
public const string DeserializeNullCollectionAsNullValue = "deserialize-null-collection-as-null-value";
public const string UseCoreDataFactoryReplacements = "use-core-datafactory-replacements";
}

Expand Down Expand Up @@ -76,6 +77,7 @@ public static void Initialize(
bool disablePaginationTopRenaming,
bool generateModelFactory,
bool publicDiscriminatorProperty,
bool deserializeNullCollectionAsNullValue,
bool useCoreDataFactoryReplacements,
IReadOnlyList<string> modelFactoryForHlc,
UnreferencedTypesHandlingOption unreferencedTypesHandling,
Expand Down Expand Up @@ -106,6 +108,7 @@ public static void Initialize(
SingleTopLevelClient = singleTopLevelClient;
GenerateModelFactory = generateModelFactory;
PublicDiscriminatorProperty = publicDiscriminatorProperty;
DeserializeNullCollectionAsNullValue = deserializeNullCollectionAsNullValue;
UnreferencedTypesHandling = unreferencedTypesHandling;
UseOverloadsBetweenProtocolAndConvenience = useOverloadsBetweenProtocolAndConvenience;
KeepNonOverloadableProtocolSignature = keepNonOverloadableProtocolSignature;
Expand Down Expand Up @@ -237,6 +240,12 @@ public static void Initialize(
/// If true, the discriminator property will be public. If false (default), the discriminator property will be internal.
/// </summary>
public static bool PublicDiscriminatorProperty { get; private set; }

/// <summary>
/// Whether we should deserialize null collections in the payload as null values if this sets to true.
/// Default value is false, where we will construct an empty collection (ChangeTrackingList or ChangeTrackingDictionary) if we get null value for collections in the payload
/// </summary>
public static bool DeserializeNullCollectionAsNullValue { get; private set; }
public static bool UseOverloadsBetweenProtocolAndConvenience { get; private set; }
public static bool KeepNonOverloadableProtocolSignature { get; private set; }

Expand Down Expand Up @@ -288,6 +297,7 @@ public static void Initialize(IPluginCommunication autoRest, string defaultNames
disablePaginationTopRenaming: GetOptionBoolValue(autoRest, Options.DisablePaginationTopRenaming),
generateModelFactory: GetOptionBoolValue(autoRest, Options.GenerateModelFactory),
publicDiscriminatorProperty: GetOptionBoolValue(autoRest, Options.PublicDiscriminatorProperty),
deserializeNullCollectionAsNullValue: GetOptionBoolValue(autoRest, Options.DeserializeNullCollectionAsNullValue),
modelFactoryForHlc: autoRest.GetValue<string[]?>(Options.ModelFactoryForHlc).GetAwaiter().GetResult() ?? Array.Empty<string>(),
unreferencedTypesHandling: GetOptionEnumValue<UnreferencedTypesHandlingOption>(autoRest, Options.UnreferencedTypesHandling),
useOverloadsBetweenProtocolAndConvenience: GetOptionBoolValue(autoRest, Options.UseOverloadsBetweenProtocolAndConvenience),
Expand Down Expand Up @@ -367,6 +377,8 @@ private static bool GetOptionBoolValue(IPluginCommunication autoRest, string opt
return false;
case Options.ShouldTreatBase64AsBinaryData:
return true;
case Options.DeserializeNullCollectionAsNullValue:
return false;
case Options.UseCoreDataFactoryReplacements:
return true;
default:
Expand Down Expand Up @@ -438,6 +450,7 @@ internal static void LoadConfiguration(JsonElement root, string? projectPath, st
ReadOption(root, Options.DisablePaginationTopRenaming),
ReadOption(root, Options.GenerateModelFactory),
ReadOption(root, Options.PublicDiscriminatorProperty),
ReadOption(root, Options.DeserializeNullCollectionAsNullValue),
ReadOption(root, Options.UseCoreDataFactoryReplacements),
oldModelFactoryEntries,
ReadEnumOption<UnreferencedTypesHandlingOption>(root, Options.UnreferencedTypesHandling),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text.Json;
using AutoRest.CSharp.Common.Output.Models;
using AutoRest.CSharp.Generation.Types;
using AutoRest.CSharp.Output.Models;
using AutoRest.CSharp.Output.Models.Requests;
Expand All @@ -15,11 +17,9 @@
using AutoRest.CSharp.Output.Models.Types;
using AutoRest.CSharp.Utilities;
using Azure;
using AutoRest.CSharp.Common.Output.Models;
using Azure.Core;
using static AutoRest.CSharp.Output.Models.MethodSignatureModifiers;
using System.Text.Json;
using Azure.Core.Pipeline;
using static AutoRest.CSharp.Output.Models.MethodSignatureModifiers;

namespace AutoRest.CSharp.Generation.Writers
{
Expand Down Expand Up @@ -143,7 +143,7 @@ public static CodeWriter WriteFieldDeclarations(this CodeWriter writer, IEnumera

public static IDisposable WriteMethodDeclaration(this CodeWriter writer, MethodSignatureBase methodBase, params string[] disabledWarnings)
{
if (methodBase.Attributes is {} attributes)
if (methodBase.Attributes is { } attributes)
{
foreach (var attribute in attributes)
{
Expand Down Expand Up @@ -620,6 +620,9 @@ internal static string GetConversion(CodeWriter writer, CSharpType from, CSharpT
}

var propertyName = property.PropertyName;
if (TypeFactory.IsCollectionType(property.ValueType) && property.IsRequired)
return writer.Scope($"if ({propertyName} != null && {typeof(Optional)}.{nameof(Optional.IsCollectionDefined)}({propertyName}))");

return writer.Scope($"if ({propertyName} != null)");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,24 @@ private static void DeserializeIntoObjectProperties(this CodeWriter writer, IEnu
var emptyStringCheck = GetEmptyStringCheckClause(property, itemVariable, shouldTreatEmptyStringAsNull);
using (writer.Scope($"if ({itemVariable}.Value.ValueKind == {typeof(JsonValueKind)}.Null{emptyStringCheck})"))
{
writer.Line($"{propertyVariables[property].Declaration} = null;");
if (Configuration.DeserializeNullCollectionAsNullValue)
{
// we will assign null to everything if we have this configuration on
writer.Line($"{propertyVariables[property].Declaration} = null;");
}
else
{
// we only assign null when it is not a collection if we have this configuration off
if (!TypeFactory.IsCollectionType(property.ValueType))
{
writer.Line($"{propertyVariables[property].Declaration} = null;");
}
else if (property.IsRequired)
{
// specially when it is required, we assign ChangeTrackingList because for optional lists we are already doing that
writer.Line($"{propertyVariables[property].Declaration} = new {TypeFactory.GetPropertyImplementationType(property.ValueType)}();");
}
}
writer.Append($"continue;");
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/AutoRest.CSharp/Common/Input/CodeModelConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ private InputModelType GetOrCreateModel(ObjectSchema schema, SchemaUsageProvider
: null,
DerivedModels: derived,
DiscriminatorValue: schema.DiscriminatorValue,
DiscriminatorPropertyName: schema.Discriminator?.Property.SerializedName);
DiscriminatorPropertyName: schema.Discriminator?.Property.SerializedName,
IsNullable: false);

_modelsCache[schema] = model;
_modelPropertiesCache[schema] = properties;
Expand Down Expand Up @@ -380,8 +381,8 @@ private static InputType CreateType(Schema schema, string? format, IReadOnlyDict
ChoiceSchema choiceSchema => CreateEnumType(choiceSchema, choiceSchema.ChoiceType, choiceSchema.Choices, true),
SealedChoiceSchema choiceSchema => CreateEnumType(choiceSchema, choiceSchema.ChoiceType, choiceSchema.Choices, false),

ArraySchema array when IsDPG => new InputListType(array.Name, CreateType(array.ElementType, modelsCache, array.NullableItems ?? false)),
DictionarySchema dictionary when IsDPG => new InputDictionaryType(dictionary.Name, InputPrimitiveType.String, CreateType(dictionary.ElementType, modelsCache, dictionary.NullableItems ?? false)),
ArraySchema array when IsDPG => new InputListType(array.Name, CreateType(array.ElementType, modelsCache, array.NullableItems ?? false), false),
DictionarySchema dictionary when IsDPG => new InputDictionaryType(dictionary.Name, InputPrimitiveType.String, CreateType(dictionary.ElementType, modelsCache, dictionary.NullableItems ?? false), false),
ObjectSchema objectSchema when IsDPG && modelsCache != null => modelsCache[objectSchema],

AnySchema when IsDPG => InputIntrinsicType.Unknown,
Expand Down Expand Up @@ -411,7 +412,7 @@ private static InputLiteralType CreateLiteralType(ConstantSchema constantSchema,
_ => rawValue
};

return new InputLiteralType("Literal", valueType, normalizedValue);
return new InputLiteralType("Literal", valueType, normalizedValue, false);
}

public static InputEnumType CreateEnumType(Schema schema, PrimitiveSchema choiceType, IEnumerable<ChoiceValue> choices, bool isExtensible) => new(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@

namespace AutoRest.CSharp.Common.Input;

internal record InputDictionaryType(string Name, InputType KeyType, InputType ValueType, bool IsNullable = false) : InputType(Name, IsNullable) { }
internal record InputDictionaryType(string Name, InputType KeyType, InputType ValueType, bool IsNullable) : InputType(Name, IsNullable) { }
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@

namespace AutoRest.CSharp.Common.Input;

internal record InputListType(string Name, InputType ElementType, bool IsNullable = false) : InputType(Name, IsNullable) { }
internal record InputListType(string Name, InputType ElementType, bool IsNullable) : InputType(Name, IsNullable) { }
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@

namespace AutoRest.CSharp.Common.Input;

internal record InputLiteralType(string Name, InputType LiteralValueType, object Value, bool IsNullable = false) : InputType(Name, IsNullable);
internal record InputLiteralType(string Name, InputType LiteralValueType, object Value, bool IsNullable) : InputType(Name, IsNullable);
4 changes: 2 additions & 2 deletions src/AutoRest.CSharp/Common/Input/InputTypes/InputModelType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

namespace AutoRest.CSharp.Common.Input
{
internal record InputModelType(string Name, string? Namespace, string? Accessibility, string? Deprecated, string? Description, InputModelTypeUsage Usage, IReadOnlyList<InputModelProperty> Properties, InputModelType? BaseModel, IReadOnlyList<InputModelType> DerivedModels, string? DiscriminatorValue, string? DiscriminatorPropertyName)
: InputType(Name)
internal record InputModelType(string Name, string? Namespace, string? Accessibility, string? Deprecated, string? Description, InputModelTypeUsage Usage, IReadOnlyList<InputModelProperty> Properties, InputModelType? BaseModel, IReadOnlyList<InputModelType> DerivedModels, string? DiscriminatorValue, string? DiscriminatorPropertyName, bool IsNullable)
: InputType(Name, IsNullable)
{
/// <summary>
/// Indicates if this model is the Unknown derived version of a model with discriminator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public InputParameter() : this(
Name: string.Empty,
NameInRequest: string.Empty,
Description: null,
Type: new InputPrimitiveType(InputTypeKind.Object),
Type: new InputPrimitiveType(InputTypeKind.Object, false),
Location: RequestLocation.None,
DefaultValue: null,
VirtualParameter: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

namespace AutoRest.CSharp.Common.Input;

internal record InputPrimitiveType(InputTypeKind Kind, bool IsNullable = false) : InputType(Kind.ToString(), IsNullable)
internal record InputPrimitiveType(InputTypeKind Kind, bool IsNullable) : InputType(Kind.ToString(), IsNullable)
{
private InputPrimitiveType(InputTypeKind kind) : this(kind, false) { }

public static InputPrimitiveType AzureLocation { get; } = new(InputTypeKind.AzureLocation);
public static InputPrimitiveType BinaryData { get; } = new(InputTypeKind.BinaryData);
public static InputPrimitiveType Boolean { get; } = new(InputTypeKind.Boolean);
Expand Down
2 changes: 1 addition & 1 deletion src/AutoRest.CSharp/Common/Input/InputTypes/InputType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@

namespace AutoRest.CSharp.Common.Input;

internal abstract record InputType(string Name, bool IsNullable = false) { }
internal abstract record InputType(string Name, bool IsNullable) { }
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@

namespace AutoRest.CSharp.Common.Input;

internal record InputUnionType(string Name, IReadOnlyList<InputType> UnionItemTypes, bool IsNullable = false) : InputType(Name, IsNullable);
internal record InputUnionType(string Name, IReadOnlyList<InputType> UnionItemTypes, bool IsNullable) : InputType(Name, IsNullable);
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ public override void Write(Utf8JsonWriter writer, InputDictionaryType value, Jso
public static InputDictionaryType CreateDictionaryType(ref Utf8JsonReader reader, string? id, string? name, JsonSerializerOptions options)
{
var isFirstProperty = id == null && name == null;
bool isNullable = false;
InputType? keyType = null;
InputType? valueType = null;
while (reader.TokenType != JsonTokenType.EndObject)
{
var isKnownProperty = reader.TryReadReferenceId(ref isFirstProperty, ref id)
|| reader.TryReadString(nameof(InputListType.Name), ref name)
|| reader.TryReadString(nameof(InputDictionaryType.Name), ref name)
|| reader.TryReadBoolean(nameof(InputDictionaryType.IsNullable), ref isNullable)
|| reader.TryReadWithConverter(nameof(InputDictionaryType.KeyType), options, ref keyType)
|| reader.TryReadWithConverter(nameof(InputDictionaryType.ValueType), options, ref valueType);

Expand All @@ -43,7 +45,7 @@ public static InputDictionaryType CreateDictionaryType(ref Utf8JsonReader reader
keyType = keyType ?? throw new JsonException("Dictionary must have key type");
valueType = valueType ?? throw new JsonException("Dictionary must have value type");

return new InputDictionaryType(name ?? "Dictionary", keyType, valueType);
return new InputDictionaryType(name ?? "Dictionary", keyType, valueType, isNullable);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public override void Write(Utf8JsonWriter writer, InputEnumType value, JsonSeria
public static InputEnumType CreateEnumType(ref Utf8JsonReader reader, string? id, string? name, JsonSerializerOptions options, ReferenceResolver resolver)
{
var isFirstProperty = id == null && name == null;
bool isNullable = false;
string? ns = null;
string? accessibility = null;
string? deprecated = null;
Expand All @@ -39,6 +40,7 @@ public static InputEnumType CreateEnumType(ref Utf8JsonReader reader, string? id
{
var isKnownProperty = reader.TryReadReferenceId(ref isFirstProperty, ref id)
|| reader.TryReadString(nameof(InputEnumType.Name), ref name)
|| reader.TryReadBoolean(nameof(InputEnumType.IsNullable), ref isNullable)
|| reader.TryReadString(nameof(InputEnumType.Namespace), ref ns)
|| reader.TryReadString(nameof(InputEnumType.Accessibility), ref accessibility)
|| reader.TryReadString(nameof(InputEnumType.Deprecated), ref deprecated)
Expand Down Expand Up @@ -98,7 +100,7 @@ public static InputEnumType CreateEnumType(ref Utf8JsonReader reader, string? id
}
valueType = currentType ?? throw new JsonException("Enum value type must be set.");

var enumType = new InputEnumType(name, ns, accessibility, deprecated, description, usage, valueType, NormalizeValues(allowedValues, valueType), isExtendable);
var enumType = new InputEnumType(name, ns, accessibility, deprecated, description, usage, valueType, NormalizeValues(allowedValues, valueType), isExtendable, IsNullable: isNullable);
if (id != null)
{
resolver.AddReference(id, enumType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ public override void Write(Utf8JsonWriter writer, InputListType value, JsonSeria
public static InputListType CreateListType(ref Utf8JsonReader reader, string? id, string? name, JsonSerializerOptions options)
{
var isFirstProperty = id == null && name == null;
bool isNullable = false;
InputType? elementType = null;
while (reader.TokenType != JsonTokenType.EndObject)
{
var isKnownProperty = reader.TryReadReferenceId(ref isFirstProperty, ref id)
|| reader.TryReadString(nameof(InputListType.Name), ref name)
|| reader.TryReadBoolean(nameof(InputListType.IsNullable), ref isNullable)
|| reader.TryReadWithConverter(nameof(InputListType.ElementType), options, ref elementType);

if (!isKnownProperty)
Expand All @@ -39,7 +41,7 @@ public static InputListType CreateListType(ref Utf8JsonReader reader, string? id
}

elementType = elementType ?? throw new JsonException("List must have element type");
return new InputListType(name ?? "List", elementType);
return new InputListType(name ?? "List", elementType, isNullable);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ public override void Write(Utf8JsonWriter writer, InputLiteralType value, JsonSe
public static InputLiteralType CreateInputLiteralType(ref Utf8JsonReader reader, string? id, string? name, JsonSerializerOptions options, ReferenceResolver resolver)
{
var isFirstProperty = id == null && name == null;
bool isNullable = false;
Object? value = null;
InputType? type = null;

while (reader.TokenType != JsonTokenType.EndObject)
{
var isKnownProperty = reader.TryReadReferenceId(ref isFirstProperty, ref id)
|| reader.TryReadString(nameof(InputListType.Name), ref name)
|| reader.TryReadBoolean(nameof(InputListType.IsNullable), ref isNullable)
|| reader.TryReadWithConverter(nameof(InputLiteralType.LiteralValueType), options, ref type);

if (isKnownProperty)
Expand All @@ -56,7 +58,7 @@ public static InputLiteralType CreateInputLiteralType(ref Utf8JsonReader reader,

value = value ?? throw new JsonException("InputConstant must have value");

var literalType = new InputLiteralType(name, type, value);
var literalType = new InputLiteralType(name, type, value, isNullable);

if (id != null)
{
Expand Down
Loading