From b66b38e9b2aa4c4a020b0f5b7dcfe8e0f46281ab Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 30 May 2024 09:40:25 -0700 Subject: [PATCH 1/3] Update template_include priority for Server-Timing to fix OD compat in WP 6.6-alpha --- .../server-timing/class-perflab-server-timing.php | 1 + plugins/performance-lab/includes/server-timing/load.php | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php index c190a2fe23..60d9f3836a 100644 --- a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php +++ b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php @@ -245,6 +245,7 @@ public function on_template_include( $passthrough = null ) { ob_start( function ( $output ) { + #$output .= 'SENT HEADER'; $this->send_header(); return $output; } diff --git a/plugins/performance-lab/includes/server-timing/load.php b/plugins/performance-lab/includes/server-timing/load.php index 4ffe61fa6e..4a14ed3354 100644 --- a/plugins/performance-lab/includes/server-timing/load.php +++ b/plugins/performance-lab/includes/server-timing/load.php @@ -45,7 +45,14 @@ function perflab_server_timing(): Perflab_Server_Timing { return $server_timing; } - add_filter( 'template_include', array( $server_timing, 'on_template_include' ), PHP_INT_MAX ); + /* + * Send the Server-Timing header right before the template is included (and rendered on the page) or else start + * output buffering at this point to then send the Server-Timing header in the output buffer callback. Note that + * a priority of PHP_INT_MAX-1 is used so that other plugins that also do output buffering can start their + * buffering at PHP_INT_MAX and ensure that they can register their own Server-Timing metrics successfully when + * the outer output buffer callback here for Server-Timing is called. Otherwise, a _doing_it_wrong() may ensue. + */ + add_filter( 'template_include', array( $server_timing, 'on_template_include' ), PHP_INT_MAX - 1 ); } return $server_timing; From d1bebcbdc39cc3e383f1b9dd718f93d8c4d975df Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 30 May 2024 10:29:56 -0700 Subject: [PATCH 2/3] Opt to start Server-Timing output buffer early --- .../class-perflab-server-timing.php | 15 +++++++++------ .../includes/server-timing/load.php | 16 ++++++++++------ 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php index 60d9f3836a..9feb8423af 100644 --- a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php +++ b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php @@ -238,19 +238,22 @@ public function use_output_buffer(): bool { * @return mixed Unmodified value of $passthrough. */ public function on_template_include( $passthrough = null ) { - if ( ! $this->use_output_buffer() ) { - $this->send_header(); - return $passthrough; - } + $this->send_header(); + return $passthrough; + } + /** + * Starts output buffering to send the Server-Timing header right before returning the buffer. + * + * @since n.e.x.t + */ + public function start_output_buffer(): void { ob_start( function ( $output ) { - #$output .= 'SENT HEADER'; $this->send_header(); return $output; } ); - return $passthrough; } /** diff --git a/plugins/performance-lab/includes/server-timing/load.php b/plugins/performance-lab/includes/server-timing/load.php index 4a14ed3354..6ebf0a3b4d 100644 --- a/plugins/performance-lab/includes/server-timing/load.php +++ b/plugins/performance-lab/includes/server-timing/load.php @@ -46,13 +46,17 @@ function perflab_server_timing(): Perflab_Server_Timing { } /* - * Send the Server-Timing header right before the template is included (and rendered on the page) or else start - * output buffering at this point to then send the Server-Timing header in the output buffer callback. Note that - * a priority of PHP_INT_MAX-1 is used so that other plugins that also do output buffering can start their - * buffering at PHP_INT_MAX and ensure that they can register their own Server-Timing metrics successfully when - * the outer output buffer callback here for Server-Timing is called. Otherwise, a _doing_it_wrong() may ensue. + * When output buffering is enabled, buffer as early as possible so that any other plugins that also do output + * buffering will be able to register Server-Timing metrics. The first output buffer callback to be registered + * is the last one to be called, so by starting the Server-Timing output buffer as soon as possible we can be + * assured that other plugins' output buffer callbacks will run before the Server-Timing one that sends the + * Server-Timing header. */ - add_filter( 'template_include', array( $server_timing, 'on_template_include' ), PHP_INT_MAX - 1 ); + if ( $server_timing->use_output_buffer() ) { + add_action( 'template_redirect', array( $server_timing, 'start_output_buffer' ), PHP_INT_MIN ); + } else { + add_filter( 'template_include', array( $server_timing, 'on_template_include' ), PHP_INT_MAX ); + } } return $server_timing; From 934e7a7fd5bbf214e7d5ea7e4ce8ca0f47396a7b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 30 May 2024 11:03:34 -0700 Subject: [PATCH 3/3] Move logic into add_hooks() method and add tests --- .../class-perflab-server-timing.php | 19 +++++++++++++++++++ .../includes/server-timing/load.php | 13 +------------ .../includes/server-timing/test-load.php | 17 +++++++++++++++++ 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php index 9feb8423af..15661c5af6 100644 --- a/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php +++ b/plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php @@ -225,6 +225,25 @@ public function use_output_buffer(): bool { return (bool) apply_filters( 'perflab_server_timing_use_output_buffer', $enabled ); } + /** + * Adds hooks to send the Server-Timing header. + * + * When output buffering is enabled, buffer as early as possible so that any other plugins that also do output + * buffering will be able to register Server-Timing metrics. The first output buffer callback to be registered + * is the last one to be called, so by starting the Server-Timing output buffer as soon as possible we can be + * assured that other plugins' output buffer callbacks will run before the Server-Timing one that sends the + * Server-Timing header. + * + * @since n.e.x.t + */ + public function add_hooks(): void { + if ( $this->use_output_buffer() ) { + add_action( 'template_redirect', array( $this, 'start_output_buffer' ), PHP_INT_MIN ); + } else { + add_filter( 'template_include', array( $this, 'on_template_include' ), PHP_INT_MAX ); + } + } + /** * Hook callback for the 'template_include' filter. * diff --git a/plugins/performance-lab/includes/server-timing/load.php b/plugins/performance-lab/includes/server-timing/load.php index 6ebf0a3b4d..8769f43aa1 100644 --- a/plugins/performance-lab/includes/server-timing/load.php +++ b/plugins/performance-lab/includes/server-timing/load.php @@ -45,18 +45,7 @@ function perflab_server_timing(): Perflab_Server_Timing { return $server_timing; } - /* - * When output buffering is enabled, buffer as early as possible so that any other plugins that also do output - * buffering will be able to register Server-Timing metrics. The first output buffer callback to be registered - * is the last one to be called, so by starting the Server-Timing output buffer as soon as possible we can be - * assured that other plugins' output buffer callbacks will run before the Server-Timing one that sends the - * Server-Timing header. - */ - if ( $server_timing->use_output_buffer() ) { - add_action( 'template_redirect', array( $server_timing, 'start_output_buffer' ), PHP_INT_MIN ); - } else { - add_filter( 'template_include', array( $server_timing, 'on_template_include' ), PHP_INT_MAX ); - } + $server_timing->add_hooks(); } return $server_timing; diff --git a/tests/plugins/performance-lab/includes/server-timing/test-load.php b/tests/plugins/performance-lab/includes/server-timing/test-load.php index c5ebe00864..f55021f79e 100644 --- a/tests/plugins/performance-lab/includes/server-timing/test-load.php +++ b/tests/plugins/performance-lab/includes/server-timing/test-load.php @@ -16,12 +16,29 @@ public function test_perflab_server_timing(): void { $this->assertTrue( has_filter( 'template_include' ) ); $server_timing = perflab_server_timing(); + $this->assertFalse( $server_timing->use_output_buffer() ); $this->assertSame( PHP_INT_MAX, has_filter( 'template_include', array( $server_timing, 'on_template_include' ) ), 'template_include filter not added' ); + $this->assertFalse( has_action( 'template_redirect', array( $server_timing, 'start_output_buffer' ) ), 'template_redirect action added' ); $server_timing2 = perflab_server_timing(); $this->assertSame( $server_timing, $server_timing2, 'Different instance returned' ); } + /** + * @covers Perflab_Server_Timing::add_hooks + */ + public function test_perflab_server_timing_with_output_buffering(): void { + remove_all_actions( 'template_redirect' ); + remove_all_filters( 'template_include' ); + + $server_timing = perflab_server_timing(); + add_filter( 'perflab_server_timing_use_output_buffer', '__return_true' ); + $this->assertTrue( $server_timing->use_output_buffer() ); + $server_timing->add_hooks(); + $this->assertFalse( has_filter( 'template_include', array( $server_timing, 'on_template_include' ) ), 'template_include filter added' ); + $this->assertSame( PHP_INT_MIN, has_action( 'template_redirect', array( $server_timing, 'start_output_buffer' ) ), 'template_redirect action not added' ); + } + public function test_perflab_server_timing_register_metric(): void { $this->assertFalse( perflab_server_timing()->has_registered_metric( 'test-metric' ) );