From: umherirrender Date: Sat, 21 Apr 2012 08:32:09 +0000 (+0200) Subject: strip off subpages direct in GenderCache X-Git-Tag: 1.31.0-rc.0~23811^2 X-Git-Url: http://git.cyclocoop.org//%27%40script%40/%27?a=commitdiff_plain;h=4989ddd4237955d7f0db0b8d616920c631e7fa09;p=lhc%2Fweb%2Fwiklou.git strip off subpages direct in GenderCache LinkBatch can also give subpages to the GenderCache and therefor it is easier to do it always in GenderCache, than in LinkBatch and Title Add unit tests for GenderCache Change-Id: Ia936ff8bb639a197b0b3a8e07c97a66edd57dd10 --- diff --git a/includes/Title.php b/includes/Title.php index 1876d75caa..6764f8f8d1 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -711,17 +711,9 @@ class Title { } } - // Strip off subpages - $pagename = $this->getText(); - if ( strpos( $pagename, '/' ) !== false ) { - list( $username , ) = explode( '/', $pagename, 2 ); - } else { - $username = $pagename; - } - if ( $wgContLang->needsGenderDistinction() && MWNamespace::hasGenderDistinction( $this->mNamespace ) ) { - $gender = GenderCache::singleton()->getGenderOf( $username, __METHOD__ ); + $gender = GenderCache::singleton()->getGenderOf( $this->getText(), __METHOD__ ); return $wgContLang->getGenderNsText( $this->mNamespace, $gender ); } diff --git a/includes/cache/GenderCache.php b/includes/cache/GenderCache.php index 2cc1061742..c27ab00c0c 100644 --- a/includes/cache/GenderCache.php +++ b/includes/cache/GenderCache.php @@ -48,7 +48,7 @@ class GenderCache { $username = $username->getName(); } - $username = strtr( $username, '_', ' ' ); + $username = self::normalizeUsername( $username ); if ( !isset( $this->cache[$username] ) ) { if ( $this->misses >= $this->missLimit && $wgUser->getName() !== $username ) { @@ -60,11 +60,7 @@ class GenderCache { } else { $this->misses++; - if ( !User::isValidUserName( $username ) ) { - $this->cache[$username] = $this->getDefault(); - } else { - $this->doQuery( $username, $caller ); - } + $this->doQuery( $username, $caller ); } } @@ -86,7 +82,6 @@ class GenderCache { foreach ( $data as $ns => $pagenames ) { if ( !MWNamespace::hasGenderDistinction( $ns ) ) continue; foreach ( array_keys( $pagenames ) as $username ) { - if ( isset( $this->cache[$username] ) ) continue; $users[$username] = true; } } @@ -102,26 +97,28 @@ class GenderCache { public function doQuery( $users, $caller = '' ) { $default = $this->getDefault(); - foreach ( (array) $users as $index => $value ) { - $name = strtr( $value, '_', ' ' ); - if ( isset( $this->cache[$name] ) ) { - // Skip users whose gender setting we already know - unset( $users[$index] ); - } else { - $users[$index] = $name; + $usersToCheck = array(); + foreach ( (array) $users as $value ) { + $name = self::normalizeUsername( $value ); + // Skip users whose gender setting we already know + if ( !isset( $this->cache[$name] ) ) { // For existing users, this value will be overwritten by the correct value $this->cache[$name] = $default; + // query only for valid names, which can be in the database + if( User::isValidUserName( $name ) ) { + $usersToCheck[] = $name; + } } } - if ( count( $users ) === 0 ) { + if ( count( $usersToCheck ) === 0 ) { return; } $dbr = wfGetDB( DB_SLAVE ); $table = array( 'user', 'user_properties' ); $fields = array( 'user_name', 'up_value' ); - $conds = array( 'user_name' => $users ); + $conds = array( 'user_name' => $usersToCheck ); $joins = array( 'user_properties' => array( 'LEFT JOIN', array( 'user_id = up_user', 'up_property' => 'gender' ) ) ); @@ -129,11 +126,20 @@ class GenderCache { if ( strval( $caller ) !== '' ) { $comment .= "/$caller"; } - $res = $dbr->select( $table, $fields, $conds, $comment, $joins, $joins ); + $res = $dbr->select( $table, $fields, $conds, $comment, array(), $joins ); foreach ( $res as $row ) { $this->cache[$row->user_name] = $row->up_value ? $row->up_value : $default; } } + private static function normalizeUsername( $username ) { + // Strip off subpages + $indexSlash = strpos( $username, '/' ); + if ( $indexSlash !== false ) { + $username = substr( $username, 0, $indexSlash ); + } + // normalize underscore/spaces + return strtr( $username, '_', ' ' ); + } } diff --git a/includes/cache/LinkBatch.php b/includes/cache/LinkBatch.php index 29fde0315b..ab33dcfbc9 100644 --- a/includes/cache/LinkBatch.php +++ b/includes/cache/LinkBatch.php @@ -195,7 +195,7 @@ class LinkBatch { } $genderCache = GenderCache::singleton(); - $genderCache->dolinkBatch( $this->data, $this->caller ); + $genderCache->doLinkBatch( $this->data, $this->caller ); return true; } diff --git a/tests/phpunit/includes/cache/GenderCacheTest.php b/tests/phpunit/includes/cache/GenderCacheTest.php new file mode 100644 index 0000000000..d62ee20436 --- /dev/null +++ b/tests/phpunit/includes/cache/GenderCacheTest.php @@ -0,0 +1,100 @@ +getID() == 0 ) { + $user->addToDatabase(); + $user->setPassword( 'UTMalePassword' ); + } + //ensure the right gender + $user->setOption( 'gender', 'male' ); + $user->saveSettings(); + + $user = User::newFromName( 'UTFemale' ); + if( $user->getID() == 0 ) { + $user->addToDatabase(); + $user->setPassword( 'UTFemalePassword' ); + } + //ensure the right gender + $user->setOption( 'gender', 'female' ); + $user->saveSettings(); + + $user = User::newFromName( 'UTDefaultGender' ); + if( $user->getID() == 0 ) { + $user->addToDatabase(); + $user->setPassword( 'UTDefaultGenderPassword' ); + } + //ensure the default gender + $user->setOption( 'gender', null ); + $user->saveSettings(); + } + + /** + * test usernames + * + * @dataProvider dataUserName + */ + function testUserName( $username, $expectedGender ) { + $genderCache = GenderCache::singleton(); + $gender = $genderCache->getGenderOf( $username ); + $this->assertEquals( $gender, $expectedGender, "GenderCache normal" ); + } + + /** + * genderCache should work with user objects, too + * + * @dataProvider dataUserName + */ + function testUserObjects( $username, $expectedGender ) { + $genderCache = GenderCache::singleton(); + $user = User::newFromName( $username ); + $gender = $genderCache->getGenderOf( $user ); + $this->assertEquals( $gender, $expectedGender, "GenderCache normal" ); + } + + function dataUserName() { + return array( + array( 'UTMale', 'male' ), + array( 'UTFemale', 'female' ), + array( 'UTDefaultGender', 'unknown' ), + array( 'UTNotExist', 'unknown' ), + //some not valid user + array( '127.0.0.1', 'unknown' ), + array( 'user@test', 'unknown' ), + ); + } + + /** + * test strip of subpages to avoid unnecessary queries + * against the never existing username + * + * @dataProvider dataStripSubpages + */ + function testStripSubpages( $pageWithSubpage, $expectedGender ) { + $genderCache = GenderCache::singleton(); + $gender = $genderCache->getGenderOf( $pageWithSubpage ); + $this->assertEquals( $gender, $expectedGender, "GenderCache must strip of subpages" ); + } + + function dataStripSubpages() { + return array( + array( 'UTMale/subpage', 'male' ), + array( 'UTFemale/subpage', 'female' ), + array( 'UTDefaultGender/subpage', 'unknown' ), + array( 'UTNotExist/subpage', 'unknown' ), + array( '127.0.0.1/subpage', 'unknown' ), + ); + } +}