Skip to content

Multiple Datasources and JPA obey Principle of Least Astonishment #10604

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

Closed
cosmocracy opened this issue Oct 11, 2017 · 8 comments
Closed

Multiple Datasources and JPA obey Principle of Least Astonishment #10604

cosmocracy opened this issue Oct 11, 2017 · 8 comments

Comments

@cosmocracy
Copy link

cosmocracy commented Oct 11, 2017

A surprising amount of work--entirely boilerplate--is currently necessary for a Spring Boot app to utilize two (or more) Datasources with JPA. There is fantastic support for a single implicit Datasource (and autowiring it for JPA use) by configuring a handful of properties. By comparison an extraordinary amount of work--annotations and interfaces/classes--are requried to support a second Datasource with JPA (EnableTransactionManagement, EnableJpaRepositories, EntityManagerFactory, EntityManagerFactoryBuilder, PlatformTransactionManager, JpaTransactionManager, DataSourceBuilder, LocalContainerEntityManagerFactoryBean, ...)

This issue proposes that Spring Boot configuration guidelines--and supporting changes to autowiring--be developed such that adding additional Datasources for JPA doesn't require departing from a configuration-over-code mindset. The current approach certainly falls afoul of the Principle of Least Astonishment:

In general engineering design contexts, the principle can be taken to mean that a component of a system should behave in a manner consistent with how users of that component are likely to expect it to behave; that is, users should not be astonished at the way it behaves

The fact that so many developers spend time troubleshooting and discussing how to accomplish this is good evidence that a simplification is in order.

See also:

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 11, 2017
@wilkinsona
Copy link
Member

Do you have a suggestion as to how this could be simplified? How would you expect Spring Boot to infer that you want multiple DataSources, multiple EntityManagers etc?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Oct 12, 2017
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Oct 19, 2017
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Oct 26, 2017
@OrangeDog
Copy link
Contributor

OrangeDog commented May 15, 2019

Reopen please? I have suggestions for the simple case of multiple databases.

Defining multiple DataSources isn't too bad but could be easier. I'm doing it like this

private static void copyNonNullProperties(Object source, Object target, String... ignoreProperties) {
    Collection<String> ignoredSet = Arrays.asList(ignoreProperties);
    BeanWrapper sourceWrapper = PropertyAccessorFactory.forBeanPropertyAccess(source);
    BeanWrapper targetWrapper = PropertyAccessorFactory.forBeanPropertyAccess(target);

    Arrays.stream(sourceWrapper.getPropertyDescriptors())
            .map(pd -> new PropertyValue(pd.getName(), sourceWrapper.getPropertyValue(pd.getName())))
            .filter(pv -> pv.getValue() != null)
            .filter(pv -> !ignoredSet.contains(pv.getName()))
            .forEach(targetWrapper::setPropertyValue);
}

@Bean
@Primary
@ConfigurationProperties("spring.datasource.hikari")
public HikariConfig defaultHikariConfig() {
    return new HikariConfig();
}

@Bean
@ConfigurationProperties("second.datasource")
public DataSourceProperties secondDataSourceProperties() {
    return new DataSourceProperties();
}

@Bean
@ConfigurationProperties("second.datasource.hikari")
public DataSource secondDataSource() {
    DataSource dataSource = secondDataSourceProperties().initializeDataSourceBuilder().type(HikariDataSource.class).build();
    copyNonNullProperties(defaultHikariConfig(), dataSource, "class");
    return dataSource;
}

@Bean
public Object telemetryDataSourceInitializer(
        @Qualifier("secondDataSource") ObjectProvider<DataSource> dataSource,
        ApplicationContext ctx
) throws ReflectiveOperationException
{
    Class<?> clazz = Class.forName("org.springframework.boot.autoconfigure.jdbc.DataSourceInitializerInvoker");
    Constructor<?> constructor = clazz.getDeclaredConstructor(
            ObjectProvider.class, DataSourceProperties.class, ApplicationContext.class
    );
    constructor.setAccessible(true);
    return constructor.newInstance(dataSource, secondDataSourceProperties(), ctx);
}

It would be nice if I didn't have to specify the type, and Boot would automatically apply the auto-configured type, corresponding vendor properties, and initialization.

It's the EMFs that are the real pain. If you are separating everything by package scan then it's not too bad, though you need to do some copy-pasting of private methods and hard-coding of Hibernate to get the vendor properties with defaults and customizers. Some duplication would be saved if the EntityManagerFactoryBuilder bean came with more defaults in place (properties, mappingResources, jta).

Downsides of package separation include having to refactor packages to change datasources, and not being able to have multiple EMFs for the same type. Any other method for assigning entities becomes a bit of a nightmare (for repositories it's manageable, because @EnableJpaRepositories accepts include and exclude filters). If the builder accepted similar scan filters, or even a custom PersistenceUnitInfo, it would be a lot easier to use. The SpringPersistenceUnitInfo should also be public so these entities don't miss out on LTW.

I think an even better solution is to make use of the @PersistenceUnit and @PersistenceContexct annotations on entity and repository classes. I'm not sure of the technical details of how that should be wired up, but it should at least be possible to filter the EMF's scan by classes annotated with a matching unitName. Perhaps it would also be possible to then only require a single @EnableJpaRepositories which could use the annotated name to do the respository-emf bindings, as long as there's a matching bean.

@wilkinsona
Copy link
Member

@OrangeDog Thanks for sharing your thoughts. We've now got #15732 which has superseded this issue.

@OrangeDog
Copy link
Contributor

@wilkinsona that doesn't cover the JPA stuff though?

@wilkinsona
Copy link
Member

It says:

We'd also need similar functionality for components that consume a DataSource such as JPA and transaction management.

We can't really know what JPA, transaction management etc will look like until we're figured out the DataSource piece as everything else will build on top of it.

@OrangeDog
Copy link
Contributor

I see. I'm not looking for full autoconfiguration, just slightly easier custom configuration that doesn't have to duplicate everything the Boot would've done. As I mentioned above, a small first change that would be helpful is if the EntityManagerFactoryBuilder and DataSourceBuilder came with more of the autoconfig defaults baked in.

If you do put in full multi-JPA autoconfiguration and I want something slightly different, I don't want to be back to doing everything myself again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants