From b0fcf86a0fd3578f1b6da2c245453ffaed508db3 Mon Sep 17 00:00:00 2001
From: davelopez <46503462+davelopez@users.noreply.github.com>
Date: Thu, 18 Feb 2021 16:02:27 +0100
Subject: [PATCH] Decode all ids and fetch them all in one query

As @mvdbeek suggested
---
 lib/galaxy/managers/groups.py | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/lib/galaxy/managers/groups.py b/lib/galaxy/managers/groups.py
index 0e43b91336b..8162bf3ebfe 100644
--- a/lib/galaxy/managers/groups.py
+++ b/lib/galaxy/managers/groups.py
@@ -1,6 +1,7 @@
 from typing import (
     Any,
     Dict,
+    List,
 )
 
 from sqlalchemy import false
@@ -47,10 +48,10 @@ class GroupsManager:
 
         group = model.Group(name=name)
         trans.sa_session.add(group)
-        user_ids = payload.get('user_ids', [])
-        users = [trans.sa_session.query(model.User).get(self._decode_id(i)) for i in user_ids]
-        role_ids = payload.get('role_ids', [])
-        roles = [trans.sa_session.query(model.Role).get(self._decode_id(i)) for i in role_ids]
+        encoded_user_ids = payload.get('user_ids', [])
+        users = self._get_users_by_encoded_ids(trans, encoded_user_ids)
+        encoded_role_ids = payload.get('role_ids', [])
+        roles = self._get_roles_by_encoded_ids(trans, encoded_role_ids)
         trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users)
         trans.sa_session.flush()
 
@@ -80,16 +81,19 @@ class GroupsManager:
             self._check_duplicated_group_name(trans, name)
             group.name = name
             trans.sa_session.add(group)
-        user_ids = payload.get('user_ids', [])
-        users = [trans.sa_session.query(model.User).get(self._decode_id(i)) for i in user_ids]
-        role_ids = payload.get('role_ids', [])
-        roles = [trans.sa_session.query(model.Role).get(self._decode_id(i)) for i in role_ids]
+        encoded_user_ids = payload.get('user_ids', [])
+        users = self._get_users_by_encoded_ids(trans, encoded_user_ids)
+        encoded_role_ids = payload.get('role_ids', [])
+        roles = self._get_roles_by_encoded_ids(trans, encoded_role_ids)
         trans.app.security_agent.set_entity_group_associations(groups=[group], roles=roles, users=users, delete_existing_assocs=False)
         trans.sa_session.flush()
 
     def _decode_id(self, encoded_id: EncodedDatabaseIdField) -> int:
         return decode_id(self._app, encoded_id)
 
+    def _decode_ids(self, encoded_ids: List[EncodedDatabaseIdField]) -> List[int]:
+        return [self._decode_id(encoded_id) for encoded_id in encoded_ids]
+
     def _check_duplicated_group_name(self, trans: ProvidesAppContext, group_name: str) -> None:
         if trans.sa_session.query(model.Group).filter(model.Group.table.c.name == group_name).first():
             raise Conflict(f"A group with name '{group_name}' already exists")
@@ -100,3 +104,13 @@ class GroupsManager:
         if group is None:
             raise ObjectNotFound(f"Group with id {encoded_id} was not found.")
         return group
+
+    def _get_users_by_encoded_ids(self, trans: ProvidesAppContext, encoded_user_ids: List[EncodedDatabaseIdField]) -> List[model.User]:
+        decoded_user_ids = self._decode_ids(encoded_user_ids)
+        users = trans.sa_session.query(model.User).filter(model.User.table.c.id.in_(decoded_user_ids)).all()
+        return users
+
+    def _get_roles_by_encoded_ids(self, trans: ProvidesAppContext, encoded_role_ids: List[EncodedDatabaseIdField]) -> List[model.Role]:
+        decoded_role_ids = self._decode_ids(encoded_role_ids)
+        roles = trans.sa_session.query(model.Role).filter(model.Role.table.c.id.in_(decoded_role_ids)).all()
+        return roles