Skip to content

Commit ce460ef

Browse files
Fix tuple resource list handling in mgmt generator (#59363)
* Fix tuple resource list handling in mgmt generator Classify array-returning tuple resource list operations as resource lists and emit all matching collection GetAll overloads so Compute extension image list operations remain available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove unintended Storage change Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert unintended Storage spec update Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9c9d782 commit ce460ef

4 files changed

Lines changed: 201 additions & 64 deletions

File tree

eng/packages/http-client-csharp-mgmt/docs/resource-detection.md

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -303,24 +303,21 @@ For each remaining operation `O`:
303303
2. If `O`'s response is not a collection of some item model `T`,
304304
skip — it cannot be a `List`.
305305
3. Otherwise, look for a resource `R` in the **detected resource set**
306-
such that `R.model == T` and `O.path` identifies a collection of
307-
`R`. A path identifies a collection of `R` when either:
308-
- `O.path` is exactly `R.instancePath` with the trailing name segment
309-
removed. This is the normal list-by-parent/list-by-resource-group
310-
shape. Singleton resources are handled the same way: the collection
311-
path is the singleton instance path with its constant singleton name
312-
removed.
313-
- `O.path` has the same ARM resource type as `R` and ends on that
314-
resource type. This covers scope-level list operations whose path
315-
intentionally omits part of the instance path, such as a
316-
subscription-level list of resource-group-scoped resources.
317-
318-
Exact collection-path matches take precedence over scope-level
319-
resource-type matches. If multiple resources still match, attach `O`
320-
to the resource with the **shortest** matching `instancePath` as
321-
`R.List`. (The shortest match is the closest containing resource
322-
whose model is `T` — i.e. `R` itself, not some deeper resource that
323-
happens to also be of model `T`.)
306+
such that `R.model == T` and `O.path` is a prefix of
307+
`R.instancePath`. This covers normal list-by-parent/list-by-resource-group
308+
operations, singleton resources, and tuple-resource list operations at
309+
intermediate collection paths.
310+
311+
If no prefix match is found, a scope-level list can still match when
312+
`O.path` has the same ARM resource type as `R`, has compatible scope
313+
nesting, and ends on that resource type. This covers scope-level list
314+
operations whose path intentionally omits part of the instance path, such as
315+
a subscription-level list of resource-group-scoped resources.
316+
317+
If multiple resources match, attach `O` to the resource with the
318+
**shortest** matching `instancePath` as `R.List`. (The shortest match is
319+
the closest containing resource whose model is `T` — i.e. `R` itself,
320+
not some deeper resource that happens to also be of model `T`.)
324321
4. If no such `R` exists, `O` falls through to Pass 2.
325322

326323
#### Pass 2 — Everything else is an `Action`

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,12 +555,11 @@ function findListTargetResource(
555555
(resource) => resource.resourceModelId === itemModelId
556556
);
557557

558-
const exactCollectionMatches = candidates.filter(
559-
(resource) =>
560-
resource.metadata.resourceIdPattern.trimLastSegment?.equals(operationPath)
558+
const collectionMatches = candidates.filter((resource) =>
559+
operationPath.isPrefixOf(resource.metadata.resourceIdPattern)
561560
);
562-
if (exactCollectionMatches.length > 0) {
563-
return shortestResourcePath(exactCollectionMatches);
561+
if (collectionMatches.length > 0) {
562+
return shortestResourcePath(collectionMatches);
564563
}
565564

566565
const operationType = operationPath.resourceType;

eng/packages/http-client-csharp-mgmt/emitter/test/resource-detection.test.ts

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2509,6 +2509,105 @@ interface ProfileRevisions {
25092509
);
25102510
});
25112511

2512+
it("tuple-resource intermediate collection path is assigned as list", async () => {
2513+
const program = await typeSpecCompile(
2514+
`
2515+
/** Virtual machine extension image resource */
2516+
model VirtualMachineExtensionImage is TrackedResource<VirtualMachineExtensionImageProperties> {
2517+
...ResourceNameParameter<
2518+
Resource = VirtualMachineExtensionImage,
2519+
KeyName = "type",
2520+
SegmentName = "types",
2521+
NamePattern = ""
2522+
>;
2523+
}
2524+
2525+
/** Virtual machine extension image properties */
2526+
model VirtualMachineExtensionImageProperties {
2527+
/** Display name */
2528+
displayName?: string;
2529+
}
2530+
2531+
alias ImageTypePath = {
2532+
...ApiVersionParameter;
2533+
...SubscriptionIdParameter;
2534+
...LocationResourceParameter;
2535+
/** Publisher name */
2536+
@path
2537+
@segment("publishers")
2538+
publisherName: string;
2539+
};
2540+
2541+
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "Testing static tuple resource paths"
2542+
interface VirtualMachineExtensionImages {
2543+
@get
2544+
@route("/subscriptions/{subscriptionId}/providers/Microsoft.Compute/locations/{location}/publishers/{publisherName}/artifacttypes/vmextension/types/{type}/versions/{version}")
2545+
get(
2546+
...ImageTypePath,
2547+
...KeysOf<VirtualMachineExtensionImage>,
2548+
/** Version name */
2549+
@path
2550+
@segment("versions")
2551+
version: string
2552+
): ArmResponse<VirtualMachineExtensionImage> | ErrorResponse;
2553+
2554+
@get
2555+
@route("/subscriptions/{subscriptionId}/providers/Microsoft.Compute/locations/{location}/publishers/{publisherName}/artifacttypes/vmextension/types")
2556+
listTypes(...ImageTypePath): ArmResponse<VirtualMachineExtensionImage[]> | ErrorResponse;
2557+
2558+
@get
2559+
@route("/subscriptions/{subscriptionId}/providers/Microsoft.Compute/locations/{location}/publishers/{publisherName}/artifacttypes/vmextension/types/{type}/versions")
2560+
listVersions(
2561+
...ImageTypePath,
2562+
...KeysOf<VirtualMachineExtensionImage>
2563+
): ArmResponse<VirtualMachineExtensionImage[]> | ErrorResponse;
2564+
}
2565+
`,
2566+
runner,
2567+
{ providerNamespace: "Microsoft.Compute" }
2568+
);
2569+
const context = createEmitterContext(program);
2570+
const sdkContext = await createCSharpSdkContext(context);
2571+
const [root] = createModel(sdkContext);
2572+
2573+
const armProviderSchema = buildArmProviderSchema(sdkContext, root);
2574+
ok(armProviderSchema);
2575+
2576+
const imageModel = root.models.find(
2577+
(m) => m.name === "VirtualMachineExtensionImage"
2578+
);
2579+
ok(imageModel, "VirtualMachineExtensionImage model should exist");
2580+
2581+
const imageResource = armProviderSchema.resources.find(
2582+
(r) =>
2583+
r.resourceModelId === imageModel.crossLanguageDefinitionId &&
2584+
r.metadata.resourceIdPattern.path.endsWith(
2585+
"/types/{type}/versions/{version}"
2586+
)
2587+
);
2588+
ok(
2589+
imageResource,
2590+
"VirtualMachineExtensionImage resource should be detected"
2591+
);
2592+
2593+
const listPaths = imageResource.metadata.methods
2594+
.filter((m) => m.kind === "List")
2595+
.map((m) => m.operationPath.path)
2596+
.sort();
2597+
2598+
deepStrictEqual(listPaths, [
2599+
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/locations/{location}/publishers/{publisherName}/artifacttypes/vmextension/types",
2600+
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/locations/{location}/publishers/{publisherName}/artifacttypes/vmextension/types/{type}/versions"
2601+
]);
2602+
strictEqual(
2603+
armProviderSchema.nonResourceMethods.some((m) =>
2604+
m.operationPath.path.includes("/artifacttypes/vmextension/types")
2605+
),
2606+
false,
2607+
"Tuple-resource list operations should not remain non-resource methods"
2608+
);
2609+
});
2610+
25122611
it("custom Azure resource with @customAzureResource decorator (TrafficManager pattern)", async () => {
25132612
const program = await typeSpecCompile(
25142613
`
@@ -2658,8 +2757,8 @@ interface TrafficEndpoints {
26582757
);
26592758
strictEqual(
26602759
trafficProfileResource.metadata.methods.length,
2661-
3,
2662-
"TrafficProfile should have 3 methods present in the code model (get, createOrUpdate, delete)"
2760+
4,
2761+
"TrafficProfile should have 4 methods present in the code model (get, createOrUpdate, delete, list)"
26632762
);
26642763

26652764
// Find the TrafficEndpoint resource

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

Lines changed: 81 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ internal sealed class ResourceCollectionClientProvider : TypeProvider
3333
private readonly IReadOnlyList<ParameterProvider> _extraCtorParameters;
3434
private readonly IReadOnlyList<FieldProvider> _extraFields;
3535
private readonly ResourceClientProvider _resource;
36-
private readonly ResourceMethod? _getAll;
36+
private readonly IReadOnlyList<ResourceMethod> _getAlls;
3737
private readonly ResourceMethod? _create;
3838
private readonly ResourceMethod? _get;
3939

@@ -60,35 +60,33 @@ internal ResourceCollectionClientProvider(ResourceClientProvider resource, Input
6060

6161
_resourceTypeExpression = Static(_resource.Type).As<ArmResource>().ResourceType();
6262

63-
InitializeMethods(resourceMethods, ref _get, ref _create, ref _getAll);
64-
_operationContext = InitializeContext(this, resourceMetadata, _getAll);
63+
var contextualPath = GetContextualPath(resourceMetadata);
64+
(_get, _create, _getAlls) = InitializeMethods(resourceMethods, contextualPath);
65+
_operationContext = InitializeContext(this, contextualPath, _getAlls.Count > 0 ? _getAlls[0] : null);
6566

66-
// this depends on _getAll being initialized
67+
// this depends on _getAlls being initialized
6768
(_extraCtorParameters, _extraFields) = BuildExtraConstructorParametersAndFields();
6869
}
6970

70-
private static OperationContext InitializeContext(ResourceCollectionClientProvider enclosingType, ArmResourceMetadata resourceMetadata, ResourceMethod? getAll)
71+
private static OperationContext InitializeContext(ResourceCollectionClientProvider enclosingType, RequestPathPattern contextualPath, ResourceMethod? canonicalGetAll)
7172
{
72-
var contextualPath = GetContextualPath(resourceMetadata);
73-
if (getAll is not null)
73+
if (canonicalGetAll is null)
7474
{
75-
var secondaryContextualPath = getAll.OperationPath;
76-
// validate the contextualPath should be an ancestor of the secondaryContextualPath, otherwise report diagnostic.
77-
if (!contextualPath.IsAncestorOf(secondaryContextualPath))
78-
{
79-
// Report diagnostic
80-
ManagementClientGenerator.Instance.Emitter.ReportDiagnostic(
81-
code: "malformed-resource-detected",
82-
message: $"The contextual path '{contextualPath}' is not an ancestor of the secondary contextual path '{secondaryContextualPath}'.",
83-
targetCrossLanguageDefinitionId: getAll.InputMethod.CrossLanguageDefinitionId
84-
);
85-
}
86-
return OperationContext.Create(contextualPath, secondaryContextualPath, enclosingType.FindField);
75+
return OperationContext.Create(contextualPath);
8776
}
88-
else
77+
78+
var secondaryContextualPath = canonicalGetAll.OperationPath;
79+
// validate the contextualPath should be an ancestor of the secondaryContextualPath, otherwise report diagnostic.
80+
if (!contextualPath.IsAncestorOf(secondaryContextualPath))
8981
{
90-
return OperationContext.Create(contextualPath);
82+
// Report diagnostic
83+
ManagementClientGenerator.Instance.Emitter.ReportDiagnostic(
84+
code: "malformed-resource-detected",
85+
message: $"The contextual path '{contextualPath}' is not an ancestor of the secondary contextual path '{secondaryContextualPath}'.",
86+
targetCrossLanguageDefinitionId: canonicalGetAll.InputMethod.CrossLanguageDefinitionId
87+
);
9188
}
89+
return OperationContext.Create(contextualPath, secondaryContextualPath, enclosingType.FindField);
9290
}
9391

9492
private FieldProvider FindField(string variableName)
@@ -128,7 +126,7 @@ private static RequestPathPattern GetContextualPath(ArmResourceMetadata resource
128126
var parameter = new ParameterProvider(
129127
contextualParameter.VariableName,
130128
$"The {contextualParameter.VariableName} for the resource.",
131-
ResourceHelpers.GetRequestPathParameterType(contextualParameter.VariableName, _getAll!.InputMethod));
129+
ResourceHelpers.GetRequestPathParameterType(contextualParameter.VariableName, _getAlls[0].InputMethod));
132130
var field = new FieldProvider(FieldModifiers.Private | FieldModifiers.ReadOnly, parameter.Type, $"_{contextualParameter.VariableName}", this, description: $"The {contextualParameter.VariableName}.");
133131
parameter.Field = field;
134132
extraParameters.Add(parameter);
@@ -137,32 +135,64 @@ private static RequestPathPattern GetContextualPath(ArmResourceMetadata resource
137135
return (extraParameters, extraFields);
138136
}
139137

140-
private static void InitializeMethods(
138+
private static (ResourceMethod? Get, ResourceMethod? Create, IReadOnlyList<ResourceMethod> GetAlls) InitializeMethods(
141139
IReadOnlyList<ResourceMethod> resourceMethods,
142-
ref ResourceMethod? getMethod,
143-
ref ResourceMethod? createMethod,
144-
ref ResourceMethod? getAllMethod)
140+
RequestPathPattern contextualPath)
145141
{
142+
ResourceMethod? getMethod = null;
143+
ResourceMethod? createMethod = null;
144+
var listMethods = new List<ResourceMethod>();
145+
146146
foreach (var method in resourceMethods)
147147
{
148-
if (getAllMethod is not null && createMethod is not null && getMethod is not null)
149-
{
150-
break; // we already have all methods we need
151-
}
152-
153148
switch (method.Kind)
154149
{
155150
case ResourceOperationKind.Read:
156-
getMethod = method;
151+
getMethod ??= method;
157152
break;
158153
case ResourceOperationKind.List:
159-
getAllMethod = method;
154+
listMethods.Add(method);
160155
break;
161156
case ResourceOperationKind.Create:
162-
createMethod = method;
157+
createMethod ??= method;
163158
break;
164159
}
165160
}
161+
162+
return (getMethod, createMethod, SortGetAllMethodsByScopeBreadth(listMethods, contextualPath));
163+
}
164+
165+
private static IReadOnlyList<ResourceMethod> SortGetAllMethodsByScopeBreadth(
166+
IReadOnlyList<ResourceMethod> listMethods,
167+
RequestPathPattern contextualPath)
168+
{
169+
if (listMethods.Count <= 1)
170+
{
171+
return listMethods;
172+
}
173+
174+
return [.. listMethods
175+
.OrderBy(m => CountExtraVariableSegments(contextualPath, m.OperationPath))
176+
.ThenBy(m => m.OperationPath.Count)];
177+
}
178+
179+
private static int CountExtraVariableSegments(RequestPathPattern contextualPath, RequestPathPattern operationPath)
180+
{
181+
if (!contextualPath.IsAncestorOf(operationPath))
182+
{
183+
return int.MaxValue;
184+
}
185+
186+
var extra = contextualPath.TrimAncestorFrom(operationPath);
187+
int count = 0;
188+
foreach (var segment in extra)
189+
{
190+
if (!segment.IsConstant)
191+
{
192+
count++;
193+
}
194+
}
195+
return count;
166196
}
167197

168198
public ResourceClientProvider Resource => _resource;
@@ -340,16 +370,28 @@ protected override MethodProvider[] BuildMethods()
340370

341371
private MethodProvider[] BuildGetAllMethods()
342372
{
343-
if (_getAll is null)
373+
if (_getAlls.Count == 0)
344374
{
345375
return [];
346376
}
347377

348-
// implement paging method GetAll
349-
_getAllSyncMethodProvider = BuildGetAllMethod(_getAll, false);
350-
var getAllAsync = BuildGetAllMethod(_getAll, true);
378+
var methods = new List<MethodProvider>(_getAlls.Count * 2);
379+
for (int i = 0; i < _getAlls.Count; i++)
380+
{
381+
var listMethod = _getAlls[i];
382+
var sync = BuildGetAllMethod(listMethod, false);
383+
var async = BuildGetAllMethod(listMethod, true);
384+
385+
if (i == 0)
386+
{
387+
_getAllSyncMethodProvider = sync;
388+
}
389+
390+
methods.Add(async);
391+
methods.Add(sync);
392+
}
351393

352-
return [getAllAsync, _getAllSyncMethodProvider];
394+
return [.. methods];
353395
}
354396

355397
private MethodProvider[] BuildEnumeratorMethods()

0 commit comments

Comments
 (0)