From 8d8b843eafaf0202513e6224130d9f4f2182af81 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Mon, 25 Sep 2023 16:23:52 +0000 Subject: [PATCH] Options, Meta APIs: Improve logic to avoid unnecessary database writes in `update_option()`. Prior to this change, a strict comparison between the old and new database value could lead to a false negative, since database values are generally stored as strings. For example, passing an integer to `update_option()` would almost always result in an update given any existing database value for that option would be that number cast to a string. This changeset adjusts the logic to perform an intentional "loose-y" comparison by casting the values to strings. Extensive coverage previously added in [56648] provides additional confidence that this does not introduce any backward compatibility issues. Props mukesh27, costdev, spacedmonkey, joemcgill, flixos90, nacin, atimmer, duck_, boonebgorges. Fixes #22192. git-svn-id: https://develop.svn.wordpress.org/trunk@56681 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/option.php | 53 ++++++++++++-- .../tests/option/isEqualDatabaseValue.php | 71 +++++++++++++++++++ tests/phpunit/tests/option/option.php | 57 +++++++++++++++ 3 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 tests/phpunit/tests/option/isEqualDatabaseValue.php diff --git a/src/wp-includes/option.php b/src/wp-includes/option.php index c207c5ca1d..2beb5eeef3 100644 --- a/src/wp-includes/option.php +++ b/src/wp-includes/option.php @@ -776,21 +776,23 @@ function update_option( $option, $value, $autoload = null ) { */ $value = apply_filters( 'pre_update_option', $value, $option, $old_value ); + /** This filter is documented in wp-includes/option.php */ + $default_value = apply_filters( "default_option_{$option}", false, $option, false ); + /* * If the new and old values are the same, no need to update. * - * Unserialized values will be adequate in most cases. If the unserialized - * data differs, the (maybe) serialized data is checked to avoid - * unnecessary database calls for otherwise identical object instances. + * An exception applies when no value is set in the database, i.e. the old value is the default. + * In that case, the new value should always be added as it may be intentional to store it rather than relying on the default. * - * See https://core.trac.wordpress.org/ticket/38903 + * See https://core.trac.wordpress.org/ticket/38903 and https://core.trac.wordpress.org/ticket/22192. */ - if ( $value === $old_value || maybe_serialize( $value ) === maybe_serialize( $old_value ) ) { + if ( $old_value !== $default_value && _is_equal_database_value( $old_value, $value ) ) { return false; } - /** This filter is documented in wp-includes/option.php */ - if ( apply_filters( "default_option_{$option}", false, $option, false ) === $old_value ) { + if ( $old_value === $default_value ) { + // Default setting for new options is 'yes'. if ( null === $autoload ) { $autoload = 'yes'; @@ -2887,3 +2889,40 @@ function filter_default_option( $default_value, $option, $passed_default ) { return $registered[ $option ]['default']; } + +/** + * Determines whether two values will be equal when stored in the database. + * + * @since 6.4.0 + * @access private + * + * @param mixed $old_value The old value to compare. + * @param mixed $new_value The new value to compare. + * @return bool True if the values are equal, false otherwise. + */ +function _is_equal_database_value( $old_value, $new_value ) { + $values = array( + 'old' => $old_value, + 'new' => $new_value, + ); + + foreach ( $values as $_key => &$_value ) { + // Cast scalars or null to a string so type discrepancies don't result in cache misses. + if ( null === $_value || is_scalar( $_value ) ) { + $_value = (string) $_value; + } + } + + if ( $values['old'] === $values['new'] ) { + return true; + } + + /* + * Unserialized values will be adequate in most cases. If the unserialized + * data differs, the (maybe) serialized data is checked to avoid + * unnecessary database calls for otherwise identical object instances. + * + * See https://core.trac.wordpress.org/ticket/38903 + */ + return maybe_serialize( $old_value ) === maybe_serialize( $new_value ); +} diff --git a/tests/phpunit/tests/option/isEqualDatabaseValue.php b/tests/phpunit/tests/option/isEqualDatabaseValue.php new file mode 100644 index 0000000000..22d1d2ac1c --- /dev/null +++ b/tests/phpunit/tests/option/isEqualDatabaseValue.php @@ -0,0 +1,71 @@ +assertSame( $expected, _is_equal_database_value( $old_value, $new_value ) ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_is_equal_database_value() { + return array( + // Equal values. + array( '123', '123', true ), + + // Not equal values. + array( '123', '456', false ), + + // Truthy. + array( 1, '1', true ), + array( 1.0, '1', true ), + array( '1', '1', true ), + array( true, '1', true ), + array( '1.0', '1', false ), + array( ' ', '1', false ), + array( array( '0' ), '1', false ), + array( new stdClass(), '1', false ), + array( 'Howdy, admin!', '1', false ), + + // False-ish values and empty strings. + array( 0, '0', true ), + array( 0.0, '0', true ), + array( '0', '0', true ), + array( '', '0', false ), + array( false, '0', false ), + array( null, '0', false ), + array( array(), '0', false ), + + // Object values. + array( (object) array( 'foo' => 'bar' ), (object) array( 'foo' => 'bar' ), true ), + array( (object) array( 'foo' => 'bar' ), (object) array( 'foo' => 'baz' ), false ), + array( (object) array( 'foo' => 'bar' ), serialize( (object) array( 'foo' => 'bar' ) ), false ), + array( serialize( (object) array( 'foo' => 'bar' ) ), (object) array( 'foo' => 'bar' ), false ), + array( serialize( (object) array( 'foo' => 'bar' ) ), (object) array( 'foo' => 'baz' ), false ), + + // Serialized values. + array( array( 'foo' => 'bar' ), serialize( array( 'foo' => 'bar' ) ), false ), + array( array( 'foo' => 'bar' ), serialize( array( 'foo' => 'baz' ) ), false ), + array( serialize( (object) array( 'foo' => 'bar' ) ), serialize( (object) array( 'foo' => 'bar' ) ), true ), + array( serialize( (object) array( 'foo' => 'bar' ) ), serialize( (object) array( 'foo' => 'baz' ) ), false ), + ); + } +} diff --git a/tests/phpunit/tests/option/option.php b/tests/phpunit/tests/option/option.php index 5aebaae290..0cef91d6d7 100644 --- a/tests/phpunit/tests/option/option.php +++ b/tests/phpunit/tests/option/option.php @@ -524,4 +524,61 @@ class Tests_Option_Option extends WP_UnitTestCase { array( null, array(), true ), ); } + + /** + * Tests that update_option() stores an option that uses + * an unfiltered default value of (bool) false. + * + * @ticket 22192 + * + * @covers ::update_option + */ + public function test_update_option_should_store_option_with_default_value_false() { + global $wpdb; + + $option = 'update_option_default_false'; + update_option( $option, false ); + + $actual = $wpdb->query( + $wpdb->prepare( + "SELECT option_name FROM $wpdb->options WHERE option_name = %s LIMIT 1", + $option + ) + ); + + $this->assertSame( 1, $actual ); + } + + /** + * Tests that update_option() stores an option that uses + * a filtered default value. + * + * @ticket 22192 + * + * @covers ::update_option + */ + public function test_update_option_should_store_option_with_filtered_default_value() { + global $wpdb; + + $option = 'update_option_custom_default'; + $default_value = 'default-value'; + + add_filter( + "default_option_{$option}", + static function () use ( $default_value ) { + return $default_value; + } + ); + + update_option( $option, $default_value ); + + $actual = $wpdb->query( + $wpdb->prepare( + "SELECT option_name FROM $wpdb->options WHERE option_name = %s LIMIT 1", + $option + ) + ); + + $this->assertSame( 1, $actual ); + } }