Migrate Audit inline comments to new storage

Summary:
Ref T4896. This is substantially identical to the process which Differential followed, and mostly copied from the original Differential migration and the Differential proxy object.

Basically, we move all the data over but the application can't tell, and the same APIs do reads and writes to the new table.

Test Plan:
 - Browsed UI before migrating, everything looked fine (but no inlines).
 - Ran migration.
 - Verified draft and published comments survived migration.
 - Added a draft.
 - Previewed draft.
 - Submitted draft.
 - Viewed standalone with drafts and published comments.
 - Sanity checked data in database, didn't see anything unusual.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4896

Differential Revision: https://secure.phabricator.com/D10018

resources/sql/autopatches/20140722.audit.3.miginlines.php

@@ -0,0 +1,77 @@
+<?php
+
+$audit_table = new PhabricatorAuditTransaction();
+$conn_w = $audit_table->establishConnection('w');
+$conn_w->openTransaction();
+
+$src_table = 'audit_inlinecomment';
+$dst_table = 'audit_transaction_comment';
+
+echo "Migrating Audit 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'];
+
+  echo "Migrating inline #{$id}...\n";
+
+  if ($row['auditCommentID']) {
+    $xaction_phid = PhabricatorPHID::generateNewPHID(
+      PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST,
+      PhabricatorRepositoryCommitPHIDType::TYPECONST);
+  } else {
+    $xaction_phid = null;
+  }
+
+  $comment_phid = PhabricatorPHID::generateNewPHID(
+    PhabricatorPHIDConstants::PHID_TYPE_XCMT,
+    PhabricatorRepositoryCommitPHIDType::TYPECONST);
+
+  queryfx(
+    $conn_w,
+    'INSERT IGNORE INTO %T
+      (id, phid, transactionPHID, authorPHID, viewPolicy, editPolicy,
+        commentVersion, content, contentSource, isDeleted,
+        dateCreated, dateModified, commitPHID, pathID,
+        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, commitPHID, pathID
+    $row['dateCreated'],
+    $row['dateModified'],
+    $row['commitPHID'],
+    $row['pathID'],
+
+    // isNewFile, lineNumber, lineLength, hasReplies, legacyCommentID
+    $row['isNewFile'],
+    $row['lineNumber'],
+    $row['lineLength'],
+    0,
+    $row['auditCommentID']);
+
+}
+
+$conn_w->saveTransaction();
+echo "Done.\n";

src/__phutil_library_map__.php

@@ -3932,10 +3932,7 @@ phutil_register_library_map(array(
     'PhabricatorAuditCommentEditor' => 'PhabricatorEditor',
     'PhabricatorAuditController' => 'PhabricatorController',
     'PhabricatorAuditDAO' => 'PhabricatorLiskDAO',
-    'PhabricatorAuditInlineComment' => array(
-      'PhabricatorAuditDAO',
-      'PhabricatorInlineCommentInterface',
-    ),
+    'PhabricatorAuditInlineComment' => 'PhabricatorInlineCommentInterface',
     'PhabricatorAuditListController' => 'PhabricatorAuditController',
     'PhabricatorAuditListView' => 'AphrontView',
     'PhabricatorAuditMailReceiver' => 'PhabricatorObjectMailReceiver',

src/applications/audit/storage/PhabricatorAuditInlineComment.php

@@ -1,39 +1,54 @@
 <?php
 
 final class PhabricatorAuditInlineComment
-  extends PhabricatorAuditDAO
   implements PhabricatorInlineCommentInterface {
 
-  protected $commitPHID;
-  protected $pathID;
-  protected $auditCommentID;
-
-  protected $authorPHID;
-  protected $isNewFile;
-  protected $lineNumber;
-  protected $lineLength;
-  protected $content;
-  protected $cache;
-
+  private $proxy;
   private $syntheticAuthor;
 
+  public function __construct() {
+    $this->proxy = new PhabricatorAuditTransactionComment();
+  }
+
+  public function __clone() {
+    $this->proxy = clone $this->proxy;
+  }
+
+  public function getTransactionCommentForSave() {
+    $content_source = PhabricatorContentSource::newForSource(
+      PhabricatorContentSource::SOURCE_LEGACY,
+      array());
+
+    $this->proxy
+      ->setViewPolicy('public')
+      ->setEditPolicy($this->getAuthorPHID())
+      ->setContentSource($content_source)
+      ->setCommentVersion(1);
+
+    return $this->proxy;
+  }
+
   public static function loadDraftComments(
     PhabricatorUser $viewer,
     $commit_phid) {
 
-    return id(new PhabricatorAuditInlineComment())->loadAllWhere(
-      'authorPHID = %s AND commitPHID = %s AND auditCommentID IS NULL',
+    $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
+      'authorPHID = %s AND commitPHID = %s AND transactionPHID IS NULL',
       $viewer->getPHID(),
       $commit_phid);
+
+    return self::buildProxies($inlines);
   }
 
   public static function loadPublishedComments(
     PhabricatorUser $viewer,
     $commit_phid) {
 
-    return id(new PhabricatorAuditInlineComment())->loadAllWhere(
-      'commitPHID = %s AND auditCommentID IS NOT NULL',
+    $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
+      'commitPHID = %s AND transactionPHID IS NOT NULL',
       $commit_phid);
+
+    return self::buildProxies($inlines);
   }
 
   public static function loadDraftAndPublishedComments(
@@ -42,18 +57,29 @@ final class PhabricatorAuditInlineComment
     $path_id = null) {
 
     if ($path_id === null) {
-      return id(new PhabricatorAuditInlineComment())->loadAllWhere(
-        'commitPHID = %s AND (auditCommentID IS NOT NULL OR authorPHID = %s)',
+      $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
+        'commitPHID = %s AND (transactionPHID IS NOT NULL OR authorPHID = %s)',
         $commit_phid,
         $viewer->getPHID());
     } else {
-      return id(new PhabricatorAuditInlineComment())->loadAllWhere(
+      $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere(
         'commitPHID = %s AND pathID = %d AND
-          (authorPHID = %s OR auditCommentID IS NOT NULL)',
+          (authorPHID = %s OR transactionPHID IS NOT NULL)',
         $commit_phid,
         $path_id,
         $viewer->getPHID());
     }
+
+    return self::buildProxies($inlines);
+  }
+
+  private static function buildProxies(array $inlines) {
+    $results = array();
+    foreach ($inlines as $key => $inline) {
+      $results[$key] = PhabricatorAuditInlineComment::newFromModernComment(
+        $inline);
+    }
+    return $results;
   }
 
   public function setSyntheticAuthor($synthetic_author) {
@@ -65,6 +91,43 @@ final class PhabricatorAuditInlineComment
     return $this->syntheticAuthor;
   }
 
+  public function openTransaction() {
+    $this->proxy->openTransaction();
+  }
+
+  public function saveTransaction() {
+    $this->proxy->saveTransaction();
+  }
+
+  public function save() {
+    $this->getTransactionCommentForSave()->save();
+
+    return $this;
+  }
+
+  public function delete() {
+    $this->proxy->delete();
+
+    return $this;
+  }
+
+  public function getID() {
+    return $this->proxy->getID();
+  }
+
+  public function getPHID() {
+    return $this->proxy->getPHID();
+  }
+
+  public static function newFromModernComment(
+    PhabricatorAuditTransactionComment $comment) {
+
+    $obj = new PhabricatorAuditInlineComment();
+    $obj->proxy = $comment;
+
+    return $obj;
+  }
+
   public function isCompatible(PhabricatorInlineCommentInterface $comment) {
     return
       ($this->getAuthorPHID() === $comment->getAuthorPHID()) &&
@@ -73,17 +136,94 @@ final class PhabricatorAuditInlineComment
   }
 
   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->getAuditCommentID();
+    return !$this->proxy->getTransactionPHID();
+  }
+
+  public function setPathID($id) {
+    $this->proxy->setPathID($id);
+    return $this;
+  }
+
+  public function getPathID() {
+    return $this->proxy->getPathID();
+  }
+
+  public function setIsNewFile($is_new) {
+    $this->proxy->setIsNewFile($is_new);
+    return $this;
+  }
+
+  public function getIsNewFile() {
+    return $this->proxy->getIsNewFile();
+  }
+
+  public function setLineNumber($number) {
+    $this->proxy->setLineNumber($number);
+    return $this;
+  }
+
+  public function getLineNumber() {
+    return $this->proxy->getLineNumber();
+  }
+
+  public function setLineLength($length) {
+    $this->proxy->setLineLength($length);
+    return $this;
+  }
+
+  public function getLineLength() {
+    return $this->proxy->getLineLength();
+  }
+
+  public function setCache($cache) {
+    return $this;
+  }
+
+  public function getCache() {
+    return null;
+  }
+
+  public function setAuthorPHID($phid) {
+    $this->proxy->setAuthorPHID($phid);
+    return $this;
+  }
+
+  public function getAuthorPHID() {
+    return $this->proxy->getAuthorPHID();
+  }
+
+  public function setCommitPHID($commit_phid) {
+    $this->proxy->setCommitPHID($commit_phid);
+    return $this;
+  }
+
+  public function getCommitPHID() {
+    return $this->proxy->getCommitPHID();
+  }
+
+  // When setting a comment ID, we also generate a phantom transaction PHID for
+  // the future transaction.
+
+  public function setAuditCommentID($id) {
+    $this->proxy->setLegacyCommentID($id);
+    $this->proxy->setTransactionPHID(
+      PhabricatorPHID::generateNewPHID(
+        PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST,
+        PhabricatorRepositoryCommitPHIDType::TYPECONST));
+    return $this;
+  }
+
+  public function getAuditCommentID() {
+    return $this->proxy->getLegacyCommentID();
   }
 
   public function setChangesetID($id) {
@@ -94,54 +234,6 @@ final class PhabricatorAuditInlineComment
     return $this->getPathID();
   }
 
-  // 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 setIsNewFile($is_new) {
-    $this->writeField('isNewFile', $is_new);
-    return $this;
-  }
-
-  public function getIsNewFile() {
-    return $this->readField('isNewFile');
-  }
-
-  public function setLineNumber($number) {
-    $this->writeField('lineNumber', $number);
-    return $this;
-  }
-
-  public function getLineNumber() {
-    return $this->readField('lineNumber');
-  }
-
-  public function setLineLength($length) {
-    $this->writeField('lineLength', $length);
-    return $this;
-  }
-
-  public function getLineLength() {
-    return $this->readField('lineLength');
-  }
-
-  public function setCache($cache) {
-    $this->writeField('cache', $cache);
-    return $this;
-  }
-
-  public function getCache() {
-    return $this->readField('cache');
-  }
-
-  public function setAuthorPHID($phid) {
-    $this->writeField('authorPHID', $phid);
-    return $this;
-  }
-
-  public function getAuthorPHID() {
-    return $this->readField('authorPHID');
-  }
 
 /* -(  PhabricatorMarkupInterface Implementation  )-------------------------- */