[MNG-6776] Inconsistent list of parameters for MojoDescriptor

maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java

@@ -19,10 +19,11 @@
  * under the License.
  */
 
-import java.util.HashMap;
-import java.util.LinkedList;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.maven.plugin.Mojo;
 import org.codehaus.plexus.component.repository.ComponentDescriptor;
@@ -55,9 +56,7 @@ public class MojoDescriptor
 
     private static final String DEFAULT_LANGUAGE = "java";
 
-    private List<Parameter> parameters;
-
-    private Map<String, Parameter> parameterMap;
+    private final ArrayList<Parameter> parameters;
 
     /** By default, the execution strategy is "once-per-session" */
     private String executionStrategy = SINGLE_PASS_EXEC_STRATEGY;
@@ -145,6 +144,7 @@ public class MojoDescriptor
      */
     public MojoDescriptor()
     {
+        this.parameters = new ArrayList<>();
         setInstantiationStrategy( DEFAULT_INSTANTIATION_STRATEGY );
         setComponentFactory( DEFAULT_LANGUAGE );
     }
@@ -186,11 +186,12 @@ public void setDeprecated( String deprecated )
     }
 
     /**
-     * @return the list of parameters
+     * @return the list of parameters copy. Any change to returned list is NOT reflected on this instance. To add
+     * parameters, use {@link #addParameter(Parameter)} method.
      */
     public List<Parameter> getParameters()
     {
-        return parameters;
+        return new ArrayList<>( parameters  );
     }
 
     /**
@@ -200,6 +201,7 @@ public List<Parameter> getParameters()
     public void setParameters( List<Parameter> parameters )
         throws DuplicateParameterException
     {
+        this.parameters.clear();
         for ( Parameter parameter : parameters )
         {
             addParameter( parameter );
@@ -213,37 +215,28 @@ public void setParameters( List<Parameter> parameters )
     public void addParameter( Parameter parameter )
         throws DuplicateParameterException
     {
-        if ( parameters != null && parameters.contains( parameter ) )
+        if ( parameters.contains( parameter ) )
         {
             throw new DuplicateParameterException( parameter.getName()
                 + " has been declared multiple times in mojo with goal: " + getGoal() + " (implementation: "
                 + getImplementation() + ")" );
         }
 
-        if ( parameters == null )
-        {
-            parameters = new LinkedList<>();
-        }
-
         parameters.add( parameter );
     }
 
     /**
-     * @return the list parameters as a Map
+     * @return the list parameters as a Map (keyed by {@link Parameter#getName()}) that is built from
+     * {@link #parameters} list on each call. In other words, the map returned is built on fly and is a copy.
+     * Any change to this map is NOT reflected on list and other way around!
      */
     public Map<String, Parameter> getParameterMap()
     {
-        if ( parameterMap == null )
+        LinkedHashMap<String, Parameter> parameterMap = new LinkedHashMap<>();
+
+        for ( Parameter pd : parameters )
         {
-            parameterMap = new HashMap<>();
-
-            if ( parameters != null )
-            {
-                for ( Parameter pd : parameters )
-                {
-                    parameterMap.put( pd.getName(), pd );
-                }
-            }
+            parameterMap.put( pd.getName(), pd );
         }
 
         return parameterMap;
@@ -539,53 +532,17 @@ public boolean equals( Object object )
         {
             MojoDescriptor other = (MojoDescriptor) object;
 
-            if ( !compareObjects( getPluginDescriptor(), other.getPluginDescriptor() ) )
-            {
-                return false;
-            }
-
-            return compareObjects( getGoal(), other.getGoal() );
-
+            return Objects.equals( getPluginDescriptor(), other.getPluginDescriptor() )
+                    && Objects.equals( getGoal(), other.getGoal() );
         }
 
         return false;
     }
 
-    private boolean compareObjects( Object first, Object second )
-    {
-        if ( first == second )
-        {
-            return true;
-        }
-
-        if ( first == null || second == null )
-        {
-            return false;
-        }
-
-        return first.equals( second );
-    }
-
     /** {@inheritDoc} */
     public int hashCode()
     {
-        int result = 1;
-
-        String goal = getGoal();
-
-        if ( goal != null )
-        {
-            result += goal.hashCode();
-        }
-
-        PluginDescriptor pd = getPluginDescriptor();
-
-        if ( pd != null )
-        {
-            result -= pd.hashCode();
-        }
-
-        return result;
+        return Objects.hash( getGoal(), getPluginDescriptor() );
     }
 
     /**

maven-plugin-api/src/test/java/org/apache/maven/plugin/descriptor/MojoDescriptorTest.java

@@ -0,0 +1,53 @@
+package org.apache.maven.plugin.descriptor;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class MojoDescriptorTest
+{
+    @Test
+    public void getParameterMap() throws DuplicateParameterException
+    {
+        MojoDescriptor mojoDescriptor = new MojoDescriptor();
+        Parameter param1 = new Parameter();
+        param1.setName( "param1" );
+        param1.setDefaultValue( "value1" );
+        mojoDescriptor.addParameter( param1 );
+
+        assertEquals( 1, mojoDescriptor.getParameters().size() );
+
+        assertEquals( mojoDescriptor.getParameters().size(), mojoDescriptor.getParameterMap().size() );
+
+        Parameter param2 = new Parameter();
+        param2.setName( "param2" );
+        param2.setDefaultValue( "value2" );
+        mojoDescriptor.addParameter( param2 );
+
+        assertEquals( 2, mojoDescriptor.getParameters().size() );
+        assertEquals( mojoDescriptor.getParameters().size(), mojoDescriptor.getParameterMap().size() );
+    }
+
+}