Optionally enforce required elements in XSDs

kwin · kwin · commit c24cf7544a98 · 2022-10-18T11:05:22.000+02:00
This closes codehaus-plexus#162
diff --git a/modello-core/src/main/java/org/codehaus/modello/ModelloParameterConstants.java b/modello-core/src/main/java/org/codehaus/modello/ModelloParameterConstants.java
@@ -71,6 +71,12 @@ public class ModelloParameterConstants
      */
     public static final String EXTENDED_CLASSNAME_SUFFIX = "modello.xpp3.extended.suffix";
 
+    /**
+     * Boolean flag relaxing XSD generation with regards to mandatory elements.
+     * If set to {@code true} will not require mandatory elements in the XML which can be useful if the XML is post processed (e.g. POM merging with parents)
+     * @since 2.1
+     */
+    public static final String XSD_MANDATORY_ELEMENTS_NOT_ENFORCED = "modello.xsd.mandatory.element.not.enforced";
     private ModelloParameterConstants()
     {
     }
diff --git a/modello-maven-plugin/src/main/java/org/codehaus/modello/maven/ModelloXsdMojo.java b/modello-maven-plugin/src/main/java/org/codehaus/modello/maven/ModelloXsdMojo.java
@@ -45,6 +45,15 @@ public class ModelloXsdMojo
     @Parameter( defaultValue = "${project.build.directory}/generated-site/resources/xsd", required = true )
     private File outputDirectory;
 
+    /**
+     * Boolean flag relaxing XSD generation with regards to mandatory elements.
+     * If set to {@code true} will not require mandatory elements in the XML which can be useful if the XML is post processed (e.g. POM merging with parents).
+     * The default value is true for backwards compatibility reasons, but should be set to false for most cases.
+     * @since 2.1.0
+     */
+    @Parameter( defaultValue = "true")
+    private boolean areMandatoryElementsNotEnforced;
+
     /**
      *
      * @since 1.0-alpha-21
@@ -65,6 +74,7 @@ protected void customizeParameters( Properties parameters )
         {
             parameters.put( ModelloParameterConstants.OUTPUT_XSD_FILE_NAME, xsdFileName );
         }
+        parameters.put( ModelloParameterConstants.XSD_MANDATORY_ELEMENTS_NOT_ENFORCED, areMandatoryElementsNotEnforced );
     }
 
     protected boolean producesCompilableResult()
diff --git a/modello-plugins/modello-plugin-xsd/src/main/java/org/codehaus/modello/plugin/xsd/XsdGenerator.java b/modello-plugins/modello-plugin-xsd/src/main/java/org/codehaus/modello/plugin/xsd/XsdGenerator.java
@@ -87,6 +87,7 @@ private void generateXsd( Properties parameters )
 
         // we assume parameters not null
         String xsdFileName = parameters.getProperty( ModelloParameterConstants.OUTPUT_XSD_FILE_NAME );
+        boolean areMandatoryElementsEnforced = !Boolean.valueOf( parameters.getProperty( ModelloParameterConstants.XSD_MANDATORY_ELEMENTS_NOT_ENFORCED ) );
 
         File f = new File( directory, objectModel.getId() + "-" + getGeneratedVersion() + ".xsd" );
 
@@ -137,7 +138,8 @@ private void generateXsd( Properties parameters )
             // Element descriptors
             // Traverse from root so "abstract" models aren't included
             int initialCapacity = objectModel.getClasses( getGeneratedVersion() ).size();
-            writeComplexTypeDescriptor( w, objectModel, root, new HashSet<ModelClass>( initialCapacity ) );
+            writeComplexTypeDescriptor( w, objectModel, root, new HashSet<ModelClass>( initialCapacity ),
+                    areMandatoryElementsEnforced );
 
             w.endElement();
         }
@@ -184,7 +186,7 @@ private static void writeDocumentation( XMLWriter w, String version, String desc
     }
 
     private void writeComplexTypeDescriptor( XMLWriter w, Model objectModel, ModelClass modelClass,
-                                             Set<ModelClass> written )
+                                             Set<ModelClass> written, boolean areMandatoryElementsEnforced )
     {
         written.add( modelClass );
 
@@ -242,9 +244,12 @@ private void writeComplexTypeDescriptor( XMLWriter w, Model objectModel, ModelCl
                 {
                     w.startElement( "xs:element" );
 
-                    // Usually, would only do this if the field is not "required", but due to inheritance, it may be
-                    // present, even if not here, so we need to let it slide
-                    w.addAttribute( "minOccurs", "0" );
+                    if ( !areMandatoryElementsEnforced || !field.isRequired() )
+                    {
+                        // Usually, would only do this if the field is not "required", but due to inheritance, it may be
+                        // present, even if not here, so we need to let it slide
+                        w.addAttribute( "minOccurs", "0" );
+                    }
                 }
 
                 String xsdType = getXsdType( field.getType() );
@@ -428,7 +433,7 @@ else if ( xsdType == null )
         {
             if ( !written.contains( fieldModelClass ) )
             {
-                writeComplexTypeDescriptor( w, objectModel, fieldModelClass, written );
+                writeComplexTypeDescriptor( w, objectModel, fieldModelClass, written, areMandatoryElementsEnforced );
             }
         }
     }
diff --git a/modello-plugins/modello-plugin-xsd/src/test/java/org/codehaus/modello/plugin/xsd/FeaturesXsdGeneratorTest.java b/modello-plugins/modello-plugin-xsd/src/test/java/org/codehaus/modello/plugin/xsd/FeaturesXsdGeneratorTest.java
@@ -80,7 +80,19 @@ public void testXsdGenerator()
         catch ( SAXParseException e )
         {
             // ok, expected exception
-            assertTrue( String.valueOf( e.getMessage() ).contains( "invalidElement" ) );
+            assertTrue( e.getMessage().contains( "invalidElement" ) );
+        }
+
+        try
+        {
+            validator.validate( new StreamSource( getClass().getResourceAsStream( "/features-missing-required.xml" ) ) );
+            fail( "parsing of features-invalid.xml should have failed" );
+        }
+        catch ( SAXParseException e )
+        {
+            // ok, expected exception
+            assertTrue( e.getMessage().contains( "description" ) );
+            assertTrue( e.getMessage().endsWith(" is expected."));
         }
 
         try
@@ -91,7 +103,7 @@ public void testXsdGenerator()
         catch ( SAXParseException e )
         {
             // ok, expected exception
-            assertTrue( String.valueOf( e.getMessage() ).contains( "transientString" ) );
+            assertTrue( e.getMessage().contains( "transientString" ) );
         }
     }
 }
diff --git a/modello-plugins/modello-plugin-xsd/src/test/java/org/codehaus/modello/plugin/xsd/ModelloXsdGeneratorTest.java b/modello-plugins/modello-plugin-xsd/src/test/java/org/codehaus/modello/plugin/xsd/ModelloXsdGeneratorTest.java
@@ -23,6 +23,7 @@
  */
 
 import org.codehaus.modello.AbstractModelloGeneratorTest;
+import org.codehaus.modello.ModelloParameterConstants;
 import org.codehaus.modello.core.ModelloCore;
 import org.codehaus.modello.model.Model;
 import org.xml.sax.SAXException;
@@ -55,6 +56,7 @@ public void testXsdGenerator()
         ModelloCore modello = (ModelloCore) lookup( ModelloCore.ROLE );
 
         Properties parameters = getModelloParameters( "1.4.0" );
+        parameters.setProperty( ModelloParameterConstants.XSD_MANDATORY_ELEMENTS_NOT_ENFORCED, "true" );
 
         Model model = modello.loadModel( getTestFile( "../../src/main/mdo/modello.mdo" ) );
 
diff --git a/modello-plugins/modello-plugin-xsd/src/test/resources/features-missing-required.xml b/modello-plugins/modello-plugin-xsd/src/test/resources/features-missing-required.xml
@@ -0,0 +1,5 @@
+<?xml version="1.0"?>
+
+<features-demo xmlns="http://codehaus-plexus.github.io/FEATURES/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+   xsi:schemaLocation="http://codehaus-plexus.github.io/FEATURES/1.0.0 http://codehaus-plexus.github.io/features-1.0.0.xsd">
+</features-demo>
diff --git a/modello-test/src/main/resources/features.xml b/modello-test/src/main/resources/features.xml
@@ -3,7 +3,7 @@
 <features-demo xmlns="http://codehaus-plexus.github.io/FEATURES/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://codehaus-plexus.github.io/FEATURES/1.0.0 http://codehaus-plexus.github.io/features-1.0.0.xsd">
   <versionField>1.0.0</versionField>
-  <!--required></required--><!-- TODO: this field is marked required but not set in the XML, should fail... -->
+  <required></required>
   <identifier>id</identifier>
   <identifierPart2><id>reference</id></identifierPart2>
 

Original file line number	Diff line number	Diff line change
`@@ -71,6 +71,12 @@ public class ModelloParameterConstants`
`71`	`71`	`*/`
`72`	`72`	`public static final String EXTENDED_CLASSNAME_SUFFIX = "modello.xpp3.extended.suffix";`
`73`	`73`
	`74`	`+ /**`
	`75`	`+ * Boolean flag relaxing XSD generation with regards to mandatory elements.`
	`76`	`+ * If set to {@code true} will not require mandatory elements in the XML which can be useful if the XML is post processed (e.g. POM merging with parents)`
	`77`	`+ * @since 2.1`
	`78`	`+ */`
	`79`	`+ public static final String XSD_MANDATORY_ELEMENTS_NOT_ENFORCED = "modello.xsd.mandatory.element.not.enforced";`
`74`	`80`	`private ModelloParameterConstants()`
`75`	`81`	`{`
`76`	`82`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,15 @@ public class ModelloXsdMojo`
`45`	`45`	`@Parameter( defaultValue = "${project.build.directory}/generated-site/resources/xsd", required = true )`
`46`	`46`	`private File outputDirectory;`
`47`	`47`
	`48`	`+ /**`
	`49`	`+ * Boolean flag relaxing XSD generation with regards to mandatory elements.`
	`50`	`+ * If set to {@code true} will not require mandatory elements in the XML which can be useful if the XML is post processed (e.g. POM merging with parents).`
	`51`	`+ * The default value is true for backwards compatibility reasons, but should be set to false for most cases.`
	`52`	`+ * @since 2.1.0`
	`53`	`+ */`
	`54`	`+ @Parameter( defaultValue = "true")`
	`55`	`+ private boolean areMandatoryElementsNotEnforced;`
	`56`	`+`
`48`	`57`	`/**`
`49`	`58`	`*`
`50`	`59`	`* @since 1.0-alpha-21`
`@@ -65,6 +74,7 @@ protected void customizeParameters( Properties parameters )`
`65`	`74`	`{`
`66`	`75`	`parameters.put( ModelloParameterConstants.OUTPUT_XSD_FILE_NAME, xsdFileName );`
`67`	`76`	`}`
	`77`	`+ parameters.put( ModelloParameterConstants.XSD_MANDATORY_ELEMENTS_NOT_ENFORCED, areMandatoryElementsNotEnforced );`
`68`	`78`	`}`
`69`	`79`
`70`	`80`	`protected boolean producesCompilableResult()`
Original file line number	Diff line number	Diff line change
`@@ -87,6 +87,7 @@ private void generateXsd( Properties parameters )`
`87`	`87`
`88`	`88`	`// we assume parameters not null`
`89`	`89`	`String xsdFileName = parameters.getProperty( ModelloParameterConstants.OUTPUT_XSD_FILE_NAME );`
	`90`	`+ boolean areMandatoryElementsEnforced = !Boolean.valueOf( parameters.getProperty( ModelloParameterConstants.XSD_MANDATORY_ELEMENTS_NOT_ENFORCED ) );`
`90`	`91`
`91`	`92`	`File f = new File( directory, objectModel.getId() + "-" + getGeneratedVersion() + ".xsd" );`
`92`	`93`
`@@ -137,7 +138,8 @@ private void generateXsd( Properties parameters )`
`137`	`138`	`// Element descriptors`
`138`	`139`	`// Traverse from root so "abstract" models aren't included`
`139`	`140`	`int initialCapacity = objectModel.getClasses( getGeneratedVersion() ).size();`
`140`		`- writeComplexTypeDescriptor( w, objectModel, root, new HashSet<ModelClass>( initialCapacity ) );`
	`141`	`+ writeComplexTypeDescriptor( w, objectModel, root, new HashSet<ModelClass>( initialCapacity ),`
	`142`	`+ areMandatoryElementsEnforced );`
`141`	`143`
`142`	`144`	`w.endElement();`
`143`	`145`	`}`
`@@ -184,7 +186,7 @@ private static void writeDocumentation( XMLWriter w, String version, String desc`
`184`	`186`	`}`
`185`	`187`
`186`	`188`	`private void writeComplexTypeDescriptor( XMLWriter w, Model objectModel, ModelClass modelClass,`
`187`		`- Set<ModelClass> written )`
	`189`	`+ Set<ModelClass> written, boolean areMandatoryElementsEnforced )`
`188`	`190`	`{`
`189`	`191`	`written.add( modelClass );`
`190`	`192`
`@@ -242,9 +244,12 @@ private void writeComplexTypeDescriptor( XMLWriter w, Model objectModel, ModelCl`
`242`	`244`	`{`
`243`	`245`	`w.startElement( "xs:element" );`
`244`	`246`
`245`		`- // Usually, would only do this if the field is not "required", but due to inheritance, it may be`
`246`		`- // present, even if not here, so we need to let it slide`
`247`		`- w.addAttribute( "minOccurs", "0" );`
	`247`	`+ if ( !areMandatoryElementsEnforced \|\| !field.isRequired() )`
	`248`	`+ {`
	`249`	`+ // Usually, would only do this if the field is not "required", but due to inheritance, it may be`
	`250`	`+ // present, even if not here, so we need to let it slide`
	`251`	`+ w.addAttribute( "minOccurs", "0" );`
	`252`	`+ }`
`248`	`253`	`}`
`249`	`254`
`250`	`255`	`String xsdType = getXsdType( field.getType() );`
`@@ -428,7 +433,7 @@ else if ( xsdType == null )`
`428`	`433`	`{`
`429`	`434`	`if ( !written.contains( fieldModelClass ) )`
`430`	`435`	`{`
`431`		`- writeComplexTypeDescriptor( w, objectModel, fieldModelClass, written );`
	`436`	`+ writeComplexTypeDescriptor( w, objectModel, fieldModelClass, written, areMandatoryElementsEnforced );`
`432`	`437`	`}`
`433`	`438`	`}`
`434`	`439`	`}`
Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,19 @@ public void testXsdGenerator()`
`80`	`80`	`catch ( SAXParseException e )`
`81`	`81`	`{`
`82`	`82`	`// ok, expected exception`
`83`		`- assertTrue( String.valueOf( e.getMessage() ).contains( "invalidElement" ) );`
	`83`	`+ assertTrue( e.getMessage().contains( "invalidElement" ) );`
	`84`	`+ }`
	`85`	`+`
	`86`	`+ try`
	`87`	`+ {`
	`88`	`+ validator.validate( new StreamSource( getClass().getResourceAsStream( "/features-missing-required.xml" ) ) );`
	`89`	`+ fail( "parsing of features-invalid.xml should have failed" );`
	`90`	`+ }`
	`91`	`+ catch ( SAXParseException e )`
	`92`	`+ {`
	`93`	`+ // ok, expected exception`
	`94`	`+ assertTrue( e.getMessage().contains( "description" ) );`
	`95`	`+ assertTrue( e.getMessage().endsWith(" is expected."));`
`84`	`96`	`}`
`85`	`97`
`86`	`98`	`try`
`@@ -91,7 +103,7 @@ public void testXsdGenerator()`
`91`	`103`	`catch ( SAXParseException e )`
`92`	`104`	`{`
`93`	`105`	`// ok, expected exception`
`94`		`- assertTrue( String.valueOf( e.getMessage() ).contains( "transientString" ) );`
	`106`	`+ assertTrue( e.getMessage().contains( "transientString" ) );`
`95`	`107`	`}`
`96`	`108`	`}`
`97`	`109`	`}`