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

Author: zepfred

core/src/main/java/ai/timefold/solver/core/api/solver/SolverJobBuilder.java

@@ -59,6 +59,10 @@ public interface SolverJobBuilder<Solution_, ProblemId_> {
 
     /**
      * Sets the best solution consumer, which may be called multiple times during the solving process.
+     * <p>
+     * Don't apply any changes to the solution instance while the solver runs.
+     * The solver's best solution instance is the same as the one in the event,
+     * and any modifications may lead to solver corruption due to its internal reuse.
      *
      * @param bestSolutionConsumer called multiple times for each new best solution on a consumer thread
      * @return this

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() {

docs/src/modules/ROOT/pages/using-timefold-solver/running-the-solver.adoc

@@ -764,6 +764,135 @@ The current `SolverManager` implementation runs on a single computer node,
 but future work aims to distribute solver loads across a cloud.
 
 
+[#solverManagerBuilder]
+== The `SolverManager` Builder
+
+The `SolverManager` also enables the creation of a builder to customize and submit a planning problem for solving.
+
+[tabs]
+====
+Java::
++
+[source,java,options="nowrap"]
+----
+public interface SolverManager<Solution_> {
+
+    SolverJobBuilder<Solution_, ProblemId_> solveBuilder();
+
+    ...
+}
+----
+
+Python::
++
+[source,python,options="nowrap"]
+----
+class SolverManager(Generic[Solution_, ProblemId_]):
+    ...
+    def solve_builder(self) -> SolverJobBuilder[Solution_, ProblemId_]:
+        ...
+----
+====
+
+=== Required settings
+
+The `SolverJobBuilder` contract includes many optional methods, but only two are required: `withProblemId(...)` and `withProblem(...)`.
+
+[tabs]
+====
+Java::
++
+[source,java,options="nowrap"]
+----
+solverManager.solveBuilder()
+        .withProblemId(problemId)
+        .withProblem(problem)
+...
+----
+
+Python::
++
+[source,python,options="nowrap"]
+----
+solver_manager.solve_builder()
+        .with_problem_id(problemId)
+        .with_problem(problem)
+...
+----
+====
+
+The job's unique ID is specified using `withProblemId(problemId)`.
+The provided ID allows for the identification of a specific problem,
+enabling actions such as checking the solving status or terminating its execution.
+In addition to the unique ID, we must specify the problem to solve using `withProblem(problem)`.
+
+=== Optional settings
+
+Additional optional methods are also included in the `SolverJobBuilder` contract:
+
+[tabs]
+====
+Java::
++
+[source,java,options="nowrap"]
+----
+solverManager.solveBuilder()
+        .withProblemId(problemId)
+        .withProblem(problem)
+        .withFirstInitializedSolutionConsumer(firstInitializedSolutionConsumer)
+        .withBestSolutionConsumer(bestSolutionConsumer)
+        .withFinalBestSolutionConsumer(finalBestSolutionConsumer)
+        .withExceptionHandler(exceptionHandler)
+        .withConfigOverride(configOverride)
+...
+----
+
+Python::
++
+[source,python,options="nowrap"]
+----
+solver_manager.solve_builder()
+        .with_problem_id(problemId)
+        .with_problem(problem)
+        .with_first_initialized_solution_consumer(on_firs_solution_changed)
+        .with_best_solution_consumer(on_best_solution_changed)
+        .with_final_best_solution_consumer(on_final_solution_changed)
+        .with_exception_handler(on_exception_handler)
+        .with_config_override(config_override)
+...
+----
+====
+
+A consumer for the first initialized solution can be configured with `withFirstInitializedSolutionConsumer(...)`.
+The solution is returned by the last phase that immediately precedes the first local search phase.
+
+Whenever a new best solution is generated by the solver,
+it can be consumed by configuring it with `withBestSolutionConsumer(...)`.
+The final best solution consumer,
+which is called at the end of the solving process,
+can be set using `withFinalBestSolutionConsumer(...)`.
+Additionally,
+an improved solution consumer capable of throttling events is available in the xref:enterprise-edition/enterprise-edition.adoc#throttlingBestSolutionEvents[Enterprise Edition].
+
+[WARNING]
+====
+Do not modify the solutions returned by the events in `withFirstInitializedSolutionConsumer(...)` and `withBestSolutionConsumer(...)`. These instances are still utilized during the solving process, and any modifications may lead to solver corruption.
+====
+
+To handle errors that may arise during the solving process,
+set up the  handling logic by defining `withExceptionHandler(...)`.
+
+Finally, to build an instance of the solver,
+xref:using-timefold-solver/configuration.adoc[a configuration step] is necessary.
+These settings are static and applied to any related solving execution.
+If you want to override certain settings for a particular job,
+such as the termination configuration, you can use the `withConfigOverride(...)` method.
+
+[NOTE]
+====
+The solver also permits the configuration of multiple solver managers with distinct settings in xref:integration/integration.adoc#integrationWithQuarkusMultipleResources[Quarkus] or xref:integration/integration.adoc#integrationWithSpringBootMultipleResources[Spring Boot].
+====
+
 [#solverManagerSolveBatch]
 === Solve batch problems