Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15431144
D20914.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
11 KB
Referenced Files
None
Subscribers
None
D20914.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -984,6 +984,7 @@
'DiffusionRepositoryEditUpdateController' => 'applications/diffusion/controller/DiffusionRepositoryEditUpdateController.php',
'DiffusionRepositoryFunctionDatasource' => 'applications/diffusion/typeahead/DiffusionRepositoryFunctionDatasource.php',
'DiffusionRepositoryHistoryManagementPanel' => 'applications/diffusion/management/DiffusionRepositoryHistoryManagementPanel.php',
+ 'DiffusionRepositoryIdentityDestructionEngineExtension' => 'applications/diffusion/identity/DiffusionRepositoryIdentityDestructionEngineExtension.php',
'DiffusionRepositoryIdentityEditor' => 'applications/diffusion/editor/DiffusionRepositoryIdentityEditor.php',
'DiffusionRepositoryIdentityEngine' => 'applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php',
'DiffusionRepositoryIdentitySearchEngine' => 'applications/diffusion/query/DiffusionRepositoryIdentitySearchEngine.php',
@@ -6968,6 +6969,7 @@
'DiffusionRepositoryEditUpdateController' => 'DiffusionRepositoryManageController',
'DiffusionRepositoryFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
'DiffusionRepositoryHistoryManagementPanel' => 'DiffusionRepositoryManagementPanel',
+ 'DiffusionRepositoryIdentityDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
'DiffusionRepositoryIdentityEditor' => 'PhabricatorApplicationTransactionEditor',
'DiffusionRepositoryIdentityEngine' => 'Phobject',
'DiffusionRepositoryIdentitySearchEngine' => 'PhabricatorApplicationSearchEngine',
diff --git a/src/applications/diffusion/identity/DiffusionRepositoryIdentityDestructionEngineExtension.php b/src/applications/diffusion/identity/DiffusionRepositoryIdentityDestructionEngineExtension.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/identity/DiffusionRepositoryIdentityDestructionEngineExtension.php
@@ -0,0 +1,40 @@
+<?php
+
+final class DiffusionRepositoryIdentityDestructionEngineExtension
+ extends PhabricatorDestructionEngineExtension {
+
+ const EXTENSIONKEY = 'repository-identities';
+
+ public function getExtensionName() {
+ return pht('Repository Identities');
+ }
+
+ public function didDestroyObject(
+ PhabricatorDestructionEngine $engine,
+ $object) {
+
+ // When users or email addresses are destroyed, queue a task to update
+ // any repository identities that are associated with them. See T13444.
+
+ $related_phids = array();
+ $email_addresses = array();
+
+ if ($object instanceof PhabricatorUser) {
+ $related_phids[] = $object->getPHID();
+ }
+
+ if ($object instanceof PhabricatorUserEmail) {
+ $email_addresses[] = $object->getAddress();
+ }
+
+ if ($related_phids || $email_addresses) {
+ PhabricatorWorker::scheduleTask(
+ 'PhabricatorRepositoryIdentityChangeWorker',
+ array(
+ 'relatedPHIDs' => $related_phids,
+ 'emailAddresses' => $email_addresses,
+ ));
+ }
+ }
+
+}
diff --git a/src/applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php b/src/applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php
--- a/src/applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php
+++ b/src/applications/diffusion/identity/DiffusionRepositoryIdentityEngine.php
@@ -95,4 +95,12 @@
return $identity;
}
+ public function didUpdateEmailAddress($raw_address) {
+ PhabricatorWorker::scheduleTask(
+ 'PhabricatorRepositoryIdentityChangeWorker',
+ array(
+ 'emailAddresses' => array($raw_address),
+ ));
+ }
+
}
diff --git a/src/applications/people/editor/PhabricatorUserEditor.php b/src/applications/people/editor/PhabricatorUserEditor.php
--- a/src/applications/people/editor/PhabricatorUserEditor.php
+++ b/src/applications/people/editor/PhabricatorUserEditor.php
@@ -89,6 +89,9 @@
$this->didVerifyEmail($user, $email);
}
+ id(new DiffusionRepositoryIdentityEngine())
+ ->didUpdateEmailAddress($email->getAddress());
+
return $this;
}
@@ -202,11 +205,8 @@
$user->endWriteLocking();
$user->saveTransaction();
- // Try and match this new address against unclaimed `RepositoryIdentity`s
- PhabricatorWorker::scheduleTask(
- 'PhabricatorRepositoryIdentityChangeWorker',
- array('userPHID' => $user->getPHID()),
- array('objectPHID' => $user->getPHID()));
+ id(new DiffusionRepositoryIdentityEngine())
+ ->didUpdateEmailAddress($email->getAddress());
return $this;
}
@@ -241,7 +241,8 @@
throw new Exception(pht('Email not owned by user!'));
}
- id(new PhabricatorDestructionEngine())
+ $destruction_engine = id(new PhabricatorDestructionEngine())
+ ->setWaitToFinalizeDestruction(true)
->destroyObject($email);
$log = PhabricatorUserLog::initializeNewLog(
@@ -255,6 +256,7 @@
$user->saveTransaction();
$this->revokePasswordResetLinks($user);
+ $destruction_engine->finalizeDestruction();
return $this;
}
@@ -327,7 +329,6 @@
}
$email->sendNewPrimaryEmail($user);
-
$this->revokePasswordResetLinks($user);
return $this;
@@ -441,6 +442,9 @@
$user->endWriteLocking();
$user->saveTransaction();
+
+ id(new DiffusionRepositoryIdentityEngine())
+ ->didUpdateEmailAddress($email->getAddress());
}
diff --git a/src/applications/repository/worker/PhabricatorRepositoryIdentityChangeWorker.php b/src/applications/repository/worker/PhabricatorRepositoryIdentityChangeWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryIdentityChangeWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryIdentityChangeWorker.php
@@ -6,30 +6,54 @@
protected function doWork() {
$viewer = PhabricatorUser::getOmnipotentUser();
- $task_data = $this->getTaskData();
- $user_phid = idx($task_data, 'userPHID');
+ $related_phids = $this->getTaskDataValue('relatedPHIDs');
+ $email_addresses = $this->getTaskDataValue('emailAddresses');
+
+ // Retain backward compatibility with older tasks which may still be in
+ // queue. Previously, this worker accepted a single "userPHID". See
+ // T13444. This can be removed in some future version of Phabricator once
+ // these tasks have likely flushed out of queue.
+ $legacy_phid = $this->getTaskDataValue('userPHID');
+ if ($legacy_phid) {
+ if (!is_array($related_phids)) {
+ $related_phids = array();
+ }
+ $related_phids[] = $legacy_phid;
+ }
- $user = id(new PhabricatorPeopleQuery())
- ->withPHIDs(array($user_phid))
- ->setViewer($viewer)
- ->executeOne();
+ // Note that we may arrive in this worker after the associated objects
+ // have already been destroyed, so we can't (and shouldn't) verify that
+ // PHIDs correspond to real objects. If you "bin/remove destroy" a user,
+ // we'll end up here with a now-bogus user PHID that we should
+ // disassociate from identities.
- $emails = id(new PhabricatorUserEmail())->loadAllWhere(
- 'userPHID = %s',
- $user->getPHID());
+ $identity_map = array();
- $identity_engine = id(new DiffusionRepositoryIdentityEngine())
- ->setViewer($viewer);
+ if ($related_phids) {
+ $identities = id(new PhabricatorRepositoryIdentityQuery())
+ ->setViewer($viewer)
+ ->withRelatedPHIDs($related_phids)
+ ->execute();
+ $identity_map += mpull($identities, null, 'getPHID');
+ }
- foreach ($emails as $email) {
+ if ($email_addresses) {
$identities = id(new PhabricatorRepositoryIdentityQuery())
->setViewer($viewer)
- ->withEmailAddresses(array($email->getAddress()))
+ ->withEmailAddresses($email_addresses)
->execute();
+ $identity_map += mpull($identities, null, 'getPHID');
+ }
- foreach ($identities as $identity) {
- $identity_engine->newUpdatedIdentity($identity);
- }
+ // If we didn't find any related identities, we're all set.
+ if (!$identity_map) {
+ return;
+ }
+
+ $identity_engine = id(new DiffusionRepositoryIdentityEngine())
+ ->setViewer($viewer);
+ foreach ($identity_map as $identity) {
+ $identity_engine->newUpdatedIdentity($identity);
}
}
diff --git a/src/applications/system/engine/PhabricatorDestructionEngine.php b/src/applications/system/engine/PhabricatorDestructionEngine.php
--- a/src/applications/system/engine/PhabricatorDestructionEngine.php
+++ b/src/applications/system/engine/PhabricatorDestructionEngine.php
@@ -5,6 +5,9 @@
private $rootLogID;
private $collectNotes;
private $notes = array();
+ private $depth = 0;
+ private $destroyedObjects = array();
+ private $waitToFinalizeDestruction = false;
public function setCollectNotes($collect_notes) {
$this->collectNotes = $collect_notes;
@@ -19,9 +22,20 @@
return PhabricatorUser::getOmnipotentUser();
}
+ public function setWaitToFinalizeDestruction($wait) {
+ $this->waitToFinalizeDestruction = $wait;
+ return $this;
+ }
+
+ public function getWaitToFinalizeDestruction() {
+ return $this->waitToFinalizeDestruction;
+ }
+
public function destroyObject(PhabricatorDestructibleInterface $object) {
+ $this->depth++;
+
$log = id(new PhabricatorSystemDestructionLog())
- ->setEpoch(time())
+ ->setEpoch(PhabricatorTime::getNow())
->setObjectClass(get_class($object));
if ($this->rootLogID) {
@@ -73,7 +87,42 @@
foreach ($extensions as $key => $extension) {
$extension->destroyObject($this, $object);
}
+
+ $this->destroyedObjects[] = $object;
+ }
+
+ $this->depth--;
+
+ // If this is a root-level invocation of "destroyObject()", flush the
+ // queue of destroyed objects and fire "didDestroyObject()" hooks. This
+ // hook allows extensions to do things like queue cache updates which
+ // might race if we fire them during object destruction.
+
+ if (!$this->depth) {
+ if (!$this->getWaitToFinalizeDestruction()) {
+ $this->finalizeDestruction();
+ }
+ }
+
+ return $this;
+ }
+
+ public function finalizeDestruction() {
+ $extensions = PhabricatorDestructionEngineExtension::getAllExtensions();
+
+ foreach ($this->destroyedObjects as $object) {
+ foreach ($extensions as $extension) {
+ if (!$extension->canDestroyObject($this, $object)) {
+ continue;
+ }
+
+ $extension->didDestroyObject($this, $object);
+ }
}
+
+ $this->destroyedObjects = array();
+
+ return $this;
}
private function getObjectPHID($object) {
diff --git a/src/applications/system/engine/PhabricatorDestructionEngineExtension.php b/src/applications/system/engine/PhabricatorDestructionEngineExtension.php
--- a/src/applications/system/engine/PhabricatorDestructionEngineExtension.php
+++ b/src/applications/system/engine/PhabricatorDestructionEngineExtension.php
@@ -14,9 +14,17 @@
return true;
}
- abstract public function destroyObject(
+ public function destroyObject(
PhabricatorDestructionEngine $engine,
- $object);
+ $object) {
+ return null;
+ }
+
+ public function didDestroyObject(
+ PhabricatorDestructionEngine $engine,
+ $object) {
+ return null;
+ }
final public static function getAllExtensions() {
return id(new PhutilClassMapQuery())
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Tue, Mar 25, 10:47 AM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7716283
Default Alt Text
D20914.diff (11 KB)
Attached To
Mode
D20914: Update repository identities after all mutations to users and email addresses
Attached
Detach File
Event Timeline
Log In to Comment