From f386e05b8fdf4f6eb473a8e477dbd3965def4d76 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 12 Feb 2024 09:48:26 +0100 Subject: [PATCH] Polishing. Refine nullability declarations and NPE guards. Reformat code. See #4510 Original pull request: #4517 --- .../core/convert/MongoConversionContext.java | 17 ++-- .../mongodb/core/convert/QueryMapper.java | 77 ++++++++++++------- .../core/mapping/MongoPersistentProperty.java | 3 + .../data/mongodb/util/BsonUtils.java | 7 +- 4 files changed, 67 insertions(+), 37 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java index 5172746cc..c1a478fa7 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MongoConversionContext.java @@ -32,19 +32,19 @@ import org.springframework.lang.Nullable; public class MongoConversionContext implements ValueConversionContext { private final PropertyValueProvider accessor; // TODO: generics - private final MongoPersistentProperty persistentProperty; + private final @Nullable MongoPersistentProperty persistentProperty; private final MongoConverter mongoConverter; - @Nullable - private final SpELContext spELContext; + @Nullable private final SpELContext spELContext; public MongoConversionContext(PropertyValueProvider accessor, - MongoPersistentProperty persistentProperty, MongoConverter mongoConverter) { + @Nullable MongoPersistentProperty persistentProperty, MongoConverter mongoConverter) { this(accessor, persistentProperty, mongoConverter, null); } public MongoConversionContext(PropertyValueProvider accessor, - MongoPersistentProperty persistentProperty, MongoConverter mongoConverter, @Nullable SpELContext spELContext) { + @Nullable MongoPersistentProperty persistentProperty, MongoConverter mongoConverter, + @Nullable SpELContext spELContext) { this.accessor = accessor; this.persistentProperty = persistentProperty; @@ -54,12 +54,17 @@ public class MongoConversionContext implements ValueConversionContext sourceMap) { + if (source instanceof Map sourceMap) { Map map = new LinkedHashMap<>(sourceMap.size(), 1F); - sourceMap.entrySet().forEach(it -> { + for (Entry entry : sourceMap.entrySet()) { - String key = ObjectUtils.nullSafeToString(converter.convertToMongoType(it.getKey())); + String key = ObjectUtils.nullSafeToString(converter.convertToMongoType(entry.getKey())); - if (it.getValue() instanceof Document document) { + if (entry.getValue() instanceof Document document) { map.put(key, getMappedObject(document, entity)); } else { - map.put(key, delegateConvertToMongoType(it.getValue(), entity)); + map.put(key, delegateConvertToMongoType(entry.getValue(), entity)); } - }); + } return map; } @@ -603,6 +609,7 @@ public class QueryMapper { return converter.convertToMongoType(source, entity == null ? null : entity.getTypeInformation()); } + @Nullable protected Object convertAssociation(Object source, Field field) { Object value = convertAssociation(source, field.getProperty()); if (value != null && field.isIdField() && field.getFieldType() != value.getClass()) { @@ -627,8 +634,7 @@ public class QueryMapper { if (source instanceof DBRef ref) { - Object id = convertId(ref.getId(), - property != null && property.isIdProperty() ? property.getFieldType() : ObjectId.class); + Object id = convertId(ref.getId(), property.isIdProperty() ? property.getFieldType() : ObjectId.class); if (StringUtils.hasText(ref.getDatabaseName())) { return new DBRef(ref.getDatabaseName(), ref.getCollectionName(), id); @@ -645,9 +651,8 @@ public class QueryMapper { return result; } - if (property.isMap()) { + if (property.isMap() && source instanceof Document dbObject) { Document result = new Document(); - Document dbObject = (Document) source; for (String key : dbObject.keySet()) { result.put(key, createReferenceFor(dbObject.get(key), property)); } @@ -661,19 +666,26 @@ public class QueryMapper { private Object convertValue(Field documentField, Object sourceValue, Object value, PropertyValueConverter> valueConverter) { - MongoConversionContext conversionContext = new MongoConversionContext(new PropertyValueProvider<>() { - @Override - public T getPropertyValue(MongoPersistentProperty property) { - throw new IllegalStateException("No enclosing property available"); - } - }, documentField.getProperty(), converter); + MongoPersistentProperty property = documentField.getProperty(); + MongoConversionContext conversionContext = new MongoConversionContext(NoPropertyPropertyValueProvider.INSTANCE, + property, converter); /* might be an $in clause with multiple entries */ - if (!documentField.getProperty().isCollectionLike() && sourceValue instanceof Collection collection) { - return collection.stream().map(it -> valueConverter.write(it, conversionContext)).collect(Collectors.toList()); + if (property != null && !property.isCollectionLike() && sourceValue instanceof Collection collection) { + + if (collection.isEmpty()) { + return collection; + } + + List converted = new ArrayList<>(collection.size()); + for (Object o : collection) { + converted.add(valueConverter.write(o, conversionContext)); + } + + return converted; } - if (!documentField.getProperty().isMap() && sourceValue instanceof Document document) { + if (property != null && !documentField.getProperty().isMap() && sourceValue instanceof Document document) { return BsonUtils.mapValues(document, (key, val) -> { if (isKeyword(key)) { @@ -687,6 +699,7 @@ public class QueryMapper { } @Nullable + @SuppressWarnings("unchecked") private Object convertIdField(Field documentField, Object source) { Object value = source; @@ -714,8 +727,8 @@ public class QueryMapper { } else { return getMappedObject(resultDbo, Optional.empty()); } - return resultDbo; + return resultDbo; } /** @@ -1454,7 +1467,6 @@ public class QueryMapper { private final Iterator iterator; private int currentIndex; - private String currentPropertyRoot; private final List pathParts; public KeyMapper(String key, @@ -1462,7 +1474,6 @@ public class QueryMapper { this.pathParts = Arrays.asList(key.split("\\.")); this.iterator = pathParts.iterator(); - this.currentPropertyRoot = iterator.next(); this.currentIndex = 0; } @@ -1578,4 +1589,14 @@ public class QueryMapper { public MongoConverter getConverter() { return converter; } + + private enum NoPropertyPropertyValueProvider implements PropertyValueProvider { + + INSTANCE; + + @Override + public T getPropertyValue(MongoPersistentProperty property) { + throw new IllegalStateException("No enclosing property source available"); + } + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java index c6c26c912..357428cd8 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/MongoPersistentProperty.java @@ -21,6 +21,7 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.data.annotation.Id; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.NonNull; import org.springframework.lang.Nullable; /** @@ -191,6 +192,8 @@ public interface MongoPersistentProperty extends PersistentProperty valueMapper.apply(entry.getKey(), entry.getValue())); } - public static Document mapEntries(Document source, Function,String> keyMapper, Function,Object> valueMapper) { + public static Document mapEntries(Document source, Function, String> keyMapper, + Function, Object> valueMapper) { - if(source.isEmpty()) { + if (source.isEmpty()) { return source; } Map target = new LinkedHashMap<>(source.size(), 1f); - for(Entry entry : source.entrySet()) { + for (Entry entry : source.entrySet()) { target.put(keyMapper.apply(entry), valueMapper.apply(entry)); } return new Document(target);