Skip to content

Conversation

@damienbeaufils
Copy link

Hello,

Since this commit, we can't extend anymore HibernateJpaAutoConfiguration class to specify hibernate mapping files like this, but we can’t either extend HibernateJpaConfiguration class because it is package-protected.
This PR changes HibernateJpaConfiguration class modifier to public to be able to extend it.
It could be a solution for this question on SO.

Thanks!

@pivotal-issuemaster
Copy link

@damienbeaufils Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@damienbeaufils Thank you for signing the Contributor License Agreement!

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

snicoll commented Oct 17, 2017

@damienbeaufils extending from an auto-configuration isn't really a great idea. Can we take a step back and discuss why you need to do this? Perhaps we can adapt the auto-configuration to take that use case into account.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Oct 17, 2017
@damienbeaufils
Copy link
Author

@snicoll I agree extending from an autoconfiguration is not great.
My use case is quite simple I think: I would like to configure the Hibernate JPA datasource provided by Spring Boot with specific XML mapping files.
The way people found is to override the LocalContainerEntityManagerFactoryBean bean from the old HibernateJpaAutoConfiguration and set mapping resources.

@snicoll
Copy link
Member

snicoll commented Oct 17, 2017

Can you show an example of such override please?

@damienbeaufils
Copy link
Author

Sure. An example:

@Configuration
public class MappingsConfiguration extends HibernateJpaAutoConfiguration {

    public MappingsConfiguration(DataSource dataSource, JpaProperties jpaProperties, ObjectProvider<JtaTransactionManager> jtaTransactionManager, ObjectProvider<TransactionManagerCustomizers> transactionManagerCustomizers, ObjectProvider<List<SchemaManagementProvider>> providers) {
        super(dataSource, jpaProperties, jtaTransactionManager, transactionManagerCustomizers, providers);
    }

    @Bean
    @Override
    public LocalContainerEntityManagerFactoryBean entityManagerFactory(EntityManagerFactoryBuilder factoryBuilder) {
        LocalContainerEntityManagerFactoryBean localContainerEntityManagerFactoryBean = getSuperEntityManagerFactory(factoryBuilder);
        localContainerEntityManagerFactoryBean.setMappingResources(
                "db/mappings/dummy.xml"
        );
        return localContainerEntityManagerFactoryBean;
    }

    LocalContainerEntityManagerFactoryBean getSuperEntityManagerFactory(EntityManagerFactoryBuilder factoryBuilder) {
        return super.entityManagerFactory(factoryBuilder);
    }
}

If you need a whole project using this code, see this repository.

@snicoll snicoll added for: team-attention An issue we'd like other members of the team to review and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 17, 2017
@wilkinsona
Copy link
Member

This feels like the sort of thing that we've previously solved with a customizer interface. If we consider this to be too much of an edge case to warrant that, a bean post-processor could be used instead.

@damienbeaufils
Copy link
Author

@wilkinsona Do you have an example of a customizer interface somewhere?

@wilkinsona
Copy link
Member

JacksonObjectMappingBuilderCustomizer is one example. It's used by JacksonAutoConfiguration.

@snicoll
Copy link
Member

snicoll commented Oct 18, 2017

perhaps mappingResources could be a spring.jpa property somehow. That would remove that need altogether (although I agree the customizer is typically what we do in such cases).

@wilkinsona
Copy link
Member

I like that idea, @snicoll. The method just takes String… so a List<String> spring.jpa.mapping-resources property would seem to fit nicely.

@snicoll
Copy link
Member

snicoll commented Oct 18, 2017

Cool.

@damienbeaufils I am not keen to change the scope of that class (extending the AutoConfiguration is kind of a smell anyway). Let's add support for this in #10684. If you find other use cases where extending is necessary we can go the customizer route.

@snicoll snicoll closed this Oct 18, 2017
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Oct 18, 2017
@damienbeaufils
Copy link
Author

@snicoll I'm fine with this solution. Thanks! If you need any feedback for #10684 before the next milestone release, feel free to ask.

@damienbeaufils damienbeaufils deleted the publicHibernateJpaConfiguration branch May 14, 2018 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants