Fix KeyError: 'name' in asyncapi generation by passing rsrc_name to g…

[dhruv-techdev]

Fixes #94

What was wrong

firestone generate asyncapi crashed every time with KeyError: 'name', even on the example files in examples/addressbook.

The code was looking for a name field in the resource file. That field was renamed to kind in #73, and this one spot was missed.

What this PR changes

One line. generate() already has the right value in rsrc_name (taken from kind), but it never passed it to get_channels(). Now it does:

+            rsrc_name=rsrc_name,

How to test

cd examples/addressbook
firestone generate -t Test -d Test -v 1 -r addressbook.yaml,person.yaml,postal_codes.yaml asyncapi

Before: crashes with KeyError: 'name'.
After: prints the AsyncAPI document.

[dhruv-techdev]

Kept this to the one-line fix. Happy to follow up with an AsyncAPI test and a regenerated examples/addressbook/asyncapi.yaml if you'd like! @ebourgeois

[ebourgeois]

Kept this to the one-line fix. Happy to follow up with an AsyncAPI test and a regenerated examples/addressbook/asyncapi.yaml if you'd like! @ebourgeois

Hi @dhruv-techdev, thanks for this! Yes please make sure too regenerate and add a test please.

[dhruv-techdev]

Done! Added test/spec/test_asyncapi.py and regenerated the example.

While regenerating, I found the output wasn't valid YAML: channels: was stuck to the previous line because of {%- endif -%} in asyncapi.jinja2. I changed it to {%- endif %}, so the line break is kept. Please have a look @ebourgeois.

[ebourgeois]

Review of the fix for #94. The fix itself is correct: passing rsrc_name into get_channels() fixes the crash, and the {%- endif %} change fixes the type: objectchannels: corruption. The new tests pass, black and pylint are clean, and the generator reproduces the committed example exactly. But generate asyncapi still crashes or produces wrong output for several realistic inputs. Most of the following are older bugs next to the change; they're in the body because GitHub only takes inline comments on changed lines. Line numbers refer to the PR head (bd51f9d); "reproduced" means confirmed with a probe against it.

Crashes on valid input

1. Missing default_query_params (firestone/spec/asyncapi.py:99-100). get_binding calls query_params.extend(meta.get("default_query_params")) with no default. The field is optional in resource.yaml; removing it from this PR's own test fixture gives TypeError: 'NoneType' object is not iterable (reproduced). openapi.py uses rsrc.get("default_query_params", []).
2. KeyError: 'items' on scalar sub-schemas (asyncapi.py:335). Any exposed property with a schema: sub-key is assumed to be an embedded array. isbn: {description: isbn, schema: {type: string}} with instance_attrs enabled crashes (reproduced). The same line also registers the raw, un-copied items under the bare property name, overwriting a top-level component with the same name (kind: person plus an addressbook.person property; reproduced).
3. Unreachable missing-key check (asyncapi.py:374). key = schema["key"] runs before the SchemaMissingAttribute check at line 393, so a schema without a key raises a bare KeyError: 'key' (reproduced). The check's message also lacks the f prefix and uses yaml, which is not imported, so moving it earlier would raise NameError.
4. Unescaped version (firestone/schema/asyncapi.jinja2:6). version: '{{ version }}' is not escaped, unlike title and description, which use tojson. -v "1.0'beta" produces YAML that fails to parse (reproduced). Use {{ version | tojson }}.

Silently wrong output

5. Nested channels dropped (asyncapi.py:368). if not channels: channels = {} treats the empty dict passed by add_instance_attrs_level (line 337) as missing, and the caller ignores the return value. With channels: {instance_attrs: true} only and an embedded resource as the first exposed property, output contains only /books/{book_id}/title and none of the nested channels (reproduced). Fix: if channels is None.
6. Query params mutate the input (asyncapi.py:110). get_binding does del param["methods"] on the shared dicts and extends schema["query_params"] in place. After the first (get) binding the method limit is gone, so city (methods: [get]) appears in the /addressbook publish binding (example line 169) and last_name on person publish. The caller's query_params keeps growing (four duplicate limit entries after one generate). openapi.get_params avoids this with copy.deepcopy.
7. Substring parameter matching (asyncapi.py:167). key_name not in baseurl instead of checking for {key_name}. kind: videos with key id gives /videos an id parameter (reproduced). Because nested get_channels appends to the shared keys list (line 375) and never pops, later sibling attributes pick up nested keys too (/books/{book_id}/owner_uuid gets uuid; reproduced). AsyncAPI parsers reject parameters missing from the channel name.
8. Collection params on instance channels (asyncapi.py:287). Instance channels get the collection schema, so /addressbook/{address_key} and /addressbook/{address_key}/person/{uuid} advertise limit/offset/city. OpenAPI only adds default_query_params at the resource level.
9. servers guard checks the wrong variable (asyncapi.jinja2:7). {% if components -%} instead of {% if servers %}; components is always truthy, so a resource without asyncapi.servers renders servers:\n {} (reproduced).

Inline comments cover the stale meta["name"] fallback, dropped descriptions and methods, leaked firestone keywords, component naming, duplicate YAML anchors, the missing make target, and test coverage.

[ebourgeois]

The root cause stays in place. This fixes the call site, but get_channel's fallback rsrc_name = meta["name"] (line 156) is still there, and resource.yaml now forbids a top-level name. Any caller of get_channel, add_resource_level, add_instance_level or get_channels that omits rsrc_name (all default to None), or a resource with kind: "", brings back KeyError: 'name' from #94. Change the fallback to meta["kind"] or make rsrc_name required. get_channels' keys=None default has the same problem: None.append raises AttributeError.

[ebourgeois]

Descriptions and method limits are silently dropped. asyncapi.py (lines 244, 249, 279, 285, 315, 316) still reads schema.descriptions, schema.items.descriptions and schema.methods, but #36 moved these to top-level descriptions.resource/instance and methods.resource/instance/instance_attrs. The regenerated example loses every custom description here: "Create a new address in this addressbook..." becomes "Publish to /addressbook", and "List all addresses..." / "Get a specific address..." become generic "Subscribe from ..." text (line 206 and the attribute channels). methods.instance_attrs: [delete,get,head,put] is ignored too, so every attribute channel still gets a publish operation. This is the same incomplete schema migration as the name to kind bug this PR fixes.

[ebourgeois]

Firestone-only keywords leak into components. The embedded person property keeps the raw schema: wrapper (items, key, query_params), and expose/schema remain on address_key and uuid. Validators and codegen see person as an untyped property with unknown keywords, so payloads are effectively not validated. openapi.py:431-437 already rewrites nested properties to $ref: '#/components/schemas/<prop>' (see examples/addressbook/openapi.yaml:105); asyncapi.py should reuse that.

[ebourgeois]

Component names don't match OpenAPI. asyncapi.py (around line 448) names components, messages and tags with the raw kind instead of rsrc.get("singular") or spec_base.to_singular(rsrc_name) as openapi.generate does. The result is identical person and persons components (lines 96 and 122), and persons is never referenced.

[ebourgeois]

Nothing keeps this file in sync. It was regenerated by hand: the Makefile has gen-openapi but no gen-asyncapi, and no test diffs generator output against this file. It stayed stale from #24 until this PR and will drift again. A make target like gen-openapi, or a golden test, would catch it.

[ebourgeois]

Duplicate YAML anchors. components and channels are dumped by separate yaml_pretty calls, and each restarts PyYAML anchor numbering at &id001. When both sections contain shared objects (e.g. two item properties sharing one dict via YAML anchors in the resource file, with instances or instance_attrs enabled), components emits a: &id001 and channels emits schema: &id001, and yaml.safe_load raises ComposerError: found duplicate anchor 'id001' (reproduced). Root fix is in _base.yaml_pretty: a Dumper whose ignore_aliases returns True, or a single dump of the whole document. That also covers openapi.jinja2.

[ebourgeois]

Happy-path-only coverage. The fixture always sets default_query_params, only enables resources and instances, and has no top-level descriptions or methods. It never exercises instance_attrs, the nested rsrc_name=prop recursion, or a comparison against examples/addressbook/asyncapi.yaml. Every crash and wrong-output bug in this review passes the suite, which is how #94 went unnoticed since #73.

[ebourgeois]

This doesn't check that the document is valid AsyncAPI. It only checks that the YAML parses and lists the top-level keys. The output puts method and query inside operation-level bindings.ws, which the AsyncAPI WebSockets bindings spec says must be empty, so a real AsyncAPI parser rejects or ignores them.