diff --git a/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java b/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java index 11c90a3b54c1..6e3d1a012c28 100644 --- a/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java +++ b/maven-plugin-api/src/main/java/org/apache/maven/plugin/descriptor/MojoDescriptor.java @@ -18,10 +18,11 @@ */ package org.apache.maven.plugin.descriptor; -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; @@ -51,9 +52,7 @@ public class MojoDescriptor extends ComponentDescriptor implements Cloneab private static final String DEFAULT_LANGUAGE = "java"; - private List parameters; - - private Map parameterMap; + private final ArrayList parameters; /** By default, the execution strategy is "once-per-session" */ private String executionStrategy = SINGLE_PASS_EXEC_STRATEGY; @@ -91,7 +90,7 @@ public class MojoDescriptor extends ComponentDescriptor implements Cloneab private String deprecated; /** - * Flags this Mojo to run it in a multi module way, i.e. aggregate the build with the set of projects listed as + * Flags this Mojo to run it in a multi-module way, i.e. aggregate the build with the set of projects listed as * modules. By default, no need to aggregate the Maven project and its child modules */ private boolean aggregator = false; @@ -140,6 +139,7 @@ public class MojoDescriptor extends ComponentDescriptor implements Cloneab * Default constructor. */ public MojoDescriptor() { + this.parameters = new ArrayList<>(); setInstantiationStrategy(DEFAULT_INSTANTIATION_STRATEGY); setComponentFactory(DEFAULT_LANGUAGE); } @@ -177,10 +177,11 @@ 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 getParameters() { - return parameters; + return new ArrayList<>(parameters); } /** @@ -188,6 +189,7 @@ public List getParameters() { * @throws DuplicateParameterException if any */ public void setParameters(List parameters) throws DuplicateParameterException { + this.parameters.clear(); for (Parameter parameter : parameters) { addParameter(parameter); } @@ -198,31 +200,25 @@ public void setParameters(List parameters) throws DuplicateParameterE * @throws DuplicateParameterException if any */ 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 getParameterMap() { - if (parameterMap == null) { - parameterMap = new HashMap<>(); - - if (parameters != null) { - for (Parameter pd : parameters) { - parameterMap.put(pd.getName(), pd); - } - } + LinkedHashMap parameterMap = new LinkedHashMap<>(); + + for (Parameter pd : parameters) { + parameterMap.put(pd.getName(), pd); } return parameterMap; @@ -321,14 +317,14 @@ public boolean requiresOnline() { } /** - * @return the binded phase name of the Mojo + * @return the bound phase name of the Mojo */ public String getPhase() { return phase; } /** - * @param phase the new binded phase name of the Mojo + * @param phase the new bound phase name of the Mojo */ public void setPhase(String phase) { this.phase = phase; @@ -482,45 +478,16 @@ public boolean equals(Object object) { if (object instanceof MojoDescriptor) { 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()); } /** diff --git a/maven-plugin-api/src/test/java/org/apache/maven/plugin/descriptor/MojoDescriptorTest.java b/maven-plugin-api/src/test/java/org/apache/maven/plugin/descriptor/MojoDescriptorTest.java new file mode 100644 index 000000000000..cf0ac23a7b47 --- /dev/null +++ b/maven-plugin-api/src/test/java/org/apache/maven/plugin/descriptor/MojoDescriptorTest.java @@ -0,0 +1,67 @@ +/* + * 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. + */ +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 junit.framework.TestCase; + +public class MojoDescriptorTest extends TestCase { + + public void testGetParameterMap() 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()); + } +}