From 586a418ef957560bb5da4554f4ea6f510e9f782b Mon Sep 17 00:00:00 2001 From: chriszarate Date: Thu, 13 Feb 2025 16:44:15 -0500 Subject: [PATCH 1/8] Fix design mistake and require __class property --- inc/Config/ArraySerializable.php | 25 +++++++++++++--------- inc/Config/ArraySerializableInterface.php | 2 ++ inc/Config/DataSource/HttpDataSource.php | 1 + inc/Config/Query/QueryInterface.php | 2 +- inc/Validation/Validator.php | 26 ++++++++++++++++------- tests/inc/Functions/FunctionsTest.php | 4 ++-- tests/inc/Validation/ValidatorTest.php | 10 ++++++--- 7 files changed, 46 insertions(+), 24 deletions(-) diff --git a/inc/Config/ArraySerializable.php b/inc/Config/ArraySerializable.php index 9b9c32147..947fdb3ef 100644 --- a/inc/Config/ArraySerializable.php +++ b/inc/Config/ArraySerializable.php @@ -29,7 +29,7 @@ protected function get_or_call_from_config( string $property_name, mixed ...$cal * @inheritDoc */ public static function from_array( array $config, ?ValidatorInterface $validator = null ): static|WP_Error { - $subclass = static::get_subclass( $config ); + $subclass = static::get_implementor( $config ); if ( null !== $subclass ) { return $subclass::from_array( $config, $validator ); } @@ -58,7 +58,7 @@ public static function from_array( array $config, ?ValidatorInterface $validator * @inheritDoc */ public function to_array(): array { - return $this->config; + return array_merge( $this->config, [ self::CLASS_REF_ATTRIBUTE => static::class ] ); } /** @@ -69,17 +69,22 @@ public static function preprocess_config( array $config ): array|WP_Error { } /** - * The config can provide a `__subclass` property that indicates that we should - * inflate using a subclass of this class. + * The config can provide a `__class` property that indicates that we should + * inflate using a specific implementor class. */ - protected static function get_subclass( array $config ): ?string { - $subclass = $config['__subclass'] ?? null; - - if ( null !== $subclass && static::class !== $subclass && is_subclass_of( $subclass, static::class, true ) ) { - return $subclass; + protected static function get_implementor( array $config ): ?string { + $subclass = $config[ self::CLASS_REF_ATTRIBUTE ] ?? null; + + if ( + null === $subclass || + static::class === $subclass || + ! class_exists( $subclass ) || + ! in_array( ArraySerializableInterface::class, class_implements( $subclass ), true ) + ) { + return null; } - return null; + return $subclass; } /** diff --git a/inc/Config/ArraySerializableInterface.php b/inc/Config/ArraySerializableInterface.php index 4fc27cf48..92f3243c5 100644 --- a/inc/Config/ArraySerializableInterface.php +++ b/inc/Config/ArraySerializableInterface.php @@ -6,6 +6,8 @@ use WP_Error; interface ArraySerializableInterface { + final public const CLASS_REF_ATTRIBUTE = '__class'; + /** * Creates an instance of the class from an array representation. * diff --git a/inc/Config/DataSource/HttpDataSource.php b/inc/Config/DataSource/HttpDataSource.php index ad42d419d..d1ef6d54a 100644 --- a/inc/Config/DataSource/HttpDataSource.php +++ b/inc/Config/DataSource/HttpDataSource.php @@ -80,6 +80,7 @@ public static function from_uuid( string $uuid ): DataSourceInterface|WP_Error { */ final public function to_array(): array { return [ + '__class' => static::class, 'service' => static::SERVICE_NAME, 'service_config' => $this->config['service_config'], 'uuid' => $this->config['uuid'], diff --git a/inc/Config/Query/QueryInterface.php b/inc/Config/Query/QueryInterface.php index f09c20322..ae2f3a062 100644 --- a/inc/Config/Query/QueryInterface.php +++ b/inc/Config/Query/QueryInterface.php @@ -12,7 +12,7 @@ */ interface QueryInterface extends ArraySerializableInterface { public function execute( array $input_variables ): array|WP_Error; - public function get_data_source(): DataSourceInterface; + public function get_data_source(): DataSourceInterface|QueryInterface; public function get_image_url(): ?string; public function get_input_schema(): array; public function get_output_schema(): array; diff --git a/inc/Validation/Validator.php b/inc/Validation/Validator.php index 3e41ae5d1..e58b11a57 100644 --- a/inc/Validation/Validator.php +++ b/inc/Validation/Validator.php @@ -188,17 +188,27 @@ private function check_non_primitive_type( array $type, mixed $value ): bool|WP_ return $this->create_error( 'Class does not implement ArraySerializableInterface', $class_ref ); } - // The config can provide a `__subclass` property that indicates that we - // should inflate using a subclass of the specified class. - $subclass = $value['__subclass'] ?? null; - if ( null !== $subclass ) { - if ( ! is_subclass_of( $subclass, $class_ref, true ) ) { - return $this->create_error( 'Class specified by __subclass must be a subclass of the specified class', $subclass ); - } + // The config must provide a `__class` property so that we know which + // class to inflate. This allows values to target subclasses of the + // specified class and also provides disambiguation when the type is + // used in a union type (one_of). + $subclass = $value[ ArraySerializableInterface::CLASS_REF_ATTRIBUTE ] ?? null; + if ( null === $subclass ) { + return $this->create_error( 'Value does not provide a __class property', $class_ref ); + } - $class_ref = $subclass; + if ( ! class_exists( $subclass ) ) { + return $this->create_error( 'Class specified by __class property does not exist', $subclass ); } + if ( $subclass !== $class_ref && ! is_subclass_of( $subclass, $class_ref, true ) ) { + return $this->create_error( 'Class specified by __class property must match or be a subclass of the target class', $subclass ); + } + + // Done with type validation, update the target class so we can validate + // the value / config. + $class_ref = $subclass; + // Validate the schema for the class we want to instantiate. Call the // config prepocessor since some classes inflate their own config. $config_validator = new Validator( $class_ref::get_config_schema(), $class_ref ); diff --git a/tests/inc/Functions/FunctionsTest.php b/tests/inc/Functions/FunctionsTest.php index 88cf3ba05..c9bd1fdfd 100644 --- a/tests/inc/Functions/FunctionsTest.php +++ b/tests/inc/Functions/FunctionsTest.php @@ -75,9 +75,9 @@ public function testRegisterBlockWithNestedConfig(): void { 'title' => 'Test Block with Nested Config', 'render_query' => [ 'query' => [ - '__subclass' => 'RemoteDataBlocks\Tests\Mocks\MockQuery', + '__class' => 'RemoteDataBlocks\Tests\Mocks\MockQuery', 'data_source' => [ - '__subclass' => 'RemoteDataBlocks\Tests\Mocks\MockDataSource', + '__class' => 'RemoteDataBlocks\Tests\Mocks\MockDataSource', 'service_config' => [ '__version' => 1, 'display_name' => 'Mock Data Source', diff --git a/tests/inc/Validation/ValidatorTest.php b/tests/inc/Validation/ValidatorTest.php index be962624a..798b5d8f9 100644 --- a/tests/inc/Validation/ValidatorTest.php +++ b/tests/inc/Validation/ValidatorTest.php @@ -594,6 +594,7 @@ public function testSerializedConfigFor(): void { $this->assertTrue( $validator->validate( [ 'config' => [ + '__class' => MockSerializableClass::class, 'boolean_value' => true, 'enum_value' => 'foo', 'string_value' => 'hello, world!', @@ -602,6 +603,7 @@ public function testSerializedConfigFor(): void { $result = $validator->validate( [ 'config' => [ + '__class' => MockSerializableClass::class, 'boolean_value' => 'NOT A BOOLEAN', 'enum_value' => 'foo', 'string_value' => 'hello, world!', @@ -629,7 +631,9 @@ public function testSerializedConfigFor(): void { $this->assertSame( 'Value must be an associative array: {}', $result->get_error_data()['child']->get_error_message() ); $result = $validator->validate( [ - 'config' => [], + 'config' => [ + '__class' => MockSerializableClass::class, + ], ] ); $this->assertInstanceOf( WP_Error::class, $result ); @@ -646,7 +650,7 @@ public function testSerializedConfigForSubclass(): void { $this->assertTrue( $validator->validate( [ 'config' => [ - '__subclass' => MockSerializableSubclass::class, + '__class' => MockSerializableSubclass::class, 'boolean_value' => true, 'enum_value' => 'foo', 'string_value' => 'hello, world!', @@ -656,7 +660,7 @@ public function testSerializedConfigForSubclass(): void { $result = $validator->validate( [ 'config' => [ - '__subclass' => MockSerializableSubclass::class, + '__class' => MockSerializableSubclass::class, 'boolean_value' => true, 'enum_value' => 'foo', 'string_value' => 'hello, world!', From 1f8cc9b312229db7b6406130cafea7344acf28e9 Mon Sep 17 00:00:00 2001 From: chriszarate Date: Thu, 13 Feb 2025 17:05:00 -0500 Subject: [PATCH 2/8] Mark ArraySerializable::from_array() as final to prevent footgun --- inc/Config/ArraySerializable.php | 2 +- tests/inc/Config/HttpDataSourceTest.php | 4 ++-- tests/inc/Config/QueryRunnerTest.php | 6 +++--- tests/inc/Config/QueryTest.php | 2 +- tests/inc/Editor/DataBinding/BlockBindingsTest.php | 12 ++++++------ tests/inc/Functions/FunctionsTest.php | 6 +++--- .../VipBlockDataApi/VipBlockDataApiTest.php | 4 ++-- tests/inc/Mocks/MockDataSource.php | 4 ++-- tests/inc/Mocks/MockQuery.php | 6 +++--- 9 files changed, 23 insertions(+), 23 deletions(-) diff --git a/inc/Config/ArraySerializable.php b/inc/Config/ArraySerializable.php index 947fdb3ef..94f39dd47 100644 --- a/inc/Config/ArraySerializable.php +++ b/inc/Config/ArraySerializable.php @@ -28,7 +28,7 @@ protected function get_or_call_from_config( string $property_name, mixed ...$cal /** * @inheritDoc */ - public static function from_array( array $config, ?ValidatorInterface $validator = null ): static|WP_Error { + final public static function from_array( array $config, ?ValidatorInterface $validator = null ): static|WP_Error { $subclass = static::get_implementor( $config ); if ( null !== $subclass ) { return $subclass::from_array( $config, $validator ); diff --git a/tests/inc/Config/HttpDataSourceTest.php b/tests/inc/Config/HttpDataSourceTest.php index 197221e7a..a50d4499b 100644 --- a/tests/inc/Config/HttpDataSourceTest.php +++ b/tests/inc/Config/HttpDataSourceTest.php @@ -16,13 +16,13 @@ public function testGetServiceMethodCannotBeOverriddenl(): void { 'endpoint' => 'http://example.com', ], ]; - $this->http_data_source = MockDataSource::from_array( $config ); + $this->http_data_source = MockDataSource::create( $config ); $this->assertSame( 'generic-http', $this->http_data_source->get_service_name() ); } public function testGetServiceMethodReturnsCorrectValue(): void { - $this->http_data_source = MockDataSource::from_array(); + $this->http_data_source = MockDataSource::create(); $this->assertEquals( 'generic-http', $this->http_data_source->get_service_name() ); } diff --git a/tests/inc/Config/QueryRunnerTest.php b/tests/inc/Config/QueryRunnerTest.php index 5aadf14de..e09bd4535 100644 --- a/tests/inc/Config/QueryRunnerTest.php +++ b/tests/inc/Config/QueryRunnerTest.php @@ -19,9 +19,9 @@ protected function setUp(): void { parent::setUp(); $this->http_client = $this->createMock( HttpClient::class ); - $this->http_data_source = MockDataSource::from_array(); + $this->http_data_source = MockDataSource::create(); - $this->query = MockQuery::from_array( [ + $this->query = MockQuery::create( [ 'data_source' => $this->http_data_source, 'query_runner' => new QueryRunner( $this->http_client ), ] ); @@ -349,7 +349,7 @@ public function testExecuteSuccessfulResponseWithObjectResponseData(): void { } public function testQueryRunnerAppliesDefaultInputVariables(): void { - $query = MockQuery::from_array( [ + $query = MockQuery::create( [ 'data_source' => $this->http_data_source, 'endpoint' => function ( array $input_variables ): string { return sprintf( diff --git a/tests/inc/Config/QueryTest.php b/tests/inc/Config/QueryTest.php index d4a11d2ed..8cd69ea7d 100644 --- a/tests/inc/Config/QueryTest.php +++ b/tests/inc/Config/QueryTest.php @@ -11,7 +11,7 @@ class QueryTest extends TestCase { private HttpQuery $query_context; protected function setUp(): void { - $this->data_source = MockDataSource::from_array(); + $this->data_source = MockDataSource::create(); $this->query_context = HttpQuery::from_array( [ 'data_source' => $this->data_source, 'output_schema' => [ 'type' => 'null' ], diff --git a/tests/inc/Editor/DataBinding/BlockBindingsTest.php b/tests/inc/Editor/DataBinding/BlockBindingsTest.php index 30c7fc942..45cfe4382 100644 --- a/tests/inc/Editor/DataBinding/BlockBindingsTest.php +++ b/tests/inc/Editor/DataBinding/BlockBindingsTest.php @@ -105,7 +105,7 @@ public function test_execute_query_returns_query_results(): void { $mock_block_config = [ 'queries' => [ - ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::from_array( [ + ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::create( [ 'input_schema' => self::MOCK_INPUT_SCHEMA, 'output_schema' => self::MOCK_OUTPUT_SCHEMA, 'query_runner' => $mock_qr, @@ -154,7 +154,7 @@ public function test_execute_query_with_overrides(): void { $mock_block_config = [ 'queries' => [ - ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::from_array( [ + ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::create( [ 'input_schema' => $input_schema, 'output_schema' => self::MOCK_OUTPUT_SCHEMA, 'query_runner' => $mock_qr, @@ -218,7 +218,7 @@ public function execute( HttpQueryInterface $query, array $input_variables ): ar $mock_block_config = [ 'queries' => [ - ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::from_array( [ + ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::create( [ 'input_schema' => $input_schema, 'output_schema' => self::MOCK_OUTPUT_SCHEMA, 'query_runner' => $mock_qr, @@ -280,7 +280,7 @@ public function execute( HttpQueryInterface $query, array $input_variables ): ar $mock_block_config = [ 'queries' => [ - ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::from_array( [ + ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::create( [ 'input_schema' => $input_schema, 'output_schema' => self::MOCK_OUTPUT_SCHEMA, 'query_runner' => $mock_qr, @@ -334,7 +334,7 @@ public function test_get_value(): void { $mock_block_config = [ 'queries' => [ - ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::from_array( [ + ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::create( [ 'input_schema' => $input_schema, 'output_schema' => self::MOCK_OUTPUT_SCHEMA, 'query_runner' => $mock_qr, @@ -389,7 +389,7 @@ public function test_get_value_with_non_string(): void { $mock_block_config = [ 'queries' => [ - ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::from_array( [ + ConfigRegistry::DISPLAY_QUERY_KEY => MockQuery::create( [ 'input_schema' => $input_schema, 'output_schema' => self::MOCK_OUTPUT_SCHEMA, 'query_runner' => $mock_qr, diff --git a/tests/inc/Functions/FunctionsTest.php b/tests/inc/Functions/FunctionsTest.php index c9bd1fdfd..de2e4be33 100644 --- a/tests/inc/Functions/FunctionsTest.php +++ b/tests/inc/Functions/FunctionsTest.php @@ -20,13 +20,13 @@ class FunctionsTest extends TestCase { protected function setUp(): void { parent::setUp(); $this->mock_logger = new MockLogger(); - $this->mock_query = MockQuery::from_array(); - $this->mock_list_query = MockQuery::from_array( [ + $this->mock_query = MockQuery::create(); + $this->mock_list_query = MockQuery::create( [ 'output_schema' => [ 'is_collection' => true, ], ] ); - $this->mock_search_query = MockQuery::from_array( [ + $this->mock_search_query = MockQuery::create( [ 'input_schema' => [ 'search' => [ 'type' => 'ui:search_input' ], ], diff --git a/tests/inc/Integrations/VipBlockDataApi/VipBlockDataApiTest.php b/tests/inc/Integrations/VipBlockDataApi/VipBlockDataApiTest.php index 490e5e60e..654849b88 100644 --- a/tests/inc/Integrations/VipBlockDataApi/VipBlockDataApiTest.php +++ b/tests/inc/Integrations/VipBlockDataApi/VipBlockDataApiTest.php @@ -147,7 +147,7 @@ public function testResolveRemoteDataSimple(): void { $mock_qr->addResult( 'title', $expected1 ); $mock_qr->addResult( 'location', $expected2 ); - $mock_query = MockQuery::from_array( [ 'query_runner' => $mock_qr ] ); + $mock_query = MockQuery::create( [ 'query_runner' => $mock_qr ] ); register_remote_data_block( [ 'title' => 'Events', 'render_query' => [ @@ -171,7 +171,7 @@ public function testResolveRemoteDataFallsBackToDbOnQuery(): void { $mock_qr->addResult( 'title', 'Happy happy hour! No networking!' ); $mock_qr->addResult( 'location', new \WP_Error( 'rdb-uh-oh', 'uh-oh!' ) ); - $mock_query = MockQuery::from_array( [ 'query_runner' => $mock_qr ] ); + $mock_query = MockQuery::create( [ 'query_runner' => $mock_qr ] ); register_remote_data_block( [ 'title' => 'Events', 'render_query' => [ diff --git a/tests/inc/Mocks/MockDataSource.php b/tests/inc/Mocks/MockDataSource.php index be90eb224..11b519444 100644 --- a/tests/inc/Mocks/MockDataSource.php +++ b/tests/inc/Mocks/MockDataSource.php @@ -20,8 +20,8 @@ class MockDataSource extends HttpDataSource { ], ]; - public static function from_array( ?array $config = self::MOCK_CONFIG, ?ValidatorInterface $validator = null ): static|WP_Error { - return parent::from_array( $config, $validator ?? new MockValidator() ); + public static function create( ?array $config = self::MOCK_CONFIG, ?ValidatorInterface $validator = null ): static|WP_Error { + return self::from_array( $config, $validator ?? new MockValidator() ); } /** diff --git a/tests/inc/Mocks/MockQuery.php b/tests/inc/Mocks/MockQuery.php index 9cf9c93a1..907cc8c8e 100644 --- a/tests/inc/Mocks/MockQuery.php +++ b/tests/inc/Mocks/MockQuery.php @@ -11,9 +11,9 @@ class MockQuery extends HttpQuery { private mixed $response_data = null; - public static function from_array( array $config = [], ?ValidatorInterface $validator = null ): static|WP_Error { - return parent::from_array( [ - 'data_source' => $config['data_source'] ?? MockDataSource::from_array(), + public static function create( array $config = [], ?ValidatorInterface $validator = null ): static|WP_Error { + return self::from_array( [ + 'data_source' => $config['data_source'] ?? MockDataSource::create(), 'display_name' => 'Mock Query', 'endpoint' => $config['endpoint'] ?? null, 'input_schema' => $config['input_schema'] ?? [], From 4641b461eccd793df7129e4ea7f2c9280068779b Mon Sep 17 00:00:00 2001 From: chriszarate Date: Thu, 13 Feb 2025 17:41:29 -0500 Subject: [PATCH 3/8] Allow queries to be used as data sources --- inc/Config/Query/HttpQuery.php | 9 ++++---- inc/Config/Query/HttpQueryInterface.php | 2 +- inc/Config/QueryRunner/QueryRunner.php | 10 +++++++-- inc/Editor/BlockManagement/ConfigRegistry.php | 4 ++-- inc/Validation/ConfigSchemas.php | 8 ++++--- tests/inc/Config/QueryTest.php | 22 +++++++++++++++++++ 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/inc/Config/Query/HttpQuery.php b/inc/Config/Query/HttpQuery.php index c28270c4b..c37a34b99 100644 --- a/inc/Config/Query/HttpQuery.php +++ b/inc/Config/Query/HttpQuery.php @@ -3,7 +3,6 @@ namespace RemoteDataBlocks\Config\Query; use RemoteDataBlocks\Config\ArraySerializable; -use RemoteDataBlocks\Config\DataSource\HttpDataSource; use RemoteDataBlocks\Config\DataSource\HttpDataSourceInterface; use RemoteDataBlocks\Config\QueryRunner\QueryRunner; use RemoteDataBlocks\Validation\ConfigSchemas; @@ -54,9 +53,9 @@ public function get_cache_ttl( array $input_variables ): null|int { /** * Get the data source associated with this query. */ - public function get_data_source(): HttpDataSourceInterface { + public function get_data_source(): HttpDataSourceInterface|HttpQueryInterface { if ( is_array( $this->config['data_source'] ) ) { - $this->config['data_source'] = HttpDataSource::from_array( $this->config['data_source'] ); + $this->config['data_source'] = ArraySerializable::from_array( $this->config['data_source'] ); } return $this->config['data_source']; @@ -66,7 +65,7 @@ public function get_data_source(): HttpDataSourceInterface { * Get the HTTP endpoint for the current query execution. */ public function get_endpoint( array $input_variables ): string { - return $this->get_or_call_from_config( 'endpoint', $input_variables ) ?? $this->get_data_source()->get_endpoint(); + return $this->get_or_call_from_config( 'endpoint', $input_variables ) ?? $this->get_data_source()->get_endpoint( $input_variables ); } /** @@ -115,7 +114,7 @@ public function get_request_body( array $input_variables ): ?array { * @param array $input_variables The input variables for this query. */ public function get_request_headers( array $input_variables ): array|WP_Error { - return $this->get_or_call_from_config( 'request_headers', $input_variables ) ?? $this->get_data_source()->get_request_headers(); + return $this->get_or_call_from_config( 'request_headers', $input_variables ) ?? $this->get_data_source()->get_request_headers( $input_variables ); } /** diff --git a/inc/Config/Query/HttpQueryInterface.php b/inc/Config/Query/HttpQueryInterface.php index 4baa2cfd7..78ae225c3 100644 --- a/inc/Config/Query/HttpQueryInterface.php +++ b/inc/Config/Query/HttpQueryInterface.php @@ -11,7 +11,7 @@ * */ interface HttpQueryInterface extends QueryInterface { - public function get_data_source(): HttpDataSourceInterface; + public function get_data_source(): HttpDataSourceInterface|HttpQueryInterface; public function get_cache_ttl( array $input_variables ): null|int; public function get_endpoint( array $input_variables ): string; public function get_request_method(): string; diff --git a/inc/Config/QueryRunner/QueryRunner.php b/inc/Config/QueryRunner/QueryRunner.php index 436ef4800..64a8dcfc6 100644 --- a/inc/Config/QueryRunner/QueryRunner.php +++ b/inc/Config/QueryRunner/QueryRunner.php @@ -115,6 +115,12 @@ protected function get_request_details( HttpQueryInterface $query, array $input_ * } */ protected function get_raw_response_data( HttpQueryInterface $query, array $input_variables ): array|WP_Error { + // If the data source is itself a query, execute it and return the results. + $data_source = $query->get_data_source(); + if ( $data_source instanceof HttpQueryInterface ) { + return $data_source->execute( $input_variables ); + } + $request_details = $this->get_request_details( $query, $input_variables ); if ( is_wp_error( $request_details ) ) { @@ -212,7 +218,7 @@ public function execute( HttpQueryInterface $query, array $input_variables ): ar } // Preprocess the response data. - $response_data = $this->preprocess_response( $query, $raw_response_data['response_data'], $input_variables ); + $response_data = $this->preprocess_response( $query, $raw_response_data['response_data'] ?? $raw_response_data, $input_variables ); // Determine if the response data is expected to be a collection. $output_schema = $query->get_output_schema(); @@ -224,7 +230,7 @@ public function execute( HttpQueryInterface $query, array $input_variables ): ar $parser = new QueryResponseParser(); $results = $parser->parse( $response_data, $output_schema ); $results = $is_collection ? $results : [ $results ]; - $metadata = $this->get_response_metadata( $query, $raw_response_data['metadata'], $results ); + $metadata = $this->get_response_metadata( $query, $raw_response_data['metadata'] ?? [], $results ); // Pagination schema defines how to extract pagination data from the response. $pagination = null; diff --git a/inc/Editor/BlockManagement/ConfigRegistry.php b/inc/Editor/BlockManagement/ConfigRegistry.php index 4802120a1..299ae7bd0 100644 --- a/inc/Editor/BlockManagement/ConfigRegistry.php +++ b/inc/Editor/BlockManagement/ConfigRegistry.php @@ -6,7 +6,7 @@ use RemoteDataBlocks\Logging\LoggerManager; use Psr\Log\LoggerInterface; -use RemoteDataBlocks\Config\Query\HttpQuery; +use RemoteDataBlocks\Config\ArraySerializable; use RemoteDataBlocks\Config\Query\QueryInterface; use RemoteDataBlocks\Editor\BlockPatterns\BlockPatterns; use RemoteDataBlocks\Validation\ConfigSchemas; @@ -178,7 +178,7 @@ private static function create_error( string $block_title, string $message ): WP private static function inflate_query( array|QueryInterface $config ): QueryInterface { if ( is_array( $config ) ) { - return HttpQuery::from_array( $config ); + return ArraySerializable::from_array( $config ); } return $config; diff --git a/inc/Validation/ConfigSchemas.php b/inc/Validation/ConfigSchemas.php index ab0b10c17..257b7402d 100644 --- a/inc/Validation/ConfigSchemas.php +++ b/inc/Validation/ConfigSchemas.php @@ -2,9 +2,9 @@ namespace RemoteDataBlocks\Validation; -use RemoteDataBlocks\Validation\Types; use RemoteDataBlocks\Config\DataSource\HttpDataSourceInterface; use RemoteDataBlocks\Config\Query\HttpQueryInterface; +use RemoteDataBlocks\Validation\Types; use RemoteDataBlocks\Config\Query\QueryInterface; use RemoteDataBlocks\Config\QueryRunner\QueryRunnerInterface; use RemoteDataBlocks\Editor\BlockManagement\ConfigRegistry; @@ -77,7 +77,7 @@ private static function generate_remote_data_block_config_schema(): array { 'render_query' => Types::object( [ 'query' => Types::one_of( Types::instance_of( QueryInterface::class ), - Types::serialized_config_for( HttpQueryInterface::class ), + Types::serialized_config_for( QueryInterface::class ), ), 'loop' => Types::nullable( Types::boolean() ), ] ), @@ -87,7 +87,7 @@ private static function generate_remote_data_block_config_schema(): array { 'display_name' => Types::nullable( Types::string() ), 'query' => Types::one_of( Types::instance_of( QueryInterface::class ), - Types::serialized_config_for( HttpQueryInterface::class ), + Types::serialized_config_for( QueryInterface::class ), ), 'type' => Types::enum( ConfigRegistry::LIST_QUERY_KEY, @@ -157,7 +157,9 @@ private static function generate_http_query_config_schema(): array { 'cache_ttl' => Types::nullable( Types::one_of( Types::callable(), Types::integer(), Types::null() ) ), 'data_source' => Types::one_of( Types::instance_of( HttpDataSourceInterface::class ), + Types::instance_of( HttpQueryInterface::class ), Types::serialized_config_for( HttpDataSourceInterface::class ), + Types::serialized_config_for( HttpQueryInterface::class ), ), 'endpoint' => Types::nullable( Types::one_of( Types::callable(), Types::url() ) ), 'image_url' => Types::nullable( Types::image_url() ), diff --git a/tests/inc/Config/QueryTest.php b/tests/inc/Config/QueryTest.php index 8cd69ea7d..b3393b7b1 100644 --- a/tests/inc/Config/QueryTest.php +++ b/tests/inc/Config/QueryTest.php @@ -5,6 +5,8 @@ use PHPUnit\Framework\TestCase; use RemoteDataBlocks\Config\Query\HttpQuery; use RemoteDataBlocks\Tests\Mocks\MockDataSource; +use RemoteDataBlocks\Tests\Mocks\MockQuery; +use RemoteDataBlocks\Tests\Mocks\MockQueryRunner; class QueryTest extends TestCase { private MockDataSource $data_source; @@ -77,4 +79,24 @@ public function testCustomPreprocessResponse(): void { $this->assertSame( $expected_json, $custom_query_context->preprocess_response( $html_data, [] ) ); } + + public function testQueryAsDataSource(): void { + $mock_qr = new MockQueryRunner(); + $mock_qr->addResult( 'foo', 'bar' ); + + $query_with_query_as_data_source = HttpQuery::from_array( [ + 'data_source' => MockQuery::create( [ 'query_runner' => $mock_qr ] ), + 'output_schema' => [ + 'type' => [ + 'nested_foo' => [ + 'path' => '$.results[0].result.foo.value', + 'type' => 'string', + ], + ], + ], + ] ); + + $result = $query_with_query_as_data_source->execute( [] )['results'][0]['result']['nested_foo']; + $this->assertSame( 'bar', $result['value'] ); + } } From b0dd773361045280ff61e9937df6f46ba29dc25e Mon Sep 17 00:00:00 2001 From: chriszarate Date: Thu, 13 Feb 2025 17:49:19 -0500 Subject: [PATCH 4/8] Add one_of serialized_config_for test --- tests/inc/Validation/ValidatorTest.php | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/inc/Validation/ValidatorTest.php b/tests/inc/Validation/ValidatorTest.php index 798b5d8f9..eca02c252 100644 --- a/tests/inc/Validation/ValidatorTest.php +++ b/tests/inc/Validation/ValidatorTest.php @@ -672,6 +672,48 @@ public function testSerializedConfigForSubclass(): void { $this->assertSame( 'Object must have valid property: extra_value', $result->get_error_data()['child']->get_error_message() ); } + public function testOneOfSerializedConfig(): void { + $schema = Types::object( [ + 'config' => Types::one_of( + Types::serialized_config_for( MockSerializableClass::class ), + Types::serialized_config_for( MockSerializableSubclass::class ) + ), + ] ); + + $validator = new Validator( $schema ); + + $this->assertTrue( $validator->validate( [ + 'config' => [ + '__class' => MockSerializableSubclass::class, + 'boolean_value' => true, + 'enum_value' => 'foo', + 'string_value' => 'hello, world!', + 'extra_value' => 'required for subclass', + ], + ] ) ); + + $this->assertTrue( $validator->validate( [ + 'config' => [ + '__class' => MockSerializableClass::class, + 'boolean_value' => true, + 'enum_value' => 'foo', + 'string_value' => 'hello, world!', + ], + ] ) ); + + $result = $validator->validate( [ + 'config' => [ + 'boolean_value' => true, + 'enum_value' => 'foo', + 'string_value' => 'hello, world!', + ], + ] ); + + $this->assertInstanceOf( WP_Error::class, $result ); + $this->assertSame( 'Object must have valid property: config', $result->get_error_message() ); + $this->assertSame( 'Value must be one of the specified types: {"boolean_value":true,"enum_value":"foo","string_value":"hello, world!"}', $result->get_error_data()['child']->get_error_message() ); + } + public function testStringMatching(): void { $schema = Types::string_matching( '/^foo$/' ); From ca2e75226cbef078e2bdad00b74a7668d1020dfa Mon Sep 17 00:00:00 2001 From: chriszarate Date: Thu, 13 Feb 2025 18:43:37 -0500 Subject: [PATCH 5/8] Remove duplicate HttpDataSource docs --- docs/extending/data-source.md | 39 +---------------------------------- 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/docs/extending/data-source.md b/docs/extending/data-source.md index bc7d512ff..582ba20d7 100644 --- a/docs/extending/data-source.md +++ b/docs/extending/data-source.md @@ -22,7 +22,7 @@ $data_source = HttpDataSource::from_array( [ ## HttpDataSource configuration -### **version**: number (required) +### \_\_version: number (required) There is no built-in versioning logic, but a version number is required for best practice reasons. Changes to the data source could significantly affect [queries](query.md). Checking the data source version is a sensible defensive practice. @@ -80,43 +80,6 @@ $zipcode_query = HttpQuery::from_array( [ ] ) ``` -The goal with design was to provide you with flexibility you need to represent any data source. - -## HttpDataSource configuration - -### **version**: number (required) - -There is no built-in versioning logic, but a version number is required for best practice reasons. Changes to the data source could significantly affect [queries](query.md). Checking the data source version is a sensible defensive practice. - -### display_name: string (required) - -The display name is used in the UI to identify your data source. - -### endpoint: string - -This is the default endpoint for the data source and can save repeated use in queries. We would suggest putting the root API URL here and then manipulating it as necessary in individual [queries](query.md). - -### request_headers: array - -Headers will be set according to the properties of the array. When providing authentication credentials, take care to keep them from appearing in code repositories. We strongly recommend using environment variables or other secure means for storage. - -## Additional parameters - -You can add any additional parameters that are necessary for your data source. In our [Airtable example](https://github.com/Automattic/remote-data-blocks/blob/trunk/example/airtable/events/register.php), you can see that we are setting values for the Airtable `base` and `table`. - -Consider adding whatever configuration would be useful to queries. As an example, queries have an `endpoint` property. Our [Zip code example](https://github.com/Automattic/remote-data-blocks/blob/trunk/example/rest-api/zip-code/zip-code.php) sets the endpoint with a function: - -```php -$zipcode_query = HttpQuery::from_array( [ - 'data_source' => $zipcode_data_source, - 'endpoint' => function ( array $input_variables ) use ( $zipcode_data_source ): string { - return $zipcode_data_source->get_endpoint() . $input_variables['zip_code']; - }, -]) -``` - -The goal with design was to provide you with flexibility you need to represent any data source. - ## Custom data sources The configuration array passed to `from_array` is very flexible, so it's usually not necessary to extend `HttpDataSource`, but you can do so if you need to add custom behavior. From a1a1c0629fe9ddba9362afa097a26992bc9f39c1 Mon Sep 17 00:00:00 2001 From: chriszarate Date: Thu, 13 Feb 2025 19:59:16 -0500 Subject: [PATCH 6/8] Add docs for nested query --- docs/extending/query.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/extending/query.md b/docs/extending/query.md index 52002d159..f1646c890 100644 --- a/docs/extending/query.md +++ b/docs/extending/query.md @@ -66,9 +66,9 @@ This example features a small subset of the customization available for a query; The `display_name` property defines the query's human-friendly name. -### data_source: HttpDataSourceInterface (required) +### data_source: HttpDataSourceInterface|HttpQueryInterface (required) -The `data_source` property provides the [data source](./data-source.md) the query uses. +The `data_source` property provides the [data source](./data-source.md) the query uses. You can also supply another query as the data source, allowing you to compose queries together. This is useful when you want to access subcollections or related data returned by a "parent" query. Query results are always cached in-memory, so there is no performance penalty for composing queries in this way. ### endpoint: string|callable From f0afd75c9105889ef0601d8f74deaf646f6a7b7d Mon Sep 17 00:00:00 2001 From: chriszarate Date: Fri, 14 Feb 2025 16:45:59 -0500 Subject: [PATCH 7/8] Guard access to DataSourceInterface method --- inc/Editor/BlockManagement/ConfigStore.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/inc/Editor/BlockManagement/ConfigStore.php b/inc/Editor/BlockManagement/ConfigStore.php index 24d4cf20a..7ace74fd3 100644 --- a/inc/Editor/BlockManagement/ConfigStore.php +++ b/inc/Editor/BlockManagement/ConfigStore.php @@ -7,6 +7,7 @@ use RemoteDataBlocks\Config\Query\QueryInterface; use RemoteDataBlocks\Logging\LoggerManager; use Psr\Log\LoggerInterface; +use RemoteDataBlocks\Config\DataSource\DataSourceInterface; use function sanitize_title_with_dashes; @@ -82,7 +83,13 @@ public static function get_data_source_type( string $block_name ): ?string { return null; } - return $query->get_data_source()->get_service_name(); + $data_source = $query->get_data_source(); + + if ( $data_source instanceof DataSourceInterface ) { + return $data_source->get_service_name(); + } + + return null; } /** From 4cfdd75c39b2a57bdc43aeed1448bca5483fdb88 Mon Sep 17 00:00:00 2001 From: chriszarate Date: Fri, 14 Feb 2025 16:46:26 -0500 Subject: [PATCH 8/8] Add integration test --- tests/integration/RDBTestCase.php | 9 +- .../blocks/BlockWithQueryAsDataSourceTest.php | 169 ++++++++++++++++++ 2 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 tests/integration/blocks/BlockWithQueryAsDataSourceTest.php diff --git a/tests/integration/RDBTestCase.php b/tests/integration/RDBTestCase.php index fecf4f933..837fab4e0 100644 --- a/tests/integration/RDBTestCase.php +++ b/tests/integration/RDBTestCase.php @@ -97,7 +97,7 @@ protected function register_remote_data_block_from_block_title( string $block_ti } protected function get_query_runner_with_response( array $response_data, int $status_code = 200 ): QueryRunner { - return new class($response_data, $status_code) extends QueryRunner { + return new class( $response_data, $status_code ) extends QueryRunner { private $response_data; private $status_code; @@ -139,6 +139,13 @@ protected function get_dom_element_by_html_id( DOMDocument $dom, string $html_id return $nodes; } + protected function get_dom_elements_by_html_class( DOMDocument $dom, string $html_class ): DOMNodeList|false { + $xpath = new DOMXPath( $dom ); + $nodes = $xpath->query( sprintf( "//*[@class='%s']", $html_class ) ); + + return $nodes; + } + protected function assertDomIdHasTextContent( DOMDocument $dom, string $html_id, string $expected_content ): void { $id_nodes = $this->get_dom_element_by_html_id( $dom, $html_id ); diff --git a/tests/integration/blocks/BlockWithQueryAsDataSourceTest.php b/tests/integration/blocks/BlockWithQueryAsDataSourceTest.php new file mode 100644 index 000000000..c945101f4 --- /dev/null +++ b/tests/integration/blocks/BlockWithQueryAsDataSourceTest.php @@ -0,0 +1,169 @@ + 12345, + 'name' => 'Crayons', + 'price' => '0.99', + 'details' => [ + 'variants' => [ + [ + 'name' => 'Burnt Sienna', + 'code' => 'burnt-sienna', + ], + [ + 'name' => 'Periwinkle', + 'code' => 'periwinkle', + ], + [ + 'name' => 'Fuscia', + 'code' => 'fuscia', + ], + ], + ], + ]; + + $toy_query = [ + '__class' => 'RemoteDataBlocks\\Config\\Query\\HttpQuery', + 'data_source' => [ + '__class' => 'RemoteDataBlocks\\Config\\DataSource\\HttpDataSource', + 'service_config' => [ + '__version' => 1, + 'display_name' => 'Test API', + // Mocked query runner will not actually make a request to the endpoint URL. + 'endpoint' => 'https://example.com/not-a-real-api', + ], + ], + 'output_schema' => [ + 'is_collection' => false, + 'type' => [ + 'id' => [ + 'name' => 'ID', + 'path' => '$.id', + 'type' => 'string', + ], + 'name' => [ + 'name' => 'Name', + 'path' => '$.name', + 'type' => 'string', + ], + 'price' => [ + 'name' => 'Price', + 'path' => '$.price', + 'type' => 'currency_in_current_locale', + ], + 'variants' => [ + 'is_collection' => true, + 'name' => 'Types', + 'path' => '$.details.variants[*]', + 'type' => [ + 'name' => [ + 'name' => 'Name', + 'path' => '$.name', + 'type' => 'string', + ], + 'code' => [ + 'name' => 'Code', + 'path' => '$.code', + 'type' => 'string', + ], + ], + ], + ], + ], + 'query_runner' => $this->get_query_runner_with_response( $test_api_response ), + ]; + + $registration_result = register_remote_data_block( [ + 'title' => 'Toy', + 'render_query' => [ + 'query' => $toy_query, + ], + ] ); + + $this->assertTrue( $registration_result ); + $this->register_remote_data_block_from_block_title( 'Toy' ); + + $registration_result = register_remote_data_block( [ + 'title' => 'Toy Variant', + 'render_query' => [ + 'loop' => true, + 'query' => [ + '__class' => 'RemoteDataBlocks\\Config\\Query\\HttpQuery', + 'data_source' => $toy_query, + 'output_schema' => [ + 'is_collection' => true, + 'path' => '$.results[0].result.variants.value[*].result', + 'type' => [ + 'name' => [ + 'name' => 'Name', + 'path' => '$.name.value', + 'type' => 'string', + ], + 'code' => [ + 'name' => 'Code', + 'path' => '$.code.value', + 'type' => 'string', + ], + ], + ], + ], + ], + ] ); + + $this->assertTrue( $registration_result ); + $this->register_remote_data_block_from_block_title( 'Toy Variant' ); + + $result_html = do_blocks(' + +
+ +

+ + + +

+ + + +

Types

+ + + +
+ +

+ + + +

+ +
+ + +
+ + '); + + $dom = self::load_html( $result_html ); + $this->assertDomIdHasTextContent( $dom, 'field-name', 'Crayons' ); + $this->assertDomIdHasTextContent( $dom, 'field-price', '$0.99' ); + + $variant_names = $this->get_dom_elements_by_html_class( $dom, 'field-variant-name' ); + $this->assertCount( 3, $variant_names, sprintf( "Should be 3 matching nodes with class 'field-variant-name' but %d found.", count( $variant_names ) ) ); + $this->assertEquals( 'Burnt Sienna', $variant_names[0]->textContent ); + $this->assertEquals( 'Periwinkle', $variant_names[1]->textContent ); + $this->assertEquals( 'Fuscia', $variant_names[2]->textContent ); + + $variant_codes = $this->get_dom_elements_by_html_class( $dom, 'field-variant-code' ); + $this->assertCount( 3, $variant_codes, sprintf( "Should be 3 matching nodes with class 'field-variant-code' but %d found.", count( $variant_codes ) ) ); + $this->assertEquals( 'burnt-sienna', $variant_codes[0]->textContent ); + $this->assertEquals( 'periwinkle', $variant_codes[1]->textContent ); + $this->assertEquals( 'fuscia', $variant_codes[2]->textContent ); + } +}