From eac5cb8c463bb04899e5aee3939fc89913cad6fa Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 24 Aug 2011 16:31:18 +0200 Subject: [PATCH] DATADOC-247 - BigInteger ids handled properly in QueryMapper. QueryMapper now tries to convert given ids to ObjectId and String. --- .../data/mongodb/core/QueryMapper.java | 47 +++++++++++------ .../core/convert/AbstractMongoConverter.java | 5 +- .../core/query/QueryMapperUnitTests.java | 50 ++++++++++++++----- 3 files changed, 71 insertions(+), 31 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryMapper.java index c7def316e..8860dfb02 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/QueryMapper.java @@ -16,11 +16,10 @@ package org.springframework.data.mongodb.core; import java.util.ArrayList; +import java.util.Arrays; import java.util.Iterator; import java.util.List; -import com.mongodb.BasicDBObject; -import com.mongodb.DBObject; import org.bson.types.BasicBSONList; import org.bson.types.ObjectId; import org.springframework.core.convert.ConversionException; @@ -29,6 +28,9 @@ import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.util.Assert; +import com.mongodb.BasicDBObject; +import com.mongodb.DBObject; + /** * A helper class to encapsulate any modifications of a Query object before it gets submitted to the database. * @@ -78,26 +80,15 @@ public class QueryMapper { String inKey = valueDbo.containsField("$in") ? "$in" : "$nin"; List ids = new ArrayList(); for (Object id : (Object[]) valueDbo.get(inKey)) { - if (!(id instanceof ObjectId)) { - try { - ObjectId oid = conversionService.convert(id, ObjectId.class); - ids.add(oid); - } catch (ConversionException ignored) { - ids.add(id); - } - } else { - ids.add(id); - } + ids.add(convertId(id)); } valueDbo.put(inKey, ids.toArray(new Object[ids.size()])); } else { value = getMappedObject((DBObject) value, entity); } } else { - try { - value = conversionService.convert(value, ObjectId.class); - } catch (ConversionException ignored) { - } + + value = convertId(value); } newKey = "_id"; } else if (key.startsWith("$") && key.endsWith("or")) { @@ -117,4 +108,28 @@ public class QueryMapper { return newDbo; } + /** + * Converts the given raw id value into either {@link ObjectId} or {@link String}. + * + * @param id + * @return + */ + @SuppressWarnings("unchecked") + public Object convertId(Object id) { + + for (Class type : Arrays.asList(ObjectId.class, String.class)) { + + if (id.getClass().isAssignableFrom(type)) { + return id; + } + + try { + return conversionService.convert(id, type); + } catch (ConversionException e) { + // Ignore + } + } + + return id; + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverter.java index b1a9240ae..31ab6e670 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverter.java @@ -62,8 +62,9 @@ public abstract class AbstractMongoConverter implements MongoConverter, Initiali } /** - * Registers converters for {@link ObjectId} handling, removes plain {@link #toString()} converter and promotes the - * configured {@link ConversionService} to {@link MappingBeanHelper}. + * Registers additional converters that will be available when using the {@link ConversionService} directly (e.g. for + * id conversion). These converters are not custom conversions as they'd introduce unwanted conversions (e.g. + * ObjectId-to-String). */ private void initializeConverters() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryMapperUnitTests.java index fd0c34773..d94a3dcf2 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryMapperUnitTests.java @@ -17,7 +17,8 @@ package org.springframework.data.mongodb.core.query; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; -import static org.mockito.Mockito.*; + +import java.math.BigInteger; import org.bson.types.ObjectId; import org.junit.Before; @@ -25,10 +26,12 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.springframework.core.convert.ConversionService; +import org.springframework.data.annotation.Id; +import org.springframework.data.mongodb.MongoDbFactory; import org.springframework.data.mongodb.core.QueryMapper; +import org.springframework.data.mongodb.core.convert.MappingMongoConverter; +import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; -import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import com.mongodb.BasicDBObject; import com.mongodb.DBObject; @@ -39,30 +42,31 @@ import com.mongodb.DBObject; * @author Oliver Gierke */ @RunWith(MockitoJUnitRunner.class) +@SuppressWarnings("unused") public class QueryMapperUnitTests { QueryMapper mapper; + MongoMappingContext context; @Mock - ConversionService converter; - @Mock - MongoPersistentEntity entity; - @Mock - MongoPersistentProperty property; + MongoDbFactory factory; @Before public void setUp() { - when(entity.getIdProperty()).thenReturn(property); - when(converter.convert(any(), eq(ObjectId.class))).thenReturn(new ObjectId()); - mapper = new QueryMapper(converter); + + context = new MongoMappingContext(); + + MappingMongoConverter converter = new MappingMongoConverter(factory, context); + converter.afterPropertiesSet(); + + mapper = new QueryMapper(converter.getConversionService()); } @Test public void translatesIdPropertyIntoIdKey() { DBObject query = new BasicDBObject("foo", "value"); - - when(property.getName()).thenReturn("foo"); + MongoPersistentEntity entity = context.getPersistentEntity(Sample.class); DBObject result = mapper.getMappedObject(query, entity); assertThat(result.get("_id"), is(notNullValue())); @@ -76,4 +80,24 @@ public class QueryMapperUnitTests { DBObject result = mapper.getMappedObject(query, null); assertThat(result.get("_id"), is(ObjectId.class)); } + + @Test + public void handlesBigIntegerIdsCorrectly() { + + DBObject dbObject = new BasicDBObject("id", new BigInteger("1")); + DBObject result = mapper.getMappedObject(dbObject, null); + assertThat(result.get("_id"), is((Object) "1")); + } + + class Sample { + + @Id + private String foo; + } + + class BigIntegerId { + + @Id + private BigInteger id; + } }