diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java index 4d8b4c94ceb..e6682206b4f 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/AffectedEntitiesUpdater.java @@ -118,13 +118,18 @@ private boolean updateShadowVariable(EntityVariablePair entityVariabl var entity = entityVariable.entity(); var shadowVariableReference = entityVariable.variableReference(); var oldValue = shadowVariableReference.memberAccessor().executeGetter(entity); + var loopDescriptor = shadowVariableReference.shadowVariableLoopedDescriptor(); + if (loopDescriptor != null) { + var oldLooped = (boolean) loopDescriptor.getValue(entity); + if (oldLooped != isLooped) { + // Loop status change; add to affected entities + affectedEntities.add(entityVariable); + } + } if (isLooped) { - // null might be a valid value, and thus it could be the case - // that is was not looped and null, then turned to looped and null, - // which is still considered a change. - affectedEntities.add(entityVariable); if (oldValue != null) { + affectedEntities.add(entityVariable); changeShadowVariableAndNotify(shadowVariableReference, entity, null); } return true; diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java index 56dac46cfdb..b49b7523227 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/declarative/DefaultShadowVariableSession.java @@ -2,12 +2,12 @@ import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor; import ai.timefold.solver.core.impl.domain.variable.supply.Supply; +import ai.timefold.solver.core.preview.api.domain.metamodel.VariableMetaModel; import org.jspecify.annotations.NullMarked; @NullMarked -public class DefaultShadowVariableSession implements Supply { - +public final class DefaultShadowVariableSession implements Supply { final VariableReferenceGraph graph; public DefaultShadowVariableSession(VariableReferenceGraph graph) { @@ -15,12 +15,22 @@ public DefaultShadowVariableSession(VariableReferenceGraph graph) { } public void beforeVariableChanged(VariableDescriptor variableDescriptor, Object entity) { - graph.beforeVariableChanged(variableDescriptor.getVariableMetaModel(), + beforeVariableChanged(variableDescriptor.getVariableMetaModel(), entity); } public void afterVariableChanged(VariableDescriptor variableDescriptor, Object entity) { - graph.afterVariableChanged(variableDescriptor.getVariableMetaModel(), + afterVariableChanged(variableDescriptor.getVariableMetaModel(), + entity); + } + + public void beforeVariableChanged(VariableMetaModel variableMetaModel, Object entity) { + graph.beforeVariableChanged(variableMetaModel, + entity); + } + + public void afterVariableChanged(VariableMetaModel variableMetaModel, Object entity) { + graph.afterVariableChanged(variableMetaModel, entity); } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java index 44b3c4dcf82..a3f58705949 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/VariableListenerSupport.java @@ -354,6 +354,12 @@ public void triggerVariableListenersInNotificationQueues() { } if (shadowVariableSession != null) { shadowVariableSession.updateVariables(); + // Some internal variable listeners (such as those used + // to check for solution corruption) might have a declarative + // shadow variable as a source and need to be triggered here. + for (var notifiable : notifiableRegistry.getAll()) { + notifiable.triggerAllNotifications(); + } } notificationQueuesAreEmpty = true; } diff --git a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/SolutionTracker.java b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/SolutionTracker.java index 0e3fccd400f..2d571af3eaf 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/SolutionTracker.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/SolutionTracker.java @@ -111,9 +111,15 @@ private List getEntitiesMissingBeforeAfterEvents(VariableSnapshotTotal List getVariableChangedViolations( diff --git a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java index 67cf7587fcb..eb37224e721 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/score/director/AbstractScoreDirector.java @@ -781,6 +781,36 @@ that could cause the scoreDifference (%s).""" } } + public SolutionTracker.SolutionCorruptionResult getSolutionCorruptionAfterUndo(Move move, + InnerScore undoInnerScore) { + var trackingWorkingSolution = solutionTracker != null; + if (trackingWorkingSolution) { + solutionTracker.setAfterUndoSolution(workingSolution); + } + // Precondition: assert that there are probably no corrupted constraints + var undoMoveToString = "Undo(%s)".formatted(move); + assertWorkingScoreFromScratch(undoInnerScore, undoMoveToString); + // Precondition: assert that shadow variables aren't stale after doing the undoMove + assertShadowVariablesAreNotStale(undoInnerScore, undoMoveToString); + if (trackingWorkingSolution) { + // Recalculate all shadow variables from scratch. + // We cannot set all shadow variables to null, since some variable listeners + // may expect them to be non-null. + // Instead, we just simulate a change to all genuine variables. + variableListenerSupport.forceTriggerAllVariableListeners(workingSolution); + solutionTracker.setUndoFromScratchSolution(workingSolution); + + // Also calculate from scratch for the before solution, since it might + // have been corrupted but was only detected now + solutionTracker.restoreBeforeSolution(); + variableListenerSupport.forceTriggerAllVariableListeners(workingSolution); + solutionTracker.setBeforeFromScratchSolution(workingSolution); + + return solutionTracker.buildSolutionCorruptionResult(); + } + return SolutionTracker.SolutionCorruptionResult.untracked(); + } + /** * @param uncorruptedScoreDirector never null * @param predicted true if the score was predicted and might have been calculated on another thread diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java new file mode 100644 index 00000000000..80c3248878e --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirector.java @@ -0,0 +1,47 @@ +package ai.timefold.solver.core.impl.solver; + +import java.util.Map; + +import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.api.score.constraint.ConstraintMatchTotal; +import ai.timefold.solver.core.api.score.constraint.Indictment; +import ai.timefold.solver.core.impl.score.constraint.ConstraintMatchPolicy; +import ai.timefold.solver.core.impl.score.director.AbstractScoreDirector; +import ai.timefold.solver.core.impl.score.director.InnerScore; + +public class MoveAssertScoreDirector> + extends AbstractScoreDirector> { + + protected MoveAssertScoreDirector(MoveAssertScoreDirectorFactory scoreDirectorFactory, + boolean lookUpEnabled, + ConstraintMatchPolicy constraintMatchPolicy, + boolean expectShadowVariablesInCorrectState) { + super(scoreDirectorFactory, lookUpEnabled, constraintMatchPolicy, expectShadowVariablesInCorrectState); + } + + @Override + public void setWorkingSolution(Solution_ workingSolution) { + super.setWorkingSolution(workingSolution, ignored -> { + }); + } + + @Override + public InnerScore calculateScore() { + return InnerScore.fullyAssigned(scoreDirectorFactory.getScoreDefinition().getZeroScore()); + } + + @Override + public Map> getConstraintMatchTotalMap() { + return Map.of(); + } + + @Override + public Map> getIndictmentMap() { + return Map.of(); + } + + @Override + public boolean requiresFlushing() { + return false; + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirectorFactory.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirectorFactory.java new file mode 100644 index 00000000000..ce0280eb8dc --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAssertScoreDirectorFactory.java @@ -0,0 +1,38 @@ +package ai.timefold.solver.core.impl.solver; + +import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.score.director.AbstractScoreDirector; +import ai.timefold.solver.core.impl.score.director.AbstractScoreDirectorFactory; + +import org.jspecify.annotations.NullMarked; + +@NullMarked +public class MoveAssertScoreDirectorFactory> + extends AbstractScoreDirectorFactory> { + public MoveAssertScoreDirectorFactory( + SolutionDescriptor solutionDescriptor) { + super(solutionDescriptor); + } + + @Override + public AbstractScoreDirector.AbstractScoreDirectorBuilder createScoreDirectorBuilder() { + return new MoveAssertScoreDirectorBuilder(this); + } + + public class MoveAssertScoreDirectorBuilder extends + AbstractScoreDirector.AbstractScoreDirectorBuilder, MoveAssertScoreDirectorBuilder> { + + protected MoveAssertScoreDirectorBuilder(MoveAssertScoreDirectorFactory scoreDirectorFactory) { + super(scoreDirectorFactory); + } + + @Override + public MoveAssertScoreDirector build() { + return new MoveAssertScoreDirector<>(scoreDirectorFactory, + lookUpEnabled, + constraintMatchPolicy, + expectShadowVariablesInCorrectState); + } + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAsserter.java b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAsserter.java new file mode 100644 index 00000000000..e33c18baa60 --- /dev/null +++ b/core/src/test/java/ai/timefold/solver/core/impl/solver/MoveAsserter.java @@ -0,0 +1,37 @@ +package ai.timefold.solver.core.impl.solver; + +import ai.timefold.solver.core.api.score.Score; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.score.director.InnerScore; +import ai.timefold.solver.core.preview.api.move.Move; + +public class MoveAsserter { + private final SolutionDescriptor solutionDescriptor; + + private MoveAsserter(SolutionDescriptor solutionDescriptor) { + this.solutionDescriptor = solutionDescriptor; + } + + public static MoveAsserter create(SolutionDescriptor solutionDescriptor) { + return new MoveAsserter<>(solutionDescriptor); + } + + public void assertMove(Solution_ solution, Move move) { + var scoreDirectorFactory = new MoveAssertScoreDirectorFactory<>(solutionDescriptor); + scoreDirectorFactory.setTrackingWorkingSolution(true); + try (var scoreDirector = scoreDirectorFactory.createScoreDirectorBuilder() + .withLookUpEnabled(false) + .buildDerived()) { + var innerScore = InnerScore.fullyAssigned((Score) scoreDirector.getScoreDefinition().getZeroScore()); + scoreDirector.setWorkingSolution(solution); + scoreDirector.executeTemporaryMove(move, true); + var corruptionResult = scoreDirector.getSolutionCorruptionAfterUndo(move, innerScore); + if (corruptionResult.isCorrupted()) { + throw new AssertionError(""" + Solution corruption caused by move (%s) or its undo. + Analysis: + %s""".formatted(move, corruptionResult.message())); + } + } + } +} diff --git a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java index 526cd35a007..4172149629d 100644 --- a/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java +++ b/core/src/test/java/ai/timefold/solver/core/preview/api/variable/declarative/dependent/DependencyValuesShadowVariableTest.java @@ -6,12 +6,17 @@ import java.time.LocalDateTime; import java.time.ZoneOffset; import java.util.List; +import java.util.Set; import ai.timefold.solver.core.api.solver.SolverFactory; import ai.timefold.solver.core.config.solver.EnvironmentMode; import ai.timefold.solver.core.config.solver.PreviewFeature; import ai.timefold.solver.core.config.solver.SolverConfig; import ai.timefold.solver.core.config.solver.termination.TerminationConfig; +import ai.timefold.solver.core.impl.domain.solution.descriptor.SolutionDescriptor; +import ai.timefold.solver.core.impl.move.streams.generic.move.ListAssignMove; +import ai.timefold.solver.core.impl.solver.MoveAsserter; +import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel; import ai.timefold.solver.core.testdomain.declarative.dependency.TestdataDependencyConstraintProvider; import ai.timefold.solver.core.testdomain.declarative.dependency.TestdataDependencyEntity; import ai.timefold.solver.core.testdomain.declarative.dependency.TestdataDependencySolution; @@ -69,4 +74,37 @@ void testSolve() { } } } + + @Test + void testLoopStatusOfEntityIsUpdatedEvenIfNoVariablesOnTheEntityChanged() { + var baseTime = LocalDateTime.ofEpochSecond(0, 0, ZoneOffset.UTC); + var entityA = new TestdataDependencyEntity(baseTime); + var entityB = new TestdataDependencyEntity(baseTime); + var entityC = new TestdataDependencyEntity(baseTime); + + var valueA = new TestdataDependencyValue("A", Duration.ofHours(5), null); + var valueB = new TestdataDependencyValue("B", Duration.ofHours(6), null); + var valueC = new TestdataDependencyValue("C", Duration.ofHours(7), List.of(valueA, valueB)); + + var schedule = new TestdataDependencySolution( + List.of(entityA, entityB, entityC), + List.of(valueA, valueB, valueC)); + + entityA.getValues().add(valueB); + entityA.getValues().add(valueA); + + var solutionDescriptor = SolutionDescriptor.buildSolutionDescriptor(Set.of(PreviewFeature.DECLARATIVE_SHADOW_VARIABLES), + TestdataDependencySolution.class, TestdataDependencyEntity.class, TestdataDependencyValue.class); + var moveAsserter = MoveAsserter.create(solutionDescriptor); + + // Tests the move [A, B] -> [C, A, B]. + // Since C depends on A and B, this is an invalid solution, + // and C.startTime/C.endTime remains null and C.isLooped is true. + // When the move is undone, C.startTime/C.endTime remains null, + // and C.isLooped is false. + moveAsserter.assertMove(schedule, new ListAssignMove<>( + (PlanningListVariableMetaModel) solutionDescriptor + .getListVariableDescriptor().getVariableMetaModel(), + valueC, entityA, 0)); + } }