From 019a13e0938e82465b36f4ada33fe111bd883d55 Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Wed, 20 Mar 2024 01:55:05 -0400 Subject: [PATCH] chore: Add internal interface for planning cloning atypical types (#727) Sometimes, the domain model has atypical types for fields (for instance, UserList instead of List or ArrayList). The default planning cloner originally failed fast when the field cannot be assigned a typical value. Now, it checks if the collection implements the internal interface. If it does, it uses it to create an empty collection/map. This allows the default planning cloner to be used when the user has control over UserList. --- .../cloner/FieldAccessingSolutionCloner.java | 16 +++- .../solution/cloner/PlanningCloneable.java | 22 ++++++ .../cloner/gizmo/GizmoCloningUtils.java | 4 +- .../gizmo/GizmoSolutionClonerImplementor.java | 79 +++++++++++++------ .../cloner/AbstractSolutionClonerTest.java | 46 +++++++++++ .../PlanningCloneableEntity.java | 26 ++++++ .../PlanningCloneableList.java | 45 +++++++++++ .../PlanningCloneableMap.java | 31 ++++++++ .../PlanningCloneableSolution.java | 44 +++++++++++ 9 files changed, 288 insertions(+), 25 deletions(-) create mode 100644 core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/PlanningCloneable.java create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableEntity.java create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableList.java create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableMap.java create mode 100644 core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableSolution.java diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/FieldAccessingSolutionCloner.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/FieldAccessingSolutionCloner.java index f18cd5be1a..6ead0d7083 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/FieldAccessingSolutionCloner.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/FieldAccessingSolutionCloner.java @@ -108,8 +108,14 @@ private C clone(C original, Map originalToCloneMap, Queue declaringClass = (Class) original.getClass(); - C clone = constructClone(declaringClass); + C clone; + if (original instanceof PlanningCloneable planningCloneable) { + clone = (C) planningCloneable.createNewInstance(); + } else { + clone = constructClone(declaringClass); + } originalToCloneMap.put(original, clone); copyFields(declaringClass, original, clone, unprocessedQueue, declaringClassMetadata); return clone; @@ -189,8 +195,12 @@ private Collection cloneCollection(Class expectedType, Collection o return cloneCollection; } + @SuppressWarnings("unchecked") private static Collection constructCloneCollection(Collection originalCollection) { // TODO Don't hardcode all standard collections + if (originalCollection instanceof PlanningCloneable planningCloneable) { + return (Collection) planningCloneable.createNewInstance(); + } if (originalCollection instanceof LinkedList) { return new LinkedList<>(); } @@ -228,8 +238,12 @@ private Map cloneMap(Class expectedType, Map originalMap, return cloneMap; } + @SuppressWarnings("unchecked") private static Map constructCloneMap(Map originalMap) { // Normally, a Map will never be selected for cloning, but extending implementations might anyway. + if (originalMap instanceof PlanningCloneable planningCloneable) { + return (Map) planningCloneable.createNewInstance(); + } if (originalMap instanceof SortedMap map) { var setComparator = map.comparator(); return new TreeMap<>(setComparator); diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/PlanningCloneable.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/PlanningCloneable.java new file mode 100644 index 0000000000..e6984c5697 --- /dev/null +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/PlanningCloneable.java @@ -0,0 +1,22 @@ +package ai.timefold.solver.core.impl.domain.solution.cloner; + +/** + * An internal interface used to construct new instances of an object + * when there is no suitable constructor. + * Used during planning cloning to create an "empty"/"blank" instance + * whose fields/items will be set to the planning clone of the original's + * fields/items. + * + * @param The type of object being cloned. + */ +public interface PlanningCloneable { + /** + * Creates a new "empty"/"blank" instance. + * If the {@link PlanningCloneable} is a {@link java.util.Collection} + * or {@link java.util.Map}, the returned instance should be + * empty and modifiable. + * + * @return never null, a new instance with the same type as the object being cloned. + */ + T createNewInstance(); +} diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoCloningUtils.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoCloningUtils.java index ff197e5a21..7eae67f93c 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoCloningUtils.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoCloningUtils.java @@ -13,6 +13,7 @@ import java.util.stream.Stream; import ai.timefold.solver.core.impl.domain.solution.cloner.DeepCloningUtils; +import ai.timefold.solver.core.impl.domain.solution.cloner.PlanningCloneable; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; public final class GizmoCloningUtils { @@ -27,7 +28,8 @@ public static Set> getDeepClonedClasses(SolutionDescriptor solutionD deepClonedClassSet.add(clazz); for (Field field : getAllFields(clazz)) { deepClonedClassSet.addAll(getDeepClonedTypeArguments(solutionDescriptor, field.getGenericType())); - if (DeepCloningUtils.isFieldDeepCloned(solutionDescriptor, field, clazz)) { + if (DeepCloningUtils.isFieldDeepCloned(solutionDescriptor, field, clazz) + && !PlanningCloneable.class.isAssignableFrom(field.getType())) { deepClonedClassSet.add(field.getType()); } } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoSolutionClonerImplementor.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoSolutionClonerImplementor.java index 4aee12444e..dabb0e5f1d 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoSolutionClonerImplementor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/solution/cloner/gizmo/GizmoSolutionClonerImplementor.java @@ -33,6 +33,7 @@ import ai.timefold.solver.core.impl.domain.common.accessor.gizmo.GizmoMemberDescriptor; import ai.timefold.solver.core.impl.domain.solution.cloner.DeepCloningUtils; import ai.timefold.solver.core.impl.domain.solution.cloner.FieldAccessingSolutionCloner; +import ai.timefold.solver.core.impl.domain.solution.cloner.PlanningCloneable; import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; import ai.timefold.solver.core.impl.util.MutableReference; @@ -323,7 +324,16 @@ private void createCloneSolutionRun(ClassCreator classCreator, SolutionDescripto GizmoSolutionOrEntityDescriptor solutionSubclassDescriptor = memoizedSolutionOrEntityDescriptorMap.computeIfAbsent(solutionSubclass, (key) -> new GizmoSolutionOrEntityDescriptor(solutionDescriptor, solutionSubclass)); - ResultHandle clone = isSubclassBranch.newInstance(MethodDescriptor.ofConstructor(solutionSubclass)); + + ResultHandle clone; + if (PlanningCloneable.class.isAssignableFrom(solutionSubclass)) { + clone = isSubclassBranch.invokeInterfaceMethod( + MethodDescriptor.ofMethod(PlanningCloneable.class, "createNewInstance", Object.class), + thisObj); + clone = isSubclassBranch.checkCast(clone, solutionSubclass); + } else { + clone = isSubclassBranch.newInstance(MethodDescriptor.ofConstructor(solutionSubclass)); + } isSubclassBranch.invokeInterfaceMethod( MethodDescriptor.ofMethod(Map.class, "put", Object.class, Object.class, Object.class), @@ -599,7 +609,14 @@ private void writeDeepCloneCollectionInstructions(BytecodeCreator bytecodeCreato ResultHandle size = bytecodeCreator .invokeInterfaceMethod(MethodDescriptor.ofMethod(Collection.class, "size", int.class), toClone); - if (List.class.isAssignableFrom(deeplyClonedFieldClass)) { + if (PlanningCloneable.class.isAssignableFrom(deeplyClonedFieldClass)) { + var emptyInstance = bytecodeCreator + .invokeInterfaceMethod(MethodDescriptor.ofMethod(PlanningCloneable.class, "createNewInstance", + Object.class), bytecodeCreator.checkCast(toClone, PlanningCloneable.class)); + bytecodeCreator.assign(cloneCollection, + bytecodeCreator.checkCast(emptyInstance, + Collection.class)); + } else if (List.class.isAssignableFrom(deeplyClonedFieldClass)) { bytecodeCreator.assign(cloneCollection, bytecodeCreator.newInstance(MethodDescriptor.ofConstructor(ArrayList.class, int.class), size)); } else if (Set.class.isAssignableFrom(deeplyClonedFieldClass)) { @@ -696,33 +713,41 @@ private void writeDeepCloneMapInstructions(BytecodeCreator bytecodeCreator, Class deeplyClonedFieldClass, java.lang.reflect.Type type, ResultHandle toClone, AssignableResultHandle cloneResultHolder, ResultHandle createdCloneMap, SortedSet> deepClonedClassesSortedSet) { - Class holderClass = deeplyClonedFieldClass; - try { - holderClass.getConstructor(); - } catch (NoSuchMethodException e) { - if (LinkedHashMap.class.isAssignableFrom(holderClass)) { - holderClass = LinkedHashMap.class; - } else if (ConcurrentHashMap.class.isAssignableFrom(holderClass)) { - holderClass = ConcurrentHashMap.class; - } else { - // Default to LinkedHashMap - holderClass = LinkedHashMap.class; + ResultHandle cloneMap; + if (PlanningCloneable.class.isAssignableFrom(deeplyClonedFieldClass)) { + var emptyInstance = bytecodeCreator + .invokeInterfaceMethod(MethodDescriptor.ofMethod(PlanningCloneable.class, "createNewInstance", + Object.class), bytecodeCreator.checkCast(toClone, PlanningCloneable.class)); + cloneMap = bytecodeCreator.checkCast(emptyInstance, Map.class); + } else { + Class holderClass = deeplyClonedFieldClass; + try { + holderClass.getConstructor(); + } catch (NoSuchMethodException e) { + if (LinkedHashMap.class.isAssignableFrom(holderClass)) { + holderClass = LinkedHashMap.class; + } else if (ConcurrentHashMap.class.isAssignableFrom(holderClass)) { + holderClass = ConcurrentHashMap.class; + } else { + // Default to LinkedHashMap + holderClass = LinkedHashMap.class; + } + } + + ResultHandle size = + bytecodeCreator.invokeInterfaceMethod(MethodDescriptor.ofMethod(Map.class, "size", int.class), toClone); + try { + holderClass.getConstructor(int.class); + cloneMap = bytecodeCreator.newInstance(MethodDescriptor.ofConstructor(holderClass, int.class), size); + } catch (NoSuchMethodException e) { + cloneMap = bytecodeCreator.newInstance(MethodDescriptor.ofConstructor(holderClass)); } } - ResultHandle cloneMap; - ResultHandle size = - bytecodeCreator.invokeInterfaceMethod(MethodDescriptor.ofMethod(Map.class, "size", int.class), toClone); ResultHandle entrySet = bytecodeCreator .invokeInterfaceMethod(MethodDescriptor.ofMethod(Map.class, "entrySet", Set.class), toClone); ResultHandle iterator = bytecodeCreator .invokeInterfaceMethod(MethodDescriptor.ofMethod(Iterable.class, "iterator", Iterator.class), entrySet); - try { - holderClass.getConstructor(int.class); - cloneMap = bytecodeCreator.newInstance(MethodDescriptor.ofConstructor(holderClass, int.class), size); - } catch (NoSuchMethodException e) { - cloneMap = bytecodeCreator.newInstance(MethodDescriptor.ofConstructor(holderClass)); - } BytecodeCreator whileLoopBlock = bytecodeCreator.whileLoop(conditionBytecode -> { ResultHandle hasNext = conditionBytecode @@ -960,7 +985,15 @@ private void createDeepCloneHelperMethod(ClassCreator classCreator, toClone, cloneMap); - ResultHandle cloneObj = noCloneBranch.newInstance(MethodDescriptor.ofConstructor(entityClass)); + ResultHandle cloneObj; + if (PlanningCloneable.class.isAssignableFrom(entityClass)) { + cloneObj = noCloneBranch.invokeInterfaceMethod( + MethodDescriptor.ofMethod(PlanningCloneable.class, "createNewInstance", Object.class), + toClone); + cloneObj = noCloneBranch.checkCast(cloneObj, entityClass); + } else { + cloneObj = noCloneBranch.newInstance(MethodDescriptor.ofConstructor(entityClass)); + } noCloneBranch.invokeInterfaceMethod( MethodDescriptor.ofMethod(Map.class, "put", Object.class, Object.class, Object.class), cloneMap, toClone, cloneObj); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/cloner/AbstractSolutionClonerTest.java b/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/cloner/AbstractSolutionClonerTest.java index 00e3fa5259..2082729f5e 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/cloner/AbstractSolutionClonerTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/domain/solution/cloner/AbstractSolutionClonerTest.java @@ -40,6 +40,8 @@ import ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning.TestdataVariousTypes; import ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning.field.TestdataFieldAnnotatedDeepCloningEntity; import ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning.field.TestdataFieldAnnotatedDeepCloningSolution; +import ai.timefold.solver.core.impl.testdata.domain.clone.planning_cloneable.PlanningCloneableEntity; +import ai.timefold.solver.core.impl.testdata.domain.clone.planning_cloneable.PlanningCloneableSolution; import ai.timefold.solver.core.impl.testdata.domain.collection.TestdataArrayBasedEntity; import ai.timefold.solver.core.impl.testdata.domain.collection.TestdataArrayBasedSolution; import ai.timefold.solver.core.impl.testdata.domain.collection.TestdataEntityCollectionPropertyEntity; @@ -1119,4 +1121,48 @@ void cloneExtendedShadowEntities() { .isNotSameAs(original.shadowEntityList.get(0)); } + @Test + void clonePlanningCloneableItems() { + var solutionDescriptor = PlanningCloneableSolution.buildSolutionDescriptor(); + var cloner = createSolutionCloner(solutionDescriptor); + + var entityA = new PlanningCloneableEntity("A"); + var entityB = new PlanningCloneableEntity("B"); + var entityC = new PlanningCloneableEntity("C"); + + var original = new PlanningCloneableSolution(List.of(entityA, entityB, entityC)); + var clone = cloner.cloneSolution(original); + + assertThat(clone.entityList) + .hasSize(3) + .isNotSameAs(original.entityList) + .first() + .isNotNull(); + + assertThat(clone.entityList.get(0)) + .isNotSameAs(original.entityList.get(0)) + .hasFieldOrPropertyWithValue("code", "A"); + + assertThat(clone.entityList.get(1)) + .isNotSameAs(original.entityList.get(1)) + .hasFieldOrPropertyWithValue("code", "B"); + + assertThat(clone.entityList.get(2)) + .isNotSameAs(original.entityList.get(2)) + .hasFieldOrPropertyWithValue("code", "C"); + + assertThat(clone.codeToEntity) + .hasSize(3) + .isNotSameAs(original.codeToEntity); + + assertThat(clone.codeToEntity.get("A")) + .isSameAs(clone.entityList.get(0)); + + assertThat(clone.codeToEntity.get("B")) + .isSameAs(clone.entityList.get(1)); + + assertThat(clone.codeToEntity.get("C")) + .isSameAs(clone.entityList.get(2)); + } + } diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableEntity.java b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableEntity.java new file mode 100644 index 0000000000..fb7a202d83 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableEntity.java @@ -0,0 +1,26 @@ +package ai.timefold.solver.core.impl.testdata.domain.clone.planning_cloneable; + +import ai.timefold.solver.core.api.domain.entity.PlanningEntity; +import ai.timefold.solver.core.api.domain.variable.PlanningVariable; +import ai.timefold.solver.core.impl.domain.solution.cloner.PlanningCloneable; +import ai.timefold.solver.core.impl.testdata.domain.TestdataValue; + +@PlanningEntity +public class PlanningCloneableEntity implements PlanningCloneable { + public String code; + @PlanningVariable + public TestdataValue value; + + public PlanningCloneableEntity(String code) { + this.code = code; + } + + public String getCode() { + return code; + } + + @Override + public PlanningCloneableEntity createNewInstance() { + return new PlanningCloneableEntity(code); + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableList.java b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableList.java new file mode 100644 index 0000000000..172ba79ad7 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableList.java @@ -0,0 +1,45 @@ +package ai.timefold.solver.core.impl.testdata.domain.clone.planning_cloneable; + +import java.util.AbstractList; +import java.util.ArrayList; +import java.util.List; + +import ai.timefold.solver.core.impl.domain.solution.cloner.PlanningCloneable; + +public class PlanningCloneableList extends AbstractList implements PlanningCloneable> { + private final List backingList; + + public PlanningCloneableList() { + this.backingList = new ArrayList<>(); + } + + @Override + public PlanningCloneableList createNewInstance() { + return new PlanningCloneableList<>(); + } + + @Override + public T get(int i) { + return backingList.get(i); + } + + @Override + public T set(int i, T item) { + return backingList.set(i, item); + } + + @Override + public void add(int i, T item) { + backingList.add(i, item); + } + + @Override + public T remove(int i) { + return backingList.remove(i); + } + + @Override + public int size() { + return backingList.size(); + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableMap.java b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableMap.java new file mode 100644 index 0000000000..a736be09da --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableMap.java @@ -0,0 +1,31 @@ +package ai.timefold.solver.core.impl.testdata.domain.clone.planning_cloneable; + +import java.util.AbstractMap; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import ai.timefold.solver.core.impl.domain.solution.cloner.PlanningCloneable; + +public class PlanningCloneableMap extends AbstractMap implements PlanningCloneable> { + private final Map backingMap; + + public PlanningCloneableMap() { + this.backingMap = new HashMap<>(); + } + + @Override + public PlanningCloneableMap createNewInstance() { + return new PlanningCloneableMap<>(); + } + + @Override + public Set> entrySet() { + return backingMap.entrySet(); + } + + @Override + public V put(K key, V value) { + return backingMap.put(key, value); + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableSolution.java b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableSolution.java new file mode 100644 index 0000000000..56a9b8cfd6 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/testdata/domain/clone/planning_cloneable/PlanningCloneableSolution.java @@ -0,0 +1,44 @@ +package ai.timefold.solver.core.impl.testdata.domain.clone.planning_cloneable; + +import java.util.List; + +import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty; +import ai.timefold.solver.core.api.domain.solution.PlanningScore; +import ai.timefold.solver.core.api.domain.solution.PlanningSolution; +import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider; +import ai.timefold.solver.core.api.score.buildin.hardsoft.HardSoftScore; +import ai.timefold.solver.core.impl.domain.solution.cloner.PlanningCloneable; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.testdata.domain.TestdataValue; + +@PlanningSolution +public class PlanningCloneableSolution implements PlanningCloneable { + public static SolutionDescriptor buildSolutionDescriptor() { + return SolutionDescriptor.buildSolutionDescriptor(PlanningCloneableSolution.class, PlanningCloneableEntity.class); + } + + @PlanningEntityCollectionProperty + public PlanningCloneableList entityList; + @ValueRangeProvider + public PlanningCloneableList valueList; + public PlanningCloneableMap codeToEntity; + + @PlanningScore + public HardSoftScore score; + + public PlanningCloneableSolution(List entityList) { + this.entityList = new PlanningCloneableList<>(); + this.valueList = new PlanningCloneableList<>(); + this.codeToEntity = new PlanningCloneableMap<>(); + this.score = null; + for (PlanningCloneableEntity entity : entityList) { + this.entityList.add(entity); + this.codeToEntity.put(entity.getCode(), entity); + } + } + + @Override + public PlanningCloneableSolution createNewInstance() { + return new PlanningCloneableSolution(List.of()); + } +}