Thiemo Mättig (WMDE) has submitted this change and it was merged.

Change subject: Turn some unlabeled number literals into named constants
......................................................................


Turn some unlabeled number literals into named constants

Unnamed constants should be avoided and easily can be avoided in
these cases. I'm aware of the problem that PHP does not have private
constants. Would be nice to have them. But I disagree that because
we do not have private constants we should not do what I'm doing here.
Readability is more important, in my opinion.

Plus very few very minor cleanups that are not relevant enough to
justify an other patch. You still think I should split this? No
problem, please tell me.

Change-Id: Ic96d51e861033512b2ec4c4c08665e74031b0da7
---
M lib/includes/modules/SitesModuleWorker.php
M lib/tests/phpunit/SitesModuleWorkerTest.php
M repo/includes/LinkedData/EntityDataRequestHandler.php
3 files changed, 28 insertions(+), 13 deletions(-)

Approvals:
  Ori.livneh: Looks good to me, but someone else must approve
  Jonas Kress (WMDE): Looks good to me, but someone else must approve
  Daniel Kinzler: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/lib/includes/modules/SitesModuleWorker.php 
b/lib/includes/modules/SitesModuleWorker.php
index 3bb8a2e..50e335a 100644
--- a/lib/includes/modules/SitesModuleWorker.php
+++ b/lib/includes/modules/SitesModuleWorker.php
@@ -22,6 +22,11 @@
 class SitesModuleWorker {
 
        /**
+        * How many seconds the result of self::getModifiedHash is cached.
+        */
+       const SITES_HASH_CACHE_DURATION = 600; // 10 minutes
+
+       /**
         * @var SettingsArray
         */
        private $settings;
@@ -121,6 +126,7 @@
        /**
         * @param MediaWikiSite $site
         * @param string[] $specialGroups
+        *
         * @return string[]
         */
        private function getSiteDetails( MediaWikiSite $site, array 
$specialGroups ) {
@@ -168,7 +174,7 @@
         * Whether it's needed to add a Site to the JS variable.
         *
         * @param Site $site
-        * @param array $groups
+        * @param string[] $groups
         *
         * @return bool
         */
@@ -204,7 +210,7 @@
 
                if ( $hash === false ) {
                        $hash = $this->computeModifiedHash();
-                       $this->cache->set( $cacheKey, $hash, 10 * 60 );
+                       $this->cache->set( $cacheKey, $hash, 
self::SITES_HASH_CACHE_DURATION );
                }
 
                return $hash;
diff --git a/lib/tests/phpunit/SitesModuleWorkerTest.php 
b/lib/tests/phpunit/SitesModuleWorkerTest.php
index 71ad43b..0f38a6a 100644
--- a/lib/tests/phpunit/SitesModuleWorkerTest.php
+++ b/lib/tests/phpunit/SitesModuleWorkerTest.php
@@ -46,8 +46,8 @@
 
                return new SitesModuleWorker(
                        new SettingsArray( array(
-                       'siteLinkGroups' => $groups,
-                       'specialSiteLinkGroups' => $specialGroups
+                               'siteLinkGroups' => $groups,
+                               'specialSiteLinkGroups' => $specialGroups
                        ) ),
                        $siteStore,
                        $cache ?: new HashBagOStuff()
diff --git a/repo/includes/LinkedData/EntityDataRequestHandler.php 
b/repo/includes/LinkedData/EntityDataRequestHandler.php
index 2833b33..92f00f0 100644
--- a/repo/includes/LinkedData/EntityDataRequestHandler.php
+++ b/repo/includes/LinkedData/EntityDataRequestHandler.php
@@ -34,9 +34,13 @@
 class EntityDataRequestHandler {
 
        /**
-        * @var int Cache duration in seconds
+        * Allowed smallest and bigest number of seconds for the "max-age=..." 
and "s-maxage=..." cache
+        * control parameters.
+        *
+        * @todo Hard maximum could be configurable somehow.
         */
-       private $maxAge = 0;
+       const MINIMUM_MAX_AGE = 0;
+       const MAXIMUM_MAX_AGE = 2678400; // 31 days
 
        /**
         * @var EntityDataSerializationService
@@ -79,6 +83,11 @@
        private $defaultFormat;
 
        /**
+        * @var int Number of seconds to cache entity data.
+        */
+       private $maxAge;
+
+       /**
         * @var bool
         */
        private $useSquids;
@@ -97,7 +106,8 @@
         * @param EntityRevisionLookup $entityRevisionLookup
         * @param EntityRedirectLookup $entityRedirectLookup
         * @param EntityDataSerializationService $serializationService
-        * @param string $defaultFormat
+        * @param EntityDataFormatProvider $entityDataFormatProvider
+        * @param string $defaultFormat The format as a file extension or MIME 
type.
         * @param int $maxAge number of seconds to cache entity data
         * @param bool $useSquids do we have web caches configured?
         * @param string|null $frameOptionsHeader for X-Frame-Options
@@ -475,13 +485,12 @@
                // NOTE: similar code as in RawAction::onView, keep in sync.
 
                //FIXME: do not cache if revision was requested explicitly!
-               $maxage = $request->getInt( 'maxage', $this->maxAge );
-               $smaxage = $request->getInt( 'smaxage', $this->maxAge );
+               $maxAge = $request->getInt( 'maxage', $this->maxAge );
+               $sMaxAge = $request->getInt( 'smaxage', $this->maxAge );
 
                // XXX: do we want public caching even for data from old 
revisions?
-               // Sanity: 0 to 31 days. // todo: Hard maximum could be 
configurable somehow.
-               $maxage  = max( 0, min( 60 * 60 * 24 * 31, $maxage ) );
-               $smaxage = max( 0, min( 60 * 60 * 24 * 31, $smaxage ) );
+               $maxAge  = max( self::MINIMUM_MAX_AGE, min( 
self::MAXIMUM_MAX_AGE, $maxAge ) );
+               $sMaxAge = max( self::MINIMUM_MAX_AGE, min( 
self::MAXIMUM_MAX_AGE, $sMaxAge ) );
 
                $response->header( 'Content-Type: ' . $contentType . '; 
charset=UTF-8' );
                $response->header( 'Access-Control-Allow-Origin: *' );
@@ -497,7 +506,7 @@
 
                // allow the client to cache this
                $mode = 'public';
-               $response->header( 'Cache-Control: ' . $mode . ', s-maxage=' . 
$smaxage . ', max-age=' . $maxage );
+               $response->header( 'Cache-Control: ' . $mode . ', s-maxage=' . 
$sMaxAge . ', max-age=' . $maxAge );
 
                ob_clean(); // remove anything that might already be in the 
output buffer.
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic96d51e861033512b2ec4c4c08665e74031b0da7
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Hoo man <h...@online.de>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to