From 9010a9d13286ae59a56a359b2d8d80a6a2556504 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Fri, 29 Nov 2019 17:17:35 +0100 Subject: [PATCH] DATAREST-1458 - Fixed rendering of compact view to association resources. The usage of text/uri-list as media type was entirely broken and not even advertised in the reference docs anymore. It's now again supported for both to-one and to-many associations via Collections. Maps are rejected as they cannot be rendered as list of URIs correctly. Updated reference documentation accordingly. Added a custom MapModel implementation of RepresentationModel as apparently using Maps with EntityModel does not unwrap the content properly due to [0]. [0] https://github.com/FasterXML/jackson-databind/issues/171 --- .../data/rest/tests/TestMatchers.java | 81 ++++++++++++++ .../data/rest/tests/mongodb/User.java | 2 + .../rest/tests/mongodb/MongoWebTests.java | 69 +++++++++++- ...RepositoryPropertyReferenceController.java | 103 +++++++++--------- 4 files changed, 197 insertions(+), 58 deletions(-) create mode 100644 spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/TestMatchers.java diff --git a/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/TestMatchers.java b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/TestMatchers.java new file mode 100644 index 000000000..18fe7afca --- /dev/null +++ b/spring-data-rest-tests/spring-data-rest-tests-core/src/test/java/org/springframework/data/rest/tests/TestMatchers.java @@ -0,0 +1,81 @@ +/* + * Copyright 2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.rest.tests; + +import java.util.Arrays; + +import org.hamcrest.Description; +import org.hamcrest.Matcher; +import org.hamcrest.TypeSafeMatcher; +import org.springframework.util.StringUtils; + +/** + * {@link Matcher} implementations useful in tests + * + * @author Oliver Drotbohm + */ +public class TestMatchers { + + /** + * Asserts that a {@link String} consists of a given number of lines. + * + * @param number + * @return + */ + public static Matcher hasNumberOfLines(int number) { + + return new TypeSafeMatcher(String.class) { + + /* + * (non-Javadoc) + * @see org.hamcrest.SelfDescribing#describeTo(org.hamcrest.Description) + */ + @Override + public void describeTo(Description description) { + description.appendText("contains " + number + " lines"); + } + + /* + * (non-Javadoc) + * @see org.hamcrest.TypeSafeMatcher#describeMismatchSafely(java.lang.Object, org.hamcrest.Description) + */ + @Override + protected void describeMismatchSafely(String item, Description mismatchDescription) { + + mismatchDescription.appendText(item) // + .appendText(" contains ") // + .appendValue(numberOfLines(item)) // + .appendText(" lines"); + } + + /* + * (non-Javadoc) + * @see org.hamcrest.TypeSafeMatcher#matchesSafely(java.lang.Object) + */ + @Override + protected boolean matchesSafely(String item) { + return numberOfLines(item) == number; + } + + private long numberOfLines(String source) { + + return Arrays.stream(source.split("\n")) // + .filter(StringUtils::hasText) // + .count(); + } + }; + } +} diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/User.java b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/User.java index fd69eee14..f5054dc2a 100644 --- a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/User.java +++ b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/main/java/org/springframework/data/rest/tests/mongodb/User.java @@ -51,6 +51,8 @@ public static enum Gender { public org.joda.time.LocalDateTime jodaDateTime; public TypeWithPattern pattern; public @DBRef(lazy = true) List colleagues; + public @DBRef(lazy = true) User manager; + public @DBRef(lazy = true) Map map; public Map colleaguesMap = new HashMap(); public static class EmailAddress { diff --git a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/tests/mongodb/MongoWebTests.java b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/tests/mongodb/MongoWebTests.java index b2e6ee08d..ea97754d7 100755 --- a/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/tests/mongodb/MongoWebTests.java +++ b/spring-data-rest-tests/spring-data-rest-tests-mongodb/src/test/java/org/springframework/data/rest/tests/mongodb/MongoWebTests.java @@ -24,6 +24,7 @@ import java.math.BigDecimal; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import org.assertj.core.api.Condition; import org.junit.After; @@ -31,6 +32,7 @@ import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.rest.tests.CommonWebTests; +import org.springframework.data.rest.tests.TestMatchers; import org.springframework.data.rest.webmvc.RestMediaTypes; import org.springframework.data.rest.webmvc.support.RepositoryEntityLinks; import org.springframework.hateoas.IanaLinkRelations; @@ -39,6 +41,7 @@ import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.web.servlet.result.MockMvcResultHandlers; import org.springframework.web.util.UriComponentsBuilder; import com.fasterxml.jackson.annotation.JsonInclude.Include; @@ -54,6 +57,8 @@ @ContextConfiguration(classes = MongoDbRepositoryConfig.class) public class MongoWebTests extends CommonWebTests { + private static final MediaType TEXT_URI_LIST = MediaType.parseMediaType("text/uri-list"); + @Autowired ProfileRepository repository; @Autowired UserRepository userRepository; @Autowired RepositoryEntityLinks entityLinks; @@ -86,15 +91,28 @@ public void populateProfiles() { userRepository.save(thomas); + User mark = new User(); + mark.firstname = "Mark"; + mark.lastname = "Paluch"; + mark.colleagues = Arrays.asList(thomas); + + userRepository.save(mark); + User oliver = new User(); oliver.firstname = "Oliver"; oliver.lastname = "Gierke"; oliver.address = address; - oliver.colleagues = Arrays.asList(thomas); + oliver.colleagues = Arrays.asList(thomas, mark); userRepository.save(oliver); - thomas.colleagues = Arrays.asList(oliver); + thomas.manager = oliver; + thomas.colleagues = Arrays.asList(oliver, mark); + thomas.map = new HashMap<>(); + thomas.map.put("oliver", oliver); + thomas.map.put("mark", mark); + userRepository.save(thomas); + } @After @@ -328,8 +346,8 @@ public void exposesETagHeaderForSearchResourceYieldingItemResource() throws Exce Profile profile = repository.findAll().iterator().next(); mvc.perform(get(link.expand(profile.getId()).getHref()))// - .andExpect(header().string("ETag", is("\"0\"")))// - .andExpect(header().string("Last-Modified", is(notNullValue()))); + .andExpect(header().string(ETAG, is("\"0\"")))// + .andExpect(header().string(LAST_MODIFIED, is(notNullValue()))); } @Test // DATAREST-835 @@ -340,7 +358,46 @@ public void doesNotAddETagHeaderForCollectionQueryResource() throws Exception { Profile profile = repository.findAll().iterator().next(); mvc.perform(get(link.expand(profile.getType()).getHref()))// - .andExpect(header().string("ETag", is(nullValue())))// - .andExpect(header().string("Last-Modified", is(nullValue()))); + .andExpect(header().string(ETAG, is(nullValue())))// + .andExpect(header().string(LAST_MODIFIED, is(nullValue()))); + } + + @Test // DATAREST-1458 + public void accessCollectionAssociationResourceAsUriList() throws Exception { + + Link usersLink = client.discoverUnique("users"); + Link userLink = assertHasContentLinkWithRel(IanaLinkRelations.SELF, client.request(usersLink)); + Link colleaguesLink = client.assertHasLinkWithRel("colleagues", client.request(userLink)); + + mvc.perform(get(colleaguesLink.expand().getHref()).accept(TEXT_URI_LIST)) // + .andExpect(status().isOk()) // + .andExpect(header().string(CONTENT_TYPE, is(TEXT_URI_LIST.toString()))) // + .andExpect(content().string(TestMatchers.hasNumberOfLines(2))); + } + + @Test // DATAREST-1458 + public void accessAssociationResourceAsUriList() throws Exception { + + Link usersLink = client.discoverUnique("users"); + Link userLink = assertHasContentLinkWithRel(IanaLinkRelations.SELF, client.request(usersLink)); + Link managerLink = client.assertHasLinkWithRel("manager", client.request(userLink)); + + mvc.perform(get(managerLink.expand().getHref()).accept(TEXT_URI_LIST)) // + .andExpect(header().string(CONTENT_TYPE, is(TEXT_URI_LIST.toString()))) // + .andExpect(content().string(TestMatchers.hasNumberOfLines(1))); + } + + @Test // DATAREST-1458 + public void accessMapCollectionAssociationResourceAsUriList() throws Exception { + + Link usersLink = client.discoverUnique("users"); + Link userLink = assertHasContentLinkWithRel(IanaLinkRelations.SELF, client.request(usersLink)); + Link mapLink = client.assertHasLinkWithRel("map", client.request(userLink)); + + mvc.perform(get(mapLink.expand().getHref())) // + .andDo(MockMvcResultHandlers.print()); + + mvc.perform(get(mapLink.expand().getHref()).accept(TEXT_URI_LIST)) // + .andExpect(status().isUnsupportedMediaType()); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java index c47041821..dc6e92b8f 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java @@ -15,22 +15,18 @@ */ package org.springframework.data.rest.webmvc; -import static org.springframework.data.rest.webmvc.ControllerUtils.*; +import static java.util.stream.Collectors.*; import static org.springframework.data.rest.webmvc.RestMediaTypes.*; -import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.*; import static org.springframework.web.bind.annotation.RequestMethod.*; import lombok.AccessLevel; import lombok.RequiredArgsConstructor; import java.io.Serializable; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; @@ -52,7 +48,6 @@ import org.springframework.data.rest.core.event.BeforeLinkDeleteEvent; import org.springframework.data.rest.core.event.BeforeLinkSaveEvent; import org.springframework.data.rest.core.mapping.PropertyAwareResourceMapping; -import org.springframework.data.rest.core.mapping.ResourceMapping; import org.springframework.data.rest.core.mapping.ResourceMetadata; import org.springframework.data.rest.webmvc.support.BackendId; import org.springframework.data.web.PagedResourcesAssembler; @@ -61,8 +56,8 @@ import org.springframework.hateoas.IanaLinkRelations; import org.springframework.hateoas.Link; import org.springframework.hateoas.LinkRelation; +import org.springframework.hateoas.Links; import org.springframework.hateoas.RepresentationModel; -import org.springframework.hateoas.server.mvc.WebMvcLinkBuilder; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; @@ -71,7 +66,11 @@ import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.server.UnsupportedMediaTypeStatusException; + +import com.fasterxml.jackson.annotation.JsonAnyGetter; /** * @author Jon Brisbin @@ -126,13 +125,9 @@ public ResponseEntity> followPropertyReference(final Root } else if (prop.property.isMap()) { - Map> resources = new HashMap>(); - - for (Map.Entry entry : ((Map) it).entrySet()) { - resources.put(entry.getKey(), assembler.toModel(entry.getValue())); - } - - return new EntityModel(resources); + return ((Map) it).entrySet().stream() // + .collect(collectingAndThen(toMap(Map.Entry::getKey, entry -> assembler.toModel(entry.getValue())), + MapModel::new)); } else { @@ -220,54 +215,34 @@ public ResponseEntity> followPropertyReference(RootResour doWithReferencedProperty(repoRequest, id, property, handler, HttpMethod.GET)); } - @RequestMapping(value = BASE_MAPPING, method = GET, - produces = { SPRING_DATA_COMPACT_JSON_VALUE, TEXT_URI_LIST_VALUE }) + @RequestMapping(value = BASE_MAPPING, method = GET, produces = TEXT_URI_LIST_VALUE) public ResponseEntity> followPropertyReferenceCompact(RootResourceInformation repoRequest, - @BackendId Serializable id, @PathVariable String property, PersistentEntityResourceAssembler assembler) - throws Exception { - - ResponseEntity> response = followPropertyReference(repoRequest, id, property, assembler); - - if (response.getStatusCode() != HttpStatus.OK) { - return response; - } - - ResourceMetadata repoMapping = repoRequest.getResourceMetadata(); - PersistentProperty persistentProp = repoRequest.getPersistentEntity().getRequiredPersistentProperty(property); - ResourceMapping propertyMapping = repoMapping.getMappingFor(persistentProp); - - RepresentationModel resource = response.getBody(); - - List links = new ArrayList(); + @BackendId Serializable id, @PathVariable String property, @RequestHeader HttpHeaders requestHeaders, + PersistentEntityResourceAssembler assembler) throws Exception { - WebMvcLinkBuilder linkBuilder = linkTo(methodOn(RepositoryPropertyReferenceController.class) - .followPropertyReference(repoRequest, id, property, assembler)); + Function> handler = prop -> prop.mapValue(it -> { - if (resource instanceof EntityModel) { + if (prop.property.isCollectionLike()) { - Object content = ((EntityModel) resource).getContent(); - if (content instanceof Iterable) { + Links links = ((Collection) it).stream() // + .map(assembler::getExpandedSelfLink) // + .collect(Links.collector()); - for (EntityModel res : (Iterable>) content) { - links.add(linkBuilder.withRel(propertyMapping.getRel())); - } + return new RepresentationModel<>(links.toList()); - } else if (content instanceof Map) { + } else if (prop.property.isMap()) { + throw new UnsupportedMediaTypeStatusException("Cannot produce compact representation of map property!"); + } - Map> map = (Map>) content; + return new RepresentationModel<>(assembler.getExpandedSelfLink(it)); - for (Entry> entry : map.entrySet()) { - Link l = new Link(entry.getValue().getRequiredLink(IanaLinkRelations.SELF).getHref(), - entry.getKey().toString()); - links.add(l); - } - } + }).orElse(new RepresentationModel<>()); - } else { - links.add(linkBuilder.withRel(propertyMapping.getRel())); - } + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(TEXT_URI_LIST); - return ControllerUtils.toResponseEntity(HttpStatus.OK, null, new EntityModel(EMPTY_RESOURCE_LIST, links)); + return ControllerUtils.toResponseEntity(HttpStatus.OK, headers, + doWithReferencedProperty(repoRequest, id, property, handler, HttpMethod.GET)); } @RequestMapping(value = BASE_MAPPING, method = { PATCH, PUT, POST }, // @@ -462,6 +437,30 @@ public ResponseEntity handle(HttpRequestMethodNotSupportedException except return exception.toResponse(); } + /** + * Custom {@link RepresentationModel} to be used with maps as {@link EntityModel} doesn't properly unwrap {@link Map}s + * due to some limitation in Jackson. + * + * @author Oliver Drotbohm + * @see https://github.com/FasterXML/jackson-databind/issues/171 + */ + private static class MapModel extends RepresentationModel { + + private Map content; + + public MapModel(Map content, Link... links) { + + super(Arrays.asList(links)); + + this.content = content; + } + + @JsonAnyGetter + public Map getContent() { + return content; + } + } + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) static class HttpRequestMethodNotSupportedException extends RuntimeException {