From e1dd82c9272481b3aace5c87f902b24d5a4206f2 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Tue, 13 Jul 2021 09:25:57 +0200 Subject: [PATCH] Prevent NullPointerException in AggregateReferenceDeserializerModifier. In AggregateReferenceDeserializerModifier we now look up the original property name on the Jackson metadata before trying to resolve the property target type. We also explicitly guard against null values returned from that lookup and opt out of the customization in those cases. --- .../AggregateReferenceResolvingModule.java | 33 +++++++- ...gateReferenceResolvingModuleUnitTests.java | 75 +++++++++++++++++++ 2 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModuleUnitTests.java diff --git a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModule.java b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModule.java index c02f91411..18c1f1539 100644 --- a/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModule.java +++ b/spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModule.java @@ -33,6 +33,7 @@ import com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder; import com.fasterxml.jackson.databind.deser.BeanDeserializerModifier; import com.fasterxml.jackson.databind.deser.SettableBeanProperty; import com.fasterxml.jackson.databind.deser.std.CollectionDeserializer; +import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.type.CollectionLikeType; @@ -89,23 +90,28 @@ public class AggregateReferenceResolvingModule extends SimpleModule { * @see com.fasterxml.jackson.databind.deser.BeanDeserializerModifier#updateBuilder(com.fasterxml.jackson.databind.DeserializationConfig, com.fasterxml.jackson.databind.BeanDescription, com.fasterxml.jackson.databind.deser.BeanDeserializerBuilder) */ @Override - public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, BeanDescription beanDesc, + public BeanDeserializerBuilder updateBuilder(DeserializationConfig config, BeanDescription description, BeanDeserializerBuilder builder) { // Type is aggregate itself, already handled by AssociationUriResolvingDeserializerModifier - if (mappings.hasMappingFor(beanDesc.getBeanClass())) { + if (mappings.hasMappingFor(description.getBeanClass())) { return builder; } - TypeInformation type = ClassTypeInformation.from(beanDesc.getBeanClass()); + TypeInformation type = ClassTypeInformation.from(description.getBeanClass()); ValueInstantiatorCustomizer customizer = new ValueInstantiatorCustomizer(builder.getValueInstantiator(), config); Iterator properties = builder.getProperties(); while (properties.hasNext()) { SettableBeanProperty property = properties.next(); + String originalPropertyName = coerceOriginalPropertyName(property, description); + TypeInformation propertyType = type.getProperty(originalPropertyName); + + if (propertyType == null) { + continue; + } - TypeInformation propertyType = type.getProperty(property.getName()); TypeInformation actualType = propertyType.getActualType(); if (!mappings.exportsMappingFor(actualType.getType())) { @@ -134,5 +140,24 @@ public class AggregateReferenceResolvingModule extends SimpleModule { return new CollectionDeserializer(collectionType, elementDeserializer, null, instantiator); } + + /** + * Tries to find the internal property name for a {@link SettableBeanProperty} that unfortunately does not allow + * accessing the original name anymore. + * + * @param property must not be {@literal null}. + * @param description must not be {@literal null}. + * @return + */ + private static String coerceOriginalPropertyName(SettableBeanProperty property, BeanDescription description) { + + for (BeanPropertyDefinition properties : description.findProperties()) { + if (properties.hasName(property.getFullName())) { + return properties.getInternalName(); + } + } + + return property.getName(); + } } } diff --git a/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModuleUnitTests.java b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModuleUnitTests.java new file mode 100644 index 000000000..51c66d390 --- /dev/null +++ b/spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/json/AggregateReferenceResolvingModuleUnitTests.java @@ -0,0 +1,75 @@ +/* + * Copyright 2021 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.webmvc.json; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext; +import org.springframework.data.mapping.context.PersistentEntities; +import org.springframework.data.rest.core.UriToEntityConverter; +import org.springframework.data.rest.core.annotation.RestResource; +import org.springframework.data.rest.core.mapping.PersistentEntitiesResourceMappings; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * Unit tests for {@link AggregateReferenceResolvingModule}. + * + * @author Oliver Drotbohm + */ +@ExtendWith(MockitoExtension.class) +public class AggregateReferenceResolvingModuleUnitTests { + + @Mock UriToEntityConverter uriToEntityConverter; + + @Test // GH-2033 + void processesArtificialPropertiesCorrectly() { + + KeyValueMappingContext context = new KeyValueMappingContext<>(); + context.getPersistentEntity(Other.class); + + PersistentEntities entities = PersistentEntities.of(context); + PersistentEntitiesResourceMappings mappings = new PersistentEntitiesResourceMappings(entities); + + ObjectMapper mapper = new ObjectMapper() + .addMixIn(SomeType.class, SomeTypeMixin.class) + .registerModule(new AggregateReferenceResolvingModule(uriToEntityConverter, mappings)); + + assertThatNoException().isThrownBy(() -> { + mapper.readValue("{}", SomeType.class); + }); + } + + public static class SomeType { + + public void setSomeProperty(Other other) {} + } + + @RestResource + public static class Other {} + + public abstract static class SomeTypeMixin { + + // Rename property to expose a property that's not named like the actual member + @JsonProperty("foo") + public abstract void setSomeProperty(Other other); + } +}