From 77bcbed9f98e012f1f9612491471fff5956c29ec Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 20 Jan 2015 13:32:43 -0800 Subject: [PATCH] Implement PolicyAwareQuery for triggers Summary: Ref T6881. I tried to cheat here by not implementing this, but we need it for destroying triggers directly with `bin/remove destroy`, since that needs to load them by PHID. So, cheat slightly less. Implement PolicyAware but not CursorPagedPolicyAware. Test Plan: - Used `bin/remove destroy` to destroy a trigger by PHID. - Browsed daemon console. - Ran trigger daemon. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T6881 Differential Revision: https://secure.phabricator.com/D11445 --- src/__phutil_library_map__.php | 3 +- .../PhabricatorDaemonConsoleController.php | 10 +++--- .../workers/PhabricatorTriggerDaemon.php | 3 ++ ...ricatorWorkerTriggerManagementWorkflow.php | 1 + .../phid/PhabricatorWorkerTriggerPHIDType.php | 10 ++---- .../query/PhabricatorWorkerTriggerQuery.php | 18 +++++++++-- .../storage/PhabricatorWorkerTrigger.php | 31 +++++++++++++++++-- 7 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4a6610d31d..4cac52a01e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -5889,12 +5889,13 @@ phutil_register_library_map(array( 'PhabricatorWorkerTrigger' => array( 'PhabricatorWorkerDAO', 'PhabricatorDestructibleInterface', + 'PhabricatorPolicyInterface', ), 'PhabricatorWorkerTriggerEvent' => 'PhabricatorWorkerDAO', 'PhabricatorWorkerTriggerManagementFireWorkflow' => 'PhabricatorWorkerTriggerManagementWorkflow', 'PhabricatorWorkerTriggerManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorWorkerTriggerPHIDType' => 'PhabricatorPHIDType', - 'PhabricatorWorkerTriggerQuery' => 'PhabricatorOffsetPagedQuery', + 'PhabricatorWorkerTriggerQuery' => 'PhabricatorPolicyAwareQuery', 'PhabricatorWorkerYieldException' => 'Exception', 'PhabricatorWorkingCopyDiscoveryTestCase' => 'PhabricatorWorkingCopyTestCase', 'PhabricatorWorkingCopyPullTestCase' => 'PhabricatorWorkingCopyTestCase', diff --git a/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php b/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php index fc65b16af7..b1b5d4c385 100644 --- a/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php +++ b/src/applications/daemon/controller/PhabricatorDaemonConsoleController.php @@ -3,9 +3,8 @@ final class PhabricatorDaemonConsoleController extends PhabricatorDaemonController { - public function processRequest() { - $request = $this->getRequest(); - $user = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $this->getViewer(); $window_start = (time() - (60 * 15)); @@ -71,7 +70,7 @@ final class PhabricatorDaemonConsoleController } $logs = id(new PhabricatorDaemonLogQuery()) - ->setViewer($user) + ->setViewer($viewer) ->withStatus(PhabricatorDaemonLogQuery::STATUS_ALIVE) ->setAllowStatusWrites(true) ->execute(); @@ -123,7 +122,7 @@ final class PhabricatorDaemonConsoleController $completed_panel->appendChild($completed_table); $daemon_table = new PhabricatorDaemonLogListView(); - $daemon_table->setUser($user); + $daemon_table->setUser($viewer); $daemon_table->setDaemonLogs($logs); $daemon_panel = new PHUIObjectBoxView(); @@ -190,6 +189,7 @@ final class PhabricatorDaemonConsoleController ->setNoDataString(pht('Task queue is empty.'))); $triggers = id(new PhabricatorWorkerTriggerQuery()) + ->setViewer($viewer) ->setOrder(PhabricatorWorkerTriggerQuery::ORDER_EXECUTION) ->needEvents(true) ->setLimit(10) diff --git a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php index cdabe3c9cd..51352cc21d 100644 --- a/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php +++ b/src/infrastructure/daemon/workers/PhabricatorTriggerDaemon.php @@ -102,6 +102,7 @@ final class PhabricatorTriggerDaemon $limit = 100; $query = id(new PhabricatorWorkerTriggerQuery()) + ->setViewer($this->getViewer()) ->withVersionBetween($cursor, null) ->setOrder(PhabricatorWorkerTriggerQuery::ORDER_VERSION) ->needEvents(true) @@ -183,6 +184,7 @@ final class PhabricatorTriggerDaemon $now = PhabricatorTime::getNow(); $triggers = id(new PhabricatorWorkerTriggerQuery()) + ->setViewer($this->getViewer()) ->setOrder(PhabricatorWorkerTriggerQuery::ORDER_EXECUTION) ->withNextEventBetween(null, $now) ->needEvents(true) @@ -249,6 +251,7 @@ final class PhabricatorTriggerDaemon $sleep = 60; $next_triggers = id(new PhabricatorWorkerTriggerQuery()) + ->setViewer($this->getViewer()) ->setOrder(PhabricatorWorkerTriggerQuery::ORDER_EXECUTION) ->setLimit(1) ->needEvents(true) diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerTriggerManagementWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerTriggerManagementWorkflow.php index 40ee3edb84..aee92f1f99 100644 --- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerTriggerManagementWorkflow.php +++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerTriggerManagementWorkflow.php @@ -22,6 +22,7 @@ abstract class PhabricatorWorkerTriggerManagementWorkflow } $triggers = id(new PhabricatorWorkerTriggerQuery()) + ->setViewer($this->getViewer()) ->withIDs($ids) ->needEvents(true) ->execute(); diff --git a/src/infrastructure/daemon/workers/phid/PhabricatorWorkerTriggerPHIDType.php b/src/infrastructure/daemon/workers/phid/PhabricatorWorkerTriggerPHIDType.php index 5d5123fdb3..02112c91b5 100644 --- a/src/infrastructure/daemon/workers/phid/PhabricatorWorkerTriggerPHIDType.php +++ b/src/infrastructure/daemon/workers/phid/PhabricatorWorkerTriggerPHIDType.php @@ -9,19 +9,15 @@ final class PhabricatorWorkerTriggerPHIDType extends PhabricatorPHIDType { } public function newObject() { - return new PhabricatorWorkerTriggerPHIDType(); + return new PhabricatorWorkerTrigger(); } protected function buildQueryForObjects( PhabricatorObjectQuery $query, array $phids) { - // TODO: Maybe straighten this out eventually, but these aren't policy - // objects and don't have an applicable query which we can return here. - // Since we should never call this normally, just leave it stubbed for - // now. - - throw new PhutilMethodNotImplementedException(); + return id(new PhabricatorWorkerTriggerQuery()) + ->withPHIDs($phids); } public function loadHandles( diff --git a/src/infrastructure/daemon/workers/query/PhabricatorWorkerTriggerQuery.php b/src/infrastructure/daemon/workers/query/PhabricatorWorkerTriggerQuery.php index 72da00bb68..2524a2e217 100644 --- a/src/infrastructure/daemon/workers/query/PhabricatorWorkerTriggerQuery.php +++ b/src/infrastructure/daemon/workers/query/PhabricatorWorkerTriggerQuery.php @@ -1,7 +1,11 @@ ids = $ids; return $this; @@ -59,7 +67,13 @@ final class PhabricatorWorkerTriggerQuery return $this; } - public function execute() { + public function nextPage(array $page) { + // NOTE: We don't implement paging because we don't currently ever need + // it and paging ORDER_EXCUTION is a hassle. + throw new PhutilMethodNotImplementedException(); + } + + public function loadPage() { $task_table = new PhabricatorWorkerTrigger(); $conn_r = $task_table->establishConnection('r'); diff --git a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php index abbe095670..c4104a5c6e 100644 --- a/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php +++ b/src/infrastructure/daemon/workers/storage/PhabricatorWorkerTrigger.php @@ -3,7 +3,8 @@ final class PhabricatorWorkerTrigger extends PhabricatorWorkerDAO implements - PhabricatorDestructibleInterface { + PhabricatorDestructibleInterface, + PhabricatorPolicyInterface { protected $triggerVersion; protected $clockClass; @@ -143,7 +144,7 @@ final class PhabricatorWorkerTrigger // gymnastics, so don't bother trying to get it totally correct for now. if ($this->getEvent()) { - return $this->getEvent()->getNextEpoch(); + return $this->getEvent()->getNextEventEpoch(); } else { return $this->getNextEventEpoch(null, $is_reschedule = false); } @@ -167,4 +168,30 @@ final class PhabricatorWorkerTrigger $this->saveTransaction(); } + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + // NOTE: Triggers are low-level infrastructure and do not have real + // policies, but implementing the policy interface allows us to use + // infrastructure like handles. + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + return PhabricatorPolicies::getMostOpenPolicy(); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return true; + } + + public function describeAutomaticCapability($capability) { + return null; + } + }