diff --git a/resources/sql/patches/20130926.dinline.php b/resources/sql/patches/20130926.dinline.php new file mode 100644 index 0000000000..75d67efdbd --- /dev/null +++ b/resources/sql/patches/20130926.dinline.php @@ -0,0 +1,90 @@ +establishConnection('w'); +$conn_w->openTransaction(); + +$src_table = 'differential_inlinecomment'; +$dst_table = 'differential_transaction_comment'; + +echo "Migrating Differential inline comments to new format...\n"; + +$content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_LEGACY, + array())->serialize(); + +$rows = new LiskRawMigrationIterator($conn_w, $src_table); +foreach ($rows as $row) { + $id = $row['id']; + + $revision_id = $row['revisionID']; + + echo "Migrating inline #{$id} (D{$revision_id})...\n"; + + $revision_row = queryfx_one( + $conn_w, + 'SELECT phid FROM %T WHERE id = %d', + $revision_table->getTableName(), + $revision_id); + if (!$revision_row) { + continue; + } + + $revision_phid = $revision_row['phid']; + + if ($row['commentID']) { + $xaction_phid = PhabricatorPHID::generateNewPHID( + PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST, + DifferentialPHIDTypeRevision::TYPECONST); + } else { + $xaction_phid = null; + } + + $comment_phid = PhabricatorPHID::generateNewPHID( + PhabricatorPHIDConstants::PHID_TYPE_XCMT, + DifferentialPHIDTypeRevision::TYPECONST); + + queryfx( + $conn_w, + 'INSERT IGNORE INTO %T + (id, phid, transactionPHID, authorPHID, viewPolicy, editPolicy, + commentVersion, content, contentSource, isDeleted, + dateCreated, dateModified, revisionPHID, changesetID, + isNewFile, lineNumber, lineLength, hasReplies, legacyCommentID) + VALUES (%d, %s, %ns, %s, %s, %s, + %d, %s, %s, %d, + %d, %d, %s, %nd, + %d, %d, %d, %d, %nd)', + $dst_table, + + // id, phid, transactionPHID, authorPHID, viewPolicy, editPolicy + $row['id'], + $comment_phid, + $xaction_phid, + $row['authorPHID'], + 'public', + $row['authorPHID'], + + // commentVersion, content, contentSource, isDeleted + 1, + $row['content'], + $content_source, + 0, + + // dateCreated, dateModified, revisionPHID, changesetID + $row['dateCreated'], + $row['dateModified'], + $revision_phid, + $row['changesetID'], + + // isNewFile, lineNumber, lineLength, hasReplies, legacyCommentID + $row['isNewFile'], + $row['lineNumber'], + $row['lineLength'], + 0, + $row['commentID']); + +} + +$conn_w->saveTransaction(); +echo "Done.\n"; diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php index 62c5d6c781..c09caa9374 100644 --- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php @@ -30,7 +30,7 @@ final class DifferentialInlineCommentEditController } return id(new DifferentialInlineComment()) - ->setRevisionID($revision_id) + ->setRevision($revision) ->setChangesetID($changeset_id); } diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php index 215a4d61b6..8eb59760c1 100644 --- a/src/applications/differential/query/DifferentialInlineCommentQuery.php +++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php @@ -51,7 +51,7 @@ final class DifferentialInlineCommentQuery } public function execute() { - $table = new DifferentialInlineComment(); + $table = new DifferentialTransactionComment(); $conn_r = $table->establishConnection('r'); $data = queryfx_all( @@ -61,7 +61,14 @@ final class DifferentialInlineCommentQuery $this->buildWhereClause($conn_r), $this->buildLimitClause($conn_r)); - return $table->loadAllFromArray($data); + $comments = $table->loadAllFromArray($data); + + foreach ($comments as $key => $value) { + $comments[$key] = DifferentialInlineComment::newFromModernComment( + $value); + } + + return $comments; } public function executeOne() { @@ -71,17 +78,35 @@ final class DifferentialInlineCommentQuery private function buildWhereClause(AphrontDatabaseConnection $conn_r) { $where = array(); + // Only find inline comments. + $where[] = qsprintf( + $conn_r, + 'changesetID IS NOT NULL'); + if ($this->revisionIDs) { - $where[] = qsprintf( + + // Look up revision PHIDs. + $revision_phids = queryfx_all( $conn_r, - 'revisionID IN (%Ld)', + 'SELECT phid FROM %T WHERE id IN (%Ld)', + id(new DifferentialRevision())->getTableName(), $this->revisionIDs); + + if (!$revision_phids) { + throw new PhabricatorEmptyQueryException(); + } + $revision_phids = ipull($revision_phids, 'phid'); + + $where[] = qsprintf( + $conn_r, + 'revisionPHID IN (%Ls)', + $revision_phids); } if ($this->notDraft) { $where[] = qsprintf( $conn_r, - 'commentID IS NOT NULL'); + 'transactionPHID IS NOT NULL'); } if ($this->ids) { @@ -95,31 +120,45 @@ final class DifferentialInlineCommentQuery list($phid, $ids) = $this->viewerAndChangesetIDs; $where[] = qsprintf( $conn_r, - 'changesetID IN (%Ld) AND (authorPHID = %s OR commentID IS NOT NULL)', + 'changesetID IN (%Ld) AND + (authorPHID = %s OR transactionPHID IS NOT NULL)', $ids, $phid); } if ($this->draftComments) { list($phid, $rev_id) = $this->draftComments; + + $rev_phid = queryfx_one( + $conn_r, + 'SELECT phid FROM %T WHERE id = %d', + id(new DifferentialRevision())->getTableName(), + $rev_id); + + if (!$rev_phid) { + throw new PhabricatorEmptyQueryException(); + } + + $rev_phid = $rev_phid['phid']; + $where[] = qsprintf( $conn_r, - 'authorPHID = %s AND revisionID = %d AND commentID IS NULL', + 'authorPHID = %s AND revisionPHID = %s AND transactionPHID IS NULL', $phid, - $rev_id); + $rev_phid); } if ($this->draftsByAuthors) { $where[] = qsprintf( $conn_r, - 'authorPHID IN (%Ls) AND commentID IS NULL', + 'authorPHID IN (%Ls) AND transactionPHID IS NULL', $this->draftsByAuthors); } if ($this->commentIDs) { $where[] = qsprintf( $conn_r, - 'commentID IN (%Ld)', + 'legacyCommentID IN (%Ld)', $this->commentIDs); } diff --git a/src/applications/differential/query/DifferentialRevisionQuery.php b/src/applications/differential/query/DifferentialRevisionQuery.php index 90284fefb9..556ec6a83c 100644 --- a/src/applications/differential/query/DifferentialRevisionQuery.php +++ b/src/applications/differential/query/DifferentialRevisionQuery.php @@ -491,6 +491,11 @@ final class DifferentialRevisionQuery $this->draftRevisions[] = substr($draft->getDraftKey(), $len); } + // TODO: Restore this after drafts are sorted out. It's now very + // expensive to get revision IDs. + + /* + $inlines = id(new DifferentialInlineCommentQuery()) ->withDraftsByAuthors($this->draftAuthors) ->execute(); @@ -498,6 +503,8 @@ final class DifferentialRevisionQuery $this->draftRevisions[] = $inline->getRevisionID(); } + */ + if (!$this->draftRevisions) { return array(); } diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php index 469bd3ea5d..af30563776 100644 --- a/src/applications/differential/storage/DifferentialInlineComment.php +++ b/src/applications/differential/storage/DifferentialInlineComment.php @@ -1,21 +1,52 @@ proxy = new DifferentialTransactionComment(); + } - private $syntheticAuthor; + public function __clone() { + $this->proxy = clone $this->proxy; + } + + public function save() { + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_LEGACY, + array()); + + $this->proxy + ->setViewPolicy('public') + ->setEditPolicy($this->getAuthorPHID()) + ->setContentSource($content_source) + ->setCommentVersion(1) + ->save(); + + return $this; + } + + public function delete() { + $this->proxy->delete(); + + return $this; + } + + public function getID() { + return $this->proxy->getID(); + } + + public static function newFromModernComment( + DifferentialTransactionComment $comment) { + + $obj = new DifferentialInlineComment(); + $obj->proxy = $comment; + + return $obj; + } public function setSyntheticAuthor($synthetic_author) { $this->syntheticAuthor = $synthetic_author; @@ -34,75 +65,112 @@ final class DifferentialInlineComment } public function setContent($content) { - $this->setCache(null); - $this->writeField('content', $content); + $this->proxy->setContent($content); return $this; } public function getContent() { - return $this->readField('content'); + return $this->proxy->getContent(); } public function isDraft() { return !$this->getCommentID(); } - // NOTE: We need to provide implementations so we conform to the shared - // interface; these are all trivial and just explicit versions of the Lisk - // defaults. - public function setChangesetID($id) { - $this->writeField('changesetID', $id); + $this->proxy->setChangesetID($id); return $this; } public function getChangesetID() { - return $this->readField('changesetID'); + return $this->proxy->getChangesetID(); } public function setIsNewFile($is_new) { - $this->writeField('isNewFile', $is_new); + $this->proxy->setIsNewFile($is_new); return $this; } public function getIsNewFile() { - return $this->readField('isNewFile'); + return $this->proxy->getIsNewFile(); } public function setLineNumber($number) { - $this->writeField('lineNumber', $number); + $this->proxy->setLineNumber($number); return $this; } public function getLineNumber() { - return $this->readField('lineNumber'); + return $this->proxy->getLineNumber(); } public function setLineLength($length) { - $this->writeField('lineLength', $length); + $this->proxy->setLineLength($length); return $this; } public function getLineLength() { - return $this->readField('lineLength'); + return $this->proxy->getLineLength(); } public function setCache($cache) { - $this->writeField('cache', $cache); return $this; } public function getCache() { - return $this->readField('cache'); + return null; } public function setAuthorPHID($phid) { - $this->writeField('authorPHID', $phid); + $this->proxy->setAuthorPHID($phid); return $this; } public function getAuthorPHID() { - return $this->readField('authorPHID'); + return $this->proxy->getAuthorPHID(); + } + + public function setRevision(DifferentialRevision $revision) { + $this->proxy->setRevisionPHID($revision->getPHID()); + return $this; + } + + // Although these are purely transitional, they're also *extra* dumb. + + public function setRevisionID($revision_id) { + $revision = id(new DifferentialRevision())->load($revision_id); + return $this->setRevision($revision); + } + + public function getRevisionID() { + $phid = $this->proxy->getRevisionPHID(); + if (!$phid) { + return null; + } + + $revision = id(new DifferentialRevision())->loadOneWhere( + 'phid = %s', + $phid); + if (!$revision) { + return null; + } + return $revision->getID(); + } + + // When setting a comment ID, we also generate a phantom transaction PHID for + // the future transaction. + + public function setCommentID($id) { + $this->proxy->setLegacyCommentID($id); + $this->proxy->setTransactionPHID( + PhabricatorPHID::generateNewPHID( + PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST, + DifferentialPHIDTypeRevision::TYPECONST)); + return $this; + } + + public function getCommentID() { + return $this->proxy->getLegacyCommentID(); } diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php index c46d0c4b58..d527c44100 100644 --- a/src/applications/differential/storage/DifferentialTransactionComment.php +++ b/src/applications/differential/storage/DifferentialTransactionComment.php @@ -5,9 +5,9 @@ final class DifferentialTransactionComment protected $revisionPHID; protected $changesetID; - protected $isNewFile; - protected $lineNumber; - protected $lineLength; + protected $isNewFile = 0; + protected $lineNumber = 0; + protected $lineLength = 0; protected $fixedState; protected $hasReplies = 0; protected $replyToCommentPHID; diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 638d1cf283..fab4e35706 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1680,6 +1680,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130915.maniphestqdrop.sql'), ), + '20130926.dinline.php' => array( + 'type' => 'php', + 'name' => $this->getPatchPath('20130926.dinline.php'), + ), ); } }