[MediaWiki-commits] [Gerrit] User::saveOptions() optimization - change (mediawiki/core)
Hashar has uploaded a new change for review.
https://gerrit.wikimedia.org/r/110516
Change subject: User::saveOptions() optimization
..
User::saveOptions() optimization
Since we only want to save non default user options, we have to strip
out any user option that match the default ones. We did that by calling
User::getDefaultOption( 'some option name' ); on each of the option.
Since the User mOptions property is a merge of the default option, we
end up doing a lot of unneeded processing. The loop roughly looks like:
User::getDefaultOption()
User::getDefaultOptions()
Language-getCode()
SearchEngine::searchableNamespaces()
language-getNamespaces()
wfRunHooks('SearcheableNamespaces')
wfRunHooks('UserGetDefaultOptions')
For EACH of the mOptions.
Instead this patch does an array_diff to strip out from mObjects any
default option. We still skip options whose value is false or null.
Test provided to make sure we only save what we want.
Change-Id: Icd7c416894f639de8d9e8cd63a99ff2b564eaf3e
---
M includes/User.php
M tests/phpunit/includes/UserTest.php
2 files changed, 56 insertions(+), 11 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/16/110516/1
diff --git a/includes/User.php b/includes/User.php
index ca3f79b..7f7a22d 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -4629,20 +4629,20 @@
return;
}
+ $defaultOptions = self::getDefaultOptions();
+
$userId = $this-getId();
$insert_rows = array();
- foreach ( $saveOptions as $key = $value ) {
- // Don't bother storing default values
- $defaultOption = self::getDefaultOption( $key );
- if ( ( is_null( $defaultOption )
- !( $value === false || is_null( $value
) ) ) ||
- $value != $defaultOption ) {
- $insert_rows[] = array(
- 'up_user' = $userId,
- 'up_property' = $key,
- 'up_value' = $value,
- );
+ $changedOptions = array_diff_assoc( $saveOptions,
$defaultOptions );
+ foreach ( $changedOptions as $key = $value ) {
+ if ( $value === false || is_null( $value ) ) {
+ continue;
}
+ $insert_rows[] = array(
+ 'up_user' = $userId,
+ 'up_property' = $key,
+ 'up_value' = $value,
+ );
}
$dbw = wfGetDB( DB_MASTER );
diff --git a/tests/phpunit/includes/UserTest.php
b/tests/phpunit/includes/UserTest.php
index ff33e82..b7df3f6 100644
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -234,4 +234,49 @@
$this-assertEquals( $wgDefaultUserOptions['cols'],
$this-user-getOption( 'cols' ) );
$this-assertEquals( 'test', $this-user-getOption(
'someoption' ) );
}
+
+ /**
+* Helper, fetch user properties from the database.
+* @param int $userId
+*/
+ function dbUserProperties( $userId ) {
+ $res = wfGetDB( DB_SLAVE )-select(
+ 'user_properties',
+ array( 'up_property', 'up_value' ),
+ array( 'up_user' = $userId ),
+ __METHOD__
+ );
+ $ret = array();
+ foreach( $res as $row ) {
+ $ret[$row-up_property] = $row-up_value;
+ }
+ return $ret;
+ }
+
+ public function testOnlySaveChangedOptions() {
+ $user = User::newFromName( 'UnitTestUser2' );
+ $user-addToDatabase();
+
+ // Fresh user only has default, so nothing should be in the DB
+ $dbProps = $this-dbUserProperties( $user-getId() );
+ $this-assertEmpty( $dbProps,
+ A new user should not have any user property saved in
the DB );
+
+ // Make sure we only save the altered option
+ $user-setOption( 'changed_opt', 'alix_20281' );
+ $user-setOption( 'switch', 1 );
+ $user-setOption( 'anotherswitch', 1 );
+ $user-saveSettings();
+
+ $expected = array (
+ 'changed_opt' = 'alix_20281',
+ 'switch' = '1',
+ 'anotherswitch' = '1',
+ );
+ $dbProps = $this-dbUserProperties( $user-getId() );
+
+
[MediaWiki-commits] [Gerrit] User::saveOptions() optimization - change (mediawiki/core)
jenkins-bot has submitted this change and it was merged.
Change subject: User::saveOptions() optimization
..
User::saveOptions() optimization
Since we only want to save non default user options, we have to strip
out any user option that match the default ones. We did that by calling
User::getDefaultOption( 'some option name' ); on each of the option.
Since the User mOptions property is a merge of the default option, we
end up doing a lot of unneeded processing. The loop roughly looks like:
User::getDefaultOption()
User::getDefaultOptions()
Language-getCode()
SearchEngine::searchableNamespaces()
language-getNamespaces()
wfRunHooks('SearcheableNamespaces')
wfRunHooks('UserGetDefaultOptions')
For EACH of the mOptions.
Instead this patch does an array_diff to strip out from mObjects any
default option. We still skip options whose value is false or null.
Test provided to make sure we only save what we want.
Change-Id: Ie98d3a17edab74401ed32f759ba11f723b56e376
---
M includes/User.php
M tests/phpunit/includes/UserTest.php
2 files changed, 56 insertions(+), 11 deletions(-)
Approvals:
Parent5446: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/User.php b/includes/User.php
index ca3f79b..7f7a22d 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -4629,20 +4629,20 @@
return;
}
+ $defaultOptions = self::getDefaultOptions();
+
$userId = $this-getId();
$insert_rows = array();
- foreach ( $saveOptions as $key = $value ) {
- // Don't bother storing default values
- $defaultOption = self::getDefaultOption( $key );
- if ( ( is_null( $defaultOption )
- !( $value === false || is_null( $value
) ) ) ||
- $value != $defaultOption ) {
- $insert_rows[] = array(
- 'up_user' = $userId,
- 'up_property' = $key,
- 'up_value' = $value,
- );
+ $changedOptions = array_diff_assoc( $saveOptions,
$defaultOptions );
+ foreach ( $changedOptions as $key = $value ) {
+ if ( $value === false || is_null( $value ) ) {
+ continue;
}
+ $insert_rows[] = array(
+ 'up_user' = $userId,
+ 'up_property' = $key,
+ 'up_value' = $value,
+ );
}
$dbw = wfGetDB( DB_MASTER );
diff --git a/tests/phpunit/includes/UserTest.php
b/tests/phpunit/includes/UserTest.php
index ff33e82..b7df3f6 100644
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -234,4 +234,49 @@
$this-assertEquals( $wgDefaultUserOptions['cols'],
$this-user-getOption( 'cols' ) );
$this-assertEquals( 'test', $this-user-getOption(
'someoption' ) );
}
+
+ /**
+* Helper, fetch user properties from the database.
+* @param int $userId
+*/
+ function dbUserProperties( $userId ) {
+ $res = wfGetDB( DB_SLAVE )-select(
+ 'user_properties',
+ array( 'up_property', 'up_value' ),
+ array( 'up_user' = $userId ),
+ __METHOD__
+ );
+ $ret = array();
+ foreach( $res as $row ) {
+ $ret[$row-up_property] = $row-up_value;
+ }
+ return $ret;
+ }
+
+ public function testOnlySaveChangedOptions() {
+ $user = User::newFromName( 'UnitTestUser2' );
+ $user-addToDatabase();
+
+ // Fresh user only has default, so nothing should be in the DB
+ $dbProps = $this-dbUserProperties( $user-getId() );
+ $this-assertEmpty( $dbProps,
+ A new user should not have any user property saved in
the DB );
+
+ // Make sure we only save the altered option
+ $user-setOption( 'changed_opt', 'alix_20281' );
+ $user-setOption( 'switch', 1 );
+ $user-setOption( 'anotherswitch', 1 );
+ $user-saveSettings();
+
+ $expected = array (
+ 'changed_opt' = 'alix_20281',
+ 'switch' = '1',
+ 'anotherswitch' = '1',
+ );
+ $dbProps = $this-dbUserProperties( $user-getId() );
+
+ $this-assertEquals( $expected,
[MediaWiki-commits] [Gerrit] User::saveOptions() optimization - change (mediawiki/core)
Hashar has uploaded a new change for review.
https://gerrit.wikimedia.org/r/64941
Change subject: User::saveOptions() optimization
..
User::saveOptions() optimization
Since we only want to save non default user options, we have to strip
out any user option that match the default ones. We did that by calling
User::getDefaultOption( 'some option name' ); on each of the option.
Since the User mOptions property is a merge of the default option, we
end up doing a lot of unneeded processing. The loop roughly looks like:
User::getDefaultOption()
User::getDefaultOptions()
Language-getCode()
SearchEngine::searchableNamespaces()
language-getNamespaces()
wfRunHooks('SearcheableNamespaces')
wfRunHooks('UserGetDefaultOptions')
For EACH of the mOptions.
Instead this patch does an array_diff to strip out from mObjects any
default option. We still skip options whose value is false or null.
Test provided to make sure we only save what we want.
Change-Id: Ie98d3a17edab74401ed32f759ba11f723b56e376
---
M includes/User.php
M tests/phpunit/includes/UserTest.php
2 files changed, 54 insertions(+), 11 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/41/64941/1
diff --git a/includes/User.php b/includes/User.php
index e2cbb81..52b58f4 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -4455,20 +4455,22 @@
return;
}
+ $defaultOptions = self::getDefaultOptions();
+
$userId = $this-getId();
$insert_rows = array();
- foreach ( $saveOptions as $key = $value ) {
- # Don't bother storing default values
- $defaultOption = self::getDefaultOption( $key );
- if ( ( is_null( $defaultOption )
- !( $value === false || is_null( $value
) ) ) ||
- $value != $defaultOption ) {
- $insert_rows[] = array(
- 'up_user' = $userId,
- 'up_property' = $key,
- 'up_value' = $value,
- );
+
+ # Only bother storing values that changed
+ $changedOptions = array_diff( $saveOptions, $defaultOptions );
+ foreach( $changedOptions as $key = $value ) {
+ if( $value === false || is_null($value) ) {
+ continue;
}
+ $insert_rows[] = array(
+ 'up_user' = $userId,
+ 'up_property' = $key,
+ 'up_value' = $value,
+ );
}
$dbw = wfGetDB( DB_MASTER );
diff --git a/tests/phpunit/includes/UserTest.php
b/tests/phpunit/includes/UserTest.php
index e777179..e22b339 100644
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -214,4 +214,45 @@
$this-assertEquals( $wgDefaultUserOptions['cols'],
$this-user-getOption( 'cols' ) );
$this-assertEquals( 'test', $this-user-getOption(
'someoption' ) );
}
+
+ /**
+* Helper, fetch user properties from the database.
+* @param int $userId
+*/
+ function dbUserProperties( $userId ) {
+ $res = wfGetDB(DB_SLAVE)-select(
+ 'user_properties',
+ array( 'up_property', 'up_value' ),
+ array( 'up_user' = $userId ),
+ __METHOD__
+ );
+
+ return $res-fetchRow();
+ }
+
+ public function testOnlySaveChangedOptions() {
+ $user = User::newFromName( 'UnitTestUser2' );
+ $user-addToDatabase();
+
+ // Fresh user only has default, so nothing should be in the DB
+ $dbProps = $this-dbUserProperties( $user-getId() );
+ $this-assertEquals( false, $dbProps,
+ A new user should not have any user property saved in
the DB );
+
+ // Make sure we only save the altered option
+ $user-setOption( 'changed_opt', 'alix_20281' );
+ $user-saveSettings();
+
+ $expected = array (
+ 'up_property' = 'changed_opt',
+ 0 = 'changed_opt',
+ 'up_value' = 'alix_20281',
+ 1 = 'alix_20281',
+ );
+ $dbProps = $this-dbUserProperties( $user-getId() );
+
+ $this-assertEquals( $expected, $dbProps,
+ non default options should be saved, and default ones
should not );
+
+
