diff --git a/src/applications/diffusion/data/DiffusionCommitRef.php b/src/applications/diffusion/data/DiffusionCommitRef.php index 5224fc872f..06dd4d5ffb 100644 --- a/src/applications/diffusion/data/DiffusionCommitRef.php +++ b/src/applications/diffusion/data/DiffusionCommitRef.php @@ -50,10 +50,6 @@ final class DiffusionCommitRef extends Phobject { ->setHashes($hashes); } - public static function newFromConduitResult(array $result) { - return self::newFromDictionary($result); - } - public function setHashes(array $hashes) { assert_instances_of($hashes, 'DiffusionCommitHash'); $this->hashes = $hashes; diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index ef65219b4b..586f1e2d0a 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -174,6 +174,11 @@ final class PhabricatorRepositoryCommit return $this; } + public function hasCommitData() { + return ($this->commitData !== self::ATTACHABLE) && + ($this->commitData !== null); + } + public function getCommitData() { return $this->assertAttached($this->commitData); } diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php index 84f9522785..c77da64ec2 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php @@ -6,6 +6,7 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO { protected $authorName = ''; protected $commitMessage = ''; protected $commitDetails = array(); + private $commitRef; protected function getConfiguration() { return array( @@ -93,8 +94,14 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO { } public function getAuthorString() { - $author = phutil_string_cast($this->authorName); + $ref = $this->getCommitRef(); + $author = $ref->getAuthor(); + if (strlen($author)) { + return $author; + } + + $author = phutil_string_cast($this->authorName); if (strlen($author)) { return $author; } @@ -103,15 +110,15 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO { } public function getAuthorDisplayName() { - return $this->getCommitDetailString('authorName'); + return $this->getCommitRef()->getAuthorName(); } public function getAuthorEmail() { - return $this->getCommitDetailString('authorEmail'); + return $this->getCommitRef()->getAuthorEmail(); } public function getAuthorEpoch() { - $epoch = $this->getCommitDetail('authorEpoch'); + $epoch = $this->getCommitRef()->getAuthorEpoch(); if ($epoch) { return (int)$epoch; @@ -121,15 +128,22 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO { } public function getCommitterString() { + $ref = $this->getCommitRef(); + + $committer = $ref->getCommitter(); + if (strlen($committer)) { + return $committer; + } + return $this->getCommitDetailString('committer'); } public function getCommitterDisplayName() { - return $this->getCommitDetailString('committerName'); + return $this->getCommitRef()->getCommitterName(); } public function getCommitterEmail() { - return $this->getCommitDetailString('committerEmail'); + return $this->getCommitRef()->getCommitterEmail(); } private function getCommitDetailString($key) { @@ -143,4 +157,35 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO { return null; } + public function setCommitRef(DiffusionCommitRef $ref) { + $this->setCommitDetail('ref', $ref->newDictionary()); + $this->commitRef = null; + + return $this; + } + + public function getCommitRef() { + if ($this->commitRef === null) { + $map = $this->getCommitDetail('ref', array()); + + if (!is_array($map)) { + $map = array(); + } + + $map = $map + array( + 'authorName' => $this->getCommitDetailString('authorName'), + 'authorEmail' => $this->getCommitDetailString('authorEmail'), + 'authorEpoch' => $this->getCommitDetailString('authorEpoch'), + 'committerName' => $this->getCommitDetailString('committerName'), + 'committerEmail' => $this->getCommitDetailString('committerEmail'), + 'message' => $this->getCommitMessage(), + ); + + $ref = DiffusionCommitRef::newFromDictionary($map); + $this->commitRef = $ref; + } + + return $this->commitRef; + } + } diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php index 656b258b40..e1990eaec3 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php @@ -125,6 +125,26 @@ abstract class PhabricatorRepositoryCommitParserWorker return array($link, $suffix); } + final protected function loadCommitData(PhabricatorRepositoryCommit $commit) { + if ($commit->hasCommitData()) { + return $commit->getCommitData(); + } + + $commit_id = $commit->getID(); + + $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( + 'commitID = %d', + $commit_id); + if (!$data) { + $data = id(new PhabricatorRepositoryCommitData()) + ->setCommitID($commit_id); + } + + $commit->attachCommitData($data); + + return $data; + } + final public function getViewer() { return PhabricatorUser::getOmnipotentUser(); } diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 3eead0a26d..1ae1084743 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -18,7 +18,10 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $ref = $commit->newCommitRef($viewer); - $this->updateCommitData($ref); + $data = $this->loadCommitData($commit); + $data->setCommitRef($ref); + + $this->updateCommitData($commit, $data); } if ($this->shouldQueueFollowupTasks()) { @@ -38,14 +41,17 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker } } - final protected function updateCommitData(DiffusionCommitRef $ref) { - $commit = $this->commit; + final protected function updateCommitData( + PhabricatorRepositoryCommit $commit, + PhabricatorRepositoryCommitData $data) { + + $ref = $data->getCommitRef(); + $viewer = $this->getViewer(); + $author = $ref->getAuthor(); $committer = $ref->getCommitter(); $has_committer = (bool)strlen($committer); - $viewer = PhabricatorUser::getOmnipotentUser(); - $identity_engine = id(new DiffusionRepositoryIdentityEngine()) ->setViewer($viewer) ->setSourcePHID($commit->getPHID()); @@ -66,13 +72,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $committer_identity = null; } - $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( - 'commitID = %d', - $commit->getID()); - if (!$data) { - $data = new PhabricatorRepositoryCommitData(); - } - $data->setCommitID($commit->getID()); $data->setAuthorName(id(new PhutilUTF8StringTruncator()) ->setMaximumBytes(255) ->truncateString((string)$author)); @@ -122,7 +121,9 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $commit->setAuthorIdentityPHID($author_identity->getPHID()); $commit->setSummary($data->getSummary()); + $commit->save(); + $data->save(); // If we're publishing this commit, we're going to queue tasks to update // referenced objects (like tasks and revisions). Otherwise, record some @@ -130,15 +131,14 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $publisher = $repository->newPublisher(); if ($publisher->shouldPublishCommit($commit)) { - $actor = PhabricatorUser::getOmnipotentUser(); - $this->closeRevisions($actor, $ref, $commit, $data); - $this->closeTasks($actor, $ref, $commit, $data); + $this->closeRevisions($viewer, $commit); + $this->closeTasks($viewer, $commit); } else { $hold_reasons = $publisher->getCommitHoldReasons($commit); - $data->setCommitDetail('holdReasons', $hold_reasons); - } - $data->save(); + $data->setCommitDetail('holdReasons', $hold_reasons); + $data->save(); + } $commit->writeImportStatusFlag( PhabricatorRepositoryCommit::IMPORTED_MESSAGE); @@ -146,9 +146,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker private function closeRevisions( PhabricatorUser $actor, - DiffusionCommitRef $ref, - PhabricatorRepositoryCommit $commit, - PhabricatorRepositoryCommitData $data) { + PhabricatorRepositoryCommit $commit) { $differential = 'PhabricatorDifferentialApplication'; if (!PhabricatorApplication::isClassInstalled($differential)) { @@ -156,6 +154,8 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker } $repository = $commit->getRepository(); + $data = $commit->getCommitData(); + $ref = $data->getCommitRef(); $field_query = id(new DiffusionLowLevelCommitFieldsQuery()) ->setRepository($repository) @@ -198,15 +198,15 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker private function closeTasks( PhabricatorUser $actor, - DiffusionCommitRef $ref, - PhabricatorRepositoryCommit $commit, - PhabricatorRepositoryCommitData $data) { + PhabricatorRepositoryCommit $commit) { $maniphest = 'PhabricatorManiphestApplication'; if (!PhabricatorApplication::isClassInstalled($maniphest)) { return; } + $data = $commit->getCommitData(); + $prefixes = ManiphestTaskStatus::getStatusPrefixMap(); $suffixes = ManiphestTaskStatus::getStatusSuffixMap(); $message = $data->getCommitMessage();