commit less models

- models don't do any commits(with few exceptions)
- all db transactions should be handled by higher level modules like controllers, celery tasks

--HG--
branch : beta

6 files changed, 149 insertions, 199 deletions

diff --git a/rhodecode/model/__init__.py b/rhodecode/model/__init__.py
index 8f7ee56a..459dcd0a 100644
--- a/rhodecode/model/__init__.py
+++ b/rhodecode/model/__init__.py
@@ -71,7 +71,7 @@ class BaseModel(object):
         if sa is not None:
             self.sa = sa
         else:
-            self.sa = meta.Session()
+            self.sa = meta.Session
 
     def _get_instance(self, cls, instance):
         """
diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py
index 41f812fc..6e91df7f 100755
--- a/rhodecode/model/db.py
+++ b/rhodecode/model/db.py
@@ -118,7 +118,7 @@ class BaseModel(object):
 
     @classmethod
     def query(cls):
-        return Session().query(cls)
+        return Session.query(cls)
 
     @classmethod
     def get(cls, id_):
@@ -132,7 +132,7 @@ class BaseModel(object):
     @classmethod
     def delete(cls, id_):
         obj = cls.query().get(id_)
-        Session().delete(obj)
+        Session.delete(obj)
 
 
 class RhodeCodeSetting(Base, BaseModel):
@@ -252,8 +252,7 @@ class RhodeCodeUi(Base, BaseModel):
         new_ui.ui_key = key
         new_ui.ui_value = val
 
-        Session().add(new_ui)
-        Session().commit()
+        Session.add(new_ui)
 
 
 class User(Base, BaseModel):
@@ -335,10 +334,8 @@ class User(Base, BaseModel):
 
     def update_lastlogin(self):
         """Update user lastlogin"""
-
         self.last_login = datetime.datetime.now()
-        Session().add(self)
-        Session().commit()
+        Session.add(self)
         log.debug('updated user %s lastlogin', self.username)
 
 
@@ -386,7 +383,6 @@ class UsersGroup(Base, BaseModel):
                                     "get_user_%s" % group_name))
         return q.scalar()
 
-
     @classmethod
     def get(cls, users_group_id, cache=False):
         users_group = cls.query()
@@ -395,68 +391,6 @@ class UsersGroup(Base, BaseModel):
                                     "get_users_group_%s" % users_group_id))
         return users_group.get(users_group_id)
 
-    @classmethod
-    def create(cls, form_data):
-        try:
-            new_users_group = cls()
-            for k, v in form_data.items():
-                setattr(new_users_group, k, v)
-
-            Session().add(new_users_group)
-            Session().commit()
-            return new_users_group
-        except:
-            log.error(traceback.format_exc())
-            Session().rollback()
-            raise
-
-    @classmethod
-    def update(cls, users_group_id, form_data):
-
-        try:
-            users_group = cls.get(users_group_id, cache=False)
-
-            for k, v in form_data.items():
-                if k == 'users_group_members':
-                    users_group.members = []
-                    Session().flush()
-                    members_list = []
-                    if v:
-                        v = [v] if isinstance(v, basestring) else v
-                        for u_id in set(v):
-                            member = UsersGroupMember(users_group_id, u_id)
-                            members_list.append(member)
-                    setattr(users_group, 'members', members_list)
-                setattr(users_group, k, v)
-
-            Session().add(users_group)
-            Session().commit()
-        except:
-            log.error(traceback.format_exc())
-            Session().rollback()
-            raise
-
-    @classmethod
-    def delete(cls, users_group_id):
-        try:
-
-            # check if this group is not assigned to repo
-            assigned_groups = UsersGroupRepoToPerm.query()\
-                .filter(UsersGroupRepoToPerm.users_group_id ==
-                        users_group_id).all()
-
-            if assigned_groups:
-                raise UsersGroupsAssignedException('RepoGroup assigned to %s' %
-                                                   assigned_groups)
-
-            users_group = cls.get(users_group_id, cache=False)
-            Session().delete(users_group)
-            Session().commit()
-        except:
-            log.error(traceback.format_exc())
-            Session().rollback()
-            raise
-
 class UsersGroupMember(Base, BaseModel):
     __tablename__ = 'users_groups_members'
     __table_args__ = {'extend_existing':True}
@@ -477,8 +411,8 @@ class UsersGroupMember(Base, BaseModel):
         ugm = UsersGroupMember()
         ugm.users_group = group
         ugm.user = user
-        Session().add(ugm)
-        Session().commit()
+        Session.add(ugm)
+        Session.commit()
         return ugm
 
 class Repository(Base, BaseModel):
@@ -521,11 +455,11 @@ class Repository(Base, BaseModel):
 
     @classmethod
     def get_by_repo_name(cls, repo_name):
-        q = Session().query(cls).filter(cls.repo_name == repo_name)
+        q = Session.query(cls).filter(cls.repo_name == repo_name)
         q = q.options(joinedload(Repository.fork))\
                 .options(joinedload(Repository.user))\
                 .options(joinedload(Repository.group))
-        return q.one()
+        return q.scalar()
 
     @classmethod
     def get_repo_forks(cls, repo_id):
@@ -538,7 +472,7 @@ class Repository(Base, BaseModel):
 
         :param cls:
         """
-        q = Session().query(RhodeCodeUi)\
+        q = Session.query(RhodeCodeUi)\
             .filter(RhodeCodeUi.ui_key == cls.url_sep())
         q = q.options(FromCache("sql_cache_short", "repository_repo_path"))
         return q.one().ui_value
@@ -574,7 +508,7 @@ class Repository(Base, BaseModel):
         Returns base full path for that repository means where it actually
         exists on a filesystem
         """
-        q = Session().query(RhodeCodeUi).filter(RhodeCodeUi.ui_key ==
+        q = Session.query(RhodeCodeUi).filter(RhodeCodeUi.ui_key ==
                                               Repository.url_sep())
         q = q.options(FromCache("sql_cache_short", "repository_repo_path"))
         return q.one().ui_value
@@ -851,7 +785,7 @@ class Permission(Base, BaseModel):
 
     @classmethod
     def get_default_perms(cls, default_user_id):
-        q = Session().query(UserRepoToPerm, Repository, cls)\
+        q = Session.query(UserRepoToPerm, Repository, cls)\
             .join((Repository, UserRepoToPerm.repository_id == Repository.repo_id))\
             .join((cls, UserRepoToPerm.permission_id == cls.permission_id))\
             .filter(UserRepoToPerm.user_id == default_user_id)
@@ -877,7 +811,7 @@ class UserRepoToPerm(Base, BaseModel):
         n.user = user
         n.repository = repository
         n.permission = permission
-        Session().add(n)
+        Session.add(n)
         return n
 
     def __repr__(self):
@@ -893,41 +827,6 @@ class UserToPerm(Base, BaseModel):
     user = relationship('User')
     permission = relationship('Permission', lazy='joined')
 
-    @classmethod
-    def has_perm(cls, user_id, perm):
-        if not isinstance(perm, Permission):
-            raise Exception('perm needs to be an instance of Permission class')
-
-        return cls.query().filter(cls.user_id == user_id)\
-            .filter(cls.permission == perm).scalar() is not None
-
-    @classmethod
-    def grant_perm(cls, user_id, perm):
-        if not isinstance(perm, Permission):
-            raise Exception('perm needs to be an instance of Permission class')
-
-        new = cls()
-        new.user_id = user_id
-        new.permission = perm
-        try:
-            Session().add(new)
-            Session().commit()
-        except:
-            Session().rollback()
-
-
-    @classmethod
-    def revoke_perm(cls, user_id, perm):
-        if not isinstance(perm, Permission):
-            raise Exception('perm needs to be an instance of Permission class')
-
-        try:
-            obj = cls.query().filter(cls.user_id == user_id)\
-                    .filter(cls.permission == perm).one()
-            Session().delete(obj)
-            Session().commit()
-        except:
-            Session().rollback()
 
 class UsersGroupRepoToPerm(Base, BaseModel):
     __tablename__ = 'users_group_repo_to_perm'
@@ -947,7 +846,7 @@ class UsersGroupRepoToPerm(Base, BaseModel):
         n.users_group = users_group
         n.repository = repository
         n.permission = permission
-        Session().add(n)
+        Session.add(n)
         return n
 
     def __repr__(self):
@@ -963,45 +862,6 @@ class UsersGroupToPerm(Base, BaseModel):
     permission = relationship('Permission')
 
 
-    @classmethod
-    def has_perm(cls, users_group_id, perm):
-        if not isinstance(perm, Permission):
-            raise Exception('perm needs to be an instance of Permission class')
-
-        return cls.query().filter(cls.users_group_id ==
-                                         users_group_id)\
-                                         .filter(cls.permission == perm)\
-                                         .scalar() is not None
-
-    @classmethod
-    def grant_perm(cls, users_group_id, perm):
-        if not isinstance(perm, Permission):
-            raise Exception('perm needs to be an instance of Permission class')
-
-        new = cls()
-        new.users_group_id = users_group_id
-        new.permission = perm
-        try:
-            Session().add(new)
-            Session().commit()
-        except:
-            Session().rollback()
-
-
-    @classmethod
-    def revoke_perm(cls, users_group_id, perm):
-        if not isinstance(perm, Permission):
-            raise Exception('perm needs to be an instance of Permission class')
-
-        try:
-            obj = cls.query().filter(cls.users_group_id == users_group_id)\
-                .filter(cls.permission == perm).one()
-            Session().delete(obj)
-            Session().commit()
-        except:
-            Session().rollback()
-
-
 class UserRepoGroupToPerm(Base, BaseModel):
     __tablename__ = 'group_to_perm'
     __table_args__ = (UniqueConstraint('group_id', 'permission_id'), {'extend_existing':True})
@@ -1103,7 +963,7 @@ class CacheInvalidation(Base, BaseModel):
         """
 
         log.debug('marking %s for invalidation' % key)
-        inv_obj = Session().query(cls)\
+        inv_obj = Session.query(cls)\
             .filter(cls.cache_key == key).scalar()
         if inv_obj:
             inv_obj.cache_active = False
@@ -1112,11 +972,11 @@ class CacheInvalidation(Base, BaseModel):
             inv_obj = CacheInvalidation(key)
 
         try:
-            Session().add(inv_obj)
-            Session().commit()
+            Session.add(inv_obj)
+            Session.commit()
         except Exception:
             log.error(traceback.format_exc())
-            Session().rollback()
+            Session.rollback()
 
     @classmethod
     def set_valid(cls, key):
@@ -1128,8 +988,8 @@ class CacheInvalidation(Base, BaseModel):
         inv_obj = CacheInvalidation.query()\
             .filter(CacheInvalidation.cache_key == key).scalar()
         inv_obj.cache_active = True
-        Session().add(inv_obj)
-        Session().commit()
+        Session.add(inv_obj)
+        Session.commit()
 
 
 class ChangesetComment(Base, BaseModel):
@@ -1157,7 +1017,7 @@ class ChangesetComment(Base, BaseModel):
         :param cls:
         :param revision:
         """
-        return Session().query(User)\
+        return Session.query(User)\
                 .filter(cls.revision == revision)\
                 .join(ChangesetComment.author).all()
 
@@ -1203,7 +1063,7 @@ class Notification(Base, BaseModel):
             assoc = UserNotification()
             assoc.notification = notification
             u.notifications.append(assoc)
-        Session().add(notification)
+        Session.add(notification)
         return notification
 
     @property
@@ -1226,7 +1086,7 @@ class UserNotification(Base, BaseModel):
 
     def mark_as_read(self):
         self.read = True
-        Session().add(self)
+        Session.add(self)
 
 class DbMigrateVersion(Base, BaseModel):
     __tablename__ = 'db_migrate_version'
diff --git a/rhodecode/model/forms.py b/rhodecode/model/forms.py
index 922efca2..c9843964 100644
--- a/rhodecode/model/forms.py
+++ b/rhodecode/model/forms.py
@@ -36,9 +36,7 @@ from rhodecode.config.routing import ADMIN_PREFIX
 from rhodecode.lib.utils import repo_name_slug
 from rhodecode.lib.auth import authenticate, get_crypt_password
 from rhodecode.lib.exceptions import LdapImportError
-from rhodecode.model.user import UserModel
-from rhodecode.model.repo import RepoModel
-from rhodecode.model.db import User, UsersGroup, RepoGroup
+from rhodecode.model.db import User, UsersGroup, RepoGroup, Repository
 from rhodecode import BACKENDS
 
 log = logging.getLogger(__name__)
@@ -68,7 +66,7 @@ def ValidUsername(edit, old_data):
             #check if user is unique
             old_un = None
             if edit:
-                old_un = UserModel().get(old_data.get('user_id')).username
+                old_un = User.get(old_data.get('user_id')).username
 
             if old_un != value or not edit:
                 if User.get_by_username(value, case_insensitive=True):
@@ -268,7 +266,7 @@ def ValidRepoName(edit, old_data):
             if  rename or create:
 
                 if group_path != '':
-                    if RepoModel().get_by_repo_name(repo_name_full,):
+                    if Repository.get_by_repo_name(repo_name_full):
                         e_dict = {'repo_name':_('This repository already '
                                                 'exists in a group "%s"') %
                                   gr.group_name}
@@ -281,7 +279,7 @@ def ValidRepoName(edit, old_data):
                         raise formencode.Invalid('', value, state,
                                                  error_dict=e_dict)
 
-                elif RepoModel().get_by_repo_name(repo_name_full):
+                elif Repository.get_by_repo_name(repo_name_full):
                         e_dict = {'repo_name':_('This repository '
                                                 'already exists')}
                         raise formencode.Invalid('', value, state,
diff --git a/rhodecode/model/repo.py b/rhodecode/model/repo.py
index 190f618f..3fb8232c 100644
--- a/rhodecode/model/repo.py
+++ b/rhodecode/model/repo.py
@@ -204,11 +204,9 @@ class RepoModel(BaseModel):
                 # rename repository
                 self.__rename_repo(old=repo_name, new=new_name)
 
-            self.sa.commit()
             return cur_repo
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def create(self, form_data, cur_user, just_db=False, fork=False):
@@ -312,10 +310,8 @@ class RepoModel(BaseModel):
         try:
             self.sa.delete(repo)
             self.__delete_repo(repo)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def delete_perm_user(self, form_data, repo_name):
@@ -325,10 +321,8 @@ class RepoModel(BaseModel):
                         == self.get_by_repo_name(repo_name))\
                 .filter(UserRepoToPerm.user_id == form_data['user_id']).one()
             self.sa.delete(obj)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def delete_perm_users_group(self, form_data, repo_name):
@@ -339,10 +333,8 @@ class RepoModel(BaseModel):
                 .filter(UsersGroupRepoToPerm.users_group_id
                         == form_data['users_group_id']).one()
             self.sa.delete(obj)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def delete_stats(self, repo_name):
@@ -356,10 +348,8 @@ class RepoModel(BaseModel):
                     .filter(Statistics.repository == \
                         self.get_by_repo_name(repo_name)).one()
             self.sa.delete(obj)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def __create_repo(self, repo_name, alias, new_parent_id, clone_uri=False):
@@ -428,11 +418,11 @@ class RepoModel(BaseModel):
         """
         rm_path = os.path.join(self.repos_path, repo.repo_name)
         log.info("Removing %s", rm_path)
-        #disable hg/git
+        # disable hg/git
         alias = repo.repo_type
         shutil.move(os.path.join(rm_path, '.%s' % alias),
                     os.path.join(rm_path, 'rm__.%s' % alias))
-        #disable repo
+        # disable repo
         shutil.move(rm_path, os.path.join(self.repos_path, 'rm__%s__%s' \
                                           % (datetime.today()\
                                              .strftime('%Y%m%d_%H%M%S_%f'),
diff --git a/rhodecode/model/user.py b/rhodecode/model/user.py
index 7912ac28..f2c34389 100644
--- a/rhodecode/model/user.py
+++ b/rhodecode/model/user.py
@@ -54,6 +54,9 @@ PERM_WEIGHTS = {'repository.none': 0,
 
 class UserModel(BaseModel):
 
+    def __get_user(self, user):
+        return self._get_instance(User, user)
+
     def get(self, user_id, cache=False):
         user = self.sa.query(User)
         if cache:
@@ -84,11 +87,9 @@ class UserModel(BaseModel):
 
             new_user.api_key = generate_api_key(form_data['username'])
             self.sa.add(new_user)
-            self.sa.commit()
             return new_user
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
 
@@ -159,7 +160,6 @@ class UserModel(BaseModel):
                 new_user.lastname = attrs['lastname']
 
                 self.sa.add(new_user)
-                self.sa.commit()
                 return new_user
             except (DatabaseError,):
                 log.error(traceback.format_exc())
@@ -200,7 +200,6 @@ class UserModel(BaseModel):
                 new_user.lastname = attrs['lastname']
 
                 self.sa.add(new_user)
-                self.sa.commit()
                 return new_user
             except (DatabaseError,):
                 log.error(traceback.format_exc())
@@ -258,10 +257,8 @@ class UserModel(BaseModel):
                     setattr(user, k, v)
 
             self.sa.add(user)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def update_my_account(self, user_id, form_data):
@@ -280,10 +277,8 @@ class UserModel(BaseModel):
                         setattr(user, k, v)
 
             self.sa.add(user)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def delete(self, user_id):
@@ -300,10 +295,8 @@ class UserModel(BaseModel):
                                                'remove those repositories') \
                                                % user.repositories)
             self.sa.delete(user)
-            self.sa.commit()
         except:
             log.error(traceback.format_exc())
-            self.sa.rollback()
             raise
 
     def reset_password_link(self, data):
@@ -477,3 +470,35 @@ class UserModel(BaseModel):
 
         return user
 
+
+
+    def has_perm(self, user, perm):
+        if not isinstance(perm, Permission):
+            raise Exception('perm needs to be an instance of Permission class')
+
+        user = self.__get_user(user)
+
+        return UserToPerm.query().filter(UserToPerm.user == user.user)\
+            .filter(UserToPerm.permission == perm).scalar() is not None
+
+    def grant_perm(self, user, perm):
+        if not isinstance(perm, Permission):
+            raise Exception('perm needs to be an instance of Permission class')
+
+        user = self.__get_user(user)
+
+        new = UserToPerm()
+        new.user = user.user
+        new.permission = perm
+        self.sa.add(new)
+
+
+    def revoke_perm(self, user, perm):
+        if not isinstance(perm, Permission):
+            raise Exception('perm needs to be an instance of Permission class')
+        
+        user = self.__get_user(user)
+        
+        obj = UserToPerm.query().filter(UserToPerm.user == user.user)\
+                .filter(UserToPerm.permission == perm).one()
+        self.sa.delete(obj)
diff --git a/rhodecode/model/users_group.py b/rhodecode/model/users_group.py
index e7d4a63b..25dec06a 100644
--- a/rhodecode/model/users_group.py
+++ b/rhodecode/model/users_group.py
@@ -27,7 +27,9 @@ import logging
 import traceback
 
 from rhodecode.model import BaseModel
-from rhodecode.model.db import UsersGroupMember, UsersGroup
+from rhodecode.model.db import UsersGroupMember, UsersGroup,\
+    UsersGroupRepoToPerm, Permission, UsersGroupToPerm
+from rhodecode.lib.exceptions import UsersGroupsAssignedException
 
 log = logging.getLogger(__name__)
 
@@ -44,15 +46,55 @@ class UsersGroupModel(BaseModel):
         return UsersGroup.get_by_group_name(name, cache, case_insensitive)
 
     def create(self, name, active=True):
-        new = UsersGroup()
-        new.users_group_name = name
-        new.users_group_active = active
-        self.sa.add(new)
-        return new
+        try:
+            new = UsersGroup()
+            new.users_group_name = name
+            new.users_group_active = active
+            self.sa.add(new)
+            return new
+        except:
+            log.error(traceback.format_exc())
+            raise
+
+    def update(self, users_group, form_data):
+
+        try:
+            users_group = self.__get_users_group(users_group)
+
+            for k, v in form_data.items():
+                if k == 'users_group_members':
+                    users_group.members = []
+                    self.sa.flush()
+                    members_list = []
+                    if v:
+                        v = [v] if isinstance(v, basestring) else v
+                        for u_id in set(v):
+                            member = UsersGroupMember(users_group.users_group_id, u_id)
+                            members_list.append(member)
+                    setattr(users_group, 'members', members_list)
+                setattr(users_group, k, v)
+
+            self.sa.add(users_group)
+        except:
+            log.error(traceback.format_exc())
+            raise
 
     def delete(self, users_group):
-        obj = self.__get_users_group(users_group)
-        self.sa.delete(obj)
+        try:
+            users_group = self.__get_users_group(users_group)
+            
+            # check if this group is not assigned to repo
+            assigned_groups = UsersGroupRepoToPerm.query()\
+                .filter(UsersGroupRepoToPerm.users_group == users_group).all()
+
+            if assigned_groups:
+                raise UsersGroupsAssignedException('RepoGroup assigned to %s' %
+                                                   assigned_groups)
+            
+            self.sa.delete(users_group)
+        except:
+            log.error(traceback.format_exc())
+            raise
 
     def add_user_to_group(self, users_group, user):
         for m in users_group.members:
@@ -73,3 +115,38 @@ class UsersGroupModel(BaseModel):
         except:
             log.error(traceback.format_exc())
             raise
+
+    def has_perm(self, users_group, perm):
+        if not isinstance(perm, Permission):
+            raise Exception('perm needs to be an instance of Permission class')
+
+        users_group = self.__get_users_group(users_group)
+
+        return UsersGroupToPerm.query()\
+            .filter(UsersGroupToPerm.users_group == users_group)\
+            .filter(UsersGroupToPerm.permission == perm).scalar() is not None
+
+    def grant_perm(self, users_group, perm):
+        if not isinstance(perm, Permission):
+            raise Exception('perm needs to be an instance of Permission class')
+
+        users_group = self.__get_users_group(users_group)
+
+        new = UsersGroupToPerm()
+        new.users_group = users_group
+        new.permission = perm
+        self.sa.add(new)
+
+
+    def revoke_perm(self, users_group, perm):
+        if not isinstance(perm, Permission):
+            raise Exception('perm needs to be an instance of Permission class')
+        
+        users_group = self.__get_users_group(users_group)
+        
+        obj = UsersGroupToPerm.query()\
+            .filter(UsersGroupToPerm.users_group == users_group)\
+            .filter(UsersGroupToPerm.permission == perm).one()
+        self.sa.delete(obj)
+
+