From 7ed8b6d7fa191cbc0b1b145eeec9fd7269289000 Mon Sep 17 00:00:00 2001 From: vrana Date: Wed, 26 Sep 2012 11:57:03 -0700 Subject: [PATCH] Recheck if revision hasn't been already closed in commit parser Summary: If attaching a commit or checking if there are any changes takes nonzero time then the revision may be closed by someone else. Cleaner solution would be to do it inside a transaction and mark the SELECT as FOR UPDATE but it would be blocking. Test Plan: Patched `$should_close` to be true, reparsed an already closed commit. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3555 --- .../PhabricatorRepositoryCommitMessageParserWorker.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 53395875ba..eeb7bf6252 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -169,7 +169,11 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $editor->setChangedByCommit($changed_by_commit); } - $editor->setMessage($message)->save(); + // Reload revision to check if someone already didn't close it. + $current = id(new DifferentialRevision())->load($revision_id); + if ($current->getStatus() != $status_closed) { + $editor->setMessage($message)->save(); + } } }