[pinot-client] JsonAsyncHttpPinotClientTransportFactory builder pattern is implemented incorrectly for scheme

[cyrilou242]

Issue:

In JsonAsyncHttpPinotClientTransportFactory, setScheme followed by withConnectionProperties may override the scheme value.

To reproduce:

public static void main(String[] args) throws NoSuchFieldException, IllegalAccessException {
    JsonAsyncHttpPinotClientTransportFactory factory = new JsonAsyncHttpPinotClientTransportFactory();
    // using reflection to print the _scheme field
    final Field field = factory.getClass().getDeclaredField("_scheme");
    field.setAccessible(true);
    
    System.out.println(field.get(factory)); 
    // prints http
    // OK, http is the default value of the builder
    
    factory.setScheme("https");
    System.out.println(field.get(factory)); 
    // prints https
    // OK, the setter works
    
    // any set of properties that does not have a value for "scheme"
    factory = factory.withConnectionProperties(new Properties());
    System.out.println(field.get(factory));
    // prints http
    // NOT OK - scheme value was overridden
  }

Cause:

The issue was introduced in
https://github.com/apache/pinot/pull/12332/files
The bug was pointed by the reviewer. The PR was still merged somehow.

To mitigate:

Users of JsonAsyncHttpPinotClientTransportFactory should not use setScheme.
They can set the scheme value with a Property in the meantime

final Properties properties = new Properties();
properties.setProperty("scheme", "https");

To fix:

Implement the logic correctly

Option 1:
prevent null in the _scheme setter, and annotate _scheme as non null
Then

_scheme = properties.getProperty("scheme", _scheme);

Option 2:
keep _scheme nullable (I really don't see a use case for this though)

_scheme = optional(properties.getProperty("scheme", _scheme)).orElse(CommonConstants.HTTP_PROTOCOL);

If you're ok I can create the PR.

[Jackie-Jiang]

@KKcorps Please take a look