DATAMONGO-1703 - Convert resolved DBRef's from source that do not match the requested property type.

We now check if already resolved DBRef's are assignable to the target property type. If not, we perform conversion again to prevent ClassCastException when trying to assign non matching types.

Remove non applicable public modifiers in ObjectPath.

Original pull request: #478.
This commit is contained in:
Christoph Strobl
2017-07-04 13:59:42 +02:00
committed by Mark Paluch
parent 1f2d0da5ed
commit 1681bcd15b
4 changed files with 310 additions and 56 deletions

View File

@@ -74,7 +74,7 @@ import com.mongodb.DBRef;
/**
* {@link MongoConverter} that uses a {@link MappingContext} to do sophisticated mapping of domain objects to
* {@link DBObject}.
*
*
* @author Oliver Gierke
* @author Jon Brisbin
* @author Patrik Wasik
@@ -101,7 +101,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Creates a new {@link MappingMongoConverter} given the new {@link DbRefResolver} and {@link MappingContext}.
*
*
* @param dbRefResolver must not be {@literal null}.
* @param mappingContext must not be {@literal null}.
*/
@@ -123,7 +123,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Creates a new {@link MappingMongoConverter} given the new {@link MongoDbFactory} and {@link MappingContext}.
*
*
* @deprecated use the constructor taking a {@link DbRefResolver} instead.
* @param mongoDbFactory must not be {@literal null}.
* @param mappingContext must not be {@literal null}.
@@ -139,12 +139,13 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
* converter and how to lookup type information from {@link DBObject}s when reading them. Uses a
* {@link DefaultMongoTypeMapper} by default. Setting this to {@literal null} will reset the {@link TypeMapper} to the
* default one.
*
*
* @param typeMapper the typeMapper to set
*/
public void setTypeMapper(MongoTypeMapper typeMapper) {
this.typeMapper = typeMapper == null
? new DefaultMongoTypeMapper(DefaultMongoTypeMapper.DEFAULT_TYPE_KEY, mappingContext) : typeMapper;
? new DefaultMongoTypeMapper(DefaultMongoTypeMapper.DEFAULT_TYPE_KEY, mappingContext)
: typeMapper;
}
/*
@@ -161,7 +162,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
* any translation but rather reject a {@link Map} with keys containing dots causing the conversion for the entire
* object to fail. If further customization of the translation is needed, have a look at
* {@link #potentiallyEscapeMapKey(String)} as well as {@link #potentiallyUnescapeMapKey(String)}.
*
*
* @param mapKeyDotReplacement the mapKeyDotReplacement to set
*/
public void setMapKeyDotReplacement(String mapKeyDotReplacement) {
@@ -340,7 +341,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Root entry method into write conversion. Adds a type discriminator to the {@link DBObject}. Shouldn't be called for
* nested conversions.
*
*
* @see org.springframework.data.mongodb.core.convert.MongoWriter#write(java.lang.Object, com.mongodb.DBObject)
*/
public void write(final Object obj, final DBObject dbo) {
@@ -364,7 +365,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Internal write conversion method which should be used for nested invocations.
*
*
* @param obj
* @param dbo
*/
@@ -520,7 +521,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
addCustomTypeKeyIfNecessary(ClassTypeInformation.from(prop.getRawType()), obj, propDbObj);
MongoPersistentEntity<?> entity = isSubtype(prop.getType(), obj.getClass())
? mappingContext.getPersistentEntity(obj.getClass()) : mappingContext.getPersistentEntity(type);
? mappingContext.getPersistentEntity(obj.getClass())
: mappingContext.getPersistentEntity(type);
writeInternal(obj, propDbObj, entity);
accessor.put(prop, propDbObj);
@@ -534,7 +536,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
* 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
*/
@@ -549,7 +551,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Writes the given {@link Collection} using the given {@link MongoPersistentProperty} information.
*
*
* @param collection must not be {@literal null}.
* @param property must not be {@literal null}.
* @return
@@ -577,7 +579,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Writes the given {@link Map} using the given {@link MongoPersistentProperty} information.
*
*
* @param map must not {@literal null}.
* @param property must not be {@literal null}.
* @return
@@ -613,7 +615,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Populates the given {@link BasicDBList} with values from the given {@link Collection}.
*
*
* @param source the collection to create a {@link BasicDBList} for, must not be {@literal null}.
* @param type the {@link TypeInformation} to consider or {@literal null} if unknown.
* @param sink the {@link BasicDBList} to write to.
@@ -643,7 +645,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Writes the given {@link Map} to the given {@link DBObject} considering the given {@link TypeInformation}.
*
*
* @param obj must not be {@literal null}.
* @param dbo must not be {@literal null}.
* @param propertyType must not be {@literal null}.
@@ -682,7 +684,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Prepares the given {@link Map} key to be converted into a {@link String}. Will invoke potentially registered custom
* conversions and escape dots from the result as they're not supported as {@link Map} key in MongoDB.
*
*
* @param key must not be {@literal null}.
* @return
*/
@@ -697,7 +699,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Potentially replaces dots in the given map key with the configured map key replacement if configured or aborts
* conversion if none is configured.
*
*
* @see #setMapKeyDotReplacement(String)
* @param source
* @return
@@ -720,7 +722,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Returns a {@link String} representation of the given {@link Map} key
*
*
* @param key
* @return
*/
@@ -731,13 +733,14 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
}
return conversions.hasCustomWriteTarget(key.getClass(), String.class)
? (String) getPotentiallyConvertedSimpleWrite(key) : key.toString();
? (String) getPotentiallyConvertedSimpleWrite(key)
: key.toString();
}
/**
* Translates the map key replacements in the given key just read with a dot in case a map key replacement has been
* configured.
*
*
* @param source
* @return
*/
@@ -767,7 +770,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Writes the given simple value to the given {@link DBObject}. Will store enum names for enum values.
*
*
* @param value
* @param dbObject must not be {@literal null}.
* @param key must not be {@literal null}.
@@ -784,7 +787,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Checks whether we have a custom conversion registered for the given value into an arbitrary simple Mongo type.
* Returns the converted value if so. If not, we perform special enum handling or simply return the value as is.
*
*
* @param value
* @return
*/
@@ -806,7 +809,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Checks whether we have a custom conversion for the given simple object. Converts the given value if so, applies
* {@link Enum} handling or returns the value as is.
*
*
* @param value
* @param target must not be {@literal null}.
* @return
@@ -879,7 +882,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Reads the given {@link BasicDBList} into a collection of the given {@link TypeInformation}.
*
*
* @param targetType must not be {@literal null}.
* @param sourceValue must not be {@literal null}.
* @param path must not be {@literal null}.
@@ -929,7 +932,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Reads the given {@link DBObject} into a {@link Map}. will recursively resolve nested {@link Map}s as well.
*
*
* @param type the {@link Map} {@link TypeInformation} to be used to unmarshall this {@link DBObject}.
* @param dbObject must not be {@literal null}
* @param path must not be {@literal null}
@@ -1031,7 +1034,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
for (Entry<Object, Object> entry : ((Map<Object, Object>) obj).entrySet()) {
TypeInformation<? extends Object> valueTypeHint = typeHint != null && typeHint.getMapValueType() != null
? typeHint.getMapValueType() : typeHint;
? typeHint.getMapValueType()
: typeHint;
converted.put(getPotentiallyConvertedSimpleWrite(entry.getKey()).toString(),
convertToMongoType(entry.getValue(), valueTypeHint));
@@ -1170,7 +1174,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Extension of {@link SpELExpressionParameterValueProvider} to recursively trigger value conversion on the raw
* resolved SpEL value.
*
*
* @author Oliver Gierke
*/
private class ConverterAwareSpELExpressionParameterValueProvider
@@ -1180,7 +1184,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Creates a new {@link ConverterAwareSpELExpressionParameterValueProvider}.
*
*
* @param evaluator must not be {@literal null}.
* @param conversionService must not be {@literal null}.
* @param delegate must not be {@literal null}.
@@ -1228,11 +1232,11 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
return (T) dbref;
}
Object object = dbref == null ? null : path.getPathItem(dbref.getId(), dbref.getCollectionName());
return (T) (object != null ? object : readAndConvertDBRef(dbref, type, path, rawType));
T object = dbref == null ? null : path.getPathItem(dbref.getId(), dbref.getCollectionName(), (Class<T>) rawType);
return object != null ? object : (T) readAndConvertDBRef(dbref, type, path, rawType);
}
private <T> T readAndConvertDBRef(DBRef dbref, TypeInformation<?> type, ObjectPath path, final Class<?> rawType) {
private <T> T readAndConvertDBRef(DBRef dbref, TypeInformation<?> type, ObjectPath path, Class<?> rawType) {
List<T> result = bulkReadAndConvertDBRefs(Collections.singletonList(dbref), type, path, rawType);
return CollectionUtils.isEmpty(result) ? null : result.iterator().next();
@@ -1262,7 +1266,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
}
List<DBObject> referencedRawDocuments = dbrefs.size() == 1
? Collections.singletonList(readRef(dbrefs.iterator().next())) : bulkReadRefs(dbrefs);
? Collections.singletonList(readRef(dbrefs.iterator().next()))
: bulkReadRefs(dbrefs);
String collectionName = dbrefs.iterator().next().getCollectionName();
List<T> targeList = new ArrayList<T>(dbrefs.size());
@@ -1297,7 +1302,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Performs the fetch operation for the given {@link DBRef}.
*
*
* @param ref
* @return
*/
@@ -1347,7 +1352,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
/**
* Marker class used to indicate we have a non root document object here that might be used within an update - so we
* need to preserve type hints for potential nested elements but need to remove it on top level.
*
*
* @author Christoph Strobl
* @since 1.8
*/

View File

@@ -21,6 +21,8 @@ import java.util.List;
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.StringUtils;
import com.mongodb.DBObject;
@@ -32,14 +34,16 @@ import com.mongodb.DBObject;
* <p>
* An immutable ordered set of target objects for {@link DBObject} to {@link Object} conversions. Object paths can be
* constructed by the {@link #toObjectPath(Object)} method and extended via {@link #push(Object)}.
*
*
* @author Thomas Darimont
* @author Oliver Gierke
* @author Mark Paluch
* @author Christoph Strobl
* @since 1.6
*/
class ObjectPath {
public static final ObjectPath ROOT = new ObjectPath();
static final ObjectPath ROOT = new ObjectPath();
private final List<ObjectPathItem> items;
@@ -50,7 +54,7 @@ class ObjectPath {
/**
* Creates a new {@link ObjectPath} from the given parent {@link ObjectPath} by adding the provided
* {@link ObjectPathItem} to it.
*
*
* @param parent can be {@literal null}.
* @param item
*/
@@ -68,9 +72,9 @@ class ObjectPath {
* @param object must not be {@literal null}.
* @param entity must not be {@literal null}.
* @param id must not be {@literal null}.
* @return
* @return new instance of {@link ObjectPath}.
*/
public ObjectPath push(Object object, MongoPersistentEntity<?> entity, Object id) {
ObjectPath push(Object object, MongoPersistentEntity<?> entity, Object id) {
Assert.notNull(object, "Object must not be null!");
Assert.notNull(entity, "MongoPersistentEntity must not be null!");
@@ -80,14 +84,15 @@ class ObjectPath {
}
/**
* Returns the object with the given id and stored in the given collection if it's contained in the {@link ObjectPath}
* .
*
* Returns the object with the given id and stored in the given collection if it's contained in the* {@link ObjectPath}.
*
* @param id must not be {@literal null}.
* @param collection must not be {@literal null} or empty.
* @return
* @deprecated use {@link #getPathItem(Object, String, Class)}.
*/
public Object getPathItem(Object id, String collection) {
@Deprecated
Object getPathItem(Object id, String collection) {
Assert.notNull(id, "Id must not be null!");
Assert.hasText(collection, "Collection name must not be null!");
@@ -96,11 +101,7 @@ class ObjectPath {
Object object = item.getObject();
if (object == null) {
continue;
}
if (item.getIdValue() == null) {
if (object == null || item.getIdValue() == null) {
continue;
}
@@ -112,16 +113,49 @@ class ObjectPath {
return null;
}
/**
* Get the object with given {@literal id}, stored in the {@literal collection} that is assignable to the given
* {@literal type} or {@literal null} if no match found.
*
* @param id must not be {@literal null}.
* @param collection must not be {@literal null} or empty.
* @param type must not be {@literal null}.
* @return {@literal null} when no match found.
* @since 2.0
*/
<T> T getPathItem(Object id, String collection, Class<T> type) {
Assert.notNull(id, "Id must not be null!");
Assert.hasText(collection, "Collection name must not be null!");
Assert.notNull(type, "Type must not be null!");
for (ObjectPathItem item : items) {
Object object = item.getObject();
if (object == null || item.getIdValue() == null) {
continue;
}
if (collection.equals(item.getCollection()) && id.equals(item.getIdValue())
&& ClassUtils.isAssignable(type, object.getClass())) {
return (T) object;
}
}
return null;
}
/**
* Returns the current object of the {@link ObjectPath} or {@literal null} if the path is empty.
*
*
* @return
*/
public Object getCurrentObject() {
Object getCurrentObject() {
return items.isEmpty() ? null : items.get(items.size() - 1).getObject();
}
/*
/*
* (non-Javadoc)
* @see java.lang.Object#toString()
*/
@@ -135,7 +169,7 @@ class ObjectPath {
List<String> strings = new ArrayList<String>(items.size());
for (ObjectPathItem item : items) {
strings.add(item.object.toString());
strings.add(ObjectUtils.nullSafeToString(item.object));
}
return StringUtils.collectionToDelimitedString(strings, " -> ");
@@ -143,7 +177,7 @@ class ObjectPath {
/**
* An item in an {@link ObjectPath}.
*
*
* @author Thomas Darimont
* @author Oliver Gierke
*/
@@ -155,7 +189,7 @@ class ObjectPath {
/**
* Creates a new {@link ObjectPathItem}.
*
*
* @param object
* @param idValue
* @param collection
@@ -167,15 +201,15 @@ class ObjectPath {
this.collection = collection;
}
public Object getObject() {
Object getObject() {
return object;
}
public Object getIdValue() {
Object getIdValue() {
return idValue;
}
public String getCollection() {
String getCollection() {
return collection;
}
}

View File

@@ -0,0 +1,111 @@
/*
* Copyright 2017 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
*
* http://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;
import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;
import static org.springframework.data.mongodb.core.query.Criteria.*;
import static org.springframework.data.mongodb.core.query.Query.*;
import lombok.Data;
import java.net.UnknownHostException;
import org.junit.Before;
import org.junit.Test;
import org.springframework.data.annotation.Id;
import org.springframework.data.mongodb.core.mapping.DBRef;
import org.springframework.data.mongodb.core.mapping.Document;
import com.mongodb.MongoClient;
/**
* {@link org.springframework.data.mongodb.core.mapping.DBRef} related integration tests for
* {@link org.springframework.data.mongodb.core.MongoTemplate}.
*
* @author Christoph Strobl
*/
public class MongoTemplateDbRefTests {
MongoTemplate template;
@Before
public void setUp() throws UnknownHostException {
template = new MongoTemplate(new MongoClient(), "mongo-template-dbref-tests");
template.dropCollection(RefCycleLoadingIntoDifferentTypeRoot.class);
template.dropCollection(RefCycleLoadingIntoDifferentTypeIntermediate.class);
template.dropCollection(RefCycleLoadingIntoDifferentTypeRootView.class);
}
@Test // DATAMONGO-1703
public void shouldLoadRefIntoDifferentTypeCorrectly() {
// init root
RefCycleLoadingIntoDifferentTypeRoot root = new RefCycleLoadingIntoDifferentTypeRoot();
root.id = "root-1";
root.content = "jon snow";
template.save(root);
// init one and set view id ref to root.id
RefCycleLoadingIntoDifferentTypeIntermediate intermediate = new RefCycleLoadingIntoDifferentTypeIntermediate();
intermediate.id = "one-1";
intermediate.refToRootView = new RefCycleLoadingIntoDifferentTypeRootView();
intermediate.refToRootView.id = root.id;
template.save(intermediate);
// add one ref to root
root.refToIntermediate = intermediate;
template.save(root);
RefCycleLoadingIntoDifferentTypeRoot loaded = template.findOne(query(where("id").is(root.id)),
RefCycleLoadingIntoDifferentTypeRoot.class);
assertThat(loaded.content, is(equalTo("jon snow")));
assertThat(loaded.getRefToIntermediate(), is(instanceOf(RefCycleLoadingIntoDifferentTypeIntermediate.class)));
assertThat(loaded.getRefToIntermediate().getRefToRootView(),
is(instanceOf(RefCycleLoadingIntoDifferentTypeRootView.class)));
assertThat(loaded.getRefToIntermediate().getRefToRootView().getContent(), is(equalTo("jon snow")));
}
@Data
@Document(collection = "cycle-with-different-type-root")
static class RefCycleLoadingIntoDifferentTypeRoot {
@Id String id;
String content;
@DBRef RefCycleLoadingIntoDifferentTypeIntermediate refToIntermediate;
}
@Data
@Document(collection = "cycle-with-different-type-intermediate")
static class RefCycleLoadingIntoDifferentTypeIntermediate {
@Id String id;
@DBRef RefCycleLoadingIntoDifferentTypeRootView refToRootView;
}
@Data
@Document(collection = "cycle-with-different-type-root")
static class RefCycleLoadingIntoDifferentTypeRootView {
@Id String id;
String content;
}
}

View File

@@ -0,0 +1,104 @@
/*
* Copyright 2017 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
*
* http://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 static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;
import org.junit.Before;
import org.junit.Test;
import org.springframework.data.mongodb.core.mapping.BasicMongoPersistentEntity;
import org.springframework.data.mongodb.core.mapping.Document;
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
import org.springframework.data.util.ClassTypeInformation;
/**
* Unit tests for {@link ObjectPath}.
*
* @author Christoph Strobl
*/
public class ObjectPathUnitTests {
MongoPersistentEntity<EntityOne> one;
MongoPersistentEntity<EntityTwo> two;
MongoPersistentEntity<EntityThree> three;
@Before
public void setUp() {
one = new BasicMongoPersistentEntity(ClassTypeInformation.from(EntityOne.class));
two = new BasicMongoPersistentEntity(ClassTypeInformation.from(EntityTwo.class));
three = new BasicMongoPersistentEntity(ClassTypeInformation.from(EntityThree.class));
}
@Test // DATAMONGO-1703
public void getPathItemShouldReturnMatch() {
ObjectPath path = ObjectPath.ROOT.push(new EntityOne(), one, "id-1");
assertThat(path.getPathItem("id-1", "one", EntityOne.class), is(notNullValue()));
}
@Test // DATAMONGO-1703
public void getPathItemShouldReturnNullWhenNoTypeMatchFound() {
ObjectPath path = ObjectPath.ROOT.push(new EntityOne(), one, "id-1");
assertThat(path.getPathItem("id-1", "one", EntityThree.class), is(nullValue()));
}
@Test // DATAMONGO-1703
public void getPathItemShouldReturnCachedItemWhenIdAndCollectionMatchAndIsAssignable() {
ObjectPath path = ObjectPath.ROOT.push(new EntityTwo(), one, "id-1");
assertThat(path.getPathItem("id-1", "one", EntityOne.class), is(notNullValue()));
}
@Test // DATAMONGO-1703
public void getPathItemShouldReturnNullWhenIdAndCollectionMatchButNotAssignable() {
ObjectPath path = ObjectPath.ROOT.push(new EntityOne(), one, "id-1");
assertThat(path.getPathItem("id-1", "one", EntityTwo.class), is(nullValue()));
}
@Test // DATAMONGO-1703
public void getPathItemShouldReturnNullWhenIdAndCollectionMatchAndAssignableToInterface() {
ObjectPath path = ObjectPath.ROOT.push(new EntityThree(), one, "id-1");
assertThat(path.getPathItem("id-1", "one", ValueInterface.class), is(notNullValue()));
}
@Document(collection = "one")
static class EntityOne {
}
static class EntityTwo extends EntityOne {
}
interface ValueInterface {
}
@Document(collection = "three")
static class EntityThree implements ValueInterface {
}
}