Changeset - 4136526cce20
[Not reviewed]
default
0 18 0
Søren Løvborg - 9 years ago 2016-09-13 19:19:59
sorenl@unity3d.com
db: remove superfluous Session.add calls

Don't re-add objects to the SQLAlchemy Session just because they were
modified. Session.add is only for freshly constructed objects that
SQLAlchemy doesn't know about yet.

The rules are quite simple:

When creating a database object by calling the constructor directly, it
must explicitly be added to the session.

When creating an object using a factory function (like "create_repo"),
the returned object has already (by convention) been added to the
session, and should not be added again.

When getting an object from the session (via Session.query or any of the
utility functions that look up objects in the database), it's already
added, and should not be added again. SQLAlchemy notices attribute
modifications automatically for all objects it knows about.
18 files changed with 5 insertions and 35 deletions:
0 comments (0 inline, 0 general)
kallithea/controllers/admin/user_groups.py
Show inline comments
 
@@ -375,13 +375,12 @@ class UserGroupsController(BaseControlle
 
        try:
 
            form = CustomDefaultPermissionsForm()()
 
            form_result = form.to_python(request.POST)
 

	
 
            inherit_perms = form_result['inherit_default_permissions']
 
            user_group.inherit_default_permissions = inherit_perms
 
            Session().add(user_group)
 
            usergroup_model = UserGroupModel()
 

	
 
            defs = UserGroupToPerm.query() \
 
                .filter(UserGroupToPerm.users_group == user_group) \
 
                .all()
 
            for ug in defs:
kallithea/controllers/admin/users.py
Show inline comments
 
@@ -323,13 +323,12 @@ class UsersController(BaseController):
 
        try:
 
            form = CustomDefaultPermissionsForm()()
 
            form_result = form.to_python(request.POST)
 

	
 
            inherit_perms = form_result['inherit_default_permissions']
 
            user.inherit_default_permissions = inherit_perms
 
            Session().add(user)
 
            user_model = UserModel()
 

	
 
            defs = UserToPerm.query() \
 
                .filter(UserToPerm.user == user) \
 
                .all()
 
            for ug in defs:
kallithea/lib/db_manage.py
Show inline comments
 
@@ -476,13 +476,12 @@ class DbManage(object):
 
                                            lastname=u'User')
 
        # based on configuration options activate/deactivate this user which
 
        # controls anonymous access
 
        if self.cli_args.get('public_access') is False:
 
            log.info('Public access disabled')
 
            user.active = False
 
            Session().add(user)
 
            Session().commit()
 

	
 
    def create_permissions(self):
 
        """
 
        Creates all permissions defined in the system
 
        """
kallithea/model/db.py
Show inline comments
 
@@ -664,13 +664,12 @@ class User(Base, BaseModel):
 
        if user is not None:
 
            return user
 

	
 
    def update_lastlogin(self):
 
        """Update user lastlogin"""
 
        self.last_login = datetime.datetime.now()
 
        Session().add(self)
 
        log.debug('updated user %s lastlogin', self.username)
 

	
 
    @classmethod
 
    def get_first_admin(cls):
 
        user = User.query().filter(User.admin == True).first()
 
        if user is None:
 
@@ -1285,19 +1284,17 @@ class Repository(Base, BaseModel):
 

	
 
    @classmethod
 
    def lock(cls, repo, user_id, lock_time=None):
 
        if lock_time is not None:
 
            lock_time = time.time()
 
        repo.locked = [user_id, lock_time]
 
        Session().add(repo)
 
        Session().commit()
 

	
 
    @classmethod
 
    def unlock(cls, repo):
 
        repo.locked = None
 
        Session().add(repo)
 
        Session().commit()
 

	
 
    @classmethod
 
    def getlock(cls, repo):
 
        return repo.locked
 

	
 
@@ -1343,13 +1340,13 @@ class Repository(Base, BaseModel):
 
                             qualified_home_url=qualified_home_url,
 
                             repo_name=self.repo_name,
 
                             repo_id=self.repo_id, **override)
 

	
 
    def set_state(self, state):
 
        self.repo_state = state
 
        Session().add(self)
 

	
 
    #==========================================================================
 
    # SCM PROPERTIES
 
    #==========================================================================
 

	
 
    def get_changeset(self, rev=None):
 
        return get_changeset_safe(self.scm_instance, rev)
 
@@ -1392,13 +1389,12 @@ class Repository(Base, BaseModel):
 
            _default = datetime.datetime.fromtimestamp(0)
 
            last_change = cs_cache.get('date') or _default
 
            log.debug('updated repo %s with new cs cache %s',
 
                      self.repo_name, cs_cache)
 
            self.updated_on = last_change
 
            self.changeset_cache = cs_cache
 
            Session().add(self)
 
            Session().commit()
 
        else:
 
            log.debug('changeset_cache for %s already up to date with %s',
 
                      self.repo_name, cs_cache['raw_id'])
 

	
 
    @property
 
@@ -2174,16 +2170,16 @@ class CacheInvalidation(Base, BaseModel)
 
        if valid_cache_keys and cache_key in valid_cache_keys:
 
            return True
 

	
 
        inv_obj = cls.query().filter(cls.cache_key == cache_key).scalar()
 
        if inv_obj is None:
 
            inv_obj = cls(cache_key, repo_name)
 
            Session().add(inv_obj)
 
        elif inv_obj.cache_active:
 
            return True
 
        inv_obj.cache_active = True
 
        Session().add(inv_obj)
 
        try:
 
            Session().commit()
 
        except sqlalchemy.exc.IntegrityError:
 
            log.error('commit of CacheInvalidation failed - retrying')
 
            Session().rollback()
 
            inv_obj = cls.query().filter(cls.cache_key == cache_key).scalar()
 
@@ -2532,13 +2528,12 @@ class UserNotification(Base, BaseModel):
 

	
 
    user = relationship('User')
 
    notification = relationship('Notification')
 

	
 
    def mark_as_read(self):
 
        self.read = True
 
        Session().add(self)
 

	
 

	
 
class Gist(Base, BaseModel):
 
    __tablename__ = 'gists'
 
    __table_args__ = (
 
        Index('g_gist_access_id_idx', 'gist_access_id'),
kallithea/model/notification.py
Show inline comments
 
@@ -184,13 +184,12 @@ class NotificationModel(BaseModel):
 
                obj = UserNotification.query() \
 
                        .filter(UserNotification.user == user) \
 
                        .filter(UserNotification.notification
 
                                == notification) \
 
                        .one()
 
                obj.read = True
 
                Session().add(obj)
 
                return True
 
        except Exception:
 
            log.error(traceback.format_exc())
 
            raise
 

	
 
    def mark_all_read_for_user(self, user, filter_=None):
 
@@ -202,15 +201,14 @@ class NotificationModel(BaseModel):
 
                                 Notification.notification_id))
 
        if filter_:
 
            q = q.filter(Notification.type_.in_(filter_))
 

	
 
        # this is a little inefficient but sqlalchemy doesn't support
 
        # update on joined tables :(
 
        for obj in q.all():
 
        for obj in q:
 
            obj.read = True
 
            Session().add(obj)
 

	
 
    def get_unread_cnt_for_user(self, user):
 
        user = self._get_user(user)
 
        return UserNotification.query() \
 
                .filter(UserNotification.read == False) \
 
                .filter(UserNotification.user == user).count()
kallithea/model/pull_request.py
Show inline comments
 
@@ -203,7 +203,6 @@ class PullRequestModel(BaseModel):
 
        Session().delete(pull_request)
 

	
 
    def close_pull_request(self, pull_request):
 
        pull_request = PullRequest.guess_instance(pull_request)
 
        pull_request.status = PullRequest.STATUS_CLOSED
 
        pull_request.updated_on = datetime.datetime.now()
 
        Session().add(pull_request)
kallithea/model/user.py
Show inline comments
 
@@ -403,13 +403,12 @@ class UserModel(BaseModel):
 
        from kallithea.lib import auth
 
        user = User.get_by_email(user_email)
 
        if user is not None:
 
            if not self.can_change_password(user):
 
                raise Exception('trying to change password for external user')
 
            user.password = auth.get_crypt_password(new_passwd)
 
            Session().add(user)
 
            Session().commit()
 
            log.info('change password for %s', user_email)
 
        if new_passwd is None:
 
            raise Exception('unable to set new password')
 

	
 
        tasks.send_email([user_email],
kallithea/tests/api/api_base.py
Show inline comments
 
@@ -276,13 +276,12 @@ class _BaseTestApi(object):
 
        self._compare_error(id_, expected, given=response.body)
 

	
 
    def test_api_pull(self):
 
        repo_name = u'test_pull'
 
        r = fixture.create_repo(repo_name, repo_type=self.REPO_TYPE)
 
        r.clone_uri = os.path.join(Ui.get_by_key('paths', '/').ui_value, self.REPO)
 
        Session.add(r)
 
        Session.commit()
 

	
 
        id_, params = _build_data(self.apikey, 'pull',
 
                                  repoid=repo_name,)
 
        response = api_call(self, params)
 

	
kallithea/tests/fixture.py
Show inline comments
 
@@ -55,20 +55,18 @@ class Fixture(object):
 

	
 
        class context(object):
 
            def __enter__(self):
 
                anon = User.get_default_user()
 
                self._before = anon.active
 
                anon.active = status
 
                Session().add(anon)
 
                Session().commit()
 
                invalidate_all_caches()
 

	
 
            def __exit__(self, exc_type, exc_val, exc_tb):
 
                anon = User.get_default_user()
 
                anon.active = self._before
 
                Session().add(anon)
 
                Session().commit()
 

	
 
        return context()
 

	
 
    def _get_repo_create_params(self, **custom):
 
        defs = dict(
kallithea/tests/functional/test_admin_gists.py
Show inline comments
 
@@ -87,13 +87,12 @@ class TestGistsController(TestController
 
        response.mustcontain('Filename cannot be inside a directory')
 

	
 
    def test_access_expired_gist(self):
 
        self.log_user()
 
        gist = _create_gist('never-see-me')
 
        gist.gist_expires = 0  # 1970
 
        Session().add(gist)
 
        Session().commit()
 

	
 
        response = self.app.get(url('gist', gist_id=gist.gist_access_id), status=404)
 

	
 
    def test_create_private(self):
 
        self.log_user()
kallithea/tests/functional/test_admin_repos.py
Show inline comments
 
@@ -473,13 +473,12 @@ class _BaseTestCase(TestController):
 
        perm = _get_permission_for_user(user='default', repo=self.REPO)
 
        assert len(perm), 1
 
        assert perm[0].permission.permission_name == 'repository.none'
 

	
 
        #update this permission back
 
        perm[0].permission = Permission.get_by_key('repository.read')
 
        Session().add(perm[0])
 
        Session().commit()
 

	
 
    def test_set_repo_fork_has_no_self_id(self):
 
        self.log_user()
 
        repo = Repository.get_by_repo_name(self.REPO)
 
        response = self.app.get(url('edit_repo_advanced', repo_name=self.REPO))
kallithea/tests/functional/test_files.py
Show inline comments
 
@@ -19,13 +19,12 @@ HG_NODE_HISTORY = fixture.load_resource(
 
GIT_NODE_HISTORY = fixture.load_resource('git_node_history_response.json')
 

	
 

	
 
def _set_downloads(repo_name, set_to):
 
    repo = Repository.get_by_repo_name(repo_name)
 
    repo.enable_downloads = set_to
 
    Session().add(repo)
 
    Session().commit()
 

	
 

	
 
class TestFilesController(TestController):
 

	
 
    def test_index(self):
kallithea/tests/functional/test_login.py
Show inline comments
 
@@ -493,13 +493,12 @@ class TestLoginController(TestController
 
            assert ['ChangesetController:changeset_raw'] == whitelist['api_access_controllers_whitelist']
 

	
 
            new_api_key = ApiKeyModel().create(TEST_USER_ADMIN_LOGIN, u'test')
 
            Session().commit()
 
            #patch the API key and make it expired
 
            new_api_key.expires = 0
 
            Session().add(new_api_key)
 
            Session().commit()
 
            with fixture.anon_access(False):
 
                self.app.get(url(controller='changeset',
 
                                 action='changeset_raw',
 
                                 repo_name=HG_REPO, revision='tip',
 
                                 api_key=new_api_key.api_key),
kallithea/tests/functional/test_pullrequests.py
Show inline comments
 
@@ -144,13 +144,12 @@ class TestPullrequestsController(TestCon
 
        response.mustcontain('Invalid reviewer "%s" specified' % invalid_user_id)
 

	
 
class TestPullrequestsGetRepoRefs(TestController):
 

	
 
    def setup_method(self, method):
 
        self.main = fixture.create_repo(u'main', repo_type='hg')
 
        Session.add(self.main)
 
        Session.commit()
 
        self.c = PullrequestsController()
 

	
 
    def teardown_method(self, method):
 
        fixture.destroy_repo(u'main')
 
        Session.commit()
kallithea/tests/functional/test_summary.py
Show inline comments
 
@@ -109,13 +109,12 @@ class TestSummaryController(TestControll
 
            """<i class="icon-globe">"""
 
        )
 

	
 
    def _enable_stats(self, repo):
 
        r = Repository.get_by_repo_name(repo)
 
        r.enable_statistics = True
 
        Session().add(r)
 
        Session().commit()
 

	
 
    def test_index_trending(self):
 
        self.log_user()
 
        #codes stats
 
        self._enable_stats(HG_REPO)
kallithea/tests/models/test_permissions.py
Show inline comments
 
@@ -687,11 +687,10 @@ class TestPermissions(TestController):
 
        #now modify permissions
 
        p = UserToPerm.query() \
 
                .filter(UserToPerm.user == self.u1) \
 
                .filter(UserToPerm.permission == old) \
 
                .one()
 
        p.permission = new
 
        Session().add(p)
 
        Session().commit()
 

	
 
        PermissionModel().create_default_permissions(user=self.u1)
 
        self._test_def_perm_equal(user=self.u1)
kallithea/tests/models/test_settings.py
Show inline comments
 
@@ -29,13 +29,12 @@ def test_list_valued_setting_creation_re
 
        Session().delete(setting)
 
        Session().flush()
 

	
 
def test_list_valued_setting_update():
 
    assert Setting.get_by_name(name) is None
 
    setting = Setting.create_or_update(name, 'spam', type='list')
 
    Session().add(setting)
 
    Session().flush()
 
    try:
 
        assert setting.app_settings_value == [u'spam']
 
        # Assign back setting value.
 
        setting.app_settings_value = setting.app_settings_value
 
        # Quirk: value is stringified on write and listified on read.
kallithea/tests/other/manual_test_vcs_operations.py
Show inline comments
 
@@ -157,13 +157,12 @@ def _add_files_and_push(vcs, DEST, **kwa
 
    return stdout, stderr
 

	
 

	
 
def set_anonymous_access(enable=True):
 
    user = User.get_by_username(User.DEFAULT_USER)
 
    user.active = enable
 
    Session().add(user)
 
    Session().commit()
 
    print '\tanonymous access is now:', enable
 
    if enable != User.get_by_username(User.DEFAULT_USER).active:
 
        raise Exception('Cannot set anonymous access')
 

	
 

	
 
@@ -188,19 +187,17 @@ class TestVCSOperations(TestController):
 
        set_anonymous_access(False)
 

	
 
    def setup_method(self, method):
 
        r = Repository.get_by_repo_name(GIT_REPO)
 
        Repository.unlock(r)
 
        r.enable_locking = False
 
        Session().add(r)
 
        Session().commit()
 

	
 
        r = Repository.get_by_repo_name(HG_REPO)
 
        Repository.unlock(r)
 
        r.enable_locking = False
 
        Session().add(r)
 
        Session().commit()
 

	
 
    def test_clone_hg_repo_by_admin(self):
 
        clone_url = _construct_url(HG_REPO)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('hg clone', clone_url)
 

	
 
@@ -277,15 +274,15 @@ class TestVCSOperations(TestController):
 

	
 
    def test_push_invalidates_cache_hg(self):
 
        key = CacheInvalidation.query().filter(CacheInvalidation.cache_key
 
                                               ==HG_REPO).scalar()
 
        if not key:
 
            key = CacheInvalidation(HG_REPO, HG_REPO)
 
            Session().add(key)
 

	
 
        key.cache_active = True
 
        Session().add(key)
 
        Session().commit()
 

	
 
        DEST = _get_tmp_dir()
 
        clone_url = _construct_url(HG_REPO, dest=DEST)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('hg clone', clone_url)
 

	
 
@@ -300,15 +297,15 @@ class TestVCSOperations(TestController):
 

	
 
    def test_push_invalidates_cache_git(self):
 
        key = CacheInvalidation.query().filter(CacheInvalidation.cache_key
 
                                               ==GIT_REPO).scalar()
 
        if not key:
 
            key = CacheInvalidation(GIT_REPO, GIT_REPO)
 
            Session().add(key)
 

	
 
        key.cache_active = True
 
        Session().add(key)
 
        Session().commit()
 

	
 
        DEST = _get_tmp_dir()
 
        clone_url = _construct_url(GIT_REPO, dest=DEST)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('git clone', clone_url)
 

	
 
@@ -364,13 +361,12 @@ class TestVCSOperations(TestController):
 
        assert 'not found' in stderr
 

	
 
    def test_clone_and_create_lock_hg(self):
 
        # enable locking
 
        r = Repository.get_by_repo_name(HG_REPO)
 
        r.enable_locking = True
 
        Session().add(r)
 
        Session().commit()
 
        # clone
 
        clone_url = _construct_url(HG_REPO)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('hg clone', clone_url)
 

	
 
        #check if lock was made
 
@@ -378,13 +374,12 @@ class TestVCSOperations(TestController):
 
        assert r.locked[0] == User.get_by_username(TEST_USER_ADMIN_LOGIN).user_id
 

	
 
    def test_clone_and_create_lock_git(self):
 
        # enable locking
 
        r = Repository.get_by_repo_name(GIT_REPO)
 
        r.enable_locking = True
 
        Session().add(r)
 
        Session().commit()
 
        # clone
 
        clone_url = _construct_url(GIT_REPO)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('git clone', clone_url)
 

	
 
        #check if lock was made
 
@@ -466,13 +461,12 @@ class TestVCSOperations(TestController):
 
    def test_push_unlocks_repository_hg(self):
 
        # enable locking
 
        fork_name = '%s_fork%s' % (HG_REPO, _RandomNameSequence().next())
 
        fixture.create_fork(HG_REPO, fork_name)
 
        r = Repository.get_by_repo_name(fork_name)
 
        r.enable_locking = True
 
        Session().add(r)
 
        Session().commit()
 
        #clone some temp
 
        DEST = _get_tmp_dir()
 
        clone_url = _construct_url(fork_name, dest=DEST)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('hg clone', clone_url)
 

	
 
@@ -493,13 +487,12 @@ class TestVCSOperations(TestController):
 
    def test_push_unlocks_repository_git(self):
 
        # enable locking
 
        fork_name = '%s_fork%s' % (GIT_REPO, _RandomNameSequence().next())
 
        fixture.create_fork(GIT_REPO, fork_name)
 
        r = Repository.get_by_repo_name(fork_name)
 
        r.enable_locking = True
 
        Session().add(r)
 
        Session().commit()
 
        #clone some temp
 
        DEST = _get_tmp_dir()
 
        clone_url = _construct_url(fork_name, dest=DEST)
 
        stdout, stderr = Command(tempfile.gettempdir()).execute('git clone', clone_url)
 

	
0 comments (0 inline, 0 general)