Skip to content

Commit 8d8b843

Browse files
felixarntzfelixarntz
committed
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
1 parent bccc6fc commit 8d8b843

3 files changed

Lines changed: 174 additions & 7 deletions

File tree

‎src/wp-includes/option.php‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -776,21 +776,23 @@ function update_option( $option, $value, $autoload = null ) {
776776
*/
777777
$value = apply_filters( 'pre_update_option', $value, $option, $old_value );
778778

779+
/** This filter is documented in wp-includes/option.php */
780+
$default_value = apply_filters( "default_option_{$option}", false, $option, false );
781+
779782
/*
780783
* If the new and old values are the same, no need to update.
781784
*
782-
* Unserialized values will be adequate in most cases. If the unserialized
783-
* data differs, the (maybe) serialized data is checked to avoid
784-
* unnecessary database calls for otherwise identical object instances.
785+
* An exception applies when no value is set in the database, i.e. the old value is the default.
786+
* In that case, the new value should always be added as it may be intentional to store it rather than relying on the default.
785787
*
786-
* See https://core.trac.wordpress.org/ticket/38903
788+
* See https://core.trac.wordpress.org/ticket/38903 and https://core.trac.wordpress.org/ticket/22192.
787789
*/
788-
if ( $value === $old_value || maybe_serialize( $value ) === maybe_serialize( $old_value ) ) {
790+
if ( $old_value !== $default_value && _is_equal_database_value( $old_value, $value ) ) {
789791
return false;
790792
}
791793

792-
/** This filter is documented in wp-includes/option.php */
793-
if ( apply_filters( "default_option_{$option}", false, $option, false ) === $old_value ) {
794+
if ( $old_value === $default_value ) {
795+
794796
// Default setting for new options is 'yes'.
795797
if ( null === $autoload ) {
796798
$autoload = 'yes';
@@ -2887,3 +2889,40 @@ function filter_default_option( $default_value, $option, $passed_default ) {
28872889

28882890
return $registered[ $option ]['default'];
28892891
}
2892+
2893+
/**
2894+
* Determines whether two values will be equal when stored in the database.
2895+
*
2896+
* @since 6.4.0
2897+
* @access private
2898+
*
2899+
* @param mixed $old_value The old value to compare.
2900+
* @param mixed $new_value The new value to compare.
2901+
* @return bool True if the values are equal, false otherwise.
2902+
*/
2903+
function _is_equal_database_value( $old_value, $new_value ) {
2904+
$values = array(
2905+
'old' => $old_value,
2906+
'new' => $new_value,
2907+
);
2908+
2909+
foreach ( $values as $_key => &$_value ) {
2910+
// Cast scalars or null to a string so type discrepancies don't result in cache misses.
2911+
if ( null === $_value || is_scalar( $_value ) ) {
2912+
$_value = (string) $_value;
2913+
}
2914+
}
2915+
2916+
if ( $values['old'] === $values['new'] ) {
2917+
return true;
2918+
}
2919+
2920+
/*
2921+
* Unserialized values will be adequate in most cases. If the unserialized
2922+
* data differs, the (maybe) serialized data is checked to avoid
2923+
* unnecessary database calls for otherwise identical object instances.
2924+
*
2925+
* See https://core.trac.wordpress.org/ticket/38903
2926+
*/
2927+
return maybe_serialize( $old_value ) === maybe_serialize( $new_value );
2928+
}

‎tests/phpunit/tests/option/isEqualDatabaseValue.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
/**
3+
* Tests for _is_equal_database_value().
4+
*
5+
* @group option
6+
*
7+
* @covers ::_is_equal_database_value
8+
*/
9+
class Tests_Option_IsEqualDatabaseValue extends WP_UnitTestCase {
10+
11+
/**
12+
* @ticket 22192
13+
*
14+
* @dataProvider data_is_equal_database_value
15+
*
16+
* @param mixed $old_value The old value to compare.
17+
* @param mixed $new_value The new value to compare.
18+
* @param bool $expected The expected result.
19+
*/
20+
public function test_is_equal_database_value( $old_value, $new_value, $expected ) {
21+
$this->assertSame( $expected, _is_equal_database_value( $old_value, $new_value ) );
22+
}
23+
24+
/**
25+
* Data provider.
26+
*
27+
* @return array
28+
*/
29+
public function data_is_equal_database_value() {
30+
return array(
31+
// Equal values.
32+
array( '123', '123', true ),
33+
34+
// Not equal values.
35+
array( '123', '456', false ),
36+
37+
// Truthy.
38+
array( 1, '1', true ),
39+
array( 1.0, '1', true ),
40+
array( '1', '1', true ),
41+
array( true, '1', true ),
42+
array( '1.0', '1', false ),
43+
array( ' ', '1', false ),
44+
array( array( '0' ), '1', false ),
45+
array( new stdClass(), '1', false ),
46+
array( 'Howdy, admin!', '1', false ),
47+
48+
// False-ish values and empty strings.
49+
array( 0, '0', true ),
50+
array( 0.0, '0', true ),
51+
array( '0', '0', true ),
52+
array( '', '0', false ),
53+
array( false, '0', false ),
54+
array( null, '0', false ),
55+
array( array(), '0', false ),
56+
57+
// Object values.
58+
array( (object) array( 'foo' => 'bar' ), (object) array( 'foo' => 'bar' ), true ),
59+
array( (object) array( 'foo' => 'bar' ), (object) array( 'foo' => 'baz' ), false ),
60+
array( (object) array( 'foo' => 'bar' ), serialize( (object) array( 'foo' => 'bar' ) ), false ),
61+
array( serialize( (object) array( 'foo' => 'bar' ) ), (object) array( 'foo' => 'bar' ), false ),
62+
array( serialize( (object) array( 'foo' => 'bar' ) ), (object) array( 'foo' => 'baz' ), false ),
63+
64+
// Serialized values.
65+
array( array( 'foo' => 'bar' ), serialize( array( 'foo' => 'bar' ) ), false ),
66+
array( array( 'foo' => 'bar' ), serialize( array( 'foo' => 'baz' ) ), false ),
67+
array( serialize( (object) array( 'foo' => 'bar' ) ), serialize( (object) array( 'foo' => 'bar' ) ), true ),
68+
array( serialize( (object) array( 'foo' => 'bar' ) ), serialize( (object) array( 'foo' => 'baz' ) ), false ),
69+
);
70+
}
71+
}

‎tests/phpunit/tests/option/option.php‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,4 +524,61 @@ public function data_update_option_type_juggling() {
524524
array( null, array(), true ),
525525
);
526526
}
527+
528+
/**
529+
* Tests that update_option() stores an option that uses
530+
* an unfiltered default value of (bool) false.
531+
*
532+
* @ticket 22192
533+
*
534+
* @covers ::update_option
535+
*/
536+
public function test_update_option_should_store_option_with_default_value_false() {
537+
global $wpdb;
538+
539+
$option = 'update_option_default_false';
540+
update_option( $option, false );
541+
542+
$actual = $wpdb->query(
543+
$wpdb->prepare(
544+
"SELECT option_name FROM $wpdb->options WHERE option_name = %s LIMIT 1",
545+
$option
546+
)
547+
);
548+
549+
$this->assertSame( 1, $actual );
550+
}
551+
552+
/**
553+
* Tests that update_option() stores an option that uses
554+
* a filtered default value.
555+
*
556+
* @ticket 22192
557+
*
558+
* @covers ::update_option
559+
*/
560+
public function test_update_option_should_store_option_with_filtered_default_value() {
561+
global $wpdb;
562+
563+
$option = 'update_option_custom_default';
564+
$default_value = 'default-value';
565+
566+
add_filter(
567+
"default_option_{$option}",
568+
static function () use ( $default_value ) {
569+
return $default_value;
570+
}
571+
);
572+
573+
update_option( $option, $default_value );
574+
575+
$actual = $wpdb->query(
576+
$wpdb->prepare(
577+
"SELECT option_name FROM $wpdb->options WHERE option_name = %s LIMIT 1",
578+
$option
579+
)
580+
);
581+
582+
$this->assertSame( 1, $actual );
583+
}
527584
}

0 commit comments

Comments
 (0)