Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EXAMPLE] Demonstrate current behavior for update_option #5250

Closed
wants to merge 4 commits into from
Closed

[EXAMPLE] Demonstrate current behavior for update_option #5250

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4f63d8
Demonstrate current behavior for update_option
joemcgill Sep 19, 2023
21eaac4
Add empty string falsey loose cases
joemcgill Sep 19, 2023
c112b1b
Update tests to confirm current WP behavior
joemcgill Sep 20, 2023
cf71d2c
Add non-scalar use cases
joemcgill Sep 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 156 additions & 0 deletions tests/phpunit/tests/option/option.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,160 @@ public function data_option_autoloading() {
array( 'autoload_false', false, 'no' ),
);
}

/**
* Ensure the database is getting updated when type changes, but not otherwise.
*
* @ticket 22192
*
* @covers ::update_option
*
* @dataProvider data_update_option_type_juggling
*/
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 );

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
*
* @covers ::update_option
*
* @dataProvider data_update_option_type_juggling
*/
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 );

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
*
* @covers ::update_option
*
* @dataProvider data_update_option_type_juggling
*/
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.
wp_cache_delete( 'alloptions', 'options' );
wp_load_alloptions();

$updated = update_option( 'foo', $new_value );

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.' );
}
}


/**
* Data provider.
*
* @return array
*/
public function data_update_option_type_juggling() {
return array(
/*
* Truthy values.
* Loosely equal truthy scalar values should never result in a DB update.
*/
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 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, true ), // Should update.
array( '', '' ),
array( '', 0, true ), // Should update.
array( '', 0.0, true ), // Should update.
array( '', false ),
array( 0, '0' ),
array( 0, '', true ), // Should update.
array( 0, 0 ),
array( 0, 0.0 ),
array( 0, false, true ), // Should update.
array( 0.0, '0' ),
array( 0.0, '', true ), // Should update.
array( 0.0, 0 ),
array( 0.0, 0.0 ),
array( 0.0, false, true ), // Should update.
array( false, '0', true ), // Should update.
array( false, '' ),
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 ),
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of checking all truthy/falsey, should we add cases for array (empty and populated), object (always truthy), and null (always falsey)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly! I was assuming we wanted to limit the use cases to only scalar values. Since arrays and null values are not scalar, they may need to be handled differently. Worth confirming though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a set of non-scalar use cases in cf71d2c. Interesting to see which loosey values result in an update and which do not.

}
}