Merge pull request #2758 from woocommerce/tweak/conditionally-load-jo…

…b-classes Conditionally load job classes
woocommerce · Jan 14, 2025 · f55f783 · f55f783
2 parents 733b466 + 04102b6
commit f55f783
Show file tree

Hide file tree

Showing 19 changed files with 297 additions and 165 deletions.
diff --git a/src/API/Site/Controllers/GTINMigrationController.php b/src/API/Site/Controllers/GTINMigrationController.php
@@ -5,10 +5,10 @@
 
 use Automattic\WooCommerce\GoogleListingsAndAds\API\TransportMethods;
 use Automattic\WooCommerce\GoogleListingsAndAds\HelperTraits\GTINMigrationUtilities;
+use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\MigrateGTIN;
 use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\RESTServer;
 use Exception;
-use WP_REST_Request as Request;
 use WP_REST_Response as Response;
 
 defined( 'ABSPATH' ) || exit;
@@ -23,21 +23,21 @@ class GTINMigrationController extends BaseController {
 	use GTINMigrationUtilities;
 
 	/**
-	 * Job responsible to run the migration in the background.
+	 * Repository to fetch job responsible to run the migration in the background.
 	 *
-	 * @var MigrateGTIN
+	 * @var JobRepository
 	 */
-	protected $job;
+	protected $job_repository;
 
 	/**
 	 * Constructor.
 	 *
-	 * @param RESTServer  $server
-	 * @param MigrateGTIN $job
+	 * @param RESTServer    $server
+	 * @param JobRepository $job_repository
 	 */
-	public function __construct( RESTServer $server, MigrateGTIN $job ) {
+	public function __construct( RESTServer $server, JobRepository $job_repository ) {
 		parent::__construct( $server );
-		$this->job = $job;
+		$this->job_repository = $job_repository;
 	}
 
 	/**
@@ -69,9 +69,10 @@ public function register_routes(): void {
 	 * @return callable
 	 */
 	protected function start_migration_callback(): callable {
-		return function ( Request $request ) {
+		return function () {
 			try {
-				if ( ! $this->job->can_schedule( [ 1 ] ) ) {
+				$job = $this->job_repository->get( MigrateGTIN::class );
+				if ( ! $job->can_schedule( [ 1 ] ) ) {
 					return new Response(
 						[
 							'status'  => 'error',
@@ -81,7 +82,7 @@ protected function start_migration_callback(): callable {
 					);
 				}
 
-				$this->job->schedule();
+				$job->schedule();
 				return new Response(
 					[
 						'status'  => 'success',

diff --git a/src/ConnectionTest.php b/src/ConnectionTest.php
@@ -26,6 +26,7 @@
 use Automattic\WooCommerce\GoogleListingsAndAds\Internal\Interfaces\ContainerAwareInterface;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\CleanupProductsJob;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\DeleteAllProducts;
+use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\MigrateGTIN;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateAllProducts;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\UpdateProducts;
@@ -1228,7 +1229,7 @@ function ( $url ) {
 				} else {
 					// schedule a job
 					/** @var UpdateProducts $update_job */
-					$update_job = $this->container->get( UpdateProducts::class );
+					$update_job = $this->container->get( JobRepository::class )->get( UpdateProducts::class );
 					$update_job->schedule( [ [ $product->get_id() ] ] );
 					$this->response = 'Successfully scheduled a job to sync the product ' . $product->get_id();
 				}
@@ -1262,7 +1263,7 @@ function ( $url ) {
 			} else {
 				// schedule a job
 				/** @var UpdateAllProducts $update_job */
-				$update_job = $this->container->get( UpdateAllProducts::class );
+				$update_job = $this->container->get( JobRepository::class )->get( UpdateAllProducts::class );
 				$update_job->schedule();
 				$this->response = 'Successfully scheduled a job to sync all products!';
 			}
@@ -1293,7 +1294,7 @@ function ( $url ) {
 			} else {
 				// schedule a job
 				/** @var DeleteAllProducts $delete_job */
-				$delete_job = $this->container->get( DeleteAllProducts::class );
+				$delete_job = $this->container->get( JobRepository::class )->get( DeleteAllProducts::class );
 				$delete_job->schedule();
 				$this->response = 'Successfully scheduled a job to delete all synced products!';
 			}
@@ -1327,15 +1328,15 @@ function ( $url ) {
 			} else {
 				// schedule a job
 				/** @var CleanupProductsJob $delete_job */
-				$delete_job = $this->container->get( CleanupProductsJob::class );
+				$delete_job = $this->container->get( JobRepository::class )->get( CleanupProductsJob::class );
 				$delete_job->schedule();
 				$this->response = 'Successfully scheduled a job to cleanup all products!';
 			}
 		}
 
 		if ( 'migrate-gtin' === $_GET['action'] && check_admin_referer( 'migrate-gtin' ) ) {
 			/** @var MigrateGTIN $job */
-			$job = $this->container->get( MigrateGTIN::class );
+			$job = $this->container->get( JobRepository::class )->get( MigrateGTIN::class );
 			$job->schedule();
 			$this->response = 'Successfully scheduled a job to migrate GTIN';
 		}

diff --git a/src/Coupon/SyncerHooks.php b/src/Coupon/SyncerHooks.php
@@ -56,21 +56,9 @@ class SyncerHooks implements Service, Registerable {
 	protected $coupon_helper;
 
 	/**
-	 *
-	 * @var UpdateCoupon
+	 * @var JobRepository
 	 */
-	protected $update_coupon_job;
-
-	/**
-	 *
-	 * @var DeleteCoupon
-	 */
-	protected $delete_coupon_job;
-
-	/**
-	 * @var CouponNotificationJob
-	 */
-	protected $coupon_notification_job;
+	protected $job_repository;
 
 	/**
 	 *
@@ -83,7 +71,6 @@ class SyncerHooks implements Service, Registerable {
 	 */
 	protected $notifications_service;
 
-
 	/**
 	 *
 	 * @var WC
@@ -115,14 +102,12 @@ public function __construct(
 		WC $wc,
 		WP $wp
 	) {
-		$this->update_coupon_job       = $job_repository->get( UpdateCoupon::class );
-		$this->delete_coupon_job       = $job_repository->get( DeleteCoupon::class );
-		$this->coupon_notification_job = $job_repository->get( CouponNotificationJob::class );
-		$this->coupon_helper           = $coupon_helper;
-		$this->merchant_center         = $merchant_center;
-		$this->notifications_service   = $notifications_service;
-		$this->wc                      = $wc;
-		$this->wp                      = $wp;
+		$this->coupon_helper         = $coupon_helper;
+		$this->job_repository        = $job_repository;
+		$this->merchant_center       = $merchant_center;
+		$this->notifications_service = $notifications_service;
+		$this->wc                    = $wc;
+		$this->wp                    = $wp;
 	}
 
 	/**
@@ -216,7 +201,7 @@ protected function handle_update_coupon( WC_Coupon $coupon ) {
 		// Schedule an update job if product sync is enabled.
 		if ( $this->coupon_helper->is_sync_ready( $coupon ) ) {
 			$this->coupon_helper->mark_as_pending( $coupon );
-			$this->update_coupon_job->schedule(
+			$this->job_repository->get( UpdateCoupon::class )->schedule(
 				[
 					[ $coupon_id ],
 				]
@@ -228,7 +213,7 @@ protected function handle_update_coupon( WC_Coupon $coupon ) {
 				$this->get_coupon_to_delete( $coupon ),
 				$this->coupon_helper->get_synced_google_ids( $coupon )
 			);
-			$this->delete_coupon_job->schedule(
+			$this->job_repository->get( DeleteCoupon::class )->schedule(
 				[
 					$coupon_to_delete,
 				]
@@ -303,7 +288,7 @@ protected function handle_delete_coupon( int $coupon_id ) {
 		$coupon_to_delete = $this->delete_requests_map[ $coupon_id ];
 		if ( ! empty( $coupon_to_delete->get_synced_google_ids() ) &&
 				! $this->is_already_scheduled_to_delete( $coupon_id ) ) {
-			$this->delete_coupon_job->schedule(
+			$this->job_repository->get( DeleteCoupon::class )->schedule(
 				[
 					$coupon_to_delete,
 				]
@@ -323,7 +308,7 @@ protected function maybe_send_delete_notification( int $coupon_id ): void {
 
 		if ( $coupon instanceof WC_Coupon && $this->coupon_helper->should_trigger_delete_notification( $coupon ) ) {
 			$this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_DELETE );
-			$this->coupon_notification_job->schedule(
+			$this->job_repository->get( CouponNotificationJob::class )->schedule(
 				[
 					'item_id' => $coupon->get_id(),
 					'topic'   => NotificationsService::TOPIC_COUPON_DELETED,
@@ -415,23 +400,23 @@ protected function set_already_scheduled_to_delete( int $coupon_id ): void {
 	protected function handle_update_coupon_notification( WC_Coupon $coupon ) {
 		if ( $this->coupon_helper->should_trigger_create_notification( $coupon ) ) {
 			$this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_CREATE );
-			$this->coupon_notification_job->schedule(
+			$this->job_repository->get( CouponNotificationJob::class )->schedule(
 				[
 					'item_id' => $coupon->get_id(),
 					'topic'   => NotificationsService::TOPIC_COUPON_CREATED,
 				]
 			);
 		} elseif ( $this->coupon_helper->should_trigger_update_notification( $coupon ) ) {
 			$this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_UPDATE );
-			$this->coupon_notification_job->schedule(
+			$this->job_repository->get( CouponNotificationJob::class )->schedule(
 				[
 					'item_id' => $coupon->get_id(),
 					'topic'   => NotificationsService::TOPIC_COUPON_UPDATED,
 				]
 			);
 		} elseif ( $this->coupon_helper->should_trigger_delete_notification( $coupon ) ) {
 			$this->coupon_helper->set_notification_status( $coupon, NotificationStatus::NOTIFICATION_PENDING_DELETE );
-			$this->coupon_notification_job->schedule(
+			$this->job_repository->get( CouponNotificationJob::class )->schedule(
 				[
 					'item_id' => $coupon->get_id(),
 					'topic'   => NotificationsService::TOPIC_COUPON_DELETED,

diff --git a/src/Infrastructure/GoogleListingsAndAdsPlugin.php b/src/Infrastructure/GoogleListingsAndAdsPlugin.php
@@ -96,7 +96,8 @@ function () {
 		add_action(
 			'init',
 			function () {
-				// register the job initializer only if it is available. see JobInitializer::is_needed.
+				// Register the job initializer only if it is available, see JobInitializer::is_needed.
+				// Note: ActionScheduler must be loaded after the init hook, so we can't load JobInitializer like a regular Service.
 				if ( $this->container->has( JobInitializer::class ) ) {
 					$this->container->get( JobInitializer::class )->register();
 				}

diff --git a/src/Internal/DependencyManagement/JobServiceProvider.php b/src/Internal/DependencyManagement/JobServiceProvider.php
@@ -136,10 +136,7 @@ public function register(): void {
 			Product\Attributes\AttributeManager::class
 		);
 
-		$this->share_with_tags(
-			JobRepository::class,
-			JobInterface::class
-		);
+		$this->share_with_tags( JobRepository::class );
 		$this->conditionally_share_with_tags(
 			JobInitializer::class,
 			JobRepository::class,

diff --git a/src/Internal/DependencyManagement/RESTServiceProvider.php b/src/Internal/DependencyManagement/RESTServiceProvider.php
@@ -61,7 +61,6 @@
 use Automattic\WooCommerce\GoogleListingsAndAds\Google\RequestReviewStatuses;
 use Automattic\WooCommerce\GoogleListingsAndAds\Google\GoogleHelper;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\JobRepository;
-use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\MigrateGTIN;
 use Automattic\WooCommerce\GoogleListingsAndAds\Jobs\ProductSyncStats;
 use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\AccountService as MerchantAccountService;
 use Automattic\WooCommerce\GoogleListingsAndAds\MerchantCenter\MerchantStatuses;
@@ -143,7 +142,7 @@ public function register(): void {
 		$this->share( AttributeMappingSyncerController::class, ProductSyncStats::class );
 		$this->share( TourController::class );
 		$this->share( RestAPIAuthController::class, OAuthService::class, MerchantAccountService::class );
-		$this->share( GTINMigrationController::class, MigrateGTIN::class );
+		$this->share( GTINMigrationController::class, JobRepository::class );
 	}
 
 	/**

diff --git a/src/Jobs/JobException.php b/src/Jobs/JobException.php
@@ -59,18 +59,18 @@ public static function stopped_due_to_high_failure_rate( string $job_name ): Job
 	}
 
 	/**
-	 * Create a new exception instance for when a job is stopped due to a high failure rate.
+	 * Create a new exception instance for when a job class is not found.
 	 *
-	 * @param string $job_name
+	 * @param string $job_classname
 	 *
 	 * @return static
 	 */
-	public static function job_does_not_exist( string $job_name ): JobException {
+	public static function job_does_not_exist( string $job_classname ): JobException {
 		return new static(
 			sprintf(
-				/* translators: %s: the job name */
-				__( 'The job named "%s" does not exist.', 'google-listings-and-ads' ),
-				$job_name
+				/* translators: %s: the job classname */
+				__( 'The job "%s" does not exist.', 'google-listings-and-ads' ),
+				$job_classname
 			)
 		);
 	}

diff --git a/src/Jobs/JobInterface.php b/src/Jobs/JobInterface.php
@@ -3,7 +3,6 @@
 
 namespace Automattic\WooCommerce\GoogleListingsAndAds\Jobs;
 
-use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service;
 use Automattic\WooCommerce\GoogleListingsAndAds\Internal\DependencyManagement\JobServiceProvider;
 
 defined( 'ABSPATH' ) || exit;
@@ -17,7 +16,7 @@
  *
  * @package Automattic\WooCommerce\GoogleListingsAndAds\Jobs
  */
-interface JobInterface extends Service {
+interface JobInterface {
 
 	/**
 	 * Get the name of the job.