From bd40e7440053ce62842f716677bc7833995680aa Mon Sep 17 00:00:00 2001 From: epriestley <git@epriestley.com> Date: Mon, 16 Sep 2013 16:02:06 -0700 Subject: [PATCH] Migrate auxiliary field storage to common field storage Summary: Ref T418. Moves data from the Maniphest-specific table to the general one. This patch is a bit gross, but mostly about getting the reads and writes aimed correctly. Future patches will clean things up. Test Plan: Migrated data across formats. Verified it survied the migration. Viewed and edited tasks' custom fields. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T418 Differential Revision: https://secure.phabricator.com/D6999 --- .../sql/patches/20130915.maniphestmigrate.php | 25 +++++++++++ src/__phutil_library_map__.php | 2 - .../ManiphestAuxiliaryFieldSpecification.php | 6 ++- .../conduit/ConduitAPI_maniphest_Method.php | 9 +--- .../maniphest/storage/ManiphestTask.php | 44 ++++++++++++------- .../storage/ManiphestTaskAuxiliaryStorage.php | 12 ----- .../view/ManiphestTransactionDetailView.php | 5 +++ .../patch/PhabricatorBuiltinPatchList.php | 4 ++ 8 files changed, 68 insertions(+), 39 deletions(-) create mode 100644 resources/sql/patches/20130915.maniphestmigrate.php delete mode 100644 src/applications/maniphest/storage/ManiphestTaskAuxiliaryStorage.php diff --git a/resources/sql/patches/20130915.maniphestmigrate.php b/resources/sql/patches/20130915.maniphestmigrate.php new file mode 100644 index 0000000000..a9013662e3 --- /dev/null +++ b/resources/sql/patches/20130915.maniphestmigrate.php @@ -0,0 +1,25 @@ +<?php + +$conn_w = id(new ManiphestTask())->establishConnection('w'); +$table_name = id(new ManiphestCustomFieldStorage())->getTableName(); + +$rows = new LiskRawMigrationIterator($conn_w, 'maniphest_taskauxiliarystorage'); + +echo "Migrating custom storage for Maniphest fields...\n"; +foreach ($rows as $row) { + $phid = $row['taskPHID']; + $name = $row['name']; + + echo "Migrating {$phid} / {$name}...\n"; + + queryfx( + $conn_w, + 'INSERT IGNORE INTO %T (objectPHID, fieldIndex, fieldValue) + VALUES (%s, %s, %s)', + $table_name, + $phid, + PhabricatorHash::digestForIndex('std:maniphest:'.$name), + $row['value']); +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 04466fadea..18454e3281 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -711,7 +711,6 @@ phutil_register_library_map(array( 'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php', 'ManiphestSubscribeController' => 'applications/maniphest/controller/ManiphestSubscribeController.php', 'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php', - 'ManiphestTaskAuxiliaryStorage' => 'applications/maniphest/storage/ManiphestTaskAuxiliaryStorage.php', 'ManiphestTaskDescriptionChangeController' => 'applications/maniphest/controller/ManiphestTaskDescriptionChangeController.php', 'ManiphestTaskDescriptionPreviewController' => 'applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php', 'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php', @@ -2783,7 +2782,6 @@ phutil_register_library_map(array( 4 => 'PhrequentTrackableInterface', 5 => 'PhabricatorCustomFieldInterface', ), - 'ManiphestTaskAuxiliaryStorage' => 'ManiphestDAO', 'ManiphestTaskDescriptionChangeController' => 'ManiphestController', 'ManiphestTaskDescriptionPreviewController' => 'ManiphestController', 'ManiphestTaskDetailController' => 'ManiphestController', diff --git a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php index 471b8f5ad1..f0df5fb888 100644 --- a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php +++ b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php @@ -42,7 +42,7 @@ abstract class ManiphestAuxiliaryFieldSpecification } public function getAuxiliaryKey() { - return $this->auxiliaryKey; + return 'std:maniphest:'.$this->auxiliaryKey; } public function setCaption($val) { @@ -235,6 +235,10 @@ abstract class ManiphestAuxiliaryFieldSpecification return true; } + public function shouldUseStorage() { + return true; + } + /* -( Legacy Migration Support )------------------------------------------- */ diff --git a/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php b/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php index dd7e9867a6..ed719f165f 100644 --- a/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php +++ b/src/applications/maniphest/conduit/ConduitAPI_maniphest_Method.php @@ -209,11 +209,6 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { $task_phids = mpull($tasks, 'getPHID'); - $all_aux = id(new ManiphestTaskAuxiliaryStorage())->loadAllWhere( - 'taskPHID in (%Ls)', - $task_phids); - $all_aux = mgroup($all_aux, 'getTaskPHID'); - $all_deps = id(new PhabricatorEdgeQuery()) ->withSourcePHIDs($task_phids) ->withEdgeTypes(array(PhabricatorEdgeConfig::TYPE_TASK_DEPENDS_ON_TASK)); @@ -221,8 +216,8 @@ abstract class ConduitAPI_maniphest_Method extends ConduitAPIMethod { $result = array(); foreach ($tasks as $task) { - $auxiliary = idx($all_aux, $task->getPHID(), array()); - $auxiliary = mpull($auxiliary, 'getValue', 'getName'); + // TODO: Batch this get as CustomField gets cleaned up. + $auxiliary = $task->loadLegacyAuxiliaryFieldMap(); $task_deps = $all_deps->getDestinationPHIDs( array($task->getPHID()), diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index 9bab1223d1..661f41aeac 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -137,17 +137,27 @@ final class ManiphestTask extends ManiphestDAO return $this; } + public function loadLegacyAuxiliaryFieldMap() { + $field_list = PhabricatorCustomField::getObjectFields( + $this, + PhabricatorCustomField::ROLE_EDIT); + $field_list->readFieldsFromStorage($this); + + $map = array(); + foreach ($field_list->getFields() as $field) { + $map[$field->getFieldKey()] = $field->getValueForStorage(); + } + + return $map; + } + public function loadAndAttachAuxiliaryAttributes() { if (!$this->getPHID()) { $this->auxiliaryAttributes = array(); return $this; } - $storage = id(new ManiphestTaskAuxiliaryStorage())->loadAllWhere( - 'taskPHID = %s', - $this->getPHID()); - - $this->auxiliaryAttributes = mpull($storage, 'getValue', 'getName'); + $this->auxiliaryAttributes = $this->loadLegacyAuxiliaryFieldMap(); return $this; } @@ -182,24 +192,26 @@ final class ManiphestTask extends ManiphestDAO } private function writeAuxiliaryUpdates() { - $table = new ManiphestTaskAuxiliaryStorage(); + $table = new ManiphestCustomFieldStorage(); $conn_w = $table->establishConnection('w'); $update = array(); $remove = array(); foreach ($this->auxiliaryDirty as $key => $dirty) { $value = $this->getAuxiliaryAttribute($key); + + $index = PhabricatorHash::digestForIndex($key); if ($value === null) { - $remove[$key] = true; + $remove[$index] = true; } else { - $update[$key] = $value; + $update[$index] = $value; } } if ($remove) { queryfx( $conn_w, - 'DELETE FROM %T WHERE taskPHID = %s AND name IN (%Ls)', + 'DELETE FROM %T WHERE objectPHID = %s AND fieldIndex IN (%Ls)', $table->getTableName(), $this->getPHID(), array_keys($remove)); @@ -207,21 +219,19 @@ final class ManiphestTask extends ManiphestDAO if ($update) { $sql = array(); - foreach ($update as $key => $val) { + foreach ($update as $index => $val) { $sql[] = qsprintf( $conn_w, - '(%s, %s, %s, %d, %d)', + '(%s, %s, %s)', $this->getPHID(), - $key, - $val, - time(), - time()); + $index, + $val); } queryfx( $conn_w, - 'INSERT INTO %T (taskPHID, name, value, dateCreated, dateModified) + 'INSERT INTO %T (objectPHID, fieldIndex, fieldValue) VALUES %Q ON DUPLICATE KEY - UPDATE value = VALUES(value), dateModified = VALUES(dateModified)', + UPDATE fieldValue = VALUES(fieldValue)', $table->getTableName(), implode(', ', $sql)); } diff --git a/src/applications/maniphest/storage/ManiphestTaskAuxiliaryStorage.php b/src/applications/maniphest/storage/ManiphestTaskAuxiliaryStorage.php deleted file mode 100644 index 83b32f1c2b..0000000000 --- a/src/applications/maniphest/storage/ManiphestTaskAuxiliaryStorage.php +++ /dev/null @@ -1,12 +0,0 @@ -<?php - -/** - * @group maniphest - */ -final class ManiphestTaskAuxiliaryStorage extends ManiphestDAO { - - protected $taskPHID; - protected $name; - protected $value; - -} diff --git a/src/applications/maniphest/view/ManiphestTransactionDetailView.php b/src/applications/maniphest/view/ManiphestTransactionDetailView.php index 7ecd644a1e..3b3c81b482 100644 --- a/src/applications/maniphest/view/ManiphestTransactionDetailView.php +++ b/src/applications/maniphest/view/ManiphestTransactionDetailView.php @@ -518,7 +518,12 @@ final class ManiphestTransactionDetailView extends ManiphestView { break; case ManiphestTransactionType::TYPE_AUXILIARY: $aux_key = $transaction->getMetadataValue('aux:key'); + + // TODO: Migrate all legacy data when everything migrates for T2217. $aux_field = $this->getAuxiliaryField($aux_key); + if (!$aux_field) { + $aux_field = $this->getAuxiliaryField('std:maniphest:'.$aux_key); + } $verb = null; if ($aux_field) { diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 158128e4f6..906f5735c2 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1600,6 +1600,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130915.maniphestcustom.sql'), ), + '20130915.maniphestmigrate.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130915.maniphestmigrate.php'), + ), ); } }