From 530e7c773e759f70076fc8cdb5b3f50b7be406f3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 24 Aug 2017 21:50:31 +0200 Subject: [PATCH] DATAREST-1121 - Skip last modified detection for query methods that project. For an execution of a projecting query method we now skip the last modified detection as it actually doesn't make sense if applied to non-aggregates. --- .../data/rest/webmvc/jpa/PersonRepository.java | 4 ++++ ...ositorySearchControllerIntegrationTests.java | 17 ++++++++++++++++- .../data/rest/webmvc/HttpHeadersPreparer.java | 3 +++ .../rest/webmvc/RepositorySearchController.java | 5 +++++ .../data/rest/webmvc/ResourceStatus.java | 14 ++++++++++++-- .../rest/webmvc/ResourceStatusUnitTests.java | 14 ++++++++++++++ 6 files changed, 54 insertions(+), 3 deletions(-) diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/PersonRepository.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/PersonRepository.java index ef311392e..c8d3a6558 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/PersonRepository.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/main/java/org/springframework/data/rest/webmvc/jpa/PersonRepository.java @@ -51,4 +51,8 @@ public interface PersonRepository extends PagingAndSortingRepository :date") Page findByCreatedUsingISO8601Date(@Param("date") @DateTimeFormat(iso = ISO.DATE_TIME) Date date, Pageable pageable); + + // DATAREST-1121 + @Query("select p.created from Person p where p.lastName = :lastname") + Date findCreatedDateByLastName(@Param("lastname") String lastName); } diff --git a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java index 292f9e8eb..d60a721d8 100755 --- a/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-jpa/src/test/java/org/springframework/data/rest/webmvc/RepositorySearchControllerIntegrationTests.java @@ -41,6 +41,7 @@ import org.springframework.hateoas.Resources; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.test.context.ContextConfiguration; import org.springframework.transaction.annotation.Transactional; @@ -74,7 +75,7 @@ public class RepositorySearchControllerIntegrationTests extends AbstractControll ResourceSupport resource = controller.listSearches(request); ResourceTester tester = ResourceTester.of(resource); - tester.assertNumberOfLinks(6); // Self link included + tester.assertNumberOfLinks(7); // Self link included tester.assertHasLinkEndingWith("findFirstPersonByFirstName", "findFirstPersonByFirstName{?firstname,projection}"); tester.assertHasLinkEndingWith("firstname", "firstname{?firstname,page,size,sort,projection}"); tester.assertHasLinkEndingWith("lastname", "lastname{?lastname,sort,projection}"); @@ -82,6 +83,7 @@ public class RepositorySearchControllerIntegrationTests extends AbstractControll "findByCreatedUsingISO8601Date{?date,page,size,sort,projection}"); tester.assertHasLinkEndingWith("findByCreatedGreaterThan", "findByCreatedGreaterThan{?date,page,size,sort,projection}"); + tester.assertHasLinkEndingWith("findCreatedDateByLastName", "findCreatedDateByLastName{?lastname}"); } @Test(expected = ResourceNotFoundException.class) @@ -177,4 +179,17 @@ public class RepositorySearchControllerIntegrationTests extends AbstractControll assertThat(searches.getDomainType()).isAssignableFrom(Person.class); } + + @Test // DATAREST-1121 + public void returnsSimpleResponseEntityForQueryMethod() { + + MultiValueMap parameters = new LinkedMultiValueMap(); + parameters.add("lastname", "Thornton"); + + ResponseEntity entity = controller.executeSearch(getResourceInformation(Person.class), parameters, + "findCreatedDateByLastName", PAGEABLE, Sort.unsorted(), assembler, new HttpHeaders()); + + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getHeaders()).isEmpty(); + } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/HttpHeadersPreparer.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/HttpHeadersPreparer.java index f1c6d0ceb..db25e133d 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/HttpHeadersPreparer.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/HttpHeadersPreparer.java @@ -73,6 +73,9 @@ public class HttpHeadersPreparer { */ public HttpHeaders prepareHeaders(PersistentEntity entity, Object value) { + Assert.notNull(entity, "PersistentEntity must not be null!"); + Assert.notNull(value, "Entity value must not be null!"); + // Add ETag HttpHeaders headers = ETag.from(entity, value).addTo(new HttpHeaders()); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositorySearchController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositorySearchController.java index cadc91225..59d34d28f 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositorySearchController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositorySearchController.java @@ -213,6 +213,11 @@ class RepositorySearchController extends AbstractRepositoryRestController { PersistentEntity entity = information.getPersistentEntity(); + // Returned value is not of the aggregates type - probably some projection + if (!entity.getType().isInstance(it)) { + return ResponseEntity.ok(it); + } + return resourceStatus.getStatusAndHeaders(headers, it, entity).toResponseEntity(// () -> assembler.toFullResource(it)); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceStatus.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceStatus.java index 59775d0cb..2ea3b5ec9 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceStatus.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/ResourceStatus.java @@ -29,6 +29,7 @@ import org.springframework.hateoas.Resource; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.util.Assert; /** * Simple abstraction to capture the status of a resource to determine whether it has been modified or not and produce @@ -41,6 +42,8 @@ import org.springframework.http.ResponseEntity; @RequiredArgsConstructor(staticName = "of") class ResourceStatus { + private static final String INVALID_DOMAIN_OBJECT = "Domain object %s is not an instance of the given PersistentEntity of type %s!"; + private final @NonNull HttpHeadersPreparer preparer; /** @@ -49,12 +52,18 @@ class ResourceStatus { * * @param requestHeaders must not be {@literal null}. * @param domainObject must not be {@literal null}. - * @param information + * @param entity must not be {@literal null}. * @return */ public StatusAndHeaders getStatusAndHeaders(HttpHeaders requestHeaders, Object domainObject, PersistentEntity entity) { + Assert.notNull(requestHeaders, "Request headers must not be null!"); + Assert.notNull(domainObject, "Domain object must not be null!"); + Assert.notNull(entity, "PersistentEntity must not be null!"); + Assert.isTrue(entity.getType().isInstance(domainObject), + String.format(INVALID_DOMAIN_OBJECT, domainObject, entity.getType())); + // Check ETag for If-Non-Match List ifNoneMatch = requestHeaders.getIfNoneMatch(); @@ -64,7 +73,8 @@ class ResourceStatus { // Check last modification for If-Modified-Since return eTag.matches(entity, domainObject) || preparer.isObjectStillValid(domainObject, requestHeaders) - ? StatusAndHeaders.notModified(responseHeaders) : StatusAndHeaders.modified(responseHeaders); + ? StatusAndHeaders.notModified(responseHeaders) + : StatusAndHeaders.modified(responseHeaders); } @RequiredArgsConstructor(access = AccessLevel.PRIVATE) diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceStatusUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceStatusUnitTests.java index 6227f30eb..0b351b00f 100755 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceStatusUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/ResourceStatusUnitTests.java @@ -21,10 +21,13 @@ import static org.mockito.Mockito.*; import lombok.Value; +import java.util.Date; import java.util.function.Supplier; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -49,6 +52,8 @@ public class ResourceStatusUnitTests { @Mock HttpHeadersPreparer preparer; @Mock Supplier supplier; + public @Rule ExpectedException exception = ExpectedException.none(); + @Before public void setUp() { @@ -87,6 +92,15 @@ public class ResourceStatusUnitTests { assertNotModified(status.getStatusAndHeaders(new HttpHeaders(), new Sample(0), entity)); } + @Test // DATAREST-1121 + public void rejectsInvalidPersistentEntityDomainObjectCombination() { + + exception.expect(IllegalArgumentException.class); + exception.expectMessage(entity.getType().getName()); + + assertModified(status.getStatusAndHeaders(new HttpHeaders(), new Date(), entity)); + } + private void assertModified(StatusAndHeaders statusAndHeaders) { assertThat(statusAndHeaders.isModified()).isTrue();