Add PropertyMapper.to(...) API designed for immutable instances

[onobc]

Overview

This proposal adds an API to PropertyMapper.isInstance that returns a user-specified default instance when the mapped value has been filtered. The current behavior is to throw an exception when the mapped value has been filtered.

Motivation

Some target objects are immutable and return a new instance w/ the mapped value rather than updating the value on the target instance. Consider the following example:

class Target {

  private String foo;

  Target(String foo) {
     this.foo = foo;
  }
  
  Target updateFoo(String foo) {
    return new Target(foo);
  }
}

And suppose we have some config prop for "foo" and we want to update the target object's "foo" w/ that value. Typically we would use something like:

PropertyMapper.get().from(someProps::foo).to(someTarget::updateFoo);

However, Source.to() does not work in the immutable case because updateFoo returns the new updated instance. Ok, lets instead use Source.toInstance() such as:

someTarget =  PropertyMapper.get().from(someProps::foo).toInstance(someTarget::updateFoo);

This works fine when the value is not filtered. However, if we add some constraints on the mapper such as:

someTarget =  PropertyMapper.get().from(someProps::foo).whenNonNull().toInstance(someTarget::updateFoo);

throws a NoSuchElementException when someProps::foo is null. This results in having to test the filter predicate manually prior to calling toInstance.

Proposed Solution

Allow the caller to pass in a default instance to use when the value is filtered (rather than throw exception). Something like this:

someTarget =  PropertyMapper.get().from(someProps::foo).whenNonNull().toInstance(someTarget::updateFoo, someTarget);

You can see where I have worked around this in https://github.com/spring-projects/spring-boot/pull/31258/files#diff-7f2cc68fbde121f72bcff98a44dc29738be96efb911ffc54b80b6a6dd334b5ccR60

[philwebb]

I wonder if we should have something like toOptionalInstance(factory) that returns an Optional result? That way we get all the orElse methods for free:

someTarget =  PropertyMapper.get().from(someProps::foo).whenNonNull().toOptionalInstance(someTarget::updateFoo).orElse(someTarget);

[onobc]

I wonder if we should have something like toOptionalInstance(factory) that returns an Optional result? That way we get all the orElse methods for free:

someTarget =  PropertyMapper.get().from(someProps::foo).whenNonNull().toOptionalInstance(someTarget::updateFoo).orElse(someTarget);

Absolutely! This gets us out of the "picking what to do in the case it is filtered value" business. No need to pass in default etc.. I will amend the PR shortly.

[philwebb]

Thanks for all the reviews. After a bit more discussion with @snicoll we decided to circle back to almost the original proposal. Rather than adding a toInstance method, I've added another to method and made it accept a BiFunction. This means you can write the immutable mapper code like this:

SenderOptions<K, V> senderOptions = SenderOptions.create(this.kafkaProperties.buildReactorProducerProperties());
KafkaProperties.Reactor.Sender senderProperties = this.kafkaProperties.getReactor().getSender();
PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull();
senderOptions = map.from(senderProperties::getCloseTimeout).to(senderOptions, SenderOptions::closeTimeout);
senderOptions = map.from(senderProperties::getMaxInFlight).to(senderOptions, SenderOptions::maxInFlight);
senderOptions = map.from(senderProperties::isStopOnError).to(senderOptions, SenderOptions::stopOnError);
return senderOptions;