Skip to content

Commit

Permalink
Jetpack Connection: Prevent unnecessary jetpack_connection_active_plu…
Browse files Browse the repository at this point in the history 
…gins option updates (#36896)

* Jetpack Connection: Only attempt to refresh connected active plugins when active plugins change
  • Loading branch information
@fgiannar
fgiannar authored Apr 17, 2024
1 parent 050970a commit f56c7a4
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 15 deletions.
4 changes: 4 additions & 0 deletions projects/packages/connection/changelog/update-connection-jetpack_connection_active_plugins
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Jetpack Connection: Prevent unnecessary jetpack_connection_active_plugins option updates
59 changes: 44 additions & 15 deletions projects/packages/connection/src/class-plugin-storage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,16 @@ class Plugin_Storage {
const PLUGINS_DISABLED_OPTION_NAME = 'jetpack_connection_disabled_plugins';

/**
* Whether this class was configured for the first time or not.
*
* @var boolean
* Transient name used as flag to indicate that the active connected plugins list needs refreshing.
*/
private static $configured = false;
const ACTIVE_PLUGINS_REFRESH_FLAG = 'jetpack_connection_active_plugins_refresh';

/**
* Refresh list of connected plugins upon intialization.
* Whether this class was configured for the first time or not.
*
* @var boolean
*/
private static $refresh_connected_plugins = false;
private static $configured = false;

/**
* Connected plugins.
Expand Down Expand Up @@ -65,11 +63,6 @@ class Plugin_Storage {
public static function upsert( $slug, array $args = array() ) {
self::$plugins[ $slug ] = $args;

// if plugin is not in the list of active plugins, refresh the list.
if ( ! array_key_exists( $slug, (array) get_option( self::ACTIVE_PLUGINS_OPTION_NAME, array() ) ) ) {
self::$refresh_connected_plugins = true;
}

return true;
}

Expand Down Expand Up @@ -167,6 +160,44 @@ public static function configure() {
return;
}

self::$configured = true;

add_action( 'update_option_active_plugins', array( __CLASS__, 'set_flag_to_refresh_active_connected_plugins' ) );

self::maybe_update_active_connected_plugins();
}

/**
* Set a flag to indicate that the active connected plugins list needs to be updated.
* This will happen when the `active_plugins` option is updated.
*
* @see configure
*/
public static function set_flag_to_refresh_active_connected_plugins() {
set_transient( self::ACTIVE_PLUGINS_REFRESH_FLAG, time() );
}

/**
* Determine if we need to update the active connected plugins list.
*/
public static function maybe_update_active_connected_plugins() {
$maybe_error = self::ensure_configured();

if ( $maybe_error instanceof WP_Error ) {
return;
}
// Only attempt to update the option if the corresponding flag is set.
if ( ! get_transient( self::ACTIVE_PLUGINS_REFRESH_FLAG ) ) {
return;
}
// Only attempt to update the option on POST requests.
// This will prevent the option from being updated multiple times due to concurrent requests.
if ( ! ( isset( $_SERVER['REQUEST_METHOD'] ) && 'POST' === $_SERVER['REQUEST_METHOD'] ) ) {
return;
}

delete_transient( self::ACTIVE_PLUGINS_REFRESH_FLAG );

if ( is_multisite() ) {
self::$current_blog_id = get_current_blog_id();
}
Expand All @@ -175,11 +206,9 @@ public static function configure() {
// self::$plugins is populated in Config::ensure_options_connection().
$number_of_plugins_differ = count( self::$plugins ) !== count( (array) get_option( self::ACTIVE_PLUGINS_OPTION_NAME, array() ) );

if ( $number_of_plugins_differ || true === self::$refresh_connected_plugins ) {
if ( $number_of_plugins_differ ) {
self::update_active_plugins_option();
}

self::$configured = true;
}

/**
Expand All @@ -188,7 +217,7 @@ public static function configure() {
* @return void
*/
public static function update_active_plugins_option() {
// Note: Since this options is synced to wpcom, if you change its structure, you have to update the sanitizer at wpcom side.
// Note: Since this option is synced to wpcom, if you change its structure, you have to update the sanitizer at wpcom side.
update_option( self::ACTIVE_PLUGINS_OPTION_NAME, self::$plugins );

if ( ! class_exists( 'Automattic\Jetpack\Sync\Settings' ) || ! \Automattic\Jetpack\Sync\Settings::is_sync_enabled() ) {
Expand Down
107 changes: 107 additions & 0 deletions projects/packages/connection/tests/php/test_plugin_storage.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,24 @@ public function set_up() {
* @after
*/
public function tear_down() {
unset( $_SERVER['REQUEST_METHOD'] );
$this->http_request_attempted = false;
Constants::clear_constants();
WorDBless_Options::init()->clear_options();
// Reset private static properties after each test.
$reflection_class = new \ReflectionClass( '\Automattic\Jetpack\Connection\Plugin_Storage' );
try {
$reflection_class->setStaticPropertyValue( 'configured', false );
$reflection_class->setStaticPropertyValue( 'plugins', array() );
} catch ( \ReflectionException $e ) { // PHP 7 compat
$configured = $reflection_class->getProperty( 'configured' );
$configured->setAccessible( true );
$configured->setValue( false );

$plugins = $reflection_class->getProperty( 'plugins' );
$plugins->setAccessible( true );
$plugins->setValue( array() );
}
}

/**
Expand Down Expand Up @@ -73,6 +88,98 @@ public function test_update_active_plugins_option_without_sync_fallback_will_ret
$this->assertFalse( $this->http_request_attempted );
}

/**
* Unit test for the `Plugin_Storage::configure()` method.
*
* @covers Automattic\Jetpack\Connection\Plugin_Storage::set_flag_to_refresh_active_connected_plugins
*/
public function test_setting_flag_on_active_plugins_option_update() {
Plugin_Storage::configure();
do_action( 'update_option_active_plugins' );
$this->assertNotFalse( get_transient( Plugin_Storage::ACTIVE_PLUGINS_REFRESH_FLAG ) );
}

/**
* Unit test for the `Plugin_Storage::maybe_update_active_connected_plugins()` method.
*
* @covers Automattic\Jetpack\Connection\Plugin_Storage::maybe_update_active_connected_plugins
*/
public function test_maybe_update_active_connected_plugins_not_configured() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );

Plugin_Storage::upsert( 'dummy-slug' );
set_transient( Plugin_Storage::ACTIVE_PLUGINS_REFRESH_FLAG, microtime() );
$_SERVER['REQUEST_METHOD'] = 'POST';

add_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10, 3 );
Plugin_Storage::maybe_update_active_connected_plugins();
remove_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10 );

$this->assertFalse( $this->http_request_attempted );
}

/**
* Unit test for the `Plugin_Storage::maybe_update_active_connected_plugins()` method.
*
* @covers Automattic\Jetpack\Connection\Plugin_Storage::maybe_update_active_connected_plugins
*/
public function test_maybe_update_active_connected_plugins_flag_not_set() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );

Plugin_Storage::upsert( 'dummy-slug' );
Plugin_Storage::configure();
$_SERVER['REQUEST_METHOD'] = 'POST';

add_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10, 3 );
Plugin_Storage::maybe_update_active_connected_plugins();
remove_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10 );

$this->assertFalse( $this->http_request_attempted );
}

/**
* Unit test for the `Plugin_Storage::maybe_update_active_connected_plugins()` method.
*
* @covers Automattic\Jetpack\Connection\Plugin_Storage::maybe_update_active_connected_plugins
*/
public function test_maybe_update_active_connected_plugins_non_post_request() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );

Plugin_Storage::upsert( 'dummy-slug' );
Plugin_Storage::configure();
set_transient( Plugin_Storage::ACTIVE_PLUGINS_REFRESH_FLAG, microtime() );

add_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10, 3 );
Plugin_Storage::maybe_update_active_connected_plugins();
remove_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10 );

$this->assertFalse( $this->http_request_attempted );
}

/**
* Unit test for the `Plugin_Storage::maybe_update_active_connected_plugins()` method.
*
* @covers Automattic\Jetpack\Connection\Plugin_Storage::maybe_update_active_connected_plugins
*/
public function test_maybe_update_active_connected_plugins_success() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );

Plugin_Storage::upsert( 'dummy-slug' );
Plugin_Storage::configure();
set_transient( Plugin_Storage::ACTIVE_PLUGINS_REFRESH_FLAG, microtime() );
$_SERVER['REQUEST_METHOD'] = 'POST';

add_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10, 3 );
Plugin_Storage::maybe_update_active_connected_plugins();
remove_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10 );

$this->assertTrue( $this->http_request_attempted );
}

/**
* Intercept remote HTTP request to WP.com, and mock the response.
* Should be hooked on the `pre_http_request` filter.
Expand Down

0 comments on commit f56c7a4

Please sign in to comment.