Skip to content

Commit 56c9783

Browse files
Christopher-ChianelliChristopher-Chianelli
authored
fix: Allow using declarative shadow variables on extended entities
- Getters will be preferred over fields unless the field is more specific than the getter - Made VariableMetaclasses check for equality via variable descriptors; previous it checked both the entity metaclass and the variable descriptors, which created two different canonical VariableMetaclasses or a variable on a base class that is extended. - Fix getting declared fields/getters
1 parent 903d028 commit 56c9783

File tree

29 files changed

+921
-232
lines changed

29 files changed

+921
-232
lines changed

‎core/src/main/java/ai/timefold/solver/core/impl/domain/common/ReflectionHelper.java

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -133,20 +133,22 @@ public static Method getGetterMethod(Class<?> containingClass, String propertyNa
133133
public static Method getDeclaredGetterMethod(Class<?> containingClass, String propertyName) {
134134
var capitalizedPropertyName = capitalizePropertyName(propertyName);
135135
var getterName = PROPERTY_ACCESSOR_PREFIX_GET + capitalizedPropertyName;
136-
try {
137-
return containingClass.getMethod(getterName);
138-
} catch (NoSuchMethodException e) {
139-
// intentionally empty
136+
var baseClass = containingClass;
137+
while (baseClass != null) {
138+
try {
139+
return containingClass.getDeclaredMethod(getterName);
140+
} catch (NoSuchMethodException e) {
141+
baseClass = baseClass.getSuperclass();
142+
}
140143
}
141144
var isserName = PROPERTY_ACCESSOR_PREFIX_IS + capitalizedPropertyName;
142-
try {
143-
144-
var method = containingClass.getDeclaredMethod(isserName);
145-
if (method.getReturnType() == boolean.class) {
146-
return method;
145+
baseClass = containingClass;
146+
while (baseClass != null) {
147+
try {
148+
return baseClass.getDeclaredMethod(isserName);
149+
} catch (NoSuchMethodException e) {
150+
baseClass = baseClass.getSuperclass();
147151
}
148-
} catch (NoSuchMethodException e) {
149-
// intentionally empty
150152
}
151153
return null;
152154
}
@@ -187,11 +189,15 @@ public static Field getField(Class<?> containingClass, String fieldName) {
187189
* @return sometimes null
188190
*/
189191
public static Field getDeclaredField(Class<?> containingClass, String fieldName) {
190-
try {
191-
return containingClass.getDeclaredField(fieldName);
192-
} catch (NoSuchFieldException e) {
193-
return null;
192+
var baseClass = containingClass;
193+
while (baseClass != null) {
194+
try {
195+
return baseClass.getDeclaredField(fieldName);
196+
} catch (NoSuchFieldException e) {
197+
baseClass = baseClass.getSuperclass();
198+
}
194199
}
200+
return null;
195201
}
196202

197203
/**

‎core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/DefaultPlanningListVariableMetaModel.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package ai.timefold.solver.core.impl.domain.solution.descriptor;
22

3+
import java.util.Objects;
4+
35
import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor;
46
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningEntityMetaModel;
57
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel;
@@ -30,6 +32,22 @@ public boolean allowsUnassignedValues() {
3032
return false;
3133
}
3234

35+
@Override
36+
public boolean equals(Object o) {
37+
// Do not use entity in equality checks;
38+
// If an entity is subclassed, that subclass will have it
39+
// own distinct VariableMetaModel
40+
if (o instanceof DefaultPlanningListVariableMetaModel<?, ?, ?> that) {
41+
return Objects.equals(variableDescriptor, that.variableDescriptor);
42+
}
43+
return false;
44+
}
45+
46+
@Override
47+
public int hashCode() {
48+
return Objects.hash(variableDescriptor);
49+
}
50+
3351
@Override
3452
public String toString() {
3553
return "Genuine List Variable '%s %s.%s' (allowsUnassignedValues: %b)"

‎core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/DefaultPlanningVariableMetaModel.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package ai.timefold.solver.core.impl.domain.solution.descriptor;
22

3+
import java.util.Objects;
4+
35
import ai.timefold.solver.core.impl.domain.variable.descriptor.BasicVariableDescriptor;
46
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningEntityMetaModel;
57
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningVariableMetaModel;
@@ -35,6 +37,22 @@ public boolean isChained() {
3537
return variableDescriptor.isChained();
3638
}
3739

40+
@Override
41+
public boolean equals(Object o) {
42+
// Do not use entity in equality checks;
43+
// If an entity is subclassed, that subclass will have it
44+
// own distinct VariableMetaModel
45+
if (o instanceof DefaultPlanningVariableMetaModel<?, ?, ?> that) {
46+
return Objects.equals(variableDescriptor, that.variableDescriptor);
47+
}
48+
return false;
49+
}
50+
51+
@Override
52+
public int hashCode() {
53+
return Objects.hash(variableDescriptor);
54+
}
55+
3856
@Override
3957
public String toString() {
4058
return "Genuine Variable '%s %s.%s' (allowsUnassigned: %b, isChained: %b)"

‎core/src/main/java/ai/timefold/solver/core/impl/domain/solution/descriptor/DefaultShadowVariableMetaModel.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package ai.timefold.solver.core.impl.domain.solution.descriptor;
22

3+
import java.util.Objects;
4+
35
import ai.timefold.solver.core.impl.domain.variable.descriptor.ShadowVariableDescriptor;
46
import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningEntityMetaModel;
57
import ai.timefold.solver.core.preview.api.domain.metamodel.ShadowVariableMetaModel;
@@ -30,10 +32,26 @@ public ShadowVariableDescriptor<Solution_> variableDescriptor() {
3032
return variableDescriptor;
3133
}
3234

35+
@Override
36+
public boolean equals(Object o) {
37+
// Do not use entity in equality checks;
38+
// If an entity is subclassed, that subclass will have it
39+
// own distinct VariableMetaModel
40+
if (o instanceof DefaultShadowVariableMetaModel<?, ?, ?> that) {
41+
return Objects.equals(variableDescriptor, that.variableDescriptor);
42+
}
43+
return false;
44+
}
45+
46+
@Override
47+
public int hashCode() {
48+
return Objects.hashCode(variableDescriptor);
49+
}
50+
3351
@Override
3452
public String toString() {
3553
return "Shadow Variable '%s %s.%s'"
36-
.formatted(type(), entity.getClass().getSimpleName(), name());
54+
.formatted(type(), entity.type().getSimpleName(), name());
3755
}
3856

3957
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,29 @@
11
package ai.timefold.solver.core.impl.domain.variable.declarative;
22

3-
import java.util.HashSet;
4-
import java.util.Set;
5-
63
import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor;
74
import ai.timefold.solver.core.impl.domain.variable.supply.Supply;
8-
import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel;
95

106
import org.jspecify.annotations.NullMarked;
11-
import org.jspecify.annotations.Nullable;
127

138
@NullMarked
149
public class DefaultShadowVariableSession<Solution_> implements Supply {
1510
final VariableReferenceGraph<Solution_> graph;
1611

17-
record EntityVariablePair(VariableMetaModel<?, ?, ?> variableMetamodel, Object entity) {
18-
// entity must be compared by identity; cannot rely on user's equals/hashCode.
19-
// variableMetamodel is guaranteed to be the same instance for the same variable.
20-
// this class is often used as a key for maps, so equals/hashCode are optimized for performance.
21-
@Override
22-
public boolean equals(@Nullable Object o) {
23-
if (o instanceof EntityVariablePair other) {
24-
return entity == other.entity && variableMetamodel == other.variableMetamodel;
25-
}
26-
return false;
27-
}
28-
29-
@Override
30-
public int hashCode() {
31-
return (31 * System.identityHashCode(entity)) ^ System.identityHashCode(variableMetamodel);
32-
}
33-
}
34-
35-
final Set<EntityVariablePair> modifiedEntityVariableSet = new HashSet<>();
36-
3712
public DefaultShadowVariableSession(VariableReferenceGraph<Solution_> graph) {
3813
this.graph = graph;
3914
}
4015

4116
public void beforeVariableChanged(VariableDescriptor<Solution_> variableDescriptor, Object entity) {
42-
if (modifiedEntityVariableSet.add(new EntityVariablePair(variableDescriptor.getVariableMetaModel(), entity))) {
43-
graph.beforeVariableChanged(variableDescriptor.getVariableMetaModel(),
44-
entity);
45-
}
17+
graph.beforeVariableChanged(variableDescriptor.getVariableMetaModel(),
18+
entity);
19+
}
20+
21+
public void afterVariableChanged(VariableDescriptor<Solution_> variableDescriptor, Object entity) {
22+
graph.afterVariableChanged(variableDescriptor.getVariableMetaModel(),
23+
entity);
4624
}
4725

4826
public void updateVariables() {
49-
for (var modifiedEntityVariable : modifiedEntityVariableSet) {
50-
graph.afterVariableChanged(
51-
modifiedEntityVariable.variableMetamodel,
52-
modifiedEntityVariable.entity);
53-
}
54-
modifiedEntityVariableSet.clear();
5527
graph.updateChanged();
5628
}
5729
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSessionFactory.java

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,21 @@ private static <Solution_> void createSourceChangeProcessors(VariableReferenceGr
104104
// non-declarative variables are not in the graph and must have their
105105
// own processor
106106
if (!sourcePart.isDeclarative()) {
107-
variableReferenceGraph.addAfterProcessor(toVariableId, (graph, entity) ->
108-
// Exploits the fact the source entity and the target entity must be the same,
109-
// since non-declarative variables can only be accessed from the root entity
110-
// i.e. paths like "otherVisit.previous"
111-
// or "visitGroup[].otherVisit.previous" are not allowed,
112-
// but paths like "previous" or
113-
// "visitGroup[].previous" are.
114-
// Without this invariant, an inverse set must be calculated
115-
// and maintained,
116-
// and this code is complicated enough.
117-
graph.markChanged(graph.lookup(fromVariableId, entity)));
107+
variableReferenceGraph.addAfterProcessor(toVariableId, (graph, entity) -> {
108+
// Exploits the fact the source entity and the target entity must be the same,
109+
// since non-declarative variables can only be accessed from the root entity
110+
// i.e. paths like "otherVisit.previous"
111+
// or "visitGroup[].otherVisit.previous" are not allowed,
112+
// but paths like "previous" or
113+
// "visitGroup[].previous" are.
114+
// Without this invariant, an inverse set must be calculated
115+
// and maintained,
116+
// and this code is complicated enough.
117+
var changed = graph.lookupOrNull(fromVariableId, entity);
118+
if (changed != null) {
119+
graph.markChanged(changed);
120+
}
121+
});
118122
}
119123
}
120124
}
@@ -131,14 +135,34 @@ private static <Solution_> void createAliasToVariableChangeProcessors(
131135
if (!alias.isDeclarative() && alias.affectGraphEdges()) {
132136
// Exploit the same fact as above
133137
variableReferenceGraph.addBeforeProcessor(sourceVariableId,
134-
(graph, toEntity) -> alias.targetEntityFunctionStartingFromVariableEntity().accept(toEntity,
135-
fromEntity -> graph.removeEdge(
136-
graph.lookup(fromVariableId, fromEntity),
137-
graph.lookup(toVariableId, toEntity))));
138+
(graph, toEntity) -> alias.targetEntityFunctionStartingFromVariableEntity()
139+
.accept(toEntity, fromEntity -> {
140+
// from/to can be null in extended models
141+
// ex: previous is used as a source, but only an extended class
142+
// has the to variable
143+
var from = graph.lookupOrNull(fromVariableId, fromEntity);
144+
if (from == null) {
145+
return;
146+
}
147+
var to = graph.lookupOrNull(toVariableId, toEntity);
148+
if (to == null) {
149+
return;
150+
}
151+
graph.removeEdge(from, to);
152+
}));
138153
variableReferenceGraph.addAfterProcessor(sourceVariableId,
139-
(graph, toEntity) -> alias.targetEntityFunctionStartingFromVariableEntity().accept(toEntity,
140-
fromEntity -> graph.addEdge(graph.lookup(fromVariableId, fromEntity),
141-
graph.lookup(toVariableId, toEntity))));
154+
(graph, toEntity) -> alias.targetEntityFunctionStartingFromVariableEntity()
155+
.accept(toEntity, fromEntity -> {
156+
var from = graph.lookupOrNull(fromVariableId, fromEntity);
157+
if (from == null) {
158+
return;
159+
}
160+
var to = graph.lookupOrNull(toVariableId, toEntity);
161+
if (to == null) {
162+
return;
163+
}
164+
graph.addEdge(from, to);
165+
}));
142166
}
143167
// Note: it is impossible to have a declarative variable affect graph edges,
144168
// since accessing a declarative variable from another declarative variable is prohibited.
@@ -161,9 +185,9 @@ private static <Solution_> void createFixedVariableRelationEdges(VariableReferen
161185
sourceRoot.valueEntityFunction()
162186
.accept(entity, fromEntity -> variableReferenceGraph.addFixedEdge(
163187
variableReferenceGraph
164-
.lookup(fromVariableId, fromEntity),
188+
.lookupOrError(fromVariableId, fromEntity),
165189
variableReferenceGraph
166-
.lookup(toVariableId, entity)));
190+
.lookupOrError(toVariableId, entity)));
167191
break;
168192
}
169193
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/EntityVariablePair.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel;
66

7+
import org.jspecify.annotations.NullMarked;
8+
9+
@NullMarked
710
public record EntityVariablePair(Object entity, VariableMetaModel<?, ?, ?> variableId,
811
VariableUpdaterInfo variableReference, int graphNodeId) {
912
@Override

0 commit comments

Comments
 (0)