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

SPI fragment can not be introduced by @NoRepositoryBean #3212

Open
quaff opened this issue Nov 25, 2024 · 2 comments · May be fixed by #3213
Open

SPI fragment can not be introduced by @NoRepositoryBean #3212

quaff opened this issue Nov 25, 2024 · 2 comments · May be fixed by #3213
Assignees
Labels
for: team-attention An issue we need to discuss as a team to make progress status: on-hold We cannot start working on this issue yet type: enhancement A general enhancement

Comments

@quaff
Copy link
Contributor

quaff commented Nov 25, 2024

Thanks to 4df7a16, Spring Data can introduce SPI fragments by specifying them in spring.factories, it works fine if repository extends the SPI interface, but it will fails if repository extends an @NoRepositoryBean base repository which extends the SPI interface.

Given:

public interface Fragment {
	String getName();
}

public class FragmentImpl implements Fragment {
	public String getName() {
		return "fragment";
	}
}

Following interface hierarchies works:

@NoRepositoryBean
interface ExcludedRepository extends JpaRepository<TestEntity, Long> {
}

interface TestEntityRepository extends ExcludedRepository, Fragment {
}

Following interface hierarchies fails:

@NoRepositoryBean
interface ExcludedRepository extends JpaRepository<TestEntity, Long>, Fragment {
}

interface TestEntityRepository extends ExcludedRepository {
}
Caused by: org.springframework.data.mapping.PropertyReferenceException: No property 'getName' found for type 'TestEntity'
	at org.springframework.data.mapping.PropertyPath.<init>(PropertyPath.java:94) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.mapping.PropertyPath.create(PropertyPath.java:455) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.mapping.PropertyPath.create(PropertyPath.java:431) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.mapping.PropertyPath.lambda$from$0(PropertyPath.java:384) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at java.base/java.util.concurrent.ConcurrentMap.computeIfAbsent(ConcurrentMap.java:330) ~[na:na]
	at org.springframework.data.mapping.PropertyPath.from(PropertyPath.java:366) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.mapping.PropertyPath.from(PropertyPath.java:344) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.repository.query.parser.Part.<init>(Part.java:81) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.repository.query.parser.PartTree$OrPart.lambda$new$0(PartTree.java:259) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) ~[na:na]
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179) ~[na:na]
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[na:na]
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:na]
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) ~[na:na]
	at org.springframework.data.repository.query.parser.PartTree$OrPart.<init>(PartTree.java:260) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.repository.query.parser.PartTree$Predicate.lambda$new$0(PartTree.java:389) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197) ~[na:na]
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179) ~[na:na]
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:992) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499) ~[na:na]
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921) ~[na:na]
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[na:na]
	at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682) ~[na:na]
	at org.springframework.data.repository.query.parser.PartTree$Predicate.<init>(PartTree.java:390) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.repository.query.parser.PartTree.<init>(PartTree.java:100) ~[spring-data-commons-3.4.0.jar:3.4.0]
	at org.springframework.data.jpa.repository.query.PartTreeJpaQuery.<init>(PartTreeJpaQuery.java:101) ~[spring-data-jpa-3.4.0.jar:3.4.0]

Here is reproducer project spifragment.zip
.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 25, 2024
@quaff
Copy link
Contributor Author

quaff commented Nov 25, 2024

It caused by FragmentMetadata::getFragmentInterfaces doesn't consider indirect super interfaces.

public Stream<String> getFragmentInterfaces(String interfaceName) {
Assert.hasText(interfaceName, "Interface name must not be null or empty");
return Arrays.stream(getClassMetadata(interfaceName).getInterfaceNames()) //
.filter(this::isCandidate);
}

I'll try to fix it later.

@mp911de mp911de self-assigned this Nov 25, 2024
quaff added a commit to quaff/spring-data-commons that referenced this issue Nov 25, 2024
@mp911de
Copy link
Member

mp911de commented Nov 27, 2024

We intentionally do not consider super-interfaces for repository fragment detection so in that sense the code works as designed and is not broken. Super-interfaces can reside in other packages and then we open a can of worms asking for consideration of fragment implementations that can reside in other packages, regardless of the SPI feature we recently shipped.

@mp911de mp911de added for: team-attention An issue we need to discuss as a team to make progress status: on-hold We cannot start working on this issue yet type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress status: on-hold We cannot start working on this issue yet type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants