From 13996f98d0a1ac81e3b3ed5e5928df0cb9f21da1 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Fri, 10 Oct 2014 15:09:09 -0400
Subject: [PATCH] Optimization: do not try to load non-existent next page. -
Includes tests.
---
.../Controller/Plugin/ResultScroller.php | 8 +-
.../Controller/Plugin/ResultScrollerTest.php | 156 +++++++++++++++++-
2 files changed, 154 insertions(+), 10 deletions(-)
diff --git a/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php b/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
index 6198ee3c8d2..f267e0cd16f 100644
--- a/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
+++ b/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
@@ -171,9 +171,11 @@ class ResultScroller extends AbstractPlugin
*/
protected function fetchNextPage($retVal, $lastSearch, $pos)
{
- // if the next page has not been fetched, then
- // fetch the next page
- if ($this->data->nextIds == null) {
+ // if the current page is NOT the last page, and the next page has not been
+ // fetched, then fetch the next page
+ if ($this->data->page < ceil($this->data->total / $this->data->limit)
+ && $this->data->nextIds == null
+ ) {
$this->data->nextIds = $this->fetchPage(
$lastSearch, $this->data->page + 1
);
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Controller/Plugin/ResultScrollerTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Controller/Plugin/ResultScrollerTest.php
index ec44cd34e8d..5168ef802af 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/Controller/Plugin/ResultScrollerTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Controller/Plugin/ResultScrollerTest.php
@@ -42,6 +42,13 @@ use VuFindTest\Unit\TestCase as TestCase;
*/
class ResultScrollerTest extends TestCase
{
+ /**
+ * Cache for record driver mocks.
+ *
+ * @var array
+ */
+ protected $recordDrivers = array();
+
/**
* Test disabled behavior
*
@@ -58,6 +65,78 @@ class ResultScrollerTest extends TestCase
$this->assertEquals($expected, $plugin->getScrollData($this->getMockRecordDriver('foo')));
}
+ /**
+ * Test scrolling for a record in the middle of the page
+ *
+ * @return void
+ */
+ public function testScrollingInMiddleOfPage()
+ {
+ $results = $this->getMockResults(1, 10, 10);
+ $plugin = $this->getMockResultScroller();
+ $plugin->expects($this->any())->method('restoreLastSearch')->will($this->returnValue($results));
+ $this->assertTrue($plugin->init($results));
+ $expected = array(
+ 'previousRecord'=>'VuFind|4', 'nextRecord'=>'VuFind|6',
+ 'currentPosition'=>5, 'resultTotal'=>10
+ );
+ $this->assertEquals($expected, $plugin->getScrollData($this->getMockRecordDriver(5)));
+ }
+
+ /**
+ * Test scrolling for a record at the start of the first page
+ *
+ * @return void
+ */
+ public function testScrollingAtStartOfFirstPage()
+ {
+ $results = $this->getMockResults(1, 10, 10);
+ $plugin = $this->getMockResultScroller();
+ $plugin->expects($this->any())->method('restoreLastSearch')->will($this->returnValue($results));
+ $this->assertTrue($plugin->init($results));
+ $expected = array(
+ 'previousRecord'=>null, 'nextRecord'=>'VuFind|2',
+ 'currentPosition'=>1, 'resultTotal'=>10
+ );
+ $this->assertEquals($expected, $plugin->getScrollData($this->getMockRecordDriver(1)));
+ }
+
+ /**
+ * Test scrolling for a record at the end of the last page (single-page example)
+ *
+ * @return void
+ */
+ public function testScrollingAtEndOfLastPage()
+ {
+ $results = $this->getMockResults(1, 10, 10);
+ $plugin = $this->getMockResultScroller();
+ $plugin->expects($this->any())->method('restoreLastSearch')->will($this->returnValue($results));
+ $this->assertTrue($plugin->init($results));
+ $expected = array(
+ 'previousRecord'=>'VuFind|9', 'nextRecord'=>null,
+ 'currentPosition'=>10, 'resultTotal'=>10
+ );
+ $this->assertEquals($expected, $plugin->getScrollData($this->getMockRecordDriver(10)));
+ }
+
+ /**
+ * Test scrolling for a record at the end of the last page (multi-page example)
+ *
+ * @return void
+ */
+ public function testScrollingAtEndOfLastPageInMultiPageScenario()
+ {
+ $results = $this->getMockResults(2, 10, 17);
+ $plugin = $this->getMockResultScroller();
+ $plugin->expects($this->any())->method('restoreLastSearch')->will($this->returnValue($results));
+ $this->assertTrue($plugin->init($results));
+ $expected = array(
+ 'previousRecord'=>'VuFind|16', 'nextRecord'=>null,
+ 'currentPosition'=>17, 'resultTotal'=>17
+ );
+ $this->assertEquals($expected, $plugin->getScrollData($this->getMockRecordDriver(17)));
+ }
+
/**
* Get mock record driver
*
@@ -68,24 +147,87 @@ class ResultScrollerTest extends TestCase
*/
protected function getMockRecordDriver($id, $source = 'VuFind')
{
- $driver = $this->getMockBuilder('VuFind\RecordDriver\AbstractBase')
- ->disableOriginalConstructor()
- ->getMock();
- $driver->expects($this->any())->method('getUniqueId')->will($this->returnValue($id));
- $driver->expects($this->any())->method('getResourceSource')->will($this->returnValue($source));
- return $driver;
+ if (!isset($this->recordDrivers["$source|$id"])) {
+ $driver = $this->getMockBuilder('VuFind\RecordDriver\AbstractBase')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $driver->expects($this->any())->method('getUniqueId')->will($this->returnValue($id));
+ $driver->expects($this->any())->method('getResourceSource')->will($this->returnValue($source));
+ $this->recordDrivers["$source|$id"] = $driver;
+ }
+ return $this->recordDrivers["$source|$id"];
}
/**
* Get mock search results
*
+ * @param int $page Current page number
+ * @param int $limit Page size
+ * @param int $total Total size of fake result set
+ *
* @return \VuFind\Search\Base\Results
*/
- protected function getMockResults()
+ protected function getMockResults($page = 0, $limit = 0, $total = 0)
{
$results = $this->getMockBuilder('VuFind\Search\Solr\Results')
->disableOriginalConstructor()
->getMock();
+ $results->expects($this->any())->method('getSearchId')->will($this->returnValue('dummy-search-id'));
+ $results->expects($this->any())->method('getParams')->will($this->returnValue($this->getMockParams($page, $limit)));
+ $results->expects($this->any())->method('getResultTotal')->will($this->returnValue($total));
+ $results->expects($this->any())->method('getResults')->will($this->returnValue($this->getResultSet($page, $limit, $total)));
return $results;
}
+
+ /**
+ * Get mock search params
+ *
+ * @param int $page Current page number
+ * @param int $limit Page size
+ *
+ * @return \VuFind\Search\Base\Params
+ */
+ protected function getMockParams($page = 0, $limit = 0)
+ {
+ $params = $this->getMockBuilder('VuFind\Search\Solr\Params')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $params->expects($this->any())->method('getPage')->will($this->returnValue($page));
+ $params->expects($this->any())->method('getLimit')->will($this->returnValue($limit));
+ return $params;
+ }
+
+ /**
+ * Get mock result scroller
+ *
+ * @param array $methods Methods to mock
+ *
+ * @return ResultScroller
+ */
+ protected function getMockResultScroller($methods = array('restoreLastSearch', 'rememberSearch'))
+ {
+ return $this->getMock('VuFind\Controller\Plugin\ResultScroller', $methods);
+ }
+
+ /**
+ * Get set of fake record drivers.
+ *
+ * @param int $page Current page number
+ * @param int $limit Page size
+ * @param int $total Total size of fake result set
+ *
+ * @return array
+ */
+ protected function getResultSet($page, $limit, $total)
+ {
+ $retVal = array();
+ for ($i = 1; $i <= $limit; $i++) {
+ $current = ($page - 1) * $limit + $i;
+ if ($current > $total) {
+ break;
+ }
+ $retVal[] = $this->getMockRecordDriver($current);
+ }
+ return $retVal;
+ }
}
\ No newline at end of file
--
GitLab