Skip to content

Commit 0148c04

Browse files
Copilotkazrael2119
Copilot
and
kazrael2119
authored
fix: narrow ARM metadata filter to only strip exclusively read-only properties (#3931)
* Initial plan * fix: narrow ARM metadata filter to only strip exclusively read-only properties The condition in buildModelInterface was too broad - it stripped any property with Visibility.Read (even if it had other visibilities too), which caused @path properties on parameter bag models to be incorrectly removed. Fix: require p.visibility?.length === 1 so only truly read-only (exclusively Visibility.Read) properties are filtered out in ARM mode. Add scenario test: models/arm/pathPropertyInParameterBag.md Agent-Logs-Url: https://github.com/Azure/autorest.typescript/sessions/d8d0fef5-8c72-4a9c-aecd-e64f91755e1c Co-authored-by: kazrael2119 <98569699+kazrael2119@users.noreply.github.com> * test: add ARM visibility unit tests for Read, Read&Create, Read&Update&Create cases Agent-Logs-Url: https://github.com/Azure/autorest.typescript/sessions/43e77bb1-a896-4e24-af7f-f4bd39018215 Co-authored-by: kazrael2119 <98569699+kazrael2119@users.noreply.github.com> * refactor: combine four ARM visibility scenario files into one armMetadataFilter.md Agent-Logs-Url: https://github.com/Azure/autorest.typescript/sessions/f5acd95e-aed2-4be5-b46d-776ced2b3502 Co-authored-by: kazrael2119 <98569699+kazrael2119@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kazrael2119 <98569699+kazrael2119@users.noreply.github.com>
1 parent f75c327 commit 0148c04

2 files changed

Lines changed: 278 additions & 1 deletion

File tree

‎packages/typespec-ts/src/modular/emitModels.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,12 +747,15 @@ function buildModelInterface(
747747
if (!hasInputUsage && p.__raw && isMetadata(context.program, p.__raw)) {
748748
return false;
749749
}
750-
// Skip required metadata properties with Read visibility for ARM as they are not intended to be in the model
750+
// Skip required metadata properties with Read-only visibility for ARM as they are not intended to be in the model
751751
// These properties are not be generated no matter they are in input or output models in most cases in HLC
752+
// Only skip properties that have exclusively Read visibility to avoid stripping @path properties
753+
// on parameter bag models that also carry other visibility flags (e.g. Create/Update)
752754
if (
753755
context.arm &&
754756
p.__raw &&
755757
isMetadata(context.program, p.__raw) &&
758+
p.visibility?.length === 1 &&
756759
p.visibility?.includes(Visibility.Read)
757760
) {
758761
return false;

‎packages/typespec-ts/test/modularUnit/scenarios/models/arm/armMetadataFilter.md‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
# ARM metadata filter should not strip @path properties from parameter bag models with no ARM base class
2+
3+
In ARM mode, the metadata filter should only strip properties that have exclusively
4+
`Lifecycle.Read` visibility (i.e. truly read-only ARM properties like `id`, `name`, `type`).
5+
It must NOT strip `@path`-decorated properties on plain parameter bag models that have no ARM
6+
base class, because those properties carry multiple visibility flags (Read + Create + Update…)
7+
and must remain accessible in the generated interface so users can set them.
8+
9+
## TypeSpec
10+
11+
```tsp
12+
import "@typespec/http";
13+
import "@typespec/rest";
14+
import "@typespec/versioning";
15+
import "@azure-tools/typespec-azure-core";
16+
import "@azure-tools/typespec-azure-resource-manager";
17+
import "@azure-tools/typespec-client-generator-core";
18+
19+
using TypeSpec.Http;
20+
using TypeSpec.Rest;
21+
using TypeSpec.Versioning;
22+
using Azure.Core;
23+
using Azure.ResourceManager;
24+
using Azure.ClientGenerator.Core;
25+
26+
@armProviderNamespace
27+
@service(#{
28+
title: "Test ARM Service",
29+
})
30+
@versioned(Versions)
31+
namespace Microsoft.Test;
32+
33+
enum Versions {
34+
@armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5)
35+
v2021_10_01: "2021-10-01",
36+
}
37+
38+
/** Parameters for the private link resource. */
39+
model PrivateLinkParameters {
40+
/** The name of the private link resource. */
41+
@path
42+
privateLinkName: string;
43+
}
44+
45+
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-operation" "test"
46+
#suppress "@azure-tools/typespec-azure-core/documentation-required" "test"
47+
op getPrivateLink(
48+
...ApiVersionParameter,
49+
...SubscriptionIdParameter,
50+
...ResourceGroupParameter,
51+
params?: PrivateLinkParameters,
52+
): ArmResponse<{}> | ErrorResponse;
53+
```
54+
55+
```yaml
56+
needTCGC: true
57+
withRawContent: true
58+
mustEmptyDiagnostic: false
59+
```
60+
61+
## Models interface PrivateLinkParameters
62+
63+
```ts models interface PrivateLinkParameters
64+
/**
65+
* This file contains only generated model types and their (de)serializers.
66+
* Disable the following rules for internal models with '_' prefix and deserializers which require 'any' for raw JSON input.
67+
*/
68+
/* eslint-disable @typescript-eslint/naming-convention */
69+
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
70+
/** Parameters for the private link resource. */
71+
export interface PrivateLinkParameters {
72+
/** The name of the private link resource. */
73+
privateLinkName: string;
74+
}
75+
```
76+
77+
# ARM metadata filter should preserve @path property with Read & Create visibility
78+
79+
In ARM mode, a `@path` property that carries both `Lifecycle.Read` AND `Lifecycle.Create`
80+
visibility flags must NOT be stripped by the metadata filter, because the filter only removes
81+
properties that are *exclusively* read-only (i.e. `visibility.length === 1 && Read`).
82+
83+
## TypeSpec
84+
85+
```tsp
86+
import "@typespec/http";
87+
import "@typespec/rest";
88+
import "@typespec/versioning";
89+
import "@azure-tools/typespec-azure-core";
90+
import "@azure-tools/typespec-azure-resource-manager";
91+
import "@azure-tools/typespec-client-generator-core";
92+
93+
using TypeSpec.Http;
94+
using TypeSpec.Rest;
95+
using TypeSpec.Versioning;
96+
using Azure.Core;
97+
using Azure.ResourceManager;
98+
using Azure.ClientGenerator.Core;
99+
100+
@armProviderNamespace
101+
@service(#{
102+
title: "Test ARM Service",
103+
})
104+
@versioned(Versions)
105+
namespace Microsoft.Test;
106+
107+
enum Versions {
108+
@armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5)
109+
v2021_10_01: "2021-10-01",
110+
}
111+
112+
model ParamBagReadCreate {
113+
@path
114+
@visibility(Lifecycle.Read, Lifecycle.Create)
115+
resourceName: string;
116+
}
117+
118+
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-operation" "test"
119+
#suppress "@azure-tools/typespec-azure-core/documentation-required" "test"
120+
op getResource(params?: ParamBagReadCreate): ArmResponse<{}> | ErrorResponse;
121+
```
122+
123+
```yaml
124+
needTCGC: true
125+
withRawContent: true
126+
mustEmptyDiagnostic: false
127+
```
128+
129+
## Models interface ParamBagReadCreate
130+
131+
```ts models interface ParamBagReadCreate
132+
/**
133+
* This file contains only generated model types and their (de)serializers.
134+
* Disable the following rules for internal models with '_' prefix and deserializers which require 'any' for raw JSON input.
135+
*/
136+
/* eslint-disable @typescript-eslint/naming-convention */
137+
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
138+
/** model interface ParamBagReadCreate */
139+
export interface ParamBagReadCreate {
140+
resourceName: string;
141+
}
142+
```
143+
144+
# ARM metadata filter should strip @path property with exclusively Read visibility
145+
146+
In ARM mode, a `@path` property that has *only* `Lifecycle.Read` visibility should be
147+
stripped by the metadata filter. Such a property is purely decorative/read-only metadata
148+
(e.g. an ARM resource system-assigned `id`) and must not appear in the model interface.
149+
150+
## TypeSpec
151+
152+
```tsp
153+
import "@typespec/http";
154+
import "@typespec/rest";
155+
import "@typespec/versioning";
156+
import "@azure-tools/typespec-azure-core";
157+
import "@azure-tools/typespec-azure-resource-manager";
158+
import "@azure-tools/typespec-client-generator-core";
159+
160+
using TypeSpec.Http;
161+
using TypeSpec.Rest;
162+
using TypeSpec.Versioning;
163+
using Azure.Core;
164+
using Azure.ResourceManager;
165+
using Azure.ClientGenerator.Core;
166+
167+
@armProviderNamespace
168+
@service(#{
169+
title: "Test ARM Service",
170+
})
171+
@versioned(Versions)
172+
namespace Microsoft.Test;
173+
174+
enum Versions {
175+
@armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5)
176+
v2021_10_01: "2021-10-01",
177+
}
178+
179+
model ParamBagReadOnly {
180+
@path
181+
@visibility(Lifecycle.Read)
182+
resourceName: string;
183+
}
184+
185+
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-operation" "test"
186+
#suppress "@azure-tools/typespec-azure-core/documentation-required" "test"
187+
op getResource(params?: ParamBagReadOnly): ArmResponse<{}> | ErrorResponse;
188+
```
189+
190+
```yaml
191+
needTCGC: true
192+
withRawContent: true
193+
mustEmptyDiagnostic: false
194+
```
195+
196+
## Models interface ParamBagReadOnly
197+
198+
```ts models interface ParamBagReadOnly
199+
/**
200+
* This file contains only generated model types and their (de)serializers.
201+
* Disable the following rules for internal models with '_' prefix and deserializers which require 'any' for raw JSON input.
202+
*/
203+
/* eslint-disable @typescript-eslint/naming-convention */
204+
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
205+
/** model interface ParamBagReadOnly */
206+
export interface ParamBagReadOnly {}
207+
```
208+
209+
# ARM metadata filter should preserve @path property with Read & Update & Create visibility
210+
211+
In ARM mode, a `@path` property that carries `Lifecycle.Read`, `Lifecycle.Update`, AND
212+
`Lifecycle.Create` visibility flags must NOT be stripped. Only properties that are
213+
*exclusively* read-only (`visibility.length === 1 && Read`) are filtered out.
214+
215+
## TypeSpec
216+
217+
```tsp
218+
import "@typespec/http";
219+
import "@typespec/rest";
220+
import "@typespec/versioning";
221+
import "@azure-tools/typespec-azure-core";
222+
import "@azure-tools/typespec-azure-resource-manager";
223+
import "@azure-tools/typespec-client-generator-core";
224+
225+
using TypeSpec.Http;
226+
using TypeSpec.Rest;
227+
using TypeSpec.Versioning;
228+
using Azure.Core;
229+
using Azure.ResourceManager;
230+
using Azure.ClientGenerator.Core;
231+
232+
@armProviderNamespace
233+
@service(#{
234+
title: "Test ARM Service",
235+
})
236+
@versioned(Versions)
237+
namespace Microsoft.Test;
238+
239+
enum Versions {
240+
@armCommonTypesVersion(Azure.ResourceManager.CommonTypes.Versions.v5)
241+
v2021_10_01: "2021-10-01",
242+
}
243+
244+
model ParamBagReadUpdateCreate {
245+
@path
246+
@visibility(Lifecycle.Read, Lifecycle.Update, Lifecycle.Create)
247+
resourceName: string;
248+
}
249+
250+
#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-operation" "test"
251+
#suppress "@azure-tools/typespec-azure-core/documentation-required" "test"
252+
op getResource(params?: ParamBagReadUpdateCreate): ArmResponse<{}> | ErrorResponse;
253+
```
254+
255+
```yaml
256+
needTCGC: true
257+
withRawContent: true
258+
mustEmptyDiagnostic: false
259+
```
260+
261+
## Models interface ParamBagReadUpdateCreate
262+
263+
```ts models interface ParamBagReadUpdateCreate
264+
/**
265+
* This file contains only generated model types and their (de)serializers.
266+
* Disable the following rules for internal models with '_' prefix and deserializers which require 'any' for raw JSON input.
267+
*/
268+
/* eslint-disable @typescript-eslint/naming-convention */
269+
/* eslint-disable @typescript-eslint/explicit-module-boundary-types */
270+
/** model interface ParamBagReadUpdateCreate */
271+
export interface ParamBagReadUpdateCreate {
272+
resourceName: string;
273+
}
274+
```

0 commit comments

Comments
 (0)