From 19157ad20beb897d3b7ac3bec1fe563ec0bfe1d5 Mon Sep 17 00:00:00 2001 From: "tao.chen" <93983997+taochen-ct@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:08:12 +0800 Subject: [PATCH] test --- backend/src/backend/scripts.py | 120 ++++++++++++++++++--------------- 1 file changed, 67 insertions(+), 53 deletions(-) diff --git a/backend/src/backend/scripts.py b/backend/src/backend/scripts.py index c7545b4..94dcc89 100644 --- a/backend/src/backend/scripts.py +++ b/backend/src/backend/scripts.py @@ -118,31 +118,17 @@ def safe_script_name(value: str, script_type: str) -> str: return name -def _jupyter_path(script_type: str, script_id: str) -> str: - """Return the in-Jupyter path used for a script. +def _jupyter_path(script_type: str, script_name: str) -> str: + """Return the Jupyter basename for a newly-created script. - The path is ``{script_id}.{ext}`` — deterministic and - collision-free because ``script_id`` is a fresh ULID. Jupyter's - contents API treats the suffix after ``/api/contents/`` as a path - relative to its ``--notebook-dir`` (the workspace root), so a - ``notebooks/`` prefix here would push the file into a non-existent - ``notebooks/`` subdir on disk. The user's ``script_name`` is kept on - the Scripts row as a display label only; the on-disk filename is - owned by the database. - - The ``/notebooks/`` segment in the user's browser URL - (``/jupyter//notebooks/.ipynb``) is jupyter's URL route - for the editor view, not a filesystem path — jupyter routes that - URL to the file at the workspace root. - - NOTE: This function returns the flat basename only (``{script_id}.{ext}``). - It must NOT be used directly as a Jupyter path for scripts that live - inside sub-directories. Callers must combine it with the parent - directory ULID (for newly-created files) or derive the real path from - ``StorageObjects.object_key`` (for existing files). + Callers MUST pass the value returned by ``safe_script_name()`` so the + extension is correct and the name is POSIX-safe. The on-disk filename + is the user's name — there is no separate ULID segment. """ - ext = ".ipynb" if script_type == "notebook" else ".py" - return f"{script_id}{ext}" + expected_suffix = ".py" if script_type == "python" else ".ipynb" + if not script_name.lower().endswith(expected_suffix): + script_name += expected_suffix + return script_name def _derive_jupyter_path( @@ -367,6 +353,7 @@ async def create_script_record( ) -> tuple[Scripts, dict[str, Any]]: parent = normalize_user_path(parent_path) if parent_path else "" scoped_prefix = user_relative_path(context) + parent_dir_row = None if parent: parent_dir_row = await session.scalar( select(StorageObjects).where( @@ -381,26 +368,32 @@ async def create_script_record( status.HTTP_404_NOT_FOUND, "parent directory not found", ) - parent_ulid = parent_dir_row.storage_object_id - else: - parent_ulid = None display_parent = parent child_path = f"{display_parent}/{name}" if display_parent else name relative_path = user_relative_path(context, child_path) logger.debug(relative_path) - # The Jupyter-side filename is owned by the database: a fresh ULID - # guarantees uniqueness, so the StorageObject-based 409 check from - # the legacy flow no longer applies. We still do a Scripts-only - # conflict check on the user-supplied name so two scripts cannot - # claim the same display name within the same workspace. + # The Jupyter-side filename is the sanitized script name; extension and + # path separators are enforced by safe_script_name(). The script_id ULID + # remains the Scripts PK only. We still do a Scripts-only conflict check + # on the user-supplied name so two scripts cannot claim the same display + # name within the same workspace. user_id = context.user.user_id - script_id = new_ulid() - jupyter_basename = _jupyter_path(script_type, script_id) - # Jupyter path inside workspace root: {user_id}/{parent_or_self}/{basename} - if parent_ulid: - jupyter_path = f"{user_id}/{parent_ulid}/{jupyter_basename}" + script_id = new_ulid() # PK only — kept as ULID + jupyter_basename = _jupyter_path(script_type, name) # name is already passed through safe_script_name + # The Jupyter path uses the parent directory's user-relative path + # (parent_dir_row.relative_path looks like "workspace/{user_id}/foo/bar"), + # NOT the parent's ULID. Strip the "workspace/{user_id}/" prefix to get + # the Jupyter-relative parent segment. + parent_relative = parent_dir_row.relative_path if parent_dir_row is not None else None + workspace_user_prefix = f"workspace/{user_id}" + if parent_relative and parent_relative.startswith(workspace_user_prefix + "/"): + parent_segment = parent_relative[len(workspace_user_prefix) + 1:] + else: + parent_segment = "" + if parent_segment: + jupyter_path = f"{user_id}/{parent_segment}/{jupyter_basename}" else: jupyter_path = f"{user_id}/{jupyter_basename}" name_clash = await session.scalar( @@ -430,9 +423,9 @@ async def create_script_record( # directory under it. ensure_directory is idempotent (GET-first pattern). try: await runtime_client.ensure_directory(workspace_id, user_id) - if parent_ulid: + if parent_segment: await runtime_client.ensure_directory( - workspace_id, f"{user_id}/{parent_ulid}" + workspace_id, f"{user_id}/{parent_segment}" ) except RuntimeClientError as exc: raise HTTPException( @@ -783,11 +776,12 @@ async def create_workspace_directory( relative_path = user_relative_path(context, child_path) scoped_prefix = user_relative_path(context) path_hash = hashlib.sha256(relative_path.encode("utf-8")).digest() + parent_dir_row = None if parent: parent_relative = f"{scoped_prefix}/{parent}" # Only an available directory row at the exact parent path counts as - # a parent. Capture its ULID so the new Jupyter directory can be nested - # under it on disk. + # a parent. Capture it so we can derive the new Jupyter path from its + # relative_path (which uses the user-supplied directory names). parent_dir_row = await session.scalar( select(StorageObjects).where( StorageObjects.workspace_id == context.workspace.workspace_id, @@ -801,9 +795,6 @@ async def create_workspace_directory( status.HTTP_404_NOT_FOUND, "parent directory not found", ) - parent_ulid = parent_dir_row.storage_object_id - else: - parent_ulid = None # Conflict check uses the unique index on (workspace_id, storage_backend, path_hash). # A soft-deleted row at the same path can be revived; an available row is a conflict. existing = await session.scalar( @@ -821,15 +812,22 @@ async def create_workspace_directory( "a file or directory with the same path already exists", ) - # Generate the Jupyter-facing directory ULID up front so the DB id and - # the on-disk directory name match. Create the directory in Jupyter + # The Jupyter-facing directory name is the sanitized user-supplied name. + # dir_id is still generated up front because the DB PK and path_hash + # collision-revival reuse depend on it. Create the directory in Jupyter # before persisting the DB row; if persistence fails we clean up. - dir_id = new_ulid() + dir_id = new_ulid() # PK only — not in the path anymore user_id = context.user.user_id - if parent_ulid: - jupyter_path = f"{user_id}/{parent_ulid}/{dir_id}" + parent_relative = parent_dir_row.relative_path if parent_dir_row is not None else None + workspace_user_prefix = f"workspace/{user_id}" + if parent_relative and parent_relative.startswith(workspace_user_prefix + "/"): + parent_segment = parent_relative[len(workspace_user_prefix) + 1:] else: - jupyter_path = f"{user_id}/{dir_id}" + parent_segment = "" + if parent_segment: + jupyter_path = f"{user_id}/{parent_segment}/{name}" + else: + jupyter_path = f"{user_id}/{name}" runtime_client = request.app.state.runtime_client workspace_id = context.workspace.workspace_id @@ -839,9 +837,9 @@ async def create_workspace_directory( # directory under it. ensure_directory is idempotent (GET-first pattern). try: await runtime_client.ensure_directory(workspace_id, user_id) - if parent_ulid: + if parent_segment: await runtime_client.ensure_directory( - workspace_id, f"{user_id}/{parent_ulid}" + workspace_id, f"{user_id}/{parent_segment}" ) except RuntimeClientError as exc: raise HTTPException( @@ -983,9 +981,18 @@ async def delete_workspace_directory( descendant.is_deleted = 1 descendant.deleted_at = datetime.now(UTC).replace(tzinfo=None) else: + # Sub-directory jupyter path: derive from relative_path (new rows carry + # the user-supplied name; legacy ULID-pathed rows fall back to the + # storage_object_id for backward compatibility). + descendant_relative = descendant.relative_path or "" + descendant_user_prefix = f"workspace/{descendant.owner_user_id}" + if descendant_relative.startswith(descendant_user_prefix + "/"): + desc_segment = descendant_relative[len(descendant_user_prefix) + 1:] + else: + desc_segment = descendant.storage_object_id try: await runtime_client.delete_directory( - workspace_id, name=f"{descendant.owner_user_id}/{descendant.storage_object_id}" + workspace_id, name=f"{descendant.owner_user_id}/{desc_segment}" ) except RuntimeClientError as exc: if exc.status_code != 404: @@ -998,8 +1005,15 @@ async def delete_workspace_directory( descendant.is_deleted = 1 descendant.deleted_at = datetime.now(UTC).replace(tzinfo=None) + # Target directory jupyter path: derive from the user-relative target + # path; fall back to the storage_object_id for legacy ULID-pathed rows. + target_user_prefix = f"workspace/{context.user.user_id}" + if target_relative.startswith(target_user_prefix + "/"): + target_segment = target_relative[len(target_user_prefix) + 1:] + else: + target_segment = target_ulid try: - await runtime_client.delete_directory(workspace_id, name=f"{context.user.user_id}/{target_ulid}") + await runtime_client.delete_directory(workspace_id, name=f"{context.user.user_id}/{target_segment}") except RuntimeClientError as exc: if exc.status_code != 404: logger.warning(