Skip to content

Commit c78861d

Browse files
committed
refactor(BA-4092): Move purge validation logic from service to repository
Move VFolderNotFound and VFolderInvalidParameter validation from VFolderService.purge() to VfolderRepository.purge_vfolder() for cleaner separation of concerns. - Repository now validates vfolder existence and purgable status - Service layer simplified to delegate to repository - Added unit tests for both service and repository layers
1 parent dcbed32 commit c78861d

File tree

6 files changed

+374
-16
lines changed

6 files changed

+374
-16
lines changed

src/ai/backend/manager/repositories/vfolder/repository.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,13 @@
5050
VFolderPermission,
5151
VFolderPermissionRow,
5252
VFolderRow,
53+
VFolderStatusSet,
5354
delete_vfolder_relation_rows,
5455
ensure_host_permission_allowed,
5556
get_sessions_by_mounted_folder,
5657
is_unmanaged,
5758
query_accessible_vfolders,
59+
vfolder_status_map,
5860
vfolders,
5961
)
6062
from ai.backend.manager.repositories.base.creator import Creator
@@ -413,18 +415,25 @@ async def delete_vfolders_forever(self, vfolder_ids: list[uuid.UUID]) -> list[VF
413415

414416
return [self._vfolder_row_to_data(row) for row in vfolder_rows]
415417

416-
417418
@vfolder_repository_resilience.apply()
418419
async def purge_vfolder(self, purger: Purger[VFolderRow]) -> VFolderData:
419420
"""
420421
Permanently delete a VFolder from DB.
421-
This should only be called for VFolders with DELETE_COMPLETE status.
422-
"""
422+
Only VFolders with purgable status (DELETE_PENDING, DELETE_COMPLETE) can be purged.
423+
424+
Raises:
425+
VFolderNotFound: If the vfolder doesn't exist.
426+
VFolderInvalidParameter: If the vfolder status is not purgable.
427+
"""
428+
vfolder_uuid = cast(uuid.UUID, purger.pk_value)
429+
vfolder_data = await self.get_by_id(vfolder_uuid)
430+
if vfolder_data is None:
431+
raise VFolderNotFound(extra_data=str(vfolder_uuid))
432+
if vfolder_data.status not in vfolder_status_map[VFolderStatusSet.PURGABLE]:
433+
raise VFolderInvalidParameter(f"Cannot purge vfolder with status {vfolder_data.status}")
423434
async with self._db.begin_session() as session:
424-
result = await execute_purger(session, purger)
425-
if result is None:
426-
raise VFolderNotFound(extra_data=str(purger.pk_value))
427-
return self._vfolder_row_to_data(result.row)
435+
await execute_purger(session, purger)
436+
return vfolder_data
428437

429438
@vfolder_repository_resilience.apply()
430439
async def get_vfolder_permissions(self, vfolder_id: uuid.UUID) -> list[VFolderPermissionData]:

src/ai/backend/manager/services/vfolder/services/vfolder.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -534,18 +534,9 @@ async def delete_forever(
534534
await self._remove_vfolder_from_storage(vfolder_data)
535535
return DeleteForeverVFolderActionResult(vfolder_uuid=action.vfolder_uuid)
536536

537-
538537
async def purge(self, action: PurgeVFolderAction) -> PurgeVFolderActionResult:
539538
"""Purge a DELETE_COMPLETE vfolder from DB (admin only)."""
540539
vfolder_uuid = cast(uuid.UUID, action.purger.pk_value)
541-
vfolder_data = await self._vfolder_repository.get_by_id(vfolder_uuid)
542-
if vfolder_data is None:
543-
raise VFolderNotFound(extra_data=str(vfolder_uuid))
544-
if vfolder_data.status not in VFolderStatusSet.PURGABLE:
545-
raise VFolderInvalidParameter(
546-
f"Cannot purge vfolder with status {vfolder_data.status}"
547-
)
548-
549540
await self._vfolder_repository.purge_vfolder(action.purger)
550541
return PurgeVFolderActionResult(vfolder_uuid=vfolder_uuid)
551542

tests/unit/manager/repositories/vfolder/test_vfolder_repository.py

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,3 +469,215 @@ async def test_get_allowed_vfolder_hosts_returns_vfolder_host_permission_map(
469469
)
470470

471471
assert isinstance(result, VFolderHostPermissionMap)
472+
473+
474+
class TestVfolderRepositoryPurge:
475+
"""Tests for VfolderRepository.purge_vfolder() method"""
476+
477+
@pytest.fixture
478+
async def db_with_cleanup(
479+
self,
480+
database_connection: ExtendedAsyncSAEngine,
481+
) -> AsyncGenerator[ExtendedAsyncSAEngine, None]:
482+
"""Database connection with tables created."""
483+
async with with_tables(
484+
database_connection,
485+
[
486+
DomainRow,
487+
UserResourcePolicyRow,
488+
ProjectResourcePolicyRow,
489+
KeyPairResourcePolicyRow,
490+
UserRoleRow,
491+
UserRow,
492+
KeyPairRow,
493+
GroupRow,
494+
VFolderRow,
495+
],
496+
):
497+
yield database_connection
498+
499+
@pytest.fixture
500+
async def test_domain_name(
501+
self,
502+
db_with_cleanup: ExtendedAsyncSAEngine,
503+
) -> str:
504+
"""Create test domain."""
505+
domain_name = f"test-domain-{uuid.uuid4().hex[:8]}"
506+
507+
async with db_with_cleanup.begin_session() as db_sess:
508+
domain = DomainRow(
509+
name=domain_name,
510+
description="Test domain",
511+
is_active=True,
512+
total_resource_slots={},
513+
allowed_vfolder_hosts={},
514+
allowed_docker_registries=[],
515+
)
516+
db_sess.add(domain)
517+
await db_sess.flush()
518+
519+
return domain_name
520+
521+
@pytest.fixture
522+
async def test_user_resource_policy_name(
523+
self,
524+
db_with_cleanup: ExtendedAsyncSAEngine,
525+
) -> str:
526+
"""Create test user resource policy."""
527+
policy_name = f"test-user-policy-{uuid.uuid4().hex[:8]}"
528+
529+
async with db_with_cleanup.begin_session() as db_sess:
530+
policy = UserResourcePolicyRow(
531+
name=policy_name,
532+
max_vfolder_count=10,
533+
max_quota_scope_size=BinarySize.finite_from_str("10GiB"),
534+
max_session_count_per_model_session=5,
535+
max_customized_image_count=3,
536+
)
537+
db_sess.add(policy)
538+
await db_sess.flush()
539+
540+
return policy_name
541+
542+
@pytest.fixture
543+
async def test_user(
544+
self,
545+
db_with_cleanup: ExtendedAsyncSAEngine,
546+
test_domain_name: str,
547+
test_user_resource_policy_name: str,
548+
) -> uuid.UUID:
549+
"""Create test user."""
550+
user_uuid = uuid.uuid4()
551+
552+
password_info = PasswordInfo(
553+
password="dummy",
554+
algorithm=PasswordHashAlgorithm.PBKDF2_SHA256,
555+
rounds=600_000,
556+
salt_size=32,
557+
)
558+
559+
async with db_with_cleanup.begin_session() as db_sess:
560+
user = UserRow(
561+
uuid=user_uuid,
562+
username=f"testuser-{user_uuid.hex[:8]}",
563+
email=f"test-{user_uuid.hex[:8]}@example.com",
564+
password=password_info,
565+
need_password_change=False,
566+
status=UserStatus.ACTIVE,
567+
status_info="active",
568+
domain_name=test_domain_name,
569+
role=UserRole.USER,
570+
resource_policy=test_user_resource_policy_name,
571+
)
572+
db_sess.add(user)
573+
await db_sess.flush()
574+
575+
return user_uuid
576+
577+
@pytest.fixture
578+
async def vfolder_repository(
579+
self,
580+
db_with_cleanup: ExtendedAsyncSAEngine,
581+
) -> VfolderRepository:
582+
"""Create VfolderRepository instance."""
583+
return VfolderRepository(db=db_with_cleanup)
584+
585+
async def _create_vfolder_in_db(
586+
self,
587+
db: ExtendedAsyncSAEngine,
588+
*,
589+
vfolder_id: uuid.UUID,
590+
domain_name: str,
591+
user_id: uuid.UUID,
592+
status: VFolderOperationStatus,
593+
) -> None:
594+
"""Helper to create a vfolder directly in DB."""
595+
async with db.begin_session() as db_sess:
596+
vfolder = VFolderRow(
597+
id=vfolder_id,
598+
name=f"test-vfolder-{vfolder_id.hex[:8]}",
599+
host="local:volume1",
600+
domain_name=domain_name,
601+
quota_scope_id=f"user:{user_id}",
602+
usage_mode=VFolderUsageMode.GENERAL,
603+
permission=VFolderMountPermission.READ_WRITE,
604+
max_files=0,
605+
max_size=None,
606+
num_files=0,
607+
cur_size=0,
608+
creator=f"test-{user_id.hex[:8]}@example.com",
609+
unmanaged_path=None,
610+
ownership_type=VFolderOwnershipType.USER,
611+
user=user_id,
612+
group=None,
613+
cloneable=False,
614+
status=status,
615+
)
616+
db_sess.add(vfolder)
617+
await db_sess.flush()
618+
619+
async def _vfolder_exists(self, db: ExtendedAsyncSAEngine, vfolder_id: uuid.UUID) -> bool:
620+
"""Check if vfolder exists in DB."""
621+
import sqlalchemy as sa
622+
623+
async with db.begin_readonly_session() as session:
624+
query = sa.select(VFolderRow.id).where(VFolderRow.id == vfolder_id)
625+
result = await session.execute(query)
626+
return result.scalar_one_or_none() is not None
627+
628+
@pytest.mark.asyncio
629+
@pytest.mark.parametrize(
630+
"status",
631+
[
632+
VFolderOperationStatus.DELETE_COMPLETE,
633+
VFolderOperationStatus.DELETE_PENDING,
634+
],
635+
ids=["delete_complete", "delete_pending"],
636+
)
637+
async def test_purge_vfolder_success(
638+
self,
639+
db_with_cleanup: ExtendedAsyncSAEngine,
640+
vfolder_repository: VfolderRepository,
641+
test_domain_name: str,
642+
test_user: uuid.UUID,
643+
status: VFolderOperationStatus,
644+
) -> None:
645+
"""Test successful purge of vfolder with purgable status."""
646+
from ai.backend.manager.repositories.base.purger import Purger
647+
648+
vfolder_id = uuid.uuid4()
649+
await self._create_vfolder_in_db(
650+
db_with_cleanup,
651+
vfolder_id=vfolder_id,
652+
domain_name=test_domain_name,
653+
user_id=test_user,
654+
status=status,
655+
)
656+
657+
# Verify vfolder exists before purge
658+
assert await self._vfolder_exists(db_with_cleanup, vfolder_id)
659+
660+
purger = Purger(row_class=VFolderRow, pk_value=vfolder_id)
661+
result = await vfolder_repository.purge_vfolder(purger)
662+
663+
# Verify result contains correct data
664+
assert result.id == vfolder_id
665+
assert result.status == status
666+
667+
# Verify vfolder is deleted from DB
668+
assert not await self._vfolder_exists(db_with_cleanup, vfolder_id)
669+
670+
@pytest.mark.asyncio
671+
async def test_purge_vfolder_not_found(
672+
self,
673+
vfolder_repository: VfolderRepository,
674+
) -> None:
675+
"""Test purge fails when vfolder doesn't exist."""
676+
from ai.backend.manager.errors.storage import VFolderNotFound
677+
from ai.backend.manager.repositories.base.purger import Purger
678+
679+
non_existent_id = uuid.uuid4()
680+
purger = Purger(row_class=VFolderRow, pk_value=non_existent_id)
681+
682+
with pytest.raises(VFolderNotFound):
683+
await vfolder_repository.purge_vfolder(purger)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
python_tests(name="tests")

tests/unit/manager/services/vfolder/__init__.py

Whitespace-only changes.

0 commit comments

Comments
 (0)