Revert "fix: clone the solution before sending it to the consumer"

Author: zepfred

core/src/main/java/ai/timefold/solver/core/impl/solver/DefaultSolver.java

@@ -204,7 +204,7 @@ public void setMonitorTagMap(Map<String, String> monitorTagMap) {
             restartSolver = checkProblemFactChanges();
         }
         outerSolvingEnded(solverScope);
-        return solverScope.cloneBestSolution();
+        return solverScope.getBestSolution();
     }
 
     public void outerSolvingStarted(SolverScope<Solution_> solverScope) {

core/src/main/java/ai/timefold/solver/core/impl/solver/event/SolverEventSupport.java

@@ -23,10 +23,7 @@ public void fireBestSolutionChanged(SolverScope<Solution_> solverScope, Solution
         var timeMillisSpent = solverScope.getBestSolutionTimeMillisSpent();
         var bestScore = solverScope.getBestScore();
         if (it.hasNext()) {
-            // We need to clone the new best solution, or we may share the same instance with user consumers.
-            // Reusing the instance can lead to inconsistent states if intermediary consumers change the solution.
-            var newBestSolutionCloned = solverScope.getScoreDirector().cloneSolution(newBestSolution);
-            var event = new DefaultBestSolutionChangedEvent<>(solver, timeMillisSpent, newBestSolutionCloned, bestScore);
+            var event = new DefaultBestSolutionChangedEvent<>(solver, timeMillisSpent, newBestSolution, bestScore);
             do {
                 it.next().bestSolutionChanged(event);
             } while (it.hasNext());

core/src/main/java/ai/timefold/solver/core/impl/solver/scope/SolverScope.java

@@ -224,10 +224,6 @@ public Solution_ getBestSolution() {
         return bestSolution.get();
     }
 
-    public Solution_ cloneBestSolution() {
-        return scoreDirector.cloneSolution(bestSolution.get());
-    }
-
     /**
      * The {@link PlanningSolution best solution} must never be the same instance
      * as the {@link PlanningSolution working solution}, it should be a (un)changed clone.
@@ -336,7 +332,7 @@ public long getMoveEvaluationSpeed() {
 
     public void setWorkingSolutionFromBestSolution() {
         // The workingSolution must never be the same instance as the bestSolution.
-        scoreDirector.setWorkingSolution(cloneBestSolution());
+        scoreDirector.setWorkingSolution(scoreDirector.cloneSolution(getBestSolution()));
     }
 
     public SolverScope<Solution_> createChildThreadSolverScope(ChildThreadType childThreadType) {

core/src/test/java/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhaseTest.java

@@ -98,8 +98,7 @@ void solveWithInitializedSolution() {
                 new TestdataEntity("e3", v3)));
 
         var solution = PlannerTestUtils.solve(solverConfig, inputProblem, false);
-        assertThat(inputProblem.getEntityList().stream().map(TestdataEntity::getValue).toList())
-                .isEqualTo(solution.getEntityList().stream().map(TestdataEntity::getValue).toList());
+        assertThat(inputProblem).isSameAs(solution);
     }
 
     @Test

core/src/test/java/ai/timefold/solver/core/impl/solver/BestSolutionHolderTest.java

@@ -13,17 +13,14 @@
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executors;
 
 import ai.timefold.solver.core.api.solver.Solver;
-import ai.timefold.solver.core.api.solver.SolverConfigOverride;
 import ai.timefold.solver.core.api.solver.SolverManager;
 import ai.timefold.solver.core.api.solver.change.ProblemChange;
 import ai.timefold.solver.core.api.solver.change.ProblemChangeDirector;
 import ai.timefold.solver.core.config.solver.SolverConfig;
 import ai.timefold.solver.core.config.solver.SolverManagerConfig;
-import ai.timefold.solver.core.config.solver.termination.TerminationConfig;
 import ai.timefold.solver.core.testdomain.TestdataEasyScoreCalculator;
 import ai.timefold.solver.core.testdomain.TestdataEntity;
 import ai.timefold.solver.core.testdomain.TestdataSolution;
@@ -189,36 +186,6 @@ void problemChangeBarrageIntermediateBestSolutionConsumer() throws InterruptedEx
 
     }
 
-    @Test
-    void testCloningBetweenPhases() throws ExecutionException, InterruptedException {
-        var solverConfig = new SolverConfig()
-                .withSolutionClass(TestdataSolution.class)
-                .withEntityClasses(TestdataEntity.class)
-                .withEasyScoreCalculatorClass(TestdataEasyScoreCalculator.class);
-
-        try (var solverManager = SolverManager.<TestdataSolution, UUID> create(solverConfig, new SolverManagerConfig())) {
-            var problem = TestdataSolution.generateSolution(2, 2);
-            problem.getEntityList().forEach(entity -> entity.setValue(null));
-            var countDownLatch = new CountDownLatch(1);
-            var solution = solverManager.solveBuilder()
-                    .withProblemId(UUID.randomUUID())
-                    .withProblem(problem)
-                    .withBestSolutionConsumer(
-                            intermediate -> {
-                                // Add a new entity to the cloned solution
-                                intermediate.getEntityList().add(new TestdataEntity("Bad Entity 3"));
-                                countDownLatch.countDown();
-                            })
-                    .withConfigOverride(new SolverConfigOverride<TestdataSolution>()
-                            .withTerminationConfig(new TerminationConfig().withMoveCountLimit(100L)))
-                    .run()
-                    .getFinalBestSolution();
-            countDownLatch.await();
-            // At this point, the events generated by the best solution consumer should not add any new entities
-            assertThat(solution.getEntityList()).hasSize(2);
-        }
-    }
-
     private record RecordedFuture(int id, CompletableFuture<Void> future) {
 
         boolean isDone() {