diff --git a/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java b/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java index a1d6dc609..7d7406e6f 100644 --- a/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java +++ b/src/main/java/org/springframework/data/redis/hash/Jackson2HashMapper.java @@ -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; @@ -233,8 +237,12 @@ public class Jackson2HashMapper implements HashMapper { if (flatten) { - return typingMapper.reader().forType(Object.class) - .readValue(untypedMapper.writeValueAsBytes(doUnflatten(hash))); + Map unflattenedHash = doUnflatten(hash); + byte[] unflattenedHashedBytes = untypedMapper.writeValueAsBytes(unflattenedHash); + Object hashedObject = typingMapper.reader().forType(Object.class) + .readValue(unflattenedHashedBytes);; + + return hashedObject; } return typingMapper.treeToValue(untypedMapper.valueToTree(hash), Object.class); @@ -248,31 +256,37 @@ public class Jackson2HashMapper implements HashMapper { private Map doUnflatten(Map source) { Map result = new LinkedHashMap<>(); - Set treatSeperate = new LinkedHashSet<>(); + Set treatSeparate = new LinkedHashSet<>(); + for (Entry 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) 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) 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 newSource = new LinkedHashMap<>(); @@ -284,12 +298,13 @@ public class Jackson2HashMapper implements HashMapper { 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) result.get(prunedKey)); + if (result.containsKey(nonIndexPartial)) { + addValueToTypedListAtIndex((List) 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)); @@ -299,6 +314,27 @@ public class Jackson2HashMapper implements HashMapper { 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 flattenMap(Iterator> source) { Map resultMap = new HashMap<>(); @@ -314,7 +350,6 @@ public class Jackson2HashMapper implements HashMapper { } while (inputMap.hasNext()) { - Entry entry = inputMap.next(); flattenElement(propertyPrefix + entry.getKey(), entry.getValue(), resultMap); } @@ -323,7 +358,6 @@ public class Jackson2HashMapper implements HashMapper { private void flattenElement(String propertyPrefix, Object source, Map resultMap) { if (!(source instanceof JsonNode)) { - resultMap.put(propertyPrefix, source); return; } @@ -337,6 +371,7 @@ public class Jackson2HashMapper implements HashMapper { while (nodes.hasNext()) { JsonNode cur = nodes.next(); + if (cur.isArray()) { this.flattenCollection(propertyPrefix, cur.elements(), resultMap); } else { @@ -370,12 +405,13 @@ public class Jackson2HashMapper implements HashMapper { 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; } - } } } @@ -390,53 +426,49 @@ public class Jackson2HashMapper implements HashMapper { 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 list, Map 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 destination) { + private void addValueToTypedListAtIndex(List listWithTypeHint, int index, Object value) { - int index = Integer.parseInt(key.substring(key.indexOf('[') + 1, key.length() - 1)); - List resultList = ((List) destination.get(1)); - if (resultList.size() < index) { - resultList.add(value); - } else { - resultList.add(index, value); + List valueList = (List) listWithTypeHint.get(1); + + if (index >= valueList.size()) { + int initialCapacity = index + 1; + List newValueList = new ArrayList<>(initialCapacity); + Collections.copy(CollectionUtils.initializeList(newValueList, initialCapacity), valueList); + listWithTypeHint.set(1, newValueList); + valueList = newValueList; } + + valueList.set(index, value); } - private List createTypedListWithValue(Object value) { + private List createTypedListWithValue(int index, Object value) { + + int initialCapacity = index + 1; + + List valueList = CollectionUtils.initializeList(new ArrayList<>(initialCapacity), initialCapacity); + valueList.set(index, value); List listWithTypeHint = new ArrayList<>(); - listWithTypeHint.add(ArrayList.class.getName()); // why jackson? why? - List values = new ArrayList<>(); - values.add(value); - listWithTypeHint.add(values); + listWithTypeHint.add(ArrayList.class.getName()); + listWithTypeHint.add(valueList); + return listWithTypeHint; } @@ -468,16 +500,16 @@ public class Jackson2HashMapper implements HashMapper { @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)); } } } @@ -500,13 +532,13 @@ public class Jackson2HashMapper implements HashMapper { Date date = dateDeserializer.deserialize(p, ctxt); - 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; } } @@ -524,17 +556,20 @@ public class Jackson2HashMapper implements HashMapper { } @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); } } } diff --git a/src/main/java/org/springframework/data/redis/support/collections/CollectionUtils.java b/src/main/java/org/springframework/data/redis/support/collections/CollectionUtils.java index d1951ba00..1daa03170 100644 --- a/src/main/java/org/springframework/data/redis/support/collections/CollectionUtils.java +++ b/src/main/java/org/springframework/data/redis/support/collections/CollectionUtils.java @@ -35,18 +35,8 @@ import org.springframework.lang.Nullable; */ public abstract class CollectionUtils { - @SuppressWarnings("unchecked") - static Collection reverse(Collection c) { - Object[] reverse = new Object[c.size()]; - int index = c.size(); - for (E e : c) { - reverse[--index] = e; - } - - return (List) Arrays.asList(reverse); - } - static Collection extractKeys(Collection stores) { + Collection keys = new ArrayList<>(stores.size()); for (RedisStore store : stores) { @@ -56,10 +46,27 @@ public abstract class CollectionUtils { return keys; } + public static List initializeList(@NonNull List list, int size) { + + for (int count = 0; count < size; count++) { + list.add(null); + } + + return list; + } + + @NonNull + public static List nullSafeList(@Nullable List list) { + return list != null ? list : Collections.emptyList(); + } + static void rename(final K key, final K newKey, RedisOperations operations) { + operations.execute(new SessionCallback() { + @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 List nullSafeList(@Nullable List list) { - return list != null ? list : Collections.emptyList(); + @SuppressWarnings("unchecked") + static Collection reverse(Collection c) { + + Object[] reverse = new Object[c.size()]; + int index = c.size(); + + for (E e : c) { + reverse[--index] = e; + } + + return (List) Arrays.asList(reverse); } } diff --git a/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperIntegrationTests.java b/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperIntegrationTests.java index 52f6f9653..d89cdd2b1 100644 --- a/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperIntegrationTests.java +++ b/src/test/java/org/springframework/data/redis/mapping/Jackson2HashMapperIntegrationTests.java @@ -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. 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.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 phoneNumber; + + User(String name) { + this.name = name; + } + + User withPhoneNumber(Integer... numbers) { + this.phoneNumber = new ArrayList<>(Arrays.asList(numbers)); + return this; + } + } }