From 7a32b13d4710f955beba040d3dece72ca2e9760b Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Wed, 15 Feb 2017 10:54:47 -0500
Subject: [PATCH] Eliminate getServiceLocator calls from controller plugins
(#918)
- More progress on ZF3 preparation.
---
module/VuFind/config/module.config.php | 2 +-
.../src/VuFind/Controller/Plugin/Factory.php | 22 +++++-
.../VuFind/Controller/Plugin/Favorites.php | 69 ++++++++++++++-----
.../src/VuFind/Controller/Plugin/Reserves.php | 23 +++++--
.../Controller/Plugin/ResultScroller.php | 24 +++++--
.../Controller/Plugin/ResultScrollerTest.php | 9 ++-
6 files changed, 113 insertions(+), 36 deletions(-)
diff --git a/module/VuFind/config/module.config.php b/module/VuFind/config/module.config.php
index 10bb79f1883..2fef28794e0 100644
--- a/module/VuFind/config/module.config.php
+++ b/module/VuFind/config/module.config.php
@@ -154,6 +154,7 @@ $config = [
],
'controller_plugins' => [
'factories' => [
+ 'favorites' => 'VuFind\Controller\Plugin\Factory::getFavorites',
'flashmessenger' => 'VuFind\Controller\Plugin\Factory::getFlashMessenger',
'followup' => 'VuFind\Controller\Plugin\Factory::getFollowup',
'holds' => 'VuFind\Controller\Plugin\Factory::getHolds',
@@ -166,7 +167,6 @@ $config = [
],
'invokables' => [
'db-upgrade' => 'VuFind\Controller\Plugin\DbUpgrade',
- 'favorites' => 'VuFind\Controller\Plugin\Favorites',
'renewals' => 'VuFind\Controller\Plugin\Renewals',
]
],
diff --git a/module/VuFind/src/VuFind/Controller/Plugin/Factory.php b/module/VuFind/src/VuFind/Controller/Plugin/Factory.php
index ba55d930e82..63c7261e64f 100644
--- a/module/VuFind/src/VuFind/Controller/Plugin/Factory.php
+++ b/module/VuFind/src/VuFind/Controller/Plugin/Factory.php
@@ -41,6 +41,22 @@ use Zend\ServiceManager\ServiceManager;
*/
class Factory
{
+ /**
+ * Construct the Favorites plugin.
+ *
+ * @param ServiceManager $sm Service manager.
+ *
+ * @return \Zend\Mvc\Controller\Plugin\Favorites
+ */
+ public static function getFavorites(ServiceManager $sm)
+ {
+ return new Favorites(
+ $sm->getServiceLocator()->get('VuFind\RecordLoader'),
+ $sm->getServiceLocator()->get('VuFind\RecordCache'),
+ $sm->getServiceLocator()->get('VuFind\Tags')
+ );
+ }
+
/**
* Construct the FlashMessenger plugin.
*
@@ -145,7 +161,8 @@ class Factory
$config = $sm->getServiceLocator()->get('VuFind\Config')->get('config');
$useIndex = isset($config->Reserves->search_enabled)
&& $config->Reserves->search_enabled;
- return new Reserves($useIndex);
+ $ss = $useIndex ? $sm->getServiceLocator()->get('VuFind\Search') : null;
+ return new Reserves($useIndex, $ss);
}
/**
@@ -161,7 +178,8 @@ class Factory
new \Zend\Session\Container(
'ResultScroller',
$sm->getServiceLocator()->get('VuFind\SessionManager')
- )
+ ),
+ $sm->getServiceLocator()->get('VuFind\SearchResultsPluginManager')
);
}
diff --git a/module/VuFind/src/VuFind/Controller/Plugin/Favorites.php b/module/VuFind/src/VuFind/Controller/Plugin/Favorites.php
index 5e3e4e5befc..8682f28f4f6 100644
--- a/module/VuFind/src/VuFind/Controller/Plugin/Favorites.php
+++ b/module/VuFind/src/VuFind/Controller/Plugin/Favorites.php
@@ -26,9 +26,11 @@
* @link https://vufind.org Main Page
*/
namespace VuFind\Controller\Plugin;
-use VuFind\Exception\LoginRequired as LoginRequiredException,
- Zend\Mvc\Controller\Plugin\AbstractPlugin,
- VuFind\Db\Row\User, VuFind\Record\Cache;
+use VuFind\Exception\LoginRequired as LoginRequiredException;
+use VuFind\Db\Row\User;
+use VuFind\Record\Cache;
+use VuFind\Record\Loader;
+use VuFind\Tags;
/**
* Zend action helper to perform favorites-related actions
@@ -39,8 +41,43 @@ use VuFind\Exception\LoginRequired as LoginRequiredException,
* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License
* @link https://vufind.org Main Page
*/
-class Favorites extends AbstractPlugin
+class Favorites extends \Zend\Mvc\Controller\Plugin\AbstractPlugin
{
+ /**
+ * Record cache
+ *
+ * @var Cache
+ */
+ protected $cache;
+
+ /**
+ * Record loader
+ *
+ * @var Loader
+ */
+ protected $loader;
+
+ /**
+ * Tag parser
+ *
+ * @var Tags
+ */
+ protected $tags;
+
+ /**
+ * Constructor
+ *
+ * @param Loader $loader Record loader
+ * @param Cache $cache Record cache
+ * @param Tags $tags Tag parser
+ */
+ public function __construct(Loader $loader, Cache $cache, Tags $tags)
+ {
+ $this->loader = $loader;
+ $this->cache = $cache;
+ $this->tags = $tags;
+ }
+
/**
* Support method for saveBulk() -- get list to save records into. Either
* retrieves existing list or creates a new one.
@@ -67,23 +104,20 @@ class Favorites extends AbstractPlugin
/**
* Support method for saveBulk() -- save a batch of records to the cache.
*
- * @param Cache $recordCache Cache service
* @param array $cacheRecordIds Array of IDs in source|id format
*
* @return void
*/
- protected function cacheBatch(Cache $recordCache, array $cacheRecordIds)
+ protected function cacheBatch(array $cacheRecordIds)
{
if ($cacheRecordIds) {
- $recordLoader = $this->getController()->getServiceLocator()
- ->get('VuFind\RecordLoader');
// Disable the cache so that we fetch latest versions, not cached ones:
- $recordLoader->setCacheContext(Cache::CONTEXT_DISABLED);
- $records = $recordLoader->loadBatch($cacheRecordIds);
+ $this->loader->setCacheContext(Cache::CONTEXT_DISABLED);
+ $records = $this->loader->loadBatch($cacheRecordIds);
// Re-enable the cache so that we actually save the records:
- $recordLoader->setCacheContext(Cache::CONTEXT_FAVORITE);
+ $this->loader->setCacheContext(Cache::CONTEXT_FAVORITE);
foreach ($records as $record) {
- $recordCache->createOrUpdate(
+ $this->cache->createOrUpdate(
$record->getUniqueID(), $record->getSourceIdentifier(),
$record->getRawData()
);
@@ -113,10 +147,7 @@ class Favorites extends AbstractPlugin
// Load helper objects needed for the saving process:
$list = $this->getList(isset($params['list']) ? $params['list'] : '', $user);
- $tagParser = $this->getController()->getServiceLocator()->get('VuFind\Tags');
- $recordCache = $this->getController()->getServiceLocator()
- ->get('VuFind\RecordCache');
- $recordCache->setContext(Cache::CONTEXT_FAVORITE);
+ $this->cache->setContext(Cache::CONTEXT_FAVORITE);
$cacheRecordIds = []; // list of record IDs to save to cache
foreach ($params['ids'] as $current) {
@@ -129,16 +160,16 @@ class Favorites extends AbstractPlugin
// Add the information to the user's account:
$tags = isset($params['mytags'])
- ? $tagParser->parse($params['mytags']) : [];
+ ? $this->tags->parse($params['mytags']) : [];
$user->saveResource($resource, $list, $tags, '', false);
// Collect record IDs for caching
- if ($recordCache->isCachable($resource->source)) {
+ if ($this->cache->isCachable($resource->source)) {
$cacheRecordIds[] = $current;
}
}
- $this->cacheBatch($recordCache, $cacheRecordIds);
+ $this->cacheBatch($cacheRecordIds);
return ['listId' => $list->id];
}
diff --git a/module/VuFind/src/VuFind/Controller/Plugin/Reserves.php b/module/VuFind/src/VuFind/Controller/Plugin/Reserves.php
index cbd33a53db6..51f8e25c4c1 100644
--- a/module/VuFind/src/VuFind/Controller/Plugin/Reserves.php
+++ b/module/VuFind/src/VuFind/Controller/Plugin/Reserves.php
@@ -26,6 +26,7 @@
* @link https://vufind.org Main Page
*/
namespace VuFind\Controller\Plugin;
+use VuFindSearch\Service;
use Zend\Mvc\Controller\Plugin\AbstractPlugin;
/**
@@ -47,15 +48,28 @@ class Reserves extends AbstractPlugin
*/
protected $useIndex;
+ /**
+ * Search service
+ *
+ * @var Service
+ */
+ protected $searchService;
+
/**
* Constructor
*
- * @param bool $useIndex Do we need to use the Solr index for reserves (true)
- * or the ILS driver (false)?
+ * @param bool $useIndex Do we need to use the Solr index for reserves
+ * (true) or the ILS driver (false)?
+ * @param Service $searchService Search service (only required when $useIndex
+ * is true).
*/
- public function __construct($useIndex = false)
+ public function __construct($useIndex = false, Service $searchService = null)
{
$this->useIndex = $useIndex;
+ if ($useIndex && null === $searchService) {
+ throw new \Exception('Missing required search service');
+ }
+ $this->searchService = $searchService;
}
/**
@@ -84,8 +98,7 @@ class Reserves extends AbstractPlugin
if ($this->useIndex()) {
// get the selected reserve record from reserves index
// and extract the bib IDs from it
- $result = $this->getController()->getServiceLocator()
- ->get('VuFind\Search')
+ $result = $this->searchService
->retrieve('SolrReserves', $course . '|' . $inst . '|' . $dept);
$bibs = [];
if ($result->getTotal() < 1) {
diff --git a/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php b/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
index 2f7a61970f5..4f39ae56587 100644
--- a/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
+++ b/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
@@ -26,8 +26,9 @@
* @link https://vufind.org/wiki/development Wiki
*/
namespace VuFind\Controller\Plugin;
-use Zend\Mvc\Controller\Plugin\AbstractPlugin,
- Zend\Session\Container as SessionContainer;
+use VuFind\Search\Results\PluginManager as ResultsManager;
+use Zend\Mvc\Controller\Plugin\AbstractPlugin;
+use Zend\Session\Container as SessionContainer;
/**
* Class for managing "next" and "previous" navigation within result sets.
@@ -54,18 +55,29 @@ class ResultScroller extends AbstractPlugin
*/
protected $data;
+ /**
+ * Results manager
+ *
+ * @var ResultsManager
+ */
+ protected $resultsManager;
+
/**
* Constructor. Create a new search result scroller.
*
* @param SessionContainer $session Session container
+ * @param ResultsManager $rm Results manager
* @param bool $enabled Is the scroller enabled?
*/
- public function __construct(SessionContainer $session, $enabled = true)
- {
+ public function __construct(SessionContainer $session, ResultsManager $rm,
+ $enabled = true
+ ) {
$this->enabled = $enabled;
// Set up session namespace for the class.
$this->data = $session;
+
+ $this->resultsManager = $rm;
}
/**
@@ -552,9 +564,7 @@ class ResultScroller extends AbstractPlugin
$row = $searchTable->getRowById($this->data->searchId, false);
if (!empty($row)) {
$minSO = $row->getSearchObject();
- $manager = $this->getController()->getServiceLocator()
- ->get('VuFind\SearchResultsPluginManager');
- $search = $minSO->deminify($manager);
+ $search = $minSO->deminify($this->resultsManager);
// The saved search does not remember its original limit;
// we should reapply it from the session data:
$search->getParams()->setLimit($this->data->limit);
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 b473f7da415..ae7fb28404a 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
@@ -50,7 +50,9 @@ class ResultScrollerTest extends TestCase
*/
public function testDisabled()
{
- $plugin = new ResultScroller(new Container('test'), false);
+ $mockManager = $this->getMockBuilder('VuFind\Search\Results\PluginManager')
+ ->disableOriginalConstructor()->getMock();
+ $plugin = new ResultScroller(new Container('test'), $mockManager, false);
$results = $this->getMockResults();
$this->assertFalse($plugin->init($results));
$expected = [
@@ -334,8 +336,11 @@ class ResultScrollerTest extends TestCase
protected function getMockResultScroller($results = null,
$methods = ['restoreLastSearch', 'rememberSearch']
) {
+ $mockManager = $this->getMockBuilder('VuFind\Search\Results\PluginManager')
+ ->disableOriginalConstructor()->getMock();
$mock = $this->getMock(
- 'VuFind\Controller\Plugin\ResultScroller', $methods, [new Container('test')]
+ 'VuFind\Controller\Plugin\ResultScroller', $methods,
+ [new Container('test'), $mockManager]
);
if (in_array('restoreLastSearch', $methods) && null !== $results) {
$mock->expects($this->any())->method('restoreLastSearch')->will($this->returnValue($results));
--
GitLab