From 4a27ba0a3fcc60ebd6a160861501069e6bb3050b Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 22 Jun 2012 14:46:35 +0200 Subject: [PATCH] DATAMONGO-466 - QueryMapper now only tries id conversion for top level document. So far the QueryMapper has tried to map id properties of nested documents to ObjectIds which it shouldn't do actually. --- .../mongodb/core/convert/QueryMapper.java | 29 +++++++------------ .../core/convert/QueryMapperUnitTests.java | 29 +++++++++++++++++-- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 7c129b63a..4c6aef514 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -25,7 +25,6 @@ import org.bson.types.ObjectId; import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionService; import org.springframework.data.mapping.PersistentEntity; -import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import org.springframework.util.Assert; @@ -73,7 +72,6 @@ public class QueryMapper { for (String key : query.keySet()) { - MongoPersistentEntity nestedEntity = getNestedEntity(entity, key); String newKey = key; Object value = query.get(key); @@ -88,7 +86,7 @@ public class QueryMapper { } valueDbo.put(inKey, ids.toArray(new Object[ids.size()])); } else { - value = getMappedObject((DBObject) value, nestedEntity); + value = getMappedObject((DBObject) value, null); } } else { value = convertId(value); @@ -100,14 +98,14 @@ public class QueryMapper { BasicBSONList newConditions = new BasicBSONList(); Iterator iter = conditions.iterator(); while (iter.hasNext()) { - newConditions.add(getMappedObject((DBObject) iter.next(), nestedEntity)); + newConditions.add(getMappedObject((DBObject) iter.next(), null)); } value = newConditions; } else if (key.equals("$ne")) { value = convertId(value); } - newDbo.put(newKey, convertSimpleOrDBObject(value, nestedEntity)); + newDbo.put(newKey, convertSimpleOrDBObject(value, null)); } return newDbo; @@ -142,26 +140,19 @@ public class QueryMapper { */ private boolean isIdKey(String key, MongoPersistentEntity entity) { - if (null != entity && entity.getIdProperty() != null) { - MongoPersistentProperty idProperty = entity.getIdProperty(); + if (entity == null) { + return false; + } + + MongoPersistentProperty idProperty = entity.getIdProperty(); + + if (idProperty != null) { return idProperty.getName().equals(key) || idProperty.getFieldName().equals(key); } return DEFAULT_ID_NAMES.contains(key); } - private MongoPersistentEntity getNestedEntity(MongoPersistentEntity entity, String key) { - - MongoPersistentProperty property = entity == null ? null : entity.getPersistentProperty(key); - - if (property == null || !property.isEntity()) { - return null; - } - - MappingContext, MongoPersistentProperty> context = converter.getMappingContext(); - return context.getPersistentEntity(property); - } - /** * Converts the given raw id value into either {@link ObjectId} or {@link String}. * diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java index cf304a4b7..4c635e305 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java @@ -84,7 +84,7 @@ public class QueryMapperUnitTests { public void convertsStringIntoObjectId() { DBObject query = new BasicDBObject("_id", new ObjectId().toString()); - DBObject result = mapper.getMappedObject(query, null); + DBObject result = mapper.getMappedObject(query, context.getPersistentEntity(IdWrapper.class)); assertThat(result.get("_id"), is(instanceOf(ObjectId.class))); } @@ -92,7 +92,7 @@ public class QueryMapperUnitTests { public void handlesBigIntegerIdsCorrectly() { DBObject dbObject = new BasicDBObject("id", new BigInteger("1")); - DBObject result = mapper.getMappedObject(dbObject, null); + DBObject result = mapper.getMappedObject(dbObject, context.getPersistentEntity(IdWrapper.class)); assertThat(result.get("_id"), is((Object) "1")); } @@ -101,7 +101,7 @@ public class QueryMapperUnitTests { ObjectId id = new ObjectId(); DBObject dbObject = new BasicDBObject("id", new BigInteger(id.toString(), 16)); - DBObject result = mapper.getMappedObject(dbObject, null); + DBObject result = mapper.getMappedObject(dbObject, context.getPersistentEntity(IdWrapper.class)); assertThat(result.get("_id"), is((Object) id)); } @@ -199,6 +199,29 @@ public class QueryMapperUnitTests { assertThat(result, is(query.getQueryObject())); } + @Test + public void doesNotHandleNestedFieldsWithDefaultIdNames() { + + BasicDBObject dbObject = new BasicDBObject("id", new ObjectId().toString()); + dbObject.put("nested", new BasicDBObject("id", new ObjectId().toString())); + + MongoPersistentEntity entity = context.getPersistentEntity(ClassWithDefaultId.class); + + DBObject result = mapper.getMappedObject(dbObject, entity); + assertThat(result.get("_id"), is(instanceOf(ObjectId.class))); + assertThat(((DBObject) result.get("nested")).get("id"), is(instanceOf(String.class))); + } + + class IdWrapper { + Object id; + } + + class ClassWithDefaultId { + + String id; + ClassWithDefaultId nested; + } + class Sample { @Id