Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Repository Layer for vFolder CRUD APIs in Manager #3558

Open
seedspirit opened this issue Feb 3, 2025 — with Lablup-Issue-Syncer · 0 comments · May be fixed by #3669
Open

Add Repository Layer for vFolder CRUD APIs in Manager #3558

seedspirit opened this issue Feb 3, 2025 — with Lablup-Issue-Syncer · 0 comments · May be fixed by #3669
Assignees

Comments

@seedspirit
Copy link
Contributor

seedspirit commented Feb 3, 2025

Motivation  

Currently, application performs database operations directly in the handler layer. This architecture presents several challenges:

  1. Database Connection Management: It becomes difficult to efficiently manage and configure database connections when they are scattered throughout the handler layer.
  2. Limited Maintainability: Making modifications to database-related methods becomes cumbersome and error-prone since the logic is not centralized.
  3. Business Logic Constraints: The current structure makes it challenging to write and test pure business logic independently of database operations.
  4. Potential Side Effects: Since the request object contains direct database access capabilities, database operations can be performed from any location in the code, potentially leading to unexpected side effects.

Also some database-related methods are functioning as "super methods" that accept a wide range of parameters and perform multiple responsibilities in a single operation. This increases code coupling and reduces maintainability, necessitating the decomposition of these methods into smaller, focused methods within the repository layer.

Required Features

To address these issues, we need to implement repository layer that will:

  • Design layer that injected minimum dependencies (not root context)
  • Encapsulate all database access logic (focusing CRUD in this issue)
  • Analyze ‘super method’ and split them into small methods

Encapsulate all database access logic (focusing CRUD in this issue)

@dataclass
class VFolderHostPermissions:
    """Represents permissions for different scopes (domain, group, user, resource policy)."""
    domain: VFolderHostPermissionMap = field(default_factory=VFolderHostPermissionMap)
    group: VFolderHostPermissionMap = field(default_factory=VFolderHostPermissionMap)
    user: VFolderHostPermissionMap = field(default_factory=VFolderHostPermissionMap)
    resource_policy: VFolderHostPermissionMap

    def __init__(self, resource_policy: Mapping[str, Any]):
        permission_map = VFolderHostPermissionMap()
        self.resource_policy = permission_map | resource_policy["allowed_vfolder_hosts"]

    def _merge_allowed_hosts(self) -> VFolderHostPermissionMap:
        result = VFolderHostPermissionMap()
        result |= self.domain
        result |= self.group
        result |= self.user
        result |= self.resource_policy

        return result

    def check_permission(self, folder_host: str, permission: VFolderPermission) -> bool:
        allowed_hosts: VFolderHostPermissionMap = self._merge_allowed_hosts()
        if folder_host not in allowed_hosts or permission not in allowed_hosts[folder_host]:
            raise InvalidAPIParameters(f"`{permission}` Not allowed in vfolder host(`{folder_host}`)")

VFolderPermissionOption: TypeAlias = Callable[[SAConnection, VFolderHostPermissions], Awaitable[VFolderHostPermissions]]


class VFolderRepository:
    def __init__(self, db: ExtendedAsyncSAEngine) -> None:
        self.db = db


    async def _vfolder_permission_domain(self, domain_name: str) -> VFolderPermissionOption:
        async def check_domain(conn: SAConnection, permissions: VFolderHostPermissions) -> VFolderHostPermissions:
            from . import domains

            query = sa.select([domains.c.allowed_vfolder_hosts]).where(
                (domains.c.name == domain_name) & 
                (domains.c.is_active)
            )
            if values := await conn.scalar(query):
                permissions.domain = values
            return permissions
        return check_domain


    async def _vfolder_permission_group(self, domain_name: str, group_id: uuid.UUID) -> VFolderPermissionOption:
        async def check_group(conn: SAConnection, permissions: VFolderHostPermissions) -> VFolderHostPermissions:
            from . import groups

            query = sa.select([groups.c.allowed_vfolder_hosts]).where(
                (groups.c.domain_name == domain_name) &
                (groups.c.id == group_id) &
                (groups.c.is_active)
            )
            if values := await conn.scalar(query):
                permissions.group = values
            return permissions
        return check_group


    async def _vfolder_permission_user(
        self,
        user_uuid: uuid.UUID,
        domain_name: str,
        group_id: Optional[uuid.UUID] = None
    ) -> VFolderPermissionOption:
        async def check_user(conn: SAConnection, permissions: VFolderHostPermissions) -> VFolderHostPermissions:
            from . import association_groups_users, groups
            
            where_conditions = [
                groups.c.domain_name == domain_name,
                groups.c.is_active
            ]
            if group_id:
                where_conditions.append(groups.c.id == group_id)

            query = (
                sa.select([groups.c.allowed_vfolder_hosts])
                .select_from(groups.join(
                    association_groups_users,
                    (
                        (groups.c.id == association_groups_users.c.group_id) &
                        (association_groups_users.c.user_id == user_uuid)
                    ),
                ))
                .where(sa.and_(*where_conditions))
            )
            if rows := (await conn.execute(query)).fetchall():
                for row in rows:
                    permissions.user | row.allowed_vfolder_hosts
            return permissions
        return check_user

    async def _get_allowed_vfolder_hosts(
        self,
        resource_policy: Mapping[str, Any],
        permission_options: list[VFolderPermissionOption]
    ) -> VFolderHostPermissions:
        vfolder_permissions = VFolderHostPermissions(resource_policy=resource_policy)
        
        for option in permission_options:
            vfolder_permissions = await option(vfolder_permissions)
        
        return vfolder_permissions

    async def check_host_permissions(
            self,
            folder_host: str,
            *,
            permission: VFolderHostPermission,
            allowed_vfolder_types: Sequence[str],
            user_uuid: uuid.UUID,
            resource_policy: Mapping[str, Any],
            domain_name: str,
            group_id: Optional[uuid.UUID] = None,
        ):
        permission_options: list[VFolderPermissionOption] = [self._vfolder_permission_domain(domain_name=domain_name)]
        if "user" in allowed_vfolder_types:
            permission_options.append(self._vfolder_permission_user(user_uuid=user_uuid, domain_name=domain_name))

        if "group" in allowed_vfolder_types and group_id is not None:
            permission_options.append(self._vfolder_permission_group(domain_name=domain_name, group_id=group_id))

        permissions: VFolderHostPermissions = await self._get_allowed_vfolder_hosts(
            resource_policy=resource_policy,
            permission_options=permission_options
        )

        permissions.check_permission(folder_host=folder_host, permission=permission)
        
        return permissions

    async def update_vfolder_status(
        self,
        vfolder_ids: list[uuid.UUID], 
        vfolder_status: VFolderOperationStatus
    ) -> None:
        ...

    # create vFolder
    async def get_user_created_vfolder_count(self, user_id) -> int:
        ...

    async def get_group_created_vfolder_count(self, group_id) -> int:
        ...

    async def get_group_vfolder_capability(
        self,
        permission: VFolderPermission,
        usage_mode: VFolderUsageMode,
        group_identity: GroupIdentity
    ) -> VFolderCapabilityInfo:
        ...

    async def get_user_vfolder_capability(
        self,
        permission: VFolderPermission,
        usage_mode: VFolderUsageMode,
        user_identity: UserIdentity
    ) -> VFolderCapabilityInfo:
        ...

    async def persist_vfolder_metadata(
        self,
        requirements: VFolderCreateRequirements
    ) -> VFolderMetadata:
        ...

    # list vFolders
    async def list_accessible_folders(
        self, 
        user_identity: UserIdentity,
        keypair: Keypair,
        group_id: Optional[uuid.UUID]
    ) -> VFolders:
        ...

    # rename vFolder
    async def patch_vFolder_name(
        self,
        vfolder_id: uuid.UUID, 
        new_name: str
    ) -> None:
        ...

    # delete vFolder
    async def delete_vFolder(
        self,
        vfolder_id: uuid.UUID
    ) -> None:
        ...

Analyze ‘super method’ (such as query_accessible_vfolders) and split them into small methods

  1. usage of query_accessible_vfolders are

!스크린샷 2025-02-03 오후 6.10.47.png|width=100%,alt="스크린샷 2025-02-03 오후 6.10.47.png"!

so we can split them with their own concerns like below. Validation logics must be implemented in service layer, not repository layer

class VFolderRepository:
   async def get_log_folder(
        self, 
        user_identity: UserIdentity,
    ) -> VFolder:
       """Get log folder"""
       
   async def list_accessible_folders(
        self, 
        user_identity: UserIdentity, 
        group_identity: Optional[GroupIdentity] = None
    ) -> VFolders:
       """Get list of accessible folders"""
       
   async def get_folder_by_id(
        self, 
        user_identity: UserIdentity,
        folder_id: UUID
    ) -> VFolder:
       """Get folder by ID"""
  1. usage of resolve_vfolder_rows

Use case of resolve_vfolder_rows can be organized like below table. But as resolve_vfolder_rows is a wrapper method of query_accessible_vfolders(for parsing aiohttp.web.Request and pass value to query_accessible_vfolders), resolve_vfolder_rows can be integrated refactored query_accessible_vfolders methods

!스크린샷 2025-02-03 오후 6.21.41.png|width=100%,alt="스크린샷 2025-02-03 오후 6.21.41.png"!

Impact  

This architectural change will help establish a more robust and maintainable system while reducing the risk of unexpected database interactions. 

Testing Scenarios  

  • Test repository methods for CRUD operation (with Mock database connections)
  • Verify CRUD repository operations returns right entity or dtos
@seedspirit seedspirit self-assigned this Feb 3, 2025
@seedspirit seedspirit linked a pull request Feb 12, 2025 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant