From ec9eb66f2dc75f2cacf993a603c0cd7b17edca5d Mon Sep 17 00:00:00 2001 From: Luis Herranz Date: Thu, 19 Oct 2023 12:35:11 +0200 Subject: [PATCH 01/18] Add the prefix-based unique id function --- src/wp-includes/block-supports/layout.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/wp-includes/block-supports/layout.php b/src/wp-includes/block-supports/layout.php index 4c1f4c2fe7b43..8c2234fc929c4 100644 --- a/src/wp-includes/block-supports/layout.php +++ b/src/wp-includes/block-supports/layout.php @@ -540,6 +540,26 @@ function wp_get_layout_style( $selector, $layout, $has_block_gap_support = false return ''; } +/** + * Generates an incremental ID that is independent per each different prefix. + * + * It is similar to `wp_unique_id`, but each prefix has it's own internal ID + * counter to make each prefix independent from each other. The ID starts at 1 + * and increments on each call. The returned value is not universally unique, + * but it is unique across the life of the PHP process and it's stable per + * prefix. + * + * @param string $prefix Prefix for the returned ID. + * @return string Incremental ID per prefix. + */ +function wp_incremental_id_per_prefix( $prefix = '' ) { + static $id_counters = array(); + if ( ! array_key_exists( $prefix, $id_counters ) ) { + $id_counters[ $prefix ] = 0; + } + return $prefix . (string) ++$id_counters[ $prefix ]; +} + /** * Renders the layout config to the block wrapper. * From e4f7eab6dfe68438f1bc9c26234be1492c8a195d Mon Sep 17 00:00:00 2001 From: Luis Herranz Date: Thu, 19 Oct 2023 12:35:32 +0200 Subject: [PATCH 02/18] Use it to create the class names --- src/wp-includes/block-supports/layout.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/block-supports/layout.php b/src/wp-includes/block-supports/layout.php index 8c2234fc929c4..9a7f4205ee9fa 100644 --- a/src/wp-includes/block-supports/layout.php +++ b/src/wp-includes/block-supports/layout.php @@ -650,7 +650,16 @@ function wp_render_layout_support_flag( $block_content, $block ) { $class_names = array(); $layout_definitions = wp_get_layout_definitions(); - $container_class = wp_unique_id( 'wp-container-' ); + + /* + * We use an incremental ID that is independent per prefix to make sure that + * rendering different numbers of blocks doesn't affect the IDs of other + * blocks. We need this to make the CSS class names stable across paginations + * for features like the enhanced pagination of the Query block. + */ + $container_class = wp_incremental_id_per_prefix( + 'wp-container-' . sanitize_title( $block['blockName'] ) . '-layout-' + ); // Set the correct layout type for blocks using legacy content width. if ( isset( $used_layout['inherit'] ) && $used_layout['inherit'] || isset( $used_layout['contentSize'] ) && $used_layout['contentSize'] ) { From 5468e1744be94a9c2b96e37b3c28f8e7aa784690 Mon Sep 17 00:00:00 2001 From: tellthemachines Date: Fri, 20 Oct 2023 11:02:32 +1100 Subject: [PATCH 03/18] Fix failing render test --- tests/phpunit/tests/blocks/render.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/blocks/render.php b/tests/phpunit/tests/blocks/render.php index 632298186eaa0..1fd8d518f4709 100644 --- a/tests/phpunit/tests/blocks/render.php +++ b/tests/phpunit/tests/blocks/render.php @@ -218,9 +218,9 @@ public function test_do_block_output( $html_filename, $server_html_filename ) { $html = do_blocks( self::strip_r( file_get_contents( $html_path ) ) ); // If blocks opt into Gutenberg's layout implementation - // the container will receive an added classname of `wp_unique_id( 'wp-container-' )` + // the container will receive an added classname of `wp_unique_id( 'wp-container-[blockname]-layout' )` // so we need to normalize the random id. - $normalized_html = preg_replace( '/wp-container-\d+/', 'wp-container-1', $html ); + $normalized_html = preg_replace( '/wp-container-[a-z-]+\d+/', 'wp-container-1', $html ); // The gallery block uses a unique class name of `wp_unique_id( 'wp-block-gallery-' )` // so we need to normalize the random id. From 2d3db4418b82b8860fd10109cdb9fefaa4b2be09 Mon Sep 17 00:00:00 2001 From: Carlos Bravo Date: Fri, 20 Oct 2023 17:29:53 +0200 Subject: [PATCH 04/18] Add changes requested --- src/wp-includes/block-supports/layout.php | 30 ++++--------------- src/wp-includes/functions.php | 22 ++++++++++++++ tests/phpunit/tests/blocks/render.php | 2 +- .../tests/functions/wpUniquePrefixedId.php | 26 ++++++++++++++++ 4 files changed, 54 insertions(+), 26 deletions(-) create mode 100644 tests/phpunit/tests/functions/wpUniquePrefixedId.php diff --git a/src/wp-includes/block-supports/layout.php b/src/wp-includes/block-supports/layout.php index 9a7f4205ee9fa..6c591ff874b8c 100644 --- a/src/wp-includes/block-supports/layout.php +++ b/src/wp-includes/block-supports/layout.php @@ -540,26 +540,6 @@ function wp_get_layout_style( $selector, $layout, $has_block_gap_support = false return ''; } -/** - * Generates an incremental ID that is independent per each different prefix. - * - * It is similar to `wp_unique_id`, but each prefix has it's own internal ID - * counter to make each prefix independent from each other. The ID starts at 1 - * and increments on each call. The returned value is not universally unique, - * but it is unique across the life of the PHP process and it's stable per - * prefix. - * - * @param string $prefix Prefix for the returned ID. - * @return string Incremental ID per prefix. - */ -function wp_incremental_id_per_prefix( $prefix = '' ) { - static $id_counters = array(); - if ( ! array_key_exists( $prefix, $id_counters ) ) { - $id_counters[ $prefix ] = 0; - } - return $prefix . (string) ++$id_counters[ $prefix ]; -} - /** * Renders the layout config to the block wrapper. * @@ -652,12 +632,12 @@ function wp_render_layout_support_flag( $block_content, $block ) { $layout_definitions = wp_get_layout_definitions(); /* - * We use an incremental ID that is independent per prefix to make sure that - * rendering different numbers of blocks doesn't affect the IDs of other - * blocks. We need this to make the CSS class names stable across paginations - * for features like the enhanced pagination of the Query block. + * We use an incremental ID that is independent per prefix to make sure that + * rendering different numbers of blocks doesn't affect the IDs of other + * blocks. We need this to make the CSS class names stable across paginations + * for features like the enhanced pagination of the Query block. */ - $container_class = wp_incremental_id_per_prefix( + $container_class = wp_unique_prefixed_id( 'wp-container-' . sanitize_title( $block['blockName'] ) . '-layout-' ); diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index c467961c98f9e..09821d1df4e37 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7830,6 +7830,28 @@ function wp_unique_id( $prefix = '' ) { return $prefix . (string) ++$id_counter; } +/** + * Generates an incremental ID that is independent per each different prefix. + * + * It is similar to `wp_unique_id`, but each prefix has its own internal ID + * counter to make each prefix independent from each other. The ID starts at 1 + * and increments on each call. The returned value is not universally unique, + * but it is unique across the life of the PHP process and it's stable per + * prefix. + * + * @since 6.4 + * + * @param string $prefix Optional. Prefix for the returned ID. Default empty string. + * @return string Incremental ID per prefix. + */ +function wp_unique_prefixed_id( $prefix = '' ) { + static $id_counters = array(); + if ( ! array_key_exists( $prefix, $id_counters ) ) { + $id_counters[ $prefix ] = 0; + } + return $prefix . (string) ++$id_counters[ $prefix ]; +} + /** * Gets last changed date for the specified cache group. * diff --git a/tests/phpunit/tests/blocks/render.php b/tests/phpunit/tests/blocks/render.php index 1fd8d518f4709..cb06030282604 100644 --- a/tests/phpunit/tests/blocks/render.php +++ b/tests/phpunit/tests/blocks/render.php @@ -218,7 +218,7 @@ public function test_do_block_output( $html_filename, $server_html_filename ) { $html = do_blocks( self::strip_r( file_get_contents( $html_path ) ) ); // If blocks opt into Gutenberg's layout implementation - // the container will receive an added classname of `wp_unique_id( 'wp-container-[blockname]-layout' )` + // the container will receive an additional, unique classname based on "wp-container-[blockname]-layout" // so we need to normalize the random id. $normalized_html = preg_replace( '/wp-container-[a-z-]+\d+/', 'wp-container-1', $html ); diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php new file mode 100644 index 0000000000000..e572f4faea979 --- /dev/null +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -0,0 +1,26 @@ +assertNotEquals( $first, $second ); + $this->assertNotEquals( $default, $second_default ); + $this->assertNotEquals( $null_id, $second_null_id ); + } +} From b6138d4b2719c523a4221e4a07ced94c95d7eab5 Mon Sep 17 00:00:00 2001 From: Carlos Bravo Date: Mon, 23 Oct 2023 10:01:07 +0200 Subject: [PATCH 05/18] Update tests, fix spacing in comment --- src/wp-includes/block-supports/layout.php | 2 +- .../tests/functions/wpUniquePrefixedId.php | 68 ++++++++++++++++--- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/wp-includes/block-supports/layout.php b/src/wp-includes/block-supports/layout.php index 6c591ff874b8c..3719aaec8dcec 100644 --- a/src/wp-includes/block-supports/layout.php +++ b/src/wp-includes/block-supports/layout.php @@ -636,7 +636,7 @@ function wp_render_layout_support_flag( $block_content, $block ) { * rendering different numbers of blocks doesn't affect the IDs of other * blocks. We need this to make the CSS class names stable across paginations * for features like the enhanced pagination of the Query block. - */ + */ $container_class = wp_unique_prefixed_id( 'wp-container-' . sanitize_title( $block['blockName'] ) . '-layout-' ); diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index e572f4faea979..66d6f9c5b37ca 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -12,15 +12,63 @@ */ class Tests_Functions_WpUniquePrefixedId extends WP_UnitTestCase { - public function test_wp_unique_prefixed_id() { - $first = wp_unique_prefixed_id( 'test' ); - $second = wp_unique_prefixed_id( 'test' ); - $null_id = wp_unique_prefixed_id( null ); - $second_null_id = wp_unique_prefixed_id( null ); - $default = wp_unique_prefixed_id(); - $second_default = wp_unique_prefixed_id(); - $this->assertNotEquals( $first, $second ); - $this->assertNotEquals( $default, $second_default ); - $this->assertNotEquals( $null_id, $second_null_id ); + /** + * Tests that the expected unique prefixed IDs are created. + * + * @ticket 59681 + * + * @dataProvider data_should_create_unique_prefixed_ids + * + * @param mixed $prefix The prefix. + * @param array $expected The next two expected IDs. + */ + public function test_should_create_unique_prefixed_ids( $prefix, $expected ) { + $id1 = wp_unique_prefixed_id( $prefix ); + $id2 = wp_unique_prefixed_id( $prefix ); + + $this->assertNotSame( $id1, $id2, 'The IDs are not unique.' ); + $this->assertSame( $expected, array( $id1, $id2 ), 'The IDs did not match the expected values.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_create_unique_prefixed_ids() { + return array( + 'prefix as null' => array( + 'prefix' => null, + 'expected' => array( '1', '2' ), + ), + 'prefix as empty string' => array( + 'prefix' => '', + 'expected' => array( '3', '4' ), + ), + 'prefix as (string) "0"' => array( + 'prefix' => '0', + 'expected' => array( '01', '02' ), + ), + 'prefix as (int) 0' => array( + 'prefix' => 0, + 'expected' => array( '03', '04' ), + ), + 'prefix as string' => array( + 'prefix' => 'test', + 'expected' => array( 'test1', 'test2' ), + ), + 'prefix as string with spaces' => array( + 'prefix' => ' ', + 'expected' => array( ' 1', ' 2' ), + ), + 'prefix as (string) "1"' => array( + 'prefix' => '1', + 'expected' => array( '11', '12' ), + ), + 'prefix as (int) 1' => array( + 'prefix' => 1, + 'expected' => array( '13', '14' ), + ), + ); } } From 89b3ba9bd7be284fc8d86817f1113bbbd77489fe Mon Sep 17 00:00:00 2001 From: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:13:24 +0200 Subject: [PATCH 06/18] Apply suggestions from code review Co-authored-by: Tonya Mork --- src/wp-includes/block-supports/layout.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/block-supports/layout.php b/src/wp-includes/block-supports/layout.php index 3719aaec8dcec..0e22dded74e44 100644 --- a/src/wp-includes/block-supports/layout.php +++ b/src/wp-includes/block-supports/layout.php @@ -632,9 +632,9 @@ function wp_render_layout_support_flag( $block_content, $block ) { $layout_definitions = wp_get_layout_definitions(); /* - * We use an incremental ID that is independent per prefix to make sure that + * Uses an incremental ID that is independent per prefix to make sure that * rendering different numbers of blocks doesn't affect the IDs of other - * blocks. We need this to make the CSS class names stable across paginations + * blocks. Makes the CSS class names stable across paginations * for features like the enhanced pagination of the Query block. */ $container_class = wp_unique_prefixed_id( From ca9731acb7b3ede05aa17e37a5f1485f32108dab Mon Sep 17 00:00:00 2001 From: Tonya Mork Date: Mon, 23 Oct 2023 14:32:45 -0500 Subject: [PATCH 07/18] Use isset() instead of array_key_exists() --- src/wp-includes/functions.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 09821d1df4e37..98cd250f63b8d 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7846,9 +7846,10 @@ function wp_unique_id( $prefix = '' ) { */ function wp_unique_prefixed_id( $prefix = '' ) { static $id_counters = array(); - if ( ! array_key_exists( $prefix, $id_counters ) ) { + if ( ! isset( $id_counters[ $prefix ] ) ) { $id_counters[ $prefix ] = 0; } + return $prefix . (string) ++$id_counters[ $prefix ]; } From 42035dea6f875148d08094e2063d9515f17a61ff Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 17:19:23 -0500 Subject: [PATCH 08/18] Defensive guard for nonstring --- src/wp-includes/functions.php | 9 +++ .../tests/functions/wpUniquePrefixedId.php | 60 +++++++++++++++---- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 98cd250f63b8d..e52bef749b59f 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7846,6 +7846,15 @@ function wp_unique_id( $prefix = '' ) { */ function wp_unique_prefixed_id( $prefix = '' ) { static $id_counters = array(); + + if ( ! is_string( $prefix ) ) { + wp_trigger_error( + __FUNCTION__, + sprintf( 'The prefix must be a string. "%s" data type given.', gettype( $prefix ) ) + ); + $prefix = ''; + } + if ( ! isset( $id_counters[ $prefix ] ) ) { $id_counters[ $prefix ] = 0; } diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 66d6f9c5b37ca..eae449d0bf490 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -37,22 +37,14 @@ public function test_should_create_unique_prefixed_ids( $prefix, $expected ) { */ public function data_should_create_unique_prefixed_ids() { return array( - 'prefix as null' => array( - 'prefix' => null, - 'expected' => array( '1', '2' ), - ), 'prefix as empty string' => array( 'prefix' => '', - 'expected' => array( '3', '4' ), + 'expected' => array( '1', '2' ), ), 'prefix as (string) "0"' => array( 'prefix' => '0', 'expected' => array( '01', '02' ), ), - 'prefix as (int) 0' => array( - 'prefix' => 0, - 'expected' => array( '03', '04' ), - ), 'prefix as string' => array( 'prefix' => 'test', 'expected' => array( 'test1', 'test2' ), @@ -65,9 +57,53 @@ public function data_should_create_unique_prefixed_ids() { 'prefix' => '1', 'expected' => array( '11', '12' ), ), - 'prefix as (int) 1' => array( - 'prefix' => 1, - 'expected' => array( '13', '14' ), + ); + } + + /** + * @ticket 59681 + * + * @dataProvider data_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given + * + * @param mixed $non_string_prefix Non-string prefix. + * @param string $expected_message Expected notice message. + * @param string $expected_value Expected unique ID. + */ + public function test_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given( $non_string_prefix, $expected_message, $expected_value ) { + $this->expectNotice(); + $this->expectNoticeMessage( $expected_message ); + + $actual = wp_unique_prefixed_id( $non_string_prefix ); + $this->assertSame( $expected_value, $actual ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given() { + $message = 'wp_unique_prefixed_id(): The prefix must be a string. "%s" data type given.'; + return array( + 'prefix as null' => array( + 'non_string_prefix' => null, + 'expected_message' => sprintf( $message, 'NULL' ), + 'expected_value' => '3', + ), + 'prefix as (int) 0' => array( + 'non_string_prefix' => 0, + 'expected_message' => sprintf( $message, 'integer' ), + 'expected_value' => '4', + ), + 'prefix as (int) 1' => array( + 'non_string_prefix' => 1, + 'expected_data_type' => sprintf( $message, 'integer' ), + 'expected_value' => '5', + ), + 'prefix as (bool) false' => array( + 'non_string_prefix' => false, + 'expected_data_type' => sprintf( $message, 'boolean' ), + 'expected_value' => '6', ), ); } From 780486b5cbdd96a3e9dc964de64b775ca3eff451 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 17:20:08 -0500 Subject: [PATCH 09/18] Use full 6.4.0 version --- src/wp-includes/functions.php | 2 +- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index e52bef749b59f..8ae4caa653f8a 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7839,7 +7839,7 @@ function wp_unique_id( $prefix = '' ) { * but it is unique across the life of the PHP process and it's stable per * prefix. * - * @since 6.4 + * @since 6.4.0 * * @param string $prefix Optional. Prefix for the returned ID. Default empty string. * @return string Incremental ID per prefix. diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index eae449d0bf490..6b600e22ea45a 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -5,7 +5,7 @@ * * @package WordPress\UnitTests * - * @since 6.4 + * @since 6.4.0 * * @group functions.php * @covers ::wp_unique_prefixed_id From 11f1cace5cb60369011f9e26e7ada5e412c70b57 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 17:25:08 -0500 Subject: [PATCH 10/18] Increment separately and then return --- src/wp-includes/functions.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 8ae4caa653f8a..473f4050b497d 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7859,7 +7859,9 @@ function wp_unique_prefixed_id( $prefix = '' ) { $id_counters[ $prefix ] = 0; } - return $prefix . (string) ++$id_counters[ $prefix ]; + $id = ++$id_counters[ $prefix ]; + + return $prefix . (string) $id; } /** From c01e7e8a4f412d613886359d74429dc65e0e07d8 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 17:28:05 -0500 Subject: [PATCH 11/18] Run tests in separate processes. The wp_unique_prefixed_id() function uses a function scoped static variable for memoization. The incremented value may be different between tests, especially over time when more tests are added. Running the tests in separate process improves the stability of these tests. --- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 6b600e22ea45a..82eeb1cbc8344 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -9,6 +9,8 @@ * * @group functions.php * @covers ::wp_unique_prefixed_id + * @runTestsInSeparateProcesses + * @preserveGlobalState disabled */ class Tests_Functions_WpUniquePrefixedId extends WP_UnitTestCase { From 1c8ab55a8d9f2338e0e15f665eec5faa77f4540d Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 17:48:57 -0500 Subject: [PATCH 12/18] Fix wpcs issues --- src/wp-includes/functions.php | 4 ++-- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wp-includes/functions.php b/src/wp-includes/functions.php index 473f4050b497d..cb490ee1764fa 100644 --- a/src/wp-includes/functions.php +++ b/src/wp-includes/functions.php @@ -7849,8 +7849,8 @@ function wp_unique_prefixed_id( $prefix = '' ) { if ( ! is_string( $prefix ) ) { wp_trigger_error( - __FUNCTION__, - sprintf( 'The prefix must be a string. "%s" data type given.', gettype( $prefix ) ) + __FUNCTION__, + sprintf( 'The prefix must be a string. "%s" data type given.', gettype( $prefix ) ) ); $prefix = ''; } diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 82eeb1cbc8344..7aa9f30af997e 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -85,7 +85,7 @@ public function test_should_raise_notice_and_use_empty_string_prefix_when_nonstr * @return array[] */ public function data_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given() { - $message = 'wp_unique_prefixed_id(): The prefix must be a string. "%s" data type given.'; + $message = 'wp_unique_prefixed_id(): The prefix must be a string. "%s" data type given.'; return array( 'prefix as null' => array( 'non_string_prefix' => null, From 7a32e2600d86a513068a858e4289bc1d3a2ce04b Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 18:04:16 -0500 Subject: [PATCH 13/18] Add additional happy path datasets --- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 7aa9f30af997e..3341c571055f9 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -59,6 +59,14 @@ public function data_should_create_unique_prefixed_ids() { 'prefix' => '1', 'expected' => array( '11', '12' ), ), + 'prefix as a (string) "."' => array( + 'prefix' => '.', + 'expected' => array( '.1', '.2' ), + ), + 'prefix as a block name' => array( + 'prefix' => 'core/list-item', + 'expected' => array( 'core/list-item1', 'core/list-item2' ), + ), ); } From 5aac2049e9d55dd21d0c589b11108cc839fde67c Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 19:22:39 -0500 Subject: [PATCH 14/18] Tests: run each dataset separately. To avoid test order, changes to run each dataset in a separate process, which each test starts with the static `$id_counters` equal to an empty array. --- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 3341c571055f9..aef4e2b66439e 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -9,8 +9,6 @@ * * @group functions.php * @covers ::wp_unique_prefixed_id - * @runTestsInSeparateProcesses - * @preserveGlobalState disabled */ class Tests_Functions_WpUniquePrefixedId extends WP_UnitTestCase { @@ -21,6 +19,9 @@ class Tests_Functions_WpUniquePrefixedId extends WP_UnitTestCase { * * @dataProvider data_should_create_unique_prefixed_ids * + * @runInSeparateProcess + * @preserveGlobalState disabled + * * @param mixed $prefix The prefix. * @param array $expected The next two expected IDs. */ @@ -75,6 +76,9 @@ public function data_should_create_unique_prefixed_ids() { * * @dataProvider data_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given * + * @runInSeparateProcess + * @preserveGlobalState disabled + * * @param mixed $non_string_prefix Non-string prefix. * @param string $expected_message Expected notice message. * @param string $expected_value Expected unique ID. From f4f99fc242a565eda8548a678cdd8423faf13e65 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 19:28:01 -0500 Subject: [PATCH 15/18] Tests: Ensure unhappy path IDs are unique. Unhappy path tests have non-string prefixes, which will be changed to the default empty string. To further check uniqueness within the dataset's expected results, the unhappy path test now allows each dataset to specify how many unique IDs to generate for its tests. --- .../tests/functions/wpUniquePrefixedId.php | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index aef4e2b66439e..7e70e3e6d42a8 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -79,16 +79,23 @@ public function data_should_create_unique_prefixed_ids() { * @runInSeparateProcess * @preserveGlobalState disabled * - * @param mixed $non_string_prefix Non-string prefix. - * @param string $expected_message Expected notice message. - * @param string $expected_value Expected unique ID. + * @param mixed $non_string_prefix Non-string prefix. + * @param int $number_of_ids_to_generate Number of IDs to generate. + * As the prefix will default to an empty string, changing the number of IDs generated within each dataset further tests ID uniqueness. + * @param string $expected_message Expected notice message. + * @param array $expected_ids Expected unique IDs. */ - public function test_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given( $non_string_prefix, $expected_message, $expected_value ) { + public function test_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given( $non_string_prefix, $number_of_ids_to_generate, $expected_message, $expected_ids ) { $this->expectNotice(); $this->expectNoticeMessage( $expected_message ); - $actual = wp_unique_prefixed_id( $non_string_prefix ); - $this->assertSame( $expected_value, $actual ); + $ids = array(); + for ( $i = 0; $i < $number_of_ids_to_generate; $i++ ) { + $ids[] = wp_unique_prefixed_id( $non_string_prefix ); + } + + $this->assertSameSets( $ids, array_unique( $ids ), 'IDs are not unique.' ); + $this->assertSameSets( $expected_ids, $ids, 'The IDs did not match the expected values.' ); } /** @@ -100,24 +107,28 @@ public function data_should_raise_notice_and_use_empty_string_prefix_when_nonstr $message = 'wp_unique_prefixed_id(): The prefix must be a string. "%s" data type given.'; return array( 'prefix as null' => array( - 'non_string_prefix' => null, - 'expected_message' => sprintf( $message, 'NULL' ), - 'expected_value' => '3', + 'non_string_prefix' => null, + 'number_of_ids_to_generate' => 2, + 'expected_message' => sprintf( $message, 'NULL' ), + 'expected_ids' => array( '1', '2' ), ), 'prefix as (int) 0' => array( - 'non_string_prefix' => 0, - 'expected_message' => sprintf( $message, 'integer' ), - 'expected_value' => '4', + 'non_string_prefix' => 0, + 'number_of_ids_to_generate' => 3, + 'expected_message' => sprintf( $message, 'integer' ), + 'expected_ids' => array( '1', '2', '3' ), ), 'prefix as (int) 1' => array( - 'non_string_prefix' => 1, - 'expected_data_type' => sprintf( $message, 'integer' ), - 'expected_value' => '5', + 'non_string_prefix' => 1, + 'number_of_ids_to_generate' => 4, + 'expected_data_type' => sprintf( $message, 'integer' ), + 'expected_ids' => array( '1', '2', '3', '4' ), ), 'prefix as (bool) false' => array( - 'non_string_prefix' => false, - 'expected_data_type' => sprintf( $message, 'boolean' ), - 'expected_value' => '6', + 'non_string_prefix' => false, + 'number_of_ids_to_generate' => 5, + 'expected_data_type' => sprintf( $message, 'boolean' ), + 'expected_ids' => array( '1', '2', '3', '4', '5' ), ), ); } From 28cbcad9f9ceca4af4e6d784fa9b48d943a2b44f Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 19:36:17 -0500 Subject: [PATCH 16/18] Test: Add double dataset --- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 7e70e3e6d42a8..66c7b47068c62 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -130,6 +130,12 @@ public function data_should_raise_notice_and_use_empty_string_prefix_when_nonstr 'expected_data_type' => sprintf( $message, 'boolean' ), 'expected_ids' => array( '1', '2', '3', '4', '5' ), ), + 'prefix as (double) 98.7' => array( + 'non_string_prefix' => 98.7, + 'number_of_ids_to_generate' => 6, + 'expected_data_type' => sprintf( $message, 'double' ), + 'expected_ids' => array( '1', '2', '3', '4', '5', '6' ), + ), ); } } From 92d93926049c3772d3138b11d842854d3a3747bd Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 20:02:55 -0500 Subject: [PATCH 17/18] Fix test dataset array => alignment --- tests/phpunit/tests/functions/wpUniquePrefixedId.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 66c7b47068c62..5ed0b51d1f78e 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -106,25 +106,25 @@ public function test_should_raise_notice_and_use_empty_string_prefix_when_nonstr public function data_should_raise_notice_and_use_empty_string_prefix_when_nonstring_given() { $message = 'wp_unique_prefixed_id(): The prefix must be a string. "%s" data type given.'; return array( - 'prefix as null' => array( + 'prefix as null' => array( 'non_string_prefix' => null, 'number_of_ids_to_generate' => 2, 'expected_message' => sprintf( $message, 'NULL' ), 'expected_ids' => array( '1', '2' ), ), - 'prefix as (int) 0' => array( + 'prefix as (int) 0' => array( 'non_string_prefix' => 0, 'number_of_ids_to_generate' => 3, 'expected_message' => sprintf( $message, 'integer' ), 'expected_ids' => array( '1', '2', '3' ), ), - 'prefix as (int) 1' => array( + 'prefix as (int) 1' => array( 'non_string_prefix' => 1, 'number_of_ids_to_generate' => 4, 'expected_data_type' => sprintf( $message, 'integer' ), 'expected_ids' => array( '1', '2', '3', '4' ), ), - 'prefix as (bool) false' => array( + 'prefix as (bool) false' => array( 'non_string_prefix' => false, 'number_of_ids_to_generate' => 5, 'expected_data_type' => sprintf( $message, 'boolean' ), From 51514403078a9affd2ee03950a9814b356a8dc5f Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Mon, 23 Oct 2023 20:55:38 -0500 Subject: [PATCH 18/18] Tests: Adds coverage for when given prefixes result in same prefix. THis test guards against a future regression should the defensive guard ever change. Props @costdev. --- .../tests/functions/wpUniquePrefixedId.php | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/phpunit/tests/functions/wpUniquePrefixedId.php b/tests/phpunit/tests/functions/wpUniquePrefixedId.php index 5ed0b51d1f78e..64a6a955a14ac 100644 --- a/tests/phpunit/tests/functions/wpUniquePrefixedId.php +++ b/tests/phpunit/tests/functions/wpUniquePrefixedId.php @@ -138,4 +138,59 @@ public function data_should_raise_notice_and_use_empty_string_prefix_when_nonstr ), ); } + + /** + * Prefixes that are or will become the same should generate unique IDs. + * + * This test is added to avoid future regressions if the function's prefix data type check is + * modified to type juggle or check for scalar data types. + * + * @ticket 59681 + * + * @dataProvider data_same_prefixes_should_generate_unique_ids + * + * @runInSeparateProcess + * @preserveGlobalState disabled + * + * @param array $prefixes The prefixes to check. + * @param array $expected The expected unique IDs. + */ + public function test_same_prefixes_should_generate_unique_ids( array $prefixes, array $expected ) { + // Suppress E_USER_NOTICE, which will be raised when a prefix is non-string. + $original_error_reporting = error_reporting(); + error_reporting( $original_error_reporting & ~E_USER_NOTICE ); + + $ids = array(); + foreach ( $prefixes as $prefix ) { + $ids[] = wp_unique_prefixed_id( $prefix ); + } + + // Reset error reporting. + error_reporting( $original_error_reporting ); + + $this->assertSameSets( $ids, array_unique( $ids ), 'IDs are not unique.' ); + $this->assertSameSets( $expected, $ids, 'The IDs did not match the expected values.' ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_same_prefixes_should_generate_unique_ids() { + return array( + 'prefixes = empty string' => array( + 'prefixes' => array( null, true, '' ), + 'expected' => array( '1', '2', '3' ), + ), + 'prefixes = 0' => array( + 'prefixes' => array( '0', 0, 0.0, false ), + 'expected' => array( '01', '1', '2', '3' ), + ), + 'prefixes = 1' => array( + 'prefixes' => array( '1', 1, 1.0, true ), + 'expected' => array( '11', '1', '2', '3' ), + ), + ); + } }