@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Give Drydock leases a resourcePHID instead of a resourceID

Summary:
Ref T9252. Leases currently have a `resourceID`, but this is a bit nonstandard and generally less flexible than giving them a `resourcePHID`.

In particular, a `resourcePHID` is easier to use when rendering interfaces, since you can get handles out of a PHID.

Add a PHID column, copy over all the PHIDs that correspond to existing IDs, then drop the ID column.

Test Plan:
- Browsed web UIs.
- Inspected database during/after migration.
- Grepped for `resourceID`.
- Allocated a new lease with `bin/drydock lease`.

Reviewers: chad, hach-que

Reviewed By: hach-que

Maniphest Tasks: T9252

Differential Revision: https://secure.phabricator.com/D14151

+39 -44
+2
resources/sql/autopatches/20150923.drydock.resourceid.1.sql
··· 1 + ALTER TABLE {$NAMESPACE}_drydock.drydock_lease 2 + ADD resourcePHID VARBINARY(64);
+5
resources/sql/autopatches/20150923.drydock.resourceid.2.sql
··· 1 + UPDATE 2 + {$NAMESPACE}_drydock.drydock_lease l, 3 + {$NAMESPACE}_drydock.drydock_resource r 4 + SET l.resourcePHID = r.phid 5 + WHERE l.resourceID = r.id;
+2
resources/sql/autopatches/20150923.drydock.resourceid.3.sql
··· 1 + ALTER TABLE {$NAMESPACE}_drydock.drydock_lease 2 + DROP resourceID;
+5 -12
src/applications/drydock/controller/DrydockLeaseViewController.php
··· 110 110 pht('Resource Type'), 111 111 $lease->getResourceType()); 112 112 113 - $resource = id(new DrydockResourceQuery()) 114 - ->setViewer($this->getViewer()) 115 - ->withIDs(array($lease->getResourceID())) 116 - ->executeOne(); 117 - 118 - if ($resource !== null) { 119 - $view->addProperty( 120 - pht('Resource'), 121 - $this->getViewer()->renderHandle($resource->getPHID())); 113 + $resource_phid = $lease->getResourcePHID(); 114 + if ($resource_phid) { 115 + $resource_display = $viewer->renderHandle($resource_phid); 122 116 } else { 123 - $view->addProperty( 124 - pht('Resource'), 125 - pht('No Resource')); 117 + $resource_display = phutil_tag('em', array(), pht('No Resource')); 126 118 } 119 + $view->addProperty(pht('Resource'), $resource_display); 127 120 128 121 $until = $lease->getUntil(); 129 122 if ($until) {
+1 -1
src/applications/drydock/controller/DrydockResourceViewController.php
··· 27 27 28 28 $leases = id(new DrydockLeaseQuery()) 29 29 ->setViewer($viewer) 30 - ->withResourceIDs(array($resource->getID())) 30 + ->withResourcePHIDs(array($resource->getPHID())) 31 31 ->execute(); 32 32 33 33 $lease_list = id(new DrydockLeaseListView())
+13 -11
src/applications/drydock/query/DrydockLeaseQuery.php
··· 4 4 5 5 private $ids; 6 6 private $phids; 7 - private $resourceIDs; 7 + private $resourcePHIDs; 8 8 private $statuses; 9 9 private $datasourceQuery; 10 10 private $needCommands; ··· 19 19 return $this; 20 20 } 21 21 22 - public function withResourceIDs(array $ids) { 23 - $this->resourceIDs = $ids; 22 + public function withResourcePHIDs(array $phids) { 23 + $this->resourcePHIDs = $phids; 24 24 return $this; 25 25 } 26 26 ··· 43 43 } 44 44 45 45 protected function willFilterPage(array $leases) { 46 - $resource_ids = array_filter(mpull($leases, 'getResourceID')); 47 - if ($resource_ids) { 46 + $resource_phids = array_filter(mpull($leases, 'getResourcePHID')); 47 + if ($resource_phids) { 48 48 $resources = id(new DrydockResourceQuery()) 49 49 ->setParentQuery($this) 50 50 ->setViewer($this->getViewer()) 51 - ->withIDs(array_unique($resource_ids)) 51 + ->withPHIDs(array_unique($resource_phids)) 52 52 ->execute(); 53 + $resources = mpull($resources, null, 'getPHID'); 53 54 } else { 54 55 $resources = array(); 55 56 } 56 57 57 58 foreach ($leases as $key => $lease) { 58 59 $resource = null; 59 - if ($lease->getResourceID()) { 60 - $resource = idx($resources, $lease->getResourceID()); 60 + if ($lease->getResourcePHID()) { 61 + $resource = idx($resources, $lease->getResourcePHID()); 61 62 if (!$resource) { 63 + $this->didRejectResult($lease); 62 64 unset($leases[$key]); 63 65 continue; 64 66 } ··· 72 74 protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { 73 75 $where = parent::buildWhereClauseParts($conn); 74 76 75 - if ($this->resourceIDs !== null) { 77 + if ($this->resourcePHIDs !== null) { 76 78 $where[] = qsprintf( 77 79 $conn, 78 - 'resourceID IN (%Ld)', 79 - $this->resourceIDs); 80 + 'resourcePHID IN (%Ls)', 81 + $this->resourcePHIDs); 80 82 } 81 83 82 84 if ($this->ids !== null) {
+5 -7
src/applications/drydock/storage/DrydockLease.php
··· 3 3 final class DrydockLease extends DrydockDAO 4 4 implements PhabricatorPolicyInterface { 5 5 6 - protected $resourceID; 6 + protected $resourcePHID; 7 7 protected $resourceType; 8 8 protected $until; 9 9 protected $ownerPHID; ··· 64 64 'until' => 'epoch?', 65 65 'resourceType' => 'text128', 66 66 'ownerPHID' => 'phid?', 67 - 'resourceID' => 'id?', 67 + 'resourcePHID' => 'phid?', 68 68 ), 69 69 self::CONFIG_KEY_SCHEMA => array( 70 - 'key_phid' => null, 71 - 'phid' => array( 72 - 'columns' => array('phid'), 73 - 'unique' => true, 70 + 'key_resource' => array( 71 + 'columns' => array('resourcePHID', 'status'), 74 72 ), 75 73 ), 76 74 ) + parent::getConfiguration(); ··· 219 217 $this->openTransaction(); 220 218 221 219 $this 222 - ->setResourceID($resource->getID()) 220 + ->setResourcePHID($resource->getPHID()) 223 221 ->setStatus($new_status) 224 222 ->save(); 225 223
+3 -3
src/applications/drydock/worker/DrydockAllocatorWorker.php
··· 462 462 'acquireLease()')); 463 463 } 464 464 465 - $lease_id = $lease->getResourceID(); 466 - $resource_id = $resource->getID(); 465 + $lease_phid = $lease->getResourcePHID(); 466 + $resource_phid = $resource->getPHID(); 467 467 468 - if ($lease_id !== $resource_id) { 468 + if ($lease_phid !== $resource_phid) { 469 469 // TODO: Destroy the lease? 470 470 throw new Exception( 471 471 pht(
+2 -9
src/applications/drydock/worker/DrydockLeaseWorker.php
··· 20 20 $actual_status)); 21 21 } 22 22 23 - $resource_id = $lease->getResourceID(); 24 - 25 - $resource = id(new DrydockResourceQuery()) 26 - ->setViewer($this->getViewer()) 27 - ->withIDs(array($resource_id)) 28 - ->executeOne(); 23 + $resource = $lease->getResource(); 29 24 if (!$resource) { 30 25 throw new PhabricatorWorkerPermanentFailureException( 31 - pht( 32 - 'Trying to activate lease on invalid resource ("%s").', 33 - $resource_id)); 26 + pht('Trying to activate lease with no resource.')); 34 27 } 35 28 36 29 $resource_status = $resource->getStatus();
+1 -1
src/applications/drydock/worker/DrydockResourceUpdateWorker.php
··· 72 72 73 73 $leases = id(new DrydockLeaseQuery()) 74 74 ->setViewer($viewer) 75 - ->withResourceIDs(array($resource->getID())) 75 + ->withResourcePHIDs(array($resource->getPHID())) 76 76 ->withStatuses($statuses) 77 77 ->execute(); 78 78