Skip to content

Commit b99546b

Browse files
spectaclehongboyangsvl
authored andcommitted
fix(artifacts): Support nested API names
Merge #6051 **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Fixes: #6050 **Problem:** Artifact services can save and list logical artifact names containing `/`, such as `reports/summary.txt`. The REST API also accepts these names through `POST /artifacts` because the filename is provided in the request body. However, read/version/metadata/delete routes treated `artifact_name` as a single path segment. That made nested artifact names visible in the list endpoint but unreachable through the path-based endpoints. **Solution:** Update artifact read/version/metadata/delete routes to use FastAPI's `{artifact_name:path}` converter so nested logical artifact names can round-trip through the REST API. Because `{artifact_name:path}` is catch-all, the version-specific routes are registered before the general artifact load route. No response schema changes are intended: endpoints that returned `types.Part` still return `types.Part`, and metadata endpoints still return `ArtifactVersion` / `list[ArtifactVersion]`. Also update artifact URI parsing so artifact references can resolve nested filenames such as `folder/file.txt`. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. Targeted artifact API tests pass locally: ```shell uv run pytest tests/unittests/cli/test_fast_api.py -k artifact -q ``` Result: ```text 9 passed, 72 deselected ``` Artifact service and URI tests pass locally: ```shell uv run pytest tests/unittests/artifacts/test_artifact_util.py tests/unittests/artifacts/test_artifact_service.py -q ``` Result: ```text 77 passed ``` Python 3.10 tox unit suite passes locally: ```shell uv tool run --from tox --with tox-uv tox -e py310 ``` Result: ```text 7045 passed, 21 skipped, 31 xfailed, 9 xpassed ``` Pre-commit hooks pass for changed files: ```shell uv run pre-commit run --files src/google/adk/cli/api_server.py src/google/adk/artifacts/artifact_util.py tests/unittests/cli/test_fast_api.py tests/unittests/artifacts/test_artifact_util.py tests/unittests/artifacts/test_artifact_service.py ``` **Manual End-to-End (E2E) Tests:** Validated a local REST API round-trip using the ADK API server route stack with `InMemoryArtifactService`: - save `reports/summary.txt` - list artifacts - load with raw slash path - load with encoded slash path - list versions - load version `0` - load version metadata - delete with encoded slash path All requests returned `200`. ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] Any dependent changes have been merged and published in downstream modules. ### Additional context No dependent changes are required. Co-authored-by: Bo Yang <ybo@google.com> COPYBARA_INTEGRATE_REVIEW=#6051 from spectaclehong:fix/artifact-slash-api 1c08a4f PiperOrigin-RevId: 931342522
1 parent 4e4bf84 commit b99546b

5 files changed

Lines changed: 238 additions & 53 deletions

File tree

src/google/adk/artifacts/artifact_util.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import re
1919
from typing import NamedTuple
20-
from typing import Optional
2120

2221
from google.genai import types
2322

@@ -27,20 +26,20 @@ class ParsedArtifactUri(NamedTuple):
2726

2827
app_name: str
2928
user_id: str
30-
session_id: Optional[str]
29+
session_id: str | None
3130
filename: str
3231
version: int
3332

3433

3534
_SESSION_SCOPED_ARTIFACT_URI_RE = re.compile(
36-
r"artifact://apps/([^/]+)/users/([^/]+)/sessions/([^/]+)/artifacts/([^/]+)/versions/(\d+)"
35+
r"artifact://apps/([^/]+)/users/([^/]+)/sessions/([^/]+)/artifacts/(.+)/versions/(\d+)"
3736
)
3837
_USER_SCOPED_ARTIFACT_URI_RE = re.compile(
39-
r"artifact://apps/([^/]+)/users/([^/]+)/artifacts/([^/]+)/versions/(\d+)"
38+
r"artifact://apps/([^/]+)/users/([^/]+)/artifacts/(.+)/versions/(\d+)"
4039
)
4140

4241

43-
def parse_artifact_uri(uri: str) -> Optional[ParsedArtifactUri]:
42+
def parse_artifact_uri(uri: str) -> ParsedArtifactUri | None:
4443
"""Parses an artifact URI.
4544
4645
Args:
@@ -52,7 +51,7 @@ def parse_artifact_uri(uri: str) -> Optional[ParsedArtifactUri]:
5251
if not uri or not uri.startswith("artifact://"):
5352
return None
5453

55-
match = _SESSION_SCOPED_ARTIFACT_URI_RE.match(uri)
54+
match = _SESSION_SCOPED_ARTIFACT_URI_RE.fullmatch(uri)
5655
if match:
5756
return ParsedArtifactUri(
5857
app_name=match.group(1),
@@ -62,7 +61,7 @@ def parse_artifact_uri(uri: str) -> Optional[ParsedArtifactUri]:
6261
version=int(match.group(5)),
6362
)
6463

65-
match = _USER_SCOPED_ARTIFACT_URI_RE.match(uri)
64+
match = _USER_SCOPED_ARTIFACT_URI_RE.fullmatch(uri)
6665
if match:
6766
return ParsedArtifactUri(
6867
app_name=match.group(1),
@@ -80,7 +79,7 @@ def get_artifact_uri(
8079
user_id: str,
8180
filename: str,
8281
version: int,
83-
session_id: Optional[str] = None,
82+
session_id: str | None = None,
8483
) -> str:
8584
"""Constructs an artifact URI.
8685

src/google/adk/cli/api_server.py

Lines changed: 64 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,29 +1219,40 @@ async def update_session(
12191219
return session
12201220

12211221
@app.get(
1222-
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name}",
1222+
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}/versions/{version_id}/metadata",
1223+
response_model=ArtifactVersion,
12231224
response_model_exclude_none=True,
12241225
)
1225-
async def load_artifact(
1226+
async def get_artifact_version_metadata(
12261227
app_name: str,
12271228
user_id: str,
12281229
session_id: str,
12291230
artifact_name: str,
1230-
version: Optional[int] = Query(None),
1231-
) -> Optional[types.Part]:
1232-
artifact = await self.artifact_service.load_artifact(
1231+
version_id: str,
1232+
) -> ArtifactVersion:
1233+
version: int | None = None
1234+
if version_id != "latest":
1235+
try:
1236+
version = int(version_id)
1237+
except ValueError:
1238+
raise HTTPException(
1239+
status_code=422, detail="Invalid version ID"
1240+
) from None
1241+
artifact_version = await self.artifact_service.get_artifact_version(
12331242
app_name=app_name,
12341243
user_id=user_id,
12351244
session_id=session_id,
12361245
filename=artifact_name,
12371246
version=version,
12381247
)
1239-
if not artifact:
1240-
raise HTTPException(status_code=404, detail="Artifact not found")
1241-
return artifact
1248+
if not artifact_version:
1249+
raise HTTPException(
1250+
status_code=404, detail="Artifact version not found"
1251+
)
1252+
return artifact_version
12421253

12431254
@app.get(
1244-
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name}/versions/metadata",
1255+
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}/versions/metadata",
12451256
response_model=list[ArtifactVersion],
12461257
response_model_exclude_none=True,
12471258
)
@@ -1258,28 +1269,6 @@ async def list_artifact_versions_metadata(
12581269
filename=artifact_name,
12591270
)
12601271

1261-
@app.get(
1262-
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name}/versions/{version_id}",
1263-
response_model_exclude_none=True,
1264-
)
1265-
async def load_artifact_version(
1266-
app_name: str,
1267-
user_id: str,
1268-
session_id: str,
1269-
artifact_name: str,
1270-
version_id: int,
1271-
) -> Optional[types.Part]:
1272-
artifact = await self.artifact_service.load_artifact(
1273-
app_name=app_name,
1274-
user_id=user_id,
1275-
session_id=session_id,
1276-
filename=artifact_name,
1277-
version=version_id,
1278-
)
1279-
if not artifact:
1280-
raise HTTPException(status_code=404, detail="Artifact not found")
1281-
return artifact
1282-
12831272
@app.post(
12841273
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts",
12851274
response_model=ArtifactVersion,
@@ -1329,29 +1318,34 @@ async def save_artifact(
13291318
return artifact_version
13301319

13311320
@app.get(
1332-
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name}/versions/{version_id}/metadata",
1333-
response_model=ArtifactVersion,
1321+
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}/versions/{version_id}",
13341322
response_model_exclude_none=True,
13351323
)
1336-
async def get_artifact_version_metadata(
1324+
async def load_artifact_version(
13371325
app_name: str,
13381326
user_id: str,
13391327
session_id: str,
13401328
artifact_name: str,
1341-
version_id: int,
1342-
) -> ArtifactVersion:
1343-
artifact_version = await self.artifact_service.get_artifact_version(
1329+
version_id: str,
1330+
) -> types.Part | None:
1331+
version: int | None = None
1332+
if version_id != "latest":
1333+
try:
1334+
version = int(version_id)
1335+
except ValueError:
1336+
raise HTTPException(
1337+
status_code=422, detail="Invalid version ID"
1338+
) from None
1339+
artifact = await self.artifact_service.load_artifact(
13441340
app_name=app_name,
13451341
user_id=user_id,
13461342
session_id=session_id,
13471343
filename=artifact_name,
1348-
version=version_id,
1344+
version=version,
13491345
)
1350-
if not artifact_version:
1351-
raise HTTPException(
1352-
status_code=404, detail="Artifact version not found"
1353-
)
1354-
return artifact_version
1346+
if not artifact:
1347+
raise HTTPException(status_code=404, detail="Artifact not found")
1348+
return artifact
13551349

13561350
@app.get(
13571351
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts",
@@ -1365,7 +1359,7 @@ async def list_artifact_names(
13651359
)
13661360

13671361
@app.get(
1368-
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name}/versions",
1362+
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}/versions",
13691363
response_model_exclude_none=True,
13701364
)
13711365
async def list_artifact_versions(
@@ -1378,8 +1372,33 @@ async def list_artifact_versions(
13781372
filename=artifact_name,
13791373
)
13801374

1375+
# Keep this catch-all artifact route after the version-specific routes.
1376+
# Artifact names may contain '/', so {artifact_name:path} would otherwise
1377+
# capture requests for /versions/... endpoints.
1378+
@app.get(
1379+
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}",
1380+
response_model_exclude_none=True,
1381+
)
1382+
async def load_artifact(
1383+
app_name: str,
1384+
user_id: str,
1385+
session_id: str,
1386+
artifact_name: str,
1387+
version: int | None = Query(None),
1388+
) -> types.Part | None:
1389+
artifact = await self.artifact_service.load_artifact(
1390+
app_name=app_name,
1391+
user_id=user_id,
1392+
session_id=session_id,
1393+
filename=artifact_name,
1394+
version=version,
1395+
)
1396+
if not artifact:
1397+
raise HTTPException(status_code=404, detail="Artifact not found")
1398+
return artifact
1399+
13811400
@app.delete(
1382-
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name}",
1401+
"/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}",
13831402
)
13841403
async def delete_artifact(
13851404
app_name: str, user_id: str, session_id: str, artifact_name: str

tests/unittests/artifacts/test_artifact_service.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,52 @@ async def test_save_load_delete(service_type, artifact_service_factory):
263263
)
264264

265265

266+
@pytest.mark.asyncio
267+
async def test_in_memory_loads_nested_artifact_reference(
268+
artifact_service_factory,
269+
):
270+
"""Tests loading an artifact reference whose target name is nested."""
271+
artifact_service = artifact_service_factory(ArtifactServiceType.IN_MEMORY)
272+
app_name = "app0"
273+
user_id = "user0"
274+
session_id = "123"
275+
target_filename = "folder/file456"
276+
target_artifact = types.Part.from_text(text="target")
277+
278+
await artifact_service.save_artifact(
279+
app_name=app_name,
280+
user_id=user_id,
281+
session_id=session_id,
282+
filename=target_filename,
283+
artifact=target_artifact,
284+
)
285+
await artifact_service.save_artifact(
286+
app_name=app_name,
287+
user_id=user_id,
288+
session_id=session_id,
289+
filename="reference",
290+
artifact=types.Part(
291+
file_data=types.FileData(
292+
file_uri=(
293+
"artifact://apps/app0/users/user0/sessions/123/artifacts/"
294+
"folder/file456/versions/0"
295+
),
296+
mime_type="text/plain",
297+
)
298+
),
299+
)
300+
301+
assert (
302+
await artifact_service.load_artifact(
303+
app_name=app_name,
304+
user_id=user_id,
305+
session_id=session_id,
306+
filename="reference",
307+
)
308+
== target_artifact
309+
)
310+
311+
266312
@pytest.mark.asyncio
267313
@pytest.mark.parametrize(
268314
"service_type",

tests/unittests/artifacts/test_artifact_util.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,21 @@ def test_parse_session_scoped_artifact_uri():
3131
assert parsed.version == 123
3232

3333

34+
def test_parse_session_scoped_artifact_uri_with_nested_filename():
35+
"""Tests parsing a session-scoped artifact URI with a nested filename."""
36+
uri = (
37+
"artifact://apps/app1/users/user1/sessions/session1/artifacts/"
38+
"folder/file1/versions/123"
39+
)
40+
parsed = artifact_util.parse_artifact_uri(uri)
41+
assert parsed is not None
42+
assert parsed.app_name == "app1"
43+
assert parsed.user_id == "user1"
44+
assert parsed.session_id == "session1"
45+
assert parsed.filename == "folder/file1"
46+
assert parsed.version == 123
47+
48+
3449
def test_parse_user_scoped_artifact_uri():
3550
"""Tests parsing a valid user-scoped artifact URI."""
3651
uri = "artifact://apps/app2/users/user2/artifacts/file2/versions/456"
@@ -43,6 +58,18 @@ def test_parse_user_scoped_artifact_uri():
4358
assert parsed.version == 456
4459

4560

61+
def test_parse_user_scoped_artifact_uri_with_nested_filename():
62+
"""Tests parsing a user-scoped artifact URI with a nested filename."""
63+
uri = "artifact://apps/app2/users/user2/artifacts/folder/file2/versions/456"
64+
parsed = artifact_util.parse_artifact_uri(uri)
65+
assert parsed is not None
66+
assert parsed.app_name == "app2"
67+
assert parsed.user_id == "user2"
68+
assert parsed.session_id is None
69+
assert parsed.filename == "folder/file2"
70+
assert parsed.version == 456
71+
72+
4673
@pytest.mark.parametrize(
4774
"invalid_uri",
4875
[
@@ -51,6 +78,7 @@ def test_parse_user_scoped_artifact_uri():
5178
"artifact://app1/user1/sessions/session1/artifacts/file1",
5279
"artifact://apps/app1/users/user1/sessions/session1/artifacts/file1",
5380
"artifact://apps/app1/users/user1/artifacts/file1",
81+
"artifact://apps/app1/users/user1/artifacts/file1/versions/1/extra",
5482
],
5583
)
5684
def test_parse_invalid_artifact_uri(invalid_uri):

0 commit comments

Comments
 (0)