From a5e4431c289dd0b41c5eb5bf25ee21a503166ab1 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Fri, 20 Dec 2024 11:18:53 -0500 Subject: [PATCH 01/20] Creating tests for Tribe__PUE__Notices class. --- tests/integration/Tribe/PUE/Notices_Test.php | 211 +++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 tests/integration/Tribe/PUE/Notices_Test.php diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php new file mode 100644 index 000000000..f75e9288d --- /dev/null +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -0,0 +1,211 @@ +clear_all_notices(); + $notices->save_notices(); + } + + /** + * Runs after each test. + * + * @after + */ + public function cleanup_notices(): void { + // Use the class method to clear all notices + $notices = tribe( 'pue.notices' ); + $notices->clear_all_notices(); + $notices->save_notices(); + } + + /** + * Data provider for test_notice_with_all_statuses. + * + * @return Generator + */ + public function notice_data_provider(): Generator { + $statuses = [ + Tribe__PUE__Notices::INVALID_KEY, + Tribe__PUE__Notices::UPGRADE_KEY, + Tribe__PUE__Notices::EXPIRED_KEY, + ]; + + // Individual statuses with multiple plugins + foreach ( $statuses as $status ) { + yield "Multiple plugins, single status ($status)" => [ + function () use ( $status ) { + $pue_notices = tribe( 'pue.notices' ); + $expected_options = [ $status => [] ]; + + foreach ( range( 1, 5 ) as $index ) { + $plugin_name = "{$status}-plugin-{$index}"; + $pue_notices->add_notice( $status, $plugin_name ); + $expected_options[ $status ][ $plugin_name ] = true; + } + + return [ + 'expected_options' => $expected_options, + 'status' => $status, + 'plugin_names' => array_keys( $expected_options[ $status ] ), + ]; + }, + "Multiple plugins should be added with status $status.", + ]; + } + + // Multiple plugins with multiple statuses + yield 'Multiple plugins, multiple statuses' => [ + function () use ( $statuses ) { + $expected_options = []; + $plugin_names = []; + + $pue_notices = tribe( 'pue.notices' ); + foreach ( $statuses as $status ) { + $expected_options[ $status ] = []; + + foreach ( range( 1, 3 ) as $index ) { + $plugin_name = "{$status}-plugin-{$index}"; + $pue_notices->add_notice( $status, $plugin_name ); + $expected_options[ $status ][ $plugin_name ] = true; + $plugin_names[] = $plugin_name; + } + } + + return [ + 'expected_options' => $expected_options, + 'status' => $statuses, + 'plugin_names' => $plugin_names, + ]; + }, + 'Multiple plugins should be added with multiple statuses.', + ]; + } + + /** + * Test using the data provider for all status scenarios. + * + * @dataProvider notice_data_provider + */ + public function test_notice_with_all_statuses( + Closure $setup_closure, + string $scenario + ): void { + // Execute the setup closure to prepare the test case + $data = $setup_closure(); + + // Retrieve the options after setup + $options = get_option( 'tribe_pue_key_notices' ); + + // Iterate through each status in the expected options + foreach ( $data['expected_options'] as $status => $expected_plugins ) { + $this->assertArrayHasKey( + $status, + $options, + "The status $status should exist in the options array." + ); + + $actual_plugins = $options[ $status ]; + + foreach ( $expected_plugins as $plugin_name => $expected_value ) { + $this->assertArrayHasKey( + $plugin_name, + $actual_plugins, + "The plugin {$plugin_name} should exist under $status." + ); + + $this->assertSame( + $expected_value, + $actual_plugins[ $plugin_name ], + "The plugin {$plugin_name} should have the value {$expected_value} under $status." + ); + } + + // Ensure the count matches + $this->assertCount( + count( $expected_plugins ), + $actual_plugins, + "The number of plugins under $status should match the expected count." + ); + } + + // Ensure the final options match the expected output + $this->assertEquals( + $data['expected_options'], + $options, + $scenario + ); + } + + public function test_merge_recursive_bug_with_same_plugin(): void { + // Plugin name to test + $plugin_name = 'plugin-merge-test'; + + // Pre-set the option to simulate existing saved notices + $initial_saved_notices = [ + Tribe__PUE__Notices::INVALID_KEY => [ + $plugin_name => [true], + ], + ]; + update_option('tribe_pue_key_notices', $initial_saved_notices); + + for ($i = 0; $i < 5; $i++) { + // Recreate the tribe instance to simulate typical usage + $pue_notices = tribe('pue.notices'); + + // Add the same notice repeatedly + $pue_notices->add_notice(Tribe__PUE__Notices::INVALID_KEY, $plugin_name); + + // Save notices to trigger merging in the next call + $pue_notices->save_notices(); + } + + // Retrieve the final notices from the database + $options = get_option('tribe_pue_key_notices'); + + codecept_debug($options); + return; + + // Assertions to check if the plugin is duplicated + $this->assertArrayHasKey( + Tribe__PUE__Notices::INVALID_KEY, + $options, + 'The "invalid_key" key should exist in the options array.' + ); + + $invalid_key_plugins = $options[Tribe__PUE__Notices::INVALID_KEY]; + + $this->assertArrayHasKey( + $plugin_name, + $invalid_key_plugins, + "The plugin {$plugin_name} should exist under 'invalid_key'." + ); + + // Ensure the value is not duplicated (array_merge_recursive bug can cause this) + $this->assertIsBool( + $invalid_key_plugins[$plugin_name], + "The plugin {$plugin_name} should not be duplicated or stored as an array." + ); + + $this->assertTrue( + $invalid_key_plugins[$plugin_name], + "The plugin {$plugin_name} should be set to true in 'invalid_key'." + ); + } + + +} + From 04a3be4d7e2f985b7dbcaceecb8af3b5b14ea1d2 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Fri, 20 Dec 2024 11:19:19 -0500 Subject: [PATCH 02/20] Implemented new method `clear_all_notices` for testing. Working on fixing duplication logic. --- src/Tribe/PUE/Notices.php | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 8d8eb00ca..ec35228c2 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -1,5 +1,4 @@ notices = array_merge_recursive( $this->notices, $this->saved_notices ); - // Cleanup + //$this->notices = array_merge($this->notices, $this->saved_notices ); + + // Cleanup. foreach ( $this->notices as $key => &$plugin_lists ) { - // Purge any elements that are not arrays + // Purge any elements that are not arrays. if ( ! is_array( $plugin_lists ) ) { unset( $this->notices[ $key ] ); continue; } + $plugin_lists = array_unique( $plugin_lists ); + foreach ( $plugin_lists as $plugin => $data ) { + $this->notices[ $key ][ $plugin ] = is_array( $data ) ? array_unique( $data ) : $data; + } } } @@ -566,4 +576,20 @@ protected function get_formatted_plugin_names( $group ) { return '' . $html . ''; } + + /** + * Clears all stored and saved notices. + * + * This method resets both the `notices` and `saved_notices` properties to empty arrays, + * effectively removing all license key notifications from memory. + * + * Note: This does not persist changes to the database. To save changes, + * ensure `save_notices()` is called after invoking this method. + * + * @return void + */ + public function clear_all_notices() { + $this->notices = []; + $this->saved_notices = []; + } } From 7f0bc1652dbc10bfa4b27fc00d7e93d12744b0d5 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Fri, 20 Dec 2024 12:04:11 -0500 Subject: [PATCH 03/20] Created new tests, added fix to `Notices.php`. --- src/Tribe/PUE/Notices.php | 8 +- tests/integration/Tribe/PUE/Notices_Test.php | 184 +++++++++++++++---- 2 files changed, 152 insertions(+), 40 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index ec35228c2..e5806b3f5 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -94,14 +94,8 @@ protected function populate() { return; } - /** - * @todo - redscar - * Potentially change with - - * wp_parse_args or array_merge - */ - $this->notices = array_merge_recursive( $this->notices, $this->saved_notices ); - //$this->notices = array_merge($this->notices, $this->saved_notices ); + $this->notices = wp_parse_args( $this->notices, $this->saved_notices ); // Cleanup. foreach ( $this->notices as $key => &$plugin_lists ) { diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php index f75e9288d..ded251c35 100644 --- a/tests/integration/Tribe/PUE/Notices_Test.php +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -108,7 +108,7 @@ public function test_notice_with_all_statuses( $data = $setup_closure(); // Retrieve the options after setup - $options = get_option( 'tribe_pue_key_notices' ); + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); // Iterate through each status in the expected options foreach ( $data['expected_options'] as $status => $expected_plugins ) { @@ -150,62 +150,180 @@ public function test_notice_with_all_statuses( ); } - public function test_merge_recursive_bug_with_same_plugin(): void { - // Plugin name to test + /** + * @test + */ + public function recursive_bug_with_same_plugin(): void { $plugin_name = 'plugin-merge-test'; - // Pre-set the option to simulate existing saved notices - $initial_saved_notices = [ - Tribe__PUE__Notices::INVALID_KEY => [ - $plugin_name => [true], - ], - ]; - update_option('tribe_pue_key_notices', $initial_saved_notices); + $pue_notices_initial = tribe( 'pue.notices' ); - for ($i = 0; $i < 5; $i++) { - // Recreate the tribe instance to simulate typical usage - $pue_notices = tribe('pue.notices'); + // Add initial notices to different keys + $pue_notices_initial->add_notice( Tribe__PUE__Notices::UPGRADE_KEY, 'initial_plugin1' ); + $pue_notices_initial->add_notice( Tribe__PUE__Notices::EXPIRED_KEY, 'initial_plugin2' ); - // Add the same notice repeatedly - $pue_notices->add_notice(Tribe__PUE__Notices::INVALID_KEY, $plugin_name); + // Simulate repeated usage of the same plugin name with different instances + for ( $j = 0; $j < 5; $j++ ) { + for ( $i = 0; $i < 5; $i++ ) { + $temp_plugin_name = $plugin_name . $j; + tribe( 'pue.notices' )->register_name( $temp_plugin_name ); + + // Recreate the tribe instance to simulate typical usage + $pue_notices = tribe( 'pue.notices' ); - // Save notices to trigger merging in the next call - $pue_notices->save_notices(); + // Add the same notice repeatedly + $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, $temp_plugin_name ); + + unset( $pue_notices ); // Clear instance to simulate separate requests + } } + $pue_notices_initial->save_notices(); + // Retrieve the final notices from the database - $options = get_option('tribe_pue_key_notices'); + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); - codecept_debug($options); - return; - // Assertions to check if the plugin is duplicated + // Check UPGRADE_KEY contains only the initial plugin $this->assertArrayHasKey( - Tribe__PUE__Notices::INVALID_KEY, + Tribe__PUE__Notices::UPGRADE_KEY, $options, - 'The "invalid_key" key should exist in the options array.' + 'The "upgrade_key" key should exist in the options array.' + ); + + $this->assertArrayHasKey( + 'initial_plugin1', + $options[ Tribe__PUE__Notices::UPGRADE_KEY ], + 'initial_plugin1 should exist under "upgrade_key".' ); - $invalid_key_plugins = $options[Tribe__PUE__Notices::INVALID_KEY]; + $this->assertTrue( + $options[ Tribe__PUE__Notices::UPGRADE_KEY ]['initial_plugin1'], + 'initial_plugin1 should have a value of true under "upgrade_key".' + ); + // Check EXPIRED_KEY contains only the initial plugin $this->assertArrayHasKey( - $plugin_name, - $invalid_key_plugins, - "The plugin {$plugin_name} should exist under 'invalid_key'." + Tribe__PUE__Notices::EXPIRED_KEY, + $options, + 'The "expired_key" key should exist in the options array.' ); - // Ensure the value is not duplicated (array_merge_recursive bug can cause this) - $this->assertIsBool( - $invalid_key_plugins[$plugin_name], - "The plugin {$plugin_name} should not be duplicated or stored as an array." + $this->assertArrayHasKey( + 'initial_plugin2', + $options[ Tribe__PUE__Notices::EXPIRED_KEY ], + 'initial_plugin2 should exist under "expired_key".' ); $this->assertTrue( - $invalid_key_plugins[$plugin_name], - "The plugin {$plugin_name} should be set to true in 'invalid_key'." + $options[ Tribe__PUE__Notices::EXPIRED_KEY ]['initial_plugin2'], + 'initial_plugin2 should have a value of true under "expired_key".' + ); + + // Check INVALID_KEY contains all plugins from the loop + $this->assertArrayHasKey( + Tribe__PUE__Notices::INVALID_KEY, + $options, + 'The "invalid_key" key should exist in the options array.' ); + + $invalid_key_plugins = $options[ Tribe__PUE__Notices::INVALID_KEY ]; + + for ( $j = 0; $j < 5; $j++ ) { + $temp_plugin_name = $plugin_name . $j; + + $this->assertArrayHasKey( + $temp_plugin_name, + $invalid_key_plugins, + "{$temp_plugin_name} should exist under 'invalid_key'." + ); + + $this->assertTrue( + $invalid_key_plugins[ $temp_plugin_name ], + "{$temp_plugin_name} should have a value of true under 'invalid_key'." + ); + } + + // Ensure there are no unexpected nesting or duplicates + foreach ( $invalid_key_plugins as $plugin => $value ) { + $this->assertNotIsArray( + $value, + "The value for {$plugin} under 'invalid_key' should not be an array." + ); + } + + // Ensure the counts match expectations + $this->assertCount( + 5, + $invalid_key_plugins, + 'There should be exactly 5 plugins under "invalid_key".' + ); + } + + /** + * @test + */ + public function option_has_partial_serialization_corruption(): void { + // Insert corrupted serialized data directly + $corrupted_data = 'a:1:{s:11:"invalid_key";a:1:{s:17:"plugin-early-init1";a:8388608:{i:0;b:1;i:1;b:1;i:2;b:1;}}'; + update_option( Tribe__PUE__Notices::STORE_KEY, $corrupted_data ); + + // Attempt to load notices + $pue_notices = new Tribe__PUE__Notices(); + + $pue_notices->save_notices(); + + // Retrieve and debug the final notices + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); + + $this->assertEmpty( $options, 'Corrupted notices should of been cleared.' ); } + /** + * @test + */ + public function option_has_serialization_corruption(): void { + // Insert corrupted serialized data directly into the option + $corrupted_data = 'a:1:{s:11:"invalid_key";a:1:{s:17:"plugin-early-init1";a:8388608:{i:0;b:1;i:1;b:1;i:2;b:1;}}'; + update_option( Tribe__PUE__Notices::STORE_KEY, $corrupted_data ); + + // Initialize the PUE Notices class + $pue_notices = new Tribe__PUE__Notices(); + + // Add three plugins to the INVALID_KEY notice + $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, 'plugin-early-init1' ); + $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, 'plugin-early-init2' ); + $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, 'plugin-early-init3' ); + + // Save the notices to persist them + $pue_notices->save_notices(); + + // Retrieve the final notices from the database + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); + + // Assertions + $this->assertArrayHasKey( + Tribe__PUE__Notices::INVALID_KEY, + $options, + 'The "invalid_key" notice should exist in the options.' + ); + + $invalid_key_notices = $options[ Tribe__PUE__Notices::INVALID_KEY ]; + // Ensure all three plugins are correctly stored + $this->assertArrayHasKey( 'plugin-early-init1', $invalid_key_notices, 'plugin-early-init1 should exist in invalid_key.' ); + $this->assertArrayHasKey( 'plugin-early-init2', $invalid_key_notices, 'plugin-early-init2 should exist in invalid_key.' ); + $this->assertArrayHasKey( 'plugin-early-init3', $invalid_key_notices, 'plugin-early-init3 should exist in invalid_key.' ); + + // Ensure their values are correctly set to true + $this->assertTrue( $invalid_key_notices['plugin-early-init1'], 'plugin-early-init1 should have a value of true.' ); + $this->assertTrue( $invalid_key_notices['plugin-early-init2'], 'plugin-early-init2 should have a value of true.' ); + $this->assertTrue( $invalid_key_notices['plugin-early-init3'], 'plugin-early-init3 should have a value of true.' ); + + // Ensure no unexpected nesting or corruption + foreach ( $invalid_key_notices as $plugin => $value ) { + $this->assertIsNotArray( $value, "The value for {$plugin} under invalid_key should not be an array." ); + } + } } From 04e12056c9316976e0c1e3bd69be499b1083e9a7 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Fri, 20 Dec 2024 12:07:05 -0500 Subject: [PATCH 04/20] Fixed typo. --- tests/integration/Tribe/PUE/Notices_Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php index ded251c35..a3effde2f 100644 --- a/tests/integration/Tribe/PUE/Notices_Test.php +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -246,7 +246,7 @@ public function recursive_bug_with_same_plugin(): void { // Ensure there are no unexpected nesting or duplicates foreach ( $invalid_key_plugins as $plugin => $value ) { - $this->assertNotIsArray( + $this->assertIsNotArray( $value, "The value for {$plugin} under 'invalid_key' should not be an array." ); From 068eefc000ba2fa3c73402ef1bbcfdbd92111be0 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Fri, 20 Dec 2024 12:12:46 -0500 Subject: [PATCH 05/20] Added `@since` --- src/Tribe/PUE/Notices.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index e5806b3f5..b0c11bcbd 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -86,6 +86,8 @@ public function register_name( $plugin_name ) { /** * Restores plugins added on previous requests to the relevant notification * groups. + * + * @sice TBD Switched from array`array_merge_recursive` to `wp_parse_args` to fix an issue with data duplication */ protected function populate() { $this->saved_notices = (array) get_option( self::STORE_KEY, [] ); @@ -94,7 +96,6 @@ protected function populate() { return; } - $this->notices = wp_parse_args( $this->notices, $this->saved_notices ); // Cleanup. From 8379874fca14d0b3a3b4e85c715d303822d69397 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Fri, 20 Dec 2024 12:13:34 -0500 Subject: [PATCH 06/20] Added use statement back. --- src/Tribe/PUE/Notices.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index b0c11bcbd..ef82fc3af 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -1,4 +1,5 @@ Date: Mon, 6 Jan 2025 14:44:24 -0500 Subject: [PATCH 07/20] Rewrote populate logic to implement sanitize_notices method that fixes corrupted DB entries. --- src/Tribe/PUE/Notices.php | 48 +++-- tests/integration/Tribe/PUE/Notices_Test.php | 213 +++++++++++++++---- 2 files changed, 211 insertions(+), 50 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index ef82fc3af..5e87d0702 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -88,31 +88,51 @@ public function register_name( $plugin_name ) { * Restores plugins added on previous requests to the relevant notification * groups. * - * @sice TBD Switched from array`array_merge_recursive` to `wp_parse_args` to fix an issue with data duplication + * @sice TBD Switched from array`array_merge_recursive` to `wp_parse_args` to fix an issue with data duplication. */ protected function populate() { - $this->saved_notices = (array) get_option( self::STORE_KEY, [] ); + // Retrieve saved notices and ensure it's an array. + $saved_notices = get_option( self::STORE_KEY, [] ); - if ( empty( $this->saved_notices ) ) { - return; + if ( ! is_array( $saved_notices ) ) { + $saved_notices = []; + update_option( self::STORE_KEY, $saved_notices ); } - $this->notices = wp_parse_args( $this->notices, $this->saved_notices ); + $this->saved_notices = $saved_notices; + + // Sanitize and merge notices. + $this->notices = $this->sanitize_notices( wp_parse_args( $this->notices, $this->saved_notices ) ); + + // Update the sanitized notices back to the database if they differ. + if ( $this->notices !== $this->saved_notices ) { + update_option( self::STORE_KEY, $this->notices ); + } + } - // Cleanup. - foreach ( $this->notices as $key => &$plugin_lists ) { - // Purge any elements that are not arrays. - if ( ! is_array( $plugin_lists ) ) { - unset( $this->notices[ $key ] ); + /** + * Recursively sanitizes notices to prevent nesting and ensure data integrity. + * + * @param array $notices The array of notices to sanitize. + * + * @return array Sanitized notices. + */ + protected function sanitize_notices( array $notices ): array { + foreach ( $notices as $key => &$plugin_list ) { + // Ensure the value is an array; otherwise, reset it. + if ( ! is_array( $plugin_list ) ) { + $plugin_list = []; continue; } - $plugin_lists = array_unique( $plugin_lists ); - foreach ( $plugin_lists as $plugin => $data ) { - $this->notices[ $key ][ $plugin ] = is_array( $data ) ? array_unique( $data ) : $data; + + foreach ( $plugin_list as $plugin => $data ) { + // Flatten deeply nested arrays and set the value to `true`. + $plugin_list[ $plugin ] = true; } } - } + return $notices; + } /** * Saves any license key notices already added. */ diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php index a3effde2f..6b1c153f3 100644 --- a/tests/integration/Tribe/PUE/Notices_Test.php +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -183,7 +183,6 @@ public function recursive_bug_with_same_plugin(): void { // Retrieve the final notices from the database $options = get_option( Tribe__PUE__Notices::STORE_KEY ); - // Check UPGRADE_KEY contains only the initial plugin $this->assertArrayHasKey( Tribe__PUE__Notices::UPGRADE_KEY, @@ -263,67 +262,209 @@ public function recursive_bug_with_same_plugin(): void { /** * @test */ - public function option_has_partial_serialization_corruption(): void { - // Insert corrupted serialized data directly - $corrupted_data = 'a:1:{s:11:"invalid_key";a:1:{s:17:"plugin-early-init1";a:8388608:{i:0;b:1;i:1;b:1;i:2;b:1;}}'; - update_option( Tribe__PUE__Notices::STORE_KEY, $corrupted_data ); + public function handles_large_notice_data(): void { + $large_data = []; + for ( $i = 0; $i < 10000; $i++ ) { + $large_data[ Tribe__PUE__Notices::INVALID_KEY ]["plugin-$i"] = true; + } - // Attempt to load notices - $pue_notices = new Tribe__PUE__Notices(); + // Save the large dataset as an option + update_option( Tribe__PUE__Notices::STORE_KEY, $large_data ); - $pue_notices->save_notices(); + // Instantiate the class to trigger `populate()` + $pue_notices = new Tribe__PUE__Notices(); - // Retrieve and debug the final notices + // Retrieve notices after `populate()` runs $options = get_option( Tribe__PUE__Notices::STORE_KEY ); - $this->assertEmpty( $options, 'Corrupted notices should of been cleared.' ); + // Assert the data remains consistent + $this->assertArrayHasKey( Tribe__PUE__Notices::INVALID_KEY, $options ); + $this->assertCount( 10000, $options[ Tribe__PUE__Notices::INVALID_KEY ], 'The large dataset should have 10,000 entries.' ); } /** * @test */ - public function option_has_serialization_corruption(): void { - // Insert corrupted serialized data directly into the option - $corrupted_data = 'a:1:{s:11:"invalid_key";a:1:{s:17:"plugin-early-init1";a:8388608:{i:0;b:1;i:1;b:1;i:2;b:1;}}'; - update_option( Tribe__PUE__Notices::STORE_KEY, $corrupted_data ); + public function handles_invalid_option_values(): void { + // Save invalid data as an option + $invalid_data = 'corrupted_string_instead_of_array'; + update_option( Tribe__PUE__Notices::STORE_KEY, $invalid_data ); - // Initialize the PUE Notices class + // Instantiate the class to trigger `populate()` $pue_notices = new Tribe__PUE__Notices(); - // Add three plugins to the INVALID_KEY notice - $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, 'plugin-early-init1' ); - $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, 'plugin-early-init2' ); - $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, 'plugin-early-init3' ); + // Retrieve notices after `populate()` runs + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); - // Save the notices to persist them - $pue_notices->save_notices(); + // Assert the option was reset to an empty array + $this->assertEmpty( $options, 'Invalid data should be cleared from the option.' ); + } - // Retrieve the final notices from the database + /** + * @test + */ + public function fixes_corrupted_notices_data(): void { + // Simulate corrupted data that our clients had. + $corrupted_data = [ + Tribe__PUE__Notices::INVALID_KEY => [ + 'Promoter' => array_fill( 0, 100, [ true ] ), + ], + ]; + + // Save the corrupted data directly into the database. + update_option( Tribe__PUE__Notices::STORE_KEY, $corrupted_data ); + + // Instantiate the class to trigger `populate()`. + $pue_notices = new Tribe__PUE__Notices(); + + // Retrieve notices after `populate()` runs. $options = get_option( Tribe__PUE__Notices::STORE_KEY ); // Assertions + $this->assertIsArray( $options, 'The notices option should be an array.' ); + + // Ensure `INVALID_KEY` exists. $this->assertArrayHasKey( Tribe__PUE__Notices::INVALID_KEY, $options, - 'The "invalid_key" notice should exist in the options.' + 'The "invalid_key" should exist in the notices.' + ); + + // Ensure `Promoter` exists under `invalid_key`. + $this->assertArrayHasKey( + 'Promoter', + $options[ Tribe__PUE__Notices::INVALID_KEY ], + 'The "Promoter" key should exist under "invalid_key".' + ); + + // Ensure `Promoter` value is true. + $this->assertTrue( + $options[ Tribe__PUE__Notices::INVALID_KEY ]['Promoter'], + 'The "Promoter" key should have a value of true.' + ); + + // Ensure no unexpected keys exist in the root array. + $this->assertCount( + 1, + $options, + 'The root notices array should only contain "invalid_key".' + ); + + // Ensure no unexpected keys exist under `invalid_key`. + $this->assertCount( + 1, + $options[ Tribe__PUE__Notices::INVALID_KEY ], + 'The "invalid_key" array should only contain "Promoter".' + ); + } + + /** + * @test + */ + public function handles_mixed_valid_and_invalid_data(): void { + $mixed_data = [ + Tribe__PUE__Notices::INVALID_KEY => [ + 'ValidPlugin' => true, + 'CorruptedPlugin' => array_fill( 0, 10, [ true ] ), + ], + 'CustomKey' => 'not_an_array', + ]; + + update_option( Tribe__PUE__Notices::STORE_KEY, $mixed_data ); + + $pue_notices = new Tribe__PUE__Notices(); + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); + + // Ensure `INVALID_KEY` exists. + $this->assertArrayHasKey( Tribe__PUE__Notices::INVALID_KEY, $options ); + + // Ensure valid data is retained. + $this->assertArrayHasKey( + 'ValidPlugin', + $options[ Tribe__PUE__Notices::INVALID_KEY ], + 'ValidPlugin should be retained under INVALID_KEY.' + ); + + $this->assertTrue( + $options[ Tribe__PUE__Notices::INVALID_KEY ]['ValidPlugin'], + 'ValidPlugin should have a value of true.' + ); + + // Make sure `CorruptedPlugin` is no longer corrupted. + + $this->assertArrayHasKey( + 'CorruptedPlugin', + $options[ Tribe__PUE__Notices::INVALID_KEY ], + 'CorruptedPlugin should not be removed from INVALID_KEY.' + ); + + $this->assertTrue( + $options[ Tribe__PUE__Notices::INVALID_KEY ]['CorruptedPlugin'], + 'CorruptedPlugin should have a value of true under INVALID_KEY.' + ); + + $this->assertIsNotArray( + $options[ Tribe__PUE__Notices::INVALID_KEY ]['CorruptedPlugin'], + 'CorruptedPlugin should not be an array under INVALID_KEY.' + ); + + $this->assertArrayHasKey( + 'CustomKey', + $options, + 'CustomKey should not be removed from the root array.' ); - $invalid_key_notices = $options[ Tribe__PUE__Notices::INVALID_KEY ]; + $this->assertIsArray( + $options['CustomKey'], + 'CustomKey should be an array in the root array.' + ); + } - // Ensure all three plugins are correctly stored - $this->assertArrayHasKey( 'plugin-early-init1', $invalid_key_notices, 'plugin-early-init1 should exist in invalid_key.' ); - $this->assertArrayHasKey( 'plugin-early-init2', $invalid_key_notices, 'plugin-early-init2 should exist in invalid_key.' ); - $this->assertArrayHasKey( 'plugin-early-init3', $invalid_key_notices, 'plugin-early-init3 should exist in invalid_key.' ); + /** + * @test + */ + public function prevent_duplicates_for_same_plugin(): void { + $plugin_name = 'DuplicatePlugin'; + $status = Tribe__PUE__Notices::INVALID_KEY; - // Ensure their values are correctly set to true - $this->assertTrue( $invalid_key_notices['plugin-early-init1'], 'plugin-early-init1 should have a value of true.' ); - $this->assertTrue( $invalid_key_notices['plugin-early-init2'], 'plugin-early-init2 should have a value of true.' ); - $this->assertTrue( $invalid_key_notices['plugin-early-init3'], 'plugin-early-init3 should have a value of true.' ); + $pue_notices = tribe( 'pue.notices' ); - // Ensure no unexpected nesting or corruption - foreach ( $invalid_key_notices as $plugin => $value ) { - $this->assertIsNotArray( $value, "The value for {$plugin} under invalid_key should not be an array." ); + // Add the same notice 50 times. + for ( $i = 0; $i < 50; $i++ ) { + $pue_notices->add_notice( $status, $plugin_name ); } + + // Save notices to persist the data. + $pue_notices->save_notices(); + + // Retrieve the final notices from the database. + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); + + // Assert that the status exists. + $this->assertArrayHasKey( + $status, + $options, + "The status $status should exist in the notices." + ); + + // Assert that the plugin exists under the status. + $this->assertArrayHasKey( + $plugin_name, + $options[ $status ], + "The plugin $plugin_name should exist under $status." + ); + + // Assert that the plugin's value is true. + $this->assertTrue( + $options[ $status ][ $plugin_name ], + "The plugin $plugin_name should have a value of true under $status." + ); + + // Ensure no duplicates exist. + $this->assertCount( + 1, + $options[ $status ], + "There should be exactly 1 entry for $status." + ); } } - From 6a06b0273e13926be483355073f41a34a976e6a6 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Mon, 6 Jan 2025 14:48:11 -0500 Subject: [PATCH 08/20] Updated WP version for slic. --- .github/workflows/tests-php-eva.yml | 3 ++- .github/workflows/tests-php.yml | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests-php-eva.yml b/.github/workflows/tests-php-eva.yml index 5c4cd2d91..91add0b7c 100644 --- a/.github/workflows/tests-php-eva.yml +++ b/.github/workflows/tests-php-eva.yml @@ -355,8 +355,9 @@ jobs: run: | ${SLIC_BIN} up wordpress ${SLIC_BIN} wp core version - ${SLIC_BIN} wp core update --force --version=6.4.1 + ${SLIC_BIN} wp core update --force --version=6.6.1 ${SLIC_BIN} wp core version + ${SLIC_BIN} wp core update-db - name: Run suite tests if: steps.skip.outputs.value != 1 run: ${SLIC_BIN} run ${{ matrix.suite }} diff --git a/.github/workflows/tests-php.yml b/.github/workflows/tests-php.yml index 465829b4a..131f80203 100644 --- a/.github/workflows/tests-php.yml +++ b/.github/workflows/tests-php.yml @@ -178,8 +178,9 @@ jobs: run: | ${SLIC_BIN} up wordpress ${SLIC_BIN} wp core version - ${SLIC_BIN} wp core update --force --version=6.4.1 + ${SLIC_BIN} wp core update --force --version=6.6.1 ${SLIC_BIN} wp core version + ${SLIC_BIN} wp core update-db - name: Run suite tests if: steps.skip.outputs.value != 1 run: ${SLIC_BIN} run ${{ matrix.suite }} From 23536d9ce8cc69c95dff50901fb327ba778790e8 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Wed, 8 Jan 2025 13:17:08 -0500 Subject: [PATCH 09/20] Reverting changes I made to tests-php-eva.yml --- .github/workflows/tests-php-eva.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests-php-eva.yml b/.github/workflows/tests-php-eva.yml index 0a94b78f1..fd68510ff 100644 --- a/.github/workflows/tests-php-eva.yml +++ b/.github/workflows/tests-php-eva.yml @@ -350,9 +350,9 @@ jobs: run: | ${SLIC_BIN} up wordpress ${SLIC_BIN} wp core version - ${SLIC_BIN} wp core update --force --version=6.6.1 + ${SLIC_BIN} wp core update --force --version=6.5 ${SLIC_BIN} wp core version - ${SLIC_BIN} wp core update-db + ${SLIC_BIN} wp theme install twentytwenty --activate - name: Run suite tests if: steps.skip.outputs.value != 1 run: ${SLIC_BIN} run ${{ matrix.suite }} From fadc09ed532e7dae4d24e3f5383a4c870d46a8ac Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Wed, 8 Jan 2025 13:19:29 -0500 Subject: [PATCH 10/20] Reverting changes I made to tests-php.yml --- .github/workflows/tests-php.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests-php.yml b/.github/workflows/tests-php.yml index 377accfe8..6ee8a6a55 100644 --- a/.github/workflows/tests-php.yml +++ b/.github/workflows/tests-php.yml @@ -178,9 +178,9 @@ jobs: run: | ${SLIC_BIN} up wordpress ${SLIC_BIN} wp core version - ${SLIC_BIN} wp core update --force --version=6.6.1 + ${SLIC_BIN} wp core update --force --version=6.5 ${SLIC_BIN} wp core version - ${SLIC_BIN} wp core update-db + ${SLIC_BIN} wp theme install twentytwenty --activate - name: Run suite tests if: steps.skip.outputs.value != 1 run: ${SLIC_BIN} run ${{ matrix.suite }} From 32cfabcaf32cffaa97f8cfe52c60927efa8b2280 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Wed, 8 Jan 2025 13:27:38 -0500 Subject: [PATCH 11/20] Added changelog entry. --- changelog/fix-PUE_Notices_duplication | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/fix-PUE_Notices_duplication diff --git a/changelog/fix-PUE_Notices_duplication b/changelog/fix-PUE_Notices_duplication new file mode 100644 index 000000000..b31a427bb --- /dev/null +++ b/changelog/fix-PUE_Notices_duplication @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Improved data sanitization for tribe_pue_key_notices to prevent memory exhaustion errors caused by corrupted data. [ET-2277] From a2f4bd4f91ccf188d53fd4ae525f08a5d52c822c Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Wed, 8 Jan 2025 15:44:05 -0500 Subject: [PATCH 12/20] Refactored populate logic to optimize it. --- src/Tribe/PUE/Notices.php | 72 ++++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 5e87d0702..8c6e0cafd 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -11,8 +11,36 @@ class Tribe__PUE__Notices { const EXPIRED_KEY = 'expired_key'; const STORE_KEY = 'tribe_pue_key_notices'; + /** + * List of registered plugin names for use in notifications. + * + * Holds an array of plugin names that are registered during + * a request. Only registered plugins will appear in notifications. + * + * @var string[] + */ protected $registered = []; + + /** + * Notices previously saved in the database. + * + * Holds the saved notices retrieved from the database, + * typically via the `get_option` function. It serves as a reference for + * comparing or merging with current notices. + * + * @var array> + */ protected $saved_notices = []; + + /** + * Notices to be displayed or processed during the current request. + * + * Contains the current set of notices, which may include + * notices added during the current request. It is merged with + * `$saved_notices` to ensure consistency. + * + * @var array> + */ protected $notices = []; protected $plugin_names = [ @@ -85,29 +113,49 @@ public function register_name( $plugin_name ) { } /** - * Restores plugins added on previous requests to the relevant notification - * groups. + * Restores and sanitizes plugin notices to ensure data integrity and prevent memory issues. + * + * Retrieves saved notices from the database, validates and sanitizes them, + * and merges them with the current request's notices. If discrepancies or corrupted data + * are detected, it updates the database to reflect the corrected state. * - * @sice TBD Switched from array`array_merge_recursive` to `wp_parse_args` to fix an issue with data duplication. + * @since TBD Switched from `array_merge_recursive` to `wp_parse_args` to fix data duplication issues. Added additional sanitation and memory safeguards to handle large data sets effectively. + * + * @return void */ protected function populate() { - // Retrieve saved notices and ensure it's an array. $saved_notices = get_option( self::STORE_KEY, [] ); - if ( ! is_array( $saved_notices ) ) { - $saved_notices = []; - update_option( self::STORE_KEY, $saved_notices ); + // If $saved_notices is not an array, reset it to an empty array. + $original_saved_notices = is_array( $saved_notices ) ? $saved_notices : []; + + // Sanitize $saved_notices. + $sanitized_saved_notices = $this->sanitize_notices( $original_saved_notices ); + + // Update the option if sanitized data differs or the original data was not an array. + if ( $saved_notices !== $sanitized_saved_notices ) { + update_option( self::STORE_KEY, $sanitized_saved_notices ); } - $this->saved_notices = $saved_notices; + unset( $saved_notices ); + + // Set the sanitized saved notices to the class property. + $this->saved_notices = $sanitized_saved_notices; + + // Unset $sanitized_saved_notices to free memory. + unset( $sanitized_saved_notices, $original_saved_notices ); - // Sanitize and merge notices. - $this->notices = $this->sanitize_notices( wp_parse_args( $this->notices, $this->saved_notices ) ); + // Merge and sanitize notices with the saved notices. + $this->notices = $this->sanitize_notices( + wp_parse_args( $this->notices, $this->saved_notices ) + ); - // Update the sanitized notices back to the database if they differ. - if ( $this->notices !== $this->saved_notices ) { + // Save the final sanitized and merged notices back to the database if they differ. + if ( $this->saved_notices !== $this->notices ) { update_option( self::STORE_KEY, $this->notices ); } + + } /** From 2e4bddfa60591261b4108a99566b7e0cfa845618 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Wed, 8 Jan 2025 15:46:22 -0500 Subject: [PATCH 13/20] Remove redundant blank lines in Notices.php Eliminated unnecessary blank lines in the `remove_dismissed_notices` method to improve code readability. No functionality was affected by this change. --- src/Tribe/PUE/Notices.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 8c6e0cafd..2bb26f7ba 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -154,8 +154,6 @@ protected function populate() { if ( $this->saved_notices !== $this->notices ) { update_option( self::STORE_KEY, $this->notices ); } - - } /** From 25e30e82caffb41453cf666b6150be729f4a5842 Mon Sep 17 00:00:00 2001 From: Dimitrios Pantazis Date: Thu, 9 Jan 2025 13:48:20 +0200 Subject: [PATCH 14/20] Simplify populate and store logic of notices --- src/Tribe/PUE/Notices.php | 37 ++++++------------------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 2bb26f7ba..e6429ad03 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -116,44 +116,17 @@ public function register_name( $plugin_name ) { * Restores and sanitizes plugin notices to ensure data integrity and prevent memory issues. * * Retrieves saved notices from the database, validates and sanitizes them, - * and merges them with the current request's notices. If discrepancies or corrupted data - * are detected, it updates the database to reflect the corrected state. + * and sets them as the current request's notices. * * @since TBD Switched from `array_merge_recursive` to `wp_parse_args` to fix data duplication issues. Added additional sanitation and memory safeguards to handle large data sets effectively. * * @return void */ protected function populate() { - $saved_notices = get_option( self::STORE_KEY, [] ); + $this->saved_notices = $this->sanitize_notices( (array) get_option( self::STORE_KEY, [] ) ); - // If $saved_notices is not an array, reset it to an empty array. - $original_saved_notices = is_array( $saved_notices ) ? $saved_notices : []; - - // Sanitize $saved_notices. - $sanitized_saved_notices = $this->sanitize_notices( $original_saved_notices ); - - // Update the option if sanitized data differs or the original data was not an array. - if ( $saved_notices !== $sanitized_saved_notices ) { - update_option( self::STORE_KEY, $sanitized_saved_notices ); - } - - unset( $saved_notices ); - - // Set the sanitized saved notices to the class property. - $this->saved_notices = $sanitized_saved_notices; - - // Unset $sanitized_saved_notices to free memory. - unset( $sanitized_saved_notices, $original_saved_notices ); - - // Merge and sanitize notices with the saved notices. - $this->notices = $this->sanitize_notices( - wp_parse_args( $this->notices, $this->saved_notices ) - ); - - // Save the final sanitized and merged notices back to the database if they differ. - if ( $this->saved_notices !== $this->notices ) { - update_option( self::STORE_KEY, $this->notices ); - } + // Init the notices as the saved notices. + $this->notices = $this->saved_notices; } /** @@ -183,6 +156,8 @@ protected function sanitize_notices( array $notices ): array { * Saves any license key notices already added. */ public function save_notices() { + $this->notices = $this->sanitize_notices( (array) $this->notices ); + update_option( self::STORE_KEY, $this->notices ); /** From 6cd59cc649ce5915bcea8f38670564e09468241d Mon Sep 17 00:00:00 2001 From: Dimitrios Pantazis Date: Thu, 9 Jan 2025 14:09:00 +0200 Subject: [PATCH 15/20] Fix test cases --- src/Tribe/PUE/Notices.php | 2 +- tests/integration/Tribe/PUE/Notices_Test.php | 43 ++++++++++++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index e6429ad03..69a2cceec 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -150,7 +150,7 @@ protected function sanitize_notices( array $notices ): array { } } - return $notices; + return array_filter( $notices ); } /** * Saves any license key notices already added. diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php index 6b1c153f3..bd36af4b4 100644 --- a/tests/integration/Tribe/PUE/Notices_Test.php +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -293,6 +293,9 @@ public function handles_invalid_option_values(): void { // Instantiate the class to trigger `populate()` $pue_notices = new Tribe__PUE__Notices(); + // The cleaning in the DB should be done just prior saving. + $pue_notices->save_notices(); + // Retrieve notices after `populate()` runs $options = get_option( Tribe__PUE__Notices::STORE_KEY ); @@ -317,6 +320,9 @@ public function fixes_corrupted_notices_data(): void { // Instantiate the class to trigger `populate()`. $pue_notices = new Tribe__PUE__Notices(); + // The cleaning in the DB should be done just prior saving. + $pue_notices->save_notices(); + // Retrieve notices after `populate()` runs. $options = get_option( Tribe__PUE__Notices::STORE_KEY ); @@ -368,12 +374,20 @@ public function handles_mixed_valid_and_invalid_data(): void { 'CorruptedPlugin' => array_fill( 0, 10, [ true ] ), ], 'CustomKey' => 'not_an_array', + 'CustomKeyButArray' => [ + 'key1' => 'value1', + 'key2' => 'value2', + ], ]; update_option( Tribe__PUE__Notices::STORE_KEY, $mixed_data ); $pue_notices = new Tribe__PUE__Notices(); - $options = get_option( Tribe__PUE__Notices::STORE_KEY ); + + // The cleaning in the DB should be done just prior saving. + $pue_notices->save_notices(); + + $options = get_option( Tribe__PUE__Notices::STORE_KEY ); // Ensure `INVALID_KEY` exists. $this->assertArrayHasKey( Tribe__PUE__Notices::INVALID_KEY, $options ); @@ -408,15 +422,36 @@ public function handles_mixed_valid_and_invalid_data(): void { 'CorruptedPlugin should not be an array under INVALID_KEY.' ); - $this->assertArrayHasKey( + $this->assertArrayNotHasKey( 'CustomKey', $options, 'CustomKey should not be removed from the root array.' ); + $this->assertArrayHasKey( + 'CustomKeyButArray', + $options, + 'CustomKeyButArray should not be removed from the root array.' + ); + $this->assertIsArray( - $options['CustomKey'], - 'CustomKey should be an array in the root array.' + $options['CustomKeyButArray'], + 'CustomKeyButArray should be an array in the root array.' + ); + + $this->assertCount( + 2, + $options, + 'The root array should only contain INVALID_KEY and CustomKeyButArray.' + ); + + $this->assertEquals( + [ + 'key1' => true, + 'key2' => true, + ], + $options['CustomKeyButArray'], + 'CustomKeyButArray should have the expected values.' ); } From 19a673f8cd124a7280966b11ce2300fc8a5a0471 Mon Sep 17 00:00:00 2001 From: Dimitrios Pantazis Date: Thu, 9 Jan 2025 14:40:18 +0200 Subject: [PATCH 16/20] Added memory exhaustion test --- tests/integration/Tribe/PUE/Notices_Test.php | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php index bd36af4b4..5fc858e50 100644 --- a/tests/integration/Tribe/PUE/Notices_Test.php +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -502,4 +502,42 @@ public function prevent_duplicates_for_same_plugin(): void { "There should be exactly 1 entry for $status." ); } + + /** + * @test + */ + public function it_should_not_exhaust_memory() { + // For the sake of testing we assume memory limit 2M => so in bytes 2 * 1024 * 1024 + $test_memory_limit = 2 * 1024 * 1024; + + $initial_memory_usage = memory_get_usage(); + + // We'll increase the memory limit by the initial used memory. + $test_memory_limit += $initial_memory_usage; + + // we'll create a large array about 3/4 of the memory limit. + $data = [ + Tribe__PUE__Notices::INVALID_KEY => [], + ]; + + while ( memory_get_usage() < $test_memory_limit * 0.75 ) { + $data[ Tribe__PUE__Notices::INVALID_KEY ][] = 'plugin-' . count( $data[ Tribe__PUE__Notices::INVALID_KEY ] ); + } + + // Save the large dataset as an option + update_option( Tribe__PUE__Notices::STORE_KEY, $data ); + + unset( $data ); + + $this->assertGreaterThan( memory_get_usage(), $test_memory_limit, 'Memory usage should not exceed the memory limit.' ); + + $pue_notices = new Tribe__PUE__Notices(); + + $this->assertGreaterThan( memory_get_usage(), $test_memory_limit, 'Memory usage should not exceed the memory limit.' ); + + // The cleaning in the DB should be done just prior saving. + $pue_notices->save_notices(); + + $this->assertGreaterThan( memory_get_usage(), $test_memory_limit, 'Memory usage should not exceed the memory limit.' ); + } } From 467171926372868ec58bf2a483547da255ed9051 Mon Sep 17 00:00:00 2001 From: Dimitrios Pantazis Date: Thu, 9 Jan 2025 14:40:56 +0200 Subject: [PATCH 17/20] Update src/Tribe/PUE/Notices.php --- src/Tribe/PUE/Notices.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 69a2cceec..61bce0ab3 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -623,6 +623,8 @@ protected function get_formatted_plugin_names( $group ) { * Note: This does not persist changes to the database. To save changes, * ensure `save_notices()` is called after invoking this method. * + * @since TBD + * * @return void */ public function clear_all_notices() { From 5f2c642d0ae8c3d908ea07814e1ec3daee68554b Mon Sep 17 00:00:00 2001 From: Dimitrios Pantazis Date: Thu, 9 Jan 2025 14:41:00 +0200 Subject: [PATCH 18/20] Update src/Tribe/PUE/Notices.php --- src/Tribe/PUE/Notices.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 61bce0ab3..64da91562 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -134,6 +134,8 @@ protected function populate() { * * @param array $notices The array of notices to sanitize. * + * @since TBD + * * @return array Sanitized notices. */ protected function sanitize_notices( array $notices ): array { From 3235b4edaf6eaddbf050c39040426c5d002be891 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Thu, 9 Jan 2025 08:52:47 -0500 Subject: [PATCH 19/20] Added TBD to `save_notices`. --- src/Tribe/PUE/Notices.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index 64da91562..e25ed7d11 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -156,6 +156,8 @@ protected function sanitize_notices( array $notices ): array { } /** * Saves any license key notices already added. + * + * @since TBD Sanitize notices prior to storing them. */ public function save_notices() { $this->notices = $this->sanitize_notices( (array) $this->notices ); @@ -626,7 +628,7 @@ protected function get_formatted_plugin_names( $group ) { * ensure `save_notices()` is called after invoking this method. * * @since TBD - * + * * @return void */ public function clear_all_notices() { From 1d1c10811be33e5792f01f646d14a1a882f19908 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Thu, 9 Jan 2025 11:10:05 -0500 Subject: [PATCH 20/20] Removed the `clear_all_notices` method and replaced `tribe()` calls with direct instantiations of `Tribe__PUE__Notices`. Additionally, updated tests to align with the new approach and removed unnecessary setup and teardown methods. --- src/Tribe/PUE/Notices.php | 18 -------- tests/integration/Tribe/PUE/Notices_Test.php | 43 ++++---------------- 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/src/Tribe/PUE/Notices.php b/src/Tribe/PUE/Notices.php index e25ed7d11..13d1d3642 100644 --- a/src/Tribe/PUE/Notices.php +++ b/src/Tribe/PUE/Notices.php @@ -617,22 +617,4 @@ protected function get_formatted_plugin_names( $group ) { return '' . $html . ''; } - - /** - * Clears all stored and saved notices. - * - * This method resets both the `notices` and `saved_notices` properties to empty arrays, - * effectively removing all license key notifications from memory. - * - * Note: This does not persist changes to the database. To save changes, - * ensure `save_notices()` is called after invoking this method. - * - * @since TBD - * - * @return void - */ - public function clear_all_notices() { - $this->notices = []; - $this->saved_notices = []; - } } diff --git a/tests/integration/Tribe/PUE/Notices_Test.php b/tests/integration/Tribe/PUE/Notices_Test.php index 5fc858e50..6dbc9f845 100644 --- a/tests/integration/Tribe/PUE/Notices_Test.php +++ b/tests/integration/Tribe/PUE/Notices_Test.php @@ -9,29 +9,6 @@ class Notices_Test extends WPTestCase { - /** - * Runs before each test. - * - * @before - */ - public function initialize_notices(): void { - $notices = tribe( 'pue.notices' ); - $notices->clear_all_notices(); - $notices->save_notices(); - } - - /** - * Runs after each test. - * - * @after - */ - public function cleanup_notices(): void { - // Use the class method to clear all notices - $notices = tribe( 'pue.notices' ); - $notices->clear_all_notices(); - $notices->save_notices(); - } - /** * Data provider for test_notice_with_all_statuses. * @@ -48,7 +25,7 @@ public function notice_data_provider(): Generator { foreach ( $statuses as $status ) { yield "Multiple plugins, single status ($status)" => [ function () use ( $status ) { - $pue_notices = tribe( 'pue.notices' ); + $pue_notices = new Tribe__PUE__Notices(); $expected_options = [ $status => [] ]; foreach ( range( 1, 5 ) as $index ) { @@ -73,7 +50,7 @@ function () use ( $statuses ) { $expected_options = []; $plugin_names = []; - $pue_notices = tribe( 'pue.notices' ); + $pue_notices = new Tribe__PUE__Notices(); foreach ( $statuses as $status ) { $expected_options[ $status ] = []; @@ -156,7 +133,7 @@ public function test_notice_with_all_statuses( public function recursive_bug_with_same_plugin(): void { $plugin_name = 'plugin-merge-test'; - $pue_notices_initial = tribe( 'pue.notices' ); + $pue_notices_initial = new Tribe__PUE__Notices(); // Add initial notices to different keys $pue_notices_initial->add_notice( Tribe__PUE__Notices::UPGRADE_KEY, 'initial_plugin1' ); @@ -165,16 +142,14 @@ public function recursive_bug_with_same_plugin(): void { // Simulate repeated usage of the same plugin name with different instances for ( $j = 0; $j < 5; $j++ ) { for ( $i = 0; $i < 5; $i++ ) { - $temp_plugin_name = $plugin_name . $j; - tribe( 'pue.notices' )->register_name( $temp_plugin_name ); - - // Recreate the tribe instance to simulate typical usage - $pue_notices = tribe( 'pue.notices' ); + $temp_plugin_name = $plugin_name . $j; + $pue_notices_internal = new Tribe__PUE__Notices(); + $pue_notices_internal->register_name( $temp_plugin_name ); // Add the same notice repeatedly - $pue_notices->add_notice( Tribe__PUE__Notices::INVALID_KEY, $temp_plugin_name ); + $pue_notices_initial->add_notice( Tribe__PUE__Notices::INVALID_KEY, $temp_plugin_name ); - unset( $pue_notices ); // Clear instance to simulate separate requests + unset( $pue_notices_internal ); // Clear instance to simulate separate requests } } @@ -462,7 +437,7 @@ public function prevent_duplicates_for_same_plugin(): void { $plugin_name = 'DuplicatePlugin'; $status = Tribe__PUE__Notices::INVALID_KEY; - $pue_notices = tribe( 'pue.notices' ); + $pue_notices = new Tribe__PUE__Notices(); // Add the same notice 50 times. for ( $i = 0; $i < 50; $i++ ) {