r22873 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r22872‎ | r22873 | r22874 >
Date:23:09, 9 June 2007
Author:robchurch
Status:old
Tags:
Comment:
* Move "new talk" flag backend functions to NewTalk::*
* Use object cache to store "new talk" status for all users and store a string value to avoid confusion over idiotic boolean conversions from BagOStuff::get() et al.
* Make columns in `user_newtalk` nullable and ditch existing indices in favour of a single UNIQUE index so we can do an INSERT IGNORE when updating, saves time [some problem with the updaters]
Modified paths:
  • /branches/robchurch/newtalk/includes/AutoLoader.php (modified) (history)
  • /branches/robchurch/newtalk/includes/NewTalk.php (added) (history)
  • /branches/robchurch/newtalk/includes/User.php (modified) (history)
  • /branches/robchurch/newtalk/maintenance/archives/patch-fix-newtalk.sql (added) (history)
  • /branches/robchurch/newtalk/maintenance/updaters.inc (modified) (history)

Diff [purge]

Index: branches/robchurch/newtalk/maintenance/archives/patch-fix-newtalk.sql
——@@ -0,0 +1,9 @@
 2+-- Alters columns in `user_newtalk` to be nullable and
 3+-- drops existing indices in favour of a single UNIQUE
 4+-- index
 5+ALTER TABLE /*wgDBprefix*/user_newtalk
 6+DROP INDEX `user_id`,
 7+DROP INDEX `user_ip`,
 8+CHANGE `user_id` `user_id` INT(5) NULL,
 9+CHANGE `user_ip` `user_ip` VARCHAR(40) NULL,
 10+ADD UNIQUE( `user_id`, `user_ip` )
Property changes on: branches/robchurch/newtalk/maintenance/archives/patch-fix-newtalk.sql
___________________________________________________________________
Added: svn:eol-style
111 + native
Index: branches/robchurch/newtalk/maintenance/updaters.inc
——@@ -932,6 +932,9 @@
933933
934934 do_backlinking_indices_update(); flush();
935935
 936+ fix_newtalk();
 937+ flush();
 938+
936939 do_restrictions_update(); flush ();
937940
938941 echo "Deleting old default messages (this may take a long time!)..."; flush();
——@@ -956,6 +959,20 @@
957960 }
958961 }
959962
 963+/**
 964+ * Fixes up some columns and indices in `user_newtalk`
 965+ */
 966+function fix_newtalk() {
 967+ global $wgDatabase;
 968+ if( $wgDatabase->indexExists( 'user_newtalk', 'user_id' ) ) {
 969+ echo( "Fixing up `user_newtalk`..." );
 970+ $wgDatabase->sourceFile( archive( 'patch-fix-newtalk.sql' ) );
 971+ echo( "done.\n" );
 972+ } else {
 973+ echo( "`user_newtalk` seems to be fixed up.\n" );
 974+ }
 975+}
 976+
960977 function do_restrictions_update() {
961978 # Adding page_restrictions table, obsoleting page.page_restrictions.
962979 # Migrating old restrictions to new table
Index: branches/robchurch/newtalk/includes/User.php
——@@ -1175,31 +1175,16 @@
11761176 return str_replace( ' ', '_', $this->getName() );
11771177 }
11781178
1179 - function getNewtalk() {
 1179+ /**
 1180+ * Is the "new messages" flag set for this user?
 1181+ *
 1182+ * @return bool
 1183+ */
 1184+ public function getNewtalk() {
11801185 $this->load();
1181 -
1182 - # Load the newtalk status if it is unloaded (mNewtalk=-1)
1183 - if( $this->mNewtalk === -1 ) {
1184 - $this->mNewtalk = false; # reset talk page status
1185 -
1186 - # Check memcached separately for anons, who have no
1187 - # entire User object stored in there.
1188 - if( !$this->mId ) {
1189 - global $wgMemc;
1190 - $key = wfMemcKey( 'newtalk', 'ip', $this->getName() );
1191 - $newtalk = $wgMemc->get( $key );
1192 - if( $newtalk != "" ) {
1193 - $this->mNewtalk = (bool)$newtalk;
1194 - } else {
1195 - $this->mNewtalk = $this->checkNewtalk( 'user_ip', $this->getName() );
1196 - $wgMemc->set( $key, (int)$this->mNewtalk, time() + 1800 );
1197 - }
1198 - } else {
1199 - $this->mNewtalk = $this->checkNewtalk( 'user_id', $this->mId );
1200 - }
1201 - }
1202 -
1203 - return (bool)$this->mNewtalk;
 1186+ if( $this->mNewtalk === -1 )
 1187+ $this->mNewtalk = NewTalk::get( $this );
 1188+ return $this->mNewtalk;
12041189 }
12051190
12061191 /**
——@@ -1217,108 +1202,36 @@
12181203 return array(array("wiki" => wfWikiID(), "link" => $utp->getLocalURL()));
12191204 }
12201205
1221 -
12221206 /**
1223 - * Perform a user_newtalk check on current slaves; if the memcached data
1224 - * is funky we don't want newtalk state to get stuck on save, as that's
1225 - * damn annoying.
 1207+ * Set the "new messages" flag for this user
12261208 *
1227 - * @param string $field
1228 - * @param mixed $id
1229 - * @return bool
1230 - * @private
 1209+ * @param bool $set Flag state
12311210 */
1232 - function checkNewtalk( $field, $id ) {
1233 - $dbr = wfGetDB( DB_SLAVE );
1234 - $ok = $dbr->selectField( 'user_newtalk', $field,
1235 - array( $field => $id ), __METHOD__ );
1236 - return $ok !== false;
1237 - }
1238 -
1239 - /**
1240 - * Add or update the
1241 - * @param string $field
1242 - * @param mixed $id
1243 - * @private
1244 - */
1245 - function updateNewtalk( $field, $id ) {
1246 - if( $this->checkNewtalk( $field, $id ) ) {
1247 - wfDebug( __METHOD__." already set ($field, $id), ignoring\n" );
1248 - return false;
 1211+ public function setNewtalk( $set = true ) {
 1212+ if( !$set ) {
 1213+ $this->deleteNewtalk();
 1214+ } elseif( !wfReadOnly() ) {
 1215+ $this->load();
 1216+ $this->mNewtalk = true;
 1217+ NewTalk::set( $this );
 1218+ $this->addWatch( $this->getTalkPage() );
 1219+ $this->invalidateCache();
12491220 }
1250 - $dbw = wfGetDB( DB_MASTER );
1251 - $dbw->insert( 'user_newtalk',
1252 - array( $field => $id ),
1253 - __METHOD__,
1254 - 'IGNORE' );
1255 - wfDebug( __METHOD__.": set on ($field, $id)\n" );
1256 - return true;
12571221 }
1258 -
 1222+
12591223 /**
1260 - * Clear the new messages flag for the given user
1261 - * @param string $field
1262 - * @param mixed $id
1263 - * @private
 1224+ * Unset the "new messages" flag for this user
12641225 */
1265 - function deleteNewtalk( $field, $id ) {
1266 - if( !$this->checkNewtalk( $field, $id ) ) {
1267 - wfDebug( __METHOD__.": already gone ($field, $id), ignoring\n" );
1268 - return false;
 1226+ public function deleteNewtalk() {
 1227+ if( !wfReadOnly() ) {
 1228+ $this->load();
 1229+ $this->mNewtalk = false;
 1230+ NewTalk::remove( $this );
 1231+ $this->invalidateCache();
12691232 }
1270 - $dbw = wfGetDB( DB_MASTER );
1271 - $dbw->delete( 'user_newtalk',
1272 - array( $field => $id ),
1273 - __METHOD__ );
1274 - wfDebug( __METHOD__.": killed on ($field, $id)\n" );
1275 - return true;
12761233 }
12771234
12781235 /**
1279 - * Update the 'You have new messages!' status.
1280 - * @param bool $val
1281 - */
1282 - function setNewtalk( $val ) {
1283 - if( wfReadOnly() ) {
1284 - return;
1285 - }
1286 -
1287 - $this->load();
1288 - $this->mNewtalk = $val;
1289 -
1290 - if( $this->isAnon() ) {
1291 - $field = 'user_ip';
1292 - $id = $this->getName();
1293 - } else {
1294 - $field = 'user_id';
1295 - $id = $this->getId();
1296 - }
1297 -
1298 - if( $val ) {
1299 - $changed = $this->updateNewtalk( $field, $id );
1300 - } else {
1301 - $changed = $this->deleteNewtalk( $field, $id );
1302 - }
1303 -
1304 - if( $changed ) {
1305 - if( $this->isAnon() ) {
1306 - // Anons have a separate memcached space, since
1307 - // user records aren't kept for them.
1308 - global $wgMemc;
1309 - $key = wfMemcKey( 'newtalk', 'ip', $val );
1310 - $wgMemc->set( $key, $val ? 1 : 0 );
1311 - } else {
1312 - if( $val ) {
1313 - // Make sure the user page is watched, so a notification
1314 - // will be sent out if enabled.
1315 - $this->addWatch( $this->getTalkPage() );
1316 - }
1317 - }
1318 - $this->invalidateCache();
1319 - }
1320 - }
1321 -
1322 - /**
13231236 * Generate a current or new-future timestamp to be stored in the
13241237 * user_touched field when we update things.
13251238 */
Index: branches/robchurch/newtalk/includes/AutoLoader.php
——@@ -235,6 +235,7 @@
236236 'StringUtils' => 'includes/StringUtils.php',
237237 'Title' => 'includes/Title.php',
238238 'User' => 'includes/User.php',
 239+ 'NewTalk' => 'includes/NewTalk.php',
239240 'MailAddress' => 'includes/UserMailer.php',
240241 'EmailNotification' => 'includes/UserMailer.php',
241242 'WatchedItem' => 'includes/WatchedItem.php',
Index: branches/robchurch/newtalk/includes/NewTalk.php
——@@ -0,0 +1,136 @@
 2+<?php
 3+
 4+/**
 5+ * Functions for managing "new messages" status
 6+ *
 7+ * @addtogroup User
 8+ * @author Rob Church <robchur@gmail.com>
 9+ */
 10+class NewTalk {
 11+
 12+ /**
 13+ * Status flag constants
 14+ */
 15+ const FLAG_SET = 'set';
 16+ const FLAG_UNSET = 'unset';
 17+
 18+ /**
 19+ * Is the "new messages" flag set for a given user?
 20+ *
 21+ * @param User $user User to check status for
 22+ * @return bool
 23+ */
 24+ public static function get( User $user ) {
 25+ global $wgMemc;
 26+ wfProfileIn( __METHOD__ );
 27+
 28+ # Attempt to read from object cache
 29+ $k = self::getCacheKey( $user );
 30+ if( ( $c = $wgMemc->get( $k ) ) !== false ) {
 31+ wfDebugLog( 'newtalk', 'Got `newtalk` flag for user `' . $user->getName() . '` from cache' );
 32+ wfProfileOut( __METHOD__ );
 33+ return $c == self::FLAG_SET;
 34+ }
 35+
 36+ # Fall back to database
 37+ list( $field, $value ) = self::getLookupConds( $user );
 38+ $set = self::getFromDB( $field, $value );
 39+
 40+ # Stash the value back in the object cache
 41+ $c = $set ? self::FLAG_SET : self::FLAG_UNSET;
 42+ $wgMemc->set( $k, $c, time() + 1800 );
 43+
 44+ # We're done
 45+ wfDebugLog( 'newtalk', 'Got `newtalk` flag for user `' . $user->getName() . '` from database' );
 46+ wfProfileOut( __METHOD__ );
 47+ return $set;
 48+ }
 49+
 50+ /**
 51+ * Set the "new messages" flag for a given user
 52+ *
 53+ * @param User $user User to update status for
 54+ */
 55+ public static function set( User $user ) {
 56+ global $wgMemc;
 57+ wfProfileIn( __METHOD__ );
 58+
 59+ # Add to database
 60+ $dbw = wfGetDB( DB_MASTER );
 61+ list( $field, $value ) = self::getLookupConds( $user );
 62+ $dbw->insert( 'user_newtalk', array( $field => $value ), __METHOD__, 'IGNORE' );
 63+
 64+ # Update object cache
 65+ $wgMemc->set( self::getCacheKey( $user ), self::FLAG_SET, time() + 1800 );
 66+
 67+ # We're done
 68+ wfDebugLog( 'newtalk', 'Set `newtalk` flag for user `' . $user->getName() . '`' );
 69+ wfProfileOut( __METHOD__ );
 70+ }
 71+
 72+ /**
 73+ * Unset the "new messages" flag for a given user
 74+ *
 75+ * @param User $user User to update status for
 76+ */
 77+ public static function remove( User $user ) {
 78+ global $wgMemc;
 79+ wfProfileIn( __METHOD__ );
 80+
 81+ # Remove from database
 82+ $dbw = wfGetDB( DB_MASTER );
 83+ list( $field, $value ) = self::getLookupConds( $user );
 84+ $dbw->delete( 'user_newtalk', array( $field => $value ), __METHOD__ );
 85+
 86+ # Update object cache
 87+ $wgMemc->set( self::getCacheKey( $user ), self::FLAG_UNSET, time() + 1800 );
 88+
 89+ # We're done
 90+ wfDebugLog( 'newtalk', 'Removed `newtalk` flag for user `' . $user->getName() . '`' );
 91+ wfProfileOut( __METHOD__ );
 92+ }
 93+
 94+ /**
 95+ * Read "new messages" status from the database
 96+ *
 97+ * @param string $field Table field to check
 98+ * @param string $value Field value to check
 99+ * @return bool
 100+ */
 101+ private static function getFromDB( $field, $value ) {
 102+ wfProfileIn( __METHOD__ );
 103+ $dbr = wfGetDB( DB_SLAVE );
 104+ $row = $dbr->selectRow( 'user_newtalk', '*', array( $field => $value ), __METHOD__ );
 105+ wfProfileOut( __METHOD__ );
 106+ return $row !== false;
 107+ }
 108+
 109+ /**
 110+ * Build the appropriate "new messages" object cache
 111+ * key for a given user
 112+ *
 113+ * @param User $user User to build key for
 114+ * @return string
 115+ */
 116+ private static function getCacheKey( User $user ) {
 117+ return $user->isLoggedIn()
 118+ ? wfMemcKey( 'newtalk', 'user', $user->getId() )
 119+ : wfMemcKey( 'newtalk', 'ip', $user->getName() );
 120+ }
 121+
 122+ /**
 123+ * Build the appropriate `user_newtalk` column/value pair
 124+ * to check for a given user
 125+ *
 126+ * @param User $user User to build pair for
 127+ * @return array
 128+ */
 129+ private static function getLookupConds( User $user ) {
 130+ return $user->isLoggedIn()
 131+ ? array( 'user_id', $user->getId() )
 132+ : array( 'user_ip', $user->getName() );
 133+ }
 134+
 135+}
 136+
 137+?>
Property changes on: branches/robchurch/newtalk/includes/NewTalk.php
___________________________________________________________________
Added: svn:eol-style
1138 + native

Status & tagging log