From 70bf63bc3adc0705ee3667d6bbf315643dea3f33 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 4 Dec 2018 05:50:54 -0800
Subject: [PATCH 1/8] Fix a transaction editor "continue;" inside "switch()"
for PHP 7.3
Summary: See <https://discourse.phabricator-community.org/t/new-git-commit-processing-fails-on-php-7-3/>. This "continue" should be a "break".
Test Plan:
{F6045490}
- Tried to assign a task to myself while I was already the owner, got an appropriate error.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D19844
---
.../maniphest/editor/ManiphestTransactionEditor.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
index 4b6c7707f6..47a6b1b4f2 100644
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -649,7 +649,7 @@ final class ManiphestTransactionEditor
$old_value = $object->getOwnerPHID();
$new_value = $xaction->getNewValue();
if ($old_value === $new_value) {
- continue;
+ break;
}
// When a task is reassigned, move the old owner to the subscriber
From 5d54f26daca83875d31efda0ab9c52db7cd78652 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Dec 2018 11:34:52 -0800
Subject: [PATCH 2/8] Support reading and querying Almanac service PHIDs via
"diffusion.repository.search"
Summary:
Ref T13222. See PHI992. If you lose all hosts in a service cluster, you may need to get a list of affected repositories to figure out which backups to pull.
Support doing this via the API.
Test Plan: Queried by service PHID and saw service PHIDs in the call results.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19848
---
.../query/PhabricatorRepositorySearchEngine.php | 13 +++++++++++++
.../repository/storage/PhabricatorRepository.php | 8 ++++++++
2 files changed, 21 insertions(+)
diff --git a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php
index 9b5b721edb..90a10e9872 100644
--- a/src/applications/repository/query/PhabricatorRepositorySearchEngine.php
+++ b/src/applications/repository/query/PhabricatorRepositorySearchEngine.php
@@ -44,6 +44,15 @@ final class PhabricatorRepositorySearchEngine
->setKey('uris')
->setDescription(
pht('Search for repositories by clone/checkout URI.')),
+ id(new PhabricatorPHIDsSearchField())
+ ->setLabel(pht('Services'))
+ ->setKey('almanacServicePHIDs')
+ ->setAliases(
+ array(
+ 'almanacServicePHID',
+ 'almanacService',
+ 'almanacServices',
+ )),
);
}
@@ -80,6 +89,10 @@ final class PhabricatorRepositorySearchEngine
$query->withURIs($map['uris']);
}
+ if ($map['almanacServicePHIDs']) {
+ $query->withAlmanacServicePHIDs($map['almanacServicePHIDs']);
+ }
+
return $query;
}
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
index 3b7918cd59..c961e06f27 100644
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -2773,6 +2773,13 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
->setDescription(
pht(
'True if the repository is importing initial commits.')),
+ id(new PhabricatorConduitSearchFieldSpecification())
+ ->setKey('almanacServicePHID')
+ ->setType('phid?')
+ ->setDescription(
+ pht(
+ 'The Almanac Service that hosts this repository, if the '.
+ 'repository is clustered.')),
);
}
@@ -2784,6 +2791,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
'shortName' => $this->getRepositorySlug(),
'status' => $this->getStatus(),
'isImporting' => (bool)$this->isImporting(),
+ 'almanacServicePHID' => $this->getAlmanacServicePHID(),
);
}
From f0eefdd0b58b8f2a859637d3f1faeae50c60af09 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 6 Dec 2018 08:10:21 -0800
Subject: [PATCH 3/8] Replace the informal "array" subtype map with a more
formal "SubtypeMap" object
Summary: Ref T13222. Ref T12588. See PHI683. To make "Create Subtask..." fancier, we need slightly more logic around subtype maps. Upgrade the plain old array into a proper object so it can have relevant methods, notably "get a list of valid child subtypes for some parent subtype".
Test Plan: Created and edited tasks, changed task subtypes. Grepped for affected symbols (`newEditEngineSubtypeMap`, `newSubtypeMap`).
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222, T12588
Differential Revision: https://secure.phabricator.com/D19852
---
src/__phutil_library_map__.php | 2 +
.../query/ManiphestTaskSearchEngine.php | 2 +-
.../maniphest/storage/ManiphestTask.php | 2 +-
.../storage/ManiphestTransaction.php | 5 ++-
.../ManiphestTaskSubtypeDatasource.php | 2 +-
.../maniphest/view/ManiphestTaskListView.php | 3 --
...itEngineConfigurationSubtypeController.php | 3 +-
.../PhabricatorEditEngineSubtype.php | 2 +-
.../PhabricatorEditEngineSubtypeMap.php | 42 +++++++++++++++++++
...habricatorApplicationTransactionEditor.php | 2 +-
...abricatorEditEngineConfigurationEditor.php | 2 +-
.../PhabricatorSubtypeEditEngineExtension.php | 4 +-
12 files changed, 56 insertions(+), 15 deletions(-)
create mode 100644 src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index e4ffb74b0f..aeba5e18ae 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2976,6 +2976,7 @@ phutil_register_library_map(array(
'PhabricatorEditEngineStaticCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineStaticCommentAction.php',
'PhabricatorEditEngineSubtype' => 'applications/transactions/editengine/PhabricatorEditEngineSubtype.php',
'PhabricatorEditEngineSubtypeInterface' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeInterface.php',
+ 'PhabricatorEditEngineSubtypeMap' => 'applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php',
'PhabricatorEditEngineSubtypeTestCase' => 'applications/transactions/editengine/__tests__/PhabricatorEditEngineSubtypeTestCase.php',
'PhabricatorEditEngineTokenizerCommentAction' => 'applications/transactions/commentaction/PhabricatorEditEngineTokenizerCommentAction.php',
'PhabricatorEditField' => 'applications/transactions/editfield/PhabricatorEditField.php',
@@ -8729,6 +8730,7 @@ phutil_register_library_map(array(
'PhabricatorEditEngineSettingsPanel' => 'PhabricatorSettingsPanel',
'PhabricatorEditEngineStaticCommentAction' => 'PhabricatorEditEngineCommentAction',
'PhabricatorEditEngineSubtype' => 'Phobject',
+ 'PhabricatorEditEngineSubtypeMap' => 'Phobject',
'PhabricatorEditEngineSubtypeTestCase' => 'PhabricatorTestCase',
'PhabricatorEditEngineTokenizerCommentAction' => 'PhabricatorEditEngineCommentAction',
'PhabricatorEditField' => 'Phobject',
diff --git a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php
index 437362fa41..f4a1f2b344 100644
--- a/src/applications/maniphest/query/ManiphestTaskSearchEngine.php
+++ b/src/applications/maniphest/query/ManiphestTaskSearchEngine.php
@@ -47,7 +47,7 @@ final class ManiphestTaskSearchEngine
// Hide the "Subtypes" constraint from the web UI if the install only
// defines one task subtype, since it isn't of any use in this case.
$subtype_map = id(new ManiphestTask())->newEditEngineSubtypeMap();
- $hide_subtypes = (count($subtype_map) == 1);
+ $hide_subtypes = ($subtype_map->getCount() == 1);
return array(
id(new PhabricatorOwnersSearchField())
diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php
index a93fe58c3f..cdac563a14 100644
--- a/src/applications/maniphest/storage/ManiphestTask.php
+++ b/src/applications/maniphest/storage/ManiphestTask.php
@@ -573,7 +573,7 @@ final class ManiphestTask extends ManiphestDAO
public function newSubtypeObject() {
$subtype_key = $this->getEditEngineSubtype();
$subtype_map = $this->newEditEngineSubtypeMap();
- return idx($subtype_map, $subtype_key);
+ return $subtype_map->getSubtype($subtype_key);
}
/* -( PhabricatorFulltextInterface )--------------------------------------- */
diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php
index e2739c1d09..297bb970d1 100644
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@@ -214,11 +214,12 @@ final class ManiphestTransaction
public function renderSubtypeName($value) {
$object = $this->getObject();
$map = $object->newEditEngineSubtypeMap();
- if (!isset($map[$value])) {
+
+ if (!$map->isValidSubtype($value)) {
return $value;
}
- return $map[$value]->getName();
+ return $map->getSubtype($value)->getName();
}
}
diff --git a/src/applications/maniphest/typeahead/ManiphestTaskSubtypeDatasource.php b/src/applications/maniphest/typeahead/ManiphestTaskSubtypeDatasource.php
index c061c694e4..cfa5592ccc 100644
--- a/src/applications/maniphest/typeahead/ManiphestTaskSubtypeDatasource.php
+++ b/src/applications/maniphest/typeahead/ManiphestTaskSubtypeDatasource.php
@@ -28,7 +28,7 @@ final class ManiphestTaskSubtypeDatasource
$results = array();
$subtype_map = id(new ManiphestTask())->newEditEngineSubtypeMap();
- foreach ($subtype_map as $key => $subtype) {
+ foreach ($subtype_map->getSubtypes() as $key => $subtype) {
$result = id(new PhabricatorTypeaheadResult())
->setIcon($subtype->getIcon())
diff --git a/src/applications/maniphest/view/ManiphestTaskListView.php b/src/applications/maniphest/view/ManiphestTaskListView.php
index ba17b8e25d..e7128fb850 100644
--- a/src/applications/maniphest/view/ManiphestTaskListView.php
+++ b/src/applications/maniphest/view/ManiphestTaskListView.php
@@ -56,9 +56,6 @@ final class ManiphestTaskListView extends ManiphestView {
Javelin::initBehavior('maniphest-list-editor');
}
- $subtype_map = id(new ManiphestTask())
- ->newEditEngineSubtypeMap();
-
foreach ($this->tasks as $task) {
$item = id(new PHUIObjectItemView())
->setUser($this->getUser())
diff --git a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php
index 92bc34c72b..9ce8d7a0e5 100644
--- a/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php
+++ b/src/applications/transactions/controller/PhabricatorEditEngineConfigurationSubtypeController.php
@@ -61,8 +61,7 @@ Choose the object **subtype** that this form should create and edit.
EOTEXT
);
- $map = $engine->newSubtypeMap();
- $map = mpull($map, 'getName');
+ $map = $engine->newSubtypeMap()->getDisplayMap();
$form = id(new AphrontFormView())
->setUser($viewer)
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php
index df367955af..23fac106d7 100644
--- a/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php
+++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtype.php
@@ -182,7 +182,7 @@ final class PhabricatorEditEngineSubtype
$map[$key] = $subtype;
}
- return $map;
+ return new PhabricatorEditEngineSubtypeMap($map);
}
}
diff --git a/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
new file mode 100644
index 0000000000..cd8e0674ae
--- /dev/null
+++ b/src/applications/transactions/editengine/PhabricatorEditEngineSubtypeMap.php
@@ -0,0 +1,42 @@
+<?php
+
+
+final class PhabricatorEditEngineSubtypeMap
+ extends Phobject {
+
+ private $subtypes;
+
+ public function __construct(array $subtypes) {
+ assert_instances_of($subtypes, 'PhabricatorEditEngineSubtype');
+
+ $this->subtypes = $subtypes;
+ }
+
+ public function getDisplayMap() {
+ return mpull($this->subtypes, 'getName');
+ }
+
+ public function getCount() {
+ return count($this->subtypes);
+ }
+
+ public function isValidSubtype($subtype_key) {
+ return isset($this->subtypes[$subtype_key]);
+ }
+
+ public function getSubtypes() {
+ return $this->subtypes;
+ }
+
+ public function getSubtype($subtype_key) {
+ if (!$this->isValidSubtype($subtype_key)) {
+ throw new Exception(
+ pht(
+ 'Subtype key "%s" does not identify a valid subtype.',
+ $subtype_key));
+ }
+
+ return $this->subtypes[$subtype_key];
+ }
+
+}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index a77b7f84a4..f354de6a1a 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -2541,7 +2541,7 @@ abstract class PhabricatorApplicationTransactionEditor
continue;
}
- if (!isset($map[$new])) {
+ if (!$map->isValidSubtype($new)) {
$errors[] = new PhabricatorApplicationTransactionValidationError(
$transaction_type,
pht('Invalid'),
diff --git a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php
index d394e87266..ccadf9b819 100644
--- a/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php
+++ b/src/applications/transactions/editor/PhabricatorEditEngineConfigurationEditor.php
@@ -64,7 +64,7 @@ final class PhabricatorEditEngineConfigurationEditor
foreach ($xactions as $xaction) {
$new = $xaction->getNewValue();
- if (isset($map[$new])) {
+ if ($map->isValidSubtype($new)) {
continue;
}
diff --git a/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php b/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php
index 7d32545416..d0b6d017f3 100644
--- a/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php
+++ b/src/applications/transactions/engineextension/PhabricatorSubtypeEditEngineExtension.php
@@ -31,7 +31,7 @@ final class PhabricatorSubtypeEditEngineExtension
$subtype_type = PhabricatorTransactions::TYPE_SUBTYPE;
$map = $object->newEditEngineSubtypeMap();
- $options = mpull($map, 'getName');
+ $options = $map->getDisplayMap();
$subtype_field = id(new PhabricatorSelectEditField())
->setKey(self::EDITKEY)
@@ -45,7 +45,7 @@ final class PhabricatorSubtypeEditEngineExtension
// If subtypes are configured, enable changing them from the bulk editor
// and comment action stack.
- if (count($map) > 1) {
+ if ($map->getCount() > 1) {
$subtype_field
->setBulkEditLabel(pht('Change subtype to'))
->setCommentActionLabel(pht('Change Subtype'))
From b88a87c43a28a6948a4fd95e7f12a5e4ffdd22c0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Dec 2018 05:49:09 -0800
Subject: [PATCH 4/8] Address a transaction issue with some audit actions not
applying correctly
Summary:
See <https://discourse.phabricator-community.org/t/cannot-accept-commits-in-audit/2166/>.
In D19842, I changed `PhabricatorEditField->shouldGenerateTransactionsFromComment()`.
- Previously, it bailed on `getIsConduitOnly()`.
- After the patch, it bails on a missing `getCommentActionLabel()`.
The old code was actually wrong, and it was previously possible to apply possibly-invalid actions in some cases (or, at least, sneak them through this layer: they would only actually apply if not validated properly).
In practice, it let a different bug through: we sometimes loaded commits without loading their audit authority, so testing whether the viewer could "Accept" the commit or not (or take some other actions like "Raise Concern") would always fail and throw an exception: "Trying to access data not attached to this object..."
Fixing the insufficiently-strict transaction generation code exposed the "authority not attached" bug, which caused some actions to fail to generate transactions.
This appeared in the UI as either an unhelpful error ("You can't post an empty comment") or an action with no effect. The unhelpful error was because we show that error if you aren't taking any //other// actions, and we wouldn't generate an "Accept" action because of the interaction of these bugs, so the code thought you were just posting an empty comment.
Test Plan: Without leaving comments, accepted and rejected commits. No more error messages, and actions took effect.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: stephan.senkbeil, hskiba
Differential Revision: https://secure.phabricator.com/D19845
---
.../audit/editor/PhabricatorAuditEditor.php | 4 -
.../controller/DiffusionCommitController.php | 2 +-
.../editor/DiffusionCommitEditEngine.php | 5 +-
.../diffusion/query/DiffusionCommitQuery.php | 92 ++++++++++++++++++-
.../storage/PhabricatorRepositoryCommit.php | 80 +++-------------
5 files changed, 108 insertions(+), 75 deletions(-)
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
index cfa84b8bd7..3f65cc80f8 100644
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -59,10 +59,6 @@ final class PhabricatorAuditEditor
$this->oldAuditStatus = $object->getAuditStatus();
- $object->loadAndAttachAuditAuthority(
- $this->getActor(),
- $this->getActingAsPHID());
-
return parent::expandTransactions($object, $xactions);
}
diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index 283278fcf8..5621b1fa12 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -45,6 +45,7 @@ final class DiffusionCommitController extends DiffusionController {
->withIdentifiers(array($commit_identifier))
->needCommitData(true)
->needAuditRequests(true)
+ ->needAuditAuthority(array($viewer))
->setLimit(100)
->needIdentities(true)
->execute();
@@ -111,7 +112,6 @@ final class DiffusionCommitController extends DiffusionController {
}
$audit_requests = $commit->getAudits();
- $commit->loadAndAttachAuditAuthority($viewer);
$commit_data = $commit->getCommitData();
$is_foreign = $commit_data->getCommitDetail('foreign-svn-stub');
diff --git a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
index 3470d02f92..8392504def 100644
--- a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
+++ b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
@@ -43,9 +43,12 @@ final class DiffusionCommitEditEngine
}
protected function newObjectQuery() {
+ $viewer = $this->getViewer();
+
return id(new DiffusionCommitQuery())
->needCommitData(true)
- ->needAuditRequests(true);
+ ->needAuditRequests(true)
+ ->needAuditAuthority(array($viewer));
}
protected function getEditorURI() {
diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php
index cf8c25baf1..05072e07c7 100644
--- a/src/applications/diffusion/query/DiffusionCommitQuery.php
+++ b/src/applications/diffusion/query/DiffusionCommitQuery.php
@@ -17,6 +17,7 @@ final class DiffusionCommitQuery
private $unreachable;
private $needAuditRequests;
+ private $needAuditAuthority;
private $auditIDs;
private $auditorPHIDs;
private $epochMin;
@@ -121,6 +122,12 @@ final class DiffusionCommitQuery
return $this;
}
+ public function needAuditAuthority(array $users) {
+ assert_instances_of($users, 'PhabricatorUser');
+ $this->needAuditAuthority = $users;
+ return $this;
+ }
+
public function withAuditIDs(array $ids) {
$this->auditIDs = $ids;
return $this;
@@ -231,14 +238,27 @@ final class DiffusionCommitQuery
}
if (count($subqueries) > 1) {
- foreach ($subqueries as $key => $subquery) {
- $subqueries[$key] = '('.$subquery.')';
+ $unions = null;
+ foreach ($subqueries as $subquery) {
+ if (!$unions) {
+ $unions = qsprintf(
+ $conn,
+ '(%Q)',
+ $subquery);
+ continue;
+ }
+
+ $unions = qsprintf(
+ $conn,
+ '%Q UNION DISTINCT (%Q)',
+ $unions,
+ $subquery);
}
$query = qsprintf(
$conn,
'%Q %Q %Q',
- implode(' UNION DISTINCT ', $subqueries),
+ $unions,
$this->buildOrderClause($conn, true),
$this->buildLimitClause($conn));
} else {
@@ -423,6 +443,72 @@ final class DiffusionCommitQuery
$commits);
}
+ if ($this->needAuditAuthority) {
+ $authority_users = $this->needAuditAuthority;
+
+ // NOTE: This isn't very efficient since we're running two queries per
+ // user, but there's currently no way to figure out authority for
+ // multiple users in one query. Today, we only ever request authority for
+ // a single user and single commit, so this has no practical impact.
+
+ // NOTE: We're querying with the viewership of query viewer, not the
+ // actual users. If the viewer can't see a project or package, they
+ // won't be able to see who has authority on it. This is safer than
+ // showing them true authority, and should never matter today, but it
+ // also doesn't seem like a significant disclosure and might be
+ // reasonable to adjust later if it causes something weird or confusing
+ // to happen.
+
+ $authority_map = array();
+ foreach ($authority_users as $authority_user) {
+ $authority_phid = $authority_user->getPHID();
+ if (!$authority_phid) {
+ continue;
+ }
+
+ $result_phids = array();
+
+ // Users have authority over themselves.
+ $result_phids[] = $authority_phid;
+
+ // Users have authority over packages they own.
+ $owned_packages = id(new PhabricatorOwnersPackageQuery())
+ ->setViewer($viewer)
+ ->withAuthorityPHIDs(array($authority_phid))
+ ->execute();
+ foreach ($owned_packages as $package) {
+ $result_phids[] = $package->getPHID();
+ }
+
+ // Users have authority over projects they're members of.
+ $projects = id(new PhabricatorProjectQuery())
+ ->setViewer($viewer)
+ ->withMemberPHIDs(array($authority_phid))
+ ->execute();
+ foreach ($projects as $project) {
+ $result_phids[] = $project->getPHID();
+ }
+
+ $result_phids = array_fuse($result_phids);
+
+ foreach ($commits as $commit) {
+ $attach_phids = $result_phids;
+
+ // NOTE: When modifying your own commits, you act only on behalf of
+ // yourself, not your packages or projects. The idea here is that you
+ // can't accept your own commits. In the future, this might change or
+ // depend on configuration.
+ $author_phid = $commit->getAuthorPHID();
+ if ($author_phid == $authority_phid) {
+ $attach_phids = array($author_phid);
+ $attach_phids = array_fuse($attach_phids);
+ }
+
+ $commit->attachAuditAuthority($authority_user, $attach_phids);
+ }
+ }
+ }
+
return $commits;
}
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
index 5615b5231f..b5fe08b6e0 100644
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -210,84 +210,32 @@ final class PhabricatorRepositoryCommit
return $this->assertAttached($this->committerIdentity);
}
- public function loadAndAttachAuditAuthority(
- PhabricatorUser $viewer,
- $actor_phid = null) {
+ public function attachAuditAuthority(
+ PhabricatorUser $user,
+ array $authority) {
- if ($actor_phid === null) {
- $actor_phid = $viewer->getPHID();
+ $user_phid = $user->getPHID();
+ if (!$user->getPHID()) {
+ throw new Exception(
+ pht('You can not attach audit authority for a user with no PHID.'));
}
- // TODO: This method is a little weird and sketchy, but worlds better than
- // what came before it. Eventually, this should probably live in a Query
- // class.
-
- // Figure out which requests the actor has authority over: these are user
- // requests where they are the auditor, and packages and projects they are
- // a member of.
-
- if (!$actor_phid) {
- $attach_key = $viewer->getCacheFragment();
- $phids = array();
- } else {
- $attach_key = $actor_phid;
- // At least currently, when modifying your own commits, you act only on
- // behalf of yourself, not your packages/projects -- the idea being that
- // you can't accept your own commits. This may change or depend on
- // config.
- $actor_is_author = ($actor_phid == $this->getAuthorPHID());
- if ($actor_is_author) {
- $phids = array($actor_phid);
- } else {
- $phids = array();
- $phids[$actor_phid] = true;
-
- $owned_packages = id(new PhabricatorOwnersPackageQuery())
- ->setViewer($viewer)
- ->withAuthorityPHIDs(array($actor_phid))
- ->execute();
- foreach ($owned_packages as $package) {
- $phids[$package->getPHID()] = true;
- }
-
- $projects = id(new PhabricatorProjectQuery())
- ->setViewer($viewer)
- ->withMemberPHIDs(array($actor_phid))
- ->execute();
- foreach ($projects as $project) {
- $phids[$project->getPHID()] = true;
- }
-
- $phids = array_keys($phids);
- }
- }
-
- $this->auditAuthorityPHIDs[$attach_key] = array_fuse($phids);
+ $this->auditAuthorityPHIDs[$user_phid] = $authority;
return $this;
}
public function hasAuditAuthority(
- PhabricatorUser $viewer,
- PhabricatorRepositoryAuditRequest $audit,
- $actor_phid = null) {
+ PhabricatorUser $user,
+ PhabricatorRepositoryAuditRequest $audit) {
- if ($actor_phid === null) {
- $actor_phid = $viewer->getPHID();
- }
-
- if (!$actor_phid) {
- $attach_key = $viewer->getCacheFragment();
- } else {
- $attach_key = $actor_phid;
- }
-
- $map = $this->assertAttachedKey($this->auditAuthorityPHIDs, $attach_key);
-
- if (!$actor_phid) {
+ $user_phid = $user->getPHID();
+ if (!$user_phid) {
return false;
}
+ $map = $this->assertAttachedKey($this->auditAuthorityPHIDs, $user_phid);
+
return isset($map[$audit->getAuditorPHID()]);
}
From 1029081b28051b7fb82d5eac89c6e40d1a6e6be4 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Dec 2018 06:12:08 -0800
Subject: [PATCH 5/8] Correct two straggling "%Q" + "implode(...)" callsites in
Revision updates
Summary:
See <https://discourse.phabricator-community.org/t/error-seems-to-be-related-with-da40f8074-and-php-7-3/2102/12>. When creating or updating revisions, we do some manual query construction to update the affected path table.
Update these queries to modern `qsprintf()`.
Test Plan: Created and updated revisions affecting paths, no more logs in the webserver log.
Reviewers: amckinley
Reviewed By: amckinley
Differential Revision: https://secure.phabricator.com/D19846
---
.../differential/editor/DifferentialTransactionEditor.php | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 7fa4cd61e2..853b024e27 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -1367,9 +1367,9 @@ final class DifferentialTransactionEditor
foreach (array_chunk($sql, 256) as $chunk) {
queryfx(
$conn_w,
- 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q',
+ 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %LQ',
$table->getTableName(),
- implode(', ', $chunk));
+ $chunk);
}
}
@@ -1444,9 +1444,9 @@ final class DifferentialTransactionEditor
if ($sql) {
queryfx(
$conn_w,
- 'INSERT INTO %T (revisionID, type, hash) VALUES %Q',
+ 'INSERT INTO %T (revisionID, type, hash) VALUES %LQ',
ArcanistDifferentialRevisionHash::TABLE_NAME,
- implode(', ', $sql));
+ $sql);
}
}
From 1e4bdc39a11beb2fe39a53e13d35773786ef0306 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Dec 2018 12:40:59 -0800
Subject: [PATCH 6/8] Add an "availaiblity" attachment for user.search
Summary: Ref T13222. See PHI990. The older `user.query` supports availability information, but it isn't currently available in a modern way. Make it available.
Test Plan: {F6048126}
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19851
---
src/__phutil_library_map__.php | 2 +
...opleAvailabilitySearchEngineAttachment.php | 46 +++++++++++++++++++
.../people/storage/PhabricatorUser.php | 5 +-
3 files changed, 52 insertions(+), 1 deletion(-)
create mode 100644 src/applications/people/engineextension/PhabricatorPeopleAvailabilitySearchEngineAttachment.php
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index aeba5e18ae..cd260f497d 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3761,6 +3761,7 @@ phutil_register_library_map(array(
'PhabricatorPeopleAnyOwnerDatasource' => 'applications/people/typeahead/PhabricatorPeopleAnyOwnerDatasource.php',
'PhabricatorPeopleApplication' => 'applications/people/application/PhabricatorPeopleApplication.php',
'PhabricatorPeopleApproveController' => 'applications/people/controller/PhabricatorPeopleApproveController.php',
+ 'PhabricatorPeopleAvailabilitySearchEngineAttachment' => 'applications/people/engineextension/PhabricatorPeopleAvailabilitySearchEngineAttachment.php',
'PhabricatorPeopleBadgesProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeopleBadgesProfileMenuItem.php',
'PhabricatorPeopleCommitsProfileMenuItem' => 'applications/people/menuitem/PhabricatorPeopleCommitsProfileMenuItem.php',
'PhabricatorPeopleController' => 'applications/people/controller/PhabricatorPeopleController.php',
@@ -9640,6 +9641,7 @@ phutil_register_library_map(array(
'PhabricatorPeopleAnyOwnerDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorPeopleApplication' => 'PhabricatorApplication',
'PhabricatorPeopleApproveController' => 'PhabricatorPeopleController',
+ 'PhabricatorPeopleAvailabilitySearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
'PhabricatorPeopleBadgesProfileMenuItem' => 'PhabricatorProfileMenuItem',
'PhabricatorPeopleCommitsProfileMenuItem' => 'PhabricatorProfileMenuItem',
'PhabricatorPeopleController' => 'PhabricatorController',
diff --git a/src/applications/people/engineextension/PhabricatorPeopleAvailabilitySearchEngineAttachment.php b/src/applications/people/engineextension/PhabricatorPeopleAvailabilitySearchEngineAttachment.php
new file mode 100644
index 0000000000..f61ebe7f01
--- /dev/null
+++ b/src/applications/people/engineextension/PhabricatorPeopleAvailabilitySearchEngineAttachment.php
@@ -0,0 +1,46 @@
+<?php
+
+final class PhabricatorPeopleAvailabilitySearchEngineAttachment
+ extends PhabricatorSearchEngineAttachment {
+
+ public function getAttachmentName() {
+ return pht('User Availability');
+ }
+
+ public function getAttachmentDescription() {
+ return pht('Get availability information for users.');
+ }
+
+ public function willLoadAttachmentData($query, $spec) {
+ $query->needAvailability(true);
+ }
+
+ public function getAttachmentForObject($object, $data, $spec) {
+
+ $until = $object->getAwayUntil();
+ if ($until) {
+ $until = (int)$until;
+ } else {
+ $until = null;
+ }
+
+ $value = $object->getDisplayAvailability();
+ if ($value === null) {
+ $value = PhabricatorCalendarEventInvitee::AVAILABILITY_AVAILABLE;
+ }
+
+ $name = PhabricatorCalendarEventInvitee::getAvailabilityName($value);
+ $color = PhabricatorCalendarEventInvitee::getAvailabilityColor($value);
+
+ $event_phid = $object->getAvailabilityEventPHID();
+
+ return array(
+ 'value' => $value,
+ 'until' => $until,
+ 'name' => $name,
+ 'color' => $color,
+ 'eventPHID' => $event_phid,
+ );
+ }
+
+}
diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php
index 651d1b75a6..7717fbf18c 100644
--- a/src/applications/people/storage/PhabricatorUser.php
+++ b/src/applications/people/storage/PhabricatorUser.php
@@ -1466,7 +1466,10 @@ final class PhabricatorUser
}
public function getConduitSearchAttachments() {
- return array();
+ return array(
+ id(new PhabricatorPeopleAvailabilitySearchEngineAttachment())
+ ->setAttachmentKey('availability'),
+ );
}
From bba4186005917d7325e9888a5ff7b989061889f9 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Dec 2018 11:51:15 -0800
Subject: [PATCH 7/8] Allow "bin/repository thaw" to accept
"--all-repositories" instead of a list of repositories
Summary:
Ref T13222. See PHI992. If you've lost an entire cluster (or have lost a device and are willing to make broad assumptions about the state the device was in) you currently have to `xargs` to thaw everything or do something else creative.
Since this workflow is broadly reasonable, provide an easier way to accomplish the goal.
Test Plan:
- Ran with `--all-repositories`, a list of repositories, both (error) and neither (error).
- Saw a helpful new list of affected repositories.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19849
---
...icatorRepositoryManagementThawWorkflow.php | 66 +++++++++++++++++--
1 file changed, 60 insertions(+), 6 deletions(-)
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php
index 4c4eed85b9..ed2e1df3e2 100644
--- a/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php
@@ -32,6 +32,12 @@ final class PhabricatorRepositoryManagementThawWorkflow
'name' => 'force',
'help' => pht('Run operations without asking for confirmation.'),
),
+ array(
+ 'name' => 'all-repositories',
+ 'help' => pht(
+ 'Apply the promotion or demotion to all repositories hosted '.
+ 'on the device.'),
+ ),
array(
'name' => 'repositories',
'wildcard' => true,
@@ -42,12 +48,6 @@ final class PhabricatorRepositoryManagementThawWorkflow
public function execute(PhutilArgumentParser $args) {
$viewer = $this->getViewer();
- $repositories = $this->loadRepositories($args, 'repositories');
- if (!$repositories) {
- throw new PhutilArgumentUsageException(
- pht('Specify one or more repositories to thaw.'));
- }
-
$promote = $args->getArg('promote');
$demote = $args->getArg('demote');
@@ -72,6 +72,60 @@ final class PhabricatorRepositoryManagementThawWorkflow
pht('No device "%s" exists.', $device_name));
}
+ $repository_names = $args->getArg('repositories');
+ $all_repositories = $args->getArg('all-repositories');
+ if ($repository_names && $all_repositories) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Specify a list of repositories or "--all-repositories", '.
+ 'but not both.'));
+ } else if (!$repository_names && !$all_repositories) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Select repositories to affect by providing a list of repositories '.
+ 'or using the "--all-repositories" flag.'));
+ }
+
+ if ($repository_names) {
+ $repositories = $this->loadRepositories($args, 'repositories');
+ if (!$repositories) {
+ throw new PhutilArgumentUsageException(
+ pht('Specify one or more repositories to thaw.'));
+ }
+ } else {
+ $repositories = array();
+
+ $services = id(new AlmanacServiceQuery())
+ ->setViewer($viewer)
+ ->withDevicePHIDs(array($device->getPHID()))
+ ->execute();
+ if ($services) {
+ $repositories = id(new PhabricatorRepositoryQuery())
+ ->setViewer($viewer)
+ ->withAlmanacServicePHIDs(mpull($services, 'getPHID'))
+ ->execute();
+ }
+
+ if (!$repositories) {
+ throw new PhutilArgumentUsageException(
+ pht('There are no repositories on the selected device.'));
+ }
+ }
+
+ $display_list = new PhutilConsoleList();
+ foreach ($repositories as $repository) {
+ $display_list->addItem(
+ pht(
+ '%s %s',
+ $repository->getMonogram(),
+ $repository->getName()));
+ }
+
+ echo tsprintf(
+ "%s\n\n%B\n",
+ pht('These repositories will be thawed:'),
+ $display_list->drawConsoleString());
+
if ($promote) {
$risk_message = pht(
'Promoting a device can cause the loss of any repository data which '.
From 1a6a0181a8985371fe1470cbea24463885850668 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 5 Dec 2018 12:22:14 -0800
Subject: [PATCH 8/8] Allow "bin/repository thaw --demote" to demote an entire
service, not just a single device
Summary: Ref T13222. See PHI992. If you lose an entire cluster, you may want to aggressively demote it out of existence. You currently need to `xargs` your way through this. Allow `--demote <service>`, which demotes all devices in a service.
Test Plan: Demoted with `--demote <device>` and `--demote <service>`. Hit the `--promote service` error.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19850
---
...icatorRepositoryManagementThawWorkflow.php | 263 ++++++++++--------
.../user/cluster/cluster_repositories.diviner | 12 +
2 files changed, 164 insertions(+), 111 deletions(-)
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php
index ed2e1df3e2..3743045c34 100644
--- a/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementThawWorkflow.php
@@ -15,10 +15,11 @@ final class PhabricatorRepositoryManagementThawWorkflow
array(
array(
'name' => 'demote',
- 'param' => 'device',
+ 'param' => 'device/service',
'help' => pht(
- 'Demote a device, discarding local changes. Clears stuck '.
- 'write locks and recovers from lost leaders.'),
+ 'Demote a device (or all devices in a service) discarding '.
+ 'local changes. Clears stuck write locks and recovers from '.
+ 'lost leaders.'),
),
array(
'name' => 'promote',
@@ -61,15 +62,53 @@ final class PhabricatorRepositoryManagementThawWorkflow
pht('Specify either --promote or --demote, but not both.'));
}
- $device_name = nonempty($promote, $demote);
+ $target_name = nonempty($promote, $demote);
- $device = id(new AlmanacDeviceQuery())
+ $devices = id(new AlmanacDeviceQuery())
->setViewer($viewer)
- ->withNames(array($device_name))
- ->executeOne();
- if (!$device) {
- throw new PhutilArgumentUsageException(
- pht('No device "%s" exists.', $device_name));
+ ->withNames(array($target_name))
+ ->execute();
+ if (!$devices) {
+ $service = id(new AlmanacServiceQuery())
+ ->setViewer($viewer)
+ ->withNames(array($target_name))
+ ->executeOne();
+
+ if (!$service) {
+ throw new PhutilArgumentUsageException(
+ pht('No device or service named "%s" exists.', $target_name));
+ }
+
+ if ($promote) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'You can not "--promote" an entire service ("%s"). Only a single '.
+ 'device may be promoted.',
+ $target_name));
+ }
+
+ $bindings = id(new AlmanacBindingQuery())
+ ->setViewer($viewer)
+ ->withServicePHIDs(array($service->getPHID()))
+ ->execute();
+ if (!$bindings) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Service "%s" is not bound to any devices.',
+ $target_name));
+ }
+
+ $interfaces = id(new AlmanacInterfaceQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(mpull($bindings, 'getInterfacePHID'))
+ ->execute();
+
+ $device_phids = mpull($interfaces, 'getDevicePHID');
+
+ $devices = id(new AlmanacDeviceQuery())
+ ->setViewer($viewer)
+ ->withPHIDs($device_phids)
+ ->execute();
}
$repository_names = $args->getArg('repositories');
@@ -97,7 +136,7 @@ final class PhabricatorRepositoryManagementThawWorkflow
$services = id(new AlmanacServiceQuery())
->setViewer($viewer)
- ->withDevicePHIDs(array($device->getPHID()))
+ ->withDevicePHIDs(mpull($devices, 'getPHID'))
->execute();
if ($services) {
$repositories = id(new PhabricatorRepositoryQuery())
@@ -108,7 +147,7 @@ final class PhabricatorRepositoryManagementThawWorkflow
if (!$repositories) {
throw new PhutilArgumentUsageException(
- pht('There are no repositories on the selected device.'));
+ pht('There are no repositories on the selected device or service.'));
}
}
@@ -150,126 +189,128 @@ final class PhabricatorRepositoryManagementThawWorkflow
pht('User aborted the workflow.'));
}
- foreach ($repositories as $repository) {
- $repository_phid = $repository->getPHID();
+ foreach ($devices as $device) {
+ foreach ($repositories as $repository) {
+ $repository_phid = $repository->getPHID();
- $write_lock = PhabricatorRepositoryWorkingCopyVersion::getWriteLock(
- $repository_phid);
+ $write_lock = PhabricatorRepositoryWorkingCopyVersion::getWriteLock(
+ $repository_phid);
- echo tsprintf(
- "%s\n",
- pht(
- 'Waiting to acquire write lock for "%s"...',
- $repository->getDisplayName()));
+ echo tsprintf(
+ "%s\n",
+ pht(
+ 'Waiting to acquire write lock for "%s"...',
+ $repository->getDisplayName()));
- $write_lock->lock(phutil_units('5 minutes in seconds'));
- try {
+ $write_lock->lock(phutil_units('5 minutes in seconds'));
+ try {
- $service = $repository->loadAlmanacService();
- if (!$service) {
- throw new PhutilArgumentUsageException(
- pht(
- 'Repository "%s" is not a cluster repository: it is not '.
- 'bound to an Almanac service.',
- $repository->getDisplayName()));
- }
-
- if ($promote) {
- // You can only promote active devices. (You may demote active or
- // inactive devices.)
- $bindings = $service->getActiveBindings();
- $bindings = mpull($bindings, null, 'getDevicePHID');
- if (empty($bindings[$device->getPHID()])) {
+ $service = $repository->loadAlmanacService();
+ if (!$service) {
throw new PhutilArgumentUsageException(
pht(
- 'Repository "%s" has no active binding to device "%s". Only '.
- 'actively bound devices can be promoted.',
- $repository->getDisplayName(),
- $device->getName()));
+ 'Repository "%s" is not a cluster repository: it is not '.
+ 'bound to an Almanac service.',
+ $repository->getDisplayName()));
}
- $versions = PhabricatorRepositoryWorkingCopyVersion::loadVersions(
- $repository->getPHID());
- $versions = mpull($versions, null, 'getDevicePHID');
-
- // Before we promote, make sure there are no outstanding versions on
- // devices with inactive bindings. If there are, you need to demote
- // these first.
- $inactive = array();
- foreach ($versions as $device_phid => $version) {
- if (isset($bindings[$device_phid])) {
- continue;
+ if ($promote) {
+ // You can only promote active devices. (You may demote active or
+ // inactive devices.)
+ $bindings = $service->getActiveBindings();
+ $bindings = mpull($bindings, null, 'getDevicePHID');
+ if (empty($bindings[$device->getPHID()])) {
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Repository "%s" has no active binding to device "%s". '.
+ 'Only actively bound devices can be promoted.',
+ $repository->getDisplayName(),
+ $device->getName()));
}
- $inactive[$device_phid] = $version;
- }
- if ($inactive) {
- $handles = $viewer->loadHandles(array_keys($inactive));
+ $versions = PhabricatorRepositoryWorkingCopyVersion::loadVersions(
+ $repository->getPHID());
+ $versions = mpull($versions, null, 'getDevicePHID');
- $handle_list = iterator_to_array($handles);
- $handle_list = mpull($handle_list, 'getName');
- $handle_list = implode(', ', $handle_list);
+ // Before we promote, make sure there are no outstanding versions
+ // on devices with inactive bindings. If there are, you need to
+ // demote these first.
+ $inactive = array();
+ foreach ($versions as $device_phid => $version) {
+ if (isset($bindings[$device_phid])) {
+ continue;
+ }
+ $inactive[$device_phid] = $version;
+ }
- throw new PhutilArgumentUsageException(
+ if ($inactive) {
+ $handles = $viewer->loadHandles(array_keys($inactive));
+
+ $handle_list = iterator_to_array($handles);
+ $handle_list = mpull($handle_list, 'getName');
+ $handle_list = implode(', ', $handle_list);
+
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Repository "%s" has versions on inactive devices. Demote '.
+ '(or reactivate) these devices before promoting a new '.
+ 'leader: %s.',
+ $repository->getDisplayName(),
+ $handle_list));
+ }
+
+ // Now, make sure there are no outstanding versions on devices with
+ // active bindings. These also need to be demoted (or promoting is
+ // a mistake or already happened).
+ $active = array_select_keys($versions, array_keys($bindings));
+ if ($active) {
+ $handles = $viewer->loadHandles(array_keys($active));
+
+ $handle_list = iterator_to_array($handles);
+ $handle_list = mpull($handle_list, 'getName');
+ $handle_list = implode(', ', $handle_list);
+
+ throw new PhutilArgumentUsageException(
+ pht(
+ 'Unable to promote "%s" for repository "%s" because this '.
+ 'cluster already has one or more unambiguous leaders: %s.',
+ $device->getName(),
+ $repository->getDisplayName(),
+ $handle_list));
+ }
+
+ PhabricatorRepositoryWorkingCopyVersion::updateVersion(
+ $repository->getPHID(),
+ $device->getPHID(),
+ 0);
+
+ echo tsprintf(
+ "%s\n",
pht(
- 'Repository "%s" has versions on inactive devices. Demote '.
- '(or reactivate) these devices before promoting a new '.
- 'leader: %s.',
- $repository->getDisplayName(),
- $handle_list));
- }
-
- // Now, make sure there are no outstanding versions on devices with
- // active bindings. These also need to be demoted (or promoting is a
- // mistake or already happened).
- $active = array_select_keys($versions, array_keys($bindings));
- if ($active) {
- $handles = $viewer->loadHandles(array_keys($active));
-
- $handle_list = iterator_to_array($handles);
- $handle_list = mpull($handle_list, 'getName');
- $handle_list = implode(', ', $handle_list);
-
- throw new PhutilArgumentUsageException(
- pht(
- 'Unable to promote "%s" for repository "%s" because this '.
- 'cluster already has one or more unambiguous leaders: %s.',
+ 'Promoted "%s" to become a leader for "%s".',
$device->getName(),
- $repository->getDisplayName(),
- $handle_list));
+ $repository->getDisplayName()));
}
- PhabricatorRepositoryWorkingCopyVersion::updateVersion(
- $repository->getPHID(),
- $device->getPHID(),
- 0);
+ if ($demote) {
+ PhabricatorRepositoryWorkingCopyVersion::demoteDevice(
+ $repository->getPHID(),
+ $device->getPHID());
- echo tsprintf(
- "%s\n",
- pht(
- 'Promoted "%s" to become a leader for "%s".',
- $device->getName(),
- $repository->getDisplayName()));
+ echo tsprintf(
+ "%s\n",
+ pht(
+ 'Demoted "%s" from leadership of repository "%s".',
+ $device->getName(),
+ $repository->getDisplayName()));
+ }
+ } catch (Exception $ex) {
+ $write_lock->unlock();
+ throw $ex;
}
- if ($demote) {
- PhabricatorRepositoryWorkingCopyVersion::demoteDevice(
- $repository->getPHID(),
- $device->getPHID());
-
- echo tsprintf(
- "%s\n",
- pht(
- 'Demoted "%s" from leadership of repository "%s".',
- $device->getName(),
- $repository->getDisplayName()));
- }
- } catch (Exception $ex) {
$write_lock->unlock();
- throw $ex;
}
-
- $write_lock->unlock();
}
return 0;
diff --git a/src/docs/user/cluster/cluster_repositories.diviner b/src/docs/user/cluster/cluster_repositories.diviner
index cdf2641933..13307d0b79 100644
--- a/src/docs/user/cluster/cluster_repositories.diviner
+++ b/src/docs/user/cluster/cluster_repositories.diviner
@@ -433,6 +433,18 @@ If you do this, **you will lose unreplicated data**. You will discard any
changes on the affected leaders which have not replicated to other devices
in the cluster.
+If you have lost an entire cluster and replaced it with new devices that you
+have restored from backups, you can aggressively wipe all memory of the old
+devices by using `--demote <service>` and `--all-repositories`. **This is
+dangerous and discards all unreplicated data in any repository on any device.**
+
+```
+phabricator/ $ ./bin/repository thaw --demote repo.corp.net --all-repositories
+```
+
+After you do this, continue below to promote a leader and restore the cluster
+to service.
+
Ambiguous Leaders
=================