You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2018/03/09 17:30:52 UTC
allura git commit: [#8194] store merge requests' new commits
Repository: allura
Updated Branches:
refs/heads/db/8194 [created] 4b9d7b22d
[#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/4b9d7b22
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/4b9d7b22
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/4b9d7b22
Branch: refs/heads/db/8194
Commit: 4b9d7b22dc5915cb97d99be1f9fee5da554b84a9
Parents: 334868b
Author: Dave Brondsema <da...@brondsema.net>
Authored: Thu Mar 8 14:30:39 2018 -0500
Committer: Dave Brondsema <da...@brondsema.net>
Committed: Fri Mar 9 12:30:46 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/4b9d7b22/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/4b9d7b22/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/4b9d7b22/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)