Status

Current state: Draft

Discussion thread: here [Change the link from the KIP proposal email archive to your own email thread]

JIRA: here [Change the link from KAFKA-1 to your own ticket]

Please keep the discussion on the mailing list rather than commenting on the wiki (wiki discussions get unwieldy fast).

Motivation

In the `ConfigEntry` class, the `value` fields can be `null`. However, the users 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` 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.