From e5639a8ed90fad70cc3e61cf01c2c9cfc185ac66 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 27 Jan 2018 05:54:21 -0800 Subject: [PATCH] Write edge transactions in a more compact way Summary: Depends on D18946. Ref T13051. Begins writing edge transactions as just a list of changed PHIDs. Test Plan: Added, edited, and removed projects. Reviewed transaction record and database. Saw no user-facing changes but a far more compact database representation. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13051 Differential Revision: https://secure.phabricator.com/D18947 --- ...habricatorApplicationTransactionEditor.php | 26 +++++++++++++++++- .../util/PhabricatorEdgeChangeRecord.php | 27 +++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 7dbd41f1e4..155592fc4e 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -999,7 +999,31 @@ abstract class PhabricatorApplicationTransactionEditor $xaction->setPHID($xaction->generatePHID()); $comment_editor->applyEdit($xaction, $xaction->getComment()); } else { - $xaction->save(); + + // TODO: This is a transitional hack to let us migrate edge + // transactions to a more efficient storage format. For now, we're + // going to write a new slim format to the database but keep the old + // bulky format on the objects so we don't have to upgrade all the + // edit logic to the new format yet. See T13051. + + $edge_type = PhabricatorTransactions::TYPE_EDGE; + if ($xaction->getTransactionType() == $edge_type) { + $bulky_old = $xaction->getOldValue(); + $bulky_new = $xaction->getNewValue(); + + $record = PhabricatorEdgeChangeRecord::newFromTransaction($xaction); + $slim_old = $record->getModernOldEdgeTransactionData(); + $slim_new = $record->getModernNewEdgeTransactionData(); + + $xaction->setOldValue($slim_old); + $xaction->setNewValue($slim_new); + $xaction->save(); + + $xaction->setOldValue($bulky_old); + $xaction->setNewValue($bulky_new); + } else { + $xaction->save(); + } } } diff --git a/src/infrastructure/edges/util/PhabricatorEdgeChangeRecord.php b/src/infrastructure/edges/util/PhabricatorEdgeChangeRecord.php index 6e0cb1d3e0..38557dc09f 100644 --- a/src/infrastructure/edges/util/PhabricatorEdgeChangeRecord.php +++ b/src/infrastructure/edges/util/PhabricatorEdgeChangeRecord.php @@ -44,10 +44,18 @@ final class PhabricatorEdgeChangeRecord return array_keys($rem); } + public function getModernOldEdgeTransactionData() { + return $this->getRemovedPHIDs(); + } + + public function getModernNewEdgeTransactionData() { + return $this->getAddedPHIDs(); + } + private function getOldDestinationPHIDs() { if ($this->xaction) { $old = $this->xaction->getOldValue(); - return ipull($old, 'dst'); + return $this->getPHIDsFromTransactionValue($old); } throw new Exception( @@ -57,12 +65,27 @@ final class PhabricatorEdgeChangeRecord private function getNewDestinationPHIDs() { if ($this->xaction) { $new = $this->xaction->getNewValue(); - return ipull($new, 'dst'); + return $this->getPHIDsFromTransactionValue($new); } throw new Exception( pht('Edge change record is not configured with any change data.')); } + private function getPHIDsFromTransactionValue($value) { + if (!$value) { + return array(); + } + + // If the list items are arrays, this is an older-style map of + // dictionaries. + $head = head($value); + if (is_array($head)) { + return ipull($value, 'dst'); + } + + // If the list items are not arrays, this is a newer-style list of PHIDs. + return $value; + } }