Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type/unremovable for reactive client CDI beans, make DataSource beans application-scoped, more consistent exceptions for unconfigured datasources #37906

Merged
merged 8 commits into from
Jan 8, 2024
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
Loading