Skip to content

Commit

Permalink
Replace validity filter with sanitization filter
Browse files Browse the repository at this point in the history
  • Loading branch information
@westonruter
westonruter committed Dec 16, 2024
1 parent 6005f4a commit 94c5e6b
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 416 deletions.
23 changes: 10 additions & 13 deletions plugins/image-prioritizer/helper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,33 +270,30 @@ static function ( $host ) {
* @since n.e.x.t
* @access private
*
* @param bool|WP_Error|mixed $validity Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
* @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema.
* @return bool|WP_Error Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
* @param array<string, mixed>|mixed $data URL Metric data.
* @return array<string, mixed> Sanitized URL Metric data.
*
* @noinspection PhpDocMissingThrowsInspection
* @throws OD_Data_Validation_Exception Except it won't because lcpElementExternalBackgroundImage is not a required property.
*/
function image_prioritizer_filter_store_url_metric_validity( $validity, OD_Strict_URL_Metric $url_metric ) {
if ( ! is_bool( $validity ) && ! ( $validity instanceof WP_Error ) ) {
$validity = (bool) $validity;
function image_prioritizer_filter_url_metric_data_pre_storage( $data ): array {
if ( ! is_array( $data ) ) {
$data = array();
}

$data = $url_metric->get( 'lcpElementExternalBackgroundImage' );
if ( is_array( $data ) && isset( $data['url'] ) && is_string( $data['url'] ) ) { // Note: The isset() and is_string() checks aren't necessary since the JSON Schema enforces them to be set.
$image_validity = image_prioritizer_validate_background_image_url( $data['url'] );
if ( isset( $data['lcpElementExternalBackgroundImage']['url'] ) && is_string( $data['lcpElementExternalBackgroundImage']['url'] ) ) {
$image_validity = image_prioritizer_validate_background_image_url( $data['lcpElementExternalBackgroundImage']['url'] );
if ( is_wp_error( $image_validity ) ) {
/**
* No WP_Exception is thrown by wp_trigger_error() since E_USER_ERROR is not passed as the error level.
*
* @noinspection PhpUnhandledExceptionInspection
*/
wp_trigger_error( __FUNCTION__, $image_validity->get_error_message() . ' Background image URL: ' . $data['url'] );
$url_metric->unset( 'lcpElementExternalBackgroundImage' );
wp_trigger_error( __FUNCTION__, $image_validity->get_error_message() . ' Background image URL: ' . $data['lcpElementExternalBackgroundImage']['url'] );
unset( $data['lcpElementExternalBackgroundImage'] );
}
}

return $validity;
return $data;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion plugins/image-prioritizer/hooks.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
add_action( 'od_init', 'image_prioritizer_init' );
add_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' );
add_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' );
add_filter( 'od_url_metric_storage_validity', 'image_prioritizer_filter_store_url_metric_validity', 10, 2 );
add_filter( 'od_url_metric_data_pre_storage', 'image_prioritizer_filter_url_metric_data_pre_storage' );
118 changes: 41 additions & 77 deletions plugins/image-prioritizer/tests/test-helper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -747,78 +747,34 @@ public function test_image_prioritizer_validate_background_image_url( Closure $s
*
* @return array<string, mixed>
*/
public function data_provider_to_test_image_prioritizer_filter_store_url_metric_validity(): array {
public function data_provider_to_test_image_prioritizer_filter_url_metric_data_pre_storage(): array {
return array(
'pass_through_true' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( true, $url_metric );
},
'assert' => function ( $value ): void {
$this->assertTrue( $value );
},
),

'pass_through_false' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( false, $url_metric );
},
'assert' => function ( $value ): void {
$this->assertFalse( $value );
},
),

'pass_through_truthy_string' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( 'so true', $url_metric );
},
'assert' => function ( $value ): void {
$this->assertTrue( $value );
},
),

'pass_through_falsy_string' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( '', $url_metric );
},
'assert' => function ( $value ): void {
$this->assertFalse( $value );
},
),

'pass_through_wp_error' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( new WP_Error( 'bad', 'Evil' ), $url_metric );
},
'assert' => function ( $value ): void {
$this->assertInstanceOf( WP_Error::class, $value );
$this->assertSame( 'bad', $value->get_error_code() );
},
),

'invalid_external_bg_image' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
$sample_url_metric_data['lcpElementExternalBackgroundImage'] = array(
'invalid_external_bg_image' => array(
'set_up' => static function ( array $url_metric_data ): array {
$url_metric_data['lcpElementExternalBackgroundImage'] = array(
'url' => 'https://bad-origin.example.com/image.jpg',
'tag' => 'DIV',
'id' => null,
'class' => null,
);
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( true, $url_metric );
$url_metric_data['viewport']['width'] = 10101;
$url_metric_data['viewport']['height'] = 20202;
return $url_metric_data;
},
'assert' => function ( $value, OD_Strict_URL_Metric $url_metric ): void {
$this->assertTrue( $value );
$this->assertNull( $url_metric->get( 'lcpElementExternalBackgroundImage' ) );
'assert' => function ( array $url_metric_data ): void {
$this->assertArrayNotHasKey( 'lcpElementExternalBackgroundImage', $url_metric_data );
$this->assertSame(
array(
'width' => 10101,
'height' => 20202,
),
$url_metric_data['viewport']
);
},
),

'valid_external_bg_image' => array(
'set_up' => static function ( array $sample_url_metric_data ): array {
'valid_external_bg_image' => array(
'set_up' => static function ( array $url_metric_data ): array {
$image_url = home_url( '/good.jpg' );

add_filter(
Expand All @@ -843,46 +799,54 @@ static function ( $pre, $parsed_args, $url ) use ( $image_url ) {
3
);

$sample_url_metric_data['lcpElementExternalBackgroundImage'] = array(
$url_metric_data['lcpElementExternalBackgroundImage'] = array(
'url' => $image_url,
'tag' => 'DIV',
'id' => null,
'class' => null,
);
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
return array( true, $url_metric );

$url_metric_data['viewport']['width'] = 30303;
$url_metric_data['viewport']['height'] = 40404;
return $url_metric_data;
},
'assert' => function ( $value, OD_Strict_URL_Metric $url_metric ): void {
$this->assertTrue( $value );
$this->assertIsArray( $url_metric->get( 'lcpElementExternalBackgroundImage' ) );
'assert' => function ( array $url_metric_data ): void {
$this->assertArrayHasKey( 'lcpElementExternalBackgroundImage', $url_metric_data );
$this->assertIsArray( $url_metric_data['lcpElementExternalBackgroundImage'] );
$this->assertSame(
array(
'url' => home_url( '/good.jpg' ),
'tag' => 'DIV',
'id' => null,
'class' => null,
),
$url_metric->get( 'lcpElementExternalBackgroundImage' )
$url_metric_data['lcpElementExternalBackgroundImage']
);
$this->assertSame(
array(
'width' => 30303,
'height' => 40404,
),
$url_metric_data['viewport']
);
},
),
);
}

/**
* Tests image_prioritizer_filter_store_url_metric_validity().
* Tests image_prioritizer_filter_url_metric_data_pre_storage().
*
* @dataProvider data_provider_to_test_image_prioritizer_filter_store_url_metric_validity
* @dataProvider data_provider_to_test_image_prioritizer_filter_url_metric_data_pre_storage
*
* @covers ::image_prioritizer_filter_store_url_metric_validity
* @covers ::image_prioritizer_filter_url_metric_data_pre_storage
* @covers ::image_prioritizer_validate_background_image_url
*/
public function test_image_prioritizer_filter_store_url_metric_validity( Closure $set_up, Closure $assert ): void {
$sample_url_metric_data = $this->get_sample_url_metric( array() )->jsonSerialize();
list( $validity, $url_metric ) = $set_up( $sample_url_metric_data );
public function test_image_prioritizer_filter_url_metric_data_pre_storage( Closure $set_up, Closure $assert ): void {
$url_metric_data = $set_up( $this->get_sample_url_metric( array() )->jsonSerialize() );

$validity = image_prioritizer_filter_store_url_metric_validity( $validity, $url_metric );
$assert( $validity, $url_metric );
$url_metric_data = image_prioritizer_filter_url_metric_data_pre_storage( $url_metric_data );
$assert( $url_metric_data );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion plugins/image-prioritizer/tests/test-hooks.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ public function test_hooks_added(): void {
$this->assertEquals( 10, has_action( 'od_init', 'image_prioritizer_init' ) );
$this->assertEquals( 10, has_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' ) );
$this->assertEquals( 10, has_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' ) );
$this->assertEquals( 10, has_filter( 'od_url_metric_storage_validity', 'image_prioritizer_filter_store_url_metric_validity' ) );
$this->assertEquals( 10, has_filter( 'od_url_metric_data_pre_storage', 'image_prioritizer_filter_url_metric_data_pre_storage' ) );
}
}
45 changes: 5 additions & 40 deletions plugins/optimization-detective/class-od-element.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,53 +208,18 @@ public function offsetSet( $offset, $value ): void {
}

/**
* Unsets a property.
* Offset to unset.
*
* This is particularly useful in conjunction with the `od_url_metric_schema_element_item_additional_properties` filter.
* This will throw an exception if the property is required by the schema.
*
* @since n.e.x.t
*
* @param string $key Property.
* This is disallowed. Attempting to unset a property will throw an error.
*
* @throws OD_Data_Validation_Exception When attempting to unset a property required by the JSON Schema.
*/
public function unset( string $key ): void {
$schema = OD_URL_Metric::get_json_schema(); // TODO: This should be a non-static method once the URL Metric is instantiated.
if (
isset( $schema['properties']['elements']['items']['properties'][ $key ]['required'] )
&& true === $schema['properties']['elements']['items']['properties'][ $key ]['required']
) {
throw new OD_Data_Validation_Exception(
esc_html(
sprintf(
/* translators: %s is the property key. */
__( 'The %s key is required for an item of elements in a URL Metric.', 'optimization-detective' ),
$key
)
)
);
}
unset( $this->data[ $key ] ); // @phpstan-ignore assign.propertyType (Above required check ensures $key is not isLCP, isLCPCandidate, xpath, intersectionRatio, intersectionRect, boundingClientRect.)
$group = $this->url_metric->get_group();
if ( $group instanceof OD_URL_Metric_Group ) {
$group->clear_cache();
}
}

/**
* Unsets an offset.
*
* This will throw an exception if the offset is required by the schema.
*
* @since n.e.x.t
* @since 0.7.0
*
* @param mixed $offset Offset.
*
* @throws OD_Data_Validation_Exception When attempting to unset a property required by the JSON Schema.
* @throws Exception When attempting to unset a property.
*/
public function offsetUnset( $offset ): void {
$this->unset( (string) $offset );
throw new Exception( 'Element data may not be unset.' );
}

/**
Expand Down
27 changes: 0 additions & 27 deletions plugins/optimization-detective/class-od-url-metric.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,33 +503,6 @@ function ( array $element ): OD_Element {
return $this->elements;
}

/**
* Unsets a property from the URL Metric.
*
* @since n.e.x.t
*
* @param string $key Key to unset.
* @throws OD_Data_Validation_Exception If the property is required an exception will be thrown.
*/
public function unset( string $key ): void {
$schema = self::get_json_schema(); // TODO: Consider capturing the schema as a private member variable once the URL Metric is constructed.
if ( isset( $schema['properties'][ $key ]['required'] ) && true === $schema['properties'][ $key ]['required'] ) {
throw new OD_Data_Validation_Exception(
esc_html(
sprintf(
/* translators: %s is the property key. */
__( 'The %s key is required at the root of a URL Metric.', 'optimization-detective' ),
$key
)
)
);
}
unset( $this->data[ $key ] ); // @phpstan-ignore assign.propertyType (Above required check ensures $key is not uuid, url, timestamp, viewport, or elements.)
if ( $this->group instanceof OD_URL_Metric_Group ) {
$this->group->clear_cache();
}
}

/**
* Specifies data which should be serialized to JSON.
*
Expand Down
Loading

0 comments on commit 94c5e6b

Please sign in to comment.