Skip to content

Commit

Permalink
Merge pull request #37906 from yrodiere/datasource-unconfigured-basics
Browse files Browse the repository at this point in the history
Fix type/unremovable for reactive client CDI beans, make DataSource beans application-scoped, more consistent exceptions for unconfigured datasources
  • Loading branch information
yrodiere authored Jan 8, 2024
2 parents 32569ff + 989ce28 commit ba97483
Show file tree
Hide file tree
Showing 51 changed files with 1,351 additions and 349 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.runtime.configuration;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import io.quarkus.dev.config.ConfigurationProblem;
Expand Down Expand Up @@ -55,7 +56,7 @@ public ConfigurationException(final String msg, Set<String> configKeys) {
*/
public ConfigurationException(final Throwable cause, Set<String> configKeys) {
super(cause);
this.configKeys = configKeys;
this.configKeys = forwardCauseConfigKeys(configKeys, cause);
}

/**
Expand All @@ -77,7 +78,7 @@ public ConfigurationException(final String msg, final Throwable cause) {
*/
public ConfigurationException(final String msg, final Throwable cause, Set<String> configKeys) {
super(msg, cause);
this.configKeys = configKeys;
this.configKeys = forwardCauseConfigKeys(configKeys, cause);
}

public ConfigurationException(Throwable cause) {
Expand All @@ -88,4 +89,12 @@ public ConfigurationException(Throwable cause) {
public Set<String> getConfigKeys() {
return configKeys;
}

private static Set<String> forwardCauseConfigKeys(Set<String> configKeys, Throwable cause) {
if (cause instanceof ConfigurationProblem) {
var merged = new HashSet<String>(configKeys);
merged.addAll(((ConfigurationProblem) cause).getConfigKeys());
}
return configKeys;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

import javax.sql.XADataSource;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Default;
import jakarta.inject.Singleton;

import org.jboss.jandex.ClassType;
import org.jboss.jandex.DotName;
Expand Down Expand Up @@ -72,6 +72,7 @@ class AgroalProcessor {

private static final String OPEN_TELEMETRY_DRIVER = "io.opentelemetry.instrumentation.jdbc.OpenTelemetryDriver";
private static final DotName DATA_SOURCE = DotName.createSimple(javax.sql.DataSource.class.getName());
private static final DotName AGROAL_DATA_SOURCE = DotName.createSimple(AgroalDataSource.class.getName());

@BuildStep
void agroal(BuildProducer<FeatureBuildItem> feature) {
Expand Down Expand Up @@ -277,7 +278,8 @@ void generateDataSourceBeans(AgroalRecorder recorder,
SyntheticBeanBuildItem.ExtendedBeanConfigurator configurator = SyntheticBeanBuildItem
.configure(AgroalDataSource.class)
.addType(DATA_SOURCE)
.scope(Singleton.class)
.addType(AGROAL_DATA_SOURCE)
.scope(ApplicationScoped.class)
.setRuntimeInit()
.unremovable()
.addInjectionPoint(ClassType.create(DotName.createSimple(DataSources.class)))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package io.quarkus.agroal.test;

import static org.assertj.core.api.Assertions.assertThat;

import jakarta.inject.Singleton;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.agroal.runtime.DataSources;
import io.quarkus.arc.Arc;
import io.quarkus.datasource.common.runtime.DataSourceUtil;
import io.quarkus.test.QuarkusUnitTest;

/**
* Check that datasources are created eagerly on application startup.
* <p>
* This has always been the case historically, so we want to keep it that way.
*/
public class EagerStartupTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withConfigurationResource("base.properties");

@Test
public void shouldStartEagerly() {
var container = Arc.container();
var instanceHandle = container.instance(DataSources.class);
// Check that the following call won't trigger a lazy initialization:
// the DataSources bean must be eagerly initialized.
assertThat(container.getActiveContext(Singleton.class).getState()
.getContextualInstances().get(instanceHandle.getBean()))
.as("Eagerly instantiated DataSources bean")
.isNotNull();
// Check that the datasource has already been eagerly created.
assertThat(instanceHandle.get().isDataSourceCreated(DataSourceUtil.DEFAULT_DATASOURCE_NAME))
.isTrue();
}

}
Original file line number Diff line number Diff line change
@@ -1,19 +1,94 @@
package io.quarkus.agroal.test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.sql.SQLException;

import javax.sql.DataSource;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.agroal.api.AgroalDataSource;
import io.quarkus.arc.Arc;
import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.test.QuarkusUnitTest;

/**
* We should be able to start the application, even with no configuration at all.
*/
public class NoConfigTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest();
static final QuarkusUnitTest config = new QuarkusUnitTest()
// The datasource won't be truly "unconfigured" if dev services are enabled
.overrideConfigKey("quarkus.devservices.enabled", "false");

@Inject
MyBean myBean;

@Test
public void testNoConfig() throws SQLException {
// we should be able to start the application, even with no configuration at all
public void dataSource_default() {
DataSource ds = Arc.container().instance(DataSource.class).get();

// The default datasource is a bit special;
// it's historically always been considered as "present" even if there was no explicit configuration.
// So the bean will never be null.
assertThat(ds).isNotNull();
// However, if unconfigured, any attempt to use it at runtime will fail.
assertThatThrownBy(() -> ds.getConnection())
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("quarkus.datasource.jdbc.url has not been defined");
}

@Test
public void agroalDataSource_default() {
AgroalDataSource ds = Arc.container().instance(AgroalDataSource.class).get();

// The default datasource is a bit special;
// it's historically always been considered as "present" even if there was no explicit configuration.
// So the bean will never be null.
assertThat(ds).isNotNull();
// However, if unconfigured, any attempt to use it at runtime will fail.
assertThatThrownBy(() -> ds.getConnection())
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("quarkus.datasource.jdbc.url has not been defined");
}

@Test
public void dataSource_named() {
DataSource ds = Arc.container().instance(DataSource.class,
new io.quarkus.agroal.DataSource.DataSourceLiteral("ds-1")).get();
// An unconfigured, named datasource has no corresponding bean.
assertThat(ds).isNull();
}

@Test
public void agroalDataSource_named() {
AgroalDataSource ds = Arc.container().instance(AgroalDataSource.class,
new io.quarkus.agroal.DataSource.DataSourceLiteral("ds-1")).get();
// An unconfigured, named datasource has no corresponding bean.
assertThat(ds).isNull();
}

@Test
public void injectedBean_default() {
assertThatThrownBy(() -> myBean.useDataSource())
.isInstanceOf(ConfigurationException.class)
.hasMessageContaining("quarkus.datasource.jdbc.url has not been defined");
}

@ApplicationScoped
public static class MyBean {
@Inject
DataSource ds;

public void useDataSource() throws SQLException {
ds.getConnection();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import jakarta.annotation.PostConstruct;
import jakarta.annotation.PreDestroy;
import jakarta.enterprise.inject.Any;
import jakarta.enterprise.inject.Default;
Expand Down Expand Up @@ -122,6 +123,10 @@ public static AgroalDataSource fromName(String dataSourceName) {
.getDataSource(dataSourceName);
}

public boolean isDataSourceCreated(String dataSourceName) {
return dataSources.containsKey(dataSourceName);
}

public AgroalDataSource getDataSource(String dataSourceName) {
return dataSources.computeIfAbsent(dataSourceName, new Function<String, AgroalDataSource>() {
@Override
Expand All @@ -131,6 +136,13 @@ public AgroalDataSource apply(String s) {
});
}

@PostConstruct
public void start() {
for (String dataSourceName : dataSourceSupport.entries.keySet()) {
getDataSource(dataSourceName);
}
}

@SuppressWarnings("resource")
public AgroalDataSource doCreateDataSource(String dataSourceName) {
if (!dataSourceSupport.entries.containsKey(dataSourceName)) {
Expand All @@ -140,6 +152,7 @@ public AgroalDataSource doCreateDataSource(String dataSourceName) {
DataSourceJdbcBuildTimeConfig dataSourceJdbcBuildTimeConfig = dataSourcesJdbcBuildTimeConfig
.dataSources().get(dataSourceName).jdbc();
DataSourceRuntimeConfig dataSourceRuntimeConfig = dataSourcesRuntimeConfig.dataSources().get(dataSourceName);

DataSourceJdbcRuntimeConfig dataSourceJdbcRuntimeConfig = dataSourcesJdbcRuntimeConfig
.getDataSourceJdbcRuntimeConfig(dataSourceName);

Expand Down
4 changes: 4 additions & 0 deletions extensions/datasource/common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
<artifactId>quarkus-datasource-common</artifactId>
<name>Quarkus - Datasource - Common</name>
<dependencies>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-core</artifactId>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import io.quarkus.runtime.configuration.ConfigurationException;

public final class DataSourceUtil {

Expand Down Expand Up @@ -34,6 +38,18 @@ public static List<String> dataSourcePropertyKeys(String datasourceName, String
}
}

public static ConfigurationException dataSourceNotConfigured(String dataSourceName) {
return new ConfigurationException(String.format(Locale.ROOT,
"Datasource '%s' is not configured."
+ " To solve this, configure datasource '%s'."
+ " Refer to https://quarkus.io/guides/datasource for guidance.",
dataSourceName, dataSourceName),
Set.of(dataSourcePropertyKey(dataSourceName, "db-kind"),
dataSourcePropertyKey(dataSourceName, "username"),
dataSourcePropertyKey(dataSourceName, "password"),
dataSourcePropertyKey(dataSourceName, "jdbc.url")));
}

private DataSourceUtil() {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private RunningDevService startDevDb(
LaunchMode launchMode, Optional<ConsoleInstalledBuildItem> consoleInstalledBuildItem,
LoggingSetupBuildItem loggingSetupBuildItem, GlobalDevServicesConfig globalDevServicesConfig) {
boolean explicitlyDisabled = !(dataSourceBuildTimeConfig.devservices().enabled().orElse(true));
String dataSourcePrettyName = DataSourceUtil.isDefault(dbName) ? "default datasource" : "datasource" + dbName;
String dataSourcePrettyName = DataSourceUtil.isDefault(dbName) ? "default datasource" : "datasource " + dbName;

if (explicitlyDisabled) {
//explicitly disabled
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package io.quarkus.flyway.test;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.CreationException;
import jakarta.enterprise.inject.Instance;
import jakarta.inject.Inject;

import org.flywaydb.core.Flyway;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;

public class FlywayExtensionConfigEmptyDefaultDatasourceTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
// The datasource won't be truly "unconfigured" if dev services are enabled
.overrideConfigKey("quarkus.devservices.enabled", "false");

@Inject
Instance<Flyway> flywayForDefaultDatasource;

@Inject
MyBean myBean;

@Test
@DisplayName("If there is no config for the default datasource, the application should boot, but Flyway should be deactivated for that datasource")
public void testBootSucceedsButFlywayDeactivated() {
assertThatThrownBy(flywayForDefaultDatasource::get)
.isInstanceOf(CreationException.class)
.cause()
.hasMessageContainingAll("Unable to find datasource '<default>' for Flyway",
"Datasource '<default>' is not configured.",
"To solve this, configure datasource '<default>'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}

@Test
@DisplayName("If there is no config for the default datasource, the application should boot even if we inject a bean that depends on Liquibase, but actually using Liquibase should fail")
public void testBootSucceedsWithInjectedBeanDependingOnFlywayButFlywayDeactivated() {
assertThatThrownBy(() -> myBean.useFlyway())
.cause()
.hasMessageContainingAll("Unable to find datasource '<default>' for Flyway",
"Datasource '<default>' is not configured.",
"To solve this, configure datasource '<default>'.",
"Refer to https://quarkus.io/guides/datasource for guidance.");
}

@ApplicationScoped
public static class MyBean {
@Inject
Flyway flywayForDefaultDatasource;

public void useFlyway() {
flywayForDefaultDatasource.getConfiguration();
}
}
}
Loading

0 comments on commit ba97483

Please sign in to comment.