Merge pull request Yoast#21178 from Yoast/21153-author-archive-pages-…

…have-slowduplicate-db-queries-looking-for-gravatars Fixes a bug where the attachment would never be marked as external.
josevarghese · Mar 13, 2024 · d84bd62 · d84bd62
2 parents afe0eee + 43ffa8f
commit d84bd62
Show file tree

Hide file tree

Showing 3 changed files with 135 additions and 3 deletions.
diff --git a/src/helpers/image-helper.php b/src/helpers/image-helper.php
@@ -304,7 +304,8 @@ public function get_best_attachment_variation( $attachment_id ) {
 	 */
 	public function get_attachment_by_url( $url, $use_link_table = true ) {
 		// Don't try to do this for external URLs.
-		if ( $this->url_helper->get_link_type( $url ) === SEO_Links::TYPE_EXTERNAL ) {
+		$parsed_url = \wp_parse_url( $url );
+		if ( $this->url_helper->get_link_type( $parsed_url ) === SEO_Links::TYPE_EXTERNAL ) {
 			return 0;
 		}
 
@@ -332,8 +333,15 @@ public function get_attachment_by_url( $url, $use_link_table = true ) {
 		if ( ! $use_link_table ) {
 			return WPSEO_Image_Utils::get_attachment_by_url( $url );
 		}
+		$cache_key = 'attachment_seo_link_object_' . \md5( $url );
 
-		$link = $this->seo_links_repository->find_one_by_url( $url );
+		$found = false;
+		$link  = \wp_cache_get( $cache_key, 'yoast-seo-attachment-link', false, $found );
+
+		if ( $found === false ) {
+			$link = $this->seo_links_repository->find_one_by_url( $url );
+			\wp_cache_set( $cache_key, $link, 'yoast-seo-attachment-link', \MINUTE_IN_SECONDS );
+		}
 		if ( ! \is_a( $link, SEO_Links::class ) ) {
 			return WPSEO_Image_Utils::get_attachment_by_url( $url );
 		}

diff --git a/tests/Unit/Helpers/Image_Helper_Test.php b/tests/Unit/Helpers/Image_Helper_Test.php
@@ -521,13 +521,42 @@ public function test_get_attachment_image_url_no_url_found() {
 		$this->assertEquals( '', $this->instance->get_attachment_image_url( 1337, 'full' ) );
 	}
 
+	/**
+	 * Tests the get_attachment_by_url function with an external image url.
+	 *
+	 * @covers ::get_attachment_by_url
+	 * @return void
+	 */
+	public function test_get_attachment_by_url_with_external_url_image() {
+		Monkey\Functions\expect( 'wp_parse_url' )
+			->once()
+			->with( 'https://example.com/image.jpg' )
+			->andReturn(
+				[
+					'scheme' => 'https',
+					'host'   => 'example.com',
+				]
+			);
+		$this->url_helper->expects( 'get_link_type' )->andReturn( SEO_Links::TYPE_EXTERNAL );
+		$this->assertEquals( 0, $this->actual_instance->get_attachment_by_url( 'https://example.com/image.jpg' ) );
+	}
+
 	/**
 	 * Tests the get_attachment_by_url function with an external image.
 	 *
 	 * @covers ::get_attachment_by_url
 	 * @return void
 	 */
 	public function test_get_attachment_by_url_with_external_image() {
+		Monkey\Functions\expect( 'wp_parse_url' )
+			->once()
+			->with( '' )
+			->andReturn(
+				[
+					'scheme' => 'https',
+					'host'   => 'example.com',
+				]
+			);
 		$this->url_helper->expects( 'get_link_type' )->andReturn( SEO_Links::TYPE_EXTERNAL );
 		$this->assertEquals( 0, $this->actual_instance->get_attachment_by_url( '' ) );
 	}
@@ -539,7 +568,15 @@ public function test_get_attachment_by_url_with_external_image() {
 	 * @return void
 	 */
 	public function test_get_attachment_by_url_with_existing_indexable() {
-
+		Monkey\Functions\expect( 'wp_parse_url' )
+			->once()
+			->with( '' )
+			->andReturn(
+				[
+					'scheme' => 'https',
+					'host'   => 'example.com',
+				]
+			);
 		$indexable                  = new Indexable_Mock();
 		$indexable->object_type     = 'post';
 		$indexable->object_sub_type = 'attachment';
@@ -557,12 +594,31 @@ public function test_get_attachment_by_url_with_existing_indexable() {
 	 * @return void
 	 */
 	public function test_get_attachment_by_url_with_existing_link() {
+		Monkey\Functions\expect( 'wp_parse_url' )
+			->once()
+			->with( 'a_dir/something' )
+			->andReturn(
+				[
+					'scheme' => 'https',
+					'host'   => 'example.com',
+				]
+			);
+
+		$url = \md5( 'a_dir/something' );
+		Monkey\Functions\expect( 'wp_cache_get' )
+			->once()
+			->with( 'attachment_seo_link_object_' . $url, 'yoast-seo-attachment-link', false, false )
+			->andReturn( false );
+
 		$link                 = new SEO_Links_Mock();
 		$link->target_post_id = 17;
 		$this->url_helper->expects( 'get_link_type' )->andReturn( SEO_Links::TYPE_INTERNAL );
 		$this->options_helper->expects( 'get' )->with( 'disable-attachment' )->andReturn( true );
 		$this->indexable_seo_links_repository->expects( 'find_one_by_url' )->andReturn( $link );
 
+		Monkey\Functions\expect( 'wp_cache_set' )
+			->once()
+			->with( 'attachment_seo_link_object_' . $url, $link, 'yoast-seo-attachment-link', \MINUTE_IN_SECONDS );
 		$this->assertEquals( 17, $this->actual_instance->get_attachment_by_url( 'a_dir/something' ) );
 	}
 }
diff --git a/tests/WP/Helpers/Get_Attachment_By_Url_Image_Helper_Test.php b/tests/WP/Helpers/Get_Attachment_By_Url_Image_Helper_Test.php
@@ -0,0 +1,68 @@
+<?php
+
+namespace Yoast\WP\SEO\Tests\WP\Helpers;
+
+use Yoast\WP\SEO\Helpers\Image_Helper;
+use Yoast\WP\SEO\Repositories\Indexable_Repository;
+use Yoast\WP\SEO\Repositories\SEO_Links_Repository;
+use Yoast\WP\SEO\Tests\WP\TestCase;
+
+/**
+ * Integration Test Class for the Image_Helper class.
+ *
+ * @phpcs:disable Yoast.NamingConventions.ObjectNameDepth.MaxExceeded -- Needed to show specific method being tested in this file.
+ *
+ * @coversDefaultClass \Yoast\WP\SEO\Helpers\Image_Helper
+ */
+final class Get_Attachment_By_Url_Image_Helper_Test extends TestCase {
+
+	/**
+	 * The instance.
+	 *
+	 * @var Image_Helper
+	 */
+	private $instance;
+
+	/**
+	 * Sets up the test class.
+	 *
+	 * @return void
+	 */
+	public function set_up(): void {
+		parent::set_up();
+		$this->instance = new Image_Helper( \YoastSEO()->classes->get( Indexable_Repository::class ), \YoastSEO()->classes->get( SEO_Links_Repository::class ), \YoastSEO()->helpers->options, \YoastSEO()->helpers->url );
+		\YoastSEO()->helpers->options->set( 'disable-attachment', true );
+
+		global $wpdb;
+
+		$wpdb->insert(
+			$wpdb->prefix . 'yoast_seo_links',
+			[
+				'url'                 => 'a_dir/something',
+				'type'                => 'internal',
+				'indexable_id'        => '101',
+				'post_id'             => '110',
+				'target_post_id'      => '112',
+				'target_indexable_id' => '344',
+				'id'                  => '113',
+			]
+		);
+	}
+
+	/**
+	 * Tests if the query cached is filled when `get_attachment_by_url` is called multiple times.
+	 *
+	 * @covers ::get_attachment_by_url
+	 * @return void
+	 */
+	public function test_get_attachment_by_url_with_existing_link_and_cache() {
+		$url       = 'a_dir/something';
+		$cache_key = 'attachment_seo_link_object_' . \md5( $url );
+		$link      = $this->instance->get_attachment_by_url( $url );
+
+		$link_cache = \wp_cache_get( $cache_key, 'yoast-seo-attachment-link' );
+		$this->assertSame( $link, $link_cache->target_post_id );
+
+		$this->assertSame( 112, $this->instance->get_attachment_by_url( 'a_dir/something' ) );
+	}
+}