From c876155d34fc36f0a15a0c3bb5dcbecdd69b1eb9 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Thu, 19 Nov 2015 12:02:41 +0100 Subject: [PATCH] DATAMONGO-1324 - Register ObjectId converters unconditionally to make sure they really get used. The presence of ObjectToObjectConverter in a DefaultConversionService causes the guard trying to register converters for ObjectIds in AbstractMongoConverter to not trigger the registration. This in turn caused ObjectId conversions to be executed via reflection instead of the straight forward method calls and thus a drop in performance for such operations. We no unconditionally register the converters to make sure they really get applied. Related tickets: SPR-13703. --- .../core/convert/AbstractMongoConverter.java | 10 +-- .../AbstractMongoConverterUnitTests.java | 90 +++++++++++++++++++ 2 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverterUnitTests.java 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 be2f90b13..b2726f960 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 @@ -75,15 +75,13 @@ public abstract class AbstractMongoConverter implements MongoConverter, Initiali */ private void initializeConverters() { - if (!conversionService.canConvert(ObjectId.class, String.class)) { - conversionService.addConverter(ObjectIdToStringConverter.INSTANCE); - } - if (!conversionService.canConvert(String.class, ObjectId.class)) { - conversionService.addConverter(StringToObjectIdConverter.INSTANCE); - } + conversionService.addConverter(ObjectIdToStringConverter.INSTANCE); + conversionService.addConverter(StringToObjectIdConverter.INSTANCE); + if (!conversionService.canConvert(ObjectId.class, BigInteger.class)) { conversionService.addConverter(ObjectIdToBigIntegerConverter.INSTANCE); } + if (!conversionService.canConvert(BigInteger.class, ObjectId.class)) { conversionService.addConverter(BigIntegerToObjectIdConverter.INSTANCE); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverterUnitTests.java new file mode 100644 index 000000000..717bd5a69 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/AbstractMongoConverterUnitTests.java @@ -0,0 +1,90 @@ +/* + * Copyright 2015 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.mockito.Mockito.*; + +import org.junit.Test; +import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.core.convert.support.GenericConversionService; +import org.springframework.data.mapping.context.MappingContext; +import org.springframework.data.mongodb.core.convert.MongoConverters.ObjectIdToStringConverter; +import org.springframework.data.mongodb.core.convert.MongoConverters.StringToObjectIdConverter; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; +import org.springframework.data.util.TypeInformation; + +import com.mongodb.DBObject; +import com.mongodb.DBRef; + +/** + * Unit tests for {@link AbstractMongoConverter}. + * + * @author Oliver Gierke + */ +public class AbstractMongoConverterUnitTests { + + /** + * @see DATAMONGO-1324 + */ + @Test + public void registersObjectIdConvertersExplicitly() { + + DefaultConversionService conversionService = spy(new DefaultConversionService()); + + new SampleMongoConverter(conversionService).afterPropertiesSet(); + + verify(conversionService).addConverter(StringToObjectIdConverter.INSTANCE); + verify(conversionService).addConverter(ObjectIdToStringConverter.INSTANCE); + } + + static class SampleMongoConverter extends AbstractMongoConverter { + + public SampleMongoConverter(GenericConversionService conversionService) { + super(conversionService); + } + + @Override + public MongoTypeMapper getTypeMapper() { + throw new UnsupportedOperationException(); + } + + @Override + public MappingContext, MongoPersistentProperty> getMappingContext() { + throw new UnsupportedOperationException(); + } + + @Override + public R read(Class type, DBObject source) { + throw new UnsupportedOperationException(); + } + + @Override + public void write(Object source, DBObject sink) { + throw new UnsupportedOperationException(); + } + + @Override + public Object convertToMongoType(Object obj, TypeInformation typeInformation) { + throw new UnsupportedOperationException(); + } + + @Override + public DBRef toDBRef(Object object, MongoPersistentProperty referingProperty) { + throw new UnsupportedOperationException(); + } + } +}