fix: mark ModelConvertor parameters and returns as non-null (#2507)

service/definition/src/main/java/ai/timefold/solver/service/definition/api/ModelConvertor.java

@@ -7,6 +7,8 @@
 import ai.timefold.solver.service.definition.api.domain.ModelRequest;
 import ai.timefold.solver.service.definition.api.domain.ModelResponse;
 
+import org.jspecify.annotations.NullMarked;
+
 /**
  * The convertor of:
  * <ul>
@@ -15,6 +17,9 @@
  * <li>The <code>SolverModel</code> representing a planning solution into the <code>ModelOutput</code> (usually forming a part
  * of the {@link ModelResponse}).</li>
  * </ul>
+ * <p>
+ * All method parameters and return values are non-null unless explicitly marked otherwise.
+ * Absence of a previous model output is expressed with an empty {@link Optional}, not a null reference.
  *
  * @param <Score_> The solver model concrete {@link ai.timefold.solver.core.api.domain.solution.PlanningScore} type.
  * @param <ModelInput_> The type of the model input part of the {@link ModelRequest}.
@@ -24,6 +29,7 @@
  *        by Timefold solver.
  * @param <ModelOutput_> The type of the model output part of the {@link ModelResponse}.
  */
+@NullMarked
 public non-sealed interface ModelConvertor<Score_ extends Score<Score_>, ModelInput_ extends ModelInput, ModelConfigurationOverrides_ extends ModelConfigOverrides, SolverModel_ extends SolverModel<Score_>, ModelOutput_ extends ModelOutput>
         extends ModelConvertorBase {
 
@@ -35,15 +41,21 @@ public non-sealed interface ModelConvertor<Score_ extends Score<Score_>, ModelIn
      * the solving process. In such cases, {@code lastModelOutput} represents the last stored output before the failure,
      * allowing the method to restore or reuse relevant state as needed.
      *
-     * @param modelInput the model input part of the request, containing the data to be converted
-     * @param modelConfig the model configuration, including any overrides to be applied
+     * @param modelInput the model input part of the request, containing the data to be converted; never null
+     * @param modelConfig the model configuration, including any overrides to be applied; never null
      * @param lastModelOutput an {@link Optional} containing the last stored model output before a failure, or empty if not
-     *        recovering
-     * @return the solver model instance to be used by the Timefold solver
+     *        recovering; never null
+     * @return the solver model instance to be used by the Timefold solver; never null
      */
     SolverModel_ toSolverModel(ModelInput_ modelInput, ModelConfig<ModelConfigurationOverrides_> modelConfig,
             Optional<ModelOutput_> lastModelOutput);
 
+    /**
+     * Converts the given {@link SolverModel_} into a {@link ModelOutput_}.
+     *
+     * @param solverModel the planning solution produced by the solver; never null
+     * @return the model output; never null
+     */
     ModelOutput_ toModelOutput(SolverModel_ solverModel);
 
     /**
@@ -54,9 +66,9 @@ SolverModel_ toSolverModel(ModelInput_ modelInput, ModelConfig<ModelConfiguratio
      * <p>
      * The updated {@link ModelInput_} can be used as a basis for new datasets by applying external changes.
      *
-     * @param modelInput The model input to be updated.
-     * @param modelOutput The model output containing changes to be applied to the model input.
-     * @return Reference to the updated {@link ModelInput_}.
+     * @param modelInput The model input to be updated; never null
+     * @param modelOutput The model output containing changes to be applied to the model input; never null
+     * @return Reference to the updated {@link ModelInput_}; never null
      */
     ModelInput_ applyOutputToInput(ModelInput_ modelInput, ModelOutput_ modelOutput);
 }

service/definition/src/test/java/ai/timefold/solver/service/definition/api/ModelConvertorNullabilityTest.java

@@ -0,0 +1,14 @@
+package ai.timefold.solver.service.definition.api;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.jspecify.annotations.NullMarked;
+import org.junit.jupiter.api.Test;
+
+class ModelConvertorNullabilityTest {
+
+    @Test
+    void modelConvertorIsNullMarked() {
+        assertThat(ModelConvertor.class.getAnnotation(NullMarked.class)).isNotNull();
+    }
+}

service/quarkus/deployment/src/test/java/ai/timefold/solver/service/quarkus/deployment/defaults/AbstractTrivialModelConvertorTest.java

@@ -0,0 +1,86 @@
+package ai.timefold.solver.service.quarkus.deployment.defaults;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNullPointerException;
+
+import java.util.Optional;
+
+import ai.timefold.solver.core.api.domain.solution.ConstraintWeightOverrides;
+import ai.timefold.solver.core.api.score.SimpleScore;
+import ai.timefold.solver.service.definition.api.ModelInput;
+import ai.timefold.solver.service.definition.api.ModelOutput;
+import ai.timefold.solver.service.definition.api.SolverModel;
+import ai.timefold.solver.service.definition.api.domain.ModelConfig;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class AbstractTrivialModelConvertorTest {
+
+    private AbstractTrivialModelConvertor<SimpleScore, TestdataModel, EmptyModelConfigOverrides, TestdataModel, TestdataModel> convertor;
+    private TestdataModel model;
+    private ModelConfig<EmptyModelConfigOverrides> modelConfig;
+
+    @BeforeEach
+    void setUp() {
+        convertor = new AbstractTrivialModelConvertor<>() {
+        };
+        model = new TestdataModel();
+        modelConfig = ModelConfig.empty();
+    }
+
+    @Test
+    void toSolverModelRejectsNullArguments() {
+        assertThatNullPointerException()
+                .isThrownBy(() -> convertor.toSolverModel(null, modelConfig, Optional.empty()))
+                .withMessage("modelInput");
+        assertThatNullPointerException()
+                .isThrownBy(() -> convertor.toSolverModel(model, null, Optional.empty()))
+                .withMessage("modelConfig");
+        assertThatNullPointerException()
+                .isThrownBy(() -> convertor.toSolverModel(model, modelConfig, null))
+                .withMessage("lastModelOutput");
+    }
+
+    @Test
+    void toModelOutputRejectsNullSolverModel() {
+        assertThatNullPointerException()
+                .isThrownBy(() -> convertor.toModelOutput(null))
+                .withMessage("solverModel");
+    }
+
+    @Test
+    void applyOutputToInputRejectsNullArguments() {
+        assertThatNullPointerException()
+                .isThrownBy(() -> convertor.applyOutputToInput(null, model))
+                .withMessage("modelInput");
+        assertThatNullPointerException()
+                .isThrownBy(() -> convertor.applyOutputToInput(model, null))
+                .withMessage("modelOutput");
+    }
+
+    @Test
+    void applyOutputToInputReturnsModelOutputWhenTypesMatch() {
+        TestdataModel modelOutput = new TestdataModel();
+
+        assertThat(convertor.applyOutputToInput(model, modelOutput)).isSameAs(modelOutput);
+    }
+
+    @Test
+    void toSolverModelUsesEmptyOptionalWhenNoPreviousOutput() {
+        assertThat(convertor.toSolverModel(model, modelConfig, Optional.empty())).isSameAs(model);
+    }
+
+    private static final class TestdataModel implements ModelInput, ModelOutput, SolverModel<SimpleScore> {
+
+        @Override
+        public SimpleScore getScore() {
+            return null;
+        }
+
+        @Override
+        public ConstraintWeightOverrides<SimpleScore> getConstraintWeightOverrides() {
+            return null;
+        }
+    }
+}

service/quarkus/runtime/src/main/java/ai/timefold/solver/service/quarkus/deployment/defaults/AbstractTrivialModelConvertor.java

@@ -1,5 +1,6 @@
 package ai.timefold.solver.service.quarkus.deployment.defaults;
 
+import java.util.Objects;
 import java.util.Optional;
 
 import ai.timefold.solver.core.api.score.Score;
@@ -12,6 +13,8 @@
 import ai.timefold.solver.service.definition.api.domain.ModelRequest;
 import ai.timefold.solver.service.definition.api.domain.ModelResponse;
 
+import org.jspecify.annotations.NullMarked;
+
 /**
  * Default model convertor for the trivial case when:
  * <ul>
@@ -27,13 +30,17 @@
  * @param <SolverModel_> The type of the {@link ai.timefold.solver.core.api.domain.solution.PlanningSolution} class to be used
  *        by Timefold solver.
  */
+@NullMarked
 public abstract class AbstractTrivialModelConvertor<Score_ extends Score<Score_>, ModelInput_ extends ModelInput, ModelConfigurationOverrides_ extends ModelConfigOverrides, SolverModel_ extends SolverModel<Score_>, ModelOutput_ extends ModelOutput>
         implements
         ModelConvertor<Score_, ModelInput_, ModelConfigurationOverrides_, SolverModel_, ModelOutput_> {
 
     @Override
     public SolverModel_ toSolverModel(ModelInput_ modelInput, ModelConfig<ModelConfigurationOverrides_> modelConfig,
             Optional<ModelOutput_> lastModelOutput) {
+        Objects.requireNonNull(modelInput, "modelInput");
+        Objects.requireNonNull(modelConfig, "modelConfig");
+        Objects.requireNonNull(lastModelOutput, "lastModelOutput");
         if (lastModelOutput.isPresent() && !(lastModelOutput.get().getClass()).equals(modelInput.getClass())) {
             throw new IllegalArgumentException(
                     "Trivial conversion is possible only when modelInput (%s) and modelOutput (%s) are of the same class."
@@ -44,17 +51,20 @@ public SolverModel_ toSolverModel(ModelInput_ modelInput, ModelConfig<ModelConfi
 
     @Override
     public ModelOutput_ toModelOutput(SolverModel_ solverModel) {
+        Objects.requireNonNull(solverModel, "solverModel");
         return (ModelOutput_) solverModel;
     }
 
     @Override
     public ModelInput_ applyOutputToInput(ModelInput_ modelInput, ModelOutput_ modelOutput) {
-        if (modelOutput != null && !modelOutput.getClass().equals(modelInput.getClass())) {
+        Objects.requireNonNull(modelInput, "modelInput");
+        Objects.requireNonNull(modelOutput, "modelOutput");
+        if (!modelOutput.getClass().equals(modelInput.getClass())) {
             throw new IllegalArgumentException(
                     "Trivial conversion is possible only when modelInput (%s) and modelOutput (%s) are of the same class."
                             .formatted(modelInput.getClass(), modelOutput.getClass()));
         }
 
-        return modelOutput != null ? (ModelInput_) modelOutput : modelInput;
+        return (ModelInput_) modelOutput;
     }
 }