From 235cb398918c86b6007e7f37e1daf3a07c681f77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Zi=C3=83=C2=B3=C3=85=E2=80=9Akowski?= Date: Fri, 2 Feb 2024 20:22:11 +0000 Subject: [PATCH] Editor: Refactor the way block bindings sources are handled It fixes the coding style issues reported. It goes further and improves the code quality it other places where the logic for block bindings was added. Follow-up for [57514]. Props: gziolo, mukesh27, youknowriad, santosguillamot. See #60282. git-svn-id: https://develop.svn.wordpress.org/trunk@57526 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/block-bindings.php | 24 ++++-- .../block-bindings/pattern-overrides.php | 46 +++++++++++ src/wp-includes/block-bindings/post-meta.php | 58 +++++++++++++ .../block-bindings/sources/pattern.php | 34 -------- .../block-bindings/sources/post-meta.php | 47 ----------- .../class-wp-block-bindings-registry.php | 8 +- src/wp-includes/class-wp-block.php | 81 +++++++++---------- src/wp-settings.php | 4 +- .../phpunit/tests/block-bindings/register.php | 28 +++---- .../{block-bindings.php => render.php} | 25 +++--- 10 files changed, 188 insertions(+), 167 deletions(-) create mode 100644 src/wp-includes/block-bindings/pattern-overrides.php create mode 100644 src/wp-includes/block-bindings/post-meta.php delete mode 100644 src/wp-includes/block-bindings/sources/pattern.php delete mode 100644 src/wp-includes/block-bindings/sources/post-meta.php rename tests/phpunit/tests/block-bindings/{block-bindings.php => render.php} (79%) diff --git a/src/wp-includes/block-bindings.php b/src/wp-includes/block-bindings.php index 108efc3d84..4fc58c3325 100644 --- a/src/wp-includes/block-bindings.php +++ b/src/wp-includes/block-bindings.php @@ -19,10 +19,10 @@ * * @since 6.5.0 * - * @param string $source_name The name of the source. It must be a string containing a namespace prefix, i.e. - * `my-plugin/my-custom-source`. It must only contain lowercase alphanumeric - * characters, the forward slash `/` and dashes. - * @param array $source_properties { + * @param string $source_name The name of the source. It must be a string containing a namespace prefix, i.e. + * `my-plugin/my-custom-source`. It must only contain lowercase alphanumeric + * characters, the forward slash `/` and dashes. + * @param array $source_properties { * The array of arguments that are used to register a source. * * @type string $label The label of the source. @@ -40,7 +40,7 @@ * } * @return array|false Source when the registration was successful, or `false` on failure. */ -function register_block_bindings_source( $source_name, array $source_properties ) { +function register_block_bindings_source( string $source_name, array $source_properties ) { return WP_Block_Bindings_Registry::get_instance()->register( $source_name, $source_properties ); } @@ -52,7 +52,7 @@ function register_block_bindings_source( $source_name, array $source_properties * @param string $source_name Block bindings source name including namespace. * @return array|false The unregistered block bindings source on success and `false` otherwise. */ -function unregister_block_bindings_source( $source_name ) { +function unregister_block_bindings_source( string $source_name ) { return WP_Block_Bindings_Registry::get_instance()->unregister( $source_name ); } @@ -66,3 +66,15 @@ function unregister_block_bindings_source( $source_name ) { function get_all_registered_block_bindings_sources() { return WP_Block_Bindings_Registry::get_instance()->get_all_registered(); } + +/** + * Retrieves a registered block bindings source. + * + * @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. + */ +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/block-bindings/pattern-overrides.php b/src/wp-includes/block-bindings/pattern-overrides.php new file mode 100644 index 0000000000..6f719101ef --- /dev/null +++ b/src/wp-includes/block-bindings/pattern-overrides.php @@ -0,0 +1,46 @@ + "foo" ). + * @param WP_Block $block_instance The block instance. + * @param string $attribute_name The name of the target attribute. + * @return mixed The value computed for the source. + */ +function _block_bindings_pattern_overrides_get_value( array $source_args, $block_instance, string $attribute_name ) { + if ( empty( $block_instance->attributes['metadata']['id'] ) ) { + return null; + } + $block_id = $block_instance->attributes['metadata']['id']; + return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, $attribute_name ), null ); +} + +/** + * Registers Pattern Overrides source in the Block Bindings registry. + * + * @since 6.5.0 + * @access private + */ +function _register_block_bindings_pattern_overrides_source() { + register_block_bindings_source( + 'core/pattern-overrides', + array( + 'label' => _x( 'Pattern Overrides', 'block bindings source' ), + 'get_value_callback' => '_block_bindings_pattern_overrides_get_value', + ) + ); +} + +add_action( 'init', '_register_block_bindings_pattern_overrides_source' ); diff --git a/src/wp-includes/block-bindings/post-meta.php b/src/wp-includes/block-bindings/post-meta.php new file mode 100644 index 0000000000..4be1ae96b7 --- /dev/null +++ b/src/wp-includes/block-bindings/post-meta.php @@ -0,0 +1,58 @@ + "foo" ). + * @return mixed The value computed for the source. + */ +function _block_bindings_post_meta_get_value( array $source_args ) { + if ( ! isset( $source_args['key'] ) ) { + return null; + } + + // Use the postId attribute if available. + if ( isset( $source_args['postId'] ) ) { + $post_id = $source_args['postId']; + } else { + // $block_instance->context['postId'] is not available in the Image block. + $post_id = get_the_ID(); + } + + // If a post isn't public, we need to prevent unauthorized users from accessing the post meta. + $post = get_post( $post_id ); + if ( ( ! is_post_publicly_viewable( $post ) && ! current_user_can( 'read_post', $post_id ) ) || post_password_required( $post ) ) { + return null; + } + + return get_post_meta( $post_id, $source_args['key'], true ); +} + +/** + * Registers Post Meta source in the block bindings registry. + * + * @since 6.5.0 + * @access private + */ +function _register_block_bindings_post_meta_source() { + register_block_bindings_source( + 'core/post-meta', + array( + 'label' => _x( 'Post Meta', 'block bindings source' ), + 'get_value_callback' => '_block_bindings_post_meta_get_value', + ) + ); +} + +add_action( 'init', '_register_block_bindings_post_meta_source' ); diff --git a/src/wp-includes/block-bindings/sources/pattern.php b/src/wp-includes/block-bindings/sources/pattern.php deleted file mode 100644 index 863f73da87..0000000000 --- a/src/wp-includes/block-bindings/sources/pattern.php +++ /dev/null @@ -1,34 +0,0 @@ -attributes, array( 'metadata', 'id' ), false ) ) { - return null; - } - $block_id = $block_instance->attributes['metadata']['id']; - return _wp_array_get( $block_instance->context, array( 'pattern/overrides', $block_id, $attribute_name ), null ); -} - - -/** - * Registers the "pattern" source for the Block Bindings API. - * - * @access private - * @since 6.5.0 - */ -function _register_block_bindings_pattern_overrides_source() { - register_block_bindings_source( - 'core/pattern-overrides', - array( - 'label' => _x( 'Pattern Overrides', 'block bindings source' ), - 'get_value_callback' => 'pattern_source_callback', - ) - ); -} - -add_action( 'init', '_register_block_bindings_pattern_overrides_source' ); diff --git a/src/wp-includes/block-bindings/sources/post-meta.php b/src/wp-includes/block-bindings/sources/post-meta.php deleted file mode 100644 index 0aa55ba180..0000000000 --- a/src/wp-includes/block-bindings/sources/post-meta.php +++ /dev/null @@ -1,47 +0,0 @@ -context['postId'] is not available in the Image block. - $post_id = get_the_ID(); - } - - // If a post isn't public, we need to prevent - // unauthorized users from accessing the post meta. - $post = get_post( $post_id ); - if ( ( ! is_post_publicly_viewable( $post ) && ! current_user_can( 'read_post', $post_id ) ) || post_password_required( $post ) ) { - return null; - } - - return get_post_meta( $post_id, $source_attrs['key'], true ); -} - -/** - * Registers the "post_meta" source for the Block Bindings API. - * - * @access private - * @since 6.5.0 - */ -function _register_block_bindings_post_meta_source() { - register_block_bindings_source( - 'core/post-meta', - array( - 'label' => _x( 'Post Meta', 'block bindings source' ), - 'get_value_callback' => 'post_meta_source_callback', - ) - ); -} - -add_action( 'init', '_register_block_bindings_post_meta_source' ); diff --git a/src/wp-includes/class-wp-block-bindings-registry.php b/src/wp-includes/class-wp-block-bindings-registry.php index f824694c49..28e8b1b601 100644 --- a/src/wp-includes/class-wp-block-bindings-registry.php +++ b/src/wp-includes/class-wp-block-bindings-registry.php @@ -57,13 +57,13 @@ final class WP_Block_Bindings_Registry { * 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 an attribute . + * - @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. * } * @return array|false Source when the registration was successful, or `false` on failure. */ - public function register( $source_name, array $source_properties ) { + public function register( string $source_name, array $source_properties ) { if ( ! is_string( $source_name ) ) { _doing_it_wrong( __METHOD__, @@ -120,7 +120,7 @@ final class WP_Block_Bindings_Registry { * @param string $source_name Block bindings source name including namespace. * @return array|false The unregistered block bindings source on success and `false` otherwise. */ - public function unregister( $source_name ) { + public function unregister( string $source_name ) { if ( ! $this->is_registered( $source_name ) ) { _doing_it_wrong( __METHOD__, @@ -156,7 +156,7 @@ final class WP_Block_Bindings_Registry { * @param string $source_name The name of the source. * @return array|null The registered block bindings source, or `null` if it is not registered. */ - public function get_registered( $source_name ) { + public function get_registered( string $source_name ) { if ( ! $this->is_registered( $source_name ) ) { return null; } diff --git a/src/wp-includes/class-wp-block.php b/src/wp-includes/class-wp-block.php index f639594b11..80757baabc 100644 --- a/src/wp-includes/class-wp-block.php +++ b/src/wp-includes/class-wp-block.php @@ -211,11 +211,11 @@ class WP_Block { * "metadata": { * "bindings": { * "title": { - * "source": "post_meta", + * "source": "core/post-meta", * "args": { "key": "text_custom_field" } * }, * "url": { - * "source": "post_meta", + * "source": "core/post-meta", * "args": { "key": "url_custom_field" } * } * } @@ -226,14 +226,14 @@ class WP_Block { * The above example will replace the `title` and `url` attributes of the Image * block with the values of the `text_custom_field` and `url_custom_field` post meta. * - * @access private * @since 6.5.0 * - * @param string $block_content Block content. - * @param array $block The full block, including name and attributes. + * @param string $block_content Block content. + * @param array $block The full block, including name and attributes. + * @return string The modified block content. */ private function process_block_bindings( $block_content ) { - $block = $this->parsed_block; + $parsed_block = $this->parsed_block; // Allowed blocks that support block bindings. // TODO: Look for a mechanism to opt-in for this. Maybe adding a property to block attributes? @@ -246,69 +246,68 @@ class WP_Block { // If the block doesn't have the bindings property, isn't one of the allowed // block types, or the bindings property is not an array, return the block content. - if ( ! isset( $block['attrs']['metadata']['bindings'] ) || - ! is_array( $block['attrs']['metadata']['bindings'] ) || - ! isset( $allowed_blocks[ $this->name ] ) - ) { + if ( + ! isset( $allowed_blocks[ $this->name ] ) || + empty( $parsed_block['attrs']['metadata']['bindings'] ) || + ! is_array( $parsed_block['attrs']['metadata']['bindings'] ) + ) { return $block_content; } - $block_bindings_sources = get_all_registered_block_bindings_sources(); $modified_block_content = $block_content; - foreach ( $block['attrs']['metadata']['bindings'] as $binding_attribute => $binding_source ) { - // If the attribute is not in the list, process next attribute. - if ( ! in_array( $binding_attribute, $allowed_blocks[ $this->name ], true ) ) { + foreach ( $parsed_block['attrs']['metadata']['bindings'] as $attribute_name => $block_binding ) { + // If the attribute is not in the allowed list, process next attribute. + if ( ! in_array( $attribute_name, $allowed_blocks[ $this->name ], true ) ) { continue; } // If no source is provided, or that source is not registered, process next attribute. - if ( ! isset( $binding_source['source'] ) || ! is_string( $binding_source['source'] ) || ! isset( $block_bindings_sources[ $binding_source['source'] ] ) ) { + if ( ! isset( $block_binding['source'] ) || ! is_string( $block_binding['source'] ) ) { continue; } - $source_callback = $block_bindings_sources[ $binding_source['source'] ]['get_value_callback']; - // Get the value based on the source. - if ( ! isset( $binding_source['args'] ) ) { - $source_args = array(); - } else { - $source_args = $binding_source['args']; - } - $source_value = call_user_func_array( $source_callback, array( $source_args, $this, $binding_attribute ) ); - // If the value is null, process next attribute. - if ( is_null( $source_value ) ) { + $block_binding_source = get_block_bindings_source( $block_binding['source'] ); + if ( null === $block_binding_source ) { continue; } - // Process the HTML based on the block and the attribute. - $modified_block_content = $this->replace_html( $modified_block_content, $this->name, $binding_attribute, $source_value ); + $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 ) ); + + // If the value is not null, process the HTML based on the block and the attribute. + if ( ! is_null( $source_value ) ) { + $modified_block_content = $this->replace_html( $modified_block_content, $attribute_name, $source_value ); + } } + return $modified_block_content; } /** - * Depending on the block attributes, replace the HTML based on the value returned by the source. + * Depending on the block attribute name, replace its value in the HTML based on the value provided. * * @since 6.5.0 * - * @param string $block_content Block content. - * @param string $block_name The name of the block to process. - * @param string $block_attr The attribute of the block we want to process. - * @param string $source_value The value used to replace the HTML. + * @param string $block_content Block content. + * @param string $attribute_name The attribute name to replace. + * @param mixed $source_value The value used to replace in the HTML. + * @return string The modified block content. */ - private function replace_html( string $block_content, string $block_name, string $block_attr, string $source_value ) { + private function replace_html( string $block_content, string $attribute_name, $source_value ) { $block_type = $this->block_type; - if ( null === $block_type || ! isset( $block_type->attributes[ $block_attr ] ) ) { + if ( ! isset( $block_type->attributes[ $attribute_name ] ) ) { return $block_content; } // Depending on the attribute source, the processing will be different. - switch ( $block_type->attributes[ $block_attr ]['source'] ) { + switch ( $block_type->attributes[ $attribute_name ]['source'] ) { case 'html': case 'rich-text': $block_reader = new WP_HTML_Tag_Processor( $block_content ); // TODO: Support for CSS selectors whenever they are ready in the HTML API. // In the meantime, support comma-separated selectors by exploding them into an array. - $selectors = explode( ',', $block_type->attributes[ $block_attr ]['selector'] ); + $selectors = explode( ',', $block_type->attributes[ $attribute_name ]['selector'] ); // Add a bookmark to the first tag to be able to iterate over the selectors. $block_reader->next_tag(); $block_reader->set_bookmark( 'iterate-selectors' ); @@ -316,7 +315,7 @@ class WP_Block { // TODO: This shouldn't be needed when the `set_inner_html` function is ready. // Store the parent tag and its attributes to be able to restore them later in the button. // The button block has a wrapper while the paragraph and heading blocks don't. - if ( 'core/button' === $block_name ) { + if ( 'core/button' === $this->name ) { $button_wrapper = $block_reader->get_tag(); $button_wrapper_attribute_names = $block_reader->get_attribute_names_with_prefix( '' ); $button_wrapper_attrs = array(); @@ -348,10 +347,10 @@ class WP_Block { foreach ( $selector_attrs as $attribute_key => $attribute_value ) { $amended_content->set_attribute( $attribute_key, $attribute_value ); } - if ( 'core/paragraph' === $block_name || 'core/heading' === $block_name ) { + if ( 'core/paragraph' === $this->name || 'core/heading' === $this->name ) { return $amended_content->get_updated_html(); } - if ( 'core/button' === $block_name ) { + if ( 'core/button' === $this->name ) { $button_markup = "<$button_wrapper>{$amended_content->get_updated_html()}"; $amended_button = new WP_HTML_Tag_Processor( $button_markup ); $amended_button->next_tag(); @@ -372,12 +371,12 @@ class WP_Block { if ( ! $amended_content->next_tag( array( // TODO: build the query from CSS selector. - 'tag_name' => $block_type->attributes[ $block_attr ]['selector'], + 'tag_name' => $block_type->attributes[ $attribute_name ]['selector'], ) ) ) { return $block_content; } - $amended_content->set_attribute( $block_type->attributes[ $block_attr ]['attribute'], esc_attr( $source_value ) ); + $amended_content->set_attribute( $block_type->attributes[ $attribute_name ]['attribute'], esc_attr( $source_value ) ); return $amended_content->get_updated_html(); break; diff --git a/src/wp-settings.php b/src/wp-settings.php index 633ce353a1..a80c661d52 100644 --- a/src/wp-settings.php +++ b/src/wp-settings.php @@ -344,6 +344,8 @@ require ABSPATH . WPINC . '/class-wp-block-parser.php'; require ABSPATH . WPINC . '/class-wp-classic-to-block-menu-converter.php'; require ABSPATH . WPINC . '/class-wp-navigation-fallback.php'; require ABSPATH . WPINC . '/block-bindings.php'; +require ABSPATH . WPINC . '/block-bindings/pattern-overrides.php'; +require ABSPATH . WPINC . '/block-bindings/post-meta.php'; require ABSPATH . WPINC . '/blocks.php'; require ABSPATH . WPINC . '/blocks/index.php'; require ABSPATH . WPINC . '/block-editor.php'; @@ -376,8 +378,6 @@ require ABSPATH . WPINC . '/fonts/class-wp-font-face.php'; require ABSPATH . WPINC . '/fonts.php'; require ABSPATH . WPINC . '/class-wp-script-modules.php'; require ABSPATH . WPINC . '/script-modules.php'; -require ABSPATH . WPINC . '/block-bindings/sources/post-meta.php'; -require ABSPATH . WPINC . '/block-bindings/sources/pattern.php'; require ABSPATH . WPINC . '/interactivity-api.php'; wp_script_modules()->add_hooks(); diff --git a/tests/phpunit/tests/block-bindings/register.php b/tests/phpunit/tests/block-bindings/register.php index a87ae1dc56..800db70a70 100644 --- a/tests/phpunit/tests/block-bindings/register.php +++ b/tests/phpunit/tests/block-bindings/register.php @@ -16,19 +16,6 @@ class Tests_Block_Bindings_Register extends WP_UnitTestCase { 'label' => 'Test source', ); - /** - * Set up before each test. - * - * @since 6.5.0 - */ - public function set_up() { - foreach ( get_all_registered_block_bindings_sources() as $source_name => $source_properties ) { - unregister_block_bindings_source( $source_name ); - } - - parent::set_up(); - } - /** * Tear down after each test. * @@ -36,7 +23,9 @@ class Tests_Block_Bindings_Register extends WP_UnitTestCase { */ public function tear_down() { foreach ( get_all_registered_block_bindings_sources() as $source_name => $source_properties ) { - unregister_block_bindings_source( $source_name ); + if ( str_starts_with( $source_name, 'test/' ) ) { + unregister_block_bindings_source( $source_name ); + } } parent::tear_down(); @@ -49,6 +38,7 @@ class Tests_Block_Bindings_Register extends WP_UnitTestCase { * * @covers ::register_block_bindings_source * @covers ::get_all_registered_block_bindings_sources + * @covers ::get_block_bindings_source */ public function test_get_all_registered() { $source_one_name = 'test/source-one'; @@ -64,13 +54,15 @@ class Tests_Block_Bindings_Register extends WP_UnitTestCase { 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 => 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 ), + 'core/post-meta' => get_block_bindings_source( 'core/post-meta' ), + 'core/pattern-overrides' => get_block_bindings_source( 'core/pattern-overrides' ), ); $registered = get_all_registered_block_bindings_sources(); - $this->assertSame( $expected, $registered ); + $this->assertEquals( $expected, $registered ); } /** diff --git a/tests/phpunit/tests/block-bindings/block-bindings.php b/tests/phpunit/tests/block-bindings/render.php similarity index 79% rename from tests/phpunit/tests/block-bindings/block-bindings.php rename to tests/phpunit/tests/block-bindings/render.php index d8e63fa6bc..a332260731 100644 --- a/tests/phpunit/tests/block-bindings/block-bindings.php +++ b/tests/phpunit/tests/block-bindings/render.php @@ -1,6 +1,6 @@ $source_properties ) { if ( str_starts_with( $source_name, 'test/' ) ) { unregister_block_bindings_source( $source_name ); } } - parent::set_up(); + parent::tear_down(); } /** @@ -38,7 +36,7 @@ class WP_Block_Bindings_Test extends WP_UnitTestCase { * * @ticket 60282 * - * @covers register_block_bindings_source + * @covers ::register_block_bindings_source */ public function test_update_block_with_value_from_source() { $get_value_callback = function () { @@ -63,7 +61,6 @@ HTML; $expected = '

test source value

'; $result = $block->render(); - // Check if the block content was updated correctly. $this->assertEquals( $expected, $result, 'The block content should be updated with the value returned by the source.' ); } @@ -72,7 +69,7 @@ HTML; * * @ticket 60282 * - * @covers register_block_bindings_source + * @covers ::register_block_bindings_source */ public function test_passing_arguments_to_source() { $get_value_callback = function ( $source_args, $block_instance, $attribute_name ) { @@ -88,19 +85,17 @@ HTML; ) ); - $key = 'test'; - + $value = 'test'; $block_content = <<

This should not appear

+

This should not appear

HTML; $parsed_blocks = parse_blocks( $block_content ); $block = new WP_Block( $parsed_blocks[0] ); - $expected = "

The attribute name is 'content' and its binding has argument 'key' with value 'test'.

"; + $expected = "

The attribute name is 'content' and its binding has argument 'key' with value '$value'.

"; $result = $block->render(); - // Check if the block content was updated correctly. $this->assertEquals( $expected, $result, 'The block content should be updated with the value returned by the source.' ); } }