Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Add hooks for external storage provider registration: `jetpack_external_storage_init` fires before the first storage read, and `jetpack_external_storage_provider_registered` fires after a provider is registered (invalidating cached connection status).
42 changes: 42 additions & 0 deletions projects/packages/connection/src/class-external-storage.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ class External_Storage {
*/
private static $provider = null;

/**
* Whether the init action has already fired.
*
* @since $$next-version$$
*
* @var bool
*/
private static $init_fired = false;

/**
* Static cache to prevent logging same event multiple times in single request.
*
Expand Down Expand Up @@ -77,6 +86,19 @@ class External_Storage {
*/
public static function register_provider( Storage_Provider_Interface $provider ) {
self::$provider = $provider;

/**
* Fires after an external storage provider is registered.
*
* This allows dependent systems (like the connection status cache in Manager)
* to invalidate state that may have been computed before the provider was available.
*
* @since $$next-version$$
*
* @param Storage_Provider_Interface $provider The registered storage provider.
*/
do_action( 'jetpack_external_storage_provider_registered', $provider );

return true;
}

Expand All @@ -91,6 +113,26 @@ public static function register_provider( Storage_Provider_Interface $provider )
* @return mixed The value from external storage, or null for database fallback.
*/
public static function get_value( $key ) {
if ( ! self::$init_fired ) {
self::$init_fired = true;

/**
* Fires before the first external storage read.
*
* Use this hook to register your storage provider via
* External_Storage::register_provider(). This fires after the connection
* package classes are loaded but before any connection status checks read
* from external storage.
*
* Useful for mu-plugins that load before the plugin providing External_Storage,
* since add_action() does not require the action or any classes to exist at
* hook-registration time.
*
* @since $$next-version$$
*/
do_action( 'jetpack_external_storage_init' );
}

$provider = self::$provider;

// Check if we have a registered provider
Expand Down
1 change: 1 addition & 0 deletions projects/packages/connection/src/class-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ private function add_connection_status_invalidation_hooks() {
add_action( 'pre_update_jetpack_option_master_user', array( $this, 'reset_connection_status' ) );
// phpcs:ignore WPCUT.SwitchBlog.SwitchBlog -- wpcom flags **every** use of switch_blog, apparently expecting valid instances to ignore or suppress the sniff.
add_action( 'switch_blog', array( $this, 'reset_connection_status' ) );
add_action( 'jetpack_external_storage_provider_registered', array( $this, 'reset_connection_status' ) );

self::$connection_invalidators_added = true;
}
Expand Down
92 changes: 92 additions & 0 deletions projects/packages/connection/tests/php/External_Storage_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ public function tearDown(): void {
$logged_events_property->setAccessible( true );
}
$logged_events_property->setValue( null, array() );

// Reset the init_fired flag
$init_fired_property = $reflection->getProperty( 'init_fired' );
// @todo Remove this call once we no longer need to support PHP <8.1.
if ( PHP_VERSION_ID < 80100 ) {
$init_fired_property->setAccessible( true );
}
$init_fired_property->setValue( null, false );

// Remove any action callbacks added during tests
remove_all_filters( 'jetpack_external_storage_init' );
remove_all_filters( 'jetpack_external_storage_provider_registered' );
}

/**
Expand Down Expand Up @@ -248,4 +260,84 @@ public function handle_error_event( $event_type, $key, $details, $environment )
$reflection = new \ReflectionMethod( $provider, 'get_empty_state_delay_threshold' );
$this->assertSame( 90, $reflection->invoke( $provider ) );
}

/**
* Test that jetpack_external_storage_init fires on first get_value() call.
*/
public function test_init_action_fires_on_first_get_value() {
$fired = 0;
add_action(
'jetpack_external_storage_init',
function () use ( &$fired ) {
++$fired;
}
);

External_Storage::get_value( 'id' );
$this->assertSame( 1, $fired, 'Init action should fire on first get_value() call' );

External_Storage::get_value( 'blog_token' );
$this->assertSame( 1, $fired, 'Init action should not fire again on subsequent calls' );
}

/**
* Test that a provider registered via the init action is used by the same get_value() call.
*/
public function test_init_action_allows_late_provider_registration() {
$provider = new class() implements \Automattic\Jetpack\Connection\Storage_Provider_Interface {
public function is_available() {
return true;
}
public function should_handle( $option_name ) {
return 'id' === $option_name;
}
public function get( $option_name ) {
return 'id' === $option_name ? 12345 : null;
}
public function get_environment_id() {
return 'test-late';
}
};

add_action(
'jetpack_external_storage_init',
function () use ( $provider ) {
External_Storage::register_provider( $provider );
}
);

$value = External_Storage::get_value( 'id' );
$this->assertSame( 12345, $value, 'Provider registered during init action should serve the triggering read' );
}

/**
* Test that jetpack_external_storage_provider_registered fires on register_provider().
*/
public function test_provider_registered_action_fires() {
$received_provider = null;
add_action(
'jetpack_external_storage_provider_registered',
function ( $provider ) use ( &$received_provider ) {
$received_provider = $provider;
}
);

$provider = new class() implements \Automattic\Jetpack\Connection\Storage_Provider_Interface {
public function is_available() {
return true;
}
public function should_handle( $option_name ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
return true;
}
public function get( $option_name ) { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable
return null;
}
public function get_environment_id() {
return 'test-registered';
}
};

External_Storage::register_provider( $provider );
$this->assertSame( $provider, $received_provider, 'Registered action should pass the provider instance' );
}
}
63 changes: 63 additions & 0 deletions projects/packages/connection/tests/php/ManagerIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -737,4 +737,67 @@ public static function get_disconnect_user_outcomes() {
),
);
}

/**
* Test that registering an external storage provider after is_connected() resets the cached status.
*/
public function test_registering_provider_invalidates_stale_is_connected() {
// Ensure clean state: no blog id, no token in DB.
\Jetpack_Options::delete_option( 'id' );
\Jetpack_Options::delete_option( 'blog_token' );
$this->reset_connection_status();

// First call: no provider, no DB data → false.
$this->assertFalse( $this->manager->is_connected() );

// Register a provider that supplies blog id and token.
$provider = new class() implements \Automattic\Jetpack\Connection\Storage_Provider_Interface {
public function is_available() {
return true;
}
public function should_handle( $option_name ) {
return in_array( $option_name, array( 'blog_token', 'id' ), true );
}
public function get( $option_name ) {
if ( 'id' === $option_name ) {
return 12345;
}
if ( 'blog_token' === $option_name ) {
return 'test.token';
}
return null;
}
public function get_environment_id() {
return 'test-invalidation';
}
};

try {
// This should fire jetpack_external_storage_provider_registered,
// which resets the memoized is_connected value.
External_Storage::register_provider( $provider );

// Next call should re-evaluate with the provider and return true.
$this->assertTrue( $this->manager->is_connected() );
} finally {
// Clean up: remove provider and reset init_fired.
$reflection = new \ReflectionClass( External_Storage::class );

$prop = $reflection->getProperty( 'provider' );
// @todo Remove this call once we no longer need to support PHP <8.1.
if ( PHP_VERSION_ID < 80100 ) {
$prop->setAccessible( true );
}
$prop->setValue( null, null );

$init_fired = $reflection->getProperty( 'init_fired' );
// @todo Remove this call once we no longer need to support PHP <8.1.
if ( PHP_VERSION_ID < 80100 ) {
$init_fired->setAccessible( true );
}
$init_fired->setValue( null, false );

$this->reset_connection_status();
}
}
}
Loading