diff --git a/resources/sql/autopatches/20140725.audit.1.migxactions.php b/resources/sql/autopatches/20140725.audit.1.migxactions.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140725.audit.1.migxactions.php @@ -0,0 +1,150 @@ +establishConnection('w'); +$rows = new LiskRawMigrationIterator($conn_w, 'audit_comment'); + +$content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_LEGACY, + array())->serialize(); + +echo "Migrating Audit comments to modern storage...\n"; +foreach ($rows as $row) { + $id = $row['id']; + echo "Migrating comment {$id}...\n"; + + $comments = queryfx_all( + $conn_w, + 'SELECT * FROM %T WHERE legacyCommentID = %d', + 'audit_transaction_comment', + $id); + + $main_comments = array(); + $inline_comments = array(); + + foreach ($comments as $comment) { + if ($comment['pathID']) { + $inline_comments[] = $comment; + } else { + $main_comments[] = $comment; + } + } + + $metadata = json_decode($row['metadata'], true); + if (!is_array($metadata)) { + $metadata = array(); + } + + $xactions = array(); + + // Build the main action transaction. + switch ($row['action']) { + case PhabricatorAuditActionConstants::ADD_AUDITORS: + $phids = idx($metadata, 'added-auditors', array()); + $xactions[] = array( + 'type' => $row['action'], + 'old' => null, + 'new' => array_fuse($phids), + ); + break; + case PhabricatorAuditActionConstants::ADD_CCS: + $phids = idx($metadata, 'added-ccs', array()); + $xactions[] = array( + 'type' => $row['action'], + 'old' => null, + 'new' => array_fuse($phids), + ); + break; + case PhabricatorAuditActionConstants::COMMENT: + case PhabricatorAuditActionConstants::INLINE: + // These actions will have their transactions created by other rules. + break; + default: + // Otherwise, this is an accept/concern/etc action. + $xactions[] = array( + 'type' => PhabricatorAuditActionConstants::ACTION, + 'old' => null, + 'new' => $row['action'], + ); + break; + } + + + // Build the main comment transaction. + foreach ($main_comments as $main) { + $xactions[] = array( + 'type' => PhabricatorTransactions::TYPE_COMMENT, + 'old' => null, + 'new' => null, + 'phid' => $main['transactionPHID'], + 'comment' => $main, + ); + } + + // Build inline comment transactions. + foreach ($inline_comments as $inline) { + $xactions[] = array( + 'type' => PhabricatorAuditActionConstants::INLINE, + 'old' => null, + 'new' => null, + 'phid' => $inline['transactionPHID'], + 'comment' => $inline, + ); + } + + foreach ($xactions as $xaction) { + // Generate a new PHID, if we don't already have one from the comment + // table. We pregenerated into the comment table to make this a little + // easier, so we only need to write to one table. + $xaction_phid = idx($xaction, 'phid'); + if (!$xaction_phid) { + $xaction_phid = PhabricatorPHID::generateNewPHID( + PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST, + PhabricatorRepositoryCommitPHIDType::TYPECONST); + } + unset($xaction['phid']); + + $comment_phid = null; + $comment_version = 0; + if (idx($xaction, 'comment')) { + $comment_phid = $xaction['comment']['phid']; + $comment_version = 1; + } + + $old = idx($xaction, 'old'); + $new = idx($xaction, 'new'); + $meta = idx($xaction, 'meta', array()); + + queryfx( + $conn_w, + 'INSERT INTO %T (phid, authorPHID, objectPHID, viewPolicy, editPolicy, + commentPHID, commentVersion, transactionType, oldValue, newValue, + contentSource, metadata, dateCreated, dateModified) + VALUES (%s, %s, %s, %s, %s, %ns, %d, %s, %ns, %ns, %s, %s, %d, %d)', + 'audit_transaction', + + // PHID, authorPHID, objectPHID + $xaction_phid, + $row['actorPHID'], + $row['targetPHID'], + + // viewPolicy, editPolicy, commentPHID, commentVersion + 'public', + $row['actorPHID'], + $comment_phid, + $comment_version, + + // transactionType, oldValue, newValue, contentSource, metadata + $xaction['type'], + json_encode($old), + json_encode($new), + $content_source, + json_encode($meta), + + // dates + $row['dateCreated'], + $row['dateModified']); + } + +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3929,10 +3929,7 @@ 'PhabricatorAsanaConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorAuditAddCommentController' => 'PhabricatorAuditController', 'PhabricatorAuditApplication' => 'PhabricatorApplication', - 'PhabricatorAuditComment' => array( - 'PhabricatorAuditDAO', - 'PhabricatorMarkupInterface', - ), + 'PhabricatorAuditComment' => 'PhabricatorMarkupInterface', 'PhabricatorAuditCommentEditor' => 'PhabricatorEditor', 'PhabricatorAuditController' => 'PhabricatorController', 'PhabricatorAuditDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/audit/constants/PhabricatorAuditActionConstants.php b/src/applications/audit/constants/PhabricatorAuditActionConstants.php --- a/src/applications/audit/constants/PhabricatorAuditActionConstants.php +++ b/src/applications/audit/constants/PhabricatorAuditActionConstants.php @@ -10,6 +10,7 @@ const ADD_CCS = 'add_ccs'; const ADD_AUDITORS = 'add_auditors'; const INLINE = 'audit:inline'; + const ACTION = 'audit:action'; public static function getActionNameMap() { $map = array( diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -262,23 +262,27 @@ $commit->updateAuditStatus($requests); $commit->save(); + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_LEGACY, + array()); + foreach ($comments as $comment) { $comment ->setActorPHID($actor->getPHID()) ->setTargetPHID($commit->getPHID()) + ->setContentSource($content_source) ->save(); } foreach ($inline_comments as $inline) { $xaction = id(new PhabricatorAuditComment()) + ->setProxyComment($inline->getTransactionCommentForSave()) ->setAction(PhabricatorAuditActionConstants::INLINE) ->setActorPHID($actor->getPHID()) ->setTargetPHID($commit->getPHID()) + ->setContentSource($content_source) ->save(); - $inline->setAuditCommentID($xaction->getID()); - $inline->save(); - $comments[] = $xaction; } diff --git a/src/applications/audit/storage/PhabricatorAuditComment.php b/src/applications/audit/storage/PhabricatorAuditComment.php --- a/src/applications/audit/storage/PhabricatorAuditComment.php +++ b/src/applications/audit/storage/PhabricatorAuditComment.php @@ -1,6 +1,6 @@ proxy = new PhabricatorAuditTransaction(); + } + + public function __clone() { + $this->proxy = clone $this->proxy; + if ($this->proxyComment) { + $this->proxyComment = clone $this->proxyComment; + } + } + + public static function newFromModernTransaction( + PhabricatorAuditTransaction $xaction) { + + $obj = new PhabricatorAuditComment(); + $obj->proxy = $xaction; + + if ($xaction->hasComment()) { + $obj->proxyComment = $xaction->getComment(); + } + + return $obj; + } public static function loadComments( PhabricatorUser $viewer, $commit_phid) { - $comments = id(new PhabricatorAuditComment())->loadAllWhere( - 'targetPHID = %s', - $commit_phid); - - if ($comments) { - $table = new PhabricatorAuditTransactionComment(); - $conn_r = $table->establishConnection('r'); - - $data = queryfx_all( - $conn_r, - 'SELECT * FROM %T WHERE legacyCommentID IN (%Ld) AND pathID IS NULL', - $table->getTableName(), - mpull($comments, 'getID')); - $texts = $table->loadAllFromArray($data); - $texts = mpull($texts, null, 'getLegacyCommentID'); - - foreach ($comments as $comment) { - $text = idx($texts, $comment->getID()); - if ($text) { - $comment->setProxyComment($text); - } - } + $xactions = id(new PhabricatorAuditTransactionQuery()) + ->setViewer($viewer) + ->withObjectPHIDs(array($commit_phid)) + ->needComments(true) + ->execute(); + + $comments = array(); + foreach ($xactions as $xaction) { + $comments[] = self::newFromModernTransaction($xaction); } return $comments; } - public function getConfiguration() { - return array( - self::CONFIG_SERIALIZATION => array( - 'metadata' => self::SERIALIZATION_JSON, - ), - self::CONFIG_AUX_PHID => true, - ) + parent::getConfiguration(); + public function getPHID() { + return $this->proxy->getPHID(); } - public function generatePHID() { - return PhabricatorPHID::generateNewPHID('ACMT'); + public function getActorPHID() { + return $this->proxy->getAuthorPHID(); } + public function setActorPHID($actor_phid) { + $this->proxy->setAuthorPHID($actor_phid); + return $this; + } + + public function setTargetPHID($target_phid) { + $this->getProxyComment()->setCommitPHID($target_phid); + $this->proxy->setObjectPHID($target_phid); + return $this; + } + + public function getTargetPHID() { + return $this->proxy->getObjectPHID(); + } public function getContent() { return $this->getProxyComment()->getContent(); } public function setContent($content) { - // NOTE: We no longer read this field, but there's no cost to continuing - // to write it in case something goes horribly wrong, since it makes it - // far easier to back out of this. - $this->content = $content; $this->getProxyComment()->setContent($content); return $this; } + public function setContentSource($content_source) { + $this->proxy->setContentSource($content_source); + $this->proxyComment->setContentSource($content_source); + return $this; + } + + public function getContentSource() { + return $this->proxy->getContentSource(); + } + private function getProxyComment() { if (!$this->proxyComment) { $this->proxyComment = new PhabricatorAuditTransactionComment(); @@ -90,39 +110,116 @@ return $this; } - public function setTargetPHID($target_phid) { - $this->getProxyComment()->setCommitPHID($target_phid); - return parent::setTargetPHID($target_phid); + public function setAction($action) { + switch ($action) { + case PhabricatorAuditActionConstants::INLINE: + case PhabricatorAuditActionConstants::ADD_CCS: + case PhabricatorAuditActionConstants::ADD_AUDITORS: + $this->proxy->setTransactionType($action); + break; + case PhabricatorAuditActionConstants::COMMENT: + $this->proxy->setTransactionType(PhabricatorTransactions::TYPE_COMMENT); + break; + default: + $this->proxy + ->setTransactionType(PhabricatorAuditActionConstants::ACTION) + ->setNewValue($action); + break; + } + + return $this; } - public function save() { - $this->openTransaction(); - $result = parent::save(); + public function getAction() { + $type = $this->proxy->getTransactionType(); + switch ($type) { + case PhabricatorTransactions::TYPE_COMMENT: + return PhabricatorAuditActionConstants::COMMENT; + case PhabricatorAuditActionConstants::INLINE: + case PhabricatorAuditActionConstants::ADD_CCS: + case PhabricatorAuditActionConstants::ADD_AUDITORS: + return $type; + default: + return $this->proxy->getNewValue(); + } + } - if (strlen($this->getContent())) { - $content_source = PhabricatorContentSource::newForSource( - PhabricatorContentSource::SOURCE_LEGACY, - array()); + public function setMetadata(array $metadata) { + if (!$this->proxy->getTransactionType()) { + throw new Exception(pht('Call setAction() before getMetadata()!')); + } + + $type = $this->proxy->getTransactionType(); + switch ($type) { + case PhabricatorAuditActionConstants::ADD_CCS: + $raw_phids = idx($metadata, self::METADATA_ADDED_CCS, array()); + break; + case PhabricatorAuditActionConstants::ADD_AUDITORS: + $raw_phids = idx($metadata, self::METADATA_ADDED_AUDITORS, array()); + break; + default: + throw new Exception(pht('No metadata expected!')); + } + + $this->proxy->setOldValue(array()); + $this->proxy->setNewValue(array_fuse($raw_phids)); + + return $this; + } - $xaction_phid = PhabricatorPHID::generateNewPHID( - PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST, - PhabricatorRepositoryCommitPHIDType::TYPECONST); + public function getMetadata() { + if (!$this->proxy->getTransactionType()) { + throw new Exception(pht('Call setAction() before getMetadata()!')); + } + + $type = $this->proxy->getTransactionType(); + $new_value = $this->proxy->getNewValue(); + switch ($type) { + case PhabricatorAuditActionConstants::ADD_CCS: + return array( + self::METADATA_ADDED_CCS => array_keys($new_value), + ); + case PhabricatorAuditActionConstants::ADD_AUDITORS: + return array( + self::METADATA_ADDED_AUDITORS => array_keys($new_value), + ); + } + + return array(); + } + + public function save() { + $this->proxy->openTransaction(); + $this->proxy + ->setViewPolicy('public') + ->setEditPolicy($this->getActorPHID()) + ->save(); - $proxy = $this->getProxyComment(); - $proxy + if (strlen($this->getContent())) { + $this->getProxyComment() ->setAuthorPHID($this->getActorPHID()) ->setViewPolicy('public') ->setEditPolicy($this->getActorPHID()) - ->setContentSource($content_source) ->setCommentVersion(1) - ->setLegacyCommentID($this->getID()) - ->setTransactionPHID($xaction_phid) + ->setTransactionPHID($this->proxy->getPHID()) + ->save(); + + $this->proxy + ->setCommentVersion(1) + ->setCommentPHID($this->getProxyComment()->getPHID()) ->save(); } + $this->proxy->saveTransaction(); - $this->saveTransaction(); + return $this; + } + + public function getDateCreated() { + return $this->proxy->getDateCreated(); + } - return $result; + public function getDateModified() { + return $this->proxy->getDateModified(); } @@ -130,7 +227,7 @@ public function getMarkupFieldKey($field) { - return 'AC:'.$this->getID(); + return 'AC:'.$this->getPHID(); } public function newMarkupEngine($field) { @@ -146,7 +243,7 @@ } public function shouldUseMarkupCache($field) { - return (bool)$this->getID(); + return (bool)$this->getPHID(); } } diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -14,6 +14,10 @@ $this->proxy = clone $this->proxy; } + public function getTransactionPHID() { + return $this->proxy->getTransactionPHID(); + } + public function getTransactionCommentForSave() { $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_LEGACY, diff --git a/src/applications/diffusion/view/DiffusionCommentListView.php b/src/applications/diffusion/view/DiffusionCommentListView.php --- a/src/applications/diffusion/view/DiffusionCommentListView.php +++ b/src/applications/diffusion/view/DiffusionCommentListView.php @@ -65,14 +65,14 @@ public function render() { - $inline_comments = mgroup($this->inlineComments, 'getAuditCommentID'); + $inline_comments = mgroup($this->inlineComments, 'getTransactionPHID'); $num = 1; $comments = array(); foreach ($this->comments as $comment) { - $inlines = idx($inline_comments, $comment->getID(), array()); + $inlines = idx($inline_comments, $comment->getPHID(), array()); $view = id(new DiffusionCommentView()) ->setMarkupEngine($this->getMarkupEngine())