From deea2f01f5d285aa0692803b285ff7f46706f0e2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 15 Feb 2019 06:24:49 -0800 Subject: [PATCH] Allow unit tests to have arbitrarily long names (>255 characters) Summary: Depends on D20179. Ref T13088. See PHI351. See PHI1018. In various cases, unit tests names are 19 paths mashed together. This is probably not an ideal name, and the test harness should probably pick a better name, but if users are fine with it and don't want to do the work to summarize on their own, accept them. We may summarize with "..." in some cases depending on how this fares in the UI. The actual implementation is a separate "strings" table which is just ``. The unit message table can end up being mostly strings, so this should reduce storage requirements a bit. For now, I'm not forcing a migration: new writes use the new table, existing rows retain the data. I plan to provide a migration tool, recommend migration, then force migration eventually. Prior to that, I'm likely to move at least some other columns to use this table (e.g., lint names), since we have a lot of similar data (arbitrarily long user string constants that we are unlikely to need to search or filter). Test Plan: {F6213819} Reviewers: amckinley Reviewed By: amckinley Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13088 Differential Revision: https://secure.phabricator.com/D20180 --- .../20190215.harbor.01.stringindex.sql | 6 +++ .../20190215.harbor.02.stringcol.sql | 2 + src/__phutil_library_map__.php | 2 + .../HarbormasterBuildUnitMessageQuery.php | 31 +++++++++++ .../storage/HarbormasterString.php | 54 +++++++++++++++++++ .../build/HarbormasterBuildUnitMessage.php | 29 ++++++++++ 6 files changed, 124 insertions(+) create mode 100644 resources/sql/autopatches/20190215.harbor.01.stringindex.sql create mode 100644 resources/sql/autopatches/20190215.harbor.02.stringcol.sql create mode 100644 src/applications/harbormaster/storage/HarbormasterString.php diff --git a/resources/sql/autopatches/20190215.harbor.01.stringindex.sql b/resources/sql/autopatches/20190215.harbor.01.stringindex.sql new file mode 100644 index 0000000000..e94b240bab --- /dev/null +++ b/resources/sql/autopatches/20190215.harbor.01.stringindex.sql @@ -0,0 +1,6 @@ +CREATE TABLE {$NAMESPACE}_harbormaster.harbormaster_string ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + stringIndex BINARY(12) NOT NULL, + stringValue LONGTEXT NOT NULL, + UNIQUE KEY `key_string` (stringIndex) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20190215.harbor.02.stringcol.sql b/resources/sql/autopatches/20190215.harbor.02.stringcol.sql new file mode 100644 index 0000000000..acfdb0f869 --- /dev/null +++ b/resources/sql/autopatches/20190215.harbor.02.stringcol.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildunitmessage + ADD nameIndex BINARY(12) NOT NULL; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 94ba258f53..ea3bf7d32d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1441,6 +1441,7 @@ phutil_register_library_map(array( 'HarbormasterStepDeleteController' => 'applications/harbormaster/controller/HarbormasterStepDeleteController.php', 'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php', 'HarbormasterStepViewController' => 'applications/harbormaster/controller/HarbormasterStepViewController.php', + 'HarbormasterString' => 'applications/harbormaster/storage/HarbormasterString.php', 'HarbormasterTargetEngine' => 'applications/harbormaster/engine/HarbormasterTargetEngine.php', 'HarbormasterTargetSearchAPIMethod' => 'applications/harbormaster/conduit/HarbormasterTargetSearchAPIMethod.php', 'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php', @@ -7070,6 +7071,7 @@ phutil_register_library_map(array( 'HarbormasterStepDeleteController' => 'HarbormasterPlanController', 'HarbormasterStepEditController' => 'HarbormasterPlanController', 'HarbormasterStepViewController' => 'HarbormasterPlanController', + 'HarbormasterString' => 'HarbormasterDAO', 'HarbormasterTargetEngine' => 'Phobject', 'HarbormasterTargetSearchAPIMethod' => 'PhabricatorSearchEngineAPIMethod', 'HarbormasterTargetWorker' => 'HarbormasterWorker', diff --git a/src/applications/harbormaster/query/HarbormasterBuildUnitMessageQuery.php b/src/applications/harbormaster/query/HarbormasterBuildUnitMessageQuery.php index fbd33a5d95..f73016a29f 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildUnitMessageQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildUnitMessageQuery.php @@ -57,6 +57,37 @@ final class HarbormasterBuildUnitMessageQuery return $where; } + protected function didFilterPage(array $messages) { + $indexes = array(); + foreach ($messages as $message) { + $index = $message->getNameIndex(); + if (strlen($index)) { + $indexes[$index] = $index; + } + } + + if ($indexes) { + $map = HarbormasterString::newIndexMap($indexes); + + foreach ($messages as $message) { + $index = $message->getNameIndex(); + + if (!strlen($index)) { + continue; + } + + $name = idx($map, $index); + if ($name === null) { + $name = pht('Unknown Unit Message ("%s")', $index); + } + + $message->setName($name); + } + } + + return $messages; + } + public function getQueryApplicationClass() { return 'PhabricatorHarbormasterApplication'; } diff --git a/src/applications/harbormaster/storage/HarbormasterString.php b/src/applications/harbormaster/storage/HarbormasterString.php new file mode 100644 index 0000000000..7493e60e21 --- /dev/null +++ b/src/applications/harbormaster/storage/HarbormasterString.php @@ -0,0 +1,54 @@ + false, + self::CONFIG_COLUMN_SCHEMA => array( + 'stringIndex' => 'bytes12', + 'stringValue' => 'text', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_string' => array( + 'columns' => array('stringIndex'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + + public static function newIndex($string) { + $index = PhabricatorHash::digestForIndex($string); + + $table = new self(); + $conn = $table->establishConnection('w'); + + queryfx( + $conn, + 'INSERT IGNORE INTO %R (stringIndex, stringValue) VALUES (%s, %s)', + $table, + $index, + $string); + + return $index; + } + + public static function newIndexMap(array $indexes) { + $table = new self(); + $conn = $table->establishConnection('r'); + + $rows = queryfx_all( + $conn, + 'SELECT stringIndex, stringValue FROM %R WHERE stringIndex IN (%Ls)', + $table, + $indexes); + + return ipull($rows, 'stringValue', 'stringIndex'); + } + +} diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php index 1f5e5e1440..9e437efaba 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php @@ -8,6 +8,7 @@ final class HarbormasterBuildUnitMessage protected $engine; protected $namespace; protected $name; + protected $nameIndex; protected $result; protected $duration; protected $properties = array(); @@ -132,6 +133,7 @@ final class HarbormasterBuildUnitMessage 'engine' => 'text255', 'namespace' => 'text255', 'name' => 'text255', + 'nameIndex' => 'bytes12', 'result' => 'text32', 'duration' => 'double?', ), @@ -260,6 +262,33 @@ final class HarbormasterBuildUnitMessage return implode("\0", $parts); } + public function save() { + if ($this->nameIndex === null) { + $this->nameIndex = HarbormasterString::newIndex($this->getName()); + } + + // See T13088. While we're letting installs do online migrations to avoid + // downtime, don't populate the "name" column for new writes. New writes + // use the "HarbormasterString" table instead. + $old_name = $this->name; + $this->name = ''; + + $caught = null; + try { + $result = parent::save(); + } catch (Exception $ex) { + $caught = $ex; + } + + $this->name = $old_name; + + if ($caught) { + throw $caught; + } + + return $result; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */