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

Merge "Remove User::$mEditCount and internal caching"

Browse source
This commit is contained in:
jenkins-bot 2021-06-18 17:57:35 +00:00 committed by Gerrit Code Review
commit 5028fe175d
5 changed files with 60 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
includes/deferred/UserEditCountUpdate.php
View file

@ -26,6 +26,10 @@ use Wikimedia\Assert\Assert;
/**
* Handles increment the edit count for a given set of users
*
* TODO we no longer need to store the instances of the relevant `UserIdentity`s, now that
* User::$mEditCount was removed the only caching lives in UserEditTracker, we should
* stop storing those objects.
*/
class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate {
/** @var array[] Map of (user ID => ('increment': int, 'instances': UserIdentity[])) */
@ -71,11 +75,10 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate {
$mwServices = MediaWikiServices::getInstance();
$lb = $mwServices->getDBLoadBalancer();
$dbw = $lb->getConnectionRef( DB_PRIMARY );
$userFactory = $mwServices->getUserFactory();
$editTracker = $mwServices->getUserEditTracker();
$fname = __METHOD__;
( new AutoCommitUpdate( $dbw, __METHOD__, function () use ( $lb, $dbw, $fname, $userFactory, $editTracker ) {
( new AutoCommitUpdate( $dbw, __METHOD__, function () use ( $lb, $dbw, $fname, $editTracker ) {
foreach ( $this->infoByUser as $userId => $info ) {
$dbw->update(
'user',
@ -97,26 +100,8 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate {
$lb->waitForMasterPos( $dbr );
$editTracker->initializeUserEditCount( $affectedInstances[0] );
}
$newCount = (int)$dbw->selectField(
'user',
'user_editcount',
[ 'user_id' => $userId ],
$fname
);
// Update the edit count in the instance caches. This is mostly useful
// for maintenance scripts, where deferred updates might run immediately
// and user instances might be reused for a long time. Only applies to
// instances where we have User objects, if we have UserIdentity only
// then invalidating the cache should be enough
foreach ( $affectedInstances as $affectedInstance ) {
if ( $affectedInstance instanceof User ) {
$affectedInstance->setEditCountInternal( $newCount );
}
}
// Clear the edit count in user cache too
$userFactory->newFromUserIdentity( $affectedInstances[0] )->invalidateCache();
// And the cache in UserEditTracker
// Clear the edit count in the UserEditTracker cache.
$editTracker->clearUserEditCache( $affectedInstances[0] );
}
} ) )->doUpdate();

39
includes/user/User.php
View file

@ -82,7 +82,7 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
* Version number to tag cached versions of serialized User objects. Should be increased when
* {@link $mCacheVars} or one of it's members changes.
*/
private const VERSION = 16;
private const VERSION = 17;
/**
* Exclude user options that are set to their default value.
@ -126,7 +126,6 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
'mEmailToken',
'mEmailTokenExpires',
'mRegistration',
'mEditCount',
// actor table
'mActorId',
];
@ -164,8 +163,6 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
protected $mEmailTokenExpires;
/** @var string */
protected $mRegistration;
/** @var int */
protected $mEditCount;
// @}
// @{
@ -1304,7 +1301,6 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
if ( $s !== false ) {
// Initialise user table data
$this->loadFromRow( $s );
$this->getEditCount(); // revalidation for nulls
return true;
}
@ -1370,7 +1366,15 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
}
if ( isset( $row->user_editcount ) ) {
$this->mEditCount = $row->user_editcount;
// Sanity check - don't try to set edit count for anonymous users
// We check the id here and not in UserEditTracker because calling
// User::getId() can trigger some other loading. This will result in
// discarding the user_editcount field for rows if the id wasn't set.
if ( $this->mId !== null && $this->mId !== 0 ) {
MediaWikiServices::getInstance()
->getUserEditTracker()
->setCachedUserEditCount( $this, (int)$row->user_editcount );
}
} else {
$all = false;
}
@ -1531,7 +1535,6 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
$this->mDatePreference = null;
$this->mBlockedby = -1; # Unset
$this->mHash = false;
$this->mEditCount = null;
$this->mThisAsAuthority = null;
if ( $wgFullyInitialised && $this->mFrom ) {
@ -2866,16 +2869,9 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
* @return int|null Null for anonymous users
*/
public function getEditCount() {
if ( !$this->getId() ) {
return null;
}
if ( $this->mEditCount === null ) {
$this->mEditCount = MediaWikiServices::getInstance()
->getUserEditTracker()
->getUserEditCount( $this );
}
return (int)$this->mEditCount;
return MediaWikiServices::getInstance()
->getUserEditTracker()
->getUserEditCount( $this );
}
/**
@ -4110,15 +4106,6 @@ class User implements Authority, IDBAccessObject, UserIdentity, UserEmailContact
MediaWikiServices::getInstance()->getUserEditTracker()->incrementUserEditCount( $this );
}
/**
* This method should not be called outside User/UserEditCountUpdate
*
* @param int $count
*/
public function setEditCountInternal( $count ) {
$this->mEditCount = $count;
}
/**
* Get the description of a given right
*

30
includes/user/UserEditTracker.php
View file

@ -59,12 +59,11 @@ class UserEditTracker {
* Get a user's edit count from the user_editcount field, falling back to initialize
*
* @param UserIdentity $user
* @return int
* @throws InvalidArgumentException If user is not registered
* @return int|null Null for anonymous users
*/
public function getUserEditCount( UserIdentity $user ) : int {
if ( !$user->getId() ) {
throw new InvalidArgumentException( __METHOD__ . ' requires a user ID' );
public function getUserEditCount( UserIdentity $user ) : ?int {
if ( !$user->isRegistered() ) {
return null;
}
$userId = $user->getId();
@ -195,7 +194,7 @@ class UserEditTracker {
}
/**
* @internal For use by User::clearInstanceCache
* @internal For use by User::clearInstanceCache()
* @param UserIdentity $user
*/
public function clearUserEditCache( UserIdentity $user ) {
@ -206,7 +205,24 @@ class UserEditTracker {
$userId = $user->getId();
$cacheKey = 'u' . (string)$userId;
$this->userEditCountCache[ $cacheKey ] = null;
unset( $this->userEditCountCache[ $cacheKey ] );
}
/**
* @internal For use by User::loadFromRow() and tests
* @param UserIdentity $user
* @param int $editCount
* @throws InvalidArgumentException If the user is not registered
*/
public function setCachedUserEditCount( UserIdentity $user, int $editCount ) {
if ( !$user->isRegistered() ) {
throw new InvalidArgumentException( __METHOD__ . ' with an anonymous user' );
}
$userId = $user->getId();
$cacheKey = 'u' . (string)$userId;
$this->userEditCountCache[ $cacheKey ] = $editCount;
}
}

26
tests/phpunit/includes/user/UserEditTrackerTest.php
View file

@ -45,7 +45,8 @@ class UserEditTrackerTest extends MediaWikiIntegrationTestCase {
'user',
[ 'user_editcount' => $count ],
[ 'user_id' => $user->getId() ],
__METHOD__ );
__METHOD__
);
}
public function testGetUserEditCount() {
@ -61,14 +62,11 @@ class UserEditTrackerTest extends MediaWikiIntegrationTestCase {
$this->assertSame( 5, $tracker->getUserEditCount( $user ) );
}
public function testGetUserEditCount_exception() {
// getUserEditCount throws if the user id is falsy
$userId = 0;
$user = new UserIdentityValue( $userId, __CLASS__ );
public function testGetUserEditCount_anon() {
// getUserEditCount returns null if the user is unregistered
$anon = UserIdentityValue::newAnonymous( '1.2.3.4' );
$tracker = $this->getServiceContainer()->getUserEditTracker();
$this->expectException( InvalidArgumentException::class );
$this->expectExceptionMessage( 'requires a user ID' );
$tracker->getUserEditCount( $user );
$this->assertNull( $tracker->getUserEditCount( $anon ) );
}
public function testGetUserEditCount_null() {
@ -162,4 +160,16 @@ class UserEditTrackerTest extends MediaWikiIntegrationTestCase {
);
}
public function testManualCache() {
// Make sure manually setting the cached value overrides the database, in case
// User::loadFromRow() is called with a row containing user_editcount that is
// different from the actual database value, the row takes precedence
$user = new UserIdentityValue( 123, __METHOD__ );
$this->setDbEditCount( $user, 5 );
$tracker = $this->getServiceContainer()->getUserEditTracker();
$tracker->setCachedUserEditCount( $user, 10 );
$this->assertSame( 10, $tracker->getUserEditCount( $user ) );
}
}

9
tests/phpunit/includes/user/UserTest.php
View file

@ -357,7 +357,6 @@ class UserTest extends MediaWikiIntegrationTestCase {
* Test User::editCount
* @group medium
* @covers User::getEditCount
* @covers User::setEditCountInternal
*/
public function testGetEditCount() {
$user = $this->getMutableTestUser()->getUser();
@ -389,14 +388,6 @@ class UserTest extends MediaWikiIntegrationTestCase {
$user->getEditCount(),
'After increasing the edit count manually, the user edit count should be 4'
);
// Update the edit count
$user->setEditCountInternal( 42 );
$this->assertSame(
42,
$user->getEditCount(),
'After setting the edit count manually, the user edit count should be 42'
);
}
/**