jenkins-bot has submitted this change and it was merged. Change subject: BSApiFileBackendStore/BSApiExtJSStoreBase: Further development ......................................................................
BSApiFileBackendStore/BSApiExtJSStoreBase: Further development * Fixed paging issue * Implemented case sensitive fallback for exact file name query * Implemented clientside File model * Tweaked performance Patch Set 2: * Fixed bug in 'numeric' filter * Changed BS.model.File field definitions Patch Set 3: * Fix for case sensitive fetching Change-Id: I30c5aa63e869875dec44ef577117f7f7908b3993 --- M includes/api/BSApiExtJSStoreBase.php M includes/api/BSApiFileBackendStore.php A resources/bluespice.extjs/BS/model/File.js 3 files changed, 154 insertions(+), 59 deletions(-) Approvals: Robert Vogel: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/api/BSApiExtJSStoreBase.php b/includes/api/BSApiExtJSStoreBase.php index 441f766..abfba09 100644 --- a/includes/api/BSApiExtJSStoreBase.php +++ b/includes/api/BSApiExtJSStoreBase.php @@ -24,7 +24,7 @@ * @author Robert Vogel <vo...@hallowelt.com> * @author Patric Wirth <wi...@hallowelt.com> * @package Bluespice_Foundation - * @copyright Copyright (C) 2011 Hallo Welt! - Medienwerkstatt GmbH, All rights reserved. + * @copyright Copyright (C) 2015 Hallo Welt! - Medienwerkstatt GmbH, All rights reserved. * @license http://www.gnu.org/copyleft/gpl.html GNU Public License v2 or later * @filesource * @@ -82,12 +82,20 @@ protected $totalProperty = 'total'; public function execute() { - $aData = $this->makeData(); + $sQuery = $this->getParameter( 'query' ); + $aData = $this->makeData( $sQuery ); $FinalData = $this->postProcessData( $aData ); $this->returnData( $FinalData ); } - protected abstract function makeData(); + /** + * @param string $sQuery Potential query provided by ExtJS component. + * This is some kind of preliminary filtering. Subclass has to decide if + * and how to process it + * @return array - Full list of of data objects. Filters, paging, sorting + * will be done by the base class + */ + protected abstract function makeData( $sQuery = '' ); /** * Creates a proper output format based on the classes properties @@ -259,30 +267,30 @@ * @return boolean */ public function filterCallback( $aDataSet ) { - $aFilter = $this->getParameter('filter'); + $aFilter = $this->getParameter( 'filter' ); foreach( $aFilter as $oFilter ) { //If just one of these filters does not apply, the dataset needs //to be removed if( $oFilter->type == 'string' ) { - $bFilterApplies = $this->filterString($oFilter, $aDataSet); + $bFilterApplies = $this->filterString( $oFilter, $aDataSet ); if( !$bFilterApplies ) { return false; } } if( $oFilter->type == 'list' ) { - $bFilterApplies = $this->filterList($oFilter, $aDataSet); + $bFilterApplies = $this->filterList( $oFilter, $aDataSet ); if( !$bFilterApplies ) { return false; } } if( $oFilter->type == 'numeric' ) { - $bFilterApplies = $this->filterNumberic($oFilter, $aDataSet); + $bFilterApplies = $this->filterNumeric( $oFilter, $aDataSet ); if( !$bFilterApplies ) { return false; } } - //TODO: Implement for type 'date', 'datetime', 'boolean' and 'numeric' + //TODO: Implement for type 'date', 'datetime' and 'boolean' } return true; @@ -294,10 +302,9 @@ * @param oject $aDataSet * @return boolean true if filter applies, false if not */ - public function filterString($oFilter, $aDataSet) { - if( !is_string($oFilter->value) ) { - //TODO: Warning - return false; + public function filterString( $oFilter, $aDataSet ) { + if( !is_string( $oFilter->value ) ) { + return false; //TODO: Warning } $sFieldValue = $aDataSet->{$oFilter->field}; $sFilterValue = $oFilter->value; @@ -331,9 +338,8 @@ * @return boolean true if filter applies, false if not */ public function filterNumeric( $oFilter, $aDataSet ) { - if( !is_numeric($oFilter->value) ) { - //TODO: Warning - return false; + if( !is_numeric( $oFilter->value ) ) { + return false; //TODO: Warning } $sFieldValue = $aDataSet->{$oFilter->field}; $iFilterValue = (int) $oFilter->value; @@ -356,10 +362,9 @@ * @param oject $aDataSet * @return boolean true if filter applies, false if not */ - public function filterList($oFilter, $aDataSet) { - if( !is_array($oFilter->value) ) { - //TODO: Warning - return false; + public function filterList( $oFilter, $aDataSet ) { + if( !is_array( $oFilter->value ) ) { + return false; //TODO: Warning } $aFieldValues = $aDataSet->{$oFilter->field}; if( empty( $aFieldValues ) ) { @@ -378,16 +383,16 @@ * @param array $aProcessedData The filtered result * @return array a trimmed version of the result */ - public function trimData($aProcessedData) { - $iStart = $this->getParameter('start'); - $iEnd = $this->getParameter('limit') + $iStart; + public function trimData( $aProcessedData ) { + $iStart = $this->getParameter( 'start' ); + $iEnd = $this->getParameter( 'limit' ) + $iStart; - if( $iEnd >= $this->iFinalDataSetCount || $iEnd === 0 ) { - $iEnd = $this->iFinalDataSetCount - 1; + if( $iEnd > $this->iFinalDataSetCount || $iEnd === 0 ) { + $iEnd = $this->iFinalDataSetCount; } $aTrimmedData = array(); - for( $i = $iStart; $i <= $iEnd; $i++ ) { + for( $i = $iStart; $i < $iEnd; $i++ ) { $aTrimmedData[] = $aProcessedData[$i]; } diff --git a/includes/api/BSApiFileBackendStore.php b/includes/api/BSApiFileBackendStore.php index 3c4c548..214553f 100644 --- a/includes/api/BSApiFileBackendStore.php +++ b/includes/api/BSApiFileBackendStore.php @@ -24,37 +24,24 @@ * @author Robert Vogel <vo...@hallowelt.com> * @author Patric Wirth <wi...@hallowelt.com> * @package Bluespice_Foundation - * @copyright Copyright (C) 2011 Hallo Welt! - Medienwerkstatt GmbH, All rights reserved. + * @copyright Copyright (C) 2015 Hallo Welt! - Medienwerkstatt GmbH, All rights reserved. * @license http://www.gnu.org/copyleft/gpl.html GNU Public License v2 or later * @filesource */ class BSApiFileBackendStore extends BSApiExtJSStoreBase { - public function makeData() { - $oDbr = wfGetDB( DB_SLAVE ); + public function makeData( $sQuery = '' ) { + $res = $this->fetchCaseInsensitive( $sQuery ); - $aContidions = array( - 'page_namespace' => NS_FILE, - 'page_title = img_name', - 'page_id = si_page' //Needed for case insensitive quering; Maybe - //implement 'query' as a implicit filter on 'img_name' field? - ); - - $sQuery = $this->getParameter( 'query' ); - if( !empty( $sQuery ) ) { - $aContidions[] = "si_title ".$oDbr->buildLike( - $oDbr->anyString(), - $sQuery, - $oDbr->anyString() - ); + //The initial query is made against the searchindex table, which holds + //lowercased and otherwise normalized titles. Unfornunately if + //one queries an exact title with dots (and colons) the result will be + //empty because the searchindex table data is stripped from those + //characters. We will fallback to a query without the use of + //searchindex, just in case... + if( $res->numRows() === 0 ) { + $res = $this->fetchCaseSensitive( $sQuery ); } - - $oImgRes = $oDbr->select( - array( 'image', 'page', 'searchindex' ), - '*', - $aContidions, - __METHOD__ - ); $bUseSecureFileStore = BsExtensionManager::isContextActive( 'MW::SecureFileStore::Active' @@ -62,22 +49,23 @@ //First query: Get all files and their pages $aReturn = array(); - foreach( $oImgRes as $oRow ) { + foreach( $res as $oRow ) { try { - $oImg = RepoGroup::singleton()->getLocalRepo()->newFileFromRow( - $oRow - ); + $oImg = RepoGroup::singleton()->getLocalRepo() + ->newFileFromRow( $oRow ); } catch (Exception $ex) { continue; } $oTitle = Title::newFromRow( $oRow ); + //No "user can read" check here, because it may be expensive. + //This may be done by hook handlers //TODO: use 'thumb.php'? - //TODO: Make thumb size editable + //TODO: Make thumb size a parameter $sThumb = $oImg->createThumb( 48, 48 ); $sUrl = $oImg->getUrl(); - if( $bUseSecureFileStore ) { + if( $bUseSecureFileStore ) { //TODO: Remove $sThumb = SecureFileStore::secureStuff( $sThumb, true ); $sUrl = SecureFileStore::secureStuff( $sUrl, true ); } @@ -95,25 +83,32 @@ 'file_user_text' => $oImg->getUser( 'text' ), 'file_extension' => $oImg->getExtension(), 'file_timestamp' => $oImg->getTimestamp(), - 'file_major_mime' => $oRow->img_major_mime, 'file_mediatype' => $oImg->getMediaType(), 'file_description' => $oImg->getDescription(), 'file_display_text' => $oImg->getName(), 'file_thumbnail_url' => $sThumb, 'page_id' => $oTitle->getArticleID(), 'page_title' => $oTitle->getText(), - 'page_is_new' => $oTitle->isNewPage(), 'page_latest' => $oTitle->getLatestRevID(), - 'page_touched' => $oTitle->getTouched(), 'page_namespace' => $oTitle->getNamespace(), - 'page_categories' => array(), + 'page_categories' => array(), //Filled by a second step below 'page_is_redirect' => $oTitle->isRedirect(), + + //For some reason 'page_is_new' and 'page_touched' are not + //initialized by 'Title::newFromRow'; Instead when calling + //'Title->isNew()' or 'Title->getTouched()' an extra query is + //being sent to the database, wich introduced a performance + //issue. As the resulting data is the same we just use the raw + //form here. + 'page_is_new' => (bool)$oRow->page_is_new, + 'page_touched' => $oRow->page_touched ); } //Second query: Get all categories of each file page $aPageIds = array_keys( $aReturn ); if( !empty( $aPageIds ) ) { + $oDbr = wfGetDB( DB_SLAVE ); $oCatRes = $oDbr->select( 'categorylinks', array( 'cl_from', 'cl_to' ), @@ -128,4 +123,58 @@ //TODO: Find out if or where this hook was used before //wfRunHooks( 'BSInsertFileGetFilesBeforeQuery', array( &$aConds, &$aNameFilters ) ); } + + public function fetchCaseInsensitive( $sQuery ) { + $oDbr = wfGetDB( DB_SLAVE ); + + $aContidions = array( + 'page_namespace' => NS_FILE, + 'page_title = img_name', + 'page_id = si_page' //Needed for case insensitive quering; Maybe + //implement 'query' as a implicit filter on 'img_name' field? + ); + + if( !empty( $sQuery ) ) { + $aContidions[] = "si_title ".$oDbr->buildLike( + $oDbr->anyString(), + strtolower( $sQuery ), //make case insensitive! + $oDbr->anyString() + ); + } + + $res = $oDbr->select( + array( 'image', 'page', 'searchindex' ), + '*', + $aContidions, + __METHOD__ + ); + + return $res; + } + + public function fetchCaseSensitive( $sQuery ) { + $oDbr = wfGetDB( DB_SLAVE ); + + $aContidions = array( + 'page_namespace' => NS_FILE, + 'page_title = img_name', + ); + + if( !empty( $sQuery ) ) { + $aContidions[] = "img_name ".$oDbr->buildLike( + $oDbr->anyString(), + str_replace(' ', '_', $sQuery ), + $oDbr->anyString() + ); + } + + $res = $oDbr->select( + array( 'image', 'page' ), + '*', + $aContidions, + __METHOD__ + ); + + return $res; + } } diff --git a/resources/bluespice.extjs/BS/model/File.js b/resources/bluespice.extjs/BS/model/File.js new file mode 100644 index 0000000..8ee12c1 --- /dev/null +++ b/resources/bluespice.extjs/BS/model/File.js @@ -0,0 +1,41 @@ +/** + * This model is used to have a unified representation of all MediaWiki repo + * files. The information is a combination of the page and the file table + */ + +Ext.define('BS.model.File', { + extend: 'Ext.data.Model', + + idProperty: 'file_name', + + fields: [ + //Those are values we can gather from the MediaWiki 'page' table. + { name: 'page_id', type: 'int', defaultValue: 0 }, + { name: 'page_namespace', type: 'int', defaultValue: -99 }, + { name: 'page_title', type: 'string', defaultValue: '' }, + { name: 'page_is_new', type: 'bool', defaultValue: true }, + { name: 'page_touched', type: 'date', defaultValue: '19700101000000', dateFormat: 'YmdHis' }, + { name: 'page_is_redirect', type: 'bool', defaultValue: false }, + { name: 'page_latest', type: 'date', defaultValue: '19700101000000', dateFormat: 'YmdHis' }, + + //Here come custom fields that are calculated on the server side + { name: 'page_categories', type: 'array', defaultValue: [] }, + + { name: 'file_name', type: 'string' }, + { name: 'file_size', type: 'int', defaultValue: 0 }, + { name: 'file_bits', type: 'int', defaultValue: 0 }, + { name: 'file_user', type: 'int', defaultValue: 0 }, + { name: 'file_width', type: 'int', defaultValue: 0 }, + { name: 'file_height', type: 'int', defaultValue: 0 }, + { name: 'file_mimetype', type: 'string', defaultValue: 'unknown/unknown' }, + { name: 'file_metadata', type: 'object', defaultValue: {} }, + { name: 'file_extension', type: 'string', defaultValue: '' }, + { name: 'file_timestamp', type: 'date', defaultValue: '19700101000000', dateFormat: 'YmdHis' }, + { name: 'file_mediatype', type: 'string', defaultValue: '' }, + { name: 'file_description', type: 'string', defaultValue: '' }, + { name: 'file_display_text', type: 'string', defaultValue: '' }, //TODO: Maybe fallback to 'file_name' + { name: 'file_thumbnail_url', type: 'string', defaultValue: '' } + ] + + //TODO: Implement getter +}); \ No newline at end of file -- To view, visit https://gerrit.wikimedia.org/r/211686 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I30c5aa63e869875dec44ef577117f7f7908b3993 Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/BlueSpiceFoundation Gerrit-Branch: master Gerrit-Owner: Robert Vogel <vo...@hallowelt.biz> Gerrit-Reviewer: Mglaser <gla...@hallowelt.biz> Gerrit-Reviewer: Pwirth <wi...@hallowelt.biz> Gerrit-Reviewer: Robert Vogel <vo...@hallowelt.biz> Gerrit-Reviewer: Tweichart <weich...@hallowelt.biz> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits