Propagate CouchbaseCustomConverters bean into MappingConverter constructor

[mikereiche]

AbstractCouchbaseConfiguration

@Bean
public MappingCouchbaseConverter mappingCouchbaseConverter(CouchbaseMappingContext couchbaseMappingContext,
		CustomConversions couchbaseCustomConversions) {
	MappingCouchbaseConverter converter = new MappingCouchbaseConverter(couchbaseMappingContext, typeKey(), couchbaseCustomConversions);
	converter.setCustomConversions(couchbaseCustomConversions);
	couchbaseMappingContext.setSimpleTypeHolder(couchbaseCustomConversions.getSimpleTypeHolder());
	return converter;
}

MappingCouchbaseConverter

public MappingCouchbaseConverter(
		final MappingContext<? extends CouchbasePersistentEntity<?>, CouchbasePersistentProperty> appingContext,
		final String typeKey,
		final CustomConversions couchbaseCustomConversions) {
	super(new DefaultConversionService(), couchbaseCustomConversions);
	this.mappingContext = mappingContext;
...
	//CustomConversions customConversions = new CouchbaseCustomConversions(Collections.emptyList()); // DO NOT CREATE

AbstractCouchbaseConverter

protected CustomConversions conversions = null; // DO NOT CREATE new CouchbaseCustomConversions(Collections.emptyList());

...
protected AbstractCouchbaseConverter(final GenericConversionService conversionService, CustomConversions customConversions) {
	this.conversionService = conversionService;
	this.conversions = customConversions != null ? customConversions : null ; // null is error??
}

[bipoool]

Hey @mikereiche can I work on this? Or on any other issue?

[mikereiche]

sure.

[bipoool]

Please assign the issue to me.

[roman-sinyakov]

Is it gonna fix the problem after migration v4->5 when custom conversions in the overridden customConversions() method of the child class of AbstractCouchbaseConfiguration effectively hide embedded convertors?

[mikereiche]

Maybe - although I'm not aware of such a problem. Can you open a new issue for that?

[mikereiche]

I looked at this - and while it will make sense to pass couchbaseCustomConverters bean into the the constructor - where the constructor is called in AbstractCouchbaseConfiguration.mappingCouchbaseConverter(), couchbaseCustomConverters is applied after creation.

	@Bean
	public MappingCouchbaseConverter mappingCouchbaseConverter(CouchbaseMappingContext couchbaseMappingContext,
			CouchbaseCustomConversions couchbaseCustomConversions) {
		MappingCouchbaseConverter converter = new MappingCouchbaseConverter(couchbaseMappingContext, typeKey());
		converter.setCustomConversions(couchbaseCustomConversions); <--- applied here
		couchbaseMappingContext.setSimpleTypeHolder(couchbaseCustomConversions.getSimpleTypeHolder());
		return converter;
	}

[bipoool]

Yup, it's better to pass it in the constructor down to AbstractCouchbaseConverter's constructor.
Changes are done and will raise the PR after proper testing.

On a side note. Shouldn't we also change the documentation where we Configuring Multiple Buckets

As this example is using new MappingCouchbaseConverter() to create mappingConverter. Which will not have the couchbaseCustomConversions. And it won't be able to find converters for JsonNode and etc.

In my opinion, we should also give an example where we also set couchbaseCustomConversions in MappingCouchbaseConverter

Something like -

mappingCouchbaseConverter.setCustomConversions(couchbaseCustomConversions);
Or
MappingCouchbaseConverter mapper = context.getBean(MappingCouchbaseConverter.class);

[bipoool]

Hey, @mikereiche raised a PR for these changes.
Please review this when ever you get some time.

[bipoool]

Any Update here? @mikereiche