Repository: allura Updated Branches: refs/heads/master 40ee1b5c9 -> 612eccae9
[#8194] store merge requests' new commits Project: http://git-wip-us.apache.org/repos/asf/allura/repo Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/612eccae Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/612eccae Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/612eccae Branch: refs/heads/master Commit: 612eccae9bb00f913c823aea9fbea2d12f18c35b Parents: 40ee1b5 Author: Dave Brondsema <d...@brondsema.net> Authored: Thu Mar 8 14:30:39 2018 -0500 Committer: Kenton Taylor <ktay...@slashdotmedia.com> Committed: Fri Mar 9 15:28:59 2018 -0500 ---------------------------------------------------------------------- Allura/allura/controllers/repository.py | 6 ++---- Allura/allura/model/repository.py | 22 ++++++++++++++-------- Allura/allura/tests/model/test_repo.py | 25 +++++++++++++++++++++---- 3 files changed, 37 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/allura/blob/612eccae/Allura/allura/controllers/repository.py ---------------------------------------------------------------------- diff --git a/Allura/allura/controllers/repository.py b/Allura/allura/controllers/repository.py index 8dcfd46..84c0baa 100644 --- a/Allura/allura/controllers/repository.py +++ b/Allura/allura/controllers/repository.py @@ -466,15 +466,12 @@ class MergeRequestController(object): if self.req.description != kw['description']: changes['Description'] = h.unidiff(self.req.description, kw['description']) self.req.description = kw['description'] - with self.req.push_downstream_context(): - self.req.downstream['commit_id'] = c.app.repo.commit(kw['source_branch'])._id if changes: self.req.add_meta_post(changes=changes) g.director.create_activity(c.user, 'updated', self.req, related_nodes=[c.project], tags=['merge-request']) - - redirect(self.req.url()) + self.refresh() @expose() @require_post() @@ -493,6 +490,7 @@ class MergeRequestController(object): def refresh(self, **kw): require_access(self.req, 'read') with self.req.push_downstream_context(): + self.req.new_commits = None # invalidate this cache self.req.downstream['commit_id'] = c.app.repo.commit(self.req.source_branch)._id redirect(self.req.url()) http://git-wip-us.apache.org/repos/asf/allura/blob/612eccae/Allura/allura/model/repository.py ---------------------------------------------------------------------- diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py index fe898e9..a859c6d 100644 --- a/Allura/allura/model/repository.py +++ b/Allura/allura/model/repository.py @@ -802,6 +802,7 @@ class MergeRequest(VersionedArtifact, ActivityObject): summary = FieldProperty(str) description = FieldProperty(str) can_merge_cache = FieldProperty({str: bool}) + new_commits = FieldProperty([S.Anything], if_missing=None) # don't access directly, use `commits` property @property def activity_name(self): @@ -839,13 +840,17 @@ class MergeRequest(VersionedArtifact, ActivityObject): def push_downstream_context(self): return h.push_context(self.downstream.project_id, self.downstream.mount_point) - @LazyProperty + @property def commits(self): - return self._commits() + if self.new_commits is not None: + return self.new_commits - def _commits(self): with self.push_downstream_context(): - return c.app.repo.merge_request_commits(self) + # update the cache key only, being careful not to touch anything else that ming will try to flush later + # this avoids race conditions with the `set_can_merge_cache()` caching and clobbering fields + new_commits = c.app.repo.merge_request_commits(self) + self.query.update({'$set': {'new_commits': new_commits}}) + return new_commits @classmethod def upsert(cls, **kw): @@ -908,11 +913,12 @@ class MergeRequest(VersionedArtifact, ActivityObject): return self.can_merge_cache.get(key) def set_can_merge_cache(self, val): - from allura import model as M key = self.can_merge_cache_key() - with utils.skip_mod_date(M.MergeRequest): - self.can_merge_cache[key] = val - session(self).flush(self) + # update the cache key only, being careful not to touch anything else that ming will try to flush later + # this avoids race conditions with the `commits()` caching and clobbering fields + can_merge_cache = self.can_merge_cache._deinstrument() + can_merge_cache[key] = val + self.query.update({'$set': {'can_merge_cache': can_merge_cache}}) def can_merge(self): """ http://git-wip-us.apache.org/repos/asf/allura/blob/612eccae/Allura/allura/tests/model/test_repo.py ---------------------------------------------------------------------- diff --git a/Allura/allura/tests/model/test_repo.py b/Allura/allura/tests/model/test_repo.py index 189fdf4..efbbb4d 100644 --- a/Allura/allura/tests/model/test_repo.py +++ b/Allura/allura/tests/model/test_repo.py @@ -677,6 +677,7 @@ class TestModelCache(unittest.TestCase): class TestMergeRequest(object): + def setUp(self): setup_basic_test() setup_global_objects() @@ -685,10 +686,19 @@ class TestMergeRequest(object): downstream={'commit_id': '12345'}, request_number=1, ) - self.mr.app = mock.Mock(forkable=True, url='/mock-app-url/') - self.mr.app.repo.commit.return_value = mock.Mock(_id='09876') - self.mr.merge_allowed = mock.Mock(return_value=True) - self.mr.discussion_thread = mock.Mock() + self._set_mr_mock_attrs(self.mr) + + def _set_mr_mock_attrs(self, mr): + mr.app = mock.Mock(forkable=True, url='/mock-app-url/') + mr.app.repo.commit.return_value = mock.Mock(_id='09876') + mr.merge_allowed = mock.Mock(return_value=True) + mr.discussion_thread = mock.Mock() + + def _reload_mr_from_db(self, mr): + session(mr).refresh(mr) + mr = M.MergeRequest.query.get(_id=mr._id) + self._set_mr_mock_attrs(mr) + return mr def test_can_merge_cache_key(self): key = self.mr.can_merge_cache_key() @@ -720,9 +730,16 @@ class TestMergeRequest(object): @mock.patch('allura.tasks.repo_tasks.can_merge', autospec=True) def test_can_merge_cached(self, can_merge_task): + # this test has to flush `mr` to the db and then reload it after changes, because set_can_merge_cache + # does a $set to the db and doesn't update the in-memory copy + session(self.mr).flush(self.mr) + self.mr.set_can_merge_cache(False) + self.mr = self._reload_mr_from_db(self.mr) assert_equal(self.mr.can_merge(), False) + self.mr.set_can_merge_cache(True) + self.mr = self._reload_mr_from_db(self.mr) assert_equal(self.mr.can_merge(), True) assert_equal(can_merge_task.post.call_count, 0)
