From 514ee3526c3d51bb9e3dfdf802d9ec35f626893a Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 24 Jul 2012 11:59:28 -0700 Subject: [PATCH] Add an event for looking up names from repositories Summary: Currently, we have this cumbersome `PhabricatorRepositoryCommitMessageDetailParser` hook. This is really old and outdated; I want to just use the Differential custom field parser. See T945 for a specific application. However, it allows installs to override author/committer association. Instead, provide an event hook for doing this. Test Plan: Added a listener, made every commit resolve to "turtle", parsed some commits, verified the events looked sane and they now correctly were all attributed to "turtle". Reviewers: btrahan, vrana, nh Reviewed By: btrahan CC: aran Maniphest Tasks: T1337 Differential Revision: https://secure.phabricator.com/D3040 --- ...torRepositoryCommitMessageDetailParser.php | 4 ++ ...torRepositoryCommitMessageParserWorker.php | 38 ++++++++++++++++++- src/docs/userguide/events.diviner | 29 ++++++++++++++ .../events/constant/PhabricatorEventType.php | 1 + 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/applications/repository/parser/PhabricatorRepositoryCommitMessageDetailParser.php b/src/applications/repository/parser/PhabricatorRepositoryCommitMessageDetailParser.php index 444efc55f8..34d4b230d8 100644 --- a/src/applications/repository/parser/PhabricatorRepositoryCommitMessageDetailParser.php +++ b/src/applications/repository/parser/PhabricatorRepositoryCommitMessageDetailParser.php @@ -69,6 +69,10 @@ abstract class PhabricatorRepositoryCommitMessageDetailParser { $display_name = $email->getDisplayName(); if ($display_name) { + $phid = $this->findUserByUserName($display_name); + if ($phid) { + return $phid; + } $phid = $this->findUserByRealName($display_name); if ($phid) { return $phid; diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 770330ec66..da89a7a698 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -52,8 +52,19 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $parser_obj->parseCommitDetails(); } - $author_phid = $data->getCommitDetail('authorPHID'); - if ($author_phid) { + $author_phid = $this->lookupUser( + $commit, + $data->getAuthorName(), + $data->getCommitDetail('authorPHID')); + $data->setCommitDetail('authorPHID', $author_phid); + + $committer_phid = $this->lookupUser( + $commit, + $data->getCommitDetail('committer'), + $data->getCommitDetail('committerPHID')); + $data->setCommitDetail('committerPHID', $committer_phid); + + if ($author_phid != $commit->getAuthorPHID()) { $commit->setAuthorPHID($author_phid); $commit->save(); } @@ -348,4 +359,27 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $revisions = msort($revisions, 'getDateModified'); return end($revisions); } + + /** + * Emit an event so installs can do custom lookup of commit authors who may + * not be naturally resolvable. + */ + private function lookupUser( + PhabricatorRepositoryCommit $commit, + $query, + $guess) { + + $type = PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER; + $data = array( + 'commit' => $commit, + 'query' => $query, + 'result' => $guess, + ); + + $event = new PhabricatorEvent($type, $data); + PhutilEventEngine::dispatchEvent($event); + + return $event->getValue('result'); + } + } diff --git a/src/docs/userguide/events.diviner b/src/docs/userguide/events.diviner index 3ca182fe96..2dfe023dac 100644 --- a/src/docs/userguide/events.diviner +++ b/src/docs/userguide/events.diviner @@ -138,6 +138,35 @@ will be available yet. Data available on this event: - `repository` The @{class:PhabricatorRepository} the commit was discovered in. +== Diffusion: Lookup User == + +The constant for this event is +`PhabricatorEventType::TYPE_DIFFUSION_LOOKUPUSER`. + +This event is dispatched when the daemons are trying to link a commit to a +Phabricator user account. You can listen for it to improve the accuracy of +associating users with their commits. + +By default, Phabricator will try to find matches based on usernames, real names, +or email addresses, but this can result in incorrect matches (e.g., if you have +several employees with the same name) or failures to match (e.g., if someone +changed their email address). Listening for this event allows you to intercept +the lookup and supplement the results from another datasource. + +Data available on this event: + + - `commit` The @{class:PhabricatorRepositoryCommit} that data is being looked + up for. + - `query` The author or committer string being looked up. This will usually + be something like "Abraham Lincoln ", but + comes from the commit metadata so it may not be well-formatted. + - `result` The current result from the lookup (Phabricator's best guess at + the user PHID of the user named in the "query"). To substitute the result + with a different result, replace this with the correct PHID in your event + listener. + +Using @{class:PhutilEmailAddress} may be helpful in parsing the query. + == Edge: Will Edit Edges == NOTE: Edge events are low-level events deep in the core. It is more difficult to diff --git a/src/infrastructure/events/constant/PhabricatorEventType.php b/src/infrastructure/events/constant/PhabricatorEventType.php index 0aa5132f26..8eeb1b7f76 100644 --- a/src/infrastructure/events/constant/PhabricatorEventType.php +++ b/src/infrastructure/events/constant/PhabricatorEventType.php @@ -31,6 +31,7 @@ final class PhabricatorEventType extends PhutilEventType { const TYPE_DIFFERENTIAL_WILLMARKGENERATED = 'differential.willMarkGenerated'; const TYPE_DIFFUSION_DIDDISCOVERCOMMIT = 'diffusion.didDiscoverCommit'; + const TYPE_DIFFUSION_LOOKUPUSER = 'diffusion.lookupUser'; const TYPE_EDGE_WILLEDITEDGES = 'edge.willEditEdges'; const TYPE_EDGE_DIDEDITEDGES = 'edge.didEditEdges';