spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
2.98k stars 1.41k forks source link

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

Open SledgeHammer01 opened 7 months ago

SledgeHammer01 commented 7 months ago

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?

mp911de commented 7 months ago

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?

SledgeHammer01 commented 7 months ago

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.

quaff commented 7 months ago

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

I'm trying fix it by https://github.com/spring-projects/spring-data-commons/pull/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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

I think you want something with this declined PR.

But if https://github.com/spring-projects/spring-data-commons/pull/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 commented 7 months ago

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 commented 7 months ago

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.