From d3374a85b86f23bf8a56ff31cf007e70aec4e483 Mon Sep 17 00:00:00 2001 From: Justin Frydman Date: Fri, 1 Mar 2024 09:55:10 -0700 Subject: [PATCH 1/2] Fix type error in filter_upgrader_pre_download --- src/Uplink/Admin/Package_Handler.php | 21 ++++++++-------- src/Uplink/Admin/Provider.php | 4 ++-- tests/wpunit/Admin/Package_HandlerTest.php | 28 +++++++++++++++------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/Uplink/Admin/Package_Handler.php b/src/Uplink/Admin/Package_Handler.php index ee34e3b2..2c47802d 100644 --- a/src/Uplink/Admin/Package_Handler.php +++ b/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() diff --git a/src/Uplink/Admin/Provider.php b/src/Uplink/Admin/Provider.php index dd8ae331..4b9a433f 100644 --- a/src/Uplink/Admin/Provider.php +++ b/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 ); diff --git a/tests/wpunit/Admin/Package_HandlerTest.php b/tests/wpunit/Admin/Package_HandlerTest.php index 4af7cac5..1fb52c16 100644 --- a/tests/wpunit/Admin/Package_HandlerTest.php +++ b/tests/wpunit/Admin/Package_HandlerTest.php @@ -1,4 +1,4 @@ -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(), [] ); @@ -34,9 +35,18 @@ public function test_it_should_return_WP_Error_if_the_package_is_empty() { $this->assertWPError( $filtered ); } + public function test_it_return_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,7 +66,7 @@ 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 ); } @@ -64,7 +74,7 @@ public function it_should_return_WP_Error_if_the_file_was_not_found() { 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 ); From 1f8c27769d781ae759f8c70fcfe19a5102e43df8 Mon Sep 17 00:00:00 2001 From: Justin Frydman Date: Fri, 1 Mar 2024 10:03:23 -0700 Subject: [PATCH 2/2] Fix typo --- tests/wpunit/Admin/Package_HandlerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/wpunit/Admin/Package_HandlerTest.php b/tests/wpunit/Admin/Package_HandlerTest.php index 1fb52c16..322c4622 100644 --- a/tests/wpunit/Admin/Package_HandlerTest.php +++ b/tests/wpunit/Admin/Package_HandlerTest.php @@ -35,7 +35,7 @@ public function test_it_returns_WP_Error_if_the_package_is_empty_string() { $this->assertWPError( $filtered ); } - public function test_it_return_WP_Error_if_the_package_is_null() { + public function test_it_returns_WP_Error_if_the_package_is_null() { $upgrader = $this->prophesize( WP_Upgrader::class ); $sut = new Package_Handler();