Use Optional return types instead of nullable boxed primitives in public API#200
Merged
Conversation
…th @JsonInclude(NON_NULL)" This reverts commit 8e0eeeb.
Contributor
There was a problem hiding this comment.
Pull request overview
Restores the Optional-returning API on mutable config classes (CopilotClientOptions, SessionConfig, etc.) that was partially reverted from #189, replacing nullable boxed-primitive getters with Optional<Boolean>, OptionalInt, and OptionalDouble, primitive setters, and new clearXxx() methods. Internal fields remain nullable boxed types and @JsonIgnore is applied to the Optional getters to keep Jackson serialization unaffected.
Changes:
- Convert ~30 public getters across 15 config/DTO classes to return Optional types, change setters to primitives, and add
clearXxx()methods. - Update all internal callers (
CliServerManager,CopilotClient,CopilotSession,SessionRequestBuilder) to use Optional APIs. - Add a new comprehensive
OptionalApiAndJacksonTest; update existing tests; removeJsonIncludeNonNullTestand (inconsistently) the@JsonInclude(NON_NULL)annotation from a subset of touched DTOs.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java | sessionIdleTimeoutSeconds/useLoggedInUser → Optional API + clear methods |
| src/main/java/com/github/copilot/sdk/json/SessionConfig.java | telemetry/discovery/sub-agent streaming getters → Optional |
| src/main/java/com/github/copilot/sdk/json/ResumeSessionConfig.java | Mirror of SessionConfig Optional changes |
| src/main/java/com/github/copilot/sdk/json/InfiniteSessionConfig.java | enabled / thresholds → Optional/OptionalDouble |
| src/main/java/com/github/copilot/sdk/json/InputOptions.java | min/maxLength → OptionalInt; removes class-level @JsonInclude(NON_NULL) |
| src/main/java/com/github/copilot/sdk/json/ModelCapabilitiesOverride.java | Supports/Limits inner classes → Optional/OptionalInt |
| src/main/java/com/github/copilot/sdk/json/ProviderConfig.java | maxPrompt/OutputTokens → OptionalInt |
| src/main/java/com/github/copilot/sdk/json/TelemetryConfig.java | captureContent → Optional; removes class-level @JsonInclude(NON_NULL) |
| src/main/java/com/github/copilot/sdk/json/SessionUiCapabilities.java | elicitation → Optional; removes @JsonInclude(NON_NULL); adds @JsonProperty |
| src/main/java/com/github/copilot/sdk/json/CustomAgentConfig.java | infer → Optional |
| src/main/java/com/github/copilot/sdk/json/UserInputRequest.java | allowFreeform → Optional; removes @JsonInclude(NON_NULL) |
| src/main/java/com/github/copilot/sdk/json/CreateSessionRequest.java | Wire DTO setters take primitives; clear methods added |
| src/main/java/com/github/copilot/sdk/json/ResumeSessionRequest.java | Wire DTO setters take primitives; clear methods added |
| src/main/java/com/github/copilot/sdk/CliServerManager.java | Adopt Optional APIs for useLoggedInUser/idle timeout/captureContent |
| src/main/java/com/github/copilot/sdk/CopilotClient.java | Use isPresent() for useLoggedInUser validation |
| src/main/java/com/github/copilot/sdk/CopilotSession.java | Use Optional APIs for elicitation check, input options, model capabilities |
| src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java | Use ifPresent to copy Optional values to wire DTOs |
| src/test/java/com/github/copilot/sdk/OptionalApiAndJacksonTest.java | New broad coverage for clearXxx, Optional getters, Jackson round-trips |
| src/test/java/com/github/copilot/sdk/JsonIncludeNonNullTest.java | Deleted; removes contract test for class-level @JsonInclude(NON_NULL) |
| src/test/java/com/github/copilot/sdk/CopilotClientTest.java | Update assertions to Optional API |
| src/test/java/com/github/copilot/sdk/ConfigCloneTest.java | Update clone/clear assertions to Optional API |
| src/test/java/com/github/copilot/sdk/DataObjectCoverageTest.java | Update to primitive setters / Optional getters |
| src/test/java/com/github/copilot/sdk/ElicitationTest.java | Update assertions for new Optional getters |
| src/test/java/com/github/copilot/sdk/ProviderConfigTest.java | Update to OptionalInt assertions |
| src/test/java/com/github/copilot/sdk/TelemetryConfigTest.java | Update to Optional assertions |
Copilot's findings
- Files reviewed: 25/25 changed files
- Comments generated: 1
…notations Revert commit 8e0eeeb which accidentally rolled back all Optional usage from PR #189. Restore Optional<Boolean>, OptionalInt, and OptionalDouble return types on public config getters, primitive setters, and clearXxx() methods. Additionally, add @JsonInclude(NON_NULL) and @JsonProperty annotations to classes that were missing them. --- Per-file changes --- src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java: Restore Optional<Boolean> for getUseLoggedInUser(), OptionalInt for getSessionIdleTimeoutSeconds(). Restore primitive setters and clearXxx() methods. Re-add @JsonIgnore on Optional-returning getters. src/main/java/com/github/copilot/sdk/json/SessionConfig.java: Restore Optional<Boolean> for getEnableSessionTelemetry(), getEnableConfigDiscovery(), getIncludeSubAgentStreamingEvents(). Restore primitive setters and clearXxx() methods. Re-add @JsonIgnore. src/main/java/com/github/copilot/sdk/json/ResumeSessionConfig.java: Same Optional pattern restored for getEnableSessionTelemetry(), getEnableConfigDiscovery(), getIncludeSubAgentStreamingEvents(). src/main/java/com/github/copilot/sdk/json/InfiniteSessionConfig.java: Restore Optional<Boolean> for getEnabled(), OptionalDouble for getBackgroundCompactionThreshold() and getBufferExhaustionThreshold(). src/main/java/com/github/copilot/sdk/json/InputOptions.java: Restore OptionalInt for getMinLength() and getMaxLength(). Add @JsonInclude(NON_NULL) to class. Add @JsonProperty on minLength and maxLength fields to ensure Jackson serialization works with @JsonIgnore on the OptionalInt getters. src/main/java/com/github/copilot/sdk/json/ModelCapabilitiesOverride.java: Restore Optional<Boolean> for Supports.getVision() and Supports.getReasoningEffort(). Restore OptionalInt for Limits.getMaxPromptTokens(), getMaxOutputTokens(), getMaxContextWindowTokens(). Restore clearXxx() methods. src/main/java/com/github/copilot/sdk/json/ProviderConfig.java: Restore OptionalInt for getMaxPromptTokens() and getMaxOutputTokens(). src/main/java/com/github/copilot/sdk/json/TelemetryConfig.java: Restore Optional<Boolean> for getCaptureContent(). Add @JsonInclude(NON_NULL) to class. src/main/java/com/github/copilot/sdk/json/SessionUiCapabilities.java: Restore Optional<Boolean> for getElicitation(). Add @JsonInclude(NON_NULL) to class. src/main/java/com/github/copilot/sdk/json/CustomAgentConfig.java: Restore Optional<Boolean> for getInfer(). src/main/java/com/github/copilot/sdk/json/UserInputRequest.java: Restore Optional<Boolean> for getAllowFreeform(). Add @JsonInclude(NON_NULL) to class. src/main/java/com/github/copilot/sdk/json/CreateSessionRequest.java: Restore primitive boolean setters and clearXxx() methods for wire DTO flags (enableSessionTelemetry, requestPermission, requestUserInput, hooks, streaming, enableConfigDiscovery, includeSubAgentStreamingEvents, requestElicitation). src/main/java/com/github/copilot/sdk/json/ResumeSessionRequest.java: Same primitive setter and clearXxx() restoration as CreateSessionRequest. src/main/java/com/github/copilot/sdk/CliServerManager.java: Update callers to use Optional APIs (.orElse(), .isPresent(), .getAsInt()) instead of null checks. src/main/java/com/github/copilot/sdk/CopilotClient.java: Update useLoggedInUser check to use .isPresent() instead of != null. src/main/java/com/github/copilot/sdk/CopilotSession.java: Update assertElicitation() to use .orElse(false). Update input() method to use .isPresent()/.getAsInt() for minLength/maxLength. Update setModel() to use .orElse(null) for vision/reasoningEffort. src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java: Update buildCreateRequest() and buildResumeRequest() to use .ifPresent() for enableSessionTelemetry, enableConfigDiscovery, includeSubAgentStreamingEvents. Restore conditional-set patterns for requestUserInput, hooks, streaming, disableResume. src/test/java/com/github/copilot/sdk/OptionalApiAndJacksonTest.java: Restore comprehensive test class with clearXxx() tests for all 12 public config classes and Jackson serialization roundtrip tests. src/test/java/com/github/copilot/sdk/JsonIncludeNonNullTest.java: New test class verifying @JsonInclude(NON_NULL) annotation presence on 11 DTO classes and serialization behavior (null fields omitted, set fields included) for InputOptions, TelemetryConfig, SessionUiCapabilities, and UserInputRequest. src/test/java/com/github/copilot/sdk/ConfigCloneTest.java: Update assertions to use Optional APIs (.orElse(), .isEmpty()). src/test/java/com/github/copilot/sdk/CopilotClientTest.java: Update assertions to use Optional APIs (.isEmpty(), Optional.of()). src/test/java/com/github/copilot/sdk/DataObjectCoverageTest.java: Update assertions to use Optional APIs (.orElse(), .getAsInt()). src/test/java/com/github/copilot/sdk/ElicitationTest.java: Update assertions to use Optional APIs (.get(), .getAsInt()). src/test/java/com/github/copilot/sdk/ProviderConfigTest.java: Update assertions to use OptionalInt APIs (.getAsInt(), .isEmpty()). src/test/java/com/github/copilot/sdk/TelemetryConfigTest.java: Update assertions to use Optional APIs (.get(), .isEmpty()).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Address the accidental reversion of important parts of the fix for #187 .
Supersedes #189