Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

PLAT-3546: Validating Config-files #797

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions bootstrap/src/main/java/se/fortnox/reactivewizard/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import se.fortnox.reactivewizard.binding.AutoBindModules;
import se.fortnox.reactivewizard.config.ConfigFactory;

import java.nio.file.NoSuchFileException;

/**
* Main application entry point. Will scan for all modules on the classpath and run them.
Expand All @@ -27,12 +24,10 @@ public class Main {
*/
public static void main(String[] args) {
try {
ConfigFactory configFactory = createConfigFactory(args);
Module bootstrap = new AbstractModule() {
@Override
protected void configure() {
bind(String[].class).annotatedWith(Names.named("args")).toInstance(args);
bind(ConfigFactory.class).toInstance(configFactory);
}
};
Guice.createInjector(new AutoBindModules(bootstrap));
Expand All @@ -41,27 +36,4 @@ protected void configure() {
System.exit(-1);
}
}

/**
* Prepares a ConfigFactory before setting up Guice.
* <p>
* As logging can be part of the configuration file and the configuration file
* could be missing, we have a side effect of setting up and initializing a LoggingFactory
* that doesn't depend on ConfigFactory, if the configuration file is missing.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear if this justification is still vald.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is no longer valid.

*
* @param args commandline arguments
* @return A ConfigFactory based on a configuration file.
* @throws NoSuchFileException If the configuration file cannot be found.
*/
private static ConfigFactory createConfigFactory(String[] args) throws NoSuchFileException {
try {
return new ConfigFactory(args);
} catch (RuntimeException runtimeException) {
Throwable cause = runtimeException.getCause();
if (cause != null && cause.getClass().isAssignableFrom(NoSuchFileException.class)) {
throw (NoSuchFileException)cause;
}
throw runtimeException;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,12 @@ public ValidationModule(InjectAnnotatedScanner injectAnnotatedScanner) {

@Override
public void configure(Binder binder) {
binder.bind(Validator.class).toProvider(() -> {
return Validation
.byDefaultProvider()
.configure()
.parameterNameProvider(new JaxRsParameterNameResolver())
.buildValidatorFactory()
.getValidator();
}).in(Scopes.SINGLETON);
binder.bind(Validator.class).toProvider(() -> Validation
.byDefaultProvider()
.configure()
.parameterNameProvider(new JaxRsParameterNameResolver())
.buildValidatorFactory()
.getValidator()).in(Scopes.SINGLETON);


// Validate config
Expand All @@ -56,4 +54,10 @@ private <T> void bindValidationProxy(Binder binder, Provider<ValidatorUtil> vali
private <T> Provider<T> validatingProvider(Class<T> iface, Provider<T> wrappedProvider, Provider<ValidatorUtil> validatorUtilProvider) {
return () -> ValidatingProxy.create(iface, wrappedProvider.get(), validatorUtilProvider.get());
}

@Override
public Integer getPrio() {
// We want this module to override the default binding of ConfigFactory
return 110;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way of creating a test to validate that this actually works, and more importantly continues to work in the future as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test that compares priorities between this module and the config module.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package se.fortnox.reactivewizard.validation;

import org.assertj.core.api.Assertions;
import org.junit.Test;
import se.fortnox.reactivewizard.binding.scanners.ConfigClassScanner;
import se.fortnox.reactivewizard.binding.scanners.InjectAnnotatedScanner;
import se.fortnox.reactivewizard.config.ConfigAutoBindModule;

import static org.mockito.Mockito.mock;

public class ValidationModuleTest {

@Test
public void validationModuleShouldGoAfterConfigModule() {
InjectAnnotatedScanner injectAnnotatedScanner = mock(InjectAnnotatedScanner.class);
ValidationModule validationModule = new ValidationModule(injectAnnotatedScanner);

ConfigClassScanner configClassScanner = mock(ConfigClassScanner.class);
ConfigAutoBindModule configAutoBindModule = new ConfigAutoBindModule(configClassScanner);

Assertions.assertThat(configAutoBindModule.getPrio()).isLessThan(validationModule.getPrio());
}
}
Loading