Completely remove the legacy hunk table

Summary: Depends on D19056. Fixes T8475. Ref T13054. Merges "ModernHunk" back into "Hunk".

Test Plan: Grepped for `modernhunk`. Reviewed revisions. Created a new revision. Used `bin/differential migrate-hunk` to migrate hunks between storage formats and back.

Maniphest Tasks: T13054, T8475

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

resources/sql/autopatches/20161213.diff.01.hunks.php

@@ -25,9 +25,9 @@ foreach (new LiskRawMigrationIterator($conn, $src_table) as $row) {
     $row['oldLen'],
     $row['newOffset'],
     $row['newLen'],
-    DifferentialModernHunk::DATATYPE_TEXT,
+    DifferentialHunk::DATATYPE_TEXT,
     'utf8',
-    DifferentialModernHunk::DATAFORMAT_RAW,
+    DifferentialHunk::DATAFORMAT_RAW,
     // In rare cases, this could be NULL. See T12090.
     (string)$row['changes'],
     $row['dateCreated'],

resources/sql/autopatches/20180210.hunk.02.renamemodern.sql

@@ -0,0 +1,2 @@
+RENAME TABLE {$NAMESPACE}_differential.differential_hunk_modern
+  TO {$NAMESPACE}_differential.differential_hunk;

src/__phutil_library_map__.php

@@ -489,7 +489,6 @@ phutil_register_library_map(array(
     'DifferentialMailEngineExtension' => 'applications/differential/engineextension/DifferentialMailEngineExtension.php',
     'DifferentialMailView' => 'applications/differential/mail/DifferentialMailView.php',
     'DifferentialManiphestTasksField' => 'applications/differential/customfield/DifferentialManiphestTasksField.php',
-    'DifferentialModernHunk' => 'applications/differential/storage/DifferentialModernHunk.php',
     'DifferentialParseCacheGarbageCollector' => 'applications/differential/garbagecollector/DifferentialParseCacheGarbageCollector.php',
     'DifferentialParseCommitMessageConduitAPIMethod' => 'applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php',
     'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php',
@@ -5651,7 +5650,6 @@ phutil_register_library_map(array(
     'DifferentialMailEngineExtension' => 'PhabricatorMailEngineExtension',
     'DifferentialMailView' => 'Phobject',
     'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField',
-    'DifferentialModernHunk' => 'DifferentialHunk',
     'DifferentialParseCacheGarbageCollector' => 'PhabricatorGarbageCollector',
     'DifferentialParseCommitMessageConduitAPIMethod' => 'DifferentialConduitAPIMethod',
     'DifferentialParseRenderTestCase' => 'PhabricatorTestCase',

src/applications/differential/engine/DifferentialDiffExtractionEngine.php

@@ -217,8 +217,8 @@ final class DifferentialDiffExtractionEngine extends Phobject {
         //   -echo "test";
         //   -(empty line)
 
-        $hunk = id(new DifferentialModernHunk())->setChanges($context);
+        $hunk = id(new DifferentialHunk())->setChanges($context);
-        $vs_hunk = id(new DifferentialModernHunk())->setChanges($vs_context);
+        $vs_hunk = id(new DifferentialHunk())->setChanges($vs_context);
         if ($hunk->makeOldFile() != $vs_hunk->makeOldFile() ||
             $hunk->makeNewFile() != $vs_hunk->makeNewFile()) {
           return true;

src/applications/differential/management/PhabricatorDifferentialMigrateHunkWorkflow.php

@@ -32,8 +32,8 @@ final class PhabricatorDifferentialMigrateHunkWorkflow
 
     $storage = $args->getArg('to');
     switch ($storage) {
-      case DifferentialModernHunk::DATATYPE_TEXT:
+      case DifferentialHunk::DATATYPE_TEXT:
-      case DifferentialModernHunk::DATATYPE_FILE:
+      case DifferentialHunk::DATATYPE_FILE:
         break;
       default:
         throw new PhutilArgumentUsageException(
@@ -44,13 +44,13 @@ final class PhabricatorDifferentialMigrateHunkWorkflow
     $old_data = $hunk->getChanges();
 
     switch ($storage) {
-      case DifferentialModernHunk::DATATYPE_TEXT:
+      case DifferentialHunk::DATATYPE_TEXT:
         $hunk->saveAsText();
         $this->logOkay(
           pht('TEXT'),
           pht('Convereted hunk to text storage.'));
         break;
-      case DifferentialModernHunk::DATATYPE_FILE:
+      case DifferentialHunk::DATATYPE_FILE:
         $hunk->saveAsFile();
         $this->logOkay(
           pht('FILE'),
@@ -71,7 +71,7 @@ final class PhabricatorDifferentialMigrateHunkWorkflow
   }
 
   private function loadHunk($id) {
-    $hunk = id(new DifferentialModernHunk())->load($id);
+    $hunk = id(new DifferentialHunk())->load($id);
     if (!$hunk) {
       throw new PhutilArgumentUsageException(
         pht(

src/applications/differential/parser/__tests__/DifferentialChangesetParserTestCase.php

@@ -3,7 +3,7 @@
 final class DifferentialChangesetParserTestCase extends PhabricatorTestCase {
 
   public function testDiffChangesets() {
-    $hunk = new DifferentialModernHunk();
+    $hunk = new DifferentialHunk();
     $hunk->setChanges("+a\n b\n-c");
     $hunk->setNewOffset(1);
     $hunk->setNewLen(2);
@@ -20,7 +20,7 @@ final class DifferentialChangesetParserTestCase extends PhabricatorTestCase {
     );
 
     foreach ($tests as $changes => $expected) {
-      $hunk = new DifferentialModernHunk();
+      $hunk = new DifferentialHunk();
       $hunk->setChanges($changes);
       $hunk->setNewOffset(11);
       $hunk->setNewLen(3);

src/applications/differential/parser/__tests__/DifferentialHunkParserTestCase.php

@@ -14,7 +14,7 @@ final class DifferentialHunkParserTestCase extends PhabricatorTestCase {
     $new_len,
     $changes) {
 
-    $hunk = id(new DifferentialModernHunk())
+    $hunk = id(new DifferentialHunk())
       ->setOldOffset($old_offset)
       ->setOldLen($old_len)
       ->setNewOffset($new_offset)

src/applications/differential/query/DifferentialHunkQuery.php

@@ -30,25 +30,12 @@ final class DifferentialHunkQuery
     }
   }
 
+  public function newResultObject() {
+    return new DifferentialHunk();
+  }
+
   protected function loadPage() {
-    $all_results = array();
+    return $this->loadStandardPage($this->newResultObject());
-
-    // Load modern hunks.
-    $table = new DifferentialModernHunk();
-    $conn_r = $table->establishConnection('r');
-
-    $modern_data = queryfx_all(
-      $conn_r,
-      'SELECT * FROM %T %Q %Q %Q',
-      $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildOrderClause($conn_r),
-      $this->buildLimitClause($conn_r));
-    $modern_results = $table->loadAllFromArray($modern_data);
-
-    // Strip all the IDs off since they're not unique and nothing should be
-    // using them.
-    return array_values($modern_results);
   }
 
   protected function willFilterPage(array $hunks) {
@@ -76,8 +63,8 @@ final class DifferentialHunkQuery
     return $hunks;
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+  protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
-    $where = array();
+    $where = parent::buildWhereClauseParts($conn);
 
     if (!$this->changesets) {
       throw new Exception(
@@ -87,13 +74,11 @@ final class DifferentialHunkQuery
     }
 
     $where[] = qsprintf(
-      $conn_r,
+      $conn,
       'changesetID IN (%Ld)',
       mpull($this->changesets, 'getID'));
 
-    $where[] = $this->buildPagingClause($conn_r);
+    return $where;
-
-    return $this->formatWhereClause($where);
   }
 
   public function getQueryApplicationClass() {

src/applications/differential/storage/DifferentialChangeset.php

@@ -118,11 +118,11 @@ final class DifferentialChangeset
   public function delete() {
     $this->openTransaction();
 
-      $modern_hunks = id(new DifferentialModernHunk())->loadAllWhere(
+      $hunks = id(new DifferentialHunk())->loadAllWhere(
         'changesetID = %d',
         $this->getID());
-      foreach ($modern_hunks as $modern_hunk) {
+      foreach ($hunks as $hunk) {
-        $modern_hunk->delete();
+        $hunk->delete();
       }
 
       $this->unsavedHunks = array();
@@ -292,7 +292,7 @@ final class DifferentialChangeset
     PhabricatorDestructionEngine $engine) {
     $this->openTransaction();
 
-      $hunks = id(new DifferentialModernHunk())->loadAllWhere(
+      $hunks = id(new DifferentialHunk())->loadAllWhere(
         'changesetID = %d',
         $this->getID());
       foreach ($hunks as $hunk) {

src/applications/differential/storage/DifferentialDiff.php

@@ -189,7 +189,7 @@ final class DifferentialDiff
       $hunks = $change->getHunks();
       if ($hunks) {
         foreach ($hunks as $hunk) {
-          $dhunk = new DifferentialModernHunk();
+          $dhunk = new DifferentialHunk();
           $dhunk->setOldOffset($hunk->getOldOffset());
           $dhunk->setOldLen($hunk->getOldLength());
           $dhunk->setNewOffset($hunk->getNewOffset());

src/applications/differential/storage/DifferentialHunk.php

@@ -1,6 +1,6 @@
 <?php
 
-abstract class DifferentialHunk
+final class DifferentialHunk
   extends DifferentialDAO
   implements
     PhabricatorPolicyInterface,
@@ -11,16 +11,55 @@ abstract class DifferentialHunk
   protected $oldLen;
   protected $newOffset;
   protected $newLen;
+  protected $dataType;
+  protected $dataEncoding;
+  protected $dataFormat;
+  protected $data;
 
   private $changeset;
   private $splitLines;
   private $structuredLines;
   private $structuredFiles = array();
 
+  private $rawData;
+  private $forcedEncoding;
+  private $fileData;
+
   const FLAG_LINES_ADDED     = 1;
   const FLAG_LINES_REMOVED   = 2;
   const FLAG_LINES_STABLE    = 4;
 
+  const DATATYPE_TEXT       = 'text';
+  const DATATYPE_FILE       = 'file';
+
+  const DATAFORMAT_RAW      = 'byte';
+  const DATAFORMAT_DEFLATED = 'gzde';
+
+  protected function getConfiguration() {
+    return array(
+      self::CONFIG_BINARY => array(
+        'data' => true,
+      ),
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'dataType' => 'bytes4',
+        'dataEncoding' => 'text16?',
+        'dataFormat' => 'bytes4',
+        'oldOffset' => 'uint32',
+        'oldLen' => 'uint32',
+        'newOffset' => 'uint32',
+        'newLen' => 'uint32',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_changeset' => array(
+          'columns' => array('changesetID'),
+        ),
+        'key_created' => array(
+          'columns' => array('dateCreated'),
+        ),
+      ),
+    ) + parent::getConfiguration();
+  }
+
   public function getAddedLines() {
     return $this->makeContent($include = '+');
   }
@@ -214,6 +253,197 @@ abstract class DifferentialHunk
   }
 
 
+/* -(  Storage  )------------------------------------------------------------ */
+
+
+  public function setChanges($text) {
+    $this->rawData = $text;
+
+    $this->dataEncoding = $this->detectEncodingForStorage($text);
+    $this->dataType = self::DATATYPE_TEXT;
+
+    list($format, $data) = $this->formatDataForStorage($text);
+
+    $this->dataFormat = $format;
+    $this->data = $data;
+
+    return $this;
+  }
+
+  public function getChanges() {
+    return $this->getUTF8StringFromStorage(
+      $this->getRawData(),
+      nonempty($this->forcedEncoding, $this->getDataEncoding()));
+  }
+
+  public function forceEncoding($encoding) {
+    $this->forcedEncoding = $encoding;
+    return $this;
+  }
+
+  private function formatDataForStorage($data) {
+    $deflated = PhabricatorCaches::maybeDeflateData($data);
+    if ($deflated !== null) {
+      return array(self::DATAFORMAT_DEFLATED, $deflated);
+    }
+
+    return array(self::DATAFORMAT_RAW, $data);
+  }
+
+  public function saveAsText() {
+    $old_type = $this->getDataType();
+    $old_data = $this->getData();
+
+    if ($old_type == self::DATATYPE_TEXT) {
+      return $this;
+    }
+
+    $raw_data = $this->getRawData();
+
+    $this->setDataType(self::DATATYPE_TEXT);
+
+    list($format, $data) = $this->formatDataForStorage($raw_data);
+    $this->setDataFormat($format);
+    $this->setData($data);
+
+    $result = $this->save();
+
+    $this->destroyData($old_type, $old_data);
+
+    return $result;
+  }
+
+  public function saveAsFile() {
+    $old_type = $this->getDataType();
+    $old_data = $this->getData();
+
+    if ($old_type == self::DATATYPE_FILE) {
+      return $this;
+    }
+
+    $raw_data = $this->getRawData();
+
+    list($format, $data) = $this->formatDataForStorage($raw_data);
+    $this->setDataFormat($format);
+
+    $file = PhabricatorFile::newFromFileData(
+      $data,
+      array(
+        'name' => 'differential-hunk',
+        'mime-type' => 'application/octet-stream',
+        'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
+      ));
+
+    $this->setDataType(self::DATATYPE_FILE);
+    $this->setData($file->getPHID());
+
+    // NOTE: Because hunks don't have a PHID and we just load hunk data with
+    // the omnipotent viewer, we do not need to attach the file to anything.
+
+    $result = $this->save();
+
+    $this->destroyData($old_type, $old_data);
+
+    return $result;
+  }
+
+  private function getRawData() {
+    if ($this->rawData === null) {
+      $type = $this->getDataType();
+      $data = $this->getData();
+
+      switch ($type) {
+        case self::DATATYPE_TEXT:
+          // In this storage type, the changes are stored on the object.
+          $data = $data;
+          break;
+        case self::DATATYPE_FILE:
+          $data = $this->loadFileData();
+          break;
+        default:
+          throw new Exception(
+            pht('Hunk has unsupported data type "%s"!', $type));
+      }
+
+      $format = $this->getDataFormat();
+      switch ($format) {
+        case self::DATAFORMAT_RAW:
+          // In this format, the changes are stored as-is.
+          $data = $data;
+          break;
+        case self::DATAFORMAT_DEFLATED:
+          $data = PhabricatorCaches::inflateData($data);
+          break;
+        default:
+          throw new Exception(
+            pht('Hunk has unsupported data encoding "%s"!', $type));
+      }
+
+      $this->rawData = $data;
+    }
+
+    return $this->rawData;
+  }
+
+  private function loadFileData() {
+    if ($this->fileData === null) {
+      $type = $this->getDataType();
+      if ($type !== self::DATATYPE_FILE) {
+        throw new Exception(
+          pht(
+            'Unable to load file data for hunk with wrong data type ("%s").',
+            $type));
+      }
+
+      $file_phid = $this->getData();
+
+      $file = $this->loadRawFile($file_phid);
+      $data = $file->loadFileData();
+
+      $this->fileData = $data;
+    }
+
+    return $this->fileData;
+  }
+
+  private function loadRawFile($file_phid) {
+    $viewer = PhabricatorUser::getOmnipotentUser();
+
+
+    $files = id(new PhabricatorFileQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($file_phid))
+      ->execute();
+    if (!$files) {
+      throw new Exception(
+        pht(
+          'Failed to load file ("%s") with hunk data.',
+          $file_phid));
+    }
+
+    $file = head($files);
+
+    return $file;
+  }
+
+  private function destroyData(
+    $type,
+    $data,
+    PhabricatorDestructionEngine $engine = null) {
+
+    if (!$engine) {
+      $engine = new PhabricatorDestructionEngine();
+    }
+
+    switch ($type) {
+      case self::DATATYPE_FILE:
+        $file = $this->loadRawFile($data);
+        $engine->destroyObject($file);
+        break;
+    }
+  }
+
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
 
@@ -237,8 +467,13 @@ abstract class DifferentialHunk
 
   public function destroyObjectPermanently(
     PhabricatorDestructionEngine $engine) {
+
+    $type = $this->getDataType();
+    $data = $this->getData();
+
+    $this->destroyData($type, $data, $engine);
+
     $this->delete();
   }
 
-
 }

src/applications/differential/storage/DifferentialModernHunk.php

@@ -1,249 +0,0 @@
-<?php
-
-final class DifferentialModernHunk extends DifferentialHunk {
-
-  const DATATYPE_TEXT       = 'text';
-  const DATATYPE_FILE       = 'file';
-
-  const DATAFORMAT_RAW      = 'byte';
-  const DATAFORMAT_DEFLATED = 'gzde';
-
-  protected $dataType;
-  protected $dataEncoding;
-  protected $dataFormat;
-  protected $data;
-
-  private $rawData;
-  private $forcedEncoding;
-  private $fileData;
-
-  public function getTableName() {
-    return 'differential_hunk_modern';
-  }
-
-  protected function getConfiguration() {
-    return array(
-      self::CONFIG_BINARY => array(
-        'data' => true,
-      ),
-      self::CONFIG_COLUMN_SCHEMA => array(
-        'dataType' => 'bytes4',
-        'dataEncoding' => 'text16?',
-        'dataFormat' => 'bytes4',
-        'oldOffset' => 'uint32',
-        'oldLen' => 'uint32',
-        'newOffset' => 'uint32',
-        'newLen' => 'uint32',
-      ),
-      self::CONFIG_KEY_SCHEMA => array(
-        'key_changeset' => array(
-          'columns' => array('changesetID'),
-        ),
-        'key_created' => array(
-          'columns' => array('dateCreated'),
-        ),
-      ),
-    ) + parent::getConfiguration();
-  }
-
-  public function setChanges($text) {
-    $this->rawData = $text;
-
-    $this->dataEncoding = $this->detectEncodingForStorage($text);
-    $this->dataType = self::DATATYPE_TEXT;
-
-    list($format, $data) = $this->formatDataForStorage($text);
-
-    $this->dataFormat = $format;
-    $this->data = $data;
-
-    return $this;
-  }
-
-  public function getChanges() {
-    return $this->getUTF8StringFromStorage(
-      $this->getRawData(),
-      nonempty($this->forcedEncoding, $this->getDataEncoding()));
-  }
-
-  public function forceEncoding($encoding) {
-    $this->forcedEncoding = $encoding;
-    return $this;
-  }
-
-  private function formatDataForStorage($data) {
-    $deflated = PhabricatorCaches::maybeDeflateData($data);
-    if ($deflated !== null) {
-      return array(self::DATAFORMAT_DEFLATED, $deflated);
-    }
-
-    return array(self::DATAFORMAT_RAW, $data);
-  }
-
-  public function saveAsText() {
-    $old_type = $this->getDataType();
-    $old_data = $this->getData();
-
-    if ($old_type == self::DATATYPE_TEXT) {
-      return $this;
-    }
-
-    $raw_data = $this->getRawData();
-
-    $this->setDataType(self::DATATYPE_TEXT);
-
-    list($format, $data) = $this->formatDataForStorage($raw_data);
-    $this->setDataFormat($format);
-    $this->setData($data);
-
-    $result = $this->save();
-
-    $this->destroyData($old_type, $old_data);
-
-    return $result;
-  }
-
-  public function saveAsFile() {
-    $old_type = $this->getDataType();
-    $old_data = $this->getData();
-
-    if ($old_type == self::DATATYPE_FILE) {
-      return $this;
-    }
-
-    $raw_data = $this->getRawData();
-
-    list($format, $data) = $this->formatDataForStorage($raw_data);
-    $this->setDataFormat($format);
-
-    $file = PhabricatorFile::newFromFileData(
-      $data,
-      array(
-        'name' => 'differential-hunk',
-        'mime-type' => 'application/octet-stream',
-        'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
-      ));
-
-    $this->setDataType(self::DATATYPE_FILE);
-    $this->setData($file->getPHID());
-
-    // NOTE: Because hunks don't have a PHID and we just load hunk data with
-    // the omnipotent viewer, we do not need to attach the file to anything.
-
-    $result = $this->save();
-
-    $this->destroyData($old_type, $old_data);
-
-    return $result;
-  }
-
-  private function getRawData() {
-    if ($this->rawData === null) {
-      $type = $this->getDataType();
-      $data = $this->getData();
-
-      switch ($type) {
-        case self::DATATYPE_TEXT:
-          // In this storage type, the changes are stored on the object.
-          $data = $data;
-          break;
-        case self::DATATYPE_FILE:
-          $data = $this->loadFileData();
-          break;
-        default:
-          throw new Exception(
-            pht('Hunk has unsupported data type "%s"!', $type));
-      }
-
-      $format = $this->getDataFormat();
-      switch ($format) {
-        case self::DATAFORMAT_RAW:
-          // In this format, the changes are stored as-is.
-          $data = $data;
-          break;
-        case self::DATAFORMAT_DEFLATED:
-          $data = PhabricatorCaches::inflateData($data);
-          break;
-        default:
-          throw new Exception(
-            pht('Hunk has unsupported data encoding "%s"!', $type));
-      }
-
-      $this->rawData = $data;
-    }
-
-    return $this->rawData;
-  }
-
-  private function loadFileData() {
-    if ($this->fileData === null) {
-      $type = $this->getDataType();
-      if ($type !== self::DATATYPE_FILE) {
-        throw new Exception(
-          pht(
-            'Unable to load file data for hunk with wrong data type ("%s").',
-            $type));
-      }
-
-      $file_phid = $this->getData();
-
-      $file = $this->loadRawFile($file_phid);
-      $data = $file->loadFileData();
-
-      $this->fileData = $data;
-    }
-
-    return $this->fileData;
-  }
-
-  private function loadRawFile($file_phid) {
-    $viewer = PhabricatorUser::getOmnipotentUser();
-
-
-    $files = id(new PhabricatorFileQuery())
-      ->setViewer($viewer)
-      ->withPHIDs(array($file_phid))
-      ->execute();
-    if (!$files) {
-      throw new Exception(
-        pht(
-          'Failed to load file ("%s") with hunk data.',
-          $file_phid));
-    }
-
-    $file = head($files);
-
-    return $file;
-  }
-
-
-  public function destroyObjectPermanently(
-    PhabricatorDestructionEngine $engine) {
-
-    $type = $this->getDataType();
-    $data = $this->getData();
-
-    $this->destroyData($type, $data, $engine);
-
-    return parent::destroyObjectPermanently($engine);
-  }
-
-
-  private function destroyData(
-    $type,
-    $data,
-    PhabricatorDestructionEngine $engine = null) {
-
-    if (!$engine) {
-      $engine = new PhabricatorDestructionEngine();
-    }
-
-    switch ($type) {
-      case self::DATATYPE_FILE:
-        $file = $this->loadRawFile($data);
-        $engine->destroyObject($file);
-        break;
-    }
-  }
-
-}

src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php

@@ -5,7 +5,7 @@ final class DifferentialHunkTestCase extends PhutilTestCase {
   public function testMakeChanges() {
     $root = dirname(__FILE__).'/hunk/';
 
-    $hunk = new DifferentialModernHunk();
+    $hunk = new DifferentialHunk();
     $hunk->setChanges(Filesystem::readFile($root.'basic.diff'));
     $hunk->setOldOffset(1);
     $hunk->setNewOffset(11);
@@ -23,7 +23,7 @@ final class DifferentialHunkTestCase extends PhutilTestCase {
     );
     $this->assertEqual($added, $hunk->getAddedLines());
 
-    $hunk = new DifferentialModernHunk();
+    $hunk = new DifferentialHunk();
     $hunk->setChanges(Filesystem::readFile($root.'newline.diff'));
     $hunk->setOldOffset(1);
     $hunk->setNewOffset(11);