From f321c29bee76fe9f96b67a77f452892bd0356e34 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Thu, 9 Jan 2014 12:57:42 -0500
Subject: [PATCH] More consistent ID processing; added tests.

---
 module/VuFind/src/VuFind/Record/Router.php    | 32 +++++++---
 .../src/VuFindTest/Record/RouterTest.php      | 62 +++++++++++++++++++
 2 files changed, 85 insertions(+), 9 deletions(-)

diff --git a/module/VuFind/src/VuFind/Record/Router.php b/module/VuFind/src/VuFind/Record/Router.php
index 1bab2da1f53..73f0e391b31 100644
--- a/module/VuFind/src/VuFind/Record/Router.php
+++ b/module/VuFind/src/VuFind/Record/Router.php
@@ -102,7 +102,7 @@ class Router
                 && $this->config->Collections->collections
             ) {
                 if (!is_object($driver)) {
-                    list($source, $id) = explode('|', $driver, 2);
+                    list($source, $id) = $this->extractSourceAndId($driver);
                     $driver = $this->loader->load($id, $source);
                 }
                 if (true === $driver->tryMethod('isCollection')) {
@@ -133,14 +133,7 @@ class Router
             $source = $driver->getResourceSource();
             $id = $driver->getUniqueId();
         } else {
-            $parts = explode('|', $driver, 2);
-            if (count($parts) < 2) {
-                $source = 'VuFind';
-                $id = $parts[0];
-            } else {
-                $source = $parts[0];
-                $id = $parts[1];
-            }
+            list($source, $id) = $this->extractSourceAndId($driver);
         }
 
         // Build URL parameters:
@@ -156,4 +149,25 @@ class Router
             'params' => $params, 'route' => $routeBase . $routeSuffix
         );
     }
+
+    /**
+     * Extract source and ID from a pipe-delimited string, adding a default
+     * source if appropriate.
+     *
+     * @param string $driver source|ID string
+     *
+     * @return array
+     */
+    protected function extractSourceAndId($driver)
+    {
+        $parts = explode('|', $driver, 2);
+        if (count($parts) < 2) {
+            $source = 'VuFind';
+            $id = $parts[0];
+        } else {
+            $source = $parts[0];
+            $id = $parts[1];
+        }
+        return array($source, $id);
+    }
 }
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Record/RouterTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Record/RouterTest.php
index 28439503fa5..d6d104b16fd 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/Record/RouterTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Record/RouterTest.php
@@ -74,6 +74,68 @@ class RouterTest extends TestCase
         );
     }
 
+    /**
+     * Test tab routing with source|id string.
+     *
+     * @return void
+     */
+    public function testTabRoutingWithString()
+    {
+        $router = $this->getRouter();
+        $this->assertEquals(
+            array('params' => array('id' => 'test', 'tab' => 'foo'), 'route' => 'summonrecord'),
+            $router->getTabRouteDetails('Summon|test', 'foo')
+        );
+    }
+
+    /**
+     * Test collection special case with source|id string.
+     *
+     * @return void
+     */
+    public function testCollectionSpecialCaseWithString()
+    {
+        $driver = $this->getDriver();
+        $driver->expects($this->once())->method('tryMethod')->with($this->equalTo('isCollection'))->will($this->returnValue(true));
+        $router = $this->getRouter($driver, array('Collections' => array('collections' => true)));
+        $this->assertEquals(
+            array('params' => array('id' => 'test', 'tab' => 'foo'), 'route' => 'collection'),
+            $router->getTabRouteDetails('VuFind|test', 'foo')
+        );
+    }
+
+    /**
+     * Test collection special case with id string having no source prefix.
+     *
+     * @return void
+     */
+    public function testCollectionSpecialCaseWithStringMissingSource()
+    {
+        $driver = $this->getDriver();
+        $driver->expects($this->once())->method('tryMethod')->with($this->equalTo('isCollection'))->will($this->returnValue(true));
+        $router = $this->getRouter($driver, array('Collections' => array('collections' => true)));
+        $this->assertEquals(
+            array('params' => array('id' => 'test', 'tab' => 'foo'), 'route' => 'collection'),
+            $router->getTabRouteDetails('test', 'foo')
+        );
+    }
+
+    /**
+     * Test collection special case with driver.
+     *
+     * @return void
+     */
+    public function testCollectionSpecialCaseWithDriver()
+    {
+        $driver = $this->getDriver();
+        $driver->expects($this->once())->method('tryMethod')->with($this->equalTo('isCollection'))->will($this->returnValue(true));
+        $router = $this->getRouter($driver, array('Collections' => array('collections' => true)));
+        $this->assertEquals(
+            array('params' => array('id' => 'test', 'tab' => 'foo'), 'route' => 'collection'),
+            $router->getTabRouteDetails($driver, 'foo')
+        );
+    }
+
     /**
      * Test routing with id string having no source prefix.
      *
-- 
GitLab