Merge pull request #68 from stellarwp/bugfix/KAD-2308/updater-type-error

[BUGFIX]: Type error in filter_upgrader_pre_download()

src/Uplink/Admin/Package_Handler.php

@@ -5,6 +5,7 @@
 use StellarWP\Uplink\API;
 use StellarWP\Uplink\Config;
 use StellarWP\Uplink\Resources;
+use WP_Error;
 use WP_Upgrader;
 
 class Package_Handler {
@@ -17,17 +18,17 @@ class Package_Handler {
 	/**
 	 * Filters the package download step to store the downloaded file with a shorter file name.
 	 *
-	 * @param bool|\WP_Error $reply    Whether to bail without returning the package.
-	 *                                 Default false.
-	 * @param string         $package  The package file name or URL.
-	 * @param WP_Upgrader    $upgrader The WP_Upgrader instance.
-	 * @param array          $hook_extra Extra arguments passed to hooked filters.
+	 * @param  bool|WP_Error  $reply        Whether to bail without returning the package.
+	 *                                      Default false.
+	 * @param  string|null    $package      The package file name or URL.
+	 * @param  WP_Upgrader    $upgrader     The WP_Upgrader instance.
+	 * @param  array          $hook_extra   Extra arguments passed to hooked filters.
 	 *
-	 * @return mixed
+	 * @return string|bool|WP_Error
 	 */
-	public function filter_upgrader_pre_download( $reply, string $package, WP_Upgrader $upgrader, $hook_extra ) {
+	public function filter_upgrader_pre_download( $reply, $package, WP_Upgrader $upgrader, $hook_extra ) {
 		if ( empty( $package ) || 'invalid_license' === $package ) {
-			return new \WP_Error(
+			return new WP_Error(
 				'download_failed',
 				__( 'Failed to update plugin. Check your license details first.', '%TEXTDOMAIN%' ),
 				''
@@ -149,7 +150,7 @@ protected function is_uplink_package_url( string $package, $hook_extra ) : bool
 	 * @param string $package The URI of the package. If this is the full path to an
 	 *                        existing local file, it will be returned untouched.
 	 *
-	 * @return string|bool|\WP_Error The full path to the downloaded package file, or a WP_Error object.
+	 * @return string|bool|WP_Error The full path to the downloaded package file, or a WP_Error object.
 	 */
 	protected function download( string $package ) {
 		if ( empty( $this->filesystem ) ) {
@@ -174,7 +175,7 @@ protected function download( string $package ) {
 		$download_file = download_url( $package );
 
 		if ( is_wp_error( $download_file ) ) {
-			return new \WP_Error(
+			return new WP_Error(
 				'download_failed',
 				$this->upgrader->strings['download_failed'],
 				$download_file->get_error_message()

src/Uplink/Admin/Provider.php

@@ -152,11 +152,11 @@ public function store_admin_notices( $page ): void {
 	 *
 	 * @param  bool|\WP_Error  $reply       Whether to bail without returning the package.
 	 *                                      Default false.
-	 * @param  string          $package     The package file name or URL.
+	 * @param  string|null     $package     The package file name or URL.
 	 * @param  \WP_Upgrader    $upgrader    The WP_Upgrader instance.
 	 * @param  array           $hook_extra  Extra arguments passed to hooked filters.
 	 *
-	 * @return mixed
+	 * @return string|bool|\WP_Error
 	 */
 	public function filter_upgrader_pre_download( $reply, $package, $upgrader, $hook_extra ) {
 		return $this->container->get( Package_Handler::class )->filter_upgrader_pre_download( $reply, $package, $upgrader, $hook_extra );

tests/wpunit/Admin/Package_HandlerTest.php

@@ -1,13 +1,14 @@
-<?php
+<?php declare( strict_types=1 );
 
 namespace wpunit\Admin;
 
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use StellarWP\Uplink\Admin\Package_Handler;
 use StellarWP\Uplink\Tests\UplinkTestCase;
+use WP_Upgrader;
 
-class Package_HandlerTest extends UplinkTestCase {
+final class Package_HandlerTest extends UplinkTestCase {
 
 	use ProphecyTrait;
 
@@ -25,18 +26,27 @@ protected function setUp(): void {
 		$this->filesystem = $this->prophesize( \WP_Filesystem_Base::class );
 	}
 
-	public function test_it_should_return_WP_Error_if_the_package_is_empty() {
-		$upgrader = $this->prophesize( \WP_Upgrader::class );
+	public function test_it_returns_WP_Error_if_the_package_is_empty_string() {
+		$upgrader = $this->prophesize( WP_Upgrader::class );
 
 		$sut      = new Package_Handler();
 		$filtered = $sut->filter_upgrader_pre_download( false, '', $upgrader->reveal(), [] );
 
 		$this->assertWPError( $filtered );
 	}
 
+	public function test_it_returns_WP_Error_if_the_package_is_null() {
+		$upgrader = $this->prophesize( WP_Upgrader::class );
+
+		$sut      = new Package_Handler();
+		$filtered = $sut->filter_upgrader_pre_download( false, null, $upgrader->reveal(), [] );
+
+		$this->assertWPError( $filtered );
+	}
+
 	public function test_it_should_not_filter_the_download_if_the_pu_get_download_flag_is_not_1() {
 		$package  = 'http://update.tri.be?pu_get_download=0';
-		$upgrader = $this->prophesize( \WP_Upgrader::class );
+		$upgrader = $this->prophesize( WP_Upgrader::class );
 
 		$sut      = new Package_Handler();
 		$filtered = $sut->filter_upgrader_pre_download( false, $package, $upgrader->reveal(), [] );
@@ -46,7 +56,7 @@ public function test_it_should_not_filter_the_download_if_the_pu_get_download_fl
 
 	public function it_should_return_WP_Error_if_the_file_was_not_found() {
 		$package           = add_query_arg( [ 'pu_get_download' => '1' ], 'http://foo.bar' );
-		$upgrader          = $this->getMockBuilder( \WP_Upgrader::class )->getMock();
+		$upgrader          = $this->getMockBuilder( WP_Upgrader::class )->getMock();
 		$upgrader->strings = [ 'download_failed' => 'meh' ];
 		$skin              = $this->prophesize( \WP_Upgrader_Skin::class );
 
@@ -56,15 +66,15 @@ public function it_should_return_WP_Error_if_the_file_was_not_found() {
 		$upgrader->skin 	  	  = $skin->reveal();
 
 		$sut      = new Package_Handler();
-		$filtered = $sut->filter_upgrader_pre_download( false, $package, $upgrader );
+		$filtered = $sut->filter_upgrader_pre_download( false, $package, $upgrader, [] );
 
 		$this->assertFalse( $filtered );
 	}
 
 	public function it_should_move_the_file_and_return_a_shorter_named_version_of_it() {
 		$url      = wp_get_attachment_url( $this->factory()->attachment->create_upload_object( codecept_data_dir( 'some-file.txt' ) ) );
 		$package  = add_query_arg( [ 'pu_get_download' => '1' ], $url );
-		$upgrader = $this->getMockBuilder( \WP_Upgrader::class )->getMock();
+		$upgrader = $this->getMockBuilder( WP_Upgrader::class )->getMock();
 		$skin     = $this->prophesize( \WP_Upgrader_Skin::class );
 
 		$skin->feedback( 'downloading_package', $package )->shouldBeCalled();
@@ -86,7 +96,7 @@ public function it_should_move_the_file_and_return_a_shorter_named_version_of_it
 		} );
 
 		$sut      = new Package_Handler();
-		$filtered = $sut->filter_upgrader_pre_download( false, $package, $upgrader );
+		$filtered = $sut->filter_upgrader_pre_download( false, $package, $upgrader, [] );
 
 		$expected_dir           = dirname( $real_temp_file_name );
 		$expected_file_exension = pathinfo( $real_temp_file_name, PATHINFO_EXTENSION );