diff --git a/src/settings/class-utils.php b/src/settings/class-utils.php index a78a1ce4..2845e492 100644 --- a/src/settings/class-utils.php +++ b/src/settings/class-utils.php @@ -56,6 +56,26 @@ class Utils { */ const REQUIRED_FEATURES = [ 'title', 'editor', 'custom-fields' ]; + /** + * How long a connection check is trusted before the settings page + * re-validates. Throttles the API call to once per window per credential + * set, keeping it off the admin render hot path. + */ + const CONNECTION_CHECK_TTL = 5 * MINUTE_IN_SECONDS; + + /** + * Timeout (seconds) for the connection-check GET. Kept at the VIP-approved + * 3-second ceiling so a slow API can't stall the settings page render. + */ + const CONNECTION_CHECK_TIMEOUT = 3; + + /** + * Transient that throttles connection re-validation. Its value is a + * fingerprint of the credentials last checked, so changing the API key or + * project ID busts the throttle and forces an immediate re-check. + */ + const CONNECTION_CHECK_TRANSIENT = 'beyondwords_api_connection_checked'; + /** * Get the post types eligible for BeyondWords audio generation. * @@ -133,31 +153,68 @@ public static function has_valid_api_connection(): bool { /** * Validate the BeyondWords REST API connection and persist the result. * - * Stores the successful timestamp in `beyondwords_valid_api_connection` - * so subsequent admin page loads can short-circuit without an API call. + * The last successful check is recorded as a timestamp in the + * `beyondwords_valid_api_connection` option, which gates the visibility of + * the Integration and Preferences tabs (see `Tabs::get_visible_tabs()`). + * + * This runs on every settings-page load, so the network call is + * short-circuited to keep it off the admin render hot path: + * + * 1. A throttle transient ({@see self::CONNECTION_CHECK_TRANSIENT}), keyed to + * a fingerprint of the current credentials, limits re-validation to once + * per {@see self::CONNECTION_CHECK_TTL}. Changing the API key or project ID + * changes the fingerprint, so saving new credentials re-validates + * immediately rather than waiting out the throttle window. + * 2. The request uses a short {@see self::CONNECTION_CHECK_TIMEOUT} timeout so + * a slow API can't block the page render. + * + * The stored flag is only cleared on a definitive auth failure (401/403) — + * `Client::call_api()` clears it on 401 and we mirror that for 403 here. A + * transient failure (timeout, DNS error, 5xx, `WP_Error`) leaves the last + * known-good flag intact so an API blip can't hide the other settings tabs. */ public static function validate_api_connection(): bool { - delete_transient( 'beyondwords_validate_api_connection' ); - delete_option( 'beyondwords_valid_api_connection' ); - $project_id = get_option( 'beyondwords_project_id' ); $api_key = get_option( 'beyondwords_api_key' ); if ( ! $project_id || ! $api_key ) { + // No credentials means no connection — reflect that in the flag. + delete_option( 'beyondwords_valid_api_connection' ); return false; } + $fingerprint = md5( (string) $project_id . '|' . (string) $api_key ); + + // Throttle: within the window, trust the last recorded result for these + // exact credentials and skip the network call entirely. + if ( get_transient( self::CONNECTION_CHECK_TRANSIENT ) === $fingerprint ) { + return self::has_valid_api_connection(); + } + $url = sprintf( '%s/projects/%d', \BeyondWords\Core\Urls::get_api_url(), $project_id ); - $response = \BeyondWords\Api\Client::call_api( 'GET', $url ); + $response = \BeyondWords\Api\Client::call_api( 'GET', $url, '', false, [], self::CONNECTION_CHECK_TIMEOUT ); + + // Record the attempt whatever the outcome, so repeated page loads don't + // re-issue the request — a down API is throttled just like a healthy one. + set_transient( self::CONNECTION_CHECK_TRANSIENT, $fingerprint, self::CONNECTION_CHECK_TTL ); + + $response_code = wp_remote_retrieve_response_code( $response ); - if ( 200 === wp_remote_retrieve_response_code( $response ) ) { + if ( 200 === (int) $response_code ) { update_option( 'beyondwords_valid_api_connection', gmdate( \DateTime::ATOM ), false ); return true; } + // 403 is a definitive auth failure (revoked key or wrong project), so + // clear the flag; call_api() has already done so for 401. Every other + // response is treated as transient and leaves the flag untouched. + if ( 403 === (int) $response_code ) { + delete_option( 'beyondwords_valid_api_connection' ); + } + $debug = sprintf( '%s: %s', - wp_remote_retrieve_response_code( $response ), + $response_code, wp_remote_retrieve_body( $response ) ); diff --git a/tests/phpunit/settings/test-settings.php b/tests/phpunit/settings/test-settings.php index 7effb925..9d03cb40 100644 --- a/tests/phpunit/settings/test-settings.php +++ b/tests/phpunit/settings/test-settings.php @@ -21,12 +21,14 @@ public function setUp(): void // Your set up methods here. delete_transient('beyondwords_settings_errors'); + delete_transient(Utils::CONNECTION_CHECK_TRANSIENT); } public function tearDown(): void { // Your tear down methods here. delete_transient('beyondwords_settings_errors'); + delete_transient(Utils::CONNECTION_CHECK_TRANSIENT); // print_settings_errors() is now screen-gated; reset the screen so a // test that sets it does not leak into the review-notice tests, which // rely on no settings screen being active. diff --git a/tests/phpunit/settings/test-utils.php b/tests/phpunit/settings/test-utils.php index c8799f38..28a7283e 100644 --- a/tests/phpunit/settings/test-utils.php +++ b/tests/phpunit/settings/test-utils.php @@ -10,11 +10,13 @@ public function setUp(): void { parent::setUp(); delete_transient('beyondwords_settings_errors'); + delete_transient(Utils::CONNECTION_CHECK_TRANSIENT); } public function tearDown(): void { delete_transient('beyondwords_settings_errors'); + delete_transient(Utils::CONNECTION_CHECK_TRANSIENT); delete_option('beyondwords_api_key'); delete_option('beyondwords_project_id'); delete_option('beyondwords_valid_api_connection'); @@ -240,6 +242,169 @@ public function validate_api_connection_queues_error_on_failure() remove_filter('pre_http_request', $filter, 10); } + /** + * A transient failure (5xx, timeout, DNS error) must NOT clear a + * previously-valid connection flag — otherwise a brief API blip hides the + * Integration and Preferences tabs and locks the operator out. + * + * @test + */ + public function validate_api_connection_preserves_flag_on_server_error() + { + update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY); + update_option('beyondwords_project_id', BEYONDWORDS_TESTS_PROJECT_ID); + update_option('beyondwords_valid_api_connection', gmdate(\DateTime::ATOM), false); + + $filter = function ($preempt, $args, $url) { + return [ + 'response' => ['code' => 500, 'message' => 'Internal Server Error'], + 'body' => '{"code":500,"message":"Internal Server Error"}', + 'headers' => [], + 'cookies' => [], + ]; + }; + add_filter('pre_http_request', $filter, 10, 3); + + $this->assertFalse(Utils::validate_api_connection()); + // The last known-good flag survives the blip. + $this->assertTrue(Utils::has_valid_api_connection()); + + remove_filter('pre_http_request', $filter, 10); + } + + /** + * A transport-level failure returns a WP_Error (no HTTP status). It is + * transient, so the connection flag must be preserved. + * + * @test + */ + public function validate_api_connection_preserves_flag_on_wp_error() + { + update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY); + update_option('beyondwords_project_id', BEYONDWORDS_TESTS_PROJECT_ID); + update_option('beyondwords_valid_api_connection', gmdate(\DateTime::ATOM), false); + + $filter = function ($preempt, $args, $url) { + return new \WP_Error('http_request_failed', 'cURL error 28: Operation timed out'); + }; + add_filter('pre_http_request', $filter, 10, 3); + + $this->assertFalse(Utils::validate_api_connection()); + $this->assertTrue(Utils::has_valid_api_connection()); + + remove_filter('pre_http_request', $filter, 10); + } + + /** + * A 403 is a definitive auth failure (revoked key or wrong project), so it + * clears the connection flag just like a 401. + * + * @test + */ + public function validate_api_connection_clears_flag_on_forbidden() + { + update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY); + update_option('beyondwords_project_id', BEYONDWORDS_TESTS_PROJECT_ID); + update_option('beyondwords_valid_api_connection', gmdate(\DateTime::ATOM), false); + + $filter = function ($preempt, $args, $url) { + return [ + 'response' => ['code' => 403, 'message' => 'Forbidden'], + 'body' => '{"code":403,"message":"Forbidden"}', + 'headers' => [], + 'cookies' => [], + ]; + }; + add_filter('pre_http_request', $filter, 10, 3); + + $this->assertFalse(Utils::validate_api_connection()); + $this->assertFalse(Utils::has_valid_api_connection()); + + remove_filter('pre_http_request', $filter, 10); + } + + /** + * Removing credentials clears the connection flag — no creds, no + * connection — without issuing an API request. + * + * @test + */ + public function validate_api_connection_clears_flag_when_credentials_removed() + { + update_option('beyondwords_valid_api_connection', gmdate(\DateTime::ATOM), false); + delete_option('beyondwords_api_key'); + delete_option('beyondwords_project_id'); + + $this->assertFalse(Utils::validate_api_connection()); + $this->assertFalse(Utils::has_valid_api_connection()); + } + + /** + * Within the throttle window the check is served from the last result + * without issuing a second API request — this keeps the uncached remote + * call off every settings-page render. + * + * @test + */ + public function validate_api_connection_throttles_repeat_checks() + { + update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY); + update_option('beyondwords_project_id', BEYONDWORDS_TESTS_PROJECT_ID); + + $calls = 0; + $filter = function ($preempt, $args, $url) use (&$calls) { + $calls++; + return [ + 'response' => ['code' => 200, 'message' => 'OK'], + 'body' => '{"id":' . BEYONDWORDS_TESTS_PROJECT_ID . '}', + 'headers' => [], + 'cookies' => [], + ]; + }; + add_filter('pre_http_request', $filter, 10, 3); + + $this->assertTrue(Utils::validate_api_connection()); + // Second call within the window short-circuits — no new API request. + $this->assertTrue(Utils::validate_api_connection()); + $this->assertSame(1, $calls); + + remove_filter('pre_http_request', $filter, 10); + } + + /** + * Changing credentials busts the throttle so the new creds are validated + * immediately rather than waiting out the window. + * + * @test + */ + public function validate_api_connection_revalidates_when_credentials_change() + { + update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY); + update_option('beyondwords_project_id', BEYONDWORDS_TESTS_PROJECT_ID); + + $calls = 0; + $filter = function ($preempt, $args, $url) use (&$calls) { + $calls++; + return [ + 'response' => ['code' => 200, 'message' => 'OK'], + 'body' => '{"id":' . BEYONDWORDS_TESTS_PROJECT_ID . '}', + 'headers' => [], + 'cookies' => [], + ]; + }; + add_filter('pre_http_request', $filter, 10, 3); + + $this->assertTrue(Utils::validate_api_connection()); + $this->assertSame(1, $calls); + + // Rotate the API key: the fingerprint differs, so validation runs again. + update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY . '-rotated'); + $this->assertTrue(Utils::validate_api_connection()); + $this->assertSame(2, $calls); + + remove_filter('pre_http_request', $filter, 10); + } + /** * @test */