Skip to content

Commit

Permalink
fix: Recurse through @DeepPlanningCloned classes to discover all @Dee…
Browse files Browse the repository at this point in the history
…pPlanningCloned classes (#1237)

The `GizmoSolutionClonerImplementor` requires a set of all
`@DeepPlanningCloned` classes so it can create cloners for all of them.
`GizmoCloningUtils.getDeepClonedClasses` incorrectly assume all
`@DeepPlanningCloned classes` are directly accessible from either
the `@PlanningSolution` class or an `@PlanningEntity` class. However,
a `@DeepPlanningCloned` class might be referenced from another
`@DeepPlanningCloned` class, which would not be detected.

In order to detect all `@DeepPlanningCloned` classes, when
a class that is `@DeepPlanningCloned` is found and was not
processed yet, it is added to the to process queue.

Fixes #1208.
  • Loading branch information
Christopher-Chianelli authored Nov 29, 2024
1 parent ee760ca commit 28201ef
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,32 @@ public static Set<Class<?>> getDeepClonedClasses(SolutionDescriptor<?> solutionD
Set<Class<?>> classesToProcess = new LinkedHashSet<>(solutionDescriptor.getEntityClassSet());
classesToProcess.add(solutionDescriptor.getSolutionClass());
classesToProcess.addAll(entitySubclasses);
for (Class<?> clazz : classesToProcess) {

// deepClonedClassSet contains all processed classes so far,
// so we can use it to determine if we need to add a class
// to the classesToProcess set.
//
// This is important, since SolverConfig does not contain
// info on @DeepPlanningCloned classes, so we need to discover
// them from the domain classes, possibly recursively
// (when a @DeepPlanningCloned class reference another @DeepPlanningCloned
// that is not referenced by any entity).
while (!classesToProcess.isEmpty()) {
var clazz = classesToProcess.iterator().next();
classesToProcess.remove(clazz);
deepClonedClassSet.add(clazz);
for (Field field : getAllFields(clazz)) {
deepClonedClassSet.addAll(getDeepClonedTypeArguments(solutionDescriptor, field.getGenericType()));
var deepClonedTypeArguments = getDeepClonedTypeArguments(solutionDescriptor, field.getGenericType());
for (var deepClonedTypeArgument : deepClonedTypeArguments) {
if (!deepClonedClassSet.contains(deepClonedTypeArgument)) {
classesToProcess.add(deepClonedTypeArgument);
deepClonedClassSet.add(deepClonedTypeArgument);
}
}
if (DeepCloningUtils.isFieldDeepCloned(solutionDescriptor, field, clazz)
&& !PlanningCloneable.class.isAssignableFrom(field.getType())) {
&& !PlanningCloneable.class.isAssignableFrom(field.getType())
&& !deepClonedClassSet.contains(field.getType())) {
classesToProcess.add(field.getType());
deepClonedClassSet.add(field.getType());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -121,6 +122,10 @@ public static void defineClonerFor(ClassCreator classCreator,
memoizedSolutionOrEntityDescriptorMap, deepClonedClassSet);
}

public static boolean isCloneableClass(Class<?> clazz) {
return !clazz.isInterface() && !Modifier.isAbstract(clazz.getModifiers());
}

/**
* Generates the constructor and implementations of SolutionCloner
* methods for the given SolutionDescriptor using the given ClassCreator
Expand All @@ -139,7 +144,7 @@ public static void defineClonerFor(Supplier<GizmoSolutionClonerImplementor> impl
Set<Class<?>> deepCloneClassesThatAreNotSolutionSet =
deepClonedClassSet.stream()
.filter(clazz -> !solutionClassSet.contains(clazz) && !clazz.isArray())
.filter(clazz -> !clazz.isInterface() && !Modifier.isAbstract(clazz.getModifiers()))
.filter(GizmoSolutionClonerImplementor::isCloneableClass)
.collect(Collectors.toSet());

Comparator<Class<?>> instanceOfComparator = getInstanceOfComparator(deepClonedClassSet);
Expand All @@ -163,7 +168,7 @@ public static void defineClonerFor(Supplier<GizmoSolutionClonerImplementor> impl
Set<Class<?>> abstractDeepCloneClassSet =
deepClonedClassSet.stream()
.filter(clazz -> !solutionClassSet.contains(clazz) && !clazz.isArray())
.filter(clazz -> clazz.isInterface() || Modifier.isAbstract(clazz.getModifiers()))
.filter(Predicate.not(GizmoSolutionClonerImplementor::isCloneableClass))
.collect(Collectors.toSet());

for (Class<?> abstractDeepClonedClass : abstractDeepCloneClassSet) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,8 @@ private void assertTestdataVariousTypes(TestdataVariousTypes original, TestdataV
softly.assertThat(cloned.deepClonedListRef)
.first()
.isSameAs(original.deepClonedListRef.get(0));
softly.assertThat(cloned.extraDeepClonedObject).isNotSameAs(original.extraDeepClonedObject);
softly.assertThat(cloned.extraDeepClonedObject.id).isSameAs(original.extraDeepClonedObject.id);
});
assertSoftly(softly -> {
softly.assertThat(cloned.shallowClonedListRef).isSameAs(original.shallowClonedListRef);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package ai.timefold.solver.core.impl.domain.solution.cloner.gizmo;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;
import java.util.Map;

import ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning.AnnotatedTestdataVariousTypes;
import ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning.ExtraDeepClonedObject;
import ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning.TestdataDeepCloningEntity;
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 org.junit.jupiter.api.Test;

class GizmoCloningUtilsTest {

@Test
void getDeepClonedClasses() {
assertThat(GizmoCloningUtils.getDeepClonedClasses(
TestdataFieldAnnotatedDeepCloningSolution.buildSolutionDescriptor(),
List.of(TestdataDeepCloningEntity.class)))
.containsExactlyInAnyOrder(
TestdataDeepCloningEntity.class,
ExtraDeepClonedObject.class,
TestdataFieldAnnotatedDeepCloningEntity.class,
AnnotatedTestdataVariousTypes.class,
TestdataFieldAnnotatedDeepCloningSolution.class,
TestdataVariousTypes.class,
List.class,
Map.class);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package ai.timefold.solver.core.impl.testdata.domain.clone.deepcloning;

import ai.timefold.solver.core.api.domain.solution.cloner.DeepPlanningClone;

@DeepPlanningClone
public class ExtraDeepClonedObject {
public String id;

public ExtraDeepClonedObject() {
}

public ExtraDeepClonedObject(String id) {
this.id = id;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public class TestdataVariousTypes {
@DeepPlanningClone
public List<String> deepClonedListRef = Collections.singletonList(stringRef);

// And a DeepPlanningCloned object
public ExtraDeepClonedObject extraDeepClonedObject = new ExtraDeepClonedObject("extra");

public record NonClonedRecord(int a, String b) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import ai.timefold.solver.core.api.domain.constraintweight.ConstraintWeight;
import ai.timefold.solver.core.api.domain.entity.PlanningEntity;
import ai.timefold.solver.core.api.domain.entity.PlanningPin;
import ai.timefold.solver.core.api.domain.entity.PlanningPinToIndex;
import ai.timefold.solver.core.api.domain.lookup.PlanningId;
import ai.timefold.solver.core.api.domain.solution.ConstraintWeightOverrides;
import ai.timefold.solver.core.api.domain.solution.PlanningEntityCollectionProperty;
Expand Down Expand Up @@ -55,6 +56,7 @@ public final class DotNames {

static final DotName PLANNING_ENTITY = DotName.createSimple(PlanningEntity.class.getName());
static final DotName PLANNING_PIN = DotName.createSimple(PlanningPin.class.getName());
static final DotName PLANNING_PIN_TO_INDEX = DotName.createSimple(PlanningPinToIndex.class.getName());
static final DotName PLANNING_ID = DotName.createSimple(PlanningId.class.getName());

static final DotName PLANNING_VARIABLE = DotName.createSimple(PlanningVariable.class.getName());
Expand All @@ -79,6 +81,7 @@ public final class DotNames {

static final DotName[] PLANNING_ENTITY_FIELD_ANNOTATIONS = {
PLANNING_PIN,
PLANNING_PIN_TO_INDEX,
PLANNING_VARIABLE,
PLANNING_LIST_VARIABLE,
ANCHOR_SHADOW_VARIABLE,
Expand All @@ -101,6 +104,7 @@ public final class DotNames {
CONSTRAINT_CONFIGURATION_PROVIDER,
CONSTRAINT_WEIGHT,
PLANNING_PIN,
PLANNING_PIN_TO_INDEX,
PLANNING_ID,
PLANNING_VARIABLE,
PLANNING_LIST_VARIABLE,
Expand Down

0 comments on commit 28201ef

Please sign in to comment.