diff --git a/src/app/Jobs/CommonJob.php b/src/app/Jobs/CommonJob.php new file mode 100644 index 00000000..6d4602d8 --- /dev/null +++ b/src/app/Jobs/CommonJob.php @@ -0,0 +1,73 @@ +handle(); + * ``` + */ +abstract class CommonJob implements ShouldQueue +{ + use Dispatchable; + use InteractsWithQueue; + use Queueable; + + /** + * The failure message. + * + * @var string + */ + public $failureMessage; + + /** + * The number of tries for this Job. + * + * @var int + */ + public $tries = 5; + + /** + * Execute the job. + * + * @return void + */ + abstract public function handle(); + + /** + * Delete the job, call the "failed" method, and raise the failed job event. + * + * @param \Throwable|null $e An Exception + * + * @return void + */ + public function fail($e = null) + { + // Save the message, for testing purposes + $this->failureMessage = $e->getMessage(); + + // @phpstan-ignore-next-line + if ($this->job) { + $this->job->fail($e); + } + } + + /** + * Check if the job has failed + * + * @return bool + */ + public function hasFailed(): bool + { + return $this->failureMessage !== null; + } +} diff --git a/src/app/Jobs/Domain/DeleteJob.php b/src/app/Jobs/Domain/DeleteJob.php index 859ea5dd..6f308f1c 100644 --- a/src/app/Jobs/Domain/DeleteJob.php +++ b/src/app/Jobs/Domain/DeleteJob.php @@ -1,28 +1,29 @@ getDomain(); // sanity checks if ($domain->isDeleted()) { $this->fail(new \Exception("Domain {$this->domainId} is already marked as deleted.")); + return; } \App\Backends\LDAP::deleteDomain($domain); $domain->status |= \App\Domain::STATUS_DELETED; $domain->save(); } } diff --git a/src/app/Jobs/DomainJob.php b/src/app/Jobs/DomainJob.php index eabb3bf0..98d54fcb 100644 --- a/src/app/Jobs/DomainJob.php +++ b/src/app/Jobs/DomainJob.php @@ -1,89 +1,66 @@ handle(); * ``` */ -abstract class DomainJob implements ShouldQueue +abstract class DomainJob extends CommonJob { - use Dispatchable; - use InteractsWithQueue; - use Queueable; - /** * The ID for the \App\Domain. This is the shortest globally unique identifier and saves Redis space * compared to a serialized version of the complete \App\Domain object. * * @var int */ protected $domainId; /** * The \App\Domain namespace property, for legibility in the queue management. * * @var string */ protected $domainNamespace; - /** - * The number of tries for this Job. - * - * @var int - */ - public $tries = 5; - /** * Create a new job instance. * * @param int $domainId The ID for the user to create. * * @return void */ public function __construct(int $domainId) { $this->domainId = $domainId; $domain = $this->getDomain(); if ($domain) { $this->domainNamespace = $domain->namespace; } } - /** - * Execute the job. - * - * @return void - */ - abstract public function handle(); - /** * Get the \App\Domain entry associated with this job. * * @return \App\Domain|null * * @throws \Exception */ protected function getDomain() { $domain = \App\Domain::withTrashed()->find($this->domainId); if (!$domain) { $this->fail(new \Exception("Domain {$this->domainId} could not be found in the database.")); } return $domain; } } diff --git a/src/app/Jobs/User/CreateJob.php b/src/app/Jobs/User/CreateJob.php index be38e1e3..551d4c8a 100644 --- a/src/app/Jobs/User/CreateJob.php +++ b/src/app/Jobs/User/CreateJob.php @@ -1,64 +1,73 @@ isDeleted()`), or * * the user is actually deleted (`$user->deleted_at`), or * * the user is already marked as ready in LDAP (`$user->isLdapReady()`). * */ class CreateJob extends UserJob { /** * Execute the job. * * @return void * * @throws \Exception */ public function handle() { $user = $this->getUser(); + if (!$user) { + return; + } + // sanity checks if ($user->isDeleted()) { $this->fail(new \Exception("User {$this->userId} is marked as deleted.")); + return; } if ($user->deleted_at) { $this->fail(new \Exception("User {$this->userId} is actually deleted.")); + return; } if ($user->isLdapReady()) { $this->fail(new \Exception("User {$this->userId} is already marked as ldap-ready.")); + return; } // see if the domain is ready $domain = $user->domain(); if (!$domain) { $this->fail(new \Exception("The domain for {$this->userId} does not exist.")); + return; } if ($domain->isDeleted()) { $this->fail(new \Exception("The domain for {$this->userId} is marked as deleted.")); + return; } if (!$domain->isLdapReady()) { $this->release(60); return; } \App\Backends\LDAP::createUser($user); $user->status |= \App\User::STATUS_LDAP_READY; $user->save(); } } diff --git a/src/app/Jobs/User/DeleteJob.php b/src/app/Jobs/User/DeleteJob.php index 18adb808..4c5cf6a3 100644 --- a/src/app/Jobs/User/DeleteJob.php +++ b/src/app/Jobs/User/DeleteJob.php @@ -1,28 +1,33 @@ getUser(); + if (!$user) { + return; + } + // sanity checks if ($user->isDeleted()) { $this->fail(new \Exception("User {$this->userId} is already marked as deleted.")); + return; } \App\Backends\LDAP::deleteUser($user); $user->status |= \App\User::STATUS_DELETED; $user->save(); } } diff --git a/src/app/Jobs/User/VerifyJob.php b/src/app/Jobs/User/VerifyJob.php index b5a17221..e27f55ed 100644 --- a/src/app/Jobs/User/VerifyJob.php +++ b/src/app/Jobs/User/VerifyJob.php @@ -1,34 +1,40 @@ getUser(); + if (!$user) { + return; + } + // sanity checks if (!$user->hasSku('mailbox')) { $this->fail(new \Exception("User {$this->userId} has no mailbox SKU.")); + return; } // the user has a mailbox (or is marked as such) if ($user->isImapReady()) { $this->fail(new \Exception("User {$this->userId} is already verified.")); + return; } if (\App\Backends\IMAP::verifyAccount($user->email)) { $user->status |= \App\User::STATUS_IMAP_READY; $user->status |= \App\User::STATUS_ACTIVE; $user->save(); } } } diff --git a/src/app/Jobs/UserJob.php b/src/app/Jobs/UserJob.php index c9fbb7de..2e1760d3 100644 --- a/src/app/Jobs/UserJob.php +++ b/src/app/Jobs/UserJob.php @@ -1,87 +1,66 @@ handle(); * ``` */ -abstract class UserJob implements ShouldQueue +abstract class UserJob extends CommonJob { - use Dispatchable; - use InteractsWithQueue; - use Queueable; - /** * The ID for the \App\User. This is the shortest globally unique identifier and saves Redis space * compared to a serialized version of the complete \App\User object. * * @var int */ protected $userId; /** * The \App\User email property, for legibility in the queue management. * * @var string */ protected $userEmail; - /** - * The number of tries for this Job. - * - * @var int - */ - public $tries = 5; - /** * Create a new job instance. * * @param int $userId The ID for the user to create. * * @return void */ public function __construct(int $userId) { $this->userId = $userId; $user = $this->getUser(); - $this->userEmail = $user->email; + if ($user) { + $this->userEmail = $user->email; + } } - /** - * Execute the job. - * - * @return void - */ - abstract public function handle(); - /** * Get the \App\User entry associated with this job. * * @return \App\User|null * * @throws \Exception */ protected function getUser() { $user = \App\User::withTrashed()->find($this->userId); if (!$user) { $this->fail(new \Exception("User {$this->userId} could not be found in the database.")); } return $user; } } diff --git a/src/tests/Feature/Jobs/UserCreateTest.php b/src/tests/Feature/Jobs/UserCreateTest.php index 266e4345..3b21d101 100644 --- a/src/tests/Feature/Jobs/UserCreateTest.php +++ b/src/tests/Feature/Jobs/UserCreateTest.php @@ -1,42 +1,78 @@ deleteTestUser('new-job-user@' . \config('app.domain')); } public function tearDown(): void { $this->deleteTestUser('new-job-user@' . \config('app.domain')); parent::tearDown(); } /** * Test job handle * * @group ldap */ public function testHandle(): void { $user = $this->getTestUser('new-job-user@' . \config('app.domain')); $this->assertFalse($user->isLdapReady()); $job = new \App\Jobs\User\CreateJob($user->id); $job->handle(); $this->assertTrue($user->fresh()->isLdapReady()); + $this->assertFalse($job->hasFailed()); + + // Test job failures + $job = new \App\Jobs\User\CreateJob($user->id); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("User {$user->id} is already marked as ldap-ready.", $job->failureMessage); + + $user->status |= User::STATUS_DELETED; + $user->save(); + + $job = new \App\Jobs\User\CreateJob($user->id); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("User {$user->id} is marked as deleted.", $job->failureMessage); + + $user->status ^= User::STATUS_DELETED; + $user->save(); + $user->delete(); + + $job = new \App\Jobs\User\CreateJob($user->id); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("User {$user->id} is actually deleted.", $job->failureMessage); + + // TODO: Test failures on domain sanity checks + + $job = new \App\Jobs\User\CreateJob(123); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("User 123 could not be found in the database.", $job->failureMessage); } }