Skip to content

Commit e6cfcfd

Browse files
westonruterwestonruter
committed
Replace validity filter with sanitization filter; unset unset()
1 parent 6005f4a commit e6cfcfd

File tree

11 files changed

+141
-433
lines changed

11 files changed

+141
-433
lines changed

‎plugins/image-prioritizer/helper.php‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -270,33 +270,30 @@ static function ( $host ) {
270270
* @since n.e.x.t
271271
* @access private
272272
*
273-
* @param bool|WP_Error|mixed $validity Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
274-
* @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema.
275-
* @return bool|WP_Error Validity. Valid if true or a WP_Error without any errors, or invalid otherwise.
273+
* @param array<string, mixed>|mixed $data URL Metric data.
274+
* @return array<string, mixed> Sanitized URL Metric data.
276275
*
277276
* @noinspection PhpDocMissingThrowsInspection
278-
* @throws OD_Data_Validation_Exception Except it won't because lcpElementExternalBackgroundImage is not a required property.
279277
*/
280-
function image_prioritizer_filter_store_url_metric_validity( $validity, OD_Strict_URL_Metric $url_metric ) {
281-
if ( ! is_bool( $validity ) && ! ( $validity instanceof WP_Error ) ) {
282-
$validity = (bool) $validity;
278+
function image_prioritizer_filter_url_metric_data_pre_storage( $data ): array {
279+
if ( ! is_array( $data ) ) {
280+
$data = array();
283281
}
284282

285-
$data = $url_metric->get( 'lcpElementExternalBackgroundImage' );
286-
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.
287-
$image_validity = image_prioritizer_validate_background_image_url( $data['url'] );
283+
if ( isset( $data['lcpElementExternalBackgroundImage']['url'] ) && is_string( $data['lcpElementExternalBackgroundImage']['url'] ) ) {
284+
$image_validity = image_prioritizer_validate_background_image_url( $data['lcpElementExternalBackgroundImage']['url'] );
288285
if ( is_wp_error( $image_validity ) ) {
289286
/**
290287
* No WP_Exception is thrown by wp_trigger_error() since E_USER_ERROR is not passed as the error level.
291288
*
292289
* @noinspection PhpUnhandledExceptionInspection
293290
*/
294-
wp_trigger_error( __FUNCTION__, $image_validity->get_error_message() . ' Background image URL: ' . $data['url'] );
295-
$url_metric->unset( 'lcpElementExternalBackgroundImage' );
291+
wp_trigger_error( __FUNCTION__, $image_validity->get_error_message() . ' Background image URL: ' . $data['lcpElementExternalBackgroundImage']['url'] );
292+
unset( $data['lcpElementExternalBackgroundImage'] );
296293
}
297294
}
298295

299-
return $validity;
296+
return $data;
300297
}
301298

302299
/**

‎plugins/image-prioritizer/hooks.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@
1313
add_action( 'od_init', 'image_prioritizer_init' );
1414
add_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' );
1515
add_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' );
16-
add_filter( 'od_url_metric_storage_validity', 'image_prioritizer_filter_store_url_metric_validity', 10, 2 );
16+
add_filter( 'od_url_metric_data_pre_storage', 'image_prioritizer_filter_url_metric_data_pre_storage' );

‎plugins/image-prioritizer/tests/test-helper.php‎

Lines changed: 41 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -747,78 +747,34 @@ public function test_image_prioritizer_validate_background_image_url( Closure $s
747747
*
748748
* @return array<string, mixed>
749749
*/
750-
public function data_provider_to_test_image_prioritizer_filter_store_url_metric_validity(): array {
750+
public function data_provider_to_test_image_prioritizer_filter_url_metric_data_pre_storage(): array {
751751
return array(
752-
'pass_through_true' => array(
753-
'set_up' => static function ( array $sample_url_metric_data ): array {
754-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
755-
return array( true, $url_metric );
756-
},
757-
'assert' => function ( $value ): void {
758-
$this->assertTrue( $value );
759-
},
760-
),
761-
762-
'pass_through_false' => array(
763-
'set_up' => static function ( array $sample_url_metric_data ): array {
764-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
765-
return array( false, $url_metric );
766-
},
767-
'assert' => function ( $value ): void {
768-
$this->assertFalse( $value );
769-
},
770-
),
771-
772-
'pass_through_truthy_string' => array(
773-
'set_up' => static function ( array $sample_url_metric_data ): array {
774-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
775-
return array( 'so true', $url_metric );
776-
},
777-
'assert' => function ( $value ): void {
778-
$this->assertTrue( $value );
779-
},
780-
),
781-
782-
'pass_through_falsy_string' => array(
783-
'set_up' => static function ( array $sample_url_metric_data ): array {
784-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
785-
return array( '', $url_metric );
786-
},
787-
'assert' => function ( $value ): void {
788-
$this->assertFalse( $value );
789-
},
790-
),
791-
792-
'pass_through_wp_error' => array(
793-
'set_up' => static function ( array $sample_url_metric_data ): array {
794-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
795-
return array( new WP_Error( 'bad', 'Evil' ), $url_metric );
796-
},
797-
'assert' => function ( $value ): void {
798-
$this->assertInstanceOf( WP_Error::class, $value );
799-
$this->assertSame( 'bad', $value->get_error_code() );
800-
},
801-
),
802-
803-
'invalid_external_bg_image' => array(
804-
'set_up' => static function ( array $sample_url_metric_data ): array {
805-
$sample_url_metric_data['lcpElementExternalBackgroundImage'] = array(
752+
'invalid_external_bg_image' => array(
753+
'set_up' => static function ( array $url_metric_data ): array {
754+
$url_metric_data['lcpElementExternalBackgroundImage'] = array(
806755
'url' => 'https://bad-origin.example.com/image.jpg',
807756
'tag' => 'DIV',
808757
'id' => null,
809758
'class' => null,
810759
);
811-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
812-
return array( true, $url_metric );
760+
$url_metric_data['viewport']['width'] = 10101;
761+
$url_metric_data['viewport']['height'] = 20202;
762+
return $url_metric_data;
813763
},
814-
'assert' => function ( $value, OD_Strict_URL_Metric $url_metric ): void {
815-
$this->assertTrue( $value );
816-
$this->assertNull( $url_metric->get( 'lcpElementExternalBackgroundImage' ) );
764+
'assert' => function ( array $url_metric_data ): void {
765+
$this->assertArrayNotHasKey( 'lcpElementExternalBackgroundImage', $url_metric_data );
766+
$this->assertSame(
767+
array(
768+
'width' => 10101,
769+
'height' => 20202,
770+
),
771+
$url_metric_data['viewport']
772+
);
817773
},
818774
),
819775

820-
'valid_external_bg_image' => array(
821-
'set_up' => static function ( array $sample_url_metric_data ): array {
776+
'valid_external_bg_image' => array(
777+
'set_up' => static function ( array $url_metric_data ): array {
822778
$image_url = home_url( '/good.jpg' );
823779

824780
add_filter(
@@ -843,46 +799,54 @@ static function ( $pre, $parsed_args, $url ) use ( $image_url ) {
843799
3
844800
);
845801

846-
$sample_url_metric_data['lcpElementExternalBackgroundImage'] = array(
802+
$url_metric_data['lcpElementExternalBackgroundImage'] = array(
847803
'url' => $image_url,
848804
'tag' => 'DIV',
849805
'id' => null,
850806
'class' => null,
851807
);
852-
$url_metric = new OD_Strict_URL_Metric( $sample_url_metric_data );
853-
return array( true, $url_metric );
808+
809+
$url_metric_data['viewport']['width'] = 30303;
810+
$url_metric_data['viewport']['height'] = 40404;
811+
return $url_metric_data;
854812
},
855-
'assert' => function ( $value, OD_Strict_URL_Metric $url_metric ): void {
856-
$this->assertTrue( $value );
857-
$this->assertIsArray( $url_metric->get( 'lcpElementExternalBackgroundImage' ) );
813+
'assert' => function ( array $url_metric_data ): void {
814+
$this->assertArrayHasKey( 'lcpElementExternalBackgroundImage', $url_metric_data );
815+
$this->assertIsArray( $url_metric_data['lcpElementExternalBackgroundImage'] );
858816
$this->assertSame(
859817
array(
860818
'url' => home_url( '/good.jpg' ),
861819
'tag' => 'DIV',
862820
'id' => null,
863821
'class' => null,
864822
),
865-
$url_metric->get( 'lcpElementExternalBackgroundImage' )
823+
$url_metric_data['lcpElementExternalBackgroundImage']
824+
);
825+
$this->assertSame(
826+
array(
827+
'width' => 30303,
828+
'height' => 40404,
829+
),
830+
$url_metric_data['viewport']
866831
);
867832
},
868833
),
869834
);
870835
}
871836

872837
/**
873-
* Tests image_prioritizer_filter_store_url_metric_validity().
838+
* Tests image_prioritizer_filter_url_metric_data_pre_storage().
874839
*
875-
* @dataProvider data_provider_to_test_image_prioritizer_filter_store_url_metric_validity
840+
* @dataProvider data_provider_to_test_image_prioritizer_filter_url_metric_data_pre_storage
876841
*
877-
* @covers ::image_prioritizer_filter_store_url_metric_validity
842+
* @covers ::image_prioritizer_filter_url_metric_data_pre_storage
878843
* @covers ::image_prioritizer_validate_background_image_url
879844
*/
880-
public function test_image_prioritizer_filter_store_url_metric_validity( Closure $set_up, Closure $assert ): void {
881-
$sample_url_metric_data = $this->get_sample_url_metric( array() )->jsonSerialize();
882-
list( $validity, $url_metric ) = $set_up( $sample_url_metric_data );
845+
public function test_image_prioritizer_filter_url_metric_data_pre_storage( Closure $set_up, Closure $assert ): void {
846+
$url_metric_data = $set_up( $this->get_sample_url_metric( array() )->jsonSerialize() );
883847

884-
$validity = image_prioritizer_filter_store_url_metric_validity( $validity, $url_metric );
885-
$assert( $validity, $url_metric );
848+
$url_metric_data = image_prioritizer_filter_url_metric_data_pre_storage( $url_metric_data );
849+
$assert( $url_metric_data );
886850
}
887851

888852
/**

‎plugins/image-prioritizer/tests/test-hooks.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,6 @@ public function test_hooks_added(): void {
1414
$this->assertEquals( 10, has_action( 'od_init', 'image_prioritizer_init' ) );
1515
$this->assertEquals( 10, has_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' ) );
1616
$this->assertEquals( 10, has_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' ) );
17-
$this->assertEquals( 10, has_filter( 'od_url_metric_storage_validity', 'image_prioritizer_filter_store_url_metric_validity' ) );
17+
$this->assertEquals( 10, has_filter( 'od_url_metric_data_pre_storage', 'image_prioritizer_filter_url_metric_data_pre_storage' ) );
1818
}
1919
}

‎plugins/optimization-detective/class-od-element.php‎

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -208,53 +208,18 @@ public function offsetSet( $offset, $value ): void {
208208
}
209209

210210
/**
211-
* Unsets a property.
211+
* Offset to unset.
212212
*
213-
* This is particularly useful in conjunction with the `od_url_metric_schema_element_item_additional_properties` filter.
214-
* This will throw an exception if the property is required by the schema.
215-
*
216-
* @since n.e.x.t
217-
*
218-
* @param string $key Property.
213+
* This is disallowed. Attempting to unset a property will throw an error.
219214
*
220-
* @throws OD_Data_Validation_Exception When attempting to unset a property required by the JSON Schema.
221-
*/
222-
public function unset( string $key ): void {
223-
$schema = OD_URL_Metric::get_json_schema(); // TODO: This should be a non-static method once the URL Metric is instantiated.
224-
if (
225-
isset( $schema['properties']['elements']['items']['properties'][ $key ]['required'] )
226-
&& true === $schema['properties']['elements']['items']['properties'][ $key ]['required']
227-
) {
228-
throw new OD_Data_Validation_Exception(
229-
esc_html(
230-
sprintf(
231-
/* translators: %s is the property key. */
232-
__( 'The %s key is required for an item of elements in a URL Metric.', 'optimization-detective' ),
233-
$key
234-
)
235-
)
236-
);
237-
}
238-
unset( $this->data[ $key ] ); // @phpstan-ignore assign.propertyType (Above required check ensures $key is not isLCP, isLCPCandidate, xpath, intersectionRatio, intersectionRect, boundingClientRect.)
239-
$group = $this->url_metric->get_group();
240-
if ( $group instanceof OD_URL_Metric_Group ) {
241-
$group->clear_cache();
242-
}
243-
}
244-
245-
/**
246-
* Unsets an offset.
247-
*
248-
* This will throw an exception if the offset is required by the schema.
249-
*
250-
* @since n.e.x.t
215+
* @since 0.7.0
251216
*
252217
* @param mixed $offset Offset.
253218
*
254-
* @throws OD_Data_Validation_Exception When attempting to unset a property required by the JSON Schema.
219+
* @throws Exception When attempting to unset a property.
255220
*/
256221
public function offsetUnset( $offset ): void {
257-
$this->unset( (string) $offset );
222+
throw new Exception( 'Element data may not be unset.' );
258223
}
259224

260225
/**

‎plugins/optimization-detective/class-od-url-metric.php‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -503,33 +503,6 @@ function ( array $element ): OD_Element {
503503
return $this->elements;
504504
}
505505

506-
/**
507-
* Unsets a property from the URL Metric.
508-
*
509-
* @since n.e.x.t
510-
*
511-
* @param string $key Key to unset.
512-
* @throws OD_Data_Validation_Exception If the property is required an exception will be thrown.
513-
*/
514-
public function unset( string $key ): void {
515-
$schema = self::get_json_schema(); // TODO: Consider capturing the schema as a private member variable once the URL Metric is constructed.
516-
if ( isset( $schema['properties'][ $key ]['required'] ) && true === $schema['properties'][ $key ]['required'] ) {
517-
throw new OD_Data_Validation_Exception(
518-
esc_html(
519-
sprintf(
520-
/* translators: %s is the property key. */
521-
__( 'The %s key is required at the root of a URL Metric.', 'optimization-detective' ),
522-
$key
523-
)
524-
)
525-
);
526-
}
527-
unset( $this->data[ $key ] ); // @phpstan-ignore assign.propertyType (Above required check ensures $key is not uuid, url, timestamp, viewport, or elements.)
528-
if ( $this->group instanceof OD_URL_Metric_Group ) {
529-
$this->group->clear_cache();
530-
}
531-
}
532-
533506
/**
534507
* Specifies data which should be serialized to JSON.
535508
*

0 commit comments

Comments
 (0)