Generate reasonable expected schemata for Audit and Auth

Summary: Ref T1191. This fills in some more features and gets audit and auth nearly generating reasonable expected schemata.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1191

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

src/__phutil_library_map__.php

@@ -1189,6 +1189,7 @@ phutil_register_library_map(array(
     'PhabricatorAuditManagementWorkflow' => 'applications/audit/management/PhabricatorAuditManagementWorkflow.php',
     'PhabricatorAuditPreviewController' => 'applications/audit/controller/PhabricatorAuditPreviewController.php',
     'PhabricatorAuditReplyHandler' => 'applications/audit/mail/PhabricatorAuditReplyHandler.php',
+    'PhabricatorAuditSchemaSpec' => 'applications/audit/storage/PhabricatorAuditSchemaSpec.php',
     'PhabricatorAuditStatusConstants' => 'applications/audit/constants/PhabricatorAuditStatusConstants.php',
     'PhabricatorAuditTransaction' => 'applications/audit/storage/PhabricatorAuditTransaction.php',
     'PhabricatorAuditTransactionComment' => 'applications/audit/storage/PhabricatorAuditTransactionComment.php',
@@ -1233,6 +1234,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthProviderConfigTransactionQuery' => 'applications/auth/query/PhabricatorAuthProviderConfigTransactionQuery.php',
     'PhabricatorAuthRegisterController' => 'applications/auth/controller/PhabricatorAuthRegisterController.php',
     'PhabricatorAuthRevokeTokenController' => 'applications/auth/controller/PhabricatorAuthRevokeTokenController.php',
+    'PhabricatorAuthSchemaSpec' => 'applications/auth/storage/PhabricatorAuthSchemaSpec.php',
     'PhabricatorAuthSession' => 'applications/auth/storage/PhabricatorAuthSession.php',
     'PhabricatorAuthSessionEngine' => 'applications/auth/engine/PhabricatorAuthSessionEngine.php',
     'PhabricatorAuthSessionGarbageCollector' => 'applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php',
@@ -4069,6 +4071,7 @@ phutil_register_library_map(array(
     'PhabricatorAuditManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'PhabricatorAuditPreviewController' => 'PhabricatorAuditController',
     'PhabricatorAuditReplyHandler' => 'PhabricatorMailReplyHandler',
+    'PhabricatorAuditSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'PhabricatorAuditTransaction' => 'PhabricatorApplicationTransaction',
     'PhabricatorAuditTransactionComment' => 'PhabricatorApplicationTransactionComment',
     'PhabricatorAuditTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
@@ -4113,6 +4116,7 @@ phutil_register_library_map(array(
     'PhabricatorAuthProviderConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'PhabricatorAuthRegisterController' => 'PhabricatorAuthController',
     'PhabricatorAuthRevokeTokenController' => 'PhabricatorAuthController',
+    'PhabricatorAuthSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'PhabricatorAuthSession' => array(
       'PhabricatorAuthDAO',
       'PhabricatorPolicyInterface',

src/applications/audit/storage/PhabricatorAuditSchemaSpec.php

@@ -0,0 +1,11 @@
+<?php
+
+final class PhabricatorAuditSchemaSpec extends PhabricatorConfigSchemaSpec {
+
+  public function buildSchemata() {
+    $this->buildTransactionSchema(
+      new PhabricatorAuditTransaction(),
+      new PhabricatorAuditTransactionComment());
+  }
+
+}

src/applications/audit/storage/PhabricatorAuditTransactionComment.php

@@ -22,4 +22,20 @@ final class PhabricatorAuditTransactionComment
     return ($this->getTransactionPHID() != null);
   }
 
+  public function getConfiguration() {
+    $config = parent::getConfiguration();
+    $config[self::CONFIG_COLUMN_SCHEMA] = array(
+      'commitPHID' => 'phid?',
+      'pathID' => 'id?',
+      'isNewFile' => 'bool',
+      'lineNumber' => 'uint32',
+      'lineLength' => 'uint32',
+      'fixedState' => 'text12?',
+      'hasReplies' => 'bool',
+      'replyToCommentPHID' => 'phid?',
+      'legacyCommentID' => 'id?',
+    ) + $config[self::CONFIG_COLUMN_SCHEMA];
+    return $config;
+  }
+
 }

src/applications/auth/storage/PhabricatorAuthFactorConfig.php

@@ -14,6 +14,11 @@ final class PhabricatorAuthFactorConfig extends PhabricatorAuthDAO {
         'properties' => self::SERIALIZATION_JSON,
       ),
       self::CONFIG_AUX_PHID => true,
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'factorKey' => 'text64',
+        'factorName' => 'text',
+        'factorSecret' => 'text',
+      ),
     ) + parent::getConfiguration();
   }
 

src/applications/auth/storage/PhabricatorAuthProviderConfig.php

@@ -29,6 +29,14 @@ final class PhabricatorAuthProviderConfig extends PhabricatorAuthDAO
       self::CONFIG_SERIALIZATION => array(
         'properties' => self::SERIALIZATION_JSON,
       ),
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'isEnabled' => 'bool',
+        'shouldAllowLogin' => 'bool',
+        'shouldAllowRegistration' => 'bool',
+        'shouldAllowLink' => 'bool',
+        'shouldAllowUnlink' => 'bool',
+        'shouldTrustEmails' => 'bool',
+      ),
     ) + parent::getConfiguration();
   }
 

src/applications/auth/storage/PhabricatorAuthSchemaSpec.php

@@ -0,0 +1,11 @@
+<?php
+
+final class PhabricatorAuthSchemaSpec extends PhabricatorConfigSchemaSpec {
+
+  public function buildSchemata() {
+    $this->buildLiskSchemata('PhabricatorAuthDAO');
+    $this->buildTransactionSchema(
+      new PhabricatorAuthProviderConfigTransaction());
+  }
+
+}

src/applications/auth/storage/PhabricatorAuthTemporaryToken.php

@@ -14,6 +14,16 @@ final class PhabricatorAuthTemporaryToken extends PhabricatorAuthDAO
   public function getConfiguration() {
     return array(
       self::CONFIG_TIMESTAMPS => false,
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'tokenType' => 'text64',
+        'tokenExpires' => 'epoch',
+        'tokenCode' => 'text64',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_token' => array(
+          'columns' => array('objectPHID', 'tokenType', 'tokenCode'),
+        ),
+      ),
     ) + parent::getConfiguration();
   }
 

src/applications/config/controller/PhabricatorConfigController.php

@@ -11,11 +11,14 @@ abstract class PhabricatorConfigController extends PhabricatorController {
 
     $nav = new AphrontSideNavFilterView();
     $nav->setBaseURI(new PhutilURI($this->getApplicationURI()));
-    $nav->addLabel(pht('Site Configuration'));
+    $nav->addLabel(pht('Configuration'));
-    $nav->addFilter('/', pht('Option Groups'));
+    $nav->addFilter('/', pht('Browse Settings'));
     $nav->addFilter('all/', pht('All Settings'));
+    $nav->addLabel(pht('Setup'));
     $nav->addFilter('issue/', pht('Setup Issues'));
+    $nav->addLabel(pht('Database'));
     $nav->addFilter('database/', pht('Database Status'));
+    $nav->addLabel(pht('Welcome'));
     $nav->addFilter('welcome/', pht('Welcome Screen'));
 
     return $nav;

src/applications/config/controller/PhabricatorConfigDatabaseController.php

@@ -525,10 +525,12 @@ final class PhabricatorConfigDatabaseController
       $actual_coltype = $actual_column->getColumnType();
       $actual_charset = $actual_column->getCharacterSet();
       $actual_collation = $actual_column->getCollation();
+      $actual_nullable = $actual_column->getNullable();
     } else {
       $actual_coltype = null;
       $actual_charset = null;
       $actual_collation = null;
+      $actual_nullable = null;
     }
 
     if ($expect_column) {
@@ -536,11 +538,13 @@ final class PhabricatorConfigDatabaseController
       $expect_coltype = $expect_column->getColumnType();
       $expect_charset = $expect_column->getCharacterSet();
       $expect_collation = $expect_column->getCollation();
+      $expect_nullable = $expect_column->getNullable();
     } else {
       $data_type = null;
       $expect_coltype = null;
       $expect_charset = null;
       $expect_collation = null;
+      $expect_nullable = null;
     }
 
 
@@ -580,6 +584,14 @@ final class PhabricatorConfigDatabaseController
           pht('Expected Collation'),
           $expect_collation,
         ),
+        array(
+          pht('Nullable'),
+          $this->getNullableString($actual_nullable),
+        ),
+        array(
+          pht('Expected Nullable'),
+          $this->getNullableString($expect_nullable),
+        ),
       ),
       $column->getIssues());
 
@@ -747,4 +759,14 @@ final class PhabricatorConfigDatabaseController
     return $view;
   }
 
+  private function getNullableString($value) {
+    if ($value === null) {
+      return '';
+    } else if ($value === true) {
+      return pht('Yes');
+    } else {
+      return pht('No');
+    }
+  }
+
 }

src/applications/config/schema/PhabricatorConfigColumnSchema.php

@@ -67,6 +67,11 @@ final class PhabricatorConfigColumnSchema
       return ((int)$matches[1]) * 4;
     }
 
+    switch ($type) {
+      case 'int(10) unsigned':
+        return 4;
+    }
+
     // TODO: Build this out to catch overlong indexes.
 
     return 0;
@@ -88,6 +93,10 @@ final class PhabricatorConfigColumnSchema
       $issues[] = self::ISSUE_COLUMNTYPE;
     }
 
+    if ($this->getNullable() !== $expect->getNullable()) {
+      $issues[] = self::ISSUE_NULLABLE;
+    }
+
     return $issues;
   }
 

src/applications/config/schema/PhabricatorConfigKeySchema.php

@@ -6,7 +6,7 @@ final class PhabricatorConfigKeySchema
   private $columnNames;
 
   public function setColumnNames(array $column_names) {
-    $this->columnNames = $column_names;
+    $this->columnNames = array_values($column_names);
     return $this;
   }
 

src/applications/config/schema/PhabricatorConfigSchemaSpec.php

@@ -42,28 +42,51 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       ->loadObjects();
 
     foreach ($objects as $object) {
-      $database = $this->getDatabase($object->getApplicationName());
+      $this->buildLiskObjectSchema($object);
-
-      $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 buildTransactionSchema(
+    PhabricatorApplicationTransaction $xaction,
+    PhabricatorApplicationTransactionComment $comment = null) {
+
+    $this->buildLiskObjectSchema($xaction);
+    if ($comment) {
+      $this->buildLiskObjectSchema($comment);
+    }
+  }
+
+  private function buildLiskObjectSchema(PhabricatorLiskDAO $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, $nullable) = $details;
+
+      $column = $this->newColumn($name)
+        ->setDataType($type)
+        ->setColumnType($column_type)
+        ->setCharacterSet($charset)
+        ->setCollation($collation)
+        ->setNullable($nullable);
+
+      $table->addColumn($column);
+    }
+
+    $keys = $object->getSchemaKeys();
+    foreach ($keys as $key_name => $key_spec) {
+      $key = $this->newKey($key_name)
+        ->setColumnNames(idx($key_spec, 'columns', array()));
+
+      $table->addKey($key);
+    }
+
+    $database->addTable($table);
+  }
+
   protected function buildEdgeSchemata(PhabricatorLiskDAO $object) {}
 
   protected function getDatabase($name) {
@@ -101,17 +124,31 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
       ->setName($name);
   }
 
+  protected function newKey($name) {
+    return id(new PhabricatorConfigKeySchema())
+      ->setName($name);
+  }
+
   private function getDetailsForDataType($data_type) {
     $column_type = null;
     $charset = null;
     $collation = null;
 
+    // If the type ends with "?", make the column nullable.
+    $nullable = false;
+    if (preg_match('/\?$/', $data_type)) {
+      $nullable = true;
+      $data_type = substr($data_type, 0, -1);
+    }
+
     switch ($data_type) {
       case 'id':
       case 'epoch':
+      case 'uint32':
         $column_type = 'int(10) unsigned';
         break;
       case 'phid':
+      case 'policy';
         $column_type = 'varchar(64)';
         $charset = 'binary';
         $collation = 'binary';
@@ -121,11 +158,34 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
         $charset = 'binary';
         $collation = 'binary';
         break;
+      case 'text128':
+        $column_type = 'varchar(128)';
+        $charset = $this->getUTF8Charset();
+        $collation = $this->getUTF8Collation();
+        break;
+      case 'text64':
+        $column_type = 'varchar(64)';
+        $charset = $this->getUTF8Charset();
+        $collation = $this->getUTF8Collation();
+        break;
+      case 'text32':
+        $column_type = 'varchar(32)';
+        $charset = $this->getUTF8Charset();
+        $collation = $this->getUTF8Collation();
+        break;
+      case 'text12':
+        $column_type = 'varchar(12)';
+        $charset = $this->getUTF8Charset();
+        $collation = $this->getUTF8Collation();
+        break;
       case 'text':
         $column_type = 'longtext';
         $charset = $this->getUTF8Charset();
         $collation = $this->getUTF8Collation();
         break;
+      case 'bool':
+        $column_type = 'tinyint(1)';
+        break;
       default:
         $column_type = pht('<unknown>');
         $charset = pht('<unknown>');
@@ -133,7 +193,7 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
         break;
     }
 
-    return array($column_type, $charset, $collation);
+    return array($column_type, $charset, $collation, $nullable);
   }
 
 }

src/applications/config/schema/PhabricatorConfigStorageSchema.php

@@ -95,9 +95,9 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
       case self::ISSUE_SURPLUS:
         return pht('Surplus');
       case self::ISSUE_CHARSET:
-        return pht('Wrong Character Set');
+        return pht('Better Character Set Available');
       case self::ISSUE_COLLATION:
-        return pht('Wrong Collation');
+        return pht('Better Collation Available');
       case self::ISSUE_COLUMNTYPE:
         return pht('Wrong Column Type');
       case self::ISSUE_NULLABLE:
@@ -149,6 +149,7 @@ abstract class PhabricatorConfigStorageSchema extends Phobject {
       case self::ISSUE_COLUMNTYPE:
       case self::ISSUE_SUBWARN:
       case self::ISSUE_KEYCOLUMNS:
+      case self::ISSUE_NULLABLE:
         return self::STATUS_WARN;
       default:
         throw new Exception(pht('Unknown schema issue "%s"!', $issue));

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -107,6 +107,12 @@ abstract class PhabricatorApplicationTransaction
         'newValue' => self::SERIALIZATION_JSON,
         'metadata' => self::SERIALIZATION_JSON,
       ),
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'commentPHID' => 'phid?',
+        'commentVersion' => 'uint32',
+        'contentSource' => 'text',
+        'transactionType' => 'text32',
+      ),
     ) + parent::getConfiguration();
   }
 

src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php

@@ -28,6 +28,18 @@ abstract class PhabricatorApplicationTransactionComment
   public function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
+      self::CONFIG_COLUMN_SCHEMA => array(
+        'transactionPHID' => 'phid?',
+        'commentVersion' => 'uint32',
+        'content' => 'text',
+        'contentSource' => 'text',
+        'isDeleted' => 'bool',
+      ),
+      self::CONFIG_KEY_SCHEMA => array(
+        'key_version' => array(
+          'columns' => array('transactionPHID', 'commentVersion'),
+        ),
+      ),
     ) + parent::getConfiguration();
   }
 

src/infrastructure/storage/lisk/LiskDAO.php

@@ -170,6 +170,7 @@ abstract class LiskDAO {
   const CONFIG_SERIALIZATION        = 'col-serialization';
   const CONFIG_BINARY               = 'binary';
   const CONFIG_COLUMN_SCHEMA        = 'col-schema';
+  const CONFIG_KEY_SCHEMA           = 'key-schema';
 
   const SERIALIZATION_NONE          = 'id';
   const SERIALIZATION_JSON          = 'json';
@@ -347,6 +348,9 @@ abstract class LiskDAO {
    * CONFIG_COLUMN_SCHEMA
    * Provide a map of columns to schema column types.
    *
+   * CONFIG_KEY_SCHEMA
+   * Provide a map of key names to key specifications.
+   *
    * @return dictionary  Map of configuration options to values.
    *
    * @task   config
@@ -1736,6 +1740,8 @@ abstract class LiskDAO {
     $builtin = array(
       'id' => 'id',
       'phid' => 'phid',
+      'viewPolicy' => 'policy',
+      'editPolicy' => 'policy',
       'dateCreated' => 'epoch',
       'dateModified' => 'epoch',
     );
@@ -1785,4 +1791,29 @@ abstract class LiskDAO {
     return $map;
   }
 
+  public function getSchemaKeys() {
+    $custom_map = $this->getConfigOption(self::CONFIG_KEY_SCHEMA);
+    if (!$custom_map) {
+      $custom_map = array();
+    }
+
+    $default_map = array();
+    foreach ($this->getAllLiskProperties() as $property) {
+      switch ($property) {
+        case 'id':
+          $default_map['PRIMARY'] = array(
+            'columns' => array('id'),
+          );
+          break;
+        case 'phid':
+          $default_map['key_phid'] = array(
+            'columns' => array('phid'),
+          );
+          break;
+      }
+    }
+
+    return $custom_map + $default_map;
+  }
+
 }