From 6b2d480fe7931d964c84c87fb6c91e4d9afdc278 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 26 Dec 2013 10:41:36 -0800 Subject: [PATCH] Make DrydockLease a policy-aware object Summary: Ref T2015. DrydockLease predates widespread adoption of policies. Make it -- and its query -- policy aware. Test Plan: Browsed leases from the web UI. Grepped for callsites. Reviewers: btrahan Reviewed By: btrahan CC: hach-que, aran Maniphest Tasks: T2015 Differential Revision: https://secure.phabricator.com/D7826 --- .../DrydockBlueprintImplementation.php | 3 +- .../controller/DrydockLeaseListController.php | 2 +- .../DrydockLeaseReleaseController.php | 7 ++- .../DrydockResourceViewController.php | 2 +- .../DrydockManagementReleaseWorkflow.php | 11 +++- .../drydock/query/DrydockLeaseQuery.php | 51 +++++++++---------- .../drydock/storage/DrydockLease.php | 25 ++++++++- .../build/HarbormasterBuildArtifact.php | 1 + 8 files changed, 66 insertions(+), 36 deletions(-) diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php index bc8c7e8539..5b794d3479 100644 --- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php @@ -26,9 +26,10 @@ abstract class DrydockBlueprintImplementation { } protected function loadLease($lease_id) { + // TODO: Get rid of this? $query = id(new DrydockLeaseQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withIDs(array($lease_id)) - ->needResources(true) ->execute(); $lease = idx($query, $lease_id); diff --git a/src/applications/drydock/controller/DrydockLeaseListController.php b/src/applications/drydock/controller/DrydockLeaseListController.php index 791299ff4f..b168f9bc7c 100644 --- a/src/applications/drydock/controller/DrydockLeaseListController.php +++ b/src/applications/drydock/controller/DrydockLeaseListController.php @@ -13,7 +13,7 @@ final class DrydockLeaseListController extends DrydockController { $pager->setOffset($request->getInt('offset')); $leases = id(new DrydockLeaseQuery()) - ->needResources(true) + ->setViewer($user) ->executeWithOffsetPager($pager); $title = pht('Leases'); diff --git a/src/applications/drydock/controller/DrydockLeaseReleaseController.php b/src/applications/drydock/controller/DrydockLeaseReleaseController.php index dd238e2467..f3cc501569 100644 --- a/src/applications/drydock/controller/DrydockLeaseReleaseController.php +++ b/src/applications/drydock/controller/DrydockLeaseReleaseController.php @@ -12,7 +12,10 @@ final class DrydockLeaseReleaseController extends DrydockController { $request = $this->getRequest(); $user = $request->getUser(); - $lease = id(new DrydockLease())->load($this->id); + $lease = id(new DrydockLeaseQuery()) + ->setViewer($user) + ->withIDs(array($this->id)) + ->executeOne(); if (!$lease) { return new Aphront404Response(); } @@ -45,7 +48,7 @@ final class DrydockLeaseReleaseController extends DrydockController { return id(new AphrontDialogResponse())->setDialog($dialog); } - $resource = $lease->loadResource(); + $resource = $lease->getResource(); $blueprint = $resource->getBlueprint(); $blueprint->releaseLease($resource, $lease); diff --git a/src/applications/drydock/controller/DrydockResourceViewController.php b/src/applications/drydock/controller/DrydockResourceViewController.php index 46ecc1a4c4..459d0160bf 100644 --- a/src/applications/drydock/controller/DrydockResourceViewController.php +++ b/src/applications/drydock/controller/DrydockResourceViewController.php @@ -29,8 +29,8 @@ final class DrydockResourceViewController extends DrydockController { $resource_uri = $this->getApplicationURI($resource_uri); $leases = id(new DrydockLeaseQuery()) + ->setViewer($user) ->withResourceIDs(array($resource->getID())) - ->needResources(true) ->execute(); $lease_list = $this->buildLeaseListView($leases); diff --git a/src/applications/drydock/management/DrydockManagementReleaseWorkflow.php b/src/applications/drydock/management/DrydockManagementReleaseWorkflow.php index 54b98bf62d..5f482786ad 100644 --- a/src/applications/drydock/management/DrydockManagementReleaseWorkflow.php +++ b/src/applications/drydock/management/DrydockManagementReleaseWorkflow.php @@ -25,14 +25,21 @@ final class DrydockManagementReleaseWorkflow "Specify one or more lease IDs to release."); } + $viewer = PhabricatorUser::getOmnipotentUser(); + + $leases = id(new DrydockLeaseQuery()) + ->setViewer($viewer) + ->withIDs($ids) + ->execute(); + foreach ($ids as $id) { - $lease = id(new DrydockLease())->load($id); + $lease = idx($leases, $id); if (!$lease) { $console->writeErr("Lease %d does not exist!\n", $id); } else if ($lease->getStatus() != DrydockLeaseStatus::STATUS_ACTIVE) { $console->writeErr("Lease %d is not 'active'!\n", $id); } else { - $resource = $lease->loadResource(); + $resource = $lease->getResource(); $blueprint = $resource->getBlueprint(); $blueprint->releaseLease($resource, $lease); diff --git a/src/applications/drydock/query/DrydockLeaseQuery.php b/src/applications/drydock/query/DrydockLeaseQuery.php index 9b67acd363..05cd40c49a 100644 --- a/src/applications/drydock/query/DrydockLeaseQuery.php +++ b/src/applications/drydock/query/DrydockLeaseQuery.php @@ -1,27 +1,22 @@ resourceIDs = $ids; - return $this; - } public function withIDs(array $ids) { $this->ids = $ids; return $this; } - public function needResources($need_resources) { - $this->needResources = $need_resources; + public function withResourceIDs(array $ids) { + $this->resourceIDs = $ids; return $this; } - public function execute() { + public function loadPage() { $table = new DrydockLease(); $conn_r = $table->establishConnection('r'); @@ -33,25 +28,23 @@ final class DrydockLeaseQuery extends PhabricatorOffsetPagedQuery { $this->buildOrderClause($conn_r), $this->buildLimitClause($conn_r)); - $leases = $table->loadAllFromArray($data); + return $table->loadAllFromArray($data); + } - if ($leases && $this->needResources) { - $resources = id(new DrydockResource())->loadAllWhere( - 'id IN (%Ld)', - mpull($leases, 'getResourceID')); + public function willFilterPage(array $leases) { + $resources = id(new DrydockResourceQuery()) + ->setParentQuery($this) + ->setViewer($this->getViewer()) + ->withIDs(mpull($leases, 'getResourceID')) + ->execute(); - foreach ($leases as $key => $lease) { - if ($lease->getResourceID()) { - $resource = idx($resources, $lease->getResourceID()); - if ($resource) { - $lease->attachResource($resource); - } else { - unset($leases[$key]); - } - } else { - unset($leases[$key]); - } + foreach ($leases as $key => $lease) { + $resource = idx($resources, $lease->getResourceID()); + if (!$resource) { + unset($leases[$key]); + continue; } + $lease->attachResource($resource); } return $leases; @@ -74,11 +67,13 @@ final class DrydockLeaseQuery extends PhabricatorOffsetPagedQuery { $this->ids); } + $where[] = $this->buildPagingClause($conn_r); + return $this->formatWhereClause($where); } - private function buildOrderClause(AphrontDatabaseConnection $conn_r) { - return qsprintf($conn_r, 'ORDER BY id DESC'); + public function getQueryApplicationClass() { + return 'PhabricatorApplicationDrydock'; } } diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php index 9e08dbd3b1..fdaac35e9f 100644 --- a/src/applications/drydock/storage/DrydockLease.php +++ b/src/applications/drydock/storage/DrydockLease.php @@ -1,6 +1,7 @@ getResource()->getPolicy($capability); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return $this->getResource()->hasAutomaticCapability($capability, $viewer); + } + + public function describeAutomaticCapability($capability) { + return pht('Leases inherit policies from the resources they lease.'); + } + } diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php index 56bf746b5d..0511c526a9 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildArtifact.php @@ -59,6 +59,7 @@ final class HarbormasterBuildArtifact extends HarbormasterDAO ->setHref($handle->getURI()); case self::TYPE_HOST: $leases = id(new DrydockLeaseQuery()) + ->setViewer($viewer) ->withIDs(array($data["drydock-lease"])) ->execute(); $lease = $leases[$data["drydock-lease"]];