recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Respect repository identities when figuring out authors/committers in Herald pre-commit hook rules

Summary:
Ref T13480. Currently, Herald commit hook rules use a raw address resolution query to identify the author and committer for a commit. This will get the wrong answer when the raw identity string has been explicitly bound to some non-default user (most often, it will fail to identify an author when one exists).

Instead, use the "IdentityEngine" to properly resolve identities.

Test Plan: Authored a commit as `X <y@example.com>`, a raw identity with no "natural" matches to users (e.g., no user with that email or username). Bound the identity to a particular user in Diffusion. Wrote a Herald pre-commit content rule, pushed the commit. Saw Herald recognize the correct user when evaluating rules.

Maniphest Tasks: T13480

Differential Revision: https://secure.phabricator.com/D20953

epriestley 41f143f7 a0a346be

+34 -4
+34 -4
src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
··· 8 8 private $revision = false; 9 9 10 10 private $affectedPackages; 11 + private $identityCache = array(); 11 12 12 13 public function getAdapterContentName() { 13 14 return pht('Commit Hook: Commit Content'); ··· 166 167 } 167 168 } 168 169 169 - private function lookupUser($author) { 170 - return id(new DiffusionResolveUserQuery()) 171 - ->withName($author) 172 - ->execute(); 170 + private function lookupUser($raw_identity) { 171 + // See T13480. After the move to repository identities, we want to look 172 + // users up in the identity table. If you push a commit which is authored 173 + // by "A Duck <duck@example.org>" and that identity is bound to user 174 + // "@mallard" in the identity table, Herald should see the author of the 175 + // commit as "@mallard" when evaluating pre-commit content rules. 176 + 177 + if (!array_key_exists($raw_identity, $this->identityCache)) { 178 + $repository = $this->getHookEngine()->getRepository(); 179 + $viewer = $this->getHookEngine()->getViewer(); 180 + 181 + $identity_engine = id(new DiffusionRepositoryIdentityEngine()) 182 + ->setViewer($viewer); 183 + 184 + // We must provide a "sourcePHID" when resolving identities, but don't 185 + // have a legitimate one yet. Just use the repository PHID as a 186 + // reasonable value. This won't actually be written to storage. 187 + $source_phid = $repository->getPHID(); 188 + $identity_engine->setSourcePHID($source_phid); 189 + 190 + // If the identity doesn't exist yet, we don't want to create it if 191 + // we haven't seen it before. It will be created later when we actually 192 + // import the commit. 193 + $identity_engine->setDryRun(true); 194 + 195 + $author_identity = $identity_engine->newResolvedIdentity($raw_identity); 196 + 197 + $effective_phid = $author_identity->getCurrentEffectiveUserPHID(); 198 + 199 + $this->identityCache[$raw_identity] = $effective_phid; 200 + } 201 + 202 + return $this->identityCache[$raw_identity]; 173 203 } 174 204 175 205 private function getCommitFields() {