Jdlrobson has uploaded a new change for review. https://gerrit.wikimedia.org/r/196458
Change subject: Hygiene: Delete all code relating to user page storage ...................................................................... Hygiene: Delete all code relating to user page storage Change-Id: I6cb35052cea983f0c96541369fb13c6ec76ac15d --- M Gather.php M extension.json M includes/Gather.hooks.php M includes/models/Collection.php M includes/specials/SpecialGather.php D includes/stores/Collection.php D includes/stores/CollectionStorage.php D includes/stores/CollectionsListStorage.php D includes/stores/ItemExtracts.php D includes/stores/ItemImages.php D includes/stores/JSONPage.php D includes/stores/UserPageCollection.php D includes/stores/UserPageCollectionsList.php D includes/stores/WatchlistCollection.php M includes/views/Collection.php D tests/phpunit/GatherHooksTest.php 16 files changed, 5 insertions(+), 587 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Gather refs/changes/58/196458/1 diff --git a/Gather.php b/Gather.php index 43061d4..50556d2 100644 --- a/Gather.php +++ b/Gather.php @@ -42,16 +42,6 @@ 'Gather\models\WithImage' => 'models/WithImage', 'Gather\models\ArraySerializable' => 'models/ArraySerializable', - 'Gather\stores\JSONPage' => 'stores/JSONPage', - 'Gather\stores\Collection' => 'stores/Collection', - 'Gather\stores\WatchlistCollection' => 'stores/WatchlistCollection', - 'Gather\stores\CollectionStorage' => 'stores/CollectionStorage', - 'Gather\stores\UserPageCollection' => 'stores/UserPageCollection', - 'Gather\stores\CollectionsListStorage' => 'stores/CollectionsListStorage', - 'Gather\stores\UserPageCollectionsList' => 'stores/UserPageCollectionsList', - 'Gather\stores\ItemExtracts' => 'stores/ItemExtracts', - 'Gather\stores\ItemImages' => 'stores/ItemImages', - 'Gather\views\View' => 'views/View', 'Gather\views\NotFound' => 'views/NotFound', 'Gather\views\NoPublic' => 'views/NoPublic', @@ -82,8 +72,6 @@ $wgExtensionFunctions[] = 'Gather\Hooks::onExtensionSetup'; $wgHooks['MobilePersonalTools'][] = 'Gather\Hooks::onMobilePersonalTools'; $wgHooks['UnitTestsList'][] = 'Gather\Hooks::onUnitTestsList'; -$wgHooks['getUserPermissionsErrors'][] = 'Gather\Hooks::onGetUserPermissionsErrors'; -$wgHooks['ContentHandlerDefaultModelFor'][] = 'Gather\Hooks::onContentHandlerDefaultModelFor'; $wgHooks['SkinMinervaDefaultModules'][] = 'Gather\Hooks::onSkinMinervaDefaultModules'; $wgHooks['MakeGlobalVariablesScript'][] = 'Gather\Hooks::onMakeGlobalVariablesScript'; $wgHooks['ResourceLoaderTestModules'][] = 'Gather\Hooks::onResourceLoaderTestModules'; diff --git a/extension.json b/extension.json index ebc3750..7bc1e9f 100644 --- a/extension.json +++ b/extension.json @@ -33,15 +33,6 @@ "Gather\\models\\CollectionsList": "includes/models/CollectionsList.php", "Gather\\models\\WithImage": "includes/models/WithImage.php", "Gather\\models\\ArraySerializable": "includes/models/ArraySerializable.php", - "Gather\\stores\\JSONPage": "includes/stores/JSONPage.php", - "Gather\\stores\\Collection": "includes/stores/Collection.php", - "Gather\\stores\\WatchlistCollection": "includes/stores/WatchlistCollection.php", - "Gather\\stores\\CollectionStorage": "includes/stores/CollectionStorage.php", - "Gather\\stores\\UserPageCollection": "includes/stores/UserPageCollection.php", - "Gather\\stores\\CollectionsListStorage": "includes/stores/CollectionsListStorage.php", - "Gather\\stores\\UserPageCollectionsList": "includes/stores/UserPageCollectionsList.php", - "Gather\\stores\\ItemExtracts": "includes/stores/ItemExtracts.php", - "Gather\\stores\\ItemImages": "includes/stores/ItemImages.php", "Gather\\views\\View": "includes/views/View.php", "Gather\\views\\NotFound": "includes/views/NotFound.php", "Gather\\views\\NoPublic": "includes/views/NoPublic.php", @@ -199,12 +190,6 @@ ], "UnitTestsList": [ "Gather\\Hooks::onUnitTestsList" - ], - "getUserPermissionsErrors": [ - "Gather\\Hooks::onGetUserPermissionsErrors" - ], - "ContentHandlerDefaultModelFor": [ - "Gather\\Hooks::onContentHandlerDefaultModelFor" ], "SkinMinervaDefaultModules": [ "Gather\\Hooks::onSkinMinervaDefaultModules" diff --git a/includes/Gather.hooks.php b/includes/Gather.hooks.php index 1789a2d..f2b6166 100644 --- a/includes/Gather.hooks.php +++ b/includes/Gather.hooks.php @@ -6,7 +6,6 @@ namespace Gather; use \SpecialPage; -use Gather\stores; use Gather\models; use Gather\views\helpers\CSS; use \MobileContext; @@ -104,28 +103,6 @@ } /** - * Disallow moving or editing gather page json files - */ - public static function onGetUserPermissionsErrors( $title, $user, $action, $result ) { - $manifest = stores\UserPageCollectionsList::MANIFEST_FILE; - $isProtectedAction = $action === 'edit' || $action === 'move'; - $titleText = $title->getText(); - if ( $title->inNamespace( NS_USER ) && $isProtectedAction && - strpos( $titleText, $manifest ) !== false - ) { - // we have a collection definition so check the user matches the title. - if ( strpos( $titleText, $user->getName() . '/' . $manifest ) !== false ) { - return true; - } else { - $result = false; - return false; - } - } else { - return true; - } - } - - /** * Load user collections */ public static function onMakeGlobalVariablesScript( &$vars, $out ) { @@ -160,25 +137,6 @@ $vars['wgGatherPageImageThumbnail'] = $thumb->getUrl(); } } - } - return true; - } - - /** - * Convert the content model of json files that are actually JSON to JSON. - * This only affects validation and UI when saving and editing, not - * loading the content. - * @param $title Title - * @param &$model string - * - * @return bool - */ - public static function onContentHandlerDefaultModelFor( $title, &$model ) { - $titleText = $title->getText(); - $manifest = stores\UserPageCollectionsList::MANIFEST_FILE; - if ( $title->inNamespace( NS_USER ) && - strpos( $titleText, $manifest ) !== false ) { - $model = CONTENT_MODEL_JSON; } return true; } diff --git a/includes/models/Collection.php b/includes/models/Collection.php index bb30cd6..672ff3b 100644 --- a/includes/models/Collection.php +++ b/includes/models/Collection.php @@ -11,13 +11,14 @@ use \User; use \ApiMain; use \FauxRequest; -use \Gather\stores\ItemExtracts; use \Title; /** * A collection with a list of items, which are represented by the CollectionItem class. */ class Collection extends CollectionBase implements IteratorAggregate { + const EXTRACTS_CHAR_LIMIT = 140; + /** * The internal collection of items. * @@ -119,7 +120,7 @@ 'glspid' => $id, 'explaintext' => true, 'exintro' => true, - 'exchars' => ItemExtracts::CHAR_LIMIT, + 'exchars' => self::EXTRACTS_CHAR_LIMIT, 'exlimit' => 50, ) ) ); try { diff --git a/includes/specials/SpecialGather.php b/includes/specials/SpecialGather.php index b141559..bd6fbf0 100644 --- a/includes/specials/SpecialGather.php +++ b/includes/specials/SpecialGather.php @@ -6,7 +6,6 @@ namespace Gather; use Gather\models; -use Gather\stores; use Gather\views; use \User; use \SpecialPage; diff --git a/includes/stores/Collection.php b/includes/stores/Collection.php deleted file mode 100644 index 5414c10..0000000 --- a/includes/stores/Collection.php +++ /dev/null @@ -1,41 +0,0 @@ -<?php - -namespace Gather\stores; - -use Gather\models; - -use \User; -use \Title; - -/** - * Abstraction for collection storage. - */ -abstract class Collection { - - /** - * Get collection items from a list of titles - * @param Title[] $titles - * - * @return models\CollectionItem[] - */ - public static function getItemsFromTitles( $titles ) { - if ( count( $titles ) > 0 ) { - $extracts = ItemExtracts::loadExtracts( $titles ); - $images = ItemImages::loadImages( $titles ); - } - - // Merge the data into models\CollectionItem - $items = array(); - foreach ( $titles as $key=>$title ) { - // Check, if this page has a page image - if ( isset( $images[$title->getArticleId()] ) ) { - $image = $images[$title->getArticleId()]; - } else { - $image = false; - } - $items[] = new models\CollectionItem( $title, $image, $extracts[$key] ); - } - - return $items; - } -} diff --git a/includes/stores/CollectionStorage.php b/includes/stores/CollectionStorage.php deleted file mode 100644 index d38c95a..0000000 --- a/includes/stores/CollectionStorage.php +++ /dev/null @@ -1,23 +0,0 @@ -<?php - -/** - * CollectionStorage.php - */ - -namespace Gather\stores; - -/** - * Interface for stores that will store/retrieve Collections - */ - -interface CollectionStorage { - - /** - * Get Collection model with an id of a user - * @param User $owner of the collection - * @param int $id of the collection - * @return models\Collection - */ - public static function newFromUserAndId( $owner, $id ); - -} diff --git a/includes/stores/CollectionsListStorage.php b/includes/stores/CollectionsListStorage.php deleted file mode 100644 index 1f3620a..0000000 --- a/includes/stores/CollectionsListStorage.php +++ /dev/null @@ -1,26 +0,0 @@ -<?php - -/** - * CollectionsListStorage.php - */ - -namespace Gather\stores; - -use Gather\models; -use \User; - -/** - * Interface for a store that loads the collections of a user. - */ -interface CollectionsListStorage { - - /** - * Get list of collections by user - * @param User $user collection list owner - * @param boolean $includePrivate if the list should show private collections or not - * @return models\CollectionsList List of collections. - */ - public static function newFromUser( User $user, $includePrivate = false ); - -} - diff --git a/includes/stores/ItemExtracts.php b/includes/stores/ItemExtracts.php deleted file mode 100644 index ce0aea8..0000000 --- a/includes/stores/ItemExtracts.php +++ /dev/null @@ -1,48 +0,0 @@ -<?php - -namespace Gather\stores; - -use \ApiQuery; -use \ApiMain; -use \FauxRequest; -use Title; - -/** - * Loading extracts for titles - */ -class ItemExtracts { - const CHAR_LIMIT = 140; - - /** - * Load extracts for a collection of titles - * @param Title[] $titles - * - * @return string[] - */ - public static function loadExtracts( array $titles ) { - $api = new ApiMain( new FauxRequest( array( - 'action' => 'query', - 'prop' => 'extracts', - 'explaintext' => true, - 'exintro' => true, - 'exchars' => ItemExtracts::CHAR_LIMIT, - 'titles' => implode( '|', $titles ), - 'exlimit' => count( $titles ), - ) ) ); - $api->execute(); - $data = $api->getResultData(); - $pages = $data['query']['pages']; - - $extracts = array(); - foreach ( $pages as $page ) { - if ( isset( $page['extract']['*'] ) ) { - $extracts[] = $page['extract']['*']; - } else { - $extracts[] = null; - } - } - return $extracts; - } - -} - diff --git a/includes/stores/ItemImages.php b/includes/stores/ItemImages.php deleted file mode 100644 index 744917a..0000000 --- a/includes/stores/ItemImages.php +++ /dev/null @@ -1,44 +0,0 @@ -<?php - -namespace Gather\stores; - -use \PageImages; -use Title; - -/** - * Loading page images for titles - */ -class ItemImages { - - /** - * Load images for a collection of titles - * @param Title[] $titles - * - * @return string[] - */ - public static function loadImages( array $titles ) { - $images = array(); - $titleIds = array(); - // get article ids for page images query - foreach ( $titles as $title ) { - $titleIds[] = $title->getArticleId(); - } - // query to get page images for all pages - // FIXME: Should probably be in PageImages extension - $dbr = wfGetDB( DB_SLAVE ); - $result = $dbr->select( 'page_props', - array( 'pp_value', 'pp_page' ), - array( 'pp_page' => $titleIds, 'pp_propname' => PageImages::PROP_NAME ), - __METHOD__ - ); - if ( $result ) { - // build results array - foreach ( $result as $row ) { - $images[$row->pp_page] = wfFindFile( $row->pp_value ); - } - } - return $images; - } - -} - diff --git a/includes/stores/JSONPage.php b/includes/stores/JSONPage.php deleted file mode 100644 index 0bb168f..0000000 --- a/includes/stores/JSONPage.php +++ /dev/null @@ -1,37 +0,0 @@ -<?php - -/** - * JSONPage.php - */ - -namespace Gather\stores; - -use \Title; -use \WikiPage; -use \FormatJson; - -/** - * Store to retrieve a page content returned as json - */ -class JSONPage { - /** - * Gets a JSON page - * @param Title $title of the page to retrieve - * @return array $json read - */ - public static function get( $title ) { - $page = WikiPage::factory( $title ); - $data = array(); - if ( $page->exists() ) { - $content = $page->getContent(); - $type = $page->getContentModel(); - if ( $type === CONTENT_MODEL_JSON ) { - $status = $content->getData(); - if ( $status->isOK() ) { - $data = wfObjectToArray( $status->getValue() ); - } - } - } - return $data; - } -} diff --git a/includes/stores/UserPageCollection.php b/includes/stores/UserPageCollection.php deleted file mode 100644 index 26f801f..0000000 --- a/includes/stores/UserPageCollection.php +++ /dev/null @@ -1,79 +0,0 @@ -<?php - -namespace Gather\stores; - -use Gather\models; - -use \User; -use \Title; - -/** - * Abstraction for json collection storage on user pages. - * FIXME: This should live in core and power Special:EditWatchlist - */ -class UserPageCollection extends Collection implements CollectionStorage { - - /** - * Get Collection model with an id of a user - * @param User $owner of the collection - * @param int $id of the collection - * @return models\Collection - */ - public static function newFromUserAndId( $owner, $id ) { - if ( $id !== 0 ) { - $collectionListData = JSONPage::get( UserPageCollectionsList::getStorageTitle( $owner ) ); - foreach ( $collectionListData as $collectionData ) { - if ( $id === $collectionData["id"] ) { - // find id in list. - return self::collectionFromJSON( $collectionData ); - } - } - // id not there. - return null; - } else { - // id 0 is the watchlist. Which loads differently - return WatchlistCollection::newFromUser( $owner ); - } - } - - /** - * Fill a collection object from json data - * Returns null if there is not enough information to fill it up. - * @param array $json data to pull information from - * - * @return models\Collection|null - */ - public static function collectionFromJSON( $json ) { - try { - if ( !isset($json['id']) || - !isset($json['owner']) || - !isset($json['title']) ) { - return null; - } - - $collection = new models\Collection( - $json['id'], - User::newFromName( $json['owner'] ), - $json['title'], - $json['description'], - $json['public'], - wfFindFile( $json['image'] ) - ); - if ( isset( $json['items'] ) ) { - // Make titles - $titles = array(); - foreach ( $json['items'] as $title ) { - if ( is_string( $title ) && isset( $title ) ) { - $titles[] = Title::newFromText( $title ); - } - } - $collection->batch( self::getItemsFromTitles( $titles ) ); - } - return $collection; - } catch (Exception $e) { - // Invalid json - return null; - } - } - -} diff --git a/includes/stores/UserPageCollectionsList.php b/includes/stores/UserPageCollectionsList.php deleted file mode 100644 index ab458bb..0000000 --- a/includes/stores/UserPageCollectionsList.php +++ /dev/null @@ -1,100 +0,0 @@ -<?php - -/** - * UserPageCollectionsList.php - */ - -namespace Gather\stores; - -use Gather\models; -use \User; -use \Title; - -/** - * Stores and retrieves collection lists from user pages - */ -class UserPageCollectionsList implements CollectionsListStorage { - const MANIFEST_FILE = 'GatherCollectionsV2.json'; - - /** - * Get list of collections by user - * @param User $user collection list owner - * @param boolean $includePrivate if the list should show private collections or not - * @return models\CollectionsList List of collections. - */ - public static function newFromUser( User $user, $includePrivate = false ) { - $includesWatchlist = false; - $collectionsList = new models\CollectionsList( $includePrivate ); - // Add collections - $collectionsData = JSONPage::get( self::getStorageTitle( $user ) ); - foreach ( $collectionsData as $collectionData ) { - $collection = self::collectionFromJSON( $collectionData ); - if ( $collection ) { - $collectionsList->add( $collection ); - // If the added collection is a watchlist make a record of it - if ( $collection->getId() === 0 ) { - $includesWatchlist = true; - } - } - } - - // if no watchlist found let's add it. - if ( !$includesWatchlist ) { - // Add watchlist - $watchlist = WatchlistCollection::newFromUser( $user ); - $watchlistInfo = new models\CollectionInfo( - $watchlist->getId(), - $watchlist->getOwner(), - $watchlist->getTitle(), - $watchlist->getDescription(), - $watchlist->isPublic(), - $watchlist->getFile() - ); - $watchlistInfo->setCount( $watchlist->getCount() ); - $collectionsList->add( $watchlistInfo ); - } - return $collectionsList; - } - - /** - * Get formatted title of the page that contains the manifest - * @param User $user - * @return Title - */ - public static function getStorageTitle( User $user ) { - $title = $user->getName() . '/' . self::MANIFEST_FILE; - return Title::makeTitleSafe( NS_USER, $title ); - } - - /** - * Get a basic collection object with the metadata from json data in the manifest - * Returns null if there is not enough info to create the object. - * @param array $json data to pull information from - * - * @return models\CollectionInfo|null - */ - public static function collectionFromJSON( $json ) { - try { - if ( !isset( $json['id'] ) || - !isset( $json['owner'] ) || - !isset( $json['title'] ) ) { - return null; - } - - $collection = new models\CollectionInfo( - $json['id'], - User::newFromName( $json['owner'] ), - $json['title'], - $json['description'], - $json['public'], - wfFindFile( $json['image'] ) - ); - $collection->setCount( $json['count'] ); - return $collection; - } catch (Exception $e) { - // Invalid json - return null; - } - } - -} diff --git a/includes/stores/WatchlistCollection.php b/includes/stores/WatchlistCollection.php deleted file mode 100644 index 9f98878..0000000 --- a/includes/stores/WatchlistCollection.php +++ /dev/null @@ -1,81 +0,0 @@ -<?php - -namespace Gather\stores; - -use Gather\models; - -use \User; -use \Title; -use \GenderCache; - -/** - * Abstraction for watchlist storage. - * FIXME: This should live in core and power Special:EditWatchlist - */ -class WatchlistCollection extends Collection { - - /** - * Get a watchlist by user - * @param User $user to lookup watchlist for - * @return models\Collection - */ - public static function newFromUser( User $user ) { - $titles = self::loadTitles( $user ); - $items = self::getItemsFromTitles( $titles ); - - // Grab first image available for the collection - $firstImage = null; - foreach ( $items as $item ) { - if ( $item->hasImage() ) { - $firstImage = $item->getFile(); - break; - } - } - - // Construct the models\Collection - $collection = new models\Collection( - 0, // Watchlist has a hardcoded id of 0 - $user, - wfMessage( 'gather-watchlist-title' )->text(), - wfMessage( 'gather-watchlist-description' )->text(), - false, // Watchlist is private - $firstImage - ); - $collection->batch( $items ); - - return $collection; - } - - /** - * Load titles of the watchlist - * @param User $user - * @return Title[] - */ - private static function loadTitles( $user ) { - $dbr = wfGetDB( DB_SLAVE ); - - $res = $dbr->select( - 'watchlist', - array( 'wl_namespace', 'wl_title'), - array( - 'wl_user' => $user->getId(), - 'wl_namespace' => 0, - ), - __METHOD__, - array( 'LIMIT' => 50,) - ); - - $titles = array(); - if ( $res->numRows() > 0 ) { - foreach ( $res as $row ) { - $title = Title::makeTitle( $row->wl_namespace, $row->wl_title ); - $titles[] = $title; - } - $res->free(); - } - GenderCache::singleton()->doTitlesArray( $titles ); - - return $titles; - } - -} diff --git a/includes/views/Collection.php b/includes/views/Collection.php index 13a04fc..f3443e0 100644 --- a/includes/views/Collection.php +++ b/includes/views/Collection.php @@ -141,8 +141,8 @@ $html .= $view->getHtml(); } } - // FIXME: Pagination(??) Note the stores\WatchlistCollection - // limits the size of the collection to 50. + // FIXME: Pagination(??) currently we + // limit the size of the collection // Pagination may or may not be needed. $html .= Html::closeElement( 'div' ); return $html; diff --git a/tests/phpunit/GatherHooksTest.php b/tests/phpunit/GatherHooksTest.php deleted file mode 100644 index 3c2fec6..0000000 --- a/tests/phpunit/GatherHooksTest.php +++ /dev/null @@ -1,34 +0,0 @@ -<?php - -/** - * @group Gather - */ -class GatherHooksTest extends MediaWikiTestCase { - public function provideGetUserPermissionsErrors() { - $manifest = Gather\stores\UserPageCollectionsList::MANIFEST_FILE; - return array( - // Edit - array( true, "User:Jdlrobson/$manifest", 'Jdlrobson', 'edit' ), - array( false, "User:Jdlrobson/$manifest", 'phudex', 'edit' ), - // View - array( true, "User:Jdlrobson/$manifest", 'Jdlrobson', 'view' ), - array( true, "User:Jdlrobson/$manifest", 'phudex', 'view' ), - // Move - array( true, "User:Jdlrobson/$manifest", 'Jdlrobson', 'move' ), - array( false, "User:Jdlrobson/$manifest", 'phuedx', 'move' ), - // Normal page editing is not disrupted - array( true, 'User:JDLR', 'Jdlrobson', 'edit' ), - array( true, 'User:JDLR/Foo', 'Jdlrobson', 'edit' ), - ); - } - - /** - * @dataProvider provideGetUserPermissionsErrors - * - */ - public function testOnGetUserPermissionsErrors( $expected, $title, $user, $action ) { - $canEdit = Gather\Hooks::onGetUserPermissionsErrors( Title::newFromText( $title ), - User::newFromName( $user ), $action, '' ); - $this->assertEquals( $expected, $canEdit ); - } -} -- To view, visit https://gerrit.wikimedia.org/r/196458 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6cb35052cea983f0c96541369fb13c6ec76ac15d Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Gather Gerrit-Branch: master Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits