Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Avoid key conflict errors in User::addToDatabase

Browse source 
* Also cleaned up the IDBAccessObject constants to cover more cases.

Bug: 66185
Change-Id: Ide28af552b3c59428923b373c0f5764414d50a1f
This commit is contained in:
Aaron Schulz 2014-06-23 17:40:08 -07:00 committed by Krinkle
parent 4dbc439745
commit 8006aa946a
2 changed files with 29 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
includes/User.php
View file

@ -56,7 +56,7 @@ class PasswordError extends MWException {
* for rendering normal pages are set in the cookie to minimize use * for rendering normal pages are set in the cookie to minimize use
* of the database. * of the database.
*/ */
class User { class User implements IDBAccessObject {
/** /**
* Global constants made accessible as class constants so that autoloader * Global constants made accessible as class constants so that autoloader
* magic can be used. * magic can be used.
@ -1169,9 +1169,10 @@ class User {
* Load user and user_group data from the database. * Load user and user_group data from the database.
* $this->mId must be set, this is how the user is identified. * $this->mId must be set, this is how the user is identified.
* *
* @param integer $flags Supports User::READ_LOCKING
* @return bool True if the user exists, false if the user is anonymous * @return bool True if the user exists, false if the user is anonymous
*/ */
public function loadFromDatabase() { public function loadFromDatabase( $flags = 0 ) {
// Paranoia // Paranoia
$this->mId = intval( $this->mId ); $this->mId = intval( $this->mId );
@ -1186,7 +1187,10 @@ class User {
'user', 'user',
self::selectFields(), self::selectFields(),
array( 'user_id' => $this->mId ), array( 'user_id' => $this->mId ),
__METHOD__ __METHOD__,
( $flags & self::READ_LOCKING == self::READ_LOCKING )
? array( 'LOCK IN SHARE MODE' )
: array()
); );
wfRunHooks( 'UserLoadFromDatabase', array( $this, &$s ) ); wfRunHooks( 'UserLoadFromDatabase', array( $this, &$s ) );
@ -3605,17 +3609,25 @@ class User {
array( 'IGNORE' ) array( 'IGNORE' )
); );
if ( !$dbw->affectedRows() ) { if ( !$dbw->affectedRows() ) {
if ( !$inWrite ) { // The queries below cannot happen in the same REPEATABLE-READ snapshot.
// XXX: Get out of REPEATABLE-READ so the SELECT below works. // Handle this by COMMIT, if possible, or by LOCK IN SHARE MODE otherwise.
// Often this case happens early in views before any writes. if ( $inWrite ) {
// This shows up at least with CentralAuth. // Can't commit due to pending writes that may need atomicity.
// This may cause some lock contention unlike the case below.
$options = array( 'LOCK IN SHARE MODE' );
$flags = self::READ_LOCKING;
} else {
// Often, this case happens early in views before any writes when
// using CentralAuth. It's should be OK to commit and break the snapshot.
$dbw->commit( __METHOD__, 'flush' ); $dbw->commit( __METHOD__, 'flush' );
$options = array();
$flags = 0;
} }
$this->mId = $dbw->selectField( 'user', 'user_id', $this->mId = $dbw->selectField( 'user', 'user_id',
array( 'user_name' => $this->mName ), __METHOD__ ); array( 'user_name' => $this->mName ), __METHOD__, $options );
$loaded = false; $loaded = false;
if ( $this->mId ) { if ( $this->mId ) {
if ( $this->loadFromDatabase() ) { if ( $this->loadFromDatabase( $flags ) ) {
$loaded = true; $loaded = true;
} }
} }

13
includes/dao/IDBAccessObject.php
View file

@ -28,10 +28,12 @@
* functions. In general, objects should assume READ_NORMAL if no flags are explicitly given, * functions. In general, objects should assume READ_NORMAL if no flags are explicitly given,
* though certain objects may assume READ_LATEST for common use case or legacy reasons. * though certain objects may assume READ_LATEST for common use case or legacy reasons.
* *
* There are three types of reads: * There are four types of reads:
* - READ_NORMAL : Potentially cached read of data (e.g. from a slave or stale replica) * - READ_NORMAL : Potentially cached read of data (e.g. from a slave or stale replica)
* - READ_LATEST : Up-to-date read as of transaction start (e.g. from master or a quorum read) * - READ_LATEST : Up-to-date read as of transaction start (e.g. from master or a quorum read)
* - READ_LOCKING : Up-to-date read as of now, that locks the records for the transaction * - READ_LOCKING : Up-to-date read as of now, that locks (shared) the records
* - READ_EXCLUSIVE : Up-to-date read as of now, that locks (exclusive) the records
* All record locks persist for the duration of the transaction.
* *
* Callers should use READ_NORMAL (or pass in no flags) unless the read determines a write. * Callers should use READ_NORMAL (or pass in no flags) unless the read determines a write.
* In theory, such cases may require READ_LOCKING, though to avoid contention, READ_LATEST is * In theory, such cases may require READ_LOCKING, though to avoid contention, READ_LATEST is
@ -47,7 +49,8 @@
interface IDBAccessObject { interface IDBAccessObject {
// Constants for object loading bitfield flags (higher => higher QoS) // Constants for object loading bitfield flags (higher => higher QoS)
const READ_LATEST = 1; // read from the master const READ_LATEST = 1; // read from the master
const READ_LOCKING = 3; // READ_LATEST and "FOR UPDATE" const READ_LOCKING = 3; // READ_LATEST (1) and "LOCK IN SHARE MODE" (2)
const READ_EXCLUSIVE = 7; // READ_LOCKING (3) and "FOR UPDATE" (4)
// Convenience constant for callers to explicitly request slave data // Convenience constant for callers to explicitly request slave data
const READ_NORMAL = 0; // read from the slave const READ_NORMAL = 0; // read from the slave