Mercurial > kallithea
changeset 6229:4136526cce20
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.
| author | Søren Løvborg <sorenl@unity3d.com> |
|---|---|
| date | Tue, 13 Sep 2016 19:19:59 +0200 |
| parents | f35ddb654668 |
| children | cd6176c0634a |
| files | kallithea/controllers/admin/user_groups.py kallithea/controllers/admin/users.py kallithea/lib/db_manage.py kallithea/model/db.py kallithea/model/notification.py kallithea/model/pull_request.py kallithea/model/user.py kallithea/tests/api/api_base.py kallithea/tests/fixture.py kallithea/tests/functional/test_admin_gists.py kallithea/tests/functional/test_admin_repos.py kallithea/tests/functional/test_files.py kallithea/tests/functional/test_login.py kallithea/tests/functional/test_pullrequests.py kallithea/tests/functional/test_summary.py kallithea/tests/models/test_permissions.py kallithea/tests/models/test_settings.py kallithea/tests/other/manual_test_vcs_operations.py |
| diffstat | 18 files changed, 5 insertions(+), 35 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/admin/user_groups.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/controllers/admin/user_groups.py Tue Sep 13 19:19:59 2016 +0200 @@ -378,7 +378,6 @@ inherit_perms = form_result['inherit_default_permissions'] user_group.inherit_default_permissions = inherit_perms - Session().add(user_group) usergroup_model = UserGroupModel() defs = UserGroupToPerm.query() \
--- a/kallithea/controllers/admin/users.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/controllers/admin/users.py Tue Sep 13 19:19:59 2016 +0200 @@ -326,7 +326,6 @@ inherit_perms = form_result['inherit_default_permissions'] user.inherit_default_permissions = inherit_perms - Session().add(user) user_model = UserModel() defs = UserToPerm.query() \
--- a/kallithea/lib/db_manage.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/lib/db_manage.py Tue Sep 13 19:19:59 2016 +0200 @@ -479,7 +479,6 @@ 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):
--- a/kallithea/model/db.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/model/db.py Tue Sep 13 19:19:59 2016 +0200 @@ -667,7 +667,6 @@ 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 @@ 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 @@ def set_state(self, state): self.repo_state = state - Session().add(self) + #========================================================================== # SCM PROPERTIES #========================================================================== @@ -1395,7 +1392,6 @@ 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 @@ 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 @@ def mark_as_read(self): self.read = True - Session().add(self) class Gist(Base, BaseModel):
--- a/kallithea/model/notification.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/model/notification.py Tue Sep 13 19:19:59 2016 +0200 @@ -187,7 +187,6 @@ == notification) \ .one() obj.read = True - Session().add(obj) return True except Exception: log.error(traceback.format_exc()) @@ -205,9 +204,8 @@ # 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)
--- a/kallithea/model/pull_request.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/model/pull_request.py Tue Sep 13 19:19:59 2016 +0200 @@ -206,4 +206,3 @@ pull_request = PullRequest.guess_instance(pull_request) pull_request.status = PullRequest.STATUS_CLOSED pull_request.updated_on = datetime.datetime.now() - Session().add(pull_request)
--- a/kallithea/model/user.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/model/user.py Tue Sep 13 19:19:59 2016 +0200 @@ -406,7 +406,6 @@ 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:
--- a/kallithea/tests/api/api_base.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/api/api_base.py Tue Sep 13 19:19:59 2016 +0200 @@ -279,7 +279,6 @@ 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',
--- a/kallithea/tests/fixture.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/fixture.py Tue Sep 13 19:19:59 2016 +0200 @@ -58,14 +58,12 @@ 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()
--- a/kallithea/tests/functional/test_admin_gists.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/functional/test_admin_gists.py Tue Sep 13 19:19:59 2016 +0200 @@ -90,7 +90,6 @@ 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)
--- a/kallithea/tests/functional/test_admin_repos.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/functional/test_admin_repos.py Tue Sep 13 19:19:59 2016 +0200 @@ -476,7 +476,6 @@ #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):
--- a/kallithea/tests/functional/test_files.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/functional/test_files.py Tue Sep 13 19:19:59 2016 +0200 @@ -22,7 +22,6 @@ 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()
--- a/kallithea/tests/functional/test_login.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/functional/test_login.py Tue Sep 13 19:19:59 2016 +0200 @@ -496,7 +496,6 @@ 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',
--- a/kallithea/tests/functional/test_pullrequests.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/functional/test_pullrequests.py Tue Sep 13 19:19:59 2016 +0200 @@ -147,7 +147,6 @@ def setup_method(self, method): self.main = fixture.create_repo(u'main', repo_type='hg') - Session.add(self.main) Session.commit() self.c = PullrequestsController()
--- a/kallithea/tests/functional/test_summary.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/functional/test_summary.py Tue Sep 13 19:19:59 2016 +0200 @@ -112,7 +112,6 @@ 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):
--- a/kallithea/tests/models/test_permissions.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/models/test_permissions.py Tue Sep 13 19:19:59 2016 +0200 @@ -690,7 +690,6 @@ .filter(UserToPerm.permission == old) \ .one() p.permission = new - Session().add(p) Session().commit() PermissionModel().create_default_permissions(user=self.u1)
--- a/kallithea/tests/models/test_settings.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/models/test_settings.py Tue Sep 13 19:19:59 2016 +0200 @@ -32,7 +32,6 @@ 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']
--- a/kallithea/tests/other/manual_test_vcs_operations.py Thu Sep 15 13:57:47 2016 +0200 +++ b/kallithea/tests/other/manual_test_vcs_operations.py Tue Sep 13 19:19:59 2016 +0200 @@ -160,7 +160,6 @@ 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 @@ 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 @@ ==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 @@ ==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 @@ # 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 @@ # 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 @@ 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 @@ 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()