fix: entity filtering NPE (#1866)

zepfred · web-flow · commit daae677fabcb · 2025-10-09T20:00:44.000+02:00
diff --git a/core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/entity/decorator/FilteringEntityByEntitySelector.java b/core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/entity/decorator/FilteringEntityByEntitySelector.java
@@ -298,7 +298,7 @@ private Object pickNext() {
             this.selected = null;
             while (entityIterator.hasNext()) {
                 var entity = entityIterator.next();
-                if (isReachable(entity)) {
+                if (entity != null && isReachable(entity)) {
                     return entity;
                 }
             }
@@ -364,7 +364,7 @@ protected Object createUpcomingSelection() {
             }
             while (entityIterator.hasNext()) {
                 var otherEntity = entityIterator.next();
-                if (isReachable(replayedEntity, otherEntity)) {
+                if (otherEntity != null && isReachable(replayedEntity, otherEntity)) {
                     return otherEntity;
                 }
             }
@@ -378,7 +378,7 @@ protected Object createPreviousSelection() {
             }
             while (entityIterator.hasPrevious()) {
                 var otherEntity = entityIterator.previous();
-                if (isReachable(replayedEntity, otherEntity)) {
+                if (otherEntity != null && isReachable(replayedEntity, otherEntity)) {
                     return otherEntity;
                 }
             }
@@ -442,10 +442,10 @@ public Object next() {
                 bailoutSize--;
                 // We expect the iterator to apply a random selection
                 var next = entityIterator.next();
-                if (isReachable(currentReplayedEntity, next)) {
+                if (next != null && isReachable(currentReplayedEntity, next)) {
                     return next;
                 }
-            } while (bailoutSize > 0);
+            } while (bailoutSize > 0 && entityIterator.hasNext());
             // If no reachable entity is found, we return the currently selected entity,
             // which will result in a non-doable move
             return currentReplayedEntity;
diff --git a/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveSelectorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/heuristic/selector/move/generic/SwapMoveSelectorTest.java
@@ -1,5 +1,6 @@
 package ai.timefold.solver.core.impl.heuristic.selector.move.generic;
 
+import static ai.timefold.solver.core.impl.heuristic.selector.SelectorTestUtils.mockEntitySelector;
 import static ai.timefold.solver.core.impl.heuristic.selector.SelectorTestUtils.phaseStarted;
 import static ai.timefold.solver.core.impl.heuristic.selector.SelectorTestUtils.solvingStarted;
 import static ai.timefold.solver.core.testdomain.list.TestdataListUtils.getEntityDescriptor;
@@ -15,18 +16,23 @@
 import java.util.List;
 import java.util.Random;
 
+import ai.timefold.solver.core.api.score.director.ScoreDirector;
 import ai.timefold.solver.core.config.heuristic.selector.common.SelectionCacheType;
 import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor;
 import ai.timefold.solver.core.impl.heuristic.selector.SelectorTestUtils;
+import ai.timefold.solver.core.impl.heuristic.selector.common.decorator.SelectionFilter;
 import ai.timefold.solver.core.impl.heuristic.selector.entity.EntitySelector;
 import ai.timefold.solver.core.impl.heuristic.selector.entity.FromSolutionEntitySelector;
 import ai.timefold.solver.core.impl.heuristic.selector.entity.decorator.FilteringEntityByEntitySelector;
+import ai.timefold.solver.core.impl.heuristic.selector.entity.decorator.FilteringEntitySelector;
+import ai.timefold.solver.core.impl.heuristic.selector.entity.mimic.ManualEntityMimicRecorder;
 import ai.timefold.solver.core.impl.heuristic.selector.entity.mimic.MimicRecordingEntitySelector;
 import ai.timefold.solver.core.impl.heuristic.selector.entity.mimic.MimicReplayingEntitySelector;
 import ai.timefold.solver.core.impl.phase.scope.AbstractPhaseScope;
 import ai.timefold.solver.core.impl.phase.scope.AbstractStepScope;
 import ai.timefold.solver.core.impl.solver.scope.SolverScope;
 import ai.timefold.solver.core.testdomain.TestdataEntity;
+import ai.timefold.solver.core.testdomain.TestdataObject;
 import ai.timefold.solver.core.testdomain.TestdataValue;
 import ai.timefold.solver.core.testdomain.valuerange.entityproviding.TestdataEntityProvidingEntity;
 import ai.timefold.solver.core.testdomain.valuerange.entityproviding.TestdataEntityProvidingSolution;
@@ -336,6 +342,103 @@ void emptyRightOriginalLeftUnequalsRight() {
         verifyPhaseLifecycle(rightEntitySelector, 1, 2, 5);
     }
 
+    @Test
+    void originalEntitiesPinned() {
+        var v1 = new TestdataValue("1");
+        var v2 = new TestdataValue("2");
+        var v3 = new TestdataValue("3");
+        var v4 = new TestdataValue("4");
+        var e1 = new TestdataAllowsUnassignedEntityProvidingEntity("A", List.of(v1, v4));
+        var e2 = new TestdataAllowsUnassignedEntityProvidingEntity("B", List.of(v2, v3));
+        var e3 = new TestdataAllowsUnassignedEntityProvidingEntity("C", List.of(v1, v4));
+        var solution = new TestdataAllowsUnassignedEntityProvidingSolution("s1");
+        solution.setEntityList(List.of(e1, e2, e3));
+
+        var scoreDirector = mockScoreDirector(TestdataAllowsUnassignedEntityProvidingSolution.buildSolutionDescriptor());
+        scoreDirector.setWorkingSolution(solution);
+
+        var leftEntitySelector = new ManualEntityMimicRecorder<>(
+                mockEntitySelector(TestdataAllowsUnassignedEntityProvidingEntity.buildEntityDescriptor(), e1, e2, e3));
+
+        var replayingEntitySelector = new MimicReplayingEntitySelector<>(leftEntitySelector);
+        var filteringEntitySelector =
+                FilteringEntitySelector.of(
+                        mockEntitySelector(TestdataAllowsUnassignedEntityProvidingEntity.buildEntityDescriptor(), e1, e2, e3),
+                        new EntityCodeFiltering<>(List.of("B", "C")));
+        var rightEntitySelector =
+                new FilteringEntityByEntitySelector<>(filteringEntitySelector, replayingEntitySelector, false);
+        var solverScope = solvingStarted(rightEntitySelector, scoreDirector);
+        phaseStarted(rightEntitySelector, solverScope);
+
+        // Regular iterator
+        // The left selector chooses A, and the right selector returns no value
+        leftEntitySelector.setRecordedEntity(e1);
+        var iterator = rightEntitySelector.iterator();
+        assertThat(iterator.hasNext()).isFalse();
+        // The left selector chooses B, and the right selector returns A
+        leftEntitySelector.setRecordedEntity(e2);
+        iterator = rightEntitySelector.iterator();
+        assertThat(iterator.hasNext()).isTrue();
+        assertThat(iterator.next()).hasToString("A");
+        // No more moves
+        assertThat(iterator.hasNext()).isFalse();
+
+        // ListIterator
+        // The left selector chooses A, and the right selector returns no value
+        leftEntitySelector.setRecordedEntity(e1);
+        var listIterator = rightEntitySelector.listIterator();
+        assertThat(listIterator.hasNext()).isFalse();
+        // B <-> A
+        leftEntitySelector.setRecordedEntity(e2);
+        listIterator = rightEntitySelector.listIterator();
+        assertThat(listIterator.hasNext()).isTrue();
+        assertThat(listIterator.next()).hasToString("A");
+        assertThat(listIterator.hasNext()).isFalse();
+        // Backward move
+        assertThat(listIterator.hasPrevious()).isTrue();
+        assertThat(listIterator.previous()).hasToString("A");
+    }
+
+    @Test
+    void randomEntitiesPinned() {
+        var v1 = new TestdataValue("1");
+        var v2 = new TestdataValue("2");
+        var v3 = new TestdataValue("3");
+        var v4 = new TestdataValue("4");
+        var e1 = new TestdataAllowsUnassignedEntityProvidingEntity("A", List.of(v1, v4), v1);
+        var e2 = new TestdataAllowsUnassignedEntityProvidingEntity("B", List.of(v2, v3), v2);
+        var e3 = new TestdataAllowsUnassignedEntityProvidingEntity("C", List.of(v1, v4));
+        var solution = new TestdataAllowsUnassignedEntityProvidingSolution("s1");
+        solution.setEntityList(List.of(e1, e2, e3));
+
+        var scoreDirector = mockScoreDirector(TestdataAllowsUnassignedEntityProvidingSolution.buildSolutionDescriptor());
+        scoreDirector.setWorkingSolution(solution);
+
+        var baseEntitySelector =
+                new FromSolutionEntitySelector<>(getEntityDescriptor(scoreDirector), SelectionCacheType.JUST_IN_TIME, true);
+        var leftEntitySelector = new ManualEntityMimicRecorder<>(baseEntitySelector);
+
+        var replayingEntitySelector = new MimicReplayingEntitySelector<>(leftEntitySelector);
+        var filteringEntitySelector =
+                FilteringEntitySelector.of(baseEntitySelector, new EntityCodeFiltering<>(List.of("B", "C")));
+        var rightEntitySelector =
+                new FilteringEntityByEntitySelector<>(filteringEntitySelector, replayingEntitySelector, true);
+        var solverScope = solvingStarted(rightEntitySelector, scoreDirector,
+                new TestRandom(0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1));
+        phaseStarted(rightEntitySelector, solverScope);
+
+        // Random iterator
+        // The left selector chooses B,
+        // and the right selector chooses A (not reachable), and then only B (excluded by the filter)
+        leftEntitySelector.setRecordedEntity(e2);
+        var iterator = rightEntitySelector.iterator();
+        assertThat(iterator.hasNext()).isTrue();
+        // Return the same as the left selector
+        assertThat(iterator.next()).hasToString("B");
+        // No more moves
+        assertThat(iterator.hasNext()).isFalse();
+    }
+
     @Test
     void singleVarRandomSelectionWithEntityValueRange() {
         var v1 = new TestdataValue("1");
@@ -492,4 +595,18 @@ void noReachableEntities() {
         var swapMove = (SwapMove<TestdataEntityProvidingSolution>) iterator.next();
         assertThat(swapMove.getLeftEntity()).isSameAs(swapMove.getRightEntity());
     }
+
+    private static class EntityCodeFiltering<Solution_> implements SelectionFilter<Solution_, Object> {
+
+        private final List<String> excludedCodes;
+
+        public EntityCodeFiltering(List<String> excludedCodes) {
+            this.excludedCodes = excludedCodes;
+        }
+
+        @Override
+        public boolean accept(ScoreDirector<Solution_> scoreDirector, Object selection) {
+            return !excludedCodes.contains(((TestdataObject) selection).getCode());
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -298,7 +298,7 @@ private Object pickNext() {`
`298`	`298`	`this.selected = null;`
`299`	`299`	`while (entityIterator.hasNext()) {`
`300`	`300`	`var entity = entityIterator.next();`
`301`		`- if (isReachable(entity)) {`
	`301`	`+ if (entity != null && isReachable(entity)) {`
`302`	`302`	`return entity;`
`303`	`303`	`}`
`304`	`304`	`}`
`@@ -364,7 +364,7 @@ protected Object createUpcomingSelection() {`
`364`	`364`	`}`
`365`	`365`	`while (entityIterator.hasNext()) {`
`366`	`366`	`var otherEntity = entityIterator.next();`
`367`		`- if (isReachable(replayedEntity, otherEntity)) {`
	`367`	`+ if (otherEntity != null && isReachable(replayedEntity, otherEntity)) {`
`368`	`368`	`return otherEntity;`
`369`	`369`	`}`
`370`	`370`	`}`
`@@ -378,7 +378,7 @@ protected Object createPreviousSelection() {`
`378`	`378`	`}`
`379`	`379`	`while (entityIterator.hasPrevious()) {`
`380`	`380`	`var otherEntity = entityIterator.previous();`
`381`		`- if (isReachable(replayedEntity, otherEntity)) {`
	`381`	`+ if (otherEntity != null && isReachable(replayedEntity, otherEntity)) {`
`382`	`382`	`return otherEntity;`
`383`	`383`	`}`
`384`	`384`	`}`
`@@ -442,10 +442,10 @@ public Object next() {`
`442`	`442`	`bailoutSize--;`
`443`	`443`	`// We expect the iterator to apply a random selection`
`444`	`444`	`var next = entityIterator.next();`
`445`		`- if (isReachable(currentReplayedEntity, next)) {`
	`445`	`+ if (next != null && isReachable(currentReplayedEntity, next)) {`
`446`	`446`	`return next;`
`447`	`447`	`}`
`448`		`- } while (bailoutSize > 0);`
	`448`	`+ } while (bailoutSize > 0 && entityIterator.hasNext());`
`449`	`449`	`// If no reachable entity is found, we return the currently selected entity,`
`450`	`450`	`// which will result in a non-doable move`
`451`	`451`	`return currentReplayedEntity;`