@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Migrate all Differential inline comments to ApplicationTransactions
Summary:
Ref T2222. This implements step (1) described there, which is moving over all the inline comments.
The old and new tables are simliar. The only real trick here is that `transactionPHID` and `legacyCommentID` mean roughly the same thing (`null` if the inline is a draft, non-null if it has been submitted) but we don't have real `transactionPHID`s yet. We just make some up -- we'll backfill them later.
Two risks here:
- I need to take a second look at the keys on this table. I think we need to tweak them a bit, and it will be less disruptive to do that before this migration than after.
- This will take a while for Facebook, and other large installs with tens of thousands of revisions. I'll communicate this.
I'm otherwise pretty satisfied with this, seems to work well and is pretty low risk / non-disruptive.
Test Plan:
- Before migrating, then after migrating:
- Made a bunch of inlines (drafts, submitted).
- Edited and deleted inlines.
- Verified inlines showed up in preview.
- Verified that inlines aren't indexed when they're drafts (`bin/search index D935`).
- Verified that inlines ARE indexed when they're not drafts.
- Verified that drafts inlines make revisions appear as "with draft" in the revision list.
- Made left, right, and draft inlines.
- Migrated (`bin/storage upgrade`).
- Verified that my inlines from before the migration still showed up.
- (Repeated all the stuff above.)
- Manually inspected the inline comment table.
Reviewers: btrahan
Reviewed By: btrahan
CC: FacebookPOC, aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D7139
+252
-44
+90
resources/sql/patches/20130926.dinline.php
+90
resources/sql/patches/20130926.dinline.php
···
1
+
<?php
2
+
3
+
$revision_table = new DifferentialRevision();
4
+
$conn_w = $revision_table->establishConnection('w');
5
+
$conn_w->openTransaction();
6
+
7
+
$src_table = 'differential_inlinecomment';
8
+
$dst_table = 'differential_transaction_comment';
9
+
10
+
echo "Migrating Differential inline comments to new format...\n";
11
+
12
+
$content_source = PhabricatorContentSource::newForSource(
13
+
PhabricatorContentSource::SOURCE_LEGACY,
14
+
array())->serialize();
15
+
16
+
$rows = new LiskRawMigrationIterator($conn_w, $src_table);
17
+
foreach ($rows as $row) {
18
+
$id = $row['id'];
19
+
20
+
$revision_id = $row['revisionID'];
21
+
22
+
echo "Migrating inline #{$id} (D{$revision_id})...\n";
23
+
24
+
$revision_row = queryfx_one(
25
+
$conn_w,
26
+
'SELECT phid FROM %T WHERE id = %d',
27
+
$revision_table->getTableName(),
28
+
$revision_id);
29
+
if (!$revision_row) {
30
+
continue;
31
+
}
32
+
33
+
$revision_phid = $revision_row['phid'];
34
+
35
+
if ($row['commentID']) {
36
+
$xaction_phid = PhabricatorPHID::generateNewPHID(
37
+
PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
38
+
DifferentialPHIDTypeRevision::TYPECONST);
39
+
} else {
40
+
$xaction_phid = null;
41
+
}
42
+
43
+
$comment_phid = PhabricatorPHID::generateNewPHID(
44
+
PhabricatorPHIDConstants::PHID_TYPE_XCMT,
45
+
DifferentialPHIDTypeRevision::TYPECONST);
46
+
47
+
queryfx(
48
+
$conn_w,
49
+
'INSERT IGNORE INTO %T
50
+
(id, phid, transactionPHID, authorPHID, viewPolicy, editPolicy,
51
+
commentVersion, content, contentSource, isDeleted,
52
+
dateCreated, dateModified, revisionPHID, changesetID,
53
+
isNewFile, lineNumber, lineLength, hasReplies, legacyCommentID)
54
+
VALUES (%d, %s, %ns, %s, %s, %s,
55
+
%d, %s, %s, %d,
56
+
%d, %d, %s, %nd,
57
+
%d, %d, %d, %d, %nd)',
58
+
$dst_table,
59
+
60
+
// id, phid, transactionPHID, authorPHID, viewPolicy, editPolicy
61
+
$row['id'],
62
+
$comment_phid,
63
+
$xaction_phid,
64
+
$row['authorPHID'],
65
+
'public',
66
+
$row['authorPHID'],
67
+
68
+
// commentVersion, content, contentSource, isDeleted
69
+
1,
70
+
$row['content'],
71
+
$content_source,
72
+
0,
73
+
74
+
// dateCreated, dateModified, revisionPHID, changesetID
75
+
$row['dateCreated'],
76
+
$row['dateModified'],
77
+
$revision_phid,
78
+
$row['changesetID'],
79
+
80
+
// isNewFile, lineNumber, lineLength, hasReplies, legacyCommentID
81
+
$row['isNewFile'],
82
+
$row['lineNumber'],
83
+
$row['lineLength'],
84
+
0,
85
+
$row['commentID']);
86
+
87
+
}
88
+
89
+
$conn_w->saveTransaction();
90
+
echo "Done.\n";
+1
-1
src/applications/differential/controller/DifferentialInlineCommentEditController.php
+1
-1
src/applications/differential/controller/DifferentialInlineCommentEditController.php
+49
-10
src/applications/differential/query/DifferentialInlineCommentQuery.php
+49
-10
src/applications/differential/query/DifferentialInlineCommentQuery.php
···
51
51
}
52
52
53
53
public function execute() {
54
-
$table = new DifferentialInlineComment();
54
+
$table = new DifferentialTransactionComment();
55
55
$conn_r = $table->establishConnection('r');
56
56
57
57
$data = queryfx_all(
···
61
61
$this->buildWhereClause($conn_r),
62
62
$this->buildLimitClause($conn_r));
63
63
64
-
return $table->loadAllFromArray($data);
64
+
$comments = $table->loadAllFromArray($data);
65
+
66
+
foreach ($comments as $key => $value) {
67
+
$comments[$key] = DifferentialInlineComment::newFromModernComment(
68
+
$value);
69
+
}
70
+
71
+
return $comments;
65
72
}
66
73
67
74
public function executeOne() {
···
71
78
private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
72
79
$where = array();
73
80
81
+
// Only find inline comments.
82
+
$where[] = qsprintf(
83
+
$conn_r,
84
+
'changesetID IS NOT NULL');
85
+
74
86
if ($this->revisionIDs) {
87
+
88
+
// Look up revision PHIDs.
89
+
$revision_phids = queryfx_all(
90
+
$conn_r,
91
+
'SELECT phid FROM %T WHERE id IN (%Ld)',
92
+
id(new DifferentialRevision())->getTableName(),
93
+
$this->revisionIDs);
94
+
95
+
if (!$revision_phids) {
96
+
throw new PhabricatorEmptyQueryException();
97
+
}
98
+
$revision_phids = ipull($revision_phids, 'phid');
99
+
75
100
$where[] = qsprintf(
76
101
$conn_r,
77
-
'revisionID IN (%Ld)',
78
-
$this->revisionIDs);
102
+
'revisionPHID IN (%Ls)',
103
+
$revision_phids);
79
104
}
80
105
81
106
if ($this->notDraft) {
82
107
$where[] = qsprintf(
83
108
$conn_r,
84
-
'commentID IS NOT NULL');
109
+
'transactionPHID IS NOT NULL');
85
110
}
86
111
87
112
if ($this->ids) {
···
95
120
list($phid, $ids) = $this->viewerAndChangesetIDs;
96
121
$where[] = qsprintf(
97
122
$conn_r,
98
-
'changesetID IN (%Ld) AND (authorPHID = %s OR commentID IS NOT NULL)',
123
+
'changesetID IN (%Ld) AND
124
+
(authorPHID = %s OR transactionPHID IS NOT NULL)',
99
125
$ids,
100
126
$phid);
101
127
}
102
128
103
129
if ($this->draftComments) {
104
130
list($phid, $rev_id) = $this->draftComments;
131
+
132
+
$rev_phid = queryfx_one(
133
+
$conn_r,
134
+
'SELECT phid FROM %T WHERE id = %d',
135
+
id(new DifferentialRevision())->getTableName(),
136
+
$rev_id);
137
+
138
+
if (!$rev_phid) {
139
+
throw new PhabricatorEmptyQueryException();
140
+
}
141
+
142
+
$rev_phid = $rev_phid['phid'];
143
+
105
144
$where[] = qsprintf(
106
145
$conn_r,
107
-
'authorPHID = %s AND revisionID = %d AND commentID IS NULL',
146
+
'authorPHID = %s AND revisionPHID = %s AND transactionPHID IS NULL',
108
147
$phid,
109
-
$rev_id);
148
+
$rev_phid);
110
149
}
111
150
112
151
if ($this->draftsByAuthors) {
113
152
$where[] = qsprintf(
114
153
$conn_r,
115
-
'authorPHID IN (%Ls) AND commentID IS NULL',
154
+
'authorPHID IN (%Ls) AND transactionPHID IS NULL',
116
155
$this->draftsByAuthors);
117
156
}
118
157
119
158
if ($this->commentIDs) {
120
159
$where[] = qsprintf(
121
160
$conn_r,
122
-
'commentID IN (%Ld)',
161
+
'legacyCommentID IN (%Ld)',
123
162
$this->commentIDs);
124
163
}
125
164
+7
src/applications/differential/query/DifferentialRevisionQuery.php
+7
src/applications/differential/query/DifferentialRevisionQuery.php
···
491
491
$this->draftRevisions[] = substr($draft->getDraftKey(), $len);
492
492
}
493
493
494
+
// TODO: Restore this after drafts are sorted out. It's now very
495
+
// expensive to get revision IDs.
496
+
497
+
/*
498
+
494
499
$inlines = id(new DifferentialInlineCommentQuery())
495
500
->withDraftsByAuthors($this->draftAuthors)
496
501
->execute();
497
502
foreach ($inlines as $inline) {
498
503
$this->draftRevisions[] = $inline->getRevisionID();
499
504
}
505
+
506
+
*/
500
507
501
508
if (!$this->draftRevisions) {
502
509
return array();
+98
-30
src/applications/differential/storage/DifferentialInlineComment.php
+98
-30
src/applications/differential/storage/DifferentialInlineComment.php
···
1
1
<?php
2
2
3
3
final class DifferentialInlineComment
4
-
extends DifferentialDAO
5
4
implements PhabricatorInlineCommentInterface {
6
5
7
-
protected $revisionID;
8
-
protected $changesetID;
9
-
protected $commentID;
6
+
private $proxy;
7
+
private $syntheticAuthor;
10
8
11
-
protected $authorPHID;
12
-
protected $isNewFile;
13
-
protected $lineNumber;
14
-
protected $lineLength;
15
-
protected $content;
16
-
protected $cache;
9
+
public function __construct() {
10
+
$this->proxy = new DifferentialTransactionComment();
11
+
}
17
12
18
-
private $syntheticAuthor;
13
+
public function __clone() {
14
+
$this->proxy = clone $this->proxy;
15
+
}
16
+
17
+
public function save() {
18
+
$content_source = PhabricatorContentSource::newForSource(
19
+
PhabricatorContentSource::SOURCE_LEGACY,
20
+
array());
21
+
22
+
$this->proxy
23
+
->setViewPolicy('public')
24
+
->setEditPolicy($this->getAuthorPHID())
25
+
->setContentSource($content_source)
26
+
->setCommentVersion(1)
27
+
->save();
28
+
29
+
return $this;
30
+
}
31
+
32
+
public function delete() {
33
+
$this->proxy->delete();
34
+
35
+
return $this;
36
+
}
37
+
38
+
public function getID() {
39
+
return $this->proxy->getID();
40
+
}
41
+
42
+
public static function newFromModernComment(
43
+
DifferentialTransactionComment $comment) {
44
+
45
+
$obj = new DifferentialInlineComment();
46
+
$obj->proxy = $comment;
47
+
48
+
return $obj;
49
+
}
19
50
20
51
public function setSyntheticAuthor($synthetic_author) {
21
52
$this->syntheticAuthor = $synthetic_author;
···
34
65
}
35
66
36
67
public function setContent($content) {
37
-
$this->setCache(null);
38
-
$this->writeField('content', $content);
68
+
$this->proxy->setContent($content);
39
69
return $this;
40
70
}
41
71
42
72
public function getContent() {
43
-
return $this->readField('content');
73
+
return $this->proxy->getContent();
44
74
}
45
75
46
76
public function isDraft() {
47
77
return !$this->getCommentID();
48
78
}
49
79
50
-
// NOTE: We need to provide implementations so we conform to the shared
51
-
// interface; these are all trivial and just explicit versions of the Lisk
52
-
// defaults.
53
-
54
80
public function setChangesetID($id) {
55
-
$this->writeField('changesetID', $id);
81
+
$this->proxy->setChangesetID($id);
56
82
return $this;
57
83
}
58
84
59
85
public function getChangesetID() {
60
-
return $this->readField('changesetID');
86
+
return $this->proxy->getChangesetID();
61
87
}
62
88
63
89
public function setIsNewFile($is_new) {
64
-
$this->writeField('isNewFile', $is_new);
90
+
$this->proxy->setIsNewFile($is_new);
65
91
return $this;
66
92
}
67
93
68
94
public function getIsNewFile() {
69
-
return $this->readField('isNewFile');
95
+
return $this->proxy->getIsNewFile();
70
96
}
71
97
72
98
public function setLineNumber($number) {
73
-
$this->writeField('lineNumber', $number);
99
+
$this->proxy->setLineNumber($number);
74
100
return $this;
75
101
}
76
102
77
103
public function getLineNumber() {
78
-
return $this->readField('lineNumber');
104
+
return $this->proxy->getLineNumber();
79
105
}
80
106
81
107
public function setLineLength($length) {
82
-
$this->writeField('lineLength', $length);
108
+
$this->proxy->setLineLength($length);
83
109
return $this;
84
110
}
85
111
86
112
public function getLineLength() {
87
-
return $this->readField('lineLength');
113
+
return $this->proxy->getLineLength();
88
114
}
89
115
90
116
public function setCache($cache) {
91
-
$this->writeField('cache', $cache);
92
117
return $this;
93
118
}
94
119
95
120
public function getCache() {
96
-
return $this->readField('cache');
121
+
return null;
97
122
}
98
123
99
124
public function setAuthorPHID($phid) {
100
-
$this->writeField('authorPHID', $phid);
125
+
$this->proxy->setAuthorPHID($phid);
101
126
return $this;
102
127
}
103
128
104
129
public function getAuthorPHID() {
105
-
return $this->readField('authorPHID');
130
+
return $this->proxy->getAuthorPHID();
131
+
}
132
+
133
+
public function setRevision(DifferentialRevision $revision) {
134
+
$this->proxy->setRevisionPHID($revision->getPHID());
135
+
return $this;
136
+
}
137
+
138
+
// Although these are purely transitional, they're also *extra* dumb.
139
+
140
+
public function setRevisionID($revision_id) {
141
+
$revision = id(new DifferentialRevision())->load($revision_id);
142
+
return $this->setRevision($revision);
143
+
}
144
+
145
+
public function getRevisionID() {
146
+
$phid = $this->proxy->getRevisionPHID();
147
+
if (!$phid) {
148
+
return null;
149
+
}
150
+
151
+
$revision = id(new DifferentialRevision())->loadOneWhere(
152
+
'phid = %s',
153
+
$phid);
154
+
if (!$revision) {
155
+
return null;
156
+
}
157
+
return $revision->getID();
158
+
}
159
+
160
+
// When setting a comment ID, we also generate a phantom transaction PHID for
161
+
// the future transaction.
162
+
163
+
public function setCommentID($id) {
164
+
$this->proxy->setLegacyCommentID($id);
165
+
$this->proxy->setTransactionPHID(
166
+
PhabricatorPHID::generateNewPHID(
167
+
PhabricatorApplicationTransactionPHIDTypeTransaction::TYPECONST,
168
+
DifferentialPHIDTypeRevision::TYPECONST));
169
+
return $this;
170
+
}
171
+
172
+
public function getCommentID() {
173
+
return $this->proxy->getLegacyCommentID();
106
174
}
107
175
108
176
+3
-3
src/applications/differential/storage/DifferentialTransactionComment.php
+3
-3
src/applications/differential/storage/DifferentialTransactionComment.php
···
5
5
6
6
protected $revisionPHID;
7
7
protected $changesetID;
8
-
protected $isNewFile;
9
-
protected $lineNumber;
10
-
protected $lineLength;
8
+
protected $isNewFile = 0;
9
+
protected $lineNumber = 0;
10
+
protected $lineLength = 0;
11
11
protected $fixedState;
12
12
protected $hasReplies = 0;
13
13
protected $replyToCommentPHID;
+4
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
+4
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php