Fix element ordering issue on a mapped de/serialized entity having List property.

Closes #2565
This commit is contained in:
John Blum
2023-05-07 11:49:18 -07:00
parent f06c65ed7a
commit 72382be0f7
3 changed files with 199 additions and 108 deletions

View File

@@ -15,12 +15,14 @@
*/
package org.springframework.data.redis.hash;
import static com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping.*;
import static com.fasterxml.jackson.databind.ObjectMapper.DefaultTyping.EVERYTHING;
import java.io.IOException;
import java.text.ParseException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Iterator;
@@ -32,11 +34,13 @@ import java.util.Map.Entry;
import java.util.Set;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.redis.support.collections.CollectionUtils;
import org.springframework.data.util.DirectFieldAccessFallbackBeanWrapper;
import org.springframework.lang.NonNull;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.NumberUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import com.fasterxml.jackson.annotation.JsonInclude.Include;
@@ -166,7 +170,8 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
/**
* Creates new {@link Jackson2HashMapper} with default {@link ObjectMapper}.
*
* @param flatten
* @param flatten boolean used to determine whether to flatten properties of the {@link Object}
* when mapping to {@literal JSON}.
*/
public Jackson2HashMapper(boolean flatten) {
@@ -174,11 +179,11 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
@Override
protected TypeResolverBuilder<?> _constructDefaultTypeResolverBuilder(DefaultTyping applicability,
PolymorphicTypeValidator ptv) {
PolymorphicTypeValidator polymorphicTypeValidator) {
return new DefaultTypeResolverBuilder(applicability, ptv) {
return new DefaultTypeResolverBuilder(applicability, polymorphicTypeValidator) {
Map<Class, Boolean> serializerPresentCache = new HashMap<>();
final Map<Class<?>, Boolean> serializerPresentCache = new HashMap<>();
public boolean useForType(JavaType t) {
@@ -195,7 +200,7 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
t = t.getReferencedType();
}
/*
/*
* check for registered serializers and make uses of those.
*/
if (serializerPresentCache.computeIfAbsent(t.getRawClass(), this::hasConfiguredSerializer)) {
@@ -208,7 +213,7 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
return super.useForType(t);
}
private Boolean hasConfiguredSerializer(Class key) {
private Boolean hasConfiguredSerializer(Class<?> key) {
if (!(_deserializationContext.getFactory() instanceof BeanDeserializerFactory)) {
return false;
@@ -244,54 +249,52 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
/**
* Creates new {@link Jackson2HashMapper}.
*
* @param mapper must not be {@literal null}.
* @param flatten
* @param mapper Jackson {@link ObjectMapper} used to map {@link Object Objects} to {@literal JSON};
* must not be {@literal null}.
* @param flatten boolean used to determine whether to flatten properties of the {@link Object}
* when mapping to {@literal JSON}.
* @see com.fasterxml.jackson.databind.ObjectMapper
*/
public Jackson2HashMapper(ObjectMapper mapper, boolean flatten) {
Assert.notNull(mapper, "Mapper must not be null!");
Assert.notNull(mapper, "ObjectMapper must not be null");
this.typingMapper = mapper;
this.flatten = flatten;
this.untypedMapper = new ObjectMapper();
this.untypedMapper.findAndRegisterModules();
this.untypedMapper.configure(SerializationFeature.WRITE_NULL_MAP_VALUES, false);
this.untypedMapper.setSerializationInclusion(Include.NON_NULL);
}
/*
* (non-Javadoc)
* @see org.springframework.data.redis.hash.HashMapper#toHash(java.lang.Object)
*/
@Override
@SuppressWarnings("unchecked")
public Map<String, Object> toHash(Object source) {
JsonNode tree = typingMapper.valueToTree(source);
return flatten ? flattenMap(tree.fields()) : untypedMapper.convertValue(tree, Map.class);
}
/*
* (non-Javadoc)
* @see org.springframework.data.redis.hash.HashMapper#fromHash(java.util.Map)
*/
@Override
public Object fromHash(Map<String, Object> hash) {
return fromHash(Object.class, hash);
}
/*
* (non-Javadoc)
* @see org.springframework.data.redis.hash.HashMapper#fromHash(Class, java.util.Map)
*/
@Override
@SuppressWarnings("unchecked")
public <R> R fromHash(Class<R> type, Map<String, Object> hash) {
try {
if (flatten) {
return typingMapper.reader().forType(type).readValue(untypedMapper.writeValueAsBytes(doUnflatten(hash)));
Map<String, Object> unflattenedHash = doUnflatten(hash);
byte[] unflattenedHashedBytes = untypedMapper.writeValueAsBytes(unflattenedHash);
Object hashedObject = typingMapper.reader().forType(type)
.readValue(unflattenedHashedBytes);;
return (R) hashedObject;
}
return typingMapper.treeToValue(untypedMapper.valueToTree(hash), type);
@@ -305,31 +308,37 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
private Map<String, Object> doUnflatten(Map<String, Object> source) {
Map<String, Object> result = new LinkedHashMap<>();
Set<String> treatSeperate = new LinkedHashSet<>();
Set<String> treatSeparate = new LinkedHashSet<>();
for (Entry<String, Object> entry : source.entrySet()) {
String key = entry.getKey();
String[] args = key.split("\\.");
String[] keyParts = key.split("\\.");
if (args.length == 1 && !args[0].contains("[")) {
if (keyParts.length == 1 && isNotIndexed(keyParts[0])) {
result.put(entry.getKey(), entry.getValue());
continue;
}
if (args.length == 1 && args[0].contains("[")) {
if (keyParts.length == 1 && isIndexed(keyParts[0])) {
String prunedKey = args[0].substring(0, args[0].indexOf('['));
if (result.containsKey(prunedKey)) {
appendValueToTypedList(args[0], entry.getValue(), (List<Object>) result.get(prunedKey));
} else {
result.put(prunedKey, createTypedListWithValue(entry.getValue()));
String indexedKeyName = keyParts[0];
String nonIndexedKeyName = stripIndex(indexedKeyName);
int index = getIndex(indexedKeyName);
if (result.containsKey(nonIndexedKeyName)) {
addValueToTypedListAtIndex((List<Object>) result.get(nonIndexedKeyName), index, entry.getValue());
}
else {
result.put(nonIndexedKeyName, createTypedListWithValue(index, entry.getValue()));
}
} else {
treatSeperate.add(key.substring(0, key.indexOf('.')));
treatSeparate.add(key.substring(0, key.indexOf('.')));
}
}
for (String partial : treatSeperate) {
for (String partial : treatSeparate) {
Map<String, Object> newSource = new LinkedHashMap<>();
@@ -341,12 +350,13 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
if (partial.endsWith("]")) {
String prunedKey = partial.substring(0, partial.indexOf('['));
String nonIndexPartial = stripIndex(partial);
int index = getIndex(partial);
if (result.containsKey(prunedKey)) {
appendValueToTypedList(partial, doUnflatten(newSource), (List<Object>) result.get(prunedKey));
if (result.containsKey(nonIndexPartial)) {
addValueToTypedListAtIndex((List<Object>) result.get(nonIndexPartial), index, doUnflatten(newSource));
} else {
result.put(prunedKey, createTypedListWithValue(doUnflatten(newSource)));
result.put(nonIndexPartial, createTypedListWithValue(index, doUnflatten(newSource)));
}
} else {
result.put(partial, doUnflatten(newSource));
@@ -356,6 +366,27 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
return result;
}
private boolean isIndexed(@NonNull String value) {
return value.indexOf('[') > -1;
}
private boolean isNotIndexed(@NonNull String value) {
return !isIndexed(value);
}
private int getIndex(@NonNull String indexedValue) {
return Integer.parseInt(indexedValue.substring(indexedValue.indexOf('[') + 1, indexedValue.length() - 1));
}
private @NonNull String stripIndex(@NonNull String indexedValue) {
int indexOfLeftBracket = indexedValue.indexOf("[");
return indexOfLeftBracket > -1
? indexedValue.substring(0, indexOfLeftBracket)
: indexedValue;
}
private Map<String, Object> flattenMap(Iterator<Entry<String, JsonNode>> source) {
Map<String, Object> resultMap = new HashMap<>();
@@ -371,7 +402,6 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
}
while (inputMap.hasNext()) {
Entry<String, JsonNode> entry = inputMap.next();
flattenElement(propertyPrefix + entry.getKey(), entry.getValue(), resultMap);
}
@@ -380,7 +410,6 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
private void flattenElement(String propertyPrefix, Object source, Map<String, Object> resultMap) {
if (!(source instanceof JsonNode)) {
resultMap.put(propertyPrefix, source);
return;
}
@@ -394,6 +423,7 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
while (nodes.hasNext()) {
JsonNode cur = nodes.next();
if (cur.isArray()) {
this.flattenCollection(propertyPrefix, cur.elements(), resultMap);
} else {
@@ -427,12 +457,13 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
try {
resultMap.put(propertyPrefix, next.binaryValue());
} catch (IOException e) {
throw new IllegalStateException(String.format("Cannot read binary value of '%s'", propertyPrefix), e);
} catch (IOException cause) {
String message = String.format("Cannot read binary value of '%s'", propertyPrefix);
throw new IllegalStateException(message, cause);
}
break;
}
}
}
}
@@ -447,53 +478,49 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
private boolean mightBeJavaType(JsonNode node) {
String textValue = node.asText();
if (!SOURCE_VERSION_PRESENT) {
if (ObjectUtils.nullSafeEquals(textValue, "java.util.Date")) {
return true;
}
if (ObjectUtils.nullSafeEquals(textValue, "java.math.BigInteger")) {
return true;
}
if (ObjectUtils.nullSafeEquals(textValue, "java.math.BigDecimal")) {
return true;
}
return false;
return Arrays.asList("java.util.Date", "java.math.BigInteger", "java.math.BigDecimal").contains(textValue);
}
return javax.lang.model.SourceVersion.isName(textValue);
return javax.lang.model.SourceVersion.isName(textValue);
}
private void flattenCollection(String propertyPrefix, Iterator<JsonNode> list, Map<String, Object> resultMap) {
int counter = 0;
while (list.hasNext()) {
for (int counter = 0; list.hasNext(); counter++) {
JsonNode element = list.next();
flattenElement(propertyPrefix + "[" + counter + "]", element, resultMap);
counter++;
}
}
@SuppressWarnings("unchecked")
private void appendValueToTypedList(String key, Object value, List<Object> destination) {
private void addValueToTypedListAtIndex(List<Object> listWithTypeHint, int index, Object value) {
int index = Integer.parseInt(key.substring(key.indexOf('[') + 1, key.length() - 1));
List<Object> resultList = ((List<Object>) destination.get(1));
if (resultList.size() < index) {
resultList.add(value);
} else {
resultList.add(index, value);
List<Object> valueList = (List<Object>) listWithTypeHint.get(1);
if (index >= valueList.size()) {
int initialCapacity = index + 1;
List<Object> newValueList = new ArrayList<>(initialCapacity);
Collections.copy(CollectionUtils.initializeList(newValueList, initialCapacity), valueList);
listWithTypeHint.set(1, newValueList);
valueList = newValueList;
}
valueList.set(index, value);
}
private List<Object> createTypedListWithValue(Object value) {
private List<Object> createTypedListWithValue(int index, Object value) {
int initialCapacity = index + 1;
List<Object> valueList = CollectionUtils.initializeList(new ArrayList<>(initialCapacity), initialCapacity);
valueList.set(index, value);
List<Object> listWithTypeHint = new ArrayList<>();
listWithTypeHint.add(ArrayList.class.getName()); // why jackson? why?
List<Object> values = new ArrayList<>();
values.add(value);
listWithTypeHint.add(values);
listWithTypeHint.add(ArrayList.class.getName());
listWithTypeHint.add(valueList);
return listWithTypeHint;
}
@@ -525,16 +552,16 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
@Override
public Date deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
Object val = delegate.deserialize(p, ctxt);
Object value = delegate.deserialize(p, ctxt);
if (val instanceof Date) {
return (Date) val;
if (value instanceof Date) {
return (Date) value;
}
try {
return ctxt.getConfig().getDateFormat().parse(val.toString());
} catch (ParseException e) {
return new Date(NumberUtils.parseNumber(val.toString(), Long.class));
return ctxt.getConfig().getDateFormat().parse(value.toString());
} catch (ParseException cause) {
return new Date(NumberUtils.parseNumber(value.toString(), Long.class));
}
}
}
@@ -553,17 +580,17 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
}
@Override
public Calendar deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
public Calendar deserialize(JsonParser parser, DeserializationContext context) throws IOException {
Date date = dateDeserializer.deserialize(p, ctxt);
Date date = dateDeserializer.deserialize(parser, context);
if (date == null) {
return null;
if (date != null) {
Calendar calendar = Calendar.getInstance();
calendar.setTime(date);
return calendar;
}
Calendar calendar = Calendar.getInstance();
calendar.setTime(date);
return calendar;
return null;
}
}
@@ -581,17 +608,20 @@ public class Jackson2HashMapper implements HashMapper<Object, String, Object>, H
}
@Override
public void serializeWithType(T value, JsonGenerator gen, SerializerProvider serializers, TypeSerializer typeSer)
throws IOException {
serialize(value, gen, serializers);
public void serializeWithType(T value, JsonGenerator jsonGenerator, SerializerProvider serializers,
TypeSerializer typeSerializer) throws IOException {
serialize(value, jsonGenerator, serializers);
}
@Override
public void serialize(T value, JsonGenerator gen, SerializerProvider serializers) throws IOException {
if (value == null) {
serializers.defaultSerializeNull(gen);
public void serialize(@Nullable T value, JsonGenerator jsonGenerator, SerializerProvider serializers)
throws IOException {
if (value != null) {
delegate.serialize(value, jsonGenerator, serializers);
} else {
delegate.serialize(value, gen, serializers);
serializers.defaultSerializeNull(jsonGenerator);
}
}
}

View File

@@ -35,18 +35,8 @@ import org.springframework.lang.Nullable;
*/
public abstract class CollectionUtils {
@SuppressWarnings("unchecked")
static <E> Collection<E> reverse(Collection<? extends E> c) {
Object[] reverse = new Object[c.size()];
int index = c.size();
for (E e : c) {
reverse[--index] = e;
}
return (List<E>) Arrays.asList(reverse);
}
static Collection<String> extractKeys(Collection<? extends RedisStore> stores) {
Collection<String> keys = new ArrayList<>(stores.size());
for (RedisStore store : stores) {
@@ -56,10 +46,27 @@ public abstract class CollectionUtils {
return keys;
}
public static <T> List<T> initializeList(@NonNull List<T> list, int size) {
for (int count = 0; count < size; count++) {
list.add(null);
}
return list;
}
@NonNull
public static <T> List<T> nullSafeList(@Nullable List<T> list) {
return list != null ? list : Collections.emptyList();
}
static <K> void rename(final K key, final K newKey, RedisOperations<K, ?> operations) {
operations.execute(new SessionCallback<Object>() {
@SuppressWarnings("unchecked")
public Object execute(RedisOperations operations) throws DataAccessException {
do {
operations.watch(key);
@@ -70,13 +77,22 @@ public abstract class CollectionUtils {
operations.multi();
}
} while (operations.exec() == null);
return null;
}
});
}
@NonNull
public static <T> List<T> nullSafeList(@Nullable List<T> list) {
return list != null ? list : Collections.emptyList();
@SuppressWarnings("unchecked")
static <E> Collection<E> reverse(Collection<? extends E> c) {
Object[] reverse = new Object[c.size()];
int index = c.size();
for (E e : c) {
reverse[--index] = e;
}
return (List<E>) Arrays.asList(reverse);
}
}

View File

@@ -15,10 +15,12 @@
*/
package org.springframework.data.redis.mapping;
import static org.assertj.core.api.Assertions.*;
import static org.assertj.core.api.Assertions.assertThat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import org.junit.jupiter.api.BeforeEach;
@@ -34,6 +36,10 @@ import org.springframework.data.redis.test.extension.RedisStanalone;
import org.springframework.data.redis.test.extension.parametrized.MethodSource;
import org.springframework.data.redis.test.extension.parametrized.ParameterizedRedisTest;
import lombok.Data;
import lombok.NoArgsConstructor;
import lombok.ToString;
/**
* Integration tests for {@link Jackson2HashMapper}.
*
@@ -85,4 +91,43 @@ public class Jackson2HashMapperIntegrationTests {
Person result = (Person) mapper.fromHash(template.<String, Object> opsForHash().entries("JON-SNOW"));
assertThat(result).isEqualTo(jon);
}
@ParameterizedRedisTest // GH-2565
public void shouldPreserveListPropertyOrderOnHashedSource() {
User jonDoe = User.as("Jon Doe")
.withPhoneNumber(9, 7, 1, 5, 5, 5, 4, 1, 8, 2);
template.opsForHash().putAll("JON-DOE", mapper.toHash(jonDoe));
User deserializedJonDoe =
(User) mapper.fromHash(template.<String, Object>opsForHash().entries("JON-DOE"));
assertThat(deserializedJonDoe).isNotNull();
assertThat(deserializedJonDoe).isNotSameAs(jonDoe);
assertThat(deserializedJonDoe.getName()).isEqualTo("Jon Doe");
assertThat(deserializedJonDoe.getPhoneNumber()).containsExactly(9, 7, 1, 5, 5, 5, 4, 1, 8, 2);
}
@Data
@ToString(of = "name")
@NoArgsConstructor
static class User {
static User as(String name) {
return new User(name);
}
private String name;
private List<Integer> phoneNumber;
User(String name) {
this.name = name;
}
User withPhoneNumber(Integer... numbers) {
this.phoneNumber = new ArrayList<>(Arrays.asList(numbers));
return this;
}
}
}