From c2a3d75a4bd068bae6e07eaabb992b4126b78dd7 Mon Sep 17 00:00:00 2001 From: Jian Hui Date: Fri, 17 Jul 2026 20:48:14 +0800 Subject: [PATCH 1/4] fix: resolve command generation regressions from the typeSpec toolchain upgrade --- .../controller/workspace_cfg_editor.py | 7 ++- .../model/configuration/_arg_builder.py | 5 ++ .../command/model/configuration/_command.py | 26 +++++++++ .../test_body_root_remap.py | 43 ++++++++++++++ .../swagger/model/schema/cmd_builder.py | 6 +- src/aaz_dev/utils/case.py | 3 + src/aaz_dev/utils/tests/__init__.py | 0 src/aaz_dev/utils/tests/test_case.py | 18 ++++++ src/typespec-aaz/src/convertor.ts | 4 +- src/typespec-aaz/test/record-type.test.ts | 41 ++++++++++++++ src/web/src/typespec/brower-host.ts | 56 +++++++++++++++++-- 11 files changed, 199 insertions(+), 10 deletions(-) create mode 100644 src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py create mode 100644 src/aaz_dev/utils/tests/__init__.py create mode 100644 src/aaz_dev/utils/tests/test_case.py create mode 100644 src/typespec-aaz/test/record-type.test.ts diff --git a/src/aaz_dev/command/controller/workspace_cfg_editor.py b/src/aaz_dev/command/controller/workspace_cfg_editor.py index 339e779c..0fd36a66 100644 --- a/src/aaz_dev/command/controller/workspace_cfg_editor.py +++ b/src/aaz_dev/command/controller/workspace_cfg_editor.py @@ -1043,10 +1043,13 @@ def _inherit_modification_in_command(cls, command, ref_command): # inherit arguments modification ref_args = [] + ref_options = {} if ref_command.arg_groups: for group in ref_command.arg_groups: - ref_args.extend(group.args) - command.generate_args(ref_args=ref_args) + for arg in group.args: + ref_args.append(arg) + ref_options[arg.var] = [*arg.options] + command.generate_args(ref_args=ref_args, ref_options=ref_options) # inherit outputs command.generate_outputs(ref_outputs=ref_command.outputs) diff --git a/src/aaz_dev/command/model/configuration/_arg_builder.py b/src/aaz_dev/command/model/configuration/_arg_builder.py index e20e592d..6995b4cb 100644 --- a/src/aaz_dev/command/model/configuration/_arg_builder.py +++ b/src/aaz_dev/command/model/configuration/_arg_builder.py @@ -113,6 +113,11 @@ def _need_flatten(self): if self.get_cls(): # not support to flatten object which is a cls. return False + if self._parent is None and self.schema.props and ( + self._arg_var.endswith("[]") or self._arg_var.endswith("{}")): + # Always flatten forked array/dict elements used as a command root body. + # Otherwise, stale inherited configs may regenerate invalid options such as "...[]" or "...{}". + return True if self._flatten is not None: return self._flatten if self.schema.client_flatten: diff --git a/src/aaz_dev/command/model/configuration/_command.py b/src/aaz_dev/command/model/configuration/_command.py index 95030d2f..c529a2d1 100644 --- a/src/aaz_dev/command/model/configuration/_command.py +++ b/src/aaz_dev/command/model/configuration/_command.py @@ -81,10 +81,36 @@ def generate_args(self, ref_args=None, ref_options=None): arg.options = [*ref_options[arg.var]] arguments[arg.var] = arg + if ref_options: + # Different generators use different body arg roots (e.g. "$parameters.*" vs "$resource.*"). + # When inheriting cfg changes from another generator, remap body-arg customizations to the + # current root so existing option overrides can still be matched and applied. + self._apply_ref_options_with_body_root_remap(arguments, ref_options) + arguments = handle_duplicated_options( arguments, has_subresource=has_subresource, operation_id=self.operations[-1].operation_id) self.arg_groups = self._build_arg_groups(arguments) + _NON_BODY_ARG_ROOTS = ("$Path", "$Query", "$Header") + + @classmethod + def _apply_ref_options_with_body_root_remap(cls, arguments, ref_options): + def root_of(var): + return var.split('.', 1)[0] + + body_roots = {root_of(var) for var in arguments if root_of(var) not in cls._NON_BODY_ARG_ROOTS} + if len(body_roots) != 1: + # only remap when there is exactly one body root; ambiguous otherwise. + return + cur_root = body_roots.pop() + for key, options in ref_options.items(): + root = root_of(key) + if root in cls._NON_BODY_ARG_ROOTS or root == cur_root: + continue + remapped = cur_root + key[len(root):] + if remapped in arguments: + arguments[remapped].options = [*options] + def generate_outputs(self, ref_outputs=None, pageable=None): if not ref_outputs: if self.outputs: diff --git a/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py b/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py new file mode 100644 index 00000000..76295eff --- /dev/null +++ b/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py @@ -0,0 +1,43 @@ +from command.model.configuration._command import CMDCommand + + +class _FakeArg: + def __init__(self, options): + self.options = options + + +def test_body_root_remap_applies_across_generator_roots(): + # TypeSpec generates body args under "$resource.*"; the inherited customizations come from a + # Swagger cfg keyed under "$parameters.*". The remap must re-apply them; Path args are untouched. + arguments = { + "$Path.applicationGatewayName": _FakeArg(["gateway-name"]), + "$resource.properties.sslCertificates[].name": _FakeArg(["ssl-certificate-name"]), + "$resource.properties.sslCertificates[].properties.password": _FakeArg(["password"]), + "$resource.properties.sslCertificates[].id": _FakeArg(["id"]), + } + ref_options = { + "$Path.applicationGatewayName": ["gateway-name"], + "$parameters.properties.sslCertificates[].name": ["n", "name"], + "$parameters.properties.sslCertificates[].properties.password": ["cert-password"], + "$parameters.properties.sslCertificates[].id": ["cert-id"], + } + CMDCommand._apply_ref_options_with_body_root_remap(arguments, ref_options) + assert arguments["$resource.properties.sslCertificates[].name"].options == ["n", "name"] + assert arguments["$resource.properties.sslCertificates[].properties.password"].options == ["cert-password"] + assert arguments["$resource.properties.sslCertificates[].id"].options == ["cert-id"] + # Path arg root is stable, so it is left as-is (already inherited via exact match earlier). + assert arguments["$Path.applicationGatewayName"].options == ["gateway-name"] + + +def test_body_root_remap_noop_when_root_matches(): + arguments = {"$resource.foo": _FakeArg(["foo"])} + CMDCommand._apply_ref_options_with_body_root_remap(arguments, {"$resource.foo": ["bar"]}) + # exact-var match is handled inline in generate_args, not here; same-root keys are skipped. + assert arguments["$resource.foo"].options == ["foo"] + + +def test_body_root_remap_skips_ambiguous_multiple_body_roots(): + arguments = {"$a.x": _FakeArg(["x"]), "$b.y": _FakeArg(["y"])} + CMDCommand._apply_ref_options_with_body_root_remap(arguments, {"$parameters.x": ["X"]}) + assert arguments["$a.x"].options == ["x"] + assert arguments["$b.y"].options == ["y"] diff --git a/src/aaz_dev/swagger/model/schema/cmd_builder.py b/src/aaz_dev/swagger/model/schema/cmd_builder.py index 9e8e6ce9..e7ff744a 100644 --- a/src/aaz_dev/swagger/model/schema/cmd_builder.py +++ b/src/aaz_dev/swagger/model/schema/cmd_builder.py @@ -238,7 +238,11 @@ def build_schema(self, schema): def _get_cls_definition_name(self, schema): assert isinstance(schema, ReferenceSchema) - schema_cls_name = f"{to_camel_case(schema.ref.split('/')[-1].replace('.', ' '))}_{self.mutability}" + # Generic type names carry angle brackets (e.g. "Record"). + # They must be stripped or the derived cls name produces invalid Python identifiers + # like "_args_record<...>" (SyntaxError). See aaz-dev-tools#562. + ref_name = re.sub(r'[<>]', '', schema.ref.split('/')[-1]) + schema_cls_name = f"{to_camel_case(ref_name.replace('.', ' '))}_{self.mutability}" if self.mutability != MutabilityEnum.Read: if self.read_only: schema_cls_name += "_read" diff --git a/src/aaz_dev/utils/case.py b/src/aaz_dev/utils/case.py index e58e010c..7ad51e9e 100644 --- a/src/aaz_dev/utils/case.py +++ b/src/aaz_dev/utils/case.py @@ -12,6 +12,9 @@ def to_camel_case(name, delimeters=""): def to_snake_case(name, separator='_'): assert isinstance(name, str) + # defense-in-depth: drop angle brackets from generic type names (e.g. "Record") so the + # result is a valid Python identifier. See aaz-dev-tools#562. + name = name.replace('<', '').replace('>', '') name = re.sub('(.)([A-Z][a-z]+)', r'\1' + separator + r'\2', name) name = re.sub('([a-z0-9])([A-Z])', r'\1' + separator + r'\2', name).lower() return name.replace('-', separator).replace('_', separator) diff --git a/src/aaz_dev/utils/tests/__init__.py b/src/aaz_dev/utils/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/aaz_dev/utils/tests/test_case.py b/src/aaz_dev/utils/tests/test_case.py new file mode 100644 index 00000000..49775adc --- /dev/null +++ b/src/aaz_dev/utils/tests/test_case.py @@ -0,0 +1,18 @@ +import ast + +from utils.case import to_snake_case, to_camel_case + + +def test_to_snake_case_strips_angle_brackets(): + # aaz-dev-tools#562: generic type names carry angle brackets that must not leak into identifiers. + result = to_snake_case("Record") + assert "<" not in result and ">" not in result + # the produced attribute/method name must be a valid Python identifier + ast.parse(f"_args_{result} = None") + + +def test_generic_cls_name_produces_valid_identifier(): + # mirrors the code generator: cls name -> "_args_" + snake_case must be assignable Python. + cls_name = to_camel_case("Record") + "_CreateOrUpdate_create" + for prefix in ("_args_", "_build_args_", "_schema_", "_build_schema_"): + ast.parse(f"{prefix}{to_snake_case(cls_name)} = None") diff --git a/src/typespec-aaz/src/convertor.ts b/src/typespec-aaz/src/convertor.ts index 357a7bdb..0ee1ff87 100644 --- a/src/typespec-aaz/src/convertor.ts +++ b/src/typespec-aaz/src/convertor.ts @@ -778,7 +778,9 @@ function convertModel2CMDObjectSchemaBase( } let pending; - if (context.supportClsSchema) { + // Record<> dict models have no usable schema identifier. + // Keep them inline rather than generating a shared cls schema. + if (context.supportClsSchema && !isRecordModelType(context.program, payloadModel)) { pending = context.pendingSchemas.getOrAdd(payloadModel, context.visibility, () => ({ type: payloadModel, visibility: context.visibility, diff --git a/src/typespec-aaz/test/record-type.test.ts b/src/typespec-aaz/test/record-type.test.ts new file mode 100644 index 00000000..5fd15e4e --- /dev/null +++ b/src/typespec-aaz/test/record-type.test.ts @@ -0,0 +1,41 @@ +import { TestHost, BasicTestRunner } from "@typespec/compiler/testing"; +import { describe, expect, it, beforeEach } from "vitest"; +import { createTypespecAazTestHost, createTypespecAazTestRunner, compileTypespecAAZOperations } from "./test-aaz.js"; +import { generateCompileArmResourceTemplate } from "./util.js"; + +describe("record type parsing", () => { + let host: TestHost; + let runner: BasicTestRunner; + + beforeEach(async () => { + host = await createTypespecAazTestHost(); + runner = await createTypespecAazTestRunner(host); + }); + + // Record<> dict models have no usable identifier name (getOpenAPITypeName + // returns "Record"). They must be inlined as additionalProps, never + // promoted to a shared cls schema, otherwise the generated Python identifier + // "_schema_record_read" is invalid syntax. See Azure/CLIPS#526. + it("record is inlined, never a cls", async () => { + const modelVar = { + // two Record props reference the same builtin model -> would + // trigger cls promotion (count >= 2) without the fix. + modelKey: + "@visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)\n @doc(\"tags a\")\n tagsA?: Record;\n" + + " @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)\n @doc(\"tags b\")\n tagsB?", + modelContent: "Record", + }; + const result = await compileTypespecAAZOperations( + generateCompileArmResourceTemplate(modelVar), + { + "operation": "get-resources-operations", + "api-version": "A", + "resources": ["/subscriptions/{}/resourcegroups/{}/providers/microsoft.mock/mockresources/{}"], + }, + runner, + ); + expect(result).toBeTruthy(); + expect(result!).not.toContain("Record"); + expect(result!).toContain("additionalProps"); + }); +}); diff --git a/src/web/src/typespec/brower-host.ts b/src/web/src/typespec/brower-host.ts index f292e51c..afcab52f 100644 --- a/src/web/src/typespec/brower-host.ts +++ b/src/web/src/typespec/brower-host.ts @@ -13,12 +13,33 @@ export function resolveVirtualPath(path: string, ...paths: string[]) { return resolvePath(rootPath, path, ...paths); } +// Axios errors are transport failures, never file-not-found responses. +// Retry transient socket exhaustion errors instead of treating the spec file as missing. +async function getWithRetry(url: string, maxAttempts = 8) { + let lastErr: any; + for (let attempt = 0; attempt < maxAttempts; attempt++) { + try { + return await axios.get(url); + } catch (e: any) { + // An error carrying an HTTP response is a real server error, not a socket issue — + // don't retry it. + if (e?.response != null) throw e; + lastErr = e; + await new Promise((r) => setTimeout(r, 100 * (attempt + 1))); + } + } + throw lastErr; +} + export async function createBrowserHost( libsToLoad: readonly string[], importOptions: LibraryImportOptions = {}, ): Promise { const virtualFs = new Map(); const jsImports = new Map>(); + // Cache stat results (including misses) to avoid repeated probes of the same paths. + // This prevents socket exhaustion and non-deterministic resource drops during compilation. + const statCache = new Map(); const libraries: Record = {}; for (const libName of libsToLoad) { @@ -151,24 +172,47 @@ export async function createBrowserHost( const spec_path = path.replace(rootPath, ""); if (!spec_path.includes("node_modules")) { + const cached = statCache.get(path); + if (cached) { + if (!cached.exists) { + const e = new Error(`File ${path} not found.`); + (e as any).code = "ENOENT"; + throw e; + } + return { + isDirectory() { + return cached.isDir; + }, + isFile() { + return cached.isFile; + }, + }; + } + let res; try { - res = await axios.get(`/Swagger/Specs/Stat${spec_path}`); - } catch { - const e = new Error(`File ${path} not found.`); - (e as any).code = "ENOENT"; - throw e; + res = await getWithRetry(`/Swagger/Specs/Stat${spec_path}`); + } catch (e: any) { + // Transport failure that survived every retry. Do NOT cache and do NOT report + // ENOENT: a false "not found" makes the compiler silently drop this spec file + // and all resources it defines. Surface the real error so it is visible. + throw new Error( + `Failed to stat ${path} after retries: ${e?.message ?? e}. ` + + `This is usually local socket exhaustion (ERR_NO_BUFFER_SPACE / ERR_ADDRESS_IN_USE).`, + ); } if (res.data.error) { + statCache.set(path, { isDir: false, isFile: false, exists: false }); const e = new Error(`File ${path} not found.`); (e as any).code = "ENOENT"; throw e; } if (res.data.isFile) { // cache the file in virtualFs - const content = await axios.get(`/Swagger/Specs/Files${spec_path}`); + const content = await getWithRetry(`/Swagger/Specs/Files${spec_path}`); virtualFs.set(path, content.data); } + statCache.set(path, { isDir: res.data.isDir, isFile: res.data.isFile, exists: true }); return { isDirectory() { return res.data.isDir; From eed439ac7ff14adc9556310d9aa850c09bb3b202 Mon Sep 17 00:00:00 2001 From: Jian Hui Date: Fri, 17 Jul 2026 21:08:01 +0800 Subject: [PATCH 2/4] fix: update history & version --- HISTORY.rst | 7 +++++++ version.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/HISTORY.rst b/HISTORY.rst index 5bc19dbf..02817353 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -3,6 +3,13 @@ Release History =============== +4.5.6 +++++++ +* Fix invalid Python identifiers generated from generic type names containing angle brackets (e.g. ``Record<...>``). +* Fix ``SyntaxError`` in generated subresource create/update commands caused by an unflattened array/dict element body argument. +* Fix loss of command argument customizations when regenerating a resource migrated from Swagger to TypeSpec, where the request body root changes from ``$parameters`` to ``$resource``. +* Harden the browser TypeSpec host with retry and stat caching for reliable resource picking. + 4.5.5 ++++++ * Fix camelCase ending with `S` incorrectly treated as plural. (#553) diff --git a/version.py b/version.py index f6866a91..e61e6c0a 100644 --- a/version.py +++ b/version.py @@ -1,5 +1,5 @@ -_MAJOR, _MINOR, _PATCH, _SUFFIX = ("4", "5", "5", "") +_MAJOR, _MINOR, _PATCH, _SUFFIX = ("4", "5", "6", "") # _PATCH: On main and in a nightly release the patch should be one ahead of the last released build. # _SUFFIX: This is mainly for nightly builds which have the suffix ".dev$DATE". See From 6452f18c6929ea9d8e29482308947dbeac1e7237 Mon Sep 17 00:00:00 2001 From: Jian Hui Date: Mon, 20 Jul 2026 08:20:58 +0800 Subject: [PATCH 3/4] fix: update history & version --- HISTORY.rst | 7 +++++++ version.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/HISTORY.rst b/HISTORY.rst index 6ea0c46c..c51e979c 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -3,6 +3,13 @@ Release History =============== +4.6.2 +++++++ +* Fix invalid Python identifiers generated from generic type names containing angle brackets (e.g. ``Record<...>``). +* Fix ``SyntaxError`` in generated subresource create/update commands caused by an unflattened array/dict element body argument. +* Fix loss of command argument customizations when regenerating a resource migrated from Swagger to TypeSpec, where the request body root changes from ``$parameters`` to ``$resource``. +* Harden the browser TypeSpec host with retry and stat caching for reliable resource picking. + 4.6.1 ++++++ * Unpinned ``setuptools`` (was ``==70.0.0``) to allow newer versions, capped at ``<81`` because setuptools 81+ drops ``setup.py``-based build support. diff --git a/version.py b/version.py index a00c0d2d..a06ec27d 100644 --- a/version.py +++ b/version.py @@ -1,5 +1,5 @@ -_MAJOR, _MINOR, _PATCH, _SUFFIX = ("4", "6", "1", "") +_MAJOR, _MINOR, _PATCH, _SUFFIX = ("4", "6", "2", "") # _PATCH: On main and in a nightly release the patch should be one ahead of the last released build. # _SUFFIX: This is mainly for nightly builds which have the suffix ".dev$DATE". See From 3a90b094361803086a8140c25d5fb61f0d6f8f06 Mon Sep 17 00:00:00 2001 From: Jian Hui Date: Wed, 22 Jul 2026 11:22:03 +0800 Subject: [PATCH 4/4] fix: align TypeSpec-generated commands with Swagger output --- HISTORY.rst | 2 + .../model/configuration/_arg_builder.py | 7 +- .../command/model/configuration/_command.py | 75 +++++++++++ .../test_body_root_remap.py | 79 ++++++++++++ src/typespec-aaz/src/convertor.ts | 34 +++-- src/typespec-aaz/src/utils.ts | 5 +- src/typespec-aaz/test/cls-naming.test.ts | 43 ++++++ .../test/custom-azure-resource.test.ts | 122 ++++++++++++++++++ 8 files changed, 351 insertions(+), 16 deletions(-) create mode 100644 src/typespec-aaz/test/cls-naming.test.ts create mode 100644 src/typespec-aaz/test/custom-azure-resource.test.ts diff --git a/HISTORY.rst b/HISTORY.rst index c51e979c..26cdaa7c 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -8,6 +8,8 @@ Release History * Fix invalid Python identifiers generated from generic type names containing angle brackets (e.g. ``Record<...>``). * Fix ``SyntaxError`` in generated subresource create/update commands caused by an unflattened array/dict element body argument. * Fix loss of command argument customizations when regenerating a resource migrated from Swagger to TypeSpec, where the request body root changes from ``$parameters`` to ``$resource``. +* Emit ``location`` as a ResourceLocation and hide the resource-envelope ``id`` (read-only ResourceId) for legacy ``@customAzureResource`` models, matching the Swagger-generated commands. +* Align generated ``@cls`` reference class names with the Swagger convertor (preserve PascalCase and drop the create/update visibility infix) so TypeSpec output matches Swagger. * Harden the browser TypeSpec host with retry and stat caching for reliable resource picking. 4.6.1 diff --git a/src/aaz_dev/command/model/configuration/_arg_builder.py b/src/aaz_dev/command/model/configuration/_arg_builder.py index 6995b4cb..1c213a8d 100644 --- a/src/aaz_dev/command/model/configuration/_arg_builder.py +++ b/src/aaz_dev/command/model/configuration/_arg_builder.py @@ -339,10 +339,9 @@ def get_hide(self): if getattr(self.schema, 'name', None) == 'id' and not self.get_required() and self._parent and \ isinstance(self.schema, CMDResourceIdSchema): if self._arg_var.split('.', maxsplit=1)[-1] == 'id': - # hide top level 'id' property when it has 'name' property, - for prop in self._parent.schema.props: - if prop.name == 'name': - return True + # hide the resource's own read-only ARM id. Swagger relied on a sibling frozen + # 'name'; TypeSpec strips read-only siblings, so hide any optional top-level id. + return True if getattr(self.schema, 'name', None) in ['userAssignedIdentities', 'type'] and self._parent and \ isinstance(self._parent.schema, CMDIdentityObjectSchema): diff --git a/src/aaz_dev/command/model/configuration/_command.py b/src/aaz_dev/command/model/configuration/_command.py index c529a2d1..eb5b83ff 100644 --- a/src/aaz_dev/command/model/configuration/_command.py +++ b/src/aaz_dev/command/model/configuration/_command.py @@ -62,6 +62,12 @@ def generate_args(self, ref_args=None, ref_options=None): ref_args.extend(group.args) ref_args = ref_args or None + if ref_args: + # Reference args may use a different body root (e.g. "$parameters.*" vs tsp's "$resource.*"), + # which breaks the arg builder's exact-var match and loses inherited customizations. Remap + # them onto the current body root so they match and are inherited. + ref_args = self._remap_ref_args_to_body_root(ref_args) + arguments = {} has_subresource = False if self.subresource_selector: @@ -111,6 +117,75 @@ def root_of(var): if remapped in arguments: arguments[remapped].options = [*options] + def _detect_body_arg_root(self): + roots = set() + for op in self.operations: + schema = None + http = getattr(op, 'http', None) + if http is not None: + request = getattr(http, 'request', None) + body = getattr(request, 'body', None) if request is not None else None + json_body = getattr(body, 'json', None) if body is not None else None + schema = getattr(json_body, 'schema', None) if json_body is not None else None + else: + for action_attr in ('instance_update', 'instance_create'): + action = getattr(op, action_attr, None) + json_body = getattr(action, 'json', None) if action is not None else None + if json_body is not None: + schema = getattr(json_body, 'schema', None) + break + name = getattr(schema, 'name', None) if schema is not None else None + if name: + # schema.name may be a full path (e.g. "resource.properties.sslCertificates[]"); + # keep only the leading root token. + root = '$' + name.replace('$', '') + root = root.split('.', 1)[0].split('[', 1)[0] + roots.add(root) + if len(roots) == 1: + return roots.pop() + return None + + def _remap_ref_args_to_body_root(self, ref_args): + def root_of(var): + return var.split('.', 1)[0].split('[', 1)[0] + + cur_root = self._detect_body_arg_root() + if not cur_root: + return ref_args + ref_roots = { + root_of(arg.var) for arg in ref_args + if getattr(arg, 'var', None) and root_of(arg.var) not in self._NON_BODY_ARG_ROOTS + } + if len(ref_roots) != 1: + # only remap when the reference args have exactly one body root. + return ref_args + ref_root = ref_roots.pop() + if ref_root == cur_root: + return ref_args + # schematics models can't be deepcopied; round-trip through primitives to clone (preserving + # polymorphic subtypes) so we don't mutate the caller's ref_args. + remapped = list(CMDArgGroup({"name": "", "args": [a.to_primitive() for a in ref_args]}).args) + for arg in remapped: + self._remap_arg_var_root(arg, ref_root, cur_root) + return remapped + + @classmethod + def _remap_arg_var_root(cls, node, old_root, new_root): + if node is None: + return + var = getattr(node, 'var', None) + if var: + if var == old_root: + node.var = new_root + elif var.startswith(old_root + '.') or var.startswith(old_root + '['): + node.var = new_root + var[len(old_root):] + for sub in (getattr(node, 'args', None) or []): + cls._remap_arg_var_root(sub, old_root, new_root) + cls._remap_arg_var_root(getattr(node, 'item', None), old_root, new_root) + additional_props = getattr(node, 'additional_props', None) + if additional_props is not None: + cls._remap_arg_var_root(getattr(additional_props, 'item', None), old_root, new_root) + def generate_outputs(self, ref_outputs=None, pageable=None): if not ref_outputs: if self.outputs: diff --git a/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py b/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py index 76295eff..4d4fd340 100644 --- a/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py +++ b/src/aaz_dev/command/tests/configuration_tests/test_body_root_remap.py @@ -41,3 +41,82 @@ def test_body_root_remap_skips_ambiguous_multiple_body_roots(): CMDCommand._apply_ref_options_with_body_root_remap(arguments, {"$parameters.x": ["X"]}) assert arguments["$a.x"].options == ["x"] assert arguments["$b.y"].options == ["y"] + + +def _ssl_ref_args(): + from command.model.configuration._arg import CMDStringArg, CMDObjectArgBase, CMDArrayArg + inner_id = CMDStringArg({"var": "$parameters.properties.sslCertificates[].id", + "options": ["cert-id"], "hide": True}) + inner_pwd = CMDStringArg({"var": "$parameters.properties.sslCertificates[].properties.password", + "options": ["cert-password"]}) + elem = CMDObjectArgBase({"args": [inner_id, inner_pwd]}) + arr = CMDArrayArg({"var": "$parameters.properties.sslCertificates", + "options": ["ssl-certs"], "item": elem}) + top = CMDStringArg({"var": "$parameters.location", "options": ["l", "location"]}) + return [arr, top] + + +def _collect_vars(node, out): + var = getattr(node, "var", None) + if var: + out[var] = node + for sub in (getattr(node, "args", None) or []): + _collect_vars(sub, out) + item = getattr(node, "item", None) + if item is not None: + _collect_vars(item, out) + + +def test_remap_arg_var_root_rewrites_nested_vars_and_preserves_customizations(): + ref_args = _ssl_ref_args() + for arg in ref_args: + CMDCommand._remap_arg_var_root(arg, "$parameters", "$resource") + collected = {} + for arg in ref_args: + _collect_vars(arg, collected) + assert set(collected) == { + "$resource.properties.sslCertificates", + "$resource.properties.sslCertificates[].id", + "$resource.properties.sslCertificates[].properties.password", + "$resource.location", + } + # hide/options survive the remap so the current generator can inherit them. + assert collected["$resource.properties.sslCertificates[].id"].hide is True + assert collected["$resource.properties.sslCertificates[].properties.password"].options == ["cert-password"] + + +class _BodyRootCommand(CMDCommand): + def __init__(self, body_root): + self._body_root = body_root + + def _detect_body_arg_root(self): + return self._body_root + + +def test_remap_ref_args_clones_and_rewrites_body_root(): + ref_args = _ssl_ref_args() + cmd = _BodyRootCommand("$resource") + remapped = cmd._remap_ref_args_to_body_root(ref_args) + # original reference args are not mutated (clone) ... + orig = {} + for arg in ref_args: + _collect_vars(arg, orig) + assert all(v.startswith("$parameters") for v in orig) + # ... and the returned args are rewritten onto the current body root. + remapped_vars = {} + for arg in remapped: + _collect_vars(arg, remapped_vars) + assert "$resource.properties.sslCertificates[].id" in remapped_vars + assert remapped_vars["$resource.properties.sslCertificates[].id"].hide is True + + +def test_remap_ref_args_noop_when_root_matches(): + ref_args = _ssl_ref_args() + cmd = _BodyRootCommand("$parameters") + assert cmd._remap_ref_args_to_body_root(ref_args) is ref_args + + +def test_remap_ref_args_noop_when_no_body_root(): + ref_args = _ssl_ref_args() + cmd = _BodyRootCommand(None) + assert cmd._remap_ref_args_to_body_root(ref_args) is ref_args diff --git a/src/typespec-aaz/src/convertor.ts b/src/typespec-aaz/src/convertor.ts index 0ee1ff87..dec10169 100644 --- a/src/typespec-aaz/src/convertor.ts +++ b/src/typespec-aaz/src/convertor.ts @@ -9,11 +9,10 @@ import { getHeaderFieldOptions, getServers, getStatusCodeDescription, - getVisibilitySuffix, resolveRequestVisibility, HttpProperty, } from "@typespec/http"; -import { isAzureResource } from "@azure-tools/typespec-azure-resource-manager"; +import { isAzureResource, isCustomAzureResource } from "@azure-tools/typespec-azure-resource-manager"; import { AAZEmitterContext, AAZOperationEmitterContext, AAZSchemaEmitterContext } from "./context.js"; import { resolveOperationId, toCamelCase } from "./utils.js"; import { TypeSpecPathItem } from "./model/path_item.js"; @@ -90,6 +89,7 @@ import { CMDUuidSchemaBase, CMDPasswordSchemaBase, CMDResourceIdSchemaBase, + CMDResourceIdSchema, CMDDateSchemaBase, CMDDateTimeSchemaBase, CMDDurationSchemaBase, @@ -932,6 +932,20 @@ function convertModel2CMDObjectSchemaBase( } as CMDResourceLocationSchema; } + if ( + context.visibility !== Visibility.Read && + isAzureResourceOverall(context, payloadModel) && + properties.id && + properties.id.type === "string" + ) { + // Legacy custom resources may leak their read-only ARM id as a writable string. Emit it + // as a ResourceId (write payloads only) so the arg builder hides it, matching swagger. + properties.id = { + ...(properties.id as CMDStringSchema), + type: "ResourceId", + } as CMDResourceIdSchema; + } + if (properties.userAssignedIdentities && properties.type) { object = { ...object, @@ -1419,13 +1433,15 @@ function getDiscriminatorInfo(context: AAZSchemaEmitterContext, model: Model): D } function isAzureResourceOverall(context: AAZSchemaEmitterContext, model: Model): boolean { - let current = model; - let isResource = isAzureResource(context.program, current); - while (!isResource && current.baseModel) { + // Also match legacy custom resources (@customAzureResource), else "location" isn't a ResourceLocation. + let current: Model | undefined = model; + while (current) { + if (isAzureResource(context.program, current) || isCustomAzureResource(context.program, current)) { + return true; + } current = current.baseModel; - isResource = isAzureResource(context.program, current); } - return !!isResource; + return false; } function convertLiteral2CMDSchemaBase( @@ -1495,10 +1511,6 @@ function processPendingSchemas( } else { const name = getOpenAPITypeName(context.program, type, context.typeNameOptions); let ref_name = toCamelCase(name.replace(/\./g, " ")); - if (group.size > 1 && visibility !== Visibility.Read) { - // TODO: handle item - ref_name += getVisibilitySuffix(verbVisibility, Visibility.Read); - } if (Visibility.Read !== visibility) { ref_name += "_" + suffix; } else { diff --git a/src/typespec-aaz/src/utils.ts b/src/typespec-aaz/src/utils.ts index caa53f96..13704d5c 100644 --- a/src/typespec-aaz/src/utils.ts +++ b/src/typespec-aaz/src/utils.ts @@ -113,7 +113,10 @@ export function toCamelCase(name: string, delimiters: string = ""): string { const parts = name.replace(/[-_]/g, " ").split(" "); const camelCasedParts = parts.map((part) => { if (part) { - return part.charAt(0).toUpperCase() + part.slice(1).toLowerCase(); + // Preserve the rest of each part's original casing to match the Python `to_camel_case` + // twin (utils/case.py); lowercasing it would collapse PascalCase names like + // "ApplicationGatewaySslPolicy" into "Applicationgatewaysslpolicy". + return part.charAt(0).toUpperCase() + part.slice(1); } return ""; }); diff --git a/src/typespec-aaz/test/cls-naming.test.ts b/src/typespec-aaz/test/cls-naming.test.ts new file mode 100644 index 00000000..fe994585 --- /dev/null +++ b/src/typespec-aaz/test/cls-naming.test.ts @@ -0,0 +1,43 @@ +import { TestHost, BasicTestRunner } from "@typespec/compiler/testing"; +import { describe, expect, it, beforeEach } from "vitest"; +import { createTypespecAazTestHost, createTypespecAazTestRunner, compileTypespecAAZOperations } from "./test-aaz.js"; +import { generateCompileArmResourceTemplate } from "./util.js"; + +describe("cls schema naming", () => { + let host: TestHost; + let runner: BasicTestRunner; + + beforeEach(async () => { + host = await createTypespecAazTestHost(); + runner = await createTypespecAazTestRunner(host); + }); + + // A cls-promoted schema must be named "_" to match the Swagger + // converter (utils/case.py + swagger cmd_builder). Two regressions used to break parity: + // 1. toCamelCase lowercased the tail -> "Identityconfigurationproperties" (see #564 feedback). + // 2. an extra getVisibilitySuffix infix -> "...CreateOrUpdate_create", absent in Swagger. + it("preserves PascalCase and omits the verb-visibility infix", async () => { + const modelVar = { + // reference IdentityConfigurationProperties a second/third time so count >= 2 -> cls promotion. + modelKey: + "@visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)\n @doc(\"extra a\")\n extraIdentityA?: IdentityConfigurationProperties;\n" + + " @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)\n @doc(\"extra b\")\n extraIdentityB?", + modelContent: "IdentityConfigurationProperties", + }; + const result = await compileTypespecAAZOperations( + generateCompileArmResourceTemplate(modelVar), + { + "operation": "get-resources-operations", + "api-version": "A", + "resources": ["/subscriptions/{}/resourcegroups/{}/providers/microsoft.mock/mockresources/{}"], + }, + runner, + ); + expect(result).toBeTruthy(); + // PascalCase preserved with a plain "_" suffix, matching Swagger. + expect(result!).toContain("IdentityConfigurationProperties_"); + // no lowercased-tail collapse and no CreateOrUpdate infix. + expect(result!).not.toContain("Identityconfigurationproperties"); + expect(result!).not.toContain("CreateOrUpdate"); + }); +}); diff --git a/src/typespec-aaz/test/custom-azure-resource.test.ts b/src/typespec-aaz/test/custom-azure-resource.test.ts new file mode 100644 index 00000000..33da484b --- /dev/null +++ b/src/typespec-aaz/test/custom-azure-resource.test.ts @@ -0,0 +1,122 @@ +import { TestHost, BasicTestRunner } from "@typespec/compiler/testing"; +import { describe, expect, it, beforeEach } from "vitest"; +import { createTypespecAazTestHost, createTypespecAazTestRunner, compileTypespecAAZOperations } from "./test-aaz.js"; +import { findObjectsWithKey } from "./util.js"; + +function collectObjectsWithName(obj: any, targetName: string): any[] { + const results: any[] = []; + const search = (value: any): void => { + if (Array.isArray(value)) { + value.forEach(search); + } else if (typeof value === "object" && value !== null) { + if (value.name === targetName) { + results.push(value); + } + Object.values(value).forEach(search); + } + }; + search(obj); + return results; +} + +// Legacy specs (e.g. Microsoft.Network) do not use the standard TrackedResource +// template. They mark their resource envelope with +// @Azure.ResourceManager.Legacy.customAzureResource(#{ isAzureResource: true }). +// isAzureResource() does not recognize those, so the emitter used to emit the +// resource "location" as a plain string instead of a ResourceLocation. See +// Azure/aaz-dev-tools#564. +const code = ` + @armProviderNamespace("Microsoft.Mock") + @service(#{ title: "Microsoft.Mock" }) + @versioned(Versions) + namespace Microsoft.Mock; + + enum Versions { A } + + interface Operations extends Azure.ResourceManager.Operations {} + + #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "test" + @Azure.ResourceManager.Legacy.customAzureResource(#{ isAzureResource: true }) + model BaseResource { + id?: string; + @visibility(Lifecycle.Read) name?: string; + @visibility(Lifecycle.Read) type?: string; + location?: string; + #suppress "@azure-tools/typespec-azure-resource-manager/arm-no-record" "test" + tags?: Record; + } + + model MockResourceProperties { + portalName?: string; + subResource?: MockSubResource; + } + + model MockSubResource { + id?: string; + subName?: string; + } + + + #suppress "@azure-tools/typespec-azure-core/composition-over-inheritance" "test" + #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "test" + model MockResource extends BaseResource { + properties?: MockResourceProperties; + + @visibility(Lifecycle.Read) + @path + @key("mockName") + @segment("mockResources") + name: string; + } + + #suppress "@azure-tools/typespec-azure-core/no-legacy-usage" "test" + @armResourceOperations + interface MockResources { + createOrUpdate is ArmResourceCreateOrReplaceAsync; + } +`; + +describe("custom azure resource (legacy) parsing", () => { + let host: TestHost; + let runner: BasicTestRunner; + + beforeEach(async () => { + host = await createTypespecAazTestHost(); + runner = await createTypespecAazTestRunner(host); + }); + + it("emits location as ResourceLocation for customAzureResource models", async () => { + const result = await compileTypespecAAZOperations( + code, + { + "operation": "get-resources-operations", + "api-version": "A", + "resources": ["/subscriptions/{}/resourcegroups/{}/providers/microsoft.mock/mockresources/{}"], + }, + runner, + ); + expect(result).toBeTruthy(); + const location = findObjectsWithKey(JSON.parse(result!), "location") as { type?: string } | undefined; + expect(location).toBeTruthy(); + expect(location?.type).toBe("ResourceLocation"); + }); + + it("emits the resource envelope id as ResourceId while leaving nested ids as string", async () => { + const result = await compileTypespecAAZOperations( + code, + { + "operation": "get-resources-operations", + "api-version": "A", + "resources": ["/subscriptions/{}/resourcegroups/{}/providers/microsoft.mock/mockresources/{}"], + }, + runner, + ); + expect(result).toBeTruthy(); + const ids = collectObjectsWithName(JSON.parse(result!), "id"); + expect(ids.length).toBeGreaterThan(0); + // The resource's own envelope id is converted to a ResourceId (so it gets hidden downstream). + expect(ids.some((id) => id.type === "ResourceId")).toBe(true); + // A nested, non-resource object's id must remain a plain string. + expect(ids.some((id) => id.type === "string")).toBe(true); + }); +});