Repository navigation
fix(xmlschema): declare map types so map references resolve - #308
Conversation
Any model with a map declaration produced an XML Schema that did not compile: - the entry element of a map referred to <Map>EntryType without a namespace prefix, which resolves to no namespace because the schema has a targetNamespace and no default namespace - a map was written as an element with an anonymous type, while fields of that map type refer to a type named after the map, which did not exist - class and enum map values were prefixed with the namespace of the map instead of the namespace that declares them, so imported types did not resolve Write a map declaration the same way a concept is written: a named xs:complexType, then an xs:element of that type. Qualify the entry type reference in the declaration and in the inline type written for map fields with a size validator, and resolve class and enum key and value types to their fully qualified name. This also covers the system types (Concept, Asset, ...), which were special-cased before. Drop the now unused mapElement argument of toXsType. Closes accordproject#305 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com>
|
Thanks for your contribution, and for opening #305 with the details alongside this PR. The CI workflows have been approved and are passing. The PR now waits for review from the Concerto maintainers. If you push more changes, the checks will run again, and it helps if the commits are signed off (DCO). Drafted by an Accord Project agent, reviewed by a maintainer. |
There was a problem hiding this comment.
Thank you for this fix, and for the careful tests. Map declarations now emit a named complexType plus a global element (the same pattern concepts and enums use), so fields that refer to a map now resolve. Imported maps and imported concept, enum and scalar values resolve to the namespace that declares them. The updated snapshot and the new test with imported types in test/codegen/fromcto/xmlschema/xmlschemavisitor.js cover this well. Nothing blocks the merge. The only point is a heads-up about an overlapping PR; once a maintainer has reviewed it and the order with that PR is settled, this looks ready.
Nothing blocking; some suggestions below. Findings: 1 nit.
Automated review by Mira, an Accord Project agent, at d97fa38. Findings are labelled blocking, should-fix or nit; a maintainer makes the final call.
| maxOccurs = String(sizeValidator.getMaxSize()); | ||
| } | ||
| const mapName = field.getType(); | ||
| const mapType = this.toXsType(field.getFullyQualifiedTypeName()); |
There was a problem hiding this comment.
nit · hygiene
PR #307 changes the map branch of visitField on the adjacent line (the <xs:element name=...> line that follows the mapType line here), and also edits the same visitField tests in test/codegen/fromcto/xmlschema/xmlschemavisitor.js. Whichever PR merges second will probably need a rebase. It would help to agree an order with #307 and rebase the other one afterwards.
Signed-off-by: Matt Roberts <7544022+mttrbrts@users.noreply.github.com>
Signed-off-by: Matt Roberts <7544022+mttrbrts@users.noreply.github.com>
Closes #305
Generated XML Schema for models with maps now compiles. Before this change, the map references didn't resolve and
xmllintrefused to compile the schema.Changes
xs:complexTypeplus anxs:elementof that type, the same way concepts are written. Fields of a map type already refer tons:MapName, and that type now exists.<Map>EntryTypereference with the map's namespace, both in the map declaration and in the inline type written for map fields with a size validator (which may refer to an imported map).Concept,Asset, ...), andtoXsTypeno longer needs itsmapElementargument.hrsnapshot, and add a test with a real model that uses imported map value types and imported maps.Flags
main,xmllint --schemafails to compileorg.acme.hr.base@1.0.0.xsdandorg.acme.hr@1.0.0.xsd(References from this schema to components in no namespace are not allowed). With this change both compile, and an instance with map entries validates, including an imported concept value and an imported enum value. The XML Schema verification intest/verificationonly checks the files against the W3C schema for schemas, so it didn't catch this.visitField, a few lines from this change. Whichever PR is merged second will need a small rebase.Screenshots or Video
N/A
Related Issues
Author Checklist
--signoffoption of git commit.mainfromfork:branchname