Gergő Tisza has uploaded a new change for review.
https://gerrit.wikimedia.org/r/320325
Change subject: Add cache layer to the service
......................................................................
Add cache layer to the service
Change-Id: Ib8feb757caf1f19c0b245fd90aec42537b0a84a7
---
M extension.json
M i18n/en.json
M i18n/qqq.json
A includes/CachedPageViewService.php
M includes/Hooks.php
M includes/ServiceWiring.php
A tests/phpunit/CachedPageViewServiceTest.php
7 files changed, 654 insertions(+), 37 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/PageViewInfo
refs/changes/25/320325/1
diff --git a/extension.json b/extension.json
index 8a24ff3..923ef1c 100644
--- a/extension.json
+++ b/extension.json
@@ -14,6 +14,7 @@
"AutoloadClasses": {
"MediaWiki\\Extensions\\PageViewInfo\\Hooks":
"includes/Hooks.php",
"MediaWiki\\Extensions\\PageViewInfo\\PageViewService":
"includes/PageViewService.php",
+ "MediaWiki\\Extensions\\PageViewInfo\\CachedPageViewService":
"includes/CachedPageViewService.php",
"MediaWiki\\Extensions\\PageViewInfo\\WikimediaPageViewService":
"includes/WikimediaPageViewService.php"
},
"MessagesDirs": {
diff --git a/i18n/en.json b/i18n/en.json
index 339d0b0..02f836c 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -8,5 +8,7 @@
"pvi-month-count": "Page views in the past 30 days",
"pvi-close": "Close",
"pvi-range": "$1 - $2",
- "pvi-invalidresponse": "Invalid response"
+ "pvi-invalidresponse": "Invalid response",
+ "pvi-cached-error": "An earlier attempt to fetch this data failed. To
limit server load, retries have been blocked for $1.",
+ "pvi-cached-error-title": "An earlier attempt to fetch page \"$1\"
failed. To limit server load, retries have been blocked for $2."
}
diff --git a/i18n/qqq.json b/i18n/qqq.json
index ba260de..6a186c0 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -9,5 +9,7 @@
"pvi-month-count": "Label for table cell containing page views in past
30 days",
"pvi-close": "Text on button to close a dialog\n{{Identical|Close}}",
"pvi-range": "Title of dialog, which is the date range the graph is
for. $1 is the starting date, $2 is the ending date.",
- "pvi-invalidresponse": "Error message when the REST API response data
does not have the expected structure."
+ "pvi-invalidresponse": "Error message when the REST API response data
does not have the expected structure.",
+ "pvi-cached-error": "Shown when the cached result is an error. $1 is
the retry delay in human-readable form (e.g. \"30 minutes\").",
+ "pvi-cached-error-title": "Shown when the cached result (which is part
of a larger resultset) is an error. $1 is the page name, $2 the retry delay in
human-readable form (e.g. \"30 minutes\")."
}
diff --git a/includes/CachedPageViewService.php
b/includes/CachedPageViewService.php
new file mode 100644
index 0000000..b3fb540
--- /dev/null
+++ b/includes/CachedPageViewService.php
@@ -0,0 +1,240 @@
+<?php
+
+namespace MediaWiki\Extensions\PageViewInfo;
+
+use BagOStuff;
+use InvalidArgumentException;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+use Status;
+use StatusValue;
+use Title;
+
+/**
+ * Wraps a PageViewService and caches the results.
+ */
+class CachedPageViewService implements PageViewService, LoggerAwareInterface {
+ const ERROR_EXPIRY = 1800;
+
+ /** @var PageViewService */
+ protected $service;
+
+ /** @var BagOStuff */
+ protected $cache;
+
+ /** @var LoggerInterface */
+ protected $logger;
+
+ /** @var string Cache prefix, in case multiple instances of this
service coexist */
+ protected $prefix;
+
+ /** @var int */
+ protected $cachedDays = 30;
+
+ public function __construct( PageViewService $service, BagOStuff
$cache, $prefix = null ) {
+ $this->service = $service;
+ $this->logger = new NullLogger();
+ $this->cache = $cache;
+ $this->prefix = $prefix;
+ }
+
+ public function setLogger( LoggerInterface $logger ) {
+ $this->logger = $logger;
+ }
+
+ /**
+ * Set the number of days that will be cached. To avoid cache
fragmentation, the inner service
+ * is always called with this number of days; if necessary, the
response will be expanded with
+ * nulls.
+ * @param int $cachedDays
+ */
+ public function setCachedDays( $cachedDays ) {
+ $this->cachedDays = $cachedDays;
+ }
+
+ public function supports( $metric, $scope ) {
+ return $this->service->supports( $metric, $scope );
+ }
+
+ public function getPageData( array $titles, $days, $metric =
self::METRIC_VIEW ) {
+ $status = $this->getTitlesWithCache( $metric, $titles );
+ $data = $status->getValue();
+ foreach ( $data as $title => $titleData ) {
+ if ( $days < $this->cachedDays ) {
+ $data[$title] = array_slice( $titleData,
-$days, null, true );
+ } elseif ( $days > $this->cachedDays ) {
+ $data[$title] = $this->extendDateRange(
$titleData, $days );
+ }
+ }
+ $status->setResult( $status->isOK(), $data );
+ return $status;
+ }
+
+ public function getSiteData( $days, $metric = self::METRIC_VIEW ) {
+ $status = $this->getWithCache( $metric, self::SCOPE_SITE );
+ if ( $status->isOK() ) {
+ $data = $status->getValue();
+ if ( $days < $this->cachedDays ) {
+ $data = array_slice( $data, -$days, null, true
);
+ } elseif ( $days > $this->cachedDays ) {
+ $data = $this->extendDateRange( $data, $days );
+ }
+ $status->setResult( true, $data );
+ }
+ return $status;
+ }
+
+ public function getTopPages( $metric = self::METRIC_VIEW ) {
+ return $this->getWithCache( $metric, self::SCOPE_TOP );
+ }
+
+ public function getCacheExpiry( $metric, $scope ) {
+ // add some random delay to avoid cache stampedes
+ return $this->service->getCacheExpiry( $metric, $scope ) +
mt_rand( 0, 600 );
+ }
+
+ /**
+ * Like BagOStuff::getWithSetCallback, but returns a StatusValue like
PageViewService calls do.
+ * Returns (and caches) null wrapped in a StatusValue on error.
+ * @param string $metric A METRIC_* constant
+ * @param string $scope A SCOPE_* constant (except SCOPE_ARTICLE which
has its own method)
+ * @return StatusValue
+ */
+ protected function getWithCache( $metric, $scope ) {
+ $key = $this->cache->makeKey( 'pvi', $this->prefix, ( $scope
=== self::SCOPE_SITE ) ?
+ $this->cachedDays : null, $metric, $scope );
+ $data = $this->cache->get( $key );
+ if ( $data === false ) { // no cached data
+ /** @var StatusValue $status */
+ switch ( $scope ) {
+ case self::SCOPE_SITE:
+ $status = $this->service->getSiteData(
$this->cachedDays, $metric );
+ break;
+ case self::SCOPE_TOP:
+ $status = $this->service->getTopPages(
$metric );
+ break;
+ default:
+ throw new InvalidArgumentException(
"invalid scope: $scope" );
+ }
+ if ( $status->isOK() ) {
+ $data = $status->getValue();
+ $expiry = $this->getCacheExpiry( $metric,
$scope );
+ } else {
+ $data = null;
+ $expiry = self::ERROR_EXPIRY;
+ }
+ $this->cache->set( $key, $data, $expiry );
+ } elseif ( $data === null ) { // cached error
+ $status = StatusValue::newGood( [] );
+ $status->fatal( 'pvi-cached-error',
\Message::durationParam( self::ERROR_EXPIRY ) );
+ } else { // valid cached data
+ $status = StatusValue::newGood( $data );
+ }
+ return $status;
+ }
+
+ /**
+ * The equivalent of getWithCache for multiple titles (ie. for
SCOPE_ARTICLE).
+ * Errors are also handled per-article.
+ * @param string $metric A METRIC_* constant
+ * @param Title[] $titles
+ * @return StatusValue
+ */
+ protected function getTitlesWithCache( $metric, array $titles = null ) {
+ // Set up the response array, without any values. This will
help preserve the order of titles.
+ $data = array_fill_keys( array_map( function ( Title $t ) {
+ return $t->getPrefixedDBkey();
+ }, $titles ), false );
+
+ // Fetch data for all titles from cache. Hopefully we are using
a cache which has
+ // a cheap getMulti implementation.
+ $titleToCacheKey = $statuses = [];
+ foreach ( $titles as $title ) {
+ $titleToCacheKey[$title->getPrefixedDBkey()] =
$this->cache->makeKey( 'pvi', $this->prefix,
+ $this->cachedDays, $metric,
self::SCOPE_ARTICLE, md5( $title->getPrefixedDBkey() ) );
+ }
+ $cacheKeyToTitle = array_flip( $titleToCacheKey );
+ $rawData = $this->cache->getMulti( array_keys( $cacheKeyToTitle
) );
+ foreach ( $rawData as $key => $value ) {
+ // BagOStuff::getMulti is unclear on how missing items
should be handled; let's
+ // assume some implementations might return that key
with a value of false
+ if ( $value !== false ) {
+ $statuses[$cacheKeyToTitle[$key]] = empty(
$value['#error'] ) ? StatusValue::newGood()
+ : StatusValue::newFatal(
'pvi-cached-error-title', $cacheKeyToTitle[$key],
+ \Message::durationParam(
self::ERROR_EXPIRY ) );
+ unset( $value['#error'] );
+ $data[$cacheKeyToTitle[$key]] = $value;
+ }
+ }
+
+ // Now get and cache the data for the remaining titles from the
real service. It might not
+ // return data for all of them.
+ foreach ( $titles as $i => $title ) {
+ if ( $data[$title->getPrefixedDBkey()] !== false ) {
+ unset( $titles[$i] );
+ }
+ }
+ $uncachedStatus = $this->service->getPageData( $titles,
$this->cachedDays, $metric );
+ foreach ( $uncachedStatus->success as $title => $succes ) {
+ $titleData = isset( $uncachedStatus->getValue()[$title]
) ?
+ $uncachedStatus->getValue()[$title] : null;
+ if ( !is_array( $titleData ) || count( $titleData ) <
$this->cachedDays ) {
+ // PageViewService is expected to return [ date
=> null ] for all requested dates
+ $this->logger->warning( 'Upstream service
returned invalid data for {title}', [
+ 'title' => $title,
+ 'statusMessage' => Status::wrap(
$uncachedStatus )->getWikiText( null, null, 'en' ),
+ ] );
+ $titleData = $this->extendDateRange( is_array(
$titleData ) ? $titleData : [],
+ $this->cachedDays );
+ }
+ $data[$title] = $titleData;
+ if ( $succes ) {
+ $statuses[$title] = StatusValue::newGood();
+ $expiry = $this->getCacheExpiry( $metric,
self::SCOPE_ARTICLE );
+ } else {
+ $data[$title]['#error'] = true;
+ $statuses[$title] = StatusValue::newFatal(
'pvi-cached-error-title', $title,
+ \Message::durationParam(
self::ERROR_EXPIRY ) );
+ $expiry = self::ERROR_EXPIRY;
+ }
+ $this->cache->set( $titleToCacheKey[$title],
$data[$title], $expiry );
+ unset( $data[$title]['#error'] );
+ }
+
+ // Almost done; we need to truncate the data at the first
"hole" (title not returned
+ // either by getMulti or getPageData) so we return a
consecutive prefix of the
+ // requested titles and do not mess up continuation.
+ $holeIndex = array_search( false, array_values( $data ), true );
+ $data = array_slice( $data, 0, $holeIndex ?: null, true );
+ $statuses = array_slice( $statuses, 0, $holeIndex ?: null, true
);
+
+ $status = StatusValue::newGood( $data );
+ array_walk( $statuses, [ $status, 'merge' ] );
+ $status->success = array_map( function( StatusValue $s ) {
+ return $s->isOK();
+ }, $statuses );
+ $status->successCount = count( array_filter( $status->success )
);
+ $status->failCount = count( $status->success ) -
$status->successCount;
+ $status->setResult( array_filter( $status->success ), $data );
+ return $status;
+ }
+
+ /**
+ * Add extra days (with a null value) to the beginning of a date range
to make it have at least
+ * ::$cachedDays days.
+ * @param array $data YYYY-MM-DD => count, ordered, has less than
$cachedDays items
+ * @param int $days
+ * @return array
+ */
+ protected function extendDateRange( $data, $days ) {
+ reset( $data );
+ // set to noon to avoid skip second and similar problems
+ $day = strtotime( key( $data ) . 'T00:00Z' ) + 12 * 3600;
+ for ( $i = $days - count( $data ); $i > 0; $i-- ) {
+ $day -= 24 * 3600;
+ $data = [ gmdate( 'Y-m-d', $day ) => null ] + $data;
+ }
+ return $data;
+ }
+}
diff --git a/includes/Hooks.php b/includes/Hooks.php
index e94ac8c..7ec083e 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -6,20 +6,27 @@
use FormatJson;
use Html;
use MediaWiki\MediaWikiServices;
-use ObjectCache;
-use Title;
class Hooks {
-
/**
* @param IContextSource $ctx
* @param array $pageInfo
*/
public static function onInfoAction( IContextSource $ctx, array
&$pageInfo ) {
- $views = self::getMonthViews( $ctx->getTitle() );
- if ( !$views ) {
+ /** @var PageViewService $pageViewService */
+ $pageViewService =
MediaWikiServices::getInstance()->getService( 'PageViewService' );
+ if ( !$pageViewService->supports( PageViewService::METRIC_VIEW,
+ PageViewService::SCOPE_ARTICLE )
+ ) {
return;
}
+ $title = $ctx->getTitle();
+ $status = $pageViewService->getPageData( [ $title ], 30,
PageViewService::METRIC_VIEW );
+ $data = $status->getValue();
+ if ( !$status->isOK() ) {
+ return;
+ }
+ $views = $data[$title->getPrefixedDBkey()];
$total = array_sum( $views );
reset( $views );
@@ -52,33 +59,6 @@
'end' => $lang->userDate( $end, $user ),
],
] );
- }
-
- protected static function getMonthViews( Title $title ) {
- $cache = ObjectCache::getLocalClusterInstance();
- $key = $cache->makeKey( 'pvi', 'month', md5(
$title->getPrefixedText() ) );
- $data = $cache->get( $key );
- if ( $data ) {
- return $data;
- }
-
- /** @var PageViewService $pageViewService */
- $pageViewService =
MediaWikiServices::getInstance()->getService( 'PageViewService' );
- if ( !$pageViewService->supports( PageViewService::METRIC_VIEW,
- PageViewService::SCOPE_ARTICLE )
- ) {
- return false;
- }
-
- $status = $pageViewService->getPageData( [ $title ], 30,
PageViewService::METRIC_VIEW );
- if ( !$status->isOK() ) {
- $cache->set( $key, false, 300 );
- }
-
- $data = $status->getValue()[$title->getPrefixedDBkey()];
- $cache->set( $key, $data, $pageViewService->getCacheExpiry(
PageViewService::METRIC_VIEW,
- PageViewService::SCOPE_ARTICLE ) );
- return $data;
}
/**
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index cbd86db..981aea9 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -4,6 +4,7 @@
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MediaWikiServices;
+use ObjectCache;
return [
'PageViewService' => function ( MediaWikiServices $services ) {
@@ -12,9 +13,14 @@
$endpoint = $extensionConfig->get(
'PageViewInfoWikimediaEndpoint' );
$project = $extensionConfig->get( 'PageViewInfoWikimediaDomain'
)
?: $mainConfig->get( 'ServerName' );
- $pageViewService = new WikimediaPageViewService( $endpoint, [
'project' => $project ],
+ $cache = ObjectCache::getLocalClusterInstance();
+ $logger = LoggerFactory::getInstance( 'PageViewInfo' );
+
+ $service = new WikimediaPageViewService( $endpoint, [ 'project'
=> $project ],
$extensionConfig->get(
'PageViewInfoWikimediaRequestLimit' ) );
- $pageViewService->setLogger( LoggerFactory::getInstance(
'PageViewInfo' ) );
- return $pageViewService;
+ $service->setLogger( $logger );
+ $cachedService = new CachedPageViewService( $service, $cache );
+ $cachedService->setLogger( $logger );
+ return $cachedService;
},
];
diff --git a/tests/phpunit/CachedPageViewServiceTest.php
b/tests/phpunit/CachedPageViewServiceTest.php
new file mode 100644
index 0000000..9772667
--- /dev/null
+++ b/tests/phpunit/CachedPageViewServiceTest.php
@@ -0,0 +1,386 @@
+<?php
+
+namespace MediaWiki\Extensions\PageViewInfo;
+
+use HashBagOStuff;
+use StatusValue;
+
+class CachedPageViewServiceTest extends \PHPUnit_Framework_TestCase {
+ /** @var CachedPageViewService */
+ protected $service;
+ /** @var \PHPUnit_Framework_MockObject_MockObject */
+ protected $mock;
+
+ protected function setUp() {
+ parent::setUp();
+ $cache = new HashBagOStuff();
+ $this->mock = $this->getMockBuilder( PageViewService::class
)->getMockForAbstractClass();
+ $this->service = new CachedPageViewService( $this->mock, $cache
);
+ $this->service->setCachedDays( 2 );
+ }
+
+ /**
+ * @dataProvider provideSupports
+ */
+ public function testSupports( $metric, $scope, $expectation ) {
+ $this->mock->expects( $this->once() )
+ ->method( 'supports' )
+ ->willReturnCallback( function ( $metric, $scope ) {
+ return $metric === PageViewService::METRIC_VIEW
||
+ $scope ===
PageViewService::SCOPE_SITE;
+ } );
+ $this->assertSame( $expectation, $this->service->supports(
$metric, $scope ) );
+ }
+
+ public function provideSupports() {
+ return [
+ [ PageViewService::METRIC_VIEW,
PageViewService::SCOPE_ARTICLE, true ],
+ [ PageViewService::METRIC_UNIQUE,
PageViewService::SCOPE_SITE, true ],
+ [ PageViewService::METRIC_UNIQUE,
PageViewService::SCOPE_ARTICLE, false ],
+ ];
+ }
+
+ public function testGetCacheExpiry() {
+ $this->mock->expects( $this->once() )
+ ->method( 'getCacheExpiry' )
+ ->willReturn( 1000 );
+ $expiry = $this->service->getCacheExpiry(
PageViewService::METRIC_VIEW,
+ PageViewService::SCOPE_ARTICLE );
+ $this->assertGreaterThanOrEqual( 1000, $expiry );
+ $this->assertLessThanOrEqual( 1600, $expiry );
+ }
+
+ public function testGetPageData() {
+ $expectedTitles = [];
+ $this->service->setCachedDays( 2 );
+ $this->mock->expects( $this->any() )
+ ->method( 'getCacheExpiry' )
+ ->willReturn( 1000 );
+ $this->mock->expects( $this->any() )
+ ->method( 'getPageData' )
+ ->with( $this->anything(), $this->anything(),
$this->logicalOr(
+ PageViewService::METRIC_VIEW,
PageViewService::METRIC_UNIQUE ) )
+ ->willReturnCallback( function ( $titles, $days,
$metric ) use ( &$expectedTitles ) {
+ $metric = ( $metric ===
PageViewService::METRIC_VIEW ) + 1;
+ $titles = array_fill_keys( array_map( function
( \Title $t ) {
+ return $t->getPrefixedDBkey();
+ }, $titles ), null );
+ $this->assertSame( $expectedTitles, array_keys(
$titles ) );
+ // 'A' => 1, 'B' => 2, ...
+ $pages = array_combine( array_map( function (
$n ) {
+ return chr( ord( 'A' ) + $n - 1 );
+ }, range( 1, 20 ) ), range( 1, 20 ) );
+ // simulate a page-per-query limit of 3
+ $base = array_slice( array_intersect_key(
$pages, $titles ), 0, 3 );
+ $perDay = array_slice( [
+ '2000-01-01' => $metric * 1,
+ '2000-01-02' => $metric * 2,
+ '2000-01-03' => $metric * 3,
+ '2000-01-04' => $metric * 4,
+ '2000-01-05' => $metric * 5,
+ ], -$days );
+ // add some errors
+ $data = array_map( function ( $multiplier ) use
( $perDay ) {
+ if ( $multiplier > 10 && $multiplier %
2 ) {
+ return null;
+ }
+ return array_map( function ( $count )
use ( $multiplier ) {
+ return $count * $multiplier;
+ }, $perDay );
+ }, $base );
+ $status = StatusValue::newGood();
+ foreach ( $data as $title => $titleData ) {
+ if ( $titleData === null ) {
+ $status->error( '500 #' .
$title );
+ }
+ }
+ $status->success = array_map( function ( $v ) {
+ return $v !== null;
+ }, $data );
+ $status->successCount = count( array_filter(
$status->success ) );
+ $status->failCount = count( $status->success )
- $status->successCount;
+ $status->setResult( $status->successCount,
array_map( function ( $titleData ) use ( $perDay ) {
+ return $titleData ?: array_fill_keys(
array_keys( $perDay ), null );
+ }, $data ) );
+ return $status;
+ } );
+ $makeTitles = function( $titles ) {
+ return array_map( function ( $t ) {
+ return \Title::newFromText( $t );
+ }, $titles );
+ };
+
+ $expectedTitles = [ 'A', 'B' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'B'
] ), 1,
+ PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-05' => 10 ],
+ 'B' => [ '2000-01-05' => 20 ],
+ ], $status->getValue() );
+ $this->assertSame( [ 'A' => true, 'B' => true ],
$status->success );
+ $this->assertSame( 2, $status->successCount );
+ $this->assertSame( 0, $status->failCount );
+
+ // second call should not trigger a new call to the wrapped
service, regardless of $days
+ // days beyond setCachedDays() should be null
+ $expectedTitles = [];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'B'
] ), 3,
+ PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-03' => null, '2000-01-04' => 8,
'2000-01-05' => 10 ],
+ 'B' => [ '2000-01-03' => null, '2000-01-04' => 16,
'2000-01-05' => 20 ],
+ ], $status->getValue() );
+ $this->assertSame( [ 'A' => true, 'B' => true ],
$status->success );
+ $this->assertSame( 2, $status->successCount );
+ $this->assertSame( 0, $status->failCount );
+
+ // caching should not mix up metrics
+ $expectedTitles = [ 'A', 'B' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'B'
] ), 1,
+ PageViewService::METRIC_UNIQUE );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-05' => 5 ],
+ 'B' => [ '2000-01-05' => 10 ],
+ ], $status->getValue() );
+ $this->assertSame( [ 'A' => true, 'B' => true ],
$status->success );
+ $this->assertSame( 2, $status->successCount );
+ $this->assertSame( 0, $status->failCount );
+
+ // titles should be cached individually
+ $expectedTitles = [ 'C' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'C'
] ), 1,
+ PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-05' => 10 ],
+ 'C' => [ '2000-01-05' => 30 ],
+ ], $status->getValue() );
+ $this->assertSame( [ 'A' => true, 'C' => true ],
$status->success );
+ $this->assertSame( 2, $status->successCount );
+ $this->assertSame( 0, $status->failCount );
+
+ // needs to handle the wrapped service returning less pages
than asked
+ $expectedTitles = [ 'D', 'E', 'F', 'G' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'B',
'C', 'D', 'E', 'F', 'G' ] ), 1,
+ PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-05' => 10 ],
+ 'B' => [ '2000-01-05' => 20 ],
+ 'C' => [ '2000-01-05' => 30 ],
+ 'D' => [ '2000-01-05' => 40 ],
+ 'E' => [ '2000-01-05' => 50 ],
+ 'F' => [ '2000-01-05' => 60 ],
+ ], $status->getValue() );
+ $this->assertSame( [ 'A' => true, 'B' => true, 'C' => true, 'D'
=> true, 'E' => true,
+ 'F' => true ], $status->success );
+ $this->assertSame( 6, $status->successCount );
+ $this->assertSame( 0, $status->failCount );
+
+ // some errors
+ $expectedTitles = [ 'K', 'L' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'K',
'L' ] ), 1,
+ PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertFalse( $status->isGood() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-05' => 10 ],
+ 'K' => [ '2000-01-05' => null ],
+ 'L' => [ '2000-01-05' => 120 ],
+ ], $status->getValue() );
+ $this->assertTrue( $status->hasMessage(
'pvi-cached-error-title' ) );
+ $this->assertFalse( $status->hasMessage( '500 #K' ) );
+ $this->assertSame( [ 'A' => true, 'K' => false, 'L' => true ],
$status->success );
+ $this->assertSame( 2, $status->successCount );
+ $this->assertSame( 1, $status->failCount );
+
+ // cached error
+ $expectedTitles = [ 'N' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'A', 'K',
'L', 'N' ] ), 1,
+ PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertFalse( $status->isGood() );
+ $this->assertSame( [
+ 'A' => [ '2000-01-05' => 10 ],
+ 'K' => [ '2000-01-05' => null ],
+ 'L' => [ '2000-01-05' => 120 ],
+ 'N' => [ '2000-01-05' => 140 ],
+ ], $status->getValue() );
+ $this->assertTrue( $status->hasMessage(
'pvi-cached-error-title' ) );
+ $this->assertSame( [ 'A' => true, 'K' => false, 'L' => true,
'N' => true ], $status->success );
+ $this->assertSame( 3, $status->successCount );
+ $this->assertSame( 1, $status->failCount );
+
+ // all errors
+ $expectedTitles = [ 'M' ];
+ $status = $this->service->getPageData( $makeTitles( [ 'K', 'M'
] ), 1,
+ PageViewService::METRIC_VIEW );
+ $this->assertFalse( $status->isOK() );
+ $this->assertFalse( $status->isGood() );
+ $this->assertSame( [
+ 'K' => [ '2000-01-05' => null ],
+ 'M' => [ '2000-01-05' => null ],
+ ], $status->getValue() );
+ $this->assertTrue( $status->hasMessage(
'pvi-cached-error-title' ) );
+ $this->assertFalse( $status->hasMessage( '500 #M' ) );
+ $this->assertSame( [ 'K' => false, 'M' => false ],
$status->success );
+ $this->assertSame( 0, $status->successCount );
+ $this->assertSame( 2, $status->failCount );
+ }
+
+ public function testGetSiteData() {
+ $cached = false;
+ $this->service->setCachedDays( 2 );
+ $this->mock->expects( $this->any() )
+ ->method( 'getCacheExpiry' )
+ ->willReturn( 1000 );
+ $this->mock->expects( $this->exactly( 2 ) )
+ ->method( 'getSiteData' )
+ ->with( $this->anything(), $this->logicalOr(
PageViewService::METRIC_VIEW,
+ PageViewService::METRIC_UNIQUE ) )
+ ->willReturnCallback( function ( $days, $metric ) use (
&$cached ) {
+ if ( $cached ) {
+ $this->fail( 'should have been cached'
);
+ }
+ $metric = ( $metric ===
PageViewService::METRIC_VIEW ) + 1;
+ return StatusValue::newGood( array_slice( [
+ '2000-01-01' => $metric * 1,
+ '2000-01-02' => $metric * 2,
+ '2000-01-03' => $metric * 3,
+ '2000-01-04' => $metric * 4,
+ '2000-01-05' => $metric * 5,
+ ], -$days ) );
+ } );
+ $status = $this->service->getSiteData( 1,
PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [ '2000-01-05' => 10 ], $status->getValue()
);
+
+ // second call should not trigger a new call to the wrapped
service, regardless of $days
+ // days beyond setCachedDays() should be null
+ $cached = true;
+ $status = $this->service->getSiteData( 3,
PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [ '2000-01-03' => null, '2000-01-04' => 8,
'2000-01-05' => 10 ],
+ $status->getValue() );
+
+ // caching should not mix up metrics
+ $cached = false;
+ $status = $this->service->getSiteData( 1,
PageViewService::METRIC_UNIQUE );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [ '2000-01-05' => 5 ], $status->getValue() );
+ }
+
+ public function testGetSiteData_error() {
+ $cached = false;
+ $this->service->setCachedDays( 2 );
+ $this->mock->expects( $this->any() )
+ ->method( 'getCacheExpiry' )
+ ->willReturn( 1000 );
+ $this->mock->expects( $this->exactly( 2 ) )
+ ->method( 'getSiteData' )
+ ->with( $this->anything(), $this->logicalOr(
PageViewService::METRIC_VIEW,
+ PageViewService::METRIC_UNIQUE ) )
+ ->willReturnCallback( function ( $days, $metric ) use (
&$cached ) {
+ if ( $cached ) {
+ $this->fail( 'should have been cached'
);
+ }
+ return $metric === PageViewService::METRIC_VIEW
? StatusValue::newFatal( '500' )
+ : StatusValue::newFatal( '500 #2' );
+ } );
+ $status = $this->service->getSiteData( 1,
PageViewService::METRIC_VIEW );
+ $this->assertFalse( $status->isOK() );
+ $this->assertTrue( $status->hasMessage( '500' ) );
+ $this->assertFalse( $status->hasMessage( 'pvi-cached-error' ) );
+
+ // second call should not trigger a new call to the wrapped
service, regardless of $days
+ $cached = true;
+ $status = $this->service->getSiteData( 3,
PageViewService::METRIC_VIEW );
+ $this->assertFalse( $status->isOK() );
+ $this->assertTrue( $status->hasMessage( 'pvi-cached-error' ) );
+
+ // caching should not mix up metrics
+ $cached = false;
+ $status = $this->service->getSiteData( 1,
PageViewService::METRIC_UNIQUE );
+ $this->assertFalse( $status->isOK() );
+ $this->assertTrue( $status->hasMessage( '500 #2' ) );
+ $this->assertFalse( $status->hasMessage( 'pvi-cached-error' ) );
+ }
+
+ public function testGetTopPages() {
+ $cached = false;
+ $this->mock->expects( $this->any() )
+ ->method( 'getCacheExpiry' )
+ ->willReturn( 1000 );
+ $this->mock->expects( $this->exactly( 2 ) )
+ ->method( 'getTopPages' )
+ ->with( $this->logicalOr( PageViewService::METRIC_VIEW,
PageViewService::METRIC_UNIQUE ) )
+ ->willReturnCallback( function ( $metric ) use (
&$cached ) {
+ if ( $cached ) {
+ $this->fail( 'should have been cached'
);
+ }
+ switch ( $metric ) {
+ case PageViewService::METRIC_VIEW:
+ return StatusValue::newGood( [
'A' => 100, 'B' => 10, 'C' => 1 ] );
+ case PageViewService::METRIC_UNIQUE:
+ return StatusValue::newGood( [
'A' => 50, 'B' => 5, 'C' => 1 ] );
+ }
+ } );
+ $status = $this->service->getTopPages(
PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [ 'A' => 100, 'B' => 10, 'C' => 1 ],
$status->getValue() );
+
+ // second call should not trigger a new call to the wrapped
service
+ $cached = true;
+ $status = $this->service->getTopPages(
PageViewService::METRIC_VIEW );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [ 'A' => 100, 'B' => 10, 'C' => 1 ],
$status->getValue() );
+
+ // caching should not mix up metrics
+ $cached = false;
+ $status = $this->service->getTopPages(
PageViewService::METRIC_UNIQUE );
+ $this->assertTrue( $status->isOK() );
+ $this->assertSame( [ 'A' => 50, 'B' => 5, 'C' => 1 ],
$status->getValue() );
+ }
+
+ public function testGetTopPages_error() {
+ $cached = false;
+ $this->mock->expects( $this->any() )
+ ->method( 'getCacheExpiry' )
+ ->willReturn( 1000 );
+ $this->mock->expects( $this->exactly( 2 ) )
+ ->method( 'getTopPages' )
+ ->with( $this->logicalOr( PageViewService::METRIC_VIEW,
PageViewService::METRIC_UNIQUE ) )
+ ->willReturnCallback( function ( $metric ) use (
&$cached ) {
+ if ( $cached ) {
+ $this->fail( 'should have been cached'
);
+ }
+ switch ( $metric ) {
+ case PageViewService::METRIC_VIEW:
+ return StatusValue::newFatal(
'500' );
+ case PageViewService::METRIC_UNIQUE:
+ return StatusValue::newFatal(
'500 #2' );
+ }
+ } );
+ $status = $this->service->getTopPages(
PageViewService::METRIC_VIEW );
+ $this->assertFalse( $status->isOK() );
+ $this->assertTrue( $status->hasMessage( '500' ) );
+ $this->assertFalse( $status->hasMessage( 'pvi-cached-error' ) );
+
+ // second call should not trigger a new call to the wrapped
service
+ $cached = true;
+ $status = $this->service->getTopPages(
PageViewService::METRIC_VIEW );
+ $this->assertFalse( $status->isOK() );
+ $this->assertFalse( $status->hasMessage( '500' ) );
+ $this->assertTrue( $status->hasMessage( 'pvi-cached-error' ) );
+
+ // caching should not mix up metrics
+ $cached = false;
+ $status = $this->service->getTopPages(
PageViewService::METRIC_UNIQUE );
+ $this->assertFalse( $status->isOK() );
+ $this->assertTrue( $status->hasMessage( '500 #2' ) );
+ $this->assertFalse( $status->hasMessage( 'pvi-cached-error' ) );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/320325
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib8feb757caf1f19c0b245fd90aec42537b0a84a7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/PageViewInfo
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
