From 4f0f95f7b58785d8f7aba55a4806874e21e1d7b6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Nov 2013 13:59:06 -0800 Subject: [PATCH] Assign PHIDs to all diffs Summary: Ref T1049. Ref T2222. `DifferentialDiff` does not currently have a PHID, but we need it for Harbormaster and ApplicationTransactions. See some discussion in D7501. (I split the SQL into two sections so we can't fail in the middle. At some point, I'd like to do a pass on the migration stuff and get this happening automatically, and also simplify the PatchList.) Test Plan: - Ran `bin/storage upgrade`. - Checked for valid PHIDs in the database. - Used `phid.query` to look up a diff by PHID. - Created a new diff and verified it got a PHID. Reviewers: btrahan, hach-que Reviewed By: btrahan CC: aran, vrana Maniphest Tasks: T2222, T1049 Differential Revision: https://secure.phabricator.com/D7513 --- .../sql/patches/20131106.diffphid.1.col.sql | 2 + .../sql/patches/20131106.diffphid.2.mig.php | 47 +++++++++++++++++++ .../sql/patches/20131106.diffphid.3.key.sql | 2 + src/__phutil_library_map__.php | 2 + .../phid/DifferentialPHIDTypeDiff.php | 45 ++++++++++++++++++ .../query/DifferentialDiffQuery.php | 13 +++++ .../differential/storage/DifferentialDiff.php | 11 +++++ .../patch/PhabricatorBuiltinPatchList.php | 12 +++++ 8 files changed, 134 insertions(+) create mode 100644 resources/sql/patches/20131106.diffphid.1.col.sql create mode 100644 resources/sql/patches/20131106.diffphid.2.mig.php create mode 100644 resources/sql/patches/20131106.diffphid.3.key.sql create mode 100644 src/applications/differential/phid/DifferentialPHIDTypeDiff.php diff --git a/resources/sql/patches/20131106.diffphid.1.col.sql b/resources/sql/patches/20131106.diffphid.1.col.sql new file mode 100644 index 0000000000..d0a4fd5538 --- /dev/null +++ b/resources/sql/patches/20131106.diffphid.1.col.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_differential.differential_diff + ADD phid VARCHAR(64) NOT NULL COLLATE utf8_bin AFTER id; diff --git a/resources/sql/patches/20131106.diffphid.2.mig.php b/resources/sql/patches/20131106.diffphid.2.mig.php new file mode 100644 index 0000000000..eee7e3047c --- /dev/null +++ b/resources/sql/patches/20131106.diffphid.2.mig.php @@ -0,0 +1,47 @@ +establishConnection('w'); + +$size = 1000; + +$row_iter = id(new LiskMigrationIterator($diff_table))->setPageSize($size); +$chunk_iter = new PhutilChunkedIterator($row_iter, $size); + +foreach ($chunk_iter as $chunk) { + $sql = array(); + + foreach ($chunk as $diff) { + $id = $diff->getID(); + echo "Migrating diff ID {$id}...\n"; + + $phid = $diff->getPHID(); + if (strlen($phid)) { + continue; + } + + $type_diff = DifferentialPHIDTypeDiff::TYPECONST; + $new_phid = PhabricatorPHID::generateNewPHID($type_diff); + + $sql[] = qsprintf( + $conn_w, + '(%d, %s)', + $id, + $new_phid); + } + + if (!$sql) { + continue; + } + + foreach (PhabricatorLiskDAO::chunkSQL($sql, ', ') as $sql_chunk) { + queryfx( + $conn_w, + 'INSERT IGNORE INTO %T (id, phid) VALUES %Q + ON DUPLICATE KEY UPDATE phid = VALUES(phid)', + $diff_table->getTableName(), + $sql_chunk); + } +} + +echo "Done.\n"; diff --git a/resources/sql/patches/20131106.diffphid.3.key.sql b/resources/sql/patches/20131106.diffphid.3.key.sql new file mode 100644 index 0000000000..63d68dd89b --- /dev/null +++ b/resources/sql/patches/20131106.diffphid.3.key.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_differential.differential_diff + ADD UNIQUE KEY `key_phid` (phid); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bd09f8da62..3c9aa6b2a8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -396,6 +396,7 @@ phutil_register_library_map(array( 'DifferentialMailPhase' => 'applications/differential/constants/DifferentialMailPhase.php', 'DifferentialManiphestTasksFieldSpecification' => 'applications/differential/field/specification/DifferentialManiphestTasksFieldSpecification.php', 'DifferentialNewDiffMail' => 'applications/differential/mail/DifferentialNewDiffMail.php', + 'DifferentialPHIDTypeDiff' => 'applications/differential/phid/DifferentialPHIDTypeDiff.php', 'DifferentialPHIDTypeRevision' => 'applications/differential/phid/DifferentialPHIDTypeRevision.php', 'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php', 'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/DifferentialPathFieldSpecification.php', @@ -2606,6 +2607,7 @@ phutil_register_library_map(array( 'DifferentialMail' => 'PhabricatorMail', 'DifferentialManiphestTasksFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail', + 'DifferentialPHIDTypeDiff' => 'PhabricatorPHIDType', 'DifferentialPHIDTypeRevision' => 'PhabricatorPHIDType', 'DifferentialParseRenderTestCase' => 'PhabricatorTestCase', 'DifferentialPathFieldSpecification' => 'DifferentialFieldSpecification', diff --git a/src/applications/differential/phid/DifferentialPHIDTypeDiff.php b/src/applications/differential/phid/DifferentialPHIDTypeDiff.php new file mode 100644 index 0000000000..5b8029bd35 --- /dev/null +++ b/src/applications/differential/phid/DifferentialPHIDTypeDiff.php @@ -0,0 +1,45 @@ +withPHIDs($phids); + } + + public function loadHandles( + PhabricatorHandleQuery $query, + array $handles, + array $objects) { + + foreach ($handles as $phid => $handle) { + $diff = $objects[$phid]; + + $id = $diff->getID(); + + $handle->setName(pht('Diff %d', $id)); + } + } + + public function canLoadNamedObject($name) { + return false; + } + +} diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php index ec3265caa2..e32b641e36 100644 --- a/src/applications/differential/query/DifferentialDiffQuery.php +++ b/src/applications/differential/query/DifferentialDiffQuery.php @@ -4,6 +4,7 @@ final class DifferentialDiffQuery extends PhabricatorCursorPagedPolicyAwareQuery { private $ids; + private $phids; private $revisionIDs; private $needChangesets = false; private $needArcanistProjects = false; @@ -13,6 +14,11 @@ final class DifferentialDiffQuery return $this; } + public function withPHIDs(array $phids) { + $this->phids = $phids; + return $this; + } + public function withRevisionIDs(array $revision_ids) { $this->revisionIDs = $revision_ids; return $this; @@ -126,6 +132,13 @@ final class DifferentialDiffQuery $this->ids); } + if ($this->phids) { + $where[] = qsprintf( + $conn_r, + 'phid IN (%Ls)', + $this->phids); + } + if ($this->revisionIDs) { $where[] = qsprintf( $conn_r, diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index 9e0a36dce9..96bb8de94f 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -34,6 +34,17 @@ final class DifferentialDiff private $arcanistProject = self::ATTACHABLE; private $revision = self::ATTACHABLE; + public function getConfiguration() { + return array( + self::CONFIG_AUX_PHID => true, + ) + parent::getConfiguration(); + } + + public function generatePHID() { + return PhabricatorPHID::generateNewPHID( + DifferentialPHIDTypeDiff::TYPECONST); + } + public function addUnsavedChangeset(DifferentialChangeset $changeset) { if ($this->changesets === null) { $this->changesets = array(); diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 04b303273c..604d333061 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1724,6 +1724,18 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20131105.buildstep.sql'), ), + '20131106.diffphid.1.col.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20131106.diffphid.1.col.sql'), + ), + '20131106.diffphid.2.mig.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20131106.diffphid.2.mig.php'), + ), + '20131106.diffphid.3.key.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20131106.diffphid.3.key.sql'), + ), ); } }