r82029 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r82028‎ | r82029 | r82030 >
Date:20:40, 12 February 2011
Author:nikerabbit
Status:ok (Comments)
Tags:
Comment:
* (bug 17160) Gender specific display text for User namespace
Second attempt for this thing..
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/GenderCache.php (added) (history)
  • /trunk/phase3/includes/LinkBatch.php (modified) (history)
  • /trunk/phase3/includes/LocalisationCache.php (modified) (history)
  • /trunk/phase3/includes/Namespace.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/LinkBatch.php
@@ -94,6 +94,7 @@
9595 wfProfileIn( __METHOD__ );
9696 $res = $this->doQuery();
9797 $ids = $this->addResultToCache( $cache, $res );
 98+ $this->doGenderQuery();
9899 wfProfileOut( __METHOD__ );
99100 return $ids;
100101 }
@@ -157,6 +158,20 @@
158159 return $res;
159160 }
160161
 162+ public function doGenderQuery() {
 163+ if ( $this->isEmpty() ) {
 164+ return false;
 165+ }
 166+
 167+ global $wgContLang;
 168+ if ( !$wgContLang->needsGenderDistinction() ) {
 169+ return false;
 170+ }
 171+
 172+ $genderCache = GenderCache::singleton();
 173+ $genderCache->dolinkBatch( $this->data, $this->caller );
 174+ }
 175+
161176 /**
162177 * Construct a WHERE clause which will match all the given titles.
163178 *
Index: trunk/phase3/includes/LocalisationCache.php
@@ -87,7 +87,7 @@
8888 'defaultUserOptionOverrides', 'linkTrail', 'namespaceAliases',
8989 'dateFormats', 'datePreferences', 'datePreferenceMigrationMap',
9090 'defaultDateFormat', 'extraUserToggles', 'specialPageAliases',
91 - 'imageFiles', 'preloadedMessages',
 91+ 'imageFiles', 'preloadedMessages', 'namespaceGenderAliases',
9292 );
9393
9494 /**
Index: trunk/phase3/includes/AutoLoader.php
@@ -98,6 +98,7 @@
9999 'ForkController' => 'includes/ForkController.php',
100100 'FormatExif' => 'includes/Exif.php',
101101 'FormOptions' => 'includes/FormOptions.php',
 102+ 'GenderCache' => 'includes/GenderCache.php',
102103 'GlobalDependency' => 'includes/CacheDependency.php',
103104 'HashBagOStuff' => 'includes/BagOStuff.php',
104105 'HashtableReplacer' => 'includes/StringUtils.php',
Index: trunk/phase3/includes/Title.php
@@ -617,6 +617,13 @@
618618 return MWNamespace::getCanonicalName( $this->mNamespace );
619619 }
620620 }
 621+
 622+ if ( $wgContLang->needsGenderDistinction() &&
 623+ MWNamespace::hasGenderDistinction( $this->mNamespace ) ) {
 624+ $gender = GenderCache::singleton()->getGenderOf( $this->getText(), __METHOD__ );
 625+ return $wgContLang->getGenderNsText( $this->mNamespace, $gender );
 626+ }
 627+
621628 return $wgContLang->getNsText( $this->mNamespace );
622629 }
623630
Index: trunk/phase3/includes/Namespace.php
@@ -276,4 +276,17 @@
277277 // Default to the global setting
278278 return $wgCapitalLinks;
279279 }
 280+
 281+ /**
 282+ * Does the namespace (potentially) have different aliases for different
 283+ * genders. Not all languages make a distinction here.
 284+ *
 285+ * @since 1.18
 286+ * @param $index int Index to check
 287+ * @return bool
 288+ */
 289+ public static function hasGenderDistinction( $index ) {
 290+ return $index == NS_USER || $index == NS_USER_TALK;
 291+ }
 292+
280293 }
Index: trunk/phase3/includes/GenderCache.php
@@ -0,0 +1,119 @@
 2+<?php
 3+
 4+/**
 5+ * Caches user genders when needed to use correct namespace alises.
 6+ * @author Niklas Laxström
 7+ * @since 1.18
 8+ */
 9+class GenderCache {
 10+ protected $cache = array();
 11+ protected $default;
 12+ protected $misses = 0;
 13+ protected $missLimit = 1000;
 14+
 15+ public static function singleton() {
 16+ static $that = null;
 17+ if ( $that === null ) {
 18+ $that = new self();
 19+ }
 20+ return $that;
 21+ }
 22+
 23+ protected function __construct() {}
 24+
 25+ /**
 26+ * Returns the default gender option in this wiki.
 27+ * @return String
 28+ */
 29+ protected function getDefault() {
 30+ if ( $this->default === null ) {
 31+ $this->default = User::getDefaultOption( 'gender' );
 32+ }
 33+ return $this->default;
 34+ }
 35+
 36+ /**
 37+ * Returns the gender for given username.
 38+ * @param $users String: username
 39+ * @param $caller String: the calling method
 40+ * @return String
 41+ */
 42+ public function getGenderOf( $username, $caller = '' ) {
 43+ global $wgUser;
 44+
 45+ $username = strtr( $username, '_', ' ' );
 46+ if ( !isset( $this->cache[$username] ) ) {
 47+
 48+ if ( $this->misses >= $this->missLimit && $wgUser->getName() !== $username ) {
 49+ if( $this->misses === $this->missLimit ) {
 50+ $this->misses++;
 51+ wfDebug( __METHOD__ . ": too many misses, returning default onwards\n" );
 52+ }
 53+ return $this->getDefault();
 54+
 55+ } else {
 56+ $this->misses++;
 57+ if ( !User::isValidUserName( $username ) ) {
 58+ $this->cache[$username] = $this->getDefault();
 59+ } else {
 60+ $this->doQuery( $username, $caller );
 61+ }
 62+ }
 63+
 64+ }
 65+
 66+ /* Undefined if there is a valid username which for some reason doesn't
 67+ * exist in the database.
 68+ */
 69+ return isset( $this->cache[$username] ) ? $this->cache[$username] : $this->getDefault();
 70+ }
 71+
 72+ /**
 73+ * Wrapper for doQuery that processes raw LinkBatch data.
 74+ */
 75+ public function doLinkBatch( $data, $caller = '' ) {
 76+ $users = array();
 77+ foreach ( $data as $ns => $pagenames ) {
 78+ if ( !MWNamespace::hasGenderDistinction( $ns ) ) continue;
 79+ foreach ( array_keys( $pagenames ) as $username ) {
 80+ if ( isset( $this->cache[$username] ) ) continue;
 81+ $users[$username] = true;
 82+ }
 83+ }
 84+
 85+ $this->doQuery( array_keys( $users ), $caller );
 86+
 87+ }
 88+
 89+ /**
 90+ * Preloads genders for given list of users.
 91+ * @param $users List|String: usernames
 92+ * @param $caller String: the calling method
 93+ */
 94+ public function doQuery( $users, $caller = '' ) {
 95+ if ( count( $users ) === 0 ) return false;
 96+
 97+ foreach ( (array) $users as $index => $value ) {
 98+ $users[$index] = strtr( $value, '_', ' ' );
 99+ }
 100+
 101+ $dbr = wfGetDB( DB_SLAVE );
 102+ $table = array( 'user', 'user_properties' );
 103+ $fields = array( 'user_name', 'up_value' );
 104+ $conds = array( 'user_name' => $users );
 105+ $joins = array( 'user_properties' =>
 106+ array( 'LEFT JOIN', array( 'user_id = up_user', 'up_property' => 'gender' ) ) );
 107+
 108+ $comment = __METHOD__;
 109+ if ( strval( $caller ) !== '' ) {
 110+ $comment .= "/$caller";
 111+ }
 112+ $res = $dbr->select( $table, $fields, $conds, $comment, $joins, $joins );
 113+
 114+ $default = $this->getDefault();
 115+ foreach ( $res as $row ) {
 116+ $this->cache[$row->user_name] = $row->up_value ? $row->up_value : $default;
 117+ }
 118+ }
 119+
 120+}
Property changes on: trunk/phase3/includes/GenderCache.php
___________________________________________________________________
Added: svn:eol-style
1121 + native
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -112,6 +112,16 @@
113113 $namespaceAliases = array();
114114
115115 /**
 116+ * Array of gender specific. namespace aliases.
 117+ * Mapping NS_xxx to array of GENDERKEY to alias.
 118+ * Example:
 119+$namespaceGenderAliases = array(
 120+ NS_USER => array( 'male' => 'Male_user', 'female' => 'Female_user' ),
 121+);
 122+ */
 123+$namespaceGenderAliases = array();
 124+
 125+/**
116126 * Deprecated, use the message array
117127 */
118128 $mathNames = array(
Index: trunk/phase3/languages/Language.php
@@ -339,6 +339,29 @@
340340 }
341341
342342 /**
 343+ * Returns gender-dependent namespace alias if available.
 344+ * @param $index Int: namespace index
 345+ * @param $gender String: gender key (male, female... )
 346+ * @return String
 347+ * @since 1.18
 348+ */
 349+ function getGenderNsText( $index, $gender ) {
 350+ $ns = self::$dataCache->getItem( $this->mCode, 'namespaceGenderAliases' );
 351+ return isset( $ns[$index][$gender] ) ? $ns[$index][$gender] : $this->getNsText( $index );
 352+ }
 353+
 354+ /**
 355+ * Whether this language makes distinguishes genders for example in
 356+ * namespaces.
 357+ * @return bool
 358+ * @since 1.18
 359+ */
 360+ function needsGenderDistinction() {
 361+ $aliases = self::$dataCache->getItem( $this->mCode, 'namespaceGenderAliases' );
 362+ return count( $aliases ) > 0;
 363+ }
 364+
 365+ /**
343366 * Get a namespace key by value, case insensitive.
344367 * Only matches namespace names for the current language, not the
345368 * canonical ones defined in Namespace.php.
@@ -366,6 +389,14 @@
367390 }
368391 }
369392 }
 393+
 394+ $genders = self::$dataCache->getItem( $this->mCode, 'namespaceGenderAliases' );
 395+ foreach ( $genders as $index => $forms ) {
 396+ foreach ( $forms as $alias ) {
 397+ $aliases[$alias] = $index;
 398+ }
 399+ }
 400+
370401 $this->namespaceAliases = $aliases;
371402 }
372403 return $this->namespaceAliases;
Index: trunk/phase3/RELEASE-NOTES
@@ -176,6 +176,8 @@
177177 regularly. Below only new and removed languages are listed, as well as
178178 changes to languages because of Bugzilla reports.
179179
 180+* (bug 17160) Gender specific display text for User namespace
 181+
180182 == Compatibility ==
181183
182184 MediaWiki 1.18 requires PHP 5.1 (5.2 recommended). PHP 4 is no longer

Follow-up revisions

RevisionCommit summaryAuthorDate
r82033Followup r82029 - typo in commentnikerabbit21:05, 12 February 2011
r82037Add gender aliases per r82029raymond21:47, 12 February 2011
r82083Bug 27385 - Set Polish $namespaceGenderAliases (r82029)raymond21:35, 13 February 2011

Comments

#Comment by Simetrical (talk | contribs)   01:17, 3 March 2011

Neat.

#Comment by Duplicatebug (talk | contribs)   20:51, 17 September 2011

The GenderCache does not cache the default value for non-existing usernames. This can produce high load, if a user is only editing pages of non-existing users and someone looks at Special:Contributions.

#Comment by Nikerabbit (talk | contribs)   09:46, 18 September 2011

Could you file a bug about that, describing the issue in more detail. We need to find out how urgent the issue is and act on it.

#Comment by Duplicatebug (talk | contribs)   18:41, 18 September 2011

User::isValidUserName( $username ) is only check for the valid of a username (non-ip or so), not for existing. For a non-existing user the method GenderCache::doQuery is called. That method does a db select, but the username is not found in the table "user" and so the result is empty/does not contain the non-existing user and therefore is not added to the cache. That is the reason for the undefined (see the comment "Undefined if there is a valid username which for some reason doesn't exist in the database.")

I have open bug 30972

#Comment by Duplicatebug (talk | contribs)   23:03, 6 January 2012

You can also check with User::isValidUserName in GenderCache::doLinkBatch to avoid db queries for IPs/IP-Ranges.

#Comment by Duplicatebug (talk | contribs)   10:36, 7 January 2012
	public function doLinkBatch( $data, $caller = '' ) {
		$default = $this->getDefault();

		$users = array();
		foreach ( $data as $ns => $pagenames ) {
			if ( !MWNamespace::hasGenderDistinction( $ns ) ) {
				continue;
			}
			foreach ( array_keys( $pagenames ) as $value ) {
				$username = strtr( $value, '_', ' ' );
				if ( isset( $this->cache[$username] ) ) {
					//already cached
					continue;
				}
				if ( !User::isValidUserName( $username ) ) {
					//invalid username, cache default
					$this->cache[$username] = $default;
					continue;
				}
				$users[$username] = true;
			}
		}

		$this->doQuery( array_keys( $users ), $caller );
	}
#Comment by Nikerabbit (talk | contribs)   10:44, 7 January 2012

Shouldn't those checks be in doQuery?

#Comment by Duplicatebug (talk | contribs)   10:47, 7 January 2012

That is also possible. Than the check in getGenderOf is also not needed at that place.

#Comment by Duplicatebug (talk | contribs)   20:46, 11 February 2012

I have create bug 34340 for this.

#Comment by Duplicatebug (talk | contribs)   00:07, 7 January 2012

There is a folder includes/cache, maybe the GenderCache.php is there better located.

#Comment by Nikerabbit (talk | contribs)   09:56, 7 January 2012

This code was committed before that directory existed.

#Comment by Duplicatebug (talk | contribs)   10:41, 7 January 2012

Oh, sorry. Added after this, with r86899.

#Comment by Duplicatebug (talk | contribs)   20:45, 11 February 2012

Thanks for r108498

Status & tagging log