[PHEE-417] Bind configuration through typed records

build.gradle

@@ -48,6 +48,9 @@ dependencies {
     annotationProcessor enforcedPlatform('org.mifos:paymenthub-ee-bom:2.0.0-SNAPSHOT')
 
     implementation 'org.springframework.boot:spring-boot-starter-web'
+    // Bean Validation for the @Validated configuration records: nothing else
+    // on the classpath provides it.
+    implementation 'org.springframework.boot:spring-boot-starter-validation'
     implementation 'org.springframework.boot:spring-boot-starter-cache'
     // The exclusion is not new: the pom had it on this same starter and the port to
     // Gradle lost it, which put hibernate-core 6.6 on the runtime classpath next to

src/main/java/org/apache/fineract/ServerApplication.java

@@ -28,6 +28,7 @@
 import org.springframework.boot.autoconfigure.flyway.FlywayAutoConfiguration;
 import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration;
 import org.springframework.boot.autoconfigure.jdbc.DataSourceTransactionManagerAutoConfiguration;
+import org.springframework.boot.context.properties.ConfigurationPropertiesScan;
 import org.springframework.boot.web.servlet.FilterRegistrationBean;
 import org.springframework.context.annotation.Bean;
 import org.springframework.security.authentication.AuthenticationManager;
@@ -48,6 +49,7 @@
 @EnableAutoConfiguration(exclude = {DataSourceAutoConfiguration.class,
         DataSourceTransactionManagerAutoConfiguration.class,
         FlywayAutoConfiguration.class})
+@ConfigurationPropertiesScan("org.apache.fineract.config.properties")
 public class ServerApplication {
 
     /**

src/main/java/org/apache/fineract/config/properties/FineractDatasourceProperties.java

@@ -0,0 +1,57 @@
+package org.apache.fineract.config.properties;
+
+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotNull;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.validation.annotation.Validated;
+
+/**
+ * The tenant database connection: {@code fineract.datasource.core.*} and {@code fineract.datasource.common.*}.
+ *
+ * <p>
+ * {@code fineract.datasource.core.auto-update} is not here. It is read as
+ * {@code @Value("${fineract.datasource.core.auto-update:true}")}: a missing key means {@code true}, but an empty one
+ * stops startup. A record cannot do both, because an empty value binds to {@code null} and a default would then turn
+ * it into {@code true}. So {@code TenantDatabaseUpgradeService} keeps that {@code @Value}.
+ * </p>
+ *
+ * @param core
+ *            the core database server and its credentials
+ * @param common
+ *            the JDBC protocol, subprotocol and driver
+ */
+@Validated
+@ConfigurationProperties(prefix = "fineract.datasource")
+public record FineractDatasourceProperties(@NotNull @Valid Core core, @NotNull @Valid Common common) {
+
+    /**
+     * {@code fineract.datasource.core.*}.
+     *
+     * @param host
+     *            database host
+     * @param port
+     *            database port
+     * @param schema
+     *            name of the core schema
+     * @param username
+     *            database user
+     * @param password
+     *            password of that user
+     */
+    public record Core(@NotNull String host, @NotNull Integer port, @NotNull String schema, @NotNull String username,
+            @NotNull String password) {
+    }
+
+    /**
+     * {@code fineract.datasource.common.*}.
+     *
+     * @param protocol
+     *            JDBC protocol, {@code jdbc}
+     * @param subprotocol
+     *            JDBC subprotocol, {@code mysql}
+     * @param driverclassName
+     *            JDBC driver class, from {@code driverclass_name}
+     */
+    public record Common(@NotNull String protocol, @NotNull String subprotocol, @NotNull String driverclassName) {
+    }
+}

src/main/java/org/apache/fineract/config/properties/TokenProperties.java

@@ -0,0 +1,42 @@
+package org.apache.fineract.config.properties;
+
+import jakarta.validation.Valid;
+import jakarta.validation.constraints.NotNull;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.validation.annotation.Validated;
+
+/**
+ * Token validity: {@code token.access.*} and {@code token.refresh.*}.
+ *
+ * <p>
+ * The values stay strings, as they were in the {@code @Value} fields this replaces: they are passed on as text to the
+ * tenant schema migrations.
+ * </p>
+ *
+ * @param access
+ *            access token settings
+ * @param refresh
+ *            refresh token settings
+ */
+@Validated
+@ConfigurationProperties(prefix = "token")
+public record TokenProperties(@NotNull @Valid Access access, @NotNull @Valid Refresh refresh) {
+
+    /**
+     * {@code token.access.*}.
+     *
+     * @param validitySeconds
+     *            access token validity, in seconds
+     */
+    public record Access(@NotNull String validitySeconds) {
+    }
+
+    /**
+     * {@code token.refresh.*}.
+     *
+     * @param validitySeconds
+     *            refresh token validity, in seconds
+     */
+    public record Refresh(@NotNull String validitySeconds) {
+    }
+}

src/main/java/org/apache/fineract/core/service/DataSourcePerTenantService.java

@@ -24,7 +24,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.DisposableBean;
-import org.springframework.beans.factory.annotation.Value;
+import org.apache.fineract.config.properties.FineractDatasourceProperties;
 import org.springframework.stereotype.Service;
 
 import javax.sql.DataSource;
@@ -38,29 +38,32 @@ public class DataSourcePerTenantService implements DisposableBean {
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
     private final Map<Long, DataSource> tenantToDataSourceMap = new HashMap<>();
 
-    @Value("${fineract.datasource.core.port}")
-    private int defaultPort;
+    private final int defaultPort;
 
-    @Value("${fineract.datasource.core.host}")
-    private String defaultHostname;
+    private final String defaultHostname;
 
-    @Value("${fineract.datasource.core.schema}")
-    private String defaultSchema;
+    private final String defaultSchema;
 
-    @Value("${fineract.datasource.core.username}")
-    private String defaultUsername;
+    private final String defaultUsername;
 
-    @Value("${fineract.datasource.core.password}")
-    private String defaultPassword;
+    private final String defaultPassword;
 
-    @Value("${fineract.datasource.common.protocol}")
-    private String jdbcProtocol;
+    private final String jdbcProtocol;
 
-    @Value("${fineract.datasource.common.subprotocol}")
-    private String jdbcSubprotocol;
+    private final String jdbcSubprotocol;
 
-    @Value("${fineract.datasource.common.driverclass_name}")
-    private String driverClass;
+    private final String driverClass;
+
+    public DataSourcePerTenantService(FineractDatasourceProperties datasource) {
+        this.defaultPort = datasource.core().port();
+        this.defaultHostname = datasource.core().host();
+        this.defaultSchema = datasource.core().schema();
+        this.defaultUsername = datasource.core().username();
+        this.defaultPassword = datasource.core().password();
+        this.jdbcProtocol = datasource.common().protocol();
+        this.jdbcSubprotocol = datasource.common().subprotocol();
+        this.driverClass = datasource.common().driverclassName();
+    }
 
     public DataSource retrieveDataSource() {
         DataSource tenantDataSource;

src/main/java/org/apache/fineract/core/service/TenantDatabaseUpgradeService.java

@@ -23,6 +23,8 @@
 import org.apache.fineract.organisation.tenant.TenantServerConnectionRepository;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.apache.fineract.config.properties.FineractDatasourceProperties;
+import org.apache.fineract.config.properties.TokenProperties;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Service;
@@ -46,32 +48,23 @@ public class TenantDatabaseUpgradeService {
     @Autowired
     private DataSourcePerTenantService dataSourcePerTenantService;
 
-    @Value("${fineract.datasource.core.host}")
-    private String hostname;
+    private final String hostname;
 
-    @Value("${fineract.datasource.core.port}")
-    private int port;
+    private final int port;
 
-    @Value("${fineract.datasource.core.username}")
-    private String username;
+    private final String username;
 
-    @Value("${fineract.datasource.core.password}")
-    private String password;
+    private final String password;
 
-    @Value("${fineract.datasource.common.protocol}")
-    private String jdbcProtocol;
+    private final String jdbcProtocol;
 
-    @Value("${fineract.datasource.common.subprotocol}")
-    private String jdbcSubprotocol;
+    private final String jdbcSubprotocol;
 
-    @Value("${fineract.datasource.common.driverclass_name}")
-    private String driverClass;
+    private final String driverClass;
 
-    @Value("${token.access.validity-seconds}")
-    private String tokenAccessValiditySeconds;
+    private final String tokenAccessValiditySeconds;
 
-    @Value("${token.refresh.validity-seconds}")
-    private String tokenRefreshValiditySeconds;
+    private final String tokenRefreshValiditySeconds;
 
     @Value("#{'${tenants}'.split(',')}")
     private List<String> tenants;
@@ -91,6 +84,18 @@ public class TenantDatabaseUpgradeService {
     @Value("${fineract.datasource.core.auto-update:true}")
     private boolean autoUpdateEnabled;
 
+    public TenantDatabaseUpgradeService(FineractDatasourceProperties datasource, TokenProperties token) {
+        this.hostname = datasource.core().host();
+        this.port = datasource.core().port();
+        this.username = datasource.core().username();
+        this.password = datasource.core().password();
+        this.jdbcProtocol = datasource.common().protocol();
+        this.jdbcSubprotocol = datasource.common().subprotocol();
+        this.driverClass = datasource.common().driverclassName();
+        this.tokenAccessValiditySeconds = token.access().validitySeconds();
+        this.tokenRefreshValiditySeconds = token.refresh().validitySeconds();
+    }
+
     @PostConstruct
     public void setupEnvironment() {
         // Both calls below write to the core schema: one applies this repository's

src/test/java/org/apache/fineract/config/properties/ShippedConfigBindsTest.java

@@ -0,0 +1,109 @@
+package org.apache.fineract.config.properties;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.autoconfigure.context.ConfigurationPropertiesAutoConfiguration;
+import org.springframework.boot.autoconfigure.validation.ValidationAutoConfiguration;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
+import org.springframework.boot.test.context.ConfigDataApplicationContextInitializer;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ * Every property these records ask for has to be there, or the service must refuse to start and say which one is
+ * missing. That is what the plain {@code @Value} declarations did before they were replaced, so these tests hold the
+ * replacement to the same promise, and they read the real application.properties rather than a copy of it.
+ */
+class ShippedConfigBindsTest {
+
+    @Configuration(proxyBeanMethods = false)
+    @EnableConfigurationProperties({ FineractDatasourceProperties.class, TokenProperties.class })
+    static class AllRecords {}
+
+    @Configuration(proxyBeanMethods = false)
+    @EnableConfigurationProperties(FineractDatasourceProperties.class)
+    static class OnlyDatasource {}
+
+    @Configuration(proxyBeanMethods = false)
+    @EnableConfigurationProperties(TokenProperties.class)
+    static class OnlyToken {}
+
+    /** One configuration class per record, keyed by the prefix that record binds. */
+    private static final Map<String, Class<?>> ONE_RECORD_EACH = Map.of("fineract.datasource", OnlyDatasource.class, "token",
+            OnlyToken.class);
+
+    private ApplicationContextRunner runner() {
+        return new ApplicationContextRunner().withConfiguration(
+                AutoConfigurations.of(ConfigurationPropertiesAutoConfiguration.class, ValidationAutoConfiguration.class));
+    }
+
+    private ApplicationContextRunner withShippedProperties() {
+        return runner().withInitializer(new ConfigDataApplicationContextInitializer());
+    }
+
+    @Test
+    void theShippedApplicationPropertiesFillEveryField() {
+        withShippedProperties().withUserConfiguration(AllRecords.class).run(context -> {
+            assertThat(context).hasNotFailed();
+            FineractDatasourceProperties datasource = context.getBean(FineractDatasourceProperties.class);
+            assertThat(datasource.core().host()).isEqualTo("operations-mysql");
+            assertThat(datasource.core().port()).isEqualTo(3306);
+            assertThat(datasource.core().schema()).isEqualTo("tenants");
+            assertThat(datasource.core().username()).isEqualTo("root");
+            assertThat(datasource.core().password()).isNotEmpty();
+            assertThat(datasource.common().protocol()).isEqualTo("jdbc");
+            assertThat(datasource.common().subprotocol()).isEqualTo("mysql");
+            // application.properties spells this key driverclass_name
+            assertThat(datasource.common().driverclassName()).isEqualTo("com.mysql.cj.jdbc.Driver");
+            TokenProperties token = context.getBean(TokenProperties.class);
+            // kept as text, because they are passed on as text to the tenant schema migrations
+            assertThat(token.access().validitySeconds()).isEqualTo("600");
+            assertThat(token.refresh().validitySeconds()).isEqualTo("43200");
+        });
+    }
+
+    @Test
+    void everyRecordRefusesToStartWhenItsSectionIsMissing() {
+        ONE_RECORD_EACH.forEach((prefix, configuration) -> runner().withUserConfiguration(configuration).run(context -> {
+            assertThat(context).as("context with nothing configured under '%s'", prefix).hasFailed();
+            assertThat(context.getStartupFailure()).as("failure for '%s'", prefix).hasStackTraceContaining("BindValidationException")
+                    .hasStackTraceContaining("Binding validation errors on " + prefix);
+        }));
+    }
+
+    @Test
+    void aMissingKeyInsideAGroupStopsStartup() {
+        // only fineract.datasource.core.password is missing, so this checks that @Valid reaches the nested record
+        runner().withUserConfiguration(OnlyDatasource.class)
+                .withPropertyValues("fineract.datasource.core.host=h", "fineract.datasource.core.port=3306",
+                        "fineract.datasource.core.schema=s", "fineract.datasource.core.username=u",
+                        "fineract.datasource.common.protocol=jdbc", "fineract.datasource.common.subprotocol=mysql",
+                        "fineract.datasource.common.driverclass_name=d")
+                .run(context -> {
+                    assertThat(context).hasFailed();
+                    assertThat(context.getStartupFailure()).hasStackTraceContaining("Binding validation errors on fineract.datasource")
+                            .hasStackTraceContaining("core.password");
+                });
+    }
+
+    @Test
+    void aValueSetToNothingOnANumberFieldStopsStartup() {
+        withShippedProperties().withUserConfiguration(OnlyDatasource.class).withPropertyValues("fineract.datasource.core.port=")
+                .run(context -> {
+                    assertThat(context).hasFailed();
+                    assertThat(context.getStartupFailure()).hasStackTraceContaining("Binding validation errors on fineract.datasource");
+                });
+    }
+
+    @Test
+    void aValueSetToNothingOnAStringFieldIsAcceptedJustAsItWasBefore() {
+        withShippedProperties().withUserConfiguration(OnlyToken.class).withPropertyValues("token.access.validity-seconds=")
+                .run(context -> {
+                    assertThat(context).hasNotFailed();
+                    assertThat(context.getBean(TokenProperties.class).access().validitySeconds()).isEmpty();
+                });
+    }
+}