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

UserOptions: remove deprecated hooks.

Browse source 
After the hooks were removed we are finally ready to stop
reading user options from primary before writing them on save.
The new save hooks only work on modified options, so options
saving code can be significantly simplified.

Change-Id: I48df616c9f35d9a0b2801ada1b7dbef0bd4ad058
This commit is contained in:
Petr Pchelko 2021-09-22 17:37:21 -07:00 committed by Ppchelko
parent cf81b18f25
commit 164ec5cb29
9 changed files with 22 additions and 316 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
RELEASE-NOTES-1.38
View file

@ -185,6 +185,8 @@ because of Phabricator reports.
has been removed.
* The constant ApiBase::PARAM_VALUE_LINKS, deprecated since 1.35 has been
removed.
* UserLoadOptions, UserSaveOptions and UserResetAllOptions hooks, deprecated
since 1.37, were removed.
* …
=== Deprecations in 1.38 ===

3
includes/HookContainer/DeprecatedHooks.php
View file

@ -53,9 +53,6 @@ class DeprecatedHooks {
'SkinGetPoweredBy' => [ 'deprecatedVersion' => '1.37' ],
'SpecialMuteSubmit' => [ 'deprecatedVersion' => '1.35', 'silent' => true ],
'UserLoadFromDatabase' => [ 'deprecatedVersion' => '1.37' ],
'UserLoadOptions' => [ 'deprecatedVersion' => '1.37' ],
'UserResetAllOptions' => [ 'deprecatedVersion' => '1.37' ],
'UserSaveOptions' => [ 'deprecatedVersion' => '1.37' ],
'UserSetCookies' => [ 'deprecatedVersion' => '1.27' ],
'WikiPageDeletionUpdates' => [ 'deprecatedVersion' => '1.32', 'silent' => true ],
'userCan' => [ 'deprecatedVersion' => '1.37' ],

26
includes/HookContainer/HookRunner.php
View file

@ -542,11 +542,8 @@ class HookRunner implements
\MediaWiki\User\Hook\UserLoadAfterLoadFromSessionHook,
\MediaWiki\User\Hook\UserLoadDefaultsHook,
\MediaWiki\User\Hook\UserLoadFromDatabaseHook,
\MediaWiki\User\Hook\UserLoadOptionsHook,
\MediaWiki\User\Hook\UserLogoutHook,
\MediaWiki\User\Hook\UserRemoveGroupHook,
\MediaWiki\User\Hook\UserResetAllOptionsHook,
\MediaWiki\User\Hook\UserSaveOptionsHook,
\MediaWiki\User\Hook\UserSaveSettingsHook,
\MediaWiki\User\Hook\UserSendConfirmationMailHook,
\MediaWiki\User\Hook\UserSetEmailAuthenticationTimestampHook,
@ -4193,13 +4190,6 @@ class HookRunner implements
);
}
public function onUserLoadOptions( $user, &$options ) {
return $this->container->run(
'UserLoadOptions',
[ $user, &$options ]
);
}
public function onLoadUserOptions( UserIdentity $user, array &$options ): void {
$this->container->run(
'LoadUserOptions',
@ -4273,22 +4263,6 @@ class HookRunner implements
);
}
public function onUserResetAllOptions( $user, &$newOptions, $options,
$resetKinds
) {
return $this->container->run(
'UserResetAllOptions',
[ $user, &$newOptions, $options, $resetKinds ]
);
}
public function onUserSaveOptions( $user, &$options, $originalOptions ) {
return $this->container->run(
'UserSaveOptions',
[ $user, &$options, $originalOptions ]
);
}
public function onSaveUserOptions( UserIdentity $user, array &$modifiedOptions, array $originalOptions ) {
return $this->container->run(
'SaveUserOptions',

25
includes/user/Hook/UserLoadOptionsHook.php
View file

@ -1,25 +0,0 @@
<?php
namespace MediaWiki\User\Hook;
use User;
/**
* This is a hook handler interface, see docs/Hooks.md.
* Use the hook name "UserLoadOptions" to register handlers implementing this interface.
*
* @deprecated since 1.37 use LoadUserOptionsHook instead.
* @ingroup Hooks
*/
interface UserLoadOptionsHook {
/**
* This hook is called when user options/preferences are being loaded from the database.
*
* @since 1.35
*
* @param User $user
* @param array &$options Options, can be modified.
* @return bool|void True or no return value to continue or false to abort
*/
public function onUserLoadOptions( $user, &$options );
}

37
includes/user/Hook/UserResetAllOptionsHook.php
View file

@ -1,37 +0,0 @@
<?php
namespace MediaWiki\User\Hook;
use User;
/**
* This is a hook handler interface, see docs/Hooks.md.
* Use the hook name "UserResetAllOptions" to register handlers implementing this interface.
*
* @deprecated since 1.37. This hook was not used and no replacement is provided
* @ingroup Hooks
*/
interface UserResetAllOptionsHook {
/**
* This hook is called when user preferences have been requested to be reset.
*
* This hook can be used to exclude certain options from being reset even when the user
* has requested that all preferences to be reset, because certain options might be stored
* in the user_properties database table despite not being visible and editable via
* Special:Preferences.
*
* @since 1.35
*
* @param User $user The user whose preferences are being reset
* @param array &$newOptions Array of new (site default) preferences
* @param array $options Array of the user's old preferences
* @param string[] $resetKinds Array containing the kinds of preferences to reset
* @return bool|void True or no return value to continue or false to abort
*/
public function onUserResetAllOptions(
$user,
&$newOptions,
$options,
$resetKinds
);
}

31
includes/user/Hook/UserSaveOptionsHook.php
View file

@ -1,31 +0,0 @@
<?php
namespace MediaWiki\User\Hook;
use User;
/**
* This is a hook handler interface, see docs/Hooks.md.
* Use the hook name "UserSaveOptions" to register handlers implementing this interface.
*
* @deprecated since 1.37 use SaveUserOptionsHook instead.
* @ingroup Hooks
*/
interface UserSaveOptionsHook {
/**
* This hook is called just before saving user preferences.
*
* Hook handlers can either add or manipulate options, or reset one back to its default
* to block changing it. Hook handlers are also allowed to abort the process by returning
* false, e.g. to save to a global profile instead. Compare to the UserSaveSettings
* hook, which is called after the preferences have been saved.
*
* @since 1.35
*
* @param User $user The User for which the options are going to be saved
* @param array &$options The user's options as an associative array, modifiable
* @param array $originalOptions The user's original options being replaced
* @return bool|void True or no return value to continue or false to abort
*/
public function onUserSaveOptions( $user, &$options, $originalOptions );
}

7
includes/user/Hook/UserSaveSettingsHook.php
View file

@ -13,13 +13,14 @@ use User;
*/
interface UserSaveSettingsHook {
/**
* This hook is called directly after user preferences have been saved to the database.
* This hook is called directly after user settings have been saved to the database.
*
* Compare to the UserSaveOptions hook, which is called before saving.
* Compare to the SaveUserOptions hook, which is called before saving and is only
* called for options managed by UserOptionsManager.
*
* @since 1.35
*
* @param User $user The User for which the options have been saved
* @param User $user The User for which the settings have been saved
* @return bool|void True or no return value to continue or false to abort
*/
public function onUserSaveSettings( $user );

69
includes/user/UserOptionsManager.php
View file

@ -249,15 +249,6 @@ class UserOptionsManager extends UserOptionsLookup {
}
}
}
// TODO: Deprecate passing full user to the hook
$this->hookRunner->onUserResetAllOptions(
$this->userFactory->newFromUserIdentity( $user ),
$newOptions,
$oldOptions,
$resetKinds
);
$this->modifiedOptions[$this->getCacheKey( $user )] = $newOptions;
}
@ -427,53 +418,29 @@ class UserOptionsManager extends UserOptionsLookup {
$userKey = $this->getCacheKey( $user );
$modifiedOptions = $this->modifiedOptions[$userKey] ?? [];
$originalOptions = $this->loadOriginalOptions( $user, self::READ_LATEST );
$originalOptions = $this->loadOriginalOptions( $user );
if ( !$this->hookRunner->onSaveUserOptions( $user, $modifiedOptions, $originalOptions ) ) {
return false;
}
// TODO: only needed for old hook.
$optionsToSave = array_merge( $originalOptions, $modifiedOptions );
// Allow hooks to abort, for instance to save to a global profile.
// Reset options to default state before saving.
if ( !$this->hookRunner->onUserSaveOptions(
$this->userFactory->newFromUserIdentity( $user ), $optionsToSave, $originalOptions )
) {
return false;
}
// Delete any rows that were in the DB but are not in $optionsToSave
$keysToDelete = array_keys(
array_diff_key( $this->optionsFromDb[$userKey], $optionsToSave )
);
// Update rows that have changed
$userId = $user->getId();
$rowsToInsert = []; // all the new preference rows
$rowsToPreserve = [];
foreach ( $optionsToSave as $key => $value ) {
$rowsToInsert = [];
$keysToDelete = [];
foreach ( $modifiedOptions as $key => $value ) {
// Don't store unchanged or default values
$defaultValue = $this->defaultOptionsLookup->getDefaultOption( $key );
$oldValue = $this->optionsFromDb[$userKey][$key] ?? null;
if ( !$this->isValueEqual( $value, $defaultValue ) ) {
$row = [
'up_user' => $userId,
if ( $value === null || $this->isValueEqual( $value, $defaultValue ) ) {
$keysToDelete[] = $key;
} elseif ( !$this->isValueEqual( $value, $oldValue ) ) {
// Update by deleting and reinserting
$rowsToInsert[] = [
'up_user' => $user->getId(),
'up_property' => $key,
'up_value' => $value,
];
if ( !$this->isValueEqual( $value, $oldValue ) ) {
// Update by deleting and reinserting
$rowsToInsert[] = $row;
if ( $oldValue !== null ) {
$keysToDelete[] = $key;
}
} else {
// Preserve old value -- save the row for caching
$rowsToPreserve[] = $row;
if ( $oldValue !== null ) {
$keysToDelete[] = $key;
}
} elseif ( $oldValue !== null ) {
// Delete row that is being set to its default
$keysToDelete[] = $key;
}
}
@ -487,7 +454,7 @@ class UserOptionsManager extends UserOptionsLookup {
$dbw->delete(
'user_properties',
[
'up_user' => $userId,
'up_user' => $user->getId(),
'up_property' => $keysToDelete
],
__METHOD__
@ -498,12 +465,6 @@ class UserOptionsManager extends UserOptionsLookup {
$dbw->insert( 'user_properties', $rowsToInsert, __METHOD__, [ 'IGNORE' ] );
}
// We are storing these into the database, so that's what we will get if we fetch again
// So, set the just saved options to the cache, but don't prevent some other part of the
// code from locking the options again but saying READ_LATEST was used for caching.
$this->setOptionsFromDb( $user, self::READ_LATEST,
array_merge( $rowsToPreserve, $rowsToInsert ) );
// It's pretty cheap to recalculate new original later
// to apply whatever adjustments we apply when fetching from DB
// and re-merge with the defaults.
@ -669,10 +630,6 @@ class UserOptionsManager extends UserOptionsLookup {
// infinite recursion if the hook attempts to reload options
$this->originalOptionsCache[$userKey] = $options;
$this->queryFlagsUsedForCaching[$userKey] = $queryFlags;
// TODO: Remove deprecated hook.
$this->hookRunner->onUserLoadOptions(
$this->userFactory->newFromUserIdentity( $user ), $options
);
$this->hookRunner->onLoadUserOptions( $user, $options );
$this->originalOptionsCache[$userKey] = $options;
return $options;

138
tests/phpunit/includes/user/UserOptionsManagerTest.php
View file

@ -136,29 +136,15 @@ class UserOptionsManagerTest extends UserOptionsLookupTest {
$this->assertSame( 42, $manager->getIntOption( $user, 'int_option' ) );
$this->assertSame( true, $manager->getBoolOption( $user, 'bool_option' ) );
$manager->saveOptions( $user );
$this->assertSame( 'user_value', $manager->getOption( $user, 'string_option' ) );
$this->assertSame( 42, $manager->getIntOption( $user, 'int_option' ) );
$this->assertSame( true, $manager->getBoolOption( $user, 'bool_option' ) );
$manager = $this->getManager();
$this->assertSame( 'user_value', $manager->getOption( $user, 'string_option' ) );
$this->assertSame( 42, $manager->getIntOption( $user, 'int_option' ) );
$this->assertSame( true, $manager->getBoolOption( $user, 'bool_option' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::loadUserOptions
*/
public function testUserLoadOptionsHook() {
$this->filterDeprecated( '/UserLoadOptions/' );
$user = $this->getTestUser()->getUser();
$this->setTemporaryHook(
'UserLoadOptions',
static function ( User $hookUser, &$options ) use ( $user ) {
if ( $hookUser->equals( $user ) ) {
$options['from_hook'] = 'value_from_hook';
}
}
);
$this->assertSame( 'value_from_hook', $this->getManager()->getOption( $user, 'from_hook' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::loadUserOptions
*/
@ -175,24 +161,6 @@ class UserOptionsManagerTest extends UserOptionsLookupTest {
$this->assertSame( 'value_from_hook', $manager->getOption( $user, 'from_hook' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
public function testUserSaveOptionsHookAbort() {
$this->filterDeprecated( '/UserSaveOptions/' );
$user = $this->getTestUser()->getUser();
$this->setTemporaryHook(
'UserSaveOptions',
static function () {
return false;
}
);
$manager = $this->getManager();
$manager->setOption( $user, 'will_be_aborted_by_hook', 'value' );
$manager->saveOptions( $user );
$this->assertNull( $this->getManager()->getOption( $user, 'will_be_aborted_by_hook' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
@ -210,27 +178,6 @@ class UserOptionsManagerTest extends UserOptionsLookupTest {
$this->assertNull( $this->getManager()->getOption( $user, 'will_be_aborted_by_hook' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
public function testUserSaveOptionsHookModify() {
$this->filterDeprecated( '/UserSaveOptions/' );
$user = $this->getTestUser()->getUser();
$this->setTemporaryHook(
'UserSaveOptions',
static function ( User $hookUser, &$options ) use ( $user ) {
if ( $hookUser->equals( $user ) ) {
$options['from_hook'] = 'value_from_hook';
}
return true;
}
);
$manager = $this->getManager();
$manager->saveOptions( $user );
$this->assertSame( 'value_from_hook', $manager->getOption( $user, 'from_hook' ) );
$this->assertSame( 'value_from_hook', $this->getManager()->getOption( $user, 'from_hook' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
@ -261,30 +208,6 @@ class UserOptionsManagerTest extends UserOptionsLookupTest {
$this->assertNull( $manager->getOption( $user, 'blocked_by_hook' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
public function testUserSaveOptionsHookOriginal() {
$this->filterDeprecated( '/UserSaveOptions/' );
$user = $this->getTestUser()->getUser();
$manager = $this->getManager();
$originalLanguage = $manager->getOption( $user, 'language' );
$manager->setOption( $user, 'language', 'ru' );
$this->setTemporaryHook(
'UserSaveOptions',
function ( User $hookUser, &$options, $originalOptions ) use ( $user, $originalLanguage ) {
if ( $hookUser->equals( $user ) ) {
$this->assertSame( $originalLanguage, $originalOptions['language'] );
$this->assertSame( 'ru', $options['language'] );
$options['language'] = 'tr';
}
return true;
}
);
$manager->saveOptions( $user );
$this->assertSame( 'tr', $manager->getOption( $user, 'language' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
@ -312,61 +235,6 @@ class UserOptionsManagerTest extends UserOptionsLookupTest {
$this->assertSame( 'tr', $manager->getOption( $user, 'language' ) );
}
/**
* @covers \MediaWiki\User\UserOptionsManager::saveOptions
* @covers \MediaWiki\User\UserOptionsManager::loadUserOptions
*/
public function testLoadOptionsHookReflectsInOriginalOptions() {
$this->filterDeprecated( '/UserSaveOptions/' );
$this->filterDeprecated( '/UserLoadOptions/' );
$user = $this->getTestUser()->getUser();
$manager = $this->getManager();
$this->setTemporaryHook(
'UserLoadOptions',
static function ( User $hookUser, &$options ) use ( $user ) {
if ( $hookUser->equals( $user ) ) {
$options['from_load_hook'] = 'from_load_hook';
}
}
);
$this->setTemporaryHook(
'UserSaveOptions',
function ( User $hookUser, &$options, $originalOptions ) use ( $user ) {
if ( $hookUser->equals( $user ) ) {
$this->assertSame( 'from_load_hook', $options['from_load_hook'] );
$this->assertSame( 'from_load_hook', $originalOptions['from_load_hook'] );
$options['from_save_hook'] = 'from_save_hook';
}
return true;
}
);
$manager->saveOptions( $user );
$this->assertSame( 'from_load_hook', $manager->getOption( $user, 'from_load_hook' ) );
$this->assertSame( 'from_save_hook', $manager->getOption( $user, 'from_save_hook' ) );
}
/**
* @covers \MediaWiki\User\UserOptionsManager::loadUserOptions
*/
public function testInfiniteRecursionOnUserLoadOptionsHook() {
$this->filterDeprecated( '/UserLoadOptions/' );
$user = $this->getTestUser()->getUser();
$manager = $this->getManager();
$recursionCounter = 0;
$this->setTemporaryHook(
'UserLoadOptions',
function ( User $hookUser ) use ( $user, $manager, &$recursionCounter ) {
if ( $hookUser->equals( $user ) ) {
$recursionCounter += 1;
$this->assertSame( 1, $recursionCounter );
$manager->loadUserOptions( $hookUser );
}
}
);
$manager->loadUserOptions( $user, UserOptionsManager::READ_LATEST );
$this->assertSame( 1, $recursionCounter );
}
/**
* @covers \MediaWiki\User\UserOptionsManager::loadUserOptions
*/