MDL-37726 stop using PREVIOUS/NEXT in install.xml files

diff --git a/config-dist.php b/config-dist.php
index 3cd7b3e..88a2c5b 100644 (file)
--- 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 56eac33..5157d2b 100644 (file)
--- 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 7fb0765..0f5b5fd 100644 (file)
--- a/lib/ddl/tests/fixtures/invalid.xml
+++ b/lib/ddl/tests/fixtures/invalid.xml
@@ -3,7 +3,7 @@
   <TABLES>
     <TABLE NAME="test_table1" COMMENT="Just a test table">
       <FIELDS>
-        <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true" NEXT="incorrect_next_field"/> <!-- invalid because of NEXT value -->
+        <FIELD NAME="id" LENGTH="10" NOTNULL="true" SEQUENCE="true" NEXT="incorrect_next_field"/> <!-- missing TYPE -->
         <FIELD NAME="course" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false" PREVIOUS="id" NEXT="name"/>
         <FIELD NAME="name" TYPE="char" LENGTH="30" NOTNULL="false" SEQUENCE="false" DEFAULT="Moodle" PREVIOUS="course" NEXT="secondname"/>
         <FIELD NAME="secondname" TYPE="char" LENGTH="30" NOTNULL="true" SEQUENCE="false" PREVIOUS="name" NEXT="intro"/>

diff --git a/lib/xmldb/xmldb.xsd b/lib/xmldb/xmldb.xsd
index d144882..5d119a6 100644 (file)
--- a/lib/xmldb/xmldb.xsd
+++ b/lib/xmldb/xmldb.xsd
@@ -67,6 +67,7 @@
       <xs:attribute name="UNSIGNED"   type="trueFalse"   use="optional" />
       <xs:attribute name="DEFAULT"    type="xs:string"   use="optional" />
       <xs:attribute name="COMMENT"    type="xs:string"   use="optional" />
+      <!-- TODO: Moodle 2.6 - Drop ignored PREVIOUS/NEXT attributes -->
       <xs:attribute name="PREVIOUS"   type="fieldName"   use="optional" />
       <xs:attribute name="NEXT"       type="fieldName"   use="optional" />
     </xs:complexType>
@@ -87,6 +88,7 @@
       <xs:attribute name="FIELDS"   type="fieldsList" use="required" />
       <xs:attribute name="HINTS"    type="xs:string"  use="optional" />
       <xs:attribute name="COMMENT"  type="xs:string"  use="optional" />
+      <!-- TODO: Moodle 2.6 - Drop ignored PREVIOUS/NEXT attributes -->
       <xs:attribute name="PREVIOUS" type="xs:NMTOKEN" use="optional" />
       <xs:attribute name="NEXT"     type="xs:NMTOKEN" use="optional" />
     </xs:complexType>
@@ -108,6 +110,7 @@
       <xs:attribute name="REFTABLE"  type="tableName"  use="optional" />
       <xs:attribute name="REFFIELDS" type="fieldsList" use="optional" />
       <xs:attribute name="COMMENT"   type="xs:string"  use="optional" />
+      <!-- TODO: Moodle 2.6 - Drop ignored PREVIOUS/NEXT attributes -->
       <xs:attribute name="PREVIOUS"  type="xs:NMTOKEN" use="optional" />
       <xs:attribute name="NEXT"      type="xs:NMTOKEN" use="optional" />
     </xs:complexType>
@@ -144,6 +147,7 @@
       </xs:sequence>
       <xs:attribute name="NAME"     type="tableName" use="required" />
       <xs:attribute name="COMMENT"  type="xs:string" use="optional" />
+      <!-- TODO: Moodle 2.6 - Drop ignored PREVIOUS/NEXT attributes -->
       <xs:attribute name="PREVIOUS" type="tableName" use="optional" />
       <xs:attribute name="NEXT"     type="tableName" use="optional" />
     </xs:complexType>

diff --git a/lib/xmldb/xmldb_field.php b/lib/xmldb/xmldb_field.php
index 7c69407..0a5062a 100644 (file)
--- 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 8e80e4c..e8c82d8 100644 (file)
--- 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 26ad368..0d47aa0 100644 (file)
--- 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 334610f..a104ec0 100644 (file)
--- 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 678a7d6..beeef75 100644 (file)
--- 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 c69c2a1..758c58f 100644 (file)
--- 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.= '      <FIELDS>' . "\n";