PLANNER-733 Apply custom properties through setter

Fixes bug: Partitioned Search config ignores customProperties and always causes partCount 4 in CloudBalancePartitioner

optaplanner-core/src/main/java/org/optaplanner/core/config/phase/custom/CustomPhaseConfig.java

@@ -17,7 +17,6 @@
 package org.optaplanner.core.config.phase.custom;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -29,7 +28,6 @@
 import org.optaplanner.core.config.solver.EnvironmentMode;
 import org.optaplanner.core.config.util.ConfigUtils;
 import org.optaplanner.core.config.util.KeyAsElementMapConverter;
-import org.optaplanner.core.impl.heuristic.common.PropertiesConfigurable;
 import org.optaplanner.core.impl.phase.custom.CustomPhase;
 import org.optaplanner.core.impl.phase.custom.CustomPhaseCommand;
 import org.optaplanner.core.impl.phase.custom.DefaultCustomPhase;

optaplanner-core/src/main/java/org/optaplanner/core/config/util/ConfigUtils.java

@@ -19,6 +19,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
@@ -43,8 +44,6 @@
 import org.optaplanner.core.impl.domain.common.accessor.FieldMemberAccessor;
 import org.optaplanner.core.impl.domain.common.accessor.MemberAccessor;
 import org.optaplanner.core.impl.domain.common.accessor.MethodMemberAccessor;
-import org.optaplanner.core.impl.heuristic.common.PropertiesConfigurable;
-import org.optaplanner.core.impl.phase.custom.CustomPhaseCommand;
 
 public class ConfigUtils {
 
@@ -57,19 +56,59 @@ public static <T> T newInstance(Object bean, String propertyName, Class<T> clazz
         }
     }
 
-    public static void applyCustomProperties(Object bean, String propertyName, Map<String, String> customProperties) {
-        if (bean instanceof PropertiesConfigurable) {
-            Map<String, String> customProperties_ = customProperties != null
-                    ? customProperties : Collections.emptyMap();
-            // Always call it to allow for initialization, even if no custom properties are configured
-            ((PropertiesConfigurable) bean).applyCustomProperties(customProperties_);
-        } else {
-            if (customProperties != null) {
-                throw new IllegalStateException("There are customProperties (" + customProperties
-                        + ") but the " + propertyName + " (" + bean.getClass()
-                        + ") does not implement " + PropertiesConfigurable.class.getSimpleName() + ".");
-            }
+    public static void applyCustomProperties(Object bean, String wrappingPropertyName,
+            Map<String, String> customProperties) {
+        if (customProperties == null) {
+            return;
         }
+        Class<?> beanClass = bean.getClass();
+        customProperties.forEach((propertyName, valueString) -> {
+            Method setterMethod = ReflectionHelper.getSetterMethod(beanClass, propertyName);
+            if (setterMethod == null) {
+                throw new IllegalStateException("The customProperty (" + propertyName
+                        + ") with value (" + valueString
+                        + ") cannot be set on class (" + beanClass
+                        + ") because it has no public setter for that property.\n"
+                        + "Maybe add a public setter for that customProperty (" + propertyName
+                        + ") on that class (" + beanClass.getSimpleName() + ").");
+            }
+            Class<?> propertyType = setterMethod.getParameterTypes()[0];
+            Object typedValue;
+            try {
+                if (propertyType.equals(String.class)) {
+                    typedValue = valueString;
+                } else if (propertyType.equals(Boolean.TYPE) || propertyType.equals(Boolean.class)) {
+                    typedValue = Boolean.parseBoolean(valueString);
+                } else if (propertyType.equals(Integer.TYPE) || propertyType.equals(Integer.class)) {
+                    typedValue = Integer.parseInt(valueString);
+                } else if (propertyType.equals(Long.TYPE) || propertyType.equals(Long.class)) {
+                    typedValue = Long.parseLong(valueString);
+                } else if (propertyType.equals(Float.TYPE) || propertyType.equals(Float.class)) {
+                    typedValue = Float.parseFloat(valueString);
+                } else if (propertyType.equals(Double.TYPE) || propertyType.equals(Double.class)) {
+                    typedValue = Double.parseDouble(valueString);
+                } else {
+                    throw new IllegalStateException("The customProperty (" + propertyName + ") of class (" + beanClass
+                            + ") has an unsupported propertyType (" + propertyType + ") for value (" + valueString + ").");
+                }
+            } catch (NumberFormatException e) {
+                throw new IllegalStateException("The customProperty (" + propertyName + ")'s value (" + valueString
+                        + ") cannot be parsed to the propertyType (" + propertyType
+                        + ") of the setterMethod (" + setterMethod + ").");
+            }
+            try {
+                setterMethod.invoke(bean, typedValue);
+            } catch (IllegalAccessException e) {
+                throw new IllegalStateException("Cannot call property (" + propertyName
+                        + ") setterMethod (" + setterMethod + ") on bean of class (" + bean.getClass()
+                        + ") for value (" + typedValue + ").", e);
+            } catch (InvocationTargetException e) {
+                throw new IllegalStateException("The property (" + propertyName
+                        + ") setterMethod (" + setterMethod + ") on bean of class (" + bean.getClass()
+                        + ") throws an exception for value (" + typedValue + ").",
+                        e.getCause());
+            }
+        });
     }
 
     public static <C extends AbstractConfig<C>> C inheritConfig(C original, C inherited) {

optaplanner-core/src/main/java/org/optaplanner/core/impl/domain/common/ReflectionHelper.java

@@ -27,6 +27,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Stream;
 
 /**
  * Avoids the usage of Introspector to work on Android too.
@@ -144,6 +145,28 @@ public static Method getSetterMethod(Class<?> containingClass, Class<?> property
         }
     }
 
+    /**
+     * @param containingClass never null
+     * @param propertyName never null
+     * @return null if it doesn't exist
+     */
+    public static Method getSetterMethod(Class<?> containingClass, String propertyName) {
+        String setterName = PROPERTY_MUTATOR_PREFIX
+                + (propertyName.isEmpty() ? "" : propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
+        Method[] methods = Arrays.stream(containingClass.getMethods())
+                .filter(method -> method.getName().equals(setterName))
+                .toArray(Method[]::new);
+        if (methods.length == 0) {
+            return null;
+        }
+        if (methods.length > 1) {
+            throw new IllegalStateException("The containingClass (" + containingClass
+                    + ") has multiple setter methods (" + Arrays.toString(methods)
+                    + ") with the propertyName (" + propertyName + ").");
+        }
+        return methods[0];
+    }
+
     public static void assertGetterMethod(Method getterMethod, Class<? extends Annotation> annotationClass) {
         if (getterMethod.getParameterTypes().length != 0) {
             throw new IllegalStateException("The getterMethod (" + getterMethod + ") with a "

optaplanner-core/src/main/java/org/optaplanner/core/impl/partitionedsearch/partitioner/SolutionPartitioner.java

@@ -21,14 +21,13 @@
 import org.optaplanner.core.api.domain.entity.PlanningEntity;
 import org.optaplanner.core.api.domain.solution.PlanningSolution;
 import org.optaplanner.core.api.domain.solution.cloner.SolutionCloner;
-import org.optaplanner.core.impl.heuristic.common.PropertiesConfigurable;
 import org.optaplanner.core.impl.score.director.ScoreDirector;
 
 /**
  * Splits one {@link PlanningSolution solution} into multiple partitions.
  * The partitions are solved and merged based on the {@link PlanningSolution#locationStrategyType()}.
  * <p>
- * To add custom properties, implement the {@link PropertiesConfigurable} interface too.
+ * To add custom properties, configure custom properties and add public setters for them.
  * @param <Solution_> the solution type, the class with the {@link PlanningSolution} annotation
  */
 public interface SolutionPartitioner<Solution_> {

optaplanner-core/src/main/java/org/optaplanner/core/impl/phase/custom/CustomPhaseCommand.java

@@ -16,13 +16,9 @@
 
 package org.optaplanner.core.impl.phase.custom;
 
-import java.util.Map;
-
 import org.optaplanner.core.api.domain.solution.PlanningSolution;
 import org.optaplanner.core.api.score.Score;
 import org.optaplanner.core.api.solver.Solver;
-import org.optaplanner.core.api.solver.SolverFactory;
-import org.optaplanner.core.impl.heuristic.common.PropertiesConfigurable;
 import org.optaplanner.core.impl.phase.Phase;
 import org.optaplanner.core.impl.score.director.ScoreDirector;
 import org.optaplanner.core.impl.solver.ProblemFactChange;
@@ -34,7 +30,7 @@
  * <p>
  * An implementation must extend {@link AbstractCustomPhaseCommand} to ensure backwards compatibility in future versions.
  * <p>
- * To add custom properties, implement the {@link PropertiesConfigurable} interface too.
+ * To add custom properties, configure custom properties and add public setters for them.
  * @param <Solution_> the solution type, the class with the {@link PlanningSolution} annotation
  * @see AbstractCustomPhaseCommand
  */

optaplanner-core/src/test/java/org/optaplanner/core/config/util/ConfigUtilsTest.java

@@ -16,6 +16,9 @@
 
 package org.optaplanner.core.config.util;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -74,4 +77,110 @@ public void ceilDivideByZero() {
         ConfigUtils.ceilDivide(20, -0);
     }
 
+    @Test
+    public void applyCustomProperties() {
+        Map<String, String> customProperties = new HashMap<>();
+        customProperties.put("string", "This is a sentence.");
+        customProperties.put("primitiveBoolean", "true");
+        customProperties.put("objectBoolean", "true");
+        customProperties.put("primitiveInt", "1");
+        customProperties.put("objectInteger", "2");
+        customProperties.put("primitiveLong", "3");
+        customProperties.put("objectLong", "4");
+        customProperties.put("primitiveFloat", "5.5");
+        customProperties.put("objectFloat", "6.6");
+        customProperties.put("primitiveDouble", "7.7");
+        customProperties.put("objectDouble", "8.8");
+        ConfigUtilsTestBean bean = new ConfigUtilsTestBean();
+        ConfigUtils.applyCustomProperties(bean, "customProperties", customProperties);
+        assertEquals(true, bean.primitiveBoolean);
+        assertEquals(Boolean.TRUE, bean.objectBoolean);
+        assertEquals(1, bean.primitiveInt);
+        assertEquals(Integer.valueOf(2), bean.objectInteger);
+        assertEquals(3L, bean.primitiveLong);
+        assertEquals(Long.valueOf(4L), bean.objectLong);
+        assertEquals(5.5F, bean.primitiveFloat, 0.0F);
+        assertEquals(Float.valueOf(6.6F), bean.objectFloat);
+        assertEquals(7.7, bean.primitiveDouble, 0.0);
+        assertEquals(Double.valueOf(8.8), bean.objectDouble);
+        assertEquals("This is a sentence.", bean.string);
+    }
+
+    @Test
+    public void applyCustomPropertiesSubset() {
+        Map<String, String> customProperties = new HashMap<>();
+        customProperties.put("string", "This is a sentence.");
+        ConfigUtilsTestBean bean = new ConfigUtilsTestBean();
+        ConfigUtils.applyCustomProperties(bean, "customProperties", customProperties);
+        assertEquals("This is a sentence.", bean.string);
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void applyCustomPropertiesNonExistingCustomProperty() {
+        Map<String, String> customProperties = new HashMap<>();
+        customProperties.put("doesNotExist", "This is a sentence.");
+        ConfigUtilsTestBean bean = new ConfigUtilsTestBean();
+        ConfigUtils.applyCustomProperties(bean, "customProperties", customProperties);
+    }
+
+    private static class ConfigUtilsTestBean {
+
+        private boolean primitiveBoolean;
+        private Boolean objectBoolean;
+        private int primitiveInt;
+        private Integer objectInteger;
+        private long primitiveLong;
+        private Long objectLong;
+        private float primitiveFloat;
+        private Float objectFloat;
+        private double primitiveDouble;
+        private Double objectDouble;
+        private String string;
+
+        public void setPrimitiveBoolean(boolean primitiveBoolean) {
+            this.primitiveBoolean = primitiveBoolean;
+        }
+
+        public void setObjectBoolean(Boolean objectBoolean) {
+            this.objectBoolean = objectBoolean;
+        }
+
+        public void setPrimitiveInt(int primitiveInt) {
+            this.primitiveInt = primitiveInt;
+        }
+
+        public void setObjectInteger(Integer objectInteger) {
+            this.objectInteger = objectInteger;
+        }
+
+        public void setPrimitiveLong(long primitiveLong) {
+            this.primitiveLong = primitiveLong;
+        }
+
+        public void setObjectLong(Long objectLong) {
+            this.objectLong = objectLong;
+        }
+
+        public void setPrimitiveFloat(float primitiveFloat) {
+            this.primitiveFloat = primitiveFloat;
+        }
+
+        public void setObjectFloat(Float objectFloat) {
+            this.objectFloat = objectFloat;
+        }
+
+        public void setPrimitiveDouble(double primitiveDouble) {
+            this.primitiveDouble = primitiveDouble;
+        }
+
+        public void setObjectDouble(Double objectDouble) {
+            this.objectDouble = objectDouble;
+        }
+
+        public void setString(String string) {
+            this.string = string;
+        }
+
+    }
+
 }

optaplanner-docs/src/main/asciidoc/Chapter-Optimization_algorithms/Chapter-Optimization_algorithms.adoc

@@ -712,7 +712,7 @@ The built-in solver phases do not have this issue.
 ====
 
 To configure values of a `CustomPhaseCommand` dynamically in the solver configuration
-(so the <<benchmarker,Benchmarker>> can tweak those parameters), use the `customProperties` element:
+(so the <<benchmarker,Benchmarker>> can tweak those parameters), add the `customProperties` element:
 
 [source,xml,options="nowrap"]
 ----
@@ -723,27 +723,18 @@ To configure values of a `CustomPhaseCommand` dynamically in the solver configur
   </customPhase>
 ----
 
-Then implement the `PropertiesConfigurable` interface to override the `applyCustomProperties()` method
-to parse each custom property when a `Solver` is build.
+Then add a public setter for each custom property, which is called when a `Solver` is build.
+Most value types are supported (including boolean, integers, doubles and strings).
 
 [source,java,options="nowrap"]
 ----
-public class MyCustomPhase extends AbstractCustomPhaseCommand<MySolution>, PropertiesConfigurable {
+public class MyCustomPhase extends AbstractCustomPhaseCommand<MySolution> {
 
-    private int mySelectionSize;
+    private int mySelectionSize = 10;
 
-    @Override
-    public void applyCustomProperties(Map<String, String> customPropertyMap) {
-        String mySelectionSizeString = customPropertyMap.remove("mySelectionSize");
-        try {
-            mySelectionSize = mySelectionSizeString == null ? 10 : Integer.parseInt(mySelectionSizeString);
-        } catch (NumberFormatException e) {
-            throw new IllegalArgumentException("The mySelectionSize (" + mySelectionSizeString + ") cannot be parsed.", e);
-        }
-        if (!customPropertyMap.isEmpty()) {
-            throw new IllegalStateException("The customProperties (" + customPropertyMap.keySet()
-                    + ") are not supported.");
-        }
+    @SuppressWarnings("unused")
+    public void setMySelectionSize(int mySelectionSize) {
+        this.mySelectionSize = mySelectionSize;
     }
 
     ...