From 6387bfd0297fdd5da16b65cb2bb40236cf43c2c9 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 15 Jul 2020 09:33:07 +0200 Subject: [PATCH] DATAREDIS-1175 - Polishing. Reformat code. Add missing nullability annotations and enforce non-null values to prevent potential null dereference. Original pull request: #546. --- .../data/redis/core/convert/Bucket.java | 4 +- .../core/convert/MappingRedisConverter.java | 88 +++++++++++-------- .../data/redis/core/convert/RedisData.java | 4 +- 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/core/convert/Bucket.java b/src/main/java/org/springframework/data/redis/core/convert/Bucket.java index a6822c518..73a5906e5 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/Bucket.java +++ b/src/main/java/org/springframework/data/redis/core/convert/Bucket.java @@ -61,7 +61,7 @@ public class Bucket { Bucket(Map data) { - Assert.notNull(data, "Inital data must not be null!"); + Assert.notNull(data, "Initial data must not be null!"); this.data = new LinkedHashMap<>(data.size()); this.data.putAll(data); } @@ -72,7 +72,7 @@ public class Bucket { * @param path must not be {@literal null} or {@link String#isEmpty()}. * @param value can be {@literal null}. */ - public void put(String path, byte[] value) { + public void put(String path, @Nullable byte[] value) { Assert.hasText(path, "Path to property must not be null or empty."); data.put(path, value); diff --git a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java index 2a9d47508..1392ca1f1 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java +++ b/src/main/java/org/springframework/data/redis/core/convert/MappingRedisConverter.java @@ -170,27 +170,27 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { * @see org.springframework.data.convert.EntityReader#read(java.lang.Class, java.lang.Object) */ @Override + @SuppressWarnings("unchecked") public R read(Class type, RedisData source) { TypeInformation readType = typeMapper.readType(source.getBucket().getPath(), ClassTypeInformation.from(type)); - if(readType.isCollectionLike()) { - return (R) readCollectionOrArray("", ArrayList.class, Object.class, source.getBucket()); - } - return readInternal("", type, source); + + return readType.isCollectionLike() + ? (R) readCollectionOrArray("", ArrayList.class, Object.class, source.getBucket()) + : doReadInternal("", type, source); + + } + + @Nullable + private R readInternal(String path, Class type, RedisData source) { + return source.getBucket().isEmpty() ? null : doReadInternal(path, type, source); } @SuppressWarnings("unchecked") - @Nullable - private R readInternal(String path, Class type, RedisData source) { - - if (source.getBucket() == null || source.getBucket().isEmpty()) { - return null; - } + private R doReadInternal(String path, Class type, RedisData source) { TypeInformation readType = typeMapper.readType(source.getBucket().getPath(), ClassTypeInformation.from(type)); - RedisPersistentEntity entity = mappingContext.getPersistentEntity(readType); - if (customConversions.hasCustomReadTarget(Map.class, readType.getType())) { Map partial = new HashMap<>(); @@ -206,6 +206,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } R instance = (R) conversionService.convert(partial, readType.getType()); + RedisPersistentEntity entity = mappingContext.getPersistentEntity(readType); if (entity != null && entity.hasIdProperty()) { PersistentPropertyAccessor propertyAccessor = entity.getPropertyAccessor(instance); @@ -221,19 +222,20 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { readType.getType()); } + RedisPersistentEntity entity = mappingContext.getRequiredPersistentEntity(readType); EntityInstantiator instantiator = entityInstantiators.getInstantiatorFor(entity); Object instance = instantiator.createInstance((RedisPersistentEntity) entity, new PersistentEntityParameterValueProvider<>(entity, new ConverterAwareParameterValueProvider(path, source, conversionService), this.conversionService)); - PersistentPropertyAccessor accessor = entity.getPropertyAccessor(instance); + PersistentPropertyAccessor accessor = entity.getPropertyAccessor(instance); entity.doWithProperties((PropertyHandler) persistentProperty -> { PreferredConstructor constructor = entity.getPersistenceConstructor(); - if (constructor.isConstructorParameter(persistentProperty)) { + if (constructor != null && constructor.isConstructorParameter(persistentProperty)) { return; } @@ -306,7 +308,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } private void readAssociation(String path, RedisData source, RedisPersistentEntity entity, - PersistentPropertyAccessor accessor) { + PersistentPropertyAccessor accessor) { entity.doWithAssociations((AssociationHandler) association -> { @@ -395,8 +397,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { sink.setKeyspace(entity.getKeySpace()); - if(entity.getTypeInformation().isCollectionLike()) { - writeCollection(entity.getKeySpace(), "", (List) source, entity.getTypeInformation().getComponentType(), sink); + if (entity.getTypeInformation().isCollectionLike()) { + writeCollection(entity.getKeySpace(), "", (List) source, entity.getTypeInformation().getRequiredComponentType(), + sink); } else { writeInternal(entity.getKeySpace(), "", source, entity.getTypeInformation(), sink); } @@ -478,7 +481,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { if (targetProperty.isCollectionLike()) { RedisPersistentEntity ref = mappingContext.getPersistentEntity(targetProperty.getRequiredAssociation() - .getInverse().getTypeInformation().getRequiredComponentType().getActualType()); + .getInverse().getTypeInformation().getRequiredComponentType().getRequiredActualType()); int i = 0; for (Object o : (Collection) pUpdate.getValue()) { @@ -502,9 +505,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } else if (targetProperty.isCollectionLike()) { Collection collection = pUpdate.getValue() instanceof Collection ? (Collection) pUpdate.getValue() - : Collections. singleton(pUpdate.getValue()); + : Collections.singleton(pUpdate.getValue()); writeCollection(entity.getKeySpace(), pUpdate.getPropertyPath(), collection, - targetProperty.getTypeInformation().getActualType(), sink); + targetProperty.getTypeInformation().getRequiredActualType(), sink); } else if (targetProperty.isMap()) { Map map = new HashMap<>(); @@ -560,7 +563,8 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { * @param typeHint * @param sink */ - private void writeInternal(String keyspace, String path, @Nullable Object value, TypeInformation typeHint, + private void writeInternal(@Nullable String keyspace, String path, @Nullable Object value, + TypeInformation typeHint, RedisData sink) { if (value == null) { @@ -587,7 +591,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } RedisPersistentEntity entity = mappingContext.getRequiredPersistentEntity(value.getClass()); - PersistentPropertyAccessor accessor = entity.getPropertyAccessor(value); + PersistentPropertyAccessor accessor = entity.getPropertyAccessor(value); entity.doWithProperties((PropertyHandler) persistentProperty -> { @@ -632,7 +636,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { if (propertyValue != null) { writeInternal(keyspace, propertyStringPath, propertyValue, - persistentProperty.getTypeInformation().getActualType(), sink); + persistentProperty.getTypeInformation().getRequiredActualType(), sink); } } else { @@ -645,13 +649,13 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { writeAssociation(path, entity, value, sink); } - private void writeAssociation(String path, RedisPersistentEntity entity, Object value, RedisData sink) { + private void writeAssociation(String path, RedisPersistentEntity entity, @Nullable Object value, RedisData sink) { if (value == null) { return; } - PersistentPropertyAccessor accessor = entity.getPropertyAccessor(value); + PersistentPropertyAccessor accessor = entity.getPropertyAccessor(value); entity.doWithAssociations((AssociationHandler) association -> { @@ -663,7 +667,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { if (association.getInverse().isCollectionLike()) { RedisPersistentEntity ref = mappingContext.getRequiredPersistentEntity( - association.getInverse().getTypeInformation().getRequiredComponentType().getActualType()); + association.getInverse().getTypeInformation().getRequiredComponentType().getRequiredActualType()); String keyspace = ref.getKeySpace(); String propertyStringPath = (!path.isEmpty() ? path + "." : "") + association.getInverse().getName(); @@ -684,11 +688,13 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { .getRequiredPersistentEntity(association.getInverse().getTypeInformation()); String keyspace = ref.getKeySpace(); - Object refId = ref.getPropertyAccessor(refObject).getProperty(ref.getIdProperty()); + if (keyspace != null) { + Object refId = ref.getPropertyAccessor(refObject).getProperty(ref.getRequiredIdProperty()); - if (refId != null) { - String propertyStringPath = (!path.isEmpty() ? path + "." : "") + association.getInverse().getName(); - sink.getBucket().put(propertyStringPath, toBytes(keyspace + ":" + refId)); + if (refId != null) { + String propertyStringPath = (!path.isEmpty() ? path + "." : "") + association.getInverse().getName(); + sink.getBucket().put(propertyStringPath, toBytes(keyspace + ":" + refId)); + } } } }); @@ -701,7 +707,8 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { * @param typeHint * @param sink */ - private void writeCollection(String keyspace, String path, @Nullable Iterable values, TypeInformation typeHint, + private void writeCollection(@Nullable String keyspace, String path, @Nullable Iterable values, + TypeInformation typeHint, RedisData sink) { if (values == null) { @@ -762,6 +769,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { } } + @Nullable private Object readCollectionOrArray(String path, Class collectionType, Class valueType, Bucket bucket) { List keys = new ArrayList<>(bucket.extractAllKeysFor(path)); @@ -800,7 +808,8 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { * @param source * @param sink */ - private void writeMap(String keyspace, String path, Class mapValueType, Map source, RedisData sink) { + private void writeMap(@Nullable String keyspace, String path, Class mapValueType, Map source, + RedisData sink) { if (CollectionUtils.isEmpty(source)) { return; @@ -898,6 +907,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { return target.isEmpty() ? null : target; } + @Nullable private Object extractMapKeyForPath(String path, String key, Class targetType) { String regex = "^(" + Pattern.quote(path) + "\\.\\[)(.*?)(\\])"; @@ -1162,9 +1172,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { public static final String DELIMITTER = ":"; public static final String PHANTOM_SUFFIX = DELIMITTER + PHANTOM; - private String keyspace; - private String id; - private boolean phantomKey; + private final String keyspace; + private final String id; + private final boolean phantomKey; /** * Parse a {@code key} into {@link KeyspaceIdentifier}. @@ -1197,7 +1207,7 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { * @param key the key. * @return {@literal true} if the key is valid. */ - public static boolean isValid(String key) { + public static boolean isValid(@Nullable String key) { if (key == null) { return false; @@ -1224,9 +1234,9 @@ public class MappingRedisConverter implements RedisConverter, InitializingBean { public static final byte DELIMITTER = ':'; public static final byte[] PHANTOM_SUFFIX = ByteUtils.concat(new byte[] { DELIMITTER }, PHANTOM); - private byte[] keyspace; - private byte[] id; - private boolean phantomKey; + private final byte[] keyspace; + private final byte[] id; + private final boolean phantomKey; /** * Parse a binary {@code key} into {@link BinaryKeyspaceIdentifier}. diff --git a/src/main/java/org/springframework/data/redis/core/convert/RedisData.java b/src/main/java/org/springframework/data/redis/core/convert/RedisData.java index bd3640e32..70435baa3 100644 --- a/src/main/java/org/springframework/data/redis/core/convert/RedisData.java +++ b/src/main/java/org/springframework/data/redis/core/convert/RedisData.java @@ -76,7 +76,7 @@ public class RedisData { * * @param id */ - public void setId(String id) { + public void setId(@Nullable String id) { this.id = id; } @@ -134,7 +134,7 @@ public class RedisData { /** * @param keyspace */ - public void setKeyspace(String keyspace) { + public void setKeyspace(@Nullable String keyspace) { this.keyspace = keyspace; }