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/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; 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