From 77a1ad389b99cd3f40749622fe7a4d6a4eceb790 Mon Sep 17 00:00:00 2001 From: Paolo Antinori Date: Sat, 21 Mar 2026 10:33:46 +0100 Subject: [PATCH] fix: Handle null planning values in ValueTabuAcceptor ValueTabuAcceptor threw NullPointerException when processing moves with nullable planning variables that contain null values. The null values represent "unassigned" state and cannot be tracked in the tabu deque. Root cause: AbstractTabuAcceptor.adjustTabuList() tried to add null elements directly to ArrayDeque, which throws NullPointerException. Fix: Skip null values in adjustTabuList() before adding to the tabu tracking structures. Null planning values are semantically "unassigned" and tracking them as tabu has no meaningful purpose. Added test: nullablePlanningValue() verifies the fix and ensures null values are skipped while non-null values are correctly tracked. Co-Authored-By: Claude Opus 4.6 --- .../acceptor/tabu/AbstractTabuAcceptor.java | 4 ++ .../acceptor/tabu/ValueTabuAcceptorTest.java | 59 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/AbstractTabuAcceptor.java b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/AbstractTabuAcceptor.java index 8f438a6eb6f..03063cd3d77 100644 --- a/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/AbstractTabuAcceptor.java +++ b/core/src/main/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/AbstractTabuAcceptor.java @@ -109,6 +109,10 @@ protected void adjustTabuList(int tabuStepIndex, Collection ta } // Add the new tabu(s) for (var tabu : tabus) { + // Skip null planning values (unassigned state) + if (tabu == null) { + continue; + } // Push tabu to the end of the line if (tabuToStepIndexMap.containsKey(tabu)) { tabuToStepIndexMap.remove(tabu); diff --git a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/ValueTabuAcceptorTest.java b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/ValueTabuAcceptorTest.java index 1565b967a8a..1e1be56ab57 100644 --- a/core/src/test/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/ValueTabuAcceptorTest.java +++ b/core/src/test/java/ai/timefold/solver/core/impl/localsearch/decider/acceptor/tabu/ValueTabuAcceptorTest.java @@ -4,7 +4,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import ai.timefold.solver.core.api.score.SimpleScore; import ai.timefold.solver.core.impl.localsearch.decider.acceptor.tabu.size.FixedTabuSizeStrategy; @@ -251,4 +253,61 @@ private static LocalSearchMoveScope buildMoveScope( return moveScope; } + @Test + void nullablePlanningValue() { + // ValueTabuAcceptor should handle null planning values (nullable planning variables) + // Previously threw NullPointerException when trying to add null to ArrayDeque + var acceptor = new ValueTabuAcceptor<>(""); + acceptor.setTabuSizeStrategy(new FixedTabuSizeStrategy<>(2)); + acceptor.setAspirationEnabled(true); + + var v0 = new TestdataValue("v0"); + var v1 = new TestdataValue("v1"); + + var solverScope = new SolverScope<>(); + solverScope.setInitializedBestScore(SimpleScore.ZERO); + var phaseScope = new LocalSearchPhaseScope<>(solverScope, 0); + acceptor.phaseStarted(phaseScope); + + var stepScope0 = new LocalSearchStepScope<>(phaseScope); + + // Build a move scope with a null planning value (nullable planning variable) + var moveScopeWithNull = buildMoveScopeWithNull(stepScope0, v0, null, v1); + + // Should accept the move (no tabu yet) + assertThat(acceptor.isAccepted(moveScopeWithNull)).isTrue(); + + // After fix: should NOT throw NullPointerException + // stepEnded() calls adjustTabuList() which should skip null values + stepScope0.setStep(moveScopeWithNull.getMove()); + acceptor.stepEnded(stepScope0); + phaseScope.setLastCompletedStepScope(stepScope0); + + // Verify that v0 and v1 are now tabu, but null was skipped + var stepScope1 = new LocalSearchStepScope<>(phaseScope); + assertThat(acceptor.isAccepted(buildMoveScopeWithNull(stepScope1, v0))).isFalse(); + assertThat(acceptor.isAccepted(buildMoveScopeWithNull(stepScope1, v1))).isFalse(); + // null values should still be accepted (they are not tracked as tabu) + assertThat(acceptor.isAccepted(buildMoveScopeWithNull(stepScope1, null))).isTrue(); + + acceptor.phaseEnded(phaseScope); + } + + private static LocalSearchMoveScope buildMoveScopeWithNull( + LocalSearchStepScope stepScope, TestdataValue... values) { + var move = mock(Move.class); + // Create a list that may contain null values (ArrayList explicitly allows null) + List valueList = new ArrayList<>(); + // Handle single-null varargs edge case: method(null) passes null as the array, not [null] + if (values != null) { + for (TestdataValue v : values) { + valueList.add(v); + } + } + when(move.getPlanningValues()).thenReturn(valueList); + var moveScope = new LocalSearchMoveScope(stepScope, 0, move); + moveScope.setInitializedScore(SimpleScore.ZERO); + return moveScope; + } + }