fix(engine): fix storing parameter types in expression resolving

add test case for method expression evaluation with 2 params of the same type

Related to CAM-14916, Closes PR #2029

Co-authored-by: tasso94 <3015690+tasso94@users.noreply.github.com>

engine/src/main/java/org/camunda/bpm/engine/impl/javax/el/BeanELResolver.java

@@ -22,14 +22,14 @@
 import java.lang.reflect.Array;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
@@ -44,7 +44,7 @@
  * for details. Because this resolver handles base objects of any type, it should be placed near the
  * end of a composite resolver. Otherwise, it will claim to have resolved a property before any
  * resolvers that come after it get a chance to test if they can do so as well.
- * 
+ *
  * @see CompositeELResolver
  * @see ELResolver
  */
@@ -143,7 +143,7 @@ private static Method findAccessibleMethod(Method method) {
 
 	private final boolean readOnly;
 	private final ConcurrentHashMap<Class<?>, BeanProperties> cache;
-	
+
 	private ExpressionFactory defaultFactory;
 
 	/**
@@ -166,7 +166,7 @@ public BeanELResolver(boolean readOnly) {
 	 * the property argument. Otherwise, returns null. Assuming the base is not null, this method
 	 * will always return Object.class. This is because any object is accepted as a key and is
 	 * coerced into a string.
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -191,7 +191,7 @@ public Class<?> getCommonPropertyType(ELContext context, Object base) {
 	 * PropertyDescriptor.getPropertyType().</li>
 	 * <li>{@link ELResolver#RESOLVABLE_AT_DESIGN_TIME} - true.</li>
 	 * </ul>
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -245,7 +245,7 @@ public void remove() {
 	 * will first be coerced to a String. If there is a BeanInfoProperty for this property and there
 	 * were no errors retrieving it, the propertyType of the propertyDescriptor is returned.
 	 * Otherwise, a PropertyNotFoundException is thrown.
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -285,7 +285,7 @@ public Class<?> getType(ELContext context, Object base, Object property) {
 	 * JavaBeans specification, then return the result of the getter call. If the getter throws an
 	 * exception, it is propagated to the caller. If the property is not found or is not readable, a
 	 * PropertyNotFoundException is thrown.
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -332,7 +332,7 @@ public Object getValue(ELContext context, Object base, Object property) {
 	 * null, the propertyResolved property of the ELContext object must be set to true by this
 	 * resolver, before returning. If this property is not true after this method is called, the
 	 * caller can safely assume no value was set.
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -374,7 +374,7 @@ public boolean isReadOnly(ELContext context, Object base, Object property) {
 	 * (as per the JavaBeans Specification), the setter method is called (passing value). If the
 	 * property exists but does not have a setter, then a PropertyNotFoundException is thrown. If
 	 * the property does not exist, a PropertyNotFoundException is thrown.
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -422,33 +422,33 @@ public void setValue(ELContext context, Object base, Object property, Object val
 	/**
 	 * If the base object is not <code>null</code>, invoke the method, with the given parameters on
 	 * this bean. The return value from the method is returned.
-	 * 
+	 *
 	 * <p>
 	 * If the base is not <code>null</code>, the <code>propertyResolved</code> property of the
 	 * <code>ELContext</code> object must be set to <code>true</code> by this resolver, before
 	 * returning. If this property is not <code>true</code> after this method is called, the caller
 	 * should ignore the return value.
 	 * </p>
-	 * 
+	 *
 	 * <p>
 	 * The provided method object will first be coerced to a <code>String</code>. The methods in the
 	 * bean is then examined and an attempt will be made to select one for invocation. If no
 	 * suitable can be found, a <code>MethodNotFoundException</code> is thrown.
-	 * 
+	 *
 	 * If the given paramTypes is not <code>null</code>, select the method with the given name and
 	 * parameter types.
-	 * 
+	 *
 	 * Else select the method with the given name that has the same number of parameters. If there
 	 * are more than one such method, the method selection process is undefined.
-	 * 
+	 *
 	 * Else select the method with the given name that takes a variable number of arguments.
-	 * 
+	 *
 	 * Note the resolution for overloaded methods will likely be clarified in a future version of
 	 * the spec.
-	 * 
+	 *
 	 * The provided parameters are coerced to the corresponding parameter types of the method, and
 	 * the method is then invoked.
-	 * 
+	 *
 	 * @param context
 	 *            The context of this evaluation.
 	 * @param base
@@ -505,7 +505,7 @@ public Object invoke(ELContext context, Object base, Object method, Class<?>[] p
   private Method findMethod(Object base, String name, Class<?>[] types, Object[] params) {
 		boolean hasTypesInitially = types != null;
 		if (!hasTypesInitially) {
-			Set<Class<?>> detectedTypes = new HashSet<>();
+			List<Class<?>> detectedTypes = new ArrayList<>();
 			Arrays.stream(params)
 					.filter(Objects::nonNull)
 					.forEach(param -> detectedTypes.add(param.getClass()));
@@ -557,7 +557,7 @@ private ExpressionFactory getExpressionFactory(ELContext context) {
 		}
 		return defaultFactory;
 	}
-	
+
 	private Object[] coerceParams(ExpressionFactory factory, Method method, Object[] params) {
 		Class<?>[] types = method.getParameterTypes();
 		Object[] args = new Object[types.length];
@@ -611,10 +611,10 @@ private void coerceValue(Object array, int index, ExpressionFactory factory, Obj
 			Array.set(array, index, factory.coerceToType(value, type));
 		}
 	}
-	
+
 	/**
 	 * Test whether the given base should be resolved by this ELResolver.
-	 * 
+	 *
 	 * @param base
 	 *            The bean to analyze.
 	 * @param property
@@ -627,7 +627,7 @@ private final boolean isResolvable(Object base) {
 
 	/**
 	 * Lookup BeanProperty for the given (base, property) pair.
-	 * 
+	 *
 	 * @param base
 	 *            The bean to analyze.
 	 * @param property
@@ -655,10 +655,10 @@ private final BeanProperty toBeanProperty(Object base, Object property) {
 	/**
 	 * This method is not part of the API, though it can be used (reflectively) by clients of this
 	 * class to remove entries from the cache when the beans are being unloaded.
-	 * 
+	 *
 	 * Note: this method is present in the reference implementation, so we're adding it here to ease
 	 * migration.
-	 * 
+	 *
 	 * @param classloader
 	 *            The classLoader used to load the beans.
 	 */

engine/src/test/java/org/camunda/bpm/engine/test/bpmn/el/ExpressionManagerTest.java

@@ -185,4 +185,20 @@ public void shouldCompareWithBigDecimal() {
     assertThat(userTask).isNotNull();
   }
 
+  @Deployment
+  @Test
+  public void shouldResolveMethodExpressionTwoParametersSameType() {
+    // given process with two service tasks that resolve expression and store the result as variable
+    Map<String, Object> vars = new HashMap<>();
+    vars.put("myVar", new ExpressionTestParameter());
+
+    // when the process is started
+    ProcessInstance processInstance = runtimeService.startProcessInstanceByKey("process", vars);
+
+    // then no exceptions are thrown and two variables are saved
+    boolean task1Var = (boolean) runtimeService.getVariable(processInstance.getId(), "task1Var");
+    assertThat(task1Var).isTrue();
+    String task2Var = (String) runtimeService.getVariable(processInstance.getId(), "task2Var");
+    assertEquals("lastParam", task2Var);
+  }
 }

engine/src/test/java/org/camunda/bpm/engine/test/bpmn/el/ExpressionTestParameter.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH
+ * under one or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information regarding copyright
+ * ownership. Camunda licenses this file to you under the Apache License,
+ * Version 2.0; 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.camunda.bpm.engine.test.bpmn.el;
+
+import java.io.Serializable;
+
+/**
+ * Class used to test resolving method parameters
+ */
+public class ExpressionTestParameter implements Serializable {
+
+  protected static final long serialVersionUID = 1L;
+
+  public boolean testMethod(String param1) {
+    return false;
+  }
+
+  public boolean testMethod(String param1, Object param2) {
+    return false;
+  }
+
+  // first service task
+  public boolean testMethod(String param1, String param2) {
+    return true;
+  }
+
+  // second service task
+  public String testMethod(String param1, String param2, String param3) {
+    return param3;
+  }
+
+}

engine/src/test/resources/org/camunda/bpm/engine/test/bpmn/el/ExpressionManagerTest.shouldResolveMethodExpressionTwoParametersSameType.bpmn20.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions
+  xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL"
+  xmlns:camunda="http://camunda.org/schema/1.0/bpmn"
+  targetNamespace="Examples">
+
+  <process id="process" name="Test method expression with two params same type" isExecutable="true">
+
+    <startEvent id="theStart" />
+    <sequenceFlow id="flow1" sourceRef="theStart" targetRef="servicetask1" />
+    <serviceTask id="servicetask1" name="ServiceTask 1"
+                 camunda:expression="${execution.setVariable('task1Var',myVar.testMethod('invoiceNumber','NO-123'))}" />
+    <sequenceFlow id="flow2" sourceRef="servicetask1" targetRef="servicetask2" />
+    <serviceTask id="servicetask2" name="ServiceTask 2"
+                 camunda:expression="${execution.setVariable('task2Var',myVar.testMethod('invoiceNumber','NO-123','lastParam'))}" />
+    <sequenceFlow id="flow3" sourceRef="servicetask2" targetRef="userTask" />
+    <userTask id="userTask" />
+    <sequenceFlow id="flow4" sourceRef="userTask" targetRef="theEnd" />
+    <endEvent id="theEnd" />
+
+  </process>
+
+</definitions>