From a516d8e61f5776d92adbe4b5d0ae293dc94031b7 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 23 Oct 2016 17:24:50 +0200 Subject: [PATCH] DATAREST-575 - Fixed property lookup in MappedProperties. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, MappedProperties didn't handle Jackson properties correctly, that do not expose a PersistentProperty, e.g. transient ones. That led to potential nullPointerExceptions in clients as the guarding hasPersistentPropertyForField(…) still answered true, as the backing cache contained an entry with a null value. We now skip those properties completely. --- .../rest/webmvc/json/MappedProperties.java | 11 ++- .../json/MappedPropertiesUnitTests.java | 90 +++++++++++++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java 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 886796866..4b31ee499 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 @@ -49,6 +49,9 @@ class MappedProperties { */ private MappedProperties(PersistentEntity entity, BeanDescription description) { + Assert.notNull(entity, "Entity must not be null!"); + Assert.notNull(description, "BeanDescription must not be null!"); + this.propertyToFieldName = new HashMap, String>(); this.fieldNameToProperty = new HashMap>(); @@ -56,8 +59,10 @@ class MappedProperties { PersistentProperty persistentProperty = entity.getPersistentProperty(property.getInternalName()); - propertyToFieldName.put(persistentProperty, property.getName()); - fieldNameToProperty.put(property.getName(), persistentProperty); + if (persistentProperty != null) { + propertyToFieldName.put(persistentProperty, property.getName()); + fieldNameToProperty.put(property.getName(), persistentProperty); + } } } @@ -100,7 +105,7 @@ class MappedProperties { /** * @param fieldName must not be empty or {@literal null}. - * @return + * @return the {@link PersistentProperty} backing the field with the field name. */ public PersistentProperty getPersistentProperty(String fieldName) { diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java new file mode 100644 index 000000000..c11c052e0 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/MappedPropertiesUnitTests.java @@ -0,0 +1,90 @@ +/* + * Copyright 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. + * You may obtain a copy of the License at + * + * http://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.webmvc.json; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + +import org.junit.Test; +import org.springframework.data.annotation.Transient; +import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentEntity; +import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * Unit tests for {@link MappedProperties}. + * + * @author Oliver Gierke + */ +public class MappedPropertiesUnitTests { + + ObjectMapper mapper = new ObjectMapper(); + KeyValueMappingContext context = new KeyValueMappingContext(); + KeyValuePersistentEntity entity = context.getPersistentEntity(Sample.class); + MappedProperties properties = MappedProperties.fromJacksonProperties(entity, mapper); + + /** + * @see DATAREST-575 + */ + @Test + public void doesNotExposeMappedPropertyForNonSpringDataPersistentProperty() { + + assertThat(properties.hasPersistentPropertyForField("notExposedBySpringData"), is(false)); + assertThat(properties.getPersistentProperty("notExposedBySpringData"), is(nullValue())); + } + + /** + * @see DATAREST-575 + */ + @Test + public void doesNotExposeMappedPropertyForNonJacksonProperty() { + + assertThat(properties.hasPersistentPropertyForField("notExposedByJackson"), is(false)); + assertThat(properties.getPersistentProperty("notExposedByJackson"), is(nullValue())); + } + + /** + * @see DATAREST-575 + */ + @Test + public void exposesProperty() { + + assertThat(properties.hasPersistentPropertyForField("exposedProperty"), is(true)); + assertThat(properties.getPersistentProperty("exposedProperty"), is(notNullValue())); + } + + /** + * @see DATAREST-575 + */ + @Test + public void exposesRenamedPropertyByExternalName() { + + assertThat(properties.hasPersistentPropertyForField("email"), is(true)); + assertThat(properties.getPersistentProperty("email"), is(notNullValue())); + assertThat(properties.getMappedName(entity.getPersistentProperty("emailAddress")), is("email")); + } + + static class Sample { + + public @Transient String notExposedBySpringData; + public @JsonIgnore String notExposedByJackson; + public String exposedProperty; + public @JsonProperty("email") String emailAddress; + } +}