test for shadowVariableCorruption + improve error message

optaplanner-core/src/main/java/org/optaplanner/core/impl/score/director/AbstractScoreDirector.java

@@ -521,18 +521,12 @@ public void assertExpectedWorkingScore(Score expectedWorkingScore, Object comple
 
     @Override
     public void assertShadowVariablesAreNotStale(Score expectedWorkingScore, Object completedAction) {
-        Map<ShadowVariableDescriptor, List<String>> shadowVariableDescriptorListMap = calculateShadowVariableViolationListMap();
-        if (!shadowVariableDescriptorListMap.isEmpty()) {
-            StringBuilder message = new StringBuilder();
-            message.append(VariableListener.class.getSimpleName()).append(" corruption after completedAction (")
-                    .append(completedAction).append("):\n");
-            shadowVariableDescriptorListMap.forEach((shadowVariableDescriptor, violationList) -> {
-                message.append(violationList.get(0));
-                if (violationList.size() > 1) {
-                    message.append("  ...\n");
-                }
-            });
-            throw new IllegalStateException(message.toString());
+        String violationMessage = createShadowVariablesViolationMessage();
+        if (violationMessage != null) {
+            throw new IllegalStateException(
+                    VariableListener.class.getSimpleName() + " corruption after completedAction ("
+                    + completedAction + "):\n"
+                    + violationMessage);
         }
         Score workingScore = calculateScore();
         if (!expectedWorkingScore.equals(workingScore)) {
@@ -555,26 +549,21 @@ public void assertShadowVariablesAreNotStale(Score expectedWorkingScore, Object
      * @return never null
      */
     protected String buildShadowVariableAnalysis(boolean predicted) {
-        Map<ShadowVariableDescriptor, List<String>> shadowVariableDescriptorListMap = calculateShadowVariableViolationListMap();
+        String violationMessage = createShadowVariablesViolationMessage();
         String workingLabel = predicted ? "working" : "corrupted";
-        if (shadowVariableDescriptorListMap.isEmpty()) {
-            return "Shadow variable corruption: none in the " + workingLabel + " scoreDirector.";
+        if (violationMessage == null) {
+            return "Shadow variable corruption in the " + workingLabel + " scoreDirector:\n"
+                    + "  None";
         }
-        final int SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT = 3;
-        StringBuilder analysis = new StringBuilder();
-        analysis.append("Shadow variable corruption in the ").append(workingLabel).append(" scoreDirector:\n");
-        shadowVariableDescriptorListMap.forEach((shadowVariableDescriptor, violationList) -> {
-            violationList.stream().limit(SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT).forEach(analysis::append);
-            if (violationList.size() >= SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT) {
-                analysis.append("    ... ").append(violationList.size() - SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT)
-                        .append(" more\n");
-            }
-        });
-        analysis.append("  Check your variable listeners.");
-        return analysis.toString();
+        return "Shadow variable corruption in the " + workingLabel + " scoreDirector:\n"
+                + violationMessage
+                + "  Maybe there is a bug in the VariableListener of those shadow variable(s).";
     }
 
-    protected Map<ShadowVariableDescriptor, List<String>> calculateShadowVariableViolationListMap() {
+    /**
+     * @return null if there are no violations
+     */
+    protected String createShadowVariablesViolationMessage() {
         Map<ShadowVariableDescriptor, List<String>> violationListMap = new TreeMap<>(
                 Comparator.comparing(ShadowVariableDescriptor::getGlobalShadowOrder));
         SolutionDescriptor<Solution_> solutionDescriptor = getSolutionDescriptor();
@@ -604,19 +593,31 @@ protected Map<ShadowVariableDescriptor, List<String>> calculateShadowVariableVio
                 Object originalValue = shadowVariableValuesMap.get(shadowVariableDescriptor);
                 if (!Objects.equals(originalValue, newValue)) {
                     List<String> violationList = violationListMap.computeIfAbsent(shadowVariableDescriptor, k -> new ArrayList<>());
-                    violationList.add("  The entity (" + entity
+                    violationList.add("    The entity (" + entity
                             + ")'s shadow variable (" + shadowVariableDescriptor.getSimpleEntityAndVariableName()
                             + ")'s corrupted value (" + originalValue + ") changed to uncorrupted value (" + newValue
                             + ") after all " + VariableListener.class.getSimpleName()
                             + "s were triggered without changes to the genuine variables.\n"
-                            + "  Maybe the " + VariableListener.class.getSimpleName() + " class ("
+                            + "      Maybe the " + VariableListener.class.getSimpleName() + " class ("
                             + shadowVariableDescriptor.getVariableListenerClass().getSimpleName()
                             + ") for that shadow variable (" + shadowVariableDescriptor.getSimpleEntityAndVariableName()
                             + ") forgot to update it when one of its sources changed.\n");
                 }
             }
         }
-        return violationListMap;
+        if (violationListMap.isEmpty()) {
+            return null;
+        }
+        final int SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT = 3;
+        StringBuilder message = new StringBuilder();
+        violationListMap.forEach((shadowVariableDescriptor, violationList) -> {
+            violationList.stream().limit(SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT).forEach(message::append);
+            if (violationList.size() >= SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT) {
+                message.append("  ... ").append(violationList.size() - SHADOW_VARIABLE_VIOLATION_DISPLAY_LIMIT)
+                        .append(" more\n");
+            }
+        });
+        return message.toString();
     }
 
     @Override

optaplanner-core/src/test/java/org/optaplanner/core/impl/score/director/easy/EasyScoreDirectorTest.java

@@ -15,10 +15,21 @@
  */
 package org.optaplanner.core.impl.score.director.easy;
 
+import java.util.Arrays;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.mockito.AdditionalAnswers;
+import org.optaplanner.core.api.score.buildin.simple.SimpleScore;
+import org.optaplanner.core.config.score.trend.InitializingScoreTrendLevel;
 import org.optaplanner.core.impl.domain.solution.descriptor.SolutionDescriptor;
+import org.optaplanner.core.impl.score.director.InnerScoreDirector;
+import org.optaplanner.core.impl.score.director.drools.DroolsScoreDirector;
+import org.optaplanner.core.impl.score.trend.InitializingScoreTrend;
+import org.optaplanner.core.impl.testdata.domain.TestdataValue;
+import org.optaplanner.core.impl.testdata.domain.shadow.corrupted.TestdataCorruptedShadowedEntity;
+import org.optaplanner.core.impl.testdata.domain.shadow.corrupted.TestdataCorruptedShadowedSolution;
 
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
@@ -45,4 +56,33 @@ private EasyScoreDirectorFactory<Object> mockEasyScoreDirectorFactory() {
         return factory;
     }
 
+    @Test(expected = IllegalStateException.class)
+    public void shadowVariableCorruption() {
+        EasyScoreDirectorFactory<TestdataCorruptedShadowedSolution> scoreDirectorFactory =
+                new EasyScoreDirectorFactory<>((EasyScoreCalculator<TestdataCorruptedShadowedSolution>) (solution_) -> SimpleScore.valueOf(0));
+        scoreDirectorFactory.setSolutionDescriptor(TestdataCorruptedShadowedSolution.buildSolutionDescriptor());
+        scoreDirectorFactory.setInitializingScoreTrend(
+                InitializingScoreTrend.buildUniformTrend(InitializingScoreTrendLevel.ONLY_DOWN, 1));
+        EasyScoreDirector<TestdataCorruptedShadowedSolution> scoreDirector = scoreDirectorFactory.buildScoreDirector(false, false);
+
+        TestdataCorruptedShadowedSolution solution = new TestdataCorruptedShadowedSolution("s1");
+        TestdataValue v1 = new TestdataValue("v1");
+        TestdataValue v2 = new TestdataValue("v2");
+        solution.setValueList(Arrays.asList(v1, v2));
+        TestdataCorruptedShadowedEntity e1 = new TestdataCorruptedShadowedEntity("e1");
+        TestdataCorruptedShadowedEntity e2 = new TestdataCorruptedShadowedEntity("e2");
+        solution.setEntityList(Arrays.asList(e1, e2));
+        scoreDirector.setWorkingSolution(solution);
+
+        scoreDirector.assertShadowVariablesAreNotStale(SimpleScore.valueOfUninitialized(-2, 0), "NoChange");
+        scoreDirector.beforeVariableChanged(e1, "value");
+        e1.setValue(v1);
+        scoreDirector.afterVariableChanged(e1, "value");
+        scoreDirector.beforeVariableChanged(e2, "value");
+        e2.setValue(v1);
+        scoreDirector.afterVariableChanged(e2, "value");
+        scoreDirector.triggerVariableListeners();
+        scoreDirector.assertShadowVariablesAreNotStale(SimpleScore.valueOfUninitialized(0, 0), "FirstChange");
+    }
+
 }

optaplanner-core/src/test/java/org/optaplanner/core/impl/testdata/domain/shadow/corrupted/TestdataCorruptedShadowedEntity.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2018 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.optaplanner.core.impl.testdata.domain.shadow.corrupted;
+
+import org.optaplanner.core.api.domain.entity.PlanningEntity;
+import org.optaplanner.core.api.domain.variable.CustomShadowVariable;
+import org.optaplanner.core.api.domain.variable.PlanningVariable;
+import org.optaplanner.core.api.domain.variable.PlanningVariableReference;
+import org.optaplanner.core.impl.domain.entity.descriptor.EntityDescriptor;
+import org.optaplanner.core.impl.domain.solution.descriptor.SolutionDescriptor;
+import org.optaplanner.core.impl.domain.variable.listener.VariableListenerAdapter;
+import org.optaplanner.core.impl.score.director.ScoreDirector;
+import org.optaplanner.core.impl.testdata.domain.TestdataObject;
+import org.optaplanner.core.impl.testdata.domain.TestdataValue;
+
+@PlanningEntity
+public class TestdataCorruptedShadowedEntity extends TestdataObject {
+
+    public static EntityDescriptor buildEntityDescriptor() {
+        SolutionDescriptor solutionDescriptor = TestdataCorruptedShadowedSolution.buildSolutionDescriptor();
+        return solutionDescriptor.findEntityDescriptorOrFail(TestdataCorruptedShadowedEntity.class);
+    }
+
+    private TestdataValue value;
+    private Integer count;
+
+    public TestdataCorruptedShadowedEntity() {
+    }
+
+    public TestdataCorruptedShadowedEntity(String code) {
+        super(code);
+    }
+
+    public TestdataCorruptedShadowedEntity(String code, TestdataValue value, int count) {
+        this(code);
+        this.value = value;
+    }
+
+    @PlanningVariable(valueRangeProviderRefs = "valueRange")
+    public TestdataValue getValue() {
+        return value;
+    }
+
+    public void setValue(TestdataValue value) {
+        this.value = value;
+    }
+
+    @CustomShadowVariable(variableListenerClass = CountUpdatingVariableListener.class,
+            sources = {@PlanningVariableReference(variableName = "value")})
+    public Integer getCount() {
+        return count;
+    }
+
+    public void setCount(Integer count) {
+        this.count = count;
+    }
+
+    // ************************************************************************
+    // Complex methods
+    // ************************************************************************
+
+    // ************************************************************************
+    // Static inner classes
+    // ************************************************************************
+
+    public static class CountUpdatingVariableListener extends VariableListenerAdapter<TestdataCorruptedShadowedEntity> {
+
+        @Override
+        public void afterEntityAdded(ScoreDirector scoreDirector, TestdataCorruptedShadowedEntity entity) {
+            updateShadow(entity, scoreDirector);
+        }
+
+        @Override
+        public void afterVariableChanged(ScoreDirector scoreDirector, TestdataCorruptedShadowedEntity entity) {
+            updateShadow(entity, scoreDirector);
+        }
+
+        private void updateShadow(TestdataCorruptedShadowedEntity entity, ScoreDirector scoreDirector) {
+            TestdataValue primaryValue = entity.getValue();
+            Integer count;
+            if (primaryValue == null) {
+                count = null;
+            } else {
+                count = (entity.getCount() == null) ? 0 : entity.getCount();
+                count++;
+            }
+            scoreDirector.beforeVariableChanged(entity, "count");
+            entity.setCount(count);
+            scoreDirector.afterVariableChanged(entity, "count");
+        }
+
+    }
+
+}

optaplanner-core/src/test/java/org/optaplanner/core/impl/testdata/domain/shadow/corrupted/TestdataCorruptedShadowedSolution.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright 2018 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.optaplanner.core.impl.testdata.domain.shadow.corrupted;
+
+import java.util.List;
+
+import org.optaplanner.core.api.domain.solution.PlanningEntityCollectionProperty;
+import org.optaplanner.core.api.domain.solution.PlanningScore;
+import org.optaplanner.core.api.domain.solution.PlanningSolution;
+import org.optaplanner.core.api.domain.solution.drools.ProblemFactCollectionProperty;
+import org.optaplanner.core.api.domain.valuerange.ValueRangeProvider;
+import org.optaplanner.core.api.score.buildin.simple.SimpleScore;
+import org.optaplanner.core.impl.domain.solution.descriptor.SolutionDescriptor;
+import org.optaplanner.core.impl.testdata.domain.TestdataObject;
+import org.optaplanner.core.impl.testdata.domain.TestdataValue;
+
+@PlanningSolution
+public class TestdataCorruptedShadowedSolution extends TestdataObject {
+
+    public static SolutionDescriptor buildSolutionDescriptor() {
+        return SolutionDescriptor.buildSolutionDescriptor(TestdataCorruptedShadowedSolution.class,
+                TestdataCorruptedShadowedEntity.class);
+    }
+
+    private List<TestdataValue> valueList;
+    private List<TestdataCorruptedShadowedEntity> entityList;
+
+    private SimpleScore score;
+
+    public TestdataCorruptedShadowedSolution() {
+    }
+
+    public TestdataCorruptedShadowedSolution(String code) {
+        super(code);
+    }
+
+    @ValueRangeProvider(id = "valueRange")
+    @ProblemFactCollectionProperty
+    public List<TestdataValue> getValueList() {
+        return valueList;
+    }
+
+    public void setValueList(List<TestdataValue> valueList) {
+        this.valueList = valueList;
+    }
+
+    @PlanningEntityCollectionProperty
+    public List<TestdataCorruptedShadowedEntity> getEntityList() {
+        return entityList;
+    }
+
+    public void setEntityList(List<TestdataCorruptedShadowedEntity> entityList) {
+        this.entityList = entityList;
+    }
+
+    @PlanningScore
+    public SimpleScore getScore() {
+        return score;
+    }
+
+    public void setScore(SimpleScore score) {
+        this.score = score;
+    }
+
+    // ************************************************************************
+    // Complex methods
+    // ************************************************************************
+
+}