From 7624a6e67cfbb873e91140d41b440792f4737015 Mon Sep 17 00:00:00 2001 From: Peter Petrov Date: Thu, 22 Aug 2024 11:47:57 +0300 Subject: [PATCH] Boost: Fix case where it's possible to not store CSS coming from the cloud (#38985) * Fix case where it's possible to send an empty list of providers * Fix resetting critical css state without providers for Cloud CSS * Reset regeneration flag when invalidating cloud css * add changelog * Fix incorrect number of params * Fix static analysis issues * Clean up admin notice transient when requesting cloud css --- projects/plugins/boost/.phan/baseline.php | 3 +- .../critical-css/Critical_CSS_Invalidator.php | 31 ++++++++++++++++--- .../lib/critical-css/Critical_CSS_State.php | 13 ++++++++ .../boost/app/lib/critical-css/Regenerate.php | 2 +- .../optimizations/cloud-css/Cloud_CSS.php | 2 +- .../fix-cloud-css-not-saved-on-theme-switch | 4 +++ 6 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 projects/plugins/boost/changelog/fix-cloud-css-not-saved-on-theme-switch diff --git a/projects/plugins/boost/.phan/baseline.php b/projects/plugins/boost/.phan/baseline.php index b58fae407fd1d..04f317e93cfd3 100644 --- a/projects/plugins/boost/.phan/baseline.php +++ b/projects/plugins/boost/.phan/baseline.php @@ -15,7 +15,7 @@ // PhanTypeMismatchReturnProbablyReal : 10+ occurrences // PhanTypeArraySuspicious : 9 occurrences // PhanTypeMismatchArgument : 8 occurrences - // PhanParamTooMany : 7 occurrences + // PhanParamTooMany : 6 occurrences // PhanPossiblyUndeclaredVariable : 6 occurrences // PhanUndeclaredConstant : 5 occurrences // PhanUndeclaredFunction : 4 occurrences @@ -50,7 +50,6 @@ 'app/data-sync/Performance_History_Entry.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeArraySuspicious'], 'app/lib/class-cli.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeMismatchArgument'], 'app/lib/critical-css/Critical_CSS_State.php' => ['PhanPluginDuplicateConditionalNullCoalescing', 'PhanTypeArraySuspiciousNullable'], - 'app/lib/critical-css/Regenerate.php' => ['PhanParamTooMany'], 'app/lib/critical-css/source-providers/providers/Archive_Provider.php' => ['PhanTypeMismatchReturnProbablyReal'], 'app/lib/critical-css/source-providers/providers/Post_ID_Provider.php' => ['PhanTypeMismatchReturnProbablyReal'], 'app/lib/critical-css/source-providers/providers/Provider.php' => ['PhanTypeMismatchArgumentInternal'], diff --git a/projects/plugins/boost/app/lib/critical-css/Critical_CSS_Invalidator.php b/projects/plugins/boost/app/lib/critical-css/Critical_CSS_Invalidator.php index 71ce3fd324986..e1013a4ae9d32 100644 --- a/projects/plugins/boost/app/lib/critical-css/Critical_CSS_Invalidator.php +++ b/projects/plugins/boost/app/lib/critical-css/Critical_CSS_Invalidator.php @@ -7,7 +7,11 @@ namespace Automattic\Jetpack_Boost\Lib\Critical_CSS; +use Automattic\Jetpack_Boost\Admin\Regenerate_Admin_Notice; use Automattic\Jetpack_Boost\Lib\Boost_Health; +use Automattic\Jetpack_Boost\Lib\Critical_CSS\Source_Providers\Source_Providers; +use Automattic\Jetpack_Boost\Modules\Modules_Setup; +use Automattic\Jetpack_Boost\Modules\Optimizations\Cloud_CSS\Cloud_CSS; use Automattic\Jetpack_Boost\Modules\Optimizations\Cloud_CSS\Cloud_CSS_Followup; /** @@ -21,21 +25,35 @@ class Critical_CSS_Invalidator { * Register hooks. */ public static function init() { - add_action( 'jetpack_boost_deactivate', array( __CLASS__, 'clear_data' ) ); + add_action( 'jetpack_boost_deactivate', array( __CLASS__, 'reset_data' ) ); add_action( 'jetpack_boost_critical_css_environment_changed', array( __CLASS__, 'handle_environment_change' ) ); add_filter( 'jetpack_boost_total_problem_count', array( __CLASS__, 'update_boost_problem_count' ) ); } /** - * Clear Critical CSS data. + * Reset Critical CSS data. + * For Cloud CSS, we need to make sure there are providers in the state, + * otherwise Cloud CSS cannot be stored after it is generated by the cloud. */ - public static function clear_data() { + public static function reset_data() { $storage = new Critical_CSS_Storage(); $storage->clear(); $state = new Critical_CSS_State(); $state->clear(); + if ( self::is_cloud_css() ) { + $state->prepare_for_generation( ( new Source_Providers() )->get_provider_sources() ); + $state->save(); + + // Clear the regenerate flag so things are nice and tidy. + jetpack_boost_ds_delete( 'critical_css_suggest_regenerate' ); + // Also clear the admin notice flag, so the notice doesn't show up in case + // the user is reverted to free Boost. + Regenerate_Admin_Notice::dismiss(); + + } + Cloud_CSS_Followup::unschedule(); } @@ -44,7 +62,7 @@ public static function clear_data() { */ public static function handle_environment_change( $is_major_change ) { if ( $is_major_change ) { - self::clear_data(); + self::reset_data(); do_action( 'jetpack_boost_critical_css_invalidated' ); } @@ -58,4 +76,9 @@ public static function update_boost_problem_count( $count ) { return $count; } + + public static function is_cloud_css() { + $optimizations = ( new Modules_Setup() )->get_status(); + return isset( $optimizations[ Cloud_CSS::get_slug() ] ) && $optimizations[ Cloud_CSS::get_slug() ]; + } } diff --git a/projects/plugins/boost/app/lib/critical-css/Critical_CSS_State.php b/projects/plugins/boost/app/lib/critical-css/Critical_CSS_State.php index 77337d121c297..07e7c978b5132 100644 --- a/projects/plugins/boost/app/lib/critical-css/Critical_CSS_State.php +++ b/projects/plugins/boost/app/lib/critical-css/Critical_CSS_State.php @@ -186,6 +186,19 @@ public function set_pending_providers( $providers ) { return $this; } + /** + * Add providers to the state, sets their status to pending + * and sets the generation status to pending. + * + * @param array $providers The providers to include in the state and set as pending. + * @return $this + */ + public function prepare_for_generation( $providers ) { + $this->set_pending_providers( $providers ); + $this->state['status'] = self::GENERATION_STATES['pending']; + return $this; + } + /** * Get fresh state */ diff --git a/projects/plugins/boost/app/lib/critical-css/Regenerate.php b/projects/plugins/boost/app/lib/critical-css/Regenerate.php index 9a74b501352b9..6b25fdf434c3c 100644 --- a/projects/plugins/boost/app/lib/critical-css/Regenerate.php +++ b/projects/plugins/boost/app/lib/critical-css/Regenerate.php @@ -45,7 +45,7 @@ public function start() { // Dismiss admin notices Regenerate_Admin_Notice::dismiss(); - jetpack_boost_ds_delete( 'critical_css_suggest_regenerate', null ); + jetpack_boost_ds_delete( 'critical_css_suggest_regenerate' ); return $data; } diff --git a/projects/plugins/boost/app/modules/optimizations/cloud-css/Cloud_CSS.php b/projects/plugins/boost/app/modules/optimizations/cloud-css/Cloud_CSS.php index c16f51b452d99..8664505d583cf 100644 --- a/projects/plugins/boost/app/modules/optimizations/cloud-css/Cloud_CSS.php +++ b/projects/plugins/boost/app/modules/optimizations/cloud-css/Cloud_CSS.php @@ -178,7 +178,7 @@ public function handle_critical_css_invalidated() { public function get_existing_sources() { $state = new Critical_CSS_State(); $data = $state->get(); - if ( isset( $data['providers'] ) ) { + if ( ! empty( $data['providers'] ) ) { $providers = $data['providers']; } else { $source_providers = new Source_Providers(); diff --git a/projects/plugins/boost/changelog/fix-cloud-css-not-saved-on-theme-switch b/projects/plugins/boost/changelog/fix-cloud-css-not-saved-on-theme-switch new file mode 100644 index 0000000000000..b583faf49b911 --- /dev/null +++ b/projects/plugins/boost/changelog/fix-cloud-css-not-saved-on-theme-switch @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Cloud CSS: Fixed not properly storing CSS returned from the cloud after a theme switch.