From 1c739f193e0cfcc811f99d56ff2d5d9dbf144ee3 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 17:50:58 +0000 Subject: [PATCH 01/14] Add WP_Block_Bindings_Source class for block bindings API --- .../class-wp-block-bindings-source.php | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 src/wp-includes/class-wp-block-bindings-source.php diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php new file mode 100644 index 0000000000000..1507b1dbd169f --- /dev/null +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -0,0 +1,109 @@ +name = $name; + + /* Validate that the source properties contain the label */ + if ( ! isset( $source_properties['label'] ) ) { + _doing_it_wrong( + __METHOD__, + __( 'The source properties must contain a label.' ), + '6.5.0' + ); + return; + } + + /* Validate that the source properties contain the get_value_callback */ + if ( ! isset( $source_properties['get_value_callback'] ) ) { + _doing_it_wrong( + __METHOD__, + __( 'The source properties must contain a get_value_callback.' ), + '6.5.0' + ); + return; + } + + $this->label = $source_properties['label']; + $this->callback = $source_properties['get_value_callback']; + } + + /** + * The name of the source. + * + * @since 6.5.0 + * @var string + */ + public $name; + + /** + * The label of the source. + * + * @since 6.5.0 + * @var string + */ + public $label; + + + /** + * The function used to get the value of the source. + * + * @since 6.5.0 + * @var callable + */ + public $callback; + + /** + * The source properties used to register a source. + * + * @since 6.5.0 + * @var array $source_properties { + * @type string $label The label of the source. + * @type callback $get_value_callback A callback executed when the source is processed during block rendering. + * The callback should have the following signature: + * + * `function ($source_args, $block_instance, $attribute_name): mixed` + * - @param array $source_args Array containing source arguments + * used to look up the override value, + * i.e. {"key": "foo"}. + * - @param WP_Block $block_instance The block instance. + * - @param string $attribute_name The name of the target attribute. + * The callback has a mixed return type; it may return a string to override + * the block's original value, null, false to remove an attribute, etc. + * } + */ + public $source_properties; + + + public function get_value( $source_properties, $block_instance, $attribute_name ) { + if ( ! isset( $source_properties['get_value_callback'] ) ) { + _doing_it_wrong( + __METHOD__, + __( 'There is no ' ), + '6.5.0' + ); + return null; + } + + return call_user_func( $this->callback, $source_properties, $block_instance, $attribute_name ); + } +} From 7ecf423539bd2a5717cd7206dfbbbebe7ab458b5 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 17:57:32 +0000 Subject: [PATCH 02/14] Refactor get_value method in WP_Block_Bindings_Source class --- .../class-wp-block-bindings-source.php | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index 1507b1dbd169f..a3baf2cab4169 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -94,16 +94,18 @@ public function __construct( string $name, array $source_properties ) { public $source_properties; - public function get_value( $source_properties, $block_instance, $attribute_name ) { - if ( ! isset( $source_properties['get_value_callback'] ) ) { - _doing_it_wrong( - __METHOD__, - __( 'There is no ' ), - '6.5.0' - ); - return null; - } - - return call_user_func( $this->callback, $source_properties, $block_instance, $attribute_name ); + /** + * Retrieves the value of the source. + * + * @since 6.5.0 + * + * @param array $source_args Array containing source arguments used to look up the override value, i.e. {"key": "foo"}. + * @param WP_Block $block_instance The block instance. + * @param string $attribute_name The name of the target attribute. + * + * @return mixed The value of the source. + */ + public function get_value( array $source_args, $block_instance, string $attribute_name ) { + return call_user_func( $this->callback, array( $source_args, $block_instance, $attribute_name ) ); } } From 1c487359b6137c36680240dc9f728409eadcd1fb Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 17:58:19 +0000 Subject: [PATCH 03/14] Use WP_Block_Bindings_Source in the block bindings API --- src/wp-includes/block-bindings.php | 8 ++++---- .../class-wp-block-bindings-registry.php | 14 +++++++------- src/wp-includes/class-wp-block.php | 5 ++--- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/wp-includes/block-bindings.php b/src/wp-includes/block-bindings.php index 4fc58c3325ba8..a4dc1626e64dd 100644 --- a/src/wp-includes/block-bindings.php +++ b/src/wp-includes/block-bindings.php @@ -38,7 +38,7 @@ * The callback has a mixed return type; it may return a string to override * the block's original value, null, false to remove an attribute, etc. * } - * @return array|false Source when the registration was successful, or `false` on failure. + * @return WP_Block_Bindings_Source|false Source when the registration was successful, or `false` on failure. */ function register_block_bindings_source( string $source_name, array $source_properties ) { return WP_Block_Bindings_Registry::get_instance()->register( $source_name, $source_properties ); @@ -50,7 +50,7 @@ function register_block_bindings_source( string $source_name, array $source_prop * @since 6.5.0 * * @param string $source_name Block bindings source name including namespace. - * @return array|false The unregistered block bindings source on success and `false` otherwise. + * @return WP_Block_Bindings_Source|false The unregistered block bindings source on success and `false` otherwise. */ function unregister_block_bindings_source( string $source_name ) { return WP_Block_Bindings_Registry::get_instance()->unregister( $source_name ); @@ -61,7 +61,7 @@ function unregister_block_bindings_source( string $source_name ) { * * @since 6.5.0 * - * @return array The array of registered block bindings sources. + * @return WP_Block_Bindings_Source[] The array of registered block bindings sources. */ function get_all_registered_block_bindings_sources() { return WP_Block_Bindings_Registry::get_instance()->get_all_registered(); @@ -73,7 +73,7 @@ function get_all_registered_block_bindings_sources() { * @since 6.5.0 * * @param string $source_name The name of the source. - * @return array|null The registered block bindings source, or `null` if it is not registered. + * @return WP_Block_Bindings_Source|null The registered block bindings source, or `null` if it is not registered. */ function get_block_bindings_source( string $source_name ) { return WP_Block_Bindings_Registry::get_instance()->get_registered( $source_name ); diff --git a/src/wp-includes/class-wp-block-bindings-registry.php b/src/wp-includes/class-wp-block-bindings-registry.php index 28e8b1b60186e..631a5dc0acc12 100644 --- a/src/wp-includes/class-wp-block-bindings-registry.php +++ b/src/wp-includes/class-wp-block-bindings-registry.php @@ -20,7 +20,7 @@ final class WP_Block_Bindings_Registry { * Holds the registered block bindings sources, keyed by source identifier. * * @since 6.5.0 - * @var array + * @var WP_Block_Bindings_Source[] */ private $sources = array(); @@ -61,7 +61,7 @@ final class WP_Block_Bindings_Registry { * The callback has a mixed return type; it may return a string to override * the block's original value, null, false to remove an attribute, etc. * } - * @return array|false Source when the registration was successful, or `false` on failure. + * @return WP_Block_Bindings_Source|false Source when the registration was successful, or `false` on failure. */ public function register( string $source_name, array $source_properties ) { if ( ! is_string( $source_name ) ) { @@ -102,8 +102,8 @@ public function register( string $source_name, array $source_properties ) { return false; } - $source = array_merge( - array( 'name' => $source_name ), + $source = new WP_Block_Bindings_Source( + $source_name, $source_properties ); @@ -118,7 +118,7 @@ public function register( string $source_name, array $source_properties ) { * @since 6.5.0 * * @param string $source_name Block bindings source name including namespace. - * @return array|false The unregistered block bindings source on success and `false` otherwise. + * @return WP_Block_Bindings_Source|false The unregistered block bindings source on success and `false` otherwise. */ public function unregister( string $source_name ) { if ( ! $this->is_registered( $source_name ) ) { @@ -142,7 +142,7 @@ public function unregister( string $source_name ) { * * @since 6.5.0 * - * @return array The array of registered sources. + * @return WP_Block_Bindings_Source[] The array of registered sources. */ public function get_all_registered() { return $this->sources; @@ -154,7 +154,7 @@ public function get_all_registered() { * @since 6.5.0 * * @param string $source_name The name of the source. - * @return array|null The registered block bindings source, or `null` if it is not registered. + * @return WP_Block_Bindings_Source|null The registered block bindings source, or `null` if it is not registered. */ public function get_registered( string $source_name ) { if ( ! $this->is_registered( $source_name ) ) { diff --git a/src/wp-includes/class-wp-block.php b/src/wp-includes/class-wp-block.php index 80757baabce39..de69fda943c2a 100644 --- a/src/wp-includes/class-wp-block.php +++ b/src/wp-includes/class-wp-block.php @@ -270,9 +270,8 @@ private function process_block_bindings( $block_content ) { continue; } - $source_callback = $block_binding_source['get_value_callback']; - $source_args = ! empty( $block_binding['args'] ) && is_array( $block_binding['args'] ) ? $block_binding['args'] : array(); - $source_value = call_user_func_array( $source_callback, array( $source_args, $this, $attribute_name ) ); + $source_args = ! empty( $block_binding['args'] ) && is_array( $block_binding['args'] ) ? $block_binding['args'] : array(); + $source_value = $block_binding_source->get_value( $source_args, $this, $attribute_name ); // If the value is not null, process the HTML based on the block and the attribute. if ( ! is_null( $source_value ) ) { From 3be4ffaf548287d653b840c6999d2df07185f85d Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 18:25:59 +0000 Subject: [PATCH 04/14] Add class-wp-block-bindings-source.php to wp-settings.php --- src/wp-settings.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wp-settings.php b/src/wp-settings.php index 22683b37d1f5d..b055636ff8d86 100644 --- a/src/wp-settings.php +++ b/src/wp-settings.php @@ -330,6 +330,7 @@ require ABSPATH . WPINC . '/sitemaps/providers/class-wp-sitemaps-taxonomies.php'; require ABSPATH . WPINC . '/sitemaps/providers/class-wp-sitemaps-users.php'; require ABSPATH . WPINC . '/class-wp-block-bindings-registry.php'; +require ABSPATH . WPINC . '/class-wp-block-bindings-source.php'; require ABSPATH . WPINC . '/class-wp-block-editor-context.php'; require ABSPATH . WPINC . '/class-wp-block-type.php'; require ABSPATH . WPINC . '/class-wp-block-pattern-categories-registry.php'; From 3b5bfc83a8996cce7bda77a91d2d64cf78e3a2bc Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 18:26:30 +0000 Subject: [PATCH 05/14] Rename "callback" to get_value_callback in WP_Block_Bindings_Source class --- .../class-wp-block-bindings-source.php | 34 ++++--------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index a3baf2cab4169..a6efcd98ba2fc 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -27,7 +27,7 @@ public function __construct( string $name, array $source_properties ) { if ( ! isset( $source_properties['label'] ) ) { _doing_it_wrong( __METHOD__, - __( 'The source properties must contain a label.' ), + __( 'The $source_properties must contain a "label".' ), '6.5.0' ); return; @@ -37,14 +37,14 @@ public function __construct( string $name, array $source_properties ) { if ( ! isset( $source_properties['get_value_callback'] ) ) { _doing_it_wrong( __METHOD__, - __( 'The source properties must contain a get_value_callback.' ), + __( 'The $source_properties must contain a "get_value_callback".' ), '6.5.0' ); return; } - $this->label = $source_properties['label']; - $this->callback = $source_properties['get_value_callback']; + $this->label = $source_properties['label']; + $this->get_value_callback = $source_properties['get_value_callback']; } /** @@ -70,29 +70,7 @@ public function __construct( string $name, array $source_properties ) { * @since 6.5.0 * @var callable */ - public $callback; - - /** - * The source properties used to register a source. - * - * @since 6.5.0 - * @var array $source_properties { - * @type string $label The label of the source. - * @type callback $get_value_callback A callback executed when the source is processed during block rendering. - * The callback should have the following signature: - * - * `function ($source_args, $block_instance, $attribute_name): mixed` - * - @param array $source_args Array containing source arguments - * used to look up the override value, - * i.e. {"key": "foo"}. - * - @param WP_Block $block_instance The block instance. - * - @param string $attribute_name The name of the target attribute. - * The callback has a mixed return type; it may return a string to override - * the block's original value, null, false to remove an attribute, etc. - * } - */ - public $source_properties; - + private $get_value_callback; /** * Retrieves the value of the source. @@ -106,6 +84,6 @@ public function __construct( string $name, array $source_properties ) { * @return mixed The value of the source. */ public function get_value( array $source_args, $block_instance, string $attribute_name ) { - return call_user_func( $this->callback, array( $source_args, $block_instance, $attribute_name ) ); + return call_user_func_array( $this->get_value_callback, array( $source_args, $block_instance, $attribute_name ) ); } } From 0c635f6be09d410e511bf213986663b20924e61a Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 19:06:07 +0000 Subject: [PATCH 06/14] Update the unit tests to use the new WP_Block_Bindings_Source class --- .../phpunit/tests/block-bindings/register.php | 17 +++++---- .../wpBlockBindingsRegistry.php | 38 ++++++++++--------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/tests/phpunit/tests/block-bindings/register.php b/tests/phpunit/tests/block-bindings/register.php index 800db70a70da6..3c3ae899e2ace 100644 --- a/tests/phpunit/tests/block-bindings/register.php +++ b/tests/phpunit/tests/block-bindings/register.php @@ -13,7 +13,8 @@ class Tests_Block_Bindings_Register extends WP_UnitTestCase { const TEST_SOURCE_NAME = 'test/source'; const TEST_SOURCE_PROPERTIES = array( - 'label' => 'Test source', + 'label' => 'Test source', + 'get_value_callback' => 'test value', ); /** @@ -39,6 +40,7 @@ public function tear_down() { * @covers ::register_block_bindings_source * @covers ::get_all_registered_block_bindings_sources * @covers ::get_block_bindings_source + * @covers WP_Block_Bindings_Source::__construct */ public function test_get_all_registered() { $source_one_name = 'test/source-one'; @@ -54,9 +56,9 @@ public function test_get_all_registered() { register_block_bindings_source( $source_three_name, $source_three_properties ); $expected = array( - $source_one_name => array_merge( array( 'name' => $source_one_name ), $source_one_properties ), - $source_two_name => array_merge( array( 'name' => $source_two_name ), $source_two_properties ), - $source_three_name => array_merge( array( 'name' => $source_three_name ), $source_three_properties ), + $source_one_name => new WP_Block_Bindings_Source( $source_one_name, $source_one_properties ), + $source_two_name => new WP_Block_Bindings_Source( $source_two_name, $source_two_properties ), + $source_three_name => new WP_Block_Bindings_Source( $source_three_name, $source_three_properties ), 'core/post-meta' => get_block_bindings_source( 'core/post-meta' ), 'core/pattern-overrides' => get_block_bindings_source( 'core/pattern-overrides' ), ); @@ -72,14 +74,15 @@ public function test_get_all_registered() { * * @covers ::register_block_bindings_source * @covers ::unregister_block_bindings_source + * @covers WP_Block_Bindings_Source::__construct */ public function test_unregister_block_source() { register_block_bindings_source( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); $result = unregister_block_bindings_source( self::TEST_SOURCE_NAME ); - $this->assertSame( - array_merge( - array( 'name' => self::TEST_SOURCE_NAME ), + $this->assertEquals( + new WP_Block_Bindings_Source( + self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ), $result diff --git a/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php b/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php index e3f205e88ce80..33bd4395cf326 100644 --- a/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php +++ b/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php @@ -15,7 +15,8 @@ class Tests_Blocks_wpBlockBindingsRegistry extends WP_UnitTestCase { const TEST_SOURCE_NAME = 'test/source'; const TEST_SOURCE_PROPERTIES = array( - 'label' => 'Test source', + 'label' => 'Test source', + 'get_value_callback' => 'test value', ); /** @@ -110,12 +111,13 @@ public function test_register_invalid_uppercase_characters() { * @ticket 60282 * * @covers WP_Block_Bindings_Registry::register + * @covers WP_Block_Bindings_Source::__construct */ public function test_register_block_binding_source() { $result = $this->registry->register( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); - $this->assertSame( - array_merge( - array( 'name' => self::TEST_SOURCE_NAME ), + $this->assertEquals( + new WP_Block_Bindings_Source( + self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ), $result @@ -143,14 +145,15 @@ public function test_unregister_not_registered_block() { * * @covers WP_Block_Bindings_Registry::register * @covers WP_Block_Bindings_Registry::unregister + * WP_Block_Bindings_Source::__construct */ public function test_unregister_block_source() { $this->registry->register( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); $result = $this->registry->unregister( self::TEST_SOURCE_NAME ); - $this->assertSame( - array_merge( - array( 'name' => self::TEST_SOURCE_NAME ), + $this->assertEquals( + new WP_Block_Bindings_Source( + self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ), $result @@ -164,6 +167,7 @@ public function test_unregister_block_source() { * * @covers WP_Block_Bindings_Registry::register * @covers WP_Block_Bindings_Registry::get_all_registered + * WP_Block_Bindings_Source::__construct */ public function test_get_all_registered() { $source_one_name = 'test/source-one'; @@ -179,13 +183,13 @@ public function test_get_all_registered() { $this->registry->register( $source_three_name, $source_three_properties ); $expected = array( - $source_one_name => array_merge( array( 'name' => $source_one_name ), $source_one_properties ), - $source_two_name => array_merge( array( 'name' => $source_two_name ), $source_two_properties ), - $source_three_name => array_merge( array( 'name' => $source_three_name ), $source_three_properties ), + $source_one_name => new WP_Block_Bindings_Source( $source_one_name, $source_one_properties ), + $source_two_name => new WP_Block_Bindings_Source( $source_two_name, $source_two_properties ), + $source_three_name => new WP_Block_Bindings_Source( $source_three_name, $source_three_properties ), ); $registered = $this->registry->get_all_registered(); - $this->assertSame( $expected, $registered ); + $this->assertEquals( $expected, $registered ); } /** @@ -210,6 +214,7 @@ public function test_get_registered_rejects_unknown_source_name() { * * @covers WP_Block_Bindings_Registry::register * @covers WP_Block_Bindings_Registry::get_registered + * @covers WP_Block_Bindings_Source::__construct */ public function test_get_registered() { $source_one_name = 'test/source-one'; @@ -224,12 +229,11 @@ public function test_get_registered() { $source_three_properties = self::TEST_SOURCE_PROPERTIES; $this->registry->register( $source_three_name, $source_three_properties ); - $result = $this->registry->get_registered( 'test/source-two' ); - $this->assertSame( - array_merge( - array( 'name' => $source_two_name ), - $source_two_properties - ), + $expected = new WP_Block_Bindings_Source( $source_two_name, $source_two_properties ); + $result = $this->registry->get_registered( 'test/source-two' ); + + $this->assertEquals( + $expected, $result ); } From 05d27f1c61845dc1f3123cc19dd8eceb4b987c02 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Tue, 6 Feb 2024 19:21:48 +0000 Subject: [PATCH 07/14] Update comment wording --- src/wp-includes/class-wp-block-bindings-source.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index a6efcd98ba2fc..b7cd6a5d198e4 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -65,7 +65,7 @@ public function __construct( string $name, array $source_properties ) { /** - * The function used to get the value of the source. + * The function used to get the value from the source. * * @since 6.5.0 * @var callable @@ -73,7 +73,7 @@ public function __construct( string $name, array $source_properties ) { private $get_value_callback; /** - * Retrieves the value of the source. + * Retrieves the value from the source. * * @since 6.5.0 * From 16641c0fcda30bd7306192fd89e48871c93d6dc8 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 15:12:34 +0000 Subject: [PATCH 08/14] Add docstring for the WP_Block_Bindings_Source constructor --- src/wp-includes/class-wp-block-bindings-source.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index b7cd6a5d198e4..fc909b4102a89 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -20,6 +20,17 @@ */ final class WP_Block_Bindings_Source { + /** + * Constructor. + * + * Do not use this constructor directly. Instead, use the + * `WP_Block_Bindings_Registry::register` method or the `register_block_bindings_source` function. + * + * @since 6.5.0 + * + * @param string $name The name of the source. + * @param array $source_properties The properties of the source. + */ public function __construct( string $name, array $source_properties ) { $this->name = $name; From 9853934296fa166ceeaa1c0f6d03797f9f3a712d Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 15:14:01 +0000 Subject: [PATCH 09/14] Move the constructor of WP_Block_Bindings_Source after the class fields --- .../class-wp-block-bindings-source.php | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index fc909b4102a89..e4cf2d403ca16 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -20,6 +20,31 @@ */ final class WP_Block_Bindings_Source { + /** + * The name of the source. + * + * @since 6.5.0 + * @var string + */ + public $name; + + /** + * The label of the source. + * + * @since 6.5.0 + * @var string + */ + public $label; + + + /** + * The function used to get the value from the source. + * + * @since 6.5.0 + * @var callable + */ + private $get_value_callback; + /** * Constructor. * @@ -58,31 +83,6 @@ public function __construct( string $name, array $source_properties ) { $this->get_value_callback = $source_properties['get_value_callback']; } - /** - * The name of the source. - * - * @since 6.5.0 - * @var string - */ - public $name; - - /** - * The label of the source. - * - * @since 6.5.0 - * @var string - */ - public $label; - - - /** - * The function used to get the value from the source. - * - * @since 6.5.0 - * @var callable - */ - private $get_value_callback; - /** * Retrieves the value from the source. * From 1f14cb0ce81b0d0eba838aa69c0231ffe739c275 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 15:15:47 +0000 Subject: [PATCH 10/14] Add __wakeup() magic method to prevent unserialization --- src/wp-includes/class-wp-block-bindings-source.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index e4cf2d403ca16..09982e6a57d15 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -97,4 +97,13 @@ public function __construct( string $name, array $source_properties ) { public function get_value( array $source_args, $block_instance, string $attribute_name ) { return call_user_func_array( $this->get_value_callback, array( $source_args, $block_instance, $attribute_name ) ); } + + /** + * Wakeup magic method. + * + * @since 6.5.0 + */ + public function __wakeup() { + throw new \LogicException( __CLASS__ . ' should never be unserialized' ); + } } From 5f36c82bb33513b1620f294c793623c23dfdce82 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 15:18:04 +0000 Subject: [PATCH 11/14] Fix import order in wp-settings.php --- src/wp-settings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wp-settings.php b/src/wp-settings.php index b055636ff8d86..b77a5348e7cfb 100644 --- a/src/wp-settings.php +++ b/src/wp-settings.php @@ -329,8 +329,8 @@ require ABSPATH . WPINC . '/sitemaps/providers/class-wp-sitemaps-posts.php'; require ABSPATH . WPINC . '/sitemaps/providers/class-wp-sitemaps-taxonomies.php'; require ABSPATH . WPINC . '/sitemaps/providers/class-wp-sitemaps-users.php'; -require ABSPATH . WPINC . '/class-wp-block-bindings-registry.php'; require ABSPATH . WPINC . '/class-wp-block-bindings-source.php'; +require ABSPATH . WPINC . '/class-wp-block-bindings-registry.php'; require ABSPATH . WPINC . '/class-wp-block-editor-context.php'; require ABSPATH . WPINC . '/class-wp-block-type.php'; require ABSPATH . WPINC . '/class-wp-block-pattern-categories-registry.php'; From 9a1ed6e6d66f1b661acee13dd2f7c69ae03515a8 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 15:56:01 +0000 Subject: [PATCH 12/14] Refactor block bindings registration and add unit tests --- .../phpunit/tests/block-bindings/register.php | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/tests/phpunit/tests/block-bindings/register.php b/tests/phpunit/tests/block-bindings/register.php index 3c3ae899e2ace..49dd203d30ca6 100644 --- a/tests/phpunit/tests/block-bindings/register.php +++ b/tests/phpunit/tests/block-bindings/register.php @@ -11,11 +11,24 @@ */ class Tests_Block_Bindings_Register extends WP_UnitTestCase { - const TEST_SOURCE_NAME = 'test/source'; - const TEST_SOURCE_PROPERTIES = array( - 'label' => 'Test source', - 'get_value_callback' => 'test value', - ); + public static $test_source_name = 'test/source'; + public static $test_source_properties = array(); + + /** + * Set up before each test. + * + * @since 6.5.0 + */ + public function set_up() { + parent::set_up(); + + self::$test_source_properties = array( + 'label' => 'Test source', + 'get_value_callback' => function () { + return 'test-value'; + }, + ); + } /** * Tear down after each test. @@ -44,15 +57,15 @@ public function tear_down() { */ public function test_get_all_registered() { $source_one_name = 'test/source-one'; - $source_one_properties = self::TEST_SOURCE_PROPERTIES; + $source_one_properties = self::$test_source_properties; register_block_bindings_source( $source_one_name, $source_one_properties ); $source_two_name = 'test/source-two'; - $source_two_properties = self::TEST_SOURCE_PROPERTIES; + $source_two_properties = self::$test_source_properties; register_block_bindings_source( $source_two_name, $source_two_properties ); $source_three_name = 'test/source-three'; - $source_three_properties = self::TEST_SOURCE_PROPERTIES; + $source_three_properties = self::$test_source_properties; register_block_bindings_source( $source_three_name, $source_three_properties ); $expected = array( @@ -77,13 +90,13 @@ public function test_get_all_registered() { * @covers WP_Block_Bindings_Source::__construct */ public function test_unregister_block_source() { - register_block_bindings_source( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); + register_block_bindings_source( self::$test_source_name, self::$test_source_properties ); - $result = unregister_block_bindings_source( self::TEST_SOURCE_NAME ); + $result = unregister_block_bindings_source( self::$test_source_name ); $this->assertEquals( new WP_Block_Bindings_Source( - self::TEST_SOURCE_NAME, - self::TEST_SOURCE_PROPERTIES + self::$test_source_name, + self::$test_source_properties ), $result ); From 729fdaa67bad919509b9f4740f83040ed8a764b7 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 16:13:51 +0000 Subject: [PATCH 13/14] Move source validation from source class to the registry class --- .../class-wp-block-bindings-registry.php | 30 +++++++++++++++++++ .../class-wp-block-bindings-source.php | 23 +------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/wp-includes/class-wp-block-bindings-registry.php b/src/wp-includes/class-wp-block-bindings-registry.php index 631a5dc0acc12..bdfc024959fc2 100644 --- a/src/wp-includes/class-wp-block-bindings-registry.php +++ b/src/wp-includes/class-wp-block-bindings-registry.php @@ -102,6 +102,36 @@ public function register( string $source_name, array $source_properties ) { return false; } + /* Validate that the source properties contain the label */ + if ( ! isset( $source_properties['label'] ) ) { + _doing_it_wrong( + __METHOD__, + __( 'The $source_properties must contain a "label".' ), + '6.5.0' + ); + return false; + } + + /* Validate that the source properties contain the get_value_callback */ + if ( ! isset( $source_properties['get_value_callback'] ) ) { + _doing_it_wrong( + __METHOD__, + __( 'The $source_properties must contain a "get_value_callback".' ), + '6.5.0' + ); + return false; + } + + /* Validate that the get_value_callback is a valid callback */ + if ( ! is_callable( $source_properties['get_value_callback'] ) ) { + _doing_it_wrong( + __METHOD__, + __( 'The "get_value_callback" parameter must be a valid callback.' ), + '6.5.0' + ); + return false; + } + $source = new WP_Block_Bindings_Source( $source_name, $source_properties diff --git a/src/wp-includes/class-wp-block-bindings-source.php b/src/wp-includes/class-wp-block-bindings-source.php index 09982e6a57d15..c2a3d4f8ae2b8 100644 --- a/src/wp-includes/class-wp-block-bindings-source.php +++ b/src/wp-includes/class-wp-block-bindings-source.php @@ -57,28 +57,7 @@ final class WP_Block_Bindings_Source { * @param array $source_properties The properties of the source. */ public function __construct( string $name, array $source_properties ) { - $this->name = $name; - - /* Validate that the source properties contain the label */ - if ( ! isset( $source_properties['label'] ) ) { - _doing_it_wrong( - __METHOD__, - __( 'The $source_properties must contain a "label".' ), - '6.5.0' - ); - return; - } - - /* Validate that the source properties contain the get_value_callback */ - if ( ! isset( $source_properties['get_value_callback'] ) ) { - _doing_it_wrong( - __METHOD__, - __( 'The $source_properties must contain a "get_value_callback".' ), - '6.5.0' - ); - return; - } - + $this->name = $name; $this->label = $source_properties['label']; $this->get_value_callback = $source_properties['get_value_callback']; } From 433c66194bbd60275d120a4b46ac7e502e287a97 Mon Sep 17 00:00:00 2001 From: Michal Czaplinski Date: Wed, 7 Feb 2024 16:18:56 +0000 Subject: [PATCH 14/14] Update the block bindings registry tests --- .../wpBlockBindingsRegistry.php | 105 ++++++++++++++---- 1 file changed, 81 insertions(+), 24 deletions(-) diff --git a/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php b/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php index 33bd4395cf326..554058f4606c8 100644 --- a/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php +++ b/tests/phpunit/tests/block-bindings/wpBlockBindingsRegistry.php @@ -13,11 +13,8 @@ */ class Tests_Blocks_wpBlockBindingsRegistry extends WP_UnitTestCase { - const TEST_SOURCE_NAME = 'test/source'; - const TEST_SOURCE_PROPERTIES = array( - 'label' => 'Test source', - 'get_value_callback' => 'test value', - ); + public static $test_source_name = 'test/source'; + public static $test_source_properties = array(); /** * Fake block bindings registry. @@ -36,6 +33,13 @@ public function set_up() { parent::set_up(); $this->registry = new WP_Block_Bindings_Registry(); + + self::$test_source_properties = array( + 'label' => 'Test source', + 'get_value_callback' => function () { + return 'test-value'; + }, + ); } /** @@ -59,7 +63,7 @@ public function tear_down() { * @expectedIncorrectUsage WP_Block_Bindings_Registry::register */ public function test_register_invalid_non_string_names() { - $result = $this->registry->register( 1, self::TEST_SOURCE_PROPERTIES ); + $result = $this->registry->register( 1, self::$test_source_properties ); $this->assertFalse( $result ); } @@ -73,7 +77,7 @@ public function test_register_invalid_non_string_names() { * @expectedIncorrectUsage WP_Block_Bindings_Registry::register */ public function test_register_invalid_names_without_namespace() { - $result = $this->registry->register( 'post-meta', self::TEST_SOURCE_PROPERTIES ); + $result = $this->registry->register( 'post-meta', self::$test_source_properties ); $this->assertFalse( $result ); } @@ -101,7 +105,60 @@ public function test_register_invalid_characters() { * @expectedIncorrectUsage WP_Block_Bindings_Registry::register */ public function test_register_invalid_uppercase_characters() { - $result = $this->registry->register( 'Core/PostMeta', self::TEST_SOURCE_PROPERTIES ); + $result = $this->registry->register( 'Core/PostMeta', self::$test_source_properties ); + $this->assertFalse( $result ); + } + + /** + * Should reject block bindings registration without a label. + * + * @ticket 60282 + * + * @covers WP_Block_Bindings_Registry::register + * + * @expectedIncorrectUsage WP_Block_Bindings_Registry::register + */ + public function test_register_invalid_missing_label() { + + // Remove the label from the properties. + unset( self::$test_source_properties['label'] ); + + $result = $this->registry->register( self::$test_source_name, self::$test_source_properties ); + $this->assertFalse( $result ); + } + + /** + * Should reject block bindings registration without a get_value_callback. + * + * @ticket 60282 + * + * @covers WP_Block_Bindings_Registry::register + * + * @expectedIncorrectUsage WP_Block_Bindings_Registry::register + */ + public function test_register_invalid_missing_get_value_callback() { + + // Remove the get_value_callback from the properties. + unset( self::$test_source_properties['get_value_callback'] ); + + $result = $this->registry->register( self::$test_source_name, self::$test_source_properties ); + $this->assertFalse( $result ); + } + + /** + * Should reject block bindings registration if `get_value_callback` is not a callable. + * + * @ticket 60282 + * + * @covers WP_Block_Bindings_Registry::register + * + * @expectedIncorrectUsage WP_Block_Bindings_Registry::register + */ + public function test_register_invalid_incorrect_callback_type() { + + self::$test_source_properties['get_value_callback'] = 'not-a-callback'; + + $result = $this->registry->register( self::$test_source_name, self::$test_source_properties ); $this->assertFalse( $result ); } @@ -114,11 +171,11 @@ public function test_register_invalid_uppercase_characters() { * @covers WP_Block_Bindings_Source::__construct */ public function test_register_block_binding_source() { - $result = $this->registry->register( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); + $result = $this->registry->register( self::$test_source_name, self::$test_source_properties ); $this->assertEquals( new WP_Block_Bindings_Source( - self::TEST_SOURCE_NAME, - self::TEST_SOURCE_PROPERTIES + self::$test_source_name, + self::$test_source_properties ), $result ); @@ -148,13 +205,13 @@ public function test_unregister_not_registered_block() { * WP_Block_Bindings_Source::__construct */ public function test_unregister_block_source() { - $this->registry->register( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); + $this->registry->register( self::$test_source_name, self::$test_source_properties ); - $result = $this->registry->unregister( self::TEST_SOURCE_NAME ); + $result = $this->registry->unregister( self::$test_source_name ); $this->assertEquals( new WP_Block_Bindings_Source( - self::TEST_SOURCE_NAME, - self::TEST_SOURCE_PROPERTIES + self::$test_source_name, + self::$test_source_properties ), $result ); @@ -171,15 +228,15 @@ public function test_unregister_block_source() { */ public function test_get_all_registered() { $source_one_name = 'test/source-one'; - $source_one_properties = self::TEST_SOURCE_PROPERTIES; + $source_one_properties = self::$test_source_properties; $this->registry->register( $source_one_name, $source_one_properties ); $source_two_name = 'test/source-two'; - $source_two_properties = self::TEST_SOURCE_PROPERTIES; + $source_two_properties = self::$test_source_properties; $this->registry->register( $source_two_name, $source_two_properties ); $source_three_name = 'test/source-three'; - $source_three_properties = self::TEST_SOURCE_PROPERTIES; + $source_three_properties = self::$test_source_properties; $this->registry->register( $source_three_name, $source_three_properties ); $expected = array( @@ -201,7 +258,7 @@ public function test_get_all_registered() { * @covers WP_Block_Bindings_Registry::get_registered */ public function test_get_registered_rejects_unknown_source_name() { - $this->registry->register( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); + $this->registry->register( self::$test_source_name, self::$test_source_properties ); $source = $this->registry->get_registered( 'test/unknown-source' ); $this->assertNull( $source ); @@ -218,15 +275,15 @@ public function test_get_registered_rejects_unknown_source_name() { */ public function test_get_registered() { $source_one_name = 'test/source-one'; - $source_one_properties = self::TEST_SOURCE_PROPERTIES; + $source_one_properties = self::$test_source_properties; $this->registry->register( $source_one_name, $source_one_properties ); $source_two_name = 'test/source-two'; - $source_two_properties = self::TEST_SOURCE_PROPERTIES; + $source_two_properties = self::$test_source_properties; $this->registry->register( $source_two_name, $source_two_properties ); $source_three_name = 'test/source-three'; - $source_three_properties = self::TEST_SOURCE_PROPERTIES; + $source_three_properties = self::$test_source_properties; $this->registry->register( $source_three_name, $source_three_properties ); $expected = new WP_Block_Bindings_Source( $source_two_name, $source_two_properties ); @@ -259,9 +316,9 @@ public function test_is_registered_for_unknown_source() { * @covers WP_Block_Bindings_Registry::is_registered */ public function test_is_registered_for_known_source() { - $this->registry->register( self::TEST_SOURCE_NAME, self::TEST_SOURCE_PROPERTIES ); + $this->registry->register( self::$test_source_name, self::$test_source_properties ); - $result = $this->registry->is_registered( self::TEST_SOURCE_NAME ); + $result = $this->registry->is_registered( self::$test_source_name ); $this->assertTrue( $result ); } }