Skip to content

Commit 66f4dd9

Browse files
temporal-spring-ai: wrap nullable fields/params in Optional<>
Addresses review feedback (thread on #2852) preferring typesafe Optional<T> over nullable fields and raw null delegation. Three sites changed: - ActivityChatModel: modelName and baseOptions fields + private ctor params are now Optional<String> / Optional<ActivityOptions>. getModelName() returns Optional<String>. Public ctors and factories still accept nullable String modelName at the API boundary (matches prior javadoc: "null for default"); they normalize via Optional.ofNullable before storing. Internal readers use .map / .orElse instead of null checks. - ActivityMcpClient: the 3-arg callTool's summary parameter is now Optional<String>. The 2-arg convenience overload passes Optional.empty(). The rebuild-with-summary branch uses .isPresent() + .get() instead of null checks. - McpToolCallback.call(...) wraps its generated summary string in Optional.of(...) before passing to callTool. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d3351a3 commit 66f4dd9

3 files changed

Lines changed: 52 additions & 38 deletions

File tree

temporal-spring-ai/src/main/java/io/temporal/springai/mcp/ActivityMcpClient.java

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import io.temporal.workflow.Workflow;
77
import java.time.Duration;
88
import java.util.Map;
9+
import java.util.Optional;
910

1011
/**
1112
* A workflow-safe wrapper for MCP (Model Context Protocol) client operations.
@@ -48,7 +49,7 @@ public class ActivityMcpClient {
4849
public static final int DEFAULT_MAX_ATTEMPTS = 3;
4950

5051
private final McpClientActivity activity;
51-
private final ActivityOptions baseOptions;
52+
private final Optional<ActivityOptions> baseOptions;
5253
private Map<String, McpSchema.ServerCapabilities> serverCapabilities;
5354
private Map<String, McpSchema.Implementation> clientInfo;
5455

@@ -58,15 +59,16 @@ public class ActivityMcpClient {
5859
* @param activity the activity stub for MCP operations
5960
*/
6061
public ActivityMcpClient(McpClientActivity activity) {
61-
this(activity, null);
62+
this(activity, Optional.empty());
6263
}
6364

6465
/**
65-
* Creates a new ActivityMcpClient. When {@code baseOptions} is non-null, {@link #callTool(String,
66+
* Creates a new ActivityMcpClient. When {@code baseOptions} is present, {@link #callTool(String,
6667
* McpSchema.CallToolRequest, String)} rebuilds the activity stub with a per-call Summary on top
67-
* of those options.
68+
* of those options. When empty, the caller supplied a pre-built stub whose options we don't know,
69+
* so we call through it as-is and drop any requested summary.
6870
*/
69-
private ActivityMcpClient(McpClientActivity activity, ActivityOptions baseOptions) {
71+
private ActivityMcpClient(McpClientActivity activity, Optional<ActivityOptions> baseOptions) {
7072
this.activity = activity;
7173
this.baseOptions = baseOptions;
7274
}
@@ -98,7 +100,7 @@ public static ActivityMcpClient create(Duration timeout, int maxAttempts) {
98100
.setRetryOptions(RetryOptions.newBuilder().setMaximumAttempts(maxAttempts).build())
99101
.build();
100102
McpClientActivity activity = Workflow.newActivityStub(McpClientActivity.class, options);
101-
return new ActivityMcpClient(activity, options);
103+
return new ActivityMcpClient(activity, Optional.of(options));
102104
}
103105

104106
/**
@@ -137,7 +139,7 @@ public Map<String, McpSchema.Implementation> getClientInfo() {
137139
* @return the tool call result
138140
*/
139141
public McpSchema.CallToolResult callTool(String clientName, McpSchema.CallToolRequest request) {
140-
return callTool(clientName, request, null);
142+
return callTool(clientName, request, Optional.empty());
141143
}
142144

143145
/**
@@ -148,19 +150,22 @@ public McpSchema.CallToolResult callTool(String clientName, McpSchema.CallToolRe
148150
*
149151
* @param clientName the name of the MCP client
150152
* @param request the tool call request
151-
* @param summary the activity Summary, or null to omit
153+
* @param summary the activity Summary, or empty to omit
152154
* @return the tool call result
153155
*/
154156
public McpSchema.CallToolResult callTool(
155-
String clientName, McpSchema.CallToolRequest request, String summary) {
156-
if (summary == null || baseOptions == null) {
157-
return activity.callTool(clientName, request);
157+
String clientName, McpSchema.CallToolRequest request, Optional<String> summary) {
158+
// Overlay the summary onto a fresh stub only when both a summary is requested AND we have
159+
// a recipe to rebuild the stub from (baseOptions). If either is missing, fall through to
160+
// the cached activity — it already has baseOptions baked in if we knew them at construction.
161+
if (summary.isPresent() && baseOptions.isPresent()) {
162+
McpClientActivity stub =
163+
Workflow.newActivityStub(
164+
McpClientActivity.class,
165+
ActivityOptions.newBuilder(baseOptions.get()).setSummary(summary.get()).build());
166+
return stub.callTool(clientName, request);
158167
}
159-
McpClientActivity stub =
160-
Workflow.newActivityStub(
161-
McpClientActivity.class,
162-
ActivityOptions.newBuilder(baseOptions).setSummary(summary).build());
163-
return stub.callTool(clientName, request);
168+
return activity.callTool(clientName, request);
164169
}
165170

166171
/**

temporal-spring-ai/src/main/java/io/temporal/springai/mcp/McpToolCallback.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import io.modelcontextprotocol.spec.McpSchema;
44
import java.util.List;
55
import java.util.Map;
6+
import java.util.Optional;
67
import org.springframework.ai.mcp.McpToolUtils;
78
import org.springframework.ai.model.ModelOptionsUtils;
89
import org.springframework.ai.tool.ToolCallback;
@@ -106,7 +107,7 @@ public String call(String toolInput) {
106107

107108
// Use the original tool name (not prefixed) when calling the MCP server
108109
McpSchema.CallToolRequest request = new McpSchema.CallToolRequest(tool.name(), arguments);
109-
String summary = "mcp: " + clientName + "." + tool.name();
110+
Optional<String> summary = Optional.of("mcp: " + clientName + "." + tool.name());
110111
McpSchema.CallToolResult result = client.callTool(clientName, request, summary);
111112

112113
// Return the result as-is (including errors) so the AI can handle them.

temporal-spring-ai/src/main/java/io/temporal/springai/model/ActivityChatModel.java

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.time.Duration;
1010
import java.util.List;
1111
import java.util.Map;
12+
import java.util.Optional;
1213
import java.util.stream.Collectors;
1314
import org.springframework.ai.chat.messages.*;
1415
import org.springframework.ai.chat.metadata.ChatResponseMetadata;
@@ -84,8 +85,8 @@ public class ActivityChatModel implements ChatModel {
8485
public static final int DEFAULT_MAX_ATTEMPTS = 3;
8586

8687
private final ChatModelActivity chatModelActivity;
87-
private final String modelName;
88-
private final ActivityOptions baseOptions;
88+
private final Optional<String> modelName;
89+
private final Optional<ActivityOptions> baseOptions;
8990
private final ToolCallingManager toolCallingManager;
9091
private final ToolExecutionEligibilityPredicate toolExecutionEligibilityPredicate;
9192

@@ -95,7 +96,7 @@ public class ActivityChatModel implements ChatModel {
9596
* @param chatModelActivity the activity stub for calling the chat model
9697
*/
9798
public ActivityChatModel(ChatModelActivity chatModelActivity) {
98-
this(chatModelActivity, null, null);
99+
this(chatModelActivity, Optional.empty(), Optional.empty());
99100
}
100101

101102
/**
@@ -105,16 +106,20 @@ public ActivityChatModel(ChatModelActivity chatModelActivity) {
105106
* @param modelName the name of the chat model to use, or null for default
106107
*/
107108
public ActivityChatModel(ChatModelActivity chatModelActivity, String modelName) {
108-
this(chatModelActivity, modelName, null);
109+
this(chatModelActivity, Optional.ofNullable(modelName), Optional.empty());
109110
}
110111

111112
/**
112113
* Internal constructor used by {@link #forModel(String, Duration, int)} and friends. When {@code
113-
* baseOptions} is non-null, each call rebuilds the activity stub with a per-call Summary on top
114-
* of those options so the Temporal UI can label the chat activity meaningfully.
114+
* baseOptions} is present, each call rebuilds the activity stub with a per-call Summary on top of
115+
* those options so the Temporal UI can label the chat activity meaningfully. When empty, the
116+
* caller supplied a pre-built stub whose options we don't know, so we call through it as-is
117+
* without a summary.
115118
*/
116119
private ActivityChatModel(
117-
ChatModelActivity chatModelActivity, String modelName, ActivityOptions baseOptions) {
120+
ChatModelActivity chatModelActivity,
121+
Optional<String> modelName,
122+
Optional<ActivityOptions> baseOptions) {
118123
this.chatModelActivity = chatModelActivity;
119124
this.modelName = modelName;
120125
this.baseOptions = baseOptions;
@@ -169,15 +174,14 @@ public static ActivityChatModel forModel(String modelName, Duration timeout, int
169174
.setRetryOptions(RetryOptions.newBuilder().setMaximumAttempts(maxAttempts).build())
170175
.build();
171176
ChatModelActivity activity = Workflow.newActivityStub(ChatModelActivity.class, options);
172-
return new ActivityChatModel(activity, modelName, options);
177+
return new ActivityChatModel(activity, Optional.ofNullable(modelName), Optional.of(options));
173178
}
174179

175180
/**
176-
* Returns the name of the chat model this instance uses.
177-
*
178-
* @return the model name, or null if using the default model
181+
* Returns the name of the chat model this instance uses, or empty if it uses the plugin default
182+
* (the {@code @Primary} {@code ChatModel} bean or the first one registered).
179183
*/
180-
public String getModelName() {
184+
public Optional<String> getModelName() {
181185
return modelName;
182186
}
183187

@@ -232,12 +236,14 @@ private ChatResponse internalCall(Prompt prompt) {
232236
}
233237

234238
private ChatModelActivity stubForCall(Prompt prompt) {
235-
if (baseOptions == null) {
236-
return chatModelActivity;
237-
}
238-
ActivityOptions withSummary =
239-
ActivityOptions.newBuilder(baseOptions).setSummary(buildSummary()).build();
240-
return Workflow.newActivityStub(ChatModelActivity.class, withSummary);
239+
return baseOptions
240+
.map(
241+
base -> {
242+
ActivityOptions withSummary =
243+
ActivityOptions.newBuilder(base).setSummary(buildSummary()).build();
244+
return Workflow.newActivityStub(ChatModelActivity.class, withSummary);
245+
})
246+
.orElse(chatModelActivity);
241247
}
242248

243249
/**
@@ -247,8 +253,7 @@ private ChatModelActivity stubForCall(Prompt prompt) {
247253
* observability label.
248254
*/
249255
private String buildSummary() {
250-
String label = modelName != null ? modelName : "default";
251-
return "chat: " + label;
256+
return "chat: " + modelName.orElse("default");
252257
}
253258

254259
private ChatModelTypes.ChatModelActivityInput createActivityInput(Prompt prompt) {
@@ -290,7 +295,10 @@ private ChatModelTypes.ChatModelActivityInput createActivityInput(Prompt prompt)
290295
}
291296
}
292297

293-
return new ChatModelTypes.ChatModelActivityInput(modelName, messages, modelOptions, tools);
298+
// The serialized record field is String (null = use activity-side default model), so unwrap
299+
// at the serialization boundary. Within this class we keep modelName as Optional<String>.
300+
return new ChatModelTypes.ChatModelActivityInput(
301+
modelName.orElse(null), messages, modelOptions, tools);
294302
}
295303

296304
private List<ChatModelTypes.Message> toActivityMessages(Message message) {

0 commit comments

Comments
 (0)