Skip to content

Commit 190e57d

Browse files
google-genai-botcopybara-github
google-genai-bot
authored and
copybara-github
committed
refactor: LlmAgent: Unwrap List from Optional, improve test coverage
PiperOrigin-RevId: 861717750
1 parent 0b63ca3 commit 190e57d

File tree

3 files changed

+60
-45
lines changed

3 files changed

+60
-45
lines changed

‎core/src/main/java/com/google/adk/agents/LlmAgent.java‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.adk.agents;
1818

1919
import static com.google.common.collect.ImmutableList.toImmutableList;
20+
import static java.util.Objects.requireNonNullElse;
2021
import static java.util.stream.Collectors.joining;
2122

2223
import com.fasterxml.jackson.core.JsonProcessingException;
@@ -103,12 +104,12 @@ public enum IncludeContents {
103104
private final Optional<Integer> maxSteps;
104105
private final boolean disallowTransferToParent;
105106
private final boolean disallowTransferToPeers;
106-
private final Optional<List<? extends BeforeModelCallback>> beforeModelCallback;
107-
private final Optional<List<? extends AfterModelCallback>> afterModelCallback;
108-
private final Optional<List<? extends OnModelErrorCallback>> onModelErrorCallback;
109-
private final Optional<List<? extends BeforeToolCallback>> beforeToolCallback;
110-
private final Optional<List<? extends AfterToolCallback>> afterToolCallback;
111-
private final Optional<List<? extends OnToolErrorCallback>> onToolErrorCallback;
107+
private final ImmutableList<? extends BeforeModelCallback> beforeModelCallback;
108+
private final ImmutableList<? extends AfterModelCallback> afterModelCallback;
109+
private final ImmutableList<? extends OnModelErrorCallback> onModelErrorCallback;
110+
private final ImmutableList<? extends BeforeToolCallback> beforeToolCallback;
111+
private final ImmutableList<? extends AfterToolCallback> afterToolCallback;
112+
private final ImmutableList<? extends OnToolErrorCallback> onToolErrorCallback;
112113
private final Optional<Schema> inputSchema;
113114
private final Optional<Schema> outputSchema;
114115
private final Optional<Executor> executor;
@@ -126,29 +127,28 @@ protected LlmAgent(Builder builder) {
126127
builder.beforeAgentCallback,
127128
builder.afterAgentCallback);
128129
this.model = Optional.ofNullable(builder.model);
129-
this.instruction =
130-
builder.instruction == null ? new Instruction.Static("") : builder.instruction;
130+
this.instruction = requireNonNullElse(builder.instruction, new Instruction.Static(""));
131131
this.globalInstruction =
132-
builder.globalInstruction == null ? new Instruction.Static("") : builder.globalInstruction;
132+
requireNonNullElse(builder.globalInstruction, new Instruction.Static(""));
133133
this.generateContentConfig = Optional.ofNullable(builder.generateContentConfig);
134134
this.exampleProvider = Optional.ofNullable(builder.exampleProvider);
135-
this.includeContents =
136-
builder.includeContents != null ? builder.includeContents : IncludeContents.DEFAULT;
135+
this.includeContents = requireNonNullElse(builder.includeContents, IncludeContents.DEFAULT);
137136
this.planning = builder.planning != null && builder.planning;
138137
this.maxSteps = Optional.ofNullable(builder.maxSteps);
139138
this.disallowTransferToParent = builder.disallowTransferToParent;
140139
this.disallowTransferToPeers = builder.disallowTransferToPeers;
141-
this.beforeModelCallback = Optional.ofNullable(builder.beforeModelCallback);
142-
this.afterModelCallback = Optional.ofNullable(builder.afterModelCallback);
143-
this.onModelErrorCallback = Optional.ofNullable(builder.onModelErrorCallback);
144-
this.beforeToolCallback = Optional.ofNullable(builder.beforeToolCallback);
145-
this.afterToolCallback = Optional.ofNullable(builder.afterToolCallback);
146-
this.onToolErrorCallback = Optional.ofNullable(builder.onToolErrorCallback);
140+
this.beforeModelCallback = requireNonNullElse(builder.beforeModelCallback, ImmutableList.of());
141+
this.afterModelCallback = requireNonNullElse(builder.afterModelCallback, ImmutableList.of());
142+
this.onModelErrorCallback =
143+
requireNonNullElse(builder.onModelErrorCallback, ImmutableList.of());
144+
this.beforeToolCallback = requireNonNullElse(builder.beforeToolCallback, ImmutableList.of());
145+
this.afterToolCallback = requireNonNullElse(builder.afterToolCallback, ImmutableList.of());
146+
this.onToolErrorCallback = requireNonNullElse(builder.onToolErrorCallback, ImmutableList.of());
147147
this.inputSchema = Optional.ofNullable(builder.inputSchema);
148148
this.outputSchema = Optional.ofNullable(builder.outputSchema);
149149
this.executor = Optional.ofNullable(builder.executor);
150150
this.outputKey = Optional.ofNullable(builder.outputKey);
151-
this.toolsUnion = builder.toolsUnion != null ? builder.toolsUnion : ImmutableList.of();
151+
this.toolsUnion = requireNonNullElse(builder.toolsUnion, ImmutableList.of());
152152
this.toolsets = extractToolsets(this.toolsUnion);
153153
this.codeExecutor = Optional.ofNullable(builder.codeExecutor);
154154

@@ -841,27 +841,27 @@ public boolean disallowTransferToPeers() {
841841
return disallowTransferToPeers;
842842
}
843843

844-
public Optional<List<? extends BeforeModelCallback>> beforeModelCallback() {
844+
public List<? extends BeforeModelCallback> beforeModelCallback() {
845845
return beforeModelCallback;
846846
}
847847

848-
public Optional<List<? extends AfterModelCallback>> afterModelCallback() {
848+
public List<? extends AfterModelCallback> afterModelCallback() {
849849
return afterModelCallback;
850850
}
851851

852-
public Optional<List<? extends BeforeToolCallback>> beforeToolCallback() {
852+
public List<? extends BeforeToolCallback> beforeToolCallback() {
853853
return beforeToolCallback;
854854
}
855855

856-
public Optional<List<? extends AfterToolCallback>> afterToolCallback() {
856+
public List<? extends AfterToolCallback> afterToolCallback() {
857857
return afterToolCallback;
858858
}
859859

860-
public Optional<List<? extends OnModelErrorCallback>> onModelErrorCallback() {
860+
public List<? extends OnModelErrorCallback> onModelErrorCallback() {
861861
return onModelErrorCallback;
862862
}
863863

864-
public Optional<List<? extends OnToolErrorCallback>> onToolErrorCallback() {
864+
public List<? extends OnToolErrorCallback> onToolErrorCallback() {
865865
return onToolErrorCallback;
866866
}
867867

@@ -871,7 +871,7 @@ public Optional<List<? extends OnToolErrorCallback>> onToolErrorCallback() {
871871
* <p>This method is only for use by Agent Development Kit.
872872
*/
873873
public List<? extends BeforeModelCallback> canonicalBeforeModelCallbacks() {
874-
return beforeModelCallback.orElse(ImmutableList.of());
874+
return beforeModelCallback;
875875
}
876876

877877
/**
@@ -880,7 +880,7 @@ public List<? extends BeforeModelCallback> canonicalBeforeModelCallbacks() {
880880
* <p>This method is only for use by Agent Development Kit.
881881
*/
882882
public List<? extends AfterModelCallback> canonicalAfterModelCallbacks() {
883-
return afterModelCallback.orElse(ImmutableList.of());
883+
return afterModelCallback;
884884
}
885885

886886
/**
@@ -889,7 +889,7 @@ public List<? extends AfterModelCallback> canonicalAfterModelCallbacks() {
889889
* <p>This method is only for use by Agent Development Kit.
890890
*/
891891
public List<? extends OnModelErrorCallback> canonicalOnModelErrorCallbacks() {
892-
return onModelErrorCallback.orElse(ImmutableList.of());
892+
return onModelErrorCallback;
893893
}
894894

895895
/**
@@ -898,7 +898,7 @@ public List<? extends OnModelErrorCallback> canonicalOnModelErrorCallbacks() {
898898
* <p>This method is only for use by Agent Development Kit.
899899
*/
900900
public List<? extends BeforeToolCallback> canonicalBeforeToolCallbacks() {
901-
return beforeToolCallback.orElse(ImmutableList.of());
901+
return beforeToolCallback;
902902
}
903903

904904
/**
@@ -907,7 +907,7 @@ public List<? extends BeforeToolCallback> canonicalBeforeToolCallbacks() {
907907
* <p>This method is only for use by Agent Development Kit.
908908
*/
909909
public List<? extends AfterToolCallback> canonicalAfterToolCallbacks() {
910-
return afterToolCallback.orElse(ImmutableList.of());
910+
return afterToolCallback;
911911
}
912912

913913
/**
@@ -916,7 +916,7 @@ public List<? extends AfterToolCallback> canonicalAfterToolCallbacks() {
916916
* <p>This method is only for use by Agent Development Kit.
917917
*/
918918
public List<? extends OnToolErrorCallback> canonicalOnToolErrorCallbacks() {
919-
return onToolErrorCallback.orElse(ImmutableList.of());
919+
return onToolErrorCallback;
920920
}
921921

922922
public Optional<Schema> inputSchema() {

‎core/src/test/java/com/google/adk/agents/ConfigAgentUtilsTest.java‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,20 +1161,25 @@ public void fromConfig_withConfiguredCallbacks_resolvesCallbacks()
11611161

11621162
String pfx = "test.callbacks.";
11631163
registry.register(
1164-
pfx + "before_agent_1", (Callbacks.BeforeAgentCallback) (ctx) -> Maybe.empty());
1164+
pfx + "before_agent_1", (Callbacks.BeforeAgentCallback) (unusedCtx) -> Maybe.empty());
11651165
registry.register(
1166-
pfx + "before_agent_2", (Callbacks.BeforeAgentCallback) (ctx) -> Maybe.empty());
1167-
registry.register(pfx + "after_agent_1", (Callbacks.AfterAgentCallback) (ctx) -> Maybe.empty());
1166+
pfx + "before_agent_2", (Callbacks.BeforeAgentCallback) (unusedCtx) -> Maybe.empty());
11681167
registry.register(
1169-
pfx + "before_model_1", (Callbacks.BeforeModelCallback) (ctx, req) -> Maybe.empty());
1168+
pfx + "after_agent_1", (Callbacks.AfterAgentCallback) (unusedCtx) -> Maybe.empty());
11701169
registry.register(
1171-
pfx + "after_model_1", (Callbacks.AfterModelCallback) (ctx, resp) -> Maybe.empty());
1170+
pfx + "before_model_1",
1171+
(Callbacks.BeforeModelCallback) (unusedCtx, unusedReq) -> Maybe.empty());
1172+
registry.register(
1173+
pfx + "after_model_1",
1174+
(Callbacks.AfterModelCallback) (unusedCtx, unusedResp) -> Maybe.empty());
11721175
registry.register(
11731176
pfx + "before_tool_1",
1174-
(Callbacks.BeforeToolCallback) (inv, tool, args, toolCtx) -> Maybe.empty());
1177+
(Callbacks.BeforeToolCallback)
1178+
(unusedInv, unusedTool, unusedArgs, unusedToolCtx) -> Maybe.empty());
11751179
registry.register(
11761180
pfx + "after_tool_1",
1177-
(Callbacks.AfterToolCallback) (inv, tool, args, toolCtx, resp) -> Maybe.empty());
1181+
(Callbacks.AfterToolCallback)
1182+
(unusedInv, unusedTool, unusedArgs, unusedToolCtx, unusedResp) -> Maybe.empty());
11781183

11791184
File configFile = tempFolder.newFile("with_callbacks.yaml");
11801185
Files.writeString(
@@ -1209,15 +1214,11 @@ public void fromConfig_withConfiguredCallbacks_resolvesCallbacks()
12091214
assertThat(agent.afterAgentCallback()).isPresent();
12101215
assertThat(agent.afterAgentCallback().get()).hasSize(1);
12111216

1212-
assertThat(llm.beforeModelCallback()).isPresent();
1213-
assertThat(llm.beforeModelCallback().get()).hasSize(1);
1214-
assertThat(llm.afterModelCallback()).isPresent();
1215-
assertThat(llm.afterModelCallback().get()).hasSize(1);
1217+
assertThat(llm.beforeModelCallback()).hasSize(1);
1218+
assertThat(llm.afterModelCallback()).hasSize(1);
12161219

1217-
assertThat(llm.beforeToolCallback()).isPresent();
1218-
assertThat(llm.beforeToolCallback().get()).hasSize(1);
1219-
assertThat(llm.afterToolCallback()).isPresent();
1220-
assertThat(llm.afterToolCallback().get()).hasSize(1);
1220+
assertThat(llm.beforeToolCallback()).hasSize(1);
1221+
assertThat(llm.afterToolCallback()).hasSize(1);
12211222
}
12221223

12231224
@Test

‎core/src/test/java/com/google/adk/agents/LlmAgentTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,13 @@ public void canonicalCallbacks_returnsEmptyListWhenNull() {
341341
assertThat(agent.canonicalBeforeToolCallbacks()).isEmpty();
342342
assertThat(agent.canonicalAfterToolCallbacks()).isEmpty();
343343
assertThat(agent.canonicalOnToolErrorCallbacks()).isEmpty();
344+
345+
assertThat(agent.beforeModelCallback()).isEmpty();
346+
assertThat(agent.afterModelCallback()).isEmpty();
347+
assertThat(agent.onModelErrorCallback()).isEmpty();
348+
assertThat(agent.beforeToolCallback()).isEmpty();
349+
assertThat(agent.afterToolCallback()).isEmpty();
350+
assertThat(agent.onToolErrorCallback()).isEmpty();
344351
}
345352

346353
@Test
@@ -371,5 +378,12 @@ public void canonicalCallbacks_returnsListWhenPresent() {
371378
assertThat(agent.canonicalBeforeToolCallbacks()).containsExactly(btc);
372379
assertThat(agent.canonicalAfterToolCallbacks()).containsExactly(atc);
373380
assertThat(agent.canonicalOnToolErrorCallbacks()).containsExactly(otec);
381+
382+
assertThat(agent.beforeModelCallback()).containsExactly(bmc);
383+
assertThat(agent.afterModelCallback()).containsExactly(amc);
384+
assertThat(agent.onModelErrorCallback()).containsExactly(omec);
385+
assertThat(agent.beforeToolCallback()).containsExactly(btc);
386+
assertThat(agent.afterToolCallback()).containsExactly(atc);
387+
assertThat(agent.onToolErrorCallback()).containsExactly(otec);
374388
}
375389
}

0 commit comments

Comments
 (0)