pull-requests: introduce operation state for pull requests to prevent from

locks during merge/update operations.

rhodecode/lib/dbmigrate/versions/092_version_4_16_0.py

@@ -0,0 +1,37 b''
+import logging
+
+from sqlalchemy import *
+
+from rhodecode.model import meta
+from rhodecode.lib.dbmigrate.versions import _reset_base, notify
+
+log = logging.getLogger(__name__)
+
+
+def upgrade(migrate_engine):
+    """
+    Upgrade operations go here.
+    Don't create your own engine; bind migrate_engine to your metadata
+    """
+    _reset_base(migrate_engine)
+    from rhodecode.lib.dbmigrate.schema import db_4_13_0_0 as db
+
+    pull_request = db.PullRequest.__table__
+    pull_request_version = db.PullRequestVersion.__table__
+
+    repo_state_1 = Column("pull_request_state", String(255), nullable=True)
+    repo_state_1.create(table=pull_request)
+
+    repo_state_2 = Column("pull_request_state", String(255), nullable=True)
+    repo_state_2.create(table=pull_request_version)
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    pass

rhodecode/lib/dbmigrate/versions/093_version_4_16_0.py

@@ -0,0 +1,41 b''
+import logging
+
+from sqlalchemy import *
+
+from rhodecode.model import meta
+from rhodecode.lib.dbmigrate.versions import _reset_base, notify
+
+log = logging.getLogger(__name__)
+
+
+def upgrade(migrate_engine):
+    """
+    Upgrade operations go here.
+    Don't create your own engine; bind migrate_engine to your metadata
+    """
+    _reset_base(migrate_engine)
+    from rhodecode.lib.dbmigrate.schema import db_4_16_0_0 as db
+
+    fixups(db, meta.Session)
+
+
+def downgrade(migrate_engine):
+    meta = MetaData()
+    meta.bind = migrate_engine
+
+
+def fixups(models, _SESSION):
+    # move the builtin token to external tokens
+
+    log.info('Updating pull request pull_request_state to %s',
+             models.PullRequest.STATE_CREATED)
+    qry = _SESSION().query(models.PullRequest)
+    qry.update({"pull_request_state": models.PullRequest.STATE_CREATED})
+    _SESSION().commit()
+
+    log.info('Updating pull_request_version pull_request_state to %s',
+             models.PullRequest.STATE_CREATED)
+    qry = _SESSION().query(models.PullRequestVersion)
+    qry.update({"pull_request_state": models.PullRequest.STATE_CREATED})
+    _SESSION().commit()
+

rhodecode/__init__.py

@@ -18,12 +18,6 b''
 # RhodeCode Enterprise Edition, including its added features, Support services,
 # and proprietary license terms, please see https://rhodecode.com/licenses/
 
-"""
-
-RhodeCode, a web based repository management software
-versioning implementation: http://www.python.org/dev/peps/pep-0386/
-"""
-
 import os
 import sys
 import platform
@@ -51,7 +45,7 b' PYRAMID_SETTINGS = {}'
 EXTENSIONS = {}
 
 __version__ = ('.'.join((str(each) for each in VERSION[:3])))
-__dbversion__ = 91  # defines current db version for migrations
+__dbversion__ = 93  # defines current db version for migrations
 __platform__ = platform.system()
 __license__ = 'AGPLv3, and Commercial License'
 __author__ = 'RhodeCode GmbH'

rhodecode/api/tests/test_get_pull_request.py

@@ -65,6 +65,7 b' class TestGetPullRequest(object):'
             'title': pull_request.title,
             'description': pull_request.description,
             'status': pull_request.status,
+            'state': pull_request.pull_request_state,
             'created_on': pull_request.created_on,
             'updated_on': pull_request.updated_on,
             'commit_ids': pull_request.revisions,

rhodecode/api/tests/test_merge_pull_request.py

@@ -29,11 +29,10 b' from rhodecode.api.tests.utils import ('
 
 @pytest.mark.usefixtures("testuser_api", "app")
 class TestMergePullRequest(object):
+
     @pytest.mark.backends("git", "hg")
     def test_api_merge_pull_request_merge_failed(self, pr_util, no_notifications):
         pull_request = pr_util.create_pull_request(mergeable=True)
-        author = pull_request.user_id
-        repo = pull_request.target_repo.repo_id
         pull_request_id = pull_request.pull_request_id
         pull_request_repo = pull_request.target_repo.repo_name
 
@@ -46,8 +45,7 b' class TestMergePullRequest(object):'
 
         # The above api call detaches the pull request DB object from the
         # session because of an unconditional transaction rollback in our
-        # middleware. Therefore we need to add it back here if we want to use
-        # it.
+        # middleware. Therefore we need to add it back here if we want to use it.
         Session().add(pull_request)
 
         expected = 'merge not possible for following reasons: ' \
@@ -55,6 +53,29 b' class TestMergePullRequest(object):'
         assert_error(id_, expected, given=response.body)
 
     @pytest.mark.backends("git", "hg")
+    def test_api_merge_pull_request_merge_failed_disallowed_state(
+            self, pr_util, no_notifications):
+        pull_request = pr_util.create_pull_request(mergeable=True, approved=True)
+        pull_request_id = pull_request.pull_request_id
+        pull_request_repo = pull_request.target_repo.repo_name
+
+        pr = PullRequest.get(pull_request_id)
+        pr.pull_request_state = pull_request.STATE_UPDATING
+        Session().add(pr)
+        Session().commit()
+
+        id_, params = build_data(
+            self.apikey, 'merge_pull_request',
+            repoid=pull_request_repo,
+            pullrequestid=pull_request_id)
+
+        response = api_call(self.app, params)
+        expected = 'Operation forbidden because pull request is in state {}, '\
+                   'only state {} is allowed.'.format(PullRequest.STATE_UPDATING,
+                                                      PullRequest.STATE_CREATED)
+        assert_error(id_, expected, given=response.body)
+
+    @pytest.mark.backends("git", "hg")
     def test_api_merge_pull_request(self, pr_util, no_notifications):
         pull_request = pr_util.create_pull_request(mergeable=True, approved=True)
         author = pull_request.user_id
@@ -123,8 +144,7 b' class TestMergePullRequest(object):'
         assert_error(id_, expected, given=response.body)
 
     @pytest.mark.backends("git", "hg")
-    def test_api_merge_pull_request_non_admin_with_userid_error(self,
-                                                                pr_util):
+    def test_api_merge_pull_request_non_admin_with_userid_error(self, pr_util):
         pull_request = pr_util.create_pull_request(mergeable=True)
         id_, params = build_data(
             self.apikey_regular, 'merge_pull_request',

rhodecode/api/views/pull_request_api.py

@@ -32,7 +32,7 b' from rhodecode.lib.base import vcs_opera'
 from rhodecode.lib.utils2 import str2bool
 from rhodecode.model.changeset_status import ChangesetStatusModel
 from rhodecode.model.comment import CommentsModel
-from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment
+from rhodecode.model.db import Session, ChangesetStatus, ChangesetComment, PullRequest
 from rhodecode.model.pull_request import PullRequestModel, MergeCheck
 from rhodecode.model.settings import SettingsModel
 from rhodecode.model.validation_schema import Invalid
@@ -128,11 +128,15 b' def get_pull_request(request, apiuser, p'
     else:
         repo = pull_request.target_repo
 
-    if not PullRequestModel().check_user_read(
-            pull_request, apiuser, api=True):
+    if not PullRequestModel().check_user_read(pull_request, apiuser, api=True):
         raise JSONRPCError('repository `%s` or pull request `%s` '
                            'does not exist' % (repoid, pullrequestid))
-    data = pull_request.get_api_data()
+
+    # NOTE(marcink): only calculate and return merge state if the pr state is 'created'
+    # otherwise we can lock the repo on calculation of merge state while update/merge
+    # is happening.
+    merge_state = pull_request.pull_request_state == pull_request.STATE_CREATED
+    data = pull_request.get_api_data(with_merge_state=merge_state)
     return data
 
 
@@ -283,8 +287,16 b' def merge_pull_request('
         else:
             raise JSONRPCError('userid is not the same as your user')
 
-    check = MergeCheck.validate(
-        pull_request, auth_user=apiuser, translator=request.translate)
+    if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+        raise JSONRPCError(
+            'Operation forbidden because pull request is in state {}, '
+            'only state {} is allowed.'.format(
+                pull_request.pull_request_state, PullRequest.STATE_CREATED))
+
+    with pull_request.set_state(PullRequest.STATE_UPDATING):
+        check = MergeCheck.validate(
+            pull_request, auth_user=apiuser,
+            translator=request.translate)
     merge_possible = not check.failed
 
     if not merge_possible:
@@ -302,8 +314,9 b' def merge_pull_request('
         request.environ, repo_name=target_repo.repo_name,
         username=apiuser.username, action='push',
         scm=target_repo.repo_type)
-    merge_response = PullRequestModel().merge_repo(
-        pull_request, apiuser, extras=extras)
+    with pull_request.set_state(PullRequest.STATE_UPDATING):
+        merge_response = PullRequestModel().merge_repo(
+            pull_request, apiuser, extras=extras)
     if merge_response.executed:
         PullRequestModel().close_pull_request(
             pull_request.pull_request_id, apiuser)
@@ -829,11 +842,18 b' def update_pull_request('
 
     commit_changes = {"added": [], "common": [], "removed": []}
     if str2bool(Optional.extract(update_commits)):
-        if PullRequestModel().has_valid_update_type(pull_request):
-            update_response = PullRequestModel().update_commits(
-                pull_request)
-            commit_changes = update_response.changes or commit_changes
-        Session().commit()
+
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            raise JSONRPCError(
+                'Operation forbidden because pull request is in state {}, '
+                'only state {} is allowed.'.format(
+                    pull_request.pull_request_state, PullRequest.STATE_CREATED))
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            if PullRequestModel().has_valid_update_type(pull_request):
+                update_response = PullRequestModel().update_commits(pull_request)
+                commit_changes = update_response.changes or commit_changes
+            Session().commit()
 
     reviewers_changes = {"added": [], "removed": []}
     if reviewers:

rhodecode/apps/repository/tests/test_repo_pullrequests.py

@@ -655,20 +655,20 b' class TestPullrequestsView(object):'
 
         # create pr from a in source to A in target
         pull_request = PullRequest()
+
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name, commit_id=commit_ids['change'])
+
         pull_request.target_repo = target
-
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor'])
+
         pull_request.revisions = [commit_ids['change']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
@@ -679,23 +679,21 b' class TestPullrequestsView(object):'
         # update PR
         self.app.post(
             route_path('pullrequest_update',
-                repo_name=target.repo_name,
-                pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token})
+                       repo_name=target.repo_name, pull_request_id=pull_request_id),
+            params={'update_commits': 'true', 'csrf_token': csrf_token})
+
+        response = self.app.get(
+            route_path('pullrequest_show',
+                       repo_name=target.repo_name,
+                       pull_request_id=pull_request.pull_request_id))
+
+        assert response.status_int == 200
+        assert 'Pull request updated to' in response.body
+        assert 'with 1 added, 0 removed commits.' in response.body
 
         # check that we have now both revisions
         pull_request = PullRequest.get(pull_request_id)
-        assert pull_request.revisions == [
-            commit_ids['change-2'], commit_ids['change']]
-
-        # TODO: johbo: this should be a test on its own
-        response = self.app.get(route_path(
-            'pullrequest_new',
-            repo_name=target.repo_name))
-        assert response.status_int == 200
-        assert 'Pull request updated to' in response.body
-        assert 'with 1 added, 0 removed commits.' in response.body
+        assert pull_request.revisions == [commit_ids['change-2'], commit_ids['change']]
 
     def test_update_target_revision(self, backend, csrf_token):
         commits = [
@@ -710,21 +708,21 b' class TestPullrequestsView(object):'
 
         # create pr from a in source to A in target
         pull_request = PullRequest()
+
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name, commit_id=commit_ids['change'])
+
         pull_request.target_repo = target
-        # TODO: johbo: Target ref should be branch based, since tip can jump
-        # from branch to branch
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor'])
+
         pull_request.revisions = [commit_ids['change']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
+
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
@@ -737,23 +735,21 b' class TestPullrequestsView(object):'
         # update PR
         self.app.post(
             route_path('pullrequest_update',
-                repo_name=target.repo_name,
-                pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token},
+                       repo_name=target.repo_name,
+                       pull_request_id=pull_request_id),
+            params={'update_commits': 'true', 'csrf_token': csrf_token},
             status=200)
 
         # check that we have now both revisions
         pull_request = PullRequest.get(pull_request_id)
         assert pull_request.revisions == [commit_ids['change-rebased']]
         assert pull_request.target_ref == 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor-new'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor-new'])
 
-        # TODO: johbo: This should be a test on its own
-        response = self.app.get(route_path(
-            'pullrequest_new',
-            repo_name=target.repo_name))
+        response = self.app.get(
+            route_path('pullrequest_show',
+                       repo_name=target.repo_name,
+                       pull_request_id=pull_request.pull_request_id))
         assert response.status_int == 200
         assert 'Pull request updated to' in response.body
         assert 'with 1 added, 1 removed commits.' in response.body
@@ -775,17 +771,14 b' class TestPullrequestsView(object):'
         # create pr from a in source to A in target
         pull_request = PullRequest()
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
+
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=backend.default_branch_name,
             commit_id=commit_ids['master-commit-3-change-2'])
 
         pull_request.target_repo = target
-        # TODO: johbo: Target ref should be branch based, since tip can jump
-        # from branch to branch
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['feat-commit-2'])
+            branch=backend.default_branch_name, commit_id=commit_ids['feat-commit-2'])
 
         pull_request.revisions = [
             commit_ids['feat-commit-1'],
@@ -793,8 +786,8 b' class TestPullrequestsView(object):'
         ]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
@@ -810,13 +803,10 b' class TestPullrequestsView(object):'
             route_path('pullrequest_update',
                 repo_name=target.repo_name,
                 pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token},
+            params={'update_commits': 'true', 'csrf_token': csrf_token},
             status=200)
 
-        response = self.app.get(route_path(
-            'pullrequest_new',
-            repo_name=target.repo_name))
+        response = self.app.get(route_path('pullrequest_new', repo_name=target.repo_name))
         assert response.status_int == 200
         response.mustcontain('Pull request updated to')
         response.mustcontain('with 0 added, 0 removed commits.')
@@ -836,21 +826,17 b' class TestPullrequestsView(object):'
         # create pr from a in source to A in target
         pull_request = PullRequest()
         pull_request.source_repo = source
-        # TODO: johbo: Make sure that we write the source ref this way!
+
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['change'])
+            branch=backend.default_branch_name, commit_id=commit_ids['change'])
         pull_request.target_repo = target
-        # TODO: johbo: Target ref should be branch based, since tip can jump
-        # from branch to branch
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend.default_branch_name,
-            commit_id=commit_ids['ancestor'])
+            branch=backend.default_branch_name, commit_id=commit_ids['ancestor'])
         pull_request.revisions = [commit_ids['change']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
         pull_request_id = pull_request.pull_request_id
@@ -863,10 +849,8 b' class TestPullrequestsView(object):'
         # update PR
         self.app.post(
             route_path('pullrequest_update',
-                repo_name=target.repo_name,
-                pull_request_id=pull_request_id),
-            params={'update_commits': 'true',
-                    'csrf_token': csrf_token},
+                       repo_name=target.repo_name, pull_request_id=pull_request_id),
+            params={'update_commits': 'true', 'csrf_token': csrf_token},
             status=200)
 
         # Expect the target reference to be updated correctly
@@ -893,13 +877,12 b' class TestPullrequestsView(object):'
         pull_request.source_ref = 'branch:{branch}:{commit_id}'.format(
             branch=branch_name, commit_id=commit_ids['new-feature'])
         pull_request.target_ref = 'branch:{branch}:{commit_id}'.format(
-            branch=backend_git.default_branch_name,
-            commit_id=commit_ids['old-feature'])
+            branch=backend_git.default_branch_name, commit_id=commit_ids['old-feature'])
         pull_request.revisions = [commit_ids['new-feature']]
         pull_request.title = u"Test"
         pull_request.description = u"Description"
-        pull_request.author = UserModel().get_by_username(
-            TEST_USER_ADMIN_LOGIN)
+        pull_request.author = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        pull_request.pull_request_state = PullRequest.STATE_CREATED
         Session().add(pull_request)
         Session().commit()
 

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -273,9 +273,22 b' class RepoPullRequestsView(RepoAppView, '
         route_name='pullrequest_show', request_method='GET',
         renderer='rhodecode:templates/pullrequests/pullrequest_show.mako')
     def pull_request_show(self):
-        pull_request_id = self.request.matchdict['pull_request_id']
+        _ = self.request.translate
+        c = self.load_default_context()
+
+        pull_request = PullRequest.get_or_404(
+            self.request.matchdict['pull_request_id'])
+        pull_request_id = pull_request.pull_request_id
 
-        c = self.load_default_context()
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            log.debug('show: forbidden because pull request is in state %s',
+                      pull_request.pull_request_state)
+            msg = _(u'Cannot show pull requests in state other than `{}`. '
+                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
+                                                      pull_request.pull_request_state)
+            h.flash(msg, category='error')
+            raise HTTPFound(h.route_path('pullrequest_show_all',
+                                         repo_name=self.db_repo_name))
 
         version = self.request.GET.get('version')
         from_version = self.request.GET.get('from_version') or version
@@ -919,12 +932,17 b' class RepoPullRequestsView(RepoAppView, '
         source_db_repo = Repository.get_by_repo_name(_form['source_repo'])
         target_db_repo = Repository.get_by_repo_name(_form['target_repo'])
 
+        if not (source_db_repo or target_db_repo):
+            h.flash(_('source_repo or target repo not found'), category='error')
+            raise HTTPFound(
+                h.route_path('pullrequest_new', repo_name=self.db_repo_name))
+
         # re-check permissions again here
         # source_repo we must have read permissions
 
         source_perm = HasRepoPermissionAny(
-            'repository.read',
-            'repository.write', 'repository.admin')(source_db_repo.repo_name)
+            'repository.read', 'repository.write', 'repository.admin')(
+            source_db_repo.repo_name)
         if not source_perm:
             msg = _('Not Enough permissions to source repo `{}`.'.format(
                 source_db_repo.repo_name))
@@ -938,8 +956,8 b' class RepoPullRequestsView(RepoAppView, '
         # target repo we must have read permissions, and also later on
         # we want to check branch permissions here
         target_perm = HasRepoPermissionAny(
-            'repository.read',
-            'repository.write', 'repository.admin')(target_db_repo.repo_name)
+            'repository.read', 'repository.write', 'repository.admin')(
+            target_db_repo.repo_name)
         if not target_perm:
             msg = _('Not Enough permissions to target repo `{}`.'.format(
                 target_db_repo.repo_name))
@@ -1042,6 +1060,15 b' class RepoPullRequestsView(RepoAppView, '
             h.flash(msg, category='error')
             return True
 
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            log.debug('update: forbidden because pull request is in state %s',
+                      pull_request.pull_request_state)
+            msg = _(u'Cannot update pull requests in state other than `{}`. '
+                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
+                                                      pull_request.pull_request_state)
+            h.flash(msg, category='error')
+            return True
+
         # only owner or admin can update it
         allowed_to_update = PullRequestModel().check_user_update(
             pull_request, self._rhodecode_user)
@@ -1084,7 +1111,9 b' class RepoPullRequestsView(RepoAppView, '
 
     def _update_commits(self, pull_request):
         _ = self.request.translate
-        resp = PullRequestModel().update_commits(pull_request)
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            resp = PullRequestModel().update_commits(pull_request)
 
         if resp.executed:
 
@@ -1097,10 +1126,9 b' class RepoPullRequestsView(RepoAppView, '
             else:
                 changed = 'nothing'
 
-            msg = _(
-                u'Pull request updated to "{source_commit_id}" with '
-                u'{count_added} added, {count_removed} removed commits. '
-                u'Source of changes: {change_source}')
+            msg = _(u'Pull request updated to "{source_commit_id}" with '
+                    u'{count_added} added, {count_removed} removed commits. '
+                    u'Source of changes: {change_source}')
             msg = msg.format(
                 source_commit_id=pull_request.source_ref_parts.commit_id,
                 count_added=len(resp.changes.added),
@@ -1109,8 +1137,7 b' class RepoPullRequestsView(RepoAppView, '
             h.flash(msg, category='success')
 
             channel = '/repo${}$/pr/{}'.format(
-                pull_request.target_repo.repo_name,
-                pull_request.pull_request_id)
+                pull_request.target_repo.repo_name, pull_request.pull_request_id)
             message = msg + (
                 ' - <a onclick="window.location.reload()">'
                 '<strong>{}</strong></a>'.format(_('Reload page')))
@@ -1143,11 +1170,26 b' class RepoPullRequestsView(RepoAppView, '
         """
         pull_request = PullRequest.get_or_404(
             self.request.matchdict['pull_request_id'])
+        _ = self.request.translate
+
+        if pull_request.pull_request_state != PullRequest.STATE_CREATED:
+            log.debug('show: forbidden because pull request is in state %s',
+                      pull_request.pull_request_state)
+            msg = _(u'Cannot merge pull requests in state other than `{}`. '
+                    u'Current state is: `{}`').format(PullRequest.STATE_CREATED,
+                                                      pull_request.pull_request_state)
+            h.flash(msg, category='error')
+            raise HTTPFound(
+                h.route_path('pullrequest_show',
+                             repo_name=pull_request.target_repo.repo_name,
+                             pull_request_id=pull_request.pull_request_id))
 
         self.load_default_context()
-        check = MergeCheck.validate(
-            pull_request, auth_user=self._rhodecode_user,
-            translator=self.request.translate)
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            check = MergeCheck.validate(
+                pull_request, auth_user=self._rhodecode_user,
+                translator=self.request.translate)
         merge_possible = not check.failed
 
         for err_type, error_msg in check.errors:
@@ -1159,8 +1201,9 b' class RepoPullRequestsView(RepoAppView, '
                 self.request.environ, repo_name=pull_request.target_repo.repo_name,
                 username=self._rhodecode_db_user.username, action='push',
                 scm=pull_request.target_repo.repo_type)
-            self._merge_pull_request(
-                pull_request, self._rhodecode_db_user, extras)
+            with pull_request.set_state(PullRequest.STATE_UPDATING):
+                self._merge_pull_request(
+                    pull_request, self._rhodecode_db_user, extras)
         else:
             log.debug("Pre-conditions failed, NOT merging.")
 

rhodecode/model/db.py

@@ -3538,6 +3538,32 b' class ChangesetStatus(Base, BaseModel):'
         return data
 
 
+class _SetState(object):
+    """
+    Context processor allowing changing state for sensitive operation such as
+    pull request update or merge
+    """
+
+    def __init__(self, pull_request, pr_state, back_state=None):
+        self._pr = pull_request
+        self._org_state = back_state or pull_request.pull_request_state
+        self._pr_state = pr_state
+
+    def __enter__(self):
+        log.debug('StateLock: entering set state context, setting state to: `%s`',
+                  self._pr_state)
+        self._pr.pull_request_state = self._pr_state
+        Session().add(self._pr)
+        Session().commit()
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        log.debug('StateLock: exiting set state context, setting state to: `%s`',
+                  self._org_state)
+        self._pr.pull_request_state = self._org_state
+        Session().add(self._pr)
+        Session().commit()
+
+
 class _PullRequestBase(BaseModel):
     """
     Common attributes of pull request and version entries.
@@ -3548,6 +3574,12 b' class _PullRequestBase(BaseModel):'
     STATUS_OPEN = u'open'
     STATUS_CLOSED = u'closed'
 
+    # available states
+    STATE_CREATING = u'creating'
+    STATE_UPDATING = u'updating'
+    STATE_MERGING = u'merging'
+    STATE_CREATED = u'created'
+
     title = Column('title', Unicode(255), nullable=True)
     description = Column(
         'description', UnicodeText().with_variant(UnicodeText(10240), 'mysql'),
@@ -3563,6 +3595,8 b' class _PullRequestBase(BaseModel):'
         'updated_on', DateTime(timezone=False), nullable=False,
         default=datetime.datetime.now)
 
+    pull_request_state = Column("pull_request_state", String(255), nullable=True)
+
     @declared_attr
     def user_id(cls):
         return Column(
@@ -3737,6 +3771,7 b' class _PullRequestBase(BaseModel):'
             'title': pull_request.title,
             'description': pull_request.description,
             'status': pull_request.status,
+            'state': pull_request.pull_request_state,
             'created_on': pull_request.created_on,
             'updated_on': pull_request.updated_on,
             'commit_ids': pull_request.revisions,
@@ -3777,6 +3812,20 b' class _PullRequestBase(BaseModel):'
 
         return data
 
+    def set_state(self, pull_request_state, final_state=None):
+        """
+        # goes from initial state to updating to initial state.
+        # initial state can be changed by specifying back_state=
+        with pull_request_obj.set_state(PullRequest.STATE_UPDATING):
+           pull_request.merge()
+
+        :param pull_request_state:
+        :param final_state:
+
+        """
+
+        return _SetState(self, pull_request_state, back_state=final_state)
+
 
 class PullRequest(Base, _PullRequestBase):
     __tablename__ = 'pull_requests'

rhodecode/model/pull_request.py

@@ -138,7 +138,7 b' class PullRequestModel(BaseModel):'
 
     def _prepare_get_all_query(self, repo_name, source=False, statuses=None,
                                opened_by=None, order_by=None,
-                               order_dir='desc'):
+                               order_dir='desc', only_created=True):
         repo = None
         if repo_name:
             repo = self._get_repo(repo_name)
@@ -159,6 +159,10 b' class PullRequestModel(BaseModel):'
         if opened_by:
             q = q.filter(PullRequest.user_id.in_(opened_by))
 
+        # only get those that are in "created" state
+        if only_created:
+            q = q.filter(PullRequest.pull_request_state == PullRequest.STATE_CREATED)
+
         if order_by:
             order_map = {
                 'name_raw': PullRequest.pull_request_id,
@@ -429,7 +433,7 b' class PullRequestModel(BaseModel):'
         pull_request.description_renderer = description_renderer
         pull_request.author = created_by_user
         pull_request.reviewer_data = reviewer_data
-
+        pull_request.pull_request_state = pull_request.STATE_CREATING
         Session().add(pull_request)
         Session().flush()
 
@@ -497,9 +501,16 b' class PullRequestModel(BaseModel):'
         # that for large repos could be long resulting in long row locks
         Session().commit()
 
-        # prepare workspace, and run initial merge simulation
-        MergeCheck.validate(
-            pull_request, auth_user=auth_user, translator=translator)
+        # prepare workspace, and run initial merge simulation. Set state during that
+        # operation
+        pull_request = PullRequest.get(pull_request.pull_request_id)
+
+        # set as merging, for simulation, and if finished to created so we mark
+        # simulation is working fine
+        with pull_request.set_state(PullRequest.STATE_MERGING,
+                                    final_state=PullRequest.STATE_CREATED):
+            MergeCheck.validate(
+                pull_request, auth_user=auth_user, translator=translator)
 
         self.notify_reviewers(pull_request, reviewer_ids)
         self._trigger_pull_request_hook(
@@ -653,9 +664,8 b' class PullRequestModel(BaseModel):'
         target_ref_id = pull_request.target_ref_parts.commit_id
 
         if not self.has_valid_update_type(pull_request):
-            log.debug(
-                "Skipping update of pull request %s due to ref type: %s",
-                pull_request, source_ref_type)
+            log.debug("Skipping update of pull request %s due to ref type: %s",
+                      pull_request, source_ref_type)
             return UpdateResponse(
                 executed=False,
                 reason=UpdateFailureReason.WRONG_REF_TYPE,
@@ -801,8 +811,7 b' class PullRequestModel(BaseModel):'
             pull_request.source_ref_parts.commit_id,
             pull_request_version.pull_request_version_id)
         Session().commit()
-        self._trigger_pull_request_hook(
-            pull_request, pull_request.author, 'update')
+        self._trigger_pull_request_hook(pull_request, pull_request.author, 'update')
 
         return UpdateResponse(
             executed=True, reason=UpdateFailureReason.NONE,
@@ -814,6 +823,7 b' class PullRequestModel(BaseModel):'
         version.title = pull_request.title
         version.description = pull_request.description
         version.status = pull_request.status
+        version.pull_request_state = pull_request.pull_request_state
         version.created_on = datetime.datetime.now()
         version.updated_on = pull_request.updated_on
         version.user_id = pull_request.user_id
@@ -1614,8 +1624,7 b' class MergeCheck(object):'
 
             msg = _('Pull request reviewer approval is pending.')
 
-            merge_check.push_error(
-                'warning', msg, cls.REVIEW_CHECK, review_status)
+            merge_check.push_error('warning', msg, cls.REVIEW_CHECK, review_status)
 
             if fail_early:
                 return merge_check
@@ -1624,7 +1633,7 b' class MergeCheck(object):'
         todos = CommentsModel().get_unresolved_todos(pull_request)
         if todos:
             log.debug("MergeCheck: cannot merge, {} "
-                      "unresolved todos left.".format(len(todos)))
+                      "unresolved TODOs left.".format(len(todos)))
 
             if len(todos) == 1:
                 msg = _('Cannot merge, {} TODO still not resolved.').format(
@@ -1645,8 +1654,7 b' class MergeCheck(object):'
         merge_check.merge_possible = merge_status
         merge_check.merge_msg = msg
         if not merge_status:
-            log.debug(
-                "MergeCheck: cannot merge, pull request merge not possible.")
+            log.debug("MergeCheck: cannot merge, pull request merge not possible.")
             merge_check.push_error('warning', msg, cls.MERGE_CHECK, None)
 
             if fail_early:
@@ -1677,6 +1685,7 b' class MergeCheck(object):'
         close_branch = model._close_branch_before_merging(pull_request)
         if close_branch:
             repo_type = pull_request.target_repo.repo_type
+            close_msg = ''
             if repo_type == 'hg':
                 close_msg = _('Source branch will be closed after merge.')
             elif repo_type == 'git':
@@ -1689,6 +1698,7 b' class MergeCheck(object):'
 
         return merge_details
 
+
 ChangeTuple = collections.namedtuple(
     'ChangeTuple', ['added', 'common', 'removed', 'total'])
 

rhodecode/tests/models/test_pullrequest.py

@@ -257,8 +257,7 b' class TestPullRequestModel(object):'
         def has_largefiles(self, repo):
             return repo == pull_request.source_repo
 
-        patcher = mock.patch.object(
-            PullRequestModel, '_has_largefiles', has_largefiles)
+        patcher = mock.patch.object(PullRequestModel, '_has_largefiles', has_largefiles)
         with patcher:
             status, msg = PullRequestModel().merge_status(pull_request)
 
@@ -270,8 +269,7 b' class TestPullRequestModel(object):'
         def has_largefiles(self, repo):
             return repo == pull_request.target_repo
 
-        patcher = mock.patch.object(
-            PullRequestModel, '_has_largefiles', has_largefiles)
+        patcher = mock.patch.object(PullRequestModel, '_has_largefiles', has_largefiles)
         with patcher:
             status, msg = PullRequestModel().merge_status(pull_request)
 
@@ -314,9 +312,50 b' class TestPullRequestModel(object):'
             self.pull_request, self.pull_request.author, 'merge')
 
         pull_request = PullRequest.get(pull_request.pull_request_id)
-        assert (
-            pull_request.merge_rev ==
-            '6126b7bfcc82ad2d3deaee22af926b082ce54cc6')
+        assert pull_request.merge_rev == '6126b7bfcc82ad2d3deaee22af926b082ce54cc6'
+
+    def test_merge_with_status_lock(self, pull_request, merge_extras):
+        user = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)
+        merge_ref = Reference(
+            'type', 'name', '6126b7bfcc82ad2d3deaee22af926b082ce54cc6')
+        self.merge_mock.return_value = MergeResponse(
+            True, True, merge_ref, MergeFailureReason.NONE)
+
+        merge_extras['repository'] = pull_request.target_repo.repo_name
+
+        with pull_request.set_state(PullRequest.STATE_UPDATING):
+            assert pull_request.pull_request_state == PullRequest.STATE_UPDATING
+            PullRequestModel().merge_repo(
+                pull_request, pull_request.author, extras=merge_extras)
+
+        assert pull_request.pull_request_state == PullRequest.STATE_CREATED
+
+        message = (
+            u'Merge pull request #{pr_id} from {source_repo} {source_ref_name}'
+            u'\n\n {pr_title}'.format(
+                pr_id=pull_request.pull_request_id,
+                source_repo=safe_unicode(
+                    pull_request.source_repo.scm_instance().name),
+                source_ref_name=pull_request.source_ref_parts.name,
+                pr_title=safe_unicode(pull_request.title)
+            )
+        )
+        self.merge_mock.assert_called_with(
+            self.repo_id, self.workspace_id,
+            pull_request.target_ref_parts,
+            pull_request.source_repo.scm_instance(),
+            pull_request.source_ref_parts,
+            user_name=user.short_contact, user_email=user.email, message=message,
+            use_rebase=False, close_branch=False
+        )
+        self.invalidation_mock.assert_called_once_with(
+            pull_request.target_repo.repo_name)
+
+        self.hook_mock.assert_called_with(
+            self.pull_request, self.pull_request.author, 'merge')
+
+        pull_request = PullRequest.get(pull_request.pull_request_id)
+        assert pull_request.merge_rev == '6126b7bfcc82ad2d3deaee22af926b082ce54cc6'
 
     def test_merge_failed(self, pull_request, merge_extras):
         user = UserModel().get_by_username(TEST_USER_ADMIN_LOGIN)