From 659204de5708f90f020f6006ca38e301784b784d Mon Sep 17 00:00:00 2001
From: Ere Maijala <ere.maijala@helsinki.fi>
Date: Thu, 3 Mar 2016 09:02:50 -0500
Subject: [PATCH] ILS driver cache improvements. - Create generic (optional)
 caching mechanism in base class. - Replace VoyagerRestful session-based
 caching with new generic mechanism.

---
 .../src/VuFind/ILS/Driver/AbstractBase.php    | 94 +++++++++++++++++++
 .../VuFind/src/VuFind/ILS/Driver/Factory.php  |  6 +-
 .../src/VuFind/ILS/Driver/VoyagerRestful.php  | 85 +++++------------
 3 files changed, 121 insertions(+), 64 deletions(-)

diff --git a/module/VuFind/src/VuFind/ILS/Driver/AbstractBase.php b/module/VuFind/src/VuFind/ILS/Driver/AbstractBase.php
index f2062252e18..8fd9125ea99 100644
--- a/module/VuFind/src/VuFind/ILS/Driver/AbstractBase.php
+++ b/module/VuFind/src/VuFind/ILS/Driver/AbstractBase.php
@@ -26,6 +26,7 @@
  * @link     https://vufind.org/wiki/development:plugins:ils_drivers Wiki
  */
 namespace VuFind\ILS\Driver;
+use Zend\Cache\Storage\StorageInterface;
 
 /**
  * Default ILS driver base class.
@@ -40,6 +41,20 @@ namespace VuFind\ILS\Driver;
  */
 abstract class AbstractBase implements DriverInterface
 {
+    /**
+     * Cache for storing ILS data temporarily (e.g. patron blocks)
+     *
+     * @var StorageInterface
+     */
+    protected $cache = null;
+
+    /**
+     * Lifetime of cache (in seconds).
+     *
+     * @var int
+     */
+    protected $cacheLifetime = 30;
+
     /**
      * Driver configuration
      *
@@ -47,6 +62,18 @@ abstract class AbstractBase implements DriverInterface
      */
     protected $config = [];
 
+    /**
+     * Set a cache storage object.
+     *
+     * @param StorageInterface $cache Cache storage interface
+     *
+     * @return void
+     */
+    public function setCacheStorage(StorageInterface $cache = null)
+    {
+        $this->cache = $cache;
+    }
+
     /**
      * Set configuration.
      *
@@ -61,4 +88,71 @@ abstract class AbstractBase implements DriverInterface
     {
         $this->config = $config;
     }
+
+    /**
+     * Add instance-specific context to a cache key suffix to ensure that
+     * multiple drivers don't accidentally share values in the cache.
+     * This implementation works anywhere but can be overridden with something more
+     * performant.
+     *
+     * @param string $key Cache key suffix
+     *
+     * @return string
+     */
+    protected function formatCacheKey($key)
+    {
+        return get_class($this) . '-' . md5(json_encode($this->config) . "|$key");
+    }
+
+    /**
+     * Helper function for fetching cached data.
+     * Data is cached for up to $this->cacheLifetime seconds so that it would be
+     * faster to process e.g. requests where multiple calls to the backend are made.
+     *
+     * @param string $key Cache entry key
+     *
+     * @return mixed|null Cached entry or null if not cached or expired
+     */
+    protected function getCachedData($key)
+    {
+        // No cache object, no cached results!
+        if (null === $this->cache) {
+            return null;
+        }
+
+        $fullKey = $this->formatCacheKey($key);
+        $item = $this->cache->getItem($fullKey);
+        if (null !== $item) {
+            // Return value if still valid:
+            if (time() - $item['time'] < $this->cacheLifetime) {
+                return $item['entry'];
+            }
+            // Clear expired item from cache:
+            $this->cache->removeItem($fullKey);
+        }
+        return null;
+    }
+
+    /**
+     * Helper function for storing cached data.
+     * Data is cached for up to $this->cacheLifetime seconds so that it would be
+     * faster to process e.g. requests where multiple calls to the backend are made.
+     *
+     * @param string $key   Cache entry key
+     * @param mixed  $entry Entry to be cached
+     *
+     * @return void
+     */
+    protected function putCachedData($key, $entry)
+    {
+        // Don't write to cache if we don't have a cache!
+        if (null === $this->cache) {
+            return;
+        }
+        $item = [
+            'time' => time(),
+            'entry' => $entry
+        ];
+        $this->cache->addItem($this->formatCacheKey($key), $item);
+    }
 }
diff --git a/module/VuFind/src/VuFind/ILS/Driver/Factory.php b/module/VuFind/src/VuFind/ILS/Driver/Factory.php
index 6858f0edbb8..ec499e857b0 100644
--- a/module/VuFind/src/VuFind/ILS/Driver/Factory.php
+++ b/module/VuFind/src/VuFind/ILS/Driver/Factory.php
@@ -172,9 +172,13 @@ class Factory
     public static function getVoyagerRestful(ServiceManager $sm)
     {
         $ils = $sm->getServiceLocator()->get('VuFind\ILSHoldSettings');
-        return new VoyagerRestful(
+        $vr = new VoyagerRestful(
             $sm->getServiceLocator()->get('VuFind\DateConverter'),
             $ils->getHoldsMode(), $ils->getTitleHoldsMode()
         );
+        $vr->setCacheStorage(
+            $sm->getServiceLocator()->get('VuFind\CacheManager')->getCache('object')
+        );
+        return $vr;
     }
 }
\ No newline at end of file
diff --git a/module/VuFind/src/VuFind/ILS/Driver/VoyagerRestful.php b/module/VuFind/src/VuFind/ILS/Driver/VoyagerRestful.php
index bf1a71dc024..b3d8ef1d532 100644
--- a/module/VuFind/src/VuFind/ILS/Driver/VoyagerRestful.php
+++ b/module/VuFind/src/VuFind/ILS/Driver/VoyagerRestful.php
@@ -5,7 +5,7 @@
  * PHP version 5
  *
  * Copyright (C) Villanova University 2007.
- * Copyright (C) The National Library of Finland 2014.
+ * Copyright (C) The National Library of Finland 2014-2016.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2,
@@ -31,8 +31,7 @@
  */
 namespace VuFind\ILS\Driver;
 use PDO, PDOException, VuFind\Exception\Date as DateException,
-    VuFind\Exception\ILS as ILSException,
-    Zend\Session\Container as SessionContainer;
+    VuFind\Exception\ILS as ILSException;
 
 /**
  * Voyager Restful ILS Driver
@@ -127,13 +126,6 @@ class VoyagerRestful extends Voyager implements \VuFindHttp\HttpServiceAwareInte
      */
     protected $titleHoldsMode;
 
-    /**
-     * Container for storing cached ILS data.
-     *
-     * @var SessionContainer
-     */
-    protected $session;
-
     /**
      * Web Services cookies. Required for at least renewals (for JSESSIONID) as
      * documented at http://www.exlibrisgroup.org/display/VoyagerOI/Renew
@@ -306,9 +298,6 @@ class VoyagerRestful extends Voyager implements \VuFindHttp\HttpServiceAwareInte
         $this->allowCancelingAvailableRequests
             = isset($this->config['Holds']['allowCancelingAvailableRequests'])
             ? $this->config['Holds']['allowCancelingAvailableRequests'] : true;
-
-        // Establish a namespace in the session for persisting cached data
-        $this->session = new SessionContainer('VoyagerRestful_' . $this->dbName);
     }
 
     /**
@@ -334,45 +323,19 @@ class VoyagerRestful extends Voyager implements \VuFindHttp\HttpServiceAwareInte
     }
 
     /**
-     * Helper function for fetching cached data.
-     * Data is cached for up to 30 seconds so that it would be faster to process
-     * e.g. requests where multiple calls to the backend are made.
-     *
-     * @param string $id Cache entry id
-     *
-     * @return mixed|null Cached entry or null if not cached or expired
-     */
-    protected function getCachedData($id)
-    {
-        if (isset($this->session->cache[$id])) {
-            $item = $this->session->cache[$id];
-            if (time() - $item['time'] < 30) {
-                return $item['entry'];
-            }
-            unset($this->session->cache[$id]);
-        }
-        return null;
-    }
-
-    /**
-     * Helper function for storing cached data.
-     * Data is cached for up to 30 seconds so that it would be faster to process
-     * e.g. requests where multiple calls to the backend are made.
+     * Add instance-specific context to a cache key suffix (to ensure that
+     * multiple drivers don't accidentally share values in the cache.
      *
-     * @param string $id    Cache entry id
-     * @param mixed  $entry Entry to be cached
+     * @param string $key Cache key suffix
      *
-     * @return void
+     * @return string
      */
-    protected function putCachedData($id, $entry)
+    protected function formatCacheKey($key)
     {
-        if (!isset($this->session->cache)) {
-            $this->session->cache = [];
-        }
-        $this->session->cache[$id] = [
-            'time' => time(),
-            'entry' => $entry
-        ];
+        // Override the base class formatting with Voyager-specific details
+        // to ensure proper caching in a MultiBackend environment.
+        return 'VoyagerRestful-'
+            . md5("{$this->ws_host}|{$this->ws_dbKey}|$key");
     }
 
     /**
@@ -1225,7 +1188,7 @@ class VoyagerRestful extends Voyager implements \VuFindHttp\HttpServiceAwareInte
      */
     protected function checkAccountBlocks($patronId)
     {
-        $cacheId = "blocks_$patronId";
+        $cacheId = "blocks|$patronId";
         $blockReason = $this->getCachedData($cacheId);
         if (null === $blockReason) {
             // Build Hierarchy
@@ -2557,8 +2520,8 @@ EOT;
      */
     protected function getUBRequestDetails($id, $patron)
     {
-        $requestId = "ub_{$id}_" . $patron['id'];
-        $data = $this->getCachedData($requestId);
+        $cacheId = "ub|$id|{$patron['id']}";
+        $data = $this->getCachedData($cacheId);
         if (!empty($data)) {
             return $data;
         }
@@ -2567,13 +2530,13 @@ EOT;
             $this->debug(
                 "getUBRequestDetails: no prefix in patron id '{$patron['id']}'"
             );
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
         list($source, $patronId) = explode('.', $patron['id'], 2);
         if (!isset($this->config['ILLRequestSources'][$source])) {
             $this->debug("getUBRequestDetails: source '$source' unknown");
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
 
@@ -2615,11 +2578,7 @@ EOT;
         );
 
         if ($response === false) {
-            $this->session->UBDetails[$requestId] = [
-                'time' => time(),
-                'data' => false
-            ];
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
         // Process
@@ -2631,7 +2590,7 @@ EOT;
         );
         foreach ($response->xpath('//ser:message') as $message) {
             // Any message means a problem, right?
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
         $requestCount = count(
@@ -2639,7 +2598,7 @@ EOT;
         );
         if ($requestCount == 0) {
             // UB request not available
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
 
@@ -2676,7 +2635,7 @@ EOT;
         );
 
         if ($response === false) {
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
         // Process
@@ -2688,7 +2647,7 @@ EOT;
         );
         foreach ($response->xpath('//ser:message') as $message) {
             // Any message means a problem, right?
-            $this->putCachedData($requestId, false);
+            $this->putCachedData($cacheId, false);
             return false;
         }
         $items = [];
@@ -2737,7 +2696,7 @@ EOT;
             'locations' => $locations,
             'requiredBy' => $requiredByDate
         ];
-        $this->putCachedData($requestId, $results);
+        $this->putCachedData($cacheId, $results);
         return $results;
     }
 
-- 
GitLab