Implement unit tests for very public interfaces of HTMLModuleManager, also added lots of error checking. tally_errors now requires unit test to be passed in as parameter.

git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@760 48356398-32a2-884e-a903-53898d9a118a

library/HTMLPurifier/HTMLModuleManager.php

@@ -45,11 +45,21 @@ class HTMLPurifier_HTMLModuleManager
     var $validModules = array();
     
     /**
-     * Modules that we will use broadly, subset of validModules. Single
+     * Collections that should be merged into $validModules
+     */
+    var $validModulesCollections = array('Safe', 'Unsafe', 'Lenient', 'Correctional');
+    
+    /**
+     * Modules that we will use broadly, subset of $validModules. Single
      * element definitions may result in us consulting validModules.
      */
     var $activeModules = array();
     
+    /**
+     * Collections that should be merged into $activeModules
+     */
+    var $activeModulesCollections = array('Safe', 'Lenient', 'Correctional');
+    
     /**
      * Current doctype for which $validModules is based
      */
@@ -120,16 +130,7 @@ class HTMLPurifier_HTMLModuleManager
     /** List of prefixes we should use for resolving small names */
     var $prefixes = array('HTMLPurifier_HTMLModule_');
     
-    /** Associative array of order keywords to an integer index */
+    var $contentSets; /**< Instance of HTMLPurifier_ContentSets */
-    var $orderKeywords = array(
-        'define' => 10,
-        'define-redefine' => 20,
-        'redefine' => 30,
-    );
-    
-    /** Instance of HTMLPurifier_ContentSets configured with full modules. */
-    var $contentSets;
-    
     var $attrTypes; /**< Instance of HTMLPurifier_AttrTypes */
     var $attrCollections; /**< Instance of HTMLPurifier_AttrCollections */
     
@@ -183,6 +184,14 @@ class HTMLPurifier_HTMLModuleManager
         $this->modules[$module->name] = $module;
     }
     
+    /**
+     * Adds a class prefix that addModule() will use to resolve a
+     * string name to a concrete class
+     */
+    function addPrefix($prefix) {
+        $this->prefixes[] = (string) $prefix;
+    }
+    
     function setup($config) {
         // retrieve the doctype
         $this->doctype = $this->getDoctype($config);
@@ -193,16 +202,8 @@ class HTMLPurifier_HTMLModuleManager
             $this->processCollections($this->$varname);
         }
         
-        // $collections variable in following instances will be dynamically
+        $this->validModules  = $this->assembleModules($this->validModulesCollections);
-        // generated once we figure out some config variables
+        $this->activeModules = $this->assembleModules($this->activeModulesCollections);
-        
-        // setup the validModules array
-        $collections = array('Safe', 'Unsafe', 'Lenient', 'Correctional');
-        $this->validModules = $this->assembleModules($collections);
-        
-        // setup the activeModules array
-        $collections = array('Safe', 'Lenient', 'Correctional');
-        $this->activeModules = $this->assembleModules($collections);
         
         // setup lookup table based on all valid modules
         foreach ($this->validModules as $module) {
@@ -274,11 +275,27 @@ class HTMLPurifier_HTMLModuleManager
         // perform inclusions
         foreach ($cols as $col_i => $col) {
             if (is_string($col)) continue; // alias, save for later
-            if (!is_array($col[0])) continue; // no inclusions to do
+            if (empty($col[0]) || !is_array($col[0])) continue; // no inclusions to do
+            $seen = array($col_i => true); // recursion reporting
             $includes = $col[0];
-            unset($cols[$col_i][0]); // remove inclusions value
+            unset($cols[$col_i][0]); // remove inclusions value, recursion guard
             for ($i = 0; isset($includes[$i]); $i++) {
                 $inc = $includes[$i];
+                if (isset($seen[$inc])) {
+                    trigger_error(
+                        "Circular inclusion detected in $col_i collection",
+                        E_USER_ERROR
+                    );
+                    continue;
+                } else {
+                    $seen[$inc] = true;
+                }
+                if (!isset($cols[$inc])) {
+                    trigger_error(
+                        "Collection $col_i tried to include undefined ".
+                        "collection $inc", E_USER_ERROR);
+                    continue;
+                }
                 foreach ($cols[$inc] as $module) {
                     if (is_array($module)) { // another inclusion!
                         foreach ($module as $inc2) $includes[] = $inc2;
@@ -296,6 +313,21 @@ class HTMLPurifier_HTMLModuleManager
             $order = array();
             foreach ($col as $module_i => $module) {
                 unset($cols[$col_i][$module_i]);
+                if (is_array($module)) {
+                    trigger_error("Illegal inclusion array at index".
+                        " $module_i found collection $col_i, inclusion".
+                        " arrays must be at start of collection (index 0)",
+                        E_USER_ERROR);
+                    continue;
+                }
+                if (!isset($this->modules[$module])) {
+                    trigger_error(
+                        "Collection $col_i references undefined ".
+                        "module $module",
+                        E_USER_ERROR
+                    );
+                    continue;
+                }
                 $module = $this->modules[$module];
                 $cols[$col_i][$module->name] = $module;
                 $order[$module->name] = $module->order;
@@ -308,6 +340,23 @@ class HTMLPurifier_HTMLModuleManager
         // hook up aliases
         foreach ($cols as $col_i => $col) {
             if (!is_string($col)) continue;
+            if (!isset($cols[$col])) {
+                trigger_error(
+                    "$col_i collection is alias to undefined $col collection",
+                    E_USER_ERROR
+                );
+                unset($cols[$col_i]);
+                continue;
+            }
+            // recursion guard
+            if (is_string($cols[$col])) {
+                trigger_error(
+                    "Cannot alias $col_i collection to alias",
+                    E_USER_ERROR
+                );
+                unset($cols[$col_i]);
+                continue;
+            }
             $cols[$col_i] = $cols[$col];
         }
         

tests/HTMLPurifier/ConfigSchemaTest.php

@@ -41,7 +41,7 @@ class HTMLPurifier_ConfigSchemaTest extends UnitTestCase
     function tearDown() {
         // testing is done, restore the old copy
         HTMLPurifier_ConfigSchema::instance($this->old_copy);
-        tally_errors();
+        tally_errors($this);
     }
     
     function test_defineNamespace() {

tests/HTMLPurifier/ConfigTest.php

@@ -20,7 +20,7 @@ class HTMLPurifier_ConfigTest extends UnitTestCase
     
     function tearDown() {
         HTMLPurifier_ConfigSchema::instance($this->old_copy);
-        tally_errors();
+        tally_errors($this);
     }
     
     // test functionality based on ConfigSchema

tests/HTMLPurifier/HTMLModuleManagerTest.php

@@ -0,0 +1,193 @@
+<?php
+
+require_once 'HTMLPurifier/HTMLModuleManager.php';
+
+// stub classes for unit testing
+class HTMLPurifier_HTMLModule_ManagerTestModule extends HTMLPurifier_HTMLModule {
+    var $name = 'ManagerTestModule';
+}
+class HTMLPurifier_HTMLModuleManagerTest_TestModule extends HTMLPurifier_HTMLModule {
+    var $name = 'TestModule';
+}
+
+class HTMLPurifier_HTMLModuleManagerTest extends UnitTestCase
+{
+    
+    /**
+     * System under test, instance of HTMLPurifier_HTMLModuleManager.
+     */
+    var $manager;
+    
+    function setup() {
+        $this->manager = new HTMLPurifier_HTMLModuleManager();
+    }
+    
+    function teardown() {
+        tally_errors($this);
+    }
+    
+    function test_addModule() {
+        $module = new HTMLPurifier_HTMLModule();
+        $module->name = 'Module';
+        
+        $module2 = new HTMLPurifier_HTMLModule();
+        $module2->name = 'Module2';
+        
+        $this->manager->addModule($module);
+        $this->assertEqual($module, $this->manager->modules['Module']);
+        $module_order = $this->manager->modules['Module']->order;
+        
+        $this->manager->addModule($module2);
+        $this->assertEqual($module2, $this->manager->modules['Module2']);
+        $module2_order = $this->manager->modules['Module2']->order;
+        $this->assertEqual($module_order + 1, $module2_order);
+    }
+    
+    function test_addModule_undefinedClass() {
+        $this->expectError('TotallyCannotBeDefined module does not exist');
+        $this->manager->addModule('TotallyCannotBeDefined');
+    }
+    
+    function test_addModule_stringExpansion() {
+        $this->manager->addModule('ManagerTestModule');
+        $this->assertIsA($this->manager->modules['ManagerTestModule'],
+            'HTMLPurifier_HTMLModule_ManagerTestModule');
+    }
+    
+    function test_addPrefix() {
+        $this->manager->addPrefix('HTMLPurifier_HTMLModuleManagerTest_');
+        $this->manager->addModule('TestModule');
+        $this->assertIsA($this->manager->modules['TestModule'],
+            'HTMLPurifier_HTMLModuleManagerTest_TestModule');
+    }
+    
+    function assertProcessCollections($input, $expect = false) {
+        if ($expect === false) $expect = $input;
+        $this->manager->processCollections($input);
+        // substitute in modules for $expect
+        foreach ($expect as $col_i => $col) {
+            foreach ($col as $mod_i => $mod) {
+                unset($expect[$col_i][$mod_i]);
+                $expect[$col_i][$mod] = $this->manager->modules[$mod];
+            }
+        }
+        $this->assertIdentical($input, $expect);
+    }
+    
+    function testImpl_processCollections() {
+        $this->assertProcessCollections(
+            array()
+        );
+        $this->assertProcessCollections(
+            array('HTML' => array('Text'))
+        );
+        $this->assertProcessCollections(
+            array('HTML' => array('Text', 'Legacy'))
+        );
+        $this->assertProcessCollections( // order is important!
+            array('HTML' => array('Legacy', 'Text')),
+            array('HTML' => array('Text', 'Legacy'))
+        );
+        $this->assertProcessCollections( // privates removed after process
+            array('_Private' => array('Legacy', 'Text')),
+            array()
+        );
+        $this->assertProcessCollections( // inclusions come first
+            array(
+                'HTML' => array(array('XHTML'), 'Legacy'),
+                'XHTML' => array('Text', 'Hypertext')
+            ),
+            array(
+                'HTML' => array('Text', 'Hypertext', 'Legacy'),
+                'XHTML' => array('Text', 'Hypertext')
+            )
+        );
+        $this->assertProcessCollections(
+            array(
+                'HTML' => array(array('_Common'), 'Legacy'),
+                '_Common' => array('Text', 'Hypertext')
+            ),
+            array(
+                'HTML' => array('Text', 'Hypertext', 'Legacy')
+            )
+        );
+        $this->assertProcessCollections( // aliasing!
+            array(
+                'HTML' => 'XHTML',
+                'XHTML' => array('Text', 'Hypertext')
+            ),
+            array(
+                'HTML' => array('Text', 'Hypertext'),
+                'XHTML' => array('Text', 'Hypertext')
+            )
+        );
+        $this->assertProcessCollections( // nested inclusions
+            array(
+                'Full' => array(array('Minimal'), 'Hypertext'),
+                'Minimal' => array(array('Bare'), 'List'),
+                'Bare' => array('Text')
+            ),
+            array(
+                'Full' => array('Text', 'Hypertext', 'List'),
+                'Minimal' => array('Text', 'List'),
+                'Bare' => array('Text')
+            )
+        );
+    }
+    
+    function testImpl_processCollections_error() {
+        $this->expectError( // active variables, watch out!
+            'Illegal inclusion array at index 1 found collection HTML, '.
+            'inclusion arrays must be at start of collection (index 0)');
+        $this->manager->processCollections(
+            $c = array(
+                'HTML' => array('Legacy', array('XHTML')),
+                'XHTML' => array('Text', 'Hypertext')
+            )
+        );
+        
+        $this->expectError('Collection HTML references undefined '.
+            'module Foobar');
+        $this->manager->processCollections(
+            $c = array(
+                'HTML' => array('Foobar')
+            )
+        );
+        
+        $this->expectError('Collection HTML tried to include undefined '.
+            'collection _Common');
+        $this->manager->processCollections(
+            $c = array(
+                'HTML' => array(array('_Common'), 'Legacy')
+            )
+        );
+        
+        // reports the first circular inclusion it runs across
+        $this->expectError('Circular inclusion detected in HTML collection');
+        $this->manager->processCollections(
+            $c = array(
+                'HTML' => array(array('XHTML')),
+                'XHTML' => array(array('HTML'))
+            )
+        );
+        
+        $this->expectError('XHTML collection is alias to undefined Foobar collection');
+        $this->manager->processCollections(
+            $c = array(
+                'XHTML' => 'Foobar'
+            )
+        );
+        
+        $this->expectError('Cannot alias XHTML collection to alias');
+        $this->manager->processCollections(
+            $c = array(
+                'XHTML' => 'Foobar',
+                'Foobar' => 'HTML',
+                'HTML' => array()
+            )
+        );
+    }
+    
+}
+
+?>

tests/tally_errors.func.php

@@ -1,6 +1,6 @@
 <?php
 
-function tally_errors() {
+function tally_errors($test) {
     // BRITTLE: relies on private code to work
     $context = &SimpleTest::getContext();
     $queue = &$context->get('SimpleErrorQueue');
@@ -9,7 +9,7 @@ function tally_errors() {
         if (count($e) != 2) return; // fut-compat
         if (!isset($e[0])) return; // fut-compat
         $e[0]->_dumper = &new SimpleDumper();
-        $this->fail('Error expectation not fulfilled: ' .
+        $test->fail('Error expectation not fulfilled: ' .
             $e[0]->testMessage(null));
     }
     $queue->_expectation_queue = array();

tests/test_files.php

@@ -50,6 +50,7 @@ $test_files[] = 'EncoderTest.php';
 $test_files[] = 'EntityLookupTest.php';
 $test_files[] = 'EntityParserTest.php';
 $test_files[] = 'GeneratorTest.php';
+$test_files[] = 'HTMLModuleManagerTest.php';
 $test_files[] = 'IDAccumulatorTest.php';
 $test_files[] = 'LanguageFactoryTest.php';
 $test_files[] = 'LanguageTest.php';