Adrian Lang has uploaded a new change for review.
https://gerrit.wikimedia.org/r/129122
Change subject: Factor out generation of site links HTML into own class
......................................................................
Factor out generation of site links HTML into own class
This has the nice side effect of reducing the number of created SiteSQLStore
instances. Since every SiteSQLStore instance maintains its own cached version
of the sites list, which it first has to fetch from the DB or cache, this
reduces both rendering time and memory consumption.
SectionEditLinkGenerator needs tests and
SectionEditLinkGenerator::getHtmlForSiteLinkGroup should be split up for that.
Change-Id: Id2e1fba574b737c15fa2dc5ee4fb6152ca240c96
---
M repo/includes/EntityView.php
M repo/includes/ItemView.php
M repo/includes/view/SectionEditLinkGenerator.php
A repo/includes/view/SiteLinksView.php
M repo/includes/view/TermBoxView.php
M repo/tests/phpunit/includes/view/SectionEditLinkGeneratorTest.php
6 files changed, 225 insertions(+), 184 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/22/129122/1
diff --git a/repo/includes/EntityView.php b/repo/includes/EntityView.php
index faa84e1..12a2855 100644
--- a/repo/includes/EntityView.php
+++ b/repo/includes/EntityView.php
@@ -6,12 +6,11 @@
use Html;
use IContextSource;
use InvalidArgumentException;
-use Language;
use ParserOutput;
-use SpecialPageFactory;
use Wikibase\Lib\PropertyDataTypeLookup;
use Wikibase\Lib\Serializers\SerializationOptions;
use Wikibase\Lib\SnakFormatter;
+use Wikibase\View\SectionEditLinkGenerator;
use Wikibase\View\SnakHtmlGenerator;
/**
@@ -404,7 +403,7 @@
$lang = $this->getLanguage();
$label = $entity->getLabel( $lang->getCode() );
- $editUrl = $this->getEditUrl( 'SetLabel', $entity, $lang );
+ $editUrl = $this->sectionEditLinkGenerator->getEditUrl(
'SetLabel', $entity, $lang );
$prefixedId = $this->getFormattedIdForEntity( $entity );
$html = wfTemplate( 'wb-label',
@@ -435,7 +434,7 @@
$lang = $this->getLanguage();
$description = $entity->getDescription( $lang->getCode() );
- $editUrl = $this->getEditUrl( 'SetDescription', $entity, $lang
);
+ $editUrl = $this->sectionEditLinkGenerator->getEditUrl(
'SetDescription', $entity, $lang );
$html = wfTemplate( 'wb-description',
wfTemplate( 'wb-property',
@@ -464,7 +463,7 @@
$lang = $this->getLanguage();
$aliases = $entity->getAliases( $lang->getCode() );
- $editUrl = $this->getEditUrl( 'SetAliases', $entity, $lang );
+ $editUrl = $this->sectionEditLinkGenerator->getEditUrl(
'SetAliases', $entity, $lang );
if ( empty( $aliases ) ) {
$html = wfTemplate( 'wb-aliases-wrapper',
@@ -606,41 +605,11 @@
*
* @return string
*/
- protected function getHtmlForEditSection( $url, $tag = 'span', $action
= 'edit', $enabled = true ) {
+ private function getHtmlForEditSection( $url, $tag = 'span', $action =
'edit', $enabled = true ) {
$key = $action === 'add' ? 'wikibase-add' : 'wikibase-edit';
$msg = $this->getContext()->msg( $key );
return $this->sectionEditLinkGenerator->getHtmlForEditSection(
$url, $msg, $tag, $enabled );
- }
-
- /**
- * Returns the url of the editlink.
- *
- * @since 0.4
- *
- * @param string $specialpagename
- * @param Entity $entity
- * @param Language $language|null
- *
- * @return string
- */
- protected function getEditUrl( $specialpagename, Entity $entity,
Language $language = null ) {
- $specialpage = SpecialPageFactory::getPage( $specialpagename );
-
- if ( $specialpage === null ) {
- return ''; //XXX: this should throw an exception?!
- }
-
- if ( $entity->getId() ) {
- $subpage = $this->getFormattedIdForEntity( $entity );
- } else {
- $subpage = ''; // can't skip this, that would confuse
the order of parameters!
- }
-
- if ( $language !== null ) {
- $subpage .= '/' . $language->getCode();
- }
- return $specialpage->getPageTitle( $subpage )->getLocalURL();
}
/**
diff --git a/repo/includes/ItemView.php b/repo/includes/ItemView.php
index 2cb5efd..9401952 100644
--- a/repo/includes/ItemView.php
+++ b/repo/includes/ItemView.php
@@ -5,6 +5,7 @@
use SiteSQLStore;
use Wikibase\DataModel\SimpleSiteLink;
use Wikibase\Repo\WikibaseRepo;
+use Wikibase\View\SiteLinksView;
/**
* Class for creating views for Wikibase\Item instances.
@@ -76,153 +77,14 @@
*/
public function getHtmlForSiteLinks( Item $item, $editable = true ) {
$groups =
WikibaseRepo::getDefaultInstance()->getSettings()->getSetting( 'siteLinkGroups'
);
- $html = '';
- foreach ( $groups as $group ) {
- $html .= $this->getHtmlForSiteLinkGroup( $item, $group,
$editable );
- }
-
- return $html;
- }
-
- /**
- * Builds and returns the HTML representing a group of a
WikibaseEntity's site-links.
- *
- * @todo: code in this function belongs in its own class!
- *
- * @since 0.4
- *
- * @param Item $item the entity to render
- * @param string $group a site group ID
- * @param bool $editable whether editing is allowed (enabled edit links)
- * @return string
- */
- public function getHtmlForSiteLinkGroup( Item $item, $group, $editable
= true ) {
- // @todo inject into constructor
- $sites = SiteSQLStore::newInstance()->getSites();
- $specialGroups =
WikibaseRepo::getDefaultInstance()->getSettings()->getSetting(
"specialSiteLinkGroups" );
-
- $allSiteLinks = $item->getSiteLinks();
- $siteLinks = array(); // site links of the currently handled
site group
-
- foreach( $allSiteLinks as $siteLink ) {
- $site = $sites->getSite( $siteLink->getSiteId() );
-
- if ( $site === null ) {
- continue;
- }
-
- if ( $site->getGroup() === $group ) {
- $siteLinks[] = $siteLink;
- }
- }
-
- $html = $thead = $tbody = $tfoot = '';
-
- $html .= wfTemplate(
- 'wb-section-heading-sitelinks',
- wfMessage( 'wikibase-sitelinks-' . $group )->parse(),
// heading
- htmlspecialchars( 'sitelinks-' . $group, ENT_QUOTES )
// ID
- // TODO: support entity-id as prefix for element IDs.
+ // FIXME: Inject this, get the SiteSQLStore instance from
WikibaseRepo
+ $siteLinksView = new SiteLinksView(
+ SiteSQLStore::newInstance(),
+ $this->sectionEditLinkGenerator
);
- // FIXME: quickfix to allow a custom site-name / handling for
groups defined in $wgSpecialSiteLinkGroups
- $siteNameMessageKey =
'wikibase-sitelinks-sitename-columnheading';
- if ( in_array( $group, $specialGroups ) ) {
- $siteNameMessageKey .= '-special';
- }
-
- if( !empty( $siteLinks ) ) {
- $thead = wfTemplate( 'wb-sitelinks-thead',
- wfMessage( $siteNameMessageKey )->parse(),
- wfMessage(
'wikibase-sitelinks-siteid-columnheading' )->parse(),
- wfMessage(
'wikibase-sitelinks-link-columnheading' )->parse()
- );
- }
-
- $i = 0;
-
- // Sort the sitelinks according to their global id
- $safetyCopy = $siteLinks; // keep a shallow copy;
- $sortOk = usort(
- $siteLinks,
- function( SimpleSiteLink $a, SimpleSiteLink $b ) {
- return strcmp( $a->getSiteId(), $b->getSiteId()
);
- }
- );
-
- if ( !$sortOk ) {
- $siteLinks = $safetyCopy;
- }
-
- // Link to SpecialPage
- $editLink = $this->getEditUrl( 'SetSiteLink', $item, null );
-
- /* @var SimpleSiteLink $link */
- foreach( $siteLinks as $siteLink ) {
- $alternatingClass = ( $i++ % 2 ) ? 'even' : 'uneven';
-
- $siteId = $siteLink->getSiteId();
- $pageName = $siteLink->getPageName();
-
- $site = $sites->hasSite( $siteId ) ? $sites->getSite(
$siteId ) : null;
-
- if ( !$site || $site->getDomain() === '' ) {
- // the link is pointing to an unknown site.
- // XXX: hide it? make it red? strike it out?
-
- $tbody .= wfTemplate( 'wb-sitelink-unknown',
- $alternatingClass,
- htmlspecialchars( $siteId ),
- htmlspecialchars(
$siteLink->getPageName() ),
- $this->getHtmlForEditSection(
$editLink, 'td' )
- );
-
- } else {
- $languageCode = $site->getLanguageCode();
- $escapedSiteId = htmlspecialchars( $siteId );
- // FIXME: this is a quickfix to allow a custom
site-name for groups defined in $wgSpecialSiteLinkGroups instead of showing the
language-name
- if ( in_array( $group, $specialGroups ) ) {
- $siteNameMsg = wfMessage(
'wikibase-sitelinks-sitename-' . $siteId );
- $siteName = $siteNameMsg->exists() ?
$siteNameMsg->parse() : $siteId;
- } else {
- // TODO: get an actual site name rather
then just the language
- $siteName = htmlspecialchars(
Utils::fetchLanguageName( $languageCode ) );
- }
-
- // TODO: for non-JS, also set the dir attribute
on the link cell;
- // but do not build language objects for each
site since it causes too much load
- // and will fail when having too much site links
- $tbody .= wfTemplate( 'wb-sitelink',
- $languageCode,
- $alternatingClass,
- $siteName,
- $escapedSiteId, // displayed site ID
- htmlspecialchars( $site->getPageUrl(
$pageName ) ),
- htmlspecialchars( $pageName ),
- $this->getHtmlForEditSection( $editLink
. '/' . $escapedSiteId, 'td' ),
- $escapedSiteId // ID used in classes
- );
- }
- }
-
- // built table footer with button to add site-links, consider
list could be complete!
- $isFull = count( $siteLinks ) >= count( $sites );
-
- $tfoot = wfTemplate( 'wb-sitelinks-tfoot',
- $isFull ? wfMessage( 'wikibase-sitelinksedittool-full'
)->parse() : '',
- $this->getHtmlForEditSection( $editLink, 'td', 'add',
!$isFull )
- );
-
- $groupName = in_array( $group, $specialGroups ) ? 'special' :
$group;
-
- return $html . wfTemplate(
- 'wb-sitelinks-table',
- $thead,
- $tbody,
- $tfoot,
- htmlspecialchars( $groupName )
- );
+ return $siteLinksView->getHtml( $item, $groups, $editable );
}
}
diff --git a/repo/includes/view/SectionEditLinkGenerator.php
b/repo/includes/view/SectionEditLinkGenerator.php
index 8a090c6..372eb89 100644
--- a/repo/includes/view/SectionEditLinkGenerator.php
+++ b/repo/includes/view/SectionEditLinkGenerator.php
@@ -1,8 +1,11 @@
<?php
-namespace Wikibase;
+namespace Wikibase\View;
+use Language;
use Message;
+use SpecialPageFactory;
+use Wikibase\Entity;
/**
* Generates HTML for a section edit link
@@ -15,7 +18,6 @@
* @author Daniel Kinzler
*/
class SectionEditLinkGenerator {
-
/**
* Returns a toolbar with an edit link for a single statement.
Equivalent to edit toolbar in JavaScript but with
@@ -59,4 +61,24 @@
wfProfileOut( __METHOD__ );
return $html;
}
+
+ public function getEditUrl( $specialpagename, Entity $entity, Language
$language = null ) {
+ $specialpage = SpecialPageFactory::getPage( $specialpagename );
+
+ if ( $specialpage === null ) {
+ return ''; //XXX: this should throw an exception?!
+ }
+
+ if ( $entity->getId() ) {
+ $subpage = $entity->getId()->getPrefixedId();
+ } else {
+ $subpage = ''; // can't skip this, that would confuse
the order of parameters!
+ }
+
+ if ( $language !== null ) {
+ $subpage .= '/' . $language->getCode();
+ }
+ return $specialpage->getPageTitle( $subpage )->getLocalURL();
+ }
+
}
diff --git a/repo/includes/view/SiteLinksView.php
b/repo/includes/view/SiteLinksView.php
new file mode 100644
index 0000000..58c3203
--- /dev/null
+++ b/repo/includes/view/SiteLinksView.php
@@ -0,0 +1,188 @@
+<?php
+
+namespace Wikibase\View;
+
+use Message;
+use SiteStore;
+use Wikibase\DataModel\SimpleSiteLink;
+use Wikibase\Item;
+use Wikibase\Repo\WikibaseRepo;
+use Wikibase\Utils;
+
+class SiteLinksView {
+
+ private $siteStore;
+ private $sectionEditLinkGenerator;
+
+ public function __construct( SiteStore $siteStore,
SectionEditLinkGenerator $sectionEditLinkGenerator ) {
+ $this->siteStore = $siteStore;
+ $this->sectionEditLinkGenerator = $sectionEditLinkGenerator;
+ }
+
+ /**
+ * Builds and returns the HTML representing a WikibaseEntity's
site-links.
+ *
+ * @since 0.1
+ *
+ * @param Item $item the entity to render
+ * @param bool $editable whether editing is allowed (enabled edit links)
+ *
+ * @return string
+ */
+ public function getHtml( Item $item, $groups, $editable = true ) {
+ $html = '';
+
+ foreach ( $groups as $group ) {
+ $html .= $this->getHtmlForSiteLinkGroup( $item, $group,
$editable );
+ }
+
+ return $html;
+ }
+
+ /**
+ * Builds and returns the HTML representing a group of a
WikibaseEntity's site-links.
+ *
+ * @todo: code in this function belongs in its own class!
+ *
+ * @since 0.4
+ *
+ * @param Item $item the entity to render
+ * @param string $group a site group ID
+ * @param bool $editable whether editing is allowed (enabled edit links)
+ * @return string
+ */
+ private function getHtmlForSiteLinkGroup( Item $item, $group, $editable
= true ) {
+ // @todo inject into constructor
+ $sites = $this->siteStore->getSites();
+ $specialGroups =
WikibaseRepo::getDefaultInstance()->getSettings()->getSetting(
"specialSiteLinkGroups" );
+
+ $allSiteLinks = $item->getSiteLinks();
+ $siteLinks = array(); // site links of the currently handled
site group
+
+ foreach( $allSiteLinks as $siteLink ) {
+ $site = $sites->getSite( $siteLink->getSiteId() );
+
+ if ( $site === null ) {
+ continue;
+ }
+
+ if ( $site->getGroup() === $group ) {
+ $siteLinks[] = $siteLink;
+ }
+ }
+
+ $html = $thead = $tbody = $tfoot = '';
+
+ $html .= wfTemplate(
+ 'wb-section-heading-sitelinks',
+ wfMessage( 'wikibase-sitelinks-' . $group )->parse(),
// heading
+ htmlspecialchars( 'sitelinks-' . $group, ENT_QUOTES )
// ID
+ // TODO: support entity-id as prefix for element IDs.
+ );
+
+ // FIXME: quickfix to allow a custom site-name / handling for
groups defined in $wgSpecialSiteLinkGroups
+ $siteNameMessageKey =
'wikibase-sitelinks-sitename-columnheading';
+ if ( in_array( $group, $specialGroups ) ) {
+ $siteNameMessageKey .= '-special';
+ }
+
+ if( !empty( $siteLinks ) ) {
+ $thead = wfTemplate( 'wb-sitelinks-thead',
+ wfMessage( $siteNameMessageKey )->parse(),
+ wfMessage(
'wikibase-sitelinks-siteid-columnheading' )->parse(),
+ wfMessage(
'wikibase-sitelinks-link-columnheading' )->parse()
+ );
+ }
+
+ $i = 0;
+
+ // Sort the sitelinks according to their global id
+ $safetyCopy = $siteLinks; // keep a shallow copy;
+ $sortOk = usort(
+ $siteLinks,
+ function( SimpleSiteLink $a, SimpleSiteLink $b ) {
+ return strcmp( $a->getSiteId(), $b->getSiteId()
);
+ }
+ );
+
+ if ( !$sortOk ) {
+ $siteLinks = $safetyCopy;
+ }
+
+ // Link to SpecialPage
+ $editLink = $this->sectionEditLinkGenerator->getEditUrl(
'SetSiteLink', $item, null );
+
+ /* @var SimpleSiteLink $link */
+ foreach( $siteLinks as $siteLink ) {
+ $alternatingClass = ( $i++ % 2 ) ? 'even' : 'uneven';
+
+ $siteId = $siteLink->getSiteId();
+ $pageName = $siteLink->getPageName();
+
+ $site = $sites->hasSite( $siteId ) ? $sites->getSite(
$siteId ) : null;
+
+ if ( !$site || $site->getDomain() === '' ) {
+ // the link is pointing to an unknown site.
+ // XXX: hide it? make it red? strike it out?
+
+ $tbody .= wfTemplate( 'wb-sitelink-unknown',
+ $alternatingClass,
+ htmlspecialchars( $siteId ),
+ htmlspecialchars(
$siteLink->getPageName() ),
+ $this->getHtmlForEditSection(
$editLink, 'td' )
+ );
+
+ } else {
+ $languageCode = $site->getLanguageCode();
+ $escapedSiteId = htmlspecialchars( $siteId );
+ // FIXME: this is a quickfix to allow a custom
site-name for groups defined in $wgSpecialSiteLinkGroups instead of showing the
language-name
+ if ( in_array( $group, $specialGroups ) ) {
+ $siteNameMsg = wfMessage(
'wikibase-sitelinks-sitename-' . $siteId );
+ $siteName = $siteNameMsg->exists() ?
$siteNameMsg->parse() : $siteId;
+ } else {
+ // TODO: get an actual site name rather
then just the language
+ $siteName = htmlspecialchars(
Utils::fetchLanguageName( $languageCode ) );
+ }
+
+ // TODO: for non-JS, also set the dir attribute
on the link cell;
+ // but do not build language objects for each
site since it causes too much load
+ // and will fail when having too much site links
+ $tbody .= wfTemplate( 'wb-sitelink',
+ $languageCode,
+ $alternatingClass,
+ $siteName,
+ $escapedSiteId, // displayed site ID
+ htmlspecialchars( $site->getPageUrl(
$pageName ) ),
+ htmlspecialchars( $pageName ),
+ $this->getHtmlForEditSection( $editLink
. '/' . $escapedSiteId, 'td' ),
+ $escapedSiteId // ID used in classes
+ );
+ }
+ }
+
+ // built table footer with button to add site-links, consider
list could be complete!
+ $isFull = count( $siteLinks ) >= count( $sites );
+
+ $tfoot = wfTemplate( 'wb-sitelinks-tfoot',
+ $isFull ? wfMessage( 'wikibase-sitelinksedittool-full'
)->parse() : '',
+ $this->getHtmlForEditSection( $editLink, 'td', 'add',
!$isFull )
+ );
+
+ $groupName = in_array( $group, $specialGroups ) ? 'special' :
$group;
+
+ return $html . wfTemplate(
+ 'wb-sitelinks-table',
+ $thead,
+ $tbody,
+ $tfoot,
+ htmlspecialchars( $groupName )
+ );
+ }
+
+ private function getHtmlForEditSection( $url, $tag = 'span', $action =
'edit', $enabled = true ) {
+ $msg = new Message( 'wikibase-' . $action );
+
+ return $this->sectionEditLinkGenerator->getHtmlForEditSection(
$url, $msg, $tag, $enabled );
+ }
+
+}
diff --git a/repo/includes/view/TermBoxView.php
b/repo/includes/view/TermBoxView.php
index 1f13260..29a0afe 100644
--- a/repo/includes/view/TermBoxView.php
+++ b/repo/includes/view/TermBoxView.php
@@ -5,7 +5,7 @@
use Language;
use SpecialPage;
use Title;
-
+use Wikibase\View\SectionEditLinkGenerator;
/**
* Generates HTML for displaying the term box, that is, the box
diff --git a/repo/tests/phpunit/includes/view/SectionEditLinkGeneratorTest.php
b/repo/tests/phpunit/includes/view/SectionEditLinkGeneratorTest.php
index 94b72e9..f5f78fc 100644
--- a/repo/tests/phpunit/includes/view/SectionEditLinkGeneratorTest.php
+++ b/repo/tests/phpunit/includes/view/SectionEditLinkGeneratorTest.php
@@ -2,7 +2,7 @@
namespace Wikibase\Test;
-use Wikibase\SectionEditLinkGenerator;
+use Wikibase\View\SectionEditLinkGenerator;
/**
* @covers Wikibase\SectionEditLinkGenerator
--
To view, visit https://gerrit.wikimedia.org/r/129122
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id2e1fba574b737c15fa2dc5ee4fb6152ca240c96
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Adrian Lang <adrian.l...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
