# HG changeset patch # User Søren Løvborg # Date 2016-09-13 19:19:59 # Node ID 4136526cce20cdd8ae24f5f55991f6ff3e3a5848 # Parent f35ddb6546688a875002a4b6219eefc85a409e4c 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. diff --git a/kallithea/controllers/admin/user_groups.py b/kallithea/controllers/admin/user_groups.py --- a/kallithea/controllers/admin/user_groups.py +++ b/kallithea/controllers/admin/user_groups.py @@ -378,7 +378,6 @@ class UserGroupsController(BaseControlle inherit_perms = form_result['inherit_default_permissions'] user_group.inherit_default_permissions = inherit_perms - Session().add(user_group) usergroup_model = UserGroupModel() defs = UserGroupToPerm.query() \ diff --git a/kallithea/controllers/admin/users.py b/kallithea/controllers/admin/users.py --- a/kallithea/controllers/admin/users.py +++ b/kallithea/controllers/admin/users.py @@ -326,7 +326,6 @@ class UsersController(BaseController): inherit_perms = form_result['inherit_default_permissions'] user.inherit_default_permissions = inherit_perms - Session().add(user) user_model = UserModel() defs = UserToPerm.query() \ diff --git a/kallithea/lib/db_manage.py b/kallithea/lib/db_manage.py --- a/kallithea/lib/db_manage.py +++ b/kallithea/lib/db_manage.py @@ -479,7 +479,6 @@ class DbManage(object): 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): diff --git a/kallithea/model/db.py b/kallithea/model/db.py --- a/kallithea/model/db.py +++ b/kallithea/model/db.py @@ -667,7 +667,6 @@ class User(Base, BaseModel): 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 @@ -1288,13 +1287,11 @@ class Repository(Base, BaseModel): 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 @@ -1346,7 +1343,7 @@ class Repository(Base, BaseModel): def set_state(self, state): self.repo_state = state - Session().add(self) + #========================================================================== # SCM PROPERTIES #========================================================================== @@ -1395,7 +1392,6 @@ class Repository(Base, BaseModel): 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', @@ -2177,10 +2173,10 @@ class CacheInvalidation(Base, BaseModel) 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: @@ -2535,7 +2531,6 @@ class UserNotification(Base, BaseModel): def mark_as_read(self): self.read = True - Session().add(self) class Gist(Base, BaseModel): diff --git a/kallithea/model/notification.py b/kallithea/model/notification.py --- a/kallithea/model/notification.py +++ b/kallithea/model/notification.py @@ -187,7 +187,6 @@ class NotificationModel(BaseModel): == notification) \ .one() obj.read = True - Session().add(obj) return True except Exception: log.error(traceback.format_exc()) @@ -205,9 +204,8 @@ class NotificationModel(BaseModel): # 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) diff --git a/kallithea/model/pull_request.py b/kallithea/model/pull_request.py --- a/kallithea/model/pull_request.py +++ b/kallithea/model/pull_request.py @@ -206,4 +206,3 @@ class PullRequestModel(BaseModel): pull_request = PullRequest.guess_instance(pull_request) pull_request.status = PullRequest.STATUS_CLOSED pull_request.updated_on = datetime.datetime.now() - Session().add(pull_request) diff --git a/kallithea/model/user.py b/kallithea/model/user.py --- a/kallithea/model/user.py +++ b/kallithea/model/user.py @@ -406,7 +406,6 @@ class UserModel(BaseModel): 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: diff --git a/kallithea/tests/api/api_base.py b/kallithea/tests/api/api_base.py --- a/kallithea/tests/api/api_base.py +++ b/kallithea/tests/api/api_base.py @@ -279,7 +279,6 @@ class _BaseTestApi(object): 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', diff --git a/kallithea/tests/fixture.py b/kallithea/tests/fixture.py --- a/kallithea/tests/fixture.py +++ b/kallithea/tests/fixture.py @@ -58,14 +58,12 @@ class Fixture(object): 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() diff --git a/kallithea/tests/functional/test_admin_gists.py b/kallithea/tests/functional/test_admin_gists.py --- a/kallithea/tests/functional/test_admin_gists.py +++ b/kallithea/tests/functional/test_admin_gists.py @@ -90,7 +90,6 @@ class TestGistsController(TestController 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) diff --git a/kallithea/tests/functional/test_admin_repos.py b/kallithea/tests/functional/test_admin_repos.py --- a/kallithea/tests/functional/test_admin_repos.py +++ b/kallithea/tests/functional/test_admin_repos.py @@ -476,7 +476,6 @@ class _BaseTestCase(TestController): #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): diff --git a/kallithea/tests/functional/test_files.py b/kallithea/tests/functional/test_files.py --- a/kallithea/tests/functional/test_files.py +++ b/kallithea/tests/functional/test_files.py @@ -22,7 +22,6 @@ GIT_NODE_HISTORY = fixture.load_resource 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() diff --git a/kallithea/tests/functional/test_login.py b/kallithea/tests/functional/test_login.py --- a/kallithea/tests/functional/test_login.py +++ b/kallithea/tests/functional/test_login.py @@ -496,7 +496,6 @@ class TestLoginController(TestController 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', diff --git a/kallithea/tests/functional/test_pullrequests.py b/kallithea/tests/functional/test_pullrequests.py --- a/kallithea/tests/functional/test_pullrequests.py +++ b/kallithea/tests/functional/test_pullrequests.py @@ -147,7 +147,6 @@ class TestPullrequestsGetRepoRefs(TestCo def setup_method(self, method): self.main = fixture.create_repo(u'main', repo_type='hg') - Session.add(self.main) Session.commit() self.c = PullrequestsController() diff --git a/kallithea/tests/functional/test_summary.py b/kallithea/tests/functional/test_summary.py --- a/kallithea/tests/functional/test_summary.py +++ b/kallithea/tests/functional/test_summary.py @@ -112,7 +112,6 @@ class TestSummaryController(TestControll 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): diff --git a/kallithea/tests/models/test_permissions.py b/kallithea/tests/models/test_permissions.py --- a/kallithea/tests/models/test_permissions.py +++ b/kallithea/tests/models/test_permissions.py @@ -690,7 +690,6 @@ class TestPermissions(TestController): .filter(UserToPerm.permission == old) \ .one() p.permission = new - Session().add(p) Session().commit() PermissionModel().create_default_permissions(user=self.u1) diff --git a/kallithea/tests/models/test_settings.py b/kallithea/tests/models/test_settings.py --- a/kallithea/tests/models/test_settings.py +++ b/kallithea/tests/models/test_settings.py @@ -32,7 +32,6 @@ def test_list_valued_setting_creation_re 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'] diff --git a/kallithea/tests/other/manual_test_vcs_operations.py b/kallithea/tests/other/manual_test_vcs_operations.py --- a/kallithea/tests/other/manual_test_vcs_operations.py +++ b/kallithea/tests/other/manual_test_vcs_operations.py @@ -160,7 +160,6 @@ def _add_files_and_push(vcs, DEST, **kwa 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: @@ -191,13 +190,11 @@ class TestVCSOperations(TestController): 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): @@ -280,9 +277,9 @@ class TestVCSOperations(TestController): ==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() @@ -303,9 +300,9 @@ class TestVCSOperations(TestController): ==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() @@ -367,7 +364,6 @@ class TestVCSOperations(TestController): # 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) @@ -381,7 +377,6 @@ class TestVCSOperations(TestController): # 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) @@ -469,7 +464,6 @@ class TestVCSOperations(TestController): 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() @@ -496,7 +490,6 @@ class TestVCSOperations(TestController): 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()