Skip to content

Commit

Permalink
refactor(http-client-csharp): do not flatten property in emitter (#4449)
Browse files Browse the repository at this point in the history
Per discussion, we thought we should not flatten properties in emitter,
to keep it as simple as possible.
The flattening logic should be implemented in generator.

part of Azure/autorest.csharp#4788

---------

Co-authored-by: Mingzhe Huang (from Dev Box) <mingzhehuang@microsoft.com>
  • Loading branch information
archerzz and Mingzhe Huang (from Dev Box) authored Oct 9, 2024
1 parent 7aee12f commit c78c614
Show file tree
Hide file tree
Showing 28 changed files with 345 additions and 71 deletions.
93 changes: 35 additions & 58 deletions packages/http-client-csharp/emitter/src/lib/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,20 @@ export function fromSdkModelType(
? fromSdkType(modelType.additionalProperties, context, typeMap)
: undefined;

const propertiesDict = new Map<SdkModelPropertyType, InputModelProperty[]>();
const propertiesDict = new Map<SdkModelPropertyType, InputModelProperty>();
for (const property of modelType.properties) {
if (property.kind !== "property") {
continue;
}
const ourProperties = fromSdkModelProperty(
property,
{
ModelName: modelTypeName,
Usage: modelType.usage,
} as LiteralTypeContext,
[],
);
propertiesDict.set(property, ourProperties);
const ourProperty = fromSdkModelProperty(property, {
ModelName: modelTypeName,
Usage: modelType.usage,
} as LiteralTypeContext);
propertiesDict.set(property, ourProperty);
}

inputModelType.discriminatorProperty = modelType.discriminatorProperty
? propertiesDict.get(modelType.discriminatorProperty)![0]
? propertiesDict.get(modelType.discriminatorProperty)
: undefined;

inputModelType.baseModel = modelType.baseModel
Expand All @@ -174,56 +170,37 @@ export function fromSdkModelType(
function fromSdkModelProperty(
property: SdkBodyModelPropertyType,
literalTypeContext: LiteralTypeContext,
flattenedNamePrefixes: string[],
): InputModelProperty[] {
// TODO -- we should consolidate the flatten somewhere else
if (!property.flatten) {
/* remove this when https://github.com/Azure/typespec-azure/issues/1483 and https://github.com/Azure/typespec-azure/issues/1488 are resolved. */
let targetType = property.type;
if (targetType.kind === "model") {
const body = targetType.properties.find((x) => x.kind === "body");
if (body) targetType = body.type;
}

const serializedName = property.serializedName;
literalTypeContext.PropertyName = serializedName;

const modelProperty: InputModelProperty = {
kind: property.kind,
name: property.name,
serializedName: serializedName,
description: property.description,
type: fromSdkType(
targetType,
context,
typeMap,
property.discriminator ? undefined : literalTypeContext, // this is a workaround because the type of discriminator property in derived models is always literal and we wrap literal into enums, which leads to a lot of extra enum types, adding this check to avoid them
),
optional: property.optional,
readOnly: isReadOnly(property), // TODO -- we might pass the visibility through and then check if there is only read to know if this is readonly
discriminator: property.discriminator,
flattenedNames:
flattenedNamePrefixes.length > 0
? flattenedNamePrefixes.concat(property.name)
: undefined,
decorators: property.decorators,
crossLanguageDefinitionId: property.crossLanguageDefinitionId,
};

return [modelProperty];
): InputModelProperty {
/* remove this when https://github.com/Azure/typespec-azure/issues/1483 and https://github.com/Azure/typespec-azure/issues/1488 are resolved. */
let targetType = property.type;
if (targetType.kind === "model") {
const body = targetType.properties.find((x) => x.kind === "body");
if (body) targetType = body.type;
}

const flattenedProperties: InputModelProperty[] = [];
const childPropertiesToFlatten = (property.type as SdkModelType).properties;
const newFlattenedNamePrefixes = flattenedNamePrefixes.concat(property.serializedName);
for (const childProperty of childPropertiesToFlatten) {
if (childProperty.kind !== "property") continue;
flattenedProperties.push(
...fromSdkModelProperty(childProperty, literalTypeContext, newFlattenedNamePrefixes),
);
}
const serializedName = property.serializedName;
literalTypeContext.PropertyName = serializedName;

const modelProperty: InputModelProperty = {
kind: property.kind,
name: property.name,
serializedName: serializedName,
description: property.description,
type: fromSdkType(
targetType,
context,
typeMap,
property.discriminator ? undefined : literalTypeContext, // this is a workaround because the type of discriminator property in derived models is always literal and we wrap literal into enums, which leads to a lot of extra enum types, adding this check to avoid them
),
optional: property.optional,
readOnly: isReadOnly(property), // TODO -- we might pass the visibility through and then check if there is only read to know if this is readonly
discriminator: property.discriminator,
flatten: property.flatten,
decorators: property.decorators,
crossLanguageDefinitionId: property.crossLanguageDefinitionId,
};

return flattenedProperties;
return modelProperty;
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/http-client-csharp/emitter/src/type/input-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export interface InputModelProperty extends InputTypeBase {
readOnly: boolean;
discriminator: boolean;
crossLanguageDefinitionId: string;
flattenedNames?: string[]; // TODO -- remove this when we are ready to move the flatten handling from emitter to the generator
flatten: boolean;
}

export function isInputModelType(type: InputType): type is InputModelType {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;

namespace Microsoft.Generator.CSharp.Input
{
public class InputModelProperty
{
public InputModelProperty(string name, string serializedName, string? description, InputType type, bool isRequired, bool isReadOnly, bool isDiscriminator, IReadOnlyList<string>? flattenedNames = null)
public InputModelProperty(string name, string serializedName, string? description, InputType type, bool isRequired, bool isReadOnly, bool isDiscriminator)
{
Name = name;
SerializedName = serializedName;
Expand All @@ -16,7 +17,6 @@ public InputModelProperty(string name, string serializedName, string? descriptio
IsRequired = isRequired;
IsReadOnly = isReadOnly;
IsDiscriminator = isDiscriminator;
FlattenedNames = flattenedNames ?? [];
}

public string Name { get; }
Expand All @@ -27,7 +27,6 @@ public InputModelProperty(string name, string serializedName, string? descriptio
public bool IsReadOnly { get; }
public bool IsDiscriminator { get; }
public InputModelType? EnclosingType { get; internal set; }
public IReadOnlyList<string> FlattenedNames { get; }
public IReadOnlyList<InputDecoratorInfo> Decorators { get; internal set; } = new List<InputDecoratorInfo>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ internal set
DiscriminatorProperty.Type,
DiscriminatorProperty.IsRequired,
DiscriminatorProperty.IsReadOnly,
DiscriminatorProperty.IsDiscriminator,
DiscriminatorProperty.FlattenedNames),
DiscriminatorProperty.IsDiscriminator),
new Dictionary<string, InputModelType>(),
null,
false)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
Expand Down Expand Up @@ -33,7 +33,6 @@ private static InputModelProperty ReadInputModelProperty(ref Utf8JsonReader read
bool isOptional = false;
bool isDiscriminator = false;
IReadOnlyList<InputDecoratorInfo>? decorators = null;
IReadOnlyList<string>? flattenedNames = null;

while (reader.TokenType != JsonTokenType.EndObject)
{
Expand All @@ -45,8 +44,7 @@ private static InputModelProperty ReadInputModelProperty(ref Utf8JsonReader read
|| reader.TryReadBoolean("readOnly", ref isReadOnly)
|| reader.TryReadBoolean("optional", ref isOptional)
|| reader.TryReadBoolean("discriminator", ref isDiscriminator)
|| reader.TryReadWithConverter("decorators", options, ref decorators)
|| reader.TryReadWithConverter("flattenNames", options, ref flattenedNames);
|| reader.TryReadWithConverter("decorators", options, ref decorators);

if (!isKnownProperty)
{
Expand All @@ -59,7 +57,7 @@ private static InputModelProperty ReadInputModelProperty(ref Utf8JsonReader read
// description = BuilderHelpers.EscapeXmlDocDescription(description);
propertyType = propertyType ?? throw new JsonException($"{nameof(InputModelProperty)} must have a property type.");

var property = new InputModelProperty(name, serializedName ?? name, description, propertyType, !isOptional, isReadOnly, isDiscriminator, flattenedNames) { Decorators = decorators ?? [] };
var property = new InputModelProperty(name, serializedName ?? name, description, propertyType, !isOptional, isReadOnly, isDiscriminator) { Decorators = decorators ?? [] };
if (id != null)
{
resolver.AddReference(id, property);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ public static InputModelProperty Property(
type,
isRequired,
isReadOnly,
isDiscriminator,
null);
isDiscriminator);
}

public static InputModelType Model(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Authentication.ApiKey.InvalidAuth.error"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Authentication.Http.Custom.InvalidAuth.error"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Authentication.OAuth2.InvalidAuth.error"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Client.Naming.Property.ClientNameModel.defaultName"
}
Expand Down Expand Up @@ -147,6 +148,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Client.Naming.Property.LanguageClientNameModel.defaultName"
}
Expand Down Expand Up @@ -176,6 +178,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Client.Naming.Property.ClientNameAndJsonEncodedNameModel.defaultName"
}
Expand Down Expand Up @@ -205,6 +208,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Client.Naming.Model.ModelWithClientClientName.defaultName"
}
Expand Down Expand Up @@ -234,6 +238,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Client.Naming.Model.ModelWithLanguageClientName.defaultName"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Bytes.DefaultBytesProperty.value"
}
Expand Down Expand Up @@ -57,6 +58,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Bytes.Base64BytesProperty.value"
}
Expand Down Expand Up @@ -86,6 +88,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Bytes.Base64urlBytesProperty.value"
}
Expand Down Expand Up @@ -130,6 +133,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Bytes.Base64urlArrayBytesProperty.value"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Datetime.DefaultDatetimeProperty.value"
}
Expand Down Expand Up @@ -71,6 +72,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Datetime.Rfc3339DatetimeProperty.value"
}
Expand Down Expand Up @@ -107,6 +109,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Datetime.Rfc7231DatetimeProperty.value"
}
Expand Down Expand Up @@ -143,6 +146,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Datetime.UnixTimestampDatetimeProperty.value"
}
Expand Down Expand Up @@ -201,6 +205,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Datetime.UnixTimestampArrayDatetimeProperty.value"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Duration.Property.DefaultDurationProperty.value"
}
Expand Down Expand Up @@ -71,6 +72,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Duration.Property.ISO8601DurationProperty.value"
}
Expand Down Expand Up @@ -107,6 +109,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Duration.Property.Int32SecondsDurationProperty.value"
}
Expand Down Expand Up @@ -143,6 +146,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Duration.Property.FloatSecondsDurationProperty.value"
}
Expand Down Expand Up @@ -179,6 +183,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Duration.Property.Float64SecondsDurationProperty.value"
}
Expand Down Expand Up @@ -237,6 +242,7 @@
"optional": false,
"readOnly": false,
"discriminator": false,
"flatten": false,
"decorators": [],
"crossLanguageDefinitionId": "Encode.Duration.Property.FloatSecondsDurationArrayProperty.value"
}
Expand Down
Loading

0 comments on commit c78c614

Please sign in to comment.