kallithea Changeset - 4136526cce20

Changeset - 4136526cce20

Parent rev.

Child rev.

[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:

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

0 comments (0 inline, 0 general)

kallithea/controllers/admin/user_groups.py

➞

Show inline comments

@@ @@ -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() \

kallithea/controllers/admin/users.py

➞

Show inline comments

@@ @@ -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() \

kallithea/lib/db_manage.py

➞

Show inline comments

@@ @@ -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):

kallithea/model/db.py

➞

Show inline comments

@@ @@ -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):

kallithea/model/notification.py

➞

Show inline comments

@@ @@ -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)

kallithea/model/pull_request.py

➞

Show inline comments

@@ @@ -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)

kallithea/model/user.py

➞

Show inline comments

@@ @@ -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:

kallithea/tests/api/api_base.py

➞

Show inline comments

@@ @@ -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',

kallithea/tests/fixture.py

➞

Show inline comments

@@ @@ -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()

kallithea/tests/functional/test_admin_gists.py

➞

Show inline comments

@@ @@ -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)

kallithea/tests/functional/test_admin_repos.py

➞

Show inline comments

@@ @@ -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):

kallithea/tests/functional/test_files.py

➞

Show inline comments

@@ @@ -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()

kallithea/tests/functional/test_login.py

➞

Show inline comments

@@ @@ -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',

kallithea/tests/functional/test_pullrequests.py

➞

Show inline comments

@@ @@ -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()

kallithea/tests/functional/test_summary.py

➞

Show inline comments

@@ @@ -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):

kallithea/tests/models/test_permissions.py

➞

Show inline comments

@@ @@ -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)

kallithea/tests/models/test_settings.py

➞

Show inline comments

@@ @@ -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']

kallithea/tests/other/manual_test_vcs_operations.py

➞

Show inline comments

@@ @@ -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()

0 comments (0 inline, 0 general)