Default behavior for BigDecimal changed and broke custom converters

[zeapo]

Since the commit 2b6730d any WritingConverters for BigDecimal and BigInteger is no longer taken in priority;

I understand that we can use the previous behavior by changing the BigDecimalRepresentation to DECIMAL128; however this is an explicit value that we have to set, otherwise any previously defined converter is no longer a priority. Which is a breaking change, as the upgrade documentation does not mention is new behavior.

Spring Data Commons take the first candidate that matches the source type if no explicit target type is defined. However, since the afore mentioned commit, Spring Data MongoDB registers its converters within the users' converters:

if (bigDecimals == BigDecimalRepresentation.STRING) {
	converters.add(BigDecimalToStringConverter.INSTANCE);
	converters.add(StringToBigDecimalConverter.INSTANCE);
	converters.add(BigIntegerToStringConverter.INSTANCE);
	converters.add(StringToBigIntegerConverter.INSTANCE);
}

if (!useNativeDriverJavaTimeCodecs) {

	converters.addAll(customConverters);
	return new ConverterConfiguration(STORE_CONVERSIONS, converters, convertiblePair -> true,
			this.propertyValueConversions);
}

The behavior should be similar to what is done for the datetimes, the code should look something like (from my limited understanding of the way this works):

private static final Set<Class<?>> BIG_DECIMAL_SIMPLE_TYPES = Set.of(BigDecimal.class, BigInteger.class);
....

if (!useNativeDriverJavaTimeCodecs) {
	StoreConversions storeConversions = StoreConversions
			.of(new SimpleTypeHolder(BIG_DECIMAL_SIMPLE_TYPES, MongoSimpleTypes.HOLDER), converters);

	return new ConverterConfiguration(storeConversions, this.customConverters, convertiblePair -> true,
			this.propertyValueConversions);
}

[mp911de]

Our release notes mention the updated default behavior. You should be able to switch back to the String-based variant to restore the previous behavior.

Do you happen to have a minimal reproducer handy so that we can verify what you would like to see fixed?

[zeapo]

We don't mind that the default behavior be updated; the issue is that the change makes it ignore any previously defined converter for the BigDecimal;

Given this existing code:

  @WritingConverter
  class BigIntegerWriteConverter : Converter<BigInteger, Number>
...
    @WritingConverter
    class BigDecimalWriteConverter : Converter<BigDecimal, Decimal128> 
....
  @Bean
  fun customConversions() =
      MongoCustomConversions.create {
          it.registerConverters(
              listOf(
                  BigIntegerWriteConverter(),
                  BigDecimalWriteConverter()
              )
          )
      }

The String-based variant takes precedence over our own converters; which in our opinion should not happen at all, regardless of the newly defined behavior.

Previous version, notice that our custom converters are at the top

New version, the new bigdecimal converters take precedence

I hope this clarifies the issue.

By setting the representation to decimal128, it does fix the issue; but this is unexpected, as we'd think that our converters would always be first.