Merge pull request #92 from newfold-labs/fix/pr-76-bugs

Fix PR 76 bugs
newfold-labs · Jul 24, 2024 · 05458a2 · 05458a2
2 parents 6af57da + ccc985b
commit 05458a2
Show file tree

Hide file tree

Showing 9 changed files with 402 additions and 34 deletions.
diff --git a/includes/EventManager.php b/includes/EventManager.php
@@ -215,14 +215,14 @@ protected function send_request_events( array $events ): void {
 				continue;
 			}
 
-			$queue = EventQueue::getInstance()->queue();
-
 			if ( is_wp_error( $response ) ) {
 				EventQueue::getInstance()->queue()->push( $events );
 				continue;
 			}
 
-			EventQueue::getInstance()->queue()->push( $response['failedEvents'] );
+			if ( ! empty( $response['failedEvents'] ) ) {
+				EventQueue::getInstance()->queue()->push( $response['failedEvents'] );
+			}
 		}
 	}
 
@@ -265,10 +265,14 @@ public function send_saved_events_batch(): void {
 			}
 
 			// Remove from the queue.
-			$queue->remove( array_keys( $response['succeededEvents'] ) );
+			if ( ! empty( $response['succeededEvents'] ) ) {
+				$queue->remove( array_keys( $response['succeededEvents'] ) );
+			}
 
 			// Release the 'reserve' we placed on the entry, so it will be tried again later.
-			$queue->release( array_keys( $response['failedEvents'] ) );
+			if ( ! empty( $response['failedEvents'] ) ) {
+				$queue->release( array_keys( $response['failedEvents'] ) );
+			}
 		}
 	}
 }
diff --git a/includes/EventQueue/Queryable.php b/includes/EventQueue/Queryable.php
@@ -27,7 +27,7 @@ protected function table() {
 	 * Returns number of affected (inserted) rows.
 	 *
 	 * @param  string  $table
-	 * @param  array[]  $rows
+	 * @param  non-empty-array  $rows
 	 *
 	 * @return bool|int
 	 */

diff --git a/includes/EventQueue/Queues/BatchQueue.php b/includes/EventQueue/Queues/BatchQueue.php
@@ -29,7 +29,7 @@ public function __construct( Container $container ) {
 	/**
 	 * Push events onto the queue
 	 *
-	 * @param  Event[]  $events
+	 * @param  non-empty-array<Event>  $events
 	 *
 	 * @return bool
 	 */

diff --git a/includes/Helpers/Plugin.php b/includes/Helpers/Plugin.php
@@ -50,9 +50,9 @@ public static function collect_installed() {
 	/**
 	 * Grab relevant data from plugin data - and only what we want
 	 *
-	 * @param array $slug The slug for the plugin
-	 * @param array $data The plugin meta data from the header
-	 * @param array $mu   Whether the plugin is installed as an mu
+	 * @param string $slug The slug for the plugin.
+	 * @param array  $data The plugin meta-data from its header.
+	 * @param array  $mu   Whether the plugin is installed as a must-use plugin.
 	 *
 	 * @return array Hiive relevant plugin details
 	 */

diff --git a/includes/HiiveConnection.php b/includes/HiiveConnection.php
@@ -292,11 +292,18 @@ public function notify( $events ) {
 			return new WP_Error( wp_remote_retrieve_response_code( $hiive_response ), wp_remote_retrieve_response_message( $hiive_response ) );
 		}
 
-		return json_decode( wp_remote_retrieve_body( $hiive_response ), true );
+		$response_body = json_decode( wp_remote_retrieve_body( $hiive_response ), true );
+
+		// If the response from Hiive is not shaped as expected, e.g. a more serious 500 error, return as an error, not as the expected array.
+		if ( ! is_array( $response_body ) || ! array_key_exists( 'succeededEvents', $response_body ) || ! array_key_exists( 'failedEvents', $response_body ) ) {
+			return new WP_Error( 'hiive_response', 'Response body does not contain succeededEvents and failedEvents keys.' );
+		}
+
+		return $response_body;
 	}
 
 	/**
-	 * Send a HTTP request to Hiive and return the body of the request.
+	 * Send an HTTP request to Hiive and return the body of the request.
 	 *
 	 * Handles throttling and reconnection, clients should handle queueing if necessary.
 	 *
@@ -319,8 +326,8 @@ public function hiive_request( string $path, ?array $payload = array(), ?array $
 		$defaults = array(
 			'method'  => 'POST',
 			'headers' => array(
-				'Content-Type'  => 'applicaton/json',
-				'Accept'        => 'applicaton/json',
+				'Content-Type'  => 'application/json',
+				'Accept'        => 'application/json',
 				'Authorization' => 'Bearer ' . self::get_auth_token(),
 			),
 			'timeout' => wp_is_serving_rest_request() ? 15 : 60, // If we're responding to the frontend, we need to be quick.
@@ -344,7 +351,7 @@ public function hiive_request( string $path, ?array $payload = array(), ?array $
 			$body = json_decode( $request_response['body'], true );
 			if ( 'Invalid token for url' === $body['message'] ) {
 				if ( $this->reconnect() ) {
-					$this->hiive_request( $path, $args );
+					$this->hiive_request( $path, $payload, $args );
 				} else {
 					return new WP_Error( 'hiive_connection', __( 'This site is not connected to the hiive.' ) );
 				}

diff --git a/tests/phpunit/includes/EventManagerTest.php b/tests/phpunit/includes/EventManagerTest.php
@@ -197,20 +197,157 @@ function () use ( $batch_queue_mock ) {
 		$hiive_connection_subscriber->expects( 'notify' )
 			->once()
 			->andReturn(
-				array( 'succeededEvents' => array( 15 => array() ), 'failedEvents' => array() )
+				array(
+					'succeededEvents' => array( 15 => array() ),
+					'failedEvents'    => array( 16 => array() ),
+				)
 			);
 
 		WP_Mock::userFunction( 'is_wp_error' )
-		       ->once()
-		       ->andReturnFalse();
+				->once()
+				->andReturnFalse();
 
 		$batch_queue_mock->expects( 'remove' )
 						->once()
 						->with( array( 15 ) );
 
 		$batch_queue_mock->expects( 'release' )
 						->once()
-						->with( array() );
+						->with( array( 16 ) );
+
+		$sut->send_saved_events_batch();
+
+		$this->assertConditionsMet();
+	}
+
+	/**
+	 * @covers ::send_saved_events_batch
+	 * @covers ::send
+	 */
+	public function test_send_saved_events_happy_path_no_failed_events(): 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( 100 )
+						->andReturn(
+							array(
+								15 => $event,
+							)
+						);
+
+		$batch_queue_mock->expects( 'reserve' )
+						->once()
+						->with( array( 15 ) );
+
+		$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );
+
+		$sut->expects( 'get_subscribers' )
+			->once()
+			->andReturn( array( $hiive_connection_subscriber ) );
+
+		$hiive_connection_subscriber->expects( 'notify' )
+			->once()
+			->andReturn(
+				array(
+					'succeededEvents' => array( 15 => array() ),
+					'failedEvents'    => array(),
+				)
+			);
+
+		WP_Mock::userFunction( 'is_wp_error' )
+				->once()
+				->andReturnFalse();
+
+		$batch_queue_mock->expects( 'remove' )
+						->once()
+						->with( array( 15 ) );
+
+		$batch_queue_mock->expects( 'release' )
+						->never();
+
+		$sut->send_saved_events_batch();
+
+		$this->assertConditionsMet();
+	}
+
+	/**
+	 * @covers ::send_saved_events_batch
+	 * @covers ::send
+	 */
+	public function test_send_saved_events_happy_path_no_successful_events(): 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( 100 )
+						->andReturn(
+							array(
+								15 => $event,
+							)
+						);
+
+		$batch_queue_mock->expects( 'reserve' )
+						->once()
+						->with( array( 15 ) );
+
+		$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );
+
+		$sut->expects( 'get_subscribers' )
+			->once()
+			->andReturn( array( $hiive_connection_subscriber ) );
+
+		$hiive_connection_subscriber->expects( 'notify' )
+			->once()
+			->andReturn(
+				array(
+					'succeededEvents' => array(),
+					'failedEvents'    => array( 16 => array() ),
+				)
+			);
+
+		WP_Mock::userFunction( 'is_wp_error' )
+				->once()
+				->andReturnFalse();
+
+		$batch_queue_mock->expects( 'remove' )
+						->never();
+
+		$batch_queue_mock->expects( 'release' )
+						->once()
+						->with( array( 16 ) );
 
 		$sut->send_saved_events_batch();
 
@@ -323,7 +460,10 @@ function () use ( $batch_queue_mock ) {
 		$hiive_connection_subscriber->expects( 'notify' )
 			->once()
 			->andReturn(
-				array( 'succeededEvents' => array(), 'failedEvents' => array( 19 => array() ) )
+				array(
+					'succeededEvents' => array(),
+					'failedEvents'    => array( 19 => array() ),
+				)
 			);
 
 		WP_Mock::userFunction( 'is_wp_error' )
@@ -334,8 +474,7 @@ function () use ( $batch_queue_mock ) {
 						->once()
 						->with( array( 19 ) );
 
-		$batch_queue_mock->expects( 'remove' )->once()
-			->with( array() );
+		$batch_queue_mock->expects( 'remove' )->never();
 
 		$sut->send_saved_events_batch();
 
@@ -344,9 +483,59 @@ function () use ( $batch_queue_mock ) {
 
 	/**
 	 * @covers ::shutdown
-	 * @covers ::send
+	 * @covers ::send_request_events
+	 */
+	public function test_shutdown_happy_path_no_failed_events(): void {
+
+		$sut = new EventManager();
+
+		$event      = Mockery::mock( Event::class )->makePartial();
+		$event->key = 'test';
+
+		$sut->push( $event );
+
+		$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;
+			}
+		);
+
+		$hiive_connection_subscriber = Mockery::mock( HiiveConnection::class );
+
+		$sut->add_subscriber( $hiive_connection_subscriber );
+
+		$hiive_connection_subscriber->expects( 'notify' )
+									->once()
+									->andReturn(
+										array(
+											'succeededEvents' => array(),
+											'failedEvents' => array(),
+										)
+									);
+
+		WP_Mock::userFunction( 'is_wp_error' )
+				->once()
+				->andReturnFalse();
+
+		$batch_queue_mock->expects( 'push' )->never();
+
+		$sut->shutdown();
+
+		$this->assertConditionsMet();
+	}
+
+	/**
+	 * @covers ::shutdown
+	 * @covers ::send_request_events
 	 */
-	public function test_shutdown_happy_path(): void {
+	public function test_shutdown_happy_path_with_failed_events(): void {
 
 		$sut = new EventManager();
 
@@ -375,15 +564,18 @@ function () use ( $batch_queue_mock ) {
 		$hiive_connection_subscriber->expects( 'notify' )
 									->once()
 									->andReturn(
-										array( 'succeededEvents' => array(), 'failedEvents' => array( 2 => array( 'key' => 'test' ) ) )
+										array(
+											'succeededEvents' => array(),
+											'failedEvents' => array( 2 => array( 'key' => 'test' ) ),
+										)
 									);
 
 		WP_Mock::userFunction( 'is_wp_error' )
 				->once()
 				->andReturnFalse();
 
 		$batch_queue_mock->expects( 'push' )->once()
-			->with(array( 2 => array( 'key' => 'test' ) ));
+			->with( array( 2 => array( 'key' => 'test' ) ) );
 
 		$sut->shutdown();
 
@@ -469,15 +661,18 @@ function () use ( $batch_queue_mock ) {
 		$hiive_connection_subscriber->expects( 'notify' )
 									->once()
 									->andReturn(
-										array( 'succeededEvents' => array(), 'failedEvents' => array( 18 => array( 'key' => 'event ') ) )
+										array(
+											'succeededEvents' => array(),
+											'failedEvents' => array( 18 => array( 'key' => 'event ' ) ),
+										)
 									);
 
 		WP_Mock::userFunction( 'is_wp_error' )
 				->once()
 				->andReturnFalse();
 
 		$batch_queue_mock->expects( 'push' )->once()
-		->with(array( 18 => array( 'key' => 'event ') ));
+		->with( array( 18 => array( 'key' => 'event ' ) ) );
 
 		$sut->shutdown();