Mailing List Archive

Tpt has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/406009 )

Change subject: Adds PageQualityLevelLookup
......................................................................

Adds PageQualityLevelLookup

Change-Id: Ief0459292f90b742337f68d622e638a40c41bf38
---
M ProofreadPage.body.php
M extension.json
M includes/Context.php
M includes/Page/ProofreadPageDbConnector.php
M includes/Parser/PagesTagParser.php
A includes/page/DatabasePageQualityLevelLookup.php
A includes/page/PageQualityLevelLookup.php
M tests/phpunit/ContextTest.php
A tests/phpunit/Page/DatabasePageQualityLevelLookupTest.php
A tests/phpunit/Page/PageQualityLevelLookupMock.php
M tests/phpunit/ProofreadPageTestCase.php
11 files changed, 267 insertions(+), 149 deletions(-)


git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ProofreadPage refs/changes/09/406009/1

diff --git a/ProofreadPage.body.php b/ProofreadPage.body.php
index b50dea5..8f00ac4 100644
--- a/ProofreadPage.body.php
+++ b/ProofreadPage.body.php
@@ -146,72 +146,39 @@

/**
* Hook function
- * @param array $page_ids Prefixed DB keys of the pages linked to, indexed by page_id
+ * @param array $pageIds Prefixed DB keys of the pages linked to, indexed by page_id
* @param array &$colours CSS classes, indexed by prefixed DB keys
* @return bool
*/
- public static function onGetLinkColours( $page_ids, &$colours ) {
+ public static function onGetLinkColours( $pageIds, &$colours ) {
global $wgTitle;
if ( !isset( $wgTitle ) ) {
return true;
}
- self::getLinkColours( $page_ids, $colours );
+
+ $inIndexNamespace = $wgTitle->inNamespace( self::getIndexNamespaceId() );
+ $pageQualityLevelLookup = Context::getDefaultContext()->getPageQualityLevelLookup();
+
+ $pageTitles = array_map( function ( $prefixedDbKey ) {
+ return Title::newFromText( $prefixedDbKey );
+ }, $pageIds );
+ $pageQualityLevelLookup->prefetchQualityLevelForTitles( $pageTitles );
+
+ /** @var Title|null $pageTitle */
+ foreach ( $pageTitles as $pageTitle ) {
+ if ( $pageTitle !== null && $pageTitle->inNamespace( self::getPageNamespaceId() ) ) {
+ $pageLevel = $pageQualityLevelLookup->getQualityLevelForPageTitle( $pageTitle );
+ if ( $pageLevel !== null ) {
+ $classes = "prp-pagequality-{$pageLevel}";
+ if ( $inIndexNamespace ) {
+ $classes .= " quality{$pageLevel}";
+ }
+ $colours[$pageTitle->getPrefixedDBkey()] = $classes;
+ }
+ }
+ }
+
return true;
- }
-
- /**
- * Return the quality colour codes to pages linked from an index page
- * @param array $page_ids Prefixed DB keys of the pages linked to, indexed by page_id
- * @param array $colours CSS classes, indexed by prefixed DB keys
- */
- private static function getLinkColours( $page_ids, &$colours ) {
- global $wgTitle;
-
- $page_namespace_id = self::getPageNamespaceId();
- $in_index_namespace = $wgTitle->inNamespace( self::getIndexNamespaceId() );
-
- $values = [];
- foreach ( $page_ids as $id => $pdbk ) {
- $title = Title::newFromText( $pdbk );
- // consider only link in page namespace
- if ( $title->inNamespace( $page_namespace_id ) ) {
- if ( $in_index_namespace ) {
- $colours[$pdbk] = 'quality1 prp-pagequality-1';
- } else {
- $colours[$pdbk] = 'prp-pagequality-1';
- }
- $values[] = intval( $id );
- }
- }
-
- // Get the names of the quality categories. Replaces earlier code which
- // called wfMessage()->inContentLanguagE() 5 times for each page.
- // ISSUE: Should the number of quality levels be adjustable?
- // ISSUE 2: Should this array be saved as a member variable?
- // How often is this code called anyway?
- $qualityCategories = [];
- for ( $i = 0; $i < 5; $i++ ) {
- $cat = Title::makeTitleSafe( NS_CATEGORY,
- wfMessage( "proofreadpage_quality{$i}_category" )->inContentLanguage()->text() );
- if ( $cat ) {
- if ( $in_index_namespace ) {
- $qualityCategories[$cat->getDBkey()] =
- 'quality' . $i . ' prp-pagequality-' . $i;
- } else {
- $qualityCategories[$cat->getDBkey()] = 'prp-pagequality-' . $i;
- }
- }
- }
-
- if ( count( $values ) ) {
- $res = ProofreadPageDbConnector::getCategoryNamesForPageIds( $values );
- foreach ( $res as $x ) {
- $pdbk = $page_ids[$x->cl_from];
- if ( array_key_exists( $x->cl_to, $qualityCategories ) ) {
- $colours[$pdbk] = $qualityCategories[$x->cl_to];
- }
- }
- }
}

/**
@@ -761,27 +728,13 @@
if ( !$title->inNamespace( self::getPageNamespaceId() ) ) {
return true;
}
- $pageid = $page->getId();

- $params = new FauxRequest( [.
- 'action' => 'query',
- 'prop' => 'proofread',
- 'pageids' => $pageid,
- ] );
-
- $api = new ApiMain( $params );
- $api->execute();
- $data = $api->getResult()->getResultData();
-
- if ( array_key_exists( 'error', $data ) ) {
- return true;
- }
-
- $info = $data['query']['pages'][$pageid];
- if ( array_key_exists( 'proofread', $info ) ) {
+ $pageQualityLevelLookup = Context::getDefaultContext()->getPageQualityLevelLookup();
+ $pageQualityLevel = $pageQualityLevelLookup->getQualityLevelForPageTitle( $title );
+ if ( $pageQualityLevel !== null ) {
$pageInfo['header-basic'][] = [
wfMessage( 'proofreadpage-pageinfo-status' ),
- wfMessage( "proofreadpage_quality{$info['proofread']['quality']}_category" ),
+ wfMessage( "proofreadpage_quality{$pageQualityLevel}_category" ),
];
}

diff --git a/extension.json b/extension.json
index e52a9e5..8997296 100644
--- a/extension.json
+++ b/extension.json
@@ -59,6 +59,7 @@
"ApiQueryProofreadInfo": "ApiQueryProofreadInfo.php",
"ProofreadPage\\FileProviderMock": "tests/phpunit/FileProviderMock.php",
"ProofreadPage\\Page\\IndexForPageLookupMock": "tests/phpunit/Page/IndexForPageLookupMock.php",
+ "ProofreadPage\\Page\\PageQualityLevelLookupMock": "tests/phpunit/Page/PageQualityLevelLookupMock.php",
"ProofreadPage\\Index\\IndexContentLookupMock": "tests/phpunit/Index/IndexContentLookupMock.php",
"ProofreadPageTestCase": "tests/phpunit/ProofreadPageTestCase.php",
"FixProofreadPagePagesContentModel": "maintenance/fixProofreadPagePagesContentModel.php",
diff --git a/includes/Context.php b/includes/Context.php
index 00730f2..3d81cce 100644
--- a/includes/Context.php
+++ b/includes/Context.php
@@ -6,7 +6,9 @@
use ProofreadPage\Index\DatabaseIndexContentLookup;
use ProofreadPage\Index\IndexContentLookup;
use ProofreadPage\Page\DatabaseIndexForPageLookup;
+use ProofreadPage\Page\DatabasePageQualityLevelLookup;
use ProofreadPage\Page\IndexForPageLookup;
+use ProofreadPage\Page\PageQualityLevelLookup;
use ProofreadPage\Pagination\PaginationFactory;
use RepoGroup;

@@ -52,17 +54,23 @@
private $indexContentLookup;

/**
+ * @var PageQualityLevelLookup
+ */
+ private $pageQualityLevelLookup;
+
+ /**
* @param int $pageNamespaceId
* @param int $indexNamespaceId
* @param FileProvider $fileProvider
* @param CustomIndexFieldsParser $customIndexFieldsParser
* @param IndexForPageLookup $indexForPageLookup
* @param IndexContentLookup $indexContentLookup
+ * @param PageQualityLevelLookup $pageQualityLevelLookup
*/
public function __construct(
$pageNamespaceId, $indexNamespaceId, FileProvider $fileProvider,
CustomIndexFieldsParser $customIndexFieldsParser, IndexForPageLookup $indexForPageLookup,
- IndexContentLookup $indexContentLookup
+ IndexContentLookup $indexContentLookup, PageQualityLevelLookup $pageQualityLevelLookup
) {
$this->pageNamespaceId = $pageNamespaceId;
$this->indexNamespaceId = $indexNamespaceId;
@@ -70,6 +78,7 @@
$this->customIndexFieldsParser = $customIndexFieldsParser;
$this->indexForPageLookup = $indexForPageLookup;
$this->indexContentLookup = $indexContentLookup;
+ $this->pageQualityLevelLookup = $pageQualityLevelLookup;
}

/**
@@ -122,6 +131,13 @@
}

/**
+ * @return PageQualityLevelLookup
+ */
+ public function getPageQualityLevelLookup() {
+ return $this->pageQualityLevelLookup;
+ }
+
+ /**
* @param bool $purge
* @return Context
*/
@@ -136,7 +152,8 @@
new FileProvider( $repoGroup ),
new CustomIndexFieldsParser(),
new DatabaseIndexForPageLookup( $indexNamespaceId, $repoGroup ),
- new DatabaseIndexContentLookup()
+ new DatabaseIndexContentLookup(),
+ new DatabasePageQualityLevelLookup( $pageNamespaceId )
);
}

diff --git a/includes/Page/ProofreadPageDbConnector.php b/includes/Page/ProofreadPageDbConnector.php
index f73b0ba..55131dc 100644
--- a/includes/Page/ProofreadPageDbConnector.php
+++ b/includes/Page/ProofreadPageDbConnector.php
@@ -22,41 +22,6 @@
class ProofreadPageDbConnector {

/**
- * @param array $pageIds
- * @return ResultWrapper
- */
- public static function getCategoryNamesForPageIds( $pageIds ) {
- $dbr = wfGetDB( DB_REPLICA );
- return $dbr->select(
- [ 'categorylinks' ],
- [ 'cl_from', 'cl_to' ],
- [ 'cl_from IN(' . implode( ',', $pageIds ) . ')' ],
- __METHOD__
- );
- }
-
- /**
- * @param array $pp
- * @param array $cat
- * @return ResultWrapper
- */
- public static function getPagesNameInCategory( $pp, $cat ) {
- $dbr = wfGetDB( DB_REPLICA );
- return $dbr->select(
- [ 'page', 'categorylinks' ],
- [ 'page_title' ],
- [.
- 'page_title' => $pp,
- 'cl_to' => $cat,
- 'page_namespace' => ProofreadPage::getPageNamespaceId()
- ],
- __METHOD__,
- null,
- [ 'categorylinks' => [ 'LEFT JOIN', 'cl_from=page_id' ] ]
- );
- }
-
- /**
* @param array $query
* @param string $cat
* @return int
diff --git a/includes/Parser/PagesTagParser.php b/includes/Parser/PagesTagParser.php
index 9232f96..1a6f0e8 100644
--- a/includes/Parser/PagesTagParser.php
+++ b/includes/Parser/PagesTagParser.php
@@ -3,9 +3,8 @@
namespace ProofreadPage\Parser;

use OutOfBoundsException;
-use ProofreadPage\Context;
+use ProofreadPage\Page\PageLevel;
use ProofreadPage\Pagination\FilePagination;
-use ProofreadPageDbConnector;
use Title;

/**
@@ -184,35 +183,19 @@
list( $from_page, $from_pagenum ) = reset( $pages );
list( $to_page, $to_pagenum ) = end( $pages );

- // find which pages have quality0
- $q0_pages = [];
- if ( !empty( $pages ) ) {
- $pp = [];
- foreach ( $pages as $item ) {
- list( $page, $pagenum ) = $item;
- $pp[] = $page->getDBkey();
- }
- $cat = str_replace( ' ', '_', wfMessage( 'proofreadpage_quality0_category' )
- ->inContentLanguage()->escaped() );
- $res = ProofreadPageDbConnector::getPagesNameInCategory( $pp, $cat );
+ $pageQualityLevelLookup = $this->context->getPageQualityLevelLookup();

- if ( $res ) {
- foreach ( $res as $o ) {
- $q0_pages[] = $o->page_title;
- }
- }
- }
+ $pageTitlesToPrefetch = array_map(function ($item) {
+ return $item[0];
+ }, $pages );
+ $pageQualityLevelLookup->prefetchQualityLevelForTitles( $pageTitlesToPrefetch );

// write the output
foreach ( $pages as $item ) {
list( $page, $pagenum ) = $item;
- if ( in_array( $page->getDBKey(), $q0_pages ) ) {
- $is_q0 = true;
- } else {
- $is_q0 = false;
- }
+ $qualityLevel = $pageQualityLevelLookup->getQualityLevelForPageTitle( $page );
$text = $page->getPrefixedText();
- if ( !$is_q0 ) {
+ if ( $qualityLevel !== PageLevel::WITHOUT_TEXT ) {
$out .= '<span>{{:MediaWiki:Proofreadpage_pagenum_template|page=' . $text .
"|num=$pagenum}}</span>";
}
@@ -230,7 +213,7 @@
} else {
$out .= '{{:' . $text . '}}';
}
- if ( !$is_q0 ) {
+ if ( $qualityLevel !== PageLevel::WITHOUT_TEXT ) {
$out .= "&#32;";
}
}
diff --git a/includes/page/DatabasePageQualityLevelLookup.php b/includes/page/DatabasePageQualityLevelLookup.php
new file mode 100644
index 0000000..8d2719b
--- /dev/null
+++ b/includes/page/DatabasePageQualityLevelLookup.php
@@ -0,0 +1,80 @@
+<?php
+
+namespace ProofreadPage\Page;
+
+use InvalidArgumentException;
+use Title;
+
+/**
+ * @licence GNU GPL v2+
+ *
+ * Allows to retrieve the quality level of a Page: page
+ */
+class DatabasePageQualityLevelLookup implements PageQualityLevelLookup {
+
+ private $pageNamespaceId;
+
+ private $cache = [];
+
+ public function __construct( $pageNamespaceId ) {
+ $this->pageNamespaceId = $pageNamespaceId;
+ }
+
+ /**
+ * @inheritDoc
+ */
+ public function getQualityLevelForPageTitle( Title $pageTitle ) {
+ if ( !$pageTitle->inNamespace( $this->pageNamespaceId ) ) {
+ throw new InvalidArgumentException( $pageTitle . ' is not in Page: namespace' );
+ }
+ $cacheKey = $pageTitle->getDBkey();
+ if ( !array_key_exists( $cacheKey, $this->cache ) ) {
+ $this->fetchQualityLevelForPageTitles( [ $pageTitle ] );
+ }
+ return $this->cache[$cacheKey];
+ }
+
+ /**
+ * @inheritDoc
+ */
+ public function prefetchQualityLevelForTitles( array $pageTitles ) {
+ $pageTitles = array_filter( $pageTitles, function ( $pageTitle ) {
+ return $pageTitle instanceof Title &&
+ $pageTitle->inNamespace( $this->pageNamespaceId ) &&
+ !array_key_exists( $pageTitle->getDBkey(), $this->cache );
+ } );
+
+ if ( empty( $pageTitles ) ) {
+ return;
+ }
+
+ $this->fetchQualityLevelForPageTitles( $pageTitles );
+ }
+
+ /**
+ * @param Title[] $pageTitles
+ */
+ private function fetchQualityLevelForPageTitles( array $pageTitles ) {
+ // We set to unknown all qualities
+ foreach ( $pageTitles as $pageTitle ) {
+ $this->cache[$pageTitle->getDBkey()] = null;
+ }
+
+ $results = wfGetDB( DB_REPLICA )->select(
+ [ 'page_props', 'page' ],
+ [ 'page_title', 'pp_value' ],
+ [.
+ 'pp_propname' => 'proofread_page_quality_level',
+ 'page_id = pp_page',
+ 'page_namespace' => $this->pageNamespaceId,
+ 'page_title' => array_map( function ( Title $pageTitle ) {
+ return $pageTitle->getDBkey();
+ }, $pageTitles )
+ ],
+ __METHOD__
+ );
+ foreach ( $results as $result ) {
+ $this->cache[$result->page_title] = $result->pp_value;
+ }
+ }
+}
diff --git a/includes/page/PageQualityLevelLookup.php b/includes/page/PageQualityLevelLookup.php
new file mode 100644
index 0000000..083f8c0
--- /dev/null
+++ b/includes/page/PageQualityLevelLookup.php
@@ -0,0 +1,22 @@
+<?php
+namespace ProofreadPage\Page;
+
+use Title;
+
+/**
+ * @licence GNU GPL v2+
+ *
+ * Allows to retrieve the quality level for a Page: page
+ */
+interface PageQualityLevelLookup {
+
+ /**
+ * @return int|null
+ */
+ public function getQualityLevelForPageTitle( Title $pageTitle );
+
+ /**
+ * @param Title[] $pageTitles
+ */
+ public function prefetchQualityLevelForTitles( array $pageTitles );
+}
diff --git a/tests/phpunit/ContextTest.php b/tests/phpunit/ContextTest.php
index e750026..778f2ac 100644
--- a/tests/phpunit/ContextTest.php
+++ b/tests/phpunit/ContextTest.php
@@ -3,8 +3,13 @@
namespace ProofreadPage;

use ProofreadPage\Index\CustomIndexFieldsParser;
+use ProofreadPage\Index\IndexContentLookup;
use ProofreadPage\Index\IndexContentLookupMock;
+use ProofreadPage\Page\IndexForPageLookup;
use ProofreadPage\Page\IndexForPageLookupMock;
+use ProofreadPage\Page\PageQualityLevelLookup;
+use ProofreadPage\Page\PageQualityLevelLookupMock;
+use ProofreadPage\Pagination\PaginationFactory;
use ProofreadPageTestCase;

/**
@@ -23,36 +28,51 @@

public function testGetFileProvider() {
$this->assertInstanceOf(
- '\ProofreadPage\FileProvider',
+ FileProvider::class,
$this->buildDummyContext()->getFileProvider()
);
}

public function testGetPaginationFactory() {
$this->assertInstanceOf(
- '\ProofreadPage\Pagination\PaginationFactory',
+ PaginationFactory::class,
$this->buildDummyContext()->getPaginationFactory()
);
}

- public function testCustomIndexFieldsParser() {
+ public function testGetCustomIndexFieldsParser() {
$this->assertInstanceOf(
- '\ProofreadPage\Index\CustomIndexFieldsParser',
+ CustomIndexFieldsParser::class,
$this->buildDummyContext()->getCustomIndexFieldsParser()
);
}

- public function testIndexForPageLookup() {
+ public function testGetIndexForPageLookup() {
$this->assertInstanceOf(
- '\ProofreadPage\Page\IndexForPageLookup',
+ IndexForPageLookup::class,
$this->buildDummyContext()->getIndexForPageLookup()
+ );
+ }
+
+ public function testGetIndexContentLookup() {
+ $this->assertInstanceOf(
+ IndexContentLookup::class,
+ $this->buildDummyContext()->getIndexContentLookup()
+ );
+ }
+
+ public function testGetPageQualityLevelLookup() {
+ $this->assertInstanceOf(
+ PageQualityLevelLookup::class,
+ $this->buildDummyContext()->getPageQualityLevelLookup()
);
}

private function buildDummyContext() {
return new Context( 42, 44,
- new FileProviderMock( [] ), new CustomIndexFieldsParser(), new IndexForPageLookupMock( [] ),
- new IndexContentLookupMock( [] )
+ new FileProviderMock( [] ), new CustomIndexFieldsParser(),
+ new IndexForPageLookupMock( [] ), new IndexContentLookupMock( [] ),
+ new PageQualityLevelLookupMock( [] )
);
}
}
diff --git a/tests/phpunit/Page/DatabasePageQualityLevelLookupTest.php b/tests/phpunit/Page/DatabasePageQualityLevelLookupTest.php
new file mode 100644
index 0000000..e2bf985
--- /dev/null
+++ b/tests/phpunit/Page/DatabasePageQualityLevelLookupTest.php
@@ -0,0 +1,38 @@
+<?php
+
+namespace ProofreadPage\Page;
+
+use ProofreadPageTestCase;
+use Title;
+
+/**
+ * @group ProofreadPage
+ * @covers \ProofreadPage\Page\DatabasePageQualityLevelLookup
+ */
+class DatabasePageQualityLevelLookupTest extends ProofreadPageTestCase {
+
+ /**
+ * @dataProvider prefetchQualityLevelForTitlesProvider
+ */
+ public function testprefetchQualityLevelForTitles(array $titles ) {
+ $lookup = new DatabasePageQualityLevelLookup( $this->getPageNamespaceId() );
+ $lookup->prefetchQualityLevelForTitles( $titles );
+ $this->assertTrue( true );
+ //The execution succeed
+ }
+
+ public function prefetchQualityLevelForTitlesProvider() {
+ return [
+ [
+ []
+ ],
+ [.
+ [
+ Title::makeTitle(NS_MAIN, 'Foo'),
+ Title::makeTitle($this->getPageNamespaceId(), 'Foo'),
+ null
+ ]
+ ]
+ ];
+ }
+}
diff --git a/tests/phpunit/Page/PageQualityLevelLookupMock.php b/tests/phpunit/Page/PageQualityLevelLookupMock.php
new file mode 100644
index 0000000..a947a98
--- /dev/null
+++ b/tests/phpunit/Page/PageQualityLevelLookupMock.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace ProofreadPage\Page;
+
+use Title;
+
+/**
+ * @licence GNU GPL v2+
+ *
+ * Provide a PageQualityLevelLookup mock based on a mapping
+ */
+class PageQualityLevelLookupMock implements PageQualityLevelLookup {
+
+ private $levelForPage = [];
+
+ public function __construct( $levelForPage ) {
+ $this->levelForPage = $levelForPage;
+ }
+
+ /**
+ * @inheritDoc
+ */
+ public function getQualityLevelForPageTitle( Title $pageTitle ) {
+ if ( !array_key_exists( $pageTitle->getDBkey(), $this->levelForPage ) ) {
+ return null;
+ }
+ return $this->levelForPage[ $pageTitle->getDBkey() ];
+ }
+
+ /**
+ * @inheritDoc
+ */
+ public function prefetchQualityLevelForTitles( array $pageTitles ) {
+ }
+}
diff --git a/tests/phpunit/ProofreadPageTestCase.php b/tests/phpunit/ProofreadPageTestCase.php
index a5b71a4..19ef5e8 100644
--- a/tests/phpunit/ProofreadPageTestCase.php
+++ b/tests/phpunit/ProofreadPageTestCase.php
@@ -7,6 +7,7 @@
use ProofreadPage\Index\IndexContent;
use ProofreadPage\Index\IndexContentLookupMock;
use ProofreadPage\Page\IndexForPageLookupMock;
+use ProofreadPage\Page\PageQualityLevelLookupMock;
use ProofreadPage\ProofreadPageInit;

/**
@@ -105,14 +106,17 @@
* @param IndexContent[] $indexContent
* @return Context
*/
- protected function getContext( array $indexForPage = [], array $indexContent = [] ) {
+ protected function getContext(
+ array $indexForPage = [], array $indexContent = [], array $levelForPage = []
+ ) {
return new Context(
ProofreadPageInit::getNamespaceId( 'page' ),
ProofreadPageInit::getNamespaceId( 'index' ),
$this->getFileProvider(),
new CustomIndexFieldsParser( self::$customIndexFieldsConfiguration ),
new IndexForPageLookupMock( $indexForPage ),
- new IndexContentLookupMock( $indexContent )
+ new IndexContentLookupMock( $indexContent ),
+ new PageQualityLevelLookupMock( $levelForPage )
);
}


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ief0459292f90b742337f68d622e638a40c41bf38
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/ProofreadPage
Gerrit-Branch: master
Gerrit-Owner: Tpt <thomaspt@hotmail.fr>

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