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 .= " "; } } 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 <thoma...@hotmail.fr> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits