Fixes for updating indices, charset/collation and engine type on plugin-created tables.

Under MySQL, new tables will be created as InnoDB with UTF-8 (utf8/utf8_bin) same as core tables.
Existing plugin tables will have table engine and default charset/collation updated, and string columns will have charset updated, at checkschema time.

Switched from 'DESCRIBE' to INFORMATION_SCHEMA for pulling column information in order to get charset. A second hit to INFORMATION_SCHEMA is also needed to get table properties.

Indices were only being created at table creation time, which ain't so hot. Now also adding/dropping indices when they change.

Fixed up some schema defs in OStatus plugin that were a bit flaky, causing extra alter tables to be run.

TODO: Generalize this infrastructure a bit more up to base schema & pg schema classes.

5 files changed, 209 insertions, 42 deletions

diff --git a/lib/mysqlschema.php b/lib/mysqlschema.php
index 485096ac4..455695366 100644
--- a/lib/mysqlschema.php
+++ b/lib/mysqlschema.php
@@ -90,15 +90,24 @@ class MysqlSchema extends Schema
      * @param string $name Name of the table to get
      *
      * @return TableDef tabledef for that table.
+     * @throws SchemaTableMissingException
      */
 
     public function getTableDef($name)
     {
-        $res = $this->conn->query('DESCRIBE ' . $name);
+        $query = "SELECT * FROM INFORMATION_SCHEMA.COLUMNS " .
+                 "WHERE TABLE_SCHEMA='%s' AND TABLE_NAME='%s'";
+        $schema = $this->conn->dsn['database'];
+        $sql = sprintf($query, $schema, $name);
+        $res = $this->conn->query($sql);
 
         if (PEAR::isError($res)) {
             throw new Exception($res->getMessage());
         }
+        if ($res->numRows() == 0) {
+            $res->free();
+            throw new SchemaTableMissingException("No such table: $name");
+        }
 
         $td = new TableDef();
 
@@ -111,9 +120,9 @@ class MysqlSchema extends Schema
 
             $cd = new ColumnDef();
 
-            $cd->name = $row['Field'];
+            $cd->name = $row['COLUMN_NAME'];
 
-            $packed = $row['Type'];
+            $packed = $row['COLUMN_TYPE'];
 
             if (preg_match('/^(\w+)\((\d+)\)$/', $packed, $match)) {
                 $cd->type = $match[1];
@@ -122,18 +131,58 @@ class MysqlSchema extends Schema
                 $cd->type = $packed;
             }
 
-            $cd->nullable = ($row['Null'] == 'YES') ? true : false;
-            $cd->key      = $row['Key'];
-            $cd->default  = $row['Default'];
-            $cd->extra    = $row['Extra'];
+            $cd->nullable = ($row['IS_NULLABLE'] == 'YES') ? true : false;
+            $cd->key      = $row['COLUMN_KEY'];
+            $cd->default  = $row['COLUMN_DEFAULT'];
+            $cd->extra    = $row['EXTRA'];
+
+            // Autoincrement is stuck into the extra column.
+            // Pull it out so we don't accidentally mod it every time...
+            $extra = preg_replace('/(^|\s)auto_increment(\s|$)/i', '$1$2', $cd->extra);
+            if ($extra != $cd->extra) {
+                $cd->extra = trim($extra);
+                $cd->auto_increment = true;
+            }
+
+            // mysql extensions -- not (yet) used by base class
+            $cd->charset  = $row['CHARACTER_SET_NAME'];
+            $cd->collate  = $row['COLLATION_NAME'];
 
             $td->columns[] = $cd;
         }
+        $res->free();
 
         return $td;
     }
 
     /**
+     * Pull the given table properties from INFORMATION_SCHEMA.
+     * Most of the good stuff is MySQL extensions.
+     *
+     * @return array
+     * @throws Exception if table info can't be looked up
+     */
+
+    function getTableProperties($table, $props)
+    {
+        $query = "SELECT %s FROM INFORMATION_SCHEMA.TABLES " .
+                 "WHERE TABLE_SCHEMA='%s' AND TABLE_NAME='%s'";
+        $schema = $this->conn->dsn['database'];
+        $sql = sprintf($query, implode(',', $props), $schema, $table);
+        $res = $this->conn->query($sql);
+
+        $row = array();
+        $ok = $res->fetchInto($row, DB_FETCHMODE_ASSOC);
+        $res->free();
+        
+        if ($ok) {
+            return $row;
+        } else {
+            throw new SchemaTableMissingException("No such table: $table");
+        }
+    }
+
+    /**
      * Gets a ColumnDef object for a single column.
      *
      * Throws an exception if the table is not found.
@@ -185,35 +234,26 @@ class MysqlSchema extends Schema
             }
 
             $sql .= $this->_columnSql($cd);
-
-            switch ($cd->key) {
-            case 'UNI':
-                $uniques[] = $cd->name;
-                break;
-            case 'PRI':
-                $primary[] = $cd->name;
-                break;
-            case 'MUL':
-                $indices[] = $cd->name;
-                break;
-            }
         }
 
-        if (count($primary) > 0) { // it really should be...
-            $sql .= ",\nconstraint primary key (" . implode(',', $primary) . ")";
+        $idx = $this->_indexList($columns);
+
+        if ($idx['primary']) {
+            $sql .= ",\nconstraint primary key (" . implode(',', $idx['primary']) . ")";
         }
 
-        foreach ($uniques as $u) {
-            $sql .= ",\nunique index {$name}_{$u}_idx ($u)";
+        foreach ($idx['uniques'] as $u) {
+            $key = $this->_uniqueKey($name, $u);
+            $sql .= ",\nunique index $key ($u)";
         }
 
-        foreach ($indices as $i) {
-            $sql .= ",\nindex {$name}_{$i}_idx ($i)";
+        foreach ($idx['indices'] as $i) {
+            $key = $this->_key($name, $i);
+            $sql .= ",\nindex $key ($i)";
         }
 
-        $sql .= "); ";
+        $sql .= ") ENGINE=InnoDB CHARACTER SET utf8 COLLATE utf8_bin; ";
 
-        common_log(LOG_INFO, $sql);
         $res = $this->conn->query($sql);
 
         if (PEAR::isError($res)) {
@@ -224,6 +264,47 @@ class MysqlSchema extends Schema
     }
 
     /**
+     * Look over a list of column definitions and list up which
+     * indices will be present
+     */
+    private function _indexList(array $columns)
+    {
+        $list = array('uniques' => array(),
+                      'primary' => array(),
+                      'indices' => array());
+        foreach ($columns as $cd) {
+            switch ($cd->key) {
+            case 'UNI':
+                $list['uniques'][] = $cd->name;
+                break;
+            case 'PRI':
+                $list['primary'][] = $cd->name;
+                break;
+            case 'MUL':
+                $list['indices'][] = $cd->name;
+                break;
+            }
+        }
+        return $list;
+    }
+
+    /**
+     * Get the unique index key name for a given column on this table
+     */
+    function _uniqueKey($tableName, $columnName)
+    {
+        return $this->_key($tableName, $columnName);
+    }
+
+    /**
+     * Get the index key name for a given column on this table
+     */
+    function _key($tableName, $columnName)
+    {
+        return "{$tableName}_{$columnName}_idx";
+    }
+
+    /**
      * Drops a table from the schema
      *
      * Throws an exception if the table is not found.
@@ -394,21 +475,20 @@ class MysqlSchema extends Schema
 
         try {
             $td = $this->getTableDef($tableName);
-        } catch (Exception $e) {
-            if (preg_match('/no such table/', $e->getMessage())) {
-                return $this->createTable($tableName, $columns);
-            } else {
-                throw $e;
-            }
+        } catch (SchemaTableMissingException $e) {
+            return $this->createTable($tableName, $columns);
         }
 
         $cur = $this->_names($td->columns);
         $new = $this->_names($columns);
 
-        $toadd  = array_diff($new, $cur);
-        $todrop = array_diff($cur, $new);
-        $same   = array_intersect($new, $cur);
-        $tomod  = array();
+        $dropIndex  = array();
+        $toadd      = array_diff($new, $cur);
+        $todrop     = array_diff($cur, $new);
+        $same       = array_intersect($new, $cur);
+        $tomod      = array();
+        $addIndex   = array();
+        $tableProps = array();
 
         foreach ($same as $m) {
             $curCol = $this->_byName($td->columns, $m);
@@ -416,10 +496,64 @@ class MysqlSchema extends Schema
 
             if (!$newCol->equals($curCol)) {
                 $tomod[] = $newCol->name;
+                continue;
+            }
+
+            // Earlier versions may have accidentally left tables at default
+            // charsets which might be latin1 or other freakish things.
+            if ($this->_isString($curCol)) {
+                if ($curCol->charset != 'utf8') {
+                    $tomod[] = $newCol->name;
+                    continue;
+                }
+            }
+        }
+
+        // Find any indices we have to change...
+        $curIdx = $this->_indexList($td->columns);
+        $newIdx = $this->_indexList($columns);
+
+        if ($curIdx['primary'] != $newIdx['primary']) {
+            if ($curIdx['primary']) {
+                $dropIndex[] = 'drop primary key';
+            }
+            if ($newIdx['primary']) {
+                $keys = implode(',', $newIdx['primary']);
+                $addIndex[] = "add constraint primary key ($keys)";
             }
         }
 
-        if (count($toadd) + count($todrop) + count($tomod) == 0) {
+        $dropUnique = array_diff($curIdx['uniques'], $newIdx['uniques']);
+        $addUnique = array_diff($newIdx['uniques'], $curIdx['uniques']);
+        foreach ($dropUnique as $columnName) {
+            $dropIndex[] = 'drop key ' . $this->_uniqueKey($tableName, $columnName);
+        }
+        foreach ($addUnique as $columnName) {
+            $addIndex[] = 'add constraint unique key ' . $this->_uniqueKey($tableName, $columnName) . " ($columnName)";;
+        }
+
+        $dropMultiple = array_diff($curIdx['indices'], $newIdx['indices']);
+        $addMultiple = array_diff($newIdx['indices'], $curIdx['indices']);
+        foreach ($dropMultiple as $columnName) {
+            $dropIndex[] = 'drop key ' . $this->_key($tableName, $columnName);
+        }
+        foreach ($addMultiple as $columnName) {
+            $addIndex[] = 'add key ' . $this->_key($tableName, $columnName) . " ($columnName)";
+        }
+
+        // Check for table properties: make sure we're using a sane
+        // engine type and charset/collation.
+        // @fixme make the default engine configurable?
+        $oldProps = $this->getTableProperties($tableName, array('ENGINE', 'TABLE_COLLATION'));
+        if (strtolower($oldProps['ENGINE']) != 'innodb') {
+            $tableProps['ENGINE'] = 'InnoDB';
+        }
+        if (strtolower($oldProps['TABLE_COLLATION']) != 'utf8_bin') {
+            $tableProps['DEFAULT CHARSET'] = 'utf8';
+            $tableProps['COLLATE'] = 'utf8_bin';
+        }
+
+        if (count($dropIndex) + count($toadd) + count($todrop) + count($tomod) + count($addIndex) + count($tableProps) == 0) {
             // nothing to do
             return true;
         }
@@ -429,6 +563,10 @@ class MysqlSchema extends Schema
 
         $phrase = array();
 
+        foreach ($dropIndex as $indexSql) {
+            $phrase[] = $indexSql;
+        }
+
         foreach ($toadd as $columnName) {
             $cd = $this->_byName($columns, $columnName);
 
@@ -445,8 +583,17 @@ class MysqlSchema extends Schema
             $phrase[] = 'MODIFY COLUMN ' . $this->_columnSql($cd);
         }
 
+        foreach ($addIndex as $indexSql) {
+            $phrase[] = $indexSql;
+        }
+
+        foreach ($tableProps as $key => $val) {
+            $phrase[] = "$key=$val";
+        }
+
         $sql = 'ALTER TABLE ' . $tableName . ' ' . implode(', ', $phrase);
 
+        common_log(LOG_DEBUG, __METHOD__ . ': ' . $sql);
         $res = $this->conn->query($sql);
 
         if (PEAR::isError($res)) {
@@ -519,6 +666,10 @@ class MysqlSchema extends Schema
             $sql .= "{$cd->type} ";
         }
 
+        if ($this->_isString($cd)) {
+            $sql .= " CHARACTER SET utf8 ";
+        }
+
         if (!empty($cd->default)) {
             $sql .= "default {$cd->default} ";
         } else {
@@ -535,4 +686,13 @@ class MysqlSchema extends Schema
 
         return $sql;
     }
+
+    /**
+     * Is this column a string type?
+     */
+    private function _isString(ColumnDef $cd)
+    {
+        $strings = array('char', 'varchar', 'text');
+        return in_array(strtolower($cd->type), $strings);
+    }
 }
diff --git a/lib/schema.php b/lib/schema.php
index 137b814e0..1503c96d4 100644
--- a/lib/schema.php
+++ b/lib/schema.php
@@ -485,3 +485,9 @@ class Schema
         return $sql;
     }
 }
+
+class SchemaTableMissingException extends Exception
+{
+    // no-op
+}
+
diff --git a/plugins/OStatus/classes/FeedSub.php b/plugins/OStatus/classes/FeedSub.php
index b848b6b1d..80ba37bc1 100644
--- a/plugins/OStatus/classes/FeedSub.php
+++ b/plugins/OStatus/classes/FeedSub.php
@@ -110,7 +110,7 @@ class FeedSub extends Memcached_DataObject
                                    /*size*/ null,
                                    /*nullable*/ false,
                                    /*key*/ 'PRI',
-                                   /*default*/ '0',
+                                   /*default*/ null,
                                    /*extra*/ null,
                                    /*auto_increment*/ true),
                      new ColumnDef('uri', 'varchar',
@@ -450,3 +450,4 @@ class FeedSub extends Memcached_DataObject
     }
 
 }
+
diff --git a/plugins/OStatus/classes/HubSub.php b/plugins/OStatus/classes/HubSub.php
index c420b3eef..cdace3c1f 100644
--- a/plugins/OStatus/classes/HubSub.php
+++ b/plugins/OStatus/classes/HubSub.php
@@ -77,7 +77,7 @@ class HubSub extends Memcached_DataObject
                      new ColumnDef('topic', 'varchar',
                                    /*size*/255,
                                    /*nullable*/false,
-                                   /*key*/'KEY'),
+                                   /*key*/'MUL'),
                      new ColumnDef('callback', 'varchar',
                                    255, false),
                      new ColumnDef('secret', 'text',
diff --git a/plugins/OStatus/classes/Magicsig.php b/plugins/OStatus/classes/Magicsig.php
index 5a46aeeb6..b0a411e5d 100644
--- a/plugins/OStatus/classes/Magicsig.php
+++ b/plugins/OStatus/classes/Magicsig.php
@@ -70,7 +70,7 @@ class Magicsig extends Memcached_DataObject
     static function schemaDef()
     {
         return array(new ColumnDef('user_id', 'integer',
-                                   null, true, 'PRI'),
+                                   null, false, 'PRI'),
                      new ColumnDef('keypair', 'varchar',
                                    255, false),
                      new ColumnDef('alg', 'varchar',
@@ -230,4 +230,4 @@ function base64_url_encode($input)
 function base64_url_decode($input)
 {
     return base64_decode(strtr($input, '-_', '+/'));
-}
\ No newline at end of file
+}