Skip to content

Commit

Permalink
chore: Improve error messages from loading YAML files (#637)
Browse files Browse the repository at this point in the history
This PR improves the loading error messages for YAML files. We now
enforce the kebab-case format from version 1.7.0 onwards. This may
affect the Spring Boot module as older versions did not enforce
kebab-case format.
  • Loading branch information
zepfred committed Feb 14, 2024
1 parent 8bdc097 commit 307630c
Show file tree
Hide file tree
Showing 17 changed files with 309 additions and 4 deletions.
5 changes: 5 additions & 0 deletions quarkus-integration/quarkus/deployment/pom.xml
Expand Up @@ -68,6 +68,11 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-config-yaml</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
@@ -0,0 +1,32 @@
package ai.timefold.solver.quarkus;

import static org.junit.jupiter.api.Assertions.assertEquals;

import ai.timefold.solver.quarkus.rest.TestdataQuarkusSolutionConfigResource;
import ai.timefold.solver.quarkus.testdata.normal.constraints.TestdataQuarkusConstraintProvider;
import ai.timefold.solver.quarkus.testdata.normal.domain.TestdataQuarkusEntity;
import ai.timefold.solver.quarkus.testdata.normal.domain.TestdataQuarkusSolution;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

class TimefoldProcessorMultipleSolversYamlTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class,
TestdataQuarkusConstraintProvider.class, TestdataQuarkusSolutionConfigResource.class)
.addAsResource("ai/timefold/solver/quarkus/multiple-solvers/application.yaml", "application.yaml"));

@Test
void solverProperties() {
String resp = RestAssured.get("/solver-config/seconds-spent-limit").asString();
assertEquals("secondsSpentLimit=0.06;secondsSpentLimit=0.12", resp);
}
}
@@ -0,0 +1,57 @@
package ai.timefold.solver.quarkus;

import static org.junit.jupiter.api.Assertions.*;

import java.time.Duration;

import jakarta.inject.Inject;

import ai.timefold.solver.core.api.domain.common.DomainAccessType;
import ai.timefold.solver.core.api.score.buildin.simple.SimpleScore;
import ai.timefold.solver.core.api.solver.SolverFactory;
import ai.timefold.solver.core.config.solver.EnvironmentMode;
import ai.timefold.solver.core.config.solver.SolverConfig;
import ai.timefold.solver.quarkus.testdata.normal.constraints.TestdataQuarkusConstraintProvider;
import ai.timefold.solver.quarkus.testdata.normal.domain.TestdataQuarkusEntity;
import ai.timefold.solver.quarkus.testdata.normal.domain.TestdataQuarkusSolution;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

class TimefoldProcessorSolverYamlTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(TestdataQuarkusEntity.class, TestdataQuarkusSolution.class,
TestdataQuarkusConstraintProvider.class)
.addAsResource("ai/timefold/solver/quarkus/single-solver/application.yaml", "application.yaml"));

@Inject
SolverConfig solverConfig;
@Inject
SolverFactory<TestdataQuarkusSolution> solverFactory;

@Test
void solverProperties() {
assertEquals(EnvironmentMode.FULL_ASSERT, solverConfig.getEnvironmentMode());
assertTrue(solverConfig.getDaemon());
assertEquals("2", solverConfig.getMoveThreadCount());
assertEquals(DomainAccessType.REFLECTION, solverConfig.getDomainAccessType());
assertEquals(null,
solverConfig.getScoreDirectorFactoryConfig().getConstraintStreamImplType());

assertNotNull(solverFactory);
}

@Test
void terminationProperties() {
assertEquals(Duration.ofHours(4), solverConfig.getTerminationConfig().getSpentLimit());
assertEquals(Duration.ofHours(5), solverConfig.getTerminationConfig().getUnimprovedSpentLimit());
assertEquals(SimpleScore.of(0).toString(), solverConfig.getTerminationConfig().getBestScoreLimit());
}
}
@@ -0,0 +1,10 @@
quarkus:
timefold:
solver:
solver1:
termination:
spent-limit: 8s
solver2:
termination:
spent-limit: 4s

@@ -0,0 +1,11 @@
quarkus:
timefold:
solver:
environment-mode: FULL_ASSERT
daemon: true
move-thread-count: 2
domain-access-type: REFLECTION
termination:
spent-limit: 4h
unimproved-spent-limit: 5h
best-score-limit: 0
5 changes: 5 additions & 0 deletions spring-integration/spring-boot-autoconfigure/pom.xml
Expand Up @@ -131,6 +131,11 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.yaml</groupId>
<artifactId>snakeyaml</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
@@ -1,5 +1,7 @@
package ai.timefold.solver.spring.boot.autoconfigure.config;

import static java.util.stream.Collectors.joining;

import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -137,6 +139,19 @@ public void setTermination(TerminationProperties termination) {
}

public void loadProperties(Map<String, Object> properties) {
// Check if the keys are valid
String invalidKeys = properties.entrySet().stream()
.filter(e -> !VALID_FIELD_NAMES_SET.contains(e.getKey()))
.map(Map.Entry::getKey)
.collect(joining(", "));

if (!invalidKeys.isBlank()) {
throw new IllegalStateException("""
The properties [%s] are not valid.
Maybe try changing the property name to kebab-case.
Here is the list of valid properties: %s"""
.formatted(invalidKeys, String.join(", ", VALID_FIELD_NAMES_SET)));
}
properties.forEach(this::loadProperty);
}

Expand All @@ -146,16 +161,16 @@ private void loadProperty(String key, Object value) {
}
switch (key) {
case SOLVER_CONFIG_XML_PROPERTY_NAME:
setSolverConfigXml((String) value);
setSolverConfigXml(value.toString());
break;
case ENVIRONMENT_MODE_PROPERTY_NAME:
setEnvironmentMode(EnvironmentMode.valueOf((String) value));
break;
case DAEMON_PROPERTY_NAME:
setDaemon(Boolean.parseBoolean((String) value));
setDaemon(Boolean.parseBoolean(value.toString()));
break;
case MOVE_THREAD_COUNT_PROPERTY_NAME:
setMoveThreadCount((String) value);
setMoveThreadCount(value.toString());
break;
case DOMAIN_ACCESS_TYPE_PROPERTY_NAME:
setDomainAccessType(DomainAccessType.valueOf((String) value));
Expand Down
@@ -1,7 +1,10 @@
package ai.timefold.solver.spring.boot.autoconfigure.config;

import static java.util.stream.Collectors.joining;

import java.time.Duration;
import java.util.Map;
import java.util.Set;

import org.springframework.boot.convert.DurationStyle;

Expand All @@ -10,6 +13,8 @@ public class TerminationProperties {
private static final String SPENT_LIMIT_PROERTY_NAME = "spent-limit";
private static final String UNIMPROVED_SPENT_LIMIT_PROERTY_NAME = "unimproved-spent-limit";
private static final String BEST_SCORE_LIMIT_PROERTY_NAME = "best-score-limit";
private static final Set<String> VALID_FIELD_NAMES_SET =
Set.of(SPENT_LIMIT_PROERTY_NAME, UNIMPROVED_SPENT_LIMIT_PROERTY_NAME, BEST_SCORE_LIMIT_PROERTY_NAME);

/**
* How long the solver can run.
Expand Down Expand Up @@ -60,6 +65,19 @@ public void setBestScoreLimit(String bestScoreLimit) {
}

public void loadProperties(Map<String, Object> properties) {
// Check if the keys are valid
String invalidKeys = properties.entrySet().stream()
.filter(e -> !VALID_FIELD_NAMES_SET.contains(e.getKey()))
.map(Map.Entry::getKey)
.collect(joining(", "));

if (!invalidKeys.isBlank()) {
throw new IllegalStateException("""
The termination properties [%s] are not valid.
Maybe try changing the property name to kebab-case.
Here is the list of valid properties: %s"""
.formatted(invalidKeys, String.join(", ", VALID_FIELD_NAMES_SET)));
}
properties.forEach(this::loadProperty);
}

Expand All @@ -75,7 +93,7 @@ private void loadProperty(String key, Object value) {
setUnimprovedSpentLimit(DurationStyle.detectAndParse((String) value));
break;
case BEST_SCORE_LIMIT_PROERTY_NAME:
setBestScoreLimit((String) value);
setBestScoreLimit(value.toString());
break;
default:
throw new IllegalStateException("The property %s is not valid.".formatted(key));
Expand Down
@@ -1,6 +1,7 @@
package ai.timefold.solver.spring.boot.autoconfigure.config;

import static ai.timefold.solver.spring.boot.autoconfigure.config.SolverProperties.VALID_FIELD_NAMES_SET;
import static java.util.stream.Collectors.joining;

import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -67,6 +68,18 @@ public void setSolver(Map<String, Object> solver) {
solverProperties.loadProperties(solver);
this.solver.put(DEFAULT_SOLVER_NAME, solverProperties);
} else {
// The values must be an instance of map
String invalidKeys = solver.entrySet().stream()
.filter(e -> e.getValue() != null && !(e.getValue() instanceof Map<?, ?>))
.map(Map.Entry::getKey)
.collect(joining(", "));
if (!invalidKeys.isBlank()) {
throw new IllegalStateException("""
The properties [%s] are not valid.
Maybe try changing the property name to kebab-case.
Here is the list of valid properties: %s"""
.formatted(invalidKeys, String.join(", ", VALID_FIELD_NAMES_SET)));
}
// Multiple solvers. We load the properties per key (or solver config)
solver.forEach((key, value) -> {
SolverProperties solverProperties = new SolverProperties();
Expand Down
Expand Up @@ -2,6 +2,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.api.Assertions.*;

import java.time.Duration;
import java.util.Collections;
Expand Down Expand Up @@ -60,6 +61,7 @@
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.core.io.ClassPathResource;
Expand Down Expand Up @@ -255,6 +257,47 @@ void solverProperties() {
});
}

@Test
void solverWithYaml() {
contextRunner
.withInitializer(new ConfigDataApplicationContextInitializer())
.withSystemProperties(
"spring.config.location=classpath:ai/timefold/solver/spring/boot/autoconfigure/single-solver/application.yaml")
.run(context -> {
SolverConfig solverConfig = context.getBean(SolverConfig.class);
assertEquals(EnvironmentMode.FULL_ASSERT, solverConfig.getEnvironmentMode());
assertTrue(solverConfig.getDaemon());
assertEquals("2", solverConfig.getMoveThreadCount());
assertEquals(DomainAccessType.REFLECTION, solverConfig.getDomainAccessType());
assertNull(solverConfig.getScoreDirectorFactoryConfig().getConstraintStreamImplType());
assertEquals(Duration.ofHours(4), solverConfig.getTerminationConfig().getSpentLimit());
assertEquals(Duration.ofHours(5), solverConfig.getTerminationConfig().getUnimprovedSpentLimit());
assertEquals(SimpleScore.of(0).toString(), solverConfig.getTerminationConfig().getBestScoreLimit());
});
}

@Test
void invalidYaml() {
assertThatCode(() -> contextRunner
.withInitializer(new ConfigDataApplicationContextInitializer())
.withSystemProperties(
"spring.config.location=classpath:ai/timefold/solver/spring/boot/autoconfigure/single-solver/invalid-application.yaml")
.run(context -> context.getBean(SolverConfig.class)))
.rootCause().message().contains("The properties", "solverConfigXml", "environmentMode", "moveThreadCount",
"domainAccessType", "are not valid", "Maybe try changing the property name to kebab-case");
}

@Test
void invalidTerminationYaml() {
assertThatCode(() -> contextRunner
.withInitializer(new ConfigDataApplicationContextInitializer())
.withSystemProperties(
"spring.config.location=classpath:ai/timefold/solver/spring/boot/autoconfigure/single-solver/invalid-termination-application.yaml")
.run(context -> context.getBean(SolverConfig.class)))
.rootCause().message().contains("The termination properties", "spentLimit", "unimprovedSpentLimit",
"bestScoreLimit", "are not valid", "Maybe try changing the property name to kebab-case");
}

@Test
void solveWithAllResources() {
contextRunner
Expand Down
Expand Up @@ -49,6 +49,7 @@

import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.core.io.ClassPathResource;
Expand Down Expand Up @@ -294,6 +295,54 @@ void solve() {
});
}

@Test
void solverWithYaml() {
contextRunner
.withInitializer(new ConfigDataApplicationContextInitializer())
.withSystemProperties(
"spring.config.location=classpath:ai/timefold/solver/spring/boot/autoconfigure/multiple-solvers/application.yaml")
.run(context -> {
TestdataSpringSolution problem = new TestdataSpringSolution();
problem.setValueList(IntStream.range(1, 3)
.mapToObj(i -> "v" + i)
.collect(Collectors.toList()));
problem.setEntityList(IntStream.range(1, 3)
.mapToObj(i -> new TestdataSpringEntity())
.collect(Collectors.toList()));

for (String solverName : List.of("solver1", "solver2")) {
SolverManager<TestdataSpringSolution, Long> solver =
(SolverManager<TestdataSpringSolution, Long>) context.getBean(solverName);
SolverJob<TestdataSpringSolution, Long> solverJob = solver.solve(1L, problem);
TestdataSpringSolution solution = solverJob.getFinalBestSolution();
assertThat(solution).isNotNull();
assertThat(solution.getScore().score()).isNotNegative();
}
});
}

@Test
void invalidYaml() {
assertThatCode(() -> contextRunner
.withInitializer(new ConfigDataApplicationContextInitializer())
.withSystemProperties(
"spring.config.location=classpath:ai/timefold/solver/spring/boot/autoconfigure/multiple-solvers/invalid-application.yaml")
.run(context -> context.getBean(SolverConfig.class)))
.rootCause().message().contains("The properties", "solverConfigXml", "environmentMode", "moveThreadCount",
"domainAccessType", "are not valid", "Maybe try changing the property name to kebab-case");
}

@Test
void invalidTerminationYaml() {
assertThatCode(() -> contextRunner
.withInitializer(new ConfigDataApplicationContextInitializer())
.withSystemProperties(
"spring.config.location=classpath:ai/timefold/solver/spring/boot/autoconfigure/multiple-solvers/invalid-termination-application.yaml")
.run(context -> context.getBean(SolverConfig.class)))
.rootCause().message().contains("The termination properties", "spentLimit", "unimprovedSpentLimit",
"bestScoreLimit", "are not valid", "Maybe try changing the property name to kebab-case");
}

@Test
void solveWithTimeOverride() {
contextRunner
Expand Down
@@ -0,0 +1,8 @@
timefold:
solver:
solver1:
termination:
best-score-limit: 0
solver2:
termination:
best-score-limit: 0

0 comments on commit 307630c

Please sign in to comment.