Skip to content

Commit 305fb3f

Browse files
author
epriestley
committedFeb 11, 2014
Migrate all Differential comment text into new storage
Summary: Ref T2222. Currently, `DifferentialComment` stores both (a) the text of comments and (b) various other transaction details. This data needs to map to both Transactions and TransactionComments in the long run. This diff separates out all the data which is bound for the TransactionComment table, so that when we migrate `DifferentialComment` itself it will //only// need to migrate into the Transactions table. This is a much simpler migration than the inline comment one was, partly because it set up infrastructure and partly because the data is less complex. Basically, I'm just proxying the read/write for the comment text into the other table. All readers already go through the Query class, and there are only three writers (preview, comment, implicit comment on diff update) which are all highly regular and straightforward to test. We can also back out of this diff very easily: doing double writes cost only one line of code (`$this->content = $content;`) so we have proper double writes and a trivial revert path. Test Plan: - Without migrating, added comments and saw them show up. - Migrated. - Saw all the old comments, and no damage to the new ones. - Added new comments. - Used comment preview. - Updated a revision to implicitly create an update comment and verified it looked OK. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2222 Differential Revision: https://secure.phabricator.com/D8196
1 parent 3a01d6d commit 305fb3f

File tree

4 files changed

+161
-2
lines changed

4 files changed

+161
-2
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE {$NAMESPACE}_differential.differential_transaction_comment
2+
CHANGE changesetID changesetID INT UNSIGNED;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
$conn_w = id(new DifferentialRevision())->establishConnection('w');
4+
$rows = new LiskRawMigrationIterator($conn_w, 'differential_comment');
5+
6+
$content_source = PhabricatorContentSource::newForSource(
7+
PhabricatorContentSource::SOURCE_LEGACY,
8+
array())->serialize();
9+
10+
echo "Migrating Differential comment text to modern storage...\n";
11+
foreach ($rows as $row) {
12+
$id = $row['id'];
13+
echo "Migrating Differential comment {$id}...\n";
14+
if (!strlen($row['content'])) {
15+
echo "Comment has no text, continuing.\n";
16+
continue;
17+
}
18+
19+
$revision = id(new DifferentialRevision())->load($row['revisionID']);
20+
if (!$revision) {
21+
echo "Comment has no valid revision, continuing.\n";
22+
continue;
23+
}
24+
25+
$revision_phid = $revision->getPHID();
26+
27+
$dst_table = 'differential_inline_comment';
28+
29+
$xaction_phid = PhabricatorPHID::generateNewPHID(
30+
PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
31+
DifferentialPHIDTypeRevision::TYPECONST);
32+
33+
$comment_phid = PhabricatorPHID::generateNewPHID(
34+
PhabricatorPHIDConstants::PHID_TYPE_XCMT,
35+
DifferentialPHIDTypeRevision::TYPECONST);
36+
37+
queryfx(
38+
$conn_w,
39+
'INSERT IGNORE INTO %T
40+
(phid, transactionPHID, authorPHID, viewPolicy, editPolicy,
41+
commentVersion, content, contentSource, isDeleted,
42+
dateCreated, dateModified, revisionPHID, changesetID,
43+
legacyCommentID)
44+
VALUES (%s, %s, %s, %s, %s,
45+
%d, %s, %s, %d,
46+
%d, %d, %s, %nd,
47+
%d)',
48+
'differential_transaction_comment',
49+
50+
// phid, transactionPHID, authorPHID, viewPolicy, editPolicy
51+
$comment_phid,
52+
$xaction_phid,
53+
$row['authorPHID'],
54+
'public',
55+
$row['authorPHID'],
56+
57+
// commentVersion, content, contentSource, isDeleted
58+
1,
59+
$row['content'],
60+
$content_source,
61+
0,
62+
63+
// dateCreated, dateModified, revisionPHID, changesetID, legacyCommentID
64+
$row['dateCreated'],
65+
$row['dateModified'],
66+
$revision_phid,
67+
null,
68+
$row['id']);
69+
}
70+
71+
echo "Done.\n";

‎src/applications/differential/query/DifferentialCommentQuery.php

+30-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,15 @@ public function execute() {
2424
$this->buildWhereClause($conn_r),
2525
$this->buildLimitClause($conn_r));
2626

27-
return $table->loadAllFromArray($data);
27+
$comments = $table->loadAllFromArray($data);
28+
29+
// We've moved the actual text storage into DifferentialTransactionComment,
30+
// so load the relevant pieces of text we need.
31+
if ($comments) {
32+
$this->loadCommentText($comments);
33+
}
34+
35+
return $comments;
2836
}
2937

3038
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
@@ -40,4 +48,25 @@ private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
4048
return $this->formatWhereClause($where);
4149
}
4250

51+
private function loadCommentText(array $comments) {
52+
$table = new DifferentialTransactionComment();
53+
$conn_r = $table->establishConnection('r');
54+
55+
$data = queryfx_all(
56+
$conn_r,
57+
'SELECT * FROM %T WHERE legacyCommentID IN (%Ld) AND changesetID IS NULL',
58+
$table->getTableName(),
59+
mpull($comments, 'getID'));
60+
$texts = $table->loadAllFromArray($data);
61+
$texts = mpull($texts, null, 'getLegacyCommentID');
62+
63+
foreach ($comments as $comment) {
64+
$text = idx($texts, $comment->getID());
65+
if ($text) {
66+
$comment->setProxyComment($text);
67+
}
68+
}
69+
}
70+
71+
4372
}

‎src/applications/differential/storage/DifferentialComment.php

+58-1
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,44 @@ final class DifferentialComment extends DifferentialDAO
1313
protected $authorPHID;
1414
protected $revisionID;
1515
protected $action;
16-
protected $content;
16+
protected $content = '';
1717
protected $cache;
1818
protected $metadata = array();
1919
protected $contentSource;
2020

2121
private $arbitraryDiffForFacebook;
22+
private $proxyComment;
23+
24+
public function getContent() {
25+
return $this->getProxyComment()->getContent();
26+
}
27+
28+
public function setContent($content) {
29+
// NOTE: We no longer read this field, but there's no cost to continuing
30+
// to write it in case something goes horribly wrong, since it makes it
31+
// far easier to back out of this.
32+
$this->content = $content;
33+
$this->getProxyComment()->setContent($content);
34+
return $this;
35+
}
36+
37+
private function getProxyComment() {
38+
if (!$this->proxyComment) {
39+
$this->proxyComment = new DifferentialTransactionComment();
40+
}
41+
return $this->proxyComment;
42+
}
43+
44+
public function setProxyComment(DifferentialTransactionComment $proxy) {
45+
if ($this->proxyComment) {
46+
throw new Exception(pht('You can not overwrite a proxy comment.'));
47+
}
48+
$this->proxyComment = $proxy;
49+
return $this;
50+
}
2251

2352
public function setRevision(DifferentialRevision $revision) {
53+
$this->getProxyComment()->setRevisionPHID($revision->getPHID());
2454
return $this->setRevisionID($revision->getID());
2555
}
2656

@@ -93,4 +123,31 @@ public function shouldUseMarkupCache($field) {
93123
return (bool)$this->getID();
94124
}
95125

126+
public function save() {
127+
$this->openTransaction();
128+
$result = parent::save();
129+
130+
$content_source = PhabricatorContentSource::newForSource(
131+
PhabricatorContentSource::SOURCE_LEGACY,
132+
array());
133+
134+
$xaction_phid = PhabricatorPHID::generateNewPHID(
135+
PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
136+
DifferentialPHIDTypeRevision::TYPECONST);
137+
138+
$proxy = $this->getProxyComment();
139+
$proxy
140+
->setAuthorPHID($this->getAuthorPHID())
141+
->setViewPolicy('public')
142+
->setEditPolicy($this->getAuthorPHID())
143+
->setContentSource($content_source)
144+
->setCommentVersion(1)
145+
->setLegacyCommentID($this->getID())
146+
->setTransactionPHID($xaction_phid)
147+
->save();
148+
$this->saveTransaction();
149+
150+
return $result;
151+
}
152+
96153
}

0 commit comments

Comments
 (0)
Failed to load comments.