[JBPM-10174] Do not call readValue in VariableScope (#2288) (#2294)

* [JBPM-10174] Do not call readValue in VariableScope

* [JBPM-10174] Sonar warnings

* [JBPM-10174] Supporting date conversion

* [JBPM-10174] Increasing coverage

Co-authored-by: Francisco Javier Tirado Sarti <65240126+fjtirado@users.noreply.github.com>

jbpm-flow/src/main/java/org/jbpm/process/core/context/variable/VariableScope.java

@@ -102,18 +102,13 @@ public Object validateVariable(String processName, String name, Object value) {
             }
         } else if (value != null) {
             DataType type = var.getType();
-            ObjectDataType obj = new ObjectDataType();
             boolean isRightType = type == null || type.verifyDataType(value);
             if (!isRightType ) {
                 if (variableStrictEnabled) {
                     throw new IllegalArgumentException("Variable '" + name + "' has incorrect data type expected:" + var
                             .getType().getStringType() + " actual:" + value.getClass().getName());
 				} else if (value instanceof String) {
-					if (type.getClass() == obj.getClass()) {
-						value = type.readValue(value.toString());
-					} else {
-						value = type.valueOf(value.toString());
-					}
+					value = type.valueOf(value.toString());
 				}
             }
         }

jbpm-flow/src/main/java/org/jbpm/process/core/datatype/impl/type/ObjectDataType.java

@@ -19,16 +19,24 @@
 import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
+import java.text.DateFormat;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.ZonedDateTime;
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.Date;
 import java.util.Map;
+import java.util.Optional;
 
 import com.thoughtworks.xstream.XStream;
 import com.thoughtworks.xstream.security.ExplicitTypePermission;
 import org.drools.reflective.classloader.ProjectClassLoader;
 import org.jbpm.process.core.datatype.DataType;
+import org.slf4j.LoggerFactory;
+import org.slf4j.Logger;
 
 import static org.kie.soup.xstream.XStreamUtils.createTrustingXStream;
 
@@ -37,6 +45,7 @@
  */
 public class ObjectDataType implements DataType {
 
+    private static final Logger logger = LoggerFactory.getLogger(ObjectDataType.class);
     private static final long serialVersionUID = 510l;
 
     private String className;
@@ -79,55 +88,83 @@ public void writeExternal(ObjectOutput out) throws IOException {
         out.writeObject(className);
     }
 
-    public boolean verifyDataType(final Object value) {
-        if (value == null || className == null) {
-            return true;
-        }
+    private static final Collection<String> prefixes = Arrays.asList("java.lang", "java.util", "java.time");
+
+    private Optional<Class<?>> getClass(Object value) {
         try {
-            Class<?> clazz = Class.forName(className, true, value.getClass().getClassLoader());
-            if (clazz.isInstance(value) || isValidDate(value)) {
-                return true;
-            }
+            return Optional.of(Class.forName(className, true, value.getClass().getClassLoader()));
         } catch (ClassNotFoundException e) {
-            // check class again
+            logger.info("Error {} loading class {}", e, className);
         }
-        // try to expand fundamental classes if it's not a FQCN
-        if(!className.contains(".")) {
-            try {
-                className = "java.lang."+className;
-                Class<?> clazz = Class.forName(className, true, value.getClass().getClassLoader());
-                if (clazz.isInstance(value)) {
-                    return true;
+        if (!className.contains(".")) {
+            for (String prefix : prefixes) {
+                String altName = prefix + "." + className;
+                try {
+                    return Optional.of(Class.forName(altName, true, classLoader));
+                } catch (ClassNotFoundException e) {
+                    logger.debug("Error {} loading class {}", e, altName);
                 }
-            } catch (ClassNotFoundException e) {
-                return false;
             }
         }
-        return false;
+        return Optional.empty();
     }
 
-    private boolean isValidDate(Object value) {
-        boolean parseable = false;
-        try{
-            parseable = LocalDate.parse((String)value)!=null;
-        } catch(Exception e) {
-            // ignore parse exception
+    public boolean verifyDataType(final Object value) {
+        if (value == null || className == null) {
+            return true;
         }
-        try{
-            parseable = LocalDateTime.parse((String)value)!=null;
-        } catch(Exception e) {
-            // ignore parse exception
+        return getClass(value).map(c -> c.isInstance(value)).orElse(false);
+    }
+
+    private Optional<Object> getObjectFromClass(final Object value) {
+        Optional<Class<?>> clazz = getClass(value);
+        if (clazz.isPresent()) {
+            Class<?> objectClass = clazz.get();
+            if (objectClass.isInstance(value)) {
+                return Optional.of(value);
+            }
+            if (Date.class.isAssignableFrom(objectClass)) {
+                return Optional.of(parseDate(value.toString()));
+            } else if (LocalDate.class.isAssignableFrom(objectClass)) {
+                return Optional.of(LocalDate.parse((value.toString())));
+            } else if (LocalDateTime.class.isAssignableFrom(objectClass)) {
+                return Optional.of(LocalDateTime.parse((value.toString())));
+            } else if (ZonedDateTime.class.isAssignableFrom(objectClass)) {
+                return Optional.of(ZonedDateTime.parse((value.toString())));
+            }
         }
-        try{
-            parseable = ZonedDateTime.parse((String)value)!=null;
-        } catch(Exception e) {
-            // ignore parse exception
+        return Optional.empty();
+    }
+
+    private static Collection<DateFormat> dateFormats = Arrays.asList(new SimpleDateFormat("yyyy-MM-ddHH:mm:ss"),
+            new SimpleDateFormat("yyyy-MM-dd"), new SimpleDateFormat("HH:mm:ss"), DateFormat.getDateInstance(),
+            DateFormat.getTimeInstance(),
+            DateFormat.getDateTimeInstance());
+
+    private Date parseDate(String toBeParsed) {
+        StringBuilder sb = new StringBuilder();
+        for (DateFormat dateFormat : dateFormats) {
+            try {
+                return dateFormat.parse(toBeParsed);
+            } catch (ParseException ex) {
+                sb.append(ex.getMessage()).append(System.lineSeparator());
+            }
         }
-        return parseable;
+        throw new IllegalArgumentException(sb.toString());
     }
 
+    @Override
     public Object readValue(String value) {
-        return getXStream().fromXML(value);
+        return value != null ? getObjectFromClass(value).orElseGet(() -> getXStream().fromXML(value)) : null;
+    }
+
+    @Override
+    public Object valueOf(String value) {
+        try {
+            return value != null ? getObjectFromClass(value).orElse(value) : null;
+        } catch (IllegalArgumentException e) {
+            return value;
+        }
     }
 
     public String writeValue(Object value) {

jbpm-flow/src/main/java/org/jbpm/process/instance/AbstractProcessInstanceFactory.java

@@ -64,7 +64,7 @@ public ProcessInstance createProcessInstance(Process process, CorrelationKey cor
                 throw new IllegalArgumentException( "This process does not support parameters!" );
             }
         }
-        variableScopeInstance.setDefaultValue(process,variableScope,variableScopeInstance);
+        variableScopeInstance.setDefaultValues(variableScope, variableScopeInstance);
         variableScopeInstance.enforceRequiredVariables();
 
         return processInstance;

jbpm-flow/src/main/java/org/jbpm/process/instance/context/variable/VariableScopeInstance.java

@@ -31,7 +31,6 @@
 import org.jbpm.process.instance.context.AbstractContextInstance;
 import org.jbpm.workflow.core.Node;
 import org.jbpm.workflow.instance.node.CompositeContextNodeInstance;
-import org.kie.api.definition.process.Process;
 import org.kie.api.runtime.KieSession;
 import org.kie.api.runtime.process.CaseData;
 import org.kie.api.runtime.process.ProcessInstance;
@@ -210,18 +209,15 @@ private boolean hasData(Object data) {
         return data != null && (!(data instanceof CharSequence) || !data.toString().trim().isEmpty());
     }
 
-    public void setDefaultValue(Process process,VariableScope variableScope,VariableScopeInstance variableScopeInstance) {        
+    public void setDefaultValues(VariableScope variableScope, VariableScopeInstance variableScopeInstance) {
         if (variableScope != null) {
             for (Variable variable : variableScope.getVariables()) {
                 String name = variable.getName();
                 Object defaultValue = variable.getMetaData("defaultValue");
                 if (variableScopeInstance.getVariable(name) == null && defaultValue != null) {
-                    variableScopeInstance.setVariable(name,
-                            variableScope.validateVariable(process.getName(), name, defaultValue));
-
+                    variableScopeInstance.setVariable(name, variable.getType().readValue(defaultValue.toString()));
                 }
             }
         }
-    }   
-
+    }
 }

jbpm-flow/src/test/java/org/jbpm/process/core/datatype/impl/ObjectDataTypeTest.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2023 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed 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.jbpm.process.core.datatype.impl;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.ZonedDateTime;
+import java.util.Date;
+
+import org.jbpm.process.core.datatype.impl.type.ObjectDataType;
+import org.junit.Test;
+
+
+public class ObjectDataTypeTest {
+
+    @Test
+    public void testReadValueNull() {
+        ObjectDataType dateType = new ObjectDataType(Date.class.getCanonicalName());
+        assertThat(dateType.readValue(null)).isNull();
+        assertThat(dateType.valueOf(null)).isNull();
+        assertThat(dateType.verifyDataType(null)).isTrue();
+    }
+
+    @Test
+    public void testReadValueDate() {
+        ObjectDataType dateType = new ObjectDataType(Date.class.getCanonicalName());
+        assertThat(dateType.readValue("2012-02-02")).isInstanceOf(Date.class);
+        assertThat(dateType.readValue("12:12:12")).isInstanceOf(Date.class);
+        assertThat(dateType.valueOf("2012-02-02")).isInstanceOf(Date.class);
+        assertThat(dateType.valueOf("12:12:12")).isInstanceOf(Date.class);
+        assertThat(dateType.valueOf("pepe")).isInstanceOf(String.class);
+        assertThatThrownBy(() -> dateType.readValue("pepe")).isInstanceOf(IllegalArgumentException.class);
+    }
+
+    @Test
+    public void testReadValueLocalDate() {
+        ObjectDataType dateType = new ObjectDataType(LocalDate.class.getCanonicalName());
+        assertThat(dateType.readValue("2012-02-02")).isInstanceOf(LocalDate.class);
+        assertThat(dateType.valueOf("2012-02-02")).isInstanceOf(LocalDate.class);
+    }
+
+    @Test
+    public void testReadValueLocalDateTime() {
+        ObjectDataType dateType = new ObjectDataType(LocalDateTime.class.getCanonicalName());
+        assertThat(dateType.readValue("2012-02-02T12:12:12")).isInstanceOf(LocalDateTime.class);
+        assertThat(dateType.valueOf("2012-02-02T12:12:12")).isInstanceOf(LocalDateTime.class);
+    }
+
+    @Test
+    public void testReadValueZonedDateTime() {
+        ObjectDataType dateType = new ObjectDataType(ZonedDateTime.class.getCanonicalName());
+        assertThat(dateType.readValue("2012-02-02T12:12:12+00:01")).isInstanceOf(ZonedDateTime.class);
+        assertThat(dateType.valueOf("2012-02-02T12:12:12+00:01")).isInstanceOf(ZonedDateTime.class);
+    }
+}