diff --git a/resources/sql/autopatches/20181219.pholio.01.imagephid.sql b/resources/sql/autopatches/20181219.pholio.01.imagephid.sql new file mode 100644 index 0000000000..870cddd950 --- /dev/null +++ b/resources/sql/autopatches/20181219.pholio.01.imagephid.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_pholio.pholio_image + ADD mockPHID VARBINARY(64); diff --git a/resources/sql/autopatches/20181219.pholio.02.imagemigrate.php b/resources/sql/autopatches/20181219.pholio.02.imagemigrate.php new file mode 100644 index 0000000000..f1fc1b3c37 --- /dev/null +++ b/resources/sql/autopatches/20181219.pholio.02.imagemigrate.php @@ -0,0 +1,35 @@ +establishConnection('w'); +$iterator = new LiskRawMigrationIterator($conn, $image->getTableName()); + +foreach ($iterator as $image_row) { + if ($image_row['mockPHID']) { + continue; + } + + $mock_id = $image_row['mockID']; + + $mock_row = queryfx_one( + $conn, + 'SELECT phid FROM %R WHERE id = %d', + $mock, + $mock_id); + + if (!$mock_row) { + continue; + } + + queryfx( + $conn, + 'UPDATE %R SET mockPHID = %s WHERE id = %d', + $image, + $mock_row['phid'], + $image_row['id']); +} diff --git a/resources/sql/autopatches/20181219.pholio.03.imageid.sql b/resources/sql/autopatches/20181219.pholio.03.imageid.sql new file mode 100644 index 0000000000..3a3cb029ac --- /dev/null +++ b/resources/sql/autopatches/20181219.pholio.03.imageid.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_pholio.pholio_image + DROP mockID; diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index d1f7daf3a5..a653272fba 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -91,7 +91,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $images = $this->getNewImages(); foreach ($images as $image) { - $image->setMockID($object->getID()); + $image->setMockPHID($object->getPHID()); $image->save(); } diff --git a/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php b/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php index 041e14fcbf..b97a5fc3c7 100644 --- a/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php +++ b/src/applications/pholio/lipsum/PhabricatorPholioMockTestDataGenerator.php @@ -65,7 +65,7 @@ final class PhabricatorPholioMockTestDataGenerator ->setActor($author) ->applyTransactions($mock, $transactions); foreach ($images as $image) { - $image->setMockID($mock->getID()); + $image->setMockPHID($mock->getPHID()); $image->save(); } diff --git a/src/applications/pholio/phid/PholioImagePHIDType.php b/src/applications/pholio/phid/PholioImagePHIDType.php index b28dbd64d5..d7a9984fd5 100644 --- a/src/applications/pholio/phid/PholioImagePHIDType.php +++ b/src/applications/pholio/phid/PholioImagePHIDType.php @@ -32,13 +32,9 @@ final class PholioImagePHIDType extends PhabricatorPHIDType { foreach ($handles as $phid => $handle) { $image = $objects[$phid]; - $id = $image->getID(); - $mock_id = $image->getMockID(); - $name = $image->getName(); - - $handle->setURI("/M{$mock_id}/{$id}/"); - $handle->setName($name); - $handle->setFullName($name); + $handle + ->setName($image->getName()) + ->setURI($image->getURI()); } } diff --git a/src/applications/pholio/query/PholioImageQuery.php b/src/applications/pholio/query/PholioImageQuery.php index 5f541f1768..0fa7bfb1a0 100644 --- a/src/applications/pholio/query/PholioImageQuery.php +++ b/src/applications/pholio/query/PholioImageQuery.php @@ -5,8 +5,7 @@ final class PholioImageQuery private $ids; private $phids; - private $mockIDs; - private $obsolete; + private $mockPHIDs; private $needInlineComments; private $mockCache = array(); @@ -21,13 +20,8 @@ final class PholioImageQuery return $this; } - public function withMockIDs(array $mock_ids) { - $this->mockIDs = $mock_ids; - return $this; - } - - public function withObsolete($obsolete) { - $this->obsolete = $obsolete; + public function withMockPHIDs(array $mock_phids) { + $this->mockPHIDs = $mock_phids; return $this; } @@ -69,18 +63,11 @@ final class PholioImageQuery $this->phids); } - if ($this->mockIDs !== null) { + if ($this->mockPHIDs !== null) { $where[] = qsprintf( $conn, - 'mockID IN (%Ld)', - $this->mockIDs); - } - - if ($this->obsolete !== null) { - $where[] = qsprintf( - $conn, - 'isObsolete = %d', - $this->obsolete); + 'mockPHID IN (%Ls)', + $this->mockPHIDs); } return $where; @@ -92,16 +79,18 @@ final class PholioImageQuery if ($this->getMockCache()) { $mocks = $this->getMockCache(); } else { - $mock_ids = mpull($images, 'getMockID'); + $mock_phids = mpull($images, 'getMockPHID'); + // DO NOT set needImages to true; recursion results! $mocks = id(new PholioMockQuery()) ->setViewer($this->getViewer()) - ->withIDs($mock_ids) + ->withPHIDs($mock_phids) ->execute(); - $mocks = mpull($mocks, null, 'getID'); + $mocks = mpull($mocks, null, 'getPHID'); } + foreach ($images as $index => $image) { - $mock = idx($mocks, $image->getMockID()); + $mock = idx($mocks, $image->getMockPHID()); if ($mock) { $image->attachMock($mock); } else { diff --git a/src/applications/pholio/query/PholioMockQuery.php b/src/applications/pholio/query/PholioMockQuery.php index 5f1711def6..9e3154ec5c 100644 --- a/src/applications/pholio/query/PholioMockQuery.php +++ b/src/applications/pholio/query/PholioMockQuery.php @@ -115,18 +115,18 @@ final class PholioMockQuery $need_inline_comments) { assert_instances_of($mocks, 'PholioMock'); - $mock_map = mpull($mocks, null, 'getID'); + $mock_map = mpull($mocks, null, 'getPHID'); $all_images = id(new PholioImageQuery()) ->setViewer($viewer) ->setMockCache($mock_map) - ->withMockIDs(array_keys($mock_map)) + ->withMockPHIDs(array_keys($mock_map)) ->needInlineComments($need_inline_comments) ->execute(); - $image_groups = mgroup($all_images, 'getMockID'); + $image_groups = mgroup($all_images, 'getMockPHID'); foreach ($mocks as $mock) { - $mock_images = idx($image_groups, $mock->getID(), array()); + $mock_images = idx($image_groups, $mock->getPHID(), array()); $mock->attachAllImages($mock_images); $active_images = mfilter($mock_images, 'getIsObsolete', true); $mock->attachImages(msort($active_images, 'getSequence')); diff --git a/src/applications/pholio/storage/PholioImage.php b/src/applications/pholio/storage/PholioImage.php index 0f800bbb0f..1440773722 100644 --- a/src/applications/pholio/storage/PholioImage.php +++ b/src/applications/pholio/storage/PholioImage.php @@ -6,7 +6,7 @@ final class PholioImage extends PholioDAO PhabricatorExtendedPolicyInterface { protected $authorPHID; - protected $mockID; + protected $mockPHID; protected $filePHID; protected $name; protected $description; @@ -29,7 +29,7 @@ final class PholioImage extends PholioDAO return array( self::CONFIG_AUX_PHID => true, self::CONFIG_COLUMN_SCHEMA => array( - 'mockID' => 'id?', + 'mockPHID' => 'phid?', 'name' => 'text128', 'description' => 'text', 'sequence' => 'uint32', @@ -37,14 +37,9 @@ final class PholioImage extends PholioDAO 'replacesImagePHID' => 'phid?', ), self::CONFIG_KEY_SCHEMA => array( - 'key_phid' => null, - 'keyPHID' => array( - 'columns' => array('phid'), - 'unique' => true, - ), - 'mockID' => array( - 'columns' => array('mockID', 'isObsolete', 'sequence'), - ), + // TODO: There should be a key starting with "mockPHID" here at a + // minimum, but it's not entirely clear what other columns we should + // have as part of the key. ), ) + parent::getConfiguration(); } @@ -71,6 +66,10 @@ final class PholioImage extends PholioDAO return $this->assertAttached($this->mock); } + public function hasMock() { + return (bool)$this->getMockPHID(); + } + public function attachInlineComments(array $inline_comments) { assert_instances_of($inline_comments, 'PholioTransactionComment'); $this->inlineComments = $inline_comments; @@ -82,6 +81,22 @@ final class PholioImage extends PholioDAO return $this->inlineComments; } + public function getURI() { + if ($this->hasMock()) { + $mock = $this->getMock(); + + $mock_uri = $mock->getURI(); + $image_id = $this->getID(); + + return "{$mock_uri}/{$image_id}/"; + } + + // For now, standalone images have no URI. We could provide one at some + // point, although it's not clear that there's any motivation to do so. + + return null; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ @@ -96,7 +111,7 @@ final class PholioImage extends PholioDAO public function getPolicy($capability) { // If the image is attached to a mock, we use an extended policy to match // the mock's permissions. - if ($this->getMockID()) { + if ($this->hasMock()) { return PhabricatorPolicies::getMostOpenPolicy(); } @@ -113,7 +128,7 @@ final class PholioImage extends PholioDAO public function getExtendedPolicy($capability, PhabricatorUser $viewer) { - if ($this->getMockID()) { + if ($this->hasMock()) { return array( array( $this->getMock(), diff --git a/src/applications/pholio/storage/PholioMock.php b/src/applications/pholio/storage/PholioMock.php index cf32a34131..62285d0a59 100644 --- a/src/applications/pholio/storage/PholioMock.php +++ b/src/applications/pholio/storage/PholioMock.php @@ -259,7 +259,7 @@ final class PholioMock extends PholioDAO $this->openTransaction(); $images = id(new PholioImageQuery()) ->setViewer($engine->getViewer()) - ->withMockIDs(array($this->getID())) + ->withMockIDs(array($this->getPHID())) ->execute(); foreach ($images as $image) { $image->delete();