Unverified Commit bd75278e authored by John Davis's avatar John Davis Committed by GitHub
Browse files

Merge pull request #19679 from jdavcs/24.2_role_name_displayed_name_description

[24.2] Fix private role name performance issue
parents cc18d879 b69a736a
Loading
Loading
Loading
Loading
+5 −1
Original line number Diff line number Diff line
@@ -396,7 +396,11 @@ class ProvidesFileSourcesUserContext(FileSourcesUserContext, FileSourceDictifiab
    def role_names(self) -> Set[str]:
        """The set of role names of this user."""
        user = self.trans.user
        return {ura.role.name for ura in user.roles} if user else set()
        role_names = set()
        if user:
            role_names = {ura.role.name for ura in user.roles}
            role_names.add(user.email)  # User's private role may have a generic name, so add user's email explicitly.
        return role_names

    @property
    def group_names(self) -> Set[str]:
+17 −9
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ from typing import (
    Generic,
    List,
    Optional,
    Set,
    Type,
    TypeVar,
)
@@ -36,6 +37,7 @@ from galaxy.model import (
    HistoryDatasetAssociation,
)
from galaxy.model.base import transaction
from galaxy.model.db.role import get_private_role_user_emails_dict
from galaxy.schema.tasks import (
    ComputeDatasetHashTaskRequest,
    PurgeDatasetsTaskRequest,
@@ -451,16 +453,24 @@ class DatasetAssociationManager(
            library_dataset = None
            dataset = dataset_assoc.dataset

        private_role_emails = get_private_role_user_emails_dict(self.session())

        # Omit duplicated roles by converting to set
        access_roles = set(dataset.get_access_roles(self.app.security_agent))
        manage_roles = set(dataset.get_manage_permissions_roles(self.app.security_agent))

        access_dataset_role_list = [
            (access_role.name, self.app.security.encode_id(access_role.id)) for access_role in access_roles
        ]
        manage_dataset_role_list = [
            (manage_role.name, self.app.security.encode_id(manage_role.id)) for manage_role in manage_roles
        ]
        def make_tuples(roles: Set):
            tuples = []
            for role in roles:
                # use role name for non-private roles, and user.email from private rules
                displayed_name = private_role_emails.get(role.id, role.name)
                role_tuple = (displayed_name, self.app.security.encode_id(role.id))
                tuples.append(role_tuple)
            return tuples

        access_dataset_role_list = make_tuples(access_roles)
        manage_dataset_role_list = make_tuples(manage_roles)

        rval = dict(access_dataset_roles=access_dataset_role_list, manage_dataset_roles=manage_dataset_role_list)
        if library_dataset is not None:
            modify_roles = set(
@@ -468,9 +478,7 @@ class DatasetAssociationManager(
                    library_dataset, self.app.security_agent.permitted_actions.LIBRARY_MODIFY
                )
            )
            modify_item_role_list = [
                (modify_role.name, self.app.security.encode_id(modify_role.id)) for modify_role in modify_roles
            ]
            modify_item_role_list = make_tuples(modify_roles)
            rval["modify_item_roles"] = modify_item_role_list
        return rval

+16 −10
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ from dataclasses import dataclass
from typing import (
    List,
    Optional,
    Set,
    Tuple,
    Union,
)
@@ -49,6 +50,7 @@ from galaxy.model import (
    LibraryFolderPermissions,
)
from galaxy.model.base import transaction
from galaxy.model.db.role import get_private_role_user_emails_dict
from galaxy.model.scoped_session import galaxy_scoped_session
from galaxy.schema.schema import LibraryFolderContentsIndexQueryPayload
from galaxy.security import RBACAgent
@@ -295,6 +297,8 @@ class FolderManager:
        :returns:   dict of current roles for all available permission types
        :rtype:     dictionary
        """
        private_role_emails = get_private_role_user_emails_dict(trans.sa_session)

        # Omit duplicated roles by converting to set
        modify_roles = set(
            trans.app.security_agent.get_roles_for_action(
@@ -312,17 +316,19 @@ class FolderManager:
            )
        )

        modify_folder_role_list = [
            (modify_role.name, trans.security.encode_id(modify_role.id)) for modify_role in modify_roles
        ]
        manage_folder_role_list = [
            (manage_role.name, trans.security.encode_id(manage_role.id)) for manage_role in manage_roles
        ]
        add_library_item_role_list = [(add_role.name, trans.security.encode_id(add_role.id)) for add_role in add_roles]
        def make_tuples(roles: Set):
            tuples = []
            for role in roles:
                # use role name for non-private roles, and user.email from private rules
                displayed_name = private_role_emails.get(role.id, role.name)
                role_tuple = (displayed_name, trans.security.encode_id(role.id))
                tuples.append(role_tuple)
            return tuples

        return dict(
            modify_folder_role_list=modify_folder_role_list,
            manage_folder_role_list=manage_folder_role_list,
            add_library_item_role_list=add_library_item_role_list,
            modify_folder_role_list=make_tuples(modify_roles),
            manage_folder_role_list=make_tuples(manage_roles),
            add_library_item_role_list=make_tuples(add_roles),
        )

    def can_add_item(self, trans, folder):
+20 −19
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ from galaxy.model.db.library import (
    get_library_ids,
    get_library_permissions_by_role,
)
from galaxy.model.db.role import get_private_role_user_emails_dict
from galaxy.util import (
    pretty_print_time_interval,
    unicodify,
@@ -277,26 +278,26 @@ class LibraryManager:
        :rtype:     dictionary
        :returns:   dict of current roles for all available permission types
        """
        access_library_role_list = [
            (access_role.name, trans.security.encode_id(access_role.id))
            for access_role in self.get_access_roles(trans, library)
        ]
        modify_library_role_list = [
            (modify_role.name, trans.security.encode_id(modify_role.id))
            for modify_role in self.get_modify_roles(trans, library)
        ]
        manage_library_role_list = [
            (manage_role.name, trans.security.encode_id(manage_role.id))
            for manage_role in self.get_manage_roles(trans, library)
        ]
        add_library_item_role_list = [
            (add_role.name, trans.security.encode_id(add_role.id)) for add_role in self.get_add_roles(trans, library)
        ]
        private_role_emails = get_private_role_user_emails_dict(trans.sa_session)
        access_roles = self.get_access_roles(trans, library)
        modify_roles = self.get_modify_roles(trans, library)
        manage_roles = self.get_manage_roles(trans, library)
        add_roles = self.get_add_roles(trans, library)

        def make_tuples(roles: Set):
            tuples = []
            for role in roles:
                # use role name for non-private roles, and user.email from private rules
                displayed_name = private_role_emails.get(role.id, role.name)
                role_tuple = (displayed_name, trans.security.encode_id(role.id))
                tuples.append(role_tuple)
            return tuples

        return dict(
            access_library_role_list=access_library_role_list,
            modify_library_role_list=modify_library_role_list,
            manage_library_role_list=manage_library_role_list,
            add_library_item_role_list=add_library_item_role_list,
            access_library_role_list=make_tuples(access_roles),
            modify_library_role_list=make_tuples(modify_roles),
            manage_library_role_list=make_tuples(manage_roles),
            add_library_item_role_list=make_tuples(add_roles),
        )

    def get_access_roles(self, trans, library: Library) -> Set[Role]:
+4 −15
Original line number Diff line number Diff line
@@ -5,10 +5,7 @@ Manager and Serializer for Roles.
import logging
from typing import List

from sqlalchemy import (
    false,
    select,
)
from sqlalchemy import select
from sqlalchemy.exc import (
    MultipleResultsFound,
    NoResultFound,
@@ -26,6 +23,7 @@ from galaxy.managers import base
from galaxy.managers.context import ProvidesUserContext
from galaxy.model import Role
from galaxy.model.base import transaction
from galaxy.model.db.role import get_displayable_roles
from galaxy.schema.schema import RoleDefinitionModel
from galaxy.util import unicodify

@@ -71,12 +69,7 @@ class RoleManager(base.ModelManager[model.Role]):
        return role

    def list_displayable_roles(self, trans: ProvidesUserContext) -> List[Role]:
        roles = []
        stmt = select(Role).where(Role.deleted == false())
        for role in trans.sa_session.scalars(stmt):
            if trans.user_is_admin or trans.app.security_agent.ok_to_display(trans.user, role):
                roles.append(role)
        return roles
        return get_displayable_roles(trans.sa_session, trans.user, trans.user_is_admin, trans.app.security_agent)

    def create_role(self, trans: ProvidesUserContext, role_definition_model: RoleDefinitionModel) -> model.Role:
        name = role_definition_model.name
@@ -84,11 +77,7 @@ class RoleManager(base.ModelManager[model.Role]):
        user_ids = role_definition_model.user_ids or []
        group_ids = role_definition_model.group_ids or []

        stmt = (
            select(Role)
            .where(Role.name == name)  # type:ignore[arg-type,comparison-overlap]  # Role.name is a SA hybrid property
            .limit(1)
        )
        stmt = select(Role).where(Role.name == name).limit(1)
        if trans.sa_session.scalars(stmt).first():
            raise Conflict(f"A role with that name already exists [{name}]")

Loading