Skip to content
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

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

Closed
ggecy opened this issue Jun 24, 2024 · 11 comments · Fixed by #3047
Closed

Comments

@ggecy
Copy link

ggecy commented Jun 24, 2024

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

  • Vaadin / Flow version: 24.4.3
  • Java version: 21
  • OS version: MacOS 14.5
  • IDE (if applicable): Intellij Idea Ultimate latest version
@knoobie
Copy link

knoobie commented Jun 24, 2024

Workaround: Exclude Hilla

@Legioth
Copy link
Member

Legioth commented Jun 24, 2024

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

@Legioth Legioth transferred this issue from vaadin/flow Jun 24, 2024
@ggecy
Copy link
Author

ggecy commented Jun 27, 2024

Workaround: Exclude Hilla

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

@Legioth
Copy link
Member

Legioth commented Jun 27, 2024

What version are you upgrading from?

@ggecy
Copy link
Author

ggecy commented Jun 28, 2024

What version are you upgrading from?

24.2.4

@Legioth
Copy link
Member

Legioth commented Jun 28, 2024

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

@Legioth
Copy link
Member

Legioth commented Jun 28, 2024

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
Copy link
Author

ggecy commented Jun 28, 2024

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);
}

@tbuchloh
Copy link

tbuchloh commented Jul 4, 2024

We have the same problem when upgrading from 23.4.9. We're not even using hilla, so we can use the workaround.

@gatno
Copy link

gatno commented Sep 30, 2024

We have the same issue, cause we are using Ebean, we have excluded DataSourceAutoConfiguration in Spring Boot.

Artur- added a commit to vaadin/flow that referenced this issue Dec 18, 2024
Artur- added a commit to vaadin/flow that referenced this issue Dec 18, 2024
@Artur- Artur- closed this as completed in c1d3131 Dec 19, 2024
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Hilla 24.7.0.alpha3 and is also targeting the upcoming stable 24.7.0 version.

taefi pushed a commit that referenced this issue Dec 19, 2024
…P: 24.6) (#3059)

fix: Make JpaFilterConverter work without an EntityManager (#3047)

Fixes #2569

Co-authored-by: Artur <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment