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/readme.txt b/plugins/optimization-detective/readme.txt index f0a8cc147..d78c4864d 100644 --- a/plugins/optimization-detective/readme.txt +++ b/plugins/optimization-detective/readme.txt @@ -246,19 +246,13 @@ The ETag is a unique identifier that changes whenever the underlying data used t When the ETag for URL Metrics in a complete viewport group no longer matches the current environment's ETag, new URL Metrics will then begin to be collected until there are no more stored URL Metrics with the old ETag. These new URL Metrics will include data relevant to the newly activated plugins and their tag visitors. -**Filter:** `od_url_metric_storage_validity` (default args: true) +**Filter:** `od_url_metric_data_pre_storage` (default arg: URL Metric data array) -Filters whether a URL Metric is valid for storage. +Filters the URL Metric data prior to validation for storage. -Three parameters are passed to this filter: +This allows for custom sanitization and validation constraints to be applied beyond what can be expressed in JSON Schema. 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 sanitization and validation logic enforced via this filter can be more expensive, such as doing filesystem checks or HTTP requests. -1. `$validity` (`bool|WP_Error`): Validity. Invalid if false or a WP_Error with errors. -2. `$url_metric` (`OD_Strict_URL_Metric`): URL Metric, already validated against the JSON Schema. -3. `$url_metric_data` (`array`): Original URL Metric data before any mutations. - -This allows for custom validation constraints to be applied beyond what can be expressed in JSON Schema. 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. - -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. **Action:** `od_url_metric_stored` (argument: `OD_URL_Metric_Store_Request_Context`) 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..52a4cc43f 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 */ @@ -131,68 +130,22 @@ static function ( array $schema ): array { $this->assertTrue( isset( $element['customProp'] ) ); $this->assertTrue( $element->offsetExists( 'customProp' ) ); - // Try setting property (which is not currently supported). - $functions = array( - static function ( OD_Element $element ): void { - $element['isLCP'] = true; - }, - static function ( OD_Element $element ): void { - $element->offsetSet( 'isLCP', true ); - }, - ); - foreach ( $functions as $function ) { - $exception = null; - try { - $function( $element ); - } catch ( Exception $e ) { - $exception = $e; - } - $this->assertInstanceOf( Exception::class, $exception ); - $this->assertFalse( $element->get( 'isLCP' ) ); - } + $this->assertEquals( $element_data, $element->jsonSerialize() ); - // 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() ); + $exception = null; + try { + $element['isLCP'] = true; + } catch ( Exception $e ) { + $exception = $e; } + $this->assertInstanceOf( Exception::class, $exception ); - $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() ); + $exception = null; + try { + unset( $element['isLCP'] ); + } catch ( Exception $e ) { // @phpstan-ignore catch.neverThrown (It is thrown by offsetUnset actually.) + $exception = $e; } + $this->assertInstanceOf( Exception::class, $exception ); // @phpstan-ignore method.impossibleType (It is thrown by offsetUnset actually.) } } 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 ) ); } /**