Skip to content

Commit c552aa5

Browse files
CopilotArcturusZhang
Copilot
and
ArcturusZhang
authored
[Azure Mgmt Generator] The Create operation for a resource should always have the CreateOrUpdate name (Azure#51418)
* Initial plan * Demonstrate the issue with Create operation naming and add test Co-authored-by: ArcturusZhang <[email protected]> * Implement fix for Create operation naming - always use "CreateOrUpdate" Co-authored-by: ArcturusZhang <[email protected]> * Remove accidentally committed tsp-output directory Co-authored-by: ArcturusZhang <[email protected]> * Implement reviewer feedback: Simplified Create operation naming fix using constructor parameter approach Co-authored-by: ArcturusZhang <[email protected]> * Address reviewer feedback: Refactor Create operation naming logic and complete test data Co-authored-by: ArcturusZhang <[email protected]> * Implement reviewer feedback: Use utility method approach for Create operation naming Co-authored-by: ArcturusZhang <[email protected]> * Move method name calculation to constructor as requested Co-authored-by: ArcturusZhang <[email protected]> * Fix failing tests and remove unnecessary null fallback for MethodSignature.Name Co-authored-by: ArcturusZhang <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: ArcturusZhang <[email protected]>
1 parent 0d75359 commit c552aa5

File tree

16 files changed

+228
-56
lines changed

16 files changed

+228
-56
lines changed

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Providers/OperationMethodProviders/ResourceOperationMethodProvider.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ internal class ResourceOperationMethodProvider
3838
protected readonly MethodSignature _signature;
3939
protected readonly MethodBodyStatement[] _bodyStatements;
4040

41+
private readonly string _methodName;
4142
private readonly CSharpType? _responseGenericType;
4243
private readonly bool _isGeneric;
4344
private readonly bool _isLongRunningOperation;
@@ -52,13 +53,15 @@ internal class ResourceOperationMethodProvider
5253
/// <param name="method">The input service method that we are building from. </param>
5354
/// <param name="convenienceMethod">The corresponding convenience method provided by the generator framework. </param>
5455
/// <param name="isAsync">Whether this method is an async method. </param>
56+
/// <param name="methodName">Optional override for the method name. If not provided, uses the convenience method name. </param>
5557
public ResourceOperationMethodProvider(
5658
TypeProvider enclosingType,
5759
RequestPathPattern contextualPath,
5860
RestClientInfo restClientInfo,
5961
InputServiceMethod method,
6062
MethodProvider convenienceMethod,
61-
bool isAsync)
63+
bool isAsync,
64+
string? methodName = null)
6265
{
6366
_enclosingType = enclosingType;
6467
_contextualPath = contextualPath;
@@ -67,6 +70,7 @@ public ResourceOperationMethodProvider(
6770
_serviceMethod = method;
6871
_convenienceMethod = convenienceMethod;
6972
_isAsync = isAsync;
73+
_methodName = methodName ?? convenienceMethod.Signature.Name;
7074
_responseGenericType = _serviceMethod.GetResponseBodyType();
7175
_isGeneric = _responseGenericType != null;
7276
_isLongRunningOperation = _serviceMethod.IsLongRunningOperation();
@@ -110,7 +114,7 @@ protected virtual MethodBodyStatement[] BuildBodyStatements()
110114
protected virtual MethodSignature CreateSignature()
111115
{
112116
return new MethodSignature(
113-
_convenienceMethod.Signature.Name,
117+
_methodName,
114118
_convenienceMethod.Signature.Description,
115119
_convenienceMethod.Signature.Modifiers,
116120
_serviceMethod.GetOperationMethodReturnType(_isAsync, _resource.Type, _resource.ResourceData.Type),

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Providers/ResourceClientProvider.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,9 +350,11 @@ protected override MethodProvider[] BuildMethods()
350350
}
351351
else
352352
{
353-
operationMethods.Add(new ResourceOperationMethodProvider(this, ContextualPath, restClientInfo, method, convenienceMethod, false));
353+
var methodName = ResourceHelpers.GetOperationMethodName(methodKind, false);
354+
operationMethods.Add(new ResourceOperationMethodProvider(this, ContextualPath, restClientInfo, method, convenienceMethod, false, methodName));
354355
var asyncConvenienceMethod = restClientInfo.RestClientProvider.GetConvenienceMethodByOperation(method.Operation, true);
355-
operationMethods.Add(new ResourceOperationMethodProvider(this, ContextualPath, restClientInfo, method, asyncConvenienceMethod, true));
356+
var asyncMethodName = ResourceHelpers.GetOperationMethodName(methodKind, true);
357+
operationMethods.Add(new ResourceOperationMethodProvider(this, ContextualPath, restClientInfo, method, asyncConvenienceMethod, true, asyncMethodName));
356358
}
357359
}
358360

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Providers/ResourceCollectionClientProvider.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,8 @@ private List<MethodProvider> BuildCreateOrUpdateMethods()
272272
foreach (var isAsync in new List<bool> { true, false })
273273
{
274274
var convenienceMethod = restClientInfo.RestClientProvider.GetConvenienceMethodByOperation(_create!.Operation, isAsync);
275-
result.Add(new ResourceOperationMethodProvider(this, ContextualPath, restClientInfo, _create, convenienceMethod, isAsync));
275+
var methodName = ResourceHelpers.GetOperationMethodName(ResourceOperationKind.Create, isAsync);
276+
result.Add(new ResourceOperationMethodProvider(this, ContextualPath, restClientInfo, _create, convenienceMethod, isAsync, methodName));
276277
}
277278

278279
return result;

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using Azure.Generator.Management.Models;
45
using Microsoft.TypeSpec.Generator.Input.Extensions;
56

67
namespace Azure.Generator.Management.Utilities
@@ -19,5 +20,20 @@ public static string GetRestClientFieldName(string restClientName)
1920
var fieldName = $"{restClientName}RestClient".ToVariableName();
2021
return $"_{fieldName}";
2122
}
23+
24+
/// <summary>
25+
/// Gets the appropriate method name for a resource operation based on its kind.
26+
/// </summary>
27+
/// <param name="operationKind">The kind of resource operation.</param>
28+
/// <param name="isAsync">Whether this is an async method.</param>
29+
/// <returns>The method name to use, or null if no override is needed.</returns>
30+
public static string? GetOperationMethodName(ResourceOperationKind operationKind, bool isAsync)
31+
{
32+
return operationKind switch
33+
{
34+
ResourceOperationKind.Create => isAsync ? "CreateOrUpdateAsync" : "CreateOrUpdate",
35+
_ => null
36+
};
37+
}
2238
}
2339
}

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/test/Common/InputResourceData.cs‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,22 @@ public static (InputClient InputClient, IReadOnlyList<InputModelType> InputModel
2929
var testNameParameter = InputFactory.Parameter("testName", InputPrimitiveType.String, location: InputRequestLocation.Path);
3030
var subscriptionIdParameter = InputFactory.Parameter("subscriptionId", new InputPrimitiveType(InputPrimitiveTypeKind.String, "uuid", "Azure.Core.uuid"), location: InputRequestLocation.Path, kind: InputParameterKind.Client);
3131
var resourceGroupParameter = InputFactory.Parameter("resourceGroupName", InputPrimitiveType.String, location: InputRequestLocation.Path, kind: InputParameterKind.Client);
32-
var operation = InputFactory.Operation(name: "get", responses: [responseType], parameters: [testNameParameter, subscriptionIdParameter, resourceGroupParameter], path: "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Tests/tests/{testName}");
33-
var crossLanguageDefinitionId = Guid.NewGuid().ToString();
32+
var dataParameter = InputFactory.Parameter("data", responseModel, location: InputRequestLocation.Body);
33+
var getOperation = InputFactory.Operation(name: "get", responses: [responseType], parameters: [testNameParameter, subscriptionIdParameter, resourceGroupParameter], path: "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Tests/tests/{testName}");
34+
var createOperation = InputFactory.Operation(name: "createTest", responses: [responseType], parameters: [testNameParameter, subscriptionIdParameter, resourceGroupParameter, dataParameter], path: "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Tests/tests/{testName}");
35+
var getCrossLanguageDefinitionId = Guid.NewGuid().ToString();
36+
var createCrossLanguageDefinitionId = Guid.NewGuid().ToString();
3437
var client = InputFactory.Client(
3538
TestClientName,
36-
methods: [InputFactory.BasicServiceMethod("get", operation, parameters: [testNameParameter, subscriptionIdParameter, resourceGroupParameter], crossLanguageDefinitionId: crossLanguageDefinitionId)],
39+
methods: [
40+
InputFactory.BasicServiceMethod("get", getOperation, parameters: [testNameParameter, subscriptionIdParameter, resourceGroupParameter], crossLanguageDefinitionId: getCrossLanguageDefinitionId),
41+
InputFactory.BasicServiceMethod("createTest", createOperation, parameters: [testNameParameter, subscriptionIdParameter, resourceGroupParameter, dataParameter], crossLanguageDefinitionId: createCrossLanguageDefinitionId)
42+
],
3743
crossLanguageDefinitionId: $"Test.{TestClientName}");
38-
decorators.Add(BuildResourceMetadata(responseModel, client, "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Tests/tests/{testName}", "Microsoft.Tests/tests", null, ResourceScope.ResourceGroup, [new ResourceMethod(crossLanguageDefinitionId, ResourceOperationKind.Get)], "ResponseType"));
44+
decorators.Add(BuildResourceMetadata(responseModel, client, "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Tests/tests/{testName}", "Microsoft.Tests/tests", null, ResourceScope.ResourceGroup, [
45+
new ResourceMethod(getCrossLanguageDefinitionId, ResourceOperationKind.Get),
46+
new ResourceMethod(createCrossLanguageDefinitionId, ResourceOperationKind.Create)
47+
], "ResponseType"));
3948
return (client, [responseModel]);
4049
}
4150

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/test/Providers/ResourceCollectionClientProviderTests.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using Azure.Core;
45
using Azure.Generator.Management.Providers;
56
using Azure.Generator.Management.Tests.Common;
67
using Azure.Generator.Management.Tests.TestHelpers;
78
using Azure.Generator.Tests.Common;
9+
using Azure.ResourceManager;
810
using Microsoft.TypeSpec.Generator.Primitives;
911
using Microsoft.TypeSpec.Generator.Providers;
1012
using NUnit.Framework;
@@ -149,5 +151,51 @@ public void Verify_GetIfExistsAsyncOperationMethod()
149151
var exptected = Helpers.GetExpectedFromFile();
150152
Assert.AreEqual(exptected, bodyStatements);
151153
}
154+
155+
[TestCase]
156+
public void Verify_CreateOrUpdateOperationMethod()
157+
{
158+
ResourceCollectionClientProvider resourceProvider = GetResourceCollectionClientProvider();
159+
var methods = resourceProvider.Methods.Select(m => m.Signature.Name).ToArray();
160+
Console.WriteLine($"Available methods: {string.Join(", ", methods)}");
161+
162+
MethodProvider createOrUpdateMethod = GetResourceCollectionClientProviderMethodByName("CreateOrUpdate");
163+
164+
// verify the method signature
165+
var signature = createOrUpdateMethod.Signature;
166+
Assert.AreEqual(MethodSignatureModifiers.Public | MethodSignatureModifiers.Virtual, signature.Modifiers);
167+
Assert.AreEqual(3, signature.Parameters.Count);
168+
Assert.AreEqual(typeof(string), signature.Parameters[0].Type.FrameworkType);
169+
Assert.IsTrue(signature.Parameters[1].Type.Name.EndsWith("Data"));
170+
Assert.AreEqual(typeof(CancellationToken), signature.Parameters[2].Type.FrameworkType);
171+
Assert.AreEqual(typeof(Response<>), signature.ReturnType?.FrameworkType);
172+
173+
// verify the method body
174+
var bodyStatements = createOrUpdateMethod.BodyStatements?.ToDisplayString();
175+
Assert.NotNull(bodyStatements);
176+
var expected = Helpers.GetExpectedFromFile();
177+
Assert.AreEqual(expected, bodyStatements);
178+
}
179+
180+
[TestCase]
181+
public void Verify_CreateOrUpdateAsyncOperationMethod()
182+
{
183+
MethodProvider createOrUpdateMethod = GetResourceCollectionClientProviderMethodByName("CreateOrUpdateAsync");
184+
185+
// verify the method signature
186+
var signature = createOrUpdateMethod.Signature;
187+
Assert.AreEqual(MethodSignatureModifiers.Public | MethodSignatureModifiers.Virtual | MethodSignatureModifiers.Async, signature.Modifiers);
188+
Assert.AreEqual(3, signature.Parameters.Count);
189+
Assert.AreEqual(typeof(string), signature.Parameters[0].Type.FrameworkType);
190+
Assert.IsTrue(signature.Parameters[1].Type.Name.EndsWith("Data"));
191+
Assert.AreEqual(typeof(CancellationToken), signature.Parameters[2].Type.FrameworkType);
192+
Assert.AreEqual(typeof(Task<>), signature.ReturnType?.FrameworkType);
193+
194+
// verify the method body
195+
var bodyStatements = createOrUpdateMethod.BodyStatements?.ToDisplayString();
196+
Assert.NotNull(bodyStatements);
197+
var expected = Helpers.GetExpectedFromFile();
198+
Assert.AreEqual(expected, bodyStatements);
199+
}
152200
}
153201
}

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/test/Providers/TestData/ResourceCollectionClientProviderTests/Verify_CreateOrUpdateAsyncOperationMethod.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using global::Azure.Core.Pipeline.DiagnosticScope scope = _testClientClientDiagnostics.CreateScope("ResponseTypeCollection.CreateOrUpdateAsync");
2+
scope.Start();
3+
try
4+
{
5+
global::Azure.RequestContext context = new global::Azure.RequestContext
6+
{
7+
CancellationToken = cancellationToken
8+
}
9+
;
10+
global::Azure.Core.HttpMessage message = _testClientRestClient.CreateCreateTestRequest(testName, global::System.Guid.Parse(this.Id.SubscriptionId), global::Samples.Models.ResponseTypeData.ToRequestContent(data), context);
11+
global::Azure.Response result = await this.Pipeline.ProcessMessageAsync(message, context).ConfigureAwait(false);
12+
global::Azure.Response<global::Samples.Models.ResponseTypeData> response = global::Azure.Response.FromValue(global::Samples.Models.ResponseTypeData.FromResponse(result), result);
13+
if ((response.Value == null))
14+
{
15+
throw new global::Azure.RequestFailedException(response.GetRawResponse());
16+
}
17+
return global::Azure.Response.FromValue(new global::Samples.ResponseTypeResource(this.Client, response.Value), response.GetRawResponse());
18+
}
19+
catch (global::System.Exception e)
20+
{
21+
scope.Failed(e);
22+
throw;
23+
}

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/test/Providers/TestData/ResourceCollectionClientProviderTests/Verify_CreateOrUpdateOperationMethod.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using global::Azure.Core.Pipeline.DiagnosticScope scope = _testClientClientDiagnostics.CreateScope("ResponseTypeCollection.CreateOrUpdate");
2+
scope.Start();
3+
try
4+
{
5+
global::Azure.RequestContext context = new global::Azure.RequestContext
6+
{
7+
CancellationToken = cancellationToken
8+
}
9+
;
10+
global::Azure.Core.HttpMessage message = _testClientRestClient.CreateCreateTestRequest(testName, global::System.Guid.Parse(this.Id.SubscriptionId), global::Samples.Models.ResponseTypeData.ToRequestContent(data), context);
11+
global::Azure.Response result = this.Pipeline.ProcessMessage(message, context);
12+
global::Azure.Response<global::Samples.Models.ResponseTypeData> response = global::Azure.Response.FromValue(global::Samples.Models.ResponseTypeData.FromResponse(result), result);
13+
if ((response.Value == null))
14+
{
15+
throw new global::Azure.RequestFailedException(response.GetRawResponse());
16+
}
17+
return global::Azure.Response.FromValue(new global::Samples.ResponseTypeResource(this.Client, response.Value), response.GetRawResponse());
18+
}
19+
catch (global::System.Exception e)
20+
{
21+
scope.Failed(e);
22+
throw;
23+
}

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/test/TestData/ResourceCollectionClientProviderTests/Verify_CreateOrUpdateAsyncOperationMethod.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using global::Azure.Core.Pipeline.DiagnosticScope scope = _testClientClientDiagnostics.CreateScope("ResponseTypeCollection.CreateOrUpdateAsync");
2+
scope.Start();
3+
try
4+
{
5+
global::Azure.RequestContext context = new global::Azure.RequestContext
6+
{
7+
CancellationToken = cancellationToken
8+
}
9+
;
10+
global::Azure.Core.HttpMessage message = _testClientRestClient.CreateCreateTestRequest(testName, global::System.Guid.Parse(this.Id.SubscriptionId), global::Samples.Models.ResponseTypeData.ToRequestContent(data), context);
11+
global::Azure.Response result = await this.Pipeline.ProcessMessageAsync(message, context).ConfigureAwait(false);
12+
global::Azure.Response<global::Samples.Models.ResponseTypeData> response = global::Azure.Response.FromValue(global::Samples.Models.ResponseTypeData.FromResponse(result), result);
13+
if ((response.Value == null))
14+
{
15+
throw new global::Azure.RequestFailedException(response.GetRawResponse());
16+
}
17+
return global::Azure.Response.FromValue(new global::Samples.ResponseTypeResource(this.Client, response.Value), response.GetRawResponse());
18+
}
19+
catch (global::System.Exception e)
20+
{
21+
scope.Failed(e);
22+
throw;
23+
}

‎eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/test/TestData/ResourceCollectionClientProviderTests/Verify_CreateOrUpdateOperationMethod.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using global::Azure.Core.Pipeline.DiagnosticScope scope = _testClientClientDiagnostics.CreateScope("ResponseTypeCollection.CreateOrUpdate");
2+
scope.Start();
3+
try
4+
{
5+
global::Azure.RequestContext context = new global::Azure.RequestContext
6+
{
7+
CancellationToken = cancellationToken
8+
}
9+
;
10+
global::Azure.Core.HttpMessage message = _testClientRestClient.CreateCreateTestRequest(testName, global::System.Guid.Parse(this.Id.SubscriptionId), global::Samples.Models.ResponseTypeData.ToRequestContent(data), context);
11+
global::Azure.Response result = this.Pipeline.ProcessMessage(message, context);
12+
global::Azure.Response<global::Samples.Models.ResponseTypeData> response = global::Azure.Response.FromValue(global::Samples.Models.ResponseTypeData.FromResponse(result), result);
13+
if ((response.Value == null))
14+
{
15+
throw new global::Azure.RequestFailedException(response.GetRawResponse());
16+
}
17+
return global::Azure.Response.FromValue(new global::Samples.ResponseTypeResource(this.Client, response.Value), response.GetRawResponse());
18+
}
19+
catch (global::System.Exception e)
20+
{
21+
scope.Failed(e);
22+
throw;
23+
}

0 commit comments

Comments
 (0)