Skip to content

Commit 18ecc54

Browse files
triceotriceo
committed
Refactor and add test coverage
# Conflicts: # core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java
1 parent 94344a3 commit 18ecc54

6 files changed

Lines changed: 127 additions & 95 deletions

File tree

‎core/src/main/java/ai/timefold/solver/core/impl/domain/entity/descriptor/EntityDescriptor.java‎

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package ai.timefold.solver.core.impl.domain.entity.descriptor;
22

3-
import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_GETTER_METHOD;
43
import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_GETTER_METHOD_WITH_SETTER;
54
import static ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory.MemberAccessorType.FIELD_OR_READ_METHOD;
65
import static ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptorValidator.assertNotMixedInheritance;
@@ -46,7 +45,6 @@
4645
import ai.timefold.solver.core.config.util.ConfigUtils;
4746
import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
4847
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
49-
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory;
5048
import ai.timefold.solver.core.impl.domain.policy.DescriptorPolicy;
5149
import ai.timefold.solver.core.impl.domain.solution.descriptor.ProblemScaleTracker;
5250
import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor;
@@ -323,44 +321,27 @@ private void processValueRangeProviderAnnotation(DescriptorPolicy descriptorPoli
323321
}
324322
}
325323

324+
@SuppressWarnings("unchecked")
326325
private void processPlanningVariableAnnotation(MutableInt variableDescriptorCounter, DescriptorPolicy descriptorPolicy,
327326
Member member) {
328-
var variableAnnotationClass = ConfigUtils.extractAnnotationClass(
329-
member, VARIABLE_ANNOTATION_CLASSES);
327+
var variableAnnotationClass = ConfigUtils.extractAnnotationClass(member, VARIABLE_ANNOTATION_CLASSES);
330328
if (variableAnnotationClass != null) {
331-
MemberAccessorFactory.MemberAccessorType memberAccessorType;
332-
if (variableAnnotationClass.equals(ShadowVariable.class)) {
333-
// Need to check only the single annotation version,
334-
// since supplier variable can only be used with a single
335-
// annotation.
336-
ShadowVariable annotation;
337-
if (member instanceof Field field) {
338-
annotation = field.getAnnotation(ShadowVariable.class);
339-
} else if (member instanceof Method method) {
340-
annotation = method.getAnnotation(ShadowVariable.class);
341-
} else {
342-
throw new IllegalStateException(
343-
"Member must be a field or a method, but was (%s).".formatted(member.getClass().getSimpleName()));
344-
}
345-
if (annotation == null) {
346-
throw new IllegalStateException("Impossible state: cannot get %s annotation on a %s annotated member (%s)."
347-
.formatted(ShadowVariable.class.getSimpleName(), ShadowVariable.class.getSimpleName(), member));
348-
}
349-
if (!annotation.supplierName().isEmpty()) {
350-
memberAccessorType = FIELD_OR_GETTER_METHOD_WITH_SETTER;
351-
} else {
352-
memberAccessorType = FIELD_OR_GETTER_METHOD;
353-
}
354-
} else if (variableAnnotationClass.equals(CustomShadowVariable.class)
355-
|| variableAnnotationClass.equals(ShadowVariable.List.class)
356-
|| variableAnnotationClass.equals(PiggybackShadowVariable.class)
357-
|| variableAnnotationClass.equals(CascadingUpdateShadowVariable.class)) {
358-
memberAccessorType = FIELD_OR_GETTER_METHOD;
329+
Object annotation;
330+
if (member instanceof Field field) {
331+
annotation = field.getAnnotation(variableAnnotationClass);
332+
} else if (member instanceof Method method) {
333+
annotation = method.getAnnotation(variableAnnotationClass);
359334
} else {
360-
memberAccessorType = FIELD_OR_GETTER_METHOD_WITH_SETTER;
335+
throw new IllegalStateException("Member must be a field or a method, but was (%s)."
336+
.formatted(member.getClass().getSimpleName()));
361337
}
362-
var memberAccessor = descriptorPolicy.getMemberAccessorFactory().buildAndCacheMemberAccessor(member,
363-
memberAccessorType, variableAnnotationClass, descriptorPolicy.getDomainAccessType());
338+
if (annotation == null) {
339+
throw new IllegalStateException("Impossible state: cannot get annotation on a %s-annotated member (%s)."
340+
.formatted(variableAnnotationClass, member));
341+
}
342+
var memberAccessor = descriptorPolicy.getMemberAccessorFactory()
343+
.buildAndCacheMemberAccessor(member, FIELD_OR_GETTER_METHOD_WITH_SETTER, variableAnnotationClass,
344+
descriptorPolicy.getDomainAccessType());
364345
registerVariableAccessor(variableDescriptorCounter.intValue(), variableAnnotationClass, memberAccessor);
365346
variableDescriptorCounter.increment();
366347
}
@@ -695,10 +676,6 @@ public GenuineVariableDescriptor<Solution_> getGenuineVariableDescriptor(String
695676
return shadowVariableLoopedDescriptor;
696677
}
697678

698-
public boolean hasAnyGenuineVariables() {
699-
return !effectiveGenuineVariableDescriptorMap.isEmpty();
700-
}
701-
702679
public boolean hasBothGenuineListAndBasicVariables() {
703680
if (!isGenuine()) {
704681
return false;
@@ -732,7 +709,7 @@ public boolean hasAnyGenuineListVariables() {
732709
}
733710

734711
public boolean isGenuine() {
735-
return hasAnyGenuineVariables();
712+
return !effectiveGenuineVariableDescriptorMap.isEmpty();
736713
}
737714

738715
public ListVariableDescriptor<Solution_> getGenuineListVariableDescriptor() {

‎core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java‎

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ private void cascadeUnassignedValues(
425425
var snapshot =
426426
ShadowVariablesAssert.takeSnapshot(scoreDirector.getSolutionDescriptor(), workingSolution);
427427

428-
forceTriggerAllVariableListeners(workingSolution);
428+
forceTriggerAllVariableListeners(workingSolution, false);
429429
return snapshot.createShadowVariablesViolationMessage(SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT);
430430
}
431431

@@ -439,9 +439,11 @@ private void cascadeUnassignedValues(
439439
* triggering listeners at this point must not change any shadow variables either.
440440
*
441441
* @param workingSolution working solution
442+
* @param resetShadowVariables true to set all shadow variable values to null
442443
*/
443-
public void forceTriggerAllVariableListeners(Solution_ workingSolution) {
444-
scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, this::simulateGenuineVariableChange);
444+
public void forceTriggerAllVariableListeners(Solution_ workingSolution, boolean resetShadowVariables) {
445+
scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution,
446+
e -> simulateGenuineVariableChange(e, resetShadowVariables));
445447
triggerVariableListenersInNotificationQueues();
446448
}
447449

@@ -454,12 +456,13 @@ public void clearAllVariableListenerEvents() {
454456
notificationQueuesAreEmpty = true;
455457
}
456458

457-
private void simulateGenuineVariableChange(Object entity) {
458-
var entityDescriptor = scoreDirector.getSolutionDescriptor()
459-
.findEntityDescriptorOrFail(entity.getClass());
460-
if (!entityDescriptor.isGenuine()) {
459+
private void simulateGenuineVariableChange(Object entity, boolean resetShadowVariables) {
460+
var solutionDescriptor = scoreDirector.getSolutionDescriptor();
461+
var entityDescriptor = solutionDescriptor.findEntityDescriptorOrFail(entity.getClass());
462+
if (!entityDescriptor.isGenuine() && !resetShadowVariables) {
461463
return;
462464
}
465+
463466
for (var variableDescriptor : entityDescriptor.getGenuineVariableDescriptorList()) {
464467
if (variableDescriptor.isListVariable()) {
465468
var descriptor = (ListVariableDescriptor<Solution_>) variableDescriptor;
@@ -471,6 +474,55 @@ private void simulateGenuineVariableChange(Object entity) {
471474
beforeVariableChanged(variableDescriptor, entity);
472475
}
473476
}
477+
478+
if (!resetShadowVariables) {
479+
return;
480+
}
481+
482+
var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors();
483+
if (shadowVariableDescriptors.isEmpty()) {
484+
return;
485+
}
486+
487+
for (var shadowVariableDescriptor : shadowVariableDescriptors) {
488+
clearShadowVariable(shadowVariableDescriptor, entity);
489+
}
490+
}
491+
492+
private static void clearShadowVariable(ShadowVariableDescriptor<?> shadowVariableDescriptor, Object entity) {
493+
var value = shadowVariableDescriptor.getValue(entity);
494+
if (value == null) {
495+
return;
496+
}
497+
498+
if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor
499+
&& value instanceof Collection<?> collection) {
500+
// Inverse collection must never be null, so just empty it.
501+
collection.clear();
502+
return;
503+
}
504+
505+
var propertyType = shadowVariableDescriptor.getVariablePropertyType();
506+
if (propertyType.isPrimitive()) {
507+
if (propertyType == boolean.class) {
508+
shadowVariableDescriptor.setValue(entity, false);
509+
} else if (propertyType == byte.class) {
510+
shadowVariableDescriptor.setValue(entity, (byte) 0);
511+
} else if (propertyType == char.class) {
512+
shadowVariableDescriptor.setValue(entity, '\u0000');
513+
} else if (propertyType == short.class) {
514+
shadowVariableDescriptor.setValue(entity, (short) 0);
515+
} else if (propertyType == int.class) {
516+
shadowVariableDescriptor.setValue(entity, 0);
517+
} else if (propertyType == long.class) {
518+
shadowVariableDescriptor.setValue(entity, 0L);
519+
} else {
520+
throw new IllegalStateException(
521+
"Impossible state: unknown primitive type %s".formatted(propertyType));
522+
}
523+
} else {
524+
shadowVariableDescriptor.setValue(entity, null);
525+
}
474526
}
475527

476528
public void assertNotificationQueuesAreEmpty() {

‎core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java‎

Lines changed: 7 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import static java.util.Objects.requireNonNull;
44

5-
import java.util.Collection;
65
import java.util.Collections;
76
import java.util.IdentityHashMap;
87
import java.util.LinkedHashSet;
@@ -28,7 +27,6 @@
2827
import ai.timefold.solver.core.impl.domain.variable.ListVariableStateSupply;
2928
import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor;
3029
import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor;
31-
import ai.timefold.solver.core.impl.domain.variable.inverserelation.InverseRelationShadowVariableDescriptor;
3230
import ai.timefold.solver.core.impl.domain.variable.listener.support.VariableListenerSupport;
3331
import ai.timefold.solver.core.impl.domain.variable.listener.support.violation.SolutionTracker;
3432
import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
@@ -360,52 +358,14 @@ protected void clearVariableListenerEvents() {
360358
}
361359

362360
@Override
363-
public void forceTriggerVariableListeners(boolean resetShadowVariablesFirst) {
361+
public void forceTriggerVariableListeners(boolean resetShadowVariables) {
364362
var solution = getWorkingSolution();
365-
if (resetShadowVariablesFirst) {
363+
if (resetShadowVariables) {
366364
var solutionDescriptor = getSolutionDescriptor();
367365
solutionDescriptor.visitAllEntities(solution, entity -> {
368-
var entityDescriptor = solutionDescriptor.findEntityDescriptor(entity.getClass());
369-
var shadowVariableDescriptors = entityDescriptor.getShadowVariableDescriptors();
370-
if (shadowVariableDescriptors.isEmpty()) {
371-
return;
372-
}
373-
for (var shadowVariableDescriptor : shadowVariableDescriptors) {
374-
var value = shadowVariableDescriptor.getValue(entity);
375-
if (value == null) {
376-
continue;
377-
}
378-
if (shadowVariableDescriptor instanceof InverseRelationShadowVariableDescriptor
379-
&& value instanceof Collection<?> collection) {
380-
// Inverse collection must never be null, so just empty it.
381-
collection.clear();
382-
continue;
383-
}
384-
var propertyType = shadowVariableDescriptor.getVariablePropertyType();
385-
if (propertyType.isPrimitive()) {
386-
if (propertyType == boolean.class) {
387-
shadowVariableDescriptor.setValue(entity, false);
388-
} else if (propertyType == byte.class) {
389-
shadowVariableDescriptor.setValue(entity, (byte) 0);
390-
} else if (propertyType == char.class) {
391-
shadowVariableDescriptor.setValue(entity, '\u0000');
392-
} else if (propertyType == short.class) {
393-
shadowVariableDescriptor.setValue(entity, (short) 0);
394-
} else if (propertyType == int.class) {
395-
shadowVariableDescriptor.setValue(entity, 0);
396-
} else if (propertyType == long.class) {
397-
shadowVariableDescriptor.setValue(entity, 0L);
398-
} else {
399-
throw new IllegalStateException(
400-
"Impossible state: unknown primitive type %s".formatted(propertyType));
401-
}
402-
} else {
403-
shadowVariableDescriptor.setValue(entity, null);
404-
}
405-
}
406366
});
407367
}
408-
variableListenerSupport.forceTriggerAllVariableListeners(solution);
368+
variableListenerSupport.forceTriggerAllVariableListeners(solution, resetShadowVariables);
409369
}
410370

411371
protected void setCalculatedScore(Score_ score) {
@@ -778,13 +738,13 @@ public void assertExpectedUndoMoveScore(Move<Solution_> move, InnerScore<Score_>
778738
// We cannot set all shadow variables to null, since some variable listeners
779739
// may expect them to be non-null.
780740
// Instead, we just simulate a change to all genuine variables.
781-
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution);
741+
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false);
782742
solutionTracker.setUndoFromScratchSolution(workingSolution);
783743

784744
// Also calculate from scratch for the before solution, since it might
785745
// have been corrupted but was only detected now
786746
solutionTracker.restoreBeforeSolution();
787-
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution);
747+
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false);
788748
solutionTracker.setBeforeFromScratchSolution(workingSolution);
789749

790750
corruptionDiagnosis = solutionTracker.buildScoreCorruptionMessage();
@@ -843,13 +803,13 @@ public SolutionTracker.SolutionCorruptionResult getSolutionCorruptionAfterUndo(M
843803
// We cannot set all shadow variables to null, since some variable listeners
844804
// may expect them to be non-null.
845805
// Instead, we just simulate a change to all genuine variables.
846-
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution);
806+
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false);
847807
solutionTracker.setUndoFromScratchSolution(workingSolution);
848808

849809
// Also calculate from scratch for the before solution, since it might
850810
// have been corrupted but was only detected now
851811
solutionTracker.restoreBeforeSolution();
852-
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution);
812+
variableListenerSupport.forceTriggerAllVariableListeners(workingSolution, false);
853813
solutionTracker.setBeforeFromScratchSolution(workingSolution);
854814

855815
return solutionTracker.buildSolutionCorruptionResult();

‎core/src/main/java/ai/timefold/solver/core/impl/score/stream/bavet/BavetConstraintFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public <Stream_ extends BavetAbstractConstraintStream<Solution_>> Stream_ share(
135135
public @NonNull <A> UniConstraintStream<A> from(@NonNull Class<A> fromClass) {
136136
assertValidFromType(fromClass);
137137
var entityDescriptor = solutionDescriptor.findEntityDescriptor(fromClass);
138-
if (entityDescriptor != null && entityDescriptor.hasAnyGenuineVariables()) {
138+
if (entityDescriptor != null && entityDescriptor.isGenuine()) {
139139
var predicate = (Predicate<A>) entityDescriptor.getIsInitializedPredicate();
140140
return share(new BavetForEachUniConstraintStream<>(this, fromClass, predicate, RetrievalSemantics.LEGACY));
141141
} else {

‎core/src/test/java/ai/timefold/solver/core/api/solver/SolutionManagerTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package ai.timefold.solver.core.api.solver;
22

3+
import static ai.timefold.solver.core.api.solver.SolutionUpdatePolicy.RESET_SHADOW_VARIABLES_ONLY;
34
import static org.assertj.core.api.Assertions.assertThat;
45
import static org.assertj.core.api.Assertions.assertThatThrownBy;
56
import static org.assertj.core.api.SoftAssertions.assertSoftly;
@@ -118,6 +119,14 @@ void updateEverything(SolutionManagerSource SolutionManagerSource) {
118119
softly.assertThat(solution.getScore()).isNotNull();
119120
softly.assertThat(solution.getEntityList().get(0).getFirstShadow()).isNotNull();
120121
});
122+
123+
var oldScore = solution.getScore();
124+
solutionManager.update(solution, RESET_SHADOW_VARIABLES_ONLY);
125+
126+
assertSoftly(softly -> {
127+
softly.assertThat(solution.getScore()).isSameAs(oldScore);
128+
softly.assertThat(solution.getEntityList().get(0).getFirstShadow()).isNotNull();
129+
});
121130
}
122131

123132
@ParameterizedTest
@@ -162,6 +171,22 @@ void updateEverythingChained(SolutionManagerSource SolutionManagerSource) {
162171
softly.assertThat(b2.getNextEntity()).isNull();
163172
softly.assertThat(c0.getNextEntity()).isNull();
164173
});
174+
175+
var oldScore = solution.getScore();
176+
solutionManager.update(solution, RESET_SHADOW_VARIABLES_ONLY);
177+
178+
assertSoftly(softly -> {
179+
softly.assertThat(solution.getScore()).isSameAs(oldScore);
180+
softly.assertThat(a0.getNextEntity()).isEqualTo(a1);
181+
softly.assertThat(a1.getAnchor()).isEqualTo(a0);
182+
softly.assertThat(a1.getNextEntity()).isNull();
183+
softly.assertThat(b0.getNextEntity()).isEqualTo(b1);
184+
softly.assertThat(b1.getAnchor()).isEqualTo(b0);
185+
softly.assertThat(b1.getNextEntity()).isEqualTo(b2);
186+
softly.assertThat(b2.getAnchor()).isEqualTo(b0);
187+
softly.assertThat(b2.getNextEntity()).isNull();
188+
softly.assertThat(c0.getNextEntity()).isNull();
189+
});
165190
}
166191

167192
@ParameterizedTest
@@ -204,6 +229,20 @@ void updateEverythingList(SolutionManagerSource SolutionManagerSource) {
204229
assertShadowedListValue(softly, b2, b, 2, b1, null);
205230
assertShadowedListValue(softly, c0, c, 0, null, null);
206231
});
232+
233+
var oldScore = solution.getScore();
234+
solutionManager.update(solution, RESET_SHADOW_VARIABLES_ONLY);
235+
236+
assertSoftly(softly -> {
237+
softly.assertThat(solution.getScore()).isSameAs(oldScore);
238+
assertShadowedListValue(softly, a0, a, 0, null, a1);
239+
assertShadowedListValue(softly, a1, a, 1, a0, null);
240+
assertShadowedListValue(softly, b0, b, 0, null, b1);
241+
assertShadowedListValue(softly, b1, b, 1, b0, b2);
242+
assertShadowedListValue(softly, b2, b, 2, b1, null);
243+
assertShadowedListValue(softly, c0, c, 0, null, null);
244+
});
245+
207246
}
208247

209248
private void assertShadowedListValueAllNull(SoftAssertions softly, TestdataListValueWithShadowHistory current) {

0 commit comments

Comments
 (0)