From 500db3cdba643c6553d40c26957f2e40a05efd50 Mon Sep 17 00:00:00 2001 From: Nestor Soriano Date: Fri, 13 May 2022 16:29:55 +0200 Subject: [PATCH] Log all db errors and exceptions in the posts to orders migrators. To achieve this: - A new class is added, TableMigrator, containing basic in-memory error storage methods, and auxiliary db_query and db_get_result methods. - MetaToCustomTableMigrator and MetaToMetaTableMigrator classes now extend TableMigrator and use these db_* methods. - process_migration_batch_for_ids is now a method in TableMigrator, it just calls the abstract process_migration_batch_for_ids_core, catches exceptions, and returns an array of errors. - PostsToOrdersMigrationController now logs all the errors returned by process_migration_batch_for_ids in all the migrator classes. - New utility method: ArrayUtil::to_ranges_string --- .../MetaToCustomTableMigrator.php | 50 ++----- .../MetaToMetaTableMigrator.php | 77 ++++------ .../PostsToOrdersMigrationController.php | 118 +++++++++------ .../CustomOrderTable/TableMigrator.php | 140 ++++++++++++++++++ .../woocommerce/src/Utilities/ArrayUtil.php | 43 ++++++ .../tests/php/src/Utilities/ArrayUtilTest.php | 34 +++++ 6 files changed, 329 insertions(+), 133 deletions(-) create mode 100644 plugins/woocommerce/src/Database/Migrations/CustomOrderTable/TableMigrator.php diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToCustomTableMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToCustomTableMigrator.php index 4d64e666981..57b0e0fd61a 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToCustomTableMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToCustomTableMigrator.php @@ -13,7 +13,7 @@ use Automattic\WooCommerce\Database\Migrations\MigrationHelper; * * @package Automattic\WooCommerce\Database\Migrations\CustomOrderTable */ -abstract class MetaToCustomTableMigrator { +abstract class MetaToCustomTableMigrator extends TableMigrator { /** * Config for tables being migrated and migrated from. See __construct() for detailed config. @@ -36,13 +36,6 @@ abstract class MetaToCustomTableMigrator { */ protected $core_column_mapping; - /** - * Store errors along with entity IDs from migrations. - * - * @var array - */ - protected $errors; - /** * MetaToCustomTableMigrator constructor. */ @@ -50,7 +43,6 @@ abstract class MetaToCustomTableMigrator { $this->schema_config = MigrationHelper::escape_schema_for_backtick( $this->get_schema_config() ); $this->meta_column_mapping = $this->get_meta_column_config(); $this->core_column_mapping = $this->get_core_column_mapping(); - $this->errors = array(); } /** @@ -224,13 +216,13 @@ abstract class MetaToCustomTableMigrator { } /** - * Process next migration batch, uses option `wc_cot_migration` to checkpoints of what have been processed so far. + * Migrate a batch of orders from the posts table to the corresponding table. * - * @param array $entity_ids List of entity IDs to perform migrations for. + * @param array $entity_ids Ids of orders ro migrate. * - * @return array List of errors happened during migration. + * @return void */ - public function process_migration_batch_for_ids( array $entity_ids ): array { + protected function process_migration_batch_for_ids_core( array $entity_ids ): void { $data = $this->fetch_data_for_migration_for_ids( $entity_ids ); foreach ( $data['errors'] as $entity_id => $errors ) { @@ -240,7 +232,7 @@ abstract class MetaToCustomTableMigrator { } if ( count( $data['data'] ) === 0 ) { - return array(); + return; } $entity_ids = array_keys( $data['data'] ); @@ -251,10 +243,6 @@ abstract class MetaToCustomTableMigrator { $to_update = array_intersect_key( $data['data'], $already_migrated ); $this->process_update_batch( $to_update, $already_migrated ); - - return array( - 'errors' => $this->errors, - ); } /** @@ -263,18 +251,15 @@ abstract class MetaToCustomTableMigrator { * @param array $batch Data to insert, will be of the form as returned by `data` in `fetch_data_for_migration_for_ids`. */ private function process_insert_batch( array $batch ): void { - global $wpdb; if ( 0 === count( $batch ) ) { return; } $queries = $this->generate_insert_sql_for_batch( $batch ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Queries should already be prepared. - $result = $wpdb->query( $queries ); - $wpdb->query( 'COMMIT;' ); // For some reason, this seems necessary on some hosts? Maybe a MySQL configuration? - if ( count( $batch ) !== $result ) { - $this->errors[] = 'Error with batch: ' . $wpdb->last_error; - } + $actual_processed_count = $this->db_query( $queries ); + $this->db_query( 'COMMIT;' ); // For some reason, this seems necessary on some hosts? Maybe a MySQL configuration? + $this->maybe_add_insert_or_update_mismatch_error( 'insert', $batch, $actual_processed_count ); } /** @@ -284,18 +269,15 @@ abstract class MetaToCustomTableMigrator { * @param array $already_migrated Maps rows to update data with their original IDs. */ private function process_update_batch( array $batch, array $already_migrated ): void { - global $wpdb; if ( 0 === count( $batch ) ) { return; } $queries = $this->generate_update_sql_for_batch( $batch, $already_migrated ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Queries should already be prepared. - $result = $wpdb->query( $queries ); - $wpdb->query( 'COMMIT;' ); // For some reason, this seems necessary on some hosts? Maybe a MySQL configuration? - if ( count( $batch ) !== $result ) { - $this->errors[] = 'Error with batch: ' . $wpdb->last_error; - } + $actual_processed_count = $this->db_query( $queries ); + $this->db_query( 'COMMIT;' ); // For some reason, this seems necessary on some hosts? Maybe a MySQL configuration? + $this->maybe_add_insert_or_update_mismatch_error( 'update', $batch, $actual_processed_count ); } @@ -316,8 +298,6 @@ abstract class MetaToCustomTableMigrator { * ) */ private function fetch_data_for_migration_for_ids( array $entity_ids ): array { - global $wpdb; - if ( empty( $entity_ids ) ) { return array( 'data' => array(), @@ -327,7 +307,7 @@ abstract class MetaToCustomTableMigrator { $entity_table_query = $this->build_entity_table_query( $entity_ids ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Output of $this->build_entity_table_query is already prepared. - $entity_data = $wpdb->get_results( $entity_table_query ); + $entity_data = $this->db_get_results( $entity_table_query ); if ( empty( $entity_data ) ) { return array( 'data' => array(), @@ -338,7 +318,7 @@ abstract class MetaToCustomTableMigrator { $meta_table_query = $this->build_meta_data_query( $entity_meta_rel_ids ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Output of $this->build_meta_data_query is already prepared. - $meta_data = $wpdb->get_results( $meta_table_query ); + $meta_data = $this->db_get_results( $meta_table_query ); return $this->process_and_sanitize_data( $entity_data, $meta_data ); } @@ -370,7 +350,7 @@ abstract class MetaToCustomTableMigrator { $entity_id_placeholder = implode( ',', array_fill( 0, count( $entity_ids ), '%d' ) ); - $already_migrated_entity_ids = $wpdb->get_results( + $already_migrated_entity_ids = $this->db_get_results( $wpdb->prepare( // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare -- All columns and table names are hardcoded. " diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToMetaTableMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToMetaTableMigrator.php index 3ec00da571b..8620783c414 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToMetaTableMigrator.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/MetaToMetaTableMigrator.php @@ -13,7 +13,7 @@ use Automattic\WooCommerce\Database\Migrations\MigrationHelper; * * @package Automattic\WooCommerce\Database\Migrations\CustomOrderTable */ -abstract class MetaToMetaTableMigrator { +abstract class MetaToMetaTableMigrator extends TableMigrator { /** * Schema config, see __construct for more details. @@ -22,13 +22,6 @@ abstract class MetaToMetaTableMigrator { */ private $schema_config; - /** - * Store errors along with entity IDs from migrations. - * - * @var array - */ - protected $errors; - /** * Returns config for the migration. * @@ -67,40 +60,35 @@ abstract class MetaToMetaTableMigrator { */ public function __construct() { $this->schema_config = $this->get_meta_config(); - $this->errors = array(); } /** - * Process migration for provided entity ids. + * Migrate a batch of orders from the posts table to the corresponding table. * - * @param array $entity_ids Entity IDs to process migration for. + * @param array $entity_ids Ids of orders ro migrate. */ - public function process_migration_batch_for_ids( array $entity_ids ): void { - global $wpdb; + protected function process_migration_batch_for_ids_core( array $entity_ids ): void { $to_migrate = $this->fetch_data_for_migration_for_ids( $entity_ids ); - $already_migrated = $this->get_already_migrated_records( array_keys( $to_migrate['data'] ) ); + $already_migrated = $this->get_already_migrated_records( array_keys( $to_migrate ) ); - $data = $this->classify_update_insert_records( $to_migrate['data'], $already_migrated ); + $data = $this->classify_update_insert_records( $to_migrate, $already_migrated ); $to_insert = $data[0]; $to_update = $data[1]; if ( ! empty( $to_insert ) ) { - $insert_queries = $this->generate_insert_sql_for_batch( $to_insert ); - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- $insert_queries should already be escaped in the generating function. - $result = $wpdb->query( $insert_queries ); - $wpdb->query( 'COMMIT;' ); - // TODO: Find and log entity ids that were not inserted. + $insert_queries = $this->generate_insert_sql_for_batch( $to_insert ); + $actual_processed_count = $this->db_query( $insert_queries ); + $this->db_query( 'COMMIT;' ); + $this->maybe_add_insert_or_update_mismatch_error( 'insert', $to_insert, $actual_processed_count ); } - if ( empty( $to_update ) ) { - return; + if ( ! empty( $to_update ) ) { + $update_queries = $this->generate_update_sql_for_batch( $to_update ); + $actual_processed_count = $this->db_query( $update_queries ); + $this->db_query( 'COMMIT;' ); + $this->maybe_add_insert_or_update_mismatch_error( 'update', $to_insert, $actual_processed_count ); } - $update_queries = $this->generate_update_sql_for_batch( $to_update ); - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- update queries are autogenerated and should already be prepared. - $result = $wpdb->query( $update_queries ); - $wpdb->query( 'COMMIT;' ); - // TODO: Find and log error updates. } /** @@ -186,34 +174,22 @@ abstract class MetaToMetaTableMigrator { * * @param array $entity_ids Array of IDs to fetch data for. * - * @return array[] Data along with errors (if any), will of the form: + * @return array[] Data, will of the form: * array( - * 'data' => array( - * 'id_1' => array( 'column1' => value1, 'column2' => value2, ...), - * ..., - * ), - * 'errors' => array( - * ..., + * 'id_1' => array( 'column1' => value1, 'column2' => value2, ...), + * ..., * ) */ - public function fetch_data_for_migration_for_ids( array $entity_ids ): array { - global $wpdb; + private function fetch_data_for_migration_for_ids( array $entity_ids ): array { if ( empty( $entity_ids ) ) { - return array( - 'data' => array(), - 'errors' => array(), - ); + return array(); } $meta_query = $this->build_meta_table_query( $entity_ids ); - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Meta query has interpolated variables, but they should all be escaped for backticks. - $meta_data_rows = $wpdb->get_results( $meta_query ); + $meta_data_rows = $this->db_get_results( $meta_query ); if ( empty( $meta_data_rows ) ) { - return array( - 'data' => array(), - 'errors' => array(), - ); + return array(); } foreach ( $meta_data_rows as $migrate_row ) { @@ -229,10 +205,7 @@ abstract class MetaToMetaTableMigrator { $to_migrate[ $migrate_row->entity_id ][ $migrate_row->meta_key ][] = $migrate_row->meta_value; } - return array( - 'data' => $to_migrate, - 'errors' => array(), - ); + return $to_migrate; } /** @@ -254,8 +227,8 @@ abstract class MetaToMetaTableMigrator { $entity_id_type_placeholder = MigrationHelper::get_wpdb_placeholder_for_type( $this->schema_config['destination']['meta']['entity_id_type'] ); $entity_ids_placeholder = implode( ',', array_fill( 0, count( $entity_ids ), $entity_id_type_placeholder ) ); - // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- All columns are hardcoded. - $data_already_migrated = $wpdb->get_results( + // phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare + $data_already_migrated = $this->db_get_results( $wpdb->prepare( " SELECT diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationController.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationController.php index 0e5a250af60..c03381c8ff4 100644 --- a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationController.php +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/PostsToOrdersMigrationController.php @@ -6,6 +6,7 @@ namespace Automattic\WooCommerce\Database\Migrations\CustomOrderTable; use Automattic\WooCommerce\Database\Migrations\MigrationErrorLogger; +use Automattic\WooCommerce\Utilities\ArrayUtil; /** * This is the main class used to perform the complete migration of orders @@ -18,62 +19,47 @@ class PostsToOrdersMigrationController { /** * Error logger for migration errors. * - * @var MigrationErrorLogger $error_logger + * @var WC_Logger */ private $error_logger; /** - * Migrator instance to migrate data into wc_order table. + * Array of objects used to perform the migration. * - * @var PostToOrderTableMigrator + * @var array */ - private $order_table_migrator; + private $all_migrators; /** - * Migrator instance to migrate billing data into address table. - * - * @var PostToOrderAddressTableMigrator + * The source name to use for logs. */ - private $billing_address_table_migrator; - - /** - * Migrator instance to migrate shipping data into address table. - * - * @var PostToOrderAddressTableMigrator - */ - private $shipping_address_table_migrator; - - /** - * Migrator instance to migrate operational data. - * - * @var PostToOrderOpTableMigrator - */ - private $operation_data_table_migrator; - - /** - * Migrator instance to migrate meta data. - * - * @var MetaToMetaTableMigrator - */ - private $meta_table_migrator; + private const LOGS_SOURCE_NAME = 'posts-to-orders-migration'; /** * PostsToOrdersMigrationController constructor. */ public function __construct() { - $this->order_table_migrator = new PostToOrderTableMigrator(); - $this->billing_address_table_migrator = new PostToOrderAddressTableMigrator( 'billing' ); - $this->shipping_address_table_migrator = new PostToOrderAddressTableMigrator( 'shipping' ); - $this->operation_data_table_migrator = new PostToOrderOpTableMigrator(); + $order_table_migrator = new PostToOrderTableMigrator(); + $billing_address_table_migrator = new PostToOrderAddressTableMigrator( 'billing' ); + $shipping_address_table_migrator = new PostToOrderAddressTableMigrator( 'shipping' ); + $operation_data_table_migrator = new PostToOrderOpTableMigrator(); - $excluded_columns = array_keys( $this->order_table_migrator->get_meta_column_config() ); - $excluded_columns = array_merge( $excluded_columns, array_keys( $this->billing_address_table_migrator->get_meta_column_config() ) ); - $excluded_columns = array_merge( $excluded_columns, array_keys( $this->shipping_address_table_migrator->get_meta_column_config() ) ); - $excluded_columns = array_merge( $excluded_columns, array_keys( $this->operation_data_table_migrator->get_meta_column_config() ) ); + $excluded_columns = array_keys( $order_table_migrator->get_meta_column_config() ); + $excluded_columns = array_merge( $excluded_columns, array_keys( $billing_address_table_migrator->get_meta_column_config() ) ); + $excluded_columns = array_merge( $excluded_columns, array_keys( $shipping_address_table_migrator->get_meta_column_config() ) ); + $excluded_columns = array_merge( $excluded_columns, array_keys( $operation_data_table_migrator->get_meta_column_config() ) ); + $meta_table_migrator = new PostMetaToOrderMetaMigrator( $excluded_columns ); - $this->meta_table_migrator = new PostMetaToOrderMetaMigrator( $excluded_columns ); - $this->error_logger = new MigrationErrorLogger(); + $this->all_migrators = array( + $order_table_migrator, + $billing_address_table_migrator, + $shipping_address_table_migrator, + $operation_data_table_migrator, + $meta_table_migrator, + ); + + $this->error_logger = wc_get_logger(); } /** @@ -82,12 +68,53 @@ class PostsToOrdersMigrationController { * @param array $order_post_ids List of post IDs of the orders to migrate. */ public function migrate_orders( array $order_post_ids ): void { - $this->order_table_migrator->process_migration_batch_for_ids( $order_post_ids ); - $this->billing_address_table_migrator->process_migration_batch_for_ids( $order_post_ids ); - $this->shipping_address_table_migrator->process_migration_batch_for_ids( $order_post_ids ); - $this->operation_data_table_migrator->process_migration_batch_for_ids( $order_post_ids ); - $this->meta_table_migrator->process_migration_batch_for_ids( $order_post_ids ); - // TODO: Return merged error array. + foreach ( $this->all_migrators as $migrator ) { + $this->do_orders_migration_step( $migrator, $order_post_ids ); + } + } + + /** + * Performs one step of the migration for a set of order posts using one given migration class. + * All database errors and exceptions are logged. + * + * @param object $migration_class The migration class to use, must have a `process_migration_batch_for_ids(array of ids)` method. + * @param array $order_post_ids List of post IDs of the orders to migrate. + * @return void + */ + private function do_orders_migration_step( object $migration_class, array $order_post_ids ): void { + $result = $migration_class->process_migration_batch_for_ids( $order_post_ids ); + + $errors = $result['errors']; + $exception = $result['exception']; + if ( null === $exception && empty( $errors ) ) { + return; + } + + $migration_class_name = ( new \ReflectionClass( $migration_class ) )->getShortName(); + $batch = ArrayUtil::to_ranges_string( $order_post_ids ); + + if ( null !== $exception ) { + $exception_class = get_class( $exception ); + $this->error_logger->error( + "$migration_class_name: when processing ids $batch: ($exception_class) {$exception->getMessage()}, {$exception->getTraceAsString()}", + array( + 'source' => self::LOGS_SOURCE_NAME, + 'ids' => $order_post_ids, + 'exception' => $exception, + ) + ); + } + + foreach ( $errors as $error ) { + $this->error_logger->error( + "$migration_class_name: when processing ids $batch: $error", + array( + 'source' => self::LOGS_SOURCE_NAME, + 'ids' => $order_post_ids, + 'exception' => $exception, + ) + ); + } } /** @@ -97,6 +124,5 @@ class PostsToOrdersMigrationController { */ public function migrate_order( int $order_post_id ): void { $this->migrate_orders( array( $order_post_id ) ); - // TODO: Return error. } } diff --git a/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/TableMigrator.php b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/TableMigrator.php new file mode 100644 index 00000000000..7c09817bf2d --- /dev/null +++ b/plugins/woocommerce/src/Database/Migrations/CustomOrderTable/TableMigrator.php @@ -0,0 +1,140 @@ +errors = array(); + } + + /** + * Add an error message to the errors list unless it's there already. + * + * @param string $error The error message to add. + * @return void + */ + protected function add_error( string $error ): void { + if ( ! in_array( $error, $this->errors, true ) ) { + $this->errors[] = $error; + } + } + + /** + * Get the list of error messages added. + * + * @return array + */ + protected function get_errors(): array { + return $this->errors; + } + + /** + * Run $wpdb->query and add the error, if any, to the errors list. + * + * @param string $query The SQL query to run. + * @return mixed Whatever $wpdb->query returns. + */ + protected function db_query( string $query ) { + global $wpdb; + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $result = $wpdb->query( $query ); + + if ( '' !== $wpdb->last_error ) { + $this->add_error( $wpdb->last_error ); + } + + return $result; + } + + /** + * Run $wpdb->get_results and add the error, if any, to the errors list. + * + * @param string|null $query The SQL query to run. + * @param string $output Any of ARRAY_A | ARRAY_N | OBJECT | OBJECT_K constants. + * @return mixed Whatever $wpdb->get_results returns. + */ + protected function db_get_results( string $query = null, string $output = OBJECT ) { + global $wpdb; + + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $result = $wpdb->get_results( $query, $output ); + + if ( '' !== $wpdb->last_error ) { + $this->add_error( $wpdb->last_error ); + } + + return $result; + } + + /** + * Migrate a batch of orders, logging any database error that could arise and the exception thrown if any. + * + * @param array $entity_ids Order ids to migrate. + * @return array An array containing the keys 'errors' (array of strings) and 'exception' (exception object or null). + */ + public function process_migration_batch_for_ids( array $entity_ids ): array { + $this->clear_errors(); + $exception = null; + + try { + $this->process_migration_batch_for_ids_core( $entity_ids ); + } catch ( \Exception $ex ) { + $exception = $ex; + } + + return array( + 'errors' => $this->get_errors(), + 'exception' => $exception, + ); + } + + /** + * The core method that actually performs the migration for the supplied batch of order ids. + * It doesn't need to deal with database errors nor with exceptions. + * + * @param array $entity_ids Order ids to migrate. + * @return void + */ + abstract protected function process_migration_batch_for_ids_core( array $entity_ids ): void; + + /** + * Check if the amount of processed database rows matches the amount of orders to process, and log an error if not. + * + * @param string $operation Operation performed, 'insert' or 'update'. + * @param array $entity_ids Order ids that were supplied for migration. + * @param int|bool $actual_processed_count Count of processed database rows or boolean false, as supplied by $wpdb->get_results. + * @return void + */ + protected function maybe_add_insert_or_update_mismatch_error( string $operation, array $entity_ids, $actual_processed_count ) { + $count = count( $entity_ids ); + + if ( false === $actual_processed_count ) { + $this->add_error( "$operation operation didn't complete, the database query failed" ); + } elseif ( $actual_processed_count < $count ) { + $this->add_error( "$operation operation didn't complete for all entities. Initial batch size: $count, actual processed size: $actual_processed_count" ); + } + } +} diff --git a/plugins/woocommerce/src/Utilities/ArrayUtil.php b/plugins/woocommerce/src/Utilities/ArrayUtil.php index 8f63dbd5f89..c000e5c752d 100644 --- a/plugins/woocommerce/src/Utilities/ArrayUtil.php +++ b/plugins/woocommerce/src/Utilities/ArrayUtil.php @@ -67,5 +67,48 @@ class ArrayUtil { public static function get_value_or_default( array $array, string $key, $default = null ) { return isset( $array[ $key ] ) ? $array[ $key ] : $default; } + + /** + * Converts an array of numbers to a human-readable range, such as "1,2,3,5" to "1-3, 5". It also supports + * floating point numbers, however with some perhaps unexpected / undefined behaviour if used within a range. + * Source: https://stackoverflow.com/a/34254663/4574 + * + * @param array $items An array (in any order, see $sort) of individual numbers. + * @param string $item_separator The string that separates sequential range groups. Defaults to ', '. + * @param string $range_separator The string that separates ranges. Defaults to '-'. A plausible example otherwise would be ' to '. + * @param bool|true $sort Sort the array prior to iterating? You'll likely always want to sort, but if not, you can set this to false. + * + * @return string + */ + public static function to_ranges_string( array $items, string $item_separator = ', ', string $range_separator = '-', bool $sort = true ): string { + if ( $sort ) { + sort( $items ); + } + + $point = null; + $range = false; + $str = ''; + + foreach ( $items as $i ) { + if ( null === $point ) { + $str .= $i; + } elseif ( ( $point + 1 ) === $i ) { + $range = true; + } else { + if ( $range ) { + $str .= $range_separator . $point; + $range = false; + } + $str .= $item_separator . $i; + } + $point = $i; + } + + if ( $range ) { + $str .= $range_separator . $point; + } + + return $str; + } } diff --git a/plugins/woocommerce/tests/php/src/Utilities/ArrayUtilTest.php b/plugins/woocommerce/tests/php/src/Utilities/ArrayUtilTest.php index f7bbc439089..461b8e7e435 100644 --- a/plugins/woocommerce/tests/php/src/Utilities/ArrayUtilTest.php +++ b/plugins/woocommerce/tests/php/src/Utilities/ArrayUtilTest.php @@ -134,4 +134,38 @@ class ArrayUtilTest extends \WC_Unit_Test_Case { $this->assertEquals( 'buzz', ArrayUtil::get_value_or_default( $array, 'fizz', 'buzz' ) ); } + + /** + * Data provider for test_to_ranges_string + * + * @return array[] + */ + public function data_provider_for_test_to_ranges_string(): array { + return array( + array( '1', array( 1 ) ), + array( '1, 3, 5, 7, 9, 11, 13-15', array( 1, 3, 5, 7, 9, 11, 13, 14, 15 ) ), + array( '1-5', array( 1, 2, 3, 4, 5 ) ), + array( '7-10', array( 7, 8, 9, 10 ) ), + array( '1-3, 5, 7-8', array( 1, 2, 3, 5, 7, 8 ) ), + array( '1-5, 10-12', array( 1, 2, 3, 4, 5, 10, 11, 12 ) ), + array( '1-5, 7', array( 1, 2, 3, 4, 5, 7 ) ), + array( '10, 12-15', array( 10, 12, 13, 14, 15 ) ), + array( '10, 12-15, 101', array( 10, 12, 13, 14, 15, 101 ) ), + array( '1-5, 7, 10-12', array( 1, 2, 3, 4, 5, 7, 10, 11, 12 ) ), + array( '1-5, 7, 10-12, 101', array( 1, 2, 3, 4, 5, 7, 10, 11, 12, 101 ) ), + array( '1-5, 7, 10, 12, 14', array( 14, 12, 10, 1, 2, 3, 4, 5, 7 ) ), + ); + } + + /** + * @testdox `to_ranges_string` works as expected with the default arguments. + * @dataProvider data_provider_for_test_to_ranges_string + * + * @param string $expected_string The expected generated string. + * @param array $input_array The input array of numbers. + */ + public function test_to_ranges_string( string $expected_string, array $input_array ) { + $actual = ArrayUtil::to_ranges_string( $input_array ); + $this->assertEquals( $expected_string, $actual ); + } }