From 32767405a4bbcd51789b37604b4fc5ec053d54fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 19 Dec 2022 12:53:21 +0100 Subject: [PATCH 01/12] wp_get_global_settings: add object cache Props felixarntz, aristath, ramonjd, spacedmonkey, anton-vlasenko, jorgefilipecosta, mmtr, mcsf --- .../global-styles-and-settings.php | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index c625166524b63..b5aff62db2864 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -21,19 +21,35 @@ * Valid values are 'all' (core, theme, and user) or 'base' (core and theme). * If empty or unknown, 'all' is used. * } + * * @return array The settings to retrieve. */ function wp_get_global_settings( $path = array(), $context = array() ) { if ( ! empty( $context['block_name'] ) ) { - $path = array_merge( array( 'blocks', $context['block_name'] ), $path ); + $new_path = array( 'blocks', $context['block_name'] ); + foreach ( $path as $subpath ) { + $new_path[] = $subpath; + } + $path = $new_path; } + // This is the default value when no origin is provided or when it is 'all'. $origin = 'custom'; - if ( isset( $context['origin'] ) && 'base' === $context['origin'] ) { + if ( + ! wp_theme_has_theme_json() || + ( isset( $context['origin'] ) && 'base' === $context['origin'] ) + ) { $origin = 'theme'; } - $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); + $cache_group = 'theme_json'; + $cache_key = 'wp_get_global_settings_' . $origin; + $settings = wp_cache_get( $cache_key, $cache_group ); + + if ( false === $settings || WP_DEBUG ) { + $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); + wp_cache_set( $cache_key, $settings, $cache_group ); + } return _wp_array_get( $settings, $path, $settings ); } @@ -281,5 +297,6 @@ function wp_theme_has_theme_json() { * @since 6.2.0 */ function wp_clean_theme_json_cache() { + wp_cache_delete( 'wp_get_global_settings', 'theme_json' ); WP_Theme_JSON_Resolver::clean_cached_data(); } From 22081b2d7bdf0897863a0f9c25097ee0cd5ac820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 19 Dec 2022 17:46:21 +0100 Subject: [PATCH 02/12] Set theme_json as non-persistent --- src/wp-includes/load.php | 2 +- src/wp-includes/ms-blogs.php | 4 ++-- tests/phpunit/includes/abstract-testcase.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/load.php b/src/wp-includes/load.php index 4c212086df34e..c2450e582f958 100644 --- a/src/wp-includes/load.php +++ b/src/wp-includes/load.php @@ -753,7 +753,7 @@ function wp_start_object_cache() { ) ); - wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) ); + wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) ); } $first_init = false; diff --git a/src/wp-includes/ms-blogs.php b/src/wp-includes/ms-blogs.php index 0ea6f85627dcb..7ff830e930863 100644 --- a/src/wp-includes/ms-blogs.php +++ b/src/wp-includes/ms-blogs.php @@ -575,7 +575,7 @@ function switch_to_blog( $new_blog_id, $deprecated = null ) { ); } - wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) ); + wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) ); } } @@ -666,7 +666,7 @@ function restore_current_blog() { ); } - wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) ); + wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) ); } } diff --git a/tests/phpunit/includes/abstract-testcase.php b/tests/phpunit/includes/abstract-testcase.php index bae787a42dee3..fa048cdfa703a 100644 --- a/tests/phpunit/includes/abstract-testcase.php +++ b/tests/phpunit/includes/abstract-testcase.php @@ -401,7 +401,7 @@ public static function flush_cache() { ) ); - wp_cache_add_non_persistent_groups( array( 'counts', 'plugins' ) ); + wp_cache_add_non_persistent_groups( array( 'counts', 'plugins', 'theme_json' ) ); } /** From 89fe7ac01f9ffcbd24c5b32cb250aeb567be243e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 19 Dec 2022 17:48:39 +0100 Subject: [PATCH 03/12] Document theme_json group is non-persistent --- src/wp-includes/global-styles-and-settings.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index b5aff62db2864..ae1ab1f9463f4 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -42,6 +42,22 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $origin = 'theme'; } + /* + * By using the 'theme_json' group, this data is marked to be non-persistent across requests. + * See `wp_cache_add_non_persistent_groups` in src/wp-includes/load.php and other places. + * + * The rationale for this is to make sure derived data from theme.json + * is always fresh from the potential modifications done via hooks + * that can use dynamic data (modify the stylesheet depending on some option, + * settings depending on user permissions, etc.). + * See some of the existing hooks to modify theme.json behaviour: + * https://make.wordpress.org/core/2022/10/10/filters-for-theme-json-data/ + * + * A different alternative considered was to invalidate the cache upon certain + * events such as options add/update/delete, user meta, etc. + * It was judged not enough, hence this approach. + * See https://github.com/WordPress/gutenberg/pull/45372 + */ $cache_group = 'theme_json'; $cache_key = 'wp_get_global_settings_' . $origin; $settings = wp_cache_get( $cache_key, $cache_group ); From a373a348178459b13b77652ae87eb8865e06200a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Fri, 20 Jan 2023 04:51:10 +0100 Subject: [PATCH 04/12] Add comment about use of WP_DEBUG --- src/wp-includes/global-styles-and-settings.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index ae1ab1f9463f4..65348867f321a 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -62,6 +62,13 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $cache_key = 'wp_get_global_settings_' . $origin; $settings = wp_cache_get( $cache_key, $cache_group ); + /* + * + * Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme + * developer's workflow. + * + * @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core. + */ if ( false === $settings || WP_DEBUG ) { $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); wp_cache_set( $cache_key, $settings, $cache_group ); From 40a4a51074fe23c919449e0da309e253b65780d1 Mon Sep 17 00:00:00 2001 From: Tonya Mork Date: Thu, 26 Jan 2023 12:14:55 -0600 Subject: [PATCH 05/12] Use $can_use_cached for consistency Co-authored-by: Felix Arntz --- src/wp-includes/global-styles-and-settings.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index 65348867f321a..945395b0bb2e1 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -60,18 +60,25 @@ function wp_get_global_settings( $path = array(), $context = array() ) { */ $cache_group = 'theme_json'; $cache_key = 'wp_get_global_settings_' . $origin; - $settings = wp_cache_get( $cache_key, $cache_group ); /* - * * Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme * developer's workflow. * * @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core. */ - if ( false === $settings || WP_DEBUG ) { + $can_use_cached = ! WP_DEBUG; + + $settings = false; + if ( $can_use_cached ) { + $settings = wp_cache_get( $cache_key, $cache_group ); + } + + if ( false === $settings ) { $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); - wp_cache_set( $cache_key, $settings, $cache_group ); + if ( $can_use_cached ) { + wp_cache_set( $cache_key, $settings, $cache_group ); + } } return _wp_array_get( $settings, $path, $settings ); From 2964c1d80af56b24544d634ba6ef47bb1517fa3c Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 26 Jan 2023 15:05:52 -0800 Subject: [PATCH 06/12] Fix indentation. --- src/wp-includes/global-styles-and-settings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index 945395b0bb2e1..2a5272d2ca4b9 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -71,7 +71,7 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $settings = false; if ( $can_use_cached ) { - $settings = wp_cache_get( $cache_key, $cache_group ); + $settings = wp_cache_get( $cache_key, $cache_group ); } if ( false === $settings ) { From 765c5c6af480514518bf933e5ab1ae2d3a628e4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 19 Dec 2022 12:53:21 +0100 Subject: [PATCH 07/12] wp_get_global_settings: add object cache Props felixarntz, aristath, ramonjd, spacedmonkey, anton-vlasenko, jorgefilipecosta, mmtr, mcsf --- .../global-styles-and-settings.php | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index a814b5fe3b7c8..4333b0f3008ab 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -21,19 +21,35 @@ * Valid values are 'all' (core, theme, and user) or 'base' (core and theme). * If empty or unknown, 'all' is used. * } + * * @return array The settings to retrieve. */ function wp_get_global_settings( $path = array(), $context = array() ) { if ( ! empty( $context['block_name'] ) ) { - $path = array_merge( array( 'blocks', $context['block_name'] ), $path ); + $new_path = array( 'blocks', $context['block_name'] ); + foreach ( $path as $subpath ) { + $new_path[] = $subpath; + } + $path = $new_path; } + // This is the default value when no origin is provided or when it is 'all'. $origin = 'custom'; - if ( isset( $context['origin'] ) && 'base' === $context['origin'] ) { + if ( + ! wp_theme_has_theme_json() || + ( isset( $context['origin'] ) && 'base' === $context['origin'] ) + ) { $origin = 'theme'; } - $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); + $cache_group = 'theme_json'; + $cache_key = 'wp_get_global_settings_' . $origin; + $settings = wp_cache_get( $cache_key, $cache_group ); + + if ( false === $settings || WP_DEBUG ) { + $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); + wp_cache_set( $cache_key, $settings, $cache_group ); + } return _wp_array_get( $settings, $path, $settings ); } @@ -317,5 +333,6 @@ function wp_theme_has_theme_json() { */ function wp_clean_theme_json_cache() { wp_cache_delete( 'wp_get_global_stylesheet', 'theme_json' ); + wp_cache_delete( 'wp_get_global_settings', 'theme_json' ); WP_Theme_JSON_Resolver::clean_cached_data(); } From 50cb3208188b539ef219693563f6a448cef191bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Mon, 19 Dec 2022 17:48:39 +0100 Subject: [PATCH 08/12] Document theme_json group is non-persistent --- src/wp-includes/global-styles-and-settings.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index 4333b0f3008ab..6590674409477 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -42,6 +42,22 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $origin = 'theme'; } + /* + * By using the 'theme_json' group, this data is marked to be non-persistent across requests. + * See `wp_cache_add_non_persistent_groups` in src/wp-includes/load.php and other places. + * + * The rationale for this is to make sure derived data from theme.json + * is always fresh from the potential modifications done via hooks + * that can use dynamic data (modify the stylesheet depending on some option, + * settings depending on user permissions, etc.). + * See some of the existing hooks to modify theme.json behaviour: + * https://make.wordpress.org/core/2022/10/10/filters-for-theme-json-data/ + * + * A different alternative considered was to invalidate the cache upon certain + * events such as options add/update/delete, user meta, etc. + * It was judged not enough, hence this approach. + * See https://github.com/WordPress/gutenberg/pull/45372 + */ $cache_group = 'theme_json'; $cache_key = 'wp_get_global_settings_' . $origin; $settings = wp_cache_get( $cache_key, $cache_group ); From 16ac7c2ef1f44de664813c9bbc12a7d0c9a631f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Fri, 20 Jan 2023 04:51:10 +0100 Subject: [PATCH 09/12] Add comment about use of WP_DEBUG --- src/wp-includes/global-styles-and-settings.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index 6590674409477..0989dc0c57a57 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -62,6 +62,13 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $cache_key = 'wp_get_global_settings_' . $origin; $settings = wp_cache_get( $cache_key, $cache_group ); + /* + * + * Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme + * developer's workflow. + * + * @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core. + */ if ( false === $settings || WP_DEBUG ) { $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); wp_cache_set( $cache_key, $settings, $cache_group ); From 7328c89b85f76b33f7b9768594757970d5adabb1 Mon Sep 17 00:00:00 2001 From: Tonya Mork Date: Thu, 26 Jan 2023 12:14:55 -0600 Subject: [PATCH 10/12] Use $can_use_cached for consistency Co-authored-by: Felix Arntz --- src/wp-includes/global-styles-and-settings.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index 0989dc0c57a57..dd315c0f97fba 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -60,18 +60,25 @@ function wp_get_global_settings( $path = array(), $context = array() ) { */ $cache_group = 'theme_json'; $cache_key = 'wp_get_global_settings_' . $origin; - $settings = wp_cache_get( $cache_key, $cache_group ); /* - * * Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme * developer's workflow. * * @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core. */ - if ( false === $settings || WP_DEBUG ) { + $can_use_cached = ! WP_DEBUG; + + $settings = false; + if ( $can_use_cached ) { + $settings = wp_cache_get( $cache_key, $cache_group ); + } + + if ( false === $settings ) { $settings = WP_Theme_JSON_Resolver::get_merged_data( $origin )->get_settings(); - wp_cache_set( $cache_key, $settings, $cache_group ); + if ( $can_use_cached ) { + wp_cache_set( $cache_key, $settings, $cache_group ); + } } return _wp_array_get( $settings, $path, $settings ); From 2d91cbd97b5599c7a1ddee8a9cdf85425ec670ad Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 26 Jan 2023 15:05:52 -0800 Subject: [PATCH 11/12] Fix indentation. --- src/wp-includes/global-styles-and-settings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index dd315c0f97fba..66109e3b83eea 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -71,7 +71,7 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $settings = false; if ( $can_use_cached ) { - $settings = wp_cache_get( $cache_key, $cache_group ); + $settings = wp_cache_get( $cache_key, $cache_group ); } if ( false === $settings ) { From 8a47ba6c47c875a0062347f39ebc67f6c496b5cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9?= <583546+oandregal@users.noreply.github.com> Date: Fri, 27 Jan 2023 10:40:33 +0100 Subject: [PATCH 12/12] Delete proper cache keys --- src/wp-includes/global-styles-and-settings.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/global-styles-and-settings.php b/src/wp-includes/global-styles-and-settings.php index 66109e3b83eea..af8b3859a928e 100644 --- a/src/wp-includes/global-styles-and-settings.php +++ b/src/wp-includes/global-styles-and-settings.php @@ -33,7 +33,12 @@ function wp_get_global_settings( $path = array(), $context = array() ) { $path = $new_path; } - // This is the default value when no origin is provided or when it is 'all'. + /* + * This is the default value when no origin is provided or when it is 'all'. + * + * The $origin is used as part of the cache key. Changes here need to account + * for clearing the cache appropiately. + */ $origin = 'custom'; if ( ! wp_theme_has_theme_json() || @@ -363,6 +368,7 @@ function wp_theme_has_theme_json() { */ function wp_clean_theme_json_cache() { wp_cache_delete( 'wp_get_global_stylesheet', 'theme_json' ); - wp_cache_delete( 'wp_get_global_settings', 'theme_json' ); + wp_cache_delete( 'wp_get_global_settings_custom', 'theme_json' ); + wp_cache_delete( 'wp_get_global_settings_theme', 'theme_json' ); WP_Theme_JSON_Resolver::clean_cached_data(); }