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

Pattern Cache using Transient #6137

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c232c70
transient support
kt-12 Feb 19, 2024
9069099
cs fixes
kt-12 Feb 19, 2024
8565cb0
caching theme
kt-12 Mar 11, 2024
6bb8ab8
Apply suggestions from code review
kt-12 Mar 11, 2024
881df5d
remove group name from filter
kt-12 Mar 11, 2024
0e4868c
Add caching to template part and block template file
kt-12 Mar 21, 2024
cd34a68
Update version
kt-12 Mar 21, 2024
7eb787d
use static variable instead
kt-12 Mar 21, 2024
f08304c
fix cache key issue.
kt-12 Mar 22, 2024
0702339
theme broken issue fix and expiry time
kt-12 Mar 22, 2024
5542277
Merge branch 'trunk' into enhancement/patern-cache-transient
kt-12 Mar 22, 2024
bfe9ece
Revert back changes
kt-12 Mar 11, 2024
425e674
revert back to old state
kt-12 Apr 1, 2024
4d92ac0
disabled cached test
kt-12 Apr 1, 2024
2a170b9
bug fix
kt-12 Apr 1, 2024
fd586f3
scope fix
kt-12 Apr 1, 2024
d82bd36
test for transient cache
kt-12 Apr 1, 2024
458b851
transient check and filter check
kt-12 Apr 1, 2024
a255a9b
cache key update
kt-12 Apr 8, 2024
899b983
int for cache_expiration
kt-12 Apr 8, 2024
0a6ed79
update code to handle transient based caching
kt-12 Apr 9, 2024
5178a5f
cs fixes
kt-12 Apr 10, 2024
814eb17
Update tests/phpunit/tests/theme/wpThemeGetBlockPatterns.php
kt-12 Apr 11, 2024
5a536a8
Update tests/phpunit/tests/theme/wpThemeGetBlockPatterns.php
kt-12 Apr 11, 2024
e34a3d5
Update src/wp-includes/class-wp-theme.php
kt-12 Apr 11, 2024
25bbb35
Apply suggestions from code review
kt-12 Apr 11, 2024
6ffe308
Update src/wp-includes/class-wp-theme.php
kt-12 Apr 11, 2024
19f2952
Merge branch 'trunk' into enhancement/patern-cache-transient
kt-12 Apr 11, 2024
a53c35e
Merge branch 'enhancement/patern-cache-transient' of https://github.c…
kt-12 Apr 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 34 additions & 3 deletions src/wp-includes/class-wp-theme.php
Expand Up @@ -1972,14 +1972,17 @@ public function get_block_patterns() {
* Gets block pattern cache.
*
* @since 6.4.0
* @since 6.6.0 Uses transients to cache regardless of site environment.
*
* @return array|false Returns an array of patterns if cache is found, otherwise false.
*/
private function get_pattern_cache() {
if ( ! $this->exists() ) {
return false;
}
$pattern_data = wp_cache_get( 'wp_theme_patterns_' . $this->stylesheet, 'theme_files' );

$pattern_data = get_site_transient( 'wp_theme_files_patterns-' . $this->cache_hash );

if ( is_array( $pattern_data ) && $pattern_data['version'] === $this->get( 'Version' ) ) {
return $pattern_data['patterns'];
}
Expand All @@ -1990,6 +1993,7 @@ private function get_pattern_cache() {
* Sets block pattern cache.
*
* @since 6.4.0
* @since 6.6.0 Uses transients to cache regardless of site environment.
*
* @param array $patterns Block patterns data to set in cache.
*/
Expand All @@ -1998,16 +2002,43 @@ private function set_pattern_cache( array $patterns ) {
'version' => $this->get( 'Version' ),
'patterns' => $patterns,
);
wp_cache_set( 'wp_theme_patterns_' . $this->stylesheet, $pattern_data, 'theme_files' );

/**
* Filters the cache expiration time for theme files.
*
* @since 6.6.0
*
* @param int $cache_expiration Cache expiration time in seconds.
* @param string $cache_type Type of cache being set.
*/
$cache_expiration = (int) apply_filters( 'wp_theme_files_cache_ttl', self::$cache_expiration, 'theme_block_patterns' );

// We don't want to cache patterns infinitely.
if ( $cache_expiration <= 0 ) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a blocker, but I don't think we should disallow setting these to never expire. I think a better validation would be to not cast the return value above and throw a doing it wrong if the return value is not an int. That would keep someone returning false thinking they were turning off caching from accidentally caching this indefinitely. Happy to go with this for now and consider as a follow-up.

Copy link
Member

Choose a reason for hiding this comment

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

@joemcgill That would work for me, though my follow up question would be how you would "sanitize" the false then? Simply use the original self::$cache_expiration?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, we'd basically do the same thing we're already doing, but instead of if ( $cache_expiration <= 0 ) { ... we'd do if ( ! is_int( $cache_expiration ) { ....

Copy link
Member

Choose a reason for hiding this comment

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

@kt-12 had originally implemented it this way actually and I had suggested to be more "forgiving" and cast to integer instead of warning when not returning an integer. Your argument around false makes sense to me, but my concern was more about someone returning '30' and that value failing, which really it doesn't have to fail since a string containing a number could be perfectly fine to let through (and cast to an integer).

But I'm onboard with your suggestion, and even with my caveat, there's always a benefit to use strict typing anyway.

_doing_it_wrong(
__METHOD__,
sprintf(
/* translators: %1$s: The filter name.*/
__( 'The %1$s filter must return an integer value greater than 0.' ),
'<code>wp_theme_files_cache_ttl</code>'
),
'6.6.0'
);

$cache_expiration = self::$cache_expiration;
}

set_site_transient( 'wp_theme_files_patterns-' . $this->cache_hash, $pattern_data, $cache_expiration );
}

/**
* Clears block pattern cache.
*
* @since 6.4.0
* @since 6.6.0 Uses transients to cache regardless of site environment.
*/
public function delete_pattern_cache() {
wp_cache_delete( 'wp_theme_patterns_' . $this->stylesheet, 'theme_files' );
delete_site_transient( 'wp_theme_files_patterns-' . $this->cache_hash );
}

/**
Expand Down
99 changes: 99 additions & 0 deletions tests/phpunit/tests/theme/wpThemeGetBlockPatterns.php
Expand Up @@ -12,6 +12,23 @@
* @covers WP_Theme::get_block_patterns
*/
class Tests_Theme_WPThemeGetBlockPatterns extends WP_UnitTestCase {
/**
* The initial cache object.
*
* @var object
*/
private $initial_cache_object;

public function set_up() {
parent::set_up();

$this->initial_cache_object = wp_using_ext_object_cache();
}

public function tear_down() {
wp_using_ext_object_cache( $this->initial_cache_object );
parent::tear_down();
}

public static function wpSetUpBeforeClass() {
// Ensure development mode is reset before running these tests.
Expand Down Expand Up @@ -39,6 +56,20 @@ private function get_pattern_cache( $wp_theme ) {
return $pattern_cache;
}

/**
* Test helper to access the private cache_hash propery of a theme.
*
* @param WP_Theme $wp_theme A WP_Theme object.
* @return array|false Returns an array of patterns if cache is found, otherwise false.
*/
private function get_cache_hash( $wp_theme ) {
$reflection = new ReflectionProperty( get_class( $wp_theme ), 'cache_hash' );
$reflection->setAccessible( true );
$cache_hash = $reflection->getValue( $wp_theme );
$reflection->setAccessible( false );
return $cache_hash;
}

/**
* @ticket 59490
*
Expand Down Expand Up @@ -196,4 +227,72 @@ public function test_should_clear_existing_cache_when_in_development_mode() {
'The cache for block theme patterns should have been cleared due to theme development mode.'
);
}

/**
* @ticket 59600
*
* @covers WP_Theme::delete_pattern_cache
*/
public function test_delete_pattern_cache_non_obj_cache() {
// Ensure object cache is disabled.
wp_using_ext_object_cache( false );

$theme = wp_get_theme( 'block-theme-patterns' );

$this->assertTrue( $theme->exists(), 'The test theme could not be found.' );

$theme->get_block_patterns();

$this->assertSameSets(
array(
'cta.php' => array(
'title' => 'Centered Call To Action',
'slug' => 'block-theme-patterns/cta',
'description' => '',
'categories' => array( 'call-to-action' ),
),
),
$this->get_pattern_cache( $theme ),
'The cache for block theme patterns should match the expected.'
);
$theme->delete_pattern_cache();
$this->assertFalse(
$this->get_pattern_cache( $theme ),
'The cache for block theme patterns should have been cleared.'
);
}

/**
* Check if the pattern cache is stored in transient if object cache is not present.
*
* @ticket 59600
*/
public function test_pattern_transient_cache_for_non_cache_site() {
// Ensure object cache is disabled.
wp_using_ext_object_cache( false );

$theme = wp_get_theme( 'block-theme-patterns' );
$theme->get_block_patterns();

$transient_key = 'wp_theme_files_patterns-' . $this->get_cache_hash( $theme );
$transient_value = get_site_transient( $transient_key );

$this->assertSameSets(
array(
'cta.php' => array(
'title' => 'Centered Call To Action',
'slug' => 'block-theme-patterns/cta',
'description' => '',
'categories' => array( 'call-to-action' ),
),
),
$transient_value['patterns'],
'The transient value should match the expected.'
);

$this->assertNotEmpty(
$this->get_pattern_cache( $theme ),
'The cache for block theme patterns is empty.'
);
}
}