Skip to content

Commit 5fd4c53

Browse files
tilgalascopybara-github
tilgalas
authored and
copybara-github
committed
feat: replace Optional type of version in BaseArtifactService.loadArtifact with Nullable
PiperOrigin-RevId: 883074383
1 parent dc51aec commit 5fd4c53

9 files changed

Lines changed: 27 additions & 47 deletions

File tree

‎core/src/main/java/com/google/adk/artifacts/BaseArtifactService.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import io.reactivex.rxjava3.core.Completable;
2323
import io.reactivex.rxjava3.core.Maybe;
2424
import io.reactivex.rxjava3.core.Single;
25-
import java.util.Optional;
25+
import org.jspecify.annotations.Nullable;
2626

2727
/** Base interface for artifact services. */
2828
public interface BaseArtifactService {
@@ -75,7 +75,7 @@ default Single<Part> saveAndReloadArtifact(
7575
/** Loads the latest version of an artifact from the service. */
7676
default Maybe<Part> loadArtifact(
7777
String appName, String userId, String sessionId, String filename) {
78-
return loadArtifact(appName, userId, sessionId, filename, Optional.empty());
78+
return loadArtifact(appName, userId, sessionId, filename, /* version= */ (Integer) null);
7979
}
8080

8181
/** Loads the latest version of an artifact from the service. */
@@ -86,21 +86,16 @@ default Maybe<Part> loadArtifact(SessionKey sessionKey, String filename) {
8686
/** Loads a specific version of an artifact from the service. */
8787
default Maybe<Part> loadArtifact(
8888
String appName, String userId, String sessionId, String filename, int version) {
89-
return loadArtifact(appName, userId, sessionId, filename, Optional.of(version));
89+
return loadArtifact(appName, userId, sessionId, filename, Integer.valueOf(version));
9090
}
9191

9292
default Maybe<Part> loadArtifact(SessionKey sessionKey, String filename, int version) {
9393
return loadArtifact(
9494
sessionKey.appName(), sessionKey.userId(), sessionKey.id(), filename, version);
9595
}
9696

97-
/**
98-
* @deprecated Use {@link #loadArtifact(String, String, String, String)} or {@link
99-
* #loadArtifact(String, String, String, String, int)} instead.
100-
*/
101-
@Deprecated
10297
Maybe<Part> loadArtifact(
103-
String appName, String userId, String sessionId, String filename, Optional<Integer> version);
98+
String appName, String userId, String sessionId, String filename, @Nullable Integer version);
10499

105100
/**
106101
* Lists all the artifact filenames within a session.

‎core/src/main/java/com/google/adk/artifacts/GcsArtifactService.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.List;
3939
import java.util.Optional;
4040
import java.util.Set;
41+
import org.jspecify.annotations.Nullable;
4142

4243
/** An artifact service implementation using Google Cloud Storage (GCS). */
4344
public final class GcsArtifactService implements BaseArtifactService {
@@ -126,8 +127,8 @@ public Single<Integer> saveArtifact(
126127
*/
127128
@Override
128129
public Maybe<Part> loadArtifact(
129-
String appName, String userId, String sessionId, String filename, Optional<Integer> version) {
130-
return version
130+
String appName, String userId, String sessionId, String filename, @Nullable Integer version) {
131+
return Optional.ofNullable(version)
131132
.map(Maybe::just)
132133
.orElseGet(
133134
() ->

‎core/src/main/java/com/google/adk/artifacts/InMemoryArtifactService.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
import java.util.HashMap;
2929
import java.util.List;
3030
import java.util.Map;
31-
import java.util.Optional;
3231
import java.util.stream.IntStream;
32+
import org.jspecify.annotations.Nullable;
3333

3434
/** An in-memory implementation of the {@link BaseArtifactService}. */
3535
public final class InMemoryArtifactService implements BaseArtifactService {
@@ -61,18 +61,17 @@ public Single<Integer> saveArtifact(
6161
*/
6262
@Override
6363
public Maybe<Part> loadArtifact(
64-
String appName, String userId, String sessionId, String filename, Optional<Integer> version) {
64+
String appName, String userId, String sessionId, String filename, @Nullable Integer version) {
6565
List<Part> versions =
6666
getArtifactsMap(appName, userId, sessionId)
6767
.computeIfAbsent(filename, unused -> new ArrayList<>());
6868

6969
if (versions.isEmpty()) {
7070
return Maybe.empty();
7171
}
72-
if (version.isPresent()) {
73-
int v = version.get();
74-
if (v >= 0 && v < versions.size()) {
75-
return Maybe.just(versions.get(v));
72+
if (version != null) {
73+
if (version >= 0 && version < versions.size()) {
74+
return Maybe.just(versions.get(version));
7675
} else {
7776
return Maybe.empty();
7877
}

‎core/src/main/java/com/google/adk/utils/InstructionUtils.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import io.reactivex.rxjava3.core.Single;
2626
import java.util.ArrayList;
2727
import java.util.List;
28-
import java.util.Optional;
2928
import java.util.regex.MatchResult;
3029
import java.util.regex.Matcher;
3130
import java.util.regex.Pattern;
@@ -167,12 +166,7 @@ private static Single<String> resolveMatchAsync(InvocationContext context, Match
167166
Maybe<Part> artifactMaybe =
168167
context
169168
.artifactService()
170-
.loadArtifact(
171-
session.appName(),
172-
session.userId(),
173-
session.id(),
174-
artifactName,
175-
Optional.empty());
169+
.loadArtifact(session.appName(), session.userId(), session.id(), artifactName);
176170

177171
return artifactMaybe
178172
.map(Part::toJson)

‎core/src/test/java/com/google/adk/artifacts/GcsArtifactServiceTest.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public void load_latestVersion_loadsCorrectly() {
158158
when(mockStorage.get(blobIdV1)).thenReturn(blobV1);
159159

160160
Optional<Part> loadedArtifact =
161-
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, Optional.empty()));
161+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME));
162162

163163
assertThat(loadedArtifact).isPresent();
164164
Optional<byte[]> actualDataOptional = loadedArtifact.get().inlineData().get().data();
@@ -177,7 +177,7 @@ public void load_specificVersion_loadsCorrectly() {
177177
when(mockStorage.get(blobIdV0)).thenReturn(blobV0);
178178

179179
Optional<Part> loadedArtifact =
180-
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, Optional.of(0)));
180+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, 0));
181181

182182
assertThat(loadedArtifact).isPresent();
183183
Optional<byte[]> actualDataOptional = loadedArtifact.get().inlineData().get().data();
@@ -197,8 +197,7 @@ public void load_userNamespace_loadsCorrectly() {
197197
when(mockStorage.get(blobIdV0)).thenReturn(blobV0);
198198

199199
Optional<Part> loadedArtifact =
200-
asOptional(
201-
service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, USER_FILENAME, Optional.empty()));
200+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, USER_FILENAME));
202201

203202
assertThat(loadedArtifact).isPresent();
204203
Optional<byte[]> actualDataOptional = loadedArtifact.get().inlineData().get().data();
@@ -216,7 +215,7 @@ public void load_versionNotFound_returnsEmpty() {
216215
when(mockStorage.get(blobIdV0)).thenReturn(null);
217216

218217
Optional<Part> loadedArtifact =
219-
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, Optional.of(0)));
218+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, 0));
220219

221220
assertThat(loadedArtifact).isEmpty();
222221
verify(mockStorage).get(blobIdV0);
@@ -227,7 +226,7 @@ public void load_noVersionsExist_returnsEmpty() {
227226
when(mockBlobPage.iterateAll()).thenReturn(ImmutableList.of());
228227

229228
Optional<Part> loadedArtifact =
230-
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, Optional.empty()));
229+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME));
231230

232231
assertThat(loadedArtifact).isEmpty();
233232
}
@@ -400,7 +399,7 @@ public void load_storageException_returnsEmpty() {
400399
when(mockStorage.get(blobIdV0)).thenThrow(new StorageException(500, "Induced error"));
401400

402401
Optional<Part> loadedArtifact =
403-
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, Optional.of(0)));
402+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, 0));
404403

405404
assertThat(loadedArtifact).isEmpty();
406405
}

‎core/src/test/java/com/google/adk/artifacts/InMemoryArtifactServiceTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public void loadArtifact_loadsLatest() {
5959
var unused2 =
6060
service.saveArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, artifact2).blockingGet();
6161
Optional<Part> result =
62-
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME, Optional.empty()));
62+
asOptional(service.loadArtifact(APP_NAME, USER_ID, SESSION_ID, FILENAME));
6363
assertThat(result).hasValue(artifact2);
6464
}
6565

‎core/src/test/java/com/google/adk/flows/llmflows/InstructionsTest.java‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.google.genai.types.Part;
3333
import io.reactivex.rxjava3.core.Maybe;
3434
import io.reactivex.rxjava3.core.Single;
35-
import java.util.Optional;
3635
import java.util.concurrent.ConcurrentHashMap;
3736
import org.junit.Before;
3837
import org.junit.Rule;
@@ -122,11 +121,7 @@ public void processRequest_agentInstructionString_noPlaceholders_appendsInstruct
122121
Session session = createSession();
123122
Part artifactPart = Part.fromText("Artifact content");
124123
when(mockArtifactService.loadArtifact(
125-
eq(session.appName()),
126-
eq(session.userId()),
127-
eq(session.id()),
128-
eq("file.txt"),
129-
eq(Optional.empty())))
124+
eq(session.appName()), eq(session.userId()), eq(session.id()), eq("file.txt")))
130125
.thenReturn(Maybe.just(artifactPart));
131126
LlmAgent agent =
132127
LlmAgent.builder().name("agent").instruction("File content: {artifact.file.txt}").build();

‎core/src/test/java/com/google/adk/tools/LoadArtifactsToolTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package com.google.adk.tools;
22

33
import static com.google.common.truth.Truth.assertThat;
4-
import static org.mockito.ArgumentMatchers.any;
4+
import static org.mockito.ArgumentMatchers.anyInt;
55
import static org.mockito.ArgumentMatchers.anyString;
66
import static org.mockito.ArgumentMatchers.nullable;
77
import static org.mockito.Mockito.mock;
@@ -105,7 +105,7 @@ public void processLlmRequest_noArtifactsInContext_completesWithoutLoading() {
105105
assertThat(finalRequest.config()).isPresent();
106106
assertThat(finalRequest.config().get().systemInstruction()).isEmpty();
107107
verify(mockArtifactService, never())
108-
.loadArtifact(anyString(), anyString(), anyString(), anyString(), any());
108+
.loadArtifact(anyString(), anyString(), anyString(), anyString(), anyInt());
109109
}
110110

111111
@Test
@@ -130,7 +130,7 @@ public void processLlmRequest_artifactsInContext_noFunctionCall_appendsInstructi
130130
assertThat(appendedInstruction).contains("call the `load_artifacts` function");
131131

132132
verify(mockArtifactService, never())
133-
.loadArtifact(anyString(), anyString(), anyString(), anyString(), any());
133+
.loadArtifact(anyString(), anyString(), anyString(), anyString(), anyInt());
134134
}
135135

136136
@Test
@@ -215,7 +215,7 @@ public void processLlmRequest_artifactsInContext_withOtherFunctionCall_doesNotLo
215215
.contains("You have a list of artifacts:");
216216

217217
verify(mockArtifactService, never())
218-
.loadArtifact(anyString(), anyString(), anyString(), anyString(), any());
218+
.loadArtifact(anyString(), anyString(), anyString(), anyString(), anyInt());
219219
assertThat(finalRequest.contents()).containsExactly(functionCallContent);
220220
}
221221
}

‎dev/src/main/java/com/google/adk/web/controller/ArtifactController.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import io.reactivex.rxjava3.core.Single;
2525
import java.util.Collections;
2626
import java.util.List;
27-
import java.util.Optional;
2827
import org.slf4j.Logger;
2928
import org.slf4j.LoggerFactory;
3029
import org.springframework.beans.factory.annotation.Autowired;
@@ -78,8 +77,7 @@ public Part loadArtifact(
7877
versionStr);
7978

8079
Maybe<Part> artifactMaybe =
81-
artifactService.loadArtifact(
82-
appName, userId, sessionId, artifactName, Optional.ofNullable(version));
80+
artifactService.loadArtifact(appName, userId, sessionId, artifactName, version);
8381

8482
Part artifact = artifactMaybe.blockingGet();
8583

@@ -126,8 +124,7 @@ public Part loadArtifactVersion(
126124
versionId);
127125

128126
Maybe<Part> artifactMaybe =
129-
artifactService.loadArtifact(
130-
appName, userId, sessionId, artifactName, Optional.of(versionId));
127+
artifactService.loadArtifact(appName, userId, sessionId, artifactName, versionId);
131128

132129
Part artifact = artifactMaybe.blockingGet();
133130

0 commit comments

Comments
 (0)