Release: Fix empty addon list when request is throttled

Author: joel-james

src/Services/Addon.php

@@ -82,12 +82,14 @@ public function __construct( Plugin $plugin, CacheInterface $cache, ApiFactory $
 	 * Get the list of addons for the host plugin.
 	 *
 	 * Returns an empty array (rather than a WP_Error) when:
-	 * - the host plugin does not declare `has_addons`,
-	 * - the API request fails, or
-	 * - the throttle window is still open.
+	 * - the host plugin does not declare `has_addons`, or
+	 * - the API request fails with no cached catalog to fall back on.
 	 *
 	 * Use `$force = true` to bypass the cache when the user explicitly
-	 * asks for a refresh from the host plugin's UI.
+	 * asks for a refresh from the host plugin's UI. A forced call that
+	 * the throttle blocks (or that the API rejects) transparently falls
+	 * back to the cached catalog — the host UI keeps showing the
+	 * last-known list instead of emptying out.
 	 *
 	 * @since 2.0.0
 	 *
@@ -100,16 +102,16 @@ public function get_addons( bool $force = false ): array {
 			return array();
 		}
 
-		if ( ! $force ) {
-			$cached = $this->cache->get( 'addons' );
-			if ( false !== $cached && is_array( $cached ) ) {
-				return $cached;
-			}
+		$cached = $this->cache->get( 'addons' );
+		$cached = ( false !== $cached && is_array( $cached ) ) ? $cached : array();
+
+		if ( ! $force && ! empty( $cached ) ) {
+			return $cached;
 		}
 
 		$addons = $this->get_remote_addons();
 		if ( is_wp_error( $addons ) ) {
-			return array();
+			return $cached;
 		}
 
 		$addons = array_map( array( $this, 'format_addon_data' ), $addons );

tests/Services/AddonTest.php

@@ -46,7 +46,10 @@ public function test_returns_cached_addons_when_available(): void {
 	public function test_force_bypasses_cache_and_persists_formatted_result(): void {
 		$cache = $this->createMock( CacheInterface::class );
 		$cache->method( 'is_throttled' )->willReturn( false );
-		$cache->expects( $this->never() )->method( 'get' );
+		// `get` is consulted even on a forced refresh so the cached
+		// catalog can serve as a fallback when the API call fails or
+		// the throttle kicks in. Returning `false` mimics "no cache".
+		$cache->method( 'get' )->willReturn( false );
 		$cache->expects( $this->once() )
 			->method( 'set' )
 			->with(
@@ -84,7 +87,7 @@ static function ( array $value ): bool {
 		$this->assertTrue( $result[0]['is_premium'] );
 	}
 
-	public function test_returns_empty_array_on_api_error(): void {
+	public function test_returns_empty_when_api_errors_and_no_cache(): void {
 		$cache = $this->createMock( CacheInterface::class );
 		$cache->method( 'get' )->willReturn( false );
 		$cache->method( 'is_throttled' )->willReturn( false );
@@ -98,7 +101,32 @@ public function test_returns_empty_array_on_api_error(): void {
 		$this->assertSame( array(), ( new Addon( $this->plugin(), $cache, $factory ) )->get_addons() );
 	}
 
-	public function test_throttled_request_returns_empty_array_and_does_not_call_api(): void {
+	public function test_force_falls_back_to_cache_when_api_errors(): void {
+		$cached = array(
+			array(
+				'id'    => 42,
+				'title' => 'Cached',
+			),
+		);
+
+		$cache = $this->createMock( CacheInterface::class );
+		$cache->method( 'get' )->willReturn( $cached );
+		$cache->method( 'is_throttled' )->willReturn( false );
+		// API failed → catalog must not be overwritten.
+		$cache->expects( $this->never() )->method( 'set' );
+
+		$api = $this->createMock( ApiClientInterface::class );
+		$api->method( 'get' )->willReturn( new WP_Error( 'fail', 'no' ) );
+
+		$factory = $this->createMock( ApiFactory::class );
+		$factory->method( 'make_for_plugin' )->willReturn( $api );
+
+		$result = ( new Addon( $this->plugin(), $cache, $factory ) )->get_addons( true );
+
+		$this->assertSame( $cached, $result );
+	}
+
+	public function test_throttled_request_returns_empty_when_no_cache(): void {
 		$cache = $this->createMock( CacheInterface::class );
 		$cache->method( 'get' )->willReturn( false );
 		$cache->method( 'is_throttled' )->with( 'addons_check' )->willReturn( true );
@@ -112,6 +140,31 @@ public function test_throttled_request_returns_empty_array_and_does_not_call_api
 		$this->assertSame( array(), ( new Addon( $this->plugin(), $cache, $factory ) )->get_addons() );
 	}
 
+	public function test_force_falls_back_to_cache_when_throttled(): void {
+		$cached = array(
+			array(
+				'id'    => 9,
+				'title' => 'Stale but fine',
+			),
+		);
+
+		$cache = $this->createMock( CacheInterface::class );
+		$cache->method( 'get' )->willReturn( $cached );
+		$cache->method( 'is_throttled' )->with( 'addons_check' )->willReturn( true );
+		// Throttle blocked the API call → catalog stays as it is.
+		$cache->expects( $this->never() )->method( 'set' );
+
+		$api = $this->createMock( ApiClientInterface::class );
+		$api->expects( $this->never() )->method( 'get' );
+
+		$factory = $this->createMock( ApiFactory::class );
+		$factory->method( 'make_for_plugin' )->willReturn( $api );
+
+		$result = ( new Addon( $this->plugin(), $cache, $factory ) )->get_addons( true );
+
+		$this->assertSame( $cached, $result );
+	}
+
 	public function test_mark_requested_called_after_request(): void {
 		$cache = $this->createMock( CacheInterface::class );
 		$cache->method( 'get' )->willReturn( false );