From 02cc5455947eb799eb888f90e139e905fbdca68f Mon Sep 17 00:00:00 2001 From: Caleb Stauffer Date: Thu, 3 Oct 2024 21:03:24 -0400 Subject: [PATCH] eliminate code smells --- .../data-stores/ActionScheduler_DBStore.php | 225 +++++++++++++----- 1 file changed, 165 insertions(+), 60 deletions(-) diff --git a/classes/data-stores/ActionScheduler_DBStore.php b/classes/data-stores/ActionScheduler_DBStore.php index b07ccfec..6e82e770 100644 --- a/classes/data-stores/ActionScheduler_DBStore.php +++ b/classes/data-stores/ActionScheduler_DBStore.php @@ -19,18 +19,30 @@ class ActionScheduler_DBStore extends ActionScheduler_Store { */ private $claim_before_date = null; - /** @var int */ + /** + * Maximum length of args. + * + * @var int + */ protected static $max_args_length = 8000; - /** @var int */ + /** + * Maximum length of index. + * + * @var int + */ protected static $max_index_length = 191; - /** @var array List of claim filters. */ - protected $claim_filters = [ + /** + * List of claim filters. + * + * @var array + */ + protected $claim_filters = array( 'group' => '', 'hooks' => '', 'exclude-groups' => '', - ]; + ); /** * Initialize the data store @@ -137,6 +149,7 @@ private function save_action_to_db( ActionScheduler_Action $action, DateTime $da */ private function build_insert_sql( array $data, $unique ) { global $wpdb; + $columns = array_keys( $data ); $values = array_values( $data ); $placeholders = array_map( array( $this, 'get_placeholder_for_column' ), $columns ); @@ -146,8 +159,9 @@ private function build_insert_sql( array $data, $unique ) { $column_sql = '`' . implode( '`, `', $columns ) . '`'; $placeholder_sql = implode( ', ', $placeholders ); $where_clause = $this->build_where_clause_for_insert( $data, $table_name, $unique ); + // phpcs:disable WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare -- $column_sql and $where_clause are already prepared. $placeholder_sql is hardcoded. - $insert_query = $wpdb->prepare( + $insert_query = $wpdb->prepare( " INSERT INTO $table_name ( $column_sql ) SELECT $placeholder_sql FROM DUAL @@ -222,7 +236,7 @@ private function get_placeholder_for_column( $column_name ) { 'extended_args', ); - return in_array( $column_name, $string_columns ) ? '%s' : '%d'; + return in_array( $column_name, $string_columns, true ) ? '%s' : '%d'; } /** @@ -264,7 +278,11 @@ protected function get_group_ids( $slugs, $create_if_not_exists = true ) { return array(); } - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; foreach ( $slugs as $slug ) { @@ -290,8 +308,13 @@ protected function get_group_ids( $slugs, $create_if_not_exists = true ) { * @return int Group ID. */ protected function create_group( $slug ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $wpdb->insert( $wpdb->actionscheduler_groups, array( 'slug' => $slug ) ); return (int) $wpdb->insert_id; @@ -305,8 +328,13 @@ protected function create_group( $slug ) { * @return ActionScheduler_Action */ public function fetch_action( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $data = $wpdb->get_row( $wpdb->prepare( "SELECT a.*, g.slug AS `group` FROM {$wpdb->actionscheduler_actions} a LEFT JOIN {$wpdb->actionscheduler_groups} g ON a.group_id=g.group_id WHERE a.action_id=%d", @@ -397,24 +425,31 @@ protected function get_query_actions_sql( array $query, $select_or_count = 'sele throw new InvalidArgumentException( __( 'Invalid value for select or count parameter. Cannot query actions.', 'action-scheduler' ) ); } - $query = wp_parse_args( $query, array( - 'hook' => '', - 'args' => null, - 'partial_args_matching' => 'off', // can be 'like' or 'json'. - 'date' => null, - 'date_compare' => '<=', - 'modified' => null, - 'modified_compare' => '<=', - 'group' => '', - 'status' => '', - 'claimed' => null, - 'per_page' => 5, - 'offset' => 0, - 'orderby' => 'date', - 'order' => 'ASC', - ) ); - - /** @var \wpdb $wpdb */ + $query = wp_parse_args( + $query, + array( + 'hook' => '', + 'args' => null, + 'partial_args_matching' => 'off', // can be 'like' or 'json'. + 'date' => null, + 'date_compare' => '<=', + 'modified' => null, + 'modified_compare' => '<=', + 'group' => '', + 'status' => '', + 'claimed' => null, + 'per_page' => 5, + 'offset' => 0, + 'orderby' => 'date', + 'order' => 'ASC', + ) + ); + + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; $db_server_info = is_callable( array( $wpdb, 'db_server_info' ) ) ? $wpdb->db_server_info() : $wpdb->db_version(); @@ -429,7 +464,7 @@ protected function get_query_actions_sql( array $query, $select_or_count = 'sele } $sql = ( 'count' === $select_or_count ) ? 'SELECT count(a.action_id)' : 'SELECT a.action_id'; - $sql .= " FROM {$wpdb->actionscheduler_actions} a"; + $sql .= " FROM {$wpdb->actionscheduler_actions} a"; $sql_params = array(); if ( ! empty( $query['group'] ) || 'group' === $query['orderby'] ) { @@ -439,12 +474,12 @@ protected function get_query_actions_sql( array $query, $select_or_count = 'sele $sql .= ' WHERE 1=1'; if ( ! empty( $query['group'] ) ) { - $sql .= ' AND g.slug=%s'; + $sql .= ' AND g.slug=%s'; $sql_params[] = $query['group']; } if ( ! empty( $query['hook'] ) ) { - $sql .= ' AND a.hook=%s'; + $sql .= ' AND a.hook=%s'; $sql_params[] = $query['hook']; } @@ -467,26 +502,28 @@ protected function get_query_actions_sql( array $query, $select_or_count = 'sele } $placeholder = isset( $supported_types[ $value_type ] ) ? $supported_types[ $value_type ] : false; if ( ! $placeholder ) { - throw new \RuntimeException( sprintf( - /* translators: %s: provided value type */ - __( 'The value type for the JSON partial matching is not supported. Must be either integer, boolean, double or string. %s type provided.', 'action-scheduler' ), - $value_type - ) ); + throw new \RuntimeException( + sprintf( + /* translators: %s: provided value type */ + __( 'The value type for the JSON partial matching is not supported. Must be either integer, boolean, double or string. %s type provided.', 'action-scheduler' ), + $value_type + ) + ); } - $sql .= ' AND JSON_EXTRACT(a.args, %s)=' . $placeholder; + $sql .= ' AND JSON_EXTRACT(a.args, %s)=' . $placeholder; $sql_params[] = '$.' . $key; $sql_params[] = $value; } break; case 'like': foreach ( $query['args'] as $key => $value ) { - $sql .= ' AND a.args LIKE %s'; + $sql .= ' AND a.args LIKE %s'; $json_partial = $wpdb->esc_like( trim( wp_json_encode( array( $key => $value ) ), '{}' ) ); $sql_params[] = "%{$json_partial}%"; } break; case 'off': - $sql .= ' AND a.args=%s'; + $sql .= ' AND a.args=%s'; $sql_params[] = $this->get_args_for_query( $query['args'] ); break; default: @@ -597,7 +634,11 @@ protected function get_query_actions_sql( array $query, $select_or_count = 'sele * @return string|array|null The IDs of actions matching the query. Null on failure. */ public function query_actions( $query = array(), $query_type = 'select' ) { - /** @var wpdb $wpdb */ + /** + * Global. + * + * @var wpdb $wpdb + */ global $wpdb; $sql = $this->get_query_actions_sql( $query, $query_type ); @@ -639,7 +680,11 @@ public function action_counts() { * @throws \InvalidArgumentException If the action update failed. */ public function cancel_action( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; $updated = $wpdb->update( @@ -688,7 +733,11 @@ public function cancel_actions_by_group( $group ) { * @param array $query_args Query parameters. */ protected function bulk_cancel_actions( $query_args ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; if ( ! is_array( $query_args ) ) { @@ -739,8 +788,13 @@ protected function bulk_cancel_actions( $query_args ) { * @throws \InvalidArgumentException If the action deletion failed. */ public function delete_action( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $deleted = $wpdb->delete( $wpdb->actionscheduler_actions, array( 'action_id' => $action_id ), array( '%d' ) ); if ( empty( $deleted ) ) { /* translators: %s is the action ID */ @@ -771,8 +825,13 @@ public function get_date( $action_id ) { * @return \DateTime The GMT date the action is scheduled to run, or the date that it ran. */ protected function get_date_gmt( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $record = $wpdb->get_row( $wpdb->prepare( "SELECT * FROM {$wpdb->actionscheduler_actions} WHERE action_id=%d", $action_id ) ); if ( empty( $record ) ) { /* translators: %s is the action ID */ @@ -812,8 +871,13 @@ public function stake_claim( $max_actions = 10, \DateTime $before_date = null, $ * @return int Claim ID. */ protected function generate_claim_id() { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $now = as_get_datetime_object(); $wpdb->insert( $wpdb->actionscheduler_claims, array( 'date_created_gmt' => $now->format( 'Y-m-d H:i:s' ) ) ); @@ -861,11 +925,15 @@ public function get_claim_filter( $filter_name ) { * @throws \RuntimeException Throws RuntimeException if unable to claim action. */ protected function claim_actions( $claim_id, $limit, \DateTime $before_date = null, $hooks = array(), $group = '' ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; - $now = as_get_datetime_object(); - $date = is_null( $before_date ) ? $now : clone $before_date; + $now = as_get_datetime_object(); + $date = is_null( $before_date ) ? $now : clone $before_date; // can't use $wpdb->update() because of the <= condition. $update = "UPDATE {$wpdb->actionscheduler_actions} SET claim_id=%d, last_attempt_gmt=%s, last_attempt_local=%s"; $params = array( @@ -892,13 +960,13 @@ protected function claim_actions( $claim_id, $limit, \DateTime $before_date = nu if ( ! empty( $hooks ) ) { $placeholders = array_fill( 0, count( $hooks ), '%s' ); - $where .= ' AND hook IN (' . join( ', ', $placeholders ) . ')'; + $where .= ' AND hook IN (' . join( ', ', $placeholders ) . ')'; $params = array_merge( $params, array_values( $hooks ) ); } $group_operator = 'IN'; if ( empty( $group ) ) { - $group = $this->get_claim_filter( 'exclude-groups' ); + $group = $this->get_claim_filter( 'exclude-groups' ); $group_operator = 'NOT IN'; } @@ -922,7 +990,7 @@ protected function claim_actions( $claim_id, $limit, \DateTime $before_date = nu } $id_list = implode( ',', array_map( 'intval', $group_ids ) ); - $where .= " AND group_id {$group_operator} ( $id_list )"; + $where .= " AND group_id {$group_operator} ( $id_list )"; } /** @@ -978,7 +1046,11 @@ public function get_claim_count() { * @return mixed */ public function get_claim_id( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; $sql = "SELECT claim_id FROM {$wpdb->actionscheduler_actions} WHERE action_id=%d"; @@ -994,7 +1066,11 @@ public function get_claim_id( $action_id ) { * @return int[] */ public function find_actions_by_claim_id( $claim_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; $action_ids = array(); @@ -1024,8 +1100,13 @@ public function find_actions_by_claim_id( $claim_id ) { * @throws \RuntimeException When unable to release actions from claim. */ public function release_claim( ActionScheduler_ActionClaim $claim ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + /** * Deadlock warning: This function modifies actions to release them from claims that have been processed. Earlier, we used to it in a atomic query, i.e. we would update all actions belonging to a particular claim_id with claim_id = 0. * While this was functionally correct, it would cause deadlock, since this update query will hold a lock on the claim_id_.. index on the action table. @@ -1062,8 +1143,13 @@ public function release_claim( ActionScheduler_ActionClaim $claim ) { * @return void */ public function unclaim_action( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $wpdb->update( $wpdb->actionscheduler_actions, array( 'claim_id' => 0 ), @@ -1080,8 +1166,13 @@ public function unclaim_action( $action_id ) { * @throws \InvalidArgumentException Throw an exception if action was not updated. */ public function mark_failure( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + + * @var \wpdb $wpdb + */ global $wpdb; + $updated = $wpdb->update( $wpdb->actionscheduler_actions, array( 'status' => self::STATUS_FAILED ), @@ -1105,7 +1196,11 @@ public function mark_failure( $action_id ) { * @return void */ public function log_execution( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; $sql = "UPDATE {$wpdb->actionscheduler_actions} SET attempts = attempts+1, status=%s, last_attempt_gmt = %s, last_attempt_local = %s WHERE action_id = %d"; @@ -1135,8 +1230,13 @@ public function log_execution( $action_id ) { * @throws \InvalidArgumentException Throw an exception if action was not updated. */ public function mark_complete( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $updated = $wpdb->update( $wpdb->actionscheduler_actions, array( @@ -1173,13 +1273,18 @@ public function mark_complete( $action_id ) { * @throws \RuntimeException Throw an exception if action status could not be retrieved. */ public function get_status( $action_id ) { - /** @var \wpdb $wpdb */ + /** + * Global. + * + * @var \wpdb $wpdb + */ global $wpdb; + $sql = "SELECT status FROM {$wpdb->actionscheduler_actions} WHERE action_id=%d"; $sql = $wpdb->prepare( $sql, $action_id ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared $status = $wpdb->get_var( $sql ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared - if ( null === $status ) { + if ( is_null( $status ) ) { throw new \InvalidArgumentException( __( 'Invalid action ID. No status found.', 'action-scheduler' ) ); } elseif ( empty( $status ) ) { throw new \RuntimeException( __( 'Unknown status found for action.', 'action-scheduler' ) );