[fix][client] Add support for loading non-serializable properties with ConsumerBuilder::loadConf

[cbornet]

Motivation

Currently ConsumerBuilder::loadConf resets non-serializable of ConsumerConfigurationData to null.
It would be useful that ConsumerBuilder::loadConf supports loading non-serializable fields.

An alternative is to not modify fields not present in the property map in ConfigurationDataUtils::loadData as proposed by #13298 . This PR is for discussion.

Modifications

In ConsumerBuilder::loadConf, save values for each non-serializable property and apply it after calling ConfigurationDataUtils::loadData

Verifying this change

• Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

• Run ConsumerBuilderImplTest::testLoadConf and ConsumerBuilderImplTest::testLoadConfNotModified

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: cbornet#7

[eolivelli]

This patch adds value.
The only downside is that in case we add a new non serialisable field we will have to add it to this list

[michaeljmarshall]

I think this approach generally makes sense since something like a messageListener would not be expected to be "deep copied".

[michaeljmarshall]

Is it worth just making these two (and some of the others) serializable?

[cbornet]

The hardest part is making them work with Jackson. Not sure it's possible for RedeliveryBackoff

[onobc]

I believe RB can be serialized by configuring a mix-in (on the mapper) that provides a json creator constructor for RB. Not saying that it is ideal, just that it can be done.

[cbornet]

RB being an interface. I'm not sure it can be done. How to decide which impl to use ?

[onobc]

In that case we could use @JsonTypeInfo + @JsonSubTypes in a mix-in. This would work well as long as we know about all of the RB impls.

NOTE: Whatever we do for RB could apply to the ClientConfigurationData.authentication as it also currently ignored and is also an interface. There may be other fields as well.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.85%. Comparing base (545f33f) to head (98e6e37).
Report is 2145 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18344      +/-   ##
============================================
+ Coverage     43.96%   45.85%   +1.89%     
+ Complexity    10358    10117     -241     
============================================
  Files           757      697      -60     
  Lines         72773    68044    -4729     
  Branches       7818     7285     -533     
============================================
- Hits          31993    31203     -790     
+ Misses        37104    33259    -3845     
+ Partials       3676     3582      -94     

Flag	Coverage Δ
unittests	45.85% <100.00%> (+1.89%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...apache/pulsar/client/impl/ConsumerBuilderImpl.java	48.41% <100.00%> (+6.70%)	⬆️

... and 90 files with indirect coverage changes

[github-actions]

The pr had no activity for 30 days, mark with Stale label.