From faab4617792f5b8a1ace4f30a32d02a192e01f25 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Fri, 30 Oct 2026 15:35:31 +0530 Subject: [PATCH] fix(xmlschema): write optional properties with minOccurs="0" XmlSchemaVisitor ignored isOptional(), so every optional field and relationship was written as a plain xs:element. XSD defaults minOccurs to 1, which made optional properties required: an instance leaving out an optional property failed validation. - Write minOccurs="0" for optional fields and relationships, and on the wrapper element of an optional map field with a size validator. - Optional arrays always get minOccurs="0", even when their size validator has a minimum, because an absent array is valid. - Pass the property's optional modifier into visitField for scalar fields. The field returned by getScalarField() does not carry it. - Move the occurrence attribute logic shared by fields and relationships into a toOccurs helper. Closes #304 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- .../fromcto/xmlschema/xmlschemavisitor.js | 69 +++++------ test/codegen/__snapshots__/codegen.js.snap | 18 +-- .../fromcto/xmlschema/xmlschemavisitor.js | 111 ++++++++++++++++++ 3 files changed, 152 insertions(+), 46 deletions(-) diff --git a/lib/codegen/fromcto/xmlschema/xmlschemavisitor.js b/lib/codegen/fromcto/xmlschema/xmlschemavisitor.js index 68f7cb6..df4336a 100644 --- a/lib/codegen/fromcto/xmlschema/xmlschemavisitor.js +++ b/lib/codegen/fromcto/xmlschema/xmlschemavisitor.js @@ -46,7 +46,8 @@ class XmlSchemaVisitor { } else if (thing.isMapDeclaration?.()) { return this.visitMapDeclaration(thing, parameters); } else if (thing.isTypeScalar?.()) { - return this.visitField(thing.getScalarField(), parameters); + // the scalar field does not carry the optional modifier of the property + return this.visitField(thing.getScalarField(), parameters, thing.isOptional()); } else if (thing.isField?.()) { return this.visitField(thing, parameters); } else if (thing.isRelationship?.()) { @@ -230,27 +231,11 @@ class XmlSchemaVisitor { * Visitor design pattern * @param {Field} field - the object being visited * @param {Object} parameters - the parameter + * @param {boolean} [isOptional] - whether the property is optional, defaults to field.isOptional() * @return {Object} the result of visiting or null * @private */ - visitField(field, parameters) { - let array = ''; - - if(field.isArray()) { - let minOccurs = '0'; - let maxOccurs = 'unbounded'; - const sizeValidator = field.getSizeValidator?.(); - if (sizeValidator) { - if (sizeValidator.getMinSize() !== null) { - minOccurs = String(sizeValidator.getMinSize()); - } - if (sizeValidator.getMaxSize() !== null) { - maxOccurs = String(sizeValidator.getMaxSize()); - } - } - array = ` minOccurs="${minOccurs}" maxOccurs="${maxOccurs}"`; - } - + visitField(field, parameters, isOptional = field.isOptional()) { const sizeValidator = field.getSizeValidator?.(); if (!field.isArray() && sizeValidator && ModelUtil.isMap?.(field)) { let minOccurs = '0'; @@ -262,7 +247,7 @@ class XmlSchemaVisitor { maxOccurs = String(sizeValidator.getMaxSize()); } const mapName = field.getType(); - parameters.fileWriter.writeLine(2, ``); + parameters.fileWriter.writeLine(2, ``); parameters.fileWriter.writeLine(3, ''); parameters.fileWriter.writeLine(4, ''); parameters.fileWriter.writeLine(5, ``); @@ -272,7 +257,7 @@ class XmlSchemaVisitor { return null; } - parameters.fileWriter.writeLine(2, ``); + parameters.fileWriter.writeLine(2, ``); return null; } @@ -297,25 +282,35 @@ class XmlSchemaVisitor { * @private */ visitRelationship(relationship, parameters) { - let array = ''; + parameters.fileWriter.writeLine(2, ``); + return null; + } - if(relationship.isArray()) { - let minOccurs = '0'; - let maxOccurs = 'unbounded'; - const sizeValidator = relationship.getSizeValidator?.(); - if (sizeValidator) { - if (sizeValidator.getMinSize() !== null) { - minOccurs = String(sizeValidator.getMinSize()); - } - if (sizeValidator.getMaxSize() !== null) { - maxOccurs = String(sizeValidator.getMaxSize()); - } - } - array = ` minOccurs="${minOccurs}" maxOccurs="${maxOccurs}"`; + /** + * Returns the minOccurs and maxOccurs attributes for the element of a property. + * Optional properties may be omitted, so they always have minOccurs="0". + * @param {Property} property - the field or relationship + * @param {boolean} isOptional - whether the property is optional + * @returns {string} the occurrence attributes, or an empty string to use the XSD defaults + * @private + */ + toOccurs(property, isOptional) { + if (!property.isArray()) { + return isOptional ? ' minOccurs="0"' : ''; } - parameters.fileWriter.writeLine(2, ``); - return null; + let minOccurs = '0'; + let maxOccurs = 'unbounded'; + const sizeValidator = property.getSizeValidator?.(); + if (sizeValidator) { + if (!isOptional && sizeValidator.getMinSize() !== null) { + minOccurs = String(sizeValidator.getMinSize()); + } + if (sizeValidator.getMaxSize() !== null) { + maxOccurs = String(sizeValidator.getMaxSize()); + } + } + return ` minOccurs="${minOccurs}" maxOccurs="${maxOccurs}"`; } /** diff --git a/test/codegen/__snapshots__/codegen.js.snap b/test/codegen/__snapshots__/codegen.js.snap index ab85f8d..4cf4c6d 100644 --- a/test/codegen/__snapshots__/codegen.js.snap +++ b/test/codegen/__snapshots__/codegen.js.snap @@ -7302,7 +7302,7 @@ xmlns:concerto="concerto" - + @@ -7437,11 +7437,11 @@ xmlns:concerto="concerto" - - - - - + + + + + @@ -7492,7 +7492,7 @@ xmlns:concerto="concerto" - + @@ -7514,7 +7514,7 @@ xmlns:concerto="concerto" - + @@ -7525,7 +7525,7 @@ xmlns:concerto="concerto" - + diff --git a/test/codegen/fromcto/xmlschema/xmlschemavisitor.js b/test/codegen/fromcto/xmlschema/xmlschemavisitor.js index eebbd31..2d348db 100644 --- a/test/codegen/fromcto/xmlschema/xmlschemavisitor.js +++ b/test/codegen/fromcto/xmlschema/xmlschemavisitor.js @@ -81,6 +81,20 @@ describe('XmlSchemaVisitor', function () { mockSpecialVisit.calledWith(thing, param).should.be.ok; }); + it('should return visitField for a scalar Field with the optional modifier of the property', () => { + let scalarField = sinon.createStubInstance(Field); + let thing = sinon.createStubInstance(Field); + thing.isTypeScalar.returns(true); + thing.isOptional.returns(true); + thing.getScalarField.returns(scalarField); + let mockSpecialVisit = sinon.stub(xmlSchemaVisitor, 'visitField'); + mockSpecialVisit.returns('Duck'); + + xmlSchemaVisitor.visit(thing, param).should.deep.equal('Duck'); + + mockSpecialVisit.calledWith(scalarField, param, true).should.be.ok; + }); + it('should return visitRelationship for a RelationshipDeclaration', () => { let thing = sinon.createStubInstance(RelationshipDeclaration); thing.isRelationship.returns(true); @@ -769,6 +783,36 @@ describe('XmlSchemaVisitor', function () { param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; }); + it('should write a line for an optional field', () => { + let param = { + fileWriter: mockFileWriter + }; + + let mockField = sinon.createStubInstance(Field); + mockField.isField.returns(true); + mockField.getFullyQualifiedTypeName.returns('String'); + mockField.getName.returns('Bob'); + mockField.isOptional.returns(true); + + xmlSchemaVisitor.visitField(mockField, param); + param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; + }); + + it('should write a line for a field that is optional because of its property', () => { + let param = { + fileWriter: mockFileWriter + }; + + let mockField = sinon.createStubInstance(Field); + mockField.isField.returns(true); + mockField.getFullyQualifiedTypeName.returns('String'); + mockField.getName.returns('Bob'); + mockField.isOptional.returns(false); + + xmlSchemaVisitor.visitField(mockField, param, true); + param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; + }); + it('should write a line for a field thats an array', () => { let param = { fileWriter: mockFileWriter @@ -841,6 +885,26 @@ describe('XmlSchemaVisitor', function () { param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; }); + it('should write a line for an optional array field with size constraints', () => { + let param = { + fileWriter: mockFileWriter + }; + + let mockField = sinon.createStubInstance(Field); + mockField.isField.returns(true); + mockField.getFullyQualifiedTypeName.returns('String'); + mockField.getName.returns('Bob'); + mockField.isArray.returns(true); + mockField.isOptional.returns(true); + mockField.getSizeValidator.returns({ + getMinSize: () => 1, + getMaxSize: () => 10 + }); + + xmlSchemaVisitor.visitField(mockField, param); + param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; + }); + it('should write an inline complexType for a map field with both size constraints', () => { let param = { fileWriter: mockFileWriter @@ -932,6 +996,38 @@ describe('XmlSchemaVisitor', function () { xmlSchemaVisitor.visitField(mockField, param); param.fileWriter.writeLine.withArgs(5, '').calledOnce.should.be.ok; }); + + it('should write an inline complexType for an optional map field with size constraints', () => { + let param = { + fileWriter: mockFileWriter + }; + + let mockMapDecl = sinon.createStubInstance(MapDeclaration); + mockMapDecl.isMapDeclaration.returns(true); + + let mockModelFile = sinon.createStubInstance(ModelFile); + mockModelFile.getType.returns(mockMapDecl); + let mockParent = sinon.createStubInstance(ClassDeclaration); + mockParent.getModelFile.returns(mockModelFile); + + let mockField = sinon.createStubInstance(Field); + mockField.isField.returns(true); + mockField.getFullyQualifiedTypeName.returns('org.acme.Labels'); + mockField.getName.returns('metadata'); + mockField.getType.returns('Labels'); + mockField.isArray.returns(false); + mockField.isOptional.returns(true); + mockField.isPrimitive.returns(false); + mockField.getParent.returns(mockParent); + mockField.getSizeValidator.returns({ + getMinSize: () => 1, + getMaxSize: () => 20 + }); + + xmlSchemaVisitor.visitField(mockField, param); + param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; + param.fileWriter.writeLine.withArgs(5, '').calledOnce.should.be.ok; + }); }); describe('visitEnumValueDeclaration', () => { @@ -966,6 +1062,21 @@ describe('XmlSchemaVisitor', function () { param.fileWriter.writeLine.withArgs(1, '+ string Bob'); }); + it('should write a line for an optional relationship', () => { + let param = { + fileWriter: mockFileWriter + }; + + let mockRelationship = sinon.createStubInstance(RelationshipDeclaration); + mockRelationship.isRelationship.returns(true); + mockRelationship.getFullyQualifiedTypeName.returns('String'); + mockRelationship.getName.returns('Bob'); + mockRelationship.isOptional.returns(true); + + xmlSchemaVisitor.visitRelationship(mockRelationship, param); + param.fileWriter.writeLine.withArgs(2, '').calledOnce.should.be.ok; + }); + it('should write a line for a relationship thats an array', () => { let param = { fileWriter: mockFileWriter