Skip to content

Commit

Permalink
Add caching for VideoPress functions with meta queries (#14803)
Browse files Browse the repository at this point in the history 
* Add caching for VideoPress functions that call WP_Query

These involve meta queries,
so it'd be good to have
some kind of caching.

* Change the since tag to 8.4.0

As Jeremy mentioned,
8.3.0 now has a code freeze.

* Revert changes to videopress_get_attachment_id_by_url()

As Dero mentioned,
it looks like this isn't used in Jetpack.

* Deprecate videopress_get_attachment_id_by_url()

As Jeremy mentioned, this should
give third-party developers a chance
to change to something else.

* Store the videopress ID in a transient if no persistent object caching

Also, store the full WP_Post in the cache,
if it is available.

* Remove needless helper method

There is actually an existing function
that does the same thing.

* Add a new function, using Leo's suggestion

Add video_get_post_id_by_guid(),
and a unit test for it.

* Move the new function's test lower in the file

To correspond to its position
in utility-functions.php.

* Set the transient expiration to an hour

Following Leo's snippet,
this should have an expiration.

* Add a see tag, correct DocBlock summary

The see tag points to a possible replacement.
Also, the DocBlock summary references the wrong function.

* Change prefix of new function from video_ to videopress_

Also change the transient cache key.
Also, simply return the post, instead of
accessing the ->ID property.

* Add since tags to new functions

videopress_get_post_id_by_guid()
now has a @SInCE tag,
in addition to other dataProviders

* Remove needless comment

This applied when it was part of
video_get_post_by_guid(),
but not to the new function it's in.

* Rename function to keep consistency

Co-authored-by: leogermani <[email protected]>
  • Loading branch information
@kienstra @leogermani
kienstra and leogermani authored Mar 23, 2020
1 parent 47e4399 commit f98f1e1
Show file tree
Hide file tree
Showing 3 changed files with 250 additions and 8 deletions.
4 changes: 2 additions & 2 deletions modules/videopress/class.videopress-player.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ public function __construct( $guid, $maxwidth = 0, $options = array() ) {

if ( ! defined( 'WP_DEBUG' ) || WP_DEBUG !== true ) {
$expire = 3600;
if ( isset( $video->expires ) && is_int( $video->expires ) ) {
$expires_diff = time() - $video->expires;
if ( isset( $this->video->expires ) && is_int( $this->video->expires ) ) {
$expires_diff = time() - $this->video->expires;
if ( $expires_diff > 0 && $expires_diff < 86400 ) { // allowed range: 1 second to 1 day
$expire = $expires_diff;
}
Expand Down
73 changes: 67 additions & 6 deletions modules/videopress/utility-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,16 @@ function videopress_get_video_details( $guid ) {
*
* Modified from https://wpscholar.com/blog/get-attachment-id-from-wp-image-url/
*
* @todo: Add some caching in here.
* @deprecated since 8.4.0
* @see videopress_get_post_id_by_guid()
*
* @param string $url
*
* @return int|bool Attachment ID on success, false on failure
*/
function videopress_get_attachment_id_by_url( $url ) {
_deprecated_function( __FUNCTION__, 'jetpack-8.4' );

$wp_upload_dir = wp_upload_dir();
// Strip out protocols, so it doesn't fail because searching for http: in https: dir.
$dir = set_url_scheme( trailingslashit( $wp_upload_dir['baseurl'] ), 'relative' );
Expand Down Expand Up @@ -619,7 +622,7 @@ function video_format_done( $info, $format ) {
*/
function video_image_url_by_guid( $guid, $format ) {

$post = video_get_post_by_guid( $guid );
$post = videopress_get_post_by_guid( $guid );

if ( is_wp_error( $post ) ) {
return null;
Expand All @@ -636,14 +639,67 @@ function video_image_url_by_guid( $guid, $format ) {
/**
* Using a GUID, find a post.
*
* @param string $guid
* @return WP_Post
* @param string $guid The post guid.
* @return WP_Post|false The post for that guid, or false if none is found.
*/
function videopress_get_post_by_guid( $guid ) {
$cache_key = 'get_post_by_guid_' . $guid;
$cache_group = 'videopress';
$cached_post = wp_cache_get( $cache_key, $cache_group );

if ( is_object( $cached_post ) && 'WP_Post' === get_class( $cached_post ) ) {
return $cached_post;
}

$post_id = videopress_get_post_id_by_guid( $guid );

if ( is_int( $post_id ) ) {
$post = get_post( $post_id );
wp_cache_set( $cache_key, $post, $cache_group, HOUR_IN_SECONDS );

return $post;
}

return false;
}

/**
* Using a GUID, find a post.
*
* Kept for backward compatibility. Use videopress_get_post_by_guid() instead.
*
* @deprecated since 8.4.0
* @see videopress_get_post_by_guid()
*
* @param string $guid The post guid.
* @return WP_Post|false The post for that guid, or false if none is found.
*/
function video_get_post_by_guid( $guid ) {
_deprecated_function( __FUNCTION__, 'jetpack-8.4' );
return videopress_get_post_by_guid( $guid );
}

/**
* Using a GUID, find the associated post ID.
*
* @since 8.4.0
* @param string $guid The guid to look for the post ID of.
* @return int|false The post ID for that guid, or false if none is found.
*/
function videopress_get_post_id_by_guid( $guid ) {
$cache_key = 'videopress_get_post_id_by_guid_' . $guid;
$cached_id = get_transient( $cache_key );

if ( is_int( $cached_id ) ) {
return $cached_id;
}

$args = array(
'post_type' => 'attachment',
'post_mime_type' => 'video/videopress',
'post_status' => 'inherit',
'no_found_rows' => true,
'fields' => 'ids',
'meta_query' => array(
array(
'key' => 'videopress_guid',
Expand All @@ -655,9 +711,14 @@ function video_get_post_by_guid( $guid ) {

$query = new WP_Query( $args );

$post = $query->next_post();
if ( $query->have_posts() ) {
$post_id = $query->next_post();
set_transient( $cache_key, $post_id, HOUR_IN_SECONDS );

return $post;
return $post_id;
}

return false;
}

/**
Expand Down
181 changes: 181 additions & 0 deletions tests/php/modules/videopress/test_functions.utility-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,187 @@
*/
class WP_Test_Jetpack_VideoPress_Utility_Functions extends WP_UnitTestCase {

/**
* Tests a helper function to get the post by guid, when there is no post found.
*
* @covers ::videopress_get_post_by_guid
* @since 8.4.0
*/
public function test_no_post_found_videopress_get_post_by_guid() {
$this->assertFalse( videopress_get_post_by_guid( wp_generate_uuid4() ) );
}

/**
* Gets the test data for test_non_cached_videopress_get_post_by_guid().
*
* @since 8.4.0
*
* @return array The test data.
*/
public function get_data_test_video_non_cached() {
return array(
'external_object_cache_is_enabled' => array(
'wp_cache_get',
true,
'get_post_by_guid_',
'videopress',
),
'external_object_cache_is_not_enabled' => array(
'get_transient',
false,
'videopress_get_post_id_by_guid_',
),
);
}

/**
* Tests a helper function to get the post by guid, when there's initially no cached value.
*
* @dataProvider get_data_test_video_non_cached
* @covers ::videopress_get_post_by_guid
* @since 8.4.0
*
* @param callable $callback The callback to get the caching.
* @param bool $should_cache_object Whether the entire WP_Post should be cached, or simply the post ID.
* @param string $cache_key_base The base of the cache key.
* @param string|null $cache_group The cache group, if any.
*/
public function test_non_cached_videopress_get_post_by_guid( $callback, $should_cache_object, $cache_key_base, $cache_group = null ) {
$guid = wp_generate_uuid4();
$expected_id = videopress_create_new_media_item( 'Example', $guid );
$expected_post = get_post( $expected_id );
$actual_post = videopress_get_post_by_guid( $guid );

$this->assertEquals( $expected_post, $actual_post );

$caching_args = array( $cache_key_base . $guid );
if ( $cache_group ) {
$caching_args[] = $cache_group;
}
$expected_cached = $should_cache_object ? $expected_post : $expected_id;

// The function should have cached the value.
$this->assertEquals(
$expected_cached,
call_user_func_array( $callback, $caching_args )
);
}

/**
* Gets the test data for test_cached_videopress_get_post_by_guid().
*
* @since 8.4.0
*
* @return array The test data.
*/
public function get_data_test_video_cached() {
return array(
'post_should_be_stored_in_cache' => array(
'wp_cache_set',
true,
'videopress',
),
'post_id_should_be_stored_in_transient' => array(
'set_transient',
false,
),
);
}

/**
* Tests a helper function to get the post by guid, when there is a cached value.
*
* As long as there is a non-expired cache value,
* this should return that instead of instantiating WP_Query.
*
* @dataProvider get_data_test_video_cached
* @covers ::videopress_get_post_by_guid
* @since 8.4.0
*
* @param callable $callback The callback to set the caching.
* @param bool $should_cache_object Whether the entire WP_Post should be cached, or simply the post ID.
* @param string|null $cache_group The cache group, if any.
*/
public function test_cached_videopress_get_post_by_guid( $callback, $should_cache_object, $cache_group = null ) {
$guid = wp_generate_uuid4();
$attachment_id = videopress_create_new_media_item( 'Example Title', $guid );
$attachment_post = get_post( $attachment_id );
$post_to_cache = $should_cache_object ? $attachment_post : $attachment_id;
$caching_args = array( 'get_post_by_guid_' . $guid, $post_to_cache );

if ( $cache_group ) {
$caching_args[] = $cache_group;
}

call_user_func_array( $callback, $caching_args );

// This should always return the WP_Post, even though the post ID is stored in the transient.
$this->assertEquals(
$attachment_post,
videopress_get_post_by_guid( $guid )
);
}

/**
* Gets the test data for test_cached_invalid_videopress_get_post_by_guid().
*
* @since 8.4.0
*
* @return array The test data.
*/
public function get_data_cached_invalid() {
return array(
'non_post_object' => array( new stdClass() ),
'int_but_not_valid_post_id' => array( PHP_INT_MAX ),
'null' => array( null ),
'zero' => array( 0 ),
);
}

/**
* Tests invalid cached values that should be ignored.
*
* Unless the cached value is a WP_Post,
* the tested method should ignore it and query for the post.
*
* @dataProvider get_data_cached_invalid
* @covers ::videopress_get_post_by_guid
* @since 8.4.0
*
* @param mixed $invalid_cached_value A cached value that should be ignored.
*/
public function test_cached_invalid_videopress_get_post_by_guid( $invalid_cached_value ) {
$guid = wp_generate_uuid4();
$attachment_id = videopress_create_new_media_item( 'Example Title', $guid );

wp_cache_set( 'get_post_by_guid_' . $guid, $invalid_cached_value, 'videopress' );

$this->assertEquals(
get_post( $attachment_id ),
videopress_get_post_by_guid( $guid )
);
}

/**
* Tests a helper function to get the post id by guid.
*
* @covers ::videopress_get_post_id_by_guid
* @since 8.4.0
*/
public function test_non_cached_videopress_get_post_id_by_guid() {
$guid = wp_generate_uuid4();
$expected_id = videopress_create_new_media_item( 'Example', $guid );
$actual_post_id = videopress_get_post_id_by_guid( $guid );

$this->assertEquals( $expected_id, $actual_post_id );

// The function should have cached the value.
$this->assertEquals(
$expected_id,
get_transient( 'videopress_get_post_id_by_guid_' . $guid )
);
}

/**
* Tests the VideoPress Flash to oEmbedable URL filter.
*
Expand Down

0 comments on commit f98f1e1

Please sign in to comment.