diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f6254a8c3b..047d315afa 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -661,6 +661,7 @@ phutil_register_library_map(array( 'HeraldTranscript' => 'applications/herald/storage/transcript/HeraldTranscript.php', 'HeraldTranscriptController' => 'applications/herald/controller/HeraldTranscriptController.php', 'HeraldTranscriptListController' => 'applications/herald/controller/HeraldTranscriptListController.php', + 'HeraldTranscriptQuery' => 'applications/herald/query/HeraldTranscriptQuery.php', 'Javelin' => 'infrastructure/javelin/Javelin.php', 'JavelinReactorExample' => 'applications/uiexample/examples/JavelinReactorExample.php', 'JavelinUIExample' => 'applications/uiexample/examples/JavelinUIExample.php', @@ -2753,6 +2754,7 @@ phutil_register_library_map(array( 'HeraldTranscript' => 'HeraldDAO', 'HeraldTranscriptController' => 'HeraldController', 'HeraldTranscriptListController' => 'HeraldController', + 'HeraldTranscriptQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'JavelinReactorExample' => 'PhabricatorUIExample', 'JavelinUIExample' => 'PhabricatorUIExample', 'JavelinViewExample' => 'PhabricatorUIExample', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 4495691ff7..88d726b168 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -93,10 +93,6 @@ abstract class HeraldAdapter { abstract public function applyHeraldEffects(array $effects); - public function isEnabled() { - return true; - } - public function isAvailableToUser(PhabricatorUser $viewer) { $applications = id(new PhabricatorApplicationQuery()) ->setViewer($viewer) diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php index 9442a2792e..7cdf5aade1 100644 --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -25,10 +25,15 @@ final class HeraldTranscriptController extends HeraldController { } public function processRequest() { + $request = $this->getRequest(); + $viewer = $request->getUser(); - $xscript = id(new HeraldTranscript())->load($this->id); + $xscript = id(new HeraldTranscriptQuery()) + ->setViewer($viewer) + ->withIDs(array($this->id)) + ->executeOne(); if (!$xscript) { - throw new Exception('Uknown transcript!'); + return new Aphront404Response(); } require_celerity_resource('herald-test-css'); @@ -46,9 +51,19 @@ final class HeraldTranscriptController extends HeraldController { pht('Details of this transcript have been garbage collected.'))); $nav->appendChild($notice); } else { + $map = HeraldAdapter::getEnabledAdapterMap($viewer); + $object_type = $object_xscript->getType(); + if (empty($map[$object_type])) { + // TODO: We should filter these out in the Query, but we have to load + // the objectTranscript right now, which is potentially enormous. We + // should denormalize the object type, or move the data into a separate + // table, and then filter this earlier (and thus raise a better error). + // For now, just block access so we don't violate policies. + throw new Exception( + pht("This transcript has an invalid or inaccessible adapter.")); + } - $this->adapter = HeraldAdapter::getAdapterForContentType( - $object_xscript->getType()); + $this->adapter = HeraldAdapter::getAdapterForContentType($object_type); $filter = $this->getFilterPHIDs(); $this->filterTranscript($xscript, $filter); diff --git a/src/applications/herald/controller/HeraldTranscriptListController.php b/src/applications/herald/controller/HeraldTranscriptListController.php index e07c9bd340..64335b5350 100644 --- a/src/applications/herald/controller/HeraldTranscriptListController.php +++ b/src/applications/herald/controller/HeraldTranscriptListController.php @@ -7,61 +7,33 @@ final class HeraldTranscriptListController extends HeraldController { $request = $this->getRequest(); $user = $request->getUser(); - // Get one page of data together with the pager. - // Pull these objects manually since the serialized fields are gigantic. - $transcript = new HeraldTranscript(); + $pager = new AphrontCursorPagerView(); + $pager->readFromRequest($request); - $conn_r = $transcript->establishConnection('r'); - $phid = $request->getStr('phid'); - $where_clause = ''; - if ($phid) { - $where_clause = qsprintf( - $conn_r, - 'WHERE objectPHID = %s', - $phid); - } - - $pager = new AphrontPagerView(); - $pager->setOffset($request->getInt('offset')); - $pager->setURI($request->getRequestURI(), 'offset'); - - $limit_clause = qsprintf( - $conn_r, - 'LIMIT %d, %d', - $pager->getOffset(), - $pager->getPageSize() + 1); - - $data = queryfx_all( - $conn_r, - 'SELECT id, objectPHID, time, duration, dryRun FROM %T - %Q - ORDER BY id DESC - %Q', - $transcript->getTableName(), - $where_clause, - $limit_clause); - - $data = $pager->sliceResults($data); + $transcripts = id(new HeraldTranscriptQuery()) + ->setViewer($user) + ->needPartialRecords(true) + ->executeWithCursorPager($pager); // Render the table. $handles = array(); - if ($data) { - $phids = ipull($data, 'objectPHID', 'objectPHID'); + if ($transcripts) { + $phids = mpull($transcripts, 'getObjectPHID', 'getObjectPHID'); $handles = $this->loadViewerHandles($phids); } $rows = array(); - foreach ($data as $xscript) { + foreach ($transcripts as $xscript) { $rows[] = array( - phabricator_date($xscript['time'], $user), - phabricator_time($xscript['time'], $user), - $handles[$xscript['objectPHID']]->renderLink(), - $xscript['dryRun'] ? 'Yes' : '', - number_format((int)(1000 * $xscript['duration'])).' ms', + phabricator_date($xscript->getTime(), $user), + phabricator_time($xscript->getTime(), $user), + $handles[$xscript->getObjectPHID()]->renderLink(), + $xscript->getDryRun() ? pht('Yes') : '', + number_format((int)(1000 * $xscript->getDuration())).' ms', phutil_tag( 'a', array( - 'href' => '/herald/transcript/'.$xscript['id'].'/', + 'href' => '/herald/transcript/'.$xscript->getID().'/', 'class' => 'button small grey', ), pht('View Transcript')), diff --git a/src/applications/herald/query/HeraldTranscriptQuery.php b/src/applications/herald/query/HeraldTranscriptQuery.php new file mode 100644 index 0000000000..8399218c61 --- /dev/null +++ b/src/applications/herald/query/HeraldTranscriptQuery.php @@ -0,0 +1,98 @@ +ids = $ids; + return $this; + } + + public function needPartialRecords($need_partial) { + $this->needPartialRecords = $need_partial; + return $this; + } + + public function loadPage() { + $transcript = new HeraldTranscript(); + $conn_r = $transcript->establishConnection('r'); + + // NOTE: Transcripts include a potentially enormous amount of serialized + // data, so we're loading only some of the fields here if the caller asked + // for partial records. + + if ($this->needPartialRecords) { + $fields = implode( + ', ', + array( + 'id', + 'phid', + 'objectPHID', + 'time', + 'duration', + 'dryRun', + 'host', + )); + } else { + $fields = '*'; + } + + $rows = queryfx_all( + $conn_r, + 'SELECT %Q FROM %T t %Q %Q %Q', + $fields, + $transcript->getTableName(), + $this->buildWhereClause($conn_r), + $this->buildOrderClause($conn_r), + $this->buildLimitClause($conn_r)); + + $transcripts = $transcript->loadAllFromArray($rows); + + if ($this->needPartialRecords) { + // Make sure nothing tries to write these; they aren't complete. + foreach ($transcripts as $transcript) { + $transcript->makeEphemeral(); + } + } + + return $transcripts; + } + + public function willFilterPage(array $transcripts) { + $phids = mpull($transcripts, 'getObjectPHID'); + + $objects = id(new PhabricatorObjectQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($phids) + ->execute(); + + foreach ($transcripts as $key => $transcript) { + if (empty($objects[$transcript->getObjectPHID()])) { + $this->didRejectResult($transcript); + unset($transcripts[$key]); + } + } + + return $transcripts; + } + + public function buildWhereClause(AphrontDatabaseConnection $conn_r) { + $where = array(); + + if ($this->ids) { + $where[] = qsprintf( + $conn_r, + 'id IN (%Ld)', + $this->ids); + } + + $where[] = $this->buildPagingClause($conn_r); + + return $this->formatWhereClause($where); + } + + +} diff --git a/src/applications/herald/storage/transcript/HeraldTranscript.php b/src/applications/herald/storage/transcript/HeraldTranscript.php index 713f91f0c5..8b04a8fcc9 100644 --- a/src/applications/herald/storage/transcript/HeraldTranscript.php +++ b/src/applications/herald/storage/transcript/HeraldTranscript.php @@ -1,6 +1,7 @@