Skip to content

Commit

Permalink
Reduce public API surface.
Browse files Browse the repository at this point in the history
Limit exposure of API methods to package visibility.
Also remove fields and methods not directly required to support current feature scope.
Finally add missing since tags and provide additional tests.

See: #2971
  • Loading branch information
christophstrobl committed Oct 12, 2023
1 parent 5d25645 commit 5afb8ad
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package org.springframework.data.mongodb.repository.support;

import java.lang.reflect.Method;
import java.util.Optional;

import com.mongodb.ReadPreference;
Expand All @@ -25,6 +24,7 @@
* execution.
*
* @author Mark Paluch
* @author Christoph Strobl
* @since 4.2
*/
public interface CrudMethodMetadata {
Expand All @@ -36,10 +36,4 @@ public interface CrudMethodMetadata {
*/
Optional<ReadPreference> getReadPreference();

/**
* Returns the {@link Method} that this metadata applies to.
*
* @return the {@link Method} that this metadata applies to.
*/
Method getMethod();
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
* information or query hints on them.
*
* @author Mark Paluch
* @author Christoph Strobl
* @since 4.2
*/
class CrudMethodMetadataPostProcessor implements RepositoryProxyPostProcessor, BeanClassLoaderAware {

Expand Down Expand Up @@ -103,13 +105,15 @@ static class CrudMethodMetadataPopulatingMethodInterceptor implements MethodInte
*/
static MethodInvocation currentInvocation() throws IllegalStateException {

MethodInvocation mi = currentInvocation.get();
MethodInvocation invocation = currentInvocation.get();

if (mi == null)
throw new IllegalStateException(
"No MethodInvocation found: Check that an AOP invocation is in progress, and that the "
+ "CrudMethodMetadataPopulatingMethodInterceptor is upfront in the interceptor chain.");
return mi;
if (invocation != null) {
return invocation;
}

throw new IllegalStateException(
"No MethodInvocation found: Check that an AOP invocation is in progress, and that the "
+ "CrudMethodMetadataPopulatingMethodInterceptor is upfront in the interceptor chain.");
}

@Override
Expand Down Expand Up @@ -163,7 +167,6 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
static class DefaultCrudMethodMetadata implements CrudMethodMetadata {

private final Optional<ReadPreference> readPreference;
private final Method method;

/**
* Creates a new {@link DefaultCrudMethodMetadata} for the given {@link Method}.
Expand All @@ -175,7 +178,6 @@ static class DefaultCrudMethodMetadata implements CrudMethodMetadata {
Assert.notNull(method, "Method must not be null");

this.readPreference = findReadPreference(method);
this.method = method;
}

private Optional<ReadPreference> findReadPreference(Method method) {
Expand All @@ -201,11 +203,6 @@ private Optional<ReadPreference> findReadPreference(Method method) {
public Optional<ReadPreference> getReadPreference() {
return readPreference;
}

@Override
public Method getMethod() {
return method;
}
}

private static class ThreadBoundTargetSource implements TargetSource {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,9 @@ public <S extends T, R> R findBy(Example<S> example,
* applied to queries.
*
* @param crudMethodMetadata
* @since 4.2
*/
public void setRepositoryMethodMetadata(CrudMethodMetadata crudMethodMetadata) {
void setRepositoryMethodMetadata(CrudMethodMetadata crudMethodMetadata) {
this.crudMethodMetadata = crudMethodMetadata;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,9 @@ public <S extends T, R, P extends Publisher<R>> P findBy(Example<S> example,
* applied to queries.
*
* @param crudMethodMetadata
* @since 4.2
*/
public void setRepositoryMethodMetadata(CrudMethodMetadata crudMethodMetadata) {
void setRepositoryMethodMetadata(CrudMethodMetadata crudMethodMetadata) {
this.crudMethodMetadata = crudMethodMetadata;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.Serializable;
import java.util.Optional;
import java.util.Set;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -41,13 +42,15 @@
import org.springframework.data.mongodb.repository.Person;
import org.springframework.data.mongodb.repository.ReadPreference;
import org.springframework.data.mongodb.repository.query.MongoEntityInformation;
import org.springframework.data.repository.ListCrudRepository;
import org.springframework.data.repository.Repository;

/**
* Unit test for {@link MongoRepositoryFactory}.
*
* @author Oliver Gierke
* @author Mark Paluch
* @author Christoph Strobl
*/
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
Expand Down Expand Up @@ -92,7 +95,21 @@ void considersCrudMethodMetadata() {
assertThat(value.getReadPreference()).isEqualTo(com.mongodb.ReadPreference.secondary());
}

interface MyPersonRepository extends Repository<Person, Long> {
@Test // GH-2971
void ignoresCrudMethodMetadataOnNonAnnotatedMethods() {

MongoRepositoryFactory factory = new MongoRepositoryFactory(template);
MyPersonRepository repository = factory.getRepository(MyPersonRepository.class);
repository.findAllById(Set.of(42L));

ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
verify(template).find(captor.capture(), eq(Person.class), eq("person"));

Query value = captor.getValue();
assertThat(value.getReadPreference()).isNull();
}

interface MyPersonRepository extends ListCrudRepository<Person, Long> {

@ReadPreference("secondary")
Optional<Person> findById(Long id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;

import java.util.Set;

import org.springframework.data.repository.reactive.ReactiveCrudRepository;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -72,7 +76,23 @@ void considersCrudMethodMetadata() {
assertThat(value.getReadPreference()).isEqualTo(com.mongodb.ReadPreference.secondary());
}

interface MyPersonRepository extends Repository<Person, Long> {
@Test // GH-2971
void ignoresCrudMethodMetadataOnNonAnnotatedMethods() {

when(template.find(any(), any(), anyString())).thenReturn(Flux.empty());

ReactiveMongoRepositoryFactory factory = new ReactiveMongoRepositoryFactory(template);
MyPersonRepository repository = factory.getRepository(MyPersonRepository.class);
repository.findAllById(Set.of(42L));

ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
verify(template).find(captor.capture(), eq(Person.class), eq("person"));

Query value = captor.getValue();
assertThat(value.getReadPreference()).isNull();
}

interface MyPersonRepository extends ReactiveCrudRepository<Person, Long> {

@ReadPreference("secondary")
Mono<Person> findById(Long id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ void shouldAddReadPreferenceToFindAllMethods(
public Optional<com.mongodb.ReadPreference> getReadPreference() {
return Optional.of(com.mongodb.ReadPreference.secondaryPreferred());
}

@Override
public Method getMethod() {
return null;
}
});
when(mongoOperations.find(any(), any(), any())).thenReturn(Flux.just("ok"));

Expand All @@ -186,11 +181,6 @@ void shouldAddReadPreferenceToFindOne() {
public Optional<com.mongodb.ReadPreference> getReadPreference() {
return Optional.of(com.mongodb.ReadPreference.secondaryPreferred());
}

@Override
public Method getMethod() {
return null;
}
});
when(mongoOperations.find(any(), any(), any())).thenReturn(Flux.just("ok"));

Expand Down Expand Up @@ -218,11 +208,6 @@ void shouldAddReadPreferenceToFluentFetchable() {
public Optional<com.mongodb.ReadPreference> getReadPreference() {
return Optional.of(com.mongodb.ReadPreference.secondaryPreferred());
}

@Override
public Method getMethod() {
return null;
}
});

repository.findBy(Example.of(new TestDummy()), FluentQuery.ReactiveFluentQuery::all).subscribe();
Expand Down

0 comments on commit 5afb8ad

Please sign in to comment.