site: Annotate CachingSiteStore as `@internal` and perform minor cleanup

* Recognise CachingSiteStore as the internal class that it is.

* Clean up the file header and move ingroup annotation, similar to
  other commits under
  <https://gerrit.wikimedia.org/r/q/message:ingroup+owner:Krinkle>.

  The one here was particularly odd as it placed the license inside
  the class doc block rather than the file doc block.

* Remove unused $cacheKey and $cacheTimeout constructor args. They
  were also not covered by any unit tests.

* Avoid use of `empty()` as per
  <https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP>

* Use a more conventional cache key in CachingSiteStore, keeping
  the dynamic part for a later segment and keeping the first segment
  explicitly tied to a single concrete key group. This also improves
  ergonomics of cache statistics e.g. in Grafana and makes the cache
  key easier to locate in codesearch/grep, and more recognisable,
  e.g. starting with the code component ("site").

Change-Id: I541ad62551ab76127d70ef665272fd1d590ab556

11 files changed, 123 insertions, 196 deletions

diff --git a/includes/site/CachingSiteStore.php b/includes/site/CachingSiteStore.php
index 76bf893ab1c9..18ef8d706680 100644
--- a/includes/site/CachingSiteStore.php
+++ b/includes/site/CachingSiteStore.php
@@ -1,9 +1,5 @@
 <?php
-
 /**
- * Represents the site configuration of a wiki.
- * Holds a list of sites (ie SiteList), with a caching layer.
- *
  * 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
@@ -19,58 +15,38 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.25
- *
  * @file
- * @ingroup Site
+ */
+
+/**
+ * Hold a configured list of sites (SiteList), with a caching layer.
  *
- * @license GPL-2.0-or-later
+ * @internal For use by core ServiceWiring only. The public interface is SiteStore
+ * @ingroup Site
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  * @author Katie Filbert < aude.wiki@gmail.com >
  */
 class CachingSiteStore implements SiteStore {
-
-	/**
-	 * @var SiteList|null
-	 */
-	private $sites = null;
-
-	/**
-	 * @var string|null
-	 */
-	private $cacheKey;
-
-	/**
-	 * @var int
-	 */
-	private $cacheTimeout;
-
-	/**
-	 * @var BagOStuff
-	 */
+	/** @var SiteStore */
+	private $siteStore;
+	/** @var BagOStuff */
 	private $cache;
 
-	/**
-	 * @var SiteStore
-	 */
-	private $siteStore;
+	/** @var string|null */
+	private $cacheKey = null;
+	/** @var SiteList|null */
+	private $sites = null;
 
 	/**
 	 * @param SiteStore $siteStore
 	 * @param BagOStuff $cache
-	 * @param string|null $cacheKey
-	 * @param int $cacheTimeout
 	 */
 	public function __construct(
 		SiteStore $siteStore,
-		BagOStuff $cache,
-		$cacheKey = null,
-		$cacheTimeout = 3600
+		BagOStuff $cache
 	) {
 		$this->siteStore = $siteStore;
 		$this->cache = $cache;
-		$this->cacheKey = $cacheKey;
-		$this->cacheTimeout = $cacheTimeout;
 	}
 
 	/**
@@ -89,8 +65,8 @@ class CachingSiteStore implements SiteStore {
 	 */
 	private function getCacheKey() {
 		if ( $this->cacheKey === null ) {
-			$type = 'SiteList#' . SiteList::getSerialVersionId();
-			$this->cacheKey = $this->cache->makeKey( "sites/$type" );
+			$version = SiteList::getSerialVersionId();
+			$this->cacheKey = $this->cache->makeKey( 'site-SiteList', $version );
 		}
 
 		return $this->cacheKey;
@@ -100,15 +76,16 @@ class CachingSiteStore implements SiteStore {
 	 * @see SiteStore::getSites
 	 *
 	 * @since 1.25
-	 *
 	 * @return SiteList
 	 */
 	public function getSites() {
 		if ( $this->sites === null ) {
 			$this->sites = $this->cache->getWithSetCallback(
 				$this->getCacheKey(),
-				$this->cacheTimeout,
-				[ $this->siteStore, 'getSites' ]
+				BagOStuff::TTL_HOUR,
+				function () {
+					return $this->siteStore->getSites();
+				}
 			);
 		}
 
@@ -119,9 +96,7 @@ class CachingSiteStore implements SiteStore {
 	 * @see SiteStore::getSite
 	 *
 	 * @since 1.25
-	 *
 	 * @param string $globalId
-	 *
 	 * @return Site|null
 	 */
 	public function getSite( $globalId ) {
@@ -134,9 +109,7 @@ class CachingSiteStore implements SiteStore {
 	 * @see SiteStore::saveSite
 	 *
 	 * @since 1.25
-	 *
 	 * @param Site $site
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSite( Site $site ) {
@@ -147,13 +120,11 @@ class CachingSiteStore implements SiteStore {
 	 * @see SiteStore::saveSites
 	 *
 	 * @since 1.25
-	 *
 	 * @param Site[] $sites
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSites( array $sites ) {
-		if ( empty( $sites ) ) {
+		if ( !$sites ) {
 			return true;
 		}
 
@@ -166,15 +137,12 @@ class CachingSiteStore implements SiteStore {
 	}
 
 	/**
-	 * Purges the internal and external cache of the site list, forcing the list.
+	 * Purge the internal and external cache of the site list, forcing the list.
 	 * of sites to be reloaded.
 	 *
-	 * Only use this for testing, as APC is typically used and is per-server
-	 *
 	 * @since 1.25
 	 */
 	public function reset() {
-		// purge cache
 		$this->cache->delete( $this->getCacheKey() );
 		$this->sites = null;
 	}
@@ -182,10 +150,11 @@ class CachingSiteStore implements SiteStore {
 	/**
 	 * Clears the list of sites stored.
 	 *
-	 * Only use this for testing, as APC is typically used and is per-server.
+	 * NOTE: The fact that this also clears the in-process cache is an internal
+	 * detail for PHPUnit testing only. The injected cache is generally APCU,
+	 * which is per-server, so the cache reset would not apply to any other web servers.
 	 *
 	 * @see SiteStore::clear()
-	 *
 	 * @return bool Success
 	 */
 	public function clear() {
diff --git a/includes/site/DBSiteStore.php b/includes/site/DBSiteStore.php
index ee4497789391..24bdf561fdba 100644
--- a/includes/site/DBSiteStore.php
+++ b/includes/site/DBSiteStore.php
@@ -1,11 +1,5 @@
 <?php
-
-use Wikimedia\Rdbms\ILoadBalancer;
-
 /**
- * Represents the site configuration of a wiki.
- * Holds a list of sites (ie SiteList), stored in the database.
- *
  * 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
@@ -21,33 +15,27 @@ use Wikimedia\Rdbms\ILoadBalancer;
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.25
- *
  * @file
- * @ingroup Site
+ */
+
+use Wikimedia\Rdbms\ILoadBalancer;
+
+/**
+ * Holds a list of sites stored in the database.
  *
- * @license GPL-2.0-or-later
+ * @since 1.25
+ * @ingroup Site
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  * @author Daniel Kinzler
  */
 class DBSiteStore implements SiteStore {
-
-	/**
-	 * @var SiteList|null
-	 */
+	/** @var SiteList|null */
 	protected $sites = null;
-
-	/**
-	 * @var ILoadBalancer
-	 */
+	/** @var ILoadBalancer */
 	private $dbLoadBalancer;
 
 	/**
 	 * @since 1.27
-	 *
-	 * @todo inject some kind of connection manager that is aware of the target wiki,
-	 * instead of injecting a LoadBalancer.
-	 *
 	 * @param ILoadBalancer $dbLoadBalancer
 	 */
 	public function __construct( ILoadBalancer $dbLoadBalancer ) {
@@ -58,7 +46,6 @@ class DBSiteStore implements SiteStore {
 	 * @see SiteStore::getSites
 	 *
 	 * @since 1.25
-	 *
 	 * @return SiteList
 	 */
 	public function getSites() {
@@ -138,9 +125,7 @@ class DBSiteStore implements SiteStore {
 	 * @see SiteStore::getSite
 	 *
 	 * @since 1.25
-	 *
 	 * @param string $globalId
-	 *
 	 * @return Site|null
 	 */
 	public function getSite( $globalId ) {
@@ -155,9 +140,7 @@ class DBSiteStore implements SiteStore {
 	 * @see SiteStore::saveSite
 	 *
 	 * @since 1.25
-	 *
 	 * @param Site $site
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSite( Site $site ) {
@@ -168,9 +151,7 @@ class DBSiteStore implements SiteStore {
 	 * @see SiteStore::saveSites
 	 *
 	 * @since 1.25
-	 *
 	 * @param Site[] $sites
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSites( array $sites ) {
diff --git a/includes/site/HashSiteStore.php b/includes/site/HashSiteStore.php
index f0a6e54e39ec..76b4b7aab131 100644
--- a/includes/site/HashSiteStore.php
+++ b/includes/site/HashSiteStore.php
@@ -1,7 +1,5 @@
 <?php
 /**
- * In-memory implementation of SiteStore.
- *
  * 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
@@ -21,35 +19,29 @@
  */
 
 /**
- * In-memory SiteStore implementation, storing sites in an associative array.
- *
- * @author Daniel Kinzler
- * @author Katie Filbert < aude.wiki@gmail.com >
+ * In-memory SiteStore implementation, stored in an associative array.
  *
  * @since 1.25
  * @ingroup Site
+ * @author Daniel Kinzler
+ * @author Katie Filbert < aude.wiki@gmail.com >
  */
 class HashSiteStore implements SiteStore {
-
-	/**
-	 * @var Site[]
-	 */
+	/** @var Site[] */
 	private $sites = [];
 
 	/**
 	 * @param Site[] $sites
 	 */
-	public function __construct( $sites = [] ) {
+	public function __construct( array $sites = [] ) {
 		$this->saveSites( $sites );
 	}
 
 	/**
-	 * Saves the provided site.
+	 * Save the provided site.
 	 *
 	 * @since 1.25
-	 *
 	 * @param Site $site
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSite( Site $site ) {
@@ -59,12 +51,10 @@ class HashSiteStore implements SiteStore {
 	}
 
 	/**
-	 * Saves the provided sites.
+	 * Save the provided sites.
 	 *
 	 * @since 1.25
-	 *
 	 * @param Site[] $sites
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSites( array $sites ) {
@@ -76,14 +66,12 @@ class HashSiteStore implements SiteStore {
 	}
 
 	/**
-	 * Returns the site with provided global id, or null if there is no such site.
+	 * Return the site with provided global ID, or null if there is no such site.
 	 *
 	 * @since 1.25
-	 *
 	 * @param string $globalId
 	 * @param string $source either 'cache' or 'recache'.
-	 *                       If 'cache', the values can (but not obliged) come from a cache.
-	 *
+	 *  If 'cache', the values can (but not obliged) come from a cache.
 	 * @return Site|null
 	 */
 	public function getSite( $globalId, $source = 'cache' ) {
@@ -91,15 +79,14 @@ class HashSiteStore implements SiteStore {
 	}
 
 	/**
-	 * Returns a list of all sites. By default this site is
-	 * fetched from the cache, which can be changed to loading
+	 * Return a list of all sites.
+	 *
+	 * By default this list is fetched from the cache, which can be changed to loading
 	 * the list from the database using the $useCache parameter.
 	 *
 	 * @since 1.25
-	 *
 	 * @param string $source either 'cache' or 'recache'.
-	 *                       If 'cache', the values can (but not obliged) come from a cache.
-	 *
+	 *  If 'cache', the values can (but not obliged) come from a cache.
 	 * @return SiteList
 	 */
 	public function getSites( $source = 'cache' ) {
@@ -107,13 +94,15 @@ class HashSiteStore implements SiteStore {
 	}
 
 	/**
-	 * Deletes all sites from the database. After calling clear(), getSites() will return an empty
-	 * list and getSite() will return null until saveSite() or saveSites() is called.
+	 * Delete all sites from the database.
+	 *
+	 * After calling clear(), getSites() will return an empty list and getSite() will
+	 * return null until saveSite() or saveSites() is called.
+	 *
 	 * @return bool
 	 */
 	public function clear() {
 		$this->sites = [];
-
 		return true;
 	}
 
diff --git a/includes/site/MediaWikiPageNameNormalizer.php b/includes/site/MediaWikiPageNameNormalizer.php
index 6ea57ac41e4f..73c80eaa8923 100644
--- a/includes/site/MediaWikiPageNameNormalizer.php
+++ b/includes/site/MediaWikiPageNameNormalizer.php
@@ -1,15 +1,5 @@
 <?php
-
-namespace MediaWiki\Site;
-
-use FormatJson;
-use Http;
-use InvalidArgumentException;
-use UtfNormal\Validator;
-
 /**
- * Service for normalizing a page name using a MediaWiki api.
- *
  * 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
@@ -25,9 +15,20 @@ use UtfNormal\Validator;
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.27
+ * @file
+ */
+
+namespace MediaWiki\Site;
+
+use FormatJson;
+use Http;
+use InvalidArgumentException;
+use UtfNormal\Validator;
+
+/**
+ * Service for normalizing a page name via a MediaWiki action API.
  *
- * @license GPL-2.0-or-later
+ * @since 1.27
  * @author John Erling Blad < jeblad@gmail.com >
  * @author Daniel Kinzler
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
diff --git a/includes/site/MediaWikiSite.php b/includes/site/MediaWikiSite.php
index 10ebb7a23543..b8cb1546bbef 100644
--- a/includes/site/MediaWikiSite.php
+++ b/includes/site/MediaWikiSite.php
@@ -1,10 +1,5 @@
 <?php
-
-use MediaWiki\Site\MediaWikiPageNameNormalizer;
-
 /**
- * Class representing a MediaWiki site.
- *
  * 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
@@ -21,19 +16,18 @@ use MediaWiki\Site\MediaWikiPageNameNormalizer;
  * http://www.gnu.org/copyleft/gpl.html
  *
  * @file
- * @ingroup Site
- * @license GPL-2.0-or-later
- * @author John Erling Blad < jeblad@gmail.com >
- * @author Daniel Kinzler
- * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  */
 
+use MediaWiki\Site\MediaWikiPageNameNormalizer;
+
 /**
  * Class representing a MediaWiki site.
  *
  * @since 1.21
- *
  * @ingroup Site
+ * @author John Erling Blad < jeblad@gmail.com >
+ * @author Daniel Kinzler
+ * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  */
 class MediaWikiSite extends Site {
 	public const PATH_FILE = 'file_path';
diff --git a/includes/site/Site.php b/includes/site/Site.php
index 32e1a058896e..0ce5a93f15e2 100644
--- a/includes/site/Site.php
+++ b/includes/site/Site.php
@@ -1,12 +1,5 @@
 <?php
-
-use MediaWiki\MainConfigNames;
-use MediaWiki\MediaWikiServices;
-use MediaWiki\Site\MediaWikiPageNameNormalizer;
-
 /**
- * Represents a single site.
- *
  * 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
@@ -22,12 +15,18 @@ use MediaWiki\Site\MediaWikiPageNameNormalizer;
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.21
- *
  * @file
- * @ingroup Site
+ */
+
+use MediaWiki\MainConfigNames;
+use MediaWiki\MediaWikiServices;
+use MediaWiki\Site\MediaWikiPageNameNormalizer;
+
+/**
+ * Represents a single site.
  *
- * @license GPL-2.0-or-later
+ * @since 1.21
+ * @ingroup Site
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  */
 class Site implements Serializable {
diff --git a/includes/site/SiteExporter.php b/includes/site/SiteExporter.php
index f051574c7e54..25c7fa7c7bcd 100644
--- a/includes/site/SiteExporter.php
+++ b/includes/site/SiteExporter.php
@@ -1,9 +1,5 @@
 <?php
-
 /**
- * Utility for exporting site entries to XML.
- * For the output file format, see docs/sitelist.md and docs/sitelist-1.0.xsd.
- *
  * 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
@@ -19,12 +15,16 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.25
- *
  * @file
- * @ingroup Site
+ */
+
+/**
+ * Utility for exporting site entries to XML.
  *
- * @license GPL-2.0-or-later
+ * For the output file format, see docs/sitelist.md and docs/sitelist-1.0.xsd.
+ *
+ * @since 1.25
+ * @ingroup Site
  * @author Daniel Kinzler
  */
 class SiteExporter {
diff --git a/includes/site/SiteImporter.php b/includes/site/SiteImporter.php
index 714391b4e128..96a2673ab6b7 100644
--- a/includes/site/SiteImporter.php
+++ b/includes/site/SiteImporter.php
@@ -1,11 +1,5 @@
 <?php
-
-use Wikimedia\RequestTimeout\TimeoutException;
-
 /**
- * Utility for importing site entries from XML.
- * For the expected format of the input, see docs/sitelist.md and docs/sitelist-1.0.xsd.
- *
  * 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
@@ -21,12 +15,18 @@ use Wikimedia\RequestTimeout\TimeoutException;
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.25
- *
  * @file
- * @ingroup Site
+ */
+
+use Wikimedia\RequestTimeout\TimeoutException;
+
+/**
+ * Utility for importing site entries from XML.
  *
- * @license GPL-2.0-or-later
+ * For the expected format of the input, see docs/sitelist.md and docs/sitelist-1.0.xsd.
+ *
+ * @since 1.25
+ * @ingroup Site
  * @author Daniel Kinzler
  */
 class SiteImporter {
diff --git a/includes/site/SiteList.php b/includes/site/SiteList.php
index eafe007885a2..1cacc0a16e7d 100644
--- a/includes/site/SiteList.php
+++ b/includes/site/SiteList.php
@@ -1,8 +1,5 @@
 <?php
-
 /**
- * Collection of Site objects.
- *
  * 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
@@ -18,12 +15,14 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.21
- *
  * @file
- * @ingroup Site
+ */
+
+/**
+ * Collection of Site objects.
  *
- * @license GPL-2.0-or-later
+ * @since 1.21
+ * @ingroup Site
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  */
 class SiteList extends GenericArrayObject {
diff --git a/includes/site/SiteLookup.php b/includes/site/SiteLookup.php
index 70fc453dd160..df0449217010 100644
--- a/includes/site/SiteLookup.php
+++ b/includes/site/SiteLookup.php
@@ -1,8 +1,5 @@
 <?php
-
 /**
- * Interface for service objects providing a lookup of Site objects.
- *
  * 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
@@ -18,31 +15,32 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.25
- *
  * @file
- * @ingroup Site
+ */
+
+/**
+ * Interface to retreive Site objects, for implementation by service classes.
+ *
+ * Default implementation is DBSiteStore.
  *
- * @license GPL-2.0-or-later
+ * @since 1.25
+ * @ingroup Site
  */
 interface SiteLookup {
 
 	/**
-	 * Returns the site with provided global id, or null if there is no such site.
+	 * Return the site with provided global ID, or null if there is no such site.
 	 *
 	 * @since 1.25
-	 *
 	 * @param string $globalId
-	 *
 	 * @return Site|null
 	 */
 	public function getSite( $globalId );
 
 	/**
-	 * Returns a list of all sites.
+	 * Return a list of all sites.
 	 *
 	 * @since 1.25
-	 *
 	 * @return SiteList
 	 */
 	public function getSites();
diff --git a/includes/site/SiteStore.php b/includes/site/SiteStore.php
index 13800d0ded6d..55c2fa0fd41c 100644
--- a/includes/site/SiteStore.php
+++ b/includes/site/SiteStore.php
@@ -1,8 +1,5 @@
 <?php
-
 /**
- * Interface for service objects providing a storage interface for Site objects.
- *
  * 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
@@ -18,12 +15,16 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  * http://www.gnu.org/copyleft/gpl.html
  *
- * @since 1.21
- *
  * @file
- * @ingroup Site
+ */
+
+/**
+ * Interface for storing and retreiving Site objects.
  *
- * @license GPL-2.0-or-later
+ * Default implementation is DBSiteStore.
+ *
+ * @since 1.21
+ * @ingroup Site
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
  */
 interface SiteStore extends SiteLookup {
@@ -32,9 +33,7 @@ interface SiteStore extends SiteLookup {
 	 * Saves the provided site.
 	 *
 	 * @since 1.21
-	 *
 	 * @param Site $site
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSite( Site $site );
@@ -43,9 +42,7 @@ interface SiteStore extends SiteLookup {
 	 * Saves the provided sites.
 	 *
 	 * @since 1.21
-	 *
 	 * @param Site[] $sites
-	 *
 	 * @return bool Success indicator
 	 */
 	public function saveSites( array $sites );