fix: update_script rewrite metadata
This commit is contained in:
@@ -1196,13 +1196,34 @@ async def update_script(
|
|||||||
) from exc
|
) from exc
|
||||||
|
|
||||||
script.updated_at = datetime.now(UTC).replace(tzinfo=None)
|
script.updated_at = datetime.now(UTC).replace(tzinfo=None)
|
||||||
storage_data = {
|
content_hash = hashlib.sha256(content).hexdigest()
|
||||||
"storage_object_id": script.current_object_id,
|
size_bytes = len(content)
|
||||||
"relative_path": jupyter_path,
|
relative_path = f"workspace/{jupyter_path}"
|
||||||
"object_key": f"{workspace_id}/{jupyter_path}",
|
|
||||||
"content_hash": hashlib.sha256(content).hexdigest(),
|
# Persist the new content fingerprint so cache/dedup/hash checks
|
||||||
"size_bytes": len(content),
|
# downstream see the post-edit values. Filename is fixed in
|
||||||
}
|
# update_script, so object_key / path_hash are unchanged — only the
|
||||||
|
# body-derived fields move. relative_path is re-derived to keep the
|
||||||
|
# response in sync with the workspace-prefixed convention used at
|
||||||
|
# create time.
|
||||||
|
if storage_object is not None:
|
||||||
|
storage_object.content_hash = content_hash
|
||||||
|
storage_object.size_bytes = size_bytes
|
||||||
|
storage_object.relative_path = relative_path
|
||||||
|
await session.flush()
|
||||||
|
await session.refresh(storage_object)
|
||||||
|
storage_data: StorageObjects | dict[str, Any] = storage_object
|
||||||
|
else:
|
||||||
|
# Jupyter-only script: no StorageObjects row to update, but the
|
||||||
|
# response still needs the workspace-prefixed path shape so the
|
||||||
|
# frontend's slice(2) reducer produces the expected basename.
|
||||||
|
storage_data = {
|
||||||
|
"storage_object_id": script.current_object_id,
|
||||||
|
"relative_path": relative_path,
|
||||||
|
"object_key": f"{workspace_id}/{jupyter_path}",
|
||||||
|
"content_hash": content_hash,
|
||||||
|
"size_bytes": size_bytes,
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
"request_id": context.request_id,
|
"request_id": context.request_id,
|
||||||
"data": script_payload(script, storage_data),
|
"data": script_payload(script, storage_data),
|
||||||
|
|||||||
@@ -343,6 +343,8 @@ def test_scripts_model_allows_duplicate_current_object_id() -> None:
|
|||||||
|
|
||||||
|
|
||||||
def _script_row() -> Scripts:
|
def _script_row() -> Scripts:
|
||||||
|
from datetime import UTC, datetime
|
||||||
|
|
||||||
return Scripts(
|
return Scripts(
|
||||||
script_id="01SCR0000000000000000000A",
|
script_id="01SCR0000000000000000000A",
|
||||||
workspace_id="01WS0000000000000000000A",
|
workspace_id="01WS0000000000000000000A",
|
||||||
@@ -352,6 +354,8 @@ def _script_row() -> Scripts:
|
|||||||
script_type="notebook",
|
script_type="notebook",
|
||||||
visibility="private",
|
visibility="private",
|
||||||
status="active",
|
status="active",
|
||||||
|
created_at=datetime.now(UTC).replace(tzinfo=None),
|
||||||
|
updated_at=datetime.now(UTC).replace(tzinfo=None),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -516,3 +520,147 @@ async def test_jupyter_check_notebook_lock_ignores_deleted_scripts() -> None:
|
|||||||
statement = call[0][0]
|
statement = call[0][0]
|
||||||
compiled = str(statement.compile(compile_kwargs={"literal_binds": True}))
|
compiled = str(statement.compile(compile_kwargs={"literal_binds": True}))
|
||||||
assert "is_deleted" in compiled
|
assert "is_deleted" in compiled
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_update_script_writes_back_storage_object_metadata(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
"""update_script must persist the new content_hash/size_bytes/relative_path
|
||||||
|
onto the StorageObjects row (not just into the response dict). Pre-fix the
|
||||||
|
DB row kept the create-time values forever, so cache/dedup/hash checks
|
||||||
|
downstream saw stale data.
|
||||||
|
|
||||||
|
Also locks in the workspace-prefixed relative_path convention so the
|
||||||
|
frontend's slice(2) reducer produces a non-empty path even for root scripts.
|
||||||
|
"""
|
||||||
|
import hashlib
|
||||||
|
|
||||||
|
from backend.schemas import UpdateScriptRequest
|
||||||
|
from backend.scripts import update_script
|
||||||
|
|
||||||
|
script = _script_row()
|
||||||
|
storage_object = _storage_object_row()
|
||||||
|
# Seed stale metadata as it would have looked after create_script_record.
|
||||||
|
user_id = "01USR0000000000000000000A"
|
||||||
|
storage_object.object_key = f"{script.workspace_id}/{user_id}/test.ipynb"
|
||||||
|
storage_object.content_hash = hashlib.sha256(b"old").hexdigest()
|
||||||
|
storage_object.size_bytes = 3
|
||||||
|
storage_object.relative_path = f"workspace/{user_id}/test.ipynb"
|
||||||
|
|
||||||
|
request = _make_request()
|
||||||
|
runtime_client = request.app.state.runtime_client
|
||||||
|
runtime_client.upload_file = AsyncMock(return_value={"name": "test.ipynb"})
|
||||||
|
runtime_client.create_notebook = AsyncMock(return_value={"name": "test.ipynb"})
|
||||||
|
context = _make_context()
|
||||||
|
session = AsyncMock()
|
||||||
|
session.flush = AsyncMock()
|
||||||
|
session.refresh = AsyncMock()
|
||||||
|
|
||||||
|
async def _fake_get_script_row(
|
||||||
|
_script_id: str,
|
||||||
|
_context: SimpleNamespace,
|
||||||
|
_session: AsyncMock,
|
||||||
|
*,
|
||||||
|
for_update: bool = False,
|
||||||
|
allow_missing_storage_object: bool = False,
|
||||||
|
) -> tuple[Scripts, StorageObjects]:
|
||||||
|
return script, storage_object
|
||||||
|
|
||||||
|
monkeypatch.setattr("backend.scripts.get_script_row", _fake_get_script_row)
|
||||||
|
|
||||||
|
new_content = '{"cells": [{"cell_type": "code", "source": ["print(1)"]}]}\n'
|
||||||
|
payload = UpdateScriptRequest(content=new_content)
|
||||||
|
|
||||||
|
result = await update_script(
|
||||||
|
script_id=script.script_id,
|
||||||
|
payload=payload,
|
||||||
|
request=request,
|
||||||
|
context=context,
|
||||||
|
session=session,
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_hash = hashlib.sha256(new_content.encode("utf-8")).hexdigest()
|
||||||
|
expected_size = len(new_content.encode("utf-8"))
|
||||||
|
|
||||||
|
# DB row carries the new metadata.
|
||||||
|
assert storage_object.content_hash == expected_hash
|
||||||
|
assert storage_object.size_bytes == expected_size
|
||||||
|
assert storage_object.relative_path == f"workspace/{user_id}/test.ipynb"
|
||||||
|
|
||||||
|
# Response echoes the same values.
|
||||||
|
data = result["data"]
|
||||||
|
assert data["content_hash"] == expected_hash
|
||||||
|
assert data["size_bytes"] == expected_size
|
||||||
|
assert data["relative_path"] == f"workspace/{user_id}/test.ipynb"
|
||||||
|
|
||||||
|
# session.flush + refresh were called to push the new metadata to DB.
|
||||||
|
session.flush.assert_awaited_once()
|
||||||
|
session.refresh.assert_awaited_once_with(storage_object)
|
||||||
|
# The runtime client received the create_notebook for the Jupyter path
|
||||||
|
# (script_type='notebook' goes through create_notebook, not upload_file).
|
||||||
|
runtime_client.create_notebook.assert_awaited_once()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_update_script_jupyter_only_uses_dict_fallback(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
"""When a script has no StorageObjects row (jupyter-only fallback path),
|
||||||
|
update_script must still return the workspace-prefixed relative_path
|
||||||
|
so the frontend's slice(2) reducer yields the basename — not an empty
|
||||||
|
string. No DB write should be attempted when there is no row to update.
|
||||||
|
"""
|
||||||
|
import hashlib
|
||||||
|
|
||||||
|
from backend.schemas import UpdateScriptRequest
|
||||||
|
from backend.scripts import update_script
|
||||||
|
|
||||||
|
script = _script_row()
|
||||||
|
user_id = "01USR0000000000000000000A"
|
||||||
|
|
||||||
|
request = _make_request()
|
||||||
|
runtime_client = request.app.state.runtime_client
|
||||||
|
runtime_client.upload_file = AsyncMock(return_value={"name": "test.ipynb"})
|
||||||
|
runtime_client.create_notebook = AsyncMock(return_value={"name": "test.ipynb"})
|
||||||
|
context = _make_context()
|
||||||
|
session = AsyncMock()
|
||||||
|
session.flush = AsyncMock()
|
||||||
|
session.refresh = AsyncMock()
|
||||||
|
|
||||||
|
async def _fake_get_script_row(
|
||||||
|
_script_id: str,
|
||||||
|
_context: SimpleNamespace,
|
||||||
|
_session: AsyncMock,
|
||||||
|
*,
|
||||||
|
for_update: bool = False,
|
||||||
|
allow_missing_storage_object: bool = False,
|
||||||
|
) -> tuple[Scripts, None]:
|
||||||
|
return script, None
|
||||||
|
|
||||||
|
monkeypatch.setattr("backend.scripts.get_script_row", _fake_get_script_row)
|
||||||
|
|
||||||
|
payload = UpdateScriptRequest(content='{"cells": []}\n')
|
||||||
|
result = await update_script(
|
||||||
|
script_id=script.script_id,
|
||||||
|
payload=payload,
|
||||||
|
request=request,
|
||||||
|
context=context,
|
||||||
|
session=session,
|
||||||
|
)
|
||||||
|
|
||||||
|
expected_hash = hashlib.sha256(b'{"cells": []}\n').hexdigest()
|
||||||
|
|
||||||
|
data = result["data"]
|
||||||
|
# Jupyter-only fallback derives jupyter_path from the script_id via
|
||||||
|
# _jupyter_path(), which appends ".ipynb". The contract we lock in is
|
||||||
|
# "always workspace-prefixed, never bare user_id/basename" so the
|
||||||
|
# frontend's slice(2) reducer is safe.
|
||||||
|
assert data["relative_path"] == f"workspace/{script.script_id}.ipynb"
|
||||||
|
assert data["relative_path"].startswith("workspace/")
|
||||||
|
assert data["content_hash"] == expected_hash
|
||||||
|
assert data["size_bytes"] == len(b'{"cells": []}\n')
|
||||||
|
|
||||||
|
# No DB write should be attempted when there is no StorageObjects row.
|
||||||
|
session.flush.assert_not_awaited()
|
||||||
|
session.refresh.assert_not_awaited()
|
||||||
|
|||||||
Reference in New Issue
Block a user