Skip to content

Commit fe90693

Browse files
google-genai-botcopybara-github
authored andcommitted
refactor: VertexAiSearchTool: Unwrap List from Optional, improve test coverage
PiperOrigin-RevId: 860144181
1 parent 2f5769d commit fe90693

File tree

2 files changed

+79
-20
lines changed

2 files changed

+79
-20
lines changed

core/src/main/java/com/google/adk/tools/VertexAiSearchTool.java

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.google.genai.types.VertexAISearch;
2626
import com.google.genai.types.VertexAISearchDataStoreSpec;
2727
import io.reactivex.rxjava3.core.Completable;
28-
import java.util.ArrayList;
2928
import java.util.List;
3029
import java.util.Optional;
3130

@@ -39,7 +38,7 @@
3938
public abstract class VertexAiSearchTool extends BaseTool {
4039
public abstract Optional<String> dataStoreId();
4140

42-
public abstract Optional<List<VertexAISearchDataStoreSpec>> dataStoreSpecs();
41+
public abstract ImmutableList<VertexAISearchDataStoreSpec> dataStoreSpecs();
4342

4443
public abstract Optional<String> searchEngineId();
4544

@@ -54,7 +53,7 @@ public abstract class VertexAiSearchTool extends BaseTool {
5453
public abstract Optional<String> dataStore();
5554

5655
public static Builder builder() {
57-
return new AutoValue_VertexAiSearchTool.Builder();
56+
return new AutoValue_VertexAiSearchTool.Builder().dataStoreSpecs(ImmutableList.of());
5857
}
5958

6059
VertexAiSearchTool() {
@@ -80,24 +79,29 @@ public Completable processLlmRequest(
8079
searchEngineId().ifPresent(vertexAiSearchBuilder::engine);
8180
filter().ifPresent(vertexAiSearchBuilder::filter);
8281
maxResults().ifPresent(vertexAiSearchBuilder::maxResults);
83-
dataStoreSpecs().ifPresent(vertexAiSearchBuilder::dataStoreSpecs);
82+
if (!dataStoreSpecs().isEmpty()) {
83+
vertexAiSearchBuilder.dataStoreSpecs(dataStoreSpecs());
84+
}
8485

8586
Tool retrievalTool =
8687
Tool.builder()
8788
.retrieval(Retrieval.builder().vertexAiSearch(vertexAiSearchBuilder.build()).build())
8889
.build();
89-
90-
List<Tool> currentTools =
91-
new ArrayList<>(
92-
llmRequest.config().flatMap(GenerateContentConfig::tools).orElse(ImmutableList.of()));
93-
currentTools.add(retrievalTool);
94-
90+
ImmutableList<Tool> tools =
91+
ImmutableList.<Tool>builder()
92+
.addAll(
93+
llmRequest
94+
.config()
95+
.flatMap(GenerateContentConfig::tools)
96+
.orElse(ImmutableList.of()))
97+
.add(retrievalTool)
98+
.build();
9599
GenerateContentConfig newConfig =
96100
llmRequest
97101
.config()
98102
.map(GenerateContentConfig::toBuilder)
99103
.orElse(GenerateContentConfig.builder())
100-
.tools(ImmutableList.copyOf(currentTools))
104+
.tools(tools)
101105
.build();
102106
llmRequestBuilder.config(newConfig);
103107
return Completable.complete();
@@ -126,14 +130,18 @@ public abstract static class Builder {
126130

127131
public final VertexAiSearchTool build() {
128132
VertexAiSearchTool tool = autoBuild();
129-
if ((tool.dataStoreId().isEmpty() && tool.searchEngineId().isEmpty())
130-
|| (tool.dataStoreId().isPresent() && tool.searchEngineId().isPresent())) {
133+
boolean hasDataStoreId =
134+
tool.dataStoreId().isPresent() && !tool.dataStoreId().get().isEmpty();
135+
boolean hasSearchEngineId =
136+
tool.searchEngineId().isPresent() && !tool.searchEngineId().get().isEmpty();
137+
if (hasDataStoreId == hasSearchEngineId) {
131138
throw new IllegalArgumentException(
132-
"Either dataStoreId or searchEngineId must be specified.");
139+
"One and only one of dataStoreId or searchEngineId must not be empty.");
133140
}
134-
if (tool.dataStoreSpecs().isPresent() && tool.searchEngineId().isEmpty()) {
141+
boolean hasDataStoreSpecs = !tool.dataStoreSpecs().isEmpty();
142+
if (hasDataStoreSpecs && !hasSearchEngineId) {
135143
throw new IllegalArgumentException(
136-
"searchEngineId must be specified if dataStoreSpecs is specified.");
144+
"searchEngineId must not be empty if dataStoreSpecs is not empty.");
137145
}
138146
return tool;
139147
}

core/src/test/java/com/google/adk/tools/VertexAiSearchToolTest.java

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public void build_noDataStoreIdOrSearchEngineId_throwsException() {
4141
assertThrows(IllegalArgumentException.class, () -> VertexAiSearchTool.builder().build());
4242
assertThat(exception)
4343
.hasMessageThat()
44-
.isEqualTo("Either dataStoreId or searchEngineId must be specified.");
44+
.isEqualTo("One and only one of dataStoreId or searchEngineId must not be empty.");
4545
}
4646

4747
@Test
@@ -52,22 +52,46 @@ public void build_bothDataStoreIdAndSearchEngineId_throwsException() {
5252
() -> VertexAiSearchTool.builder().dataStoreId("ds1").searchEngineId("se1").build());
5353
assertThat(exception)
5454
.hasMessageThat()
55-
.isEqualTo("Either dataStoreId or searchEngineId must be specified.");
55+
.isEqualTo("One and only one of dataStoreId or searchEngineId must not be empty.");
5656
}
5757

5858
@Test
5959
public void build_dataStoreSpecsWithoutSearchEngineId_throwsException() {
60+
VertexAISearchDataStoreSpec spec =
61+
VertexAISearchDataStoreSpec.builder().dataStore("ds1").build();
6062
IllegalArgumentException exception =
6163
assertThrows(
6264
IllegalArgumentException.class,
6365
() ->
6466
VertexAiSearchTool.builder()
6567
.dataStoreId("ds1")
66-
.dataStoreSpecs(ImmutableList.of())
68+
.dataStoreSpecs(ImmutableList.of(spec))
6769
.build());
6870
assertThat(exception)
6971
.hasMessageThat()
70-
.isEqualTo("searchEngineId must be specified if dataStoreSpecs is specified.");
72+
.isEqualTo("searchEngineId must not be empty if dataStoreSpecs is not empty.");
73+
}
74+
75+
@Test
76+
public void build_emptyDataStoreId_throwsException() {
77+
IllegalArgumentException exception =
78+
assertThrows(
79+
IllegalArgumentException.class,
80+
() -> VertexAiSearchTool.builder().dataStoreId("").build());
81+
assertThat(exception)
82+
.hasMessageThat()
83+
.isEqualTo("One and only one of dataStoreId or searchEngineId must not be empty.");
84+
}
85+
86+
@Test
87+
public void build_emptySearchEngineId_throwsException() {
88+
IllegalArgumentException exception =
89+
assertThrows(
90+
IllegalArgumentException.class,
91+
() -> VertexAiSearchTool.builder().searchEngineId("").build());
92+
assertThat(exception)
93+
.hasMessageThat()
94+
.isEqualTo("One and only one of dataStoreId or searchEngineId must not be empty.");
7195
}
7296

7397
@Test
@@ -82,6 +106,19 @@ public void build_withSearchEngineId_succeeds() {
82106
assertThat(tool.searchEngineId()).hasValue("se1");
83107
}
84108

109+
@Test
110+
public void build_withSearchEngineIdAndDataStoreSpecs_succeeds() {
111+
VertexAISearchDataStoreSpec spec =
112+
VertexAISearchDataStoreSpec.builder().dataStore("ds1").build();
113+
VertexAiSearchTool tool =
114+
VertexAiSearchTool.builder()
115+
.searchEngineId("se1")
116+
.dataStoreSpecs(ImmutableList.of(spec))
117+
.build();
118+
assertThat(tool.searchEngineId()).hasValue("se1");
119+
assertThat(tool.dataStoreSpecs()).containsExactly(spec);
120+
}
121+
85122
@Test
86123
public void processLlmRequest_addsRetrievalTool() {
87124
VertexAiSearchTool tool =
@@ -135,4 +172,18 @@ public void processLlmRequest_withDataStoreSpecs_addsRetrievalTool() {
135172
.get())
136173
.containsExactly(spec);
137174
}
175+
176+
@Test
177+
public void processLlmRequest_nonGeminiModel_throwsException() {
178+
VertexAiSearchTool tool = VertexAiSearchTool.builder().searchEngineId("se1").build();
179+
LlmRequest.Builder llmRequestBuilder = LlmRequest.builder().model("other-model");
180+
tool.processLlmRequest(llmRequestBuilder, ToolContext.builder(invocationContext).build())
181+
.test()
182+
.assertError(
183+
throwable ->
184+
throwable instanceof IllegalArgumentException
185+
&& throwable
186+
.getMessage()
187+
.equals("Vertex AI Search tool is only supported for Gemini models."));
188+
}
138189
}

0 commit comments

Comments
 (0)