From ec95f9e14f14c6391c6beaccc68372c5145f1f45 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 12 Sep 2016 17:26:09 +0200 Subject: [PATCH] DATAREST-883 - Polishing. Switched to use @RequiredArgsConstructor where possible. Slightly rearranged test cases. Inlined JacksonMappingAwareSortTranslator to not expose it as bean unless necessary. Use static Jackson BeanClassIntrospector to avoid unnecessary recreation. Original pull request: #222. --- .../RepositoryRestMvcConfiguration.java | 21 ++-- .../JacksonMappingAwareSortTranslator.java | 12 +-- .../rest/webmvc/json/MappedProperties.java | 10 +- ...wareDefaultedPageableArgumentResolver.java | 6 +- .../MappingAwarePageableArgumentResolver.java | 6 +- .../MappingAwareSortArgumentResolver.java | 9 +- .../webmvc/support/DomainClassResolver.java | 7 +- .../data/rest/webmvc/jpa/JpaWebTests.java | 99 +++++++++---------- .../rest/webmvc/jpa/TestDataPopulator.java | 1 + .../webmvc/json/SortTranslatorUnitTests.java | 1 + 10 files changed, 82 insertions(+), 90 deletions(-) diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java index f3a2133e7..0492666af 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/config/RepositoryRestMvcConfiguration.java @@ -33,10 +33,10 @@ import org.springframework.beans.factory.config.PropertiesFactoryBean; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.ComponentScan.Filter; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.context.annotation.ImportResource; -import org.springframework.context.annotation.ComponentScan.Filter; import org.springframework.context.support.MessageSourceAccessor; import org.springframework.context.support.ReloadableResourceBundleMessageSource; import org.springframework.core.Ordered; @@ -156,7 +156,7 @@ import com.fasterxml.jackson.databind.SerializationFeature; @Configuration @EnableHypermediaSupport(type = HypermediaType.HAL) @ComponentScan(basePackageClasses = RepositoryRestController.class, - includeFilters = @Filter(BasePathAwareController.class) , useDefaultFilters = false) + includeFilters = @Filter(BasePathAwareController.class), useDefaultFilters = false) @ImportResource("classpath*:META-INF/spring-data-rest/**/*.xml") @Import({ SpringDataJacksonConfiguration.class, EnableSpringDataWebSupport.QuerydslActivator.class }) public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebConfiguration implements InitializingBean { @@ -693,12 +693,6 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon return resolver; } - @Bean - public JacksonMappingAwareSortTranslator sortMethodArgumentTranslator() { - return new JacksonMappingAwareSortTranslator(objectMapper(), repositories(), - DomainClassResolver.create(repositories(), resourceMappings(), baseUri())); - } - @Bean public PluginRegistry> backendIdConverterRegistry() { @@ -726,13 +720,14 @@ public class RepositoryRestMvcConfiguration extends HateoasAwareSpringDataWebCon HateoasPageableHandlerMethodArgumentResolver pageableResolver = pageableResolver(); + JacksonMappingAwareSortTranslator sortTranslator = new JacksonMappingAwareSortTranslator(objectMapper(), + repositories(), DomainClassResolver.of(repositories(), resourceMappings(), baseUri())); - HandlerMethodArgumentResolver sortResolver = new MappingAwareSortArgumentResolver(sortMethodArgumentTranslator(), - sortResolver()); - HandlerMethodArgumentResolver jacksonPageableResolver = new MappingAwarePageableArgumentResolver( - sortMethodArgumentTranslator(), pageableResolver); + HandlerMethodArgumentResolver sortResolver = new MappingAwareSortArgumentResolver(sortTranslator, sortResolver()); + HandlerMethodArgumentResolver jacksonPageableResolver = new MappingAwarePageableArgumentResolver(sortTranslator, + pageableResolver); HandlerMethodArgumentResolver defaultedPageableResolver = new MappingAwareDefaultedPageableArgumentResolver( - sortMethodArgumentTranslator(), pageableResolver); + sortTranslator, pageableResolver); return Arrays.asList(defaultedPageableResolver, jacksonPageableResolver, sortResolver, serverHttpRequestMethodArgumentResolver(), repoRequestArgumentResolver(), persistentEntityArgumentResolver(), diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java index 8954cf5a9..eb8fba93a 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/JacksonMappingAwareSortTranslator.java @@ -36,6 +36,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; * repositories. * * @author Mark Paluch + * @since 2.6, 2.5.3, 2.4.5 */ public class JacksonMappingAwareSortTranslator { @@ -72,7 +73,7 @@ public class JacksonMappingAwareSortTranslator { * @return a {@link Sort} containing translated property names or {@literal null} the resulting {@link Sort} contains * no properties. */ - protected Sort translateMethodParameter(Sort input, MethodParameter parameter, NativeWebRequest webRequest) { + protected Sort translateSort(Sort input, MethodParameter parameter, NativeWebRequest webRequest) { Assert.notNull(input, "Sort must not be null!"); Assert.notNull(parameter, "MethodParameter must not be null!"); @@ -89,6 +90,8 @@ public class JacksonMappingAwareSortTranslator { * Translates {@link Sort} orders from Jackson-mapped field names to {@link PersistentProperty} names. * * @author Mark Paluch + * @author Oliver Gierke + * @since 2.6, 2.5.3, 2.4.5 */ static class SortTranslator { @@ -120,17 +123,14 @@ public class JacksonMappingAwareSortTranslator { for (Order order : input) { if (mappedProperties.hasPersistentPropertyForField(order.getProperty())) { + PersistentProperty persistentProperty = mappedProperties.getPersistentProperty(order.getProperty()); Order mappedOrder = new Order(order.getDirection(), persistentProperty.getName(), order.getNullHandling()); filteredOrders.add(order.isIgnoreCase() ? mappedOrder.ignoreCase() : mappedOrder); } } - if (filteredOrders.isEmpty()) { - return null; - } - - return new Sort(filteredOrders); + return filteredOrders.isEmpty() ? null : new Sort(filteredOrders); } } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java index 19ca4ae2b..886796866 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappedProperties.java @@ -36,6 +36,8 @@ import com.fasterxml.jackson.databind.introspect.ClassIntrospector; */ class MappedProperties { + private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector(); + private final Map, String> propertyToFieldName; private final Map> fieldNameToProperty; @@ -68,9 +70,7 @@ class MappedProperties { */ public static MappedProperties fromJacksonProperties(PersistentEntity entity, ObjectMapper mapper) { - ClassIntrospector introspector = new BasicClassIntrospector(); - - BeanDescription description = introspector.forDeserialization(mapper.getDeserializationConfig(), + BeanDescription description = INTROSPECTOR.forDeserialization(mapper.getDeserializationConfig(), mapper.constructType(entity.getType()), mapper.getDeserializationConfig()); return new MappedProperties(entity, description); @@ -93,7 +93,7 @@ class MappedProperties { */ public boolean hasPersistentPropertyForField(String fieldName) { - Assert.hasText(fieldName, "Field name must not be empty or null!"); + Assert.hasText(fieldName, "Field name must not be null or empty!"); return fieldNameToProperty.containsKey(fieldName); } @@ -104,7 +104,7 @@ class MappedProperties { */ public PersistentProperty getPersistentProperty(String fieldName) { - Assert.hasText(fieldName, "Field name must not be empty or null!"); + Assert.hasText(fieldName, "Field name must not be null or empty!"); return fieldNameToProperty.get(fieldName); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareDefaultedPageableArgumentResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareDefaultedPageableArgumentResolver.java index 07ea1ee2f..bc690eb87 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareDefaultedPageableArgumentResolver.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareDefaultedPageableArgumentResolver.java @@ -36,7 +36,8 @@ import org.springframework.web.method.support.ModelAndViewContainer; * {@link Sort}. * * @author Mark Paluch - * @since 2.6 + * @author Oliver Gierke + * @since 2.6, 2.5.3, 2.4.5 */ public class MappingAwareDefaultedPageableArgumentResolver implements HandlerMethodArgumentResolver { @@ -83,10 +84,9 @@ public class MappingAwareDefaultedPageableArgumentResolver implements HandlerMet return new DefaultedPageable(pageable, delegate.isFallbackPageable(pageable)); } - Sort translated = translator.translateMethodParameter(pageable.getSort(), parameter, webRequest); + Sort translated = translator.translateSort(pageable.getSort(), parameter, webRequest); pageable = new PageRequest(pageable.getPageNumber(), pageable.getPageSize(), translated); return new DefaultedPageable(pageable, delegate.isFallbackPageable(pageable)); } - } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolver.java index 2669ab1e7..f6a722440 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolver.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwarePageableArgumentResolver.java @@ -35,7 +35,8 @@ import org.springframework.web.method.support.ModelAndViewContainer; * {@link Sort}. * * @author Mark Paluch - * @since 2.6 + * @author Oliver Gierke + * @since 2.6, 2.5.3, 2.4.5 */ public class MappingAwarePageableArgumentResolver implements HandlerMethodArgumentResolver { @@ -82,8 +83,7 @@ public class MappingAwarePageableArgumentResolver implements HandlerMethodArgume return null; } - Sort translated = translator.translateMethodParameter(pageable.getSort(), parameter, webRequest); + Sort translated = translator.translateSort(pageable.getSort(), parameter, webRequest); return new PageRequest(pageable.getPageNumber(), pageable.getPageSize(), translated); } - } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareSortArgumentResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareSortArgumentResolver.java index ef99f2ab9..3227c0424 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareSortArgumentResolver.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/MappingAwareSortArgumentResolver.java @@ -32,7 +32,8 @@ import org.springframework.web.method.support.ModelAndViewContainer; * resolved to their property names. Unknown properties are removed from {@link Sort}. * * @author Mark Paluch - * @since 2.6 + * @author Oliver Gierke + * @since 2.6, 2.5.3, 2.4.5 */ public class MappingAwareSortArgumentResolver implements HandlerMethodArgumentResolver { @@ -75,10 +76,6 @@ public class MappingAwareSortArgumentResolver implements HandlerMethodArgumentRe Sort sort = delegate.resolveArgument(parameter, mavContainer, webRequest, binderFactory); - if (sort == null) { - return null; - } - - return translator.translateMethodParameter(sort, parameter, webRequest); + return sort == null ? null : translator.translateSort(sort, parameter, webRequest); } } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DomainClassResolver.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DomainClassResolver.java index 227242476..094dd1c7f 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DomainClassResolver.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/support/DomainClassResolver.java @@ -31,7 +31,8 @@ import org.springframework.web.context.request.NativeWebRequest; * requests} related to mapped and exported {@link Repositories}. * * @author Mark Paluch - * @since 2.6 + * @author Oliver Gierke + * @since 2.6, 2.5.3, 2.4.5 */ public class DomainClassResolver { @@ -64,7 +65,7 @@ public class DomainClassResolver { * @param mappings must not be {@literal null}. * @param baseUri must not be {@literal null}. */ - public static DomainClassResolver create(Repositories repositories, ResourceMappings mappings, BaseUri baseUri) { + public static DomainClassResolver of(Repositories repositories, ResourceMappings mappings, BaseUri baseUri) { return new DomainClassResolver(repositories, mappings, baseUri); } @@ -89,7 +90,9 @@ public class DomainClassResolver { } for (Class domainType : repositories) { + ResourceMetadata mapping = mappings.getMetadataFor(domainType); + if (mapping.getPath().matches(repositoryKey) && mapping.isExported()) { return domainType; } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java index b0f7400a1..47107060d 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/JpaWebTests.java @@ -20,7 +20,6 @@ import static org.junit.Assert.*; import static org.springframework.data.rest.webmvc.util.TestUtils.*; import static org.springframework.http.HttpHeaders.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; -import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; import net.minidev.json.JSONArray; @@ -570,57 +569,6 @@ public class JpaWebTests extends CommonWebTests { andExpect(client.hasLinkWithRel("self")); } - /** - * @see DATAREST-883 - */ - @Test - public void exectuesSearchThatTakesAMappedSortProperty() throws Exception { - - Link booksLink = client.discoverUnique("books"); - Link searchLink = client.discoverUnique(booksLink, "search"); - Link findBySortedLink = client.discoverUnique(searchLink, "find-by-sorted"); - - // Assert sort options advertised - assertThat(findBySortedLink.isTemplated(), is(true)); - assertThat(findBySortedLink.getVariableNames(), hasItems("sort", "projection")); - - // Assert results returned as specified - client.follow(findBySortedLink.expand("sales,desc")).// - andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data (Second Edition)")).// - andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data")).// - andExpect(client.hasLinkWithRel("self")); - - client.follow(findBySortedLink.expand("sales,asc")).// - andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data")).// - andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data (Second Edition)")).// - andExpect(client.hasLinkWithRel("self")); - } - - /** - * @see DATAREST-883 - */ - @Test - public void exectuesCustomQuerySearchThatTakesAMappedSortProperty() throws Exception { - - Link booksLink = client.discoverUnique("books"); - Link searchLink = client.discoverUnique(booksLink, "search"); - Link findByLink = client.discoverUnique(searchLink, "find-spring-books-sorted"); - - // Assert sort options advertised - assertThat(findByLink.isTemplated(), is(true)); - - // Assert results returned as specified - client.follow(findByLink.expand("0", "10", "sales,desc")).// - andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data (Second Edition)")).// - andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data")).// - andExpect(client.hasLinkWithRel("self")); - - client.follow(findByLink.expand("0", "10", "unknown,asc,sales,asc")).// - andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data")).// - andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data (Second Edition)")).// - andExpect(client.hasLinkWithRel("self")); - } - /** * @see DATAREST-160 */ @@ -707,6 +655,53 @@ public class JpaWebTests extends CommonWebTests { assertThat(links.hasLink("person"), is(true)); } + /** + * @see DATAREST-883 + */ + @Test + public void exectuesSearchThatTakesAMappedSortProperty() throws Exception { + + Link findBySortedLink = client.discoverUnique("books", "search", "find-by-sorted"); + + // Assert sort options advertised + assertThat(findBySortedLink.isTemplated(), is(true)); + assertThat(findBySortedLink.getVariableNames(), hasItems("sort", "projection")); + + // Assert results returned as specified + client.follow(findBySortedLink.expand("sales,desc")).// + andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data (Second Edition)")).// + andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data")).// + andExpect(client.hasLinkWithRel("self")); + + client.follow(findBySortedLink.expand("sales,asc")).// + andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data")).// + andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data (Second Edition)")).// + andExpect(client.hasLinkWithRel("self")); + } + + /** + * @see DATAREST-883 + */ + @Test + public void exectuesCustomQuerySearchThatTakesAMappedSortProperty() throws Exception { + + Link findByLink = client.discoverUnique("books", "search", "find-spring-books-sorted"); + + // Assert sort options advertised + assertThat(findByLink.isTemplated(), is(true)); + + // Assert results returned as specified + client.follow(findByLink.expand("0", "10", "sales,desc")).// + andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data (Second Edition)")).// + andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data")).// + andExpect(client.hasLinkWithRel("self")); + + client.follow(findByLink.expand("0", "10", "unknown,asc,sales,asc")).// + andExpect(jsonPath("$._embedded.books[0].title").value("Spring Data")).// + andExpect(jsonPath("$._embedded.books[1].title").value("Spring Data (Second Edition)")).// + andExpect(client.hasLinkWithRel("self")); + } + private List preparePersonResources(Person primary, Person... persons) throws Exception { Link peopleLink = client.discoverUnique("people"); diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java index c7d0fe48f..95f9dcc40 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/jpa/TestDataPopulator.java @@ -22,6 +22,7 @@ import org.springframework.beans.factory.annotation.Autowired; /** * @author Jon Brisbin * @author Oliver Gierke + * @author Mark Paluch */ public class TestDataPopulator { diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java index 345043d4a..6ee98d18e 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/SortTranslatorUnitTests.java @@ -30,6 +30,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; * Unit tests for {@link JacksonMappingAwareSortTranslator.SortTranslator}. * * @author Mark Paluch + * @author Oliver Gierke * @soundtrack dkn - Out Of This World (original version) */ public class SortTranslatorUnitTests {