Skip to content

fix(xmlschema): write optional properties with minOccurs="0" #307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mttrbrts merged 1 commit into from
Nov 11, 2026
Merged

fix(xmlschema): write optional properties with minOccurs="0" #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 32 additions & 37 deletions lib/codegen/fromcto/xmlschema/xmlschemavisitor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?.()) {
Expand Down Expand Up @@ -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';
Expand All @@ -262,7 +247,7 @@ class XmlSchemaVisitor {
maxOccurs = String(sizeValidator.getMaxSize());
}
const mapName = field.getType();
parameters.fileWriter.writeLine(2, `<xs:element name="${this.toXsName(field.getName())}">`);
parameters.fileWriter.writeLine(2, `<xs:element name="${this.toXsName(field.getName())}"${this.toOccurs(field, isOptional)}>`);

@mira-accord-project mira-accord-project Bot Nov 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit · hygiene

Pull request #308 also edits the map branch of visitField: it changes the const mapName line directly above this <xs:element name=...> line, and edits the same visitField map tests. Whichever of the two merges second will probably hit a merge conflict. There is nothing to change in this PR's logic. It would help to agree an order with #308 and rebase the other one afterwards, re-running npm run test:updateSnapshots if the snapshot needs regenerating.

parameters.fileWriter.writeLine(3, '<xs:complexType>');
parameters.fileWriter.writeLine(4, '<xs:sequence>');
parameters.fileWriter.writeLine(5, `<xs:element name="entry" type="${mapName}EntryType" minOccurs="${minOccurs}" maxOccurs="${maxOccurs}"/>`);
Expand All @@ -272,7 +257,7 @@ class XmlSchemaVisitor {
return null;
}

parameters.fileWriter.writeLine(2, `<xs:element name="${this.toXsName(field.getName())}" type="${this.toXsType(field.getFullyQualifiedTypeName())}"${array}/>`);
parameters.fileWriter.writeLine(2, `<xs:element name="${this.toXsName(field.getName())}" type="${this.toXsType(field.getFullyQualifiedTypeName())}"${this.toOccurs(field, isOptional)}/>`);

return null;
}
Expand All @@ -297,25 +282,35 @@ class XmlSchemaVisitor {
* @private
*/
visitRelationship(relationship, parameters) {
let array = '';
parameters.fileWriter.writeLine(2, `<xs:element name="${this.toXsName(relationship.getName())}" type="${this.toXsType(relationship.getFullyQualifiedTypeName())}"${this.toOccurs(relationship, relationship.isOptional())}/>`);
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, `<xs:element name="${this.toXsName(relationship.getName())}" type="${this.toXsType(relationship.getFullyQualifiedTypeName())}"${array}/>`);
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}"`;
}

/**
Expand Down
18 changes: 9 additions & 9 deletions test/codegen/__snapshots__/codegen.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7302,7 +7302,7 @@ xmlns:concerto="concerto"
<xs:sequence>
<xs:element name="street" type="xs:string"/>
<xs:element name="city" type="xs:string"/>
<xs:element name="state" type="org.acme.hr.base:State"/>
<xs:element name="state" type="org.acme.hr.base:State" minOccurs="0"/>
<xs:element name="zipCode" type="xs:string"/>
<xs:element name="country" type="xs:string"/>
</xs:sequence>
Expand Down Expand Up @@ -7437,11 +7437,11 @@ xmlns:concerto="concerto"
<xs:sequence>
<xs:element name="name" type="xs:string"/>
<xs:element name="headquarters" type="org.acme.hr.base:Address"/>
<xs:element name="companyProperties" type="org.acme.hr:CompanyProperties"/>
<xs:element name="employeeDirectory" type="org.acme.hr:EmployeeDirectory"/>
<xs:element name="employeeTShirtSizes" type="org.acme.hr.base:EmployeeTShirtSizes"/>
<xs:element name="employeeProfiles" type="org.acme.hr:EmployeeProfiles"/>
<xs:element name="employeeSocialSecurityNumbers" type="org.acme.hr:EmployeeSocialSecurityNumbers"/>
<xs:element name="companyProperties" type="org.acme.hr:CompanyProperties" minOccurs="0"/>
<xs:element name="employeeDirectory" type="org.acme.hr:EmployeeDirectory" minOccurs="0"/>
<xs:element name="employeeTShirtSizes" type="org.acme.hr.base:EmployeeTShirtSizes" minOccurs="0"/>
<xs:element name="employeeProfiles" type="org.acme.hr:EmployeeProfiles" minOccurs="0"/>
<xs:element name="employeeSocialSecurityNumbers" type="org.acme.hr:EmployeeSocialSecurityNumbers" minOccurs="0"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
Expand Down Expand Up @@ -7492,7 +7492,7 @@ xmlns:concerto="concerto"
<xs:element name="email" type="xs:string"/>
<xs:element name="firstName" type="xs:string"/>
<xs:element name="lastName" type="xs:string"/>
<xs:element name="middleNames" type="xs:string"/>
<xs:element name="middleNames" type="xs:string" minOccurs="0"/>
<xs:element name="homeAddress" type="org.acme.hr.base:Address"/>
<xs:element name="ssn" type="xs:string"/>
<xs:element name="height" type="xs:double"/>
Expand All @@ -7514,7 +7514,7 @@ xmlns:concerto="concerto"
<xs:element name="department" type="org.acme.hr:Department"/>
<xs:element name="officeAddress" type="org.acme.hr.base:Address"/>
<xs:element name="companyAssets" type="org.acme.hr:Equipment" minOccurs="0" maxOccurs="unbounded"/>
<xs:element name="manager" type="org.acme.hr:Manager"/>
<xs:element name="manager" type="org.acme.hr:Manager" minOccurs="0"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
Expand All @@ -7525,7 +7525,7 @@ xmlns:concerto="concerto"
<xs:extension base="org.acme.hr:Person">
<xs:sequence>
<xs:element name="company" type="org.acme.hr:Company"/>
<xs:element name="manager" type="org.acme.hr:Manager"/>
<xs:element name="manager" type="org.acme.hr:Manager" minOccurs="0"/>
</xs:sequence>
</xs:extension>
</xs:complexContent>
Expand Down
111 changes: 111 additions & 0 deletions test/codegen/fromcto/xmlschema/xmlschemavisitor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -769,6 +783,36 @@ describe('XmlSchemaVisitor', function () {
param.fileWriter.writeLine.withArgs(2, '<xs:element name="Bob" type="org.acme:Foo"/>').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, '<xs:element name="Bob" type="xs:string" minOccurs="0"/>').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, '<xs:element name="Bob" type="xs:string" minOccurs="0"/>').calledOnce.should.be.ok;
});

it('should write a line for a field thats an array', () => {
let param = {
fileWriter: mockFileWriter
Expand Down Expand Up @@ -841,6 +885,26 @@ describe('XmlSchemaVisitor', function () {
param.fileWriter.writeLine.withArgs(2, '<xs:element name="Bob" type="xs:string" minOccurs="0" maxOccurs="5"/>').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, '<xs:element name="Bob" type="xs:string" minOccurs="0" maxOccurs="10"/>').calledOnce.should.be.ok;
});

it('should write an inline complexType for a map field with both size constraints', () => {
let param = {
fileWriter: mockFileWriter
Expand Down Expand Up @@ -932,6 +996,38 @@ describe('XmlSchemaVisitor', function () {
xmlSchemaVisitor.visitField(mockField, param);
param.fileWriter.writeLine.withArgs(5, '<xs:element name="entry" type="LabelsEntryType" minOccurs="0" maxOccurs="10"/>').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, '<xs:element name="metadata" minOccurs="0">').calledOnce.should.be.ok;
param.fileWriter.writeLine.withArgs(5, '<xs:element name="entry" type="LabelsEntryType" minOccurs="1" maxOccurs="20"/>').calledOnce.should.be.ok;
});
});

describe('visitEnumValueDeclaration', () => {
Expand Down Expand Up @@ -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, '<xs:element name="Bob" type="xs:string" minOccurs="0"/>').calledOnce.should.be.ok;
});

it('should write a line for a relationship thats an array', () => {
let param = {
fileWriter: mockFileWriter
Expand Down
Loading