Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
wiki.techinc.nl/tests/phpunit/includes/user/UserOptionsManagerTest.php

331 lines
12 KiB
PHP
Raw Normal View History

Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
<?php
use MediaWiki\Config\ServiceOptions;
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
use MediaWiki\User\UserIdentity;
use MediaWiki\User\UserIdentityValue;
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
use MediaWiki\User\UserOptionsLookup;
use MediaWiki\User\UserOptionsManager;
use Psr\Log\NullLogger;
rdbms: Add more return typehints See full rationale at I59068cfed10aabf6c6002f9e9312a6ef6e7e9441. Using IDatabase for now instead of DBConnRef for better BC. Change-Id: Ie75aaf46ba91779e8706b10efeefa9580857f489
2021-08-31 17:38:46 +00:00
use Wikimedia\Rdbms\DBConnRef;
Improvements to user preferences fetching/saving == Status quo == When saving user preferences, we want to lock the rows to avoid accidentally overriding a concurrent options update. So usually what extensions do is: $value = $mngr->getOption( 'option', ..., READ_LOCKING ); if ( $value !== 'new_value' ) { $mngr->setOption( 'option', 'new_value' ); $mngr->saveOptions() } Previously for extra caution we've ignored all caches in options manager if >= READ_LOCKING flags were passed. This resulted in re-reading all the options multiple times. At worst, 3 times: 1. If READ_NORMAL read was made for update - that's once, 2. On setOption, one more read, forcefully from primary 3. On saveOptions, one more read from primary, non-locking, to figure out which option keys need to be deleted. Also, READ_LOCKING was not used where it clearly had to be used, for example right before the update. This was trying to fix any kind of error on part of the manager clients, unsuccessfully so. == New approach == 1. Cache modified user options separately from originals and merge them on demand. This means when refetching originals with LOCKING we don't wipe out all modifications made to the cache with setOption. Extra bonus - we no longer need to load all options to set an option. 2. Split the originals cache into 2 layers - one for stuff that comes from DB directly, and one with applied normalizations and whatever hooks modify. This let's us avoid refetching DB options after we save them, but still let's the hooks execute on newly set options after they're saved. 3. Cache options with all query flags. This is a bit controversial, but ideally LOCKING flags will be applied on options fetch right before they are saved. We have to re-read options with LOCKING in saveOptions to avoid races, but if the caller did 'getOption( ..., LOCKING), setOption(), save()' we will not need to re-select with LOCKING again. Bug: T280220 Change-Id: Ibed2789f5260b725fd806b4470631aa30d814ce6
2021-06-11 18:48:19 +00:00
use Wikimedia\Rdbms\ILoadBalancer;
Reapply "Update user_touched after saving user options." This reverts commit 4d19d06455874730fe8020f071b2bcbe2f12b5a9. Reason for revert: CenralAuth tests fixed Bug: T284354 Depends-On: I9d681baeca0df4808335e7bececfd114cdad2f0e Change-Id: I483b8f61dcfae70c5a50399391b361cf5310ae24
2021-10-25 19:56:47 +00:00
use Wikimedia\Timestamp\ConvertibleTimestamp;
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
/**
* @group Database
* @covers MediaWiki\User\UserOptionsManager
*/
class UserOptionsManagerTest extends UserOptionsLookupTest {
Reapply "Update user_touched after saving user options." This reverts commit 4d19d06455874730fe8020f071b2bcbe2f12b5a9. Reason for revert: CenralAuth tests fixed Bug: T284354 Depends-On: I9d681baeca0df4808335e7bececfd114cdad2f0e Change-Id: I483b8f61dcfae70c5a50399391b361cf5310ae24
2021-10-25 19:56:47 +00:00
protected function setUp(): void {
parent::setUp();
$this->tablesUsed[] = 'user';
$this->tablesUsed[] = 'user_properties';
}
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
/**
* @param array $overrides supported keys:
* - 'language' - string language code
* - 'defaults' - array default preferences
* - 'lb' - ILoadBalancer
* - 'hookContainer' - HookContainer
* @return UserOptionsManager
*/
private function getManager( array $overrides = [] ) {
phpunit: use ->getServiceContainer() in integration tests Change-Id: I38299cb65eeaadfdc0eb05db4e8c0b0119cfb37d
2022-01-12 20:13:39 +00:00
$services = $this->getServiceContainer();
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
return new UserOptionsManager(
new ServiceOptions(
UserOptionsManager::CONSTRUCTOR_OPTIONS,
Split TimeCorrection parser into separate class The parsing of the timecorrection useroption was split over multiple classes. Combine into a single class and add some testcases. Change-Id: I2cadac00e46dff2bc7d81ac2f294ea2ae4e72f47
2021-04-15 20:54:58 +00:00
new HashConfig( [
'HiddenPrefs' => [ 'hidden_user_option' ],
'LocalTZoffset' => 0,
] )
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
),
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
$this->getDefaultManager(
$overrides['language'] ?? 'qqq',
$overrides['defaults'] ?? []
),
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
$services->getLanguageConverterFactory(),
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
$overrides['lb'] ?? $services->getDBLoadBalancer(),
Hooks::run() call site migration Migrate all callers of Hooks::run() to use the new HookContainer/HookRunner system. General principles: * Use DI if it is already used. We're not changing the way state is managed in this patch. * HookContainer is always injected, not HookRunner. HookContainer is a service, it's a more generic interface, it is the only thing that provides isRegistered() which is needed in some cases, and a HookRunner can be efficiently constructed from it (confirmed by benchmark). Because HookContainer is needed for object construction, it is also needed by all factories. * "Ask your friendly local base class". Big hierarchies like SpecialPage and ApiBase have getHookContainer() and getHookRunner() methods in the base class, and classes that extend that base class are not expected to know or care where the base class gets its HookContainer from. * ProtectedHookAccessorTrait provides protected getHookContainer() and getHookRunner() methods, getting them from the global service container. The point of this is to ease migration to DI by ensuring that call sites ask their local friendly base class rather than getting a HookRunner from the service container directly. * Private $this->hookRunner. In some smaller classes where accessor methods did not seem warranted, there is a private HookRunner property which is accessed directly. Very rarely (two cases), there is a protected property, for consistency with code that conventionally assumes protected=private, but in cases where the class might actually be overridden, a protected accessor is preferred over a protected property. * The last resort: Hooks::runner(). Mostly for static, file-scope and global code. In a few cases it was used for objects with broken construction schemes, out of horror or laziness. Constructors with new required arguments: * AuthManager * BadFileLookup * BlockManager * ClassicInterwikiLookup * ContentHandlerFactory * ContentSecurityPolicy * DefaultOptionsManager * DerivedPageDataUpdater * FullSearchResultWidget * HtmlCacheUpdater * LanguageFactory * LanguageNameUtils * LinkRenderer * LinkRendererFactory * LocalisationCache * MagicWordFactory * MessageCache * NamespaceInfo * PageEditStash * PageHandlerFactory * PageUpdater * ParserFactory * PermissionManager * RevisionStore * RevisionStoreFactory * SearchEngineConfig * SearchEngineFactory * SearchFormWidget * SearchNearMatcher * SessionBackend * SpecialPageFactory * UserNameUtils * UserOptionsManager * WatchedItemQueryService * WatchedItemStore Constructors with new optional arguments: * DefaultPreferencesFactory * Language * LinkHolderArray * MovePage * Parser * ParserCache * PasswordReset * Router setHookContainer() now required after construction: * AuthenticationProvider * ResourceLoaderModule * SearchEngine Change-Id: Id442b0dbe43aba84bd5cf801d86dedc768b082c7
2020-03-19 02:42:09 +00:00
new NullLogger(),
Reapply "Update user_touched after saving user options." This reverts commit 4d19d06455874730fe8020f071b2bcbe2f12b5a9. Reason for revert: CenralAuth tests fixed Bug: T284354 Depends-On: I9d681baeca0df4808335e7bececfd114cdad2f0e Change-Id: I483b8f61dcfae70c5a50399391b361cf5310ae24
2021-10-25 19:56:47 +00:00
$overrides['hookContainer'] ?? $services->getHookContainer(),
$services->getUserFactory()
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
);
}
protected function getLookup(
string $langCode = 'qqq',
array $defaultOptionsOverrides = []
build: Updating dependencies composer: * mediawiki/mediawiki-codesniffer: 36.0.0 → 37.0.0 The following sniffs now pass and were enabled: * Generic.ControlStructures.InlineControlStructure * MediaWiki.PHPUnit.AssertCount.NotUsed npm: * svgo: 2.3.0 → 2.3.1 * https://npmjs.com/advisories/1754 (CVE-2021-33587) Change-Id: I2a9bbee2fecbf7259876d335f565ece4b3622426
2021-07-22 03:11:47 +00:00
): UserOptionsLookup {
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
return $this->getManager( [
'language' => $langCode,
'defaults' => $defaultOptionsOverrides,
] );
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
}
/**
* @covers MediaWiki\User\UserOptionsManager::getOption
*/
public function testGetOptionsExcludeDefaults() {
Remove more defaults for flag UserOptionsManager::EXCLUDE_DEFAULTS Use the isValueEqual function from I572446faa8d40801a9adc3aee4b26d97c18000a1 to remove more bool values, if the default options and the user option differs in real type. Bug: T291748 Change-Id: I6e61c11d8aed27b4b559e74849e0056e5eef3638
2021-09-29 00:10:56 +00:00
$manager = $this->getManager( [ 'defaults' => [
'null_vs_false' => null,
'null_vs_string' => null,
'false_vs_int' => false,
'false_vs_string' => false,
'int_vs_string' => 0,
'true_vs_int' => true,
'true_vs_string' => true,
] ] );
$user = $this->getAnon( __METHOD__ );
$manager->setOption( $user, 'null_vs_false', false );
$manager->setOption( $user, 'null_vs_string', '' );
$manager->setOption( $user, 'false_vs_int', 0 );
$manager->setOption( $user, 'false_vs_string', '0' );
$manager->setOption( $user, 'int_vs_string', '0' );
$manager->setOption( $user, 'true_vs_int', 1 );
$manager->setOption( $user, 'true_vs_string', '1' );
$manager->setOption( $user, 'new_option', 'new_value' );
$expected = [
// Note that the old, relaxed array_diff-approach considered null equal to false and ""
'null_vs_false' => false,
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
'language' => 'en',
'variant' => 'en',
Remove more defaults for flag UserOptionsManager::EXCLUDE_DEFAULTS Use the isValueEqual function from I572446faa8d40801a9adc3aee4b26d97c18000a1 to remove more bool values, if the default options and the user option differs in real type. Bug: T291748 Change-Id: I6e61c11d8aed27b4b559e74849e0056e5eef3638
2021-09-29 00:10:56 +00:00
'new_option' => 'new_value',
];
$this->assertSame( $expected, $manager->getOptions( $user, UserOptionsManager::EXCLUDE_DEFAULTS ) );
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
}
/**
* @covers MediaWiki\User\UserOptionsManager::getOption
*/
public function testGetOptionHiddenPref() {
$user = $this->getAnon( __METHOD__ );
$manager = $this->getManager();
$manager->setOption( $user, 'hidden_user_option', 'hidden_value' );
$this->assertNull( $manager->getOption( $user, 'hidden_user_option' ) );
$this->assertSame( 'hidden_value',
$manager->getOption( $user, 'hidden_user_option', null, true ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::setOption
*/
public function testSetOptionNullIsDefault() {
$user = $this->getAnon( __METHOD__ );
$manager = $this->getManager();
$manager->setOption( $user, 'default_string_option', 'override_value' );
$this->assertSame( 'override_value', $manager->getOption( $user, 'default_string_option' ) );
$manager->setOption( $user, 'default_string_option', null );
$this->assertSame( 'string_value', $manager->getOption( $user, 'default_string_option' ) );
}
/**
* @covers MediaWiki\User\UserOptionsManager::getOption
* @covers MediaWiki\User\UserOptionsManager::setOption
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
public function testGetSetSave() {
$user = $this->getTestUser()->getUser();
$manager = $this->getManager();
$this->assertSame( [], $manager->getOptions( $user, UserOptionsManager::EXCLUDE_DEFAULTS ) );
$manager->setOption( $user, 'string_option', 'user_value' );
$manager->setOption( $user, 'int_option', 42 );
$manager->setOption( $user, 'bool_option', true );
$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->saveOptions( $user );
UserOptions: remove deprecated hooks. 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
2021-09-23 00:37:21 +00:00
$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' ) );
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
$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' ) );
}
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
/**
* @covers MediaWiki\User\UserOptionsManager::loadUserOptions
*/
public function testLoadUserOptionsHook() {
$user = UserIdentityValue::newRegistered( 42, 'Test' );
$manager = $this->getManager( [
'hookContainer' => $this->createHookContainer( [
'LoadUserOptions' => function ( UserIdentity $hookUser, array &$options ) use ( $user ) {
$this->assertTrue( $hookUser->equals( $user ) );
$options['from_hook'] = 'value_from_hook';
}
] )
] );
$this->assertSame( 'value_from_hook', $manager->getOption( $user, 'from_hook' ) );
}
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
public function testSaveUserOptionsHookAbort() {
$manager = $this->getManager( [
'hookContainer' => $this->createHookContainer( [
'SaveUserOptions' => static function () {
return false;
}
] )
] );
$user = UserIdentityValue::newRegistered( 42, 'Test' );
$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
*/
public function testSaveUserOptionsHookModify() {
$user = UserIdentityValue::newRegistered( 42, 'Test' );
$manager = $this->getManager( [
'defaults' => [
'reset_to_default_by_hook' => 'default',
],
'hookContainer' => $this->createHookContainer( [
'SaveUserOptions' => function ( UserIdentity $hookUser, array &$modifiedOptions ) use ( $user ) {
$this->assertTrue( $user->equals( $hookUser ) );
$modifiedOptions['reset_to_default_by_hook'] = null;
unset( $modifiedOptions['blocked_by_hook'] );
$modifiedOptions['new_from_hook'] = 'value_from_hook';
}
] ),
] );
$manager->setOption( $user, 'reset_to_default_by_hook', 'not default' );
$manager->setOption( $user, 'blocked_by_hook', 'blocked value' );
$manager->saveOptions( $user );
$this->assertSame( 'value_from_hook', $manager->getOption( $user, 'new_from_hook' ) );
$this->assertSame( 'default', $manager->getOption( $user, 'reset_to_default_by_hook' ) );
$this->assertNull( $manager->getOption( $user, 'blocked_by_hook' ) );
$manager->clearUserOptionsCache( $user );
$this->assertSame( 'value_from_hook', $manager->getOption( $user, 'new_from_hook' ) );
$this->assertSame( 'default', $manager->getOption( $user, 'reset_to_default_by_hook' ) );
$this->assertNull( $manager->getOption( $user, 'blocked_by_hook' ) );
}
Adjust method signature of onSaveUserOptions This allows hook callers to compare before and after the save Bug: T287397 Depends-On: I20098ae076b282296670d1116e14bbd29ea76b11 Change-Id: I4d09008bc2bc10afc3742b74564e5ef90ecfe5bf
2021-07-26 17:25:12 +00:00
/**
* @covers MediaWiki\User\UserOptionsManager::saveOptions
*/
public function testSaveUserOptionsHookOriginal() {
$user = UserIdentityValue::newRegistered( 42, 'Test' );
$manager = $this->getManager( [
'language' => 'ja',
'hookContainer' => $this->createHookContainer( [
'SaveUserOptions' => function (
UserIdentity $hookUser,
array &$modifiedOptions,
array $originalOptions
) use ( $user ) {
if ( $hookUser->equals( $user ) ) {
$this->assertSame( 'ja', $originalOptions['language'] );
$this->assertSame( 'ru', $modifiedOptions['language'] );
$modifiedOptions['language'] = 'tr';
}
return true;
}
] ),
] );
$manager->setOption( $user, 'language', 'ru' );
$manager->saveOptions( $user );
$this->assertSame( 'tr', $manager->getOption( $user, 'language' ) );
}
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
/**
* @covers \MediaWiki\User\UserOptionsManager::loadUserOptions
*/
public function testInfiniteRecursionOnLoadUserOptionsHook() {
$user = UserIdentityValue::newRegistered( 42, 'Test' );
$manager = $this->getManager( [
'hookContainer' => $this->createHookContainer( [
'LoadUserOptions' => function ( UserIdentity $hookUser ) use ( $user, &$manager, &$recursionCounter ) {
if ( $hookUser->equals( $user ) ) {
$recursionCounter += 1;
$this->assertSame( 1, $recursionCounter );
$manager->loadUserOptions( $hookUser );
}
}
] )
] );
$recursionCounter = 0;
$manager->loadUserOptions( $user, UserOptionsManager::READ_LATEST );
$this->assertSame( 1, $recursionCounter );
}
UserOptionsManager: take into account $queryFlags when caching - Don't care about $queryFlags for anons since nothing can be stored in the database for anons. - For loading the options - discard the old cache in case higher query flags are used. This means that 'setOption' has to by default reload the options to ensure changing the options start from LATEST. This codepath shouldn't be executed in reality cause we should be already loading the user with READ_LATEST if we want to update the options. Where that was not done - it was probably a bug. Also, expose optional $queryFlags parameter for UserOptionsLookup methods. Otherwise there's no way to read from master using public API. Bug: T248527 Change-Id: Id7b9868ecdfba89bfafd4618365fe520ec59fcfe
2020-05-28 18:40:49 +00:00
public function testSaveOptionsForAnonUser() {
$this->expectException( InvalidArgumentException::class );
$this->getManager()->saveOptions( $this->getAnon( __METHOD__ ) );
}
UserOptionsManager: fix options reset. This is going to fix the bug, but it's not going far enough, READ_LATEST needs to be bumped to READ_EXCLUSIVE. The problem is that the options mananger cache serves dual purpose: on option lookup it's a cache, while it also holds the modifications made to options before saving. We need to require the options fetched with READ_EXCLUSIVE before we are going to save them, and not discard the options cache if they were already read with READ_EXCLUSIVE. Relying on the callers to use correct query flags seems very prone to errors. Before this was handled with User::getInstanceForUpdate. I wonder if we should establish a similar pattern and remove the query flags from the individual methods paramaters, but establish that UserOptionsLookup service, responsible for reading-only, will use replica DB, while UserOptionsManager will use master and do locking reads. The usage pattern would then be - if you only need to read the options, use lookup. If you have an intention of modifying the options - grab and instance of the manager, and go into master by default. Thoughts? Bug: T255842 Change-Id: I399ab0da8880320fd9d5f725ead8a62026cd7b7d
2020-06-24 22:33:19 +00:00
/**
* @covers \MediaWiki\User\UserOptionsManager::resetOptions
*/
public function testUserOptionsSaveAfterReset() {
$user = $this->getTestUser()->getUser();
$manager = $this->getManager();
$manager->setOption( $user, 'test_option', 'test_value' );
$manager->saveOptions( $user );
$manager->clearUserOptionsCache( $user );
$this->assertSame( 'test_value', $manager->getOption( $user, 'test_option' ) );
$manager->resetOptions( $user, RequestContext::getMain(), 'all' );
$this->assertNull( $manager->getOption( $user, 'test_option' ) );
$manager->saveOptions( $user );
$manager->clearUserOptionsCache( $user );
$this->assertNull( $manager->getOption( $user, 'test_option' ) );
}
Improvements to user preferences fetching/saving == Status quo == When saving user preferences, we want to lock the rows to avoid accidentally overriding a concurrent options update. So usually what extensions do is: $value = $mngr->getOption( 'option', ..., READ_LOCKING ); if ( $value !== 'new_value' ) { $mngr->setOption( 'option', 'new_value' ); $mngr->saveOptions() } Previously for extra caution we've ignored all caches in options manager if >= READ_LOCKING flags were passed. This resulted in re-reading all the options multiple times. At worst, 3 times: 1. If READ_NORMAL read was made for update - that's once, 2. On setOption, one more read, forcefully from primary 3. On saveOptions, one more read from primary, non-locking, to figure out which option keys need to be deleted. Also, READ_LOCKING was not used where it clearly had to be used, for example right before the update. This was trying to fix any kind of error on part of the manager clients, unsuccessfully so. == New approach == 1. Cache modified user options separately from originals and merge them on demand. This means when refetching originals with LOCKING we don't wipe out all modifications made to the cache with setOption. Extra bonus - we no longer need to load all options to set an option. 2. Split the originals cache into 2 layers - one for stuff that comes from DB directly, and one with applied normalizations and whatever hooks modify. This let's us avoid refetching DB options after we save them, but still let's the hooks execute on newly set options after they're saved. 3. Cache options with all query flags. This is a bit controversial, but ideally LOCKING flags will be applied on options fetch right before they are saved. We have to re-read options with LOCKING in saveOptions to avoid races, but if the caller did 'getOption( ..., LOCKING), setOption(), save()' we will not need to re-select with LOCKING again. Bug: T280220 Change-Id: Ibed2789f5260b725fd806b4470631aa30d814ce6
2021-06-11 18:48:19 +00:00
public function testOptionsForUpdateNotRefetchedBeforeInsert() {
rdbms: Add more return typehints See full rationale at I59068cfed10aabf6c6002f9e9312a6ef6e7e9441. Using IDatabase for now instead of DBConnRef for better BC. Change-Id: Ie75aaf46ba91779e8706b10efeefa9580857f489
2021-08-31 17:38:46 +00:00
$mockDb = $this->createMock( DBConnRef::class );
Improvements to user preferences fetching/saving == Status quo == When saving user preferences, we want to lock the rows to avoid accidentally overriding a concurrent options update. So usually what extensions do is: $value = $mngr->getOption( 'option', ..., READ_LOCKING ); if ( $value !== 'new_value' ) { $mngr->setOption( 'option', 'new_value' ); $mngr->saveOptions() } Previously for extra caution we've ignored all caches in options manager if >= READ_LOCKING flags were passed. This resulted in re-reading all the options multiple times. At worst, 3 times: 1. If READ_NORMAL read was made for update - that's once, 2. On setOption, one more read, forcefully from primary 3. On saveOptions, one more read from primary, non-locking, to figure out which option keys need to be deleted. Also, READ_LOCKING was not used where it clearly had to be used, for example right before the update. This was trying to fix any kind of error on part of the manager clients, unsuccessfully so. == New approach == 1. Cache modified user options separately from originals and merge them on demand. This means when refetching originals with LOCKING we don't wipe out all modifications made to the cache with setOption. Extra bonus - we no longer need to load all options to set an option. 2. Split the originals cache into 2 layers - one for stuff that comes from DB directly, and one with applied normalizations and whatever hooks modify. This let's us avoid refetching DB options after we save them, but still let's the hooks execute on newly set options after they're saved. 3. Cache options with all query flags. This is a bit controversial, but ideally LOCKING flags will be applied on options fetch right before they are saved. We have to re-read options with LOCKING in saveOptions to avoid races, but if the caller did 'getOption( ..., LOCKING), setOption(), save()' we will not need to re-select with LOCKING again. Bug: T280220 Change-Id: Ibed2789f5260b725fd806b4470631aa30d814ce6
2021-06-11 18:48:19 +00:00
$mockDb->expects( $this->once() ) // This is critical what we are testing
->method( 'select' )
->willReturn( new FakeResultWrapper( [
[
'up_value' => 'blabla',
'up_property' => 'test_option',
]
] ) );
$mockLoadBalancer = $this->createMock( ILoadBalancer::class );
$mockLoadBalancer
->method( 'getConnectionRef' )
->willReturn( $mockDb );
$user = $this->getTestUser()->getUser();
Introduce new hooks for UserOptionsManager Bug: T286576 Change-Id: Ib960ec594d376e086da868d216dd85c8ea6bba14
2021-07-13 21:05:10 +00:00
$manager = $this->getManager( [
'lb' => $mockLoadBalancer,
] );
Improvements to user preferences fetching/saving == Status quo == When saving user preferences, we want to lock the rows to avoid accidentally overriding a concurrent options update. So usually what extensions do is: $value = $mngr->getOption( 'option', ..., READ_LOCKING ); if ( $value !== 'new_value' ) { $mngr->setOption( 'option', 'new_value' ); $mngr->saveOptions() } Previously for extra caution we've ignored all caches in options manager if >= READ_LOCKING flags were passed. This resulted in re-reading all the options multiple times. At worst, 3 times: 1. If READ_NORMAL read was made for update - that's once, 2. On setOption, one more read, forcefully from primary 3. On saveOptions, one more read from primary, non-locking, to figure out which option keys need to be deleted. Also, READ_LOCKING was not used where it clearly had to be used, for example right before the update. This was trying to fix any kind of error on part of the manager clients, unsuccessfully so. == New approach == 1. Cache modified user options separately from originals and merge them on demand. This means when refetching originals with LOCKING we don't wipe out all modifications made to the cache with setOption. Extra bonus - we no longer need to load all options to set an option. 2. Split the originals cache into 2 layers - one for stuff that comes from DB directly, and one with applied normalizations and whatever hooks modify. This let's us avoid refetching DB options after we save them, but still let's the hooks execute on newly set options after they're saved. 3. Cache options with all query flags. This is a bit controversial, but ideally LOCKING flags will be applied on options fetch right before they are saved. We have to re-read options with LOCKING in saveOptions to avoid races, but if the caller did 'getOption( ..., LOCKING), setOption(), save()' we will not need to re-select with LOCKING again. Bug: T280220 Change-Id: Ibed2789f5260b725fd806b4470631aa30d814ce6
2021-06-11 18:48:19 +00:00
$manager->getOption(
$user,
'test_option',
null,
false,
UserOptionsManager::READ_LOCKING
);
$manager->getOption( $user, 'test_option2' );
$manager->setOption( $user, 'test_option', 'test_value' );
$manager->setOption( $user, 'test_option2', 'test_value2' );
$manager->saveOptions( $user );
}
Reapply "Update user_touched after saving user options." This reverts commit 4d19d06455874730fe8020f071b2bcbe2f12b5a9. Reason for revert: CenralAuth tests fixed Bug: T284354 Depends-On: I9d681baeca0df4808335e7bececfd114cdad2f0e Change-Id: I483b8f61dcfae70c5a50399391b361cf5310ae24
2021-10-25 19:56:47 +00:00
/**
* @covers \MediaWiki\User\UserOptionsManager::saveOptions
*/
public function testUpdatesUserTouched() {
$user = $this->getTestUser()->getUser();
$userTouched = $user->getDBTouched();
$newTouched = ConvertibleTimestamp::convert(
TS_MW,
intval( ConvertibleTimestamp::convert( TS_UNIX, $userTouched ) ) + 100
);
ConvertibleTimestamp::setFakeTime( $newTouched );
$manager = $this->getManager();
$manager->setOption( $user, 'test_option', 'test_value' );
$manager->saveOptions( $user );
$this->assertSame( $newTouched, $user->getDBTouched() );
$user->clearInstanceCache();
$this->assertSame( $newTouched, $user->getDBTouched() );
}
Introduce UserOptionsManager and DefaultOptionsManager This converts user options management to a separate service for use in DI context. User options are accessed quite early on in installation process and full-on options management depends on the database. Prior we have protected from accessing the DB by setting a hacky $wgUser with 0 id, and relying on the implementation that it doesn't go into the database to get the default user options. Now we can't really do that since DBLoadBalancer is required to instantiate the options manager. Instead, we redefine the options manager with a DefaultOptionsManager, that only provides access to default options and doesn't require DB access. UserOptionsManager uses PreferencesFactory, however injecting it will produce a cyclic dependency. The problem is that we separate options to different kinds, which are inferred from the PreferencesFactory declaration for those options (e.g. if it's a radio button in the UI declaration, the option is of multiselect kind). This is plain wrong, the dependency should be wise versa. This will be addressed separately, since it's requires larger refactoring. For now the PreferencesFactory is obtained on demand. This will be addressed in a followup. Bug: T248527 Change-Id: I74917c5eaec184d188911a319895b941ed55ee87
2020-01-17 06:21:28 +00:00
}
Reference in a new issue Copy permalink