From 862a947d7c72478b97bf372ae29930ebbffc2783 Mon Sep 17 00:00:00 2001 From: Peter Fabian Date: Fri, 19 May 2023 12:35:37 +0200 Subject: [PATCH] Install/update: Make the DELETE in 2 queries instead of 60+ (#37472) --- .../woocommerce/changelog/try-install-optim-2 | 4 ++ .../woocommerce/includes/class-wc-install.php | 46 +++++++++++++-- .../woocommerce/src/Admin/Notes/DataStore.php | 2 +- .../php/includes/class-wc-install-test.php | 59 +++++++++++++++++++ 4 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 plugins/woocommerce/changelog/try-install-optim-2 diff --git a/plugins/woocommerce/changelog/try-install-optim-2 b/plugins/woocommerce/changelog/try-install-optim-2 new file mode 100644 index 00000000000..c04ab984d37 --- /dev/null +++ b/plugins/woocommerce/changelog/try-install-optim-2 @@ -0,0 +1,4 @@ +Significance: minor +Type: dev + +Optimize installation routine by reducing the number of DELETE statments for admin notices. diff --git a/plugins/woocommerce/includes/class-wc-install.php b/plugins/woocommerce/includes/class-wc-install.php index db740b6cfa1..bfa7db4f7e3 100644 --- a/plugins/woocommerce/includes/class-wc-install.php +++ b/plugins/woocommerce/includes/class-wc-install.php @@ -891,10 +891,42 @@ class WC_Install { ); } - foreach ( $obsolete_notes_names as $obsolete_notes_name ) { - $wpdb->delete( $wpdb->prefix . 'wc_admin_notes', array( 'name' => $obsolete_notes_name ) ); - $wpdb->delete( $wpdb->prefix . 'wc_admin_note_actions', array( 'name' => $obsolete_notes_name ) ); + $note_names_placeholder = substr( str_repeat( ',%s', count( $obsolete_notes_names ) ), 1 ); + + $note_ids = $wpdb->get_results( + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare -- Ignored for allowing interpolation in the IN statement. + $wpdb->prepare( + "SELECT note_id FROM {$wpdb->prefix}wc_admin_notes WHERE name IN ( $note_names_placeholder )", + $obsolete_notes_names + ), + ARRAY_N + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare. + ); + + if ( ! $note_ids ) { + return; } + + $note_ids = array_column( $note_ids, 0 ); + $note_ids_placeholder = substr( str_repeat( ',%d', count( $note_ids ) ), 1 ); + + $wpdb->query( + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare -- Ignored for allowing interpolation in the IN statement. + $wpdb->prepare( + "DELETE FROM {$wpdb->prefix}wc_admin_notes WHERE note_id IN ( $note_ids_placeholder )", + $note_ids + ) + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare. + ); + + $wpdb->query( + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare -- Ignored for allowing interpolation in the IN statement. + $wpdb->prepare( + "DELETE FROM {$wpdb->prefix}wc_admin_note_actions WHERE note_id IN ( $note_ids_placeholder )", + $note_ids + ) + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare. + ); } /** @@ -1512,7 +1544,9 @@ CREATE TABLE {$wpdb->prefix}wc_category_lookup ( $tables = self::get_tables(); foreach ( $tables as $table ) { - $wpdb->query( "DROP TABLE IF EXISTS {$table}" ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared + $wpdb->query( "DROP TABLE IF EXISTS {$table}" ); + // phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared } } @@ -1538,7 +1572,7 @@ CREATE TABLE {$wpdb->prefix}wc_category_lookup ( } if ( ! isset( $wp_roles ) ) { - $wp_roles = new WP_Roles(); // @codingStandardsIgnoreLine + $wp_roles = new WP_Roles(); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited } // Dummy gettext calls to get strings in the catalog. @@ -1668,7 +1702,7 @@ CREATE TABLE {$wpdb->prefix}wc_category_lookup ( } if ( ! isset( $wp_roles ) ) { - $wp_roles = new WP_Roles(); // @codingStandardsIgnoreLine + $wp_roles = new WP_Roles(); // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited } $capabilities = self::get_core_capabilities(); diff --git a/plugins/woocommerce/src/Admin/Notes/DataStore.php b/plugins/woocommerce/src/Admin/Notes/DataStore.php index d128edd8065..1e7f735123d 100644 --- a/plugins/woocommerce/src/Admin/Notes/DataStore.php +++ b/plugins/woocommerce/src/Admin/Notes/DataStore.php @@ -391,7 +391,7 @@ class DataStore extends \WC_Data_Store_WP implements \WC_Object_Data_Store_Inter * @param string $type Comma separated list of note types. * @param string $status Comma separated list of statuses. * @param string $context Optional argument that the woocommerce_note_where_clauses filter can use to determine whether to apply extra conditions. Extensions should define their own contexts and use them to avoid adding to notes where clauses when not needed. - * @return array An array of objects containing a note id. + * @return string Count of objects with given type, status and context. */ public function get_notes_count( $type = array(), $status = array(), $context = self::WC_ADMIN_NOTE_OPER_GLOBAL ) { global $wpdb; diff --git a/plugins/woocommerce/tests/php/includes/class-wc-install-test.php b/plugins/woocommerce/tests/php/includes/class-wc-install-test.php index 0afda1bbea1..3ae13770efc 100644 --- a/plugins/woocommerce/tests/php/includes/class-wc-install-test.php +++ b/plugins/woocommerce/tests/php/includes/class-wc-install-test.php @@ -1,5 +1,7 @@ assertEmpty( $db_delta_result ); } + /** + * Test that delete_obsolete_notes deletes notes. + */ + public function test_delete_obsolete_notes_deletes_notes() { + $data_store = \WC_Data_Store::load( 'admin-note' ); + + $note_name = 'wc-admin-welcome-note'; + + $note = new Note(); + $note->set_name( $note_name ); + $note->set_status( Note::E_WC_ADMIN_NOTE_UNACTIONED ); + $note->add_action( 'test-action', 'Primary Action', 'https://example.com', Note::E_WC_ADMIN_NOTE_UNACTIONED, true ); + $note->add_action( 'test-action-2', 'Action 2', 'https://example.com' ); + $data_store->create( $note ); + + $this->assertEquals( 1, count( $data_store->get_notes_with_name( $note_name ) ) ); + + WC_Install::delete_obsolete_notes(); + + $this->assertEmpty( $data_store->get_notes_with_name( $note_name ) ); + + } + + /** + * Test that delete_obsolete_notes doesn't delete other notes. + */ + public function test_delete_obsolete_notes_deletes_only_selected_notes() { + $data_store = \WC_Data_Store::load( 'admin-note' ); + + $note_name = 'wc-admin-welcome-note'; + + $note = new Note(); + $note->set_name( $note_name ); + $note->set_status( Note::E_WC_ADMIN_NOTE_UNACTIONED ); + $note->add_action( 'test-action', 'Primary Action', 'https://example.com', Note::E_WC_ADMIN_NOTE_UNACTIONED, true ); + $note->add_action( 'test-action-2', 'Action 2', 'https://example.com' ); + $data_store->create( $note ); + + $note_name_2 = 'wc-admin-welcome-note-from-the-queen'; + + $note_2 = new Note(); + $note_2->set_name( $note_name_2 ); + $note_2->set_status( Note::E_WC_ADMIN_NOTE_UNACTIONED ); + $note_2->add_action( 'test-action', 'Primary Action', 'https://example.com', Note::E_WC_ADMIN_NOTE_UNACTIONED, true ); + $note_2->add_action( 'test-action-2', 'Action 2', 'https://example.com' ); + $data_store->create( $note_2 ); + + $this->assertEquals( '2', $data_store->get_notes_count( array( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ), array() ) ); + + WC_Install::delete_obsolete_notes(); + + $this->assertEmpty( $data_store->get_notes_with_name( $note_name ) ); + $this->assertEquals( '1', $data_store->get_notes_count( array( Note::E_WC_ADMIN_NOTE_INFORMATIONAL ), array() ) ); + + $data_store->delete( $note_2 ); + } + }