← All changes
|
includes/seo/class-performance-data-collector.php
+243
-48
1.0.2
→
2.9.0
View file →
| @@ -12,8 +12,13 @@ | ||
| 12 | 12 | declare(strict_types=1); |
| 13 | 13 | |
| 14 | 14 | namespace ThinkRank\SEO; |
| 15 | 15 | |
| 16 | +// Prevent direct access. | |
| 17 | +if ( ! defined( 'ABSPATH' ) ) { | |
| 18 | + exit; | |
| 19 | +} | |
| 20 | + | |
| 16 | 21 | use ThinkRank\Integrations\Google_PageSpeed_Client; |
| 17 | 22 | |
| 18 | 23 | /** |
| 19 | 24 | * Performance Data Collector Class |
| @@ -25,13 +30,14 @@ | ||
| 25 | 30 | */ |
| 26 | 31 | class Performance_Data_Collector { |
| 27 | 32 | |
| 28 | 33 | /** |
| 29 | - * Performance Monitoring Manager instance | |
| 34 | + * Performance Monitoring Manager instance (lazy — only built when a | |
| 35 | + * collection actually runs, not on every request) | |
| 30 | 36 | * |
| 31 | - * @var Performance_Monitoring_Manager | |
| 37 | + * @var Performance_Monitoring_Manager|null | |
| 32 | 38 | */ |
| 33 | - private Performance_Monitoring_Manager $performance_manager; | |
| 39 | + private ?Performance_Monitoring_Manager $performance_manager = null; | |
| 34 | 40 | |
| 35 | 41 | /** |
| 36 | 42 | * Google PageSpeed Client instance |
| 37 | 43 | * |
| @@ -47,16 +53,14 @@ | ||
| 47 | 53 | /** |
| 48 | 54 | * Constructor |
| 49 | 55 | */ |
| 50 | 56 | public function __construct() { |
| 51 | - $this->performance_manager = new Performance_Monitoring_Manager(); | |
| 57 | + $this->pagespeed_client = null; | |
| 52 | 58 | |
| 53 | - // Initialize PageSpeed client with proper class loading | |
| 54 | - $this->initialize_pagespeed_client(); | |
| 59 | + // Register cron hooks. Routed through the wrapper so a scheduled failure | |
| 60 | + // gets logged — WP-Cron throws the return value away. | |
| 61 | + add_action(self::CRON_HOOK, [$this, 'collect_performance_data_via_cron']); | |
| 55 | 62 | |
| 56 | - // Register cron hooks | |
| 57 | - add_action(self::CRON_HOOK, [$this, 'collect_performance_data']); | |
| 58 | - | |
| 59 | 63 | // Schedule cron if not already scheduled |
| 60 | 64 | if (!wp_next_scheduled(self::CRON_HOOK)) { |
| 61 | 65 | wp_schedule_event(time(), 'daily', self::CRON_HOOK); |
| 62 | 66 | } |
| @@ -62,8 +66,24 @@ | ||
| 62 | 66 | } |
| 63 | 67 | } |
| 64 | 68 | |
| 65 | 69 | /** |
| 70 | + * Get the Performance Monitoring Manager, constructing it on first use. | |
| 71 | + * | |
| 72 | + * The collector is instantiated on every request (bootstrap + REST), so the | |
| 73 | + * manager chain (Settings_Manager, SEO_Settings_Manager, …) must not be | |
| 74 | + * built until a collection actually needs it. | |
| 75 | + * | |
| 76 | + * @return Performance_Monitoring_Manager | |
| 77 | + */ | |
| 78 | + private function get_performance_manager(): Performance_Monitoring_Manager { | |
| 79 | + if ($this->performance_manager === null) { | |
| 80 | + $this->performance_manager = new Performance_Monitoring_Manager(); | |
| 81 | + } | |
| 82 | + return $this->performance_manager; | |
| 83 | + } | |
| 84 | + | |
| 85 | + /** | |
| 66 | 86 | * Initialize PageSpeed client with proper error handling |
| 67 | 87 | * |
| 68 | 88 | * @return void |
| 69 | 89 | */ |
| @@ -84,40 +104,106 @@ | ||
| 84 | 104 | } |
| 85 | 105 | } |
| 86 | 106 | |
| 87 | 107 | try { |
| 88 | - // Get Google PageSpeed API key from settings | |
| 89 | - $api_key = $this->get_pagespeed_api_key(); | |
| 90 | - | |
| 91 | - if (empty($api_key)) { | |
| 108 | + // Either credential is enough. This used to require an OAuth token, | |
| 109 | + // which locked out sites configured with only a PageSpeed API key — | |
| 110 | + // the credential Google_PageSpeed_Client::for_site() actually | |
| 111 | + // *prefers*, since a dedicated key bills its own project quota. Those | |
| 112 | + // sites could never collect and got the same generic failure. | |
| 113 | + if (!$this->has_pagespeed_credentials()) { | |
| 92 | 114 | $this->pagespeed_client = null; |
| 115 | + $this->last_error = __('Connect Google or add a PageSpeed API key to collect Core Web Vitals.', 'thinkrank'); | |
| 116 | + $this->last_error_code = self::ERROR_NOT_CONFIGURED; | |
| 93 | 117 | return; |
| 94 | 118 | } |
| 95 | 119 | |
| 96 | - $this->pagespeed_client = new Google_PageSpeed_Client($api_key); | |
| 120 | + $this->pagespeed_client = Google_PageSpeed_Client::for_site(); | |
| 97 | 121 | } catch (\Exception $e) { |
| 98 | 122 | $this->pagespeed_client = null; |
| 123 | + $this->last_error = $e->getMessage(); | |
| 124 | + $this->last_error_code = self::ERROR_NOT_CONFIGURED; | |
| 99 | 125 | } |
| 100 | 126 | } |
| 101 | 127 | |
| 102 | 128 | /** |
| 103 | - * Get Google PageSpeed API key from settings | |
| 129 | + * Whether this site has a credential the PageSpeed API will accept. | |
| 104 | 130 | * |
| 105 | - * @return string API key or empty string if not configured | |
| 131 | + * @return bool | |
| 106 | 132 | */ |
| 107 | - private function get_pagespeed_api_key(): string { | |
| 108 | - // Get the API key from integrations settings | |
| 109 | - $integrations_settings = get_option('thinkrank_integrations_settings', []); | |
| 110 | - return $integrations_settings['google_pagespeed_api_key'] ?? ''; | |
| 133 | + private function has_pagespeed_credentials(): bool { | |
| 134 | + // One predicate, next to the for_site() auth order it mirrors: keeping a | |
| 135 | + // second copy here is how the Performance tab drifted into demanding | |
| 136 | + // OAuth specifically (#519). | |
| 137 | + return Google_PageSpeed_Client::site_has_credentials(); | |
| 111 | 138 | } |
| 112 | 139 | |
| 113 | 140 | /** |
| 141 | + * Option storing the timestamp of the last successful collection, | |
| 142 | + * used by the 7-day auto-refresh gate. | |
| 143 | + */ | |
| 144 | + private const LAST_COLLECTED_OPTION = 'thinkrank_cwv_last_collected'; | |
| 145 | + | |
| 146 | + /** | |
| 147 | + * How long a successful measurement satisfies automatic collections. | |
| 148 | + * Lighthouse lab data is effectively static week-to-week (RankMath uses | |
| 149 | + * the same 7-day gate), and staying frugal keeps every install inside | |
| 150 | + * the shared PageSpeed quota. | |
| 151 | + */ | |
| 152 | + private const AUTO_REFRESH_GAP = 7 * DAY_IN_SECONDS; | |
| 153 | + | |
| 154 | + /** | |
| 155 | + * Failure classes a collection can end in. Every one of these used to | |
| 156 | + * collapse into a bare `false` and then into the literal string | |
| 157 | + * "Data collection failed", which told the user nothing and made the REST | |
| 158 | + * route answer 500 for conditions that are not server faults. | |
| 159 | + */ | |
| 160 | + public const ERROR_NOT_CONFIGURED = 'not_configured'; | |
| 161 | + public const ERROR_URL_UNREACHABLE = 'url_unreachable'; | |
| 162 | + public const ERROR_RATE_LIMITED = 'rate_limited'; | |
| 163 | + public const ERROR_RECENT_FAILURE = 'recent_failure'; | |
| 164 | + public const ERROR_STORAGE_FAILED = 'storage_failed'; | |
| 165 | + public const ERROR_API_FAILED = 'api_failed'; | |
| 166 | + | |
| 167 | + /** | |
| 168 | + * Human-readable reason the last collection failed. | |
| 169 | + * | |
| 170 | + * @var string | |
| 171 | + */ | |
| 172 | + private string $last_error = ''; | |
| 173 | + | |
| 174 | + /** | |
| 175 | + * Machine-readable class of the last failure — one of the ERROR_* constants. | |
| 176 | + * | |
| 177 | + * @var string | |
| 178 | + */ | |
| 179 | + private string $last_error_code = ''; | |
| 180 | + | |
| 181 | + /** | |
| 114 | 182 | * Collect performance data for the site |
| 115 | 183 | * |
| 184 | + * @param bool $force Bypass the 7-day auto-refresh gate (manual refresh). | |
| 116 | 185 | * @return bool Success status |
| 117 | 186 | */ |
| 118 | - public function collect_performance_data(): bool { | |
| 187 | + public function collect_performance_data(bool $force = false): bool { | |
| 188 | + $this->last_error = ''; | |
| 189 | + $this->last_error_code = ''; | |
| 190 | + | |
| 119 | 191 | try { |
| 192 | + // Auto-collections (cron / background) re-measure at most every | |
| 193 | + // 7 days; only an explicit user refresh forces a new audit. | |
| 194 | + if (!$force) { | |
| 195 | + $last = (int) get_option(self::LAST_COLLECTED_OPTION, 0); | |
| 196 | + if ($last && (time() - $last) < self::AUTO_REFRESH_GAP) { | |
| 197 | + return true; | |
| 198 | + } | |
| 199 | + } | |
| 200 | + | |
| 201 | + // Build the client here rather than in the constructor: the collector is | |
| 202 | + // instantiated on ordinary requests too, and the token must be read (and | |
| 203 | + // refreshed) at collection time to avoid using a stale one. | |
| 204 | + $this->initialize_pagespeed_client(); | |
| 205 | + | |
| 120 | 206 | $home_url = home_url(); |
| 121 | 207 | |
| 122 | 208 | // Test both mobile and desktop |
| 123 | 209 | $devices = ['mobile', 'desktop']; |
| @@ -123,14 +209,18 @@ | ||
| 123 | 209 | $devices = ['mobile', 'desktop']; |
| 124 | 210 | $success = true; |
| 125 | 211 | |
| 126 | 212 | foreach ($devices as $device) { |
| 127 | - $device_success = $this->collect_device_performance_data($home_url, $device); | |
| 213 | + $device_success = $this->collect_device_performance_data($home_url, $device, $force); | |
| 128 | 214 | if (!$device_success) { |
| 129 | 215 | $success = false; |
| 130 | 216 | } |
| 131 | 217 | } |
| 132 | 218 | |
| 219 | + if ($success) { | |
| 220 | + update_option(self::LAST_COLLECTED_OPTION, time(), false); | |
| 221 | + } | |
| 222 | + | |
| 133 | 223 | // Run data cleanup (keep 1 year of data) |
| 134 | 224 | $this->cleanup_old_data(365); |
| 135 | 225 | |
| 136 | 226 | return $success; |
| @@ -135,13 +225,101 @@ | ||
| 135 | 225 | |
| 136 | 226 | return $success; |
| 137 | 227 | |
| 138 | 228 | } catch (\Exception $e) { |
| 229 | + $this->record_error($e); | |
| 139 | 230 | return false; |
| 140 | 231 | } |
| 141 | 232 | } |
| 142 | 233 | |
| 143 | 234 | /** |
| 235 | + * Cron entry point. | |
| 236 | + * | |
| 237 | + * WP-Cron discards a callback's return value, so a hook that returns false is | |
| 238 | + * still reported as having run successfully — this collection could fail on | |
| 239 | + * every scheduled pass with the only evidence being an empty table. Log the | |
| 240 | + * reason instead. | |
| 241 | + * | |
| 242 | + * @since 1.31.0 | |
| 243 | + * @return void | |
| 244 | + */ | |
| 245 | + public function collect_performance_data_via_cron(): void { | |
| 246 | + if ($this->collect_performance_data()) { | |
| 247 | + return; | |
| 248 | + } | |
| 249 | + | |
| 250 | + // Not-configured is a configuration state, not a failure: it is the | |
| 251 | + // default for a fresh install, it never resolves on its own, and the | |
| 252 | + // REST layer already reports it to the UI. Logging it wrote a line to | |
| 253 | + // every unconfigured site's error log on every scheduled run (#585). | |
| 254 | + if ($this->last_error_code === self::ERROR_NOT_CONFIGURED) { | |
| 255 | + return; | |
| 256 | + } | |
| 257 | + | |
| 258 | + error_log( // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- the only record that a silent cron failure happened. | |
| 259 | + sprintf( | |
| 260 | + 'ThinkRank [performance]: scheduled Core Web Vitals collection failed (%s) — %s', | |
| 261 | + $this->last_error_code !== '' ? $this->last_error_code : 'unknown', | |
| 262 | + $this->last_error !== '' ? $this->last_error : 'no reason reported' | |
| 263 | + ) | |
| 264 | + ); | |
| 265 | + } | |
| 266 | + | |
| 267 | + /** | |
| 268 | + * Reason the last collection failed, for the REST layer to report. | |
| 269 | + * | |
| 270 | + * @since 1.31.0 | |
| 271 | + * @return array{code: string, message: string} Empty strings when the last | |
| 272 | + * run did not fail. | |
| 273 | + */ | |
| 274 | + public function get_last_error(): array { | |
| 275 | + return [ | |
| 276 | + 'code' => $this->last_error_code, | |
| 277 | + 'message' => $this->last_error, | |
| 278 | + ]; | |
| 279 | + } | |
| 280 | + | |
| 281 | + /** | |
| 282 | + * Classify an exception from the PageSpeed call into a failure class. | |
| 283 | + * | |
| 284 | + * The distinctions matter to the caller: an unreachable site and an | |
| 285 | + * exhausted quota need different advice, and neither is a server fault. | |
| 286 | + * | |
| 287 | + * @since 1.31.0 | |
| 288 | + * @param \Exception $e Exception thrown while collecting. | |
| 289 | + * @return void | |
| 290 | + */ | |
| 291 | + private function record_error(\Exception $e): void { | |
| 292 | + $message = $e->getMessage(); | |
| 293 | + $this->last_error = $message; | |
| 294 | + | |
| 295 | + if ((int) $e->getCode() === Google_PageSpeed_Client::CODE_REMEMBERED_FAILURE) { | |
| 296 | + $this->last_error_code = self::ERROR_RECENT_FAILURE; | |
| 297 | + return; | |
| 298 | + } | |
| 299 | + | |
| 300 | + // Lighthouse could not load the page: not public, DNS/TLS failure, or the | |
| 301 | + // server refused the fetch. | |
| 302 | + if (stripos($message, 'FAILED_DOCUMENT_REQUEST') !== false | |
| 303 | + || stripos($message, 'ERRORED_DOCUMENT_REQUEST') !== false | |
| 304 | + || stripos($message, 'DNS_FAILURE') !== false | |
| 305 | + || stripos($message, 'net::') !== false) { | |
| 306 | + $this->last_error_code = self::ERROR_URL_UNREACHABLE; | |
| 307 | + return; | |
| 308 | + } | |
| 309 | + | |
| 310 | + // The base client throws with the HTTP status as the exception code. | |
| 311 | + if ((int) $e->getCode() === 429 | |
| 312 | + || stripos($message, 'rate limit') !== false | |
| 313 | + || stripos($message, 'quota') !== false) { | |
| 314 | + $this->last_error_code = self::ERROR_RATE_LIMITED; | |
| 315 | + return; | |
| 316 | + } | |
| 317 | + | |
| 318 | + $this->last_error_code = self::ERROR_API_FAILED; | |
| 319 | + } | |
| 320 | + | |
| 321 | + /** | |
| 144 | 322 | * Collect performance data for specific device type |
| 145 | 323 | * |
| 146 | 324 | * @param string $url URL to test |
| 147 | 325 | * @param string $device_type Device type (mobile/desktop) |
| @@ -146,33 +324,29 @@ | ||
| 146 | 324 | * @param string $url URL to test |
| 147 | 325 | * @param string $device_type Device type (mobile/desktop) |
| 148 | 326 | * @return bool Success status |
| 149 | 327 | */ |
| 150 | - private function collect_device_performance_data(string $url, string $device_type): bool { | |
| 328 | + private function collect_device_performance_data(string $url, string $device_type, bool $force = false): bool { | |
| 151 | 329 | try { |
| 152 | 330 | // Check if PageSpeed client is available |
| 153 | 331 | if (!$this->pagespeed_client) { |
| 332 | + if ($this->last_error === '') { | |
| 333 | + $this->last_error = __('Connect Google or add a PageSpeed API key to collect Core Web Vitals.', 'thinkrank'); | |
| 334 | + $this->last_error_code = self::ERROR_NOT_CONFIGURED; | |
| 335 | + } | |
| 154 | 336 | return false; |
| 155 | 337 | } |
| 156 | 338 | |
| 157 | - // Get Core Web Vitals data | |
| 158 | - $core_web_vitals = $this->pagespeed_client->get_core_web_vitals($url, $device_type); | |
| 339 | + // One snapshot provides both the Core Web Vitals and the performance | |
| 340 | + // score — previously this ran two full Lighthouse audits per device. | |
| 341 | + $snapshot = $this->pagespeed_client->get_pagespeed_snapshot($url, $device_type, $force); | |
| 159 | 342 | |
| 160 | - // Run full PageSpeed test to get performance score | |
| 161 | - $pagespeed_result = $this->pagespeed_client->run_pagespeed_test($url, $device_type, ['performance']); | |
| 162 | - | |
| 163 | - // Extract performance score | |
| 164 | - $performance_score = 0; | |
| 165 | - if (isset($pagespeed_result['lighthouseResult']['categories']['performance']['score'])) { | |
| 166 | - $performance_score = $pagespeed_result['lighthouseResult']['categories']['performance']['score'] * 100; | |
| 167 | - } | |
| 168 | - | |
| 169 | 343 | // Prepare data for storage |
| 170 | - $performance_data = $core_web_vitals; | |
| 171 | - $performance_data['performance_score'] = $performance_score; | |
| 344 | + $performance_data = $snapshot['core_web_vitals']; | |
| 345 | + $performance_data['performance_score'] = $snapshot['performance_score']; | |
| 172 | 346 | |
| 173 | 347 | // Store in database |
| 174 | - $stored = $this->performance_manager->store_historical_performance_data( | |
| 348 | + $stored = $this->get_performance_manager()->store_historical_performance_data( | |
| 175 | 349 | $performance_data, |
| 176 | 350 | 'site', |
| 177 | 351 | null, |
| 178 | 352 | $device_type |
| @@ -179,11 +353,21 @@ | ||
| 179 | 353 | ); |
| 180 | 354 | |
| 181 | 355 | |
| 182 | 356 | |
| 357 | + if (!$stored) { | |
| 358 | + $this->last_error = sprintf( | |
| 359 | + /* translators: %s: device type (mobile or desktop). */ | |
| 360 | + __('Measured %s successfully but could not store the result.', 'thinkrank'), | |
| 361 | + $device_type | |
| 362 | + ); | |
| 363 | + $this->last_error_code = self::ERROR_STORAGE_FAILED; | |
| 364 | + } | |
| 365 | + | |
| 183 | 366 | return $stored; |
| 184 | - | |
| 367 | + | |
| 185 | 368 | } catch (\Exception $e) { |
| 369 | + $this->record_error($e); | |
| 186 | 370 | return false; |
| 187 | 371 | } |
| 188 | 372 | } |
| 189 | 373 | |
| @@ -196,25 +380,36 @@ | ||
| 196 | 380 | $results = [ |
| 197 | 381 | 'success' => false, |
| 198 | 382 | 'message' => '', |
| 199 | 383 | 'data_collected' => false, |
| 384 | + 'error_code' => '', | |
| 200 | 385 | 'errors' => [] |
| 201 | 386 | ]; |
| 202 | 387 | |
| 203 | 388 | try { |
| 204 | - $success = $this->collect_performance_data(); | |
| 205 | - | |
| 389 | + // Manual refresh always re-measures (bypasses the 7-day gate). | |
| 390 | + $success = $this->collect_performance_data(true); | |
| 391 | + | |
| 206 | 392 | if ($success) { |
| 207 | 393 | $results['success'] = true; |
| 208 | 394 | $results['data_collected'] = true; |
| 209 | 395 | $results['message'] = __('Performance data collected successfully', 'thinkrank'); |
| 210 | 396 | } else { |
| 211 | - $results['message'] = __('Failed to collect performance data', 'thinkrank'); | |
| 212 | - $results['errors'][] = 'Data collection failed'; | |
| 397 | + $error = $this->get_last_error(); | |
| 398 | + $results['message'] = $error['message'] !== '' | |
| 399 | + ? $error['message'] | |
| 400 | + : __('Failed to collect performance data', 'thinkrank'); | |
| 401 | + $results['error_code'] = $error['code']; | |
| 402 | + $results['errors'][] = $results['message']; | |
| 213 | 403 | } |
| 214 | - | |
| 404 | + | |
| 215 | 405 | } catch (\Exception $e) { |
| 216 | - $results['message'] = __('Error during data collection', 'thinkrank'); | |
| 406 | + $this->record_error($e); | |
| 407 | + $error = $this->get_last_error(); | |
| 408 | + $results['message'] = $error['message'] !== '' | |
| 409 | + ? $error['message'] | |
| 410 | + : __('Error during data collection', 'thinkrank'); | |
| 411 | + $results['error_code'] = $error['code']; | |
| 217 | 412 | $results['errors'][] = $e->getMessage(); |
| 218 | 413 | } |
| 219 | 414 | |
| 220 | 415 | return $results; |
| @@ -253,11 +448,11 @@ | ||
| 253 | 448 | "DELETE FROM %s WHERE measured_at < %%s", |
| 254 | 449 | $table_name |
| 255 | 450 | ); |
| 256 | 451 | |
| 257 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Performance data cleanup requires direct database access | |
| 452 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Performance data cleanup requires direct database access | |
| 258 | 453 | $deleted = $wpdb->query( |
| 259 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 454 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 260 | 455 | $wpdb->prepare($sql, $cutoff_date) |
| 261 | 456 | ); |
| 262 | 457 | |
| 263 | 458 | return $deleted !== false ? (int) $deleted : 0; |
| @@ -323,11 +518,11 @@ | ||
| 323 | 518 | AND context_type = 'site' |
| 324 | 519 | AND measured_by = 'google_pagespeed' |
| 325 | 520 | ", $table_name); |
| 326 | 521 | |
| 327 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Performance statistics retrieval requires direct database access | |
| 522 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Performance statistics retrieval requires direct database access | |
| 328 | 523 | $stats = $wpdb->get_row( |
| 329 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 524 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 330 | 525 | $wpdb->prepare($sql, $start_date), |
| 331 | 526 | ARRAY_A |
| 332 | 527 | ); |
| 333 | 528 | |