Ensuring nullability contracts #1387
Conversation
scottf
commented
Aug 11, 2025
scottf
commented
- ConsumerInfo
- StreamInfo
- Unit tests reflect tightening
- Address IDE review notes
- ConsumerInfo - StreamInfo - Unit tests reflect tightening - Address IDE review notes
| @@ -17,7 +17,7 @@ | |||
| * Consume Options are provided to customize the consume operation. | |||
| */ | |||
| public class ConsumeOptions extends BaseConsumeOptions { | |||
| public static ConsumeOptions DEFAULT_CONSUME_OPTIONS = ConsumeOptions.builder().build(); | |||
| public static final ConsumeOptions DEFAULT_CONSUME_OPTIONS = ConsumeOptions.builder().build(); | |||
There was a problem hiding this comment.
Should have been final
| @@ -26,7 +26,7 @@ | |||
| * Fetch Consume Options are provided to customize the fetch operation. | |||
| */ | |||
| public class FetchConsumeOptions extends BaseConsumeOptions { | |||
| public static FetchConsumeOptions DEFAULT_FETCH_OPTIONS = FetchConsumeOptions.builder().build(); | |||
| public static final FetchConsumeOptions DEFAULT_FETCH_OPTIONS = FetchConsumeOptions.builder().build(); | |||
There was a problem hiding this comment.
Should have been final
| @@ -31,7 +32,7 @@ public abstract class ApiResponse<T> { | |||
| protected final JsonValue jv; | |||
|
|
|||
| private final String type; | |||
| private final Error error; | |||
| private Error error; | |||
There was a problem hiding this comment.
make this not final to support the subclass constructor setting it.
This is part of the effort to ensure that ConsumerInfo and StreamInfo can indicate they are an error if they don't parse correctly, because they can't throw an exception during construction, they didn't before.
| pushBound = readBoolean(jv, PUSH_BOUND); | ||
|
|
||
| timestamp = readDate(jv, TIMESTAMP); | ||
| if (hasError()) { |
There was a problem hiding this comment.
When it's an already error there is no need to parse, but since vars are final we do need to set them.
| } | ||
| else { | ||
| JsonValue jvConfig = nullValueIsError(jv, CONFIG, JsonValue.EMPTY_MAP) ; | ||
| configuration = ConsumerConfiguration.builder().jsonValue(jvConfig).build(); |
There was a problem hiding this comment.
can't be null, but turns the object into an error. This would never happen except with data from somewhere other than the server, and all this is to just match the nullability contracts. Same with stream/name/created below.
| sourceInfos = SourceInfo.optionalListOf(readValue(jv, SOURCES)); | ||
| alternates = StreamAlternate.optionalListOf(readValue(jv, ALTERNATES)); | ||
| timestamp = readDate(jv, TIMESTAMP); | ||
| if (hasError()) { |
There was a problem hiding this comment.
When it's an already error there is no need to parse, but since vars are final we do need to set them.
| JsonValue jvConfig = nullValueIsError(jv, CONFIG, JsonValue.NULL); | ||
| config = (jvConfig == JsonValue.NULL) | ||
| ? StreamConfiguration.builder().name(UNDEFINED).build() | ||
| : StreamConfiguration.instance(jvConfig); |
There was a problem hiding this comment.
can't be null, but turns the object into an error. This would never happen except with data from somewhere other than the server, and all this is to just match the nullability contracts. Same with create time below.
| @@ -41,7 +41,7 @@ public enum ManageResult {MESSAGE, STATUS_HANDLED, STATUS_TERMINUS, STATUS_ERROR | |||
|
|
|||
| protected long lastStreamSeq; | |||
| protected long lastConsumerSeq; | |||
| protected AtomicLong lastMsgReceivedNanoTime; | |||
| protected final AtomicLong lastMsgReceivedNanoTime; | |||
There was a problem hiding this comment.
should have been final
| deliverPolicy, | ||
| headersOnly, | ||
| fromRevision, | ||
| getHandler(watcher, !ignoreDeletes), |
There was a problem hiding this comment.
The ide suggested and then helped me refactor the handler to a method
| deliverPolicy, | ||
| headersOnly, | ||
| ULONG_UNSET, | ||
| getHandler(watcher, !ignoreDeletes), |
There was a problem hiding this comment.
The ide suggested and then helped me refactor the handler to a method
