make the permission update function idempotent

rhodecode/controllers/api/api.py

@@ -40,8 +40,8 b' from rhodecode.model.scm import ScmModel'
 from rhodecode.model.repo import RepoModel
 from rhodecode.model.user import UserModel
 from rhodecode.model.users_group import UserGroupModel
-from rhodecode.model.permission import PermissionModel
-from rhodecode.model.db import Repository, RhodeCodeSetting, UserIpMap
+from rhodecode.model.db import Repository, RhodeCodeSetting, UserIpMap,\
+    Permission
 from rhodecode.lib.compat import json
 
 log = logging.getLogger(__name__)
@@ -139,7 +139,7 b' def get_perm_or_error(permid):'
 
     :param userid:
     """
-    perm = PermissionModel().get_permission_by_name(permid)
+    perm = Permission.get_by_key(permid)
     if perm is None:
         raise JSONRPCError('permission `%s` does not exist' % (permid))
     return perm

rhodecode/model/permission.py

@@ -28,11 +28,10 b' import traceback'
 
 from sqlalchemy.exc import DatabaseError
 
-from rhodecode.lib.caching_query import FromCache
-
 from rhodecode.model import BaseModel
 from rhodecode.model.db import User, Permission, UserToPerm, UserRepoToPerm,\
     UserRepoGroupToPerm
+from rhodecode.lib.utils2 import str2bool
 
 log = logging.getLogger(__name__)
 
@@ -44,76 +43,32 b' class PermissionModel(BaseModel):'
 
     cls = Permission
 
-    def get_permission(self, permission_id, cache=False):
-        """
-        Get's permissions by id
-
-        :param permission_id: id of permission to get from database
-        :param cache: use Cache for this query
-        """
-        perm = self.sa.query(Permission)
-        if cache:
-            perm = perm.options(FromCache("sql_cache_short",
-                                          "get_permission_%s" % permission_id))
-        return perm.get(permission_id)
-
-    def get_permission_by_name(self, name, cache=False):
-        """
-        Get's permissions by given name
-
-        :param name: name to fetch
-        :param cache: Use cache for this query
-        """
-        perm = self.sa.query(Permission)\
-            .filter(Permission.permission_name == name)
-        if cache:
-            perm = perm.options(FromCache("sql_cache_short",
-                                          "get_permission_%s" % name))
-        return perm.scalar()
-
     def update(self, form_result):
-        perm_user = self.sa.query(User)\
-                        .filter(User.username ==
-                                form_result['perm_user_name']).scalar()
-        u2p = self.sa.query(UserToPerm).filter(UserToPerm.user ==
-                                               perm_user).all()
-        if len(u2p) != len(User.DEFAULT_PERMISSIONS):
-            raise Exception('Defined: %s should be %s  permissions for default'
-                            ' user. This should not happen please verify'
-                            ' your database' % (len(u2p), len(User.DEFAULT_PERMISSIONS)))
+        perm_user = User.get_by_username(username=form_result['perm_user_name'])
+        u2p = self.sa.query(UserToPerm).filter(UserToPerm.user == perm_user).all()
 
         try:
-            # stage 1 change defaults
+            def _make_new(usr, perm_name):
+                new = UserToPerm()
+                new.user = usr
+                new.permission = Permission.get_by_key(perm_name)
+                return new
+            # clear current entries, to make this function idempotent
+            # it will fix even if we define more permissions or permissions
+            # are somehow missing
             for p in u2p:
-                if p.permission.permission_name.startswith('repository.'):
-                    p.permission = self.get_permission_by_name(
-                                       form_result['default_repo_perm'])
-                    self.sa.add(p)
-
-                elif p.permission.permission_name.startswith('group.'):
-                    p.permission = self.get_permission_by_name(
-                                       form_result['default_group_perm'])
-                    self.sa.add(p)
-
-                elif p.permission.permission_name.startswith('hg.register.'):
-                    p.permission = self.get_permission_by_name(
-                                       form_result['default_register'])
-                    self.sa.add(p)
-
-                elif p.permission.permission_name.startswith('hg.create.'):
-                    p.permission = self.get_permission_by_name(
-                                        form_result['default_create'])
-                    self.sa.add(p)
-
-                elif p.permission.permission_name.startswith('hg.fork.'):
-                    p.permission = self.get_permission_by_name(
-                                        form_result['default_fork'])
+                self.sa.delete(p)
+            #create fresh set of permissions
+            for def_perm_key in ['default_repo_perm', 'default_group_perm',
+                                 'default_register', 'default_create',
+                                 'default_fork']:
+                p = _make_new(perm_user, form_result[def_perm_key])
                     self.sa.add(p)
 
             #stage 2 update all default permissions for repos if checked
             if form_result['overwrite_default_repo'] == True:
                 _def_name = form_result['default_repo_perm'].split('repository.')[-1]
-                _def = self.get_permission_by_name('repository.' + _def_name)
+                _def = Permission.get_by_key('repository.' + _def_name)
                 # repos
                 for r2p in self.sa.query(UserRepoToPerm)\
                                .filter(UserRepoToPerm.user == perm_user)\
@@ -127,7 +82,7 b' class PermissionModel(BaseModel):'
             if form_result['overwrite_default_group'] == True:
                 _def_name = form_result['default_group_perm'].split('group.')[-1]
                 # groups
-                _def = self.get_permission_by_name('group.' + _def_name)
+                _def = Permission.get_by_key('group.' + _def_name)
                 for g2p in self.sa.query(UserRepoGroupToPerm)\
                                .filter(UserRepoGroupToPerm.user == perm_user)\
                                .all():
@@ -136,9 +91,11 b' class PermissionModel(BaseModel):'
 
             # stage 3 set anonymous access
             if perm_user.username == 'default':
-                perm_user.active = bool(form_result['anonymous'])
+                perm_user.active = str2bool(form_result['anonymous'])
                 self.sa.add(perm_user)
 
+            self.sa.commit()
         except (DatabaseError,):
             log.error(traceback.format_exc())
+            self.sa.rollback()
             raise