diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index e80b8f41c..9b1c72c15 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -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|mixed $data URL Metric data. + * @return array 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; } /** diff --git a/plugins/image-prioritizer/hooks.php b/plugins/image-prioritizer/hooks.php index fbfe8c6b3..63420cd6d 100644 --- a/plugins/image-prioritizer/hooks.php +++ b/plugins/image-prioritizer/hooks.php @@ -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' ); diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index 45e91cccd..05de49635 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/plugins/image-prioritizer/tests/test-helper.php @@ -747,78 +747,34 @@ public function test_image_prioritizer_validate_background_image_url( Closure $s * * @return array */ - 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( @@ -843,18 +799,20 @@ 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' ), @@ -862,7 +820,14 @@ static function ( $pre, $parsed_args, $url ) use ( $image_url ) { 'id' => null, 'class' => null, ), - $url_metric->get( 'lcpElementExternalBackgroundImage' ) + $url_metric_data['lcpElementExternalBackgroundImage'] + ); + $this->assertSame( + array( + 'width' => 30303, + 'height' => 40404, + ), + $url_metric_data['viewport'] ); }, ), @@ -870,19 +835,18 @@ static function ( $pre, $parsed_args, $url ) use ( $image_url ) { } /** - * 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 ); } /** diff --git a/plugins/image-prioritizer/tests/test-hooks.php b/plugins/image-prioritizer/tests/test-hooks.php index 3c2e02014..416523cb1 100644 --- a/plugins/image-prioritizer/tests/test-hooks.php +++ b/plugins/image-prioritizer/tests/test-hooks.php @@ -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' ) ); } } diff --git a/plugins/optimization-detective/class-od-element.php b/plugins/optimization-detective/class-od-element.php index be8400ce4..c7f243d99 100644 --- a/plugins/optimization-detective/class-od-element.php +++ b/plugins/optimization-detective/class-od-element.php @@ -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.' ); } /** diff --git a/plugins/optimization-detective/class-od-url-metric.php b/plugins/optimization-detective/class-od-url-metric.php index b68045e6e..f4a4e25cc 100644 --- a/plugins/optimization-detective/class-od-url-metric.php +++ b/plugins/optimization-detective/class-od-url-metric.php @@ -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. * diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 198b57284..940928769 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -186,85 +186,64 @@ function od_handle_rest_request( WP_REST_Request $request ) { OD_Storage_Lock::set_lock(); try { - // The "strict" URL Metric class is being used here to ensure additionalProperties of all objects are disallowed. - $url_metric = new OD_Strict_URL_Metric( - array_merge( - $data, - array( - // Now supply the readonly args which were omitted from the REST API params due to being `readonly`. - 'timestamp' => microtime( true ), - 'uuid' => wp_generate_uuid4(), - 'etag' => $request->get_param( 'current_etag' ), - ) + $data = array_merge( + $data, + array( + // Now supply the readonly args which were omitted from the REST API params due to being `readonly`. + 'timestamp' => microtime( true ), + 'uuid' => wp_generate_uuid4(), + 'etag' => $request->get_param( 'current_etag' ), ) ); - } catch ( OD_Data_Validation_Exception $e ) { - return new WP_Error( - 'rest_invalid_param', - sprintf( - /* translators: %s is exception name */ - __( 'Failed to validate URL Metric: %s', 'optimization-detective' ), - $e->getMessage() - ), - array( 'status' => 400 ) - ); - } - try { /** - * Filters whether a URL Metric is valid for storage. + * Filters the URL Metric data prior to validation for storage. * - * This allows for custom validation constraints to be applied beyond what can be expressed in JSON Schema. This is - * also necessary because the 'validate_callback' key in a JSON Schema is not respected when gathering the REST API - * endpoint args via the {@see rest_get_endpoint_args_for_schema()} function. Besides this, the REST API doesn't - * support 'validate_callback' for any nested arguments in any case, meaning that custom constraints would be able - * to be applied to multidimensional objects, such as the items inside 'elements'. + * This allows for custom sanitization and validation constraints to be applied beyond what can be expressed in + * JSON Schema. This is also necessary because the 'validate_callback' key in a JSON Schema is not respected when + * gathering the REST API endpoint args via the {@see rest_get_endpoint_args_for_schema()} function. Besides this, + * the REST API doesn't support 'validate_callback' for any nested arguments in any case, meaning that custom + * constraints would be able to be applied to multidimensional objects, such as the items inside 'elements'. * * This filter only applies when storing a URL Metric via the REST API. It does not run when a stored URL Metric is - * loaded from the od_url_metrics post type. This means that validation logic enforced via this filter can be more - * expensive, such as doing filesystem checks or HTTP requests. + * loaded from the od_url_metrics post type. This means that sanitization and validation logic enforced via this + * filter can be more expensive, such as doing filesystem checks or HTTP requests. * - * In addition to having the filter return `false` or a non-empty `WP_Error` to block storing the URL Metric, a - * plugin may also mutate the OD_URL_Metric instance passed by reference to the filter callback. This is useful - * for plugins in particular to unset extended properties which couldn't be validated using JSON Schema alone. + * To fail validation for the provided URL Metric data, an OD_Data_Validation_Exception exception should be thrown. + * Otherwise, any filter callbacks must return an array consisting of the sanitized URL Metric data. * * @since n.e.x.t * - * @param bool|WP_Error $validity Validity. Invalid if false or a WP_Error with errors. - * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema. - * @param array $url_metric_data Original URL Metric data before any mutations. + * @param array $data URL Metric data. This is the Data type from OD_URL_Metric. */ - $validity = apply_filters( 'od_url_metric_storage_validity', true, $url_metric, $url_metric->jsonSerialize() ); + $data = apply_filters( 'od_url_metric_data_pre_storage', $data ); + + // The "strict" URL Metric class is being used here to ensure additionalProperties of all objects are disallowed. + $url_metric = new OD_Strict_URL_Metric( $data ); } catch ( Exception $e ) { - $error_data = array( - 'status' => 500, - ); - if ( WP_DEBUG ) { - $error_data['exception'] = array( - 'class' => get_class( $e ), - 'message' => $e->getMessage(), - 'code' => $e->getCode(), + $error_data = array(); + if ( $e instanceof OD_Data_Validation_Exception ) { + $error_message = sprintf( + /* translators: %s is exception name */ + __( 'Failed to validate URL Metric: %s', 'optimization-detective' ), + $e->getMessage() ); + $error_data['status'] = 400; + } else { + $error_message = sprintf( + /* translators: %s is exception name */ + __( 'Failed to validate URL Metric: %s', 'optimization-detective' ), + __( 'An unrecognized exception was thrown', 'optimization-detective' ) + ); + $error_data['status'] = 500; + if ( WP_DEBUG ) { + $error_data['exception_class'] = get_class( $e ); + $error_data['exception_message'] = $e->getMessage(); + $error_data['exception_code'] = $e->getCode(); + } } - $validity = new WP_Error( - 'exception', - sprintf( - /* translators: %s is the filter name 'od_url_metric_storage_validity' */ - __( 'An %s filter callback threw an exception.', 'optimization-detective' ), - 'od_url_metric_storage_validity' - ), - $error_data - ); - } - if ( false === (bool) $validity || ( $validity instanceof WP_Error && $validity->has_errors() ) ) { - if ( false === $validity ) { - $validity = new WP_Error( 'invalid_url_metric', __( 'Validity of URL Metric was rejected by filter.', 'optimization-detective' ) ); - } - $error_code = $validity->get_error_code(); - if ( ! isset( $validity->error_data[ $error_code ]['status'] ) ) { - $validity->error_data[ $error_code ]['status'] = 400; - } - return $validity; + + return new WP_Error( 'rest_invalid_param', $error_message, $error_data ); } // TODO: This should be changed from store_url_metric($slug, $url_metric) instead be update_post( $slug, $group_collection ). As it stands, store_url_metric() is duplicating logic here. @@ -277,8 +256,8 @@ function od_handle_rest_request( WP_REST_Request $request ) { 'status' => 500, ); if ( WP_DEBUG ) { - $error_data['code'] = $result->get_error_code(); - $error_data['message'] = $result->get_error_message(); + $error_data['error_code'] = $result->get_error_code(); + $error_data['error_message'] = $result->get_error_message(); } return new WP_Error( 'unable_to_store_url_metric', diff --git a/plugins/optimization-detective/tests/storage/test-rest-api.php b/plugins/optimization-detective/tests/storage/test-rest-api.php index 0caf8b5a3..01684eb3a 100644 --- a/plugins/optimization-detective/tests/storage/test-rest-api.php +++ b/plugins/optimization-detective/tests/storage/test-rest-api.php @@ -41,14 +41,14 @@ static function ( array $properties ) use ( $property_name ): array { }; return array( - 'not_extended' => array( + 'not_extended' => array( 'set_up' => function (): array { return $this->get_valid_params(); }, 'expect_stored' => true, 'expected_status' => 200, ), - 'extended' => array( + 'extended' => array( 'set_up' => function () use ( $add_root_extra_property ): array { $add_root_extra_property( 'extra' ); $params = $this->get_valid_params(); @@ -58,84 +58,39 @@ static function ( array $properties ) use ( $property_name ): array { 'expect_stored' => true, 'expected_status' => 200, ), - 'extended_but_unset' => array( - 'set_up' => function () use ( $add_root_extra_property ): array { - $add_root_extra_property( 'unset_prop' ); - add_filter( - 'od_url_metric_storage_validity', - static function ( $validity, OD_URL_Metric $url_metric ) { - $url_metric->unset( 'extra' ); - return $validity; - }, - 10, - 2 - ); - $params = $this->get_valid_params(); - $params['unset_prop'] = 'bar'; - return $params; - }, - 'expect_stored' => true, - 'expected_status' => 200, - ), - 'extended_and_rejected_by_returning_false' => array( - 'set_up' => function () use ( $add_root_extra_property ): array { - $add_root_extra_property( 'extra' ); - add_filter( 'od_url_metric_storage_validity', '__return_false' ); - - $params = $this->get_valid_params(); - $params['extra'] = 'foo'; - return $params; - }, - 'expect_stored' => false, - 'expected_status' => 400, - ), - 'extended_and_rejected_by_returning_wp_error' => array( - 'set_up' => function () use ( $add_root_extra_property ): array { - $add_root_extra_property( 'extra' ); - add_filter( - 'od_url_metric_storage_validity', - static function () { - return new WP_Error( 'rejected', 'Rejected!' ); - } - ); - - $params = $this->get_valid_params(); - $params['extra'] = 'foo'; - return $params; - }, - 'expect_stored' => false, - 'expected_status' => 400, - ), - 'rejected_by_returning_wp_error_with_forbidden_status' => array( + 'rejected_by_generic_exception' => array( 'set_up' => function (): array { add_filter( - 'od_url_metric_storage_validity', - static function () { - return new WP_Error( 'forbidden', 'Forbidden!', array( 'status' => 403 ) ); + 'od_url_metric_data_pre_storage', + static function ( $data ): array { + if ( count( $data ) > 0 ) { + throw new Exception( 'bad' ); + } + return $data; } ); return $this->get_valid_params(); }, 'expect_stored' => false, - 'expected_status' => 403, + 'expected_status' => 500, ), - 'rejected_by_exception' => array( + 'rejected_by_validation_exception' => array( 'set_up' => function (): array { add_filter( - 'od_url_metric_storage_validity', - static function ( $validity, OD_URL_Metric $url_metric ) { - $url_metric->unset( 'viewport' ); - return $validity; - }, - 10, - 2 + 'od_url_metric_data_pre_storage', + static function ( $data ): array { + if ( count( $data ) > 0 ) { + throw new OD_Data_Validation_Exception( 'bad' ); + } + return $data; + } ); return $this->get_valid_params(); }, 'expect_stored' => false, - 'expected_status' => 500, + 'expected_status' => 400, ), - 'with_cache_purge_post_id' => array( + 'with_cache_purge_post_id' => array( 'set_up' => function (): array { $params = $this->get_valid_params(); $params['cache_purge_post_id'] = self::factory()->post->create(); @@ -160,22 +115,17 @@ static function ( $validity, OD_URL_Metric $url_metric ) { * @covers ::od_trigger_page_cache_invalidation */ public function test_rest_request_good_params( Closure $set_up, bool $expect_stored, int $expected_status ): void { - $filtered_url_metric_obj = null; $filtered_url_metric_data = null; $filter_called = 0; add_filter( - 'od_url_metric_storage_validity', - function ( $validity, $url_metric, $url_metric_data ) use ( &$filter_called, &$filtered_url_metric_obj, &$filtered_url_metric_data ) { - $this->assertTrue( $validity ); - $this->assertInstanceOf( OD_URL_Metric::class, $url_metric ); + 'od_url_metric_data_pre_storage', + function ( $url_metric_data ) use ( &$filter_called, &$filtered_url_metric_data ) { $this->assertIsArray( $url_metric_data ); - $filtered_url_metric_obj = $url_metric; $filtered_url_metric_data = $url_metric_data; $filter_called++; - return $validity; + return $url_metric_data; }, - 0, - 3 + 0 ); $stored_context = null; @@ -233,7 +183,6 @@ function ( OD_URL_Metric_Store_Request_Context $context ) use ( &$stored_context ); $this->assertInstanceOf( OD_URL_Metric_Store_Request_Context::class, $stored_context ); - $this->assertSame( $stored_context->url_metric, $filtered_url_metric_obj ); $this->assertSame( $stored_context->url_metric->jsonSerialize(), $filtered_url_metric_data ); // Now check that od_trigger_page_cache_invalidation() cleaned caches as expected. diff --git a/plugins/optimization-detective/tests/test-class-od-element.php b/plugins/optimization-detective/tests/test-class-od-element.php index 1b980d421..c4650c770 100644 --- a/plugins/optimization-detective/tests/test-class-od-element.php +++ b/plugins/optimization-detective/tests/test-class-od-element.php @@ -24,7 +24,6 @@ class Test_OD_Element extends WP_UnitTestCase { * @covers ::offsetExists * @covers ::offsetGet * @covers ::offsetSet - * @covers ::unset * @covers ::offsetUnset * @covers ::jsonSerialize */ @@ -151,48 +150,6 @@ static function ( OD_Element $element ): void { $this->assertFalse( $element->get( 'isLCP' ) ); } - // Try unsetting a required property. - $functions = array( - static function ( OD_Element $element ): void { - unset( $element['isLCP'] ); - }, - static function ( OD_Element $element ): void { - $element->unset( 'isLCP' ); - }, - static function ( OD_Element $element ): void { - $element->offsetUnset( 'isLCP' ); - }, - ); - foreach ( $functions as $function ) { - $exception = null; - try { - $function( $element ); - } catch ( Exception $e ) { - $exception = $e; - } - $this->assertInstanceOf( Exception::class, $exception ); - $this->assertArrayHasKey( 'isLCP', $element->jsonSerialize() ); - } - $this->assertEquals( $element_data, $element->jsonSerialize() ); - - // Try unsetting a non-required property. - $functions = array( - static function ( OD_Element $element ): void { - unset( $element['customProp'] ); - }, - static function ( OD_Element $element ): void { - $element->unset( 'customProp' ); - }, - static function ( OD_Element $element ): void { - $element->offsetUnset( 'customProp' ); - }, - ); - foreach ( $functions as $function ) { - $cloned_element = clone $element; - $function( $cloned_element ); - $this->assertFalse( $cloned_element->offsetExists( 'customProp' ) ); - $this->assertArrayNotHasKey( 'customProp', $cloned_element->jsonSerialize() ); - } } } diff --git a/plugins/optimization-detective/tests/test-class-od-url-metric.php b/plugins/optimization-detective/tests/test-class-od-url-metric.php index df2fa0144..51b23c243 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metric.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metric.php @@ -277,7 +277,6 @@ static function ( $value ) { * @covers ::get_json_schema * @covers ::set_group * @covers ::get_group - * @covers ::unset * * @dataProvider data_provider_to_test_constructor * @@ -337,15 +336,6 @@ static function ( OD_Element $element ) { $this->assertTrue( wp_is_uuid( $url_metric->get_uuid() ) ); $this->assertSame( $url_metric->get_uuid(), $url_metric->get( 'uuid' ) ); - $exception = null; - try { - $url_metric->unset( 'elements' ); - } catch ( OD_Data_Validation_Exception $e ) { - $exception = $e; - } - $this->assertInstanceOf( OD_Data_Validation_Exception::class, $exception ); - $url_metric->unset( 'does_not_exist' ); - $serialized = $url_metric->jsonSerialize(); if ( ! array_key_exists( 'uuid', $data ) ) { $this->assertTrue( wp_is_uuid( $serialized['uuid'] ) ); @@ -407,12 +397,6 @@ static function ( array $properties ): array { $this->assertArrayHasKey( 'isTouch', $extended_data ); $this->assertTrue( $extended_data['isTouch'] ); $this->assertTrue( $extended_url_metric->get( 'isTouch' ) ); - - $this->assertTrue( $extended_url_metric->get( 'isTouch' ) ); - $extended_url_metric->unset( 'isTouch' ); - $this->assertNull( $extended_url_metric->get( 'isTouch' ) ); - $extended_data = $extended_url_metric->jsonSerialize(); - $this->assertArrayNotHasKey( 'isTouch', $extended_data ); }, ), @@ -505,13 +489,6 @@ static function ( array $properties ): array { $this->assertArrayHasKey( 'isColorful', $extended_data['elements'][0] ); $this->assertFalse( $extended_data['elements'][0]['isColorful'] ); $this->assertFalse( $extended_url_metric->get_elements()[0]['isColorful'] ); - - $element = $extended_url_metric->get_elements()[0]; - $this->assertFalse( $element->get( 'isColorful' ) ); - $element->unset( 'isColorful' ); - $this->assertNull( $element->get( 'isColorful' ) ); - $extended_data = $extended_url_metric->jsonSerialize(); - $this->assertArrayNotHasKey( 'isColorful', $extended_data['elements'][0] ); }, ), @@ -577,8 +554,6 @@ static function ( array $properties ): array { * Tests construction with extended schema. * * @covers ::get_json_schema - * @covers ::unset - * @covers OD_Element::unset * * @dataProvider data_provider_to_test_constructor_with_extended_schema * diff --git a/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 957acd1a4..ef05fc2ab 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php @@ -204,8 +204,6 @@ public function data_provider_sample_size_and_breakpoints(): array { * * @covers ::clear_cache * @covers OD_URL_Metric_Group::clear_cache - * @covers OD_URL_Metric::unset - * @covers OD_Element::unset */ public function test_clear_cache(): void { $collection = new OD_URL_Metric_Group_Collection( array(), md5( '' ), array(), 1, DAY_IN_SECONDS ); @@ -226,58 +224,13 @@ public function test_clear_cache(): void { $this->assertSame( array(), $collection_result_cache_reflection_property->getValue( $collection ) ); // Test that adding a URL metric to a collection clears the caches. - add_filter( - 'od_url_metric_schema_root_additional_properties', - static function ( $schema ) { - $schema['new_prop_at_root'] = array( - 'type' => 'string', - ); - return $schema; - } - ); - add_filter( - 'od_url_metric_schema_element_additional_properties', - static function ( $schema ) { - $schema['new_prop_at_element'] = array( - 'type' => 'string', - ); - return $schema; - } - ); $collection_result_cache_reflection_property->setValue( $collection, $populated_value ); $group_result_cache_reflection_property->setValue( $group, $populated_value ); - $collection->add_url_metric( - $this->get_sample_url_metric( - array( - 'element' => array( - 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', - 'new_prop_at_element' => 'hey there', - ), - 'extended_root' => array( - 'new_prop_at_root' => 'hola', - ), - ) - ) - ); + $collection->add_url_metric( $this->get_sample_url_metric( array() ) ); $url_metric = $group->getIterator()->current(); $this->assertInstanceOf( OD_URL_Metric::class, $url_metric ); $this->assertSame( array(), $collection_result_cache_reflection_property->getValue( $collection ) ); $this->assertSame( array(), $group_result_cache_reflection_property->getValue( $group ) ); - - // Test that modifying a URL Metric empties the cache of the collection and the group. - $collection_result_cache_reflection_property->setValue( $collection, $populated_value ); - $group_result_cache_reflection_property->setValue( $group, $populated_value ); - $url_metric->unset( 'new_prop_at_root' ); - $this->assertSame( array(), $collection_result_cache_reflection_property->getValue( $collection ) ); - $this->assertSame( array(), $group_result_cache_reflection_property->getValue( $group ) ); - - // Test that modifying a URL Metric element empties the cache of the collection and the group. - $element = $url_metric->get_elements()[0]; - $collection_result_cache_reflection_property->setValue( $collection, $populated_value ); - $group_result_cache_reflection_property->setValue( $group, $populated_value ); - $element->unset( 'new_prop_at_element' ); - $this->assertSame( array(), $collection_result_cache_reflection_property->getValue( $collection ) ); - $this->assertSame( array(), $group_result_cache_reflection_property->getValue( $group ) ); } /**