feat: Report missing before/after calls on undo corruption (#433)

Co-authored-by: Lukáš Petrovický <[email protected]>

Author: Christopher-Chianelli

benchmark/src/main/resources/benchmark.xsd

@@ -2444,6 +2444,9 @@
     <xs:restriction base="xs:string">
 
 
+      <xs:enumeration value="TRACKED_FULL_ASSERT"/>
+                  
+      
       <xs:enumeration value="FULL_ASSERT"/>
 
 

core/core-impl/src/main/java/ai/timefold/solver/core/config/solver/EnvironmentMode.java

@@ -19,6 +19,23 @@
  */
 @XmlEnum
 public enum EnvironmentMode {
+    /**
+     * This mode turns on {@link #FULL_ASSERT} and enables variable tracking to fail-fast on a bug in a {@link Move}
+     * implementation,
+     * a constraint, the engine itself or something else at the highest performance cost.
+     * <p>
+     * Because it tracks genuine and shadow variables, it is able to report precisely what variables caused the corruption and
+     * report any missed {@link ai.timefold.solver.core.api.domain.variable.VariableListener} events.
+     * <p>
+     * This mode is reproducible (see {@link #REPRODUCIBLE} mode).
+     * <p>
+     * This mode is intrusive because it calls the {@link InnerScoreDirector#calculateScore()} more frequently than a non assert
+     * mode.
+     * <p>
+     * This mode is by far the slowest of all the modes.
+     */
+    TRACKED_FULL_ASSERT(true),
+
     /**
      * This mode turns on all assertions
      * to fail-fast on a bug in a {@link Move} implementation, a constraint, the engine itself or something else
@@ -111,4 +128,7 @@ public boolean isReproducible() {
         return this != NON_REPRODUCIBLE;
     }
 
+    public boolean isTracking() {
+        return this == TRACKED_FULL_ASSERT;
+    }
 }

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

@@ -236,6 +236,15 @@ public void forceTriggerAllVariableListeners(Solution_ workingSolution) {
         triggerVariableListenersInNotificationQueues();
     }
 
+    /**
+     * Sets all shadow variables to null and then force triggers all variable listeners.
+     * This effectively recalculates the shadow variables from scratch.
+     */
+    public void recalculateAllShadowVariablesFromScratch(Solution_ workingSolution) {
+        scoreDirector.getSolutionDescriptor().visitAllEntities(workingSolution, this::resetShadowVariables);
+        forceTriggerAllVariableListeners(workingSolution);
+    }
+
     private void simulateGenuineVariableChange(Object entity) {
         var entityDescriptor = scoreDirector.getSolutionDescriptor()
                 .findEntityDescriptorOrFail(entity.getClass());
@@ -255,6 +264,17 @@ private void simulateGenuineVariableChange(Object entity) {
         }
     }
 
+    private void resetShadowVariables(Object entity) {
+        var entityDescriptor = scoreDirector.getSolutionDescriptor()
+                .findEntityDescriptorOrFail(entity.getClass());
+
+        for (var variableDescriptor : entityDescriptor.getShadowVariableDescriptors()) {
+            if (!variableDescriptor.getVariablePropertyType().isPrimitive()) {
+                variableDescriptor.setValue(entity, null);
+            }
+        }
+    }
+
     public void assertNotificationQueuesAreEmpty() {
         if (!notificationQueuesAreEmpty) {
             throw new IllegalStateException("The notificationQueues might not be empty (" + notificationQueuesAreEmpty

core/core-impl/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/ListVariableTracker.java

@@ -0,0 +1,118 @@
+package ai.timefold.solver.core.impl.domain.variable.listener.support.violation;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import ai.timefold.solver.core.api.domain.variable.ListVariableListener;
+import ai.timefold.solver.core.api.score.director.ScoreDirector;
+import ai.timefold.solver.core.impl.domain.variable.descriptor.ListVariableDescriptor;
+import ai.timefold.solver.core.impl.domain.variable.descriptor.VariableDescriptor;
+import ai.timefold.solver.core.impl.domain.variable.listener.SourcedVariableListener;
+import ai.timefold.solver.core.impl.domain.variable.supply.Demand;
+import ai.timefold.solver.core.impl.domain.variable.supply.Supply;
+import ai.timefold.solver.core.impl.domain.variable.supply.SupplyManager;
+import ai.timefold.solver.core.impl.score.director.InnerScoreDirector;
+
+/**
+ * Tracks variable listener events for a given {@link ai.timefold.solver.core.api.domain.variable.PlanningListVariable}.
+ */
+public class ListVariableTracker<Solution_>
+        implements SourcedVariableListener<Solution_>, ListVariableListener<Solution_, Object, Object>, Supply {
+    private final ListVariableDescriptor<Solution_> variableDescriptor;
+    private final List<Object> beforeVariableChangedEntityList;
+    private final List<Object> afterVariableChangedEntityList;
+
+    public ListVariableTracker(ListVariableDescriptor<Solution_> variableDescriptor) {
+        this.variableDescriptor = variableDescriptor;
+        beforeVariableChangedEntityList = new ArrayList<>();
+        afterVariableChangedEntityList = new ArrayList<>();
+    }
+
+    @Override
+    public VariableDescriptor<Solution_> getSourceVariableDescriptor() {
+        return variableDescriptor;
+    }
+
+    @Override
+    public void resetWorkingSolution(ScoreDirector<Solution_> scoreDirector) {
+        beforeVariableChangedEntityList.clear();
+        afterVariableChangedEntityList.clear();
+    }
+
+    @Override
+    public void beforeEntityAdded(ScoreDirector<Solution_> scoreDirector, Object entity) {
+
+    }
+
+    @Override
+    public void afterEntityAdded(ScoreDirector<Solution_> scoreDirector, Object entity) {
+
+    }
+
+    @Override
+    public void beforeEntityRemoved(ScoreDirector<Solution_> scoreDirector, Object entity) {
+
+    }
+
+    @Override
+    public void afterEntityRemoved(ScoreDirector<Solution_> scoreDirector, Object entity) {
+
+    }
+
+    @Override
+    public void afterListVariableElementUnassigned(ScoreDirector<Solution_> scoreDirector, Object element) {
+
+    }
+
+    @Override
+    public void beforeListVariableChanged(ScoreDirector<Solution_> scoreDirector, Object entity, int fromIndex, int toIndex) {
+        beforeVariableChangedEntityList.add(entity);
+    }
+
+    @Override
+    public void afterListVariableChanged(ScoreDirector<Solution_> scoreDirector, Object entity, int fromIndex, int toIndex) {
+        afterVariableChangedEntityList.add(entity);
+    }
+
+    public List<String> getEntitiesMissingBeforeAfterEvents(
+            List<VariableId<Solution_>> changedVariables) {
+        List<String> out = new ArrayList<>();
+        for (var changedVariable : changedVariables) {
+            if (!variableDescriptor.equals(changedVariable.variableDescriptor())) {
+                continue;
+            }
+            Object entity = changedVariable.entity();
+            if (!beforeVariableChangedEntityList.contains(entity)) {
+                out.add("Entity (" + entity
+                        + ") is missing a beforeListVariableChanged call for list variable ("
+                        + variableDescriptor.getVariableName() + ").");
+            }
+            if (!afterVariableChangedEntityList.contains(entity)) {
+                out.add("Entity (" + entity
+                        + ") is missing a afterListVariableChanged call for list variable ("
+                        + variableDescriptor.getVariableName() + ").");
+            }
+        }
+        beforeVariableChangedEntityList.clear();
+        afterVariableChangedEntityList.clear();
+        return out;
+    }
+
+    public TrackerDemand demand() {
+        return new TrackerDemand();
+    }
+
+    /**
+     * In order for the {@link ListVariableTracker} to be registered as a variable listener,
+     * it needs to be passed to the {@link InnerScoreDirector#getSupplyManager()}, which requires a {@link Demand}.
+     * <p>
+     * Unlike most other {@link Demand}s, there will only be one instance of
+     * {@link ListVariableTracker} in the {@link InnerScoreDirector} for each list variable.
+     */
+    public class TrackerDemand implements Demand<ListVariableTracker<Solution_>> {
+        @Override
+        public ListVariableTracker<Solution_> createExternalizedSupply(SupplyManager supplyManager) {
+            return ListVariableTracker.this;
+        }
+    }
+}