Merge pull request #103 from newfold-labs/fix-queue

newfold-labs · Oct 17, 2024 · fad23fc · fad23fc
2 parents 89f0957 + 740b33c
commit fad23fc
Show file tree

Hide file tree

Showing 4 changed files with 172 additions and 21 deletions.
diff --git 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,21 +251,29 @@ public function send_saved_events_batch(): void {
 		 *
 		 * @var array<int,Event> $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 ) {
 			/**
 			 * @var array{succeededEvents:array,failedEvents:array}|WP_Error $response
 			 */
 			$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
@@ -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
@@ -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();
+	}
 }