jenkins-bot has submitted this change and it was merged.

Change subject: SMW\Store\Collector eliminate duplicate code and split 
responsibility
......................................................................


SMW\Store\Collector eliminate duplicate code and split responsibility

Cache handling was moved into its own SMW\ResultCacheMapper class since
its is only a by-product of a Collector. A Collector is has no direct
responsibility about how results are stored or managed.

SMW\ResultCacheMapper
Code coverage: 100%
CRAP: 13

Change-Id: I40a1548e370e1c4f16c0a1ad6e6621d58334ad5c
---
M docs/doxygen.group.definitions.php
A includes/ArrayAccessor.php
M includes/Setup.php
A includes/cache/ResultCacheMapper.php
M includes/storage/Collector.php
M includes/storage/SQLStore/StatisticsCollector.php
M includes/storage/SQLStore/UnusedPropertiesCollector.php
M includes/storage/SQLStore/WantedPropertiesCollector.php
A tests/phpunit/includes/ArrayAccessorTest.php
A tests/phpunit/includes/cache/ResultCacheMapperTest.php
M tests/phpunit/includes/specials/UnusedPropertiesPageTest.php
M tests/phpunit/includes/specials/WantedPropertiesPageTest.php
M tests/phpunit/includes/storage/sqlstore/UnusedPropertiesCollectorTest.php
M tests/phpunit/includes/storage/sqlstore/WantedPropertiesCollectorTest.php
14 files changed, 802 insertions(+), 279 deletions(-)

Approvals:
  Mwjames: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/docs/doxygen.group.definitions.php 
b/docs/doxygen.group.definitions.php
index 1822c35..e4503a7 100644
--- a/docs/doxygen.group.definitions.php
+++ b/docs/doxygen.group.definitions.php
@@ -18,6 +18,13 @@
  */
 
 /**
+ * This group contains members that are related to accessors
+ *
+ * @defgroup Accessor Accessor
+ * @ingroup SMW
+ */
+
+/**
  * This group contains members that are related to parser hooks and functions
  *
  * @defgroup ParserFunction ParserFunction
diff --git a/includes/ArrayAccessor.php b/includes/ArrayAccessor.php
new file mode 100644
index 0000000..69e5802
--- /dev/null
+++ b/includes/ArrayAccessor.php
@@ -0,0 +1,136 @@
+<?php
+
+namespace SMW;
+
+use ArrayObject;
+use InvalidArgumentException;
+
+/**
+ * Interface specifying access to an object
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @since 1.9
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @author mwjames
+ */
+interface Accessor {
+
+       /**
+        * Returns if a specific key can be accessed
+        *
+        * @since 1.9
+        *
+        * @param  mixed $key specific key
+        *
+        * @return boolean
+        */
+       public function has( $key );
+
+       /**
+        * Returns a value for a specific key
+        *
+        * @since 1.9
+        *
+        * @param  mixed $key specific key
+        *
+        * @return mixed
+        */
+       public function get( $key );
+
+       /**
+        * Sets a value for a specific key
+        *
+        * @since 1.9
+        *
+        * @param  mixed $key
+        * @param  mixed $value
+        *
+        * @return boolean
+        */
+       public function set( $key, $value );
+
+}
+
+/**
+ * This class enables access to an arbitrary array
+ *
+ * @ingroup Accessor
+ */
+class ArrayAccessor extends ArrayObject implements Accessor {
+
+       /**
+        * Returns if a specified key is set or not
+        *
+        * @since 1.9
+        *
+        * @param mixed $key
+        *
+        * @return boolean
+        */
+       public function has( $key ) {
+               return $this->offsetExists( $key );
+       }
+
+       /**
+        * Exports the ArrayObject to an array
+        *
+        * @since 1.9
+        *
+        * @return array
+        */
+       public function toArray() {
+               return $this->getArrayCopy();
+       }
+
+       /**
+        *  Sets the value to a specific key
+        *
+        * @since 1.9
+        *
+        * @param string $key
+        * @param mixed $value
+        *
+        * @return ArrayAccessor
+        */
+       public function set( $key, $value ) {
+               $this[$key] = $value;
+               return $this;
+       }
+
+       /**
+        * Returns a container value
+        *
+        * @since 1.9
+        *
+        * @param string $key
+        *
+        * @return mixed
+        * @throws InvalidArgumentException
+        */
+       public function get( $key ) {
+
+               if ( !( $this->has( $key ) ) ) {
+                       throw new InvalidArgumentException( "'{$key}' is 
unknown" );
+               }
+
+               return $this->offsetGet( $key );
+       }
+
+}
diff --git a/includes/Setup.php b/includes/Setup.php
index 347a1ca..66c873b 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -151,7 +151,12 @@
        $wgAutoloadClasses['SMW\NamespaceExaminer']     = $incDir . 
'NamespaceExaminer.php';
        $wgAutoloadClasses['SMW\Profiler']              = $incDir . 
'Profiler.php';
 
-       $wgAutoloadClasses['SMW\CacheHandler']             = $incDir . 
'/handlers/CacheHandler.php';
+       $wgAutoloadClasses['SMW\Accessor']              = $incDir . 
'ArrayAccessor.php';
+       $wgAutoloadClasses['SMW\ArrayAccessor']         = $incDir . 
'ArrayAccessor.php';
+
+       $wgAutoloadClasses['SMW\CacheHandler']          = $incDir . 
'/handlers/CacheHandler.php';
+
+       $wgAutoloadClasses['SMW\ResultCacheMapper']     = $incDir . 
'/cache/ResultCacheMapper.php';
 
        // Formatters
        $wgAutoloadClasses['SMW\ArrayFormatter']             = $incDir . 
'formatters/ArrayFormatter.php';
diff --git a/includes/cache/ResultCacheMapper.php 
b/includes/cache/ResultCacheMapper.php
new file mode 100644
index 0000000..4da5872
--- /dev/null
+++ b/includes/cache/ResultCacheMapper.php
@@ -0,0 +1,159 @@
+<?php
+
+namespace SMW;
+
+use InvalidArgumentException;
+use MWTimestamp;
+
+/**
+ * Handling of cached results
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @since 1.9
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @author mwjames
+ */
+
+/**
+ * Handling of cached results
+ *
+ * @ingroup SMW
+ */
+class ResultCacheMapper {
+
+       /** @var ArrayAccessor */
+       protected $cacheAccessor;
+
+       /**
+        * @since 1.9
+        *
+        * @param ArrayAccessor $cacheAccessor
+        */
+       public function __construct( ArrayAccessor $cacheAccessor ) {
+               $this->cacheAccessor = $cacheAccessor;
+       }
+
+       /**
+        * Service function that fetches and returns results from cache
+        *
+        * @note Id generation is the responsibility of the object consumer 
where
+        * for example "requestOptions" is part of the cache key allowing for
+        * each individual requestOption setting (limit, offset) to retrieve the
+        * cache object independently
+        *
+        * @note Use json_encode() to stringify the requestOptions object as
+        * it is comparable faster than serialize()
+        *
+        * @note The cache key itself is being represented as mapper:< md5 hash 
>
+        *
+        * @since 1.9
+        *
+        * @return array|false
+        */
+       public function fetchFromCache() {
+
+               $result = $this->getCache()
+                       ->setCacheEnabled( $this->cacheAccessor->get( 'enabled' 
) )
+                       ->key( 'mapper', md5( $this->cacheAccessor->get( 'id' ) 
) )
+                       ->get();
+
+               return $result ? $this->mapping( $result ) : $result;
+       }
+
+       /**
+        * Service function that stores results as a cache object
+        *
+        * @note The cache object stores the time and its results as serialized
+        * array in order to allow any arbitrary content to be cacheable
+        *
+        * @note Results are serialized as they can contain an array of objects
+        * where when retrieved from cache those objects are going to be
+        * unserialized to restore the original object
+        *
+        * @since 1.9
+        *
+        * @param array $results
+        */
+       public function recache( array $results ) {
+
+               $this->getCache()
+                       ->setCacheEnabled( $this->cacheAccessor->get( 'enabled' 
) && $results !== array() )
+                       ->set( array( 'time' => $this->getTimestamp(), 'result' 
=> serialize( $results ) ), $this->cacheAccessor->get( 'expiry' )
+               );
+       }
+
+       /**
+        * Returns a CacheHandler instance
+        *
+        * @since 1.9
+        *
+        * @return CacheHandler
+        */
+       public function getCacheDate() {
+               return $this->cacheAccessor->has( 'cacheDate' ) ? 
$this->cacheAccessor->get( 'cacheDate' ) : null;
+       }
+
+       /**
+        * Returns a CacheHandler instance
+        *
+        * @since 1.9
+        *
+        * @return CacheHandler
+        */
+       protected function getCache() {
+               return CacheHandler::newFromId( $this->cacheAccessor->get( 
'type' ) );
+       }
+
+       /**
+        * Service function that remaps an array of cached content and returns
+        * unserialized objects and the timestamp of the cached content
+        *
+        * @since 1.9
+        *
+        * @param array $resultCache
+        *
+        * @return array
+        */
+       protected function mapping( array $resultCache ) {
+               $this->cacheAccessor->set( 'cacheDate', isset( 
$resultCache['time'] ) ? $resultCache['time'] : null );
+               return isset( $resultCache['result'] ) ? unserialize( 
$resultCache['result'] ) : array();
+       }
+
+       /**
+        * Returns a timestamp
+        *
+        * @todo Apparently MW 1.19 does not have a MWTimestamp class, please
+        * remove this clutter as soon as MW 1.19 is not supported any longer
+        *
+        * @since 1.9
+        *
+        * @return integer
+        */
+       protected function getTimestamp() {
+               if ( class_exists( 'MWTimestamp' ) ) {
+                       $timestamp = new MWTimestamp();
+                       return $timestamp->getTimestamp( TS_UNIX );
+               } else {
+                       // @codeCoverageIgnoreStart
+                       return wfTimestamp( TS_UNIX );
+                       // @codeCoverageIgnoreEnd
+               }
+       }
+}
diff --git a/includes/storage/Collector.php b/includes/storage/Collector.php
index b98be03..f8eb439 100644
--- a/includes/storage/Collector.php
+++ b/includes/storage/Collector.php
@@ -2,10 +2,11 @@
 
 namespace SMW\Store;
 
-use SMW\CacheHandler;
+use SMW\ResultCacheMapper;
 use SMW\DIProperty;
 use SMW\Settings;
 
+use InvalidArgumentException;
 use MWTimestamp;
 
 /**
@@ -30,72 +31,124 @@
  * @since 1.9
  *
  * @file
- * @ingroup SMW
  *
- * @licence GNU GPL v2+
+ * @license GNU GPL v2+
  * @author mwjames
  */
 interface Collectible {}
 
 /**
- * Collectors base class
+ * Collector base class
  *
- * @ingroup SMW
+ * @ingroup Store
  */
 abstract class Collector implements Collectible {
 
-       /** @var Store */
-       protected $store;
+       /** @var array */
+       protected $results = array();
 
-       /** @var Settings */
-       protected $settings;
+       /** @var SMWRequestOptions */
+       protected $requestOptions = null;
 
        /** @var boolean */
        protected $isCached = false;
+
+       /** @var string */
+       protected $cacheDate = null;
 
        /**
         * Collects and returns information in an associative array
         *
         * @since 1.9
         */
-       public abstract function getResults();
+       public function getResults() {
+
+               $resultCache = new ResultCacheMapper( $this->cacheAccessor() );
+
+               $results = $resultCache->fetchFromCache();
+
+               if ( $results ) {
+
+                       $this->isCached  = true;
+                       $this->results   = $results;
+                       $this->cacheDate = $resultCache->getCacheDate();
+                       wfDebug( get_called_class() . ' served from cache' . 
"\n" );
+
+               } else {
+
+                       $this->results  = $this->doCollect();
+                       $this->isCached = false;
+                       $resultCache->recache( $this->results );
+               }
+
+               return $this->results;
+       }
 
        /**
-        * Returns if the results are cached
+        * Set request options
         *
         * @since 1.9
+        *
+        * @param SMWRequestOptions $requestOptions
+        *
+        * @return Collector
         */
-       public abstract function isCached();
+       public function setRequestOptions( $requestOptions ) {
+               $this->requestOptions = $requestOptions;
+               return $this;
+       }
 
        /**
-        * Returns a timestamp
+        * Returns whether or not results have been cached
         *
-        * @todo Apparently MW 1.19 does not have a MWTimestamp class, please
-        * remove this clutter as soon as MW 1.19 is not supported any longer
+        * @since 1.9
+        *
+        * @return boolean
+        */
+       public function isCached() {
+               return $this->isCached;
+       }
+
+       /**
+        * In case results were cached, it returns the timestamp of the cached
+        * object
+        *
+        * @since 1.9
+        *
+        * @return string|null
+        */
+       public function getCacheDate() {
+               return $this->cacheDate;
+       }
+
+       /**
+        * Returns number of available results
         *
         * @since 1.9
         *
         * @return integer
         */
-       public function getTimestamp() {
-               if ( class_exists( 'MWTimestamp' ) ) {
-                       $timestamp = new MWTimestamp();
-                       return $timestamp->getTimestamp( TS_UNIX );
-               } else {
-                       return wfTimestamp( TS_UNIX );
-               }
+       public function getCount() {
+               return count( $this->results );
        }
 
        /**
-        * Returns a CacheHandler instance
+        * Sub-class is responsible for returning an associative array
         *
         * @since 1.9
         *
-        * @return CacheHandler
+        * @return array
         */
-       public function getCache() {
-               return CacheHandler::newFromId( $this->settings->get( 
'smwgCacheType' ) );
-       }
+       protected abstract function doCollect();
+
+       /**
+        * Sub-class is responsible for returning a ArrayAccessor object
+        *
+        * @since 1.9
+        *
+        * @return ArrayAccessor
+        */
+       protected abstract function cacheAccessor();
 
        /**
         * Returns table definition for a given property type
diff --git a/includes/storage/SQLStore/StatisticsCollector.php 
b/includes/storage/SQLStore/StatisticsCollector.php
index 0200116..3b70ac8 100644
--- a/includes/storage/SQLStore/StatisticsCollector.php
+++ b/includes/storage/SQLStore/StatisticsCollector.php
@@ -3,6 +3,8 @@
 namespace SMW\SQLStore;
 
 use SMW\Store\Collector;
+
+use SMW\ArrayAccessor;
 use SMW\DIProperty;
 use SMW\Settings;
 use SMW\Store;
@@ -43,11 +45,14 @@
  */
 class StatisticsCollector extends Collector {
 
+       /** @var Store */
+       protected $store;
+
+       /** @var Settings */
+       protected $settings;
+
        /** @var DatabaseBase */
        protected $dbConnection;
-
-       /** @var array */
-       protected $results = array();
 
        /**
         * // FIXME The store itself should know which database connection is 
being
@@ -67,12 +72,11 @@
        }
 
        /**
-        * Factory method for immediate instantiation of a StatisticsCollector 
object
+        * Factory method for an immediate instantiation of a 
StatisticsCollector object
         *
         * @par Example:
         * @code
         * $statistics = \SMW\SQLStore\StatisticsCollector::newFromStore( 
$store )
-        *
         * $statistics->getResults();
         * @endcode
         *
@@ -93,7 +97,27 @@
        }
 
        /**
-        * Collects and returns statistical information as an associative array
+        * Set-up details used for the Cache instantiation
+        *
+        * @see $smwgStatisticsCache
+        * @see $smwgStatisticsCacheExpiry
+        *
+        * @since 1.9
+        *
+        * @return array
+        */
+       protected function cacheAccessor() {
+
+               return new ArrayAccessor( array(
+                       'id'      => 'smwgStatisticsCache' . json_encode( 
$this->requestOptions ),
+                       'type'    => $this->settings->get( 'smwgCacheType' ),
+                       'enabled' => $this->settings->get( 
'smwgStatisticsCache' ),
+                       'expiry'  => $this->settings->get( 
'smwgStatisticsCacheExpiry' )
+               ) );
+       }
+
+       /**
+        * Collects statistical information as an associative array
         * with the following keys:
         *
         * - 'PROPUSES': Number of property instances (value assignments) in 
the dbConnection
@@ -106,57 +130,23 @@
         * - 'SUBOBJECTS': Number of declared subobjects
         * - 'QUERYFORMATS': Array of used formats and its usage count
         *
-        * @note Results are cacheable if the necessary settings are available
-        *
-        * @see $smwgStatisticsCache
-        * @see $smwgStatisticsCacheExpiry
-        *
         * @since 1.9
         *
-        * @return array
+        * @return DIProperty[]
         */
-       public function getResults() {
+       protected function doCollect() {
 
-               $results = $this->getCache()->setCacheEnabled( 
$this->settings->get( 'smwgStatisticsCache' ) )
-                       ->key( 'collector', 'stats', 'store' )
-                       ->get();
-
-               if ( $results ) {
-
-                       $this->isCached = true;
-                       $this->results = $results;
-                       wfDebug( __METHOD__ . ' statistics served from cache' . 
"\n");
-
-               } else {
-
-                       $this->results = array(
-                               'OWNPAGE' => $this->getPropertyPageCount(),
-                               'QUERY' => $this->getQueryCount(),
-                               'QUERYSIZE' => $this->getQuerySize(),
-                               'QUERYFORMATS' => $this->getQueryFormatsCount(),
-                               'CONCEPTS' => $this->getConceptCount(),
-                               'SUBOBJECTS' => $this->getSubobjectCount(),
-                               'DECLPROPS' => 
$this->getDeclaredPropertiesCount(),
-                               'PROPUSES' => $this->getPropertyUsageCount(),
-                               'USEDPROPS' => $this->getUsedPropertiesCount()
-                       );
-
-                       $this->isCached = false;
-                       $this->getCache()->set( $this->results, 
$this->settings->get( 'smwgStatisticsCacheExpiry' ) );
-               }
-
-               return $this->results;
-       }
-
-       /**
-        * Returns if the current collection has been recalled from cache
-        *
-        * @since 1.9
-        *
-        * @return boolean
-        */
-       public function isCached() {
-               return $this->isCached;
+               return array(
+                       'OWNPAGE' => $this->getPropertyPageCount(),
+                       'QUERY' => $this->getQueryCount(),
+                       'QUERYSIZE' => $this->getQuerySize(),
+                       'QUERYFORMATS' => $this->getQueryFormatsCount(),
+                       'CONCEPTS' => $this->getConceptCount(),
+                       'SUBOBJECTS' => $this->getSubobjectCount(),
+                       'DECLPROPS' => $this->getDeclaredPropertiesCount(),
+                       'PROPUSES' => $this->getPropertyUsageCount(),
+                       'USEDPROPS' => $this->getUsedPropertiesCount()
+               );
        }
 
        /**
diff --git a/includes/storage/SQLStore/UnusedPropertiesCollector.php 
b/includes/storage/SQLStore/UnusedPropertiesCollector.php
index 43377d1..ee152af 100644
--- a/includes/storage/SQLStore/UnusedPropertiesCollector.php
+++ b/includes/storage/SQLStore/UnusedPropertiesCollector.php
@@ -5,7 +5,7 @@
 use SMW\Store\Collector;
 
 use SMW\InvalidPropertyException;
-use SMW\CacheHandler;
+use SMW\ArrayAccessor;
 use SMW\DIProperty;
 use SMW\Settings;
 use SMW\Profiler;
@@ -46,18 +46,19 @@
 /**
  * Collects unused properties from a store entity
  *
- * @ingroup SMW
+ * @ingroup Collector
+ * @ingroup SQLStore
  */
 class UnusedPropertiesCollector extends Collector {
 
+       /** @var Store */
+       protected $store;
+
+       /** @var Settings */
+       protected $settings;
+
        /** @var DatabaseBase */
        protected $dbConnection;
-
-       /** @var SMWRequestOptions */
-       protected $requestOptions = null;
-
-       /** @var array */
-       protected $results = array();
 
        /**
         * @since 1.9
@@ -73,11 +74,12 @@
        }
 
        /**
-        * Factory method for immediate instantiation of a 
UnusedPropertiesCollector object
+        * Factory method for an immediate instantiation of a 
UnusedPropertiesCollector object
         *
         * @par Example:
         * @code
-        *  $properties = 
\SMW\SQLStore\UnusedPropertiesCollector::newFromStore( $store )->getResults();
+        *  $properties = 
\SMW\SQLStore\UnusedPropertiesCollector::newFromStore( $store )
+        *  $properties->getResults();
         * @endcode
         *
         * @since 1.9
@@ -97,75 +99,23 @@
        }
 
        /**
-        * Collects and returns unused properties
+        * Set-up details used for the Cache instantiation
         *
         * @see $smwgUnusedPropertiesCache
         * @see $smwgUnusedPropertiesCacheExpiry
         *
         * @since 1.9
         *
-        * @return DIProperty[]
+        * @return array
         */
-       public function getResults() {
+       protected function cacheAccessor() {
 
-               $useCache = $this->settings->get( 'smwgUnusedPropertiesCache' );
-               $results  = $this->getCache()->setCacheEnabled( $useCache )
-                       ->key( 'collector', md5( 'unused-' . serialize( 
$this->requestOptions ) ) )
-                       ->get();
-
-               if ( $results ) {
-
-                       $this->isCached = true;
-                       $this->results  = isset( $results['data'] ) ? 
unserialize( $results['data'] ) : array();
-                       wfDebug( __METHOD__ . ' served from cache' . "\n" );
-
-               } else {
-
-                       $this->isCached = false;
-                       $this->results  = $this->getUnusedProperties();
-                       $this->getCache()->setCacheEnabled( $useCache && 
$this->results !== array() )->set(
-                               array( 'time' => $this->getTimestamp(), 'data' 
=> serialize( $this->results ) ),
-                               $this->settings->get( 
'smwgUnusedPropertiesCacheExpiry' )
-                       );
-               }
-
-               return $this->results;
-       }
-
-       /**
-        * Whether return results are cached
-        *
-        * @since 1.9
-        *
-        * @return boolean
-        */
-       public function isCached() {
-               return $this->isCached;
-       }
-
-       /**
-        * Returns number of available results
-        *
-        * @since 1.9
-        *
-        * @return integer
-        */
-       public function count() {
-               return count( $this->results );
-       }
-
-       /**
-        * Set request options
-        *
-        * @since 1.9
-        *
-        * @param SMWRequestOptions $requestOptions
-        *
-        * @return UnusedPropertiesCollector
-        */
-       public function setRequestOptions( $requestOptions ) {
-               $this->requestOptions = $requestOptions;
-               return $this;
+               return new ArrayAccessor( array(
+                       'id'      => 'smwgUnusedPropertiesCache' . json_encode( 
$this->requestOptions ),
+                       'type'    => $this->settings->get( 'smwgCacheType' ),
+                       'enabled' => $this->settings->get( 
'smwgUnusedPropertiesCache' ),
+                       'expiry'  => $this->settings->get( 
'smwgUnusedPropertiesCacheExpiry' )
+               ) );
        }
 
        /**
@@ -175,7 +125,7 @@
         *
         * @return DIProperty[]
         */
-       protected function getUnusedProperties() {
+       protected function doCollect() {
                Profiler::In( __METHOD__ );
 
                $result = array();
diff --git a/includes/storage/SQLStore/WantedPropertiesCollector.php 
b/includes/storage/SQLStore/WantedPropertiesCollector.php
index f413a29..5924fbb 100644
--- a/includes/storage/SQLStore/WantedPropertiesCollector.php
+++ b/includes/storage/SQLStore/WantedPropertiesCollector.php
@@ -3,6 +3,8 @@
 namespace SMW\SQLStore;
 
 use SMW\Store\Collector;
+
+use SMW\ArrayAccessor;
 use SMW\DIProperty;
 use SMW\Profiler;
 use SMW\Settings;
@@ -45,14 +47,14 @@
  */
 class WantedPropertiesCollector extends Collector {
 
+       /** @var Store */
+       protected $store;
+
+       /** @var Settings */
+       protected $settings;
+
        /** @var DatabaseBase */
        protected $dbConnection;
-
-       /** @var SMWRequestOptions */
-       protected $requestOptions = null;
-
-       /** @var array */
-       protected $results = array();
 
        /**
         * @since 1.9
@@ -68,11 +70,12 @@
        }
 
        /**
-        * Factory method for immediate instantiation of a 
WantedPropertiesCollector object
+        * Factory method for an immediate instantiation of a 
WantedPropertiesCollector object
         *
         * @par Example:
         * @code
-        *  $properties = 
\SMW\SQLStore\WantedPropertiesCollector::newFromStore( $store )->getResults();
+        *  $properties = 
\SMW\SQLStore\WantedPropertiesCollector::newFromStore( $store )
+        *  $properties->getResults();
         * @endcode
         *
         * @since 1.9
@@ -92,15 +95,46 @@
        }
 
        /**
-        * Collects and returns wanted properties
+        * Set-up details used for the Cache instantiation
         *
-        * @par Example:
-        * @code
-        *  $wantedProperties = 
\SMW\SQLStore\WantedPropertiesCollector::newFromStore( $store );
+        * @since 1.9
         *
-        *  $results = $wantedProperties->setRequestOptions( null 
)->getResults()
-        *  $count = $wantedProperties->count()
-        * @endcode
+        * @return array
+        */
+       protected function cacheAccessor() {
+
+               return new ArrayAccessor( array(
+                       'id'      => 'smwgWantedPropertiesCache' . 
$this->settings->get( 'smwgPDefaultType' ) . json_encode( $this->requestOptions 
),
+                       'type'    => $this->settings->get( 'smwgCacheType' ),
+                       'enabled' => $this->settings->get( 
'smwgWantedPropertiesCache' ),
+                       'expiry'  => $this->settings->get( 
'smwgWantedPropertiesCacheExpiry' )
+               ) );
+       }
+
+       /**
+        * Returns unused properties
+        *
+        * @since 1.9
+        *
+        * @return DIProperty[]
+        */
+       protected function doCollect() {
+
+               $result = array();
+
+               // Wanted Properties must have the default type
+               $this->propertyTables = $this->getPropertyTables( 
$this->settings->get( 'smwgPDefaultType' ) );
+
+               // anything else would be crazy, but let's fail gracefully even 
if the whole world is crazy
+               if ( !$this->propertyTables->isFixedPropertyTable() ) {
+                       $result = $this->doQuery();
+               }
+
+               return $result;
+       }
+
+       /**
+        * Returns wanted properties
         *
         * @note This function is very resource intensive and needs to be 
cached on
         * medium/large wikis.
@@ -109,85 +143,7 @@
         *
         * @return DIProperty[]
         */
-       public function getResults() {
-
-               $type = $this->settings->get( 'smwgPDefaultType' );
-
-               $useCache = $this->settings->get( 'smwgWantedPropertiesCache' );
-               $results  = $this->getCache()->setCacheEnabled( $useCache )
-                       ->key( 'collector', md5( 'wanted-' . $type . serialize( 
$this->requestOptions ) ) )
-                       ->get();
-
-               if ( $results ) {
-
-                       $this->isCached = true;
-                       $this->results  = isset( $results['data'] ) ? 
unserialize( $results['data'] ) : array();
-                       wfDebug( __METHOD__ . ' served from cache' . "\n");
-
-               } else {
-
-                       // Wanted Properties must have the default type
-                       $this->propertyTables = $this->getPropertyTables( $type 
);
-
-                       // anything else would be crazy, but let's fail 
gracefully even if the whole world is crazy
-                       if ( !$this->propertyTables->isFixedPropertyTable() ) {
-                               $this->results = $this->getWantedProperties();
-                       }
-
-                       $this->isCached = false;
-                       $this->getCache()->setCacheEnabled( $useCache && 
$this->results !== array() )->set(
-                               array( 'time' => $this->getTimestamp(), 'data' 
=> serialize( $this->results ) ),
-                               $this->settings->get( 
'smwgWantedPropertiesCacheExpiry' )
-                       );
-               }
-
-               return $this->results;
-       }
-
-       /**
-        * Whether return results are cached
-        *
-        * @since 1.9
-        *
-        * @return boolean
-        */
-       public function isCached() {
-               return $this->isCached;
-       }
-
-       /**
-        * Returns number of available results
-        *
-        * @since 1.9
-        *
-        * @return integer
-        */
-       public function count() {
-               return count( $this->results );
-       }
-
-       /**
-        * Set options
-        *
-        * @since 1.9
-        *
-        * @param SMWRequestOptions $requestOptions
-        *
-        * @return WantedPropertiesCollector
-        */
-       public function setRequestOptions( $requestOptions ) {
-               $this->requestOptions = $requestOptions;
-               return $this;
-       }
-
-       /**
-        * Returns wanted properties
-        *
-        * @since 1.9
-        *
-        * @return DIProperty[]
-        */
-       protected function getWantedProperties() {
+       protected function doQuery() {
                Profiler::In( __METHOD__ );
 
                $result = array();
diff --git a/tests/phpunit/includes/ArrayAccessorTest.php 
b/tests/phpunit/includes/ArrayAccessorTest.php
new file mode 100644
index 0000000..7c0c77d
--- /dev/null
+++ b/tests/phpunit/includes/ArrayAccessorTest.php
@@ -0,0 +1,110 @@
+<?php
+
+namespace SMW\Test;
+
+use SMW\ArrayAccessor;
+
+/**
+ * Tests for the ArrayAccessor class
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @since 1.9
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @author mwjames
+ */
+
+/**
+ * @covers \SMW\ArrayAccessor
+ *
+ * @ingroup Test
+ *
+ * @group SMW
+ * @group SMWExtension
+ */
+class ArrayAccessorTest extends SemanticMediaWikiTestCase {
+
+       /**
+        * Returns the name of the class to be tested
+        *
+        * @return string|false
+        */
+       public function getClass() {
+               return '\SMW\ArrayAccessor';
+       }
+
+       /**
+        * Helper method that returns a ArrayAccessor object
+        *
+        * @since 1.9
+        *
+        * @param $result
+        *
+        * @return ArrayAccessor
+        */
+       private function getInstance( array $setup = array() ) {
+               return new ArrayAccessor( $setup );
+       }
+
+       /**
+        * @test ArrayAccessor::__construct
+        *
+        * @since 1.9
+        */
+       public function testConstructor() {
+               $this->assertInstanceOf( $this->getClass(), 
$this->getInstance() );
+       }
+
+       /**
+        * @test ArrayAccessor::get
+        *
+        * @since 1.9
+        */
+       public function testInvalidArgumentException() {
+
+               $this->setExpectedException( 'InvalidArgumentException' );
+
+               $instance = $this->getInstance();
+               $this->assertInternalType( 'string', $instance->get( 'lala' ) );
+       }
+
+       /**
+        * @test ArrayAccessor::get
+        * @test ArrayAccessor::set
+        * @test ArrayAccessor::toArray
+        *
+        * @since 1.9
+        */
+       public function testRoundTrip() {
+
+               $id       = $this->getRandomString();
+               $expected = array( $id => array( $this->getRandomString(), 
$this->getRandomString() ) );
+               $instance = $this->getInstance( $expected );
+
+               // Get
+               $this->assertInternalType( 'array', $instance->get( $id ) );
+               $this->assertEquals( $expected, $instance->toArray() );
+
+               // Set
+               $set = $this->getRandomString();
+               $instance->set( $id, $set );
+               $this->assertEquals( $set, $instance->get( $id ) );
+
+       }
+}
diff --git a/tests/phpunit/includes/cache/ResultCacheMapperTest.php 
b/tests/phpunit/includes/cache/ResultCacheMapperTest.php
new file mode 100644
index 0000000..c99e6fb
--- /dev/null
+++ b/tests/phpunit/includes/cache/ResultCacheMapperTest.php
@@ -0,0 +1,111 @@
+<?php
+
+namespace SMW\Test;
+
+use SMW\ResultCacheMapper;
+
+use SMW\ArrayAccessor;
+
+/**
+ * Tests for the ResultCacheMapper class
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @since 1.9
+ *
+ * @file
+ *
+ * @license GNU GPL v2+
+ * @author mwjames
+ */
+
+/**
+ * @covers \SMW\ResultCacheMapper
+ *
+ * @ingroup Test
+ *
+ * @group SMW
+ * @group SMWExtension
+ */
+class ResultCacheMapperTest extends SemanticMediaWikiTestCase {
+
+       /**
+        * Returns the name of the class to be tested
+        *
+        * @return string|false
+        */
+       public function getClass() {
+               return '\SMW\ResultCacheMapper';
+       }
+
+       /**
+        * Helper method that returns a ResultCacheMapper object
+        *
+        * @since 1.9
+        *
+        * @param $result
+        *
+        * @return ResultCacheMapper
+        */
+       private function getInstance( $cacheId = 'Foo', $cacheEnabled = true, 
$cacheExpiry = 10 ) {
+
+               $setup = array(
+                       'id'      => $cacheId,
+                       'type'    => 'hash',
+                       'enabled' => $cacheEnabled,
+                       'expiry'  => $cacheExpiry
+               );
+
+               return new ResultCacheMapper( new ArrayAccessor( $setup ) );
+       }
+
+       /**
+        * @test ResultCacheMapper::__construct
+        *
+        * @since 1.9
+        */
+       public function testConstructor() {
+               $this->assertInstanceOf( $this->getClass(), 
$this->getInstance() );
+       }
+
+       /**
+        * @test ResultCacheMapper::recache
+        * @test ResultCacheMapper::fetchFromCache
+        *
+        * @since 1.9
+        */
+       public function testRoundTrip() {
+
+               $id       = $this->getRandomString();
+               $expected = array( $this->getRandomString(), 
$this->getRandomString() );
+               $instance = $this->getInstance( $id, true, rand( 100, 200 ) );
+
+               // Initial fetch(without any data present) must fail
+               $result = $instance->fetchFromCache();
+               $this->assertFalse( $result );
+               $this->assertInternalType( 'null', $instance->getCacheDate() );
+
+               // Invoke object
+               $instance->recache( $expected );
+
+               // Re-fetch data from cache
+               $result = $instance->fetchFromCache();
+
+               $this->assertInternalType( 'array', $result );
+               $this->assertInternalType( 'string', $instance->getCacheDate() 
);
+               $this->assertEquals( $expected, $result );
+       }
+}
diff --git a/tests/phpunit/includes/specials/UnusedPropertiesPageTest.php 
b/tests/phpunit/includes/specials/UnusedPropertiesPageTest.php
index 3c621ec..446bdb6 100644
--- a/tests/phpunit/includes/specials/UnusedPropertiesPageTest.php
+++ b/tests/phpunit/includes/specials/UnusedPropertiesPageTest.php
@@ -160,29 +160,45 @@
        }
 
        /**
-        * Helper method that returns a SMWUnusedPropertiesPage object
+        * Helper method that returns a Collector object
         *
         * @since 1.9
         *
         * @param $result
         *
-        * @return SMWUnusedPropertiesPage
+        * @return Collector
         */
-       private function getInstance( $result = null, $values = array() ) {
+       private function getMockCollector( $result = null ) {
 
-               // Collector stub object
-               $collector = $this->getMockForAbstractClass( 
'\SMW\Store\Collector' );
+               $collector = $this->getMockBuilder( '\SMW\Store\Collector' )
+                       ->setMethods( array( 'cacheAccessor', 'doCollect', 
'getResults' ) )
+                       ->getMock();
 
                $collector->expects( $this->any() )
                        ->method( 'getResults' )
                        ->will( $this->returnValue( $result ) );
+
+               return $collector;
+       }
+
+       /**
+        * Helper method that returns a SMWUnusedPropertiesPage object
+        *
+        * @since 1.9
+        *
+        * @param $result
+        * @param $values
+        *
+        * @return SMWUnusedPropertiesPage
+        */
+       private function getInstance( $result = null, $values = array() ) {
 
                // Store stub object
                $store = $this->getMockStore( $values );
 
                $store->expects( $this->any() )
                        ->method( 'getUnusedPropertiesSpecial' )
-                       ->will( $this->returnValue( $collector ) );
+                       ->will( $this->returnValue( $this->getMockCollector( 
$result ) ) );
 
                $instance = new SMWUnusedPropertiesPage( $store, 
$this->getSettings() );
                $instance->setContext( RequestContext::getMain() );
diff --git a/tests/phpunit/includes/specials/WantedPropertiesPageTest.php 
b/tests/phpunit/includes/specials/WantedPropertiesPageTest.php
index 1cd0a1c..2190e5a 100644
--- a/tests/phpunit/includes/specials/WantedPropertiesPageTest.php
+++ b/tests/phpunit/includes/specials/WantedPropertiesPageTest.php
@@ -138,6 +138,27 @@
                return $property;
        }
 
+       /**
+        * Helper method that returns a Collector object
+        *
+        * @since 1.9
+        *
+        * @param $result
+        *
+        * @return Collector
+        */
+       private function getMockCollector( $result = null ) {
+
+               $collector = $this->getMockBuilder( '\SMW\Store\Collector' )
+                       ->setMethods( array( 'cacheAccessor', 'doCollect', 
'getResults' ) )
+                       ->getMock();
+
+               $collector->expects( $this->any() )
+                       ->method( 'getResults' )
+                       ->will( $this->returnValue( $result ) );
+
+               return $collector;
+       }
 
        /**
         * Helper method that returns a SMWWantedPropertiesPage object
@@ -150,19 +171,12 @@
         */
        private function getInstance( $result = null ) {
 
-               // Collector stub object
-               $collector = $this->getMockForAbstractClass( 
'\SMW\Store\Collector' );
-
-               $collector->expects( $this->any() )
-                       ->method( 'getResults' )
-                       ->will( $this->returnValue( $result ) );
-
                // Store stub object
                $store = $this->getMockStore();
 
                $store->expects( $this->any() )
                        ->method( 'getWantedPropertiesSpecial' )
-                       ->will( $this->returnValue( $collector ) );
+                       ->will( $this->returnValue( $this->getMockCollector( 
$result ) ) );
 
                $instance = new SMWWantedPropertiesPage( $store, 
$this->getSettings() );
                $instance->setContext( RequestContext::getMain() );
diff --git 
a/tests/phpunit/includes/storage/sqlstore/UnusedPropertiesCollectorTest.php 
b/tests/phpunit/includes/storage/sqlstore/UnusedPropertiesCollectorTest.php
index 68b5443..5602f23 100644
--- a/tests/phpunit/includes/storage/sqlstore/UnusedPropertiesCollectorTest.php
+++ b/tests/phpunit/includes/storage/sqlstore/UnusedPropertiesCollectorTest.php
@@ -128,7 +128,7 @@
 
        /**
         * @test UnusedPropertiesCollector::getResults
-        * @test UnusedPropertiesCollector::count
+        * @test UnusedPropertiesCollector::getCount
         *
         * @since 1.9
         */
@@ -144,7 +144,7 @@
                $instance->setRequestOptions( $requestOptions );
 
                $this->assertEquals( $expected, $instance->getResults() );
-               $this->assertEquals( 1, $instance->count() );
+               $this->assertEquals( 1, $instance->getCount() );
 
        }
 
@@ -166,7 +166,7 @@
                $message = MessageFormatter::newFromArray( 
$this->getLanguage(), array( $results[0]->getErrors() ) )->getHtml();
 
                $this->assertInternalType( 'array', $results );
-               $this->assertEquals( 1, $instance->count() );
+               $this->assertEquals( 1, $instance->getCount() );
                $this->assertInstanceOf( 'SMWDIError', $results[0] );
                $this->assertContains( $property, $message );
 
diff --git 
a/tests/phpunit/includes/storage/sqlstore/WantedPropertiesCollectorTest.php 
b/tests/phpunit/includes/storage/sqlstore/WantedPropertiesCollectorTest.php
index 3a7f37b..0eff584 100644
--- a/tests/phpunit/includes/storage/sqlstore/WantedPropertiesCollectorTest.php
+++ b/tests/phpunit/includes/storage/sqlstore/WantedPropertiesCollectorTest.php
@@ -55,6 +55,34 @@
        }
 
        /**
+        * Helper method that returns a Database object
+        *
+        * @since 1.9
+        *
+        * @param $smwTitle
+        * @param $count
+        *
+        * @return Database
+        */
+       private function getMockDBConnection( $smwTitle = 'Foo', $count = 1 ) {
+
+               // Injection object expected as the DB fetchObject
+               $returnFetchObject = new \StdClass;
+               $returnFetchObject->count = $count;
+               $returnFetchObject->smw_title = $smwTitle;
+
+               // Database stub object to make the test independent from any 
real DB
+               $connection = $this->getMock( 'DatabaseMysql' );
+
+               // Override method with expected return objects
+               $connection->expects( $this->any() )
+                       ->method( 'select' )
+                       ->will( $this->returnValue( array( $returnFetchObject ) 
) );
+
+               return $connection;
+       }
+
+       /**
         * Helper method that returns a WantedPropertiesCollector object
         *
         * @since 1.9
@@ -68,19 +96,7 @@
        private function getInstance( $property = 'Foo', $count = 1, 
$cacheEnabled = false ) {
 
                $store = StoreFactory::getStore();
-
-               // Injection object expected as the DB fetchObject
-               $returnFetchObject = new \StdClass;
-               $returnFetchObject->count = $count;
-               $returnFetchObject->smw_title = $property;
-
-               // Database stub object to make the test independent from any 
real DB
-               $connection = $this->getMock( 'DatabaseMysql' );
-
-               // Override method with expected return objects
-               $connection->expects( $this->any() )
-                       ->method( 'select' )
-                       ->will( $this->returnValue( array( $returnFetchObject ) 
) );
+               $connection = $this->getMockDBConnection( $property, $count );
 
                // Settings to be used
                $settings = Settings::newFromArray( array(
@@ -115,7 +131,7 @@
 
        /**
         * @test WantedPropertiesCollector::getResults
-        * @test WantedPropertiesCollector::count
+        * @test WantedPropertiesCollector::getCount
         *
         * @since 1.9
         */
@@ -131,7 +147,7 @@
                );
 
                $this->assertEquals( $expected, $instance->getResults() );
-               $this->assertEquals( 1, $instance->count() );
+               $this->assertEquals( 1, $instance->getCount() );
 
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/71260
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I40a1548e370e1c4f16c0a1ad6e6621d58334ad5c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <[email protected]>
Gerrit-Reviewer: Mwjames <[email protected]>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to