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

WP CLI support #119

Merged
merged 3 commits into from Nov 22, 2017

Conversation

3 participants
@junaidbhura
Contributor

junaidbhura commented Nov 21, 2017

This fixes: #116

Adds a new WP CLI command to clear the cache with the following options:

wp autoptimize clear flushes the cache (clears and re-builds)
wp autoptimize clear --type=css clears the CSS cache
wp autoptimize clear --type=js clears the JS cache

@futtta

This comment has been minimized.

Show comment
Hide comment
@futtta

futtta Nov 21, 2017

Owner

Super, thanks a million @junaidbhura! I'll review and merge or provide feedback in case I see issues!

Owner

futtta commented Nov 21, 2017

Super, thanks a million @junaidbhura! I'll review and merge or provide feedback in case I see issues!

@futtta

This comment has been minimized.

Show comment
Hide comment
@futtta

futtta Nov 21, 2017

Owner

OK, 4 remarks/ questions @junaidbhura :

  1. The autoptimized files are created with the user under which the PHP-webserver process is running, WP-CLI runs with the user-account of the PHP-CLI-user. Does the code handle possible filesystem rights issues?
  2. This will likely not work for multisite-installs, where if I understand correctly this code will always remove the main site's cache. Could you add a param for site when on multisite and switch to the correct site (in which case AUTOPTIMIZE_CACHE_DIR will automatically be set to wp-content/cache/autoptimize/<site_id>) OR when on multisite append the site-id to AUTOPTIMIZE_CACHE_DIR?
  3. for only JS/CSS: When "static delivery" is turned off (get_option('autoptimize_cache_nogzip') !== 1) then CSS and JS are not stored in subdirectories, but are both stored in AUTOPTIMIZE_CACHE_DIR, so the --type=css and --type=js options won't work there, do you think you could you add logic to catch that?
  4. for only JS/CSS: when only removing CSS or JS you're not going through AO's autoptimize::clearall, which has one disavantage; we're not trying to clear page cache's caches meaning the page cache could hold cached HTML that links to purged AO'ed CSS or JS. Autoptimize has an action that is triggered for this (for caching plugins that integrate with AO) and has code to clear some page caches itself as well, cfr. https://github.com/futtta/autoptimize/blob/master/classes/autoptimizeCache.php#L96-L105. it would be safe to do something similar when clearing only JS or CSS-cache :-)

frank

Owner

futtta commented Nov 21, 2017

OK, 4 remarks/ questions @junaidbhura :

  1. The autoptimized files are created with the user under which the PHP-webserver process is running, WP-CLI runs with the user-account of the PHP-CLI-user. Does the code handle possible filesystem rights issues?
  2. This will likely not work for multisite-installs, where if I understand correctly this code will always remove the main site's cache. Could you add a param for site when on multisite and switch to the correct site (in which case AUTOPTIMIZE_CACHE_DIR will automatically be set to wp-content/cache/autoptimize/<site_id>) OR when on multisite append the site-id to AUTOPTIMIZE_CACHE_DIR?
  3. for only JS/CSS: When "static delivery" is turned off (get_option('autoptimize_cache_nogzip') !== 1) then CSS and JS are not stored in subdirectories, but are both stored in AUTOPTIMIZE_CACHE_DIR, so the --type=css and --type=js options won't work there, do you think you could you add logic to catch that?
  4. for only JS/CSS: when only removing CSS or JS you're not going through AO's autoptimize::clearall, which has one disavantage; we're not trying to clear page cache's caches meaning the page cache could hold cached HTML that links to purged AO'ed CSS or JS. Autoptimize has an action that is triggered for this (for caching plugins that integrate with AO) and has code to clear some page caches itself as well, cfr. https://github.com/futtta/autoptimize/blob/master/classes/autoptimizeCache.php#L96-L105. it would be safe to do something similar when clearing only JS or CSS-cache :-)

frank

@junaidbhura

This comment has been minimized.

Show comment
Hide comment
@junaidbhura

junaidbhura Nov 21, 2017

Contributor

Hey @futtta no worries 👍

Let me try and address a few of your questions / notes:

User Permissions

WP CLI users will be quite familiar with user permissions, since they would be working with it on a regular basis. I can foresee two types of use cases for this, please feel free to add any more that you can think of:

Clearing via WP CLI Only

These are folks who only use WP CLI to clear the cache. In this case, user permissions should not be a problem.

Clearing via a combination of WP CLI and the WordPress back-end

If, for some reason, folks want to be able to do it both ways, they can just make sure they are running the correct user permission with the command, like:

sudo -u www-data wp autoptimize clear

Assuming www-data is the correct user.

WordPress Multisite

WP CLI provides a method for people to choose the blog when running the command:

wp --url=myothersite.com autoptimize clear

When this is run, WordPress will switch to that blog, so AUTOPTIMIZE_CACHE_DIR will automatically have the value wp-content/cache/autoptimize/<site_id>.

More info here.

Clearing only JS or CSS

Okay, I thought it would be a little easier than that 😀

I've updated the code to now only support autoptimizeCache::clearall. So now the only command supported is:

wp autoptimize clear

Typically, WP CLI commands only access core functions, and do not have additional functionality. It's just an easy way to use the plugin from the command line.

Could you enhance the function autoptimizeCache::clearall to accept some parameters, since you know how it works best? Something like this:

static function clearall( $type = '' ) {
	if(!autoptimizeCache::cacheavail()) {
		return false;
	}

	if ( empty( $type ) ) {
		$dirs = array("","js","css");
	} else {
		// TODO: Logic for type
		$dirs = array( $type );
	}

        // scan the cachedirs        
	foreach ( $dirs as $scandirName ) {
		$scan[$scandirName] = scandir(AUTOPTIMIZE_CACHE_DIR.$scandirName);
	}

If this functionality is added to the core, I can help add it in the WP CLI command. So if you need to change this function, it's abstracted away from the CLI command.

Moving forward

In my opinion, the best way forward would be:

  1. We run with what we have now as a version 1
  2. You could perhaps enhance the function or come up with some way to clear individual caches in the core
  3. We could then update the CLI command to harness the new functionality

Let me know what you think! 😀

Contributor

junaidbhura commented Nov 21, 2017

Hey @futtta no worries 👍

Let me try and address a few of your questions / notes:

User Permissions

WP CLI users will be quite familiar with user permissions, since they would be working with it on a regular basis. I can foresee two types of use cases for this, please feel free to add any more that you can think of:

Clearing via WP CLI Only

These are folks who only use WP CLI to clear the cache. In this case, user permissions should not be a problem.

Clearing via a combination of WP CLI and the WordPress back-end

If, for some reason, folks want to be able to do it both ways, they can just make sure they are running the correct user permission with the command, like:

sudo -u www-data wp autoptimize clear

Assuming www-data is the correct user.

WordPress Multisite

WP CLI provides a method for people to choose the blog when running the command:

wp --url=myothersite.com autoptimize clear

When this is run, WordPress will switch to that blog, so AUTOPTIMIZE_CACHE_DIR will automatically have the value wp-content/cache/autoptimize/<site_id>.

More info here.

Clearing only JS or CSS

Okay, I thought it would be a little easier than that 😀

I've updated the code to now only support autoptimizeCache::clearall. So now the only command supported is:

wp autoptimize clear

Typically, WP CLI commands only access core functions, and do not have additional functionality. It's just an easy way to use the plugin from the command line.

Could you enhance the function autoptimizeCache::clearall to accept some parameters, since you know how it works best? Something like this:

static function clearall( $type = '' ) {
	if(!autoptimizeCache::cacheavail()) {
		return false;
	}

	if ( empty( $type ) ) {
		$dirs = array("","js","css");
	} else {
		// TODO: Logic for type
		$dirs = array( $type );
	}

        // scan the cachedirs        
	foreach ( $dirs as $scandirName ) {
		$scan[$scandirName] = scandir(AUTOPTIMIZE_CACHE_DIR.$scandirName);
	}

If this functionality is added to the core, I can help add it in the WP CLI command. So if you need to change this function, it's abstracted away from the CLI command.

Moving forward

In my opinion, the best way forward would be:

  1. We run with what we have now as a version 1
  2. You could perhaps enhance the function or come up with some way to clear individual caches in the core
  3. We could then update the CLI command to harness the new functionality

Let me know what you think! 😀

@futtta futtta merged commit 1e58f67 into futtta:master Nov 22, 2017

1 check passed

Scrutinizer 1 updated code elements
Details

@junaidbhura junaidbhura deleted the junaidbhura:wp-cli branch Nov 22, 2017

zytzagoo added a commit to zytzagoo/autoptimize that referenced this pull request Nov 28, 2017

WP CLI support (#119)
Basic WP CLI support thanks to @junaidbhura !
@classyentrepreneur

This comment has been minimized.

Show comment
Hide comment
@classyentrepreneur

classyentrepreneur Jun 30, 2018

Thanks for making this happen, @junaidbhura!

Could you please verify that running wp autoptimize clear --network will remove cache from all sites in a multi-site network?

classyentrepreneur commented Jun 30, 2018

Thanks for making this happen, @junaidbhura!

Could you please verify that running wp autoptimize clear --network will remove cache from all sites in a multi-site network?

@junaidbhura

This comment has been minimized.

Show comment
Hide comment
@junaidbhura

junaidbhura Jul 1, 2018

Contributor

No worries, @classyentrepreneur :)

I think it should, but I haven't tested that bit out!

Contributor

junaidbhura commented Jul 1, 2018

No worries, @classyentrepreneur :)

I think it should, but I haven't tested that bit out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment