From dfef8e2f0757c2ad2eb57e367b74a6a60066953d Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 23 Jun 2015 11:37:14 -0700 Subject: [PATCH 1/6] MetaMTA - more progress towards a mail application Summary: Ref T5791. This diff does a few things... - Adds code to write recipients to edges on save - Makes Query performance for policy filtering okay-ish - Adds a Search Engine for PhabricatorMetaMTAMail - Adds "working" List Controller - Inbox and Outbox both work - Adds stub View Controller Test Plan: ran `./bin/storage upgrade` and saw my inbox and outbox start getting data. played with application and saw new entries in inbox and outbox Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T5791 Differential Revision: https://secure.phabricator.com/D13397 --- src/__phutil_library_map__.php | 8 ++ .../PhabricatorMetaMTAApplication.php | 3 + .../PhabricatorMetaMTAMailListController.php | 30 +++++ .../PhabricatorMetaMTAMailViewController.php | 10 ++ ...ricatorMetaMTAMailHasRecipientEdgeType.php | 8 ++ .../query/PhabricatorMetaMTAMailQuery.php | 54 +++++++++ .../PhabricatorMetaMTAMailSearchEngine.php | 112 ++++++++++++++++++ .../storage/PhabricatorMetaMTAMail.php | 24 +++- 8 files changed, 244 insertions(+), 5 deletions(-) create mode 100644 src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php create mode 100644 src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php create mode 100644 src/applications/metamta/edge/PhabricatorMetaMTAMailHasRecipientEdgeType.php create mode 100644 src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 65c3aa6e16..d192db6b1c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2108,10 +2108,14 @@ phutil_register_library_map(array( 'PhabricatorMetaMTAMail' => 'applications/metamta/storage/PhabricatorMetaMTAMail.php', 'PhabricatorMetaMTAMailBody' => 'applications/metamta/view/PhabricatorMetaMTAMailBody.php', 'PhabricatorMetaMTAMailBodyTestCase' => 'applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php', + 'PhabricatorMetaMTAMailHasRecipientEdgeType' => 'applications/metamta/edge/PhabricatorMetaMTAMailHasRecipientEdgeType.php', + 'PhabricatorMetaMTAMailListController' => 'applications/metamta/controller/PhabricatorMetaMTAMailListController.php', 'PhabricatorMetaMTAMailPHIDType' => 'applications/metamta/phid/PhabricatorMetaMTAMailPHIDType.php', 'PhabricatorMetaMTAMailQuery' => 'applications/metamta/query/PhabricatorMetaMTAMailQuery.php', + 'PhabricatorMetaMTAMailSearchEngine' => 'applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php', 'PhabricatorMetaMTAMailSection' => 'applications/metamta/view/PhabricatorMetaMTAMailSection.php', 'PhabricatorMetaMTAMailTestCase' => 'applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php', + 'PhabricatorMetaMTAMailViewController' => 'applications/metamta/controller/PhabricatorMetaMTAMailViewController.php', 'PhabricatorMetaMTAMailableDatasource' => 'applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php', 'PhabricatorMetaMTAMailableFunctionDatasource' => 'applications/metamta/typeahead/PhabricatorMetaMTAMailableFunctionDatasource.php', 'PhabricatorMetaMTAMailgunReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php', @@ -5754,10 +5758,14 @@ phutil_register_library_map(array( ), 'PhabricatorMetaMTAMailBody' => 'Phobject', 'PhabricatorMetaMTAMailBodyTestCase' => 'PhabricatorTestCase', + 'PhabricatorMetaMTAMailHasRecipientEdgeType' => 'PhabricatorEdgeType', + 'PhabricatorMetaMTAMailListController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAMailPHIDType' => 'PhabricatorPHIDType', 'PhabricatorMetaMTAMailQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorMetaMTAMailSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorMetaMTAMailSection' => 'Phobject', 'PhabricatorMetaMTAMailTestCase' => 'PhabricatorTestCase', + 'PhabricatorMetaMTAMailViewController' => 'PhabricatorMetaMTAController', 'PhabricatorMetaMTAMailableDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorMetaMTAMailableFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', 'PhabricatorMetaMTAMailgunReceiveController' => 'PhabricatorMetaMTAController', diff --git a/src/applications/metamta/application/PhabricatorMetaMTAApplication.php b/src/applications/metamta/application/PhabricatorMetaMTAApplication.php index 802b3f1661..ce38ef8313 100644 --- a/src/applications/metamta/application/PhabricatorMetaMTAApplication.php +++ b/src/applications/metamta/application/PhabricatorMetaMTAApplication.php @@ -37,6 +37,9 @@ final class PhabricatorMetaMTAApplication extends PhabricatorApplication { public function getRoutes() { return array( '/mail/' => array( + '(query/(?P[^/]+)/)?' => + 'PhabricatorMetaMTAMailListController', + 'detail/(?P[1-9]\d*)/' => 'PhabricatorMetaMTAMailViewController', 'sendgrid/' => 'PhabricatorMetaMTASendGridReceiveController', 'mailgun/' => 'PhabricatorMetaMTAMailgunReceiveController', ), diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php new file mode 100644 index 0000000000..0651068550 --- /dev/null +++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailListController.php @@ -0,0 +1,30 @@ +setQueryKey($request->getURIData('queryKey')) + ->setSearchEngine(new PhabricatorMetaMTAMailSearchEngine()) + ->setNavigation($this->buildSideNav()); + + return $this->delegateToController($controller); + } + + public function buildSideNav() { + $user = $this->getRequest()->getUser(); + + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); + + id(new PhabricatorMetaMTAMailSearchEngine()) + ->setViewer($user) + ->addNavigationItems($nav->getMenu()); + + $nav->selectFilter(null); + + return $nav; + } + +} diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php new file mode 100644 index 0000000000..56b8f895b4 --- /dev/null +++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php @@ -0,0 +1,10 @@ +ids = $ids; @@ -16,6 +18,16 @@ final class PhabricatorMetaMTAMailQuery return $this; } + public function withActorPHIDs(array $phids) { + $this->actorPHIDs = $phids; + return $this; + } + + public function withRecipientPHIDs(array $phids) { + $this->recipientPHIDs = $phids; + return $this; + } + protected function loadPage() { return $this->loadStandardPage($this->newResultObject()); } @@ -37,11 +49,53 @@ final class PhabricatorMetaMTAMailQuery $this->phids); } + if ($this->actorPHIDs !== null) { + $where[] = qsprintf( + $conn_r, + 'mail.actorPHID IN (%Ls)', + $this->actorPHIDs); + } + + if ($this->recipientPHIDs !== null) { + $where[] = qsprintf( + $conn_r, + 'recipient.dst IN (%Ls)', + $this->recipientPHIDs); + } + + $viewer = $this->getViewer(); + $where[] = qsprintf( + $conn_r, + 'edge.dst = %s OR actorPHID = %s', + $viewer->getPHID(), + $viewer->getPHID()); + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); } + protected function buildJoinClause(AphrontDatabaseConnection $conn) { + $joins = array(); + + $joins[] = qsprintf( + $conn, + 'LEFT JOIN %T edge ON mail.phid = edge.src AND edge.type = %d', + PhabricatorEdgeConfig::TABLE_NAME_EDGE, + PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST); + + if ($this->recipientPHIDs !== null) { + $joins[] = qsprintf( + $conn, + 'LEFT JOIN %T recipient '. + 'ON mail.phid = recipient.src AND recipient.type = %d', + PhabricatorEdgeConfig::TABLE_NAME_EDGE, + PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST); + } + + return implode(' ', $joins); + } + protected function getPrimaryTableAlias() { return 'mail'; } diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php b/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php new file mode 100644 index 0000000000..f385dd1fee --- /dev/null +++ b/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php @@ -0,0 +1,112 @@ +setLabel(pht('Actors')) + ->setKey('actorPHIDs') + ->setAliases(array('actor', 'actors')), + id(new PhabricatorSearchUsersField()) + ->setLabel(pht('Recipients')) + ->setKey('recipientPHIDs') + ->setAliases(array('recipient', 'recipients')), + ); + } + + protected function buildQueryFromParameters(array $map) { + $query = $this->newQuery(); + + if ($map['actorPHIDs']) { + $query->withActorPHIDs($map['actorPHIDs']); + } + + if ($map['recipientPHIDs']) { + $query->withRecipientPHIDs($map['recipientPHIDs']); + } + + return $query; + } + + protected function getURI($path) { + return '/mail/'.$path; + } + + protected function getBuiltinQueryNames() { + $names = array( + 'inbox' => pht('Inbox'), + 'outbox' => pht('Outbox'), + ); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + $viewer = $this->requireViewer(); + + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'inbox': + return $query->setParameter( + 'recipientPHIDs', + array($viewer->getPHID())); + case 'outbox': + return $query->setParameter( + 'actorPHIDs', + array($viewer->getPHID())); + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + protected function getRequiredHandlePHIDsForResultList( + array $objects, + PhabricatorSavedQuery $query) { + + $phids = array(); + foreach ($objects as $mail) { + $phids[] = $mail->getExpandedRecipientPHIDs(); + } + return array_mergev($phids); + } + + protected function renderResultList( + array $mails, + PhabricatorSavedQuery $query, + array $handles) { + + assert_instances_of($mails, 'PhabricatorMetaMTAMail'); + $viewer = $this->requireViewer(); + $list = new PHUIObjectItemListView(); + + foreach ($mails as $mail) { + + $header = pht('Mail %d: TODO.', $mail->getID()); + $item = id(new PHUIObjectItemView()) + ->setHeader($header); + $list->addItem($item); + } + + return $list; + } +} diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index d232f17cb4..5d7bb37a6a 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -366,9 +366,20 @@ final class PhabricatorMetaMTAMail // method. $this->openTransaction(); - // Save to generate a task ID. + // Save to generate a mail ID and PHID. $result = parent::save(); + // Write the recipient edges. + $editor = new PhabricatorEdgeEditor(); + $edge_type = PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST; + $actor_phids = array_unique(array_merge( + $this->getAllActorPHIDs(), + $this->getExpandedRecipientPHIDs())); + foreach ($actor_phids as $actor_phid) { + $editor->addEdge($this->getPHID(), $edge_type, $actor_phid); + } + $editor->save(); + // Queue a task to send this mail. $mailer_task = PhabricatorWorker::scheduleTask( 'PhabricatorMetaMTAWorker', @@ -813,11 +824,15 @@ final class PhabricatorMetaMTAMail } public function loadAllActors() { - $actor_phids = $this->getAllActorPHIDs(); - $actor_phids = $this->expandRecipients($actor_phids); + $actor_phids = $this->getExpandedRecipientPHIDs(); return $this->loadActors($actor_phids); } + public function getExpandedRecipientPHIDs() { + $actor_phids = $this->getAllActorPHIDs(); + return $this->expandRecipients($actor_phids); + } + private function getAllActorPHIDs() { return array_merge( array($this->getParam('from')), @@ -1025,8 +1040,7 @@ final class PhabricatorMetaMTAMail } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - $actor_phids = $this->getAllActorPHIDs(); - $actor_phids = $this->expandRecipients($actor_phids); + $actor_phids = $this->getExpandedRecipientPHIDs(); return in_array($viewer->getPHID(), $actor_phids); } From 7e0249d68c47180f069615078d728024c7e3cc80 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 23 Jun 2015 12:55:44 -0700 Subject: [PATCH 2/6] MetaMTA - more progress to mail app Summary: Ref T5791. This diff adds a "sensitive" flag to `PhabricatorMetaMTAMail`, defaults it to true in the constructor, and then sets it to false in teh application transaction editor. Assumption here is that sensitive emails are basically all the emails that don't flow through the application transaction editor. This diff also gets a basic "mail view" page up and going. This diff also fixes a bug writing recipient edges; the actor was being included. This bug also fixes a querying bug; we shouldn't do the automagic join of $viewer is recipient or $viewer is actor if folks are querying for recipients or actors already. The bug manifested itself as having the "inbox" be inbox + outbox. Test Plan: viewd list of messages. viewed message detail. Reviewers: epriestley Reviewed By: epriestley Subscribers: epriestley, Korvin Maniphest Tasks: T5791 Differential Revision: https://secure.phabricator.com/D13406 --- .../PhabricatorMetaMTAApplication.php | 4 + .../PhabricatorMetaMTAMailViewController.php | 83 ++++++++++++++++++- .../query/PhabricatorMetaMTAMailQuery.php | 26 +++--- .../PhabricatorMetaMTAMailSearchEngine.php | 15 +++- .../storage/PhabricatorMetaMTAMail.php | 25 ++++-- ...habricatorApplicationTransactionEditor.php | 1 + 6 files changed, 134 insertions(+), 20 deletions(-) diff --git a/src/applications/metamta/application/PhabricatorMetaMTAApplication.php b/src/applications/metamta/application/PhabricatorMetaMTAApplication.php index ce38ef8313..9f6633f9c4 100644 --- a/src/applications/metamta/application/PhabricatorMetaMTAApplication.php +++ b/src/applications/metamta/application/PhabricatorMetaMTAApplication.php @@ -6,6 +6,10 @@ final class PhabricatorMetaMTAApplication extends PhabricatorApplication { return pht('MetaMTA'); } + public function getBaseURI() { + return '/mail/'; + } + public function getFontIcon() { return 'fa-send'; } diff --git a/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php index 56b8f895b4..859b2baf88 100644 --- a/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php +++ b/src/applications/metamta/controller/PhabricatorMetaMTAMailViewController.php @@ -4,7 +4,88 @@ final class PhabricatorMetaMTAMailViewController extends PhabricatorMetaMTAController { public function handleRequest(AphrontRequest $request) { - // TODO + $viewer = $request->getUser(); + + $mail = id(new PhabricatorMetaMTAMailQuery()) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) + ->executeOne(); + if (!$mail) { + return new Aphront404Response(); + } + + if ($mail->hasSensitiveContent()) { + $title = pht('Content Redacted'); + } else { + $title = $mail->getSubject(); + } + $header = id(new PHUIHeaderView()) + ->setHeader($title) + ->setUser($this->getRequest()->getUser()) + ->setPolicyObject($mail); + + $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb( + 'Mail '.$mail->getID()); + $object_box = id(new PHUIObjectBoxView()) + ->setHeader($header) + ->addPropertyList($this->buildPropertyView($mail)); + + return $this->buildApplicationPage( + array( + $crumbs, + $object_box, + ), + array( + 'title' => $title, + 'pageObjects' => array($mail->getPHID()), + )); + } + + private function buildPropertyView(PhabricatorMetaMTAMail $mail) { + $viewer = $this->getViewer(); + + $properties = id(new PHUIPropertyListView()) + ->setUser($viewer) + ->setObject($mail); + + if ($mail->getActorPHID()) { + $actor_str = $viewer->renderHandle($mail->getActorPHID()); + } else { + $actor_str = pht('Generated by Phabricator'); + } + $properties->addProperty( + pht('Actor'), + $actor_str); + + if ($mail->getFrom()) { + $from_str = $viewer->renderHandle($mail->getFrom()); + } else { + $from_str = pht('Sent by Phabricator'); + } + $properties->addProperty( + pht('From'), + $from_str); + + if ($mail->getToPHIDs()) { + $to_list = $viewer->renderHandleList($mail->getToPHIDs()); + } else { + $to_list = pht('None'); + } + $properties->addProperty( + pht('To'), + $to_list); + + if ($mail->getCcPHIDs()) { + $cc_list = $viewer->renderHandleList($mail->getCcPHIDs()); + } else { + $cc_list = pht('None'); + } + $properties->addProperty( + pht('Cc'), + $cc_list); + + return $properties; } } diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMailQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAMailQuery.php index f4d22bc0e7..a0965b7133 100644 --- a/src/applications/metamta/query/PhabricatorMetaMTAMailQuery.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAMailQuery.php @@ -63,12 +63,14 @@ final class PhabricatorMetaMTAMailQuery $this->recipientPHIDs); } - $viewer = $this->getViewer(); - $where[] = qsprintf( - $conn_r, - 'edge.dst = %s OR actorPHID = %s', - $viewer->getPHID(), - $viewer->getPHID()); + if ($this->actorPHIDs === null && $this->recipientPHIDs === null) { + $viewer = $this->getViewer(); + $where[] = qsprintf( + $conn_r, + 'edge.dst = %s OR actorPHID = %s', + $viewer->getPHID(), + $viewer->getPHID()); + } $where[] = $this->buildPagingClause($conn_r); @@ -78,11 +80,13 @@ final class PhabricatorMetaMTAMailQuery protected function buildJoinClause(AphrontDatabaseConnection $conn) { $joins = array(); - $joins[] = qsprintf( - $conn, - 'LEFT JOIN %T edge ON mail.phid = edge.src AND edge.type = %d', - PhabricatorEdgeConfig::TABLE_NAME_EDGE, - PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST); + if ($this->actorPHIDs === null && $this->recipientPHIDs === null) { + $joins[] = qsprintf( + $conn, + 'LEFT JOIN %T edge ON mail.phid = edge.src AND edge.type = %d', + PhabricatorEdgeConfig::TABLE_NAME_EDGE, + PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST); + } if ($this->recipientPHIDs !== null) { $joins[] = qsprintf( diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php b/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php index f385dd1fee..04055e756b 100644 --- a/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAMailSearchEngine.php @@ -100,10 +100,21 @@ final class PhabricatorMetaMTAMailSearchEngine $list = new PHUIObjectItemListView(); foreach ($mails as $mail) { + if ($mail->hasSensitiveContent()) { + $header = pht( + 'Mail %d: < content redacted >', + $mail->getID()); + } else { + $header = pht( + 'Mail %d: %s', + $mail->getID(), + $mail->getSubject()); + } - $header = pht('Mail %d: TODO.', $mail->getID()); $item = id(new PHUIObjectItemView()) - ->setHeader($header); + ->setObject($mail) + ->setHeader($header) + ->setHref($this->getURI('detail/'.$mail->getID())); $list->addItem($item); } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php index 5d7bb37a6a..ad7c07ed8e 100644 --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -25,7 +25,7 @@ final class PhabricatorMetaMTAMail public function __construct() { $this->status = self::STATUS_QUEUE; - $this->parameters = array(); + $this->parameters = array('sensitive' => true); parent::__construct(); } @@ -262,6 +262,15 @@ final class PhabricatorMetaMTAMail return $this; } + public function setSensitiveContent($bool) { + $this->setParam('sensitive', $bool); + return $this; + } + + public function hasSensitiveContent() { + return $this->getParam('sensitive', true); + } + public function setHTMLBody($html) { $this->setParam('html-body', $html); return $this; @@ -372,11 +381,15 @@ final class PhabricatorMetaMTAMail // Write the recipient edges. $editor = new PhabricatorEdgeEditor(); $edge_type = PhabricatorMetaMTAMailHasRecipientEdgeType::EDGECONST; - $actor_phids = array_unique(array_merge( - $this->getAllActorPHIDs(), - $this->getExpandedRecipientPHIDs())); - foreach ($actor_phids as $actor_phid) { - $editor->addEdge($this->getPHID(), $edge_type, $actor_phid); + $recipient_phids = array_merge( + $this->getToPHIDs(), + $this->getCcPHIDs()); + $expanded_phids = $this->expandRecipients($recipient_phids); + $all_phids = array_unique(array_merge( + $recipient_phids, + $expanded_phids)); + foreach ($all_phids as $curr_phid) { + $editor->addEdge($this->getPHID(), $edge_type, $curr_phid); } $editor->save(); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index ea541e3cdc..eb53b3b2e0 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -2334,6 +2334,7 @@ abstract class PhabricatorApplicationTransactionEditor } $mail + ->setSensitiveContent(false) ->setFrom($this->getActingAsPHID()) ->setSubjectPrefix($this->getMailSubjectPrefix()) ->setVarySubjectPrefix('['.$action.']') From b074cdeb4c504e26aac1c9b0f568a8cc53770e0f Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Jun 2015 13:33:09 -0700 Subject: [PATCH 3/6] Reduce Lint/Unit rendering duplication Summary: Ref T8096. No functional changes, just a bit less code. Test Plan: Viewed some revisions, saw the same stuff as before. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8096 Differential Revision: https://secure.phabricator.com/D13404 --- src/__phutil_library_map__.php | 6 +- .../DifferentialHarbormasterField.php | 96 ++++++++++++++++ .../customfield/DifferentialLintField.php | 106 +++++------------- .../customfield/DifferentialUnitField.php | 98 ++++------------ 4 files changed, 149 insertions(+), 157 deletions(-) create mode 100644 src/applications/differential/customfield/DifferentialHarbormasterField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d192db6b1c..aa55cfbf3a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -376,6 +376,7 @@ phutil_register_library_map(array( 'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php', 'DifferentialGitHubLandingStrategy' => 'applications/differential/landing/DifferentialGitHubLandingStrategy.php', 'DifferentialGitSVNIDField' => 'applications/differential/customfield/DifferentialGitSVNIDField.php', + 'DifferentialHarbormasterField' => 'applications/differential/customfield/DifferentialHarbormasterField.php', 'DifferentialHiddenComment' => 'applications/differential/storage/DifferentialHiddenComment.php', 'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php', 'DifferentialHostedGitLandingStrategy' => 'applications/differential/landing/DifferentialHostedGitLandingStrategy.php', @@ -3743,6 +3744,7 @@ phutil_register_library_map(array( 'DifferentialGetWorkingCopy' => 'Phobject', 'DifferentialGitHubLandingStrategy' => 'DifferentialLandingStrategy', 'DifferentialGitSVNIDField' => 'DifferentialCustomField', + 'DifferentialHarbormasterField' => 'DifferentialCustomField', 'DifferentialHiddenComment' => 'DifferentialDAO', 'DifferentialHostField' => 'DifferentialCustomField', 'DifferentialHostedGitLandingStrategy' => 'DifferentialLandingStrategy', @@ -3768,7 +3770,7 @@ phutil_register_library_map(array( 'DifferentialLandingStrategy' => 'Phobject', 'DifferentialLegacyHunk' => 'DifferentialHunk', 'DifferentialLineAdjustmentMap' => 'Phobject', - 'DifferentialLintField' => 'DifferentialCustomField', + 'DifferentialLintField' => 'DifferentialHarbormasterField', 'DifferentialLintStatus' => 'Phobject', 'DifferentialLocalCommitsView' => 'AphrontView', 'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField', @@ -3844,7 +3846,7 @@ phutil_register_library_map(array( 'DifferentialTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView', - 'DifferentialUnitField' => 'DifferentialCustomField', + 'DifferentialUnitField' => 'DifferentialHarbormasterField', 'DifferentialUnitStatus' => 'Phobject', 'DifferentialUnitTestResult' => 'Phobject', 'DifferentialUpdateRevisionConduitAPIMethod' => 'DifferentialConduitAPIMethod', diff --git a/src/applications/differential/customfield/DifferentialHarbormasterField.php b/src/applications/differential/customfield/DifferentialHarbormasterField.php new file mode 100644 index 0000000000..5998b8ba82 --- /dev/null +++ b/src/applications/differential/customfield/DifferentialHarbormasterField.php @@ -0,0 +1,96 @@ +getDiffPropertyKeys(); + + $properties = id(new DifferentialDiffProperty())->loadAllWhere( + 'diffID = %d AND name IN (%Ls)', + $diff->getID(), + $keys); + $properties = mpull($properties, 'getData', 'getName'); + + foreach ($keys as $key) { + $diff->attachProperty($key, idx($properties, $key)); + } + + $messages = array(); + + $buildable = $diff->getBuildable(); + if ($buildable) { + $target_phids = array(); + foreach ($buildable->getBuilds() as $build) { + foreach ($build->getBuildTargets() as $target) { + $target_phids[] = $target->getPHID(); + } + } + + if ($target_phids) { + $messages = $this->loadHarbormasterTargetMessages($target_phids); + } + } + + if (!$messages) { + // No Harbormaster messages, so look for legacy messages and make them + // look like modern messages. + $legacy_messages = $diff->getProperty($this->getLegacyProperty()); + if ($legacy_messages) { + // 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); + + foreach ($legacy_messages as $message) { + try { + $modern = $this->newModernMessage($message); + $messages[] = $modern; + } catch (Exception $ex) { + // Ignore any poorly formatted messages. + } + } + } + } + + $status = $this->renderHarbormasterStatus($diff, $messages); + + if ($messages) { + $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename'); + foreach ($path_map as $path => $id) { + $href = '#C'.$id.'NL'; + + // TODO: When the diff is not the right-hand-size diff, we should + // ideally adjust this URI to be absolute. + + $path_map[$path] = $href; + } + + $view = $this->newHarbormasterMessageView($messages) + ->setPathURIMap($path_map); + } else { + $view = null; + } + + return array( + $status, + $view, + ); + } + +} diff --git a/src/applications/differential/customfield/DifferentialLintField.php b/src/applications/differential/customfield/DifferentialLintField.php index 6681dd035d..87cf97f109 100644 --- a/src/applications/differential/customfield/DifferentialLintField.php +++ b/src/applications/differential/customfield/DifferentialLintField.php @@ -1,7 +1,7 @@ getFieldName(); } - public function renderDiffPropertyViewValue(DifferentialDiff $diff) { - // TODO: This load is slightly inefficient, but most of this is moving - // to Harbormaster and this simplifies the transition. Eat 1-2 extra - // queries for now. - $keys = array( + protected function getLegacyProperty() { + return 'arc:lint'; + } + + protected function getDiffPropertyKeys() { + return array( 'arc:lint', 'arc:lint-excuse', ); + } - $properties = id(new DifferentialDiffProperty())->loadAllWhere( - 'diffID = %d AND name IN (%Ls)', - $diff->getID(), - $keys); - $properties = mpull($properties, 'getData', 'getName'); + protected function loadHarbormasterTargetMessages(array $target_phids) { + return id(new HarbormasterBuildLintMessage())->loadAllWhere( + 'buildTargetPHID IN (%Ls) LIMIT 25', + $target_phids); + } - foreach ($keys as $key) { - $diff->attachProperty($key, idx($properties, $key)); - } + protected function newHarbormasterMessageView(array $messages) { + return id(new HarbormasterLintPropertyView()) + ->setLintMessages($messages); + } - $status = $this->renderLintStatus($diff); - - $lint = array(); - - $buildable = $diff->getBuildable(); - if ($buildable) { - $target_phids = array(); - foreach ($buildable->getBuilds() as $build) { - foreach ($build->getBuildTargets() as $target) { - $target_phids[] = $target->getPHID(); - } - } - - $lint = id(new HarbormasterBuildLintMessage())->loadAllWhere( - 'buildTargetPHID IN (%Ls) LIMIT 25', - $target_phids); - } - - if (!$lint) { - // No Harbormaster messages, so look for legacy messages and make them - // look like modern messages. - $legacy_lint = $diff->getProperty('arc:lint'); - if ($legacy_lint) { - // 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_lint = array_slice($legacy_lint, 0, 100); - - $target = new HarbormasterBuildTarget(); - foreach ($legacy_lint as $message) { - try { - $modern = HarbormasterBuildLintMessage::newFromDictionary( - $target, - $this->getModernLintMessageDictionary($message)); - $lint[] = $modern; - } catch (Exception $ex) { - // Ignore any poorly formatted messages. - } - } - } - } - - if ($lint) { - $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename'); - foreach ($path_map as $path => $id) { - $href = '#C'.$id.'NL'; - - // TODO: When the diff is not the right-hand-size diff, we should - // ideally adjust this URI to be absolute. - - $path_map[$path] = $href; - } - - $view = id(new HarbormasterLintPropertyView()) - ->setPathURIMap($path_map) - ->setLintMessages($lint); - } else { - $view = null; - } - - return array( - $status, - $view, - ); + protected function newModernMessage(array $message) { + return HarbormasterBuildLintMessage::newFromDictionary( + new HarbormasterBuildTarget(), + $this->getModernLintMessageDictionary($message)); } public function getWarningsForDetailView() { @@ -141,7 +82,10 @@ final class DifferentialLintField return $warnings; } - private function renderLintStatus(DifferentialDiff $diff) { + protected function renderHarbormasterStatus( + DifferentialDiff $diff, + array $messages) { + $colors = array( DifferentialLintStatus::LINT_NONE => 'grey', DifferentialLintStatus::LINT_OKAY => 'green', diff --git a/src/applications/differential/customfield/DifferentialUnitField.php b/src/applications/differential/customfield/DifferentialUnitField.php index 36798b7eb3..544dbfd053 100644 --- a/src/applications/differential/customfield/DifferentialUnitField.php +++ b/src/applications/differential/customfield/DifferentialUnitField.php @@ -1,7 +1,7 @@ getFieldName(); } - public function renderDiffPropertyViewValue(DifferentialDiff $diff) { - // TODO: See DifferentialLintField. - $keys = array( + protected function getLegacyProperty() { + return 'arc:unit'; + } + + protected function getDiffPropertyKeys() { + return array( 'arc:unit', 'arc:unit-excuse', ); + } - $properties = id(new DifferentialDiffProperty())->loadAllWhere( - 'diffID = %d AND name IN (%Ls)', - $diff->getID(), - $keys); - $properties = mpull($properties, 'getData', 'getName'); + protected function loadHarbormasterTargetMessages(array $target_phids) { + return id(new HarbormasterBuildUnitMessage())->loadAllWhere( + 'buildTargetPHID IN (%Ls) LIMIT 25', + $target_phids); + } - foreach ($keys as $key) { - $diff->attachProperty($key, idx($properties, $key)); - } + protected function newModernMessage(array $message) { + return HarbormasterBuildUnitMessage::newFromDictionary( + new HarbormasterBuildTarget(), + $this->getModernUnitMessageDictionary($message)); + } - $status = $this->renderUnitStatus($diff); - - $unit = array(); - - $buildable = $diff->getBuildable(); - if ($buildable) { - $target_phids = array(); - foreach ($buildable->getBuilds() as $build) { - foreach ($build->getBuildTargets() as $target) { - $target_phids[] = $target->getPHID(); - } - } - - $unit = id(new HarbormasterBuildUnitMessage())->loadAllWhere( - 'buildTargetPHID IN (%Ls) LIMIT 25', - $target_phids); - } - - if (!$unit) { - $legacy_unit = $diff->getProperty('arc:unit'); - if ($legacy_unit) { - // Show the top 100 legacy unit messages. - $legacy_unit = array_slice($legacy_unit, 0, 100); - - $target = new HarbormasterBuildTarget(); - foreach ($legacy_unit as $message) { - try { - $modern = HarbormasterBuildUnitMessage::newFromDictionary( - $target, - $this->getModernUnitMessageDictionary($message)); - $unit[] = $modern; - } catch (Exception $ex) { - // Just ignore it if legacy messages aren't formatted like - // we expect. - } - } - } - } - - if ($unit) { - $path_map = mpull($diff->loadChangesets(), 'getID', 'getFilename'); - foreach ($path_map as $path => $id) { - $href = '#C'.$id.'NL'; - - // TODO: When the diff is not the right-hand-size diff, we should - // ideally adjust this URI to be absolute. - - $path_map[$path] = $href; - } - - $view = id(new HarbormasterUnitPropertyView()) - ->setPathURIMap($path_map) - ->setUnitMessages($unit); - } else { - $view = null; - } - - return array( - $status, - $view, - ); + protected function newHarbormasterMessageView(array $messages) { + return id(new HarbormasterUnitPropertyView()) + ->setUnitMessages($messages); } public function getWarningsForDetailView() { @@ -132,8 +80,10 @@ final class DifferentialUnitField return $warnings; } + protected function renderHarbormasterStatus( + DifferentialDiff $diff, + array $messages) { - private function renderUnitStatus(DifferentialDiff $diff) { $colors = array( DifferentialUnitStatus::UNIT_NONE => 'grey', DifferentialUnitStatus::UNIT_OKAY => 'green', From 9656e6e6b1b26698e9eafa19e5950a6392ce6c7b Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Jun 2015 13:33:53 -0700 Subject: [PATCH 4/6] Link prominently to Harbormaster Buildables in build result output Summary: Ref T8096. We don't currently link to the buildable, which I think contributes to Harbormaster feeling a little scattered. Test Plan: {F528095} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8096 Differential Revision: https://secure.phabricator.com/D13405 --- .../event/HarbormasterUIEventListener.php | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/applications/harbormaster/event/HarbormasterUIEventListener.php b/src/applications/harbormaster/event/HarbormasterUIEventListener.php index e965e67fb6..d0a79f4b12 100644 --- a/src/applications/harbormaster/event/HarbormasterUIEventListener.php +++ b/src/applications/harbormaster/event/HarbormasterUIEventListener.php @@ -51,22 +51,17 @@ final class HarbormasterUIEventListener return; } - $buildables = id(new HarbormasterBuildableQuery()) + $buildable = id(new HarbormasterBuildableQuery()) ->setViewer($user) ->withManualBuildables(false) ->withBuildablePHIDs(array($buildable_phid)) - ->execute(); - if (!$buildables) { + ->needBuilds(true) + ->executeOne(); + if (!$buildable) { return; } - $builds = id(new HarbormasterBuildQuery()) - ->setViewer($user) - ->withBuildablePHIDs(mpull($buildables, 'getPHID')) - ->execute(); - if (!$builds) { - return; - } + $builds = $buildable->getBuilds(); $build_handles = id(new PhabricatorHandleQuery()) ->setViewer($user) @@ -75,6 +70,29 @@ final class HarbormasterUIEventListener $status_view = new PHUIStatusListView(); + $buildable_status = $buildable->getBuildableStatus(); + $buildable_icon = HarbormasterBuildable::getBuildableStatusIcon( + $buildable_status); + $buildable_color = HarbormasterBuildable::getBuildableStatusColor( + $buildable_status); + $buildable_name = HarbormasterBuildable::getBuildableStatusName( + $buildable_status); + + $target = phutil_tag( + 'a', + array( + 'href' => '/'.$buildable->getMonogram(), + ), + pht('Buildable %d', $buildable->getID())); + + $target = phutil_tag('strong', array(), $target); + + $status_view + ->addItem( + id(new PHUIStatusItemView()) + ->setIcon($buildable_icon, $buildable_color, $buildable_name) + ->setTarget($target)); + foreach ($builds as $build) { $item = new PHUIStatusItemView(); $item->setTarget($build_handles[$build->getPHID()]->renderLink()); From 716bd4e4b41b1a27cebf48ad4e651a29b9bf014c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Jun 2015 13:34:25 -0700 Subject: [PATCH 5/6] Improve lint/unit limit, sort, view all, collapse behaviors Summary: Ref T8096. Various tweaks here: - Sort result lists by importance (even lint -- "errors first" seems better than "alphabetical by file", I think?). - Do sane stuff with display limits. - Add a "view all" view. - Don't show a huge table of passing tests in Differential. - Link to full results. Test Plan: See screenshots. Reviewers: chad, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8096 Differential Revision: https://secure.phabricator.com/D13407 --- resources/celerity/map.php | 6 +- src/__phutil_library_map__.php | 4 ++ .../DifferentialHarbormasterField.php | 15 +++- .../customfield/DifferentialLintField.php | 1 + .../customfield/DifferentialUnitField.php | 68 ++++++++++++++++++- .../PhabricatorHarbormasterApplication.php | 6 ++ .../HarbormasterBuildableViewController.php | 39 +++++++++-- .../HarbormasterLintMessagesController.php | 64 +++++++++++++++++ .../HarbormasterUnitMessagesController.php | 64 +++++++++++++++++ .../build/HarbormasterBuildLintMessage.php | 22 ++++++ .../build/HarbormasterBuildUnitMessage.php | 22 ++++++ .../view/HarbormasterLintPropertyView.php | 21 ++++-- .../view/HarbormasterUnitPropertyView.php | 15 +++- .../PhabricatorUSEnglishTranslation.php | 7 ++ .../differential/table-of-contents.css | 5 ++ 15 files changed, 340 insertions(+), 19 deletions(-) create mode 100644 src/applications/harbormaster/controller/HarbormasterLintMessagesController.php create mode 100644 src/applications/harbormaster/controller/HarbormasterUnitMessagesController.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index a838caceee..6bc17f938e 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'core.pkg.css' => 'eb51e6dc', 'core.pkg.js' => '711e63c0', 'darkconsole.pkg.js' => 'e7393ebb', - 'differential.pkg.css' => '1ca3c116', + 'differential.pkg.css' => '7b52b9be', 'differential.pkg.js' => 'ebef29b1', 'diffusion.pkg.css' => '591664fa', 'diffusion.pkg.js' => '0115b37c', @@ -64,7 +64,7 @@ return array( 'rsrc/css/application/differential/revision-comment.css' => '14b8565a', 'rsrc/css/application/differential/revision-history.css' => '0e8eb855', 'rsrc/css/application/differential/revision-list.css' => 'f3c47d33', - 'rsrc/css/application/differential/table-of-contents.css' => '63f3ef4a', + 'rsrc/css/application/differential/table-of-contents.css' => 'ae4b7a55', 'rsrc/css/application/diffusion/diffusion-icons.css' => '9c5828da', 'rsrc/css/application/diffusion/diffusion-readme.css' => '2106ea08', 'rsrc/css/application/diffusion/diffusion-source.css' => '66fdf661', @@ -515,7 +515,7 @@ return array( 'differential-revision-comment-css' => '14b8565a', 'differential-revision-history-css' => '0e8eb855', 'differential-revision-list-css' => 'f3c47d33', - 'differential-table-of-contents-css' => '63f3ef4a', + 'differential-table-of-contents-css' => 'ae4b7a55', 'diffusion-icons-css' => '9c5828da', 'diffusion-readme-css' => '2106ea08', 'diffusion-source-css' => '66fdf661', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index aa55cfbf3a..2b06ac7d6e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -891,6 +891,7 @@ phutil_register_library_map(array( 'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php', 'HarbormasterHTTPRequestBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php', 'HarbormasterLeaseHostBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php', + 'HarbormasterLintMessagesController' => 'applications/harbormaster/controller/HarbormasterLintMessagesController.php', 'HarbormasterLintPropertyView' => 'applications/harbormaster/view/HarbormasterLintPropertyView.php', 'HarbormasterManagePlansCapability' => 'applications/harbormaster/capability/HarbormasterManagePlansCapability.php', 'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php', @@ -919,6 +920,7 @@ phutil_register_library_map(array( 'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php', 'HarbormasterThrowExceptionBuildStep' => 'applications/harbormaster/step/HarbormasterThrowExceptionBuildStep.php', 'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php', + 'HarbormasterUnitMessagesController' => 'applications/harbormaster/controller/HarbormasterUnitMessagesController.php', 'HarbormasterUnitPropertyView' => 'applications/harbormaster/view/HarbormasterUnitPropertyView.php', 'HarbormasterUploadArtifactBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterUploadArtifactBuildStepImplementation.php', 'HarbormasterWaitForPreviousBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterWaitForPreviousBuildStepImplementation.php', @@ -4357,6 +4359,7 @@ phutil_register_library_map(array( 'HarbormasterDAO' => 'PhabricatorLiskDAO', 'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation', + 'HarbormasterLintMessagesController' => 'HarbormasterController', 'HarbormasterLintPropertyView' => 'AphrontView', 'HarbormasterManagePlansCapability' => 'PhabricatorPolicyCapability', 'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow', @@ -4385,6 +4388,7 @@ phutil_register_library_map(array( 'HarbormasterTargetWorker' => 'HarbormasterWorker', 'HarbormasterThrowExceptionBuildStep' => 'HarbormasterBuildStepImplementation', 'HarbormasterUIEventListener' => 'PhabricatorEventListener', + 'HarbormasterUnitMessagesController' => 'HarbormasterController', 'HarbormasterUnitPropertyView' => 'AphrontView', 'HarbormasterUploadArtifactBuildStepImplementation' => 'HarbormasterBuildStepImplementation', 'HarbormasterWaitForPreviousBuildStepImplementation' => 'HarbormasterBuildStepImplementation', diff --git a/src/applications/differential/customfield/DifferentialHarbormasterField.php b/src/applications/differential/customfield/DifferentialHarbormasterField.php index 5998b8ba82..f84bb74f29 100644 --- a/src/applications/differential/customfield/DifferentialHarbormasterField.php +++ b/src/applications/differential/customfield/DifferentialHarbormasterField.php @@ -81,12 +81,23 @@ abstract class DifferentialHarbormasterField $path_map[$path] = $href; } - $view = $this->newHarbormasterMessageView($messages) - ->setPathURIMap($path_map); + $view = $this->newHarbormasterMessageView($messages); + if ($view) { + $view->setPathURIMap($path_map); + } } else { $view = null; } + if ($view) { + $view = phutil_tag( + 'div', + array( + 'class' => 'differential-harbormaster-table-view', + ), + $view); + } + return array( $status, $view, diff --git a/src/applications/differential/customfield/DifferentialLintField.php b/src/applications/differential/customfield/DifferentialLintField.php index 87cf97f109..52be0e393c 100644 --- a/src/applications/differential/customfield/DifferentialLintField.php +++ b/src/applications/differential/customfield/DifferentialLintField.php @@ -50,6 +50,7 @@ final class DifferentialLintField protected function newHarbormasterMessageView(array $messages) { return id(new HarbormasterLintPropertyView()) + ->setLimit(25) ->setLintMessages($messages); } diff --git a/src/applications/differential/customfield/DifferentialUnitField.php b/src/applications/differential/customfield/DifferentialUnitField.php index 544dbfd053..aa2edc4622 100644 --- a/src/applications/differential/customfield/DifferentialUnitField.php +++ b/src/applications/differential/customfield/DifferentialUnitField.php @@ -44,7 +44,7 @@ final class DifferentialUnitField protected function loadHarbormasterTargetMessages(array $target_phids) { return id(new HarbormasterBuildUnitMessage())->loadAllWhere( - 'buildTargetPHID IN (%Ls) LIMIT 25', + 'buildTargetPHID IN (%Ls)', $target_phids); } @@ -55,7 +55,19 @@ final class DifferentialUnitField } protected function newHarbormasterMessageView(array $messages) { + foreach ($messages as $key => $message) { + if ($message->getResult() == ArcanistUnitTestResult::RESULT_PASS) { + unset($messages[$key]); + } + } + + if (!$messages) { + return null; + } + return id(new HarbormasterUnitPropertyView()) + ->setLimit(10) + ->setHidePassingTests(true) ->setUnitMessages($messages); } @@ -97,6 +109,55 @@ final class DifferentialUnitField $message = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff); + $note = array(); + + $groups = mgroup($messages, 'getResult'); + + $groups = array_select_keys( + $groups, + array( + ArcanistUnitTestResult::RESULT_FAIL, + ArcanistUnitTestResult::RESULT_BROKEN, + ArcanistUnitTestResult::RESULT_UNSOUND, + ArcanistUnitTestResult::RESULT_SKIP, + ArcanistUnitTestResult::RESULT_PASS, + )) + $groups; + + foreach ($groups as $result => $group) { + $count = new PhutilNumber(count($group)); + switch ($result) { + case ArcanistUnitTestResult::RESULT_PASS: + $note[] = pht('%s Passed Test(s)', $count); + break; + case ArcanistUnitTestResult::RESULT_FAIL: + $note[] = pht('%s Failed Test(s)', $count); + break; + case ArcanistUnitTestResult::RESULT_SKIP: + $note[] = pht('%s Skipped Test(s)', $count); + break; + case ArcanistUnitTestResult::RESULT_BROKEN: + $note[] = pht('%s Broken Test(s)', $count); + break; + case ArcanistUnitTestResult::RESULT_UNSOUND: + $note[] = pht('%s Unsound Test(s)', $count); + break; + default: + $note[] = pht('%s Other Test(s)', $count); + break; + } + } + + $buildable = $diff->getBuildable(); + if ($buildable) { + $full_results = '/harbormaster/unit/'.$buildable->getID().'/'; + $note[] = phutil_tag( + 'a', + array( + 'href' => $full_results, + ), + pht('View Full Results')); + } + $excuse = $diff->getProperty('arc:unit-excuse'); if (strlen($excuse)) { $excuse = array( @@ -104,14 +165,17 @@ final class DifferentialUnitField ' ', phutil_escape_html_newlines($excuse), ); + $note[] = $excuse; } + $note = phutil_implode_html(" \xC2\xB7 ", $note); + $status = id(new PHUIStatusListView()) ->addItem( id(new PHUIStatusItemView()) ->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color) ->setTarget($message) - ->setNote($excuse)); + ->setNote($note)); return $status; } diff --git a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php index fd491f707b..016e2a5cbc 100644 --- a/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php +++ b/src/applications/harbormaster/application/PhabricatorHarbormasterApplication.php @@ -77,6 +77,12 @@ final class PhabricatorHarbormasterApplication extends PhabricatorApplication { 'run/(?P\d+)/' => 'HarbormasterPlanRunController', '(?P\d+)/' => 'HarbormasterPlanViewController', ), + 'unit/' => array( + '(?P\d+)/' => 'HarbormasterUnitMessagesController', + ), + 'lint/' => array( + '(?P\d+)/' => 'HarbormasterLintMessagesController', + ), ), ); } diff --git a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php index 461425ba3d..ab779758e8 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildableViewController.php @@ -25,7 +25,7 @@ final class HarbormasterBuildableViewController ->needBuildTargets(true) ->execute(); - list($lint, $unit) = $this->renderLintAndUnit($builds); + list($lint, $unit) = $this->renderLintAndUnit($buildable, $builds); $buildable->attachBuilds($builds); $object = $buildable->getBuildableObject(); @@ -255,7 +255,10 @@ final class HarbormasterBuildableViewController return $box; } - private function renderLintAndUnit(array $builds) { + private function renderLintAndUnit( + HarbormasterBuildable $buildable, + array $builds) { + $viewer = $this->getViewer(); $targets = array(); @@ -272,20 +275,32 @@ final class HarbormasterBuildableViewController $target_phids = mpull($targets, 'getPHID'); $lint_data = id(new HarbormasterBuildLintMessage())->loadAllWhere( - 'buildTargetPHID IN (%Ls) LIMIT 25', + 'buildTargetPHID IN (%Ls)', $target_phids); $unit_data = id(new HarbormasterBuildUnitMessage())->loadAllWhere( - 'buildTargetPHID IN (%Ls) LIMIT 25', + 'buildTargetPHID IN (%Ls)', $target_phids); if ($lint_data) { $lint_table = id(new HarbormasterLintPropertyView()) ->setUser($viewer) + ->setLimit(10) ->setLintMessages($lint_data); + $lint_href = $this->getApplicationURI('lint/'.$buildable->getID().'/'); + + $lint_header = id(new PHUIHeaderView()) + ->setHeader(pht('Lint Messages')) + ->addActionLink( + id(new PHUIButtonView()) + ->setTag('a') + ->setHref($lint_href) + ->setIconFont('fa-list-ul') + ->setText('View All')); + $lint = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Lint Messages')) + ->setHeader($lint_header) ->appendChild($lint_table); } else { $lint = null; @@ -294,10 +309,22 @@ final class HarbormasterBuildableViewController if ($unit_data) { $unit_table = id(new HarbormasterUnitPropertyView()) ->setUser($viewer) + ->setLimit(25) ->setUnitMessages($unit_data); + $unit_href = $this->getApplicationURI('unit/'.$buildable->getID().'/'); + + $unit_header = id(new PHUIHeaderView()) + ->setHeader(pht('Unit Tests')) + ->addActionLink( + id(new PHUIButtonView()) + ->setTag('a') + ->setHref($unit_href) + ->setIconFont('fa-list-ul') + ->setText('View All')); + $unit = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Unit Tests')) + ->setHeader($unit_header) ->appendChild($unit_table); } else { $unit = null; diff --git a/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php b/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php new file mode 100644 index 0000000000..befe0e450b --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterLintMessagesController.php @@ -0,0 +1,64 @@ +getViewer(); + + $buildable = id(new HarbormasterBuildableQuery()) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) + ->needBuilds(true) + ->needTargets(true) + ->executeOne(); + if (!$buildable) { + return new Aphront404Response(); + } + + $id = $buildable->getID(); + + $target_phids = array(); + foreach ($buildable->getBuilds() as $build) { + foreach ($build->getBuildTargets() as $target) { + $target_phids[] = $target->getPHID(); + } + } + + $lint_data = array(); + if ($target_phids) { + $lint_data = id(new HarbormasterBuildLintMessage())->loadAllWhere( + 'buildTargetPHID IN (%Ls)', + $target_phids); + } else { + $lint_data = array(); + } + + $lint_table = id(new HarbormasterLintPropertyView()) + ->setUser($viewer) + ->setLintMessages($lint_data); + + $lint = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Lint Messages')) + ->appendChild($lint_table); + + $crumbs = $this->buildApplicationCrumbs(); + $this->addBuildableCrumb($crumbs, $buildable); + $crumbs->addTextCrumb(pht('Lint')); + + $title = array( + $buildable->getMonogram(), + pht('Lint'), + ); + + return $this->buildApplicationPage( + array( + $crumbs, + $lint, + ), + array( + 'title' => $title, + )); + } + +} diff --git a/src/applications/harbormaster/controller/HarbormasterUnitMessagesController.php b/src/applications/harbormaster/controller/HarbormasterUnitMessagesController.php new file mode 100644 index 0000000000..cf56f8e4d3 --- /dev/null +++ b/src/applications/harbormaster/controller/HarbormasterUnitMessagesController.php @@ -0,0 +1,64 @@ +getViewer(); + + $buildable = id(new HarbormasterBuildableQuery()) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) + ->needBuilds(true) + ->needTargets(true) + ->executeOne(); + if (!$buildable) { + return new Aphront404Response(); + } + + $id = $buildable->getID(); + + $target_phids = array(); + foreach ($buildable->getBuilds() as $build) { + foreach ($build->getBuildTargets() as $target) { + $target_phids[] = $target->getPHID(); + } + } + + $unit_data = array(); + if ($target_phids) { + $unit_data = id(new HarbormasterBuildUnitMessage())->loadAllWhere( + 'buildTargetPHID IN (%Ls)', + $target_phids); + } else { + $unit_data = array(); + } + + $unit_table = id(new HarbormasterUnitPropertyView()) + ->setUser($viewer) + ->setUnitMessages($unit_data); + + $unit = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Unit Tests')) + ->appendChild($unit_table); + + $crumbs = $this->buildApplicationCrumbs(); + $this->addBuildableCrumb($crumbs, $buildable); + $crumbs->addTextCrumb(pht('Unit Tests')); + + $title = array( + $buildable->getMonogram(), + pht('Unit Tests'), + ); + + return $this->buildApplicationPage( + array( + $crumbs, + $unit, + ), + array( + 'title' => $title, + )); + } + +} diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php index 27b0948af7..c9957b0149 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildLintMessage.php @@ -95,4 +95,26 @@ final class HarbormasterBuildLintMessage return $this; } + public function getSortKey() { + // TODO: Maybe use more numeric values after T6861. + $map = array( + ArcanistLintSeverity::SEVERITY_ERROR => 'A', + ArcanistLintSeverity::SEVERITY_WARNING => 'B', + ArcanistLintSeverity::SEVERITY_AUTOFIX => 'C', + ArcanistLintSeverity::SEVERITY_ADVICE => 'Y', + ArcanistLintSeverity::SEVERITY_DISABLED => 'Z', + ); + + $severity = idx($map, $this->getSeverity(), 'N'); + + $parts = array( + $severity, + $this->getPath(), + sprintf('%08d', $this->getLine()), + $this->getCode(), + ); + + return implode("\0", $parts); + } + } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php index 3ee67e7280..7e53cad291 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php @@ -97,4 +97,26 @@ final class HarbormasterBuildUnitMessage return $this; } + public function getSortKey() { + // TODO: Maybe use more numeric values after T6861. + $map = array( + ArcanistUnitTestResult::RESULT_FAIL => 'A', + ArcanistUnitTestResult::RESULT_BROKEN => 'B', + ArcanistUnitTestResult::RESULT_UNSOUND => 'C', + ArcanistUnitTestResult::RESULT_PASS => 'Z', + ); + + $result = idx($map, $this->getResult(), 'N'); + + $parts = array( + $result, + $this->getEngine(), + $this->getNamespace(), + $this->getName(), + $this->getID(), + ); + + return implode("\0", $parts); + } + } diff --git a/src/applications/harbormaster/view/HarbormasterLintPropertyView.php b/src/applications/harbormaster/view/HarbormasterLintPropertyView.php index f2591daae5..abfaa7e222 100644 --- a/src/applications/harbormaster/view/HarbormasterLintPropertyView.php +++ b/src/applications/harbormaster/view/HarbormasterLintPropertyView.php @@ -4,6 +4,7 @@ final class HarbormasterLintPropertyView extends AphrontView { private $pathURIMap = array(); private $lintMessages = array(); + private $limit; public function setPathURIMap(array $map) { $this->pathURIMap = $map; @@ -16,9 +17,21 @@ final class HarbormasterLintPropertyView extends AphrontView { return $this; } + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + public function render() { + $messages = $this->lintMessages; + $messages = msort($messages, 'getSortKey'); + + if ($this->limit) { + $messages = array_slice($messages, 0, $this->limit); + } + $rows = array(); - foreach ($this->lintMessages as $message) { + foreach ($messages as $message) { $path = $message->getPath(); $line = $message->getLine(); @@ -40,8 +53,8 @@ final class HarbormasterLintPropertyView extends AphrontView { } $rows[] = array( - $location, $severity, + $location, $message->getCode(), $message->getName(), ); @@ -50,15 +63,15 @@ final class HarbormasterLintPropertyView extends AphrontView { $table = id(new AphrontTableView($rows)) ->setHeaders( array( - pht('Location'), pht('Severity'), + pht('Location'), pht('Code'), pht('Message'), )) ->setColumnClasses( array( - 'pri', null, + 'pri', null, 'wide', )); diff --git a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php index 8bff9744f7..107272246d 100644 --- a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php +++ b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php @@ -4,6 +4,7 @@ final class HarbormasterUnitPropertyView extends AphrontView { private $pathURIMap = array(); private $unitMessages = array(); + private $limit; public function setPathURIMap(array $map) { $this->pathURIMap = $map; @@ -16,11 +17,22 @@ final class HarbormasterUnitPropertyView extends AphrontView { return $this; } + public function setLimit($limit) { + $this->limit = $limit; + return $this; + } + public function render() { + $messages = $this->unitMessages; + $messages = msort($messages, 'getSortKey'); + + if ($this->limit) { + $messages = array_slice($messages, 0, $this->limit); + } $rows = array(); $any_duration = false; - foreach ($this->unitMessages as $message) { + foreach ($messages as $message) { $result = $this->renderResult($message->getResult()); $duration = $message->getDuration(); @@ -48,7 +60,6 @@ final class HarbormasterUnitPropertyView extends AphrontView { ); } - $table = id(new AphrontTableView($rows)) ->setHeaders( array( diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index b13dfe1b50..64198ae14e 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1170,6 +1170,13 @@ final class PhabricatorUSEnglishTranslation 'This call takes %s parameters, but only %s are documented.', ), ), + + '%s Passed Test(s)' => '%s Passed', + '%s Failed Test(s)' => '%s Failed', + '%s Skipped Test(s)' => '%s Skipped', + '%s Broken Test(s)' => '%s Broken', + '%s Unsound Test(s)' => '%s Unsound', + '%s Other Test(s)' => '%s Other', ); } diff --git a/webroot/rsrc/css/application/differential/table-of-contents.css b/webroot/rsrc/css/application/differential/table-of-contents.css index 9e8e0a088f..b769776301 100644 --- a/webroot/rsrc/css/application/differential/table-of-contents.css +++ b/webroot/rsrc/css/application/differential/table-of-contents.css @@ -83,3 +83,8 @@ table.aphront-table-view td.differential-toc-ftype { border-top: 1px solid {$thinblueborder}; padding: 8px; } + +.differential-harbormaster-table-view { + margin: 4px 0; + border: 1px solid {$thinblueborder}; +} From 3215899925e2b9a5bdb82be374303db869957593 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Jun 2015 13:36:16 -0700 Subject: [PATCH 6/6] Execute Maniphest batch edits in the background with a web UI progress bar Summary: Ref T8637. This does nothing interesting, just has empty scaffolding for a bulk job queue. Basic idea is that when you do something like a batch edit in Maniphest, we: - Create a BulkJob with all the details. - Queue a worker to start the job. - Send you to a progress bar page for the job. In the background: - The "start job" worker creates a ton of Task objects, then queues worker tasks to do the work. In the foreground: - Fancy ajax animates the progress bar and it goes wooosh. In general: - Big jobs actually work. - Jobs get logged. - You can monitor jobs. - Terrible junk like T8637 should be much harder to write and much easier to catch and diagnose. Test Plan: No interesting code/beahavior yet. Clean `storage adjust`. {F526411} Reviewers: chad, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8637 Differential Revision: https://secure.phabricator.com/D13392 --- resources/celerity/map.php | 8 + .../sql/autopatches/20150622.bulk.1.job.sql | 15 + .../sql/autopatches/20150622.bulk.2.task.sql | 9 + .../autopatches/20150622.bulk.3.xaction.sql | 19 ++ .../sql/autopatches/20150622.bulk.4.edge.sql | 16 + src/__phutil_library_map__.php | 42 +++ .../PhabricatorDaemonsApplication.php | 9 + ...PhabricatorDaemonBulkJobListController.php | 31 ++ ...bricatorDaemonBulkJobMonitorController.php | 165 ++++++++++ ...PhabricatorDaemonBulkJobViewController.php | 83 +++++ .../PhabricatorDaemonController.php | 3 + .../bulk/ManiphestTaskEditBulkJobType.php | 296 ++++++++++++++++++ .../ManiphestBatchEditController.php | 268 ++-------------- .../PhabricatorContentSource.php | 2 + .../PhabricatorWorkerBulkJobTestCase.php | 10 + .../PhabricatorWorkerBulkJobCreateWorker.php | 51 +++ .../PhabricatorWorkerBulkJobTaskWorker.php | 46 +++ .../bulk/PhabricatorWorkerBulkJobType.php | 28 ++ .../bulk/PhabricatorWorkerBulkJobWorker.php | 138 ++++++++ .../editor/PhabricatorWorkerBulkJobEditor.php | 87 +++++ .../phid/PhabricatorWorkerBulkJobPHIDType.php | 37 +++ .../query/PhabricatorWorkerBulkJobQuery.php | 106 +++++++ .../PhabricatorWorkerBulkJobSearchEngine.php | 98 ++++++ ...abricatorWorkerBulkJobTransactionQuery.php | 10 + .../storage/PhabricatorWorkerBulkJob.php | 272 ++++++++++++++++ .../PhabricatorWorkerBulkJobTransaction.php | 51 +++ .../storage/PhabricatorWorkerBulkTask.php | 46 +++ .../storage/PhabricatorWorkerSchemaSpec.php | 10 + .../PhabricatorUSEnglishTranslation.php | 5 + .../rsrc/css/application/daemon/bulk-job.css | 32 ++ .../daemon/behavior-bulk-job-reload.js | 18 ++ 31 files changed, 1767 insertions(+), 244 deletions(-) create mode 100644 resources/sql/autopatches/20150622.bulk.1.job.sql create mode 100644 resources/sql/autopatches/20150622.bulk.2.task.sql create mode 100644 resources/sql/autopatches/20150622.bulk.3.xaction.sql create mode 100644 resources/sql/autopatches/20150622.bulk.4.edge.sql create mode 100644 src/applications/daemon/controller/PhabricatorDaemonBulkJobListController.php create mode 100644 src/applications/daemon/controller/PhabricatorDaemonBulkJobMonitorController.php create mode 100644 src/applications/daemon/controller/PhabricatorDaemonBulkJobViewController.php create mode 100644 src/applications/maniphest/bulk/ManiphestTaskEditBulkJobType.php create mode 100644 src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerBulkJobTestCase.php create mode 100644 src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobCreateWorker.php create mode 100644 src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobTaskWorker.php create mode 100644 src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobType.php create mode 100644 src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobWorker.php create mode 100644 src/infrastructure/daemon/workers/editor/PhabricatorWorkerBulkJobEditor.php create mode 100644 src/infrastructure/daemon/workers/phid/PhabricatorWorkerBulkJobPHIDType.php create mode 100644 src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobQuery.php create mode 100644 src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php create mode 100644 src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobTransactionQuery.php create mode 100644 src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJob.php create mode 100644 src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJobTransaction.php create mode 100644 src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkTask.php create mode 100644 src/infrastructure/daemon/workers/storage/PhabricatorWorkerSchemaSpec.php create mode 100644 webroot/rsrc/css/application/daemon/bulk-job.css create mode 100644 webroot/rsrc/js/application/daemon/behavior-bulk-job-reload.js diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 6bc17f938e..ee119787c5 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -55,6 +55,7 @@ return array( 'rsrc/css/application/conpherence/widget-pane.css' => '2af42ebe', 'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4', 'rsrc/css/application/countdown/timer.css' => '86b7b0a0', + 'rsrc/css/application/daemon/bulk-job.css' => 'df9c1d4a', 'rsrc/css/application/dashboard/dashboard.css' => '17937d22', 'rsrc/css/application/diff/inline-comment-summary.css' => '51efda3a', 'rsrc/css/application/differential/add-comment.css' => 'c47f8c40', @@ -343,6 +344,7 @@ return array( 'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '93568464', 'rsrc/js/application/countdown/timer.js' => 'e4cc26b3', + 'rsrc/js/application/daemon/behavior-bulk-job-reload.js' => 'edf8a145', 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '469c0d9e', 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '82439934', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', @@ -495,6 +497,7 @@ return array( 'aphront-two-column-view-css' => '16ab3ad2', 'aphront-typeahead-control-css' => '0e403212', 'auth-css' => '44975d4b', + 'bulk-job-css' => 'df9c1d4a', 'calendar-icon-css' => '98ce946d', 'changeset-view-manager' => '58562350', 'conduit-api-css' => '7bc725c4', @@ -541,6 +544,7 @@ return array( 'javelin-behavior-aphront-more' => 'a80d0378', 'javelin-behavior-audio-source' => '59b251eb', 'javelin-behavior-audit-preview' => 'd835b03a', + 'javelin-behavior-bulk-job-reload' => 'edf8a145', 'javelin-behavior-choose-control' => '6153c708', 'javelin-behavior-config-reorder-fields' => 'b6993408', 'javelin-behavior-conpherence-drag-and-drop-photo' => 'cf86d16a', @@ -1925,6 +1929,10 @@ return array( 'javelin-uri', 'phabricator-notification', ), + 'edf8a145' => array( + 'javelin-behavior', + 'javelin-uri', + ), 'eeaa9e5a' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/resources/sql/autopatches/20150622.bulk.1.job.sql b/resources/sql/autopatches/20150622.bulk.1.job.sql new file mode 100644 index 0000000000..a0cdb5d678 --- /dev/null +++ b/resources/sql/autopatches/20150622.bulk.1.job.sql @@ -0,0 +1,15 @@ +CREATE TABLE {$NAMESPACE}_worker.worker_bulkjob ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + phid VARBINARY(64) NOT NULL, + authorPHID VARBINARY(64) NOT NULL, + jobTypeKey VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL, + status VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL, + parameters LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + size INT UNSIGNED NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_phid` (phid), + KEY `key_type` (jobTypeKey), + KEY `key_author` (authorPHID), + KEY `key_status` (status) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20150622.bulk.2.task.sql b/resources/sql/autopatches/20150622.bulk.2.task.sql new file mode 100644 index 0000000000..f98c205180 --- /dev/null +++ b/resources/sql/autopatches/20150622.bulk.2.task.sql @@ -0,0 +1,9 @@ +CREATE TABLE {$NAMESPACE}_worker.worker_bulktask ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + bulkJobPHID VARBINARY(64) NOT NULL, + objectPHID VARBINARY(64) NOT NULL, + status VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL, + data LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + KEY `key_job` (bulkJobPHID, status), + KEY `key_object` (objectPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20150622.bulk.3.xaction.sql b/resources/sql/autopatches/20150622.bulk.3.xaction.sql new file mode 100644 index 0000000000..27aa2d5caf --- /dev/null +++ b/resources/sql/autopatches/20150622.bulk.3.xaction.sql @@ -0,0 +1,19 @@ +CREATE TABLE {$NAMESPACE}_worker.worker_bulkjobtransaction ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + phid VARBINARY(64) NOT NULL, + authorPHID VARBINARY(64) NOT NULL, + objectPHID VARBINARY(64) NOT NULL, + viewPolicy VARBINARY(64) NOT NULL, + editPolicy VARBINARY(64) NOT NULL, + commentPHID VARBINARY(64) DEFAULT NULL, + commentVersion INT UNSIGNED NOT NULL, + transactionType VARCHAR(32) COLLATE {$COLLATE_TEXT} NOT NULL, + oldValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + newValue LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + contentSource LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + metadata LONGTEXT COLLATE {$COLLATE_TEXT} NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_phid` (`phid`), + KEY `key_object` (`objectPHID`) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20150622.bulk.4.edge.sql b/resources/sql/autopatches/20150622.bulk.4.edge.sql new file mode 100644 index 0000000000..3d81a1bcf6 --- /dev/null +++ b/resources/sql/autopatches/20150622.bulk.4.edge.sql @@ -0,0 +1,16 @@ +CREATE TABLE {$NAMESPACE}_worker.edge ( + src VARBINARY(64) NOT NULL, + type INT UNSIGNED NOT NULL, + dst VARBINARY(64) NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + seq INT UNSIGNED NOT NULL, + dataID INT UNSIGNED, + PRIMARY KEY (src, type, dst), + KEY `src` (src, type, dateCreated, seq), + UNIQUE KEY `key_dst` (dst, type, src) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; + +CREATE TABLE {$NAMESPACE}_worker.edgedata ( + id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + data LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT} +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2b06ac7d6e..72a99b56df 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1082,6 +1082,7 @@ phutil_register_library_map(array( 'ManiphestTaskDependedOnByTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php', 'ManiphestTaskDependsOnTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php', 'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php', + 'ManiphestTaskEditBulkJobType' => 'applications/maniphest/bulk/ManiphestTaskEditBulkJobType.php', 'ManiphestTaskEditController' => 'applications/maniphest/controller/ManiphestTaskEditController.php', 'ManiphestTaskHasCommitEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php', 'ManiphestTaskHasMockEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasMockEdgeType.php', @@ -1708,6 +1709,9 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldStringIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStringIndexStorage.php', 'PhabricatorCustomHeaderConfigType' => 'applications/config/custom/PhabricatorCustomHeaderConfigType.php', 'PhabricatorDaemon' => 'infrastructure/daemon/PhabricatorDaemon.php', + 'PhabricatorDaemonBulkJobListController' => 'applications/daemon/controller/PhabricatorDaemonBulkJobListController.php', + 'PhabricatorDaemonBulkJobMonitorController' => 'applications/daemon/controller/PhabricatorDaemonBulkJobMonitorController.php', + 'PhabricatorDaemonBulkJobViewController' => 'applications/daemon/controller/PhabricatorDaemonBulkJobViewController.php', 'PhabricatorDaemonConsoleController' => 'applications/daemon/controller/PhabricatorDaemonConsoleController.php', 'PhabricatorDaemonController' => 'applications/daemon/controller/PhabricatorDaemonController.php', 'PhabricatorDaemonDAO' => 'applications/daemon/storage/PhabricatorDaemonDAO.php', @@ -2820,6 +2824,19 @@ phutil_register_library_map(array( 'PhabricatorWorkerActiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php', 'PhabricatorWorkerArchiveTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerArchiveTask.php', 'PhabricatorWorkerArchiveTaskQuery' => 'infrastructure/daemon/workers/query/PhabricatorWorkerArchiveTaskQuery.php', + 'PhabricatorWorkerBulkJob' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJob.php', + 'PhabricatorWorkerBulkJobCreateWorker' => 'infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobCreateWorker.php', + 'PhabricatorWorkerBulkJobEditor' => 'infrastructure/daemon/workers/editor/PhabricatorWorkerBulkJobEditor.php', + 'PhabricatorWorkerBulkJobPHIDType' => 'infrastructure/daemon/workers/phid/PhabricatorWorkerBulkJobPHIDType.php', + 'PhabricatorWorkerBulkJobQuery' => 'infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobQuery.php', + 'PhabricatorWorkerBulkJobSearchEngine' => 'infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php', + 'PhabricatorWorkerBulkJobTaskWorker' => 'infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobTaskWorker.php', + 'PhabricatorWorkerBulkJobTestCase' => 'infrastructure/daemon/workers/__tests__/PhabricatorWorkerBulkJobTestCase.php', + 'PhabricatorWorkerBulkJobTransaction' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJobTransaction.php', + 'PhabricatorWorkerBulkJobTransactionQuery' => 'infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobTransactionQuery.php', + 'PhabricatorWorkerBulkJobType' => 'infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobType.php', + 'PhabricatorWorkerBulkJobWorker' => 'infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobWorker.php', + 'PhabricatorWorkerBulkTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerBulkTask.php', 'PhabricatorWorkerDAO' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerDAO.php', 'PhabricatorWorkerLeaseQuery' => 'infrastructure/daemon/workers/query/PhabricatorWorkerLeaseQuery.php', 'PhabricatorWorkerManagementCancelWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementCancelWorkflow.php', @@ -2829,6 +2846,7 @@ phutil_register_library_map(array( 'PhabricatorWorkerManagementRetryWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementRetryWorkflow.php', 'PhabricatorWorkerManagementWorkflow' => 'infrastructure/daemon/workers/management/PhabricatorWorkerManagementWorkflow.php', 'PhabricatorWorkerPermanentFailureException' => 'infrastructure/daemon/workers/exception/PhabricatorWorkerPermanentFailureException.php', + 'PhabricatorWorkerSchemaSpec' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerSchemaSpec.php', 'PhabricatorWorkerTask' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerTask.php', 'PhabricatorWorkerTaskData' => 'infrastructure/daemon/workers/storage/PhabricatorWorkerTaskData.php', 'PhabricatorWorkerTaskDetailController' => 'applications/daemon/controller/PhabricatorWorkerTaskDetailController.php', @@ -4589,6 +4607,7 @@ phutil_register_library_map(array( 'ManiphestTaskDependedOnByTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskDependsOnTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskDetailController' => 'ManiphestController', + 'ManiphestTaskEditBulkJobType' => 'PhabricatorWorkerBulkJobType', 'ManiphestTaskEditController' => 'ManiphestController', 'ManiphestTaskHasCommitEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasMockEdgeType' => 'PhabricatorEdgeType', @@ -5300,6 +5319,9 @@ phutil_register_library_map(array( 'PhabricatorCustomFieldStringIndexStorage' => 'PhabricatorCustomFieldIndexStorage', 'PhabricatorCustomHeaderConfigType' => 'PhabricatorConfigOptionType', 'PhabricatorDaemon' => 'PhutilDaemon', + 'PhabricatorDaemonBulkJobListController' => 'PhabricatorDaemonController', + 'PhabricatorDaemonBulkJobMonitorController' => 'PhabricatorDaemonController', + 'PhabricatorDaemonBulkJobViewController' => 'PhabricatorDaemonController', 'PhabricatorDaemonConsoleController' => 'PhabricatorDaemonController', 'PhabricatorDaemonController' => 'PhabricatorController', 'PhabricatorDaemonDAO' => 'PhabricatorLiskDAO', @@ -6603,6 +6625,25 @@ phutil_register_library_map(array( 'PhabricatorWorkerActiveTask' => 'PhabricatorWorkerTask', 'PhabricatorWorkerArchiveTask' => 'PhabricatorWorkerTask', 'PhabricatorWorkerArchiveTaskQuery' => 'PhabricatorQuery', + 'PhabricatorWorkerBulkJob' => array( + 'PhabricatorWorkerDAO', + 'PhabricatorPolicyInterface', + 'PhabricatorSubscribableInterface', + 'PhabricatorApplicationTransactionInterface', + 'PhabricatorDestructibleInterface', + ), + 'PhabricatorWorkerBulkJobCreateWorker' => 'PhabricatorWorkerBulkJobWorker', + 'PhabricatorWorkerBulkJobEditor' => 'PhabricatorApplicationTransactionEditor', + 'PhabricatorWorkerBulkJobPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorWorkerBulkJobQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorWorkerBulkJobSearchEngine' => 'PhabricatorApplicationSearchEngine', + 'PhabricatorWorkerBulkJobTaskWorker' => 'PhabricatorWorkerBulkJobWorker', + 'PhabricatorWorkerBulkJobTestCase' => 'PhabricatorTestCase', + 'PhabricatorWorkerBulkJobTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorWorkerBulkJobTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorWorkerBulkJobType' => 'Phobject', + 'PhabricatorWorkerBulkJobWorker' => 'PhabricatorWorker', + 'PhabricatorWorkerBulkTask' => 'PhabricatorWorkerDAO', 'PhabricatorWorkerDAO' => 'PhabricatorLiskDAO', 'PhabricatorWorkerLeaseQuery' => 'PhabricatorQuery', 'PhabricatorWorkerManagementCancelWorkflow' => 'PhabricatorWorkerManagementWorkflow', @@ -6612,6 +6653,7 @@ phutil_register_library_map(array( 'PhabricatorWorkerManagementRetryWorkflow' => 'PhabricatorWorkerManagementWorkflow', 'PhabricatorWorkerManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorWorkerPermanentFailureException' => 'Exception', + 'PhabricatorWorkerSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorWorkerTask' => 'PhabricatorWorkerDAO', 'PhabricatorWorkerTaskData' => 'PhabricatorWorkerDAO', 'PhabricatorWorkerTaskDetailController' => 'PhabricatorDaemonController', diff --git a/src/applications/daemon/application/PhabricatorDaemonsApplication.php b/src/applications/daemon/application/PhabricatorDaemonsApplication.php index f7f3cb77ee..c605be63b6 100644 --- a/src/applications/daemon/application/PhabricatorDaemonsApplication.php +++ b/src/applications/daemon/application/PhabricatorDaemonsApplication.php @@ -46,6 +46,15 @@ final class PhabricatorDaemonsApplication extends PhabricatorApplication { '(?P[1-9]\d*)/' => 'PhabricatorDaemonLogViewController', ), 'event/(?P[1-9]\d*)/' => 'PhabricatorDaemonLogEventViewController', + 'bulk/' => array( + '(?:query/(?P[^/]+)/)?' => + 'PhabricatorDaemonBulkJobListController', + 'monitor/(?P\d+)/' => + 'PhabricatorDaemonBulkJobMonitorController', + 'view/(?P\d+)/' => + 'PhabricatorDaemonBulkJobViewController', + + ), ), ); } diff --git a/src/applications/daemon/controller/PhabricatorDaemonBulkJobListController.php b/src/applications/daemon/controller/PhabricatorDaemonBulkJobListController.php new file mode 100644 index 0000000000..ee8d4f5bf4 --- /dev/null +++ b/src/applications/daemon/controller/PhabricatorDaemonBulkJobListController.php @@ -0,0 +1,31 @@ +setQueryKey($request->getURIData('queryKey')) + ->setSearchEngine(new PhabricatorWorkerBulkJobSearchEngine()) + ->setNavigation($this->buildSideNavView()); + return $this->delegateToController($controller); + } + + protected function buildSideNavView($for_app = false) { + $user = $this->getRequest()->getUser(); + + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); + + id(new PhabricatorWorkerBulkJobSearchEngine()) + ->setViewer($user) + ->addNavigationItems($nav->getMenu()); + $nav->selectFilter(null); + + return $nav; + } +} diff --git a/src/applications/daemon/controller/PhabricatorDaemonBulkJobMonitorController.php b/src/applications/daemon/controller/PhabricatorDaemonBulkJobMonitorController.php new file mode 100644 index 0000000000..63ba3cacb1 --- /dev/null +++ b/src/applications/daemon/controller/PhabricatorDaemonBulkJobMonitorController.php @@ -0,0 +1,165 @@ +getViewer(); + + $job = id(new PhabricatorWorkerBulkJobQuery()) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) + ->executeOne(); + if (!$job) { + return new Aphront404Response(); + } + + // If the user clicks "Continue" on a completed job, take them back to + // whatever application sent them here. + if ($request->getStr('done')) { + if ($request->isFormPost()) { + $done_uri = $job->getDoneURI(); + return id(new AphrontRedirectResponse())->setURI($done_uri); + } + } + + $title = pht('Bulk Job %d', $job->getID()); + + if ($job->getStatus() == PhabricatorWorkerBulkJob::STATUS_CONFIRM) { + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $job, + PhabricatorPolicyCapability::CAN_EDIT); + + if ($can_edit) { + if ($request->isFormPost()) { + $type_status = PhabricatorWorkerBulkJobTransaction::TYPE_STATUS; + + $xactions = array(); + $xactions[] = id(new PhabricatorWorkerBulkJobTransaction()) + ->setTransactionType($type_status) + ->setNewValue(PhabricatorWorkerBulkJob::STATUS_WAITING); + + $editor = id(new PhabricatorWorkerBulkJobEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->applyTransactions($job, $xactions); + + return id(new AphrontRedirectResponse()) + ->setURI($job->getMonitorURI()); + } else { + return $this->newDialog() + ->setTitle(pht('Confirm Bulk Job')) + ->appendParagraph($job->getDescriptionForConfirm()) + ->appendParagraph( + pht('Start work on this bulk job?')) + ->addCancelButton($job->getManageURI(), pht('Details')) + ->addSubmitButton(pht('Start Work')); + } + } else { + return $this->newDialog() + ->setTitle(pht('Waiting For Confirmation')) + ->appendParagraph( + pht( + 'This job is waiting for confirmation before work begins.')) + ->addCancelButotn($job->getManageURI(), pht('Details')); + } + } + + + $dialog = $this->newDialog() + ->setTitle(pht('%s: %s', $title, $job->getStatusName())) + ->addCancelButton($job->getManageURI(), pht('Details')); + + switch ($job->getStatus()) { + case PhabricatorWorkerBulkJob::STATUS_WAITING: + $dialog->appendParagraph( + pht('This job is waiting for tasks to be queued.')); + break; + case PhabricatorWorkerBulkJob::STATUS_RUNNING: + $dialog->appendParagraph( + pht('This job is running.')); + break; + case PhabricatorWorkerBulkJob::STATUS_COMPLETE: + $dialog->appendParagraph( + pht('This job is complete.')); + break; + } + + $counts = $job->loadTaskStatusCounts(); + if ($counts) { + $dialog->appendParagraph($this->renderProgress($counts)); + } + + switch ($job->getStatus()) { + case PhabricatorWorkerBulkJob::STATUS_COMPLETE: + $dialog->addHiddenInput('done', true); + $dialog->addSubmitButton(pht('Continue')); + break; + default: + Javelin::initBehavior('bulk-job-reload'); + break; + } + + return $dialog; + } + + private function renderProgress(array $counts) { + $this->requireResource('bulk-job-css'); + + $states = array( + PhabricatorWorkerBulkTask::STATUS_DONE => array( + 'class' => 'bulk-job-progress-slice-green', + ), + PhabricatorWorkerBulkTask::STATUS_RUNNING => array( + 'class' => 'bulk-job-progress-slice-blue', + ), + PhabricatorWorkerBulkTask::STATUS_WAITING => array( + 'class' => 'bulk-job-progress-slice-empty', + ), + PhabricatorWorkerBulkTask::STATUS_FAIL => array( + 'class' => 'bulk-job-progress-slice-red', + ), + ); + + $total = array_sum($counts); + $offset = 0; + $bars = array(); + foreach ($states as $state => $spec) { + $size = idx($counts, $state, 0); + if (!$size) { + continue; + } + + $classes = array(); + $classes[] = 'bulk-job-progress-slice'; + $classes[] = $spec['class']; + + $width = ($size / $total); + $bars[] = phutil_tag( + 'div', + array( + 'class' => implode(' ', $classes), + 'style' => + 'left: '.sprintf('%.2f%%', 100 * $offset).'; '. + 'width: '.sprintf('%.2f%%', 100 * $width).';', + ), + ''); + + $offset += $width; + } + + return phutil_tag( + 'div', + array( + 'class' => 'bulk-job-progress-bar', + ), + $bars); + } + +} diff --git a/src/applications/daemon/controller/PhabricatorDaemonBulkJobViewController.php b/src/applications/daemon/controller/PhabricatorDaemonBulkJobViewController.php new file mode 100644 index 0000000000..6fc3af724e --- /dev/null +++ b/src/applications/daemon/controller/PhabricatorDaemonBulkJobViewController.php @@ -0,0 +1,83 @@ +getViewer(); + + $job = id(new PhabricatorWorkerBulkJobQuery()) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) + ->executeOne(); + if (!$job) { + return new Aphront404Response(); + } + + $title = pht('Bulk Job %d', $job->getID()); + + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addTextCrumb(pht('Bulk Jobs'), '/daemon/bulk/'); + $crumbs->addTextCrumb($title); + + $properties = $this->renderProperties($job); + $actions = $this->renderActions($job); + $properties->setActionList($actions); + + $box = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->addPropertyList($properties); + + $timeline = $this->buildTransactionTimeline( + $job, + new PhabricatorWorkerBulkJobTransactionQuery()); + $timeline->setShouldTerminate(true); + + return $this->buildApplicationPage( + array( + $crumbs, + $box, + $timeline, + ), + array( + 'title' => $title, + )); + } + + private function renderProperties(PhabricatorWorkerBulkJob $job) { + $viewer = $this->getViewer(); + + $view = id(new PHUIPropertyListView()) + ->setUser($viewer) + ->setObject($job); + + $view->addProperty( + pht('Author'), + $viewer->renderHandle($job->getAuthorPHID())); + + $view->addProperty(pht('Status'), $job->getStatusName()); + + return $view; + } + + private function renderActions(PhabricatorWorkerBulkJob $job) { + $viewer = $this->getViewer(); + + $actions = id(new PhabricatorActionListView()) + ->setUser($viewer) + ->setObject($job); + + $actions->addAction( + id(new PhabricatorActionView()) + ->setHref($job->getDoneURI()) + ->setIcon('fa-arrow-circle-o-right') + ->setName(pht('Continue'))); + + return $actions; + } + +} diff --git a/src/applications/daemon/controller/PhabricatorDaemonController.php b/src/applications/daemon/controller/PhabricatorDaemonController.php index 24fbceb710..3b1d17a70b 100644 --- a/src/applications/daemon/controller/PhabricatorDaemonController.php +++ b/src/applications/daemon/controller/PhabricatorDaemonController.php @@ -10,6 +10,9 @@ abstract class PhabricatorDaemonController extends PhabricatorController { $nav->addFilter('/', pht('Console')); $nav->addFilter('log', pht('All Daemons')); + $nav->addLabel(pht('Bulk Jobs')); + $nav->addFilter('bulk', pht('Manage Bulk Jobs')); + return $nav; } diff --git a/src/applications/maniphest/bulk/ManiphestTaskEditBulkJobType.php b/src/applications/maniphest/bulk/ManiphestTaskEditBulkJobType.php new file mode 100644 index 0000000000..e24b1e7bd4 --- /dev/null +++ b/src/applications/maniphest/bulk/ManiphestTaskEditBulkJobType.php @@ -0,0 +1,296 @@ +getSize())); + } + + public function getJobSize(PhabricatorWorkerBulkJob $job) { + return count($job->getParameter('taskPHIDs', array())); + } + + public function getDoneURI(PhabricatorWorkerBulkJob $job) { + return $job->getParameter('doneURI'); + } + + public function createTasks(PhabricatorWorkerBulkJob $job) { + $tasks = array(); + + foreach ($job->getParameter('taskPHIDs', array()) as $phid) { + $tasks[] = PhabricatorWorkerBulkTask::initializeNewTask($job, $phid); + } + + return $tasks; + } + + public function runTask( + PhabricatorUser $actor, + PhabricatorWorkerBulkJob $job, + PhabricatorWorkerBulkTask $task) { + + $object = id(new ManiphestTaskQuery()) + ->setViewer($actor) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withPHIDs(array($task->getObjectPHID())) + ->executeOne(); + if (!$object) { + return; + } + + $field_list = PhabricatorCustomField::getObjectFields( + $object, + PhabricatorCustomField::ROLE_EDIT); + $field_list->readFieldsFromStorage($object); + + $actions = $job->getParameter('actions'); + $xactions = $this->buildTransactions($actions, $object); + + $editor = id(new ManiphestTransactionEditor()) + ->setActor($actor) + ->setContentSource($job->newContentSource()) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true) + ->applyTransactions($object, $xactions); + } + + private function buildTransactions($actions, ManiphestTask $task) { + $value_map = array(); + $type_map = array( + 'add_comment' => PhabricatorTransactions::TYPE_COMMENT, + 'assign' => ManiphestTransaction::TYPE_OWNER, + 'status' => ManiphestTransaction::TYPE_STATUS, + 'priority' => ManiphestTransaction::TYPE_PRIORITY, + 'add_project' => PhabricatorTransactions::TYPE_EDGE, + 'remove_project' => PhabricatorTransactions::TYPE_EDGE, + 'add_ccs' => PhabricatorTransactions::TYPE_SUBSCRIBERS, + 'remove_ccs' => PhabricatorTransactions::TYPE_SUBSCRIBERS, + 'space' => PhabricatorTransactions::TYPE_SPACE, + ); + + $edge_edit_types = array( + 'add_project' => true, + 'remove_project' => true, + 'add_ccs' => true, + 'remove_ccs' => true, + ); + + $xactions = array(); + foreach ($actions as $action) { + if (empty($type_map[$action['action']])) { + throw new Exception(pht("Unknown batch edit action '%s'!", $action)); + } + + $type = $type_map[$action['action']]; + + // Figure out the current value, possibly after modifications by other + // batch actions of the same type. For example, if the user chooses to + // "Add Comment" twice, we should add both comments. More notably, if the + // user chooses "Remove Project..." and also "Add Project...", we should + // avoid restoring the removed project in the second transaction. + + if (array_key_exists($type, $value_map)) { + $current = $value_map[$type]; + } else { + switch ($type) { + case PhabricatorTransactions::TYPE_COMMENT: + $current = null; + break; + case ManiphestTransaction::TYPE_OWNER: + $current = $task->getOwnerPHID(); + break; + case ManiphestTransaction::TYPE_STATUS: + $current = $task->getStatus(); + break; + case ManiphestTransaction::TYPE_PRIORITY: + $current = $task->getPriority(); + break; + case PhabricatorTransactions::TYPE_EDGE: + $current = $task->getProjectPHIDs(); + break; + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + $current = $task->getSubscriberPHIDs(); + break; + case PhabricatorTransactions::TYPE_SPACE: + $current = PhabricatorSpacesNamespaceQuery::getObjectSpacePHID( + $task); + break; + } + } + + // Check if the value is meaningful / provided, and normalize it if + // necessary. This discards, e.g., empty comments and empty owner + // changes. + + $value = $action['value']; + switch ($type) { + case PhabricatorTransactions::TYPE_COMMENT: + if (!strlen($value)) { + continue 2; + } + break; + case PhabricatorTransactions::TYPE_SPACE: + if (empty($value)) { + continue 2; + } + $value = head($value); + break; + case ManiphestTransaction::TYPE_OWNER: + if (empty($value)) { + continue 2; + } + $value = head($value); + $no_owner = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN; + if ($value === $no_owner) { + $value = null; + } + break; + case PhabricatorTransactions::TYPE_EDGE: + if (empty($value)) { + continue 2; + } + break; + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + if (empty($value)) { + continue 2; + } + break; + } + + // If the edit doesn't change anything, go to the next action. This + // check is only valid for changes like "owner", "status", etc, not + // for edge edits, because we should still apply an edit like + // "Remove Projects: A, B" to a task with projects "A, B". + + if (empty($edge_edit_types[$action['action']])) { + if ($value == $current) { + continue; + } + } + + // Apply the value change; for most edits this is just replacement, but + // some need to merge the current and edited values (add/remove project). + + switch ($type) { + case PhabricatorTransactions::TYPE_COMMENT: + if (strlen($current)) { + $value = $current."\n\n".$value; + } + break; + case PhabricatorTransactions::TYPE_EDGE: + $is_remove = $action['action'] == 'remove_project'; + + $current = array_fill_keys($current, true); + $value = array_fill_keys($value, true); + + $new = $current; + $did_something = false; + + if ($is_remove) { + foreach ($value as $phid => $ignored) { + if (isset($new[$phid])) { + unset($new[$phid]); + $did_something = true; + } + } + } else { + foreach ($value as $phid => $ignored) { + if (empty($new[$phid])) { + $new[$phid] = true; + $did_something = true; + } + } + } + + if (!$did_something) { + continue 2; + } + + $value = array_keys($new); + break; + case PhabricatorTransactions::TYPE_SUBSCRIBERS: + $is_remove = $action['action'] == 'remove_ccs'; + + $current = array_fill_keys($current, true); + + $new = array(); + $did_something = false; + + if ($is_remove) { + foreach ($value as $phid) { + if (isset($current[$phid])) { + $new[$phid] = true; + $did_something = true; + } + } + if ($new) { + $value = array('-' => array_keys($new)); + } + } else { + $new = array(); + foreach ($value as $phid) { + $new[$phid] = true; + $did_something = true; + } + if ($new) { + $value = array('+' => array_keys($new)); + } + } + if (!$did_something) { + continue 2; + } + + break; + } + + $value_map[$type] = $value; + } + + $template = new ManiphestTransaction(); + + foreach ($value_map as $type => $value) { + $xaction = clone $template; + $xaction->setTransactionType($type); + + switch ($type) { + case PhabricatorTransactions::TYPE_COMMENT: + $xaction->attachComment( + id(new ManiphestTransactionComment()) + ->setContent($value)); + break; + case PhabricatorTransactions::TYPE_EDGE: + $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; + $xaction + ->setMetadataValue('edge:type', $project_type) + ->setNewValue( + array( + '=' => array_fuse($value), + )); + break; + default: + $xaction->setNewValue($value); + break; + } + + $xactions[] = $xaction; + } + + return $xactions; + } +} diff --git a/src/applications/maniphest/controller/ManiphestBatchEditController.php b/src/applications/maniphest/controller/ManiphestBatchEditController.php index e4086b2d18..1cee711dd2 100644 --- a/src/applications/maniphest/controller/ManiphestBatchEditController.php +++ b/src/applications/maniphest/controller/ManiphestBatchEditController.php @@ -45,8 +45,7 @@ final class ManiphestBatchEditController extends ManiphestController { if (!$tasks) { throw new Exception( - pht( - "You don't have permission to edit any of the selected tasks.")); + pht("You don't have permission to edit any of the selected tasks.")); } if ($project) { @@ -62,27 +61,32 @@ final class ManiphestBatchEditController extends ManiphestController { $actions = phutil_json_decode($actions); } - if ($request->isFormPost() && is_array($actions)) { - foreach ($tasks as $task) { - $field_list = PhabricatorCustomField::getObjectFields( - $task, - PhabricatorCustomField::ROLE_EDIT); - $field_list->readFieldsFromStorage($task); + if ($request->isFormPost() && $actions) { + $job = PhabricatorWorkerBulkJob::initializeNewJob( + $viewer, + new ManiphestTaskEditBulkJobType(), + array( + 'taskPHIDs' => mpull($tasks, 'getPHID'), + 'actions' => $actions, + 'cancelURI' => $cancel_uri, + 'doneURI' => $redirect_uri, + )); - $xactions = $this->buildTransactions($actions, $task); - if ($xactions) { - // TODO: Set content source to "batch edit". + $type_status = PhabricatorWorkerBulkJobTransaction::TYPE_STATUS; - $editor = id(new ManiphestTransactionEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true) - ->applyTransactions($task, $xactions); - } - } + $xactions = array(); + $xactions[] = id(new PhabricatorWorkerBulkJobTransaction()) + ->setTransactionType($type_status) + ->setNewValue(PhabricatorWorkerBulkJob::STATUS_CONFIRM); - return id(new AphrontRedirectResponse())->setURI($redirect_uri); + $editor = id(new PhabricatorWorkerBulkJobEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->applyTransactions($job, $xactions); + + return id(new AphrontRedirectResponse()) + ->setURI($job->getMonitorURI()); } $handles = ManiphestTaskListView::loadTaskHandles($viewer, $tasks); @@ -210,228 +214,4 @@ final class ManiphestBatchEditController extends ManiphestController { )); } - private function buildTransactions($actions, ManiphestTask $task) { - $value_map = array(); - $type_map = array( - 'add_comment' => PhabricatorTransactions::TYPE_COMMENT, - 'assign' => ManiphestTransaction::TYPE_OWNER, - 'status' => ManiphestTransaction::TYPE_STATUS, - 'priority' => ManiphestTransaction::TYPE_PRIORITY, - 'add_project' => PhabricatorTransactions::TYPE_EDGE, - 'remove_project' => PhabricatorTransactions::TYPE_EDGE, - 'add_ccs' => PhabricatorTransactions::TYPE_SUBSCRIBERS, - 'remove_ccs' => PhabricatorTransactions::TYPE_SUBSCRIBERS, - 'space' => PhabricatorTransactions::TYPE_SPACE, - ); - - $edge_edit_types = array( - 'add_project' => true, - 'remove_project' => true, - 'add_ccs' => true, - 'remove_ccs' => true, - ); - - $xactions = array(); - foreach ($actions as $action) { - if (empty($type_map[$action['action']])) { - throw new Exception(pht("Unknown batch edit action '%s'!", $action)); - } - - $type = $type_map[$action['action']]; - - // Figure out the current value, possibly after modifications by other - // batch actions of the same type. For example, if the user chooses to - // "Add Comment" twice, we should add both comments. More notably, if the - // user chooses "Remove Project..." and also "Add Project...", we should - // avoid restoring the removed project in the second transaction. - - if (array_key_exists($type, $value_map)) { - $current = $value_map[$type]; - } else { - switch ($type) { - case PhabricatorTransactions::TYPE_COMMENT: - $current = null; - break; - case ManiphestTransaction::TYPE_OWNER: - $current = $task->getOwnerPHID(); - break; - case ManiphestTransaction::TYPE_STATUS: - $current = $task->getStatus(); - break; - case ManiphestTransaction::TYPE_PRIORITY: - $current = $task->getPriority(); - break; - case PhabricatorTransactions::TYPE_EDGE: - $current = $task->getProjectPHIDs(); - break; - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - $current = $task->getSubscriberPHIDs(); - break; - case PhabricatorTransactions::TYPE_SPACE: - $current = PhabricatorSpacesNamespaceQuery::getObjectSpacePHID( - $task); - break; - } - } - - // Check if the value is meaningful / provided, and normalize it if - // necessary. This discards, e.g., empty comments and empty owner - // changes. - - $value = $action['value']; - switch ($type) { - case PhabricatorTransactions::TYPE_COMMENT: - if (!strlen($value)) { - continue 2; - } - break; - case PhabricatorTransactions::TYPE_SPACE: - if (empty($value)) { - continue 2; - } - $value = head($value); - break; - case ManiphestTransaction::TYPE_OWNER: - if (empty($value)) { - continue 2; - } - $value = head($value); - $no_owner = PhabricatorPeopleNoOwnerDatasource::FUNCTION_TOKEN; - if ($value === $no_owner) { - $value = null; - } - break; - case PhabricatorTransactions::TYPE_EDGE: - if (empty($value)) { - continue 2; - } - break; - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - if (empty($value)) { - continue 2; - } - break; - } - - // If the edit doesn't change anything, go to the next action. This - // check is only valid for changes like "owner", "status", etc, not - // for edge edits, because we should still apply an edit like - // "Remove Projects: A, B" to a task with projects "A, B". - - if (empty($edge_edit_types[$action['action']])) { - if ($value == $current) { - continue; - } - } - - // Apply the value change; for most edits this is just replacement, but - // some need to merge the current and edited values (add/remove project). - - switch ($type) { - case PhabricatorTransactions::TYPE_COMMENT: - if (strlen($current)) { - $value = $current."\n\n".$value; - } - break; - case PhabricatorTransactions::TYPE_EDGE: - $is_remove = $action['action'] == 'remove_project'; - - $current = array_fill_keys($current, true); - $value = array_fill_keys($value, true); - - $new = $current; - $did_something = false; - - if ($is_remove) { - foreach ($value as $phid => $ignored) { - if (isset($new[$phid])) { - unset($new[$phid]); - $did_something = true; - } - } - } else { - foreach ($value as $phid => $ignored) { - if (empty($new[$phid])) { - $new[$phid] = true; - $did_something = true; - } - } - } - - if (!$did_something) { - continue 2; - } - - $value = array_keys($new); - break; - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - $is_remove = $action['action'] == 'remove_ccs'; - - $current = array_fill_keys($current, true); - - $new = array(); - $did_something = false; - - if ($is_remove) { - foreach ($value as $phid) { - if (isset($current[$phid])) { - $new[$phid] = true; - $did_something = true; - } - } - if ($new) { - $value = array('-' => array_keys($new)); - } - } else { - $new = array(); - foreach ($value as $phid) { - $new[$phid] = true; - $did_something = true; - } - if ($new) { - $value = array('+' => array_keys($new)); - } - } - if (!$did_something) { - continue 2; - } - - break; - } - - $value_map[$type] = $value; - } - - $template = new ManiphestTransaction(); - - foreach ($value_map as $type => $value) { - $xaction = clone $template; - $xaction->setTransactionType($type); - - switch ($type) { - case PhabricatorTransactions::TYPE_COMMENT: - $xaction->attachComment( - id(new ManiphestTransactionComment()) - ->setContent($value)); - break; - case PhabricatorTransactions::TYPE_EDGE: - $project_type = PhabricatorProjectObjectHasProjectEdgeType::EDGECONST; - $xaction - ->setMetadataValue('edge:type', $project_type) - ->setNewValue( - array( - '=' => array_fuse($value), - )); - break; - default: - $xaction->setNewValue($value); - break; - } - - $xactions[] = $xaction; - } - - return $xactions; - } - } diff --git a/src/applications/metamta/contentsource/PhabricatorContentSource.php b/src/applications/metamta/contentsource/PhabricatorContentSource.php index c291b2069b..6d6a8ee896 100644 --- a/src/applications/metamta/contentsource/PhabricatorContentSource.php +++ b/src/applications/metamta/contentsource/PhabricatorContentSource.php @@ -15,6 +15,7 @@ final class PhabricatorContentSource extends Phobject { const SOURCE_DAEMON = 'daemon'; const SOURCE_LIPSUM = 'lipsum'; const SOURCE_PHORTUNE = 'phortune'; + const SOURCE_BULK = 'bulk'; private $source; private $params = array(); @@ -79,6 +80,7 @@ final class PhabricatorContentSource extends Phobject { self::SOURCE_LIPSUM => pht('Lipsum'), self::SOURCE_UNKNOWN => pht('Old World'), self::SOURCE_PHORTUNE => pht('Phortune'), + self::SOURCE_BULK => pht('Bulk Edit'), ); } diff --git a/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerBulkJobTestCase.php b/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerBulkJobTestCase.php new file mode 100644 index 0000000000..58ddaf0339 --- /dev/null +++ b/src/infrastructure/daemon/workers/__tests__/PhabricatorWorkerBulkJobTestCase.php @@ -0,0 +1,10 @@ +assertTrue(true); + } + +} diff --git a/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobCreateWorker.php b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobCreateWorker.php new file mode 100644 index 0000000000..7c1b5dcada --- /dev/null +++ b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobCreateWorker.php @@ -0,0 +1,51 @@ +acquireJobLock(); + + $job = $this->loadJob(); + $actor = $this->loadActor($job); + + $status = $job->getStatus(); + switch ($status) { + case PhabricatorWorkerBulkJob::STATUS_WAITING: + // This is what we expect. Other statuses indicate some kind of race + // is afoot. + break; + default: + throw new PhabricatorWorkerPermanentFailureException( + pht( + 'Found unexpected job status ("%s").', + $status)); + } + + $tasks = $job->createTasks(); + foreach ($tasks as $task) { + $task->save(); + } + + $this->updateJobStatus( + $job, + PhabricatorWorkerBulkJob::STATUS_RUNNING); + + $lock->unlock(); + + foreach ($tasks as $task) { + PhabricatorWorker::scheduleTask( + 'PhabricatorWorkerBulkJobTaskWorker', + array( + 'jobID' => $job->getID(), + 'taskID' => $task->getID(), + ), + array( + 'priority' => PhabricatorWorker::PRIORITY_BULK, + )); + } + + $this->updateJob($job); + } + +} diff --git a/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobTaskWorker.php b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobTaskWorker.php new file mode 100644 index 0000000000..12de74c3ec --- /dev/null +++ b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobTaskWorker.php @@ -0,0 +1,46 @@ +acquireTaskLock(); + + $task = $this->loadTask(); + $status = $task->getStatus(); + switch ($task->getStatus()) { + case PhabricatorWorkerBulkTask::STATUS_WAITING: + // This is what we expect. + break; + default: + throw new PhabricatorWorkerPermanentFailureException( + pht( + 'Found unexpected task status ("%s").', + $status)); + } + + $task + ->setStatus(PhabricatorWorkerBulkTask::STATUS_RUNNING) + ->save(); + + $lock->unlock(); + + $job = $this->loadJob(); + $actor = $this->loadActor($job); + + try { + $job->runTask($actor, $task); + $status = PhabricatorWorkerBulkTask::STATUS_DONE; + } catch (Exception $ex) { + phlog($ex); + $status = PhabricatorWorkerBulkTask::STATUS_FAIL; + } + + $task + ->setStatus($status) + ->save(); + + $this->updateJob($job); + } + +} diff --git a/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobType.php b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobType.php new file mode 100644 index 0000000000..a5e29bc101 --- /dev/null +++ b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobType.php @@ -0,0 +1,28 @@ +getManageURI(); + } + + final public static function getAllJobTypes() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getBulkJobTypeKey') + ->execute(); + } + +} diff --git a/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobWorker.php b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobWorker.php new file mode 100644 index 0000000000..1f6644ae71 --- /dev/null +++ b/src/infrastructure/daemon/workers/bulk/PhabricatorWorkerBulkJobWorker.php @@ -0,0 +1,138 @@ +getJobID()) + ->lock(15); + } + + final protected function acquireTaskLock() { + return PhabricatorGlobalLock::newLock('bulktask.'.$this->getTaskID()) + ->lock(15); + } + + final protected function getJobID() { + $data = $this->getTaskData(); + $id = idx($data, 'jobID'); + if (!$id) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Worker has no job ID.')); + } + return $id; + } + + final protected function getTaskID() { + $data = $this->getTaskData(); + $id = idx($data, 'taskID'); + if (!$id) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Worker has no task ID.')); + } + return $id; + } + + final protected function loadJob() { + $id = $this->getJobID(); + $job = id(new PhabricatorWorkerBulkJobQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withIDs(array($id)) + ->executeOne(); + if (!$job) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Worker has invalid job ID ("%s").', $id)); + } + return $job; + } + + final protected function loadTask() { + $id = $this->getTaskID(); + $task = id(new PhabricatorWorkerBulkTask())->load($id); + if (!$task) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Worker has invalid task ID ("%s").', $id)); + } + return $task; + } + + final protected function loadActor(PhabricatorWorkerBulkJob $job) { + $actor_phid = $job->getAuthorPHID(); + $actor = id(new PhabricatorPeopleQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs(array($actor_phid)) + ->executeOne(); + if (!$actor) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Worker has invalid actor PHID ("%s").', $actor_phid)); + } + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $actor, + $job, + PhabricatorPolicyCapability::CAN_EDIT); + + if (!$can_edit) { + throw new PhabricatorWorkerPermanentFailureException( + pht('Job actor does not have permission to edit job.')); + } + + return $actor; + } + + final protected function updateJob(PhabricatorWorkerBulkJob $job) { + $has_work = $this->hasRemainingWork($job); + if ($has_work) { + return; + } + + $lock = $this->acquireJobLock(); + + $job = $this->loadJob(); + if ($job->getStatus() == PhabricatorWorkerBulkJob::STATUS_RUNNING) { + if (!$this->hasRemainingWork($job)) { + $this->updateJobStatus( + $job, + PhabricatorWorkerBulkJob::STATUS_COMPLETE); + } + } + + $lock->unlock(); + } + + private function hasRemainingWork(PhabricatorWorkerBulkJob $job) { + return (bool)queryfx_one( + $job->establishConnection('r'), + 'SELECT * FROM %T WHERE bulkJobPHID = %s + AND status NOT IN (%Ls) LIMIT 1', + id(new PhabricatorWorkerBulkTask())->getTableName(), + $job->getPHID(), + array( + PhabricatorWorkerBulkTask::STATUS_DONE, + PhabricatorWorkerBulkTask::STATUS_FAIL, + )); + } + + protected function updateJobStatus(PhabricatorWorkerBulkJob $job, $status) { + $type_status = PhabricatorWorkerBulkJobTransaction::TYPE_STATUS; + + $xactions = array(); + $xactions[] = id(new PhabricatorWorkerBulkJobTransaction()) + ->setTransactionType($type_status) + ->setNewValue($status); + + $daemon_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_DAEMON, + array()); + + $app_phid = id(new PhabricatorDaemonsApplication())->getPHID(); + + $editor = id(new PhabricatorWorkerBulkJobEditor()) + ->setActor(PhabricatorUser::getOmnipotentUser()) + ->setActingAsPHID($app_phid) + ->setContentSource($daemon_source) + ->setContinueOnMissingFields(true) + ->applyTransactions($job, $xactions); + } + +} diff --git a/src/infrastructure/daemon/workers/editor/PhabricatorWorkerBulkJobEditor.php b/src/infrastructure/daemon/workers/editor/PhabricatorWorkerBulkJobEditor.php new file mode 100644 index 0000000000..b23c987d6d --- /dev/null +++ b/src/infrastructure/daemon/workers/editor/PhabricatorWorkerBulkJobEditor.php @@ -0,0 +1,87 @@ +getTransactionType()) { + case PhabricatorWorkerBulkJobTransaction::TYPE_STATUS: + return $object->getStatus(); + } + } + + protected function getCustomTransactionNewValue( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PhabricatorWorkerBulkJobTransaction::TYPE_STATUS: + return $xaction->getNewValue(); + } + } + + protected function applyCustomInternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + $type = $xaction->getTransactionType(); + $new = $xaction->getNewValue(); + + switch ($type) { + case PhabricatorWorkerBulkJobTransaction::TYPE_STATUS: + $object->setStatus($xaction->getNewValue()); + return; + } + + return parent::applyCustomInternalTransaction($object, $xaction); + } + + protected function applyCustomExternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + $type = $xaction->getTransactionType(); + $new = $xaction->getNewValue(); + + switch ($type) { + case PhabricatorWorkerBulkJobTransaction::TYPE_STATUS: + switch ($new) { + case PhabricatorWorkerBulkJob::STATUS_WAITING: + PhabricatorWorker::scheduleTask( + 'PhabricatorWorkerBulkJobCreateWorker', + array( + 'jobID' => $object->getID(), + ), + array( + 'priority' => PhabricatorWorker::PRIORITY_BULK, + )); + break; + } + return; + } + + return parent::applyCustomExternalTransaction($object, $xaction); + } + + + +} diff --git a/src/infrastructure/daemon/workers/phid/PhabricatorWorkerBulkJobPHIDType.php b/src/infrastructure/daemon/workers/phid/PhabricatorWorkerBulkJobPHIDType.php new file mode 100644 index 0000000000..7550d1cf02 --- /dev/null +++ b/src/infrastructure/daemon/workers/phid/PhabricatorWorkerBulkJobPHIDType.php @@ -0,0 +1,37 @@ +withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + foreach ($handles as $phid => $handle) { + $job = $objects[$phid]; + + $id = $job->getID(); + + $handle->setName(pht('Bulk Job %d', $id)); + } + } + +} diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobQuery.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobQuery.php new file mode 100644 index 0000000000..32a9419a33 --- /dev/null +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobQuery.php @@ -0,0 +1,106 @@ +ids = $ids; + return $this; + } + + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + + public function withAuthorPHIDs(array $author_phids) { + $this->authorPHIDs = $author_phids; + return $this; + } + + public function withBulkJobTypes(array $job_types) { + $this->bulkJobTypes = $job_types; + return $this; + } + + public function withStatuses(array $statuses) { + $this->statuses = $statuses; + return $this; + } + + public function newResultObject() { + return new PhabricatorWorkerBulkJob(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function willFilterPage(array $page) { + $map = PhabricatorWorkerBulkJobType::getAllJobTypes(); + + foreach ($page as $key => $job) { + $implementation = idx($map, $job->getJobTypeKey()); + if (!$implementation) { + $this->didRejectResult($job); + unset($page[$key]); + continue; + } + $job->attachJobImplementation($implementation); + } + + return $page; + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->phids !== null) { + $where[] = qsprintf( + $conn, + 'phid IN (%Ls)', + $this->phids); + } + + if ($this->authorPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'authorPHID IN (%Ls)', + $this->authorPHIDs); + } + + if ($this->bulkJobTypes !== null) { + $where[] = qsprintf( + $conn, + 'bulkJobType IN (%Ls)', + $this->bulkJobTypes); + } + + if ($this->statuses !== null) { + $where[] = qsprintf( + $conn, + 'status IN (%Ls)', + $this->statuses); + } + + return $where; + } + + public function getQueryApplicationClass() { + return 'PhabricatorDaemonsApplication'; + } + +} diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php new file mode 100644 index 0000000000..e27cd04f16 --- /dev/null +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobSearchEngine.php @@ -0,0 +1,98 @@ +newQuery(); + + if ($map['authorPHIDs']) { + $query->withAuthorPHIDs($map['authorPHIDs']); + } + + return $query; + } + + protected function buildCustomSearchFields() { + return array( + id(new PhabricatorSearchUsersField()) + ->setLabel(pht('Authors')) + ->setKey('authorPHIDs') + ->setAliases(array('author', 'authors')), + ); + } + + protected function getURI($path) { + return '/daemon/bulk/'.$path; + } + + protected function getBuiltinQueryNames() { + $names = array(); + + if ($this->requireViewer()->isLoggedIn()) { + $names['authored'] = pht('Authored Jobs'); + } + + $names['all'] = pht('All Jobs'); + + return $names; + } + + public function buildSavedQueryFromBuiltin($query_key) { + + $query = $this->newSavedQuery(); + $query->setQueryKey($query_key); + + switch ($query_key) { + case 'all': + return $query; + case 'authored': + return $query->setParameter( + 'authorPHIDs', + array($this->requireViewer()->getPHID())); + } + + return parent::buildSavedQueryFromBuiltin($query_key); + } + + protected function renderResultList( + array $jobs, + PhabricatorSavedQuery $query, + array $handles) { + assert_instances_of($jobs, 'PhabricatorWorkerBulkJob'); + + $viewer = $this->requireViewer(); + + $list = id(new PHUIObjectItemListView()) + ->setUser($viewer); + foreach ($jobs as $job) { + $size = pht('%s Bulk Task(s)', new PhutilNumber($job->getSize())); + + $item = id(new PHUIObjectItemView()) + ->setObjectName(pht('Bulk Job %d', $job->getID())) + ->setHeader($job->getJobName()) + ->addAttribute(phabricator_datetime($job->getDateCreated(), $viewer)) + ->setHref($job->getManageURI()) + ->addIcon($job->getStatusIcon(), $job->getStatusName()) + ->addIcon('none', $size); + + $list->addItem($item); + } + + // TODO: Needs new wrapper when merging to redesign. + + return $list; + } +} diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobTransactionQuery.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobTransactionQuery.php new file mode 100644 index 0000000000..350277a888 --- /dev/null +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerBulkJobTransactionQuery.php @@ -0,0 +1,10 @@ + true, + self::CONFIG_SERIALIZATION => array( + 'parameters' => self::SERIALIZATION_JSON, + ), + self::CONFIG_COLUMN_SCHEMA => array( + 'jobTypeKey' => 'text32', + 'status' => 'text32', + 'size' => 'uint32', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_type' => array( + 'columns' => array('jobTypeKey'), + ), + 'key_author' => array( + 'columns' => array('authorPHID'), + ), + 'key_status' => array( + 'columns' => array('status'), + ), + ), + ) + parent::getConfiguration(); + } + + public static function initializeNewJob( + PhabricatorUser $actor, + PhabricatorWorkerBulkJobType $type, + array $parameters) { + + $job = id(new PhabricatorWorkerBulkJob()) + ->setAuthorPHID($actor->getPHID()) + ->setJobTypeKey($type->getBulkJobTypeKey()) + ->setParameters($parameters) + ->attachJobImplementation($type); + + $job->setSize($job->computeSize()); + + return $job; + } + + public function generatePHID() { + return PhabricatorPHID::generateNewPHID( + PhabricatorWorkerBulkJobPHIDType::TYPECONST); + } + + public function getMonitorURI() { + return '/daemon/bulk/monitor/'.$this->getID().'/'; + } + + public function getManageURI() { + return '/daemon/bulk/view/'.$this->getID().'/'; + } + + public function getParameter($key, $default = null) { + return idx($this->parameters, $key, $default); + } + + public function setParameter($key, $value) { + $this->parameters[$key] = $value; + return $this; + } + + public function loadTaskStatusCounts() { + $table = new PhabricatorWorkerBulkTask(); + $conn_r = $table->establishConnection('r'); + $rows = queryfx_all( + $conn_r, + 'SELECT status, COUNT(*) N FROM %T WHERE bulkJobPHID = %s + GROUP BY status', + $table->getTableName(), + $this->getPHID()); + + return ipull($rows, 'N', 'status'); + } + + public function newContentSource() { + return PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_BULK, + array( + 'jobID' => $this->getID(), + )); + } + + public function getStatusIcon() { + $map = array( + self::STATUS_CONFIRM => 'fa-question', + self::STATUS_WAITING => 'fa-clock-o', + self::STATUS_RUNNING => 'fa-clock-o', + self::STATUS_COMPLETE => 'fa-check grey', + ); + + return idx($map, $this->getStatus(), 'none'); + } + + public function getStatusName() { + $map = array( + self::STATUS_CONFIRM => pht('Confirming'), + self::STATUS_WAITING => pht('Waiting'), + self::STATUS_RUNNING => pht('Running'), + self::STATUS_COMPLETE => pht('Complete'), + ); + + return idx($map, $this->getStatus(), $this->getStatus()); + } + + +/* -( Job Implementation )------------------------------------------------- */ + + + protected function getJobImplementation() { + return $this->assertAttached($this->jobImplementation); + } + + public function attachJobImplementation(PhabricatorWorkerBulkJobType $type) { + $this->jobImplementation = $type; + return $this; + } + + private function computeSize() { + return $this->getJobImplementation()->getJobSize($this); + } + + public function getCancelURI() { + return $this->getJobImplementation()->getCancelURI($this); + } + + public function getDoneURI() { + return $this->getJobImplementation()->getDoneURI($this); + } + + public function getDescriptionForConfirm() { + return $this->getJobImplementation()->getDescriptionForConfirm($this); + } + + public function createTasks() { + return $this->getJobImplementation()->createTasks($this); + } + + public function runTask( + PhabricatorUser $actor, + PhabricatorWorkerBulkTask $task) { + return $this->getJobImplementation()->runTask($actor, $this, $task); + } + + public function getJobName() { + return $this->getJobImplementation()->getJobName($this); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + ); + } + + public function getPolicy($capability) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return PhabricatorPolicies::getMostOpenPolicy(); + case PhabricatorPolicyCapability::CAN_EDIT: + return $this->getAuthorPHID(); + } + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + public function describeAutomaticCapability($capability) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_EDIT: + return pht('Only the owner of a bulk job can edit it.'); + default: + return null; + } + } + + +/* -( PhabricatorSubscribableInterface )----------------------------------- */ + + + public function isAutomaticallySubscribed($phid) { + return false; + } + + public function shouldShowSubscribersProperty() { + return true; + } + + public function shouldAllowSubscription($phid) { + return true; + } + + +/* -( PhabricatorApplicationTransactionInterface )------------------------- */ + + + public function getApplicationTransactionEditor() { + return new PhabricatorWorkerBulkJobEditor(); + } + + public function getApplicationTransactionObject() { + return $this; + } + + public function getApplicationTransactionTemplate() { + return new PhabricatorWorkerBulkJobTransaction(); + } + + public function willRenderTimeline( + PhabricatorApplicationTransactionView $timeline, + AphrontRequest $request) { + return $timeline; + } + +/* -( PhabricatorDestructibleInterface )----------------------------------- */ + + + public function destroyObjectPermanently( + PhabricatorDestructionEngine $engine) { + + $this->openTransaction(); + + // We're only removing the actual task objects. This may leave stranded + // workers in the queue itself, but they'll just flush out automatically + // when they can't load bulk job data. + + $task_table = new PhabricatorWorkerBulkTask(); + $conn_w = $task_table->establishConnection('w'); + queryfx( + $conn_w, + 'DELETE FROM %T WHERE bulkJobPHID = %s', + $task_table->getPHID(), + $this->getPHID()); + + $this->delete(); + $this->saveTransaction(); + } + + +} diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJobTransaction.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJobTransaction.php new file mode 100644 index 0000000000..9ac6b19716 --- /dev/null +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkJobTransaction.php @@ -0,0 +1,51 @@ +getAuthorPHID(); + + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + $type = $this->getTransactionType(); + switch ($type) { + case self::TYPE_STATUS: + if ($old === null) { + return pht( + '%s created this bulk job.', + $this->renderHandleLink($author_phid)); + } else { + switch ($new) { + case PhabricatorWorkerBulkJob::STATUS_WAITING: + return pht( + '%s confirmed this job.', + $this->renderHandleLink($author_phid)); + case PhabricatorWorkerBulkJob::STATUS_RUNNING: + return pht( + '%s marked this job as running.', + $this->renderHandleLink($author_phid)); + case PhabricatorWorkerBulkJob::STATUS_COMPLETE: + return pht( + '%s marked this job complete.', + $this->renderHandleLink($author_phid)); + } + } + break; + } + + return parent::getTitle(); + } + +} diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkTask.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkTask.php new file mode 100644 index 0000000000..382e435571 --- /dev/null +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerBulkTask.php @@ -0,0 +1,46 @@ + false, + self::CONFIG_SERIALIZATION => array( + 'data' => self::SERIALIZATION_JSON, + ), + self::CONFIG_COLUMN_SCHEMA => array( + 'status' => 'text32', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_job' => array( + 'columns' => array('bulkJobPHID', 'status'), + ), + 'key_object' => array( + 'columns' => array('objectPHID'), + ), + ), + ) + parent::getConfiguration(); + } + + public static function initializeNewTask( + PhabricatorWorkerBulkJob $job, + $object_phid) { + + return id(new PhabricatorWorkerBulkTask()) + ->setBulkJobPHID($job->getPHID()) + ->setStatus(self::STATUS_WAITING) + ->setObjectPHID($object_phid); + } + +} diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerSchemaSpec.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerSchemaSpec.php new file mode 100644 index 0000000000..2897f523c6 --- /dev/null +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerSchemaSpec.php @@ -0,0 +1,10 @@ +buildEdgeSchemata(new PhabricatorWorkerBulkJob()); + } + +} diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index 64198ae14e..453312f837 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1177,6 +1177,11 @@ final class PhabricatorUSEnglishTranslation '%s Broken Test(s)' => '%s Broken', '%s Unsound Test(s)' => '%s Unsound', '%s Other Test(s)' => '%s Other', + + '%s Bulk Task(s)' => array( + '%s Task', + '%s Tasks', + ), ); } diff --git a/webroot/rsrc/css/application/daemon/bulk-job.css b/webroot/rsrc/css/application/daemon/bulk-job.css new file mode 100644 index 0000000000..206a4dd831 --- /dev/null +++ b/webroot/rsrc/css/application/daemon/bulk-job.css @@ -0,0 +1,32 @@ +/** + * @provides bulk-job-css + */ + +.bulk-job-progress-bar { + position: relative; + width: 100%; + border: 1px solid {$lightgreyborder}; + height: 32px; +} + +.bulk-job-progress-slice { + position: absolute; + top: 0; + bottom: 0; +} + +.bulk-job-progress-slice-green { + background-color: {$green}; +} + +.bulk-job-progress-slice-blue { + background-color: {$blue}; +} + +.bulk-job-progress-slice-red { + background-color: {$red}; +} + +.bulk-job-progress-slice-empty { + background-color: {$lightbluebackground}; +} diff --git a/webroot/rsrc/js/application/daemon/behavior-bulk-job-reload.js b/webroot/rsrc/js/application/daemon/behavior-bulk-job-reload.js new file mode 100644 index 0000000000..293f18b22c --- /dev/null +++ b/webroot/rsrc/js/application/daemon/behavior-bulk-job-reload.js @@ -0,0 +1,18 @@ +/** + * @provides javelin-behavior-bulk-job-reload + * @requires javelin-behavior + * javelin-uri + */ + +JX.behavior('bulk-job-reload', function() { + + // TODO: It would be nice to have a pretty Ajax progress bar here, but just + // reload the page for now. + + function reload() { + JX.$U().go(); + } + + setTimeout(reload, 1000); + +});