diff --git a/resources/sql/autopatches/20210625.owners.01.authority.sql b/resources/sql/autopatches/20210625.owners.01.authority.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20210625.owners.01.authority.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_owners.owners_package + ADD authorityMode VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20210625.owners.02.authority-default.sql b/resources/sql/autopatches/20210625.owners.02.authority-default.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20210625.owners.02.authority-default.sql @@ -0,0 +1,3 @@ +UPDATE {$NAMESPACE}_owners.owners_package + SET authorityMode = 'strong' + WHERE authorityMode = ''; 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 @@ -3966,6 +3966,7 @@ 'PhabricatorOwnersOwner' => 'applications/owners/storage/PhabricatorOwnersOwner.php', 'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php', 'PhabricatorOwnersPackageAuditingTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php', + 'PhabricatorOwnersPackageAuthorityTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuthorityTransaction.php', 'PhabricatorOwnersPackageAutoreviewTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php', 'PhabricatorOwnersPackageContextFreeGrammar' => 'applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php', 'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php', @@ -10582,6 +10583,7 @@ 'PhabricatorNgramsInterface', ), 'PhabricatorOwnersPackageAuditingTransaction' => 'PhabricatorOwnersPackageTransactionType', + 'PhabricatorOwnersPackageAuthorityTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageAutoreviewTransaction' => 'PhabricatorOwnersPackageTransactionType', 'PhabricatorOwnersPackageContextFreeGrammar' => 'PhutilContextFreeGrammar', 'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource', diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -311,9 +311,17 @@ // which the actor may be able to use their authority over to gain the // ability to force-accept for other packages. This query doesn't apply // dominion rules yet, and we'll bypass those rules later on. + + // See T13657. We ignore "watcher" packages which don't grant their owners + // permission to force accept anything. + $authority_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) + ->withAuthorityModes( + array( + PhabricatorOwnersPackage::AUTHORITY_STRONG, + )) ->withAuthorityPHIDs(array($viewer->getPHID())) ->withControl($repository_phid, $paths); $authority_packages = $authority_query->execute(); diff --git a/src/applications/owners/controller/PhabricatorOwnersDetailController.php b/src/applications/owners/controller/PhabricatorOwnersDetailController.php --- a/src/applications/owners/controller/PhabricatorOwnersDetailController.php +++ b/src/applications/owners/controller/PhabricatorOwnersDetailController.php @@ -197,6 +197,12 @@ $name = idx($spec, 'short', $dominion); $view->addProperty(pht('Dominion'), $name); + $authority_mode = $package->getAuthorityMode(); + $authority_map = PhabricatorOwnersPackage::getAuthorityOptionsMap(); + $spec = idx($authority_map, $authority_mode, array()); + $name = idx($spec, 'short', $authority_mode); + $view->addProperty(pht('Authority'), $name); + $auto = $package->getAutoReview(); $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap(); $spec = idx($autoreview_map, $auto, array()); diff --git a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php --- a/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php +++ b/src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php @@ -90,6 +90,9 @@ $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap(); $dominion_map = ipull($dominion_map, 'name'); + $authority_map = PhabricatorOwnersPackage::getAuthorityOptionsMap(); + $authority_map = ipull($authority_map, 'name'); + return array( id(new PhabricatorTextEditField()) ->setKey('name') @@ -118,6 +121,16 @@ ->setIsCopyable(true) ->setValue($object->getDominion()) ->setOptions($dominion_map), + id(new PhabricatorSelectEditField()) + ->setKey('authority') + ->setLabel(pht('Authority')) + ->setDescription( + pht('Change package authority rules.')) + ->setTransactionType( + PhabricatorOwnersPackageAuthorityTransaction::TRANSACTIONTYPE) + ->setIsCopyable(true) + ->setValue($object->getAuthorityMode()) + ->setOptions($authority_map), id(new PhabricatorSelectEditField()) ->setKey('autoReview') ->setLabel(pht('Auto Review')) diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -10,6 +10,7 @@ private $repositoryPHIDs; private $paths; private $statuses; + private $authorityModes; private $controlMap = array(); private $controlResults; @@ -77,6 +78,11 @@ return $this; } + public function withAuthorityModes(array $modes) { + $this->authorityModes = $modes; + return $this; + } + public function withNameNgrams($ngrams) { return $this->withNgramsConstraint( new PhabricatorOwnersPackageNameNgrams(), @@ -231,6 +237,13 @@ $where[] = qsprintf($conn, '%LO', $clauses); } + if ($this->authorityModes !== null) { + $where[] = qsprintf( + $conn, + 'authorityMode IN (%Ls)', + $this->authorityModes); + } + return $where; } diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php --- a/src/applications/owners/storage/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php @@ -21,6 +21,7 @@ protected $dominion; protected $properties = array(); protected $auditingState; + protected $authorityMode; private $paths = self::ATTACHABLE; private $owners = self::ATTACHABLE; @@ -41,6 +42,9 @@ const DOMINION_STRONG = 'strong'; const DOMINION_WEAK = 'weak'; + const AUTHORITY_STRONG = 'strong'; + const AUTHORITY_WEAK = 'weak'; + const PROPERTY_IGNORED = 'ignored'; public static function initializeNewPackage(PhabricatorUser $actor) { @@ -58,6 +62,7 @@ ->setAuditingState(PhabricatorOwnersAuditRule::AUDITING_NONE) ->setAutoReview(self::AUTOREVIEW_NONE) ->setDominion(self::DOMINION_STRONG) + ->setAuthorityMode(self::AUTHORITY_STRONG) ->setViewPolicy($view_policy) ->setEditPolicy($edit_policy) ->attachPaths(array()) @@ -115,6 +120,19 @@ ); } + public static function getAuthorityOptionsMap() { + return array( + self::AUTHORITY_STRONG => array( + 'name' => pht('Strong (Package Owns Paths)'), + 'short' => pht('Strong'), + ), + self::AUTHORITY_WEAK => array( + 'name' => pht('Weak (Package Watches Paths)'), + 'short' => pht('Weak'), + ), + ); + } + protected function getConfiguration() { return array( // This information is better available from the history table. @@ -130,6 +148,7 @@ 'status' => 'text32', 'autoReview' => 'text32', 'dominion' => 'text32', + 'authorityMode' => 'text32', ), ) + parent::getConfiguration(); } @@ -568,6 +587,10 @@ return PhabricatorOwnersAuditRule::newFromState($this->getAuditingState()); } + public function getHasStrongAuthority() { + return ($this->getAuthorityMode() === self::AUTHORITY_STRONG); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -696,6 +719,10 @@ ->setKey('dominion') ->setType('map') ->setDescription(pht('Dominion setting information.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('authority') + ->setType('map') + ->setDescription(pht('Authority setting information.')), id(new PhabricatorConduitSearchFieldSpecification()) ->setKey('ignored') ->setType('map') @@ -747,6 +774,23 @@ 'short' => $dominion_short, ); + + $authority_value = $this->getAuthorityMode(); + $authority_map = self::getAuthorityOptionsMap(); + if (isset($authority_map[$authority_value])) { + $authority_label = $authority_map[$authority_value]['name']; + $authority_short = $authority_map[$authority_value]['short']; + } else { + $authority_label = pht('Unknown ("%s")', $authority_value); + $authority_short = pht('Unknown ("%s")', $authority_value); + } + + $authority = array( + 'value' => $authority_value, + 'label' => $authority_label, + 'short' => $authority_short, + ); + // Force this to always emit as a JSON object even if empty, never as // a JSON list. $ignored = $this->getIgnoredPathAttributes(); @@ -762,6 +806,7 @@ 'review' => $review, 'audit' => $audit, 'dominion' => $dominion, + 'authority' => $authority, 'ignored' => $ignored, ); } diff --git a/src/applications/owners/xaction/PhabricatorOwnersPackageAuthorityTransaction.php b/src/applications/owners/xaction/PhabricatorOwnersPackageAuthorityTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/owners/xaction/PhabricatorOwnersPackageAuthorityTransaction.php @@ -0,0 +1,56 @@ +getAuthorityMode(); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $map = PhabricatorOwnersPackage::getAuthorityOptionsMap(); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (empty($map[$new])) { + $valid = array_keys($map); + + $errors[] = $this->newInvalidError( + pht( + 'Authority setting "%s" is not valid. '. + 'Valid settings are: %s.', + $new, + implode(', ', $valid)), + $xaction); + } + } + + return $errors; + } + + public function applyInternalEffects($object, $value) { + $object->setAuthorityMode($value); + } + + public function getTitle() { + $map = PhabricatorOwnersPackage::getAuthorityOptionsMap(); + $map = ipull($map, 'short'); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $old = idx($map, $old, $old); + $new = idx($map, $new, $new); + + return pht( + '%s adjusted package authority rules from %s to %s.', + $this->renderAuthor(), + $this->renderValue($old), + $this->renderValue($new)); + } + +}