From 76f2fcdedbcac02d3068910de4650ccba298160b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20S=CC=8Ckoda?= Date: Sun, 27 Jan 2013 12:19:29 +0100 Subject: [PATCH] MDL-37726 stop using PREVIOUS/NEXT in install.xml files --- config-dist.php | 11 +-- lib/ddl/tests/ddl_test.php | 18 ----- lib/ddl/tests/fixtures/invalid.xml | 2 +- lib/xmldb/xmldb.xsd | 4 + lib/xmldb/xmldb_field.php | 14 ---- lib/xmldb/xmldb_index.php | 14 ---- lib/xmldb/xmldb_key.php | 14 ---- lib/xmldb/xmldb_object.php | 121 +---------------------------- lib/xmldb/xmldb_structure.php | 7 +- lib/xmldb/xmldb_table.php | 35 +-------- 10 files changed, 13 insertions(+), 227 deletions(-) diff --git a/config-dist.php b/config-dist.php index 3cd7b3ecd23..88a2c5b274f 100644 --- a/config-dist.php +++ b/config-dist.php @@ -498,17 +498,8 @@ $CFG->admin = 'admin'; // Divert all outgoing emails to this address to test and debug emailing features // $CFG->divertallemailsto = 'root@localhost.local'; // NOT FOR PRODUCTION SERVERS! // -// special magic evil developer only wanting to edit the xmldb files manually -// AND don't use the XMLDBEditor nor the prev/next stuff at all (Mahara and others) -// Uncomment these if you're lazy like Penny +// Uncomment if you want to allow empty comments when modifying install.xml files. // $CFG->xmldbdisablecommentchecking = true; // NOT FOR PRODUCTION SERVERS! -// $CFG->xmldbdisablenextprevchecking = true; // NOT FOR PRODUCTION SERVERS! -// -// Special magic - evil developer only wanting to edit xmldb files manually -// AND allowing the XMLDBEditor to reconstruct the prev/next elements every -// time one file is loaded and saved (Moodle). -// Uncomment this if you're lazy like Petr -// $CFG->xmldbreconstructprevnext = true; // NOT FOR PRODUCTION SERVERS! // // Since 2.0 sql queries are not shown during upgrade by default. // Please note that this setting may produce very long upgrade page on large sites. diff --git a/lib/ddl/tests/ddl_test.php b/lib/ddl/tests/ddl_test.php index 56eac3320ba..5157d2b4ea0 100644 --- a/lib/ddl/tests/ddl_test.php +++ b/lib/ddl/tests/ddl_test.php @@ -1400,12 +1400,6 @@ class ddl_testcase extends database_driver_testcase { $this->assertTrue($e instanceof moodle_exception); } - // Real file but invalid xml file - $devhack = false; - if (!empty($CFG->xmldbdisablenextprevchecking)) { - $CFG->xmldbdisablenextprevchecking = false; - $devhack = true; - } try { $dbman->delete_tables_from_xmldb_file(__DIR__ . '/fixtures/invalid.xml'); $this->assertTrue(false); @@ -1413,9 +1407,6 @@ class ddl_testcase extends database_driver_testcase { $this->resetDebugging(); $this->assertTrue($e instanceof moodle_exception); } - if ($devhack) { - $CFG->xmldbdisablenextprevchecking = true; - } // Check that the table has not been deleted from DB $this->assertTrue($dbman->table_exists('test_table1')); @@ -1441,12 +1432,6 @@ class ddl_testcase extends database_driver_testcase { $this->assertTrue($e instanceof moodle_exception); } - // Real but invalid xml file - $devhack = false; - if (!empty($CFG->xmldbdisablenextprevchecking)) { - $CFG->xmldbdisablenextprevchecking = false; - $devhack = true; - } try { $dbman->install_from_xmldb_file(__DIR__ . '/fixtures/invalid.xml'); $this->assertTrue(false); @@ -1454,9 +1439,6 @@ class ddl_testcase extends database_driver_testcase { $this->resetDebugging(); $this->assertTrue($e instanceof moodle_exception); } - if ($devhack) { - $CFG->xmldbdisablenextprevchecking = true; - } // Check that the table has not yet been created in DB $this->assertFalse($dbman->table_exists('test_table1')); diff --git a/lib/ddl/tests/fixtures/invalid.xml b/lib/ddl/tests/fixtures/invalid.xml index 7fb07653852..0f5b5fd28a3 100644 --- a/lib/ddl/tests/fixtures/invalid.xml +++ b/lib/ddl/tests/fixtures/invalid.xml @@ -3,7 +3,7 @@ - + diff --git a/lib/xmldb/xmldb.xsd b/lib/xmldb/xmldb.xsd index d144882f06c..5d119a6b3ee 100644 --- a/lib/xmldb/xmldb.xsd +++ b/lib/xmldb/xmldb.xsd @@ -67,6 +67,7 @@ + @@ -87,6 +88,7 @@ + @@ -108,6 +110,7 @@ + @@ -144,6 +147,7 @@ + diff --git a/lib/xmldb/xmldb_field.php b/lib/xmldb/xmldb_field.php index 7c69407e5a6..0a5062adade 100644 --- a/lib/xmldb/xmldb_field.php +++ b/lib/xmldb/xmldb_field.php @@ -394,14 +394,6 @@ class xmldb_field extends xmldb_object { $this->comment = trim($xmlarr['@']['COMMENT']); } - if (isset($xmlarr['@']['PREVIOUS'])) { - $this->previous = trim($xmlarr['@']['PREVIOUS']); - } - - if (isset($xmlarr['@']['NEXT'])) { - $this->next = trim($xmlarr['@']['NEXT']); - } - // Set some attributes if ($result) { $this->loaded = true; @@ -533,12 +525,6 @@ class xmldb_field extends xmldb_object { if ($this->comment) { $o.= ' COMMENT="' . htmlspecialchars($this->comment) . '"'; } - if ($this->previous) { - $o.= ' PREVIOUS="' . $this->previous . '"'; - } - if ($this->next) { - $o.= ' NEXT="' . $this->next . '"'; - } $o.= '/>' . "\n"; return $o; diff --git a/lib/xmldb/xmldb_index.php b/lib/xmldb/xmldb_index.php index 8e80e4c8a84..e8c82d8f733 100644 --- a/lib/xmldb/xmldb_index.php +++ b/lib/xmldb/xmldb_index.php @@ -209,14 +209,6 @@ class xmldb_index extends xmldb_object { $this->comment = trim($xmlarr['@']['COMMENT']); } - if (isset($xmlarr['@']['PREVIOUS'])) { - $this->previous = trim($xmlarr['@']['PREVIOUS']); - } - - if (isset($xmlarr['@']['NEXT'])) { - $this->next = trim($xmlarr['@']['NEXT']); - } - // Set some attributes if ($result) { $this->loaded = true; @@ -258,12 +250,6 @@ class xmldb_index extends xmldb_object { if ($this->comment) { $o.= ' COMMENT="' . htmlspecialchars($this->comment) . '"'; } - if ($this->previous) { - $o.= ' PREVIOUS="' . $this->previous . '"'; - } - if ($this->next) { - $o.= ' NEXT="' . $this->next . '"'; - } $o.= '/>' . "\n"; return $o; diff --git a/lib/xmldb/xmldb_key.php b/lib/xmldb/xmldb_key.php index 26ad368b49e..0d47aa036d5 100644 --- a/lib/xmldb/xmldb_key.php +++ b/lib/xmldb/xmldb_key.php @@ -271,14 +271,6 @@ class xmldb_key extends xmldb_object { $this->comment = trim($xmlarr['@']['COMMENT']); } - if (isset($xmlarr['@']['PREVIOUS'])) { - $this->previous = trim($xmlarr['@']['PREVIOUS']); - } - - if (isset($xmlarr['@']['NEXT'])) { - $this->next = trim($xmlarr['@']['NEXT']); - } - // Set some attributes if ($result) { $this->loaded = true; @@ -384,12 +376,6 @@ class xmldb_key extends xmldb_object { if ($this->comment) { $o.= ' COMMENT="' . htmlspecialchars($this->comment) . '"'; } - if ($this->previous) { - $o.= ' PREVIOUS="' . $this->previous . '"'; - } - if ($this->next) { - $o.= ' NEXT="' . $this->next . '"'; - } $o.= '/>' . "\n"; return $o; diff --git a/lib/xmldb/xmldb_object.php b/lib/xmldb/xmldb_object.php index 334610f8b0c..a104ec0a60a 100644 --- a/lib/xmldb/xmldb_object.php +++ b/lib/xmldb/xmldb_object.php @@ -239,11 +239,6 @@ class xmldb_object { * @return bool true if $arr modified */ public function fixPrevNext(&$arr) { - global $CFG; - - if (empty($CFG->xmldbreconstructprevnext)) { - return false; - } $tweaked = false; $prev = null; @@ -267,113 +262,6 @@ class xmldb_object { return $tweaked; } - /** - * This function will check that all the elements in one array - * have a consistent info in their previous/next fields - * @param array $arr - * @return bool true means ok, false invalid prev/next present - */ - public function checkPreviousNextValues($arr) { - global $CFG; - if (!empty($CFG->xmldbdisablenextprevchecking)) { - return true; - } - $result = true; - // Check that only one element has the previous not set - if ($arr) { - $counter = 0; - foreach($arr as $element) { - if (!$element->getPrevious()) { - $counter++; - } - } - if ($counter != 1) { - debugging('The number of objects with previous not set is different from 1', DEBUG_DEVELOPER); - $result = false; - } - } - // Check that only one element has the next not set - if ($result && $arr) { - $counter = 0; - foreach($arr as $element) { - if (!$element->getNext()) { - $counter++; - } - } - if ($counter != 1) { - debugging('The number of objects with next not set is different from 1', DEBUG_DEVELOPER); - $result = false; - } - } - // Check that all the previous elements are existing elements - if ($result && $arr) { - foreach($arr as $element) { - if ($element->getPrevious()) { - $i = $this->findObjectInArray($element->getPrevious(), $arr); - if ($i === null) { - debugging('Object ' . $element->getName() . ' says PREVIOUS="' . $element->getPrevious() . '" but that other object does not exist.', DEBUG_DEVELOPER); - $result = false; - } - } - } - } - // Check that all the next elements are existing elements - if ($result && $arr) { - foreach($arr as $element) { - if ($element->getNext()) { - $i = $this->findObjectInArray($element->getNext(), $arr); - if ($i === null) { - debugging('Object ' . $element->getName() . ' says NEXT="' . $element->getNext() . '" but that other object does not exist.', DEBUG_DEVELOPER); - $result = false; - } - } - } - } - // Check that there aren't duplicates in the previous values - if ($result && $arr) { - $existarr = array(); - foreach($arr as $element) { - if (in_array($element->getPrevious(), $existarr)) { - $result = false; - debugging('Object ' . $element->getName() . ' says PREVIOUS="' . $element->getPrevious() . '" but another object has already said that!', DEBUG_DEVELOPER); - } else { - $existarr[] = $element->getPrevious(); - } - } - } - // Check that there aren't duplicates in the next values - if ($result && $arr) { - $existarr = array(); - foreach($arr as $element) { - if (in_array($element->getNext(), $existarr)) { - $result = false; - debugging('Object ' . $element->getName() . ' says NEXT="' . $element->getNext() . '" but another object has already said that!', DEBUG_DEVELOPER); - } else { - $existarr[] = $element->getNext(); - } - } - } - // Check that there aren't next values pointing to themselves - if ($result && $arr) { - foreach($arr as $element) { - if ($element->getNext() == $element->getName()) { - $result = false; - debugging('Object ' . $element->getName() . ' says NEXT="' . $element->getNext() . '" and that is wrongly recursive!', DEBUG_DEVELOPER); - } - } - } - // Check that there aren't prev values pointing to themselves - if ($result && $arr) { - foreach($arr as $element) { - if ($element->getPrevious() == $element->getName()) { - $result = false; - debugging('Object ' . $element->getName() . ' says PREVIOUS="' . $element->getPrevious() . '" and that is wrongly recursive!', DEBUG_DEVELOPER); - } - } - } - return $result; - } - /** * This function will order all the elements in one array, following * the previous/next rules @@ -381,11 +269,8 @@ class xmldb_object { * @return array|bool */ public function orderElements($arr) { - global $CFG; $result = true; - if (!empty($CFG->xmldbdisablenextprevchecking)) { - return $arr; - } + // Create a new array $newarr = array(); if (!empty($arr)) { @@ -411,9 +296,7 @@ class xmldb_object { // Compare number of elements between original and new array if ($result && count($arr) != count($newarr)) { $result = false; - } - // Check that previous/next is ok (redundant but...) - if ($this->checkPreviousNextValues($newarr)) { + } else if ($newarr) { $result = $newarr; } else { $result = false; diff --git a/lib/xmldb/xmldb_structure.php b/lib/xmldb/xmldb_structure.php index 678a7d6e7c3..beeef751720 100644 --- a/lib/xmldb/xmldb_structure.php +++ b/lib/xmldb/xmldb_structure.php @@ -278,13 +278,8 @@ class xmldb_structure extends xmldb_object { $this->debug($this->errormsg); $result = false; } - // Check previous & next are ok (duplicates and existing tables) + // Compute prev/next. $this->fixPrevNext($this->tables); - if ($result && !$this->checkPreviousNextValues($this->tables)) { - $this->errormsg = 'Some TABLES previous/next values are incorrect'; - $this->debug($this->errormsg); - $result = false; - } // Order tables if ($result && !$this->orderTables($this->tables)) { $this->errormsg = 'Error ordering the tables'; diff --git a/lib/xmldb/xmldb_table.php b/lib/xmldb/xmldb_table.php index c69c2a1c6f2..758c58f25d2 100644 --- a/lib/xmldb/xmldb_table.php +++ b/lib/xmldb/xmldb_table.php @@ -511,12 +511,6 @@ class xmldb_table extends xmldb_object { $this->debug($this->errormsg); $result = false; } - if (isset($xmlarr['@']['PREVIOUS'])) { - $this->previous = trim($xmlarr['@']['PREVIOUS']); - } - if (isset($xmlarr['@']['NEXT'])) { - $this->next = trim($xmlarr['@']['NEXT']); - } // Iterate over fields if (isset($xmlarr['#']['FIELDS']['0']['#']['FIELD'])) { @@ -548,13 +542,8 @@ class xmldb_table extends xmldb_object { $this->debug($this->errormsg); $result = false; } - // Check previous & next are ok (duplicates and existing fields) + // Compute prev/next. $this->fixPrevNext($this->fields); - if ($result && !$this->checkPreviousNextValues($this->fields)) { - $this->errormsg = 'Some FIELDS previous/next values are incorrect'; - $this->debug($this->errormsg); - $result = false; - } // Order fields if ($result && !$this->orderFields($this->fields)) { $this->errormsg = 'Error ordering the fields'; @@ -593,13 +582,8 @@ class xmldb_table extends xmldb_object { $this->debug($this->errormsg); $result = false; } - // Check previous & next are ok (duplicates and existing keys) + // Compute prev/next. $this->fixPrevNext($this->keys); - if ($result && !$this->checkPreviousNextValues($this->keys)) { - $this->errormsg = 'Some KEYS previous/next values are incorrect'; - $this->debug($this->errormsg); - $result = false; - } // Order keys if ($result && !$this->orderKeys($this->keys)) { $this->errormsg = 'Error ordering the keys'; @@ -637,13 +621,8 @@ class xmldb_table extends xmldb_object { $this->debug($this->errormsg); $result = false; } - // Check previous & next are ok (duplicates and existing INDEXES) + // Compute prev/next. $this->fixPrevNext($this->indexes); - if ($result && !$this->checkPreviousNextValues($this->indexes)) { - $this->errormsg = 'Some INDEXES previous/next values are incorrect'; - $this->debug($this->errormsg); - $result = false; - } // Order indexes if ($result && !$this->orderIndexes($this->indexes)) { $this->errormsg = 'Error ordering the indexes'; @@ -734,13 +713,7 @@ class xmldb_table extends xmldb_object { if ($this->comment) { $o.= ' COMMENT="' . htmlspecialchars($this->comment) . '"'; } - if ($this->previous) { - $o.= ' PREVIOUS="' . $this->previous . '"'; - } - if ($this->next) { - $o.= ' NEXT="' . $this->next . '"'; - } - $o.= '>' . "\n"; + $o.= '>' . "\n"; // Now the fields if ($this->fields) { $o.= ' ' . "\n";