Fixed #682: Restoring the ability to provide an empty "newVersion" ar…

…gument
mojohaus · Sep 12, 2022 · fb89924 · fb89924
1 parent 8178231
commit fb89924
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 84 deletions.
diff --git a/src/main/java/org/codehaus/mojo/versions/SetPropertyMojo.java b/src/main/java/org/codehaus/mojo/versions/SetPropertyMojo.java
@@ -22,12 +22,9 @@
 import javax.xml.stream.XMLStreamException;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 import java.util.Map;
 
-import org.apache.commons.lang3.StringUtils;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugins.annotations.Mojo;
@@ -37,7 +34,9 @@
 import org.codehaus.mojo.versions.rewriting.ModifiedPomXMLEventReader;
 import org.codehaus.mojo.versions.utils.PropertiesVersionsFileReader;
 
-import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.defaultString;
+import static org.apache.commons.lang3.StringUtils.isEmpty;
+import static org.apache.commons.lang3.StringUtils.split;
 
 /**
  * Set a property to a given version without any sanity checks. Please be careful this can lead to changes which might
@@ -83,31 +82,6 @@ public class SetPropertyMojo
     @Parameter( property = "propertiesVersionsFile" )
     private String propertiesVersionsFile;
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void execute() throws MojoExecutionException, MojoFailureException
-    {
-        List<String> problems = new ArrayList<>();
-        if ( isBlank( propertiesVersionsFile ) )
-        {
-            if ( isBlank( newVersion ) )
-            {
-                problems.add( "newVersion must not be empty" );
-            }
-            if ( isBlank( property ) )
-            {
-                problems.add( "property must not be empty" );
-            }
-        }
-        if ( !problems.isEmpty() )
-        {
-            throw new MojoExecutionException( "Invalid execution arguments: " + String.join( ", ", problems ) );
-        }
-        super.execute();
-    }
-
     /**
      * @param pom the pom to update.
      * @throws MojoExecutionException when things go wrong
@@ -118,9 +92,9 @@ public void execute() throws MojoExecutionException, MojoFailureException
     protected void update( ModifiedPomXMLEventReader pom )
         throws MojoExecutionException, MojoFailureException, XMLStreamException
     {
-        Property[] propertiesConfig = null;
-        String properties = "";
-        if ( !StringUtils.isEmpty( propertiesVersionsFile ) )
+        Property[] propertiesConfig;
+        String properties;
+        if ( !isEmpty( propertiesVersionsFile ) )
         {
             logWrongConfigWarning();
             getLog().debug( "Reading properties and versions to update from file: " + propertiesVersionsFile );
@@ -139,10 +113,10 @@ protected void update( ModifiedPomXMLEventReader pom )
             propertiesConfig = reader.getPropertiesConfig();
             properties = reader.getProperties();
         }
-        else if ( !StringUtils.isEmpty( property ) )
+        else if ( !isEmpty( property ) )
         {
             getLog().debug( "Reading properties and versions to update from property and newVersion " );
-            propertiesConfig = Arrays.stream( StringUtils.split( property, "," ) ).map(
+            propertiesConfig = Arrays.stream( split( property, "," ) ).map(
                     prp ->
                     {
                         Property propertyConfig = new Property( prp );
@@ -177,17 +151,18 @@ private void update( ModifiedPomXMLEventReader pom, Property[] propertiesConfig,
             {
                 continue;
             }
-            PomHelper.setPropertyVersion( pom, version.getProfileId(), currentProperty.getName(), newVersionGiven );
+            PomHelper.setPropertyVersion( pom, version.getProfileId(), currentProperty.getName(), 
+                    defaultString( newVersionGiven ) );
         }
     }
 
     private void logWrongConfigWarning()
     {
-        if ( !StringUtils.isEmpty( property ) )
+        if ( !isEmpty( property ) )
         {
             getLog().warn( "-Dproperty provided but will be ignored as -DpropertiesVersionsFile is used" );
         }
-        if ( !StringUtils.isEmpty( newVersion ) )
+        if ( !isEmpty( newVersion ) )
         {
             getLog().warn( "-DnewVersion provided but will be ignored as -DpropertiesVersionsFile is used" );
         }

diff --git a/src/test/java/org/codehaus/mojo/versions/SetPropertyMojoTest.java b/src/test/java/org/codehaus/mojo/versions/SetPropertyMojoTest.java
@@ -19,13 +19,16 @@
  * under the License.
  */
 
+import java.nio.file.Files;
+
 import org.apache.maven.plugin.MojoExecutionException;
 import org.codehaus.mojo.versions.utils.BaseMojoTestCase;
 import org.junit.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.matchesPattern;
 
 /**
  * Basic tests for {@linkplain SetPropertyMojoTest}.
@@ -39,26 +42,49 @@ public void testNullNewVersion()
             throws Exception
     {
         SetPropertyMojo mojo = createMojo( "set-property",
-                "src/test/resources/org/codehaus/mojo/set-property/null-new-version-pom.xml" );
+                "target/test-classes/org/codehaus/mojo/set-property/pom.xml" );
         assertThat( mojo.getProject().getProperties(), is( mojo.getProject().getModel().getProperties() ) );
-        try
-        {
-            mojo.execute();
-            fail();
-        }
-        catch ( MojoExecutionException e )
-        {
-            assertThat( e.getMessage(),
-                    containsString( "Invalid execution arguments: newVersion must not be empty" ) );
-        }
+
+        setVariableValueToObject( mojo, "property", "dummy-api-version" );
+        setVariableValueToObject( mojo, "newVersion", null );
+
+        mojo.execute();
+
+        String output = String.join( "", Files.readAllLines( mojo.getProject().getFile().toPath() ) )
+                .replaceAll( "\\s*", "" );
+        assertThat( output,
+                matchesPattern( ".*<properties>.*<dummy-api-version></dummy-api-version>.*</properties>.*" ) );
+    }
+
+    @Test
+    public void testNewVersionEmpty()
+            throws Exception
+    {
+        SetPropertyMojo mojo = createMojo( "set-property",
+                "target/test-classes/org/codehaus/mojo/set-property/pom.xml" );
+        assertThat( mojo.getProject().getProperties(), is( mojo.getProject().getModel().getProperties() ) );
+
+        setVariableValueToObject( mojo, "property", "dummy-api-version" );
+        setVariableValueToObject( mojo, "newVersion", "" );
+
+        mojo.execute();
+
+        String output = String.join( "", Files.readAllLines( mojo.getProject().getFile().toPath() ) )
+                .replaceAll( "\\s*", "" );
+        assertThat( output,
+                matchesPattern( ".*<properties>.*<dummy-api-version></dummy-api-version>.*</properties>.*" ) );
     }
 
     @Test
     public void testNullProperty()
             throws Exception
     {
         SetPropertyMojo mojo = createMojo( "set-property",
-                "src/test/resources/org/codehaus/mojo/set-property/null-property-pom.xml" );
+                "src/test/resources/org/codehaus/mojo/set-property/pom.xml" );
+
+        setVariableValueToObject( mojo, "property", null );
+        setVariableValueToObject( mojo, "propertiesVersionsFile", null );
+        setVariableValueToObject( mojo, "newVersion", "2.0.0" );
         try
         {
             mojo.execute();
@@ -67,7 +93,7 @@ public void testNullProperty()
         catch ( MojoExecutionException e )
         {
             assertThat( e.getMessage(),
-                    containsString( "Invalid execution arguments: property must not be empty" ) );
+                    containsString( "Please provide either 'property' or 'propertiesVersionsFile' parameter." ) );
         }
     }
 }
diff --git a/src/test/java/org/codehaus/mojo/versions/utils/BaseMojoTestCase.java b/src/test/java/org/codehaus/mojo/versions/utils/BaseMojoTestCase.java
@@ -57,6 +57,7 @@ public abstract class BaseMojoTestCase extends AbstractMojoTestCase
      * Lookup the mojo leveraging the actual subprojects pom
      * and injects the project using the given pom file path.
      *
+     * @param <T>         target Mojo subclass
      * @param goal        to execute on the plugin
      * @param pomFilePath path to the pom project to inject
      * @return a Mojo instance

diff --git a/src/test/resources/org/codehaus/mojo/set-property/null-new-version-pom.xml b/src/test/resources/org/codehaus/mojo/set-property/null-new-version-pom.xml
diff --git a/...s/mojo/set-property/null-property-pom.xml → ...es/org/codehaus/mojo/set-property/pom.xml b/...s/mojo/set-property/null-property-pom.xml → ...es/org/codehaus/mojo/set-property/pom.xml
@@ -22,9 +22,7 @@
         <plugins>
             <plugin>
                 <artifactId>versions-maven-plugin</artifactId>
-                <configuration>
-                    <newVersion>2.0.0</newVersion>
-                </configuration>
+                <configuration/>
             </plugin>
         </plugins>
     </build>