From 3c28482f1c24208932b5a8951cd804246a16a5d8 Mon Sep 17 00:00:00 2001 From: Youngjin Jo Date: Fri, 4 Oct 2024 23:49:02 +0900 Subject: [PATCH 1/5] fix: return default workspace_group_info because of return type Signed-off-by: Youngjin Jo --- src/spaceone/identity/service/workspace_group_service.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/spaceone/identity/service/workspace_group_service.py b/src/spaceone/identity/service/workspace_group_service.py index c24cdca..48823e5 100644 --- a/src/spaceone/identity/service/workspace_group_service.py +++ b/src/spaceone/identity/service/workspace_group_service.py @@ -654,9 +654,8 @@ def update_user_info( update_user_info(user, user_info_map) for user in workspace_group_users ] workspace_group_info.users = updated_users - return workspace_group_info.to_dict() - return {} + return workspace_group_info.to_dict() def get_users_info_list( self, From f6f4766d949a55327a12f0eee362fe7373b38d67 Mon Sep 17 00:00:00 2001 From: Youngjin Jo Date: Sat, 5 Oct 2024 00:37:50 +0900 Subject: [PATCH 2/5] refactor: get method for workspace group and workspace group user Signed-off-by: Youngjin Jo --- .../manager/workspace_group_manager.py | 14 ++++++ .../service/workspace_group_service.py | 44 +++++++++---------- .../service/workspace_group_user_service.py | 19 +++----- 3 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/spaceone/identity/manager/workspace_group_manager.py b/src/spaceone/identity/manager/workspace_group_manager.py index 98c8faf..9b4c14f 100644 --- a/src/spaceone/identity/manager/workspace_group_manager.py +++ b/src/spaceone/identity/manager/workspace_group_manager.py @@ -79,6 +79,20 @@ def list_workspace_groups(self, query: dict) -> Tuple[QuerySet, int]: def stat_workspace_group(self, query: dict) -> dict: return self.workspace_group_model.stat(**query) + def get_workspace_group_with_users( + self, domain_id: str, workspace_group_id: str + ) -> Tuple[WorkspaceGroup, List[str]]: + workspace_group_vo = self.get_workspace_group(domain_id, workspace_group_id) + + workspace_group_user_ids = [] + if workspace_group_vo.users: + old_users, new_users = self.get_unique_user_ids( + domain_id, workspace_group_id, workspace_group_vo.users + ) + workspace_group_user_ids = old_users + new_users + + return workspace_group_vo, workspace_group_user_ids + @staticmethod def _check_existence_of_workspace_or_user( workspace_vos: QuerySet, workspace_group_vo: WorkspaceGroup diff --git a/src/spaceone/identity/service/workspace_group_service.py b/src/spaceone/identity/service/workspace_group_service.py index 48823e5..73d1b0b 100644 --- a/src/spaceone/identity/service/workspace_group_service.py +++ b/src/spaceone/identity/service/workspace_group_service.py @@ -1,6 +1,6 @@ import logging from datetime import datetime -from typing import Dict, List, Union +from typing import Dict, List, Union, Any from spaceone.core.error import ERROR_INVALID_PARAMETER, ERROR_NOT_FOUND from spaceone.core.service import ( @@ -237,25 +237,15 @@ def get( Returns: WorkspaceGroupResponse: """ - workspace_group_id = params.workspace_group_id - domain_id = params.domain_id - - workspace_group_vo = self.workspace_group_mgr.get_workspace_group( - domain_id, workspace_group_id - ) - - workspace_group_user_ids = [] - if workspace_group_vo.users: - old_users, new_users = self.workspace_group_mgr.get_unique_user_ids( - domain_id, workspace_group_id, workspace_group_vo.users + workspace_group_vo, workspace_group_user_ids = ( + self.workspace_group_mgr.get_workspace_group_with_users( + params.domain_id, params.workspace_group_id ) - - workspace_group_user_ids: List[str] = old_users + new_users - - workspace_group_dict = self.add_user_name_and_state_to_users( - workspace_group_user_ids, workspace_group_vo, domain_id ) - return WorkspaceGroupResponse(**workspace_group_dict) + workspace_group_info = self.add_user_name_and_state_to_users( + workspace_group_user_ids, workspace_group_vo, params.domain_id + ) + return WorkspaceGroupResponse(**workspace_group_info) @transaction(permission="identity:WorkspaceGroup.read", role_types=["DOMAIN_ADMIN"]) @append_query_filter(["workspace_group_id", "name", "domain_id"]) @@ -599,18 +589,18 @@ def add_user(user_info, workspace_group_workspace_id=None): def add_user_name_and_state_to_users( self, workspace_group_user_ids: List[str], - workspace_group_info: Union[WorkspaceGroup, Dict[str, str]], + workspace_group_info: Union[WorkspaceGroup, Dict[str, Any]], domain_id: str, - ) -> Dict[str, str]: + ) -> Dict[str, Any]: """Add user's name and state to users in workspace group. Since the user's name and state are not in user of workspace group in database, we need to add user's name and state to users in the Application layer. Args: workspace_group_user_ids: 'List[str]' - workspace_group_info: 'Union[WorkspaceGroup, Dict[str, str]]' + workspace_group_info: 'Union[WorkspaceGroup, Dict[str, Any]]' domain_id: 'str' Returns: - workspace_group_info: 'Dict[str, str]' + workspace_group_info: 'Dict[str, Any]' """ def update_user_info( @@ -653,9 +643,15 @@ def update_user_info( updated_users = [ update_user_info(user, user_info_map) for user in workspace_group_users ] - workspace_group_info.users = updated_users + if isinstance(workspace_group_info, dict): + workspace_group_info["users"] = updated_users + else: + workspace_group_info.users = updated_users - return workspace_group_info.to_dict() + if not isinstance(workspace_group_info, dict): + return workspace_group_info.to_dict() + else: + return workspace_group_info def get_users_info_list( self, diff --git a/src/spaceone/identity/service/workspace_group_user_service.py b/src/spaceone/identity/service/workspace_group_user_service.py index 39b90e5..fb67989 100644 --- a/src/spaceone/identity/service/workspace_group_user_service.py +++ b/src/spaceone/identity/service/workspace_group_user_service.py @@ -194,24 +194,15 @@ def get( Returns: WorkspaceGroupResponse: """ - workspace_group_id = params.workspace_group_id - domain_id = params.domain_id - - workspace_group_vo = self.workspace_group_mgr.get_workspace_group( - domain_id, workspace_group_id - ) - - workspace_group_user_ids = [] - if workspace_group_vo.users: - old_users, new_users = self.workspace_group_mgr.get_unique_user_ids( - domain_id, workspace_group_id, workspace_group_vo.users + workspace_group_vo, workspace_group_user_ids = ( + self.workspace_group_mgr.get_workspace_group_with_users( + params.domain_id, params.workspace_group_id ) - - workspace_group_user_ids: List[str] = old_users + new_users + ) workspace_group_dict = ( self.workspace_group_svc.add_user_name_and_state_to_users( - workspace_group_user_ids, workspace_group_vo, domain_id + workspace_group_user_ids, workspace_group_vo, params.domain_id ) ) return WorkspaceGroupResponse(**workspace_group_dict) From 763b349856ead7c4ef34d30bb67089704051c4ef Mon Sep 17 00:00:00 2001 From: Youngjin Jo Date: Sat, 5 Oct 2024 00:45:12 +0900 Subject: [PATCH 3/5] refactor: list method for workspace group and workspace group user Signed-off-by: Youngjin Jo --- .../service/workspace_group_service.py | 75 +++++++++++++------ 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/src/spaceone/identity/service/workspace_group_service.py b/src/spaceone/identity/service/workspace_group_service.py index 73d1b0b..1607598 100644 --- a/src/spaceone/identity/service/workspace_group_service.py +++ b/src/spaceone/identity/service/workspace_group_service.py @@ -1,5 +1,6 @@ import logging from datetime import datetime +from mongoengine import QuerySet from typing import Dict, List, Union, Any from spaceone.core.error import ERROR_INVALID_PARAMETER, ERROR_NOT_FOUND @@ -268,32 +269,44 @@ def list( Returns: WorkspaceGroupsResponse: """ + # query = params.query + # + # workspace_group_vos, total_count = ( + # self.workspace_group_mgr.list_workspace_groups(query) + # ) + # + # workspace_groups_info = [] + # for workspace_group_vo in workspace_group_vos: + # workspace_group_users = workspace_group_vo.users or [] + # old_users = list( + # set( + # [user_info["user_id"] for user_info in workspace_group_users] + # if workspace_group_users + # else [] + # ) + # ) + # new_users = list( + # set([user_info["user_id"] for user_info in workspace_group_users]) + # ) + # + # workspace_group_user_ids: List[str] = old_users + new_users + # + # workspace_group_dict = self.add_user_name_and_state_to_users( + # workspace_group_user_ids, workspace_group_vo, params.domain_id + # ) + # workspace_groups_info.append(workspace_group_dict) + # + # return WorkspaceGroupsResponse( + # results=workspace_groups_info, total_count=total_count + # ) query = params.query workspace_group_vos, total_count = ( self.workspace_group_mgr.list_workspace_groups(query) ) - - workspace_groups_info = [] - for workspace_group_vo in workspace_group_vos: - workspace_group_users = workspace_group_vo.users or [] - old_users = list( - set( - [user_info["user_id"] for user_info in workspace_group_users] - if workspace_group_users - else [] - ) - ) - new_users = list( - set([user_info["user_id"] for user_info in workspace_group_users]) - ) - - workspace_group_user_ids: List[str] = old_users + new_users - - workspace_group_dict = self.add_user_name_and_state_to_users( - workspace_group_user_ids, workspace_group_vo, params.domain_id - ) - workspace_groups_info.append(workspace_group_dict) + workspace_groups_info = self.get_workspace_groups_info( + workspace_group_vos, params.domain_id + ) return WorkspaceGroupsResponse( results=workspace_groups_info, total_count=total_count @@ -734,6 +747,26 @@ def remove_users_from_workspace_group( return updated_users + def get_workspace_groups_info( + self, workspace_group_vos: QuerySet, domain_id + ) -> List[Dict[str, Any]]: + workspace_groups_info = [] + for workspace_group_vo in workspace_group_vos: + workspace_group_user_ids = self._get_workspace_group_user_ids( + workspace_group_vo + ) + workspace_group_dict = self.add_user_name_and_state_to_users( + workspace_group_user_ids, workspace_group_vo, domain_id + ) + workspace_groups_info.append(workspace_group_dict) + return workspace_groups_info + + @staticmethod + def _get_workspace_group_user_ids(workspace_group_vo: WorkspaceGroup) -> List[str]: + workspace_group_users = workspace_group_vo.users or [] + user_ids = set(user_info["user_id"] for user_info in workspace_group_users) + return list(user_ids) + @staticmethod def check_user_state(old_user_id: str, old_user_state: str) -> None: if old_user_state in ["DISABLED", "DELETED"]: From 87ab4d5f0aab289d479574d75e444b59e077e9ab Mon Sep 17 00:00:00 2001 From: Youngjin Jo Date: Sat, 5 Oct 2024 01:00:22 +0900 Subject: [PATCH 4/5] refactor: update user_count code of workspace Signed-off-by: Youngjin Jo --- .../identity/service/role_binding_service.py | 57 +++++++++---------- .../service/workspace_group_service.py | 17 +----- 2 files changed, 28 insertions(+), 46 deletions(-) diff --git a/src/spaceone/identity/service/role_binding_service.py b/src/spaceone/identity/service/role_binding_service.py index 437fe4e..c735b9c 100644 --- a/src/spaceone/identity/service/role_binding_service.py +++ b/src/spaceone/identity/service/role_binding_service.py @@ -119,20 +119,7 @@ def create_role_binding(self, params: dict): rb_vo = self.role_binding_manager.create_role_binding(params) if workspace_vo: - user_rb_ids = self.role_binding_manager.stat_role_bindings( - query={ - "distinct": "user_id", - "filter": [ - {"k": "workspace_id", "v": workspace_id, "o": "eq"}, - {"k": "domain_id", "v": domain_id, "o": "eq"}, - ], - } - ).get("results", []) - user_rb_total_count = len(user_rb_ids) - - self.workspace_mgr.update_workspace_by_vo( - {"user_count": user_rb_total_count}, workspace_vo - ) + self.update_workspace_user_count(domain_id, workspace_id) return rb_vo @@ -462,25 +449,35 @@ def _get_latest_role_type(before: str, after: str) -> str: return after + def update_workspace_user_count(self, domain_id: str, workspace_id: str) -> None: + workspace_vo = self.workspace_mgr.get_workspace( + domain_id=domain_id, workspace_id=workspace_id + ) + + if workspace_vo: + user_rb_total_count = self._get_workspace_user_count( + domain_id, workspace_id + ) + self.workspace_mgr.update_workspace_by_vo( + {"user_count": user_rb_total_count}, workspace_vo + ) + def delete_role_binding_by_vo( self, rb_vo: RoleBinding, domain_id: str, workspace_id: str = None ): self.role_binding_manager.delete_role_binding_by_vo(rb_vo) if workspace_id: - workspace_vo = self.workspace_mgr.get_workspace(workspace_id, domain_id) - - user_rb_ids = self.role_binding_manager.stat_role_bindings( - query={ - "distinct": "user_id", - "filter": [ - {"k": "workspace_id", "v": workspace_id, "o": "eq"}, - {"k": "domain_id", "v": domain_id, "o": "eq"}, - ], - } - ).get("results", []) - user_rb_total_count = len(user_rb_ids) - - self.workspace_mgr.update_workspace_by_vo( - {"user_count": user_rb_total_count}, workspace_vo - ) + self.update_workspace_user_count(domain_id, workspace_id) + + def _get_workspace_user_count(self, domain_id: str, workspace_id: str) -> int: + user_rb_ids = self.role_binding_manager.stat_role_bindings( + query={ + "distinct": "user_id", + "filter": [ + {"k": "workspace_id", "v": workspace_id, "o": "eq"}, + {"k": "domain_id", "v": domain_id, "o": "eq"}, + ], + } + ).get("results", []) + return len(user_rb_ids) diff --git a/src/spaceone/identity/service/workspace_group_service.py b/src/spaceone/identity/service/workspace_group_service.py index 1607598..4c37dee 100644 --- a/src/spaceone/identity/service/workspace_group_service.py +++ b/src/spaceone/identity/service/workspace_group_service.py @@ -728,22 +728,7 @@ def remove_users_from_workspace_group( {"user_count": user_rb_total_count}, workspace_vo ) else: - workspace_vo = self.workspace_mgr.get_workspace(workspace_id, domain_id) - if workspace_vo: - user_rb_ids = self.rb_mgr.stat_role_bindings( - query={ - "distinct": "user_id", - "filter": [ - {"k": "workspace_id", "v": workspace_id, "o": "eq"}, - {"k": "domain_id", "v": domain_id, "o": "eq"}, - ], - } - ).get("results", []) - user_rb_total_count = len(user_rb_ids) - - self.workspace_mgr.update_workspace_by_vo( - {"user_count": user_rb_total_count}, workspace_vo - ) + self.rb_svc.update_workspace_user_count(domain_id, workspace_id) return updated_users From 7c396f3097f958688a7fd2ba8310527ceaf02614 Mon Sep 17 00:00:00 2001 From: Youngjin Jo Date: Sat, 5 Oct 2024 01:03:02 +0900 Subject: [PATCH 5/5] refactor: list method for workspace group and workspace group user Signed-off-by: Youngjin Jo --- .../service/workspace_group_service.py | 30 ------------------- .../service/workspace_group_user_service.py | 28 ++--------------- 2 files changed, 3 insertions(+), 55 deletions(-) diff --git a/src/spaceone/identity/service/workspace_group_service.py b/src/spaceone/identity/service/workspace_group_service.py index 4c37dee..5efd65d 100644 --- a/src/spaceone/identity/service/workspace_group_service.py +++ b/src/spaceone/identity/service/workspace_group_service.py @@ -269,36 +269,6 @@ def list( Returns: WorkspaceGroupsResponse: """ - # query = params.query - # - # workspace_group_vos, total_count = ( - # self.workspace_group_mgr.list_workspace_groups(query) - # ) - # - # workspace_groups_info = [] - # for workspace_group_vo in workspace_group_vos: - # workspace_group_users = workspace_group_vo.users or [] - # old_users = list( - # set( - # [user_info["user_id"] for user_info in workspace_group_users] - # if workspace_group_users - # else [] - # ) - # ) - # new_users = list( - # set([user_info["user_id"] for user_info in workspace_group_users]) - # ) - # - # workspace_group_user_ids: List[str] = old_users + new_users - # - # workspace_group_dict = self.add_user_name_and_state_to_users( - # workspace_group_user_ids, workspace_group_vo, params.domain_id - # ) - # workspace_groups_info.append(workspace_group_dict) - # - # return WorkspaceGroupsResponse( - # results=workspace_groups_info, total_count=total_count - # ) query = params.query workspace_group_vos, total_count = ( diff --git a/src/spaceone/identity/service/workspace_group_user_service.py b/src/spaceone/identity/service/workspace_group_user_service.py index fb67989..29a2844 100644 --- a/src/spaceone/identity/service/workspace_group_user_service.py +++ b/src/spaceone/identity/service/workspace_group_user_service.py @@ -233,31 +233,9 @@ def list( workspace_group_vos, total_count = ( self.workspace_group_mgr.list_workspace_groups(query) ) - - workspace_groups_info = [] - for workspace_group_vo in workspace_group_vos: - workspace_group_users = workspace_group_vo.users or [] - old_users = list( - set( - [user_info["user_id"] for user_info in workspace_group_users] - if workspace_group_users - else [] - ) - ) - new_users = list( - set([user_info["user_id"] for user_info in workspace_group_users]) - ) - - workspace_group_user_ids: List[str] = old_users + new_users - - workspace_group_dict = ( - self.workspace_group_svc.add_user_name_and_state_to_users( - workspace_group_user_ids, - workspace_group_vo, - params.domain_id, - ) - ) - workspace_groups_info.append(workspace_group_dict) + workspace_groups_info = self.workspace_group_svc.get_workspace_groups_info( + workspace_group_vos, params.domain_id + ) return WorkspaceGroupsResponse( results=workspace_groups_info, total_count=total_count