Skip to content

Commit

Permalink
fix: Use Named Solver fields and method parameters to determine list …
Browse files Browse the repository at this point in the history
…of solver names in Quarkus (#1308)

If a user is using named solvers, they should have `@Named` annotated
fields or method parameters corresponding to the name solver. If they
don't have such fields, said named solvers are not accessible anyways.

Thus, Set(Named injected Solver types) = Set(accessible Solver names).
So instead of using the keys of the SolverBuildTimeConfig as the
set of solver names, we can use Set(Named injected Solver types)
instead. This removes the need to include runtime properties
in SolverBuildTimeConfig, since the map keys are no longer
used to determine names.

As a consequence, there is a behaviour change in the very contrived
case of "exactly one named solver in properties (no default), no Named annotations,
injected default solver". Since there was no Named annotations,
the (empty) default solver properties will be checked instead of the single
named solver properties. I consider the old behaviour to be a bug
(named properties should not affect default properties),
but the tests expect that behaviour.
  • Loading branch information
Christopher-Chianelli authored Jan 10, 2025
1 parent 4316a5e commit d3ae68f
Show file tree
Hide file tree
Showing 19 changed files with 407 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@

import java.util.concurrent.ExecutionException;

import jakarta.inject.Inject;
import jakarta.inject.Named;

import ai.timefold.solver.benchmark.quarkus.testdata.normal.constraints.TestdataQuarkusConstraintProvider;
import ai.timefold.solver.benchmark.quarkus.testdata.normal.domain.TestdataQuarkusEntity;
import ai.timefold.solver.benchmark.quarkus.testdata.normal.domain.TestdataQuarkusSolution;
import ai.timefold.solver.core.api.solver.SolverManager;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
Expand Down Expand Up @@ -35,6 +39,14 @@ class TimefoldBenchmarkProcessorMultipleSolversConfigTest {
When defining multiple solvers, the benchmark feature is not enabled.
Consider using separate <solverBenchmark> instances for evaluating different solver configurations."""));

@Inject
@Named("solver1")
SolverManager<?, ?> solverManager1;

@Inject
@Named("solver2")
SolverManager<?, ?> solverManager2;

@Test
void benchmark() throws ExecutionException, InterruptedException {
fail("It won't be executed");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

import java.util.Arrays;
import java.util.List;
import java.util.Set;

import jakarta.inject.Named;

import ai.timefold.solver.core.api.domain.constraintweight.ConstraintConfigurationProvider;
import ai.timefold.solver.core.api.domain.constraintweight.ConstraintWeight;
Expand Down Expand Up @@ -32,11 +35,18 @@
import ai.timefold.solver.core.api.score.calculator.EasyScoreCalculator;
import ai.timefold.solver.core.api.score.calculator.IncrementalScoreCalculator;
import ai.timefold.solver.core.api.score.stream.ConstraintProvider;
import ai.timefold.solver.core.api.solver.SolverFactory;
import ai.timefold.solver.core.api.solver.SolverManager;
import ai.timefold.solver.core.config.solver.SolverConfig;
import ai.timefold.solver.core.config.solver.SolverManagerConfig;

import org.jboss.jandex.DotName;

public final class DotNames {
// Jakarta classes
static final DotName NAMED = DotName.createSimple(Named.class);

// Timefold classes
static final DotName PLANNING_SOLUTION = DotName.createSimple(PlanningSolution.class.getName());
static final DotName PLANNING_ENTITY_COLLECTION_PROPERTY =
DotName.createSimple(PlanningEntityCollectionProperty.class.getName());
Expand Down Expand Up @@ -75,6 +85,11 @@ public final class DotNames {
static final DotName CASCADING_UPDATE_SHADOW_VARIABLE =
DotName.createSimple(CascadingUpdateShadowVariable.class.getName());

static final DotName SOLVER_CONFIG = DotName.createSimple(SolverConfig.class.getName());
static final DotName SOLVER_MANAGER_CONFIG = DotName.createSimple(SolverManagerConfig.class.getName());
static final DotName SOLVER_FACTORY = DotName.createSimple(SolverFactory.class.getName());
static final DotName SOLVER_MANAGER = DotName.createSimple(SolverManager.class.getName());

// Need to use String since timefold-solver-test is not on the compile classpath
static final DotName CONSTRAINT_VERIFIER =
DotName.createSimple("ai.timefold.solver.test.api.score.stream.ConstraintVerifier");
Expand Down Expand Up @@ -121,6 +136,12 @@ public final class DotNames {
CASCADING_UPDATE_SHADOW_VARIABLE
};

static final Set<DotName> SOLVER_INJECTABLE_TYPES = Set.of(
SOLVER_CONFIG,
SOLVER_MANAGER_CONFIG,
SOLVER_FACTORY,
SOLVER_MANAGER);

public enum BeanDefiningAnnotations {
PLANNING_SCORE(DotNames.PLANNING_SCORE, "scoreDefinitionClass"),
PLANNING_SOLUTION(DotNames.PLANNING_SOLUTION, "solutionCloner"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.util.Map;

import ai.timefold.solver.core.config.solver.SolverConfig;
import ai.timefold.solver.quarkus.deployment.config.TimefoldBuildTimeConfig;

import io.quarkus.builder.item.SimpleBuildItem;

Expand All @@ -18,6 +19,10 @@ public SolverConfigBuildItem(Map<String, SolverConfig> solverConfig, GeneratedGi
this.generatedGizmoClasses = generatedGizmoClasses;
}

public boolean isDefaultSolverConfig(String solverName) {
return solverConfigurations.size() <= 1 || TimefoldBuildTimeConfig.DEFAULT_SOLVER_NAME.equals(solverName);
}

public Map<String, SolverConfig> getSolverConfigMap() {
return solverConfigurations;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,25 @@ SolverConfigBuildItem recordAndRegisterBuildTimeBeans(CombinedIndexBuildItem com
BuildProducer<BytecodeTransformerBuildItem> transformers) {
IndexView indexView = combinedIndex.getIndex();

// Step 0 - determine list of names used for injected solver components
var solverNames = new HashSet<String>();
var solverConfigMap = new HashMap<String, SolverConfig>();
for (var namedItem : indexView.getAnnotations(DotNames.NAMED)) {
var target = namedItem.target();
DotName type = switch (target.kind()) {
case CLASS -> target.asClass().name();
case FIELD -> target.asField().type().name();
case METHOD_PARAMETER -> target.asMethodParameter().type().name();
case RECORD_COMPONENT -> target.asRecordComponent().type().name();
case TYPE, METHOD -> null;
};
if (type != null && DotNames.SOLVER_INJECTABLE_TYPES.contains(type)) {
var annotationValue = namedItem.value();
var value = (annotationValue != null) ? annotationValue.asString() : "";
solverNames.add(value);
}
}

// Only skip this extension if everything is missing. Otherwise, if some parts are missing, fail fast later.
if (indexView.getAnnotations(DotNames.PLANNING_SOLUTION).isEmpty()
&& indexView.getAnnotations(DotNames.PLANNING_ENTITY).isEmpty()) {
Expand All @@ -194,24 +213,24 @@ SolverConfigBuildItem recordAndRegisterBuildTimeBeans(CombinedIndexBuildItem com
+ "application.properties entries (quarkus.index-dependency.<name>.group-id"
+ " and quarkus.index-dependency.<name>.artifact-id).");
additionalBeans.produce(new AdditionalBeanBuildItem(UnavailableTimefoldBeanProvider.class));
Map<String, SolverConfig> solverConfigMap = new HashMap<>();
this.timefoldBuildTimeConfig.solver().keySet().forEach(solverName -> solverConfigMap.put(solverName, null));
solverNames.forEach(solverName -> solverConfigMap.put(solverName, null));
return new SolverConfigBuildItem(solverConfigMap, null);
}

// Quarkus extensions must always use getContextClassLoader()
// Internally, Timefold defaults the ClassLoader to getContextClassLoader() too
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
var classLoader = Thread.currentThread().getContextClassLoader();
TimefoldRecorder.assertNoUnmatchedProperties(solverNames,
timefoldBuildTimeConfig.solver().keySet());

Map<String, SolverConfig> solverConfigMap = new HashMap<>();
// Step 1 - create all SolverConfig
// If the config map is empty, we build the config using the default solver name
if (timefoldBuildTimeConfig.solver().isEmpty()) {
if (solverNames.isEmpty()) {
solverConfigMap.put(TimefoldBuildTimeConfig.DEFAULT_SOLVER_NAME,
createSolverConfig(classLoader, TimefoldBuildTimeConfig.DEFAULT_SOLVER_NAME));
} else {
// One config per solver mapped name
this.timefoldBuildTimeConfig.solver().keySet().forEach(solverName -> solverConfigMap.put(solverName,
solverNames.forEach(solverName -> solverConfigMap.put(solverName,
createSolverConfig(classLoader, solverName)));
}

Expand Down Expand Up @@ -558,23 +577,23 @@ private SolverConfig loadSolverConfig(IndexView indexView,
@Record(RUNTIME_INIT)
void recordAndRegisterRuntimeBeans(TimefoldRecorder recorder, RecorderContext recorderContext,
BuildProducer<SyntheticBeanBuildItem> syntheticBeanBuildItemBuildProducer,
SolverConfigBuildItem solverConfigBuildItem,
TimefoldRuntimeConfig runtimeConfig) {
SolverConfigBuildItem solverConfigBuildItem) {
// Skip this extension if everything is missing.
if (solverConfigBuildItem.getGeneratedGizmoClasses() == null) {
return;
}

recorder.assertNoUnmatchedRuntimeProperties(solverConfigBuildItem.getSolverConfigMap().keySet());
// Using the same name for synthetic beans is impossible, even if they are different types.
// Therefore, we allow only the injection of SolverManager, except for the default solver,
// which can inject all resources to be retro-compatible.
solverConfigBuildItem.getSolverConfigMap().forEach((key, value) -> {
if (timefoldBuildTimeConfig.isDefaultSolverConfig(key)) {
if (solverConfigBuildItem.isDefaultSolverConfig(key)) {
// The two configuration resources are required for DefaultTimefoldBeanProvider
// to produce all available managed beans for the default solver.
syntheticBeanBuildItemBuildProducer.produce(SyntheticBeanBuildItem.configure(SolverConfig.class)
.scope(Singleton.class)
.supplier(recorder.solverConfigSupplier(key, value, runtimeConfig,
.supplier(recorder.solverConfigSupplier(key, value,
GizmoMemberAccessorEntityEnhancer.getGeneratedGizmoMemberAccessorMap(recorderContext,
solverConfigBuildItem
.getGeneratedGizmoClasses().generatedGizmoMemberAccessorClassSet),
Expand All @@ -588,11 +607,12 @@ void recordAndRegisterRuntimeBeans(TimefoldRecorder recorder, RecorderContext re
SolverManagerConfig solverManagerConfig = new SolverManagerConfig();
syntheticBeanBuildItemBuildProducer.produce(SyntheticBeanBuildItem.configure(SolverManagerConfig.class)
.scope(Singleton.class)
.supplier(recorder.solverManagerConfig(solverManagerConfig, runtimeConfig))
.supplier(recorder.solverManagerConfig(solverManagerConfig))
.setRuntimeInit()
.defaultBean()
.done());
} else {
}
if (!TimefoldBuildTimeConfig.DEFAULT_SOLVER_NAME.equals(key)) {
// The default SolverManager instance is generated by DefaultTimefoldBeanProvider
syntheticBeanBuildItemBuildProducer.produce(
// We generate all required resources only to create a SolverManager and set it as managed bean
Expand All @@ -602,7 +622,7 @@ void recordAndRegisterRuntimeBeans(TimefoldRecorder recorder, RecorderContext re
Type.create(DotName.createSimple(value.getSolutionClass().getName()),
Type.Kind.CLASS),
TypeVariable.create(Object.class.getName())))
.supplier(recorder.solverManager(key, value, runtimeConfig,
.supplier(recorder.solverManager(key, value,
GizmoMemberAccessorEntityEnhancer.getGeneratedGizmoMemberAccessorMap(recorderContext,
solverConfigBuildItem
.getGeneratedGizmoClasses().generatedGizmoMemberAccessorClassSet),
Expand All @@ -622,11 +642,10 @@ public void recordAndRegisterDevUIBean(
TimefoldDevUIRecorder devUIRecorder,
RecorderContext recorderContext,
SolverConfigBuildItem solverConfigBuildItem,
TimefoldRuntimeConfig runtimeConfig,
BuildProducer<SyntheticBeanBuildItem> syntheticBeans) {
syntheticBeans.produce(SyntheticBeanBuildItem.configure(DevUISolverConfig.class)
.scope(ApplicationScoped.class)
.supplier(devUIRecorder.solverConfigSupplier(solverConfigBuildItem.getSolverConfigMap(), runtimeConfig,
.supplier(devUIRecorder.solverConfigSupplier(solverConfigBuildItem.getSolverConfigMap(),
GizmoMemberAccessorEntityEnhancer.getGeneratedGizmoMemberAccessorMap(recorderContext,
solverConfigBuildItem
.getGeneratedGizmoClasses().generatedGizmoMemberAccessorClassSet),
Expand Down Expand Up @@ -701,10 +720,6 @@ private void applySolverProperties(IndexView indexView, String solverName, Solve
applyScoreDirectorFactoryProperties(indexView, solverConfig);

// Override the current configuration with values from the solver properties
timefoldBuildTimeConfig.getSolverConfig(solverName).flatMap(SolverBuildTimeConfig::environmentMode)
.ifPresent(solverConfig::setEnvironmentMode);
timefoldBuildTimeConfig.getSolverConfig(solverName).flatMap(SolverBuildTimeConfig::daemon)
.ifPresent(solverConfig::setDaemon);
timefoldBuildTimeConfig.getSolverConfig(solverName).flatMap(SolverBuildTimeConfig::domainAccessType)
.ifPresent(solverConfig::setDomainAccessType);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@

import ai.timefold.solver.core.api.domain.common.DomainAccessType;
import ai.timefold.solver.core.api.score.stream.ConstraintStreamImplType;
import ai.timefold.solver.core.config.solver.EnvironmentMode;
import ai.timefold.solver.core.config.solver.SolverConfig;
import ai.timefold.solver.core.config.solver.termination.TerminationConfig;
import ai.timefold.solver.quarkus.config.SolverRuntimeConfig;
import ai.timefold.solver.quarkus.config.TerminationRuntimeConfig;

import io.quarkus.runtime.annotations.ConfigGroup;

Expand All @@ -25,47 +22,23 @@ public interface SolverBuildTimeConfig {
* A classpath resource to read the specific solver configuration XML.
* If this property isn't specified, that solverConfig.xml is optional.
*/
// Build time - classes in the SolverConfig are visited by SolverConfig.visitReferencedClasses
// which generates the constructor of classes used by Quarkus
Optional<String> solverConfigXml();

/**
* Enable runtime assertions to detect common bugs in your implementation during development.
* Defaults to {@link EnvironmentMode#REPRODUCIBLE}.
*/
Optional<EnvironmentMode> environmentMode();

/**
* Enable daemon mode. In daemon mode, non-early termination pauses the solver instead of stopping it,
* until the next problem fact change arrives.
* This is often useful for real-time planning.
* Defaults to "false".
*/
Optional<Boolean> daemon();

/**
* Determines how to access the fields and methods of domain classes.
* Defaults to {@link DomainAccessType#GIZMO}.
*/
// Build time - GIZMO classes are only generated if at least one solver
// has domain access type GIZMO
Optional<DomainAccessType> domainAccessType();

/**
* Note: this setting is only available
* for <a href="https://timefold.ai/docs/timefold-solver/latest/enterprise-edition/enterprise-edition">Timefold Solver
* Enterprise Edition</a>.
* Enable multithreaded solving for a single problem, which increases CPU consumption.
* Defaults to {@value SolverConfig#MOVE_THREAD_COUNT_NONE}.
* Other options include {@value SolverConfig#MOVE_THREAD_COUNT_AUTO}, a number
* or formula based on the available processor count.
*/
Optional<String> moveThreadCount();

/**
* Configuration properties regarding {@link TerminationConfig}.
*/
TerminationRuntimeConfig termination();

/**
* Enable the Nearby Selection quick configuration.
*/
// Build time - visited by SolverConfig.visitReferencedClasses
// which generates the constructor used by Quarkus
Optional<Class<?>> nearbyDistanceMeterClass();

/**
Expand All @@ -86,5 +59,6 @@ public interface SolverBuildTimeConfig {
* will no longer be triggered.
* Defaults to "false".
*/
// Build time - modifies the ConstraintProvider class if set
Optional<Boolean> constraintStreamAutomaticNodeSharing();
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ public interface TimefoldBuildTimeConfig {
@WithUnnamedKey(DEFAULT_SOLVER_NAME)
Map<String, SolverBuildTimeConfig> solver();

default boolean isDefaultSolverConfig(String solverName) {
// 1 - No solver configuration, which means we will use a default empty SolverConfig and default Solver name
// 2 - Only one solve config. It will be the default one.
return solver().isEmpty() || solver().size() == 1 && getSolverConfig(solverName).isPresent();
}

default Optional<SolverBuildTimeConfig> getSolverConfig(String solverName) {
return Optional.ofNullable(solver().get(solverName));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

import jakarta.inject.Inject;
import jakarta.inject.Named;

import ai.timefold.solver.core.api.solver.SolverManager;
import ai.timefold.solver.quarkus.testdata.dummy.DummyTestdataQuarkusEasyScoreCalculator;
import ai.timefold.solver.quarkus.testdata.dummy.DummyTestdataQuarkusIncrementalScoreCalculator;
import ai.timefold.solver.quarkus.testdata.dummy.DummyTestdataQuarkusShadowVariableEasyScoreCalculator;
Expand Down Expand Up @@ -226,6 +230,14 @@ class TimefoldProcessorMultipleSolversInvalidConstraintClassTest {
.hasMessageContaining(
"Unused classes ([ai.timefold.solver.quarkus.testdata.dummy.DummyTestdataQuarkusShadowVariableIncrementalScoreCalculator]) that implements IncrementalScoreCalculator were found."));

@Inject
@Named("solver1")
SolverManager<?, ?> solverManager1;

@Inject
@Named("solver2")
SolverManager<?, ?> solverManager2;

@Test
void test() {
fail("Should not call this method.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

import jakarta.inject.Inject;
import jakarta.inject.Named;

import ai.timefold.solver.core.api.solver.SolverManager;
import ai.timefold.solver.quarkus.testdata.normal.domain.TestdataQuarkusSolution;

import org.jboss.shrinkwrap.api.ShrinkWrap;
Expand All @@ -26,6 +30,14 @@ class TimefoldProcessorMultipleSolversInvalidEntityClassTest {
.hasMessageContaining(
"No classes were found with a @PlanningEntity annotation."));

@Inject
@Named("solver1")
SolverManager<?, ?> solverManager1;

@Inject
@Named("solver2")
SolverManager<?, ?> solverManager2;

@Test
void test() {
fail("Should not call this method.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.fail;

import jakarta.inject.Inject;
import jakarta.inject.Named;

import ai.timefold.solver.core.api.solver.SolverManager;
import ai.timefold.solver.quarkus.testdata.chained.domain.TestdataChainedQuarkusSolution;
import ai.timefold.solver.quarkus.testdata.normal.constraints.TestdataQuarkusConstraintProvider;
import ai.timefold.solver.quarkus.testdata.normal.domain.TestdataQuarkusEntity;
Expand Down Expand Up @@ -78,6 +82,14 @@ class TimefoldProcessorMultipleSolversInvalidSolutionClassTest {
.hasMessageContaining(
"Unused classes ([ai.timefold.solver.quarkus.testdata.chained.domain.TestdataChainedQuarkusSolution]) found with a @PlanningSolution annotation."));

@Inject
@Named("solver1")
SolverManager<?, ?> solverManager1;

@Inject
@Named("solver2")
SolverManager<?, ?> solverManager2;

@Test
void test() {
fail("Should not call this method.");
Expand Down
Loading

0 comments on commit d3ae68f

Please sign in to comment.