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.
This commit is contained in:
Oliver Gierke
2016-09-12 17:26:09 +02:00
parent a999bd3ca8
commit 216e267fce
10 changed files with 110 additions and 186 deletions

View File

@@ -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<Link> preparePersonResources(Person primary, Person... persons) throws Exception {
Link peopleLink = client.discoverUnique("people");

View File

@@ -22,6 +22,7 @@ import org.springframework.beans.factory.annotation.Autowired;
/**
* @author Jon Brisbin
* @author Oliver Gierke
* @author Mark Paluch
*/
public class TestDataPopulator {

View File

@@ -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;
@@ -95,9 +95,9 @@ import org.springframework.data.rest.webmvc.json.MappingAwareDefaultedPageableAr
import org.springframework.data.rest.webmvc.json.MappingAwarePageableArgumentResolver;
import org.springframework.data.rest.webmvc.json.MappingAwareSortArgumentResolver;
import org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module;
import org.springframework.data.rest.webmvc.json.PersistentEntityToJsonSchemaConverter;
import org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module.LookupObjectSerializer;
import org.springframework.data.rest.webmvc.json.PersistentEntityJackson2Module.NestedEntitySerializer;
import org.springframework.data.rest.webmvc.json.PersistentEntityToJsonSchemaConverter;
import org.springframework.data.rest.webmvc.json.PersistentEntityToJsonSchemaConverter.ValueTypeSchemaPropertyCustomizerFactory;
import org.springframework.data.rest.webmvc.mapping.Associations;
import org.springframework.data.rest.webmvc.mapping.LinkCollector;
@@ -166,7 +166,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 {
@@ -741,12 +741,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<BackendIdConverter, Class<?>> backendIdConverterRegistry() {
@@ -793,13 +787,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(),

View File

@@ -15,6 +15,9 @@
*/
package org.springframework.data.rest.webmvc.json;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import java.util.ArrayList;
import java.util.List;
@@ -30,16 +33,13 @@ import org.springframework.web.context.request.NativeWebRequest;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
/**
* Translator for {@link Sort} arguments that is aware of Jackson-Mapping on domain classes. Jackson field names are
* translated to {@link PersistentProperty} names. Domain class are looked up by resolving request URLs to mapped
* repositories.
*
* @author Mark Paluch
* @since 2.6
* @since 2.6, 2.5.3
*/
@RequiredArgsConstructor
public class JacksonMappingAwareSortTranslator {
@@ -57,7 +57,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!");
@@ -74,7 +74,8 @@ public class JacksonMappingAwareSortTranslator {
* Translates {@link Sort} orders from Jackson-mapped field names to {@link PersistentProperty} names.
*
* @author Mark Paluch
* @since 2.6
* @author Oliver Gierke
* @since 2.6, 2.5.3
*/
@RequiredArgsConstructor
static class SortTranslator {
@@ -95,17 +96,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);
}
}
}

View File

@@ -36,6 +36,8 @@ import com.fasterxml.jackson.databind.introspect.ClassIntrospector;
*/
class MappedProperties {
private static final ClassIntrospector INTROSPECTOR = new BasicClassIntrospector();
private final Map<PersistentProperty<?>, String> propertyToFieldName;
private final Map<String, PersistentProperty<?>> 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);
}

View File

@@ -15,13 +15,15 @@
*/
package org.springframework.data.rest.webmvc.json;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.core.MethodParameter;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.rest.webmvc.support.DefaultedPageable;
import org.springframework.data.web.PageableHandlerMethodArgumentResolver;
import org.springframework.util.Assert;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
@@ -36,29 +38,14 @@ import org.springframework.web.method.support.ModelAndViewContainer;
* {@link Sort}.
*
* @author Mark Paluch
* @since 2.6
* @author Oliver Gierke
* @since 2.6, 2.5.3
*/
@RequiredArgsConstructor
public class MappingAwareDefaultedPageableArgumentResolver implements HandlerMethodArgumentResolver {
private final PageableHandlerMethodArgumentResolver delegate;
private final JacksonMappingAwareSortTranslator translator;
/**
* Creates a new {@link MappingAwareDefaultedPageableArgumentResolver} for the given
* {@link JacksonMappingAwareSortTranslator} and {@link PageableHandlerMethodArgumentResolver}.
*
* @param translator must not be {@literal null}.
* @param delegate must not be {@literal null}.
*/
public MappingAwareDefaultedPageableArgumentResolver(JacksonMappingAwareSortTranslator translator,
PageableHandlerMethodArgumentResolver delegate) {
Assert.notNull(translator, "JacksonMappingSortTranslator must not be null!");
Assert.notNull(delegate, "PageableHandlerMethodArgumentResolver must not be null!");
this.translator = translator;
this.delegate = delegate;
}
private final @NonNull JacksonMappingAwareSortTranslator translator;
private final @NonNull PageableHandlerMethodArgumentResolver delegate;
/*
* (non-Javadoc)
@@ -83,10 +70,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));
}
}

View File

@@ -15,12 +15,14 @@
*/
package org.springframework.data.rest.webmvc.json;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.core.MethodParameter;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.web.PageableHandlerMethodArgumentResolver;
import org.springframework.util.Assert;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
@@ -35,29 +37,14 @@ import org.springframework.web.method.support.ModelAndViewContainer;
* {@link Sort}.
*
* @author Mark Paluch
* @since 2.6
* @author Oliver Gierke
* @since 2.6, 2.5.3
*/
@RequiredArgsConstructor
public class MappingAwarePageableArgumentResolver implements HandlerMethodArgumentResolver {
private final JacksonMappingAwareSortTranslator translator;
private final PageableHandlerMethodArgumentResolver delegate;
/**
* Creates a new {@link MappingAwarePageableArgumentResolver} for the given {@link JacksonMappingAwareSortTranslator}
* and {@link PageableHandlerMethodArgumentResolver}.
*
* @param translator must not be {@literal null}.
* @param delegate must not be {@literal null}.
*/
public MappingAwarePageableArgumentResolver(JacksonMappingAwareSortTranslator translator,
PageableHandlerMethodArgumentResolver delegate) {
Assert.notNull(translator, "JacksonMappingSortTranslator must not be null!");
Assert.notNull(delegate, "PageableHandlerMethodArgumentResolver must not be null!");
this.translator = translator;
this.delegate = delegate;
}
private final @NonNull JacksonMappingAwareSortTranslator translator;
private final @NonNull PageableHandlerMethodArgumentResolver delegate;
/*
* (non-Javadoc)
@@ -82,7 +69,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);
}

View File

@@ -15,10 +15,12 @@
*/
package org.springframework.data.rest.webmvc.json;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import org.springframework.core.MethodParameter;
import org.springframework.data.domain.Sort;
import org.springframework.data.web.SortHandlerMethodArgumentResolver;
import org.springframework.util.Assert;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
@@ -32,29 +34,14 @@ 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
*/
@RequiredArgsConstructor
public class MappingAwareSortArgumentResolver implements HandlerMethodArgumentResolver {
private final JacksonMappingAwareSortTranslator translator;
private final SortHandlerMethodArgumentResolver delegate;
/**
* Creates a new {@link MappingAwareSortArgumentResolver} for the given {@link JacksonMappingAwareSortTranslator} and
* {@link SortHandlerMethodArgumentResolver}.
*
* @param translator must not be {@literal null}.
* @param delegate must not be {@literal null}.
*/
public MappingAwareSortArgumentResolver(JacksonMappingAwareSortTranslator translator,
SortHandlerMethodArgumentResolver delegate) {
Assert.notNull(translator, "JacksonMappingSortTranslator must not be null!");
Assert.notNull(delegate, "PageableHandlerMethodArgumentResolver must not be null!");
this.translator = translator;
this.delegate = delegate;
}
private final @NonNull JacksonMappingAwareSortTranslator translator;
private final @NonNull SortHandlerMethodArgumentResolver delegate;
/*
* (non-Javadoc)
@@ -75,10 +62,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);
}
}

View File

@@ -15,6 +15,9 @@
*/
package org.springframework.data.rest.webmvc.support;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import java.lang.reflect.Method;
import org.springframework.data.repository.support.Repositories;
@@ -31,42 +34,15 @@ 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
*/
@RequiredArgsConstructor(staticName = "of")
public class DomainClassResolver {
private final Repositories repositories;
private final ResourceMappings mappings;
private final BaseUri baseUri;
/**
* Creates a new {@link DomainClassResolver} for the given {@link Repositories} and {@link ResourceMappings}.
*
* @param repositories must not be {@literal null}.
* @param mappings must not be {@literal null}.
* @param baseUri must not be {@literal null}.
*/
private DomainClassResolver(Repositories repositories, ResourceMappings mappings, BaseUri baseUri) {
Assert.notNull(repositories, "Repositories must not be null!");
Assert.notNull(mappings, "ResourceMappings must not be null!");
Assert.notNull(baseUri, "BaseUri must not be null!");
this.repositories = repositories;
this.mappings = mappings;
this.baseUri = baseUri;
}
/**
* Creates a new {@link DomainClassResolver} for the given {@link Repositories} and {@link ResourceMappings}.
*
* @param repositories must not be {@literal null}.
* @param mappings must not be {@literal null}.
* @param baseUri must not be {@literal null}.
*/
public static DomainClassResolver create(Repositories repositories, ResourceMappings mappings, BaseUri baseUri) {
return new DomainClassResolver(repositories, mappings, baseUri);
}
private final @NonNull Repositories repositories;
private final @NonNull ResourceMappings mappings;
private final @NonNull BaseUri baseUri;
/**
* Resolves a domain class that is associated with the {@link NativeWebRequest}
@@ -89,7 +65,9 @@ public class DomainClassResolver {
}
for (Class<?> domainType : repositories) {
ResourceMetadata mapping = mappings.getMetadataFor(domainType);
if (mapping.getPath().matches(repositoryKey) && mapping.isExported()) {
return domainType;
}

View File

@@ -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 {