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

DATAREST-1176 - Add ability to only expose repository methods explicitly declared for exposure #286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static org.springframework.http.HttpMethod.*;

import lombok.NonNull;
import lombok.RequiredArgsConstructor;

import java.lang.reflect.Method;
import java.util.Collections;
Expand Down Expand Up @@ -49,11 +48,14 @@ public class CrudMethodsSupportedHttpMethods implements SupportedHttpMethods {
*
* @param crudMethods must not be {@literal null}.
*/
public CrudMethodsSupportedHttpMethods(CrudMethods crudMethods) {
public CrudMethodsSupportedHttpMethods(CrudMethods crudMethods, RepositoryResourceMappings provider) {

Assert.notNull(crudMethods, "CrudMethods must not be null!");

this.exposedMethods = new DefaultExposureAwareCrudMethods(crudMethods);
boolean exportedDefault = provider.getRepositoryDetectionStrategy()
!= RepositoryDetectionStrategy.RepositoryDetectionStrategies.EXPLICIT_METHOD_ANNOTATED;

this.exposedMethods = new DefaultExposureAwareCrudMethods(crudMethods, exportedDefault);
}

/*
Expand Down Expand Up @@ -139,10 +141,15 @@ public Set<HttpMethod> getMethodsFor(PersistentProperty<?> property) {
/**
* @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)
Expand Down Expand Up @@ -180,12 +187,12 @@ public boolean exposesFindAll() {
return exposes(crudMethods.getFindAllMethod());
}

private static boolean exposes(Optional<Method> method) {
private boolean exposes(Optional<Method> method) {

return method.map(it -> {

RestResource annotation = AnnotationUtils.findAnnotation(it, RestResource.class);
return annotation == null ? true : annotation.exported();
return annotation == null ? exportedDefault : annotation.exported();

}).orElse(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public RepositoryAwareResourceMetadata(PersistentEntity<?, ?> entity, Collection
this.mapping = mapping;
this.provider = provider;
this.repositoryMetadata = repositoryMetadata;
this.crudMethodsSupportedHttpMethods = new CrudMethodsSupportedHttpMethods(repositoryMetadata.getCrudMethods());
this.crudMethodsSupportedHttpMethods = new CrudMethodsSupportedHttpMethods(repositoryMetadata.getCrudMethods(), provider);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ public boolean isExported(RepositoryMetadata metadata) {
*/
ANNOTATED {

@Override
public boolean isExported(RepositoryMetadata metadata) {
return isExplicitlyExported(metadata.getRepositoryInterface(), false);
}
},

/**
* 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.
*/
EXPLICIT_METHOD_ANNOTATED {

@Override
public boolean isExported(RepositoryMetadata metadata) {
return isExplicitlyExported(metadata.getRepositoryInterface(), false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,16 @@ class RepositoryMethodResourceMapping implements MethodResourceMapping {
* @param method must not be {@literal null}.
* @param resourceMapping must not be {@literal null}.
*/
public RepositoryMethodResourceMapping(Method method, ResourceMapping resourceMapping, RepositoryMetadata metadata) {
public RepositoryMethodResourceMapping(Method method, ResourceMapping resourceMapping, RepositoryMetadata metadata,
RepositoryDetectionStrategy strategy) {

Assert.notNull(method, "Method must not be null!");
Assert.notNull(resourceMapping, "ResourceMapping must not be null!");

RestResource annotation = AnnotationUtils.findAnnotation(method, RestResource.class);
String resourceRel = resourceMapping.getRel();

this.isExported = annotation != null ? annotation.exported() : true;
this.isExported = determineIsExported(strategy, annotation);
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());
Expand All @@ -84,6 +85,14 @@ public RepositoryMethodResourceMapping(Method method, ResourceMapping resourceMa
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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
public class RepositoryResourceMappings extends PersistentEntitiesResourceMappings {

private final Repositories repositories;
private final RepositoryDetectionStrategy strategy;
private final Map<Class<?>, SearchResourceMappings> searchCache = new HashMap<Class<?>, SearchResourceMappings>();

/**
Expand Down Expand Up @@ -73,6 +74,7 @@ public RepositoryResourceMappings(Repositories repositories, PersistentEntities
Assert.notNull(strategy, "RepositoryDetectionStrategy must not be null!");

this.repositories = repositories;
this.strategy = strategy;
this.populateCache(repositories, relProvider, strategy);
}

Expand Down Expand Up @@ -118,7 +120,7 @@ public SearchResourceMappings getSearchResourceMappings(Class<?> domainType) {
if (resourceMapping.isExported()) {
for (Method queryMethod : repositoryInformation.getQueryMethods()) {
RepositoryMethodResourceMapping methodMapping = new RepositoryMethodResourceMapping(queryMethod,
resourceMapping, repositoryInformation);
resourceMapping, repositoryInformation, strategy);
if (methodMapping.isExported()) {
mappings.add(methodMapping);
}
Expand Down Expand Up @@ -156,4 +158,8 @@ public boolean hasMappingFor(Class<?> type) {
public boolean isMapped(PersistentProperty<?> property) {
return repositories.hasRepositoryFor(property.getActualType()) && super.isMapped(property);
}

public RepositoryDetectionStrategy getRepositoryDetectionStrategy() {
return strategy;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.data.annotation.ReadOnlyProperty;
import org.springframework.data.annotation.Reference;
Expand All @@ -47,6 +48,9 @@
@RunWith(MockitoJUnitRunner.class)
public class CrudMethodsSupportedHttpMethodsUnitTests {

@Mock
private RepositoryResourceMappings mappings;

@Test // DATACMNS-589, DATAREST-409
public void doesNotSupportAnyHttpMethodForEmptyRepository() {

Expand Down Expand Up @@ -118,12 +122,12 @@ public void doesNotSupportDeleteIfNoFindOneAvailable() {
assertMethodsSupported(getSupportedHttpMethodsFor(NoFindOne.class), ITEM, false, DELETE);
}

private static SupportedHttpMethods getSupportedHttpMethodsFor(Class<?> repositoryInterface) {
private SupportedHttpMethods getSupportedHttpMethodsFor(Class<?> repositoryInterface) {

RepositoryMetadata metadata = new DefaultRepositoryMetadata(repositoryInterface);
CrudMethods crudMethods = new DefaultCrudMethods(metadata);

return new CrudMethodsSupportedHttpMethods(crudMethods);
return new CrudMethodsSupportedHttpMethods(crudMethods, mappings);
}

private static void assertMethodsSupported(SupportedHttpMethods methods, ResourceType type, boolean supported,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ public void annotatedHonorsAnnotationsOnly() {
});
}

@Test // DATAREST-1176
public void onlyExplicitAnnotatedMethodsAreExposed() {

assertExposures(EXPLICIT_METHOD_ANNOTATED, new HashMap<Class<?>, Boolean>() {
{
put(AnnotatedRepository.class, true);
put(HiddenRepository.class, false);
put(PublicRepository.class, false);
put(PackageProtectedRepository.class, false);
}
});
}

private static void assertExposures(RepositoryDetectionStrategy strategy, Map<Class<?>, Boolean> expected) {

for (Entry<Class<?>, Boolean> entry : expected.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ public void doesNotIncludePageableAsParameter() throws Exception {
}

private RepositoryMethodResourceMapping getMappingFor(Method method) {
return new RepositoryMethodResourceMapping(method, resourceMapping, metadata);
RepositoryDetectionStrategy strategy = RepositoryDetectionStrategies.DEFAULT;
return new RepositoryMethodResourceMapping(method, resourceMapping, metadata, strategy);
}

static class Person {}
Expand Down