More resilient unit tests + return errors as expected from fetching functions.

This commit is contained in:
Vedanshu Jain 2023-07-03 19:24:36 +05:30
parent 56d0bb2fb9
commit 3c59b8b4dc
6 changed files with 78 additions and 47 deletions

View File

@ -73,12 +73,17 @@ class PostsToOrdersMigrationController {
$this->error_logger = WC()->call_function( 'wc_get_logger' );
$data = array();
foreach ( $this->all_migrators as $name => $migrator ) {
$data[ $name ] = $migrator->fetch_sanitized_migration_data( $order_post_ids );
if ( ! empty( $data['errors'] ) ) {
$this->handle_migration_error( $order_post_ids, $data['errors'], null, null, $name );
return;
try {
foreach ( $this->all_migrators as $name => $migrator ) {
$data[ $name ] = $migrator->fetch_sanitized_migration_data( $order_post_ids );
if ( ! empty( $data[ $name ]['errors'] ) ) {
$this->handle_migration_error( $order_post_ids, $data[ $name ]['errors'], null, null, $name );
return;
}
}
} catch ( \Exception $e ) {
$this->handle_migration_error( $order_post_ids, $data, $e, null, 'Fetching data' );
return;
}
$using_transactions = $this->maybe_start_transaction();
@ -93,7 +98,7 @@ class PostsToOrdersMigrationController {
}
$this->handle_migration_error( $order_post_ids, $errors, $exception, $using_transactions, $name );
break;
return;
}
$this->commit_transaction();
@ -146,6 +151,12 @@ class PostsToOrdersMigrationController {
* @return bool|null True if transaction started, false if transactions won't be used, null if transaction failed to start.
*/
private function maybe_start_transaction(): ?bool {
$use_transactions = get_option( CustomOrdersTableController::USE_DB_TRANSACTIONS_OPTION, 'yes' );
if ( 'yes' !== $use_transactions ) {
return null;
}
$transaction_isolation_level = get_option( CustomOrdersTableController::DB_TRANSACTIONS_ISOLATION_LEVEL_OPTION, CustomOrdersTableController::DEFAULT_DB_TRANSACTIONS_ISOLATION_LEVEL );
$set_transaction_isolation_level_command = "SET TRANSACTION ISOLATION LEVEL $transaction_isolation_level";

View File

@ -225,6 +225,7 @@ abstract class MetaToCustomTableMigrator extends TableMigrator {
* @return array[] Data to be migrated. Would be of the form: array( 'data' => array( ... ), 'errors' => array( ... ) ).
*/
public function fetch_sanitized_migration_data( $entity_ids ) {
$this->clear_errors();
$data = $this->fetch_data_for_migration_for_ids( $entity_ids );
foreach ( $data['errors'] as $entity_id => $errors ) {
@ -232,7 +233,10 @@ abstract class MetaToCustomTableMigrator extends TableMigrator {
$this->add_error( "Error importing data for post with id $entity_id: column $column_name: $error_message" );
}
}
return $data;
return array(
'data' => $data['data'],
'errors' => $this->get_errors(),
);
}
/**

View File

@ -68,6 +68,7 @@ abstract class MetaToMetaTableMigrator extends TableMigrator {
* @return array[] Data to be migrated. Would be of the form: array( 'data' => array( ... ), 'errors' => array( ... ) ).
*/
public function fetch_sanitized_migration_data( $entity_ids ) {
$this->clear_errors();
$to_migrate = $this->fetch_data_for_migration_for_ids( $entity_ids );
if ( empty( $to_migrate ) ) {
return array(
@ -78,7 +79,10 @@ abstract class MetaToMetaTableMigrator extends TableMigrator {
$already_migrated = $this->get_already_migrated_records( array_keys( $to_migrate ) );
return $this->classify_update_insert_records( $to_migrate, $already_migrated );
return array(
'data' => $this->classify_update_insert_records( $to_migrate, $already_migrated ),
'errors' => $this->get_errors(),
);
}
/**
@ -99,6 +103,9 @@ abstract class MetaToMetaTableMigrator extends TableMigrator {
* @return array Array of errors and exception if any.
*/
public function process_migration_data( array $data ) {
if ( isset( $data['data'] ) ) {
$data = $data['data'];
}
$this->clear_errors();
$exception = null;

View File

@ -36,6 +36,10 @@ abstract class TableMigrator {
* @return void
*/
protected function add_error( string $error ): void {
if ( is_null( $this->errors ) ) {
$this->errors = array();
}
if ( ! in_array( $error, $this->errors, true ) ) {
$this->errors[] = $error;
}

View File

@ -36,8 +36,6 @@ class CustomOrdersTableController {
/**
* The name of the option that tells whether database transactions are to be used or not for data synchronization.
*
* @deprecated We only use READ UNCOMMITTED isolation level, which provides us with correctness guarantee in new table, without locking post tables.
*/
public const USE_DB_TRANSACTIONS_OPTION = 'woocommerce_use_db_transactions_for_custom_orders_table_data_sync';

View File

@ -379,6 +379,8 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
public function test_database_errors_during_migrations_are_logged() {
global $wpdb;
update_option( CustomOrdersTableController::USE_DB_TRANSACTIONS_OPTION, 'no' );
$fake_logger = $this->use_fake_logger();
$wpdb_mock = new DynamicDecorator( $wpdb );
@ -390,19 +392,8 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
$wpdb_mock = $args[0];
$query = $args[1];
if ( StringUtil::contains( $query, 'wc_orders' ) ) {
$wpdb_mock->state = 'Something failed!';
}
}
);
$wpdb_mock->register_property_get_replacement(
'last_error',
function( $replacement_object ) {
if ( $replacement_object->state ) {
return $replacement_object->state;
} else {
return $replacement_object->decorated_object->last_error;
if ( StringUtil::contains( $query, 'posts' ) ) {
$wpdb_mock->decorated_object->last_error = 'Something failed!';
}
}
);
@ -417,7 +408,7 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
}
);
$this->assertEquals( 'PostMetaToOrderMetaMigrator: when processing ids 1-3: Something failed!', $actual_errors[0]['message'] );
$this->assertTrue( str_contains( $actual_errors[0]['message'], 'when processing ids 1-3: Something failed!' ) );
$this->assertEquals( array( 1, 2, 3 ), $actual_errors[0]['data']['ids'] );
$this->assertEquals( PostsToOrdersMigrationController::LOGS_SOURCE_NAME, $actual_errors[0]['data']['source'] );
}
@ -440,7 +431,7 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
function( ...$args ) use ( $exception ) {
$query = $args[1];
if ( StringUtil::contains( $query, 'wc_orders' ) ) {
if ( StringUtil::contains( $query, 'posts' ) ) {
throw $exception;
}
}
@ -456,7 +447,7 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
}
);
$this->assertTrue( StringUtil::starts_with( $actual_errors[0]['message'], 'PostMetaToOrderMetaMigrator: when processing ids 1-3: (Exception) Something failed!' ) );
$this->assertTrue( StringUtil::contains( $actual_errors[0]['message'], 'when processing ids 1-3: (Exception) Something failed!' ) );
$this->assertEquals( $exception, $actual_errors[0]['data']['exception'] );
$this->assertEquals( PostsToOrdersMigrationController::LOGS_SOURCE_NAME, $actual_errors[0]['data']['source'] );
}
@ -617,9 +608,12 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
$wpdb_mock = $this->use_wpdb_mock();
$wpdb_mock->register_method_replacement(
'get_results',
function( ...$args ) {
throw new \Exception( 'Something failed!' );
'query',
function( $wpdb_decorator, $query ) {
if ( str_contains( $query, 'INSERT INTO ' . OrdersTableDataStore::get_orders_table_name() ) ) {
throw new \Exception( 'Something failed!' );
}
return $this->fake_query_transaction_logger( $wpdb_decorator, $query, false );
}
);
@ -705,30 +699,43 @@ WHERE order_id = {$order_id} AND meta_key = 'non_unique_key_1' AND meta_value in
$wpdb_decorator = $args[0];
$query = $args[1];
$is_transaction_related_query =
StringUtil::contains( $query, 'TRANSACTION' ) ||
StringUtil::contains( $query, 'COMMIT' ) ||
StringUtil::contains( $query, 'ROLLBACK' );
if ( $is_transaction_related_query ) {
if ( $transaction_fails instanceof \Exception ) {
throw $transaction_fails;
} elseif ( $transaction_fails ) {
$wpdb_decorator->decorated_object->last_error = 'Something failed!';
return false;
} else {
$this->executed_transaction_statements[] = $query;
return true;
}
} else {
return $wpdb_decorator->call_original_method( 'query', $args );
}
return $this->fake_query_transaction_logger( $wpdb_decorator, $query, $transaction_fails );
}
);
return $wpdb_mock;
}
/**
* Helper method to log and optionally error any transaction related query.
*
* @param DynamicDecorator $wpdb_decorator The $wpdb decorator.
* @param string $query The query.
* @param bool $transaction_fails False if the transaction related queries won't fail, 'error' if they produce a db error, or an Exception object that they will throw.
*
* @return bool
*/
private function fake_query_transaction_logger( $wpdb_decorator, $query, $transaction_fails ) {
$is_transaction_related_query =
StringUtil::contains( $query, 'TRANSACTION' ) ||
StringUtil::contains( $query, 'COMMIT' ) ||
StringUtil::contains( $query, 'ROLLBACK' );
if ( $is_transaction_related_query ) {
if ( $transaction_fails instanceof \Exception ) {
throw $transaction_fails;
} elseif ( $transaction_fails ) {
$wpdb_decorator->decorated_object->last_error = 'Something failed!';
return false;
} else {
$this->executed_transaction_statements[] = $query;
return true;
}
} else {
return $wpdb_decorator->call_original_method( 'query', $query );
}
}
/**
* Test that orders are migrated and verified without errors.
*/