Spring/spring-data-rest
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAREST-1176 - Polishing.

Browse Source 
Moved the flag to decide whether to expose repository methods by default to the RepositoryDetectionStrategy interface, so that it can be directly accessed and the test on the particular enum value is not needed anymore and thus also not duplicated into different parts of the codebase.

Added more tests to actually verify behavior on CrudMethodsSupportedHttpMethods. DefaultExposureAwareCrudMethods uses @RequiredArgumentConstructor again.
This commit is contained in:
Oliver Gierke
2018-01-15 15:08:45 +01:00
parent 5172f89281
commit 6b61d2b026
8 changed files with 88 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/CrudMethodsSupportedHttpMethods.java
View File

@@ -19,6 +19,7 @@ import static org.springframework.data.rest.core.mapping.ResourceType.*;
import static org.springframework.http.HttpMethod.*;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import java.lang.reflect.Method;
import java.util.Collections;
@@ -47,15 +48,14 @@ public class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
* Creates a new {@link CrudMethodsSupportedHttpMethods} for the given {@link CrudMethods}.
*
* @param crudMethods must not be {@literal null}.
* @param methodsExposedByDefault whether repository methods should be considered exposed by default or need to be
* annotated with {@link RestResource} to really be visible.
*/
public CrudMethodsSupportedHttpMethods(CrudMethods crudMethods, RepositoryResourceMappings provider) {
public CrudMethodsSupportedHttpMethods(CrudMethods crudMethods, boolean methodsExposedByDefault) {
Assert.notNull(crudMethods, "CrudMethods must not be null!");
boolean exportedDefault = provider.getRepositoryDetectionStrategy()
!= RepositoryDetectionStrategy.RepositoryDetectionStrategies.EXPLICIT_METHOD_ANNOTATED;
this.exposedMethods = new DefaultExposureAwareCrudMethods(crudMethods, exportedDefault);
this.exposedMethods = new DefaultExposureAwareCrudMethods(crudMethods, methodsExposedByDefault);
}
/*
@@ -141,16 +141,12 @@ public class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
/**
* @author Oliver Gierke
*/
@RequiredArgsConstructor
private static class DefaultExposureAwareCrudMethods implements ExposureAwareCrudMethods {
private final @NonNull CrudMethods crudMethods;
private final boolean exportedDefault;
DefaultExposureAwareCrudMethods(CrudMethods crudMethods, boolean exportedDefault) {
this.crudMethods = crudMethods;
this.exportedDefault = exportedDefault;
}
/*
* (non-Javadoc)
* @see org.springframework.data.rest.core.mapping.ExposureAwareCrudMethods#exposesSave()

3
spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryAwareResourceMetadata.java
View File

@@ -57,7 +57,8 @@ class RepositoryAwareResourceMetadata implements ResourceMetadata {
this.mapping = mapping;
this.provider = provider;
this.repositoryMetadata = repositoryMetadata;
this.crudMethodsSupportedHttpMethods = new CrudMethodsSupportedHttpMethods(repositoryMetadata.getCrudMethods(), provider);
this.crudMethodsSupportedHttpMethods = new CrudMethodsSupportedHttpMethods(repositoryMetadata.getCrudMethods(),
provider.exposeMethodsByDefault());
}
/**

30
spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryDetectionStrategy.java
View File

@@ -26,6 +26,7 @@ import org.springframework.data.rest.core.annotation.RestResource;
* The strategy to determine whether a given repository is to be exported by Spring Data REST.
*
* @author Oliver Gierke
* @author Tobias Weiß
* @since 2.5
* @soundtrack Katinka - Ausverkauf
*/
@@ -39,6 +40,17 @@ public interface RepositoryDetectionStrategy {
*/
boolean isExported(RepositoryMetadata metadata);
/**
* Returns whether to expose repository methods by default, i.e. without the need to explicitly annotate them with
* {@link RestResource}.
*
* @return
* @since 3.1
*/
default boolean exposeMethodsByDefault() {
return true;
}
/**
* A variety of strategies to determine repository exposure.
*
@@ -99,15 +111,27 @@ public interface RepositoryDetectionStrategy {
},
/**
* Behaves like the annotated strategy on repository level. But it does not export all methods
* of an exported Repository. The methods have to be annotated explicitly too.
* Behaves like the {@link RepositoryDetectionStrategies#ANNOTATED} strategy on repository level, but only exports
* the methods of the repository that have been explicitly annotated with {@link RestResource}. CRUD methods need to
* be annotated, too, for the default collection resource exposure to be applied.
*
* @since 3.1
*/
EXPLICIT_METHOD_ANNOTATED {
EXPLICITLY_ANNOTATED {
@Override
public boolean isExported(RepositoryMetadata metadata) {
return isExplicitlyExported(metadata.getRepositoryInterface(), false);
}
/*
* (non-Javadoc)
* @see org.springframework.data.rest.core.mapping.RepositoryDetectionStrategy#exposeMethodsByDefault()
*/
@Override
public boolean exposeMethodsByDefault() {
return false;
}
};
/**

15
spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryMethodResourceMapping.java
View File

@@ -42,7 +42,6 @@ import org.springframework.util.StringUtils;
*/
class RepositoryMethodResourceMapping implements MethodResourceMapping {
@SuppressWarnings("unchecked") //
private static final Collection<Class<?>> IMPLICIT_PARAMETER_TYPES = Arrays.asList(Pageable.class, Sort.class);
private static final AnnotationAttribute PARAM_VALUE = new AnnotationAttribute(Param.class);
@@ -61,9 +60,11 @@ class RepositoryMethodResourceMapping implements MethodResourceMapping {
*
* @param method must not be {@literal null}.
* @param resourceMapping must not be {@literal null}.
* @param metadata can be {@literal null}.
* @param whether the methods are supposed to be exported by default.
*/
public RepositoryMethodResourceMapping(Method method, ResourceMapping resourceMapping, RepositoryMetadata metadata,
RepositoryDetectionStrategy strategy) {
boolean exposeMethodsByDefault) {
Assert.notNull(method, "Method must not be null!");
Assert.notNull(resourceMapping, "ResourceMapping must not be null!");
@@ -71,7 +72,7 @@ class RepositoryMethodResourceMapping implements MethodResourceMapping {
RestResource annotation = AnnotationUtils.findAnnotation(method, RestResource.class);
String resourceRel = resourceMapping.getRel();
this.isExported = determineIsExported(strategy, annotation);
this.isExported = annotation != null ? annotation.exported() : exposeMethodsByDefault;
this.rel = annotation == null || !StringUtils.hasText(annotation.rel()) ? method.getName() : annotation.rel();
this.path = annotation == null || !StringUtils.hasText(annotation.path()) ? new Path(method.getName())
: new Path(annotation.path());
@@ -85,14 +86,6 @@ class RepositoryMethodResourceMapping implements MethodResourceMapping {
this.metadata = metadata;
}
private boolean determineIsExported(RepositoryDetectionStrategy strategy, RestResource annotation) {
boolean exportedDefault = strategy
!= RepositoryDetectionStrategy.RepositoryDetectionStrategies.EXPLICIT_METHOD_ANNOTATED;
return annotation != null ? annotation.exported() : exportedDefault;
}
private static final List<ParameterMetadata> discoverParameterMetadata(Method method, String baseRel) {
List<ParameterMetadata> result = new ArrayList<ParameterMetadata>();

18
spring-data-rest-core/src/main/java/org/springframework/data/rest/core/mapping/RepositoryResourceMappings.java
View File

@@ -26,6 +26,7 @@ import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.repository.core.RepositoryInformation;
import org.springframework.data.repository.support.Repositories;
import org.springframework.data.rest.core.annotation.RestResource;
import org.springframework.data.rest.core.config.RepositoryRestConfiguration;
import org.springframework.hateoas.RelProvider;
import org.springframework.hateoas.core.EvoInflectorRelProvider;
@@ -65,8 +66,8 @@ public class RepositoryResourceMappings extends PersistentEntitiesResourceMappin
* @param strategy must not be {@literal null}.
* @param relProvider must not be {@literal null}.
*/
public RepositoryResourceMappings(Repositories repositories, PersistentEntities entities, RepositoryDetectionStrategy strategy,
RelProvider relProvider) {
public RepositoryResourceMappings(Repositories repositories, PersistentEntities entities,
RepositoryDetectionStrategy strategy, RelProvider relProvider) {
super(entities);
@@ -120,7 +121,7 @@ public class RepositoryResourceMappings extends PersistentEntitiesResourceMappin
if (resourceMapping.isExported()) {
for (Method queryMethod : repositoryInformation.getQueryMethods()) {
RepositoryMethodResourceMapping methodMapping = new RepositoryMethodResourceMapping(queryMethod,
resourceMapping, repositoryInformation, strategy);
resourceMapping, repositoryInformation, exposeMethodsByDefault());
if (methodMapping.isExported()) {
mappings.add(methodMapping);
}
@@ -159,7 +160,14 @@ public class RepositoryResourceMappings extends PersistentEntitiesResourceMappin
return repositories.hasRepositoryFor(property.getActualType()) && super.isMapped(property);
}
public RepositoryDetectionStrategy getRepositoryDetectionStrategy() {
return strategy;
/**
* Returns whether to expose repository methods by default, i.e. without the need to explicitly annotate them with
* {@link RestResource}.
*
* @since 3.1
* @see RepositoryDetectionStrategy#exposeMethodsByDefault()
*/
public boolean exposeMethodsByDefault() {
return strategy.exposeMethodsByDefault();
}
}

33
spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/CrudMethodsSupportedHttpMethodsUnitTests.java
View File

@@ -16,6 +16,7 @@
package org.springframework.data.rest.core.mapping;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import static org.springframework.data.rest.core.mapping.ResourceType.*;
import static org.springframework.http.HttpMethod.*;
@@ -23,6 +24,7 @@ import java.util.List;
import java.util.Optional;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
@@ -48,8 +50,12 @@ import org.springframework.http.HttpMethod;
@RunWith(MockitoJUnitRunner.class)
public class CrudMethodsSupportedHttpMethodsUnitTests {
@Mock
private RepositoryResourceMappings mappings;
@Mock RepositoryResourceMappings mappings;
@Before
public void setUp() {
when(mappings.exposeMethodsByDefault()).thenReturn(true);
}
@Test // DATACMNS-589, DATAREST-409
public void doesNotSupportAnyHttpMethodForEmptyRepository() {
@@ -122,12 +128,24 @@ public class CrudMethodsSupportedHttpMethodsUnitTests {
assertMethodsSupported(getSupportedHttpMethodsFor(NoFindOne.class), ITEM, false, DELETE);
}
@Test // DATAREST-1176
public void onlyExposesExplicitlyAnnotatedMethodsIfConfigured() {
reset(mappings);
when(mappings.exposeMethodsByDefault()).thenReturn(false);
assertMethodsSupported(getSupportedHttpMethodsFor(MethodsExplicitlyExportedRepository.class), COLLECTION, true,
POST, OPTIONS);
assertMethodsSupported(getSupportedHttpMethodsFor(MethodsExplicitlyExportedRepository.class), ITEM, true, OPTIONS,
PUT, PATCH);
}
private SupportedHttpMethods getSupportedHttpMethodsFor(Class<?> repositoryInterface) {
RepositoryMetadata metadata = new DefaultRepositoryMetadata(repositoryInterface);
CrudMethods crudMethods = new DefaultCrudMethods(metadata);
return new CrudMethodsSupportedHttpMethods(crudMethods, mappings);
return new CrudMethodsSupportedHttpMethods(crudMethods, mappings.exposeMethodsByDefault());
}
private static void assertMethodsSupported(SupportedHttpMethods methods, ResourceType type, boolean supported,
@@ -165,6 +183,15 @@ public class CrudMethodsSupportedHttpMethodsUnitTests {
interface EntityRepository extends CrudRepository<Entity, Long> {}
interface MethodsExplicitlyExportedRepository extends Repository<Object, Long> {
@RestResource
<S extends Object> S save(S entity);
@RestResource
void delete(Object entity);
}
class Entity {
Entity embedded;

4
spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/RepositoryDetectionStrategiesUnitTests.java
View File

@@ -92,7 +92,7 @@ public class RepositoryDetectionStrategiesUnitTests {
@Test // DATAREST-1176
public void onlyExplicitAnnotatedMethodsAreExposed() {
assertExposures(EXPLICIT_METHOD_ANNOTATED, new HashMap<Class<?>, Boolean>() {
assertExposures(EXPLICITLY_ANNOTATED, new HashMap<Class<?>, Boolean>() {
{
put(AnnotatedRepository.class, true);
put(HiddenRepository.class, false);
@@ -100,6 +100,8 @@ public class RepositoryDetectionStrategiesUnitTests {
put(PackageProtectedRepository.class, false);
}
});
assertThat(EXPLICITLY_ANNOTATED.exposeMethodsByDefault()).isFalse();
}
private static void assertExposures(RepositoryDetectionStrategy strategy, Map<Class<?>, Boolean> expected) {

7
spring-data-rest-core/src/test/java/org/springframework/data/rest/core/mapping/RepositoryMethodResourceMappingUnitTests.java
View File

@@ -38,9 +38,9 @@ import org.springframework.data.rest.core.mapping.RepositoryDetectionStrategy.Re
*/
public class RepositoryMethodResourceMappingUnitTests {
RepositoryDetectionStrategy strategy = RepositoryDetectionStrategies.DEFAULT;
RepositoryMetadata metadata = new DefaultRepositoryMetadata(PersonRepository.class);
RepositoryCollectionResourceMapping resourceMapping = new RepositoryCollectionResourceMapping(metadata,
RepositoryDetectionStrategies.DEFAULT);
RepositoryCollectionResourceMapping resourceMapping = new RepositoryCollectionResourceMapping(metadata, strategy);
@Test
public void defaultsMappingToMethodName() throws Exception {
@@ -131,8 +131,7 @@ public class RepositoryMethodResourceMappingUnitTests {
}
private RepositoryMethodResourceMapping getMappingFor(Method method) {
RepositoryDetectionStrategy strategy = RepositoryDetectionStrategies.DEFAULT;
return new RepositoryMethodResourceMapping(method, resourceMapping, metadata, strategy);
return new RepositoryMethodResourceMapping(method, resourceMapping, metadata, strategy.exposeMethodsByDefault());
}
static class Person {}