From e98850c2ab07f37e965a481d67d229eef0e3ff85 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:20:19 +0200 Subject: [PATCH 01/12] =?UTF-8?q?Don=E2=80=99t=20restrict=20where=20the=20?= =?UTF-8?q?purge=20can=20run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since this is a cron-job --- classes/class-admin.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/classes/class-admin.php b/classes/class-admin.php index d137c3f6f..98e225b80 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -686,17 +686,6 @@ public function purge_schedule_setup() { public function purge_scheduled_action() { global $wpdb; - // Don't purge when in Network Admin unless Stream is network activated - if ( - is_multisite() - && - is_network_admin() - && - ! $this->plugin->is_network_activated() - ) { - return; - } - if ( is_multisite() && $this->plugin->is_network_activated() ) { $options = (array) get_site_option( 'wp_stream_network', array() ); } else { From 15128b189b716ee24347d9075d6271dab3e7e2bd Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:25:57 +0200 Subject: [PATCH 02/12] This should depend on the plugin state instead --- classes/class-settings.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/classes/class-settings.php b/classes/class-settings.php index fa00b7b20..77348a1cd 100644 --- a/classes/class-settings.php +++ b/classes/class-settings.php @@ -270,13 +270,7 @@ public function add_display_name_search_columns( $search_columns, $search, $quer public function get_option_key() { $option_key = $this->option_key; - $current_page = wp_stream_filter_input( INPUT_GET, 'page' ); - - if ( ! $current_page ) { - $current_page = wp_stream_filter_input( INPUT_GET, 'action' ); - } - - if ( 'wp_stream_network_settings' === $current_page ) { + if ( $this->plugin->is_network_activated() ) { $option_key = $this->network_options_key; } From 64f4ac3da4a435e897feddb5e2a271bc7cb2934d Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:29:11 +0200 Subject: [PATCH 03/12] Use the helper for fetching the plugin settings --- classes/class-admin.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/classes/class-admin.php b/classes/class-admin.php index 98e225b80..48d30b2c1 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -686,11 +686,7 @@ public function purge_schedule_setup() { public function purge_scheduled_action() { global $wpdb; - if ( is_multisite() && $this->plugin->is_network_activated() ) { - $options = (array) get_site_option( 'wp_stream_network', array() ); - } else { - $options = (array) get_option( 'wp_stream', array() ); - } + $options = $this->plugin->settings->get_options(); if ( ! empty( $options['general_keep_records_indefinitely'] ) || ! isset( $options['general_records_ttl'] ) ) { return; From e67e1ee9852b8ce03b56cd1c2b25752ef48f0773 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:29:25 +0200 Subject: [PATCH 04/12] Describe why we might bail early --- classes/class-admin.php | 1 + 1 file changed, 1 insertion(+) diff --git a/classes/class-admin.php b/classes/class-admin.php index 48d30b2c1..36b44c1c1 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -688,6 +688,7 @@ public function purge_scheduled_action() { $options = $this->plugin->settings->get_options(); + // Ensure we don't want to keep the records indefinitely. if ( ! empty( $options['general_keep_records_indefinitely'] ) || ! isset( $options['general_records_ttl'] ) ) { return; } From 85b1815a248f30026bbfd3adcde036c9f750ad63 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:38:05 +0200 Subject: [PATCH 05/12] Introduce helpers for getting certain settings --- classes/class-settings.php | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/classes/class-settings.php b/classes/class-settings.php index 77348a1cd..fe6197247 100644 --- a/classes/class-settings.php +++ b/classes/class-settings.php @@ -1161,4 +1161,38 @@ public function get_settings_translations( $labels ) { return $labels; } + + /** + * Get the record TTL sanitized. + * + * @return integer|null Returns the time-to-live in seconds or null if empty or not set. + */ + public function records_ttl() { + $options = $this->get_options(); + + if ( isset( $options['general_records_ttl'] ) ) { + $ttl = abs( $options['general_records_ttl'] ); + + if ( $ttl > 0 ) { + return $ttl; + } + } + + return null; + } + + /** + * Should the records be stored indefinitely. + * + * @return boolean + */ + public function keep_records_indefinitely() { + $options = $this->get_options(); + + if ( ! empty( $options['general_keep_records_indefinitely'] ) ) { + return true; + } + + return false; + } } From 342e6645d4dfa1f88b9d831edd2cb813e42fba04 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:38:20 +0200 Subject: [PATCH 06/12] Use the new helpers instead --- classes/class-admin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/class-admin.php b/classes/class-admin.php index 36b44c1c1..912e90fde 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -686,10 +686,10 @@ public function purge_schedule_setup() { public function purge_scheduled_action() { global $wpdb; - $options = $this->plugin->settings->get_options(); + $days = $this->plugin->settings->records_ttl(); // Ensure we don't want to keep the records indefinitely. - if ( ! empty( $options['general_keep_records_indefinitely'] ) || ! isset( $options['general_records_ttl'] ) ) { + if ( $this->plugin->settings->keep_records_indefinitely() || empty( $days ) ) { return; } From 9418eff42e74c2ad085e3347114f8b253ed77f2e Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:44:11 +0200 Subject: [PATCH 07/12] Simplify the timestamp offset generation --- classes/class-admin.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/classes/class-admin.php b/classes/class-admin.php index 912e90fde..43f12694b 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -693,13 +693,8 @@ public function purge_scheduled_action() { return; } - $days = $options['general_records_ttl']; - $timezone = new DateTimeZone( 'UTC' ); - $date = new DateTime( 'now', $timezone ); - - $date->sub( DateInterval::createFromDateString( "$days days" ) ); - - $where = $wpdb->prepare( ' AND `stream`.`created` < %s', $date->format( 'Y-m-d H:i:s' ) ); + $timestamp = strtotime( sprintf( '%d days ago', $days ) ); + $where = $wpdb->prepare( ' AND `stream`.`created` < %s', gmdate( 'Y-m-d H:i:s', $timestamp ) ); // Multisite but NOT network activated, only purge the current blog if ( is_multisite() && ! $this->plugin->is_network_activated() ) { From 69e0906907a6e3c296981f5e55175f2860f870a3 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:46:14 +0200 Subject: [PATCH 08/12] Per coding standards --- classes/class-admin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/classes/class-admin.php b/classes/class-admin.php index 43f12694b..506819f9b 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -696,7 +696,7 @@ public function purge_scheduled_action() { $timestamp = strtotime( sprintf( '%d days ago', $days ) ); $where = $wpdb->prepare( ' AND `stream`.`created` < %s', gmdate( 'Y-m-d H:i:s', $timestamp ) ); - // Multisite but NOT network activated, only purge the current blog + // Multisite but NOT network activated, only purge the current blog. if ( is_multisite() && ! $this->plugin->is_network_activated() ) { $where .= $wpdb->prepare( ' AND `blog_id` = %d', get_current_blog_id() ); } From 3444d85332279c751760ba7dcd500b792c01f9cc Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:49:28 +0200 Subject: [PATCH 09/12] Use the API instead --- tests/tests/test-class-admin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tests/test-class-admin.php b/tests/tests/test-class-admin.php index 8dce314c7..084b328d3 100644 --- a/tests/tests/test-class-admin.php +++ b/tests/tests/test-class-admin.php @@ -268,8 +268,8 @@ public function test_purge_schedule_setup() { } public function test_purge_scheduled_action() { - // Set the TTL to one day - if ( is_multisite() && is_plugin_active_for_network( $this->plugin->locations['plugin'] ) ) { + // Set the TTL to one day. + if ( $this->admin->plugin->is_network_activated() ) { $options = (array) get_site_option( 'wp_stream_network', array() ); $options['general_records_ttl'] = '1'; update_site_option( 'wp_stream_network', $options ); From 2336970f5ec99dcb04c2f7957ac7cbf2e3c6229d Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:49:43 +0200 Subject: [PATCH 10/12] This is already handled by the methods calling is_plugin_active_for_network() --- classes/class-admin.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/classes/class-admin.php b/classes/class-admin.php index 506819f9b..951bde081 100644 --- a/classes/class-admin.php +++ b/classes/class-admin.php @@ -125,11 +125,6 @@ public function __construct( $plugin ) { add_action( 'init', array( $this, 'init' ) ); - // Ensure function used in various methods is pre-loaded. - if ( ! function_exists( 'is_plugin_active_for_network' ) ) { - require_once ABSPATH . '/wp-admin/includes/plugin.php'; - } - // User and role caps. add_filter( 'user_has_cap', array( $this, 'filter_user_caps' ), 10, 4 ); add_filter( 'role_has_cap', array( $this, 'filter_role_caps' ), 10, 3 ); From d8744870582d47b71cca7d8cfeb882e27b9934ce Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:52:28 +0200 Subject: [PATCH 11/12] Use the API in tests too --- tests/tests/test-class-admin.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/tests/test-class-admin.php b/tests/tests/test-class-admin.php index 084b328d3..9930ff357 100644 --- a/tests/tests/test-class-admin.php +++ b/tests/tests/test-class-admin.php @@ -31,9 +31,7 @@ public function test_construct() { $this->assertNotEmpty( $this->admin->plugin ); $this->assertInstanceOf( '\WP_Stream\Plugin', $this->admin->plugin ); - $this->assertTrue( function_exists( 'is_plugin_active_for_network' ) ); - - if ( is_multisite() && is_plugin_active_for_network( $this->plugin->locations['plugin'] ) && ! is_network_admin() ) { + if ( is_multisite() && $this->admin->plugin->is_network_activated() && ! is_network_admin() ) { $this->assertTrue( $this->admin->disable_access ); } else { $this->assertFalse( $this->admin->disable_access ); From 58bf268799ef29f09568db076de83d20af0a8c73 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 25 Mar 2020 12:58:47 +0200 Subject: [PATCH 12/12] Test that fresh records are still there --- tests/tests/test-class-admin.php | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/tests/test-class-admin.php b/tests/tests/test-class-admin.php index 9930ff357..38c27a971 100644 --- a/tests/tests/test-class-admin.php +++ b/tests/tests/test-class-admin.php @@ -279,29 +279,37 @@ public function test_purge_scheduled_action() { global $wpdb; - // Create (two day old) dummy records + // Create a fresh stream record that shouldn't be deleted. + $stream_data = $this->dummy_stream_data(); + $stream_data['created'] = date( 'Y-m-d h:i:s' ); + $wpdb->insert( $wpdb->stream, $stream_data ); + $fresh_stream_id = $wpdb->insert_id; + $this->assertNotFalse( $fresh_stream_id ); + + // Create (two day old) dummy records. $stream_data = $this->dummy_stream_data(); $stream_data['created'] = date( 'Y-m-d h:i:s', strtotime( '2 days ago' ) ); $wpdb->insert( $wpdb->stream, $stream_data ); $stream_id = $wpdb->insert_id; $this->assertNotFalse( $stream_id ); - // Create dummy meta + // Create dummy meta. $meta_data = $this->dummy_meta_data( $stream_id ); $wpdb->insert( $wpdb->streammeta, $meta_data ); $meta_id = $wpdb->insert_id; $this->assertNotFalse( $meta_id ); - // Purge old records and meta + // Purge old records and meta. $this->admin->purge_scheduled_action(); - // Check if the old records have been cleared + $fresh_stream_results = $wpdb->get_row( "SELECT * FROM {$wpdb->stream} WHERE ID = $fresh_stream_id" ); + $this->assertNotEmpty( $fresh_stream_results, 'fresh record from now is still there' ); + $stream_results = $wpdb->get_row( "SELECT * FROM {$wpdb->stream} WHERE ID = $stream_id" ); - $this->assertEmpty( $stream_results ); + $this->assertEmpty( $stream_results, 'Old records have been cleared' ); - // Check if the old meta has been cleared $meta_results = $wpdb->get_row( "SELECT * FROM {$wpdb->streammeta} WHERE meta_id = $meta_id" ); - $this->assertEmpty( $meta_results ); + $this->assertEmpty( $meta_results, 'Old meta has been cleared' ); } public function test_plugin_action_links() {