Repository navigation
Version: 1.1.1 fix(compiler): preserve wrapping sequence bounds - #145
Conversation
collectParticles and collectWildcards in src/compiler/schemaCompiler.ts recursed into nested xs:sequence/xs:all/xs:choice compositor nodes without ever reading the compositor's own minOccurs/maxOccurs, so bounds declared on a wrapping compositor (e.g. <xs:sequence maxOccurs="2">) were silently dropped and the contained element/xs:any particles kept only their own occurrence, producing a singular type instead of an array. Thread the inherited compositor occurrence through both recursions and combine it multiplicatively with each particle's own bounds (matching XSD group-multiplicity semantics), via two small helpers (combineOccurrence, applyInheritedOccurrence) mirroring a fix already validated in production against a local dist/ patch. Add test/unit/compiler-compositor-occurs.test.ts covering the issue's exact example, min/max multiplication, unbounded propagation, and xs:any wildcards. Update test/unit/client-choice-union-types.test.ts, whose all-optional-mode assertions were unknowingly encoding this same bug's output; the fix now produces the optional-field shape already documented in docs/concepts.md for xs:choice with its own minOccurs="0". Verified against a real-world production WSDL: the freshly generated catalog.json is structurally identical (all types, all min/max) to output previously validated via the user's own dist/ patch. Closes TechSpokes#141
|
@BCsabaEngine thanks for the PR, I'll keep it here for now as the agents after inspection found deeper issues in logic I had so far, so they'll try to see if we could reuse your code and the new approach combined to fix the whole thing at once. I'll keep you updated. |
|
I am already very grateful for your work. |
This is my playground ;), easy way to learn typescript .... or not. |
|
@BCsabaEngine thanks again for this. I reviewed the production WSDL privately, and your diagnosis is correct: this PR fixes the real problem they are hitting. The good news is that the affected structures in that WSDL are simpler than the general XSD cases we uncovered. The relevant cases are sequences wrapping a single repeated element, for example: <xs:sequence maxOccurs="2">
<xs:element name="item" type="tns:ItemType"/>
</xs:sequence>which should generate: item: ItemType[];The same pattern is used with Before shipping it, I’d like to make the patch a little narrower so we fix the production case without partially changing the broader compositor model:
The traversal can stay close to what you already have. Conceptually, something like this: type Occurrence = {
min: number;
max: number | "unbounded";
};
const ONE: Occurrence = {min: 1, max: 1};
const recurse = (
groupNode: any,
inherited: Occurrence = ONE,
inheritSequenceOccurs = true,
) => {
for (const e of getChildrenWithLocalName(groupNode, "element")) {
const particle = compileElementParticle(ownerTypeName, e);
if (!particle) continue;
if (particle.max === 0 || inherited.max === 0) {
// Leave disabled-particle behavior unchanged in this hotfix.
out.push(particle);
} else {
out.push(applyInheritedOccurrence(particle, inherited));
}
}
for (const sub of getChildrenWithLocalName(groupNode, "sequence")) {
const own = readOccurrence(sub);
if (inheritSequenceOccurs && own.max !== 0) {
recurse(sub, combineOccurrence(inherited, own), true);
} else {
recurse(sub, ONE, false);
}
}
// Keep traversing existing structures, but do not carry the new
// sequence occurrence semantics through them in this patch.
for (const comp of ["choice", "all"]) {
for (const sub of getChildrenWithLocalName(groupNode, comp)) {
recurse(sub, ONE, false);
}
}
};That is only a sketch; please keep whatever implementation is cleaner in the current compiler. For the regression tests, these are the kinds of expectations I’d like to see: <xs:sequence minOccurs="0" maxOccurs="unbounded">
<xs:element name="item" type="tns:ItemType"/>
</xs:sequence>item?: ItemType[];and: <xs:sequence minOccurs="0" maxOccurs="5">
<xs:element name="item" type="tns:ItemType"/>
</xs:sequence>item?: ItemType[];
// catalog occurrence: {min: 0, max: 5}I don’t think you need to take on anything from the larger refactor. I’ll handle rebasing/integration, the private check against the full WSDL, and the release work. Your PR found the real issue and gives us the fix we need. I just want to keep this release focused so we can get the production case fixed without introducing half of the larger compositor redesign at the same time. Thanks again for the work on this. |
|
My aim is simply to offer pointers and ideas to help improve your codebase. The choice of solution is up to you, and the resulting code is your property and responsibility. Please let me know if I can assist with testing or validation. |
|
Close the PR when you think it's ready. The npm package is what really matters to me. :) |
Integrate current main while narrowing the original compositor occurrence contribution to uninterrupted sequence ancestry. Preserve choice, all, wildcard, and disabled-particle behavior. Add generated consumer and installed-package regression coverage.
Problem and result
Wrapping
xs:sequencebounds were ignored, somaxOccurs="2"produced a scalar child andminOccurs="0"could produce a required property. This patch carries occurrence bounds through uninterrupted sequence ancestry, producing the expected arrays and optional fields.The implementation retains the previous behavior across
xs:choice,xs:all, disabled sequence subtrees, disabled elements, and wildcards. It does not add sequence-order validation or runtime finite-length enforcement. Corrected metadata can activate existing OpenAPI array-wrapper flattening; regenerated consumers should review their types and REST schemas.Validation
Based on the original diagnosis and contribution by @BCsabaEngine, with maintainer integration and narrowed compatibility boundaries.
Closes #141.