From e793a3657a2e618fa08c2aaba90c8db7a72bafe7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Jul 2013 12:11:34 -0700 Subject: [PATCH] Use a real Query class to load daemon information Summary: Ref T3557. This stuff does a bunch of nonsense in the View right now. Instead, do it in a real Query class. Fixes a long-standing bug which prevented "all daemons" from showing more than 3 days' worth of data. Test Plan: Viewed `/daemon/`, viewed "All Daemons". Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3557 Differential Revision: https://secure.phabricator.com/D6544 --- src/__phutil_library_map__.php | 13 ++- .../PhabricatorDaemonConsoleController.php | 10 +- .../PhabricatorDaemonLogListController.php | 19 ++- .../query/PhabricatorDaemonLogQuery.php | 108 +++++++++++++++++- .../daemon/storage/PhabricatorDaemonDAO.php | 0 .../daemon/storage/PhabricatorDaemonLog.php | 44 +++++++ .../storage/PhabricatorDaemonLogEvent.php | 0 .../view/PhabricatorDaemonLogListView.php | 38 +----- .../daemon/storage/PhabricatorDaemonLog.php | 25 ---- 9 files changed, 173 insertions(+), 84 deletions(-) rename src/{infrastructure => applications}/daemon/storage/PhabricatorDaemonDAO.php (100%) create mode 100644 src/applications/daemon/storage/PhabricatorDaemonLog.php rename src/{infrastructure => applications}/daemon/storage/PhabricatorDaemonLogEvent.php (100%) delete mode 100644 src/infrastructure/daemon/storage/PhabricatorDaemonLog.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 359adc1d55..f8af211340 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1019,10 +1019,10 @@ phutil_register_library_map(array( 'PhabricatorDaemonCombinedLogController' => 'applications/daemon/controller/PhabricatorDaemonCombinedLogController.php', 'PhabricatorDaemonConsoleController' => 'applications/daemon/controller/PhabricatorDaemonConsoleController.php', 'PhabricatorDaemonController' => 'applications/daemon/controller/PhabricatorDaemonController.php', - 'PhabricatorDaemonDAO' => 'infrastructure/daemon/storage/PhabricatorDaemonDAO.php', + 'PhabricatorDaemonDAO' => 'applications/daemon/storage/PhabricatorDaemonDAO.php', 'PhabricatorDaemonEventListener' => 'applications/daemon/event/PhabricatorDaemonEventListener.php', - 'PhabricatorDaemonLog' => 'infrastructure/daemon/storage/PhabricatorDaemonLog.php', - 'PhabricatorDaemonLogEvent' => 'infrastructure/daemon/storage/PhabricatorDaemonLogEvent.php', + 'PhabricatorDaemonLog' => 'applications/daemon/storage/PhabricatorDaemonLog.php', + 'PhabricatorDaemonLogEvent' => 'applications/daemon/storage/PhabricatorDaemonLogEvent.php', 'PhabricatorDaemonLogEventsView' => 'applications/daemon/view/PhabricatorDaemonLogEventsView.php', 'PhabricatorDaemonLogListController' => 'applications/daemon/controller/PhabricatorDaemonLogListController.php', 'PhabricatorDaemonLogListView' => 'applications/daemon/view/PhabricatorDaemonLogListView.php', @@ -3023,11 +3023,16 @@ phutil_register_library_map(array( 'PhabricatorDaemonController' => 'PhabricatorController', 'PhabricatorDaemonDAO' => 'PhabricatorLiskDAO', 'PhabricatorDaemonEventListener' => 'PhutilEventListener', - 'PhabricatorDaemonLog' => 'PhabricatorDaemonDAO', + 'PhabricatorDaemonLog' => + array( + 0 => 'PhabricatorDaemonDAO', + 1 => 'PhabricatorPolicyInterface', + ), 'PhabricatorDaemonLogEvent' => 'PhabricatorDaemonDAO', 'PhabricatorDaemonLogEventsView' => 'AphrontView', 'PhabricatorDaemonLogListController' => 'PhabricatorDaemonController', 'PhabricatorDaemonLogListView' => 'AphrontView', + 'PhabricatorDaemonLogQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorDaemonLogViewController' => 'PhabricatorDaemonController', 'PhabricatorDaemonManagementDebugWorkflow' => 'PhabricatorDaemonManagementWorkflow', 'PhabricatorDaemonManagementLaunchWorkflow' => 'PhabricatorDaemonManagementWorkflow', diff --git a/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php b/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php index 63c76e3ae3..4fbff890de 100644 --- a/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php +++ b/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php @@ -57,13 +57,13 @@ final class PhabricatorDaemonConsoleController $completed_panel->appendChild($completed_table); $completed_panel->setNoBackground(); - $logs = id(new PhabricatorDaemonLog())->loadAllWhere( - '`status` = %s ORDER BY id DESC', - 'run'); - + $logs = id(new PhabricatorDaemonLogQuery()) + ->setViewer($user) + ->withStatus(PhabricatorDaemonLogQuery::STATUS_ALIVE) + ->execute(); $daemon_header = id(new PhabricatorHeaderView()) - ->setHeader(pht('Recent Daemons')); + ->setHeader(pht('Active Daemons')); $daemon_table = new PhabricatorDaemonLogListView(); $daemon_table->setUser($user); diff --git a/src/applications/daemon/controller/PhabricatorDaemonLogListController.php b/src/applications/daemon/controller/PhabricatorDaemonLogListController.php index 9dd56ee74a..75a473dcc0 100644 --- a/src/applications/daemon/controller/PhabricatorDaemonLogListController.php +++ b/src/applications/daemon/controller/PhabricatorDaemonLogListController.php @@ -5,20 +5,14 @@ final class PhabricatorDaemonLogListController public function processRequest() { $request = $this->getRequest(); + $viewer = $request->getUser(); - $pager = new AphrontPagerView(); - $pager->setOffset($request->getInt('page')); + $pager = new AphrontCursorPagerView(); + $pager->readFromRequest($request); - $clause = '1 = 1'; - - $logs = id(new PhabricatorDaemonLog())->loadAllWhere( - '%Q ORDER BY id DESC LIMIT %d, %d', - $clause, - $pager->getOffset(), - $pager->getPageSize() + 1); - - $logs = $pager->sliceResults($logs); - $pager->setURI($request->getRequestURI(), 'page'); + $logs = id(new PhabricatorDaemonLogQuery()) + ->setViewer($viewer) + ->executeWithCursorPager($pager); $daemon_table = new PhabricatorDaemonLogListView(); $daemon_table->setUser($request->getUser()); @@ -33,6 +27,7 @@ final class PhabricatorDaemonLogListController $nav->selectFilter('log'); $nav->setCrumbs($crumbs); $nav->appendChild($daemon_table); + $nav->appendChild($pager); return $this->buildApplicationPage( $nav, diff --git a/src/applications/daemon/query/PhabricatorDaemonLogQuery.php b/src/applications/daemon/query/PhabricatorDaemonLogQuery.php index 45e56275a2..5f47eb5a92 100644 --- a/src/applications/daemon/query/PhabricatorDaemonLogQuery.php +++ b/src/applications/daemon/query/PhabricatorDaemonLogQuery.php @@ -1,6 +1,12 @@ status = $status; + return $this; + } + + public function loadPage() { + $table = new PhabricatorDaemonLog(); + $conn_r = $table->establishConnection('r'); + + $data = queryfx_all( + $conn_r, + 'SELECT * FROM %T %Q %Q %Q', + $table->getTableName(), + $this->buildWhereClause($conn_r), + $this->buildOrderClause($conn_r), + $this->buildLimitClause($conn_r)); + + return $table->loadAllFromArray($data); + } + + public function willFilterPage(array $daemons) { + $unknown_delay = PhabricatorDaemonLogQuery::getTimeUntilUnknown(); + $dead_delay = PhabricatorDaemonLogQuery::getTimeUntilDead(); + + $status_running = PhabricatorDaemonLog::STATUS_RUNNING; + $status_unknown = PhabricatorDaemonLog::STATUS_UNKNOWN; + $status_wait = PhabricatorDaemonLog::STATUS_WAIT; + $status_exited = PhabricatorDaemonLog::STATUS_EXITED; + $status_dead = PhabricatorDaemonLog::STATUS_DEAD; + + $filter = array_fuse($this->getStatusConstants()); + + foreach ($daemons as $key => $daemon) { + $status = $daemon->getStatus(); + $seen = $daemon->getDateModified(); + + $is_running = ($status == $status_running) || + ($status == $status_wait); + + // If we haven't seen the daemon recently, downgrade its status to + // unknown. + $unknown_time = ($seen + $unknown_delay); + if ($is_running && ($unknown_time < time())) { + $status = $status_unknown; + } + + // If the daemon hasn't been seen in quite a while, assume it is dead. + $dead_time = ($seen + $dead_delay); + if (($status == $status_unknown) && ($dead_time < time())) { + $status = $status_dead; + } + + // If we changed the daemon's status, update it. + if ($status != $daemon->getStatus()) { + $guard = AphrontWriteGuard::beginScopedUnguardedWrites(); + $daemon->setStatus($status)->save(); + unset($guard); + } + + // If the daemon no longer matches the filter, get rid of it. + if ($filter) { + if (empty($filter[$daemon->getStatus()])) { + unset($daemons[$key]); + } + } + } + + return $daemons; + } + + private function buildWhereClause(AphrontDatabaseConnection $conn_r) { + $where = array(); + + if ($this->getStatusConstants()) { + $where[] = qsprintf( + $conn_r, + 'status IN (%Ls)', + $this->getStatusConstants()); + } + + $where[] = $this->buildPagingClause($conn_r); + return $this->formatWhereClause($where); + } + + private function getStatusConstants() { + $status = $this->status; + switch ($status) { + case self::STATUS_ALL: + return array(); + case self::STATUS_ALIVE: + return array( + PhabricatorDaemonLog::STATUS_UNKNOWN, + PhabricatorDaemonLog::STATUS_RUNNING, + PhabricatorDaemonLog::STATUS_WAIT, + ); + default: + throw new Exception("Unknown status '{$status}'!"); + } + } + } diff --git a/src/infrastructure/daemon/storage/PhabricatorDaemonDAO.php b/src/applications/daemon/storage/PhabricatorDaemonDAO.php similarity index 100% rename from src/infrastructure/daemon/storage/PhabricatorDaemonDAO.php rename to src/applications/daemon/storage/PhabricatorDaemonDAO.php diff --git a/src/applications/daemon/storage/PhabricatorDaemonLog.php b/src/applications/daemon/storage/PhabricatorDaemonLog.php new file mode 100644 index 0000000000..b836d9ae81 --- /dev/null +++ b/src/applications/daemon/storage/PhabricatorDaemonLog.php @@ -0,0 +1,44 @@ + array( + 'argv' => self::SERIALIZATION_JSON, + ), + ) + parent::getConfiguration(); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + return PhabricatorPolicies::POLICY_ADMIN; + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + +} diff --git a/src/infrastructure/daemon/storage/PhabricatorDaemonLogEvent.php b/src/applications/daemon/storage/PhabricatorDaemonLogEvent.php similarity index 100% rename from src/infrastructure/daemon/storage/PhabricatorDaemonLogEvent.php rename to src/applications/daemon/storage/PhabricatorDaemonLogEvent.php diff --git a/src/applications/daemon/view/PhabricatorDaemonLogListView.php b/src/applications/daemon/view/PhabricatorDaemonLogListView.php index 303c8cfbdc..a27d90b525 100644 --- a/src/applications/daemon/view/PhabricatorDaemonLogListView.php +++ b/src/applications/daemon/view/PhabricatorDaemonLogListView.php @@ -19,43 +19,6 @@ final class PhabricatorDaemonLogListView extends AphrontView { $list = id(new PhabricatorObjectItemListView()); foreach ($this->daemonLogs as $log) { - - // TODO: VVV Move this stuff to a Query class. VVV - - $expect_heartbeat = PhabricatorDaemonLogQuery::getTimeUntilUnknown(); - $assume_dead = PhabricatorDaemonLogQuery::getTimeUntilDead(); - - $status_running = PhabricatorDaemonLog::STATUS_RUNNING; - $status_unknown = PhabricatorDaemonLog::STATUS_UNKNOWN; - $status_wait = PhabricatorDaemonLog::STATUS_WAIT; - $status_exited = PhabricatorDaemonLog::STATUS_EXITED; - $status_dead = PhabricatorDaemonLog::STATUS_DEAD; - - $status = $log->getStatus(); - $heartbeat_timeout = $log->getDateModified() + $expect_heartbeat; - if ($status == $status_running && $heartbeat_timeout < time()) { - $status = $status_unknown; - } - - if ($status == $status_unknown && $assume_dead < time()) { - $guard = AphrontWriteGuard::beginScopedUnguardedWrites(); - $log->setStatus($status_dead)->save(); - unset($guard); - } - - if ($status != $status_running && - $log->getDateModified() + (3 * 86400) < time()) { - // Don't show rows that haven't been running for more than - // three days. We should probably prune these out of the - // DB similar to the code above, but we don't need to be - // conservative and do it only on the same host - - // TODO: This should not apply to the "all daemons" view! - continue; - } - - // TODO: ^^^^ ALL THAT STUFF ^^^ - $id = $log->getID(); $epoch = $log->getDateCreated(); @@ -65,6 +28,7 @@ final class PhabricatorDaemonLogListView extends AphrontView { ->setHref("/daemon/log/{$id}/") ->addIcon('none', phabricator_datetime($epoch, $this->user)); + $status = $log->getStatus(); switch ($status) { case PhabricatorDaemonLog::STATUS_RUNNING: $item->setBarColor('green'); diff --git a/src/infrastructure/daemon/storage/PhabricatorDaemonLog.php b/src/infrastructure/daemon/storage/PhabricatorDaemonLog.php deleted file mode 100644 index c8afb2de4e..0000000000 --- a/src/infrastructure/daemon/storage/PhabricatorDaemonLog.php +++ /dev/null @@ -1,25 +0,0 @@ - array( - 'argv' => self::SERIALIZATION_JSON, - ), - ) + parent::getConfiguration(); - } - -}