vaadin / hilla

Build better business applications, faster. No more juggling REST endpoints or deciphering GraphQL queries. Hilla seamlessly connects Spring Boot and React to accelerate application development.
https://hilla.dev
Apache License 2.0
862 stars 58 forks source link

BLOCKER: Cannot upgrade to Vaadin 24.4 because of com.vaadin.hilla.crud.JpaFilterConverter #2569

Open ggecy opened 1 week ago

ggecy commented 1 week ago

Description of the bug

It is not so uncommon to have multiple datasources defined in spring boot application and by extension multiple entity managers which will cause the com.vaadin.hilla.crud.JpaFilterConverter to fail to autowire the entity manager since it cannot pick an entity manager factory it should use (usually specified by e.g. @PersistenceContext annotation's unitName attribute). You can partially get around this issue if you mark one of the factories as @Primary however that is not always possible, especially if the entity manager factory marked as primary is not the one that should be used by JpaFilterConverter. It should be possible to customize JpaFilterConverter bean where one can choose the correct entity manager to use. This is a blocker for upgrage to vaadin 24.4.

Expected behavior

I need to be able specify which entity manager is used by com.vaadin.hilla.crud.JpaFilterConverter

Minimal reproducible example

  1. Define multiple datasource beans in spring boot application (along with LocalContainerEntityManagerFactoryBean and PlatformTransactionManager for each datasource).
  2. Application using vaadin 24.4.3 will fail to start because it will not be able to autowire he EntityManager in com.vaadin.hilla.crud.JpaFilterConverter

Versions

knoobie commented 1 week ago

Workaround: Exclude Hilla

Legioth commented 1 week ago

Moving this issue to the Hilla repo since it's caused by code related to a Hilla feature.

ggecy commented 5 days ago

Workaround: Exclude Hilla

That is not an acceptable workaround. I need to have access to Hilla.

Legioth commented 5 days ago

What version are you upgrading from?

ggecy commented 4 days ago

What version are you upgrading from?

24.2.4

Legioth commented 4 days ago

So no use of previous Hilla versions? Why is use of Hilla then becoming a requirement when uprading to 24.4?

Legioth commented 4 days ago

Let me elaborate a bit: yes, it seems that Hilla is utterly broken for the scenario with multiple data sources. This blocks users from starting to use Hilla if they already have that kind of setup and it prevents existing Hilla users from starting to use that kind of setup. We should at the very least fix this so that it's only the feature that actually uses JpaFilterConverter (i.e. AutoCrud) that would be unavailable if there are multiple data providers rather than the current situation where Hilla cannot be on the classpath at all.

But that's a quite different kind of blocker compared to if Vaadin 24.2 applications that don't use Hilla cannot upgrade to using Vaadin 24.4 without Hilla.

ggecy commented 4 days ago

Let me elaborate a bit: yes, it seems that Hilla is utterly broken for the scenario with multiple data sources. This blocks users from starting to use Hilla if they already have that kind of setup and it prevents existing Hilla users from starting to use that kind of setup. We should at the very least fix this so that it's only the feature that actually uses JpaFilterConverter (i.e. AutoCrud) that would be unavailable if there are multiple data providers rather than the current situation where Hilla cannot be on the classpath at all.

But that's a quite different kind of blocker compared to if Vaadin 24.2 applications that don't use Hilla cannot upgrade to using Vaadin 24.4 without Hilla.

Yes, however we want to start converting to hybrid application with hilla views and this is a problem now. And it’s theoretically a very simple fix - just remove the @Component annotation from JpaFilterConverter, add enityManager as constructor parameter, then in some autoconfiguration class define

@Bean
@ConditionalOnMissingBean
public JpaFilterConverter jpaFilterConverter(EntityManager entityManager) {
    return new JpaFilterConverter(entityManager);
}

I could then define the bean myself in my configuration class as e.g.:

@Bean
public JpaFilterConverter jpaFilterConverter(@Qualifier(PRIMARY_WRITER_ENTITY_MANAGER_FACTORY) EntityManager entityManager) {
    return new JpaFilterConverter(entityManager);
}