From aa481dba570dc0f1491ecaaff1038abc617ae33a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 18 Sep 2014 08:25:34 -0700 Subject: [PATCH] Begin generating meaningful expected schemata Summary: Ref T1191. This lays some groundwork for generating the expected schemata, so we can compare them to the actual schemata and produce a meaningful diff. - In general, each application will subclass `PhabricatorConfigSchemaSpec` and provide a definition of the tables it expects. - This class has helper methods to mostly-automatically build table definitions for Lisk and (in the future) edges. - When building expected schema, we specify a "data type", like "epoch". This is the type of data the application stores in the column, from the application's point of view. The SchemaSpec converts this into the best avilable storage type: for example, "text" will translate to `utf8mb4` if it's availalbe, or `binary` if not. This gives us a layer of indirection to insulate us from craziness. Test Plan: See screenshots. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T1191 Differential Revision: https://secure.phabricator.com/D10497 --- src/__phutil_library_map__.php | 2 + .../PhabricatorConfigDatabaseController.php | 150 +++++++++++++++++- .../schema/PhabricatorConfigColumnSchema.php | 10 ++ .../schema/PhabricatorConfigSchemaQuery.php | 13 +- .../schema/PhabricatorConfigSchemaSpec.php | 134 +++++++++++++++- .../storage/DifferentialSchemaSpec.php | 10 ++ src/infrastructure/storage/lisk/LiskDAO.php | 73 +++++++++ 7 files changed, 378 insertions(+), 14 deletions(-) create mode 100644 src/applications/differential/storage/DifferentialSchemaSpec.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index eff1269544..2e036f89f9 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -336,6 +336,7 @@ phutil_register_library_map(array( 'DifferentialRevisionStatus' => 'applications/differential/constants/DifferentialRevisionStatus.php', 'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/DifferentialRevisionUpdateHistoryView.php', 'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php', + 'DifferentialSchemaSpec' => 'applications/differential/storage/DifferentialSchemaSpec.php', 'DifferentialSearchIndexer' => 'applications/differential/search/DifferentialSearchIndexer.php', 'DifferentialSetDiffPropertyConduitAPIMethod' => 'applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php', 'DifferentialStoredCustomField' => 'applications/differential/customfield/DifferentialStoredCustomField.php', @@ -3138,6 +3139,7 @@ phutil_register_library_map(array( 'DifferentialRevisionSearchEngine' => 'PhabricatorApplicationSearchEngine', 'DifferentialRevisionUpdateHistoryView' => 'AphrontView', 'DifferentialRevisionViewController' => 'DifferentialController', + 'DifferentialSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'DifferentialSearchIndexer' => 'PhabricatorSearchDocumentIndexer', 'DifferentialSetDiffPropertyConduitAPIMethod' => 'DifferentialConduitAPIMethod', 'DifferentialStoredCustomField' => 'DifferentialCustomField', diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseController.php b/src/applications/config/controller/PhabricatorConfigDatabaseController.php index fb93c35259..4cd02ae126 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseController.php @@ -77,7 +77,15 @@ final class PhabricatorConfigDatabaseController $crumbs->addTextCrumb( $this->database, $this->getApplicationURI('database/'.$this->database.'/')); - $crumbs->addTextCrumb($this->table); + if ($this->column) { + $crumbs->addTextCrumb( + $this->table, + $this->getApplicationURI( + 'database/'.$this->database.'/'.$this->table.'/')); + $crumbs->addTextCrumb($this->column); + } else { + $crumbs->addTextCrumb($this->table); + } } else { $crumbs->addTextCrumb($this->database); } @@ -169,6 +177,8 @@ final class PhabricatorConfigDatabaseController PhabricatorConfigServerSchema $actual, $database_name) { + $collation_issue = PhabricatorConfigStorageSchema::ISSUE_COLLATION; + $database = $comp->getDatabase($database_name); if (!$database) { return new Aphront404Response(); @@ -176,9 +186,7 @@ final class PhabricatorConfigDatabaseController $rows = array(); foreach ($database->getTables() as $table_name => $table) { - $status = $table->getStatus(); - $issues = $table->getIssues(); $rows[] = array( $this->renderIcon($status), @@ -189,7 +197,9 @@ final class PhabricatorConfigDatabaseController '/database/'.$database_name.'/'.$table_name.'/'), ), $table_name), - $table->getCollation(), + $this->renderAttr( + $table->getCollation(), + $table->hasIssue($collation_issue)), ); } @@ -263,6 +273,10 @@ final class PhabricatorConfigDatabaseController $database_name, $table_name) { + $type_issue = PhabricatorConfigStorageSchema::ISSUE_COLUMNTYPE; + $charset_issue = PhabricatorConfigStorageSchema::ISSUE_CHARSET; + $collation_issue = PhabricatorConfigStorageSchema::ISSUE_COLLATION; + $database = $comp->getDatabase($database_name); if (!$database) { return new Aphront404Response(); @@ -273,10 +287,32 @@ final class PhabricatorConfigDatabaseController return new Aphront404Response(); } + $actual_database = $actual->getDatabase($database_name); + $actual_table = null; + if ($actual_database) { + $actual_table = $actual_database->getTable($table_name); + } + + $expect_database = $expect->getDatabase($database_name); + $expect_table = null; + if ($expect_database) { + $expect_table = $expect_database->getTable($table_name); + } + $rows = array(); foreach ($table->getColumns() as $column_name => $column) { + $expect_column = null; + if ($expect_table) { + $expect_column = $expect_table->getColumn($column_name); + } + $status = $column->getStatus(); + $data_type = null; + if ($expect_column) { + $data_type = $expect_column->getDataType(); + } + $rows[] = array( $this->renderIcon($status), phutil_tag( @@ -289,9 +325,16 @@ final class PhabricatorConfigDatabaseController $column_name.'/'), ), $column_name), - $column->getColumnType(), - $column->getCharacterSet(), - $column->getCollation(), + $data_type, + $this->renderAttr( + $column->getColumnType(), + $column->hasIssue($type_issue)), + $this->renderAttr( + $column->getCharacterSet(), + $column->hasIssue($charset_issue)), + $this->renderAttr( + $column->getCollation(), + $column->hasIssue($collation_issue)), ); } @@ -300,6 +343,7 @@ final class PhabricatorConfigDatabaseController array( null, pht('Table'), + pht('Data Type'), pht('Column Type'), pht('Character Set'), pht('Collation'), @@ -310,13 +354,34 @@ final class PhabricatorConfigDatabaseController 'wide pri', null, null, + null, null )); $title = pht('Database Status: %s.%s', $database_name, $table_name); + if ($actual_table) { + $actual_collation = $actual_table->getCollation(); + } else { + $actual_collation = null; + } + + if ($expect_table) { + $expect_collation = $expect_table->getCollation(); + } else { + $expect_collation = null; + } + $properties = $this->buildProperties( array( + array( + pht('Collation'), + $actual_collation, + ), + array( + pht('Expected Collation'), + $expect_collation, + ), ), $table->getIssues()); @@ -351,6 +416,49 @@ final class PhabricatorConfigDatabaseController return new Aphront404Response(); } + $actual_database = $actual->getDatabase($database_name); + $actual_table = null; + $actual_column = null; + if ($actual_database) { + $actual_table = $actual_database->getTable($table_name); + if ($actual_table) { + $actual_column = $actual_table->getColumn($column_name); + } + } + + $expect_database = $expect->getDatabase($database_name); + $expect_table = null; + $expect_column = null; + if ($expect_database) { + $expect_table = $expect_database->getTable($table_name); + if ($expect_table) { + $expect_column = $expect_table->getColumn($column_name); + } + } + + if ($actual_column) { + $actual_coltype = $actual_column->getColumnType(); + $actual_charset = $actual_column->getCharacterSet(); + $actual_collation = $actual_column->getCollation(); + } else { + $actual_coltype = null; + $actual_charset = null; + $actual_collation = null; + } + + if ($expect_column) { + $data_type = $expect_column->getDataType(); + $expect_coltype = $expect_column->getColumnType(); + $expect_charset = $expect_column->getCharacterSet(); + $expect_collation = $expect_column->getCollation(); + } else { + $data_type = null; + $expect_coltype = null; + $expect_charset = null; + $expect_collation = null; + } + + $title = pht( 'Database Status: %s.%s.%s', $database_name, @@ -359,6 +467,34 @@ final class PhabricatorConfigDatabaseController $properties = $this->buildProperties( array( + array( + pht('Data Type'), + $data_type, + ), + array( + pht('Column Type'), + $actual_coltype, + ), + array( + pht('Expected Column Type'), + $expect_coltype, + ), + array( + pht('Character Set'), + $actual_charset, + ), + array( + pht('Expected Character Set'), + $expect_charset, + ), + array( + pht('Collation'), + $actual_collation, + ), + array( + pht('Expected Collation'), + $expect_collation, + ), ), $column->getIssues()); diff --git a/src/applications/config/schema/PhabricatorConfigColumnSchema.php b/src/applications/config/schema/PhabricatorConfigColumnSchema.php index 0a7a7d5fe1..26fc58070d 100644 --- a/src/applications/config/schema/PhabricatorConfigColumnSchema.php +++ b/src/applications/config/schema/PhabricatorConfigColumnSchema.php @@ -6,6 +6,7 @@ final class PhabricatorConfigColumnSchema private $characterSet; private $collation; private $columnType; + private $dataType; public function setColumnType($column_type) { $this->columnType = $column_type; @@ -20,6 +21,15 @@ final class PhabricatorConfigColumnSchema return array(); } + public function setDataType($data_type) { + $this->dataType = $data_type; + return $this; + } + + public function getDataType() { + return $this->dataType; + } + public function setCollation($collation) { $this->collation = $collation; return $this; diff --git a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php index e08db0d959..2e193dc120 100644 --- a/src/applications/config/schema/PhabricatorConfigSchemaQuery.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaQuery.php @@ -102,7 +102,7 @@ final class PhabricatorConfigSchemaQuery extends Phobject { // collation. This is most correct, and will sort properly. $utf8_charset = 'utf8mb4'; - $utf8_collate = 'utf8mb4_unicode_ci'; + $utf8_collation = 'utf8mb4_unicode_ci'; } else { // If utf8mb4 is not available, we use binary. This allows us to store // 4-byte unicode characters. This has some tradeoffs: @@ -115,7 +115,7 @@ final class PhabricatorConfigSchemaQuery extends Phobject { // to prevent this. $utf8_charset = 'binary'; - $utf8_collate = 'binary'; + $utf8_collation = 'binary'; } $specs = id(new PhutilSymbolLoader()) @@ -124,10 +124,11 @@ final class PhabricatorConfigSchemaQuery extends Phobject { $server_schema = new PhabricatorConfigServerSchema(); foreach ($specs as $spec) { - $spec->setUTF8Charset($utf8_charset); - $spec->setUTF8Collate($utf8_collate); - - $spec->buildSchemata($server_schema); + $spec + ->setUTF8Collation($utf8_collation) + ->setUTF8Charset($utf8_charset) + ->setServer($server_schema) + ->buildSchemata($server_schema); } return $server_schema; diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php index 2f2adff5fe..f2adbbe7c3 100644 --- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php +++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php @@ -2,6 +2,138 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject { - abstract public function buildSchemata(PhabricatorConfigServerSchema $server); + private $server; + private $utf8Charset; + private $utf8Collation; + + public function setUTF8Collation($utf8_collation) { + $this->utf8Collation = $utf8_collation; + return $this; + } + + public function getUTF8Collation() { + return $this->utf8Collation; + } + + public function setUTF8Charset($utf8_charset) { + $this->utf8Charset = $utf8_charset; + return $this; + } + + public function getUTF8Charset() { + return $this->utf8Charset; + } + + public function setServer(PhabricatorConfigServerSchema $server) { + $this->server = $server; + return $this; + } + + public function getServer() { + return $this->server; + } + + abstract public function buildSchemata(); + + protected function buildLiskSchemata($base) { + + $objects = id(new PhutilSymbolLoader()) + ->setAncestorClass($base) + ->loadObjects(); + + foreach ($objects as $object) { + $database = $this->getDatabase($object->getApplicationName()); + + $table = $this->newTable($object->getTableName()); + + $cols = $object->getSchemaColumns(); + foreach ($cols as $name => $type) { + $details = $this->getDetailsForDataType($type); + list($column_type, $charset, $collation) = $details; + + $column = $this->newColumn($name) + ->setDataType($type) + ->setColumnType($column_type) + ->setCharacterSet($charset) + ->setCollation($collation); + + $table->addColumn($column); + } + + $database->addTable($table); + } + } + + protected function buildEdgeSchemata(PhabricatorLiskDAO $object) {} + + protected function getDatabase($name) { + $server = $this->getServer(); + + $database = $server->getDatabase($this->getNamespacedDatabase($name)); + if (!$database) { + $database = $this->newDatabase($name); + $server->addDatabase($database); + } + + return $database; + } + + protected function newDatabase($name) { + return id(new PhabricatorConfigDatabaseSchema()) + ->setName($this->getNamespacedDatabase($name)) + ->setCharacterSet($this->getUTF8Charset()) + ->setCollation($this->getUTF8Collation()); + } + + protected function getNamespacedDatabase($name) { + $namespace = PhabricatorLiskDAO::getStorageNamespace(); + return $namespace.'_'.$name; + } + + protected function newTable($name) { + return id(new PhabricatorConfigTableSchema()) + ->setName($name) + ->setCollation($this->getUTF8Collation()); + } + + protected function newColumn($name) { + return id(new PhabricatorConfigColumnSchema()) + ->setName($name); + } + + private function getDetailsForDataType($data_type) { + $column_type = null; + $charset = null; + $collation = null; + + switch ($data_type) { + case 'id': + case 'epoch': + $column_type = 'int(10) unsigned'; + break; + case 'phid': + $column_type = 'varchar(64)'; + $charset = 'binary'; + $collation = 'binary'; + break; + case 'blob': + $column_type = 'longblob'; + $charset = 'binary'; + $collation = 'binary'; + break; + case 'text': + $column_type = 'longtext'; + $charset = $this->getUTF8Charset(); + $collation = $this->getUTF8Collation(); + break; + default: + $column_type = pht(''); + $charset = pht(''); + $collation = pht(''); + break; + } + + return array($column_type, $charset, $collation); + } } diff --git a/src/applications/differential/storage/DifferentialSchemaSpec.php b/src/applications/differential/storage/DifferentialSchemaSpec.php new file mode 100644 index 0000000000..e568da0a45 --- /dev/null +++ b/src/applications/differential/storage/DifferentialSchemaSpec.php @@ -0,0 +1,10 @@ +buildLiskSchemata('DifferentialDAO'); +// $this->addEdgeSchemata($server, new DifferentialRevision()); + } + +} diff --git a/src/infrastructure/storage/lisk/LiskDAO.php b/src/infrastructure/storage/lisk/LiskDAO.php index d4deac02c3..6b74cb5656 100644 --- a/src/infrastructure/storage/lisk/LiskDAO.php +++ b/src/infrastructure/storage/lisk/LiskDAO.php @@ -169,6 +169,7 @@ abstract class LiskDAO { const CONFIG_AUX_PHID = 'auxiliary-phid'; const CONFIG_SERIALIZATION = 'col-serialization'; const CONFIG_BINARY = 'binary'; + const CONFIG_COLUMN_SCHEMA = 'col-schema'; const SERIALIZATION_NONE = 'id'; const SERIALIZATION_JSON = 'json'; @@ -343,6 +344,9 @@ abstract class LiskDAO { * they store binary data. These columns will not raise an error when * handling binary writes. * + * CONFIG_COLUMN_SCHEMA + * Provide a map of columns to schema column types. + * * @return dictionary Map of configuration options to values. * * @task config @@ -1712,4 +1716,73 @@ abstract class LiskDAO { return $this->getConfigOption(self::CONFIG_BINARY); } + + public function getSchemaColumns() { + $custom_map = $this->getConfigOption(self::CONFIG_COLUMN_SCHEMA); + if (!$custom_map) { + $custom_map = array(); + } + + $serialization = $this->getConfigOption(self::CONFIG_SERIALIZATION); + if (!$serialization) { + $serialization = array(); + } + + $serialization_map = array( + self::SERIALIZATION_JSON => 'text', + self::SERIALIZATION_PHP => 'blob', + ); + + $builtin = array( + 'id' => 'id', + 'phid' => 'phid', + 'dateCreated' => 'epoch', + 'dateModified' => 'epoch', + ); + + $map = array(); + foreach ($this->getAllLiskProperties() as $property) { + // First, use types specified explicitly in the table configuration. + $type = idx($custom_map, $property); + if ($type) { + $map[$property] = $type; + continue; + } + + // If we don't have an explicit type, try a builtin type for the + // column. + $type = idx($builtin, $property); + if ($type) { + $map[$property] = $type; + continue; + } + + // If the column has serialization, we can infer the column type. + if (isset($serialization[$property])) { + $type = idx($serialization_map, $serialization[$property]); + if ($type) { + $map[$property] = $type; + continue; + } + } + + // If the column is named `somethingPHID`, infer it is a PHID. + if (preg_match('/[a-z]PHID$/', $property)) { + $map[$property] = 'phid'; + continue; + } + + // If the column is named `somethingID`, infer it is an ID. + if (preg_match('/[a-z]ID$/', $property)) { + $map[$property] = 'id'; + continue; + } + + // We don't know the type of this column. + $map[$property] = null; + } + + return $map; + } + }