Skip to content

Commit 0d1a512

Browse files
jopemachineclaude
andcommitted
refactor(BA-6156): type the upload session id as TusSessionId
UploadTokenData.session is now typed as the common TusSessionId NewType so the session id flows type-safely from the token through the handlers into the Valkey-backed engine. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ff94c53 commit 0d1a512

2 files changed

Lines changed: 76 additions & 22 deletions

File tree

src/ai/backend/storage/api/client.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
from ai.backend.common.metrics.http import build_api_metric_middleware
4343
from ai.backend.common.middlewares.exception import general_exception_middleware
4444
from ai.backend.common.typed_validators import PydanticJWTValidator
45-
from ai.backend.common.types import BinarySize, VFolderID
45+
from ai.backend.common.types import BinarySize, TusSessionId, VFolderID
4646
from ai.backend.logging import BraceStyleAdapter
4747
from ai.backend.storage import __version__
4848
from ai.backend.storage.dto.context import StorageRootCtx
@@ -99,7 +99,7 @@ class UploadTokenData(TypedDict):
9999
volume: str
100100
vfid: VFolderID
101101
relpath: str
102-
session: str
102+
session: TusSessionId
103103
size: int
104104

105105

@@ -349,10 +349,11 @@ class Params(TypedDict):
349349
session_id=token_data["session"],
350350
total_size=int(token_data["size"]),
351351
valkey_client=ctx.valkey_tus_client,
352+
lock_factory=ctx.tus_lock_factory,
352353
)
353354
)
354355
state = await session.read_state()
355-
headers = _tus_response_headers(
356+
headers = _prepare_tus_session_headers(
356357
upload_offset=state.committed_offset,
357358
upload_length=int(token_data["size"]),
358359
)
@@ -428,6 +429,7 @@ class Params(TypedDict):
428429
session_id=token_data["session"],
429430
total_size=total_size,
430431
valkey_client=ctx.valkey_tus_client,
432+
lock_factory=ctx.tus_lock_factory,
431433
)
432434
)
433435
await session.ensure_initialized()
@@ -464,7 +466,7 @@ class Params(TypedDict):
464466
await session.assemble(target_path)
465467
await session.cleanup()
466468

467-
headers = _tus_response_headers(
469+
headers = _prepare_tus_session_headers(
468470
upload_offset=state.committed_offset,
469471
upload_length=total_size,
470472
)
@@ -478,7 +480,7 @@ def _resolve_tus_upload_session_dir(volume: AbstractVolume, token_data: UploadTo
478480
_TUS_HEADER_LIST = "Tus-Resumable, Upload-Length, Upload-Metadata, Upload-Offset, Content-Type"
479481

480482

481-
def _tus_response_headers(*, upload_offset: int, upload_length: int) -> dict[str, str]:
483+
def _prepare_tus_session_headers(*, upload_offset: int, upload_length: int) -> dict[str, str]:
482484
return {
483485
"Access-Control-Allow-Origin": "*",
484486
"Access-Control-Allow-Headers": _TUS_HEADER_LIST,

tests/unit/storage/api/test_tus_upload.py

Lines changed: 69 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@
2424
from ai.backend.common import config
2525
from ai.backend.common.clients.valkey_client.valkey_tus import ValkeyTusClient
2626
from ai.backend.common.defs import REDIS_STREAM_LOCK, REDIS_TUS_DB
27+
from ai.backend.common.lock import DistributedLockFactory
2728
from ai.backend.common.typed_validators import HostPortPair as HostPortPairModel
2829
from ai.backend.common.types import RedisConnectionInfo, ValkeyTarget
2930
from ai.backend.storage.api.client import tus_upload_part
3031
from ai.backend.storage.errors import InvalidAPIParameters, UploadOffsetMismatchError
32+
from ai.backend.storage.services.upload.tus_session import create_tus_lock_factory
3133
from ai.backend.testutils.bootstrap import redis_container # noqa: F401
3234

3335

@@ -36,25 +38,35 @@ async def valkey_tus_client(
3638
redis_container: tuple[str, HostPortPairModel], # noqa: F811
3739
) -> AsyncIterator[ValkeyTusClient]:
3840
hostport_pair = redis_container[1]
39-
lock_redis = RedisConnectionInfo(
40-
Redis.from_url(f"redis://{hostport_pair.address}/{REDIS_STREAM_LOCK}"),
41-
sentinel=None,
42-
name="test.tus.api.lock",
43-
service_name=None,
44-
redis_helper_config=config.redis_helper_default_config,
45-
)
4641
client = await ValkeyTusClient.create(
4742
ValkeyTarget(addr=hostport_pair.address),
4843
db_id=REDIS_TUS_DB,
4944
human_readable_name="test.tus.api",
50-
lock_redis=lock_redis,
5145
)
5246
try:
5347
yield client
5448
finally:
5549
await client.close()
5650

5751

52+
@pytest.fixture
53+
async def tus_lock_factory(
54+
redis_container: tuple[str, HostPortPairModel], # noqa: F811
55+
) -> AsyncIterator[DistributedLockFactory]:
56+
hostport_pair = redis_container[1]
57+
lock_redis = RedisConnectionInfo(
58+
Redis.from_url(f"redis://{hostport_pair.address}/{REDIS_STREAM_LOCK}"),
59+
sentinel=None,
60+
name="test.tus.api.lock",
61+
service_name=None,
62+
redis_helper_config=config.redis_helper_default_config,
63+
)
64+
try:
65+
yield create_tus_lock_factory(lock_redis)
66+
finally:
67+
await lock_redis.close()
68+
69+
5870
@dataclasses.dataclass(slots=True)
5971
class _PatchEnv:
6072
vfpath: Path
@@ -70,6 +82,7 @@ def _build_request(
7082
body: bytes | None,
7183
offset_header: str | None,
7284
valkey_client: ValkeyTusClient,
85+
lock_factory: DistributedLockFactory,
7386
) -> MagicMock:
7487
volume = MagicMock()
7588
volume.mangle_vfpath.return_value = vfpath
@@ -79,6 +92,7 @@ def _build_request(
7992
ctx.get_volume.return_value.__aenter__ = AsyncMock(return_value=volume)
8093
ctx.get_volume.return_value.__aexit__ = AsyncMock(return_value=None)
8194
ctx.valkey_tus_client = valkey_client
95+
ctx.tus_lock_factory = lock_factory
8296

8397
request = MagicMock(spec=web.Request)
8498
request.app = {"ctx": ctx}
@@ -144,7 +158,10 @@ def _patch_handler_params(token_data: dict[str, Any]) -> Any:
144158

145159
class TestUploadOffsetHeaderValidation:
146160
async def test_missing_offset_header_raises(
147-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
161+
self,
162+
patch_env: _PatchEnv,
163+
valkey_tus_client: ValkeyTusClient,
164+
tus_lock_factory: DistributedLockFactory,
148165
) -> None:
149166
request = _build_request(
150167
vfpath=patch_env.vfpath,
@@ -153,6 +170,7 @@ async def test_missing_offset_header_raises(
153170
body=None,
154171
offset_header=None,
155172
valkey_client=valkey_tus_client,
173+
lock_factory=tus_lock_factory,
156174
)
157175
token_data = _token_data(session_id=patch_env.session_id, total_size=1024, relpath="f.bin")
158176
cp = _patch_handler_params(token_data)
@@ -163,7 +181,10 @@ async def test_missing_offset_header_raises(
163181
cp.stop()
164182

165183
async def test_non_integer_offset_header_raises(
166-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
184+
self,
185+
patch_env: _PatchEnv,
186+
valkey_tus_client: ValkeyTusClient,
187+
tus_lock_factory: DistributedLockFactory,
167188
) -> None:
168189
request = _build_request(
169190
vfpath=patch_env.vfpath,
@@ -172,6 +193,7 @@ async def test_non_integer_offset_header_raises(
172193
body=None,
173194
offset_header="not-a-number",
174195
valkey_client=valkey_tus_client,
196+
lock_factory=tus_lock_factory,
175197
)
176198
token_data = _token_data(session_id=patch_env.session_id, total_size=1024, relpath="f.bin")
177199
cp = _patch_handler_params(token_data)
@@ -182,7 +204,10 @@ async def test_non_integer_offset_header_raises(
182204
cp.stop()
183205

184206
async def test_negative_offset_raises_conflict(
185-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
207+
self,
208+
patch_env: _PatchEnv,
209+
valkey_tus_client: ValkeyTusClient,
210+
tus_lock_factory: DistributedLockFactory,
186211
) -> None:
187212
request = _build_request(
188213
vfpath=patch_env.vfpath,
@@ -191,6 +216,7 @@ async def test_negative_offset_raises_conflict(
191216
body=None,
192217
offset_header="-1",
193218
valkey_client=valkey_tus_client,
219+
lock_factory=tus_lock_factory,
194220
)
195221
token_data = _token_data(session_id=patch_env.session_id, total_size=1024, relpath="f.bin")
196222
cp = _patch_handler_params(token_data)
@@ -201,7 +227,10 @@ async def test_negative_offset_raises_conflict(
201227
cp.stop()
202228

203229
async def test_offset_above_total_size_raises_conflict(
204-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
230+
self,
231+
patch_env: _PatchEnv,
232+
valkey_tus_client: ValkeyTusClient,
233+
tus_lock_factory: DistributedLockFactory,
205234
) -> None:
206235
request = _build_request(
207236
vfpath=patch_env.vfpath,
@@ -210,6 +239,7 @@ async def test_offset_above_total_size_raises_conflict(
210239
body=None,
211240
offset_header="2048",
212241
valkey_client=valkey_tus_client,
242+
lock_factory=tus_lock_factory,
213243
)
214244
token_data = _token_data(session_id=patch_env.session_id, total_size=1024, relpath="f.bin")
215245
cp = _patch_handler_params(token_data)
@@ -222,7 +252,10 @@ async def test_offset_above_total_size_raises_conflict(
222252

223253
class TestSessionNotFound:
224254
async def test_missing_session_dir_raises_not_found(
225-
self, tmp_path: Path, valkey_tus_client: ValkeyTusClient
255+
self,
256+
tmp_path: Path,
257+
valkey_tus_client: ValkeyTusClient,
258+
tus_lock_factory: DistributedLockFactory,
226259
) -> None:
227260
vfpath = tmp_path / "vfpath"
228261
vfpath.mkdir()
@@ -235,6 +268,7 @@ async def test_missing_session_dir_raises_not_found(
235268
body=b"",
236269
offset_header="0",
237270
valkey_client=valkey_tus_client,
271+
lock_factory=tus_lock_factory,
238272
)
239273
token_data = _token_data(session_id="missing-session", total_size=1024, relpath="f.bin")
240274
cp = _patch_handler_params(token_data)
@@ -247,7 +281,10 @@ async def test_missing_session_dir_raises_not_found(
247281

248282
class TestHappyPath:
249283
async def test_single_chunk_upload_completes_and_assembles(
250-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
284+
self,
285+
patch_env: _PatchEnv,
286+
valkey_tus_client: ValkeyTusClient,
287+
tus_lock_factory: DistributedLockFactory,
251288
) -> None:
252289
payload = b"hello world" * 100
253290
request = _build_request(
@@ -257,6 +294,7 @@ async def test_single_chunk_upload_completes_and_assembles(
257294
body=payload,
258295
offset_header="0",
259296
valkey_client=valkey_tus_client,
297+
lock_factory=tus_lock_factory,
260298
)
261299
token_data = _token_data(
262300
session_id=patch_env.session_id,
@@ -277,7 +315,10 @@ async def test_single_chunk_upload_completes_and_assembles(
277315
assert list((patch_env.session_dir / "chunks").glob("*.dat")) == []
278316

279317
async def test_two_chunks_assemble_in_order(
280-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
318+
self,
319+
patch_env: _PatchEnv,
320+
valkey_tus_client: ValkeyTusClient,
321+
tus_lock_factory: DistributedLockFactory,
281322
) -> None:
282323
token_data = _token_data(
283324
session_id=patch_env.session_id, total_size=2048, relpath="result.bin"
@@ -292,6 +333,7 @@ async def test_two_chunks_assemble_in_order(
292333
body=b"A" * 1024,
293334
offset_header="0",
294335
valkey_client=valkey_tus_client,
336+
lock_factory=tus_lock_factory,
295337
)
296338
await tus_upload_part(first)
297339

@@ -302,6 +344,7 @@ async def test_two_chunks_assemble_in_order(
302344
body=b"B" * 1024,
303345
offset_header="1024",
304346
valkey_client=valkey_tus_client,
347+
lock_factory=tus_lock_factory,
305348
)
306349
response = await tus_upload_part(second)
307350
finally:
@@ -312,7 +355,10 @@ async def test_two_chunks_assemble_in_order(
312355
assert final_path.read_bytes() == b"A" * 1024 + b"B" * 1024
313356

314357
async def test_duplicate_chunk_replay_is_idempotent(
315-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
358+
self,
359+
patch_env: _PatchEnv,
360+
valkey_tus_client: ValkeyTusClient,
361+
tus_lock_factory: DistributedLockFactory,
316362
) -> None:
317363
token_data = _token_data(
318364
session_id=patch_env.session_id, total_size=2048, relpath="result.bin"
@@ -327,6 +373,7 @@ async def test_duplicate_chunk_replay_is_idempotent(
327373
body=payload,
328374
offset_header="0",
329375
valkey_client=valkey_tus_client,
376+
lock_factory=tus_lock_factory,
330377
)
331378
await tus_upload_part(first)
332379

@@ -338,6 +385,7 @@ async def test_duplicate_chunk_replay_is_idempotent(
338385
body=payload,
339386
offset_header="0",
340387
valkey_client=valkey_tus_client,
388+
lock_factory=tus_lock_factory,
341389
)
342390
response = await tus_upload_part(replay)
343391
finally:
@@ -346,7 +394,10 @@ async def test_duplicate_chunk_replay_is_idempotent(
346394
assert response.headers["Upload-Offset"] == "1024"
347395

348396
async def test_chunk_exceeding_declared_size_raises(
349-
self, patch_env: _PatchEnv, valkey_tus_client: ValkeyTusClient
397+
self,
398+
patch_env: _PatchEnv,
399+
valkey_tus_client: ValkeyTusClient,
400+
tus_lock_factory: DistributedLockFactory,
350401
) -> None:
351402
token_data = _token_data(
352403
session_id=patch_env.session_id, total_size=10, relpath="result.bin"
@@ -358,6 +409,7 @@ async def test_chunk_exceeding_declared_size_raises(
358409
body=b"too-much-data", # 13 bytes > 10
359410
offset_header="0",
360411
valkey_client=valkey_tus_client,
412+
lock_factory=tus_lock_factory,
361413
)
362414
cp = _patch_handler_params(token_data)
363415
try:

0 commit comments

Comments
 (0)