Skip to content

Commit 17d0414

Browse files
authored
Remove internal setter from collection-type property (#52554)
* Remove internal setter from collection-type property * typo * set flatten property * fix
1 parent 28634c9 commit 17d0414

File tree

9 files changed

+79
-81
lines changed

9 files changed

+79
-81
lines changed

eng/packages/http-client-csharp-mgmt/emitter/src/emitter.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ import {
99
$onEmit as $onAzureEmit,
1010
AzureEmitterOptions
1111
} from "@azure-typespec/http-client-csharp";
12-
import { azureSDKContextOptions } from "./sdk-context-options.js";
12+
import { azureSDKContextOptions, flattenPropertyDecorator } from "./sdk-context-options.js";
1313
import { updateClients } from "./resource-detection.js";
14+
import { DecoratorInfo } from "@azure-tools/typespec-client-generator-core";
1415

1516
export async function $onEmit(context: EmitContext<AzureEmitterOptions>) {
1617
context.options["generator-name"] ??= "ManagementClientGenerator";
@@ -25,6 +26,22 @@ export async function $onEmit(context: EmitContext<AzureEmitterOptions>) {
2526
sdkContext: CSharpEmitterContext
2627
): CodeModel {
2728
updateClients(codeModel, sdkContext);
29+
setFlattenProperty(codeModel, sdkContext);
2830
return codeModel;
2931
}
3032
}
33+
34+
function setFlattenProperty(codeModel: CodeModel, sdkContext: CSharpEmitterContext): void {
35+
for (const model of sdkContext.sdkPackage.models) {
36+
for (const property of model.properties) {
37+
if (property.flatten ) {
38+
39+
const flattenPropertyMetadataDecorator: DecoratorInfo = {
40+
name: flattenPropertyDecorator,
41+
arguments: {}
42+
};
43+
property.decorators.push(flattenPropertyMetadataDecorator);
44+
}
45+
}
46+
}
47+
}

eng/packages/http-client-csharp-mgmt/emitter/src/sdk-context-options.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,6 @@ export const resourceGroupResource =
7676
const resourceGroupResourceRegex =
7777
"Azure\\.ResourceManager\\.@resourceGroupResource";
7878

79-
const flattenPropertyRegex =
80-
"Azure\\.ClientGenerator\\.Core\\.@flattenProperty";
81-
8279
// TODO: add this decorator to TCGC
8380
export const resourceMetadata = "Azure.ClientGenerator.Core.@resourceSchema";
8481
const resourceMetadataRegex =
@@ -88,6 +85,8 @@ export const nonResourceMethodMetadata =
8885
const nonResourceMethodMetadataRegex =
8986
"Azure\\.ClientGenerator\\.Core\\.@nonResourceMethodSchema";
9087

88+
export const flattenPropertyDecorator = "Azure.ResourceManager.@flattenProperty";
89+
9190
export const azureSDKContextOptions: CreateSdkContextOptions = {
9291
versioning: {
9392
previewStringRegex: /-preview$/
@@ -104,7 +103,6 @@ export const azureSDKContextOptions: CreateSdkContextOptions = {
104103
armResourceOperationsRegex,
105104
armResourceUpdateRegex,
106105
armResourceReadRegex,
107-
flattenPropertyRegex,
108106
parentResourceRegex,
109107
resourceGroupResourceRegex,
110108
singletonRegex,

eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/ManagementInputLibrary.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public class ManagementInputLibrary : InputLibrary
1515
{
1616
private const string ResourceMetadataDecoratorName = "Azure.ClientGenerator.Core.@resourceSchema";
1717
private const string NonResourceMethodMetadata = "Azure.ClientGenerator.Core.@nonResourceMethodSchema";
18-
private const string FlattenPropertyDecoratorName = "Azure.ClientGenerator.Core.@flattenProperty";
18+
private const string FlattenPropertyDecoratorName = "Azure.ResourceManager.@flattenProperty";
1919

2020
private IReadOnlyDictionary<string, InputServiceMethod>? _inputServiceMethodsByCrossLanguageDefinitionId;
2121
private IReadOnlyDictionary<InputServiceMethod, InputClient>? _intMethodClientMap;

eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Utilities/PropertyHelpers.cs

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -80,30 +80,36 @@ private static bool HasDefaultPublicCtor(ModelProvider? innerModel)
8080
return false;
8181
}
8282

83-
public static MethodBodyStatement BuildGetter(bool? includeGetterNullCheck, PropertyProvider internalProperty, TypeProvider innerModel, PropertyProvider singleProperty)
83+
public static MethodBodyStatement BuildGetter(bool? includeGetterNullCheck, PropertyProvider internalProperty, TypeProvider innerModel, PropertyProvider innerProperty)
8484
{
8585
var checkNullExpression = This.Property(internalProperty.Name).Is(Null);
86+
// For collection types, we do not do null check and initialization in getter, they have been initialized in constructor.
87+
if (innerProperty.Type.IsCollection)
88+
{
89+
return new List<MethodBodyStatement>() { Return(new MemberExpression(internalProperty, innerProperty.Name)) };
90+
}
91+
8692
if (includeGetterNullCheck == true)
8793
{
8894
return new List<MethodBodyStatement> {
8995
new IfStatement(checkNullExpression)
9096
{
9197
internalProperty.Assign(New.Instance(innerModel.Type)).Terminate()
9298
},
93-
Return(new MemberExpression(internalProperty, singleProperty.Name))
99+
Return(new MemberExpression(internalProperty, innerProperty.Name))
94100
};
95101
}
96102
else if (includeGetterNullCheck == false)
97103
{
98-
return Return(new TernaryConditionalExpression(checkNullExpression, Default, new MemberExpression(internalProperty, singleProperty.Name)));
104+
return Return(new TernaryConditionalExpression(checkNullExpression, Default, new MemberExpression(internalProperty, innerProperty.Name)));
99105
}
100106
else
101107
{
102108
if (innerModel.Type.IsNullable)
103109
{
104-
return Return(new MemberExpression(internalProperty.AsVariableExpression.NullConditional(), singleProperty.Name));
110+
return Return(new MemberExpression(internalProperty.AsVariableExpression.NullConditional(), innerProperty.Name));
105111
}
106-
return Return(new MemberExpression(internalProperty, singleProperty.Name));
112+
return Return(new MemberExpression(internalProperty, innerProperty.Name));
107113
}
108114
}
109115

@@ -122,24 +128,6 @@ public static MethodBodyStatement BuildSetterForPropertyFlatten(ModelProvider in
122128
return setter;
123129
}
124130

125-
public static Dictionary<ValueExpression, ValueExpression> PopulateCollectionProperties(IEnumerable<PropertyProvider> collectionTypeProperties)
126-
{
127-
var result = new Dictionary<ValueExpression, ValueExpression>();
128-
foreach (var property in collectionTypeProperties)
129-
{
130-
var propertyValue = Value.Property(property.Name);
131-
if (property.Type.IsList)
132-
{
133-
result.Add(Identifier(property.Name), New.Instance(ManagementClientGenerator.Instance.TypeFactory.ListInitializationType.MakeGenericType(property.Type.Arguments)));
134-
}
135-
if (property.Type.IsDictionary)
136-
{
137-
result.Add(Identifier(property.Name), New.Instance(ManagementClientGenerator.Instance.TypeFactory.DictionaryInitializationType.MakeGenericType(property.Type.Arguments)));
138-
}
139-
}
140-
return result;
141-
}
142-
143131
public static MethodBodyStatement BuildSetterForSafeFlatten(bool includeSetterCheck, ModelProvider innerModel, PropertyProvider internalProperty, PropertyProvider innerProperty)
144132
{
145133
var isOverriddenValueType = IsOverriddenValueType(innerProperty);

eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Visitors/FlattenPropertyVisitor.cs

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Microsoft.TypeSpec.Generator.Providers;
1010
using Microsoft.TypeSpec.Generator.Snippets;
1111
using Microsoft.TypeSpec.Generator.Statements;
12+
using System;
1213
using System.Collections.Generic;
1314
using System.Linq;
1415
using static Microsoft.TypeSpec.Generator.Snippets.Snippet;
@@ -36,41 +37,55 @@ internal class FlattenPropertyVisitor : ScmLibraryVisitor
3637

3738
if (type is ModelProvider model && _collectionTypeProperties.TryGetValue(model, out var value))
3839
{
39-
foreach (var (internalProperty, collectionProperties) in value)
40+
foreach (var internalProperty in value)
4041
{
41-
var innerCollectionProperties = collectionProperties.Select(x => x.InnerProperty);
42-
var initializationMethod = BuildInitializationMethod(innerCollectionProperties, internalProperty, model);
43-
var publicConstructor = model.Constructors.Single(m => m.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
44-
var invokeInitialization = This.Invoke(initializationMethod.Signature.Name).Terminate();
42+
var publicConstructor = model.Constructors.SingleOrDefault(m => m.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
43+
if (publicConstructor is null)
44+
{
45+
continue;
46+
}
4547

48+
var internalPropertyTypeConstructor = ManagementClientGenerator.Instance.TypeFactory.CSharpTypeMap[internalProperty.Type]!.Constructors.Single(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public));
49+
var initializationParameters = PopulateInitializationParameters(publicConstructor, internalPropertyTypeConstructor);
50+
var initialization = internalProperty.Assign(New.Instance(internalProperty.Type, initializationParameters)).Terminate();
4651
// If the property is a collection type, we need to ensure that it is initialized
4752
if (publicConstructor.BodyStatements is null)
4853
{
49-
publicConstructor.Update(bodyStatements: new List<MethodBodyStatement> { invokeInitialization });
54+
publicConstructor.Update(bodyStatements: new List<MethodBodyStatement> { initialization });
5055
}
5156
else
5257
{
5358
var body = publicConstructor.BodyStatements.ToList();
54-
body.Add(invokeInitialization);
59+
body.Add(initialization);
5560
publicConstructor.Update(bodyStatements: body);
5661
}
57-
model.Update(methods: [.. model.Methods, initializationMethod]);
5862
}
5963
}
6064

6165
return base.PostVisitType(type);
6266
}
6367

64-
internal const string s_initializationMethodName = "Initialize";
65-
private MethodProvider BuildInitializationMethod(IEnumerable<PropertyProvider> collectionTypeProperties, PropertyProvider internalProperty, ModelProvider model)
68+
private ValueExpression[] PopulateInitializationParameters(ConstructorProvider publicConstructor, ConstructorProvider internalPropertyTypeConstructor)
6669
{
67-
var signature = new MethodSignature($"{s_initializationMethodName}{internalProperty.Type.Name}", null, MethodSignatureModifiers.Private, null, null, []);
68-
MethodBodyStatement[] body = [
69-
new IfStatement(This.Property(internalProperty.Name).Is(Null))
70-
{
71-
internalProperty.Assign(New.Instance(internalProperty.Type, PropertyHelpers.PopulateCollectionProperties(collectionTypeProperties))).Terminate()
72-
},];
73-
return new MethodProvider(signature, body, model);
70+
var parameters = new List<ValueExpression>();
71+
foreach (var parameter in internalPropertyTypeConstructor.Signature.Parameters)
72+
{
73+
if (parameter.Type.IsList)
74+
{
75+
parameters.Add(New.Instance(ManagementClientGenerator.Instance.TypeFactory.ListInitializationType.MakeGenericType(parameter.Type.Arguments)));
76+
}
77+
else if (parameter.Type.IsDictionary)
78+
{
79+
parameters.Add(New.Instance(ManagementClientGenerator.Instance.TypeFactory.DictionaryInitializationType.MakeGenericType(parameter.Type.Arguments)));
80+
}
81+
else
82+
{
83+
var constructorParameter = publicConstructor.Signature.Parameters.Single(p => p.Name.Equals(parameter.Name, System.StringComparison.OrdinalIgnoreCase));
84+
85+
parameters.Add(constructorParameter);
86+
}
87+
}
88+
return parameters.ToArray();
7489
}
7590

7691
private void UpdateModelFactory(ModelFactoryProvider modelFactory)
@@ -256,7 +271,7 @@ int GetAdditionalPropertyIndex()
256271
// So that, we can use this to update the model factory methods later.
257272
private readonly Dictionary<CSharpType, Dictionary<string, List<(bool IsOverriddenValueType, PropertyProvider FlattenedProperty)>>> _flattenedModelTypes = new();
258273
// TODO: Workadound to initialize all collection-type properties in all collection-type setters, remove this once we have lazy initializtion for collection-type properties
259-
private readonly Dictionary<ModelProvider, Dictionary<PropertyProvider, List<(PropertyProvider FlattenedProperty, PropertyProvider InnerProperty)>>> _collectionTypeProperties = new();
274+
private readonly Dictionary<ModelProvider, HashSet<PropertyProvider>> _collectionTypeProperties = new();
260275
private void FlattenProperties(ModelProvider model)
261276
{
262277
var isFlattened = false;
@@ -281,6 +296,7 @@ private void FlattenProperties(ModelProvider model)
281296

282297
foreach (var innerProperty in innerProperties)
283298
{
299+
CollectFlattenTypeCollectionProperty(property, innerProperty, model);
284300
// flatten the property to public and associate it with the internal property
285301
var (_, includeGetterNullCheck, _) = PropertyHelpers.GetFlags(property, innerProperty);
286302
var flattenPropertyName = innerProperty.Name; // TODO: handle name conflicts
@@ -304,7 +320,6 @@ private void FlattenProperties(ModelProvider model)
304320
innerProperty.Attributes);
305321

306322
flattenedProperties.Add((isOverriddenValueType, flattenedProperty));
307-
AddInternalSetterForFlattenTypeCollectionProperty(property, innerProperty, flattenedProperty, model);
308323
}
309324
// make the internalized properties internal
310325
property.Update(modifiers: property.Modifiers & ~MethodSignatureModifiers.Public | MethodSignatureModifiers.Internal);
@@ -322,28 +337,20 @@ private void FlattenProperties(ModelProvider model)
322337
}
323338

324339
// TODO: workaround to add internal setter, we should remove this once we add lazy initialization for collection type properties
325-
private void AddInternalSetterForFlattenTypeCollectionProperty(PropertyProvider internalProperty, PropertyProvider innerProperty, PropertyProvider flattenedProperty, ModelProvider modelProvider)
340+
private void CollectFlattenTypeCollectionProperty(PropertyProvider internalProperty, PropertyProvider innerProperty, ModelProvider modelProvider)
326341
{
327342
if (innerProperty.Type.IsCollection)
328343
{
329344
if (_collectionTypeProperties.TryGetValue(modelProvider, out var value))
330345
{
331-
if (value.TryGetValue(internalProperty, out var properties))
332-
{
333-
properties.Add((flattenedProperty, innerProperty));
334-
}
335-
else
336-
{
337-
value.Add(internalProperty, [(flattenedProperty, innerProperty)]);
338-
}
346+
value.Add(internalProperty);
339347
}
340348
else
341349
{
342-
var dict = new Dictionary<PropertyProvider, List<(PropertyProvider FlattenedProperty, PropertyProvider InnerProperty)>>();
343-
dict.Add(internalProperty, [(flattenedProperty, innerProperty)]);
344-
_collectionTypeProperties.Add(modelProvider, dict);
350+
var set = new HashSet<PropertyProvider>();
351+
set.Add(internalProperty);
352+
_collectionTypeProperties.Add(modelProvider, set);
345353
}
346-
innerProperty.Update(body: new AutoPropertyBody(true, MethodSignatureModifiers.Internal));
347354
}
348355
}
349356

eng/packages/http-client-csharp-mgmt/generator/TestProjects/Local/Mgmt-TypeSpec/src/Generated/BarSettingsResourceData.cs

Lines changed: 3 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

eng/packages/http-client-csharp-mgmt/generator/TestProjects/Local/Mgmt-TypeSpec/src/Generated/Models/BarMiddleNestedQuotaProperties.cs

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

eng/packages/http-client-csharp-mgmt/generator/TestProjects/Local/Mgmt-TypeSpec/src/Generated/Models/BarNestedQuotaProperties.cs

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

eng/packages/http-client-csharp-mgmt/generator/TestProjects/Local/Mgmt-TypeSpec/tspCodeModel.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5109,7 +5109,7 @@
51095109
"flatten": true,
51105110
"decorators": [
51115111
{
5112-
"name": "Azure.ClientGenerator.Core.@flattenProperty",
5112+
"name": "Azure.ResourceManager.@flattenProperty",
51135113
"arguments": {}
51145114
}
51155115
],

0 commit comments

Comments
 (0)