diff --git a/includes/EventManager.php b/includes/EventManager.php index 12ee25b..9daf245 100644 --- a/includes/EventManager.php +++ b/includes/EventManager.php @@ -121,6 +121,14 @@ public function add_minutely_schedule( $schedules ) { */ public function shutdown(): void { + // Due to a bug sending too many events, we are temporarily disabling these. + $disabled_events = array( 'pageview', 'wp_mail', 'plugin_updated' ); + foreach ( $this->queue as $index => $event ) { + if ( in_array( $event->key, $disabled_events, true ) ) { + unset( $this->queue[ $index ] ); + } + } + // Separate out the async events $async = array(); foreach ( $this->queue as $index => $event ) { @@ -211,6 +219,9 @@ protected function send_request_events( array $events ): void { */ $response = $subscriber->notify( $events ); + // Due to an unidentified bug causing events to be resent, we are temporarily disabling retries. + continue; + if ( ! ( $subscriber instanceof HiiveConnection ) ) { continue; } @@ -240,14 +251,19 @@ public function send_saved_events_batch(): void { * * @var array $events */ - $events = $queue->pull( 100 ); + $events = $queue->pull( 50 ); // If queue is empty, do nothing. if ( empty( $events ) ) { return; } - $queue->reserve( array_keys( $events ) ); + // Reserve the events in the queue so they are not processed by another instance. + if ( ! $queue->reserve( array_keys( $events ) ) ) { + // If the events fail to reserve, they will be repeatedly retried. + // It would be good to log this somewhere. + return; + } foreach ( $this->get_subscribers() as $subscriber ) { /** @@ -255,6 +271,9 @@ public function send_saved_events_batch(): void { */ $response = $subscriber->notify( $events ); + // Due to an unidentified bug causing events to be resent, we are temporarily disabling retries. + continue; + if ( ! ( $subscriber instanceof HiiveConnection ) ) { continue; } @@ -274,5 +293,8 @@ public function send_saved_events_batch(): void { $queue->release( array_keys( $response['failedEvents'] ) ); } } + + // Due to an unidentified bug causing events to be resent, we are temporarily disabling retries. + $queue->remove( array_keys( $events ) ); } } diff --git a/includes/HiiveConnection.php b/includes/HiiveConnection.php index 8475ad1..b8871c7 100644 --- a/includes/HiiveConnection.php +++ b/includes/HiiveConnection.php @@ -322,6 +322,12 @@ public function notify( $events ) { */ public function hiive_request( string $path, ?array $payload = array(), ?array $args = array() ) { + /** + * @see \WP_Http::request() + * @see https://developer.wordpress.org/reference/hooks/http_headers_useragent/ + */ + add_filter( 'http_headers_useragent', array( $this, 'add_plugin_name_version_to_user_agent' ), 10, 2 ); + // If for some reason we are not connected, bail out now. // If we are not connected, the throttling logic should eventually reconnect. if ( ! self::is_connected() ) { @@ -363,6 +369,8 @@ public function hiive_request( string $path, ?array $payload = array(), ?array $ } } + remove_filter( 'http_headers_useragent', array( $this, 'add_plugin_name_version_to_user_agent' ) ); + return $request_response; } @@ -405,4 +413,24 @@ public function get_core_data() { return apply_filters( 'newfold_wp_data_module_core_data_filter', $data ); } + + /** + * Add the plugin name and version to the user agent string + * + * @param string $user_agent E.g. "WordPress/6.4.3; https://example.org". + * @param string $url E.g. "https://hiive.cloud/api/sites/v2/events". + * + * @return string E.g. "WordPress/6.4.3; bluehost/1.2.3; https://example.org". + */ + public function add_plugin_name_version_to_user_agent( string $user_agent, string $url ): string { + $container = container(); + $plugin_brand = sanitize_title( $container->plugin()->brand ); + $plugin_version = $container->plugin()->get( 'version', '0' ); + + $user_agent_parts = array_map( 'trim', explode( ';', $user_agent ) ); + + array_splice( $user_agent_parts, 1, 0, "{$plugin_brand}/{$plugin_version}" ); + + return implode( '; ', $user_agent_parts ); + } } diff --git a/tests/phpunit/includes/EventManagerTest.php b/tests/phpunit/includes/EventManagerTest.php index cd74a8f..2fcfcb0 100644 --- a/tests/phpunit/includes/EventManagerTest.php +++ b/tests/phpunit/includes/EventManagerTest.php @@ -154,10 +154,11 @@ public function test_init(): void { /** * @covers ::send_saved_events_batch - * @covers ::send */ public function test_send_saved_events_happy_path(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $batch_queue_mock = Mockery::mock( BatchQueue::class ); \Patchwork\redefine( @@ -186,7 +187,8 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'reserve' ) ->once() - ->with( array( 15 ) ); + ->with( array( 15 ) ) + ->andReturnTrue(); $hiive_connection_subscriber = Mockery::mock( HiiveConnection::class ); @@ -222,10 +224,11 @@ function () use ( $batch_queue_mock ) { /** * @covers ::send_saved_events_batch - * @covers ::send */ public function test_send_saved_events_happy_path_no_failed_events(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $batch_queue_mock = Mockery::mock( BatchQueue::class ); \Patchwork\redefine( @@ -254,7 +257,8 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'reserve' ) ->once() - ->with( array( 15 ) ); + ->with( array( 15 ) ) + ->andReturnTrue(); $hiive_connection_subscriber = Mockery::mock( HiiveConnection::class ); @@ -289,10 +293,11 @@ function () use ( $batch_queue_mock ) { /** * @covers ::send_saved_events_batch - * @covers ::send */ public function test_send_saved_events_happy_path_no_successful_events(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $batch_queue_mock = Mockery::mock( BatchQueue::class ); \Patchwork\redefine( @@ -312,7 +317,7 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'pull' ) ->once() - ->with( 100 ) + ->with( 50 ) ->andReturn( array( 15 => $event, @@ -321,7 +326,8 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'reserve' ) ->once() - ->with( array( 15 ) ); + ->with( array( 15 ) ) + ->andReturnTrue(); $hiive_connection_subscriber = Mockery::mock( HiiveConnection::class ); @@ -356,10 +362,11 @@ function () use ( $batch_queue_mock ) { /** * @covers ::send_saved_events_batch - * @covers ::send */ public function test_send_saved_events_wp_error_from_hiive_connection(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $batch_queue_mock = Mockery::mock( BatchQueue::class ); \Patchwork\redefine( @@ -379,7 +386,7 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'pull' ) ->once() - ->with( 100 ) + ->with( 50 ) ->andReturn( array( 15 => $event, @@ -388,7 +395,8 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'reserve' ) ->once() - ->with( array( 15 ) ); + ->with( array( 15 ) ) + ->andReturnTrue(); $hiive_connection_subscriber = Mockery::mock( HiiveConnection::class ); @@ -417,10 +425,11 @@ function () use ( $batch_queue_mock ) { /** * @covers ::send_saved_events_batch - * @covers ::send */ public function test_send_saved_events_failures_from_hiive(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $batch_queue_mock = Mockery::mock( BatchQueue::class ); \Patchwork\redefine( @@ -440,7 +449,7 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'pull' ) ->once() - ->with( 100 ) + ->with( 50 ) ->andReturn( array( 16 => $event, @@ -449,7 +458,8 @@ function () use ( $batch_queue_mock ) { $batch_queue_mock->expects( 'reserve' ) ->once() - ->with( array( 16 ) ); + ->with( array( 16 ) ) + ->andReturnTrue(); $hiive_connection_subscriber = Mockery::mock( HiiveConnection::class ); @@ -487,6 +497,8 @@ function () use ( $batch_queue_mock ) { */ public function test_shutdown_happy_path_no_failed_events(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $sut = new EventManager(); $event = Mockery::mock( Event::class )->makePartial(); @@ -537,6 +549,8 @@ function () use ( $batch_queue_mock ) { */ public function test_shutdown_happy_path_with_failed_events(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $sut = new EventManager(); $event = Mockery::mock( Event::class )->makePartial(); @@ -584,10 +598,12 @@ function () use ( $batch_queue_mock ) { /** * @covers ::shutdown - * @covers ::send + * @covers ::send_request_events */ public function test_shutdown_hiive_connection_wp_error(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $sut = new EventManager(); $event = Mockery::mock( Event::class )->makePartial(); @@ -630,10 +646,12 @@ function () use ( $batch_queue_mock ) { /** * @covers ::shutdown - * @covers ::send + * @covers ::send_request_events */ public function test_shutdown_hiive_500_error(): void { + $this->markTestSkipped( 'Due to an unidentified bug causing events to be resent, we are temporarily disabling retries.' ); + $sut = new EventManager(); $event = Mockery::mock( Event::class )->makePartial(); @@ -678,4 +696,47 @@ function () use ( $batch_queue_mock ) { $this->assertConditionsMet(); } + + /** + * @covers ::send_saved_events_batch + */ + public function test_send_saved_events_reserve_fails(): void { + + $batch_queue_mock = Mockery::mock( BatchQueue::class ); + + \Patchwork\redefine( + array( EventQueue::class, '__construct' ), + function () {} + ); + \Patchwork\redefine( + array( EventQueue::class, 'queue' ), + function () use ( $batch_queue_mock ) { + return $batch_queue_mock; + } + ); + + $sut = Mockery::mock( EventManager::class )->makePartial(); + + $event = Mockery::mock( Event::class ); + + $batch_queue_mock->expects( 'pull' ) + ->once() + ->with( 50 ) + ->andReturn( + array( + 15 => $event, + ) + ); + + $batch_queue_mock->expects( 'reserve' ) + ->once() + ->with( array( 15 ) ) + ->andReturnFalse(); + + $sut->expects( 'get_subscribers' )->never(); + + $sut->send_saved_events_batch(); + + $this->assertConditionsMet(); + } } diff --git a/tests/phpunit/includes/HiiveConnectionTest.php b/tests/phpunit/includes/HiiveConnectionTest.php index 614dfb1..4c0265f 100644 --- a/tests/phpunit/includes/HiiveConnectionTest.php +++ b/tests/phpunit/includes/HiiveConnectionTest.php @@ -17,6 +17,7 @@ public function setUp(): void { parent::setUp(); WP_Mock::passthruFunction( '__' ); + WP_Mock::passthruFunction( 'sanitize_title' ); WP_Mock::userFunction( 'wp_json_encode' )->andReturnUsing( function ( $input ) { @@ -46,8 +47,6 @@ public function test_plugin_sends_boxname_to_hiive(): void { $plugin->expects( 'get' )->once()->with( 'version', '0' )->andReturn( '1.2.3' ); container()->set( 'plugin', $plugin ); - WP_Mock::passthruFunction( 'sanitize_title' ); - WP_Mock::userFunction( 'get_option' )->once()->with( 'newfold_cache_level', 2 )->andReturn( 2 ); WP_Mock::userFunction( 'get_option' )->once()->with( 'newfold_cloudflare_enabled', false )->andReturn( false ); WP_Mock::userFunction( 'get_option' )->once()->with( 'admin_email' )->andReturn( 'admin@example.com' ); @@ -77,8 +76,6 @@ public function test_plugin_sends_server_path_to_hiive(): void { $plugin->expects( 'get' )->once()->with( 'version', '0' )->andReturn( '1.2.3' ); container()->set( 'plugin', $plugin ); - WP_Mock::passthruFunction( 'sanitize_title' ); - WP_Mock::userFunction( 'get_option' )->once()->with( 'newfold_cache_level', 2 )->andReturn( 2 ); WP_Mock::userFunction( 'get_option' )->once()->with( 'newfold_cloudflare_enabled', false )->andReturn( false ); WP_Mock::userFunction( 'get_option' )->once()->with( 'admin_email' )->andReturn( 'admin@example.com' ); @@ -114,6 +111,8 @@ function ( string $constant_name ): string { public function test_hiive_request_returns_wperror_when_no_auth_token(): void { $sut = new HiiveConnection(); + WP_Mock::expectFilterAdded( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ), 10, 2 ); + WP_Mock::userFunction( 'get_option' ) ->with( 'nfd_data_token' ) ->once() @@ -129,9 +128,12 @@ public function test_hiive_request_returns_wperror_when_no_auth_token(): void { * @covers ::notify */ public function test_notify_success(): void { + $sut = Mockery::mock( HiiveConnection::class )->makePartial(); $sut->expects( 'get_core_data' )->once()->andReturn( array( 'brand' => 'etc' ) ); + WP_Mock::expectFilterAdded( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ), 10, 2 ); + $event = Mockery::mock( Event::class ); WP_Mock::userFunction( 'get_option' ) @@ -172,6 +174,15 @@ function ( $request_response ) { } ); + /** + * WP_Mock::expectFilterRemoved( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ) ); + * + * @see https://github.com/10up/wp_mock/pull/246 + */ + WP_Mock::userFunction( 'NewfoldLabs\WP\Module\Data\remove_filter' ) + ->once() + ->with( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ) ); + $sut->notify( array( $event ) ); $this->assertConditionsMet(); @@ -186,6 +197,8 @@ function ( $request_response ) { public function test_notify_bad_token(): void { $sut = Mockery::mock( HiiveConnection::class )->makePartial(); + WP_Mock::expectFilterAdded( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ), 10, 2 ); + $event = Mockery::mock( Event::class ); $event->category = 'admin'; $event->key = 'plugin_search'; @@ -325,6 +338,15 @@ function ( string $constant_name ) use ( $temp_dir ) { ->with( 'nfd_data_token', 'hiive_auth_token' ) ->once()->andReturnTrue(); + /** + * WP_Mock::expectFilterRemoved( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ) ); + * + * @see https://github.com/10up/wp_mock/pull/246 + */ + WP_Mock::userFunction( 'NewfoldLabs\WP\Module\Data\remove_filter' ) + ->twice() + ->with( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ) ); + $sut->notify( array( $event ) ); $this->assertConditionsMet(); @@ -338,6 +360,8 @@ function ( string $constant_name ) use ( $temp_dir ) { public function test_fails_to_reconnect() { $sut = Mockery::mock( HiiveConnection::class )->makePartial(); + WP_Mock::expectFilterAdded( 'http_headers_useragent', array( $sut, 'add_plugin_name_version_to_user_agent' ), 10, 2 ); + WP_Mock::userFunction( 'get_option' ) ->with( 'nfd_data_token' ) ->twice() @@ -369,4 +393,20 @@ public function test_fails_to_reconnect() { self::assertInstanceOf( \WP_Error::class, $result ); self::assertEquals( 'This site is not connected to the hiive.', $result->get_error_message() ); } + + /** + * @covers ::add_plugin_name_version_to_user_agent + */ + public function test_add_plugin_name_version_to_user_agent(): void { + $plugin = Mockery::mock( Plugin::class ); + $plugin->brand = 'bluehost'; + $plugin->expects( 'get' )->once()->with( 'version', '0' )->andReturn( '1.2.3' ); + container()->set( 'plugin', $plugin ); + + $sut = new HiiveConnection(); + + $result = $sut->add_plugin_name_version_to_user_agent( 'WordPress/6.4.3; https://example.org', '' ); + + self::assertEquals( 'WordPress/6.4.3; bluehost/1.2.3; https://example.org', $result ); + } }