Persist unknown properties in stored URL Metrics but reject in new UR…

…L Metric storage requests

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

@@ -0,0 +1,63 @@
+<?php
+/**
+ * Optimization Detective: OD_Strict_URL_Metric class
+ *
+ * @package optimization-detective
+ * @since n.e.x.t
+ */
+
+// Exit if accessed directly.
+if ( ! defined( 'ABSPATH' ) ) {
+	exit;
+}
+
+/**
+ * Representation of the measurements taken from a single client's visit to a specific URL without additionalProperties allowed.
+ *
+ * This is used exclusively in the REST API endpoint for capturing new URL metrics to prevent invalid additional data from being
+ * submitted in the request. For URL metrics which have been stored the looser OD_URL_Metric class is used instead.
+ *
+ * @since n.e.x.t
+ * @access private
+ */
+final class OD_Strict_URL_Metric extends OD_URL_Metric {
+
+	/**
+	 * Gets JSON schema for URL Metric without additionalProperties.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @return array<string, mixed> Schema.
+	 */
+	public static function get_json_schema(): array {
+		return self::falsify_additional_properties( parent::get_json_schema() );
+	}
+
+	/**
+	 * Recursively processes the schema to ensure that all objects have additionalProperties set to false.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @param mixed $schema Schema.
+	 * @return mixed Processed schema.
+	 */
+	private static function falsify_additional_properties( $schema ) {
+		if ( ! isset( $schema['type'] ) ) {
+			return $schema;
+		}
+		if ( 'object' === $schema['type'] ) {
+			$schema['additionalProperties'] = false;
+			if ( isset( $schema['properties'] ) && is_array( $schema['properties'] ) ) {
+				$schema['properties'] = array_map(
+					static function ( $property_schema ) {
+						return self::falsify_additional_properties( $property_schema );
+					},
+					$schema['properties']
+				);
+			}
+		} elseif ( 'array' === $schema['type'] && isset( $schema['items'] ) && is_array( $schema['items'] ) ) {
+			$schema['items'] = self::falsify_additional_properties( $schema['items'] );
+		}
+		return $schema;
+	}
+}

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

@@ -53,14 +53,14 @@
  * @since 0.1.0
  * @access private
  */
-final class OD_URL_Metric implements JsonSerializable {
+class OD_URL_Metric implements JsonSerializable {
 
 	/**
 	 * Data.
 	 *
 	 * @var Data
 	 */
-	private $data;
+	protected $data;
 
 	/**
 	 * Constructor.
@@ -123,7 +123,7 @@ private function prepare_data( array $data ): array {
 	 */
 	public static function get_json_schema(): array {
 		/*
-		 * The intersectionRect and clientBoundingRect are both instances of the DOMRectReadOnly, which
+		 * The intersectionRect and boundingClientRect are both instances of the DOMRectReadOnly, which
 		 * the following schema describes. See <https://developer.mozilla.org/en-US/docs/Web/API/DOMRectReadOnly>.
 		 * Note that 'number' is used specifically instead of 'integer' because the values are all specified as
 		 * floats/doubles.
@@ -232,11 +232,18 @@ public static function get_json_schema(): array {
 							'intersectionRect'   => $dom_rect_schema,
 							'boundingClientRect' => $dom_rect_schema,
 						),
-						'additionalProperties' => false,
+
+						// Additional properties may be added to the schema for items of elements via the od_url_metric_schema_element_item_additional_properties filter.
+						// See further explanation below.
+						'additionalProperties' => true,
 					),
 				),
 			),
-			'additionalProperties' => false,
+			// Additional root properties may be added to the schema via the od_url_metric_schema_root_additional_properties filter.
+			// Therefore, additionalProperties is set to true so that additional properties defined in the extended schema may persist
+			// in a stored URL Metric even when the extension is deactivated. For REST API requests, the OD_Strict_URL_Metric
+			// which sets this to false so that newly-submitted URL Metrics only ever include the known properties.
+			'additionalProperties' => true,
 		);
 
 		/**

plugins/optimization-detective/load.php

@@ -99,6 +99,7 @@ static function ( string $version ): void {
 		require_once __DIR__ . '/class-od-data-validation-exception.php';
 		require_once __DIR__ . '/class-od-html-tag-processor.php';
 		require_once __DIR__ . '/class-od-url-metric.php';
+		require_once __DIR__ . '/class-od-strict-url-metric.php';
 		require_once __DIR__ . '/class-od-url-metrics-group.php';
 		require_once __DIR__ . '/class-od-url-metrics-group-collection.php';
 

plugins/optimization-detective/storage/rest-api.php

@@ -67,7 +67,7 @@ function od_register_endpoint(): void {
 			'methods'             => 'POST',
 			'args'                => array_merge(
 				$args,
-				rest_get_endpoint_args_for_schema( OD_URL_Metric::get_json_schema() )
+				rest_get_endpoint_args_for_schema( OD_Strict_URL_Metric::get_json_schema() )
 			),
 			'callback'            => static function ( WP_REST_Request $request ) {
 				return od_handle_rest_request( $request );
@@ -128,19 +128,23 @@ function od_handle_rest_request( WP_REST_Request $request ) {
 	OD_Storage_Lock::set_lock();
 
 	try {
-		$url_metric = new OD_URL_Metric(
-			array_merge(
-				wp_array_slice_assoc(
-					$request->get_params(),
-					array_keys( OD_URL_Metric::get_json_schema()['properties'] )
-				),
-				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(),
-				)
+		$data = $request->get_params();
+		// Remove params which are only used for the REST API request and which are not part of a URL Metric.
+		unset(
+			$data['slug'],
+			$data['nonce']
+		);
+		$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(),
 			)
 		);
+
+		// 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 ( OD_Data_Validation_Exception $e ) {
 		return new WP_Error(
 			'rest_invalid_param',
@@ -174,7 +178,7 @@ function od_handle_rest_request( WP_REST_Request $request ) {
 	 *
 	 *     @var int                                   $post_id
 	 *     @var WP_REST_Request<array<string, mixed>> $request
-	 *     @var OD_URL_Metric                         $url_metric
+	 *     @var OD_Strict_URL_Metric                  $url_metric
 	 *     @var OD_URL_Metrics_Group                  $group
 	 *     @var OD_URL_Metrics_Group_Collection       $group_collection
 	 * }

plugins/optimization-detective/tests/storage/test-class-od-url-metrics-post-type.php

@@ -102,27 +102,34 @@ public function data_provider_test_get_url_metrics_from_post(): array {
 		$valid_content_with_uuid            = $valid_content;
 		$valid_content_with_uuid[0]['uuid'] = wp_generate_uuid4();
 
+		$valid_content_with_extra_property             = $valid_content_with_uuid;
+		$valid_content_with_extra_property[0]['extra'] = 'foo';
+
 		return array(
-			'malformed_json'  => array(
+			'malformed_json'        => array(
 				'post_content'   => '{"bad":',
 				'expected_value' => array(),
 			),
-			'not_array_json'  => array(
+			'not_array_json'        => array(
 				'post_content'   => '{"cool":"beans"}',
 				'expected_value' => array(),
 			),
-			'missing_keys'    => array(
+			'missing_keys'          => array(
 				'post_content'   => '[{},{},{}]',
 				'expected_value' => array(),
 			),
-			'valid_sans_uuid' => array(
+			'valid_sans_uuid'       => array(
 				'post_content'   => wp_json_encode( $valid_content ),
 				'expected_value' => $valid_content,
 			),
-			'valid_with_uuid' => array(
+			'valid_with_uuid'       => array(
 				'post_content'   => wp_json_encode( $valid_content_with_uuid ),
 				'expected_value' => $valid_content_with_uuid,
 			),
+			'valid_with_extra_prop' => array(
+				'post_content'   => wp_json_encode( $valid_content_with_extra_property ),
+				'expected_value' => $valid_content_with_extra_property,
+			),
 		);
 	}
 

plugins/optimization-detective/tests/storage/test-rest-api.php

@@ -22,15 +22,49 @@ public function test_od_register_endpoint_hooked(): void {
 		$this->assertSame( 10, has_action( 'rest_api_init', 'od_register_endpoint' ) );
 	}
 
+	/**
+	 * Data provider.
+	 *
+	 * @return array<string, mixed>
+	 */
+	public function data_provider_to_test_rest_request_good_params(): array {
+		return array(
+			'not_extended' => array(
+				'set_up' => function () {
+					return $this->get_valid_params();
+				},
+			),
+			'extended'     => array(
+				'set_up' => function () {
+					add_filter(
+						'od_url_metric_schema_root_additional_properties',
+						static function ( array $properties ): array {
+							$properties['extra'] = array(
+								'type' => 'string',
+							);
+							return $properties;
+						}
+					);
+
+					$params = $this->get_valid_params();
+					$params['extra'] = 'foo';
+					return $params;
+				},
+			),
+		);
+	}
+
 	/**
 	 * Test good params.
 	 *
+	 * @dataProvider data_provider_to_test_rest_request_good_params
+	 *
 	 * @covers ::od_register_endpoint
 	 * @covers ::od_handle_rest_request
 	 */
-	public function test_rest_request_good_params(): void {
+	public function test_rest_request_good_params( Closure $set_up ): void {
+		$valid_params = $set_up();
 		$request      = new WP_REST_Request( 'POST', self::ROUTE );
-		$valid_params = $this->get_valid_params();
 		$this->assertCount( 0, get_posts( array( 'post_type' => OD_URL_Metrics_Post_Type::SLUG ) ) );
 		$request->set_body_params( $valid_params );
 		$response = rest_get_server()->dispatch( $request );
@@ -47,6 +81,13 @@ public function test_rest_request_good_params(): void {
 		$this->assertCount( 1, $url_metrics, 'Expected number of URL metrics stored.' );
 		$this->assertSame( $valid_params['elements'], $url_metrics[0]->get_elements() );
 		$this->assertSame( $valid_params['viewport']['width'], $url_metrics[0]->get_viewport_width() );
+
+		$expected_data = $valid_params;
+		unset( $expected_data['nonce'], $expected_data['slug'] );
+		$this->assertSame(
+			$expected_data,
+			wp_array_slice_assoc( $url_metrics[0]->jsonSerialize(), array_keys( $expected_data ) )
+		);
 	}
 
 	/**
@@ -151,6 +192,19 @@ function ( $params ) {
 						),
 					),
 				),
+				'invalid_root_property'                    => array(
+					'is_touch' => false,
+				),
+				'invalid_element_property'                 => array(
+					'elements' => array(
+						array_merge(
+							$valid_element,
+							array(
+								'is_big' => true,
+							)
+						),
+					),
+				),
 			)
 		);
 	}
@@ -406,14 +460,14 @@ private function get_valid_params( array $extras = array() ): array {
 				),
 			)
 		)->jsonSerialize();
+		unset( $data['timestamp'], $data['uuid'] ); // Since these are readonly.
 		$data = array_merge(
 			array(
 				'slug'  => $slug,
 				'nonce' => od_get_url_metrics_storage_nonce( $slug, $data['url'] ),
 			),
 			$data
 		);
-		unset( $data['timestamp'] ); // Since provided by default args.
 		if ( count( $extras ) > 0 ) {
 			$data = $this->recursive_merge( $data, $extras );
 		}

plugins/optimization-detective/tests/test-class-od-strict-url-metric.php

@@ -0,0 +1,62 @@
+<?php
+/**
+ * Tests for optimization-detective class OD_Strict_URL_Metric.
+ *
+ * @package optimization-detective
+ *
+ * @coversDefaultClass OD_Strict_URL_Metric
+ */
+class Test_OD_Strict_URL_Metric extends WP_UnitTestCase {
+	use Optimization_Detective_Test_Helpers;
+
+	/**
+	 * Tests get_json_schema().
+	 *
+	 * @covers ::get_json_schema
+	 */
+	public function test_get_json_schema(): void {
+		add_filter(
+			'od_url_metric_schema_root_additional_properties',
+			static function ( array $properties ) {
+				$properties['colors'] = array(
+					'type'                 => 'object',
+					'properties'           => array(
+						'hex' => array(
+							'type' => 'string',
+						),
+					),
+					'additionalProperties' => true,
+				);
+				return $properties;
+			}
+		);
+		add_filter(
+			'od_url_metric_schema_element_item_additional_properties',
+			static function ( array $properties ) {
+				$properties['region'] = array(
+					'type'                 => 'object',
+					'properties'           => array(
+						'continent' => array(
+							'type' => 'string',
+						),
+					),
+					'additionalProperties' => true,
+				);
+				return $properties;
+			}
+		);
+		$loose_schema = OD_URL_Metric::get_json_schema();
+		$this->assertTrue( $loose_schema['additionalProperties'] );
+		$this->assertFalse( $loose_schema['properties']['viewport']['additionalProperties'] ); // The viewport is never extensible. Only the root and the elements are.
+		$this->assertTrue( $loose_schema['properties']['elements']['items']['additionalProperties'] );
+		$this->assertTrue( $loose_schema['properties']['elements']['items']['properties']['region']['additionalProperties'] );
+		$this->assertTrue( $loose_schema['properties']['colors']['additionalProperties'] );
+
+		$strict_schema = OD_Strict_URL_Metric::get_json_schema();
+		$this->assertFalse( $strict_schema['additionalProperties'] );
+		$this->assertFalse( $strict_schema['properties']['viewport']['additionalProperties'] );
+		$this->assertFalse( $strict_schema['properties']['elements']['items']['additionalProperties'] );
+		$this->assertFalse( $strict_schema['properties']['elements']['items']['properties']['region']['additionalProperties'] );
+		$this->assertFalse( $strict_schema['properties']['colors']['additionalProperties'] );
+	}
+}