refactor: no score director in custom phase

Author: triceo

core/src/main/java/ai/timefold/solver/core/api/solver/phase/PhaseCommand.java

@@ -1,46 +1,34 @@
 package ai.timefold.solver.core.api.solver.phase;
 
-import java.util.function.BooleanSupplier;
-
 import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
-import ai.timefold.solver.core.api.score.Score;
-import ai.timefold.solver.core.api.score.director.ScoreDirector;
 import ai.timefold.solver.core.api.solver.Solver;
 import ai.timefold.solver.core.api.solver.change.ProblemChange;
 import ai.timefold.solver.core.impl.phase.Phase;
-import ai.timefold.solver.core.impl.score.director.InnerScoreDirector;
+import ai.timefold.solver.core.preview.api.move.Move;
 
 import org.jspecify.annotations.NullMarked;
 
 /**
  * Runs a custom algorithm as a {@link Phase} of the {@link Solver} that changes the planning variables.
- * To change problem facts, use {@link Solver#addProblemChange(ProblemChange)} instead.
- * <p>
- * To add custom properties, configure custom properties and add public setters for them.
+ * To change problem facts and to add or remove entities, use {@link Solver#addProblemChange(ProblemChange)} instead.
  *
  * @param <Solution_> the solution type, the class with the {@link PlanningSolution} annotation
  */
 @NullMarked
 public interface PhaseCommand<Solution_> {
 
     /**
-     * Changes {@link PlanningSolution working solution} of {@link ScoreDirector#getWorkingSolution()}.
-     * When the {@link PlanningSolution working solution} is modified,
-     * the {@link ScoreDirector} must be correctly notified
-     * (through {@link ScoreDirector#beforeVariableChanged(Object, String)} and
-     * {@link ScoreDirector#afterVariableChanged(Object, String)}),
-     * otherwise calculated {@link Score}s will be corrupted.
+     * Changes the current {@link PhaseCommandContext#getWorkingSolution() working solution}.
+     * The solver is notified of the changes through {@link PhaseCommandContext},
+     * specifically through {@link PhaseCommandContext#execute(Move)}.
+     * Any other modifications to the working solution are strictly forbidden
+     * and will likely cause the solver to be in an inconsistent state and throw an exception later on.
      * <p>
-     * Don't forget to call {@link ScoreDirector#triggerVariableListeners()} after each set of changes
-     * (especially before every {@link InnerScoreDirector#calculateScore()} call)
-     * to ensure all shadow variables are updated.
+     * Don't forget to check {@link PhaseCommandContext#isPhaseTerminated() termination status} frequently
+     * to allow the solver to gracefully terminate when necessary.
      *
-     * @param scoreDirector the {@link ScoreDirector} that needs to get notified of the changes.
-     * @param isPhaseTerminated long-running command implementations should check this periodically
-     *        and terminate early if it returns true.
-     *        Otherwise the terminations configured by the user will have no effect,
-     *        as the solver can only terminate itself when a command has ended.
+     * @param context the context of the command, providing access to the working solution and allowing move execution
      */
-    void changeWorkingSolution(ScoreDirector<Solution_> scoreDirector, BooleanSupplier isPhaseTerminated);
+    void changeWorkingSolution(PhaseCommandContext<Solution_> context);
 
 }

core/src/main/java/ai/timefold/solver/core/api/solver/phase/PhaseCommandContext.java

@@ -0,0 +1,98 @@
+package ai.timefold.solver.core.api.solver.phase;
+
+import java.util.function.Function;
+
+import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningSolutionMetaModel;
+import ai.timefold.solver.core.preview.api.move.Move;
+import ai.timefold.solver.core.preview.api.move.Rebaser;
+
+import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
+
+/**
+ * The context of a command that is executed during a custom phase.
+ * It provides access to the working solution and allows executing moves.
+ *
+ * @param <Solution_> the type of the solution
+ * @see PhaseCommand
+ */
+@NullMarked
+public interface PhaseCommandContext<Solution_> {
+
+    /**
+     * Returns the meta-model of the {@link #getWorkingSolution() working solution}.
+     *
+     * @return the meta-model of the working solution
+     */
+    PlanningSolutionMetaModel<Solution_> getSolutionMetaModel();
+
+    /**
+     * Returns the current working solution.
+     * It must not be modified directly,
+     * but only through {@link #execute(Move)} or {@link #executeTemporarily(Move, Function)}.
+     * Direct modifications will cause the solver to be in an inconsistent state and likely throw an exception later on.
+     *
+     * @return the current working solution
+     */
+    Solution_ getWorkingSolution();
+
+    /**
+     * Long-running command implementations should check this periodically and terminate early if it returns true.
+     * Otherwise the terminations configured by the user will have no effect,
+     * as the solver can only terminate itself when a command has ended.
+     *
+     * @return true if the solver has requested the phase to terminate,
+     *         for example because the time limit has been reached.
+     */
+    boolean isPhaseTerminated();
+
+    /**
+     * As defined by {@link #execute(Move, boolean)},
+     * but with the guarantee of a fresh score.
+     */
+    default void execute(Move<Solution_> move) {
+        execute(move, true);
+    }
+
+    /**
+     * Executes the given move and updates the working solution,
+     * optionally without recalculating the score for performance reasons.
+     *
+     * @param move the move to execute
+     * @param guaranteeFreshScore if true, the score of {@link #getWorkingSolution()} after this method returns
+     *        is guaranteed to be up-to-date;
+     *        otherwise it may be stale as the solver will skip recalculating it for performance reasons.
+     */
+    void execute(Move<Solution_> move, boolean guaranteeFreshScore);
+
+    /**
+     * As defined by {@link #executeTemporarily(Move, Function, boolean)},
+     * with the guarantee of a fresh score.
+     */
+    default <Result_> @Nullable Result_ executeTemporarily(Move<Solution_> move,
+            Function<Solution_, @Nullable Result_> temporarySolutionConsumer) {
+        return executeTemporarily(move, temporarySolutionConsumer, true);
+    }
+
+    /**
+     * Executes the given move temporarily and returns the result of the given consumer.
+     * The working solution is reverted to its original state after the consumer has been executed,
+     * optionally without recalculating the score for performance reasons.
+     *
+     * @param move the move to execute temporarily
+     * @param temporarySolutionConsumer the consumer to execute with the temporarily modified solution;
+     *        this solution must not be modified any further.
+     * @param guaranteeFreshScore if true, the score of {@link #getWorkingSolution()} after this method returns
+     *        is guaranteed to be up-to-date;
+     *        otherwise it may be stale as the solver will skip recalculating it for performance reasons.
+     * @return the result of the consumer
+     */
+    <Result_> @Nullable Result_ executeTemporarily(Move<Solution_> move,
+            Function<Solution_, @Nullable Result_> temporarySolutionConsumer, boolean guaranteeFreshScore);
+
+    /**
+     * As defined by {@link Rebaser#rebase(Object)}, but for the working solution of this context.
+     */
+    <T> @Nullable T lookupWorkingObject(@Nullable T original);
+
+}

core/src/main/java/ai/timefold/solver/core/impl/move/AbstractMove.java

@@ -27,7 +27,7 @@ public final String describe() {
         var metaModels = variableMetaModels();
         var substring = switch (metaModels.size()) {
             case 0 -> "";
-            case 1 -> OPENING_PARENTHESES + getVariableDescriptor(metaModels.get(0)).getSimpleEntityAndVariableName()
+            case 1 -> OPENING_PARENTHESES + getVariableDescriptor(metaModels.getFirst()).getSimpleEntityAndVariableName()
                     + CLOSING_PARENTHESES;
             default -> {
                 var stringBuilder = new StringBuilder()
@@ -37,7 +37,7 @@ public final String describe() {
                     if (first) {
                         first = false;
                     } else {
-                        stringBuilder.append(", ");
+                        stringBuilder.append(",");
                     }
                     stringBuilder.append(getVariableDescriptor(variableMetaModel).getSimpleEntityAndVariableName());
                 }
@@ -48,8 +48,6 @@ public final String describe() {
         return getClass().getSimpleName() + substring;
     }
 
-    public abstract String toString();
-
     public abstract List<? extends GenuineVariableMetaModel<Solution_, ?, ?>> variableMetaModels();
 
     @SuppressWarnings("unchecked")

core/src/main/java/ai/timefold/solver/core/impl/move/MoveDirector.java

@@ -1,7 +1,9 @@
 package ai.timefold.solver.core.impl.move;
 
+import java.util.List;
 import java.util.Objects;
 import java.util.function.BiFunction;
+import java.util.function.Function;
 
 import ai.timefold.solver.core.api.score.Score;
 import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor;
@@ -16,6 +18,7 @@
 import ai.timefold.solver.core.preview.api.domain.metamodel.ElementPosition;
 import ai.timefold.solver.core.preview.api.domain.metamodel.GenuineVariableMetaModel;
 import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningListVariableMetaModel;
+import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningSolutionMetaModel;
 import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningVariableMetaModel;
 import ai.timefold.solver.core.preview.api.domain.metamodel.UnassignedElement;
 import ai.timefold.solver.core.preview.api.move.Move;
@@ -46,6 +49,11 @@ public MoveDirector(InnerScoreDirector<Solution_, Score_> scoreDirector) {
         }
     }
 
+    @Override
+    public PlanningSolutionMetaModel<Solution_> getSolutionMetaModel() {
+        return backingScoreDirector.getSolutionDescriptor().getMetaModel();
+    }
+
     @Override
     public final <Entity_, Value_> void assignValueAndAdd(
             PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel, Value_ planningValue,
@@ -65,6 +73,26 @@ public final <Entity_, Value_> void assignValueAndAdd(
         externalScoreDirector.triggerVariableListeners();
     }
 
+    @Override
+    public <Entity_, Value_> void assignValuesAndAdd(
+            PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel, List<Value_> values,
+            Entity_ destinationEntity, int destinationIndex) {
+        var variableDescriptor =
+                ((DefaultPlanningListVariableMetaModel<Solution_, Entity_, Value_>) variableMetaModel).variableDescriptor();
+        for (var value : values) {
+            if (!(getPositionOf(variableMetaModel, value) instanceof UnassignedElement)) {
+                throw new IllegalStateException("Cannot assign an already assigned value (%s).".formatted(value));
+            }
+            externalScoreDirector.beforeListVariableElementAssigned(variableDescriptor, value);
+        }
+        externalScoreDirector.beforeListVariableChanged(variableDescriptor, destinationEntity, 0, 0);
+        variableDescriptor.getValue(destinationEntity).addAll(destinationIndex, values);
+        externalScoreDirector.afterListVariableChanged(variableDescriptor, destinationEntity, 0, values.size());
+        for (var value : values) {
+            externalScoreDirector.afterListVariableElementAssigned(variableDescriptor, value);
+        }
+    }
+
     @Override
     public final <Entity_, Value_> void assignValueAndSet(
             PlanningListVariableMetaModel<Solution_, Entity_, Value_> variableMetaModel, Value_ planningValue,
@@ -273,10 +301,24 @@ public <Entity_, Value_> boolean isValueInRange(GenuineVariableMetaModel<Solutio
 
     /**
      * Execute a given move and make sure shadow variables are up to date after that.
+     * Does not run a score calculation.
      */
     public final void execute(Move<Solution_> move) {
+        execute(move, false);
+    }
+
+    /**
+     * Execute a given move and make sure shadow variables are up to date after that.
+     *
+     * @param guaranteeFreshScore if true, a score calculation is forced after executing the move,
+     *        to ensure the score is up to date.
+     */
+    public final void execute(Move<Solution_> move, boolean guaranteeFreshScore) {
         move.execute(this);
         externalScoreDirector.triggerVariableListeners();
+        if (guaranteeFreshScore) {
+            backingScoreDirector.calculateScore();
+        }
     }
 
     public final InnerScore<Score_> executeTemporary(Move<Solution_> move) {
@@ -289,11 +331,22 @@ public final InnerScore<Score_> executeTemporary(Move<Solution_> move) {
 
     public <Result_> Result_ executeTemporary(Move<Solution_> move,
             TemporaryMovePostprocessor<Solution_, Score_, Result_> postprocessor) {
+        try (var ephemeralMoveDirector = ephemeral()) {
+            ephemeralMoveDirector.execute(move);
+            var score = backingScoreDirector.calculateScore();
+            return postprocessor.apply(score, ephemeralMoveDirector.createUndoMove());
+        }
+    }
+
+    public <Result_> @Nullable Result_ executeTemporary(Move<Solution_> move,
+            Function<Solution_, @Nullable Result_> postprocessor, boolean guaranteeFreshScore) {
         var ephemeralMoveDirector = ephemeral();
-        ephemeralMoveDirector.execute(move);
-        var score = backingScoreDirector.calculateScore();
-        var result = postprocessor.apply(score, ephemeralMoveDirector.createUndoMove());
+        ephemeralMoveDirector.execute(move, true);
+        var result = postprocessor.apply(backingScoreDirector.getWorkingSolution());
         ephemeralMoveDirector.close(); // This undoes the move.
+        if (guaranteeFreshScore) {
+            backingScoreDirector.calculateScore();
+        }
         return result;
     }
 

core/src/main/java/ai/timefold/solver/core/impl/phase/custom/CustomPhaseCommand.java

@@ -84,9 +84,9 @@ public void phaseStarted(AbstractPhaseScope<Solution_> phaseScope) {
     }
 
     private void doStep(CustomStepScope<Solution_> stepScope, PhaseCommand<Solution_> customPhaseCommand) {
-        var scoreDirector = stepScope.getScoreDirector();
-        customPhaseCommand.changeWorkingSolution(scoreDirector,
+        var commandContext = new DefaultPhaseCommandContext<>(stepScope.getMoveDirector(),
                 () -> phaseTermination.isPhaseTerminated(stepScope.getPhaseScope()));
+        customPhaseCommand.changeWorkingSolution(commandContext);
         calculateWorkingStepScore(stepScope, customPhaseCommand);
         var solver = stepScope.getPhaseScope().getSolverScope().getSolver();
         solver.getBestSolutionRecaller().processWorkingSolutionDuringStep(stepScope);

core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java

@@ -0,0 +1,57 @@
+package ai.timefold.solver.core.impl.phase.custom;
+
+import java.util.Objects;
+import java.util.function.BooleanSupplier;
+import java.util.function.Function;
+
+import ai.timefold.solver.core.api.solver.phase.PhaseCommandContext;
+import ai.timefold.solver.core.impl.move.MoveDirector;
+import ai.timefold.solver.core.preview.api.domain.metamodel.PlanningSolutionMetaModel;
+import ai.timefold.solver.core.preview.api.move.Move;
+
+import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
+
+@NullMarked
+final class DefaultPhaseCommandContext<Solution_> implements PhaseCommandContext<Solution_> {
+
+    private final MoveDirector<Solution_, ?> moveDirector;
+    private final BooleanSupplier isPhaseTerminated;
+
+    public DefaultPhaseCommandContext(MoveDirector<Solution_, ?> moveDirector, BooleanSupplier isPhaseTerminated) {
+        this.moveDirector = Objects.requireNonNull(moveDirector);
+        this.isPhaseTerminated = Objects.requireNonNull(isPhaseTerminated);
+    }
+
+    @Override
+    public PlanningSolutionMetaModel<Solution_> getSolutionMetaModel() {
+        return moveDirector.getSolutionMetaModel();
+    }
+
+    @Override
+    public Solution_ getWorkingSolution() {
+        return moveDirector.getScoreDirector().getWorkingSolution();
+    }
+
+    @Override
+    public boolean isPhaseTerminated() {
+        return isPhaseTerminated.getAsBoolean();
+    }
+
+    @Override
+    public <T> T lookupWorkingObject(T original) {
+        return moveDirector.rebase(original);
+    }
+
+    @Override
+    public void execute(Move<Solution_> move, boolean guaranteeFreshScore) {
+        moveDirector.execute(move, guaranteeFreshScore);
+    }
+
+    @Override
+    public @Nullable <Result_> Result_ executeTemporarily(Move<Solution_> move,
+            Function<Solution_, @Nullable Result_> temporarySolutionConsumer, boolean guaranteeFreshScore) {
+        return moveDirector.executeTemporary(move, temporarySolutionConsumer, guaranteeFreshScore);
+    }
+
+}