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

Add support for wp embed cache clear --all command (#67) #74

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
},
"require-dev": {
"wp-cli/entity-command": "^1.3 || ^2",
"wp-cli/cache-command": "^1.0 || ^2",
"wp-cli/wp-cli-tests": "^4"
},
"config": {
"process-timeout": 7200,
"sort-packages": true,
"allow-plugins": {
"dealerdirect/phpcodesniffer-composer-installer": true,
"johnpbloch/wordpress-core-installer": true
"dealerdirect/phpcodesniffer-composer-installer": true
}
},
"extra": {
Expand Down
41 changes: 38 additions & 3 deletions features/cache.feature
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,20 @@ Feature: Manage oEmbed cache.
Background:
Given a WP install

Scenario: Clear oEmbed cache with no arguments
When I try `wp embed cache clear`
Then STDERR should be:
"""
Error: You must specify at least one post id or use --all
"""

Scenario: Clear oEmbed cache with both arguments
When I try `wp embed cache clear 1 --all`
Then STDERR should be:
"""
Error: You cannot specify both a post id and use --all
"""

Scenario: Clear oEmbed cache for an empty post
When I run `wp post create --post_title="Foo Bar" --porcelain`
Then STDOUT should be a number
Expand All @@ -11,14 +25,14 @@ Feature: Manage oEmbed cache.
When I try `wp embed cache clear {POST_ID}`
Then STDERR should be:
"""
Error: No cache to clear!
Error: No oEmbed cache to clear!
"""

Scenario: Clear oEmbed cache for a post
When I run `wp post-meta add 1 _oembed_foo 'bar'`
When I run `wp post meta add 1 _oembed_foo 'bar'`
Then STDOUT should not be empty

When I run `wp post-meta get 1 _oembed_foo`
When I run `wp post meta get 1 _oembed_foo`
Then STDOUT should be:
"""
bar
Expand All @@ -30,6 +44,27 @@ Feature: Manage oEmbed cache.
Success: Cleared oEmbed cache.
"""

Scenario: Clear all oEmbed caches
# No oEmbed caches yet
When I try `wp embed cache clear --all`
Then STDERR should be:
"""
Error: No oEmbed caches to clear!
"""

# Generate some various oEmbed caches types
When I run `wp post generate --post_type=oembed_cache --post_date="2000-01-01" --post_title=$(wp eval 'echo md5( "foo" );') --count=10`
And I run `wp post meta add 1 _oembed_$(wp eval 'echo md5( "foo" );') foo`
And I run `wp post meta add 1 _oembed_$(wp eval 'echo md5( "bar" );') bar`
Copy link
Member

Choose a reason for hiding this comment

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

Can we use WordPress APIs to generate real oEmbed cache values?

If we do so, these will be true integration tests and can alert us to breakage if core ever changes its behavior.

# Make sure we don't clear non-oEmbed caches
And I run `wp post meta add 1 _oembed_foo bar`
And I run `wp transient set oembed_$(wp eval 'echo md5( "bar" );') bar $(wp eval 'echo DAY_IN_SECONDS;')`
And I run `wp embed cache clear --all`
Then STDOUT should be:
"""
Success: Cleared 3 oEmbed caches: 1 post cache, 1 oembed post cache, 1 transient cache.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Can we also add some similar-looking meta, and then include verification that it doesn't get unexpectedly deleted?


Scenario: Trigger and clear oEmbed cache for a post
When I run `wp post create --post_title=Foo --post_type=post --post_status=publish --post_content="[embed]https://www.youtube.com/watch?v=dQw4w9WgXcQ[/embed]" --porcelain`
Then STDOUT should be a number
Expand Down
116 changes: 105 additions & 11 deletions src/Cache_Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@
namespace WP_CLI\Embeds;

use WP_CLI;
use WP_CLI\Process;
use WP_CLI\Utils;
use WP_CLI\Formatter;
use WP_CLI_Command;

/**
Expand All @@ -20,34 +18,130 @@ class Cache_Command extends WP_CLI_Command {
*
* ## OPTIONS
*
* <post_id>
* [<post_id>]
* : ID of the post to clear the cache for.
*
* [--all]
* : Clear all oEmbed caches.
*
* ## EXAMPLES
*
* # Clear cache for a post
* $ wp embed cache clear 123
* Success: Cleared oEmbed cache.

Choose a reason for hiding this comment

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

Let's change the message to include the post ID.

Suggested change
* Success: Cleared oEmbed cache.
* Success: Cleared oEmbed cache for post 123.

*
* # Clear all caches
* $ wp embed cache clear --all
* Success: Cleared all oEmbed caches.
*/
public function clear( $args, $assoc_args ) {
/** @var \WP_Embed $wp_embed */
global $wp_embed;
global $wp_embed, $wpdb;

$all = \WP_CLI\Utils\get_flag_value( $assoc_args, 'all' );

$post_id = isset( $args[0] ) ? $args[0] : null;

if ( null === $all && null === $post_id ) {
WP_CLI::error( 'You must specify at least one post ID or use --all.' );
}

if ( $post_id && $all ) {
WP_CLI::error( 'You cannot specify both a post ID and use --all.' );
}

// Delete all oEmbed caches.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is superfluous

if ( $all ) {
// Get post meta oEmbed caches
$oembed_post_meta_post_ids = (array) $wpdb->get_col(
"SELECT DISTINCT post_id FROM $wpdb->postmeta
WHERE meta_key REGEXP '^_oembed_[0-9a-f]{32}$'
Copy link
Member

Choose a reason for hiding this comment

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

Could we define these regular expressions as class-level constants, and link to where they're defined in core?

0-9a-f]{32} seems like a magical value, so it'd be good to document where it comes from.

OR meta_key REGEXP '^_oembed_time_[0-9a-f]{32}$'"
);
// Get posts oEmbed caches
$oembed_post_post_ids = (array) $wpdb->get_col(
"SELECT ID FROM $wpdb->posts
WHERE post_type = 'oembed_cache'
AND post_status = 'publish'
AND post_name REGEXP '^[0-9a-f]{32}$'"
);

// Get transient oEmbed caches
$oembed_transients = $wpdb->get_col(
"SELECT option_name FROM $wpdb->options
WHERE option_name REGEXP '^_transient_oembed_[0-9a-f]{32}$'"
);

$oembed_caches = array(
'post' => $oembed_post_meta_post_ids,
'oembed post' => $oembed_post_post_ids,
'transient' => $oembed_transients,
);

$post_id = $args[0];
$total = array_sum( array_map( function( $items ) {
return count( $items );
}, $oembed_caches ) );

// Delete post meta oEmbed caches
foreach ( $oembed_post_meta_post_ids as $post_id ) {
$wp_embed->delete_oembed_caches( $post_id );
}

// Delete posts oEmbed caches
foreach ( $oembed_post_post_ids as $post_id ) {
wp_delete_post( $post_id, true );
}

// Delete transient oEmbed caches
foreach ( $oembed_transients as $option_name ) {
delete_transient( str_replace( '_transient_', '', $option_name ) );
}

if ( $total > 0 ) {
$details = array();
foreach ( $oembed_caches as $type => $items ) {
$count = count( $items );
$details[] = sprintf(
'%1$d %2$s %3$s',
$count,
$type,
Utils\pluralize( 'cache', $count )
);
}

$message = sprintf(
'Cleared %1$d oEmbed %2$s: %3$s.',
$total,
Utils\pluralize( 'cache', $total ),
implode( ', ', $details )
);

WP_CLI::success( $message );
} else {
WP_CLI::error( 'No oEmbed caches to clear!' );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
WP_CLI::error( 'No oEmbed caches to clear!' );
WP_CLI::error( 'No oEmbed caches to clear.' );

Should this be WP_CLI::success()?

Copy link
Author

Choose a reason for hiding this comment

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

Currently is WP_CLI::error();, I stuck to what's in place right now.

But agree on the change. Makes more sense. You tell me.

}

if ( wp_using_ext_object_cache() ) {
WP_CLI::warning( 'Oembed transients are stored in an external object cache, and this command only deletes those stored in the database. You must flush the cache to delete all transients.' );
}

return;
}

$post_metas = get_post_custom_keys( $post_id );
// Delete oEmbed caches for a given post.
$count_metas = get_post_custom_keys( $post_id );

if ( $post_metas ) {
$post_metas = array_filter(
$post_metas,
if ( $count_metas ) {
$count_metas = array_filter(
$count_metas,
function ( $v ) {
return '_oembed_' === substr( $v, 0, 8 );
}
);
}

if ( empty( $post_metas ) ) {
WP_CLI::error( 'No cache to clear!' );
if ( empty( $count_metas ) ) {
WP_CLI::error( 'No oEmbed cache to clear.' );
}

$wp_embed->delete_oembed_caches( $post_id );
Expand Down