Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use proper URL for resized images in the ImageObject schema #21622

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"Yoast\\WP\\SEO\\Composer\\Actions::check_coding_standards"
],
"check-cs-thresholds": [
"@putenv YOASTCS_THRESHOLD_ERRORS=2480",
"@putenv YOASTCS_THRESHOLD_ERRORS=2476",
"@putenv YOASTCS_THRESHOLD_WARNINGS=252",
"Yoast\\WP\\SEO\\Composer\\Actions::check_cs_thresholds"
],
Expand Down
2 changes: 1 addition & 1 deletion src/generators/schema/main-image.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function generate() {

// The first image in the content.
if ( $this->context->main_image_url ) {
return $this->helpers->schema->image->generate_from_url( $image_id, $this->context->main_image_url );
return $this->helpers->schema->image->generate_from_url( $image_id, $this->context->main_image_url, '', false, true, true );
}

return false;
Expand Down
58 changes: 47 additions & 11 deletions src/helpers/schema/image-helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,59 @@ public function __construct( HTML_Helper $html, Language_Helper $language, Main_
/**
* Find an image based on its URL and generate a Schema object for it.
*
* @param string $schema_id The `@id` to use for the returned image.
* @param string $url The image URL to base our object on.
* @param string $caption An optional caption.
* @param bool $add_hash Whether a hash will be added as a suffix in the @id.
* @param bool $use_link_table Whether the SEO Links table will be used to retrieve the id.
*
* @return array Schema ImageObject array.
* @param string $schema_id The `@id` to use for the returned image.
* @param string $url The image URL to base our object on.
* @param string $caption An optional caption.
* @param bool $add_hash Whether a hash will be added as a suffix in the @id.
* @param bool $use_link_table Whether the SEO Links table will be used to retrieve the id.
* @param bool $check_for_resized Whether to account for resized images.
*
* @return array<string, string> Schema ImageObject array.
*/
public function generate_from_url( $schema_id, $url, $caption = '', $add_hash = false, $use_link_table = true ) {
public function generate_from_url( $schema_id, $url, $caption = '', $add_hash = false, $use_link_table = true, $check_for_resized = false ) {
$attachment_id = $this->image->get_attachment_by_url( $url, $use_link_table );
if ( $attachment_id > 0 ) {
// First check if we should use the resized image and whether it is a resized image indeed.
if ( $check_for_resized && \preg_match( '/-(\d+)x(\d+)\.(jpg|jpeg|png|gif|webp)$/', $url ) ) {
return $this->generate_from_resized_url( $schema_id, $attachment_id, $caption, $add_hash, $url );
}

return $this->generate_from_attachment_id( $schema_id, $attachment_id, $caption, $add_hash );
}

return $this->simple_image_object( $schema_id, $url, $caption, $add_hash );
}

/**
* Retrieve data about an image from the database and use it to generate a Schema object.
*
* @param string $schema_id The `@id` to use for the returned image.
* @param int $attachment_id The attachment to retrieve data from.
* @param string $caption The caption string, if there is one.
* @param bool $add_hash Whether a hash will be added as a suffix in the @id.
* @param string $resized_url The url of the resized image.
*
* @return array<string, string> Schema ImageObject array.
*/
public function generate_from_resized_url( $schema_id, $attachment_id, $caption = '', $add_hash = false, $resized_url = '' ) {
$data = $this->generate_object();

$id_suffix = ( $add_hash ) ? \md5( $resized_url ) : '';

$data['@id'] = $schema_id . $id_suffix;
$data['url'] = $resized_url;
$data['contentUrl'] = $resized_url;

if ( \preg_match( '/-(\d+)x(\d+)\.(jpg|jpeg|png|gif|webp)$/', $resized_url, $matches ) ) {
$data['width'] = $matches[1];
$data['height'] = $matches[2];
}

$data = $this->add_caption( $data, $attachment_id, $caption );

return $data;
}

/**
* Retrieve data about an image from the database and use it to generate a Schema object.
*
Expand All @@ -73,7 +109,7 @@ public function generate_from_url( $schema_id, $url, $caption = '', $add_hash =
* @param string $caption The caption string, if there is one.
* @param bool $add_hash Whether a hash will be added as a suffix in the @id.
*
* @return array Schema ImageObject array.
* @return array<string, string> Schema ImageObject array.
*/
public function generate_from_attachment_id( $schema_id, $attachment_id, $caption = '', $add_hash = false ) {
$data = $this->generate_object();
Expand All @@ -98,7 +134,7 @@ public function generate_from_attachment_id( $schema_id, $attachment_id, $captio
* @param string $caption The caption string, if there is one.
* @param bool $add_hash Whether a hash will be added as a suffix in the @id.
*
* @return array Schema ImageObject array.
* @return array<string, string> Schema ImageObject array.
*/
public function generate_from_attachment_meta( $schema_id, $attachment_meta, $caption = '', $add_hash = false ) {
$data = $this->generate_object();
Expand All @@ -125,7 +161,7 @@ public function generate_from_attachment_meta( $schema_id, $attachment_meta, $ca
* @param string $caption A caption, if set.
* @param bool $add_hash Whether a hash will be added as a suffix in the @id.
*
* @return array Schema ImageObject array.
* @return array<string, string> Schema ImageObject array.
*/
public function simple_image_object( $schema_id, $url, $caption = '', $add_hash = false ) {
$data = $this->generate_object();
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Generators/Schema/Main_Image_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public function test_generate_from_url() {

$this->schema_image->expects( 'generate_from_url' )
->once()
->with( $image_id, $image_url )
->with( $image_id, $image_url, '', false, true, true )
->andReturn( $image_schema );

self::assertEquals( $image_schema, $this->instance->generate() );
Expand Down
168 changes: 168 additions & 0 deletions tests/Unit/Helpers/Schema/Image_Helper_Test.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,78 @@ public function test_generate_from_url_with_found_attachment_id() {
);
}

/**
* Tests generating the schema from url with a found attachment id.
*
* @covers ::generate_from_url
* @dataProvider provide_generate_from_resized_url_with_found_attachment_id
*
* @param string $url The url of the image to create schema for.
* @param int $generate_from_resized_times The times we generate from resized URL.
* @param int $generate_from_attachment_id_times The times we generate from attachment ID.
*
* @return void
*/
public function test_generate_from_resized_url_with_found_attachment_id( $url, $generate_from_resized_times, $generate_from_attachment_id_times ) {
$this->image
->expects( 'get_attachment_by_url' )
->once()
->with( $url, true )
->andReturn( 1337 );

$this->instance
->expects( 'generate_from_resized_url' )
->times( $generate_from_resized_times )
->with( '#schema-image-ABC', 1337, '', false, $url )
->andReturn( [] );

$this->instance
->expects( 'generate_from_attachment_id' )
->times( $generate_from_attachment_id_times )
->with( '#schema-image-ABC', 1337, '', false )
->andReturn( [] );

$this->assertEquals(
[],
$this->instance->generate_from_url( '#schema-image-ABC', $url, '', false, true, true )
);
}

/**
* Data provider for test_generate_from_resized_url_with_found_attachment_id.
*
* @return array<string, string>
*/
public static function provide_generate_from_resized_url_with_found_attachment_id() {
return [
'Generate for resized image' => [
'url' => 'https://example.org/image-300x300.jpg',
'generate_from_resized_times' => 1,
'generate_from_attachment_id_times' => 0,
],
'Generate for non-resized image' => [
'url' => 'https://example.org/image.jpg',
'generate_from_resized_times' => 0,
'generate_from_attachment_id_times' => 1,
],
'Generate for non-resized image that had the same format with resized in filename' => [
'url' => 'https://example.org/image-300x300-1.jpg',
'generate_from_resized_times' => 0,
'generate_from_attachment_id_times' => 1,
],
'Generate for non-resized image that has a similar format with resized but with capital X' => [
'url' => 'https://example.org/image-300X300.jpg',
'generate_from_resized_times' => 0,
'generate_from_attachment_id_times' => 1,
],
'Generate for other file types that are not image' => [
'url' => 'https://example.org/image.pdf',
'generate_from_resized_times' => 0,
'generate_from_attachment_id_times' => 1,
],
];
}

/**
* Tests generating the schema from url no found attachment id.
*
Expand Down Expand Up @@ -168,6 +240,102 @@ public function test_generate_from_attachment_id_with_caption_and_image_dimensio
);
}

/**
* Tests the generate_from_resized_url method.
*
* @covers ::generate_from_resized_url
* @covers ::generate_object
* @covers ::add_image_size
* @covers ::add_caption
* @dataProvider provide_generate_from_resized_url
*
* @param string $url The url of the image to create schema for.
* @param string|array<string,string> $expected The times we generate from resized URL.
*
* @return void
*/
public function test_generate_from_resized_url( $url, $expected ) {
$this->image
->expects( 'get_attachment_image_url' )
->never();

$this->image
->expects( 'get_metadata' )
->never();

$this->language
->expects( 'add_piece_language' )
->once()
->andReturnUsing( [ $this, 'set_language' ] );

$this->assertEquals(
$expected,
$this->instance->generate_from_resized_url(
'https://example.com/#/schema/logo/image/',
1337,
'Caption',
false,
$url
)
);
}

/**
* Data provider for test_generate_from_resized_url.
*
* @return array<string, string|array>
*/
public static function provide_generate_from_resized_url() {
return [
'Generate for resized image' => [
'url' => 'https://example.org/image-400x300.jpg',
'expected' => [
'@type' => 'ImageObject',
'@id' => 'https://example.com/#/schema/logo/image/',
'url' => 'https://example.org/image-400x300.jpg',
'contentUrl' => 'https://example.org/image-400x300.jpg',
'width' => '400',
'height' => '300',
'caption' => 'Caption',
'inLanguage' => 'language',
],
],
'Generate for non-resized image' => [
'url' => 'https://example.org/image.jpg',
'expected' => [
'@type' => 'ImageObject',
'@id' => 'https://example.com/#/schema/logo/image/',
'url' => 'https://example.org/image.jpg',
'contentUrl' => 'https://example.org/image.jpg',
'caption' => 'Caption',
'inLanguage' => 'language',
],
],
'Generate for non-resized image that had the same format with resized in filename' => [
'url' => 'https://example.org/image-300x300-1.jpg',
'expected' => [
'@type' => 'ImageObject',
'@id' => 'https://example.com/#/schema/logo/image/',
'url' => 'https://example.org/image-300x300-1.jpg',
'contentUrl' => 'https://example.org/image-300x300-1.jpg',
'caption' => 'Caption',
'inLanguage' => 'language',
],
],
'Generate for non-resized image that has a similar format with resized but with capital X' => [
'url' => 'https://example.org/image-300X300.jpg',
'expected' => [
'@type' => 'ImageObject',
'@id' => 'https://example.com/#/schema/logo/image/',
'url' => 'https://example.org/image-300X300.jpg',
'contentUrl' => 'https://example.org/image-300X300.jpg',
'caption' => 'Caption',
'inLanguage' => 'language',
],
],
];
}

/**
* Tests the generate_from_attachment_id method.
*
Expand Down