From f4f63d873a124d1003675e23ba357dd521c56c13 Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Tue, 19 Sep 2023 17:21:45 -0500 Subject: [PATCH 1/4] Demonstrate current behavior for update_option --- tests/phpunit/tests/option/option.php | 107 ++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/tests/phpunit/tests/option/option.php b/tests/phpunit/tests/option/option.php index a16a6431d9de9..ae3c6e9e718bb 100644 --- a/tests/phpunit/tests/option/option.php +++ b/tests/phpunit/tests/option/option.php @@ -368,4 +368,111 @@ public function data_option_autoloading() { array( 'autoload_false', false, 'no' ), ); } + + /** + * Ensure the database is getting updated when type changes, but not otherwise. + * + * @ticket 22192 + * @group test + * + * @covers ::add_option + * + * @dataProvider data_update_option_type_juggling + */ + public function test_update_loosey_options( $old_value, $new_value ) { + add_option( 'foo', $old_value ); + + // Comparison will happen against value cached during add_option() above. + $updated = update_option( 'foo', $new_value ); + + $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + } + + /** + * Ensure the database is getting updated when type changes, but not otherwise. + * + * @ticket 22192 + * @group test + * + * @covers ::add_option + * + * @dataProvider data_update_option_type_juggling + */ + public function test_update_loosey_options_from_db( $old_value, $new_value ) { + add_option( 'foo', $old_value ); + + // Delete cache. + wp_cache_delete( 'alloptions', 'options' ); + $updated = update_option( 'foo', $new_value ); + + $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + } + + /** + * Ensure the database is getting updated when type changes, but not otherwise. + * + * @ticket 22192 + * @group test + * + * @covers ::add_option + * + * @dataProvider data_update_option_type_juggling + */ + public function test_update_loosey_options_from_refreshed_cache( $old_value, $new_value ) { + add_option( 'foo', $old_value ); + + // Delete and refresh cache from DB. + wp_cache_delete( 'alloptions', 'options' ); + wp_load_alloptions(); + + $updated = update_option( 'foo', $new_value ); + + $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + } + + + /** + * Data provider. + * + * @return array + */ + public function data_update_option_type_juggling() { + return array( + // Truthy. + array( '1', '1' ), + array( '1', 1 ), + array( '1', 1.0 ), + array( '1', true ), + array( 1, '1' ), + array( 1, 1 ), + array( 1, 1.0 ), + array( 1, true ), + array( 1.0, '1' ), + array( 1.0, 1 ), + array( 1.0, 1.0 ), + array( 1.0, true ), + array( true, '1' ), + array( true, 1 ), + array( true, 1.0 ), + array( true, true ), + + // Falsey. + array( '0', '0' ), + array( '0', 0 ), + array( '0', 0.0 ), + array( '0', false ), + array( 0, '0' ), + array( 0, 0 ), + array( 0, 0.0 ), + array( 0, false ), + array( 0.0, '0' ), + array( 0.0, 0 ), + array( 0.0, 0.0 ), + array( 0.0, false ), + array( false, '0' ), + array( false, 0 ), + array( false, 0.0 ), + array( false, false ), + ); + } } From 21eaac4e65022bd41ac037f0dc1660b48c31162b Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Tue, 19 Sep 2023 17:36:19 -0500 Subject: [PATCH 2/4] Add empty string falsey loose cases --- tests/phpunit/tests/option/option.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/phpunit/tests/option/option.php b/tests/phpunit/tests/option/option.php index ae3c6e9e718bb..a2bba4a53b258 100644 --- a/tests/phpunit/tests/option/option.php +++ b/tests/phpunit/tests/option/option.php @@ -461,15 +461,22 @@ public function data_update_option_type_juggling() { array( '0', 0 ), array( '0', 0.0 ), array( '0', false ), + array( '', '' ), + array( '', 0 ), + array( '', 0.0 ), + array( '', false ), array( 0, '0' ), + array( 0, '' ), array( 0, 0 ), array( 0, 0.0 ), array( 0, false ), array( 0.0, '0' ), + array( 0.0, '' ), array( 0.0, 0 ), array( 0.0, 0.0 ), array( 0.0, false ), array( false, '0' ), + array( false, '' ), array( false, 0 ), array( false, 0.0 ), array( false, false ), From c112b1b045a12e1a6e0895f477df3c6149cc0eca Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Wed, 20 Sep 2023 11:29:35 -0500 Subject: [PATCH 3/4] Update tests to confirm current WP behavior --- tests/phpunit/tests/option/option.php | 63 +++++++++++++++++---------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/tests/phpunit/tests/option/option.php b/tests/phpunit/tests/option/option.php index a2bba4a53b258..7a9cff9ad7c5c 100644 --- a/tests/phpunit/tests/option/option.php +++ b/tests/phpunit/tests/option/option.php @@ -373,52 +373,57 @@ public function data_option_autoloading() { * Ensure the database is getting updated when type changes, but not otherwise. * * @ticket 22192 - * @group test * - * @covers ::add_option + * @covers ::update_option * * @dataProvider data_update_option_type_juggling */ - public function test_update_loosey_options( $old_value, $new_value ) { + public function test_update_loosey_options( $old_value, $new_value, $update = false ) { add_option( 'foo', $old_value ); // Comparison will happen against value cached during add_option() above. $updated = update_option( 'foo', $new_value ); - $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + if ( $update ) { + $this->assertTrue( $updated, 'This loosely equal option should trigger an update.' ); + } else { + $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + } } /** * Ensure the database is getting updated when type changes, but not otherwise. * * @ticket 22192 - * @group test * - * @covers ::add_option + * @covers ::update_option * * @dataProvider data_update_option_type_juggling */ - public function test_update_loosey_options_from_db( $old_value, $new_value ) { + public function test_update_loosey_options_from_db( $old_value, $new_value, $update = false ) { add_option( 'foo', $old_value ); // Delete cache. wp_cache_delete( 'alloptions', 'options' ); $updated = update_option( 'foo', $new_value ); - $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + if ( $update ) { + $this->assertTrue( $updated, 'This loosely equal option should trigger an update.' ); + } else { + $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + } } /** * Ensure the database is getting updated when type changes, but not otherwise. * * @ticket 22192 - * @group test * - * @covers ::add_option + * @covers ::update_option * * @dataProvider data_update_option_type_juggling */ - public function test_update_loosey_options_from_refreshed_cache( $old_value, $new_value ) { + public function test_update_loosey_options_from_refreshed_cache( $old_value, $new_value, $update = false ) { add_option( 'foo', $old_value ); // Delete and refresh cache from DB. @@ -427,7 +432,11 @@ public function test_update_loosey_options_from_refreshed_cache( $old_value, $ne $updated = update_option( 'foo', $new_value ); - $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + if ( $update ) { + $this->assertTrue( $updated, 'This loosely equal option should trigger an update.' ); + } else { + $this->assertFalse( $updated, 'Loosely equal option should not trigger an update.' ); + } } @@ -438,7 +447,10 @@ public function test_update_loosey_options_from_refreshed_cache( $old_value, $ne */ public function data_update_option_type_juggling() { return array( - // Truthy. + /* + * Truthy values. + * Loosely equal truthy scalar values should never result in a DB update. + */ array( '1', '1' ), array( '1', 1 ), array( '1', 1.0 ), @@ -456,29 +468,32 @@ public function data_update_option_type_juggling() { array( true, 1.0 ), array( true, true ), - // Falsey. + /* + * Falsey values. + * Loosely equal falsey scalar values only sometimes result in a DB update. + */ array( '0', '0' ), array( '0', 0 ), array( '0', 0.0 ), - array( '0', false ), + array( '0', false, true ), // Should update. array( '', '' ), - array( '', 0 ), - array( '', 0.0 ), + array( '', 0, true ), // Should update. + array( '', 0.0, true ), // Should update. array( '', false ), array( 0, '0' ), - array( 0, '' ), + array( 0, '', true ), // Should update. array( 0, 0 ), array( 0, 0.0 ), - array( 0, false ), + array( 0, false, true ), // Should update. array( 0.0, '0' ), - array( 0.0, '' ), + array( 0.0, '', true ), // Should update. array( 0.0, 0 ), array( 0.0, 0.0 ), - array( 0.0, false ), - array( false, '0' ), + array( 0.0, false, true ), // Should update. + array( false, '0', true ), // Should update. array( false, '' ), - array( false, 0 ), - array( false, 0.0 ), + array( false, 0, true ), // Should update. + array( false, 0.0, true ), // Should update. array( false, false ), ); } From cf71d2c9049b49db05e68411844cdc55e7670674 Mon Sep 17 00:00:00 2001 From: Joe McGill Date: Wed, 20 Sep 2023 11:43:44 -0500 Subject: [PATCH 4/4] Add non-scalar use cases --- tests/phpunit/tests/option/option.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/phpunit/tests/option/option.php b/tests/phpunit/tests/option/option.php index 7a9cff9ad7c5c..5aebaae290fbb 100644 --- a/tests/phpunit/tests/option/option.php +++ b/tests/phpunit/tests/option/option.php @@ -495,6 +495,33 @@ public function data_update_option_type_juggling() { array( false, 0, true ), // Should update. array( false, 0.0, true ), // Should update. array( false, false ), + + /* + * Non scalar values. + * Loosely equal non-scalar values should almost always result in an update. + */ + array( false, array(), true ), + array( 'false', array(), true ), + array( '', array(), true ), + array( 0, array(), true ), + array( '0', array(), true ), + array( false, null ), // Does not update. + array( 'false', null, true ), + array( '', null ), // Does not update. + array( 0, null, true ), + array( '0', null, true ), + array( array(), false, true ), + array( array(), 'false', true ), + array( array(), '', true ), + array( array(), 0, true ), + array( array(), '0', true ), + array( array(), null, true ), + array( null, false ), // Does not update. + array( null, 'false', true ), + array( null, '' ), // Does not update. + array( null, 0, true ), + array( null, '0', true ), + array( null, array(), true ), ); } }