Skip to content

Require explicit representation configuration for UUID, BigDecimal, and BigInteger #5044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
* Base class for Spring Data MongoDB to be extended for JavaConfiguration usage.
*
* @author Mark Paluch
* @author Hyunsang Han
* @since 2.0
*/
public abstract class MongoConfigurationSupport {
Expand Down Expand Up @@ -226,9 +227,15 @@ protected boolean autoIndexCreation() {
protected MongoClientSettings mongoClientSettings() {

MongoClientSettings.Builder builder = MongoClientSettings.builder();
builder.uuidRepresentation(UuidRepresentation.STANDARD);
configureClientSettings(builder);
return builder.build();

MongoClientSettings settings = builder.build();

if (settings.getUuidRepresentation() == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would break applications for users that do not use UUIDs.

throw new IllegalStateException("UUID representation must be explicitly configured.");
}

return settings;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import org.bson.UuidRepresentation;
import org.jspecify.annotations.Nullable;
import org.springframework.beans.factory.config.AbstractFactoryBean;
import org.springframework.dao.DataAccessException;
Expand Down Expand Up @@ -52,6 +51,7 @@
*
* @author Christoph Strobl
* @author Mark Paluch
* @author Hyunsang Han
*/
public class MongoClientFactoryBean extends AbstractFactoryBean<MongoClient> implements PersistenceExceptionTranslator {

Expand Down Expand Up @@ -162,7 +162,6 @@ protected MongoClientSettings computeClientSetting() {
getOrDefault(port, "" + ServerAddress.defaultPort())));

Builder builder = MongoClientSettings.builder().applyConnectionString(connectionString);
builder.uuidRepresentation(UuidRepresentation.STANDARD);

if (mongoClientSettings != null) {

Expand Down Expand Up @@ -305,7 +304,13 @@ protected MongoClientSettings computeClientSetting() {
});
}

return builder.build();
MongoClientSettings settings = builder.build();

if (settings.getUuidRepresentation() == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

throw new IllegalStateException("UUID representation must be explicitly configured.");
}

return settings;
}

private <T> void applySettings(Consumer<T> settingsBuilder, @Nullable T value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
*
* @author Mark Paluch
* @author Christoph Strobl
* @author Hyunsang Han
* @since 2.0
* @see org.springframework.data.convert.CustomConversions
* @see org.springframework.data.mapping.model.SimpleTypeHolder
Expand Down Expand Up @@ -159,7 +160,7 @@ public static class MongoConverterConfigurationAdapter {
private static final Set<Class<?>> JAVA_DRIVER_TIME_SIMPLE_TYPES = Set.of(LocalDate.class, LocalTime.class, LocalDateTime.class);

private boolean useNativeDriverJavaTimeCodecs = false;
private BigDecimalRepresentation bigDecimals = BigDecimalRepresentation.DECIMAL128;
private @Nullable BigDecimalRepresentation bigDecimals;
private final List<Object> customConverters = new ArrayList<>();

private final PropertyValueConversions internalValueConversion = PropertyValueConversions.simple(it -> {});
Expand Down Expand Up @@ -313,8 +314,8 @@ public MongoConverterConfigurationAdapter useSpringDataJavaTimeCodecs() {
}

/**
* Configures the representation to for {@link java.math.BigDecimal} and {@link java.math.BigInteger} values in
* MongoDB. Defaults to {@link BigDecimalRepresentation#DECIMAL128}.
* Configures the representation for {@link java.math.BigDecimal} and {@link java.math.BigInteger} values in
* MongoDB. This configuration is required and must be explicitly set.
*
* @param representation the representation to use.
* @return this.
Expand Down Expand Up @@ -375,6 +376,10 @@ ConverterConfiguration createConverterConfiguration() {
svc.init();
}

if (bigDecimals == null) {
throw new IllegalStateException("BigDecimal representation must be explicitly configured.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same breaking behavior as for UUIDs. We would also break applications for users that do not use big decimals.

}

List<Object> converters = new ArrayList<>(STORE_CONVERTERS.size() + 7);

if (bigDecimals == BigDecimalRepresentation.STRING) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
import org.springframework.data.spel.ExtensionAwareEvaluationContextProvider;
import org.springframework.test.util.ReflectionTestUtils;

import org.bson.UuidRepresentation;

import com.mongodb.MongoClientSettings;
import com.mongodb.client.MongoClient;

/**
Expand All @@ -53,6 +56,7 @@
* @author Oliver Gierke
* @author Thomas Darimont
* @author Mark Paluch
* @author Hyunsang Han
*/
public class AbstractMongoConfigurationUnitTests {

Expand Down Expand Up @@ -114,6 +118,40 @@ public void lifecycleCallbacksAreInvokedInAppropriateOrder() {
context.close();
}

@Test // GH-5037
public void requiresExplicitUuidRepresentationConfiguration() {

assertThatThrownBy(() -> {
AbstractMongoClientConfiguration config = new AbstractMongoClientConfiguration() {
@Override
protected String getDatabaseName() {
return "test";
}
};
config.mongoClientSettings();
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("UUID representation must be explicitly configured");
}

@Test // GH-5037
public void worksWithExplicitUuidRepresentationConfiguration() {

AbstractMongoClientConfiguration config = new AbstractMongoClientConfiguration() {
@Override
protected String getDatabaseName() {
return "test";
}

@Override
protected void configureClientSettings(MongoClientSettings.Builder builder) {
builder.uuidRepresentation(UuidRepresentation.STANDARD);
}
};

MongoClientSettings settings = config.mongoClientSettings();
assertThat(settings.getUuidRepresentation()).isEqualTo(UuidRepresentation.STANDARD);
}

@Test // DATAMONGO-725
public void shouldBeAbleToConfigureCustomTypeMapperViaJavaConfig() {

Expand Down Expand Up @@ -158,6 +196,11 @@ protected String getDatabaseName() {
return "database";
}

@Override
protected void configureClientSettings(MongoClientSettings.Builder builder) {
builder.uuidRepresentation(UuidRepresentation.STANDARD);
}

@Override
public MongoClient mongoClient() {
return Mockito.mock(MongoClient.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

import org.junit.jupiter.api.Test;

import org.bson.UuidRepresentation;

import com.mongodb.ConnectionString;
import com.mongodb.MongoClientSettings;
import com.mongodb.ServerAddress;
Expand All @@ -29,6 +31,7 @@
* Unit tests for {@link MongoClientFactoryBean}.
*
* @author Christoph Strobl
* @author Hyunsang Han
*/
class MongoClientFactoryBeanUnitTests {

Expand Down Expand Up @@ -89,4 +92,26 @@ void hostAndPortPlusConnectionStringError() {
factoryBean.setPort(27017);
assertThatExceptionOfType(IllegalStateException.class).isThrownBy(factoryBean::createInstance);
}

@Test // GH-5037
void requiresExplicitUuidRepresentationConfiguration() {

MongoClientFactoryBean factoryBean = new MongoClientFactoryBean();

assertThatThrownBy(factoryBean::computeClientSetting)
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("UUID representation must be explicitly configured");
}

@Test // GH-5037
void worksWithExplicitUuidRepresentationConfiguration() {

MongoClientFactoryBean factoryBean = new MongoClientFactoryBean();
factoryBean.setMongoClientSettings(
MongoClientSettings.builder().uuidRepresentation(UuidRepresentation.STANDARD).build());

MongoClientSettings settings = factoryBean.computeClientSetting();

assertThat(settings.getUuidRepresentation()).isEqualTo(UuidRepresentation.STANDARD);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@
* @author Roman Puchkovskiy
* @author Heesu Jung
* @author Julia Lee
* @author Hyunsang Han
*/
@ExtendWith(MockitoExtension.class)
class MappingMongoConverterUnitTests {
Expand All @@ -135,8 +136,10 @@ class MappingMongoConverterUnitTests {
@BeforeEach
void beforeEach() {

MongoCustomConversions conversions = new MongoCustomConversions(
Arrays.asList(new ByteBufferToDoubleHolderConverter()));
MongoCustomConversions conversions = MongoCustomConversions.create(config -> {
config.bigDecimal(MongoCustomConversions.BigDecimalRepresentation.DECIMAL128);
config.registerConverter(new ByteBufferToDoubleHolderConverter());
});

mappingContext = new MongoMappingContext();
mappingContext.setApplicationContext(context);
Expand Down Expand Up @@ -414,6 +417,30 @@ void readsClassWithBigDecimal() {
assertThat(result.collection.get(0)).isEqualTo(BigDecimal.valueOf(2.5d));
}

@Test // GH-5037
void requiresExplicitBigDecimalRepresentationConfiguration() {

assertThatThrownBy(() -> {
MongoCustomConversions customConversions = new MongoCustomConversions(Collections.emptyList());
MappingMongoConverter converter = new MappingMongoConverter(mock(DbRefResolver.class), mappingContext);
converter.setCustomConversions(customConversions);
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("BigDecimal representation must be explicitly configured");
}

@Test // GH-5037
void worksWithExplicitBigDecimalRepresentationConfiguration() {

MongoCustomConversions customConversions = MongoCustomConversions.create(config -> {
config.bigDecimal(MongoCustomConversions.BigDecimalRepresentation.DECIMAL128);
});

MappingMongoConverter converter = new MappingMongoConverter(mock(DbRefResolver.class), mappingContext);
converter.setCustomConversions(customConversions);
assertThat(converter).isNotNull();
assertThat(converter.getCustomConversions()).isEqualTo(customConversions);
}

@Test
void writesNestedCollectionsCorrectly() {

Expand Down Expand Up @@ -3407,6 +3434,26 @@ void usesStringNumericFormat() {
assertThat(document).containsEntry("map.foo", "2.5");
}

@Test // GH-5037
void requiresExplicitConfigurationForBothBigDecimalAndUuid() {

MongoCustomConversions conversions = MongoCustomConversions.create(
it -> it.registerConverter(new ByteBufferToDoubleHolderConverter())
.bigDecimal(MongoCustomConversions.BigDecimalRepresentation.DECIMAL128));

assertThat(conversions).isNotNull();

assertThatThrownBy(() -> {
new MongoCustomConversions();
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("BigDecimal representation must be explicitly configured");

assertThatThrownBy(() -> {
MongoCustomConversions.create(it -> it.registerConverter(new ByteBufferToDoubleHolderConverter()));
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("BigDecimal representation must be explicitly configured");
}

private MappingMongoConverter createConverter(
MongoCustomConversions.BigDecimalRepresentation bigDecimalRepresentation) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,24 @@
import org.springframework.data.convert.PropertyValueConverter;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mongodb.core.convert.MongoCustomConversions.BigDecimalRepresentation;
import org.springframework.data.mongodb.core.convert.QueryMapperUnitTests.Foo;

/**
* Unit tests for {@link MongoCustomConversions}.
*
* @author Christoph Strobl
* @author Hyunsang Han
*/
class MongoCustomConversionsUnitTests {

@Test // DATAMONGO-2349
void nonAnnotatedConverterForJavaTimeTypeShouldOnlyBeRegisteredAsReadingConverter() {

MongoCustomConversions conversions = new MongoCustomConversions(
Collections.singletonList(new DateToZonedDateTimeConverter()));
MongoCustomConversions conversions = MongoCustomConversions.create(config -> {
config.bigDecimal(BigDecimalRepresentation.DECIMAL128);
config.registerConverter(new DateToZonedDateTimeConverter());
});

assertThat(conversions.hasCustomReadTarget(Date.class, ZonedDateTime.class)).isTrue();
assertThat(conversions.hasCustomWriteTarget(Date.class)).isFalse();
Expand All @@ -56,7 +60,7 @@ void propertyValueConverterRegistrationWorksAsExpected() {
when(owner.getType()).thenReturn(Foo.class);

MongoCustomConversions conversions = MongoCustomConversions.create(config -> {

config.bigDecimal(BigDecimalRepresentation.DECIMAL128);
config.configurePropertyConversions(
registry -> registry.registerConverter(Foo.class, "name", mock(PropertyValueConverter.class)));
});
Expand All @@ -68,12 +72,36 @@ void propertyValueConverterRegistrationWorksAsExpected() {
void doesNotReturnConverterForNativeTimeTimeIfUsingDriverCodec() {

MongoCustomConversions conversions = MongoCustomConversions.create(config -> {
config.bigDecimal(BigDecimalRepresentation.DECIMAL128);
config.useNativeDriverJavaTimeCodecs();
});

assertThat(conversions.getCustomWriteTarget(Date.class)).isEmpty();
}

@Test // GH-5037
void requiresExplicitBigDecimalRepresentationConfiguration() {

assertThatThrownBy(() -> {
MongoCustomConversions.create(config -> {
config.useSpringDataJavaTimeCodecs();
});
}).isInstanceOf(IllegalStateException.class)
.hasMessageContaining("BigDecimal representation must be explicitly configured");
}

@Test // GH-5037
void worksWithExplicitBigDecimalRepresentationConfiguration() {

MongoCustomConversions conversions = MongoCustomConversions.create(config -> {
config.bigDecimal(BigDecimalRepresentation.DECIMAL128);
config.useSpringDataJavaTimeCodecs();
});

assertThat(conversions).isNotNull();
assertThat(conversions.getSimpleTypeHolder()).isNotNull();
}

static class DateToZonedDateTimeConverter implements Converter<Date, ZonedDateTime> {

@Override
Expand Down