DATACMNS-1023 - We prefer direct matches on reactive repository query method overloads.

We now attempt to look up a target class method based on exact name and parameter types before falling back on the more expensive type matches. This also eliminates the possibility of invalid method matches as described in the ticket.

Related ticket: DATACMNS-943.
Original Pull Request: #194
This commit is contained in:
Mark Paluch
2017-03-30 16:18:59 +02:00
committed by Christoph Strobl
parent b5af2e9d96
commit e73710374d
2 changed files with 31 additions and 4 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2016 the original author or authors.
* Copyright 2016-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,6 +16,9 @@
package org.springframework.data.repository.core.support;
import static org.springframework.core.GenericTypeResolver.*;
import static org.springframework.util.ReflectionUtils.*;
import lombok.Value;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
@@ -29,7 +32,6 @@ import java.util.function.Supplier;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import lombok.Value;
import org.springframework.core.MethodParameter;
import org.springframework.core.ResolvableType;
import org.springframework.core.convert.ConversionService;
@@ -76,7 +78,10 @@ public class ReactiveRepositoryInformation extends DefaultRepositoryInformation
@Override
Method getTargetClassMethod(Method method, Optional<Class<?>> baseClass) {
return baseClass.flatMap(it -> {
Supplier<Optional<Method>> directMatch = () -> baseClass
.map(it -> findMethod(it, method.getName(), method.getParameterTypes()));
Supplier<Optional<Method>> detailedComparison = () -> baseClass.flatMap(it -> {
List<Supplier<Optional<Method>>> suppliers = new ArrayList<>();
@@ -88,8 +93,9 @@ public class ReactiveRepositoryInformation extends DefaultRepositoryInformation
suppliers.add(() -> getMethodCandidate(method, it, matchParameterOrComponentType(getRepositoryInterface())));
return Optionals.firstNonEmpty(Streamable.of(suppliers));
});
}).orElse(method);
return Optionals.firstNonEmpty(directMatch, detailedComparison).orElse(method);
}
/**

View File

@@ -18,6 +18,7 @@ package org.springframework.data.repository.core.support;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import reactor.core.publisher.Flux;
import rx.Observable;
import java.io.Serializable;
@@ -28,6 +29,7 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.junit.MockitoJUnitRunner;
import org.reactivestreams.Publisher;
import org.springframework.data.repository.core.RepositoryInformation;
import org.springframework.data.repository.core.RepositoryMetadata;
import org.springframework.data.repository.reactive.ReactiveCrudRepository;
import org.springframework.data.repository.reactive.ReactiveSortingRepository;
@@ -112,6 +114,19 @@ public class ReactiveRepositoryInformationUnitTests {
assertThat(reference.getParameterTypes()[0], is(equalTo(Object.class)));
}
@Test // DATACMNS-1023
public void usesCorrectSaveOverload() throws Exception {
RepositoryMetadata metadata = new DefaultRepositoryMetadata(DummyRepository.class);
RepositoryInformation information = new ReactiveRepositoryInformation(metadata, ReactiveCrudRepository.class,
Optional.empty());
Method method = DummyRepository.class.getMethod("save", Iterable.class);
assertThat(information.getTargetClassMethod(method),
is(ReactiveCrudRepository.class.getMethod("save", Iterable.class)));
}
interface RxJava1InterfaceWithGenerics extends RxJava1CrudRepository<User, String> {}
interface ReactiveJavaInterfaceWithGenerics extends ReactiveCrudRepository<User, String> {}
@@ -121,6 +136,12 @@ public class ReactiveRepositoryInformationUnitTests {
}
interface DummyRepository extends ReactiveCrudRepository<User, Integer> {
@Override
<S extends User> Flux<S> save(Iterable<S> entities);
}
static class User {
String id;