Merge pull request #111 from xwp/fix/UVP-210

Cache api check
xwp · May 19, 2020 · 3ef01a2 · 3ef01a2
2 parents aeae051 + 211b4e2
commit 3ef01a2
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 23 deletions.
diff --git a/php/class-api.php b/php/class-api.php
@@ -134,13 +134,15 @@ public function search( $query, $page = 1, $per_page = 10, $orientation = null,
 	 * @return array|WP_Error
 	 */
 	public function send_request( $path, array $args = [] ) {
-		$api_check = $this->check_api_credentials();
-		if ( is_wp_error( $api_check ) ) {
-			return $api_check;
+		if ( ! isset( $args['client_id'] ) ) {
+			$api_check = $this->check_api_credentials();
+			if ( is_wp_error( $api_check ) ) {
+				return $api_check;
+			}
+			$credentials       = $this->plugin->settings->get_credentials();
+			$args['client_id'] = $credentials['applicationId'];
 		}
 		$url               = 'https://api.unsplash.com' . $path;
-		$credentials       = $this->plugin->settings->get_credentials();
-		$args['client_id'] = $credentials['applicationId'];
 		$cache_key         = $args;
 		$cache_key['path'] = $path;
 		$cache             = new Api_Cache( $cache_key );
@@ -265,30 +267,56 @@ public function check_api_credentials() {
 	 * Check the API status.
 	 *
 	 * @param array $credentials The API credentials.
+	 * @param bool  $cached Optional. If the request should be cached or not. Default to false.
+	 * @param bool  $wp_error Optional. Return WP_Error object. Default to false.
 	 * @return bool|WP_Error
 	 */
-	public function check_api_status( $credentials = [] ) {
+	public function check_api_status( $credentials = [], $cached = false, $wp_error = false ) {
 		if ( empty( $credentials ) ) {
 			$credentials = $this->plugin->settings->get_credentials();
 		}
 
 		if ( empty( $credentials['applicationId'] ) ) {
-			return false;
+			if ( ! $wp_error ) {
+				return false;
+			}
+			$settings_link = sprintf(
+				'<a href="%1$s">%2$s</a>',
+				esc_url( get_admin_url( null, 'options-general.php?page=unsplash' ) ),
+				esc_html__( 'Unsplash', 'unsplash' )
+			);
+
+			return new WP_Error(
+				'missing_api_credential',
+				sprintf(
+					/* translators: %s: Link to Unsplash settings page. */
+					esc_html__( 'The Unsplash plugin has not been provided the API access key. Please visit the %s settings page and confirm that the API access key has been provided.', 'unsplash' ),
+					$settings_link
+				),
+				[
+					'status' => rest_authorization_required_code(),
+				]
+			);
 		}
 
 		$args = [
 			'client_id' => $credentials['applicationId'],
 			'page'      => 1,
 			'per_page'  => 1,
 		];
+		if ( ! $cached ) {
+			$args['cb'] = microtime();
+		}
 
-		$response = $this->get_remote( add_query_arg( $args, 'https://api.unsplash.com/photos' ) );
-
-		if ( ! is_wp_error( $response ) && 200 === wp_remote_retrieve_response_code( $response ) ) {
+		$response = $this->send_request( '/photos', $args );
+		if ( ! is_wp_error( $response ) ) {
 			return true;
 		}
+		if ( ! $wp_error ) {
+			return false;
+		}
 
-		return false;
+		return $response;
 	}
 
 	/**

diff --git a/php/class-plugin.php b/php/class-plugin.php
@@ -525,12 +525,18 @@ public function admin_notice() {
 		}
 
 		$credentials = $this->settings->get_credentials();
-		if (
-			! empty( $credentials['applicationId'] )
-			&& $this->rest_controller->api->check_api_credentials()
-			&& $this->rest_controller->api->check_api_status( $credentials )
-		) {
-			return false;
+		if ( ! empty( $credentials['applicationId'] ) && $this->rest_controller->api->check_api_credentials() ) {
+			$status = $this->rest_controller->api->check_api_status( $credentials, true, true );
+			if ( ! is_wp_error( $status ) ) {
+				return false;
+			}
+
+			$message = $status->get_error_message();
+			if ( $message ) {
+				printf( '<div class="notice notice-error is-dismissible"><p>%1$s</p></div>', wp_kses_post( $message ) );
+			}
+
+			return;
 		}
 
 		$class   = 'notice notice-warning is-dismissible';

diff --git a/tests/phpunit/php/class-test-api.php b/tests/phpunit/php/class-test-api.php
@@ -7,6 +7,8 @@
 
 namespace Unsplash;
 
+use WP_Error;
+
 /**
  * Tests for the API class.
  *
@@ -189,7 +191,7 @@ public function test_check_api_status() {
 		$plugin = new Plugin();
 		$plugin->init();
 		$api    = new API( $plugin );
-		$result = $api->check_api_status();
+		$result = $api->check_api_status( [], false );
 		$this->assertTrue( $result );
 	}
 
@@ -201,9 +203,11 @@ public function test_check_api_status() {
 	public function test_check_api_status_missing_credentials() {
 		$plugin = new Plugin();
 		$plugin->init();
-		$api    = new API( $plugin );
-		$result = $api->check_api_status( [ 'applicationId' => '' ] );
-		$this->assertFalse( $result );
+		$api     = new API( $plugin );
+		$result1 = $api->check_api_status( [ 'applicationId' => '' ], false );
+		$result2 = $api->check_api_status( [ 'applicationId' => '' ], false, true );
+		$this->assertFalse( $result1 );
+		$this->assertInstanceOf( WP_Error::class, $result2 );
 	}
 
 	/**
@@ -216,11 +220,28 @@ public function test_check_api_status_response_failed() {
 		$plugin->init();
 		$api = new API( $plugin );
 		add_filter( 'http_response', '__return_false' );
-		$result = $api->check_api_status();
+		$result1 = $api->check_api_status( [], false );
+		$result2 = $api->check_api_status( [], false, true );
 		remove_filter( 'http_response', '__return_false' );
-		$this->assertFalse( $result );
+		$this->assertFalse( $result1 );
+		$this->assertInstanceOf( WP_Error::class, $result2 );
 	}
 
+	/**
+	 * Test check_api_status().
+	 *
+	 * @covers \Unsplash\API::check_api_status()
+	 */
+	public function test_check_api_cached() {
+		$plugin = new Plugin();
+		$plugin->init();
+		$api = new API( $plugin );
+		add_filter( 'http_response', '__return_false' );
+		$result1 = $api->check_api_status( [], true );
+		$result2 = $api->check_api_status( [], true );
+		remove_filter( 'http_response', '__return_false' );
+		$this->assertSame( $result1, $result2 );
+	}
 	/**
 	 * Return a valid url but not the correct one.
 	 *

diff --git a/tests/phpunit/php/class-test-plugin.php b/tests/phpunit/php/class-test-plugin.php
@@ -294,6 +294,23 @@ public function test_admin_notice() {
 		remove_filter( 'unsplash_api_credentials', [ $this, 'disable_unsplash_api_credentials' ] );
 	}
 
+	/**
+	 * Test for admin_notice()
+	 *
+	 * @see Plugin::admin_notice()
+	 */
+	public function test_admin_notice_connection() {
+		add_filter( 'unsplash_api_credentials', [ $this, 'invalid_unsplash_api_credentials' ] );
+		wp_set_current_user( self::$admin_id );
+		set_current_screen( 'post.php' );
+		$plugin = get_plugin_instance();
+		ob_start();
+		$plugin->admin_notice();
+		$output = ob_get_clean();
+		$this->assertContains( 'The Unsplash API credentials supplied are not authorized. Please visit the Unsplash settings page to reconnect to Unsplash now.', wp_strip_all_tags( $output ) );
+		remove_filter( 'unsplash_api_credentials', [ $this, 'invalid_unsplash_api_credentials' ] );
+	}
+
 	/**
 	 * Test for admin_notice()
 	 *
@@ -365,4 +382,17 @@ public function disable_unsplash_api_credentials() {
 			'utmSource'     => '',
 		];
 	}
+
+	/**
+	 * Invalid Unsplash api details.
+	 *
+	 * @return array
+	 */
+	public function invalid_unsplash_api_credentials() {
+		return [
+			'applicationId' => 'foo-bar',
+			'secret'        => '',
+			'utmSource'     => '',
+		];
+	}
 }