Skip to content

Fragment repository implementation should be imported even it's not in basePackages of @EnableJpaRepositories #3287

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
quaff opened this issue Jan 2, 2024 · 2 comments
Labels
status: invalid An issue that we don't feel is valid

Comments

@quaff
Copy link
Contributor

quaff commented Jan 2, 2024

Given:

package com.example.demo;

public interface CustomRepository<T> {
	
	void customSave(T entity);

}
package com.example.demo;

public class CustomRepositoryImpl<T> implements CustomRepository<T> {

	@Override
	public void customSave(T entity) {
		throw new UnsupportedOperationException("Not Implemented");
	}

}
package test;

import org.springframework.data.jpa.repository.JpaRepository;

import com.example.demo.CustomRepository;

public interface TestEntityRepository extends JpaRepository<TestEntity, Long>, CustomRepository<TestEntity> {

}

Then:

com.example.demo.CustomRepositoryImpl should be registered as repository fragment implementation implicitly since com.example.demo.CustomRepository is declared as repository fragment.

Actual:

Here is a failed test

package test;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.domain.EntityScan;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.test.context.ContextConfiguration;

@DataJpaTest
//@EnableJpaRepositories(basePackageClasses = { TestEntityRepository.class, CustomRepository.class }) // will works
@EnableJpaRepositories(basePackageClasses = { TestEntityRepository.class })
@EntityScan(basePackageClasses = TestEntity.class)
@ContextConfiguration(classes = TestEntityRepositoryTests.class)
public class TestEntityRepositoryTests {

	@Autowired
	private TestEntityRepository testEntityRepository;

	@Test
	void test() {
		assertThatThrownBy(() -> testEntityRepository.customSave(new TestEntity()))
				.isInstanceOf(UnsupportedOperationException.class).hasMessage("Not Implemented");
	}

}

Minimal reproducible project: repository-scan.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 2, 2024
@mp911de mp911de added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 2, 2024
@mp911de
Copy link
Member

mp911de commented Jan 2, 2024

This is by design. Custom implementations/fragments use two different strategies though:

  1. Fragments ($FragmentInterfaceName + $ImplementationPostfix -> Fragment CustomRepository + Impl)
  2. Legacy custom implementation ($RepositoryName + $ImplementationPostfix -> Repository Foo + Impl)

Both schemes use the base package as defined in the @Enable…Repositories declaration to detect from bean candidates. Using the base package as filter enables configuration to select different implementations based on the base package.

Fragment implementations must reside in the same package as their fragment interface and fragment implementations must be within one of the specified base packages.

@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2024
@quaff
Copy link
Contributor Author

quaff commented Jan 3, 2024

It's a valid use case that reuse fragment repositories as common library share by applications, normally application use different base package name, then @Enable…Repositories must include both base package name and common library base package name, It would be great if we can use @EnableJpaRepositories instead of @EnableJpaRepositories(basePackages={"com.library", "com.application"}), the library fragment repository should be automatic activated if it's extended by application repository.

I propose that $FragmentInterfaceName + $ImplementationPostfix should be default implementation no matter it's in base packages or not, It could be overridden by other implementation in base packages.

Or It should be available if it's registered as spring bean:

package test;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.domain.EntityScan;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.context.annotation.Bean;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.test.context.ContextConfiguration;

import com.example.demo.CustomRepository;
import com.example.demo.CustomRepositoryImpl;

@DataJpaTest
@EnableJpaRepositories(basePackageClasses = { TestEntityRepository.class })
@EntityScan(basePackageClasses = TestEntity.class)
@ContextConfiguration(classes = TestEntityRepositoryTests.Config.class)
public class TestEntityRepositoryTests {

	@Autowired
	private TestEntityRepository testEntityRepository;

	@Test
	void test() {
		assertThatThrownBy(() -> testEntityRepository.customSave(new TestEntity()))
				.isInstanceOf(UnsupportedOperationException.class).hasMessage("Not Implemented");
	}

	static class Config {
		@Bean
		CustomRepository<?> customRepositoryImpl() {
			return new CustomRepositoryImpl<>();
		}
	}
}

In a nutshell, it's not a good idea that @Enable…Repositories include third party library package name, It should be available if it's on classpath, It's idiomatic in spring boot. WDYT? @mp911de

quaff added a commit to quaff/spring-data-commons that referenced this issue Jan 5, 2024
It's an enhancement that should not break existing behaviors.

The default implementation is `$FragmentInterfaceName + $ImplementationPostfix`, for example `com.example.FragmentImpl` is the default implementation of `com.example.Fragment`.

It's useful for sharing repository fragments as library, application doesn't have to include library package in `@Enable…Repositories`, and it will back off if application provides custom implementation.

See spring-projects/spring-data-jpa#3287
quaff added a commit to quaff/spring-data-commons that referenced this issue Jan 5, 2024
It's an enhancement that should not break existing behaviors.

The default implementation is `$FragmentInterfaceName + $ImplementationPostfix`, for example `com.example.FragmentImpl` is the default implementation of `com.example.Fragment`.

It's useful for sharing repository fragments as library, application doesn't have to include library package in `@Enable…Repositories`, and it will back off if application provides custom implementation.

See spring-projects/spring-data-jpa#3287
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants