CRM-20926 - Resolve "exception"/"exceptions" discrepency. Improve test.

The PHPIDS config format uses the property: ``` exceptions => array('field_1)` ``` But our XML format uses the tag: ``` <exception>field_1</exception> ``` Grammatically, those both make sense. But they were getting confused. This modifies the XML parser to map the tags `<exception>` to property `exceptions`, and it updates the unit-test to ensure that the data (from the route and the default config) are correctly merged.
civicrm · Aug 4, 2017 · 36d4fa1 · 36d4fa1
1 parent 4535c1f
commit 36d4fa1
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 21 deletions.
diff --git a/CRM/Core/IDS.php b/CRM/Core/IDS.php
@@ -69,17 +69,7 @@ public function check($route) {
       return NULL;
     }
 
-    $config = \CRM_Core_IDS::createStandardConfig();
-    foreach (array('json', 'html', 'exception') as $section) {
-      if (isset($route['ids_arguments'][$section])) {
-        foreach ($route['ids_arguments'][$section] as $v) {
-          $config['General'][$section][] = $v;
-        }
-        $config['General'][$section] = array_unique($config['General'][$section]);
-      }
-    }
-
-    $init = self::create($config);
+    $init = self::create(self::createRouteConfig($route));
 
     // Add request url and user agent.
     $_REQUEST['IDS_request_uri'] = $_SERVER['REQUEST_URI'];
@@ -195,6 +185,26 @@ public static function createStandardConfig() {
     return $result;
   }
 
+  /**
+   * @param array $route
+   * @return array
+   */
+  public static function createRouteConfig($route) {
+    $config = \CRM_Core_IDS::createStandardConfig();
+    foreach (array('json', 'html', 'exceptions') as $section) {
+      if (isset($route['ids_arguments'][$section])) {
+        if (!isset($config['General'][$section])) {
+          $config['General'][$section] = array();
+        }
+        foreach ($route['ids_arguments'][$section] as $v) {
+          $config['General'][$section][] = $v;
+        }
+        $config['General'][$section] = array_unique($config['General'][$section]);
+      }
+    }
+    return $config;
+  }
+
   /**
    * This function reacts on the values in the incoming results array.
    *

diff --git a/CRM/Core/Menu.php b/CRM/Core/Menu.php
@@ -135,10 +135,10 @@ public static function readXML($xml, &$menu) {
 
       if ($item->ids_arguments) {
         $ids = array();
-        foreach (array('json', 'html', 'exception') as $type) {
-          $ids[$type] = array();
-          foreach ($item->ids_arguments->{$type} as $value) {
-            $ids[$type][] = (string) $value;
+        foreach (array('json' => 'json', 'html' => 'html', 'exception' => 'exceptions') as $tag => $attr) {
+          $ids[$attr] = array();
+          foreach ($item->ids_arguments->{$tag} as $value) {
+            $ids[$attr][] = (string) $value;
           }
         }
         $menu[$path]['ids_arguments'] = $ids;

diff --git a/tests/phpunit/CRM/Core/MenuTest.php b/tests/phpunit/CRM/Core/MenuTest.php
@@ -42,7 +42,7 @@ public function testReadXML_IDS() {
          <ids_arguments>
           <json>alpha</json>
           <json>beta</json>
-          <html>gamma</html>
+          <exception>gamma</exception>
         </ids_arguments>
       </item>
     </menu>
@@ -53,8 +53,14 @@ public function testReadXML_IDS() {
     $this->assertTrue(isset($menu['civicrm/foo/bar']));
     $this->assertEquals('Foo Bar', $menu['civicrm/foo/bar']['title']);
     $this->assertEquals(array('alpha', 'beta'), $menu['civicrm/foo/bar']['ids_arguments']['json']);
-    $this->assertEquals(array('gamma'), $menu['civicrm/foo/bar']['ids_arguments']['html']);
-    $this->assertEquals(array(), $menu['civicrm/foo/bar']['ids_arguments']['exception']);
+    $this->assertEquals(array('gamma'), $menu['civicrm/foo/bar']['ids_arguments']['exceptions']);
+    $this->assertEquals(array(), $menu['civicrm/foo/bar']['ids_arguments']['html']);
+
+    $idsConfig = CRM_Core_IDS::createRouteConfig($menu['civicrm/foo/bar']);
+    $this->assertTrue(in_array('alpha', $idsConfig['General']['json'])); // XML
+    $this->assertTrue(in_array('beta', $idsConfig['General']['json'])); // XML
+    $this->assertTrue(in_array('gamma', $idsConfig['General']['exceptions'])); // XML
+    $this->assertTrue(in_array('thankyou_text', $idsConfig['General']['exceptions'])); // Inherited
   }
 
   /**
@@ -64,17 +70,17 @@ public function testReadXML_IDS() {
   public function testModuleData() {
     CRM_Core_Menu::store(TRUE);
     $item = CRM_Core_Menu::get('civicrm/case');
-    $this->assertFalse(isset($item['ids_arguments']['exception']));
+    $this->assertFalse(isset($item['ids_arguments']['exceptions']));
     $this->assertFalse(isset($item['whimsy']));
 
     CRM_Utils_Hook::singleton()->setHook('civicrm_alterMenu', function(&$items){
-      $items['civicrm/case']['ids_arguments']['exception'][] = 'foobar';
+      $items['civicrm/case']['ids_arguments']['exceptions'][] = 'foobar';
       $items['civicrm/case']['whimsy'] = 'godliness';
     });
 
     CRM_Core_Menu::store(TRUE);
     $item = CRM_Core_Menu::get('civicrm/case');
-    $this->assertTrue(in_array('foobar', $item['ids_arguments']['exception']));
+    $this->assertTrue(in_array('foobar', $item['ids_arguments']['exceptions']));
     $this->assertEquals('godliness', $item['whimsy']);
   }