From da0e9ee1d75e65682058e736548f939ce5907aee Mon Sep 17 00:00:00 2001
From: Sergey Golitsynskiy <sgolitsynskiy@gmail.com>
Date: Fri, 8 Oct 2021 20:33:47 -0400
Subject: [PATCH] Fix bug, 2 SAWarnings for non_private_roles (see note)

Same bug as in:
https://github.com/galaxyproject/galaxy/commit/143bb98c26d58e51f12b6bf5f0361e976f6b1853
https://github.com/galaxyproject/galaxy/commit/285bb51cad66b76bb32cbb8046e64ef04b62ff02
---
 lib/tool_shed/webapp/model/__init__.py    |  8 +++++++
 lib/tool_shed/webapp/model/mapping.py     |  5 +---
 test/unit/shed_unit/model/test_mapping.py | 28 +++++++++++++++++++++--
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/lib/tool_shed/webapp/model/__init__.py b/lib/tool_shed/webapp/model/__init__.py
index 7530718534eb..af6aa8e39536 100644
--- a/lib/tool_shed/webapp/model/__init__.py
+++ b/lib/tool_shed/webapp/model/__init__.py
@@ -108,6 +108,14 @@ class User(Base, Dictifiable, _HasTable):
     dict_collection_visible_keys = ['id', 'username']
     dict_element_visible_keys = ['id', 'username']
     bootstrap_admin_user = False
+    non_private_roles = relationship(
+        'UserRoleAssociation',
+        viewonly=True,
+        primaryjoin=(lambda:
+            (User.id == UserRoleAssociation.user_id)  # type: ignore
+            & (UserRoleAssociation.role_id == Role.id)  # type: ignore
+            & not_(Role.name == User.email))  # type: ignore
+    )
 
     def __init__(self, email=None, password=None):
         self.email = email
diff --git a/lib/tool_shed/webapp/model/mapping.py b/lib/tool_shed/webapp/model/mapping.py
index 634d7f298c58..643c74b2751b 100644
--- a/lib/tool_shed/webapp/model/mapping.py
+++ b/lib/tool_shed/webapp/model/mapping.py
@@ -4,7 +4,7 @@
 """
 import logging
 
-from sqlalchemy import Boolean, Column, DateTime, desc, false, ForeignKey, Integer, not_, Table, TEXT, true
+from sqlalchemy import Boolean, Column, DateTime, desc, false, ForeignKey, Integer, Table, TEXT, true
 from sqlalchemy.orm import relation
 
 import tool_shed.webapp.model
@@ -81,9 +81,6 @@
 mapper_registry.map_imperatively(UserRoleAssociation, UserRoleAssociation.table,
        properties=dict(
            user=relation(User, backref="roles"),
-           non_private_roles=relation(User,
-                                      backref="non_private_roles",
-                                      primaryjoin=((User.id == UserRoleAssociation.table.c.user_id) & (UserRoleAssociation.table.c.role_id == Role.id) & not_(Role.name == User.email))),
            role=relation(Role, back_populates='users')))
 
 mapper_registry.map_imperatively(Repository, Repository.table,
diff --git a/test/unit/shed_unit/model/test_mapping.py b/test/unit/shed_unit/model/test_mapping.py
index fc2902eec9c0..0a2e78444c17 100644
--- a/test/unit/shed_unit/model/test_mapping.py
+++ b/test/unit/shed_unit/model/test_mapping.py
@@ -751,6 +751,8 @@ def test_relationships(
         password_reset_token,
         user_group_association,
         user_role_association,
+        role_factory,
+        user_role_association_factory
     ):
         obj = cls_()
         obj.email = get_unique_value()
@@ -760,9 +762,16 @@ def test_relationships(
         obj.api_keys.append(api_keys)
         obj.reset_tokens.append(password_reset_token)
         obj.groups.append(user_group_association)
-        obj.roles.append(user_role_association)
         obj.repository_reviews.append(repository_review)
 
+        _private_role = role_factory(name=obj.email)
+        private_user_role = user_role_association_factory(obj, _private_role)
+        obj.roles.append(private_user_role)
+
+        _non_private_role = role_factory(name='a')
+        non_private_user_role = user_role_association_factory(obj, _non_private_role)
+        obj.roles.append(non_private_user_role)
+
         with dbcleanup(session, obj) as obj_id:
             stored_obj = get_stored_obj(session, cls_, obj_id)
             assert stored_obj.active_repositories == [repository]
@@ -770,8 +779,9 @@ def test_relationships(
             assert stored_obj.api_keys == [api_keys]
             assert stored_obj.reset_tokens == [password_reset_token]
             assert stored_obj.groups == [user_group_association]
-            assert stored_obj.roles == [user_role_association]
             assert stored_obj.repository_reviews == [repository_review]
+            assert are_same_entity_collections(stored_obj.roles, [private_user_role, non_private_user_role])
+            assert stored_obj.non_private_roles == [non_private_user_role]
 
 
 class TestUserGroupAssociation(BaseTest):
@@ -975,6 +985,13 @@ def make_instance(*args, **kwds):
     return make_instance
 
 
+@pytest.fixture
+def role_factory(model):
+    def make_instance(*args, **kwds):
+        return model.Role(*args, **kwds)
+    return make_instance
+
+
 @pytest.fixture
 def user_factory(model):
     def make_instance(*args, **kwds):
@@ -985,6 +1002,13 @@ def make_instance(*args, **kwds):
     return make_instance
 
 
+@pytest.fixture
+def user_role_association_factory(model):
+    def make_instance(*args, **kwds):
+        return model.UserRoleAssociation(*args, **kwds)
+    return make_instance
+
+
 # Test utilities
 
 def dbcleanup_wrapper(session, obj, where_clause=None):