Skip to content

Commit

Permalink
MDL-37726 stop using PREVIOUS/NEXT in install.xml files
Browse files Browse the repository at this point in the history
  • Loading branch information
@skodak
skodak committed Jan 29, 2013
1 parent c6cbc3c commit 76f2fcd
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 227 deletions.
11 changes: 1 addition & 10 deletions config-dist.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,17 +498,8 @@
// Divert all outgoing emails to this address to test and debug emailing features
// $CFG->divertallemailsto = '[email protected]'; // 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.
Expand Down
18 changes: 0 additions & 18 deletions lib/ddl/tests/ddl_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1400,22 +1400,13 @@ public function testDeleteTablesFromXmldbFile() {
$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);
} catch (Exception $e) {
$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'));
Expand All @@ -1441,22 +1432,13 @@ public function testInstallFromXmldbFile() {
$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);
} catch (Exception $e) {
$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'));
Expand Down
2 changes: 1 addition & 1 deletion lib/ddl/tests/fixtures/invalid.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
4 changes: 4 additions & 0 deletions lib/xmldb/xmldb.xsd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
14 changes: 0 additions & 14 deletions lib/xmldb/xmldb_field.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,14 +394,6 @@ public function arr2xmldb_field($xmlarr) {
$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;
Expand Down Expand Up @@ -533,12 +525,6 @@ public function xmlOutput() {
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;
Expand Down
14 changes: 0 additions & 14 deletions lib/xmldb/xmldb_index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,6 @@ public function arr2xmldb_index($xmlarr) {
$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;
Expand Down Expand Up @@ -258,12 +250,6 @@ public function xmlOutput() {
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;
Expand Down
14 changes: 0 additions & 14 deletions lib/xmldb/xmldb_key.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,6 @@ public function arr2xmldb_key($xmlarr) {
$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;
Expand Down Expand Up @@ -384,12 +376,6 @@ public function xmlOutput() {
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;
Expand Down
121 changes: 2 additions & 119 deletions lib/xmldb/xmldb_object.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,6 @@ public function checkNameValues($arr) {
* @return bool true if $arr modified
*/
public function fixPrevNext(&$arr) {
global $CFG;

if (empty($CFG->xmldbreconstructprevnext)) {
return false;
}
$tweaked = false;

$prev = null;
Expand All @@ -267,125 +262,15 @@ public function fixPrevNext(&$arr) {
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
* @param array $arr
* @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)) {
Expand All @@ -411,9 +296,7 @@ public function orderElements($arr) {
// 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;
Expand Down
7 changes: 1 addition & 6 deletions lib/xmldb/xmldb_structure.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,8 @@ public function arr2xmldb_structure($xmlarr) {
$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';
Expand Down
Loading

0 comments on commit 76f2fcd

Please sign in to comment.