Skip to content

@EnableJpaRepositories(repositoryBaseClass = ...) doesn't support dependency injection #3384

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
SledgeHammer01 opened this issue Feb 26, 2024 · 10 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@SledgeHammer01
Copy link

I have a configuration class that specifies a custom repo implementation:

@Configuration
@EnableJpaRepositories(
    basePackages = "xxx",
    entityManagerFactoryRef = "xxx",
    repositoryBaseClass = CustomRepositoryImpl.class,
    transactionManagerRef = "xxx")

This forces a constructor signature of:

  public CustomRepositoryImpl(
      JpaEntityInformation<T, ID> entityInformation, EntityManager entityManager)

And does not support @Autowired or any method I could find to pass in beans.

Can this be modified to instantiate the class via IOC to support pulling in beans?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 26, 2024
@mp911de
Copy link
Member

mp911de commented Feb 27, 2024

This is by design as the base repository class is parametrized using the entity information and EntityManager. JpaEntityInformation is specific to each repository and therefore, we cannot let the Spring context manage repository base class instances.

Taking a step back, what do you want to achieve that isn't achievable by using repository fragments?

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Feb 27, 2024
@SledgeHammer01
Copy link
Author

SledgeHammer01 commented Feb 27, 2024

This is by design as the base repository class is parametrized using the entity information and EntityManager. JpaEntityInformation is specific to each repository and therefore, we cannot let the Spring context manage repository base class instances.

Taking a step back, what do you want to achieve that isn't achievable by using repository fragments?

Unless my understanding of repository fragments is incorrect (which is entirely possible), fragments seem to have a few requirements that didn't meet my needs:

  1. they are for a one off repository implementation rather then shared across multiple repositories?
  2. the location of the Impl class had to be in the same package? (which doesn't make sense if you want to apply to multiple repositories)

So, I went the extending the base class route.

Also, the main problem regarding the dependency injection I raised was due to the fact that the functionality I'm adding requires a per repository @ConfigurationProperties bean.

Basically, I'm trying to do something similar to the @EnableJpaRepositories functionality for multiple repositories.

DataSource1 -> Repo1
DataSource2 -> Repo2

Spring supports that scenario out of the box, but I'm going for:

DataSource1 + ConfigurationProperties1 -> Repo1
DataSource2 + ConfigurationProperties2 -> Repo2

I'm trying to follow the pattern that Spring lays out with auto injecting the data source, but since I can't enhance the annotation, I'm following the other Spring pattern of using a naming convention.

So, if the entity for the repo is called EntityOne, for example, I expect a configuration properties bean named entityOneRepoProperties.

To do this today, I have to have a custom factory and factory bean that @autowire the ApplicationContext and pass in the configuration properties through modifying this part:

Object repository =
    getTargetRepositoryViaReflection(
        information, **myProperties**, entityInformation, entityManager);

If I could have simply injected an ApplicationContext into the repo impl, I could have done that much easier.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 27, 2024
@quaff
Copy link
Contributor

quaff commented Feb 28, 2024

they are for a one off repository implementation rather then shared across multiple repositories?

I'm trying fix it by spring-projects/spring-data-commons#3018.

the location of the Impl class had to be in the same package? (which doesn't make sense if you want to apply to multiple repositories)

It's not true, but must be in basePackages.

@SledgeHammer01
Copy link
Author

SledgeHammer01 commented Feb 28, 2024

It's not true, but must be in basePackages.

Yeah, that's what I meant. That doesn't make sense if you want to share the fragment across multiple basePackages. Why not look in the same package as the interface as a secondary place? If you want it in a shared location, you need to use repositoryBaseClass which gets into the whole issue I mentioned.

@quaff
Copy link
Contributor

quaff commented Feb 28, 2024

Yeah, that's what I meant. That doesn't make sense if you want to share the fragment across multiple basePackages. Why not look in the same package as the interface as a secondary place? If you want it in a shared location, you need to use repositoryBaseClass which gets into the whole issue I mentioned.

I agree, the PR will address it, would you take some time to verify?

@SledgeHammer01
Copy link
Author

SledgeHammer01 commented Feb 28, 2024

Yeah, that's what I meant. That doesn't make sense if you want to share the fragment across multiple basePackages. Why not look in the same package as the interface as a secondary place? If you want it in a shared location, you need to use repositoryBaseClass which gets into the whole issue I mentioned.

I agree, the PR will address it, would you take some time to verify?

I think that fix will make it easier for sharing fragments, but in my case, I don't think it will work since I need to get a bean in the implementation so had to use a full blown implementation and custom repositoryFactoryBeanClass.

Is there a way to get a bean into the fragment?

@quaff
Copy link
Contributor

quaff commented Feb 28, 2024

Is there a way to get a bean into the fragment?

I think you want something with this declined PR.

But if spring-projects/spring-data-commons#3018 is accepted, you can do like this:

public interface BeanProvider {

	<T> T getBean(Class<T> clazz);

}

@RequiredArgsConstructor
public class BeanProviderImpl implements BeanProvider {

	private final BeanFactory beanFactory;

	@Override
	public <T> T getBean(Class<T> clazz) {
		return this.beanFactory.getBean(clazz);
	}

}


interface TestEntityRepository extends CrudRepository<TestEntity, Long>, BeanProvider {

	default void saveByEntityManager(TestEntity entity) {
		getBean(EntityManager.class).persist(entity);
	}

	default void saveBySelf(TestEntity entity) {
		getBean(TestEntityRepository.class).save(entity);
	}

}

@mp911de
Copy link
Member

mp911de commented Feb 28, 2024

You can take a more invasive approach by subclassing JpaRepositoryFactory and overriding getTargetRepository(…). For your case, you must provide AutowireCapableBeanFactory to your JpaRepositoryFactory subclass and call autowire on the resulting instance.

Finally, you need to configure @EnableJpaRepositories(repositoryFactoryBeanClass=YourJpaRepositoryFactoryBean.class) to stitch all the pieces together.

@SledgeHammer01
Copy link
Author

You can take a more invasive approach by subclassing JpaRepositoryFactory and overriding getTargetRepository(…). For your case, you must provide AutowireCapableBeanFactory to your JpaRepositoryFactory subclass and call autowire on the resulting instance.

Finally, you need to configure @EnableJpaRepositories(repositoryFactoryBeanClass=YourJpaRepositoryFactoryBean.class) to stitch all the pieces together.

Yeah, that's what I ended up doing, but I didn't make it autowired, I just added it to the getTargetRepositoryViaReflection() call as an extra parameter since I "calculate" the desired bean name at runtime ala what Spring does.

Its definitely "invasive" :).

What @quaff proposed above should make things a lot easier.

@mp911de mp911de added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Oct 23, 2024
@mp911de
Copy link
Member

mp911de commented Oct 23, 2024

We do not want to introduce more complexity than we already have in that area. This isn't a common request and having that extra mile on your side is fine for us for the time being.

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2024
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

No branches or pull requests

4 participants