Increase the storage size for commit summaries

Summary:
Fixes T11453. Currently, commit message summaries are limited to 80 bytes. This may only be 20-40 characters for CJK languages or langauges with Cyrillic script.

Increase storage size to 255, then truncate to the shorter of 255 bytes or 80 glyphs. This preserves the same behavior for latin languages, but is less tight for Russian, etc.

Some minor additional changes:

  - Provide a way to ask "how much data fits in this column?" so we don't have to duplicate column lengths across summary checks or UI errors like "title too long".
  - Remove the `text80` datatype, since no other columns use it and we have no use cases (or likely use cases) for it.

Test Plan:
  - Made a commit with a Cyrillic title, saw reasonable summarization in UI:

{F1757522}

  - Added and ran unit tests.
  - Grepped for removed `SUMMARY_MAX_LENGTH` constant.
  - Grepped for removed `text80` data type.

Reviewers: avivey, chad

Reviewed By: avivey

Subscribers: avivey

Maniphest Tasks: T11453

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

resources/sql/autopatches/20160810.commit.01.summarylength.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_repository.repository_commit
+  CHANGE summary summary VARCHAR(255) NOT NULL COLLATE {$COLLATE_TEXT};

src/__phutil_library_map__.php

@@ -3381,6 +3381,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryCommitPHIDType' => 'applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php',
     'PhabricatorRepositoryCommitParserWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php',
     'PhabricatorRepositoryCommitRef' => 'applications/repository/engine/PhabricatorRepositoryCommitRef.php',
+    'PhabricatorRepositoryCommitTestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryCommitTestCase.php',
     'PhabricatorRepositoryConfigOptions' => 'applications/repository/config/PhabricatorRepositoryConfigOptions.php',
     'PhabricatorRepositoryDAO' => 'applications/repository/storage/PhabricatorRepositoryDAO.php',
     'PhabricatorRepositoryDiscoveryEngine' => 'applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php',
@@ -8338,6 +8339,7 @@ phutil_register_library_map(array(
     'PhabricatorRepositoryCommitPHIDType' => 'PhabricatorPHIDType',
     'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker',
     'PhabricatorRepositoryCommitRef' => 'Phobject',
+    'PhabricatorRepositoryCommitTestCase' => 'PhabricatorTestCase',
     'PhabricatorRepositoryConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'PhabricatorRepositoryDAO' => 'PhabricatorLiskDAO',
     'PhabricatorRepositoryDiscoveryEngine' => 'PhabricatorRepositoryEngine',

src/applications/config/schema/PhabricatorConfigSchemaSpec.php

@@ -70,7 +70,12 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       }
 
       $details = $this->getDetailsForDataType($type);
-      list($column_type, $charset, $collation, $nullable, $auto) = $details;
+
+      $column_type = $details['type'];
+      $charset = $details['charset'];
+      $collation = $details['collation'];
+      $nullable = $details['nullable'];
+      $auto = $details['auto'];
 
       $column = $this->newColumn($name)
         ->setDataType($type)
@@ -182,11 +187,17 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       ->setName($name);
   }
 
+  public function getMaximumByteLengthForDataType($data_type) {
+    $info = $this->getDetailsForDataType($data_type);
+    return idx($info, 'bytes');
+  }
+
   private function getDetailsForDataType($data_type) {
     $column_type = null;
     $charset = null;
     $collation = null;
     $auto = false;
+    $bytes = null;
 
     // If the type ends with "?", make the column nullable.
     $nullable = false;
@@ -211,7 +222,6 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
         'text255' => true,
         'text160' => true,
         'text128' => true,
-        'text80' => true,
         'text64' => true,
         'text40' => true,
         'text32' => true,
@@ -237,6 +247,10 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       $type = $matches[1];
       $size = idx($matches, 2);
 
+      if ($size) {
+        $bytes = $size;
+      }
+
       switch ($type) {
         case 'text':
           if ($is_binary) {
@@ -363,7 +377,14 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       }
     }
 
-    return array($column_type, $charset, $collation, $nullable, $auto);
+    return array(
+      'type' => $column_type,
+      'charset' => $charset,
+      'collation' => $collation,
+      'nullable' => $nullable,
+      'auto' => $auto,
+      'bytes' => $bytes,
+    );
   }
 
 }

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -108,7 +108,7 @@ final class PhabricatorRepositoryCommit
         'mailKey' => 'bytes20',
         'authorPHID' => 'phid?',
         'auditStatus' => 'uint32',
-        'summary' => 'text80',
+        'summary' => 'text255',
         'importStatus' => 'uint32',
       ),
       self::CONFIG_KEY_SCHEMA => array(

src/applications/repository/storage/PhabricatorRepositoryCommitData.php

@@ -2,12 +2,6 @@
 
 final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
 
-  /**
-   * NOTE: We denormalize this into the commit table; make sure the sizes
-   * match up.
-   */
-  const SUMMARY_MAX_LENGTH = 80;
-
   protected $commitID;
   protected $authorName    = '';
   protected $commitMessage = '';
@@ -38,10 +32,14 @@ final class PhabricatorRepositoryCommitData extends PhabricatorRepositoryDAO {
   }
 
   public static function summarizeCommitMessage($message) {
+    $max_bytes = id(new PhabricatorRepositoryCommit())
+      ->getColumnMaximumByteLength('summary');
+
     $summary = phutil_split_lines($message, $retain_endings = false);
     $summary = head($summary);
     $summary = id(new PhutilUTF8StringTruncator())
-      ->setMaximumBytes(self::SUMMARY_MAX_LENGTH)
+      ->setMaximumBytes($max_bytes)
+      ->setMaximumGlyphs(80)
       ->truncateString($summary);
 
     return $summary;

src/applications/repository/storage/__tests__/PhabricatorRepositoryCommitTestCase.php

@@ -0,0 +1,38 @@
+<?php
+
+final class PhabricatorRepositoryCommitTestCase
+  extends PhabricatorTestCase {
+
+  public function testSummarizeCommits() {
+    // Cyrillic "zhe".
+    $zhe = "\xD0\xB6";
+
+    // Symbol "Snowman".
+    $snowman = "\xE2\x98\x83";
+
+    // Emoji "boar".
+    $boar = "\xF0\x9F\x90\x97";
+
+    // Proper unicode truncation is tested elsewhere, this is just making
+    // sure column length handling is sane.
+
+    $map = array(
+      '' => 0,
+      'a' => 1,
+      str_repeat('a', 81) => 82,
+      str_repeat('a', 255) => 82,
+      str_repeat('aa ', 30) => 80,
+      str_repeat($zhe, 300) => 161,
+      str_repeat($snowman, 300) => 240,
+      str_repeat($boar, 300) => 255,
+    );
+
+    foreach ($map as $input => $expect) {
+      $actual = PhabricatorRepositoryCommitData::summarizeCommitMessage(
+        $input);
+      $this->assertEqual($expect, strlen($actual));
+    }
+
+  }
+
+}

src/infrastructure/storage/lisk/LiskDAO.php

@@ -1831,7 +1831,6 @@ abstract class LiskDAO extends Phobject {
     return $this->getConfigOption(self::CONFIG_BINARY);
   }
 
-
   public function getSchemaColumns() {
     $custom_map = $this->getConfigOption(self::CONFIG_COLUMN_SCHEMA);
     if (!$custom_map) {
@@ -1953,4 +1952,21 @@ abstract class LiskDAO extends Phobject {
     return $custom_map + $default_map;
   }
 
+  public function getColumnMaximumByteLength($column) {
+    $map = $this->getSchemaColumns();
+
+    if (!isset($map[$column])) {
+      throw new Exception(
+        pht(
+          'Object (of class "%s") does not have a column "%s".',
+          get_class($this),
+          $column));
+    }
+
+    $data_type = $map[$column];
+
+    return id(new PhabricatorStorageSchemaSpec())
+      ->getMaximumByteLengthForDataType($data_type);
+  }
+
 }