Merge branch 'MDL-67886_master' of git://github.com/mdjnelson/moodle
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Wed, 22 Apr 2020 23:36:40 +0000 (01:36 +0200)
committerAdrian Greeve <abgreeve@gmail.com>
Thu, 23 Apr 2020 07:01:40 +0000 (15:01 +0800)
admin/tool/xmldb/actions/check_indexes/check_indexes.class.php
admin/tool/xmldb/lang/en/tool_xmldb.php
lib/ddl/database_manager.php
lib/ddl/tests/ddl_test.php
lib/dtl/database_exporter.php
lib/dtl/database_importer.php
lib/upgrade.txt

index ba03bac..b7c175c 100644 (file)
@@ -44,11 +44,13 @@ class check_indexes extends XMLDBCheckAction {
 
         // Get needed strings
         $this->loadStrings(array(
+            'extraindexesfound' => 'tool_xmldb',
             'missing' => 'tool_xmldb',
             'key' => 'tool_xmldb',
             'index' => 'tool_xmldb',
             'missingindexes' => 'tool_xmldb',
-            'nomissingindexesfound' => 'tool_xmldb',
+            'nomissingorextraindexesfound' => 'tool_xmldb',
+            'yesextraindexesfound' => 'tool_xmldb',
             'yesmissingindexesfound' => 'tool_xmldb',
         ));
     }
@@ -58,6 +60,7 @@ class check_indexes extends XMLDBCheckAction {
         $dbman = $DB->get_manager();
 
         $o = '';
+        $dbindexes = $DB->get_indexes($xmldb_table->getName());
         $missing_indexes = array();
 
         // Keys
@@ -88,6 +91,7 @@ class check_indexes extends XMLDBCheckAction {
                     // Check if the index exists in DB
                     if ($dbman->index_exists($xmldb_table, $xmldb_index)) {
                         $o.='<font color="green">' . $this->str['ok'] . '</font>';
+                        $this->remove_index_from_dbindex($dbindexes, $xmldb_index);
                     } else {
                         $o.='<font color="red">' . $this->str['missing'] . '</font>';
                         // Add the missing index to the list
@@ -109,6 +113,7 @@ class check_indexes extends XMLDBCheckAction {
                 // Check if the index exists in DB
                 if ($dbman->index_exists($xmldb_table, $xmldb_index)) {
                     $o.='<font color="green">' . $this->str['ok'] . '</font>';
+                    $this->remove_index_from_dbindex($dbindexes, $xmldb_index);
                 } else {
                     $o.='<font color="red">' . $this->str['missing'] . '</font>';
                     // Add the missing index to the list
@@ -122,6 +127,14 @@ class check_indexes extends XMLDBCheckAction {
             $o.='        </ul>';
         }
 
+        // Hack - skip for table 'search_simpledb_index' as this plugin adds indexes dynamically on install
+        // which are not included in install.xml. See search/engine/simpledb/db/install.php.
+        if ($xmldb_table->getName() != 'search_simpledb_index') {
+            foreach ($dbindexes as $indexname => $index) {
+                $missing_indexes[] = $indexname;
+            }
+        }
+
         return array($o, $missing_indexes);
     }
 
@@ -129,33 +142,56 @@ class check_indexes extends XMLDBCheckAction {
         global $DB;
         $dbman = $DB->get_manager();
 
+        $missingindexes = [];
+        $extraindexes = [];
+
+        foreach ($missing_indexes as $missingindex) {
+            if (is_object($missingindex)) {
+                $missingindexes[] = $missingindex;
+            } else {
+                $extraindexes[] = $missingindex;
+            }
+        }
+
         $s = '';
         $r = '<table class="generaltable boxaligncenter boxwidthwide" border="0" cellpadding="5" cellspacing="0" id="results">';
         $r.= '  <tr><td class="generalboxcontent">';
         $r.= '    <h2 class="main">' . $this->str['searchresults'] . '</h2>';
-        $r.= '    <p class="centerpara">' . $this->str['missingindexes'] . ': ' . count($missing_indexes) . '</p>';
+        $r .= '    <p class="centerpara">' . $this->str['missingindexes'] . ': ' . count($missingindexes) . '</p>';
+        $r .= '    <p class="centerpara">' . $this->str['extraindexesfound'] . ': ' . count($extraindexes) . '</p>';
         $r.= '  </td></tr>';
         $r.= '  <tr><td class="generalboxcontent">';
 
-        // If we have found missing indexes inform about them
-        if (count($missing_indexes)) {
-            $r.= '    <p class="centerpara">' . $this->str['yesmissingindexesfound'] . '</p>';
-            $r.= '        <ul>';
-            foreach ($missing_indexes as $obj) {
-                $xmldb_table = $obj->table;
-                $xmldb_index = $obj->index;
-                $sqlarr = $dbman->generator->getAddIndexSQL($xmldb_table, $xmldb_index);
-                $r.= '            <li>' . $this->str['table'] . ': ' . $xmldb_table->getName() . '. ' .
-                                          $this->str['index'] . ': ' . $xmldb_index->readableInfo() . '</li>';
-                $sqlarr = $dbman->generator->getEndedStatements($sqlarr);
-                $s.= '<code>' . str_replace("\n", '<br />', implode('<br />', $sqlarr)) . '</code><br />';
+        // If we have found missing indexes or extra indexes inform the user about them.
+        if (!empty($missingindexes) || !empty($extraindexes)) {
+            if ($missingindexes) {
+                $r.= '    <p class="centerpara">' . $this->str['yesmissingindexesfound'] . '</p>';
+                $r.= '        <ul>';
+                foreach ($missingindexes as $obj) {
+                    $xmldb_table = $obj->table;
+                    $xmldb_index = $obj->index;
+                    $sqlarr = $dbman->generator->getAddIndexSQL($xmldb_table, $xmldb_index);
+                    $r.= '            <li>' . $this->str['table'] . ': ' . $xmldb_table->getName() . '. ' .
+                                              $this->str['index'] . ': ' . $xmldb_index->readableInfo() . '</li>';
+                    $sqlarr = $dbman->generator->getEndedStatements($sqlarr);
+                    $s.= '<code>' . str_replace("\n", '<br />', implode('<br />', $sqlarr)) . '</code><br />';
 
+                }
+                $r.= '        </ul>';
+                // Add the SQL statements (all together)
+                $r.= '<hr />' . $s;
+            }
+            if ($extraindexes) {
+                $r .= '<p class="centerpara">' . $this->str['yesextraindexesfound'] . '</p>';
+                $r .= '<ul>';
+                foreach ($extraindexes as $ei) {
+                    $r .= '<li>' . $ei . '</li>';
+                }
+                $r .= '</ul>';
+                $r .= '<hr />';
             }
-            $r.= '        </ul>';
-            // Add the SQL statements (all together)
-            $r.= '<hr />' . $s;
         } else {
-            $r.= '    <p class="centerpara">' . $this->str['nomissingindexesfound'] . '</p>';
+            $r .= '<p class="centerpara">' . $this->str['nomissingorextraindexesfound'] . '</p>';
         }
         $r.= '  </td></tr>';
         $r.= '  <tr><td class="generalboxcontent">';
@@ -166,4 +202,18 @@ class check_indexes extends XMLDBCheckAction {
 
         return $r;
     }
+
+    /**
+     * Removes an index from the array $dbindexes if it is found.
+     *
+     * @param array $dbindexes
+     * @param xmldb_index $index
+     */
+    private function remove_index_from_dbindex(array &$dbindexes, xmldb_index $index) {
+        foreach ($dbindexes as $key => $dbindex) {
+            if ($dbindex['columns'] == $index->getFields()) {
+                unset($dbindexes[$key]);
+            }
+        }
+    }
 }
index c465ed1..77ababc 100644 (file)
@@ -99,6 +99,7 @@ $string['edit_xml_file'] = 'Edit XML file';
 $string['enumvaluesincorrect'] = 'Incorrect values for enum field';
 $string['expected'] = 'Expected';
 $string['extensionrequired'] = 'Sorry - the PHP extension \'{$a}\' is required for this action. Please install the extension if you want to use this feature.';
+$string['extraindexesfound'] = 'Extra indexes found';
 $string['field'] = 'Field';
 $string['fieldnameempty'] = 'Name field empty';
 $string['fields'] = 'Fields';
@@ -157,7 +158,7 @@ $string['newtablefrommysql'] = 'New table from MySQL';
 $string['new_table_from_mysql'] = 'New table from MySQL';
 $string['nofieldsspecified'] = 'No fields specified';
 $string['nomasterprimaryuniquefound'] = 'The column(s) that your foreign key references must be included in a primary or unique KEY in the referenced table. Note that the column being in a UNIQUE INDEX is not good enough.';
-$string['nomissingindexesfound'] = 'No missing indexes have been found, your DB doesn\'t need further actions.';
+$string['nomissingorextraindexesfound'] = 'No missing or extra indexes have been found, your DB doesn\'t need further actions.';
 $string['noreffieldsspecified'] = 'No reference fields specified';
 $string['noreftablespecified'] = 'Specified reference table not found';
 $string['noviolatedforeignkeysfound'] = 'No violated foreign keys found';
@@ -215,6 +216,7 @@ $string['wronglengthforenum'] = 'Incorrect length for enum field';
 $string['wrongnumberofreffields'] = 'Wrong number of reference fields';
 $string['wrongreservedwords'] = 'Currently used reserved words<br />(note that table names aren\'t important if using $CFG->prefix)';
 $string['wrongoraclesemantics'] = 'Wrong Oracle BYTE semantics found';
+$string['yesextraindexesfound'] = 'The following additional indexes were found.';
 $string['yesmissingindexesfound'] = '<p>Some missing indexes have been found in your DB. Here are their details and the needed SQL statements to be executed with your favourite SQL interface to create all of them. Remember to backup your data first!</p>
 <p>After doing that, it\'s highly recommended to execute this utility again to check that no more missing indexes are found.</p>';
 $string['yeswrongdefaultsfound'] = '<p>Some inconsistent defaults have been found in your DB. Here are their details and the needed SQL statements to be executed with your favourite SQL interface to fix them all. Remember to backup your data first!</p>
index c74b839..7181a52 100644 (file)
@@ -962,6 +962,8 @@ class database_manager {
             'extracolumns' => true,
             'missingcolumns' => true,
             'changedcolumns' => true,
+            'missingindexes' => true,
+            'extraindexes' => true
         );
 
         $typesmap = array(
@@ -1001,6 +1003,7 @@ class database_manager {
 
             /** @var database_column_info[] $dbfields */
             $dbfields = $this->mdb->get_columns($tablename, false);
+            $dbindexes = $this->mdb->get_indexes($tablename);
             /** @var xmldb_field[] $fields */
             $fields = $table->getFields();
 
@@ -1096,6 +1099,61 @@ class database_manager {
                 unset($dbfields[$fieldname]);
             }
 
+            // Check for missing indexes/keys.
+            if ($options['missingindexes']) {
+                // Check the foreign keys.
+                if ($keys = $table->getKeys()) {
+                    foreach ($keys as $key) {
+                        // Primary keys are skipped.
+                        if ($key->getType() == XMLDB_KEY_PRIMARY) {
+                            continue;
+                        }
+
+                        $keyname = $key->getName();
+
+                        // Create the interim index.
+                        $index = new xmldb_index('anyname');
+                        $index->setFields($key->getFields());
+                        switch ($key->getType()) {
+                            case XMLDB_KEY_UNIQUE:
+                            case XMLDB_KEY_FOREIGN_UNIQUE:
+                                $index->setUnique(true);
+                                break;
+                            case XMLDB_KEY_FOREIGN:
+                                $index->setUnique(false);
+                                break;
+                        }
+                        if (!$this->index_exists($table, $index)) {
+                            $errors[$tablename][] = $this->get_missing_index_error($table, $index, $keyname);
+                        } else {
+                            $this->remove_index_from_dbindex($dbindexes, $index);
+                        }
+                    }
+                }
+
+                // Check the indexes.
+                if ($indexes = $table->getIndexes()) {
+                    foreach ($indexes as $index) {
+                        if (!$this->index_exists($table, $index)) {
+                            $errors[$tablename][] = $this->get_missing_index_error($table, $index, $index->getName());
+                        } else {
+                            $this->remove_index_from_dbindex($dbindexes, $index);
+                        }
+                    }
+                }
+            }
+
+            // Check if we should show the extra indexes.
+            if ($options['extraindexes']) {
+                // Hack - skip for table 'search_simpledb_index' as this plugin adds indexes dynamically on install
+                // which are not included in install.xml. See search/engine/simpledb/db/install.php.
+                if ($tablename != 'search_simpledb_index') {
+                    foreach ($dbindexes as $indexname => $index) {
+                        $errors[$tablename][] = "Unexpected index '$indexname'.";
+                    }
+                }
+            }
+
             // Check for extra columns (indicates unsupported hacks) - modify install.xml if you want to pass validation.
             foreach ($dbfields as $fieldname => $dbfield) {
                 if ($options['extracolumns']) {
@@ -1127,4 +1185,34 @@ class database_manager {
 
         return $errors;
     }
+
+    /**
+     * Returns a string describing the missing index error.
+     *
+     * @param xmldb_table $table
+     * @param xmldb_index $index
+     * @param string $indexname
+     * @return string
+     */
+    private function get_missing_index_error(xmldb_table $table, xmldb_index $index, string $indexname): string {
+        $sqlarr = $this->generator->getAddIndexSQL($table, $index);
+        $sqlarr = $this->generator->getEndedStatements($sqlarr);
+        $sqltoadd = reset($sqlarr);
+
+        return "Missing index '" . $indexname . "' " . "(" . $index->readableInfo() . "). \n" . $sqltoadd;
+    }
+
+    /**
+     * Removes an index from the array $dbindexes if it is found.
+     *
+     * @param array $dbindexes
+     * @param xmldb_index $index
+     */
+    private function remove_index_from_dbindex(array &$dbindexes, xmldb_index $index) {
+        foreach ($dbindexes as $key => $dbindex) {
+            if ($dbindex['columns'] == $index->getFields()) {
+                unset($dbindexes[$key]);
+            }
+        }
+    }
 }
index 9ffeb10..9cee313 100644 (file)
@@ -2441,4 +2441,58 @@ class core_ddl_testcase extends database_driver_testcase {
         }
     */
 
+    /**
+     * Tests check_database_schema().
+     */
+    public function test_check_database_schema() {
+        global $CFG, $DB;
+
+        $dbmanager = $DB->get_manager();
+
+        // Create a table in the database we will be using to compare with a schema.
+        $table = new xmldb_table('test_check_db_schema');
+        $table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
+        $table->add_field('extracolumn', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
+        $table->add_field('courseid', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
+        $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
+        $table->add_key('extraindex', XMLDB_KEY_UNIQUE, array('extracolumn'));
+        $table->setComment("This is a test table, you can drop it safely.");
+        $dbmanager->create_table($table);
+
+        // Remove the column so it is not added to the schema and gets reported as an extra column.
+        $table->deleteField('extracolumn');
+
+        // Change the 'courseid' field to a float in the schema so it gets reported as different.
+        $table->deleteField('courseid');
+        $table->add_field('courseid', XMLDB_TYPE_NUMBER, '10, 2', null, XMLDB_NOTNULL, null, null);
+
+        // Add another column to the schema that won't be present in the database and gets reported as missing.
+        $table->add_field('missingcolumn', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, null);
+
+        // Add another key to the schema that won't be present in the database and gets reported as missing.
+        $table->add_key('missingkey', XMLDB_KEY_FOREIGN, array('courseid'), 'course', array('id'));
+
+        // Remove the key from the schema which will still be present in the database and reported as extra.
+        $table->deleteKey('extraindex');
+
+        $schema = new xmldb_structure('testschema');
+        $schema->addTable($table);
+
+        // Things we want to check for -
+        // 1. Changed columns.
+        // 2. Missing columns.
+        // 3. Missing indexes.
+        // 4. Unexpected index.
+        // 5. Extra columns.
+        $errors = $dbmanager->check_database_schema($schema)['test_check_db_schema'];
+        $strmissing = "Missing index 'missingkey' (not unique (courseid)). " . PHP_EOL .
+            "CREATE INDEX {$CFG->prefix}testchecdbsche_cou_ix ON {$CFG->prefix}test_check_db_schema (courseid);";
+
+        $this->assertCount(5, $errors);
+        $this->assertContains("column 'courseid' has incorrect type 'I', expected 'N'", $errors);
+        $this->assertContains("column 'missingcolumn' is missing", $errors);
+        $this->assertContains($strmissing, $errors);
+        $this->assertContains("Unexpected index '{$CFG->prefix}testchecdbsche_ext_uix'.", $errors);
+        $this->assertContains("column 'extracolumn' is not expected (I)", $errors);
+    }
 }
index 23677f6..2e53d55 100644 (file)
@@ -129,7 +129,11 @@ abstract class database_exporter {
     public function export_database($description=null) {
         global $CFG;
 
-        $options = array('changedcolumns' => false); // Column types may be fixed by transfer.
+        $options = [
+            'changedcolumns' => false, // Column types may be fixed by transfer.
+            'missingindexes' => false, // No need to worry about indexes for transfering data.
+            'extraindexes' => false
+        ];
         if ($this->check_schema and $errors = $this->manager->check_database_schema($this->schema, $options)) {
             $details = '';
             foreach ($errors as $table=>$items) {
index b14bad7..0306461 100644 (file)
@@ -110,7 +110,11 @@ class database_importer {
             throw new dbtransfer_exception('importversionmismatchexception', $a);
         }
 
-        $options = array('changedcolumns' => false); // Column types may be fixed by transfer.
+        $options = [
+            'changedcolumns' => false, // Column types may be fixed by transfer.
+            'missingindexes' => false, // No need to worry about indexes for transfering data.
+            'extraindexes' => false
+        ];
         if ($this->check_schema and $errors = $this->manager->check_database_schema($this->schema, $options)) {
             $details = '';
             foreach ($errors as $table=>$items) {
index 4a71a5a..f9908eb 100644 (file)
@@ -42,6 +42,7 @@ information provided here is intended especially for developers.
   define('READ_ONLY_SESSION', true); Note - this also requires $CFG->enable_read_only_sessions to be set to true.
 * External functions can be called without requiring a session lock if they define 'readonlysession' => true in
   db/services.php. Note - this also requires $CFG->enable_read_only_sessions to be set to true.
+* database_manager::check_database_schema() now checks for missing and extra indexes.
 
 === 3.8 ===
 * Add CLI option to notify all cron tasks to stop: admin/cli/cron.php --stop