Custom implementation of CrudRepository.save() not called if generics not declared exactly (reopener) [DATACMNS-1225]

[spring-projects-issues]

Sebastian J opened DATACMNS-1225 and commented

This is a reopener to DATACMNS-1008 (unsure if the old one should be reopened or a new ticket should be created).

The issue is still the same and can be reproduced with these classes:

public interface FooRepository extends CrudRepository<Foo, Long>, FooRepositoryCustom {
	// Workaround as per DATACMNS-1008 still works
	//<X extends Foo> X save(X foo);
}

public interface FooRepositoryCustom {
	Foo doNonstandardThing(Foo foo);
}

public class FooRepositoryImpl implements FooRepositoryCustom {
	
    public Foo save(Foo foo) {
        // Do things
    	return foo;
    }
}

@Service
public class FooService {
    private final FooRepository fooRepo;
    
    @Autowired
    public FooService(FooRepository fooRepo) {
        this.fooRepo = fooRepo;
    }

    public void processFoo(Foo foo) {
    	fooRepo.doNonstandardThing(foo);
        fooRepo.save(foo);
    }
}

As previously, the save method in the custom implementation is never called.

From debugging it looks to me that the error occurs here: org.springframework.data.repository.core.support.RepositoryComposition.RepositoryFragments.invoke(Method, Object[]) as ReflectionUtils.findMethod in org.springframework.data.repository.core.support.RepositoryFragment.hasMethod(Method) will never find the custom method.
It is looking for a method save(Object) method (due to the generic erasure) but the custom method's parameter is of type Foo

---

Affects: 2.0.1 (Kay SR1), 2.0.2 (Kay SR2)

Reference URL: https://github.com/derjust/spring-data-dynamodb/tree/issue_77/src/test/java/org/socialsignin/spring/data/dynamodb/issue77

Issue Links:

• DATACMNS-1008 Custom implementation of CrudRepository.save(…) not called if generics not declared exactly

[spring-projects-issues]

Mark Paluch commented

This issue is caused by the way how we compare arguments. We attempt to find the right method in multiple passes of which the first pass checks for exact matches. The exact check does not consider generics hence the method we're attempting to resolve is CrudRepository.save(Object) and not CrudRepository.save(Foo). Adding generics to your repository interface changes the method to look up to CrudRepository.save(Foo) so it's an effect of type erasure.

While we should improve here I'd like to share my concerns about invoking a method that is called by a set of implicit assumptions. A save method FooRepositoryImpl.save(Foo) is not bound by any contract to your repository interface and from a Java class perspective, it's unrelated. A cleaner approach would be introducing a base repository extending from your Simple…Repository that is typed with CrudRepository<T, ID> to ensure type safety and a clean contract.

Replacing MethodLookups.direct() with a variant that resolves generics and checks for exact parameter matches (by comparing equality, not assignability) should solve the issue.

Oliver Drotbohm WDYT?