diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MapUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MapUtils.java deleted file mode 100644 index 75bc72bea..000000000 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MapUtils.java +++ /dev/null @@ -1,111 +0,0 @@ -/* - * 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.mongodb.core.convert; - -import java.util.Collection; -import java.util.Collections; -import java.util.Map; - -import org.bson.Document; -import org.bson.conversions.Bson; - -import org.springframework.lang.Nullable; -import org.springframework.util.CollectionUtils; - -import com.mongodb.DBObject; - -/** - * @author Mark Paluch - */ -class MapUtils { - /** - * Returns given object as {@link Collection}. Will return the {@link Collection} as is if the source is a - * {@link Collection} already, will convert an array into a {@link Collection} or simply create a single element - * collection for everything else. - * - * @param source - * @return - */ - static Collection asCollection(Object source) { - - if (source instanceof Collection) { - return (Collection) source; - } - - return source.getClass().isArray() ? CollectionUtils.arrayToList(source) : Collections.singleton(source); - } - - @SuppressWarnings("unchecked") - static Map asMap(Bson bson) { - - if (bson instanceof Document) { - return (Document) bson; - } - - if (bson instanceof DBObject) { - return ((DBObject) bson).toMap(); - } - - throw new IllegalArgumentException( - String.format("Cannot read %s. as map. Given Bson must be a Document or DBObject!", bson.getClass())); - } - - static void addToMap(Bson bson, String key, @Nullable Object value) { - - if (bson instanceof Document) { - ((Document) bson).put(key, value); - return; - } - if (bson instanceof DBObject) { - ((DBObject) bson).put(key, value); - return; - } - throw new IllegalArgumentException(String.format( - "Cannot add key/value pair to %s. as map. Given Bson must be a Document or DBObject!", bson.getClass())); - } - - static void addAllToMap(Bson bson, Map value) { - - if (bson instanceof Document) { - ((Document) bson).putAll(value); - return; - } - - if (bson instanceof DBObject) { - ((DBObject) bson).putAll(value); - return; - } - - throw new IllegalArgumentException( - String.format("Cannot add all to %s. Given Bson must be a Document or DBObject.", bson.getClass())); - } - - static void removeFromMap(Bson bson, String key) { - - if (bson instanceof Document) { - ((Document) bson).remove(key); - return; - } - - if (bson instanceof DBObject) { - ((DBObject) bson).removeField(key); - return; - } - - throw new IllegalArgumentException( - String.format("Cannot remove from %s. Given Bson must be a Document or DBObject.", bson.getClass())); - } -} diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 4cd799238..33586de36 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -70,6 +70,7 @@ import org.springframework.data.mongodb.core.mapping.event.AfterConvertCallback; import org.springframework.data.mongodb.core.mapping.event.AfterConvertEvent; import org.springframework.data.mongodb.core.mapping.event.AfterLoadEvent; import org.springframework.data.mongodb.core.mapping.event.MongoMappingEvent; +import org.springframework.data.mongodb.util.BsonUtils; import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; @@ -270,17 +271,25 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } protected S read(TypeInformation type, Bson bson) { - return doRead(getConversionContext(ObjectPath.ROOT), type, bson); + return readDocument(getConversionContext(ObjectPath.ROOT), bson, type); } + /** + * Conversion method to materialize an object from a {@link Bson document}. Can be overridden by subclasses. + * + * @param context must not be {@literal null} + * @param bson must not be {@literal null} + * @param typeHint the {@link TypeInformation} to be used to unmarshall this {@link Document}. + * @return the converted object, will never be {@literal null}. + * @since 3.2 + */ @SuppressWarnings("unchecked") - private S doRead(ConversionContext context, TypeInformation type, Bson bson) { + protected S readDocument(ConversionContext context, Bson bson, + TypeInformation typeHint) { - Assert.notNull(bson, "Bson must not be null!"); - - // TODO: Cleanup duplication - TypeInformation typeToUse = typeMapper.readType(bson, type); - Class rawType = typeToUse.getType(); + Document document = bson instanceof BasicDBObject ? new Document((BasicDBObject) bson) : (Document) bson; + TypeInformation typeToRead = typeMapper.readType(document, typeHint); + Class rawType = typeToRead.getType(); if (conversions.hasCustomReadTarget(bson.getClass(), rawType)) { return doConvert(bson, rawType); @@ -303,34 +312,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return (S) bson; } - return context.convert(bson, typeToUse); - } - - /** - * Conversion method to materialize an object from a {@link Bson document}. Can be overridden by subclasses. - * - * @param context must not be {@literal null} - * @param bson must not be {@literal null} - * @param typeHint the {@link TypeInformation} to be used to unmarshall this {@link Document}. - * @return the converted object, will never be {@literal null}. - * @since 3.2 - */ - @SuppressWarnings("unchecked") - protected S readDocument(ConversionContext context, Bson bson, - TypeInformation typeHint) { - - // TODO: Cleanup duplication - - Document document = bson instanceof BasicDBObject ? new Document((BasicDBObject) bson) : (Document) bson; - TypeInformation typeToRead = typeMapper.readType(document, typeHint); - Class rawType = typeToRead.getType(); - - if (conversions.hasCustomReadTarget(bson.getClass(), rawType)) { - return doConvert(bson, rawType); - } - if (typeToRead.isMap()) { - return (S) bson; + return context.convert(bson, typeToRead); } if (BSON.isAssignableFrom(typeHint)) { @@ -572,9 +555,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Object target = obj instanceof LazyLoadingProxy ? ((LazyLoadingProxy) obj).getTarget() : obj; writeInternal(target, bson, type); - if (MapUtils.asMap(bson).containsKey("_id") && MapUtils.asMap(bson).get("_id") == null) { - MapUtils.removeFromMap(bson, "_id"); - } + BsonUtils.removeNullId(bson); if (requiresTypeHint(entityType)) { typeMapper.writeType(type, bson); @@ -608,7 +589,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (customTarget.isPresent()) { Document result = doConvert(obj, Document.class); - MapUtils.addAllToMap(bson, result); + BsonUtils.addAllToMap(bson, result); return; } @@ -709,12 +690,14 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } if (valueType.isCollectionLike()) { - List collectionInternal = createCollection(MapUtils.asCollection(obj), prop); + + List collectionInternal = createCollection(BsonUtils.asCollection(obj), prop); accessor.put(prop, collectionInternal); return; } if (valueType.isMap()) { + Bson mapDbObj = createMap((Map) obj, prop); accessor.put(prop, mapDbObj); return; @@ -859,7 +842,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App collection.add(getPotentiallyConvertedSimpleWrite(element, componentType != null ? componentType.getType() : Object.class)); } else if (element instanceof Collection || elementType.isArray()) { - collection.add(writeCollectionInternal(MapUtils.asCollection(element), componentType, new BasicDBList())); + collection.add(writeCollectionInternal(BsonUtils.asCollection(element), componentType, new BasicDBList())); } else { Document document = new Document(); writeInternal(element, document, componentType); @@ -890,14 +873,14 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (val == null || conversions.isSimpleType(val.getClass())) { writeSimpleInternal(val, bson, simpleKey); } else if (val instanceof Collection || val.getClass().isArray()) { - MapUtils.addToMap(bson, simpleKey, - writeCollectionInternal(MapUtils.asCollection(val), propertyType.getMapValueType(), new BasicDBList())); + BsonUtils.addToMap(bson, simpleKey, + writeCollectionInternal(BsonUtils.asCollection(val), propertyType.getMapValueType(), new BasicDBList())); } else { Document document = new Document(); TypeInformation valueTypeInfo = propertyType.isMap() ? propertyType.getMapValueType() : ClassTypeInformation.OBJECT; writeInternal(val, document, valueTypeInfo); - MapUtils.addToMap(bson, simpleKey, document); + BsonUtils.addToMap(bson, simpleKey, document); } } else { throw new MappingException("Cannot use a complex object as a key value."); @@ -997,7 +980,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * @param key must not be {@literal null}. */ private void writeSimpleInternal(@Nullable Object value, Bson bson, String key) { - MapUtils.addToMap(bson, key, getPotentiallyConvertedSimpleWrite(value, Object.class)); + BsonUtils.addToMap(bson, key, getPotentiallyConvertedSimpleWrite(value, Object.class)); } private void writeSimpleInternal(@Nullable Object value, Bson bson, MongoPersistentProperty property) { @@ -1035,7 +1018,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (value instanceof byte[]) { return value; } - return MapUtils.asCollection(value); + return BsonUtils.asCollection(value); } return Enum.class.isAssignableFrom(value.getClass()) ? ((Enum) value).name() : value; @@ -1198,7 +1181,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Class rawKeyType = keyType != null ? keyType.getType() : Object.class; Class rawValueType = valueType.getType(); - Map sourceMap = MapUtils.asMap(bson); + Map sourceMap = BsonUtils.asMap(bson); Map map = CollectionFactory.createMap(mapType, rawKeyType, sourceMap.keySet().size()); if (!DBRef.class.equals(rawValueType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceMap.values())) { @@ -1322,7 +1305,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return newDocument; } - // TODO: hide + // TODO: hide in 4.0 public List maybeConvertList(Iterable source, @Nullable TypeInformation typeInformation) { List newDbl = new ArrayList<>(); @@ -1462,7 +1445,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App maybeEmitEvent( new AfterLoadEvent<>(document, (Class) type.getType(), collectionName)); - target = (T) doRead(context, type, document); + target = (T) readDocument(context, document, type); } if (target != null) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java index 5b036072b..85525b13b 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/util/BsonUtils.java @@ -17,12 +17,14 @@ package org.springframework.data.mongodb.util; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.Map; import java.util.StringJoiner; import java.util.function.Function; import java.util.stream.StreamSupport; +import org.bson.BSONObject; import org.bson.BsonBinary; import org.bson.BsonBoolean; import org.bson.BsonDouble; @@ -36,11 +38,11 @@ import org.bson.codecs.DocumentCodec; import org.bson.conversions.Bson; import org.bson.json.JsonParseException; import org.bson.types.ObjectId; - import org.springframework.core.convert.converter.Converter; import org.springframework.data.mongodb.CodecRegistryProvider; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -50,6 +52,8 @@ import com.mongodb.DBRef; import com.mongodb.MongoClientSettings; /** + * Internal API for operations on {@link Bson} elements that can be either {@link Document} or {@link DBObject}. + * * @author Christoph Strobl * @author Mark Paluch * @since 2.0 @@ -70,6 +74,9 @@ public class BsonUtils { if (bson instanceof BasicDBObject) { return ((BasicDBObject) bson); } + if (bson instanceof DBObject) { + return ((DBObject) bson).toMap(); + } return (Map) bson.toBsonDocument(Document.class, MongoClientSettings.getDefaultCodecRegistry()); } @@ -77,14 +84,110 @@ public class BsonUtils { public static void addToMap(Bson bson, String key, @Nullable Object value) { if (bson instanceof Document) { + ((Document) bson).put(key, value); return; } - if (bson instanceof DBObject) { - ((DBObject) bson).put(key, value); + if (bson instanceof BSONObject) { + + ((BSONObject) bson).put(key, value); return; } - throw new IllegalArgumentException("o_O what's that? Cannot add value to " + bson.getClass()); + + throw new IllegalArgumentException(String.format( + "Cannot add key/value pair to %s. as map. Given Bson must be a Document or BSONObject!", bson.getClass())); + } + + /** + * Add all entries from the given {@literal source} {@link Map} to the {@literal target}. + * + * @param target must not be {@literal null}. + * @param source must not be {@literal null}. + * @since 3.2 + */ + public static void addAllToMap(Bson target, Map source) { + + if (target instanceof Document) { + + ((Document) target).putAll(source); + return; + } + + if (target instanceof BSONObject) { + + ((BSONObject) target).putAll(source); + return; + } + + throw new IllegalArgumentException( + String.format("Cannot add all to %s. Given Bson must be a Document or BSONObject.", target.getClass())); + } + + /** + * Check if a given entry (key/value pair) is present in the given {@link Bson}. + * + * @param bson must not be {@literal null}. + * @param key must not be {@literal null}. + * @param value can be {@literal null}. + * @return {@literal true} if (key/value pair) is present. + * @since 3.2 + */ + public static boolean contains(Bson bson, String key, @Nullable Object value) { + + if (bson instanceof Document) { + + Document doc = (Document) bson; + return doc.containsKey(key) && ObjectUtils.nullSafeEquals(doc.get(key), value); + } + if (bson instanceof BSONObject) { + + BSONObject bsonObject = (BSONObject) bson; + return bsonObject.containsField(key) && ObjectUtils.nullSafeEquals(bsonObject.get(key), value); + } + + Map map = asMap(bson); + return map.containsKey(key) && ObjectUtils.nullSafeEquals(map.get(key), value); + } + + /** + * Remove {@code _id : null} from the given {@link Bson} if present. + * + * @param bson must not be {@literal null}. + * @since 3.2 + */ + public static boolean removeNullId(Bson bson) { + + if (!contains(bson, "_id", null)) { + return false; + } + + removeFrom(bson, "_id"); + return true; + } + + /** + * Remove the given {@literal key} from the {@link Bson} value. + * + * @param bson must not be {@literal null}. + * @param key must not be {@literal null}. + * @since 3.2 + */ + static void removeFrom(Bson bson, String key) { + + if (bson instanceof Document) { + + ((Document) bson).remove(key); + return; + } + + if (bson instanceof BSONObject) { + + ((BSONObject) bson).removeField(key); + return; + } + + throw new IllegalArgumentException( + String.format("Cannot remove from %s. Given Bson must be a Document or BSONObject.", bson.getClass())); } /** @@ -282,6 +385,24 @@ public class BsonUtils { .orElseGet(() -> new DocumentCodec(codecRegistryProvider.getCodecRegistry()))); } + /** + * Returns given object as {@link Collection}. Will return the {@link Collection} as is if the source is a + * {@link Collection} already, will convert an array into a {@link Collection} or simply create a single element + * collection for everything else. + * + * @param source must not be {@literal null}. + * @return never {@literal null}. + * @since 3.2 + */ + public static Collection asCollection(Object source) { + + if (source instanceof Collection) { + return (Collection) source; + } + + return source.getClass().isArray() ? CollectionUtils.arrayToList(source) : Collections.singleton(source); + } + @Nullable private static String toJson(@Nullable Object value) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java index 824eee4f2..8210dd9a6 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/util/json/BsonUtilsTest.java @@ -17,11 +17,16 @@ package org.springframework.data.mongodb.util.json; import static org.assertj.core.api.Assertions.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + import org.bson.BsonDouble; import org.bson.BsonInt32; import org.bson.BsonInt64; import org.bson.BsonObjectId; import org.bson.BsonString; +import org.bson.Document; import org.bson.types.ObjectId; import org.junit.jupiter.api.Test; import org.springframework.data.mongodb.util.BsonUtils; @@ -64,4 +69,46 @@ class BsonUtilsTest { assertThatExceptionOfType(IllegalArgumentException.class) .isThrownBy(() -> BsonUtils.simpleToBsonValue(new Object())); } + + @Test // GH-3571 + void removeNullIdIfNull() { + + Document source = new Document("_id", null).append("value", "v-1"); + + assertThat(BsonUtils.removeNullId(source)).isTrue(); + assertThat(source).doesNotContainKey("_id").containsKey("value"); + } + + @Test // GH-3571 + void removeNullIdDoesNotTouchNonNullOn() { + + Document source = new Document("_id", "id-value").append("value", "v-1"); + + assertThat(BsonUtils.removeNullId(source)).isFalse(); + assertThat(source).containsKeys("_id", "value"); + } + + @Test // GH-3571 + void asCollectionDoesNotModifyCollection() { + + Object source = new ArrayList<>(0); + + assertThat(BsonUtils.asCollection(source)).isSameAs(source); + } + + @Test // GH-3571 + void asCollectionConvertsArrayToCollection() { + + Object source = new String[]{"one", "two"}; + + assertThat((Collection)BsonUtils.asCollection(source)).containsExactly("one", "two"); + } + + @Test // GH-3571 + void asCollectionConvertsWrapsNonIterable() { + + Object source = 100L; + + assertThat((Collection)BsonUtils.asCollection(source)).containsExactly(source); + } }