From a4f88fe1ac7e3b58edfba08c13e6fc645e86f77d Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 6 Dec 2016 11:57:11 +0100 Subject: [PATCH] DATAREST-910 - Polishing. Minor code reorganizations. Eagerly create SortTranslator to avoid repeated instantiation. Original pull request: #232. --- .../JacksonMappingAwareSortTranslator.java | 54 +++++++++++++------ .../rest/webmvc/json/WrappedProperties.java | 19 ++++--- .../webmvc/support/DomainClassResolver.java | 2 + .../data/rest/webmvc/util/UriUtils.java | 2 +- .../webmvc/json/SortTranslatorUnitTests.java | 8 +-- .../json/WrappedPropertiesUnitTests.java | 17 +++--- .../rest/webmvc/util/UriUtilsUnitTests.java | 1 - 7 files changed, 63 insertions(+), 40 deletions(-) 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 a55634da5..01ec64094 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 @@ -44,15 +44,35 @@ import com.fasterxml.jackson.databind.ObjectMapper; * repositories. {@link Sort} translation is skipped if a domain class cannot be resolved. * * @author Mark Paluch + * @author Oliver Gierke * @since 2.6 */ @RequiredArgsConstructor public class JacksonMappingAwareSortTranslator { - private final @NonNull ObjectMapper objectMapper; - private final @NonNull Repositories repositories; - private final @NonNull DomainClassResolver domainClassResolver; - private final @NonNull PersistentEntities persistentEntities; + private final Repositories repositories; + private final DomainClassResolver domainClassResolver; + private final SortTranslator sortTranslator; + + /** + * Creates a new {@link JacksonMappingAwareSortTranslator} for the given {@link ObjectMapper}, {@link Repositories}, + * {@link DomainClassResolver} and {@link PersistentEntities}. + * + * @param objectMapper must not be {@literal null}. + * @param repositories must not be {@literal null}. + * @param domainClassResolver must not be {@literal null}. + * @param persistentEntities must not be {@literal null}. + */ + public JacksonMappingAwareSortTranslator(ObjectMapper objectMapper, Repositories repositories, + DomainClassResolver domainClassResolver, PersistentEntities persistentEntities) { + + Assert.notNull(repositories, "Repositories must not be null!"); + Assert.notNull(domainClassResolver, "DomainClassResolver must not be null!"); + + this.repositories = repositories; + this.domainClassResolver = domainClassResolver; + this.sortTranslator = new SortTranslator(persistentEntities, objectMapper); + } /** * Translates Jackson field names within a {@link Sort} to {@link PersistentProperty} property names. @@ -71,13 +91,13 @@ public class JacksonMappingAwareSortTranslator { Class domainClass = domainClassResolver.resolve(parameter.getMethod(), webRequest); - if (domainClass != null) { - - PersistentEntity persistentEntity = repositories.getPersistentEntity(domainClass); - return new SortTranslator(persistentEntities, objectMapper).translateSort(input, persistentEntity); + if (domainClass == null) { + return input; } - return input; + PersistentEntity persistentEntity = repositories.getPersistentEntity(domainClass); + + return sortTranslator.translateSort(input, persistentEntity); } /** @@ -125,9 +145,7 @@ public class JacksonMappingAwareSortTranslator { String mappedPropertyPath = getMappedPropertyPath(rootEntity, iteratorSource); if (mappedPropertyPath != null) { - - Order mappedOrder = new Order(order.getDirection(), mappedPropertyPath, order.getNullHandling()); - filteredOrders.add(order.isIgnoreCase() ? mappedOrder.ignoreCase() : mappedOrder); + filteredOrders.add(order.withProperty(mappedPropertyPath)); } } @@ -203,10 +221,13 @@ public class JacksonMappingAwareSortTranslator { this.currentType = persistentEntity; if (persistentEntity != null) { + this.currentProperties = MappedProperties.fromJacksonProperties(currentType, objectMapper); this.currentWrappedProperties = WrappedProperties.fromJacksonProperties(persistentEntities, currentType, objectMapper); + } else { + this.currentProperties = null; this.currentWrappedProperties = null; } @@ -222,7 +243,7 @@ public class JacksonMappingAwareSortTranslator { * @return */ public static TypedSegment create(PersistentEntities persistentEntities, ObjectMapper objectMapper, - PersistentEntity rootEntity) { + PersistentEntity rootEntity) { Assert.notNull(persistentEntities, "PersistentEntities must not be null!"); Assert.notNull(objectMapper, "ObjectMapper must not be null!"); @@ -237,7 +258,7 @@ public class JacksonMappingAwareSortTranslator { * @param persistentProperty must not be {@literal null}. * @return */ - public TypedSegment next(PersistentProperty persistentProperty) { + public TypedSegment next(PersistentProperty persistentProperty) { Assert.notNull(persistentProperty, "PersistentProperty must not be null!"); @@ -245,12 +266,13 @@ public class JacksonMappingAwareSortTranslator { return new TypedSegment(this, persistentEntity); } - protected boolean hasPersistentPropertyForField(String fieldName) { + private boolean hasPersistentPropertyForField(String fieldName) { + return currentType != null && (currentProperties.hasPersistentPropertyForField(fieldName) || currentWrappedProperties.hasPersistentPropertiesForField(fieldName)); } - protected List> getPersistentProperties(String fieldName) { + private List> getPersistentProperties(String fieldName) { if (currentWrappedProperties.hasPersistentPropertiesForField(fieldName)) { return currentWrappedProperties.getPersistentProperties(fieldName); diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java index a95da27ba..664213863 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/WrappedProperties.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.rest.webmvc.json; import lombok.NonNull; @@ -88,6 +87,7 @@ class WrappedProperties { public boolean hasPersistentPropertiesForField(String fieldName) { Assert.hasText(fieldName, "Field name must not be null or empty!"); + return fieldNameToProperties.containsKey(fieldName); } @@ -112,8 +112,8 @@ class WrappedProperties { @RequiredArgsConstructor static class JacksonUnwrappedPropertiesResolver { - final @NonNull PersistentEntities persistentEntities; - final @NonNull ObjectMapper mapper; + private final @NonNull PersistentEntities persistentEntities; + private final @NonNull ObjectMapper mapper; /** * Resolve {@code @JsonUnwrapped} field names to a list of involved {@link PersistentProperty properties}. @@ -142,7 +142,6 @@ class WrappedProperties { for (BeanPropertyDefinition property : getMappedProperties(entity)) { AnnotatedMember annotatedMember = findAnnotatedMember(property); - PersistentProperty persistentProperty = entity.getPersistentProperty(property.getInternalName()); if (isJsonUnwrapped(annotatedMember)) { @@ -199,7 +198,12 @@ class WrappedProperties { return withInternalName; } - private AnnotatedMember findAnnotatedMember(BeanPropertyDefinition property) { + private BeanDescription getBeanDescription(Class type) { + return INTROSPECTOR.forDeserialization(mapper.getDeserializationConfig(), mapper.constructType(type), + mapper.getDeserializationConfig()); + } + + private static AnnotatedMember findAnnotatedMember(BeanPropertyDefinition property) { if (property.getPrimaryMember() != null) { return property.getPrimaryMember(); @@ -220,10 +224,5 @@ class WrappedProperties { return primaryMember.hasAnnotation(JsonUnwrapped.class) && primaryMember.getAnnotation(JsonUnwrapped.class).enabled(); } - - private BeanDescription getBeanDescription(Class type) { - return INTROSPECTOR.forDeserialization(mapper.getDeserializationConfig(), mapper.constructType(type), - mapper.getDeserializationConfig()); - } } } 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 b2c098405..c8c1e3204 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 @@ -61,7 +61,9 @@ public class DomainClassResolver { String repositoryKey = UriUtils.findMappingVariable("repository", method, lookupPath); if (!StringUtils.hasText(repositoryKey)) { + List pathSegments = UriUtils.getPathSegments(method); + if (!pathSegments.isEmpty()) { repositoryKey = pathSegments.get(0); } diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/util/UriUtils.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/util/UriUtils.java index a0004565f..865be79f5 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/util/UriUtils.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/util/UriUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2016 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. 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 a961ddb53..f4a00edda 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 @@ -42,10 +42,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; */ public class SortTranslatorUnitTests { - private ObjectMapper objectMapper = new ObjectMapper(); - private KeyValueMappingContext mappingContext; - private PersistentEntities persistentEntities; - private SortTranslator sortTranslator; + ObjectMapper objectMapper = new ObjectMapper(); + KeyValueMappingContext mappingContext; + PersistentEntities persistentEntities; + SortTranslator sortTranslator; @Before public void setUp() { diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/WrappedPropertiesUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/WrappedPropertiesUnitTests.java index 8975253df..341b40768 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/WrappedPropertiesUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/WrappedPropertiesUnitTests.java @@ -13,12 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.rest.webmvc.json; import static org.hamcrest.MatcherAssert.*; import static org.hamcrest.Matchers.*; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; + import java.util.Collections; import java.util.List; @@ -34,10 +37,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonUnwrapped; import com.fasterxml.jackson.databind.ObjectMapper; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - /** * Unit tests for {@link WrappedProperties}. * @@ -45,9 +44,10 @@ import lombok.NoArgsConstructor; */ public class WrappedPropertiesUnitTests { - private static final ObjectMapper MAPPER = new ObjectMapper(); - private KeyValueMappingContext mappingContext; - private PersistentEntities persistentEntities; + static final ObjectMapper MAPPER = new ObjectMapper(); + + KeyValueMappingContext mappingContext; + PersistentEntities persistentEntities; @Before public void setUp() { @@ -55,6 +55,7 @@ public class WrappedPropertiesUnitTests { mappingContext = new KeyValueMappingContext(); mappingContext.getPersistentEntity(MultiLevelNesting.class); mappingContext.getPersistentEntity(SyntheticProperties.class); + persistentEntities = new PersistentEntities(Collections.singleton(mappingContext)); } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/util/UriUtilsUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/util/UriUtilsUnitTests.java index aba5fd072..4568e97ed 100644 --- a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/util/UriUtilsUnitTests.java +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/util/UriUtilsUnitTests.java @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.rest.webmvc.util; import static org.hamcrest.Matchers.*;