Skip to content

Commit 47bc057

Browse files
Copilotslachiewiczslawekjaranowski
authored
Fix XSD generator to respect required field attribute (#528)
* Fix required attribute not being respected in XSD generation - Modified XsdGenerator to respect field.isRequired() directly instead of depending on enforceMandatoryElements parameter - Removed required=true from type field in modello.mdo since type is optional when association is present - Required fields now correctly omit minOccurs="0" in generated XSD, making them mandatory * Add comment explaining backward compatibility for enforceMandatoryElements parameter - Created RequiredFieldXsdGeneratorTest to verify required fields are enforced by default - Added required-field.mdo test model with both required and optional fields - Test validates that XML missing required fields fails validation - Test validates that XML with required fields passes validation Co-authored-by: slawekjaranowski <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: slachiewicz <[email protected]> Co-authored-by: slawekjaranowski <[email protected]>
1 parent f570710 commit 47bc057

File tree

4 files changed

+152
-4
lines changed

4 files changed

+152
-4
lines changed

‎modello-plugins/modello-plugin-xsd/src/main/java/org/codehaus/modello/plugin/xsd/XsdGenerator.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ private void generateXsd(Map<String, Object> parameters) throws IOException, Mod
8787

8888
// we assume parameters not null
8989
String xsdFileName = (String) parameters.get(ModelloParameterConstants.OUTPUT_XSD_FILE_NAME);
90+
// enforceMandatoryElements parameter is kept for backward compatibility but is no longer used.
91+
// Required fields are now always enforced based on field.isRequired() value.
9092
boolean enforceMandatoryElements =
9193
Boolean.parseBoolean((String) parameters.get(ModelloParameterConstants.XSD_ENFORCE_MANDATORY_ELEMENTS));
9294

@@ -232,9 +234,7 @@ private void writeComplexTypeDescriptor(
232234
}
233235
w.startElement("xs:element");
234236

235-
if (!enforceMandatoryElements || !field.isRequired()) {
236-
// Usually, would only do this if the field is not "required", but due to inheritance, it may be
237-
// present, even if not here, so we need to let it slide
237+
if (!field.isRequired()) {
238238
w.addAttribute("minOccurs", "0");
239239
}
240240
}

‎modello-plugins/modello-plugin-xsd/src/test/java/org/codehaus/modello/plugin/xsd/RequiredFieldXsdGeneratorTest.java‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package org.codehaus.modello.plugin.xsd;
2+
3+
/*
4+
* Copyright (c) 2004, Codehaus.org
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy of
7+
* this software and associated documentation files (the "Software"), to deal in
8+
* the Software without restriction, including without limitation the rights to
9+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
10+
* of the Software, and to permit persons to whom the Software is furnished to do
11+
* so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in all
14+
* copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
22+
* SOFTWARE.
23+
*/
24+
25+
import javax.inject.Inject;
26+
import javax.xml.XMLConstants;
27+
import javax.xml.transform.stream.StreamSource;
28+
import javax.xml.validation.Schema;
29+
import javax.xml.validation.SchemaFactory;
30+
import javax.xml.validation.Validator;
31+
32+
import java.io.File;
33+
import java.io.StringReader;
34+
import java.util.Map;
35+
36+
import org.codehaus.modello.AbstractModelloGeneratorTest;
37+
import org.codehaus.modello.core.ModelloCore;
38+
import org.codehaus.modello.model.Model;
39+
import org.codehaus.plexus.testing.PlexusTest;
40+
import org.junit.jupiter.api.Test;
41+
import org.xml.sax.SAXParseException;
42+
43+
import static org.junit.jupiter.api.Assertions.assertTrue;
44+
import static org.junit.jupiter.api.Assertions.fail;
45+
46+
/**
47+
* Test that verifies required fields are properly enforced in generated XSD schemas
48+
* without needing the XSD_ENFORCE_MANDATORY_ELEMENTS parameter.
49+
*
50+
* @author Modello Team
51+
*/
52+
@PlexusTest
53+
public class RequiredFieldXsdGeneratorTest extends AbstractModelloGeneratorTest {
54+
@Inject
55+
private ModelloCore modello;
56+
57+
public RequiredFieldXsdGeneratorTest() {
58+
super("required-field");
59+
}
60+
61+
@Test
62+
public void testRequiredFieldsEnforcedWithoutParameter() throws Throwable {
63+
Model model = modello.loadModel(getXmlResourceReader("/required-field.mdo"));
64+
65+
// Generate XSD WITHOUT the XSD_ENFORCE_MANDATORY_ELEMENTS parameter
66+
Map<String, Object> parameters = getModelloParameters("1.0.0");
67+
// Note: NOT setting XSD_ENFORCE_MANDATORY_ELEMENTS parameter
68+
69+
modello.generate(model, "xsd", parameters);
70+
71+
SchemaFactory sf = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
72+
Schema schema = sf.newSchema(new StreamSource(new File(getOutputDirectory(), "test-1.0.0.xsd")));
73+
Validator validator = schema.newValidator();
74+
75+
// Test 1: Valid XML with all required fields should pass
76+
String validXml = "<?xml version=\"1.0\"?>\n"
77+
+ "<testModel xmlns=\"http://codehaus-plexus.github.io/TEST/1.0.0\">\n"
78+
+ " <requiredField>value</requiredField>\n"
79+
+ "</testModel>";
80+
81+
validator.validate(new StreamSource(new StringReader(validXml)));
82+
83+
// Test 2: XML missing required field should fail
84+
String missingRequiredXml = "<?xml version=\"1.0\"?>\n"
85+
+ "<testModel xmlns=\"http://codehaus-plexus.github.io/TEST/1.0.0\">\n"
86+
+ " <optionalField>value</optionalField>\n"
87+
+ "</testModel>";
88+
89+
try {
90+
validator.validate(new StreamSource(new StringReader(missingRequiredXml)));
91+
fail("Validation should have failed for XML missing required field");
92+
} catch (SAXParseException e) {
93+
// Expected - the error should mention the required field
94+
assertTrue(
95+
e.getMessage().contains("requiredField") || e.getMessage().contains("expected"),
96+
"Error message should indicate missing required field: " + e.getMessage());
97+
}
98+
99+
// Test 3: XML with optional field omitted should pass
100+
String withoutOptionalXml = "<?xml version=\"1.0\"?>\n"
101+
+ "<testModel xmlns=\"http://codehaus-plexus.github.io/TEST/1.0.0\">\n"
102+
+ " <requiredField>value</requiredField>\n"
103+
+ "</testModel>";
104+
105+
validator.validate(new StreamSource(new StringReader(withoutOptionalXml)));
106+
}
107+
}

‎modello-plugins/modello-plugin-xsd/src/test/resources/required-field.mdo‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<model xmlns="http://codehaus-plexus.github.io/MODELLO/2.0.0"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="http://codehaus-plexus.github.io/MODELLO/2.0.0 https://codehaus-plexus.github.io/modello/xsd/modello-2.0.0.xsd"
5+
xml.namespace="http://codehaus-plexus.github.io/TEST/${version}"
6+
xml.schemaLocation="http://codehaus-plexus.github.io/TEST/${version} http://codehaus-plexus.github.io/test-${version}.xsd">
7+
<id>test</id>
8+
<name>Test</name>
9+
<description>
10+
Simple test model for required field validation.
11+
</description>
12+
<defaults>
13+
<default>
14+
<key>package</key>
15+
<value>org.codehaus.modello.test</value>
16+
</default>
17+
</defaults>
18+
<classes>
19+
<class rootElement="true">
20+
<name>TestModel</name>
21+
<version>1.0.0+</version>
22+
<description>
23+
A simple model to test required field enforcement.
24+
</description>
25+
<fields>
26+
<field>
27+
<name>requiredField</name>
28+
<version>1.0.0+</version>
29+
<required>true</required>
30+
<type>String</type>
31+
<description>This field is required and must be present.</description>
32+
</field>
33+
<field>
34+
<name>optionalField</name>
35+
<version>1.0.0+</version>
36+
<type>String</type>
37+
<description>This field is optional and may be omitted.</description>
38+
</field>
39+
</fields>
40+
</class>
41+
</classes>
42+
</model>

‎src/main/mdo/modello.mdo‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,6 @@
672672
<field xml.insertParentFieldsUpTo="versionRange">
673673
<name>type</name>
674674
<version>1.0.0+</version>
675-
<required>true</required>
676675
<type>String</type>
677676
<description><![CDATA[
678677
Simple type for this field (or array of such type). Can be one of : <code>boolean</code>, <code>byte</code>,

0 commit comments

Comments
 (0)