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.

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

Improve wp-cache config functions #487

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stodorovic
Copy link

@stodorovic stodorovic commented Dec 8, 2017

  • Improve regular expressions in functions wp_cache_replace_line and wp_cache_update_rejected_pages
  • Copy fixed CS (wp-cache-config-sample.php) from PHPCS fixes for wp-cache-config-sample.php #484 props @robjarsen
  • Introduce function wpsc_var_export

@stodorovic
Copy link
Author

Tests for function wpsc_var_export:

var_dump( wpsc_var_export( false ) );
var_dump( wpsc_var_export( true ) );
var_dump( wpsc_var_export( 0 ) );
var_dump( wpsc_var_export( '0' ) );
var_dump( wpsc_var_export( 'test' ) );
var_dump( wpsc_var_export( array( ) ) );
var_dump( wpsc_var_export( array( 'bot1', 'bot2' ) ) );
var_dump( wpsc_var_export( array( 0 => 'bot1', 1 => 'bot2' ) ) );
$obj = new stdclass();
$obj->test = 'test';
var_dump( wpsc_var_export( $obj ) );

Output:

test-scripts/wpsc/test1.php:19:
string(5) "false"
test-scripts/wpsc/test1.php:20:
string(4) "true"
test-scripts/wpsc/test1.php:21:
string(1) "0"
test-scripts/wpsc/test1.php:22:
string(3) "'0'"
test-scripts/wpsc/test1.php:23:
string(6) "'test'"
test-scripts/wpsc/test1.php:24:
string(7) "array()"
test-scripts/wpsc/test1.php:25:
string(23) "array( 'bot1', 'bot2' )"
test-scripts/wpsc/test1.php:26:
string(23) "array( 'bot1', 'bot2' )"
test-scripts/wpsc/test1.php:29:
string(52) "stdClass::__set_state(array(    'test' => 'test', ))"

I've also checked wp_cache_setting. It seems that all variants work as should.

I'll avoid replace spaces when $var is object because it could make some side effects. For arrays, we don't need keys and config file is more readable.

@donnchawp
Copy link
Contributor

That looks great, but I'll leave this until the next release after this one as it's a critical part of the plugin.

@stodorovic
Copy link
Author

I agree. We need to carefully test everything. I'm still testing it..

@@ -1707,7 +1707,7 @@ function wp_cache_time_update() {
}

function wp_cache_edit_max_time() {
global $cache_max_time, $wp_cache_config_file, $valid_nonce, $super_cache_enabled, $cache_schedule_type, $cache_scheduled_time, $cache_schedule_interval, $cache_time_interval, $cache_gc_email_me, $wp_cache_preload_on;
Copy link
Contributor

Choose a reason for hiding this comment

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

$valid_nonce needs to be a global variable in the functions that update settings because the REST API in rest/class.wp-super-cache-rest-update-settings.php sets it to true so it can use these update functions.

If we want to verify the nonce in each update function the REST API update code has to be modified to generate a valid POST nonce too. Probably best to keep that for a different PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants