diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php index 1991580116..2258c4fdcb 100644 --- a/src/applications/differential/controller/DifferentialController.php +++ b/src/applications/differential/controller/DifferentialController.php @@ -1,228 +1,226 @@ getRequest()->getUser(); $nav = new AphrontSideNavFilterView(); $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); id(new DifferentialRevisionSearchEngine()) ->setViewer($viewer) ->addNavigationItems($nav->getMenu()); $nav->selectFilter(null); return $nav; } public function buildApplicationMenu() { return $this->buildSideNavView(true)->getMenu(); } protected function buildTableOfContents( array $changesets, array $visible_changesets, array $coverage) { $viewer = $this->getViewer(); $toc_view = id(new PHUIDiffTableOfContentsListView()) ->setViewer($viewer) ->setBare(true); $have_owners = PhabricatorApplication::isClassInstalledForViewer( 'PhabricatorOwnersApplication', $viewer); if ($have_owners) { $repository_phid = null; if ($changesets) { $changeset = head($changesets); $diff = $changeset->getDiff(); $repository_phid = $diff->getRepositoryPHID(); } if (!$repository_phid) { $have_owners = false; } else { if ($viewer->getPHID()) { $packages = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withAuthorityPHIDs(array($viewer->getPHID())) ->execute(); $toc_view->setAuthorityPackages($packages); } - // TODO: For Subversion, we should adjust these paths to be relative to - // the repository root where possible. - $paths = mpull($changesets, 'getFilename'); + $paths = mpull($changesets, 'getOwnersFilename'); $control_query = id(new PhabricatorOwnersPackageQuery()) ->setViewer($viewer) ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) ->withControl($repository_phid, $paths); $control_query->execute(); } } foreach ($changesets as $changeset_id => $changeset) { $is_visible = isset($visible_changesets[$changeset_id]); $anchor = $changeset->getAnchorName(); $filename = $changeset->getFilename(); $coverage_id = 'differential-mcoverage-'.md5($filename); $item = id(new PHUIDiffTableOfContentsItemView()) ->setChangeset($changeset) ->setIsVisible($is_visible) ->setAnchor($anchor) ->setCoverage(idx($coverage, $filename)) ->setCoverageID($coverage_id); if ($have_owners) { $packages = $control_query->getControllingPackagesForPath( $repository_phid, - $changeset->getFilename()); + $changeset->getOwnersFilename()); $item->setPackages($packages); } $toc_view->addItem($item); } return $toc_view; } protected function loadDiffProperties(array $diffs) { $diffs = mpull($diffs, null, 'getID'); $properties = id(new DifferentialDiffProperty())->loadAllWhere( 'diffID IN (%Ld)', array_keys($diffs)); $properties = mgroup($properties, 'getDiffID'); foreach ($diffs as $id => $diff) { $values = idx($properties, $id, array()); $values = mpull($values, 'getData', 'getName'); $diff->attachDiffProperties($values); } } protected function loadHarbormasterData(array $diffs) { $viewer = $this->getViewer(); $diffs = mpull($diffs, null, 'getPHID'); $buildables = id(new HarbormasterBuildableQuery()) ->setViewer($viewer) ->withBuildablePHIDs(array_keys($diffs)) ->withManualBuildables(false) ->needBuilds(true) ->needTargets(true) ->execute(); $buildables = mpull($buildables, null, 'getBuildablePHID'); foreach ($diffs as $phid => $diff) { $diff->attachBuildable(idx($buildables, $phid)); } $target_map = array(); foreach ($diffs as $phid => $diff) { $target_map[$phid] = $diff->getBuildTargetPHIDs(); } $all_target_phids = array_mergev($target_map); if ($all_target_phids) { $unit_messages = id(new HarbormasterBuildUnitMessage())->loadAllWhere( 'buildTargetPHID IN (%Ls)', $all_target_phids); $unit_messages = mgroup($unit_messages, 'getBuildTargetPHID'); } else { $unit_messages = array(); } foreach ($diffs as $phid => $diff) { $target_phids = idx($target_map, $phid, array()); $messages = array_select_keys($unit_messages, $target_phids); $messages = array_mergev($messages); $diff->attachUnitMessages($messages); } // For diffs with no messages, look for legacy unit messages stored on the // diff itself. foreach ($diffs as $phid => $diff) { if ($diff->getUnitMessages()) { continue; } if (!$diff->hasDiffProperty('arc:unit')) { continue; } $legacy_messages = $diff->getProperty('arc:unit'); if (!$legacy_messages) { continue; } // Show the top 100 legacy lint messages. Previously, we showed some // by default and let the user toggle the rest. With modern messages, // we can send the user to the Harbormaster detail page. Just show // "a lot" of messages in legacy cases to try to strike a balance // between implementation simplicitly and compatibility. $legacy_messages = array_slice($legacy_messages, 0, 100); $messages = array(); foreach ($legacy_messages as $message) { $messages[] = HarbormasterBuildUnitMessage::newFromDictionary( new HarbormasterBuildTarget(), $this->getModernUnitMessageDictionary($message)); } $diff->attachUnitMessages($messages); } } private function getModernUnitMessageDictionary(array $map) { // Strip out `null` values to satisfy stricter typechecks. foreach ($map as $key => $value) { if ($value === null) { unset($map[$key]); } } // Cast duration to a float since it used to be a string in some // cases. if (isset($map['duration'])) { $map['duration'] = (double)$map['duration']; } return $map; } protected function getDiffTabLabels(array $diffs) { // Make sure we're only going to render unique diffs. $diffs = mpull($diffs, null, 'getID'); $labels = array(pht('Left'), pht('Right')); $results = array(); foreach ($diffs as $diff) { if (count($diffs) == 2) { $label = array_shift($labels); $label = pht('%s (Diff %d)', $label, $diff->getID()); } else { $label = pht('Diff %d', $diff->getID()); } $results[] = array( $label, $diff, ); } return $results; } } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 97ad60fc12..bf7faa2700 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1,1862 +1,1860 @@ getTransactionType() == $type_update) { return $xaction; } } return null; } public function setIsCloseByCommit($is_close_by_commit) { $this->isCloseByCommit = $is_close_by_commit; return $this; } public function getIsCloseByCommit() { return $this->isCloseByCommit; } public function setChangedPriorToCommitURI($uri) { $this->changedPriorToCommitURI = $uri; return $this; } public function getChangedPriorToCommitURI() { return $this->changedPriorToCommitURI; } public function setRepositoryPHIDOverride($phid_or_null) { $this->repositoryPHIDOverride = $phid_or_null; return $this; } public function getTransactionTypes() { $types = parent::getTransactionTypes(); $types[] = PhabricatorTransactions::TYPE_COMMENT; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorTransactions::TYPE_INLINESTATE; $types[] = DifferentialTransaction::TYPE_ACTION; $types[] = DifferentialTransaction::TYPE_INLINE; $types[] = DifferentialTransaction::TYPE_STATUS; $types[] = DifferentialTransaction::TYPE_UPDATE; return $types; } protected function getCustomTransactionOldValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_ACTION: return null; case DifferentialTransaction::TYPE_INLINE: return null; case DifferentialTransaction::TYPE_UPDATE: if ($this->getIsNewObject()) { return null; } else { return $object->getActiveDiff()->getPHID(); } } return parent::getCustomTransactionOldValue($object, $xaction); } protected function getCustomTransactionNewValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_ACTION: case DifferentialTransaction::TYPE_UPDATE: return $xaction->getNewValue(); case DifferentialTransaction::TYPE_INLINE: return null; } return parent::getCustomTransactionNewValue($object, $xaction); } protected function transactionHasEffect( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $actor_phid = $this->getActingAsPHID(); switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_INLINE: return $xaction->hasComment(); case DifferentialTransaction::TYPE_ACTION: $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; $action_type = $xaction->getNewValue(); switch ($action_type) { case DifferentialAction::ACTION_CLOSE: return ($object->getStatus() != $status_closed); case DifferentialAction::ACTION_ABANDON: return ($object->getStatus() != $status_abandoned); case DifferentialAction::ACTION_RECLAIM: return ($object->getStatus() == $status_abandoned); case DifferentialAction::ACTION_REOPEN: return ($object->getStatus() == $status_closed); case DifferentialAction::ACTION_RETHINK: return ($object->getStatus() != $status_plan); case DifferentialAction::ACTION_CLAIM: return ($actor_phid != $object->getAuthorPHID()); } } return parent::transactionHasEffect($object, $xaction); } protected function applyCustomInternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_INLINE: return; case DifferentialTransaction::TYPE_UPDATE: if (!$this->getIsCloseByCommit()) { switch ($object->getStatus()) { case $status_revision: case $status_plan: case $status_abandoned: $object->setStatus($status_review); break; } } $diff = $this->requireDiff($xaction->getNewValue()); $object->setLineCount($diff->getLineCount()); if ($this->repositoryPHIDOverride !== false) { $object->setRepositoryPHID($this->repositoryPHIDOverride); } else { $object->setRepositoryPHID($diff->getRepositoryPHID()); } $object->attachActiveDiff($diff); // TODO: Update the `diffPHID` once we add that. return; case DifferentialTransaction::TYPE_ACTION: switch ($xaction->getNewValue()) { case DifferentialAction::ACTION_ABANDON: $object->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED); return; case DifferentialAction::ACTION_RETHINK: $object->setStatus($status_plan); return; case DifferentialAction::ACTION_RECLAIM: $object->setStatus($status_review); return; case DifferentialAction::ACTION_REOPEN: $object->setStatus($status_review); return; case DifferentialAction::ACTION_CLOSE: $old_status = $object->getStatus(); $object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED); $was_accepted = ($old_status == $status_accepted); $object->setProperty( DifferentialRevision::PROPERTY_CLOSED_FROM_ACCEPTED, $was_accepted); return; case DifferentialAction::ACTION_CLAIM: $object->setAuthorPHID($this->getActingAsPHID()); return; default: throw new Exception( pht( 'Differential action "%s" is not a valid action!', $xaction->getNewValue())); } break; } return parent::applyCustomInternalTransaction($object, $xaction); } protected function expandTransactions( PhabricatorLiskDAO $object, array $xactions) { foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_INLINESTATE: // If we have an "Inline State" transaction already, the caller // built it for us so we don't need to expand it again. $this->didExpandInlineState = true; break; case DifferentialRevisionAcceptTransaction::TRANSACTIONTYPE: case DifferentialRevisionRejectTransaction::TRANSACTIONTYPE: case DifferentialRevisionResignTransaction::TRANSACTIONTYPE: // If we have a review transaction, we'll skip marking the user // as "Commented" later. This should get cleaner after T10967. $this->hasReviewTransaction = true; break; } } return parent::expandTransactions($object, $xactions); } protected function expandTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $results = parent::expandTransaction($object, $xaction); $actor = $this->getActor(); $actor_phid = $this->getActingAsPHID(); $type_edge = PhabricatorTransactions::TYPE_EDGE; $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; $edge_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST; $edge_ref_task = DifferentialRevisionHasTaskEdgeType::EDGECONST; $is_sticky_accept = PhabricatorEnv::getEnvConfig( 'differential.sticky-accept'); $downgrade_rejects = false; $downgrade_accepts = false; if ($this->getIsCloseByCommit()) { // Never downgrade reviewers when we're closing a revision after a // commit. } else { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $downgrade_rejects = true; if (!$is_sticky_accept) { // If "sticky accept" is disabled, also downgrade the accepts. $downgrade_accepts = true; } break; case DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE: $downgrade_rejects = true; if ((!$is_sticky_accept) || ($object->getStatus() != $status_plan)) { // If the old state isn't "changes planned", downgrade the accepts. // This exception allows an accepted revision to go through // "Plan Changes" -> "Request Review" and return to "accepted" if // the author didn't update the revision, essentially undoing the // "Plan Changes". $downgrade_accepts = true; } break; } } $new_accept = DifferentialReviewerStatus::STATUS_ACCEPTED; $new_reject = DifferentialReviewerStatus::STATUS_REJECTED; $old_accept = DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER; $old_reject = DifferentialReviewerStatus::STATUS_REJECTED_OLDER; if ($downgrade_rejects || $downgrade_accepts) { // When a revision is updated, change all "reject" to "rejected older // revision". This means we won't immediately push the update back into // "needs review", but outstanding rejects will still block it from // moving to "accepted". // We also do this for "Request Review", even though the diff is not // updated directly. Essentially, this acts like an update which doesn't // actually change the diff text. $edits = array(); foreach ($object->getReviewers() as $reviewer) { if ($downgrade_rejects) { if ($reviewer->getReviewerStatus() == $new_reject) { $edits[$reviewer->getReviewerPHID()] = array( 'data' => array( 'status' => $old_reject, ), ); } } if ($downgrade_accepts) { if ($reviewer->getReviewerStatus() == $new_accept) { $edits[$reviewer->getReviewerPHID()] = array( 'data' => array( 'status' => $old_accept, ), ); } } } if ($edits) { $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => $edits)); } } if ($downgrade_accepts || $downgrade_rejects) { $void_type = DifferentialRevisionVoidTransaction::TRANSACTIONTYPE; $results[] = id(new DifferentialTransaction()) ->setTransactionType($void_type) ->setIgnoreOnNoEffect(true) ->setNewValue(true); } $is_commandeer = false; switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: if ($this->getIsCloseByCommit()) { // Don't bother with any of this if this update is a side effect of // commit detection. break; } // When a revision is updated and the diff comes from a branch named // "T123" or similar, automatically associate the commit with the // task that the branch names. $maniphest = 'PhabricatorManiphestApplication'; if (PhabricatorApplication::isClassInstalled($maniphest)) { $diff = $this->requireDiff($xaction->getNewValue()); $branch = $diff->getBranch(); // No "$", to allow for branches like T123_demo. $match = null; if (preg_match('/^T(\d+)/i', $branch, $match)) { $task_id = $match[1]; $tasks = id(new ManiphestTaskQuery()) ->setViewer($this->getActor()) ->withIDs(array($task_id)) ->execute(); if ($tasks) { $task = head($tasks); $task_phid = $task->getPHID(); $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_ref_task) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => array($task_phid => $task_phid))); } } } break; case PhabricatorTransactions::TYPE_COMMENT: // When a user leaves a comment, upgrade their reviewer status from // "added" to "commented" if they're also a reviewer. We may further // upgrade this based on other actions in the transaction group. if ($this->hasReviewTransaction) { // If we're also applying a review transaction, skip this. break; } $status_added = DifferentialReviewerStatus::STATUS_ADDED; $status_commented = DifferentialReviewerStatus::STATUS_COMMENTED; $data = array( 'status' => $status_commented, ); $edits = array(); foreach ($object->getReviewers() as $reviewer) { if ($reviewer->getReviewerPHID() == $actor_phid) { if ($reviewer->getReviewerStatus() == $status_added) { $edits[$actor_phid] = array( 'data' => $data, ); } } } if ($edits) { $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => $edits)); } break; case DifferentialRevisionCommandeerTransaction::TRANSACTIONTYPE: $is_commandeer = true; break; case DifferentialTransaction::TYPE_ACTION: $action_type = $xaction->getNewValue(); switch ($action_type) { case DifferentialAction::ACTION_CLAIM: $is_commandeer = true; break; } break; } if ($is_commandeer) { $results[] = $this->newCommandeerReviewerTransaction($object); } if (!$this->didExpandInlineState) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_COMMENT: case DifferentialTransaction::TYPE_ACTION: case DifferentialTransaction::TYPE_UPDATE: case DifferentialTransaction::TYPE_INLINE: $this->didExpandInlineState = true; $actor_phid = $this->getActingAsPHID(); $actor_is_author = ($object->getAuthorPHID() == $actor_phid); if (!$actor_is_author) { break; } $state_map = PhabricatorTransactions::getInlineStateMap(); $inlines = id(new DifferentialDiffInlineCommentQuery()) ->setViewer($this->getActor()) ->withRevisionPHIDs(array($object->getPHID())) ->withFixedStates(array_keys($state_map)) ->execute(); if (!$inlines) { break; } $old_value = mpull($inlines, 'getFixedState', 'getPHID'); $new_value = array(); foreach ($old_value as $key => $state) { $new_value[$key] = $state_map[$state]; } $results[] = id(new DifferentialTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) ->setIgnoreOnNoEffect(true) ->setOldValue($old_value) ->setNewValue($new_value); break; } } return $results; } protected function applyCustomExternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_ACTION: return; case DifferentialTransaction::TYPE_INLINE: $reply = $xaction->getComment()->getReplyToComment(); if ($reply && !$reply->getHasReplies()) { $reply->setHasReplies(1)->save(); } return; case DifferentialTransaction::TYPE_UPDATE: // Now that we're inside the transaction, do a final check. $diff = $this->requireDiff($xaction->getNewValue()); // TODO: It would be slightly cleaner to just revalidate this // transaction somehow using the same validation code, but that's // not easy to do at the moment. $revision_id = $diff->getRevisionID(); if ($revision_id && ($revision_id != $object->getID())) { throw new Exception( pht( 'Diff is already attached to another revision. You lost '. 'a race?')); } // TODO: This can race with diff updates, particularly those from // Harbormaster. See discussion in T8650. $diff->setRevisionID($object->getID()); $diff->save(); // Update Harbormaster to set the containerPHID correctly for any // existing buildables. We may otherwise have buildables stuck with // the old (`null`) container. // TODO: This is a bit iffy, maybe we can find a cleaner approach? // In particular, this could (rarely) be overwritten by Harbormaster // workers. $table = new HarbormasterBuildable(); $conn_w = $table->establishConnection('w'); queryfx( $conn_w, 'UPDATE %T SET containerPHID = %s WHERE buildablePHID = %s', $table->getTableName(), $object->getPHID(), $diff->getPHID()); return; } return parent::applyCustomExternalTransaction($object, $xaction); } protected function applyBuiltinExternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_INLINESTATE: $table = new DifferentialTransactionComment(); $conn_w = $table->establishConnection('w'); foreach ($xaction->getNewValue() as $phid => $state) { queryfx( $conn_w, 'UPDATE %T SET fixedState = %s WHERE phid = %s', $table->getTableName(), $state, $phid); } break; } return parent::applyBuiltinExternalTransaction($object, $xaction); } protected function mergeEdgeData($type, array $u, array $v) { $result = parent::mergeEdgeData($type, $u, $v); switch ($type) { case DifferentialRevisionHasReviewerEdgeType::EDGECONST: // When the same reviewer has their status updated by multiple // transactions, we want the strongest status to win. An example of // this is when a user adds a comment and also accepts a revision which // they are a reviewer on. The comment creates a "commented" status, // while the accept creates an "accepted" status. Since accept is // stronger, it should win and persist. $u_status = idx($u, 'status'); $v_status = idx($v, 'status'); $u_str = DifferentialReviewerStatus::getStatusStrength($u_status); $v_str = DifferentialReviewerStatus::getStatusStrength($v_status); if ($u_str > $v_str) { $result['status'] = $u_status; } else { $result['status'] = $v_status; } break; } return $result; } protected function applyFinalEffects( PhabricatorLiskDAO $object, array $xactions) { // Load the most up-to-date version of the revision and its reviewers, // so we don't need to try to deduce the state of reviewers by examining // all the changes made by the transactions. Then, update the reviewers // on the object to make sure we're acting on the current reviewer set // (and, for example, sending mail to the right people). $new_revision = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->needReviewers(true) ->needActiveDiffs(true) ->withIDs(array($object->getID())) ->executeOne(); if (!$new_revision) { throw new Exception( pht('Failed to load revision from transaction finalization.')); } $object->attachReviewers($new_revision->getReviewers()); $object->attachActiveDiff($new_revision->getActiveDiff()); $object->attachRepository($new_revision->getRepository()); foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $diff = $this->requireDiff($xaction->getNewValue(), true); // Update these denormalized index tables when we attach a new // diff to a revision. $this->updateRevisionHashTable($object, $diff); $this->updateAffectedPathTable($object, $diff); break; } } $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; $status_revision = ArcanistDifferentialRevisionStatus::NEEDS_REVISION; $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; $is_sticky_accept = PhabricatorEnv::getEnvConfig( 'differential.sticky-accept'); $old_status = $object->getStatus(); $active_diff = $object->getActiveDiff(); switch ($old_status) { case $status_accepted: case $status_revision: case $status_review: // Try to move a revision to "accepted". We look for: // // - at least one accepting reviewer who is a user; and // - no rejects; and // - no rejects of older diffs; and // - no blocking reviewers. $has_accepting_user = false; $has_rejecting_reviewer = false; $has_rejecting_older_reviewer = false; $has_blocking_reviewer = false; foreach ($object->getReviewers() as $reviewer) { $reviewer_status = $reviewer->getReviewerStatus(); switch ($reviewer_status) { case DifferentialReviewerStatus::STATUS_REJECTED: $active_phid = $active_diff->getPHID(); if ($reviewer->isRejected($active_phid)) { $has_rejecting_reviewer = true; } break; case DifferentialReviewerStatus::STATUS_REJECTED_OLDER: $has_rejecting_older_reviewer = true; break; case DifferentialReviewerStatus::STATUS_BLOCKING: $has_blocking_reviewer = true; break; case DifferentialReviewerStatus::STATUS_ACCEPTED: if ($reviewer->isUser()) { $active_phid = $active_diff->getPHID(); if ($reviewer->isAccepted($active_phid)) { $has_accepting_user = true; } } break; } } $new_status = null; if ($has_accepting_user && !$has_rejecting_reviewer && !$has_rejecting_older_reviewer && !$has_blocking_reviewer) { $new_status = $status_accepted; } else if ($has_rejecting_reviewer) { // This isn't accepted, and there's at least one rejecting reviewer, // so the revision needs changes. This usually happens after a // "reject". $new_status = $status_revision; } else if ($old_status == $status_accepted) { // This revision was accepted, but it no longer satisfies the // conditions for acceptance. This usually happens after an accepting // reviewer resigns or is removed. $new_status = $status_review; } if ($new_status !== null && ($new_status != $old_status)) { $xaction = id(new DifferentialTransaction()) ->setTransactionType(DifferentialTransaction::TYPE_STATUS) ->setOldValue($old_status) ->setNewValue($new_status); $xaction = $this->populateTransaction($object, $xaction)->save(); $xactions[] = $xaction; $object->setStatus($new_status)->save(); } break; default: // Revisions can't transition out of other statuses (like closed or // abandoned) as a side effect of reviewer status changes. break; } $this->markReviewerComments($object, $xactions); return $xactions; } protected function validateTransaction( PhabricatorLiskDAO $object, $type, array $xactions) { $errors = parent::validateTransaction($object, $type, $xactions); $config_self_accept_key = 'differential.allow-self-accept'; $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key); foreach ($xactions as $xaction) { switch ($type) { case PhabricatorTransactions::TYPE_EDGE: switch ($xaction->getMetadataValue('edge:type')) { case DifferentialRevisionHasReviewerEdgeType::EDGECONST: // Prevent the author from becoming a reviewer. // NOTE: This is pretty gross, but this restriction is unusual. // If we end up with too much more of this, we should try to clean // this up -- maybe by moving validation to after transactions // are adjusted (so we can just examine the final value) or adding // a second phase there? $author_phid = $object->getAuthorPHID(); $new = $xaction->getNewValue(); $add = idx($new, '+', array()); $eq = idx($new, '=', array()); $phids = array_keys($add + $eq); foreach ($phids as $phid) { if (($phid == $author_phid) && !$allow_self_accept && !$xaction->getIsCommandeerSideEffect()) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), pht('The author of a revision can not be a reviewer.'), $xaction); } } break; } break; case DifferentialTransaction::TYPE_UPDATE: $diff = $this->loadDiff($xaction->getNewValue()); if (!$diff) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), pht('The specified diff does not exist.'), $xaction); } else if (($diff->getRevisionID()) && ($diff->getRevisionID() != $object->getID())) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), pht( 'You can not update this revision to the specified diff, '. 'because the diff is already attached to another revision.'), $xaction); } break; case DifferentialTransaction::TYPE_ACTION: $error = $this->validateDifferentialAction( $object, $type, $xaction, $xaction->getNewValue()); if ($error) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), $error, $xaction); } break; } } return $errors; } private function validateDifferentialAction( DifferentialRevision $revision, $type, DifferentialTransaction $xaction, $action) { $author_phid = $revision->getAuthorPHID(); $actor_phid = $this->getActingAsPHID(); $actor_is_author = ($author_phid == $actor_phid); $config_abandon_key = 'differential.always-allow-abandon'; $always_allow_abandon = PhabricatorEnv::getEnvConfig($config_abandon_key); $config_close_key = 'differential.always-allow-close'; $always_allow_close = PhabricatorEnv::getEnvConfig($config_close_key); $config_reopen_key = 'differential.allow-reopen'; $allow_reopen = PhabricatorEnv::getEnvConfig($config_reopen_key); $config_self_accept_key = 'differential.allow-self-accept'; $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key); $revision_status = $revision->getStatus(); $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; switch ($action) { case DifferentialAction::ACTION_CLAIM: // You can claim a revision if you're not the owner. If you are, this // is a no-op rather than invalid. if ($revision_status == $status_closed) { return pht( 'You can not commandeer this revision because it has already been '. 'closed.'); } break; case DifferentialAction::ACTION_ABANDON: if (!$actor_is_author && !$always_allow_abandon) { return pht( 'You can not abandon this revision because you do not own it. '. 'You can only abandon revisions you own.'); } if ($revision_status == $status_closed) { return pht( 'You can not abandon this revision because it has already been '. 'closed.'); } // NOTE: Abandons of already-abandoned revisions are treated as no-op // instead of invalid. Other abandons are OK. break; case DifferentialAction::ACTION_RECLAIM: if (!$actor_is_author) { return pht( 'You can not reclaim this revision because you do not own '. 'it. You can only reclaim revisions you own.'); } if ($revision_status == $status_closed) { return pht( 'You can not reclaim this revision because it has already been '. 'closed.'); } // NOTE: Reclaims of other non-abandoned revisions are treated as no-op // instead of invalid. break; case DifferentialAction::ACTION_REOPEN: if (!$allow_reopen) { return pht( 'The reopen action is not enabled on this Phabricator install. '. 'Adjust your configuration to enable it.'); } // NOTE: If the revision is not closed, this is caught as a no-op // instead of an invalid transaction. break; case DifferentialAction::ACTION_RETHINK: if (!$actor_is_author) { return pht( 'You can not plan changes to this revision because you do not '. 'own it. To plan changes to a revision, you must be its owner.'); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: // These are OK. break; case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: // Let this through, it's a no-op. break; case ArcanistDifferentialRevisionStatus::ABANDONED: return pht( 'You can not plan changes to this revision because it has '. 'been abandoned.'); case ArcanistDifferentialRevisionStatus::CLOSED: return pht( 'You can not plan changes to this revision because it has '. 'already been closed.'); default: throw new Exception( pht( 'Encountered unexpected revision status ("%s") when '. 'validating "%s" action.', $revision_status, $action)); } break; case DifferentialAction::ACTION_CLOSE: // We force revisions closed when we discover a corresponding commit. // In this case, revisions are allowed to transition to closed from // any state. This is an automated action taken by the daemons. if (!$this->getIsCloseByCommit()) { if (!$actor_is_author && !$always_allow_close) { return pht( 'You can not close this revision because you do not own it. To '. 'close a revision, you must be its owner.'); } if ($revision_status != $status_accepted) { return pht( 'You can not close this revision because it has not been '. 'accepted. You can only close accepted revisions.'); } } break; } return null; } protected function sortTransactions(array $xactions) { $xactions = parent::sortTransactions($xactions); $head = array(); $tail = array(); foreach ($xactions as $xaction) { $type = $xaction->getTransactionType(); if ($type == DifferentialTransaction::TYPE_INLINE) { $tail[] = $xaction; } else { $head[] = $xaction; } } return array_values(array_merge($head, $tail)); } protected function requireCapabilities( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) {} return parent::requireCapabilities($object, $xaction); } protected function shouldPublishFeedStory( PhabricatorLiskDAO $object, array $xactions) { return true; } protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { return true; } protected function getMailTo(PhabricatorLiskDAO $object) { $phids = array(); $phids[] = $object->getAuthorPHID(); foreach ($object->getReviewers() as $reviewer) { $phids[] = $reviewer->getReviewerPHID(); } return $phids; } protected function getMailAction( PhabricatorLiskDAO $object, array $xactions) { $action = parent::getMailAction($object, $xactions); $strongest = $this->getStrongestAction($object, $xactions); switch ($strongest->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $count = new PhutilNumber($object->getLineCount()); $action = pht('%s, %s line(s)', $action, $count); break; } return $action; } protected function getMailSubjectPrefix() { return PhabricatorEnv::getEnvConfig('metamta.differential.subject-prefix'); } protected function getMailThreadID(PhabricatorLiskDAO $object) { // This is nonstandard, but retains threading with older messages. $phid = $object->getPHID(); return "differential-rev-{$phid}-req"; } protected function buildReplyHandler(PhabricatorLiskDAO $object) { return id(new DifferentialReplyHandler()) ->setMailReceiver($object); } protected function buildMailTemplate(PhabricatorLiskDAO $object) { $id = $object->getID(); $title = $object->getTitle(); $original_title = $object->getOriginalTitle(); $subject = "D{$id}: {$title}"; $thread_topic = "D{$id}: {$original_title}"; return id(new PhabricatorMetaMTAMail()) ->setSubject($subject) ->addHeader('Thread-Topic', $thread_topic); } protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { $body = new PhabricatorMetaMTAMailBody(); $body->setViewer($this->requireActor()); $revision_uri = PhabricatorEnv::getProductionURI('/D'.$object->getID()); $this->addHeadersAndCommentsToMailBody( $body, $xactions, pht('View Revision'), $revision_uri); $type_inline = DifferentialTransaction::TYPE_INLINE; $inlines = array(); foreach ($xactions as $xaction) { if ($xaction->getTransactionType() == $type_inline) { $inlines[] = $xaction; } } if ($inlines) { $this->appendInlineCommentsForMail($object, $inlines, $body); } $changed_uri = $this->getChangedPriorToCommitURI(); if ($changed_uri) { $body->addLinkSection( pht('CHANGED PRIOR TO COMMIT'), $changed_uri); } $this->addCustomFieldsToMailBody($body, $object, $xactions); $body->addLinkSection( pht('REVISION DETAIL'), $revision_uri); $update_xaction = null; foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $update_xaction = $xaction; break; } } if ($update_xaction) { $diff = $this->requireDiff($update_xaction->getNewValue(), true); $body->addTextSection( pht('AFFECTED FILES'), $this->renderAffectedFilesForMail($diff)); $config_key_inline = 'metamta.differential.inline-patches'; $config_inline = PhabricatorEnv::getEnvConfig($config_key_inline); $config_key_attach = 'metamta.differential.attach-patches'; $config_attach = PhabricatorEnv::getEnvConfig($config_key_attach); if ($config_inline || $config_attach) { $body_limit = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); $patch = $this->buildPatchForMail($diff); if ($config_inline) { $lines = substr_count($patch, "\n"); $bytes = strlen($patch); // Limit the patch size to the smaller of 256 bytes per line or // the mail body limit. This prevents degenerate behavior for patches // with one line that is 10MB long. See T11748. $byte_limits = array(); $byte_limits[] = (256 * $config_inline); $byte_limits[] = $body_limit; $byte_limit = min($byte_limits); $lines_ok = ($lines <= $config_inline); $bytes_ok = ($bytes <= $byte_limit); if ($lines_ok && $bytes_ok) { $this->appendChangeDetailsForMail($object, $diff, $patch, $body); } else { // TODO: Provide a helpful message about the patch being too // large or lengthy here. } } if ($config_attach) { $name = pht('D%s.%s.patch', $object->getID(), $diff->getID()); $mime_type = 'text/x-patch; charset=utf-8'; $body->addAttachment( new PhabricatorMetaMTAAttachment($patch, $name, $mime_type)); } } } return $body; } public function getMailTagsMap() { return array( DifferentialTransaction::MAILTAG_REVIEW_REQUEST => pht('A revision is created.'), DifferentialTransaction::MAILTAG_UPDATED => pht('A revision is updated.'), DifferentialTransaction::MAILTAG_COMMENT => pht('Someone comments on a revision.'), DifferentialTransaction::MAILTAG_CLOSED => pht('A revision is closed.'), DifferentialTransaction::MAILTAG_REVIEWERS => pht("A revision's reviewers change."), DifferentialTransaction::MAILTAG_CC => pht("A revision's CCs change."), DifferentialTransaction::MAILTAG_OTHER => pht('Other revision activity not listed above occurs.'), ); } protected function supportsSearch() { return true; } protected function expandCustomRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, array $changes, PhutilMarkupEngine $engine) { $flat_blocks = mpull($changes, 'getNewValue'); $huge_block = implode("\n\n", $flat_blocks); $task_map = array(); $task_refs = id(new ManiphestCustomFieldStatusParser()) ->parseCorpus($huge_block); foreach ($task_refs as $match) { foreach ($match['monograms'] as $monogram) { $task_id = (int)trim($monogram, 'tT'); $task_map[$task_id] = true; } } $rev_map = array(); $rev_refs = id(new DifferentialCustomFieldDependsOnParser()) ->parseCorpus($huge_block); foreach ($rev_refs as $match) { foreach ($match['monograms'] as $monogram) { $rev_id = (int)trim($monogram, 'dD'); $rev_map[$rev_id] = true; } } $edges = array(); $task_phids = array(); $rev_phids = array(); if ($task_map) { $tasks = id(new ManiphestTaskQuery()) ->setViewer($this->getActor()) ->withIDs(array_keys($task_map)) ->execute(); if ($tasks) { $task_phids = mpull($tasks, 'getPHID', 'getPHID'); $edge_related = DifferentialRevisionHasTaskEdgeType::EDGECONST; $edges[$edge_related] = $task_phids; } } if ($rev_map) { $revs = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->withIDs(array_keys($rev_map)) ->execute(); $rev_phids = mpull($revs, 'getPHID', 'getPHID'); // NOTE: Skip any write attempts if a user cleverly implies a revision // depends upon itself. unset($rev_phids[$object->getPHID()]); if ($revs) { $depends = DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST; $edges[$depends] = $rev_phids; } } $this->setUnmentionablePHIDMap(array_merge($task_phids, $rev_phids)); $result = array(); foreach ($edges as $type => $specs) { $result[] = id(new DifferentialTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) ->setMetadataValue('edge:type', $type) ->setNewValue(array('+' => $specs)); } return $result; } private function appendInlineCommentsForMail( PhabricatorLiskDAO $object, array $inlines, PhabricatorMetaMTAMailBody $body) { $section = id(new DifferentialInlineCommentMailView()) ->setViewer($this->getActor()) ->setInlines($inlines) ->buildMailSection(); $header = pht('INLINE COMMENTS'); $section_text = "\n".$section->getPlaintext(); $style = array( 'margin: 6px 0 12px 0;', ); $section_html = phutil_tag( 'div', array( 'style' => implode(' ', $style), ), $section->getHTML()); $body->addPlaintextSection($header, $section_text, false); $body->addHTMLSection($header, $section_html); } private function appendChangeDetailsForMail( PhabricatorLiskDAO $object, DifferentialDiff $diff, $patch, PhabricatorMetaMTAMailBody $body) { $section = id(new DifferentialChangeDetailMailView()) ->setViewer($this->getActor()) ->setDiff($diff) ->setPatch($patch) ->buildMailSection(); $header = pht('CHANGE DETAILS'); $section_text = "\n".$section->getPlaintext(); $style = array( 'margin: 6px 0 12px 0;', ); $section_html = phutil_tag( 'div', array( 'style' => implode(' ', $style), ), $section->getHTML()); $body->addPlaintextSection($header, $section_text, false); $body->addHTMLSection($header, $section_html); } private function loadDiff($phid, $need_changesets = false) { $query = id(new DifferentialDiffQuery()) ->withPHIDs(array($phid)) ->setViewer($this->getActor()); if ($need_changesets) { $query->needChangesets(true); } return $query->executeOne(); } private function requireDiff($phid, $need_changesets = false) { $diff = $this->loadDiff($phid, $need_changesets); if (!$diff) { throw new Exception(pht('Diff "%s" does not exist!', $phid)); } return $diff; } /* -( Herald Integration )------------------------------------------------- */ protected function shouldApplyHeraldRules( PhabricatorLiskDAO $object, array $xactions) { if ($this->getIsNewObject()) { return true; } foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: if (!$this->getIsCloseByCommit()) { return true; } break; case DifferentialRevisionCommandeerTransaction::TRANSACTIONTYPE: // When users commandeer revisions, we may need to trigger // signatures or author-based rules. return true; case DifferentialTransaction::TYPE_ACTION: switch ($xaction->getNewValue()) { case DifferentialAction::ACTION_CLAIM: // When users commandeer revisions, we may need to trigger // signatures or author-based rules. return true; } break; } } return parent::shouldApplyHeraldRules($object, $xactions); } protected function didApplyHeraldRules( PhabricatorLiskDAO $object, HeraldAdapter $adapter, HeraldTranscript $transcript) { $repository = $object->getRepository(); if (!$repository) { return array(); } if (!$this->affectedPaths) { return array(); } $packages = PhabricatorOwnersPackage::loadAffectedPackages( $repository, $this->affectedPaths); if (!$packages) { return array(); } // Remove packages that the revision author is an owner of. If you own // code, you don't need another owner to review it. $authority = id(new PhabricatorOwnersPackageQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withPHIDs(mpull($packages, 'getPHID')) ->withAuthorityPHIDs(array($object->getAuthorPHID())) ->execute(); $authority = mpull($authority, null, 'getPHID'); foreach ($packages as $key => $package) { $package_phid = $package->getPHID(); if (isset($authority[$package_phid])) { unset($packages[$key]); continue; } } if (!$packages) { return array(); } $auto_subscribe = array(); $auto_review = array(); $auto_block = array(); foreach ($packages as $package) { switch ($package->getAutoReview()) { case PhabricatorOwnersPackage::AUTOREVIEW_SUBSCRIBE: $auto_subscribe[] = $package; break; case PhabricatorOwnersPackage::AUTOREVIEW_REVIEW: $auto_review[] = $package; break; case PhabricatorOwnersPackage::AUTOREVIEW_BLOCK: $auto_block[] = $package; break; case PhabricatorOwnersPackage::AUTOREVIEW_NONE: default: break; } } $owners_phid = id(new PhabricatorOwnersApplication()) ->getPHID(); $xactions = array(); if ($auto_subscribe) { $xactions[] = $object->getApplicationTransactionTemplate() ->setAuthorPHID($owners_phid) ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) ->setNewValue( array( '+' => mpull($auto_subscribe, 'getPHID'), )); } $specs = array( array($auto_review, false), array($auto_block, true), ); foreach ($specs as $spec) { list($reviewers, $blocking) = $spec; if (!$reviewers) { continue; } $phids = mpull($reviewers, 'getPHID'); $xaction = $this->newAutoReviewTransaction($object, $phids, $blocking); if ($xaction) { $xactions[] = $xaction; } } return $xactions; } private function newAutoReviewTransaction( PhabricatorLiskDAO $object, array $phids, $is_blocking) { // TODO: This is substantially similar to DifferentialReviewersHeraldAction // and both are needlessly complex. This logic should live in the normal // transaction application pipeline. See T10967. $reviewers = $object->getReviewers(); $reviewers = mpull($reviewers, null, 'getReviewerPHID'); if ($is_blocking) { $new_status = DifferentialReviewerStatus::STATUS_BLOCKING; } else { $new_status = DifferentialReviewerStatus::STATUS_ADDED; } $new_strength = DifferentialReviewerStatus::getStatusStrength( $new_status); $current = array(); foreach ($phids as $phid) { if (!isset($reviewers[$phid])) { continue; } // If we're applying a stronger status (usually, upgrading a reviewer // into a blocking reviewer), skip this check so we apply the change. $old_strength = DifferentialReviewerStatus::getStatusStrength( $reviewers[$phid]->getReviewerStatus()); if ($old_strength <= $new_strength) { continue; } $current[] = $phid; } $phids = array_diff($phids, $current); if (!$phids) { return null; } $phids = array_fuse($phids); $value = array(); foreach ($phids as $phid) { if ($is_blocking) { $value[] = 'blocking('.$phid.')'; } else { $value[] = $phid; } } $owners_phid = id(new PhabricatorOwnersApplication()) ->getPHID(); $reviewers_type = DifferentialRevisionReviewersTransaction::TRANSACTIONTYPE; return $object->getApplicationTransactionTemplate() ->setAuthorPHID($owners_phid) ->setTransactionType($reviewers_type) ->setNewValue( array( '+' => $value, )); } protected function buildHeraldAdapter( PhabricatorLiskDAO $object, array $xactions) { $revision = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->withPHIDs(array($object->getPHID())) ->needActiveDiffs(true) ->needReviewers(true) ->executeOne(); if (!$revision) { throw new Exception( pht('Failed to load revision for Herald adapter construction!')); } $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( $revision, $revision->getActiveDiff()); return $adapter; } /** * Update the table which links Differential revisions to paths they affect, * so Diffusion can efficiently find pending revisions for a given file. */ private function updateAffectedPathTable( DifferentialRevision $revision, DifferentialDiff $diff) { $repository = $revision->getRepository(); if (!$repository) { // The repository where the code lives is untracked. return; } $path_prefix = null; $local_root = $diff->getSourceControlPath(); if ($local_root) { // We're in a working copy which supports subdirectory checkouts (e.g., // SVN) so we need to figure out what prefix we should add to each path // (e.g., trunk/projects/example/) to get the absolute path from the // root of the repository. DVCS systems like Git and Mercurial are not // affected. // Normalize both paths and check if the repository root is a prefix of // the local root. If so, throw it away. Note that this correctly handles // the case where the remote path is "/". $local_root = id(new PhutilURI($local_root))->getPath(); $local_root = rtrim($local_root, '/'); $repo_root = id(new PhutilURI($repository->getRemoteURI()))->getPath(); $repo_root = rtrim($repo_root, '/'); if (!strncmp($repo_root, $local_root, strlen($repo_root))) { $path_prefix = substr($local_root, strlen($repo_root)); } } $changesets = $diff->getChangesets(); $paths = array(); foreach ($changesets as $changeset) { $paths[] = $path_prefix.'/'.$changeset->getFilename(); } // Save the affected paths; we'll use them later to query Owners. This // uses the un-expanded paths. $this->affectedPaths = $paths; // Mark this as also touching all parent paths, so you can see all pending // changes to any file within a directory. $all_paths = array(); foreach ($paths as $local) { foreach (DiffusionPathIDQuery::expandPathToRoot($local) as $path) { $all_paths[$path] = true; } } $all_paths = array_keys($all_paths); $path_ids = PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths( $all_paths); $table = new DifferentialAffectedPath(); $conn_w = $table->establishConnection('w'); $sql = array(); foreach ($path_ids as $path_id) { $sql[] = qsprintf( $conn_w, '(%d, %d, %d, %d)', $repository->getID(), $path_id, time(), $revision->getID()); } queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', $table->getTableName(), $revision->getID()); foreach (array_chunk($sql, 256) as $chunk) { queryfx( $conn_w, 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q', $table->getTableName(), implode(', ', $chunk)); } } /** * Update the table connecting revisions to DVCS local hashes, so we can * identify revisions by commit/tree hashes. */ private function updateRevisionHashTable( DifferentialRevision $revision, DifferentialDiff $diff) { $vcs = $diff->getSourceControlSystem(); if ($vcs == DifferentialRevisionControlSystem::SVN) { // Subversion has no local commit or tree hash information, so we don't // have to do anything. return; } $property = id(new DifferentialDiffProperty())->loadOneWhere( 'diffID = %d AND name = %s', $diff->getID(), 'local:commits'); if (!$property) { return; } $hashes = array(); $data = $property->getData(); switch ($vcs) { case DifferentialRevisionControlSystem::GIT: foreach ($data as $commit) { $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT, $commit['commit'], ); $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_GIT_TREE, $commit['tree'], ); } break; case DifferentialRevisionControlSystem::MERCURIAL: foreach ($data as $commit) { $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT, $commit['rev'], ); } break; } $conn_w = $revision->establishConnection('w'); $sql = array(); foreach ($hashes as $info) { list($type, $hash) = $info; $sql[] = qsprintf( $conn_w, '(%d, %s, %s)', $revision->getID(), $type, $hash); } queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', ArcanistDifferentialRevisionHash::TABLE_NAME, $revision->getID()); if ($sql) { queryfx( $conn_w, 'INSERT INTO %T (revisionID, type, hash) VALUES %Q', ArcanistDifferentialRevisionHash::TABLE_NAME, implode(', ', $sql)); } } private function renderAffectedFilesForMail(DifferentialDiff $diff) { $changesets = $diff->getChangesets(); $filenames = mpull($changesets, 'getDisplayFilename'); sort($filenames); $count = count($filenames); $max = 250; if ($count > $max) { $filenames = array_slice($filenames, 0, $max); $filenames[] = pht('(%d more files...)', ($count - $max)); } return implode("\n", $filenames); } private function renderPatchHTMLForMail($patch) { return phutil_tag('pre', array('style' => 'font-family: monospace;'), $patch); } private function buildPatchForMail(DifferentialDiff $diff) { $format = PhabricatorEnv::getEnvConfig('metamta.differential.patch-format'); return id(new DifferentialRawDiffRenderer()) ->setViewer($this->getActor()) ->setFormat($format) ->setChangesets($diff->getChangesets()) ->buildPatch(); } protected function willPublish(PhabricatorLiskDAO $object, array $xactions) { // Reload to pick up the active diff and reviewer status. return id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->needReviewers(true) ->needActiveDiffs(true) ->withIDs(array($object->getID())) ->executeOne(); } protected function getCustomWorkerState() { return array( 'changedPriorToCommitURI' => $this->changedPriorToCommitURI, ); } protected function loadCustomWorkerState(array $state) { $this->changedPriorToCommitURI = idx($state, 'changedPriorToCommitURI'); return $this; } private function newCommandeerReviewerTransaction( DifferentialRevision $revision) { $actor_phid = $this->getActingAsPHID(); $owner_phid = $revision->getAuthorPHID(); // If the user is commandeering, add the previous owner as a // reviewer and remove the actor. $edits = array( '-' => array( $actor_phid, ), '+' => array( $owner_phid, ), ); // NOTE: We're setting setIsCommandeerSideEffect() on this because normally // you can't add a revision's author as a reviewer, but this action swaps // them after validation executes. $xaction_type = DifferentialRevisionReviewersTransaction::TRANSACTIONTYPE; return id(new DifferentialTransaction()) ->setTransactionType($xaction_type) ->setIgnoreOnNoEffect(true) ->setIsCommandeerSideEffect(true) ->setNewValue($edits); } public function getActiveDiff($object) { if ($this->getIsNewObject()) { return null; } else { return $object->getActiveDiff(); } } /** * When a reviewer makes a comment, mark the last revision they commented * on. * * This allows us to show a hint to help authors and other reviewers quickly * distinguish between reviewers who have participated in the discussion and * reviewers who haven't been part of it. */ private function markReviewerComments($object, array $xactions) { $acting_phid = $this->getActingAsPHID(); if (!$acting_phid) { return; } $diff = $this->getActiveDiff($object); if (!$diff) { return; } $has_comment = false; foreach ($xactions as $xaction) { if ($xaction->hasComment()) { $has_comment = true; break; } } if (!$has_comment) { return; } $reviewer_table = new DifferentialReviewer(); $conn = $reviewer_table->establishConnection('w'); queryfx( $conn, 'UPDATE %T SET lastCommentDiffPHID = %s WHERE revisionPHID = %s AND reviewerPHID = %s', $reviewer_table->getTableName(), $diff->getPHID(), $object->getPHID(), $acting_phid); } - - } diff --git a/src/applications/differential/storage/DifferentialChangeset.php b/src/applications/differential/storage/DifferentialChangeset.php index 448cdb6343..be83c5e73b 100644 --- a/src/applications/differential/storage/DifferentialChangeset.php +++ b/src/applications/differential/storage/DifferentialChangeset.php @@ -1,222 +1,239 @@ array( 'metadata' => self::SERIALIZATION_JSON, 'oldProperties' => self::SERIALIZATION_JSON, 'newProperties' => self::SERIALIZATION_JSON, 'awayPaths' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( 'oldFile' => 'bytes?', 'filename' => 'bytes', 'changeType' => 'uint32', 'fileType' => 'uint32', 'addLines' => 'uint32', 'delLines' => 'uint32', // T6203/NULLABILITY // These should all be non-nullable, and store reasonable default // JSON values if empty. 'awayPaths' => 'text?', 'metadata' => 'text?', 'oldProperties' => 'text?', 'newProperties' => 'text?', ), self::CONFIG_KEY_SCHEMA => array( 'diffID' => array( 'columns' => array('diffID'), ), ), ) + parent::getConfiguration(); } public function getAffectedLineCount() { return $this->getAddLines() + $this->getDelLines(); } public function attachHunks(array $hunks) { assert_instances_of($hunks, 'DifferentialHunk'); $this->hunks = $hunks; return $this; } public function getHunks() { return $this->assertAttached($this->hunks); } public function getDisplayFilename() { $name = $this->getFilename(); if ($this->getFileType() == DifferentialChangeType::FILE_DIRECTORY) { $name .= '/'; } return $name; } + public function getOwnersFilename() { + // TODO: For Subversion, we should adjust these paths to be relative to + // the repository root where possible. + + $path = $this->getFilename(); + + if (!isset($path[0])) { + return '/'; + } + + if ($path[0] != '/') { + $path = '/'.$path; + } + + return $path; + } + public function addUnsavedHunk(DifferentialHunk $hunk) { if ($this->hunks === self::ATTACHABLE) { $this->hunks = array(); } $this->hunks[] = $hunk; $this->unsavedHunks[] = $hunk; return $this; } public function save() { $this->openTransaction(); $ret = parent::save(); foreach ($this->unsavedHunks as $hunk) { $hunk->setChangesetID($this->getID()); $hunk->save(); } $this->saveTransaction(); return $ret; } public function delete() { $this->openTransaction(); $modern_hunks = id(new DifferentialModernHunk())->loadAllWhere( 'changesetID = %d', $this->getID()); foreach ($modern_hunks as $modern_hunk) { $modern_hunk->delete(); } $this->unsavedHunks = array(); queryfx( $this->establishConnection('w'), 'DELETE FROM %T WHERE id = %d', self::TABLE_CACHE, $this->getID()); $ret = parent::delete(); $this->saveTransaction(); return $ret; } public function getSortKey() { $sort_key = $this->getFilename(); // Sort files with ".h" in them first, so headers (.h, .hpp) come before // implementations (.c, .cpp, .cs). $sort_key = str_replace('.h', '.!h', $sort_key); return $sort_key; } public function makeNewFile() { $file = mpull($this->getHunks(), 'makeNewFile'); return implode('', $file); } public function makeOldFile() { $file = mpull($this->getHunks(), 'makeOldFile'); return implode('', $file); } public function makeChangesWithContext($num_lines = 3) { $with_context = array(); foreach ($this->getHunks() as $hunk) { $context = array(); $changes = explode("\n", $hunk->getChanges()); foreach ($changes as $l => $line) { $type = substr($line, 0, 1); if ($type == '+' || $type == '-') { $context += array_fill($l - $num_lines, 2 * $num_lines + 1, true); } } $with_context[] = array_intersect_key($changes, $context); } return array_mergev($with_context); } public function getAnchorName() { return substr(md5($this->getFilename()), 0, 8); } public function getAbsoluteRepositoryPath( PhabricatorRepository $repository = null, DifferentialDiff $diff = null) { $base = '/'; if ($diff && $diff->getSourceControlPath()) { $base = id(new PhutilURI($diff->getSourceControlPath()))->getPath(); } $path = $this->getFilename(); $path = rtrim($base, '/').'/'.ltrim($path, '/'); $svn = PhabricatorRepositoryType::REPOSITORY_TYPE_SVN; if ($repository && $repository->getVersionControlSystem() == $svn) { $prefix = $repository->getDetail('remote-uri'); $prefix = id(new PhutilURI($prefix))->getPath(); if (!strncmp($path, $prefix, strlen($prefix))) { $path = substr($path, strlen($prefix)); } $path = '/'.ltrim($path, '/'); } return $path; } public function getWhitespaceMatters() { $config = PhabricatorEnv::getEnvConfig('differential.whitespace-matters'); foreach ($config as $regexp) { if (preg_match($regexp, $this->getFilename())) { return true; } } return false; } public function attachDiff(DifferentialDiff $diff) { $this->diff = $diff; return $this; } public function getDiff() { return $this->assertAttached($this->diff); } /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, ); } public function getPolicy($capability) { return $this->getDiff()->getPolicy($capability); } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { return $this->getDiff()->hasAutomaticCapability($capability, $viewer); } } diff --git a/src/applications/differential/storage/DifferentialReviewer.php b/src/applications/differential/storage/DifferentialReviewer.php index b202baddf2..3aa9bf6362 100644 --- a/src/applications/differential/storage/DifferentialReviewer.php +++ b/src/applications/differential/storage/DifferentialReviewer.php @@ -1,122 +1,127 @@ array( 'reviewerStatus' => 'text64', 'lastActionDiffPHID' => 'phid?', 'lastCommentDiffPHID' => 'phid?', 'lastActorPHID' => 'phid?', 'voidedPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_revision' => array( 'columns' => array('revisionPHID', 'reviewerPHID'), 'unique' => true, ), 'key_reviewer' => array( 'columns' => array('reviewerPHID', 'revisionPHID'), ), ), ) + parent::getConfiguration(); } public function isUser() { $user_type = PhabricatorPeopleUserPHIDType::TYPECONST; return (phid_get_type($this->getReviewerPHID()) == $user_type); } + public function isPackage() { + $package_type = PhabricatorOwnersPackagePHIDType::TYPECONST; + return (phid_get_type($this->getReviewerPHID()) == $package_type); + } + public function attachAuthority(PhabricatorUser $user, $has_authority) { $this->authority[$user->getCacheFragment()] = $has_authority; return $this; } public function hasAuthority(PhabricatorUser $viewer) { $cache_fragment = $viewer->getCacheFragment(); return $this->assertAttachedKey($this->authority, $cache_fragment); } public function isResigned() { $status_resigned = DifferentialReviewerStatus::STATUS_RESIGNED; return ($this->getReviewerStatus() == $status_resigned); } public function isRejected($diff_phid) { $status_rejected = DifferentialReviewerStatus::STATUS_REJECTED; if ($this->getReviewerStatus() != $status_rejected) { return false; } if ($this->getVoidedPHID()) { return false; } if ($this->isCurrentAction($diff_phid)) { return true; } return false; } public function isAccepted($diff_phid) { $status_accepted = DifferentialReviewerStatus::STATUS_ACCEPTED; if ($this->getReviewerStatus() != $status_accepted) { return false; } // If this accept has been voided (for example, but a reviewer using // "Request Review"), don't count it as a real "Accept" even if it is // against the current diff PHID. if ($this->getVoidedPHID()) { return false; } if ($this->isCurrentAction($diff_phid)) { return true; } $sticky_key = 'differential.sticky-accept'; $is_sticky = PhabricatorEnv::getEnvConfig($sticky_key); if ($is_sticky) { return true; } return false; } private function isCurrentAction($diff_phid) { if (!$diff_phid) { return true; } $action_phid = $this->getLastActionDiffPHID(); if (!$action_phid) { return true; } if ($action_phid == $diff_phid) { return true; } return false; } } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index 7189c5f5b4..72204256e5 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -1,683 +1,921 @@ setViewer($actor) ->withClasses(array('PhabricatorDifferentialApplication')) ->executeOne(); $view_policy = $app->getPolicy( DifferentialDefaultViewCapability::CAPABILITY); return id(new DifferentialRevision()) ->setViewPolicy($view_policy) ->setAuthorPHID($actor->getPHID()) ->attachRepository(null) ->attachActiveDiff(null) ->attachReviewers(array()) ->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); } protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, self::CONFIG_SERIALIZATION => array( 'attached' => self::SERIALIZATION_JSON, 'unsubscribed' => self::SERIALIZATION_JSON, 'properties' => self::SERIALIZATION_JSON, ), self::CONFIG_COLUMN_SCHEMA => array( 'title' => 'text255', 'originalTitle' => 'text255', 'status' => 'text32', 'summary' => 'text', 'testPlan' => 'text', 'authorPHID' => 'phid?', 'lastReviewerPHID' => 'phid?', 'lineCount' => 'uint32?', 'mailKey' => 'bytes40', 'branchName' => 'text255?', 'repositoryPHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( 'key_phid' => null, 'phid' => array( 'columns' => array('phid'), 'unique' => true, ), 'authorPHID' => array( 'columns' => array('authorPHID', 'status'), ), 'repositoryPHID' => array( 'columns' => array('repositoryPHID'), ), // If you (or a project you are a member of) is reviewing a significant // fraction of the revisions on an install, the result set of open // revisions may be smaller than the result set of revisions where you // are a reviewer. In these cases, this key is better than keys on the // edge table. 'key_status' => array( 'columns' => array('status', 'phid'), ), ), ) + parent::getConfiguration(); } public function setProperty($key, $value) { $this->properties[$key] = $value; return $this; } public function getProperty($key, $default = null) { return idx($this->properties, $key, $default); } public function hasRevisionProperty($key) { return array_key_exists($key, $this->properties); } public function getMonogram() { $id = $this->getID(); return "D{$id}"; } public function getURI() { return '/'.$this->getMonogram(); } public function setTitle($title) { $this->title = $title; if (!$this->getID()) { $this->originalTitle = $title; } return $this; } public function loadIDsByCommitPHIDs($phids) { if (!$phids) { return array(); } $revision_ids = queryfx_all( $this->establishConnection('r'), 'SELECT * FROM %T WHERE commitPHID IN (%Ls)', self::TABLE_COMMIT, $phids); return ipull($revision_ids, 'revisionID', 'commitPHID'); } public function loadCommitPHIDs() { if (!$this->getID()) { return ($this->commits = array()); } $commits = queryfx_all( $this->establishConnection('r'), 'SELECT commitPHID FROM %T WHERE revisionID = %d', self::TABLE_COMMIT, $this->getID()); $commits = ipull($commits, 'commitPHID'); return ($this->commits = $commits); } public function getCommitPHIDs() { return $this->assertAttached($this->commits); } public function getActiveDiff() { // TODO: Because it's currently technically possible to create a revision // without an associated diff, we allow an attached-but-null active diff. // It would be good to get rid of this once we make diff-attaching // transactional. return $this->assertAttached($this->activeDiff); } public function attachActiveDiff($diff) { $this->activeDiff = $diff; return $this; } public function getDiffIDs() { return $this->assertAttached($this->diffIDs); } public function attachDiffIDs(array $ids) { rsort($ids); $this->diffIDs = array_values($ids); return $this; } public function attachCommitPHIDs(array $phids) { $this->commits = array_values($phids); return $this; } public function getAttachedPHIDs($type) { return array_keys(idx($this->attached, $type, array())); } public function setAttachedPHIDs($type, array $phids) { $this->attached[$type] = array_fill_keys($phids, array()); return $this; } public function generatePHID() { return PhabricatorPHID::generateNewPHID( DifferentialRevisionPHIDType::TYPECONST); } public function loadActiveDiff() { return id(new DifferentialDiff())->loadOneWhere( 'revisionID = %d ORDER BY id DESC LIMIT 1', $this->getID()); } public function save() { if (!$this->getMailKey()) { $this->mailKey = Filesystem::readRandomCharacters(40); } return parent::save(); } public function getHashes() { return $this->assertAttached($this->hashes); } public function attachHashes(array $hashes) { $this->hashes = $hashes; return $this; } + public function canReviewerForceAccept( + PhabricatorUser $viewer, + DifferentialReviewer $reviewer) { + + if (!$reviewer->isPackage()) { + return false; + } + + $map = $this->getReviewerForceAcceptMap($viewer); + if (!$map) { + return false; + } + + if (isset($map[$reviewer->getReviewerPHID()])) { + return true; + } + + return false; + } + + private function getReviewerForceAcceptMap(PhabricatorUser $viewer) { + $fragment = $viewer->getCacheFragment(); + + if (!array_key_exists($fragment, $this->forceMap)) { + $map = $this->newReviewerForceAcceptMap($viewer); + $this->forceMap[$fragment] = $map; + } + + return $this->forceMap[$fragment]; + } + + private function newReviewerForceAcceptMap(PhabricatorUser $viewer) { + $diff = $this->getActiveDiff(); + if (!$diff) { + return null; + } + + $repository_phid = $diff->getRepositoryPHID(); + if (!$repository_phid) { + return null; + } + + $paths = array(); + + try { + $changesets = $diff->getChangesets(); + } catch (Exception $ex) { + $changesets = id(new DifferentialChangesetQuery()) + ->setViewer($viewer) + ->withDiffs(array($diff)) + ->execute(); + } + + foreach ($changesets as $changeset) { + $paths[] = $changeset->getOwnersFilename(); + } + + if (!$paths) { + return null; + } + + $reviewer_phids = array(); + foreach ($this->getReviewers() as $reviewer) { + if (!$reviewer->isPackage()) { + continue; + } + + $reviewer_phids[] = $reviewer->getReviewerPHID(); + } + + if (!$reviewer_phids) { + return null; + } + + // Load all the reviewing packages which have control over some of the + // paths in the change. These are packages which the actor may be able + // to force-accept on behalf of. + $control_query = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) + ->withPHIDs($reviewer_phids) + ->withControl($repository_phid, $paths); + $control_packages = $control_query->execute(); + if (!$control_packages) { + return null; + } + + // Load all the packages which have potential control over some of the + // paths in the change and are owned by the actor. These are packages + // 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. + $authority_query = id(new PhabricatorOwnersPackageQuery()) + ->setViewer($viewer) + ->withStatuses(array(PhabricatorOwnersPackage::STATUS_ACTIVE)) + ->withAuthorityPHIDs(array($viewer->getPHID())) + ->withControl($repository_phid, $paths); + $authority_packages = $authority_query->execute(); + if (!$authority_packages) { + return null; + } + $authority_packages = mpull($authority_packages, null, 'getPHID'); + + // Build a map from each path in the revision to the reviewer packages + // which control it. + $control_map = array(); + foreach ($paths as $path) { + $control_packages = $control_query->getControllingPackagesForPath( + $repository_phid, + $path); + + // Remove packages which the viewer has authority over. We don't need + // to check these for force-accept because they can just accept them + // normally. + $control_packages = mpull($control_packages, null, 'getPHID'); + foreach ($control_packages as $phid => $control_package) { + if (isset($authority_packages[$phid])) { + unset($control_packages[$phid]); + } + } + + if (!$control_packages) { + continue; + } + + $control_map[$path] = $control_packages; + } + + if (!$control_map) { + return null; + } + + // From here on out, we only care about paths which we have at least one + // controlling package for. + $paths = array_keys($control_map); + + // Now, build a map from each path to the packages which would control it + // if there were no dominion rules. + $authority_map = array(); + foreach ($paths as $path) { + $authority_packages = $authority_query->getControllingPackagesForPath( + $repository_phid, + $path, + $ignore_dominion = true); + + $authority_map[$path] = mpull($authority_packages, null, 'getPHID'); + } + + // For each path, find the most general package that the viewer has + // authority over. For example, we'll prefer a package that owns "/" to a + // package that owns "/src/". + $force_map = array(); + foreach ($authority_map as $path => $package_map) { + $path_fragments = PhabricatorOwnersPackage::splitPath($path); + $fragment_count = count($path_fragments); + + // Find the package that we have authority over which has the most + // general match for this path. + $best_match = null; + $best_package = null; + foreach ($package_map as $package_phid => $package) { + $package_paths = $package->getPathsForRepository($repository_phid); + foreach ($package_paths as $package_path) { + + // NOTE: A strength of 0 means "no match". A strength of 1 means + // that we matched "/", so we can not possibly find another stronger + // match. + + $strength = $package_path->getPathMatchStrength( + $path_fragments, + $fragment_count); + if (!$strength) { + continue; + } + + if ($strength < $best_match || !$best_package) { + $best_match = $strength; + $best_package = $package; + if ($strength == 1) { + break 2; + } + } + } + } + + if ($best_package) { + $force_map[$path] = array( + 'strength' => $best_match, + 'package' => $best_package, + ); + } + } + + // For each path which the viewer owns a package for, find other packages + // which that authority can be used to force-accept. Once we find a way to + // force-accept a package, we don't need to keep loooking. + $has_control = array(); + foreach ($force_map as $path => $spec) { + $path_fragments = PhabricatorOwnersPackage::splitPath($path); + $fragment_count = count($path_fragments); + + $authority_strength = $spec['strength']; + + $control_packages = $control_map[$path]; + foreach ($control_packages as $control_phid => $control_package) { + if (isset($has_control[$control_phid])) { + continue; + } + + $control_paths = $control_package->getPathsForRepository( + $repository_phid); + foreach ($control_paths as $control_path) { + $strength = $control_path->getPathMatchStrength( + $path_fragments, + $fragment_count); + + if (!$strength) { + continue; + } + + if ($strength > $authority_strength) { + $authority = $spec['package']; + $has_control[$control_phid] = array( + 'authority' => $authority, + 'phid' => $authority->getPHID(), + ); + break; + } + } + } + } + + // Return a map from packages which may be force accepted to the packages + // which permit that forced acceptance. + return ipull($has_control, 'phid'); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, PhabricatorPolicyCapability::CAN_EDIT, ); } public function getPolicy($capability) { switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: return $this->getViewPolicy(); case PhabricatorPolicyCapability::CAN_EDIT: return $this->getEditPolicy(); } } public function hasAutomaticCapability($capability, PhabricatorUser $user) { // A revision's author (which effectively means "owner" after we added // commandeering) can always view and edit it. $author_phid = $this->getAuthorPHID(); if ($author_phid) { if ($user->getPHID() == $author_phid) { return true; } } return false; } public function describeAutomaticCapability($capability) { $description = array( pht('The owner of a revision can always view and edit it.'), ); switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: $description[] = pht("A revision's reviewers can always view it."); $description[] = pht( 'If a revision belongs to a repository, other users must be able '. 'to view the repository in order to view the revision.'); break; } return $description; } /* -( PhabricatorExtendedPolicyInterface )--------------------------------- */ public function getExtendedPolicy($capability, PhabricatorUser $viewer) { $extended = array(); switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: $repository_phid = $this->getRepositoryPHID(); $repository = $this->getRepository(); // Try to use the object if we have it, since it will save us some // data fetching later on. In some cases, we might not have it. $repository_ref = nonempty($repository, $repository_phid); if ($repository_ref) { $extended[] = array( $repository_ref, PhabricatorPolicyCapability::CAN_VIEW, ); } break; } return $extended; } /* -( PhabricatorTokenReceiverInterface )---------------------------------- */ public function getUsersToNotifyOfTokenGiven() { return array( $this->getAuthorPHID(), ); } public function getReviewers() { return $this->assertAttached($this->reviewerStatus); } public function attachReviewers(array $reviewers) { assert_instances_of($reviewers, 'DifferentialReviewer'); $reviewers = mpull($reviewers, null, 'getReviewerPHID'); $this->reviewerStatus = $reviewers; return $this; } public function getReviewerPHIDs() { $reviewers = $this->getReviewers(); return mpull($reviewers, 'getReviewerPHID'); } public function getReviewerPHIDsForEdit() { $reviewers = $this->getReviewers(); $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING; $value = array(); foreach ($reviewers as $reviewer) { $phid = $reviewer->getReviewerPHID(); if ($reviewer->getReviewerStatus() == $status_blocking) { $value[] = 'blocking('.$phid.')'; } else { $value[] = $phid; } } return $value; } public function getRepository() { return $this->assertAttached($this->repository); } public function attachRepository(PhabricatorRepository $repository = null) { $this->repository = $repository; return $this; } public function isClosed() { return DifferentialRevisionStatus::isClosedStatus($this->getStatus()); } public function isAbandoned() { $status_abandoned = ArcanistDifferentialRevisionStatus::ABANDONED; return ($this->getStatus() == $status_abandoned); } public function isAccepted() { $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; return ($this->getStatus() == $status_accepted); } public function getStatusIcon() { $map = array( ArcanistDifferentialRevisionStatus::NEEDS_REVIEW => 'fa-code grey', ArcanistDifferentialRevisionStatus::NEEDS_REVISION => 'fa-refresh red', ArcanistDifferentialRevisionStatus::CHANGES_PLANNED => 'fa-headphones red', ArcanistDifferentialRevisionStatus::ACCEPTED => 'fa-check green', ArcanistDifferentialRevisionStatus::CLOSED => 'fa-check-square-o black', ArcanistDifferentialRevisionStatus::ABANDONED => 'fa-plane black', ); return idx($map, $this->getStatus()); } public function getStatusDisplayName() { $status = $this->getStatus(); return ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( $status); } public function getFlag(PhabricatorUser $viewer) { return $this->assertAttachedKey($this->flags, $viewer->getPHID()); } public function attachFlag( PhabricatorUser $viewer, PhabricatorFlag $flag = null) { $this->flags[$viewer->getPHID()] = $flag; return $this; } public function getHasDraft(PhabricatorUser $viewer) { return $this->assertAttachedKey($this->drafts, $viewer->getCacheFragment()); } public function attachHasDraft(PhabricatorUser $viewer, $has_draft) { $this->drafts[$viewer->getCacheFragment()] = $has_draft; return $this; } /* -( HarbormasterBuildableInterface )------------------------------------- */ public function getHarbormasterBuildableDisplayPHID() { return $this->getHarbormasterContainerPHID(); } public function getHarbormasterBuildablePHID() { return $this->loadActiveDiff()->getPHID(); } public function getHarbormasterContainerPHID() { return $this->getPHID(); } public function getHarbormasterPublishablePHID() { return $this->getPHID(); } public function getBuildVariables() { return array(); } public function getAvailableBuildVariables() { return array(); } /* -( PhabricatorSubscribableInterface )----------------------------------- */ public function isAutomaticallySubscribed($phid) { if ($phid == $this->getAuthorPHID()) { return true; } // TODO: This only happens when adding or removing CCs, and is safe from a // policy perspective, but the subscription pathway should have some // opportunity to load this data properly. For now, this is the only case // where implicit subscription is not an intrinsic property of the object. if ($this->reviewerStatus == self::ATTACHABLE) { $reviewers = id(new DifferentialRevisionQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withPHIDs(array($this->getPHID())) ->needReviewers(true) ->executeOne() ->getReviewers(); } else { $reviewers = $this->getReviewers(); } foreach ($reviewers as $reviewer) { if ($reviewer->getReviewerPHID() == $phid) { return true; } } return false; } /* -( PhabricatorCustomFieldInterface )------------------------------------ */ public function getCustomFieldSpecificationForRole($role) { return PhabricatorEnv::getEnvConfig('differential.fields'); } public function getCustomFieldBaseClass() { return 'DifferentialCustomField'; } public function getCustomFields() { return $this->assertAttached($this->customFields); } public function attachCustomFields(PhabricatorCustomFieldAttachment $fields) { $this->customFields = $fields; return $this; } /* -( PhabricatorApplicationTransactionInterface )------------------------- */ public function getApplicationTransactionEditor() { return new DifferentialTransactionEditor(); } public function getApplicationTransactionObject() { return $this; } public function getApplicationTransactionTemplate() { return new DifferentialTransaction(); } public function willRenderTimeline( PhabricatorApplicationTransactionView $timeline, AphrontRequest $request) { $viewer = $request->getViewer(); $render_data = $timeline->getRenderData(); $left = $request->getInt('left', idx($render_data, 'left')); $right = $request->getInt('right', idx($render_data, 'right')); $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) ->withIDs(array($left, $right)) ->execute(); $diffs = mpull($diffs, null, 'getID'); $left_diff = $diffs[$left]; $right_diff = $diffs[$right]; $old_ids = $request->getStr('old', idx($render_data, 'old')); $new_ids = $request->getStr('new', idx($render_data, 'new')); $old_ids = array_filter(explode(',', $old_ids)); $new_ids = array_filter(explode(',', $new_ids)); $type_inline = DifferentialTransaction::TYPE_INLINE; $changeset_ids = array_merge($old_ids, $new_ids); $inlines = array(); foreach ($timeline->getTransactions() as $xaction) { if ($xaction->getTransactionType() == $type_inline) { $inlines[] = $xaction->getComment(); $changeset_ids[] = $xaction->getComment()->getChangesetID(); } } if ($changeset_ids) { $changesets = id(new DifferentialChangesetQuery()) ->setViewer($request->getUser()) ->withIDs($changeset_ids) ->execute(); $changesets = mpull($changesets, null, 'getID'); } else { $changesets = array(); } foreach ($inlines as $key => $inline) { $inlines[$key] = DifferentialInlineComment::newFromModernComment( $inline); } $query = id(new DifferentialInlineCommentQuery()) ->needHidden(true) ->setViewer($viewer); // NOTE: This is a bit sketchy: this method adjusts the inlines as a // side effect, which means it will ultimately adjust the transaction // comments and affect timeline rendering. $query->adjustInlinesForChangesets( $inlines, array_select_keys($changesets, $old_ids), array_select_keys($changesets, $new_ids), $this); return $timeline ->setChangesets($changesets) ->setRevision($this) ->setLeftDiff($left_diff) ->setRightDiff($right_diff); } /* -( PhabricatorDestructibleInterface )----------------------------------- */ public function destroyObjectPermanently( PhabricatorDestructionEngine $engine) { $this->openTransaction(); $diffs = id(new DifferentialDiffQuery()) ->setViewer($engine->getViewer()) ->withRevisionIDs(array($this->getID())) ->execute(); foreach ($diffs as $diff) { $engine->destroyObject($diff); } $conn_w = $this->establishConnection('w'); queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', self::TABLE_COMMIT, $this->getID()); // we have to do paths a little differentally as they do not have // an id or phid column for delete() to act on $dummy_path = new DifferentialAffectedPath(); queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', $dummy_path->getTableName(), $this->getID()); $this->delete(); $this->saveTransaction(); } /* -( PhabricatorFulltextInterface )--------------------------------------- */ public function newFulltextEngine() { return new DifferentialRevisionFulltextEngine(); } /* -( PhabricatorConduitResultInterface )---------------------------------- */ public function getFieldSpecificationsForConduit() { return array( id(new PhabricatorConduitSearchFieldSpecification()) ->setKey('title') ->setType('string') ->setDescription(pht('The revision title.')), id(new PhabricatorConduitSearchFieldSpecification()) ->setKey('authorPHID') ->setType('phid') ->setDescription(pht('Revision author PHID.')), ); } public function getFieldValuesForConduit() { return array( 'title' => $this->getTitle(), 'authorPHID' => $this->getAuthorPHID(), ); } public function getConduitSearchAttachments() { return array(); } /* -( PhabricatorDraftInterface )------------------------------------------ */ public function newDraftEngine() { return new DifferentialRevisionDraftEngine(); } } diff --git a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php index 4f6bedd698..7d1f1a92ef 100644 --- a/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionAcceptTransaction.php @@ -1,198 +1,222 @@ getReviewers(); $options = array(); $value = array(); // Put the viewer's user reviewer first, if it exists, so that "Accept as // yourself" is always at the top. $head = array(); $tail = array(); foreach ($reviewers as $key => $reviewer) { if ($reviewer->isUser()) { $head[$key] = $reviewer; } else { $tail[$key] = $reviewer; } } $reviewers = $head + $tail; $diff_phid = $this->getActiveDiffPHID($revision); $reviewer_phids = array(); // If the viewer isn't a reviewer, add them to the list of options first. // This happens when you navigate to some revision you aren't involved in: // you can accept and become a reviewer. $viewer_phid = $viewer->getPHID(); if ($viewer_phid) { if (!isset($reviewers[$viewer_phid])) { $reviewer_phids[$viewer_phid] = $viewer_phid; } } + $default_unchecked = array(); foreach ($reviewers as $reviewer) { + $reviewer_phid = $reviewer->getReviewerPHID(); + if (!$reviewer->hasAuthority($viewer)) { // If the viewer doesn't have authority to act on behalf of a reviewer, - // don't include that reviewer as an option. - continue; + // we check if they can accept by force. + if ($revision->canReviewerForceAccept($viewer, $reviewer)) { + $default_unchecked[$reviewer_phid] = true; + } else { + continue; + } } if ($reviewer->isAccepted($diff_phid)) { // If a reviewer is already in a full "accepted" state, don't // include that reviewer as an option. continue; } - $reviewer_phid = $reviewer->getReviewerPHID(); $reviewer_phids[$reviewer_phid] = $reviewer_phid; } $handles = $viewer->loadHandles($reviewer_phids); + $head = array(); + $tail = array(); foreach ($reviewer_phids as $reviewer_phid) { - $options[$reviewer_phid] = pht( - 'Accept as %s', - $viewer->renderHandle($reviewer_phid)); + $is_force = isset($default_unchecked[$reviewer_phid]); - $value[] = $reviewer_phid; + if ($is_force) { + $tail[] = $reviewer_phid; + + $options[$reviewer_phid] = pht( + 'Force accept as %s', + $viewer->renderHandle($reviewer_phid)); + } else { + $head[] = $reviewer_phid; + $value[] = $reviewer_phid; + + $options[$reviewer_phid] = pht( + 'Accept as %s', + $viewer->renderHandle($reviewer_phid)); + } } + // Reorder reviewers so "force accept" reviewers come at the end. + $options = + array_select_keys($options, $head) + + array_select_keys($options, $tail); + return array($options, $value); } public function generateOldValue($object) { $actor = $this->getActor(); return $this->isViewerFullyAccepted($object, $actor); } public function applyExternalEffects($object, $value) { $status = DifferentialReviewerStatus::STATUS_ACCEPTED; $actor = $this->getActor(); $this->applyReviewerEffect($object, $actor, $value, $status); } protected function validateAction($object, PhabricatorUser $viewer) { if ($object->isClosed()) { throw new Exception( pht( 'You can not accept this revision because it has already been '. 'closed. Only open revisions can be accepted.')); } $config_key = 'differential.allow-self-accept'; if (!PhabricatorEnv::getEnvConfig($config_key)) { if ($this->isViewerRevisionAuthor($object, $viewer)) { throw new Exception( pht( 'You can not accept this revision because you are the revision '. 'author. You can only accept revisions you do not own. You can '. 'change this behavior by adjusting the "%s" setting in Config.', $config_key)); } } if ($this->isViewerFullyAccepted($object, $viewer)) { throw new Exception( pht( 'You can not accept this revision because you have already '. 'accepted it.')); } } protected function validateOptionValue($object, $actor, array $value) { if (!$value) { throw new Exception( pht( 'When accepting a revision, you must accept on behalf of at '. 'least one reviewer.')); } list($options) = $this->getActionOptions($actor, $object); foreach ($value as $phid) { if (!isset($options[$phid])) { throw new Exception( pht( 'Reviewer "%s" is not a valid reviewer which you have authority '. 'to accept on behalf of.', $phid)); } } } public function getTitle() { $new = $this->getNewValue(); if (is_array($new) && $new) { return pht( '%s accepted this revision as %s reviewer(s): %s.', $this->renderAuthor(), phutil_count($new), $this->renderHandleList($new)); } else { return pht( '%s accepted this revision.', $this->renderAuthor()); } } public function getTitleForFeed() { return pht( '%s accepted %s.', $this->renderAuthor(), $this->renderObject()); } } diff --git a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php index f232398533..8e1c437c53 100644 --- a/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionActionTransaction.php @@ -1,171 +1,176 @@ getPhobjectClassConstant('ACTIONKEY', 32); } public function isActionAvailable($object, PhabricatorUser $viewer) { try { $this->validateAction($object, $viewer); return true; } catch (Exception $ex) { return false; } } abstract protected function validateAction($object, PhabricatorUser $viewer); abstract protected function getRevisionActionLabel(); protected function validateOptionValue($object, $actor, array $value) { return null; } public function getCommandKeyword() { return null; } public function getCommandAliases() { return array(); } public function getCommandSummary() { return null; } protected function getRevisionActionOrder() { return 1000; } public function getActionStrength() { return 3; } public function getRevisionActionOrderVector() { return id(new PhutilSortVector()) ->addInt($this->getRevisionActionOrder()); } protected function getRevisionActionGroupKey() { return DifferentialRevisionEditEngine::ACTIONGROUP_REVISION; } protected function getRevisionActionDescription() { return null; } public static function loadAllActions() { return id(new PhutilClassMapQuery()) ->setAncestorClass(__CLASS__) ->setUniqueMethod('getRevisionActionKey') ->execute(); } protected function isViewerRevisionAuthor( DifferentialRevision $revision, PhabricatorUser $viewer) { if (!$viewer->getPHID()) { return false; } return ($viewer->getPHID() === $revision->getAuthorPHID()); } protected function getActionOptions( PhabricatorUser $viewer, DifferentialRevision $revision) { return array( array(), null, ); } public function newEditField( DifferentialRevision $revision, PhabricatorUser $viewer) { // Actions in the "review" group, like "Accept Revision", do not require // that the actor be able to edit the revision. $group_review = DifferentialRevisionEditEngine::ACTIONGROUP_REVIEW; $is_review = ($this->getRevisionActionGroupKey() == $group_review); $field = id(new PhabricatorApplyEditField()) ->setKey($this->getRevisionActionKey()) ->setTransactionType($this->getTransactionTypeConstant()) ->setCanApplyWithoutEditCapability($is_review) ->setValue(true); if ($this->isActionAvailable($revision, $viewer)) { $label = $this->getRevisionActionLabel(); if ($label !== null) { $field->setCommentActionLabel($label); $description = $this->getRevisionActionDescription(); $field->setActionDescription($description); $group_key = $this->getRevisionActionGroupKey(); $field->setCommentActionGroupKey($group_key); // Currently, every revision action conflicts with every other // revision action: for example, you can not simultaneously Accept and // Reject a revision. // Under some configurations, some combinations of actions are sort of // technically permissible. For example, you could reasonably Reject // and Abandon a revision if "anyone can abandon anything" is enabled. // It's not clear that these combinations are actually useful, so just // keep things simple for now. $field->setActionConflictKey('revision.action'); list($options, $value) = $this->getActionOptions($viewer, $revision); - if (count($options) > 1) { + + // Show the options if the user can select on behalf of two or more + // reviewers, or can force-accept on behalf of one or more reviewers. + $can_multi = (count($options) > 1); + $can_force = (count($value) < count($options)); + if ($can_multi || $can_force) { $field->setOptions($options); $field->setValue($value); } } } return $field; } public function validateTransactions($object, array $xactions) { $errors = array(); $actor = $this->getActor(); $action_exception = null; try { $this->validateAction($object, $actor); } catch (Exception $ex) { $action_exception = $ex; } foreach ($xactions as $xaction) { if ($action_exception) { $errors[] = $this->newInvalidError( $action_exception->getMessage(), $xaction); continue; } $new = $xaction->getNewValue(); if (!is_array($new)) { continue; } try { $this->validateOptionValue($object, $actor, $new); } catch (Exception $ex) { $errors[] = $this->newInvalidError( $ex->getMessage(), $xaction); } } return $errors; } } diff --git a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php index 4e3e7b29e6..019c4c036f 100644 --- a/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionReviewTransaction.php @@ -1,252 +1,272 @@ getActor(); list($options, $default) = $this->getActionOptions($viewer, $object); sort($default); sort($value); if ($default === $value) { return true; } return $value; } protected function isViewerAnyReviewer( DifferentialRevision $revision, PhabricatorUser $viewer) { return ($this->getViewerReviewerStatus($revision, $viewer) !== null); } protected function isViewerAnyAuthority( DifferentialRevision $revision, PhabricatorUser $viewer) { $reviewers = $revision->getReviewers(); foreach ($revision->getReviewers() as $reviewer) { if ($reviewer->hasAuthority($viewer)) { return true; } } return false; } protected function isViewerFullyAccepted( DifferentialRevision $revision, PhabricatorUser $viewer) { return $this->isViewerReviewerStatusFully( $revision, $viewer, DifferentialReviewerStatus::STATUS_ACCEPTED); } protected function isViewerFullyRejected( DifferentialRevision $revision, PhabricatorUser $viewer) { return $this->isViewerReviewerStatusFully( $revision, $viewer, DifferentialReviewerStatus::STATUS_REJECTED); } protected function getViewerReviewerStatus( DifferentialRevision $revision, PhabricatorUser $viewer) { if (!$viewer->getPHID()) { return null; } foreach ($revision->getReviewers() as $reviewer) { if ($reviewer->getReviewerPHID() != $viewer->getPHID()) { continue; } return $reviewer->getReviewerStatus(); } return null; } private function isViewerReviewerStatusFully( DifferentialRevision $revision, PhabricatorUser $viewer, $require_status) { // If the user themselves is not a reviewer, the reviews they have // authority over can not all be in any set of states since their own // personal review has no state. $status = $this->getViewerReviewerStatus($revision, $viewer); if ($status === null) { return false; } $active_phid = $this->getActiveDiffPHID($revision); $status_accepted = DifferentialReviewerStatus::STATUS_ACCEPTED; $status_rejected = DifferentialReviewerStatus::STATUS_REJECTED; $is_accepted = ($require_status == $status_accepted); $is_rejected = ($require_status == $status_rejected); // Otherwise, check that all reviews they have authority over are in // the desired set of states. foreach ($revision->getReviewers() as $reviewer) { if (!$reviewer->hasAuthority($viewer)) { - continue; + $can_force = false; + + if ($is_accepted) { + if ($revision->canReviewerForceAccept($viewer, $reviewer)) { + $can_force = true; + } + } + + if (!$can_force) { + continue; + } } $status = $reviewer->getReviewerStatus(); if ($status != $require_status) { return false; } // Here, we're primarily testing if we can remove a void on the review. if ($is_accepted) { if (!$reviewer->isAccepted($active_phid)) { return false; } } if ($is_rejected) { if (!$reviewer->isRejected($active_phid)) { return false; } } // This is a broader check to see if we can update the diff where the // last action occurred. if ($reviewer->getLastActionDiffPHID() != $active_phid) { return false; } } return true; } protected function applyReviewerEffect( DifferentialRevision $revision, PhabricatorUser $viewer, $value, $status) { $map = array(); // When you accept or reject, you may accept or reject on behalf of all // reviewers you have authority for. When you resign, you only affect // yourself. $with_authority = ($status != DifferentialReviewerStatus::STATUS_RESIGNED); + $with_force = ($status == DifferentialReviewerStatus::STATUS_ACCEPTED); + if ($with_authority) { foreach ($revision->getReviewers() as $reviewer) { - if ($reviewer->hasAuthority($viewer)) { - $map[$reviewer->getReviewerPHID()] = $status; + if (!$reviewer->hasAuthority($viewer)) { + if (!$with_force) { + continue; + } + + if (!$revision->canReviewerForceAccept($viewer, $reviewer)) { + continue; + } } + + $map[$reviewer->getReviewerPHID()] = $status; } } // In all cases, you affect yourself. $map[$viewer->getPHID()] = $status; // If the user has submitted a specific list of reviewers to act as (by // unchecking some checkboxes under "Accept"), only affect those reviewers. if (is_array($value)) { $map = array_select_keys($map, $value); } // Convert reviewer statuses into edge data. foreach ($map as $reviewer_phid => $reviewer_status) { $map[$reviewer_phid] = array( 'data' => array( 'status' => $reviewer_status, ), ); } // This is currently double-writing: to the old (edge) store and the new // (reviewer) store. Do the old edge write first. $src_phid = $revision->getPHID(); $edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST; $editor = new PhabricatorEdgeEditor(); foreach ($map as $dst_phid => $edge_data) { if ($status == DifferentialReviewerStatus::STATUS_RESIGNED) { // TODO: For now, we just remove these reviewers. In the future, we will // store resignations explicitly. $editor->removeEdge($src_phid, $edge_type, $dst_phid); } else { $editor->addEdge($src_phid, $edge_type, $dst_phid, $edge_data); } } $editor->save(); // Now, do the new write. if ($map) { $diff = $this->getEditor()->getActiveDiff($revision); if ($diff) { $diff_phid = $diff->getPHID(); } else { $diff_phid = null; } $table = new DifferentialReviewer(); $reviewers = $table->loadAllWhere( 'revisionPHID = %s AND reviewerPHID IN (%Ls)', $src_phid, array_keys($map)); $reviewers = mpull($reviewers, null, 'getReviewerPHID'); foreach ($map as $dst_phid => $edge_data) { $reviewer = idx($reviewers, $dst_phid); if (!$reviewer) { $reviewer = id(new DifferentialReviewer()) ->setRevisionPHID($src_phid) ->setReviewerPHID($dst_phid); } $old_status = $reviewer->getReviewerStatus(); $reviewer->setReviewerStatus($status); if ($diff_phid) { $reviewer->setLastActionDiffPHID($diff_phid); } if ($old_status !== $status) { $reviewer->setLastActorPHID($this->getActingAsPHID()); } // Clear any outstanding void on this reviewer. A void may be placed // by the author using "Request Review" when a reviewer has already // accepted. $reviewer->setVoidedPHID(null); try { $reviewer->save(); } catch (AphrontDuplicateKeyQueryException $ex) { // At least for now, just ignore it if we lost a race. } } } } } diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php index 8194dd7a6a..a1c10cd5e9 100644 --- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php +++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php @@ -1,411 +1,419 @@ ownerPHIDs = $phids; return $this; } /** * Query owner authority. This will expand authorities, so a user PHID will * match both packages they own directly and packages owned by a project they * are a member of. */ public function withAuthorityPHIDs(array $phids) { $this->authorityPHIDs = $phids; return $this; } public function withPHIDs(array $phids) { $this->phids = $phids; return $this; } public function withIDs(array $ids) { $this->ids = $ids; return $this; } public function withRepositoryPHIDs(array $phids) { $this->repositoryPHIDs = $phids; return $this; } public function withPaths(array $paths) { $this->paths = $paths; return $this; } public function withStatuses(array $statuses) { $this->statuses = $statuses; return $this; } public function withControl($repository_phid, array $paths) { if (empty($this->controlMap[$repository_phid])) { $this->controlMap[$repository_phid] = array(); } foreach ($paths as $path) { $path = (string)$path; $this->controlMap[$repository_phid][$path] = $path; } // We need to load paths to execute control queries. $this->needPaths = true; return $this; } public function withNameNgrams($ngrams) { return $this->withNgramsConstraint( new PhabricatorOwnersPackageNameNgrams(), $ngrams); } public function needPaths($need_paths) { $this->needPaths = $need_paths; return $this; } public function newResultObject() { return new PhabricatorOwnersPackage(); } protected function willExecute() { $this->controlResults = array(); } protected function loadPage() { return $this->loadStandardPage($this->newResultObject()); } protected function willFilterPage(array $packages) { $package_ids = mpull($packages, 'getID'); $owners = id(new PhabricatorOwnersOwner())->loadAllWhere( 'packageID IN (%Ld)', $package_ids); $owners = mgroup($owners, 'getPackageID'); foreach ($packages as $package) { $package->attachOwners(idx($owners, $package->getID(), array())); } return $packages; } protected function didFilterPage(array $packages) { $package_ids = mpull($packages, 'getID'); if ($this->needPaths) { $paths = id(new PhabricatorOwnersPath())->loadAllWhere( 'packageID IN (%Ld)', $package_ids); $paths = mgroup($paths, 'getPackageID'); foreach ($packages as $package) { $package->attachPaths(idx($paths, $package->getID(), array())); } } if ($this->controlMap) { foreach ($packages as $package) { // If this package is archived, it's no longer a controlling package // for any path. In particular, it can not force active packages with // weak dominion to give up control. if ($package->isArchived()) { continue; } $this->controlResults[$package->getID()] = $package; } } return $packages; } protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { $joins = parent::buildJoinClauseParts($conn); if ($this->shouldJoinOwnersTable()) { $joins[] = qsprintf( $conn, 'JOIN %T o ON o.packageID = p.id', id(new PhabricatorOwnersOwner())->getTableName()); } if ($this->shouldJoinPathTable()) { $joins[] = qsprintf( $conn, 'JOIN %T rpath ON rpath.packageID = p.id', id(new PhabricatorOwnersPath())->getTableName()); } return $joins; } protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); if ($this->phids !== null) { $where[] = qsprintf( $conn, 'p.phid IN (%Ls)', $this->phids); } if ($this->ids !== null) { $where[] = qsprintf( $conn, 'p.id IN (%Ld)', $this->ids); } if ($this->repositoryPHIDs !== null) { $where[] = qsprintf( $conn, 'rpath.repositoryPHID IN (%Ls)', $this->repositoryPHIDs); } if ($this->authorityPHIDs !== null) { $authority_phids = $this->expandAuthority($this->authorityPHIDs); $where[] = qsprintf( $conn, 'o.userPHID IN (%Ls)', $authority_phids); } if ($this->ownerPHIDs !== null) { $where[] = qsprintf( $conn, 'o.userPHID IN (%Ls)', $this->ownerPHIDs); } if ($this->paths !== null) { $where[] = qsprintf( $conn, 'rpath.path IN (%Ls)', $this->getFragmentsForPaths($this->paths)); } if ($this->statuses !== null) { $where[] = qsprintf( $conn, 'p.status IN (%Ls)', $this->statuses); } if ($this->controlMap) { $clauses = array(); foreach ($this->controlMap as $repository_phid => $paths) { $fragments = $this->getFragmentsForPaths($paths); $clauses[] = qsprintf( $conn, '(rpath.repositoryPHID = %s AND rpath.path IN (%Ls))', $repository_phid, $fragments); } $where[] = implode(' OR ', $clauses); } return $where; } protected function shouldGroupQueryResultRows() { if ($this->shouldJoinOwnersTable()) { return true; } if ($this->shouldJoinPathTable()) { return true; } return parent::shouldGroupQueryResultRows(); } public function getBuiltinOrders() { return array( 'name' => array( 'vector' => array('name'), 'name' => pht('Name'), ), ) + parent::getBuiltinOrders(); } public function getOrderableColumns() { return parent::getOrderableColumns() + array( 'name' => array( 'table' => $this->getPrimaryTableAlias(), 'column' => 'name', 'type' => 'string', 'unique' => true, 'reverse' => true, ), ); } protected function getPagingValueMap($cursor, array $keys) { $package = $this->loadCursorObject($cursor); return array( 'id' => $package->getID(), 'name' => $package->getName(), ); } public function getQueryApplicationClass() { return 'PhabricatorOwnersApplication'; } protected function getPrimaryTableAlias() { return 'p'; } private function shouldJoinOwnersTable() { if ($this->ownerPHIDs !== null) { return true; } if ($this->authorityPHIDs !== null) { return true; } return false; } private function shouldJoinPathTable() { if ($this->repositoryPHIDs !== null) { return true; } if ($this->paths !== null) { return true; } if ($this->controlMap) { return true; } return false; } private function expandAuthority(array $phids) { $projects = id(new PhabricatorProjectQuery()) ->setViewer($this->getViewer()) ->withMemberPHIDs($phids) ->execute(); $project_phids = mpull($projects, 'getPHID'); return array_fuse($phids) + array_fuse($project_phids); } private function getFragmentsForPaths(array $paths) { $fragments = array(); foreach ($paths as $path) { foreach (PhabricatorOwnersPackage::splitPath($path) as $fragment) { $fragments[$fragment] = $fragment; } } return $fragments; } /* -( Path Control )------------------------------------------------------- */ /** * Get a list of all packages which control a path or its parent directories, * ordered from weakest to strongest. * * The first package has the most specific claim on the path; the last * package has the most general claim. Multiple packages may have claims of * equal strength, so this ordering is primarily one of usability and * convenience. * * @return list List of controlling packages. */ - public function getControllingPackagesForPath($repository_phid, $path) { + public function getControllingPackagesForPath( + $repository_phid, + $path, + $ignore_dominion = false) { $path = (string)$path; if (!isset($this->controlMap[$repository_phid][$path])) { throw new PhutilInvalidStateException('withControl'); } if ($this->controlResults === null) { throw new PhutilInvalidStateException('execute'); } $packages = $this->controlResults; $weak_dominion = PhabricatorOwnersPackage::DOMINION_WEAK; $path_fragments = PhabricatorOwnersPackage::splitPath($path); $fragment_count = count($path_fragments); $matches = array(); foreach ($packages as $package_id => $package) { $best_match = null; $include = false; $repository_paths = $package->getPathsForRepository($repository_phid); foreach ($repository_paths as $package_path) { $strength = $package_path->getPathMatchStrength( $path_fragments, $fragment_count); if ($strength > $best_match) { $best_match = $strength; $include = !$package_path->getExcluded(); } } if ($best_match && $include) { + if ($ignore_dominion) { + $is_weak = false; + } else { + $is_weak = ($package->getDominion() == $weak_dominion); + } $matches[$package_id] = array( 'strength' => $best_match, - 'weak' => ($package->getDominion() == $weak_dominion), + 'weak' => $is_weak, 'package' => $package, ); } } $matches = isort($matches, 'strength'); $matches = array_reverse($matches); $first_id = null; foreach ($matches as $package_id => $match) { if ($first_id === null) { $first_id = $package_id; continue; } if ($match['weak']) { unset($matches[$package_id]); } } return array_values(ipull($matches, 'package')); } }