From 5bb5d6daf68a533b1f5925de7b9beb8839aab27f Mon Sep 17 00:00:00 2001 From: alaca Date: Fri, 21 Feb 2025 13:02:49 +0100 Subject: [PATCH 01/36] feature: BatchMigration interface --- .../Migrations/Contracts/BatchMigration.php | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/Framework/Migrations/Contracts/BatchMigration.php diff --git a/src/Framework/Migrations/Contracts/BatchMigration.php b/src/Framework/Migrations/Contracts/BatchMigration.php new file mode 100644 index 0000000000..9058d78359 --- /dev/null +++ b/src/Framework/Migrations/Contracts/BatchMigration.php @@ -0,0 +1,30 @@ + Date: Fri, 21 Feb 2025 13:03:37 +0100 Subject: [PATCH 02/36] refactor: make run method optional if BatchMigration interface is implemented --- src/Framework/Migrations/Contracts/Migration.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Framework/Migrations/Contracts/Migration.php b/src/Framework/Migrations/Contracts/Migration.php index 7991a782eb..da444f6d19 100644 --- a/src/Framework/Migrations/Contracts/Migration.php +++ b/src/Framework/Migrations/Contracts/Migration.php @@ -16,9 +16,15 @@ abstract class Migration /** * Bootstrap migration logic. * + * @unreleased this method is now optional if migration class implements BatchMigration interface * @since 2.9.0 */ - abstract public function run(); + public function run() + { + if ( ! is_subclass_of($this, BatchMigration::class)) { + throw new RuntimeException('run method is not defined.'); + } + } /** * Return a unique identifier for the migration From 783e27f9a255b94b208f61bdc10452b1b440b9fa Mon Sep 17 00:00:00 2001 From: alaca Date: Fri, 21 Feb 2025 13:04:10 +0100 Subject: [PATCH 03/36] feature: add support for batch migrations --- src/Framework/Migrations/MigrationsRunner.php | 94 ++++++++++++++++--- 1 file changed, 81 insertions(+), 13 deletions(-) diff --git a/src/Framework/Migrations/MigrationsRunner.php b/src/Framework/Migrations/MigrationsRunner.php index 0f8ca2d598..ef6b43f9e3 100644 --- a/src/Framework/Migrations/MigrationsRunner.php +++ b/src/Framework/Migrations/MigrationsRunner.php @@ -3,8 +3,11 @@ namespace Give\Framework\Migrations; use Exception; +use Give\Framework\Database\DB; use Give\Framework\Database\Exceptions\DatabaseQueryException; +use Give\Framework\Migrations\Contracts\BatchMigration; use Give\Framework\Migrations\Contracts\Migration; +use Give\Framework\Support\Facades\ActionScheduler\AsBackgroundJobs; use Give\Log\Log; use Give\MigrationLog\MigrationLogFactory; use Give\MigrationLog\MigrationLogRepository; @@ -67,12 +70,11 @@ public function __construct( /** * Run database migrations. * - * @since 2.9.0 + * @unreleased add support for batch processing + * @since 2.9.0 */ public function run() { - global $wpdb; - if ( ! $this->hasMigrationToRun()) { return; } @@ -94,7 +96,7 @@ public function run() ksort($migrations); - foreach ($migrations as $key => $migrationClass) { + foreach ($migrations as $migrationClass) { $migrationId = $migrationClass::id(); if (in_array($migrationId, $this->completedMigrations, true)) { @@ -103,19 +105,30 @@ public function run() $migrationLog = $this->migrationLogFactory->make($migrationId); - // Begin transaction - $wpdb->query('START TRANSACTION'); - try { /** @var Migration $migration */ $migration = give($migrationClass); - $migration->run(); - - // Save migration status - $migrationLog->setStatus(MigrationLogStatus::SUCCESS); + if (is_subclass_of($migration, BatchMigration::class)) { + $status = $this->runBatch($migration); + + if ($status === MigrationLogStatus::RUNNING) { + give()->notices->register_notice( + [ + 'id' => $migrationId, + 'description' => esc_html__('Running DB migration: ' . $migration::title(), 'give'), + ] + ); + break; + } + + $migrationLog->setStatus($status); + } else { + $migration->run(); + $migrationLog->setStatus(MigrationLogStatus::SUCCESS); + } } catch (Exception $exception) { - $wpdb->query('ROLLBACK'); + DB::rollback(); $migrationLog->setStatus(MigrationLogStatus::FAILED); $migrationLog->setError($exception); @@ -152,7 +165,7 @@ public function run() } // Commit transaction if successful - $wpdb->query('COMMIT'); + DB::commit(); } } @@ -167,4 +180,59 @@ public function hasMigrationToRun() { return (bool)array_diff($this->migrationRegister->getRegisteredIds(), $this->completedMigrations); } + + /** + * Run migration batch + * + * @unreleased + * @throws Exception + */ + public function runBatch(BatchMigration $migration): string + { + $group = $migration::id(); + $actionHook = 'givewp-batch-' . $group; + + add_action($actionHook, function ($batchNumber) use ($migration) { + DB::beginTransaction(); + + try { + $migration->runBatch($batchNumber); + + DB::commit(); + } catch (Exception $e) { + DB::rollback(); + throw new Exception($e->getMessage(), 0, $e); + } + }); + + $actions = AsBackgroundJobs::getActionsByGroup($group); + + // register actions - initial run + if (empty($actions)) { + $batches = ceil($migration->getItemsCount() / $migration->getBatchSize()); + + for ($i = 0; $i < $batches; $i++) { + AsBackgroundJobs::enqueueAsyncAction($actionHook, [$i], $group); + } + + return MigrationLogStatus::RUNNING; + } + + $pendingActions = AsBackgroundJobs::getActionsByGroup($group, 'pending'); + + if ( ! empty($pendingActions)) { + return MigrationLogStatus::RUNNING; + } + + $failedActions = AsBackgroundJobs::getActionsByGroup($group, 'failed'); + + if ( ! empty($failedActions)) { + return MigrationLogStatus::FAILED; + } + + // todo: discuss deleting actions + // AsBackgroundJobs::deleteActionsByGroup($group); + + return MigrationLogStatus::SUCCESS; + } } From da54e5faa14cbd63fc1c072d713b5239dd6e8982 Mon Sep 17 00:00:00 2001 From: alaca Date: Fri, 21 Feb 2025 13:04:44 +0100 Subject: [PATCH 04/36] feature: add enqueueAction method; refactor parameter position --- .../ActionScheduler/AsBackgroundJobs.php | 7 +++++- .../AsBackgroundJobsFacade.php | 22 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobs.php b/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobs.php index 6724b6b860..c5d0a9f3ce 100644 --- a/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobs.php +++ b/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobs.php @@ -7,9 +7,14 @@ /** * @since 3.6.0 * + * @unreleased + * - added enqueueAction method + * - switch parameter $status position with $returnFormat position + * * @method static int enqueueAsyncAction(string $hook, array $args, string $group, bool $unique = false, int $priority = 10) + * @method static int enqueueAction(int $timestamp, string $hook, array $args, string $group, bool $unique = false, int $priority = 10) * @method static array getActionByHookArgsGroup(string $hook, array $args, string $group, string $returnFormat = OBJECT, string $status = '') - * @method static array getActionsByGroup(string $group, string $returnFormat = OBJECT, string $status = '') + * @method static array getActionsByGroup(string $group, string $status = '', string $returnFormat = OBJECT) * @method static int deleteActionsByGroup(string $group, string $status = '') */ class AsBackgroundJobs extends Facade diff --git a/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php b/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php index 929a1c2456..423ac76a15 100644 --- a/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php +++ b/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php @@ -42,6 +42,25 @@ public function enqueueAsyncAction( } } + /** + * @unreleased + */ + public function enqueueAction( + int $timestamp, + string $hook, + array $args, + string $group, + bool $unique = false, + int $priority = 10 + ): int { + $enqueuedAction = $this->getActionByHookArgsGroup($hook, $args, $group, 'ids'); + if (empty($enqueuedAction)) { + return as_schedule_single_action($timestamp, $hook, $args, $group, $unique, $priority); + } + + return $enqueuedAction[0]; + } + /** * @since 3.6.0 * @@ -76,6 +95,7 @@ public function getActionByHookArgsGroup( /** * @since 3.6.0 + * @unreleased - switch parameter $status position with $returnFormat position * * @param string $group The group to assign this job to. * @param string $returnFormat OBJECT, ARRAY_A, or ids. @@ -83,7 +103,7 @@ public function getActionByHookArgsGroup( * * @return array */ - public function getActionsByGroup(string $group, string $returnFormat = OBJECT, string $status = ''): array + public function getActionsByGroup(string $group, string $status = '', string $returnFormat = OBJECT): array { $args = [ 'group' => $group, From 38d040efd00f26c1553df460ab372d28c4a3286e Mon Sep 17 00:00:00 2001 From: alaca Date: Fri, 21 Feb 2025 13:05:06 +0100 Subject: [PATCH 05/36] feature: add new status - RUNNING --- src/MigrationLog/MigrationLogStatus.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/MigrationLog/MigrationLogStatus.php b/src/MigrationLog/MigrationLogStatus.php index b50271754e..342821a6fc 100644 --- a/src/MigrationLog/MigrationLogStatus.php +++ b/src/MigrationLog/MigrationLogStatus.php @@ -6,6 +6,7 @@ * Class MigrationLogStatus * @package Give\MigrationLog * + * @unreleased add running status * @since 2.10.0 */ class MigrationLogStatus @@ -13,6 +14,7 @@ class MigrationLogStatus const SUCCESS = 'success'; const FAILED = 'failed'; const PENDING = 'pending'; + const RUNNING = 'running'; /** * Get default migration status @@ -35,6 +37,7 @@ public static function getAll() MigrationLogStatus::SUCCESS => esc_html__('Success', 'give'), MigrationLogStatus::FAILED => esc_html__('Failed', 'give'), MigrationLogStatus::PENDING => esc_html__('Pending', 'give'), + MigrationLogStatus::RUNNING => esc_html__('Running', 'give'), ]; } From 9d498abe424ba02fdfd1b22b8a1b62bd441b0c1d Mon Sep 17 00:00:00 2001 From: alaca Date: Fri, 21 Feb 2025 13:05:26 +0100 Subject: [PATCH 06/36] refactor: migration run hook --- src/Framework/Migrations/MigrationsServiceProvider.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Framework/Migrations/MigrationsServiceProvider.php b/src/Framework/Migrations/MigrationsServiceProvider.php index 0985009920..ebf7e6883d 100644 --- a/src/Framework/Migrations/MigrationsServiceProvider.php +++ b/src/Framework/Migrations/MigrationsServiceProvider.php @@ -30,6 +30,8 @@ public function boot() { Hooks::addAction('admin_init', ManualMigration::class, '__invoke', 0); Hooks::addAction('admin_init', MigrationsRunner::class, 'run', 0); - Hooks::addAction('give_upgrades', MigrationsRunner::class, 'run', 0); + //Hooks::addAction('give_upgrades', MigrationsRunner::class, 'run', 0); + // running batch actions via cron doesn't trigger give_upgrades and all registered actions fail + Hooks::addAction('action_scheduler_init', MigrationsRunner::class, 'run'); } } From 27d6d0d01329367478c2c3e9b96c9ac3464fb7df Mon Sep 17 00:00:00 2001 From: alaca Date: Fri, 21 Feb 2025 13:06:00 +0100 Subject: [PATCH 07/36] refactor: use batch processing --- .../Migrations/Donations/AddCampaignId.php | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/Campaigns/Migrations/Donations/AddCampaignId.php b/src/Campaigns/Migrations/Donations/AddCampaignId.php index 26aa60a658..08c54920a0 100644 --- a/src/Campaigns/Migrations/Donations/AddCampaignId.php +++ b/src/Campaigns/Migrations/Donations/AddCampaignId.php @@ -5,13 +5,14 @@ use Give\Donations\ValueObjects\DonationMetaKeys; use Give\Framework\Database\DB; use Give\Framework\Database\Exceptions\DatabaseQueryException; +use Give\Framework\Migrations\Contracts\BatchMigration; use Give\Framework\Migrations\Contracts\Migration; use Give\Framework\Migrations\Exceptions\DatabaseMigrationException; /** * @unreleased */ -class AddCampaignId extends Migration +class AddCampaignId extends Migration implements BatchMigration { /** * @inheritDoc @@ -41,7 +42,7 @@ public static function timestamp(): string * @inheritDoc * @throws DatabaseMigrationException */ - public function run() + public function runBatch($batchNumber) { $relationships = []; @@ -63,6 +64,8 @@ public function run() [DonationMetaKeys::FORM_ID(), 'formId'] ) ->where('post_type', 'give_payment') + ->offset($batchNumber) + ->limit($this->getBatchSize()) ->getAll(); $donationMeta = []; @@ -86,7 +89,26 @@ public function run() ->insert($donationMeta, ['%d', '%s', '%d']); } } catch (DatabaseQueryException $exception) { - throw new DatabaseMigrationException("An error occurred while adding campaign ID to the donation meta table", 0, $exception); + throw new DatabaseMigrationException("An error occurred while adding campaign ID to the donation meta table", + 0, $exception); } } + + /** + * @inheritDoc + */ + public function getItemsCount(): int + { + return DB::table('posts') + ->where('post_type', 'give_payment') + ->count(); + } + + /** + * @inheritDoc + */ + public function getBatchSize(): int + { + return 50; + } } From 35a729067c5dee1cf7cd9b9eff82c318ea51bbfe Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:10:17 +0100 Subject: [PATCH 08/36] feature: add isBatchMigration prop --- src/API/Endpoints/Migrations/GetMigrations.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/API/Endpoints/Migrations/GetMigrations.php b/src/API/Endpoints/Migrations/GetMigrations.php index 82719a6393..457bae4d1f 100644 --- a/src/API/Endpoints/Migrations/GetMigrations.php +++ b/src/API/Endpoints/Migrations/GetMigrations.php @@ -2,6 +2,7 @@ namespace Give\API\Endpoints\Migrations; +use Give\Framework\Migrations\Contracts\BatchMigration; use Give\Framework\Migrations\Contracts\Migration; use Give\Framework\Migrations\MigrationsRegister; use Give\MigrationLog\Helpers\MigrationHelper; @@ -161,6 +162,7 @@ public function handleRequest(WP_REST_Request $request) 'run_order' => $this->migrationHelper->getRunOrderForMigration($migration->getId()), 'source' => $migrationClass::source(), 'title' => $migrationClass::title(), + 'isBatchMigration' => is_subclass_of($migrationClass, BatchMigration::class) ]; } From f79a0401dfac7de1166c6b5405de05cde2e25782 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:11:49 +0100 Subject: [PATCH 09/36] feature: add endpoints for running batch migration and for reschedule failed actions --- src/API/Endpoints/Migrations/RunMigration.php | 136 +++++++++++++++--- 1 file changed, 119 insertions(+), 17 deletions(-) diff --git a/src/API/Endpoints/Migrations/RunMigration.php b/src/API/Endpoints/Migrations/RunMigration.php index 7871653278..0c59915780 100644 --- a/src/API/Endpoints/Migrations/RunMigration.php +++ b/src/API/Endpoints/Migrations/RunMigration.php @@ -3,6 +3,10 @@ namespace Give\API\Endpoints\Migrations; use Exception; +use Give\Framework\Database\DB; +use Give\Framework\Migrations\Contracts\BatchMigration; +use Give\Framework\Migrations\Contracts\Migration; +use Give\Framework\Migrations\Controllers\BatchMigrationRunner; use Give\Framework\Migrations\MigrationsRegister; use Give\MigrationLog\MigrationLogFactory; use Give\MigrationLog\MigrationLogStatus; @@ -11,16 +15,13 @@ /** * Class RunMigration - * @package Give\API\Endpoints\Migrations + * @package Give\API\Endpoints\Migrations * - * @since 2.10.0 + * @unreleased run batch migrations + * @since 2.10.0 */ class RunMigration extends Endpoint { - - /** @var string */ - protected $endpoint = 'migrations/run-migration'; - /** * @var MigrationsRegister */ @@ -34,7 +35,7 @@ class RunMigration extends Endpoint /** * RunMigration constructor. * - * @param MigrationsRegister $migrationsRegister + * @param MigrationsRegister $migrationsRegister * @param MigrationLogFactory $migrationLogFactory */ public function __construct( @@ -52,23 +53,58 @@ public function registerRoute() { register_rest_route( 'give-api/v2', - $this->endpoint, + 'migrations/run-migration', [ [ 'methods' => 'POST', - 'callback' => [$this, 'handleRequest'], + 'callback' => [$this, 'runMigration'], 'permission_callback' => [$this, 'permissionsCheck'], 'args' => [ 'id' => [ - 'validate_callback' => function ($param) { - return ! empty(trim($param)); - }, + 'type' => 'string', + 'required' => true, ], ], ], 'schema' => [$this, 'getSchema'], ] ); + + register_rest_route( + 'give-api/v2', + 'migrations/run-batch-migration', + [ + [ + 'methods' => 'POST', + 'callback' => [$this, 'runBatchMigration'], + 'permission_callback' => [$this, 'permissionsCheck'], + 'args' => [ + 'id' => [ + 'type' => 'string', + 'required' => true, + ], + ], + ], + ] + ); + + register_rest_route( + 'give-api/v2', + 'migrations/reschedule-failed-actions', + [ + [ + 'methods' => 'POST', + 'callback' => [$this, 'rescheduleFailedActions'], + 'permission_callback' => [$this, 'permissionsCheck'], + 'args' => [ + 'id' => [ + 'type' => 'string', + 'required' => true, + ], + ], + ], + ] + ); } /** @@ -94,17 +130,19 @@ public function getSchema() * * @return WP_REST_Response */ - public function handleRequest(WP_REST_Request $request) + public function runMigration(WP_REST_Request $request): WP_REST_Response { - global $wpdb; $migrationId = $request->get_param('id'); $migrationLog = $this->migrationLogFactory->make($migrationId); // Begin transaction - $wpdb->query('START TRANSACTION'); + DB::beginTransaction(); try { $migrationClass = $this->migrationRegister->getMigration($migrationId); + /** + * @var Migration $migration + */ $migration = give($migrationClass); $migration->run(); // Save migration status @@ -112,11 +150,11 @@ public function handleRequest(WP_REST_Request $request) $migrationLog->setError(null); $migrationLog->save(); - $wpdb->query('COMMIT'); + DB::commit(); return new WP_REST_Response(['status' => true]); } catch (Exception $exception) { - $wpdb->query('ROLLBACK'); + DB::rollback(); $migrationLog->setStatus(MigrationLogStatus::FAILED); $migrationLog->setError($exception); @@ -131,4 +169,68 @@ public function handleRequest(WP_REST_Request $request) ); } + + /** + * Run batch migration + * + * @unreleased + */ + public function runBatchMigration(WP_REST_Request $request): WP_REST_Response + { + $migrationId = $request->get_param('id'); + $migrationClass = $this->migrationRegister->getMigration($migrationId); + + if ( ! is_subclass_of($migrationClass, BatchMigration::class)) { + return new WP_REST_Response([ + 'status' => false, + 'message' => 'Migration is not an instance of ' . BatchMigration::class, + ]); + } + + try { + // We are not running migration directly, + // we just have to set migration status to PENDING and Migration Runner will handle it + $migrationLog = $this->migrationLogFactory->make($migrationId); + $migrationLog->setStatus(MigrationLogStatus::PENDING); + $migrationLog->save(); + } catch (Exception $e) { + return new WP_REST_Response([ + 'status' => false, + 'message' => $e->getMessage(), + ]); + } + + return new WP_REST_Response(['status' => true]); + } + + /** + * Reschedule failed actions + * + * @unreleased + */ + public function rescheduleFailedActions(WP_REST_Request $request): WP_REST_Response + { + $migrationId = $request->get_param('id'); + $migrationClass = $this->migrationRegister->getMigration($migrationId); + $migration = give($migrationClass); + + if ( ! is_subclass_of($migration, BatchMigration::class)) { + return new WP_REST_Response([ + 'status' => false, + 'message' => 'Migration is not an instance of ' . BatchMigration::class, + ]); + } + + try { + (new BatchMigrationRunner($migration))->rescheduleFailedActions(); + } catch (Exception $e) { + return new WP_REST_Response([ + 'status' => false, + 'message' => $e->getMessage(), + ]); + } + + return new WP_REST_Response(['status' => true]); + } + } From 3311632adfa4059d7f40e4d1cc13c577d1ab8bf1 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:12:24 +0100 Subject: [PATCH 10/36] refactor: extend BatchMigration class --- src/Campaigns/Migrations/Donations/AddCampaignId.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Campaigns/Migrations/Donations/AddCampaignId.php b/src/Campaigns/Migrations/Donations/AddCampaignId.php index 08c54920a0..0d28936130 100644 --- a/src/Campaigns/Migrations/Donations/AddCampaignId.php +++ b/src/Campaigns/Migrations/Donations/AddCampaignId.php @@ -6,13 +6,12 @@ use Give\Framework\Database\DB; use Give\Framework\Database\Exceptions\DatabaseQueryException; use Give\Framework\Migrations\Contracts\BatchMigration; -use Give\Framework\Migrations\Contracts\Migration; use Give\Framework\Migrations\Exceptions\DatabaseMigrationException; /** * @unreleased */ -class AddCampaignId extends Migration implements BatchMigration +class AddCampaignId extends BatchMigration { /** * @inheritDoc From cbdc32d76f732b8b1df63f52364e7f16ee0fd3eb Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:12:51 +0100 Subject: [PATCH 11/36] feature: add BaseMigration class --- .../Migrations/Contracts/BaseMigration.php | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/Framework/Migrations/Contracts/BaseMigration.php diff --git a/src/Framework/Migrations/Contracts/BaseMigration.php b/src/Framework/Migrations/Contracts/BaseMigration.php new file mode 100644 index 0000000000..70919b91f8 --- /dev/null +++ b/src/Framework/Migrations/Contracts/BaseMigration.php @@ -0,0 +1,61 @@ + Date: Mon, 24 Feb 2025 08:13:32 +0100 Subject: [PATCH 12/36] refactor: extend BaseMigration class --- .../Migrations/Contracts/BatchMigration.php | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Framework/Migrations/Contracts/BatchMigration.php b/src/Framework/Migrations/Contracts/BatchMigration.php index 9058d78359..9e41149953 100644 --- a/src/Framework/Migrations/Contracts/BatchMigration.php +++ b/src/Framework/Migrations/Contracts/BatchMigration.php @@ -3,28 +3,31 @@ namespace Give\Framework\Migrations\Contracts; /** + * Extend this class when you need database migration to run in batches. + * * @unreleased */ -interface BatchMigration +abstract class BatchMigration extends BaseMigration { /** - * @unreleased - * * Get the number of items per batch + * + * @unreleased */ - public function getBatchSize(): int; + abstract public function getBatchSize(): int; /** - * @unreleased * * Get the total items count + * + * @unreleased */ - public function getItemsCount(): int; + abstract public function getItemsCount(): int; /** - * @unreleased - * * Run batch + * + * @unreleased */ - public function runBatch($batchNumber); + abstract public function runBatch($batchNumber); } From 9d07d6ae221a0db660bcab995ebadbec6b800276 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:13:55 +0100 Subject: [PATCH 13/36] refactor: extend BaseMigration class --- .../Migrations/Contracts/Migration.php | 61 +------------------ 1 file changed, 3 insertions(+), 58 deletions(-) diff --git a/src/Framework/Migrations/Contracts/Migration.php b/src/Framework/Migrations/Contracts/Migration.php index da444f6d19..b56de74d7f 100644 --- a/src/Framework/Migrations/Contracts/Migration.php +++ b/src/Framework/Migrations/Contracts/Migration.php @@ -2,75 +2,20 @@ namespace Give\Framework\Migrations\Contracts; -use Give\Framework\Exceptions\Primitives\RuntimeException; - /** * Class Migration * * Extend this class when create database migration. up and timestamp are required member functions * + * @unreleased extend BaseMigration class * @since 2.9.0 */ -abstract class Migration +abstract class Migration extends BaseMigration { /** * Bootstrap migration logic. * - * @unreleased this method is now optional if migration class implements BatchMigration interface - * @since 2.9.0 - */ - public function run() - { - if ( ! is_subclass_of($this, BatchMigration::class)) { - throw new RuntimeException('run method is not defined.'); - } - } - - /** - * Return a unique identifier for the migration - * - * @return string - */ - public static function id() - { - throw new RuntimeException('A unique ID must be provided for the migration'); - } - - /** - * Return a Unix Timestamp for when the migration was created - * - * Example: strtotime( '2020-09-16 12:30:00') - * * @since 2.9.0 - * - * @return int Unix timestamp for when the migration was created - */ - public static function timestamp() - { - throw new RuntimeException('This method must be overridden to return a valid unix timestamp'); - } - - /** - * Return migration title - * - * @since 2.10.0 - * - * @return string - */ - public static function title() - { - return static::id(); - } - - /** - * Return migration source - * - * @since 2.10.0 - * - * @return string */ - public static function source() - { - return esc_html__('GiveWP Core', 'give'); - } + abstract public function run(); } From 20994e0e1b689a00c59b8954a16ac71a10aa58bc Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:14:22 +0100 Subject: [PATCH 14/36] feature: add BatchMigrationRunner controller --- .../Controllers/BatchMigrationRunner.php | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 src/Framework/Migrations/Controllers/BatchMigrationRunner.php diff --git a/src/Framework/Migrations/Controllers/BatchMigrationRunner.php b/src/Framework/Migrations/Controllers/BatchMigrationRunner.php new file mode 100644 index 0000000000..18bbb04a01 --- /dev/null +++ b/src/Framework/Migrations/Controllers/BatchMigrationRunner.php @@ -0,0 +1,162 @@ +migration = $migration; + $this->actionSchedulerStore = ActionScheduler_Store::instance(); + $this->registerAction(); + } + + /** + * Run batch migration + * + * @unreleased + */ + public function run(): string + { + $actions = $this->actionSchedulerStore->query_actions([ + 'group' => $this->getGroup(), + 'per_page' => 0, + ]); + + if ( ! $actions) { + $batches = ceil($this->migration->getItemsCount() / $this->migration->getBatchSize()); + + // Register migration action for each batch + for ($i = 0; $i < $batches; $i++) { + as_enqueue_async_action($this->getHook(), [$i], $this->getGroup()); + } + + return MigrationLogStatus::RUNNING; + } + + $pendingActions = (int)$this->actionSchedulerStore->query_actions([ + 'group' => $this->getGroup(), + 'status' => [ + ActionScheduler_Store::STATUS_RUNNING, + ActionScheduler_Store::STATUS_PENDING, + ], + ], 'count'); + + if ($pendingActions) { + return MigrationLogStatus::RUNNING; + } + + $failedActions = (int)$this->actionSchedulerStore->query_actions([ + 'group' => $this->getGroup(), + 'status' => [ + ActionScheduler_Store::STATUS_FAILED, + ActionScheduler_Store::STATUS_CANCELED, + ], + ], 'count'); + + if ($failedActions) { + return MigrationLogStatus::INCOMPLETE; + } + + // If everything went well, delete scheduled actions + foreach ($actions as $actionId) { + $this->actionSchedulerStore->delete_action($actionId); + } + + return MigrationLogStatus::SUCCESS; + } + + /** + * @unreleased + */ + public function getHook(): string + { + return 'givewp-batch-' . $this->migration::id(); + } + + /** + * @unreleased + */ + public function getGroup(): string + { + return $this->migration::id(); + } + + /** + * Register batch migration action + * + * @unreleased + * + * @throws Exception + */ + private function registerAction() + { + add_action($this->getHook(), function ($batchNumber) { + DB::beginTransaction(); + + try { + $this->migration->runBatch($batchNumber); + + DB::commit(); + } catch (Exception $e) { + DB::rollback(); + throw new Exception($e->getMessage(), 0, $e); + } + }); + } + + /** + * Reschedule failed and canceled actions + * + * @unreleased + */ + public function rescheduleFailedActions() + { + $failedActions = $this->actionSchedulerStore->query_actions([ + 'group' => $this->getGroup(), + 'per_page' => 0, + 'status' => [ + ActionScheduler_Store::STATUS_FAILED, + ActionScheduler_Store::STATUS_CANCELED, + ], + ]); + + if ( ! is_array($failedActions)) { + return; + } + + foreach ($failedActions as $actionId) { + $action = $this->actionSchedulerStore->fetch_action($actionId); + + // Reschedule new action + as_enqueue_async_action($this->getHook(), $action->get_args(), $this->getGroup()); + + // Delete failed action + $this->actionSchedulerStore->delete_action($actionId); + } + } +} From ea5662e43769e19f1ff7a690265b77abd6e3920e Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:15:07 +0100 Subject: [PATCH 15/36] refactor: add check for BaseMigration --- src/Framework/Migrations/MigrationsRegister.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Framework/Migrations/MigrationsRegister.php b/src/Framework/Migrations/MigrationsRegister.php index cca8c89a7f..5649626e34 100644 --- a/src/Framework/Migrations/MigrationsRegister.php +++ b/src/Framework/Migrations/MigrationsRegister.php @@ -3,7 +3,7 @@ namespace Give\Framework\Migrations; use Give\Framework\Exceptions\Primitives\InvalidArgumentException; -use Give\Framework\Migrations\Contracts\Migration; +use Give\Framework\Migrations\Contracts\BaseMigration; class MigrationsRegister { @@ -81,8 +81,8 @@ public function getRegisteredIds() */ public function addMigration($migrationClass) { - if ( ! is_subclass_of($migrationClass, Migration::class)) { - throw new InvalidArgumentException('Class must extend the ' . Migration::class . ' class'); + if ( ! is_subclass_of($migrationClass, BaseMigration::class)) { + throw new InvalidArgumentException('Class must extend the ' . BaseMigration::class . ' class'); } $migrationId = $migrationClass::id(); From 2c3fa32215db465bbb74b45b9e2e163c17dc3f1f Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:15:45 +0100 Subject: [PATCH 16/36] refactor: use BatchMigrationRunner for running batch migrations --- src/Framework/Migrations/MigrationsRunner.php | 87 ++++++------------- 1 file changed, 28 insertions(+), 59 deletions(-) diff --git a/src/Framework/Migrations/MigrationsRunner.php b/src/Framework/Migrations/MigrationsRunner.php index ef6b43f9e3..955b89a3f3 100644 --- a/src/Framework/Migrations/MigrationsRunner.php +++ b/src/Framework/Migrations/MigrationsRunner.php @@ -5,9 +5,10 @@ use Exception; use Give\Framework\Database\DB; use Give\Framework\Database\Exceptions\DatabaseQueryException; +use Give\Framework\Migrations\Contracts\BaseMigration; use Give\Framework\Migrations\Contracts\BatchMigration; use Give\Framework\Migrations\Contracts\Migration; -use Give\Framework\Support\Facades\ActionScheduler\AsBackgroundJobs; +use Give\Framework\Migrations\Controllers\BatchMigrationRunner; use Give\Log\Log; use Give\MigrationLog\MigrationLogFactory; use Give\MigrationLog\MigrationLogRepository; @@ -90,7 +91,7 @@ public function run() $migrations = []; foreach ($this->migrationRegister->getMigrations() as $migrationClass) { - /* @var Migration $migrationClass */ + /* @var BaseMigration $migrationClass */ $migrations[$migrationClass::timestamp() . '_' . $migrationClass::id()] = $migrationClass; } @@ -110,18 +111,41 @@ public function run() $migration = give($migrationClass); if (is_subclass_of($migration, BatchMigration::class)) { - $status = $this->runBatch($migration); + $status = (new BatchMigrationRunner($migration))->run(); if ($status === MigrationLogStatus::RUNNING) { give()->notices->register_notice( [ 'id' => $migrationId, - 'description' => esc_html__('Running DB migration: ' . $migration::title(), 'give'), + 'description' => sprintf( + esc_html__('Running DB migration: %s', 'give'), + $migration::title() + ), ] ); + + // Update status to RUNNING + if (MigrationLogStatus::RUNNING !== $migrationLog->getStatus()) { + $migrationLog->setStatus(MigrationLogStatus::RUNNING); + $migrationLog->save(); + } + break; } + if ($status === MigrationLogStatus::INCOMPLETE) { + give()->notices->register_notice( + [ + 'id' => $migrationId, + 'type' => 'warning', + 'description' => sprintf( + esc_html__('Incomplete DB migration: %s', 'give'), + $migration::title() + ), + ] + ); + } + $migrationLog->setStatus($status); } else { $migration->run(); @@ -180,59 +204,4 @@ public function hasMigrationToRun() { return (bool)array_diff($this->migrationRegister->getRegisteredIds(), $this->completedMigrations); } - - /** - * Run migration batch - * - * @unreleased - * @throws Exception - */ - public function runBatch(BatchMigration $migration): string - { - $group = $migration::id(); - $actionHook = 'givewp-batch-' . $group; - - add_action($actionHook, function ($batchNumber) use ($migration) { - DB::beginTransaction(); - - try { - $migration->runBatch($batchNumber); - - DB::commit(); - } catch (Exception $e) { - DB::rollback(); - throw new Exception($e->getMessage(), 0, $e); - } - }); - - $actions = AsBackgroundJobs::getActionsByGroup($group); - - // register actions - initial run - if (empty($actions)) { - $batches = ceil($migration->getItemsCount() / $migration->getBatchSize()); - - for ($i = 0; $i < $batches; $i++) { - AsBackgroundJobs::enqueueAsyncAction($actionHook, [$i], $group); - } - - return MigrationLogStatus::RUNNING; - } - - $pendingActions = AsBackgroundJobs::getActionsByGroup($group, 'pending'); - - if ( ! empty($pendingActions)) { - return MigrationLogStatus::RUNNING; - } - - $failedActions = AsBackgroundJobs::getActionsByGroup($group, 'failed'); - - if ( ! empty($failedActions)) { - return MigrationLogStatus::FAILED; - } - - // todo: discuss deleting actions - // AsBackgroundJobs::deleteActionsByGroup($group); - - return MigrationLogStatus::SUCCESS; - } } From 10afdacf4be33d2ef90ad1e78807ea41f8e9a88f Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:16:36 +0100 Subject: [PATCH 17/36] refactor: run migrations on action_scheduler_init hook --- src/Framework/Migrations/MigrationsServiceProvider.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Framework/Migrations/MigrationsServiceProvider.php b/src/Framework/Migrations/MigrationsServiceProvider.php index ebf7e6883d..81ebf692b9 100644 --- a/src/Framework/Migrations/MigrationsServiceProvider.php +++ b/src/Framework/Migrations/MigrationsServiceProvider.php @@ -29,9 +29,7 @@ public function register() public function boot() { Hooks::addAction('admin_init', ManualMigration::class, '__invoke', 0); - Hooks::addAction('admin_init', MigrationsRunner::class, 'run', 0); - //Hooks::addAction('give_upgrades', MigrationsRunner::class, 'run', 0); - // running batch actions via cron doesn't trigger give_upgrades and all registered actions fail - Hooks::addAction('action_scheduler_init', MigrationsRunner::class, 'run'); + Hooks::addAction('action_scheduler_init', MigrationsRunner::class, 'run', 0); + Hooks::addAction('give_upgrades', MigrationsRunner::class, 'run', 0); } } From 698f631c28ec613f574e5081ce60cdc5bea328e1 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:17:08 +0100 Subject: [PATCH 18/36] refactor: remove method and fix parameter position --- .../AsBackgroundJobsFacade.php | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php b/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php index 423ac76a15..564f17e7f0 100644 --- a/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php +++ b/src/Framework/Support/Facades/ActionScheduler/AsBackgroundJobsFacade.php @@ -42,25 +42,6 @@ public function enqueueAsyncAction( } } - /** - * @unreleased - */ - public function enqueueAction( - int $timestamp, - string $hook, - array $args, - string $group, - bool $unique = false, - int $priority = 10 - ): int { - $enqueuedAction = $this->getActionByHookArgsGroup($hook, $args, $group, 'ids'); - if (empty($enqueuedAction)) { - return as_schedule_single_action($timestamp, $hook, $args, $group, $unique, $priority); - } - - return $enqueuedAction[0]; - } - /** * @since 3.6.0 * @@ -128,7 +109,7 @@ public function getActionsByGroup(string $group, string $status = '', string $re */ public function deleteActionsByGroup(string $group, string $status = ''): int { - $actions = $this->getActionsByGroup($group, 'ids', $status); + $actions = $this->getActionsByGroup($group, $status, 'ids'); $deletedActions = 0; foreach ($actions as $actionID) { From 5ff0d21d1f5b49c685343131c83e1526bafe3d32 Mon Sep 17 00:00:00 2001 From: alaca Date: Mon, 24 Feb 2025 08:17:59 +0100 Subject: [PATCH 19/36] feature: add support for running batch migrations and continue failed batch migrations --- src/MigrationLog/Admin/Migrations/index.js | 96 +++++++++++++--------- 1 file changed, 55 insertions(+), 41 deletions(-) diff --git a/src/MigrationLog/Admin/Migrations/index.js b/src/MigrationLog/Admin/Migrations/index.js index 5055878434..6d002af596 100644 --- a/src/MigrationLog/Admin/Migrations/index.js +++ b/src/MigrationLog/Admin/Migrations/index.js @@ -56,32 +56,40 @@ const Migrations = () => { }; }); - API.post('/run-migration', {id: migrationRunModal.id}) - .then((response) => { - if (response.data.status) { - closeMigrationRunModal(); - } else { - setMigrationRunModal((previousState) => { - return { - ...previousState, - type: 'error', - error: true, - errorMessage: response.data.message, - }; - }); - } - // Invalidate the cache - mutate(getEndpoint('/get-migrations', parameters)); - }) - .catch(() => { - setMigrationRunModal((previousState) => { - return { - ...previousState, - type: 'error', - error: true, - }; - }); - }); + let url = '/run-migration'; + + if (migrationRunModal.isBatchMigration) { + url = migrationRunModal.status === 'incomplete' + ? '/reschedule-failed-actions' + : '/run-batch-migration' + } + + API.post(url, {id: migrationRunModal.id}) + .then((response) => { + if (response.data.status) { + closeMigrationRunModal(); + } else { + setMigrationRunModal((previousState) => { + return { + ...previousState, + type: 'error', + error: true, + errorMessage: response.data.message, + }; + }); + } + // Invalidate the cache + mutate(getEndpoint('/get-migrations', parameters)); + }) + .catch(() => { + setMigrationRunModal((previousState) => { + return { + ...previousState, + type: 'error', + error: true, + }; + }); + }); }; const openMigrationModal = (migration) => { @@ -98,11 +106,13 @@ const Migrations = () => { setMigrationModal({visible: false}); }; - const openMigrationRunModal = (migrationId) => { + const openMigrationRunModal = (migration) => { setMigrationRunModal({ - id: migrationId, + id: migration.id, visible: true, type: 'warning', + isBatchMigration: migration.isBatchMigration, + status: migration.status, }); }; @@ -136,13 +146,13 @@ const Migrations = () => { {__('Migration Failed', 'give')} - + - - + + - + ); }; @@ -158,7 +168,7 @@ const Migrations = () => { {migrationRunModal.error ? ( <> - +

{__('Database update failed!', 'give')}

{migrationRunModal.errorMessage && ( {migrationRunModal.errorMessage} @@ -169,7 +179,7 @@ const Migrations = () => { ) : ( <> - +
{__('Running Update', 'give')}
)} @@ -178,7 +188,7 @@ const Migrations = () => { <> - + {__('Create a Backup Before Running Database Update', 'give')} @@ -187,7 +197,7 @@ const Migrations = () => { {__('Notice', 'give')}: {__( 'We strongly recommend you create a complete backup of your WordPress files and database prior to performing an update. We are not responsible for any misuse, deletions, white screens, fatal errors, or any other issue resulting from the use of this plugin.', - 'give' + 'give', )} @@ -276,13 +286,17 @@ const Migrations = () => { const columnFilters = { status: (type) =>