fix: Remove app name from FileArtifactService directory structure

Co-authored-by: George Weale <gweale@google.com>
PiperOrigin-RevId: 834462462
This commit is contained in:
George Weale
2025-11-19 15:12:07 -08:00
committed by Copybara-Service
parent dc3f60cc93
commit 12db84f5cd
2 changed files with 17 additions and 53 deletions
@@ -188,20 +188,18 @@ class FileArtifactService(BaseArtifactService):
# Storage layout matches the cloud and in-memory services:
# root/
# ── apps/
# └── {app_name}/
# ── users/
# └── {user_id}/
# ── sessions/
#└── {session_id}/
# │ │ └── artifacts/
#└── {artifact_path}/ # derived from filename
#└── versions/
#└── {version}/
# │ ├── {original_filename}
# │ │ └── metadata.json
# │ └── artifacts/
# │ └── {artifact_path}/...
# ── users/
# └── {user_id}/
# ── sessions/
# └── {session_id}/
# ── artifacts/
# │ └── {artifact_path}/ # derived from filename
# └── versions/
# │ └── {version}/
# │ ├── {original_filename}
# │ └── metadata.json
# └── artifacts/
# └── {artifact_path}/...
#
# Artifact paths are derived from the provided filenames: separators create
# nested directories, and path traversal is rejected to keep the layout
@@ -217,19 +215,18 @@ class FileArtifactService(BaseArtifactService):
self.root_dir = Path(root_dir).expanduser().resolve()
self.root_dir.mkdir(parents=True, exist_ok=True)
def _base_root(self, app_name: str, user_id: str) -> Path:
"""Returns the artifacts root directory for an app/user combination."""
return self.root_dir / "apps" / app_name / "users" / user_id
def _base_root(self, user_id: str, /) -> Path:
"""Returns the artifacts root directory for a user."""
return self.root_dir / "users" / user_id
def _scope_root(
self,
app_name: str,
user_id: str,
session_id: Optional[str],
filename: str,
) -> Path:
"""Returns the directory that represents the artifact scope."""
base = self._base_root(app_name, user_id)
base = self._base_root(user_id)
if _is_user_scoped(session_id, filename):
return _user_artifacts_dir(base)
if not session_id:
@@ -240,14 +237,12 @@ class FileArtifactService(BaseArtifactService):
def _artifact_dir(
self,
app_name: str,
user_id: str,
session_id: Optional[str],
filename: str,
) -> Path:
"""Builds the directory path for an artifact."""
scope_root = self._scope_root(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -258,7 +253,6 @@ class FileArtifactService(BaseArtifactService):
def _build_artifact_version(
self,
*,
app_name: str,
user_id: str,
session_id: Optional[str],
filename: str,
@@ -270,7 +264,6 @@ class FileArtifactService(BaseArtifactService):
metadata.canonical_uri
if metadata and metadata.canonical_uri
else self._canonical_uri(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -289,7 +282,6 @@ class FileArtifactService(BaseArtifactService):
def _canonical_uri(
self,
*,
app_name: str,
user_id: str,
session_id: Optional[str],
filename: str,
@@ -297,7 +289,6 @@ class FileArtifactService(BaseArtifactService):
) -> str:
"""Builds the canonical file:// URI for an artifact payload."""
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -336,7 +327,6 @@ class FileArtifactService(BaseArtifactService):
"""
return await asyncio.to_thread(
self._save_artifact_sync,
app_name,
user_id,
filename,
artifact,
@@ -346,7 +336,6 @@ class FileArtifactService(BaseArtifactService):
def _save_artifact_sync(
self,
app_name: str,
user_id: str,
filename: str,
artifact: types.Part,
@@ -355,7 +344,6 @@ class FileArtifactService(BaseArtifactService):
) -> int:
"""Saves an artifact to disk and returns its version."""
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -386,7 +374,6 @@ class FileArtifactService(BaseArtifactService):
raise ValueError("Artifact must have either inline_data or text content.")
canonical_uri = self._canonical_uri(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -421,7 +408,6 @@ class FileArtifactService(BaseArtifactService):
) -> Optional[types.Part]:
return await asyncio.to_thread(
self._load_artifact_sync,
app_name,
user_id,
filename,
session_id,
@@ -430,7 +416,6 @@ class FileArtifactService(BaseArtifactService):
def _load_artifact_sync(
self,
app_name: str,
user_id: str,
filename: str,
session_id: Optional[str],
@@ -438,7 +423,6 @@ class FileArtifactService(BaseArtifactService):
) -> Optional[types.Part]:
"""Loads an artifact from disk."""
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -493,21 +477,19 @@ class FileArtifactService(BaseArtifactService):
) -> list[str]:
return await asyncio.to_thread(
self._list_artifact_keys_sync,
app_name,
user_id,
session_id,
)
def _list_artifact_keys_sync(
self,
app_name: str,
user_id: str,
session_id: Optional[str],
) -> list[str]:
"""Lists artifact filenames for the given session/user."""
filenames: set[str] = set()
base_root = self._base_root(app_name, user_id)
base_root = self._base_root(user_id)
if session_id:
session_root = _session_artifacts_dir(base_root, session_id)
@@ -550,7 +532,6 @@ class FileArtifactService(BaseArtifactService):
"""
await asyncio.to_thread(
self._delete_artifact_sync,
app_name,
user_id,
filename,
session_id,
@@ -558,13 +539,11 @@ class FileArtifactService(BaseArtifactService):
def _delete_artifact_sync(
self,
app_name: str,
user_id: str,
filename: str,
session_id: Optional[str],
) -> None:
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -585,7 +564,6 @@ class FileArtifactService(BaseArtifactService):
"""Lists all versions stored for an artifact."""
return await asyncio.to_thread(
self._list_versions_sync,
app_name,
user_id,
filename,
session_id,
@@ -593,13 +571,11 @@ class FileArtifactService(BaseArtifactService):
def _list_versions_sync(
self,
app_name: str,
user_id: str,
filename: str,
session_id: Optional[str],
) -> list[int]:
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -618,7 +594,6 @@ class FileArtifactService(BaseArtifactService):
"""Lists metadata for each artifact version on disk."""
return await asyncio.to_thread(
self._list_artifact_versions_sync,
app_name,
user_id,
filename,
session_id,
@@ -626,13 +601,11 @@ class FileArtifactService(BaseArtifactService):
def _list_artifact_versions_sync(
self,
app_name: str,
user_id: str,
filename: str,
session_id: Optional[str],
) -> list[ArtifactVersion]:
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -644,7 +617,6 @@ class FileArtifactService(BaseArtifactService):
metadata = _read_metadata(metadata_path)
artifact_versions.append(
self._build_artifact_version(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -667,7 +639,6 @@ class FileArtifactService(BaseArtifactService):
"""Gets metadata for a specific artifact version."""
return await asyncio.to_thread(
self._get_artifact_version_sync,
app_name,
user_id,
filename,
session_id,
@@ -676,14 +647,12 @@ class FileArtifactService(BaseArtifactService):
def _get_artifact_version_sync(
self,
app_name: str,
user_id: str,
filename: str,
session_id: Optional[str],
version: Optional[int],
) -> Optional[ArtifactVersion]:
artifact_dir = self._artifact_dir(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -701,7 +670,6 @@ class FileArtifactService(BaseArtifactService):
metadata_path = _metadata_path(artifact_dir, version_to_read)
metadata = _read_metadata(metadata_path)
return self._build_artifact_version(
app_name=app_name,
user_id=user_id,
session_id=session_id,
filename=filename,
@@ -614,8 +614,6 @@ async def test_file_metadata_camelcase(tmp_path, artifact_service_factory):
metadata_path = (
tmp_path
/ "artifacts"
/ "apps"
/ "myapp"
/ "users"
/ "user123"
/ "sessions"
@@ -677,8 +675,6 @@ async def test_file_list_artifact_versions(tmp_path, artifact_service_factory):
version_payload_path = (
tmp_path
/ "artifacts"
/ "apps"
/ "myapp"
/ "users"
/ "user123"
/ "sessions"