Page MenuHomePhabricator

D16385.diff
No OneTemporary

D16385.diff

diff --git a/resources/sql/autopatches/20160810.commit.01.summarylength.sql b/resources/sql/autopatches/20160810.commit.01.summarylength.sql
new file mode 100644
--- /dev/null
+++ b/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};
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3381,6 +3381,7 @@
'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 @@
'PhabricatorRepositoryCommitPHIDType' => 'PhabricatorPHIDType',
'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker',
'PhabricatorRepositoryCommitRef' => 'Phobject',
+ 'PhabricatorRepositoryCommitTestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorRepositoryDAO' => 'PhabricatorLiskDAO',
'PhabricatorRepositoryDiscoveryEngine' => 'PhabricatorRepositoryEngine',
diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
--- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
+++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
@@ -70,7 +70,12 @@
}
$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 @@
->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 @@
'text255' => true,
'text160' => true,
'text128' => true,
- 'text80' => true,
'text64' => true,
'text40' => true,
'text32' => true,
@@ -237,6 +247,10 @@
$type = $matches[1];
$size = idx($matches, 2);
+ if ($size) {
+ $bytes = $size;
+ }
+
switch ($type) {
case 'text':
if ($is_binary) {
@@ -363,7 +377,14 @@
}
}
- return array($column_type, $charset, $collation, $nullable, $auto);
+ return array(
+ 'type' => $column_type,
+ 'charset' => $charset,
+ 'collation' => $collation,
+ 'nullable' => $nullable,
+ 'auto' => $auto,
+ 'bytes' => $bytes,
+ );
}
}
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -108,7 +108,7 @@
'mailKey' => 'bytes20',
'authorPHID' => 'phid?',
'auditStatus' => 'uint32',
- 'summary' => 'text80',
+ 'summary' => 'text255',
'importStatus' => 'uint32',
),
self::CONFIG_KEY_SCHEMA => array(
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php b/src/applications/repository/storage/PhabricatorRepositoryCommitData.php
--- a/src/applications/repository/storage/PhabricatorRepositoryCommitData.php
+++ b/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 @@
}
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;
diff --git a/src/applications/repository/storage/__tests__/PhabricatorRepositoryCommitTestCase.php b/src/applications/repository/storage/__tests__/PhabricatorRepositoryCommitTestCase.php
new file mode 100644
--- /dev/null
+++ b/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));
+ }
+
+ }
+
+}
diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php
--- a/src/infrastructure/storage/lisk/LiskDAO.php
+++ b/src/infrastructure/storage/lisk/LiskDAO.php
@@ -1831,7 +1831,6 @@
return $this->getConfigOption(self::CONFIG_BINARY);
}
-
public function getSchemaColumns() {
$custom_map = $this->getConfigOption(self::CONFIG_COLUMN_SCHEMA);
if (!$custom_map) {
@@ -1953,4 +1952,21 @@
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);
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Wed, Jun 5, 8:26 AM (2 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6287964
Default Alt Text
D16385.diff (7 KB)

Event Timeline