Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

In the `ConfigEntry` class, sometimes the `value` fields can be null`null`. However, the users might assume the fields will not be null.can easily assume it can't be and encounter NullPointerException since the type is not explicit. Thus we want to change its type to `Optional<String>` to make it explicit that it might not be set. The users can write code like `leaderConfig.value().length()` which throws a null pointer exception. 

Furthermore, in our implementation here the `AlterConfigsRequest.ConfigEntry` value is requires not be null, however there is nothing preventing `configEntry.value()` to not returning null. Similar cases can also be found in here.

Public Interfaces

We plan to change the `value` field of the class to be `Optional<String>` to make it explicitly to the caller that it might well be null. And change the `value()` method to return an `Optional<String>` as well.

public String value() {
return value;
}
=>
public Optional<String> value() {
return value;
}


Alternative we can keep the original `value` method(which should be marked as ) and add a new method

public Optional<String> valueOptional() {
return value;
}

Proposed Changes

We plan to change the type of the value field to be an `Optional<String>` which requires us to also change the getter, `equals` and `hashCode` method.methods.

private final String value; => private final Optional<String> value;


@Override
public boolean equals(Object o) {
if (this == o)
return true;
if (o == null || getClass() != o.getClass())
return false;

ConfigEntry that = (ConfigEntry) o;

return this.name.equals(that.name) &&
this.value.equals(that.value) &&
this.isSensitive == that.isSensitive &&
this.isReadOnly == that.isReadOnly &&
this.source == that.source &&
Objects.equals(this.synonyms, that.synonyms);
}

@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + name.hashCode();
result = prime * result + value.hashCode();
result = prime * result + (isSensitive ? 1 : 0);
result = prime * result + (isReadOnly ? 1 : 0);
result = prime * result + source.hashCode();
result = prime * result + synonyms.hashCode();
return result;
}

Compatibility, Deprecation, and Migration Plan

Various callers that uses the constructor and getter will need to be changed to account for the change in type.

Rejected Alternatives

We have also consider using a default sentinel value to represent value not set, e.g. "". However semantically, null and empty string is not always equivalent. For example we can have a configuration of bootstrap.servers which has the default value of "" and also with another configuration ssl.Key.Password with a default value of null.