Skip to content

Commit

Permalink
Connection\Manager: is_connected: Memoize (#39361)
Browse files Browse the repository at this point in the history
  • Loading branch information
mreishus authored Sep 23, 2024
1 parent 1d8d319 commit 202fec8
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 3 deletions.
5 changes: 5 additions & 0 deletions projects/packages/connection/changelog/update-is-connected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Internal performance change for is_connected()


65 changes: 62 additions & 3 deletions projects/packages/connection/src/class-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,20 @@ class Manager {
*/
private static $disconnected_users = array();

/**
* Cached connection status.
*
* @var bool|null True if the site is connected, false if not, null if not determined yet.
*/
private static $is_connected = null;

/**
* Tracks whether connection status invalidation hooks have been added.
*
* @var bool
*/
private static $connection_invalidators_added = false;

/**
* Initialize the object.
* Make sure to call the "Configure" first.
Expand Down Expand Up @@ -140,6 +154,8 @@ public static function configure() {
add_action( 'deleted_user', array( $manager, 'disconnect_user_force' ), 9, 1 );
add_action( 'remove_user_from_blog', array( $manager, 'disconnect_user_force' ), 9, 1 );

$manager->add_connection_status_invalidation_hooks();

// Set up package version hook.
add_filter( 'jetpack_package_versions', __NAMESPACE__ . '\Package_Version::send_package_version_to_tracker' );

Expand All @@ -157,6 +173,27 @@ public static function configure() {
Partner::init();
}

/**
* Adds hooks to invalidate the memoized connection status.
*/
private function add_connection_status_invalidation_hooks() {
if ( self::$connection_invalidators_added ) {
return;
}

// Force is_connected() to recompute after important actions.
add_action( 'jetpack_site_registered', array( $this, 'reset_connection_status' ) );
add_action( 'jetpack_site_disconnected', array( $this, 'reset_connection_status' ) );
add_action( 'jetpack_sync_register_user', array( $this, 'reset_connection_status' ) );
add_action( 'pre_update_jetpack_option_id', array( $this, 'reset_connection_status' ) );
add_action( 'pre_update_jetpack_option_blog_token', array( $this, 'reset_connection_status' ) );
add_action( 'pre_update_jetpack_option_user_token', array( $this, 'reset_connection_status' ) );
add_action( 'pre_update_jetpack_option_user_tokens', array( $this, 'reset_connection_status' ) );
add_action( 'switch_blog', array( $this, 'reset_connection_status' ) );

self::$connection_invalidators_added = true;
}

/**
* Sets up the XMLRPC request handlers.
*
Expand Down Expand Up @@ -596,9 +633,31 @@ public function is_registered() {
* @return bool
*/
public function is_connected() {
$has_blog_id = (bool) \Jetpack_Options::get_option( 'id' );
$has_blog_token = (bool) $this->get_tokens()->get_access_token();
return $has_blog_id && $has_blog_token;
if ( self::$is_connected === null ) {
if ( ! self::$connection_invalidators_added ) {
$this->add_connection_status_invalidation_hooks();
}

$has_blog_id = (bool) \Jetpack_Options::get_option( 'id' );
if ( $has_blog_id ) {
$has_blog_token = (bool) $this->get_tokens()->get_access_token();
self::$is_connected = ( $has_blog_id && $has_blog_token );
} else {
// Short-circuit, no need to check for tokens if there's no blog ID.
self::$is_connected = false;
}
}
return self::$is_connected;
}

/**
* Resets the memoized connection status.
* This will force the connection status to be recomputed on the next check.
*
* @since $$next-version$$
*/
public function reset_connection_status() {
self::$is_connected = null;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Test_Identity_Crisis extends BaseTestCase {
public function set_up() {
Constants::set_constant( 'JETPACK_DISABLE_RAW_OPTIONS', true );
StatusCache::clear();
$this->reset_connection_status();
}

/**
Expand All @@ -43,6 +44,20 @@ public function tear_down() {
$instance->setAccessible( true );
$instance->setValue( null, null );
$instance->setAccessible( false );
$this->reset_connection_status();
}

/**
* Reset the connection status.
* Needed because the connection status is memoized and not reset between tests.
* WorDBless does not fire the options update hooks that would reset the connection status.
*/
public function reset_connection_status() {
static $manager = null;
if ( ! $manager ) {
$manager = new \Automattic\Jetpack\Connection\Manager();
}
$manager->reset_connection_status();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ class Test_REST_Endpoints extends TestCase {
*/
private $api_host_original;

/**
* Reset the connection status.
* Needed because the connection status is memoized and not reset between tests.
* WorDBless does not fire the options update hooks that would reset the connection status.
*/
public function reset_connection_status() {
static $manager = null;
if ( ! $manager ) {
$manager = new \Automattic\Jetpack\Connection\Manager();
}
$manager->reset_connection_status();
}

/**
* Setting up the test.
*
Expand All @@ -57,6 +70,7 @@ public function set_up() {
Constants::set_constant( 'JETPACK__API_BASE', 'https://jetpack.wordpress.com/jetpack.' );

set_transient( 'jetpack_assumed_site_creation_date', '2020-02-28 01:13:27' );
$this->reset_connection_status();
}

/**
Expand All @@ -74,6 +88,7 @@ public function tear_down() {
$_GET = array();

Connection_Rest_Authentication::init()->reset_saved_auth_state();
$this->reset_connection_status();
}

/**
Expand Down
17 changes: 17 additions & 0 deletions projects/packages/connection/tests/php/test-rest-endpoints.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public function set_up() {
Constants::$set_constants['JETPACK__API_BASE'] = 'https://jetpack.wordpress.com/jetpack.';

set_transient( 'jetpack_assumed_site_creation_date', '2020-02-28 01:13:27' );
$this->reset_connection_status();
}

/**
Expand Down Expand Up @@ -137,6 +138,20 @@ public function tear_down() {
$_GET = array();

Connection_Rest_Authentication::init()->reset_saved_auth_state();
$this->reset_connection_status();
}

/**
* Reset the connection status.
* Needed because the connection status is memoized and not reset between tests.
* WorDBless does not fire the options update hooks that would reset the connection status.
*/
public function reset_connection_status() {
static $manager = null;
if ( ! $manager ) {
$manager = new \Automattic\Jetpack\Connection\Manager();
}
$manager->reset_connection_status();
}

/**
Expand Down Expand Up @@ -1764,6 +1779,7 @@ private function setup_reconnect_test( $invalid_token ) {
}

add_filter( 'jetpack_options', array( $this, 'mock_jetpack_options' ), 10, 2 );
$this->reset_connection_status();
}

/**
Expand Down Expand Up @@ -1816,5 +1832,6 @@ private function shutdown_reconnect_test( $invalid_token ) {
}

remove_filter( 'jetpack_options', array( $this, 'mock_jetpack_options' ), 10 );
$this->reset_connection_status();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class ManagerIntegrationTest extends \WorDBless\BaseTestCase {
public function set_up() {
$this->manager = new Manager();
Constants::set_constant( 'JETPACK__API_BASE', 'https://jetpack.wordpress.com/jetpack.' );
$this->reset_connection_status();
}

/**
Expand All @@ -36,6 +37,20 @@ public function set_up() {
*/
public function tear_down() {
Constants::clear_constants();
$this->reset_connection_status();
}

/**
* Reset the connection status.
* Needed because the connection status is memoized and not reset between tests.
* WorDBless does not fire the options update hooks that would reset the connection status.
*/
public function reset_connection_status() {
static $manager = null;
if ( ! $manager ) {
$manager = new \Automattic\Jetpack\Connection\Manager();
}
$manager->reset_connection_status();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Test_Package_Version_Tracker extends TestCase {
public function set_up() {
Constants::set_constant( 'JETPACK__WPCOM_JSON_API_BASE', 'https://public-api.wordpress.com' );
Sync_Settings::update_settings( array( 'disable' => true ) );
$this->reset_connection_status();
}

/**
Expand All @@ -66,6 +67,20 @@ public function tear_down() {

delete_transient( Package_Version_Tracker::CACHED_FAILED_REQUEST_KEY );
delete_transient( Package_Version_Tracker::RATE_LIMITER_KEY );
$this->reset_connection_status();
}

/**
* Reset the connection status.
* Needed because the connection status is memoized and not reset between tests.
* WorDBless does not fire the options update hooks that would reset the connection status.
*/
public function reset_connection_status() {
static $manager = null;
if ( ! $manager ) {
$manager = new \Automattic\Jetpack\Connection\Manager();
}
$manager->reset_connection_status();
}

/**
Expand Down Expand Up @@ -305,6 +320,7 @@ function () {
public function test_maybe_update_package_versions_with_sync_disabled_remote_request_success() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();

add_filter( 'pre_http_request', array( $this, 'intercept_http_request_success' ) );

Expand Down Expand Up @@ -360,6 +376,7 @@ function () {
public function test_maybe_update_package_versions_with_sync_disabled_remote_request_failure() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();

add_filter( 'pre_http_request', array( $this, 'intercept_http_request_failure' ) );

Expand Down Expand Up @@ -393,6 +410,7 @@ public function test_remote_package_versions_will_not_be_updated_if_a_previous_f

\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();

add_filter( 'pre_http_request', array( $this, 'intercept_http_request_failure' ) );

Expand Down
22 changes: 22 additions & 0 deletions projects/packages/connection/tests/php/test_plugin_storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class Test_Plugin_Storage extends TestCase {
public function set_up() {
Constants::set_constant( 'JETPACK__WPCOM_JSON_API_BASE', 'https://public-api.wordpress.com' );
Sync_Settings::update_settings( array( 'disable' => true ) );
$this->reset_connection_status();
}

/**
Expand All @@ -61,6 +62,20 @@ public function tear_down() {
$plugins->setAccessible( true );
$plugins->setValue( array() );
}
$this->reset_connection_status();
}

/**
* Reset the connection status.
* Needed because the connection status is memoized and not reset between tests.
* WorDBless does not fire the options update hooks that would reset the connection status.
*/
public function reset_connection_status() {
static $manager = null;
if ( ! $manager ) {
$manager = new \Automattic\Jetpack\Connection\Manager();
}
$manager->reset_connection_status();
}

/**
Expand All @@ -71,6 +86,7 @@ public function tear_down() {
public function test_update_active_plugins_option_without_sync_will_trigger_fallback() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();

add_filter( 'pre_http_request', array( $this, 'intercept_remote_request' ), 10, 3 );
Plugin_Storage::update_active_plugins_option();
Expand Down Expand Up @@ -109,6 +125,7 @@ public function test_setting_flag_on_active_plugins_option_update() {
public function test_maybe_update_active_connected_plugins_not_configured() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();

Plugin_Storage::upsert( 'dummy-slug' );
set_transient( Plugin_Storage::ACTIVE_PLUGINS_REFRESH_FLAG, microtime() );
Expand All @@ -130,6 +147,7 @@ public function test_maybe_update_active_connected_plugins_not_configured() {
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 );
$this->reset_connection_status();

Plugin_Storage::upsert( 'dummy-slug' );
Plugin_Storage::configure();
Expand All @@ -151,6 +169,7 @@ public function test_maybe_update_active_connected_plugins_flag_not_set() {
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 );
$this->reset_connection_status();

Plugin_Storage::upsert( 'dummy-slug' );
Plugin_Storage::configure();
Expand All @@ -172,6 +191,7 @@ public function test_maybe_update_active_connected_plugins_non_post_request() {
public function test_maybe_update_active_connected_plugins_success() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();

Plugin_Storage::upsert( 'dummy-slug' );
Plugin_Storage::configure();
Expand All @@ -195,6 +215,7 @@ public function test_maybe_update_active_connected_plugins_success() {
public function test_maybe_update_active_connected_plugins_success_same_plugins() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();
$stored_value = array( 'dummy-slug' => array() );
update_option( Plugin_Storage::ACTIVE_PLUGINS_OPTION_NAME, $stored_value );

Expand All @@ -220,6 +241,7 @@ public function test_maybe_update_active_connected_plugins_success_same_plugins(
public function test_maybe_update_active_connected_plugins_success_same_count_different_plugins() {
\Jetpack_Options::update_option( 'blog_token', 'asdasd.123123' );
\Jetpack_Options::update_option( 'id', 1234 );
$this->reset_connection_status();
update_option( Plugin_Storage::ACTIVE_PLUGINS_OPTION_NAME, array( 'dummy-slug2' => array() ) );

Plugin_Storage::upsert( 'dummy-slug' );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Internal unit test changes


Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ public function mock_connection() {
( new Tokens() )->update_blog_token( 'test.test' );
Jetpack_Options::update_option( 'id', 123 );
Constants::set_constant( 'JETPACK__WPCOM_JSON_API_BASE', 'https://public-api.wordpress.com' );

// Since this uses WorDBless, our typical invalidation on option update does not work, so invalidate manually
$manager = new \Automattic\Jetpack\Connection\Manager();
$manager->reset_connection_status();
}

/**
Expand Down
5 changes: 5 additions & 0 deletions projects/packages/stats/changelog/update-is-connected
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Internal unit test changes


Loading

0 comments on commit 202fec8

Please sign in to comment.