diff --git a/src/app/Jobs/CommonJob.php b/src/app/Jobs/CommonJob.php new file mode 100644 --- /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 --- a/src/app/Jobs/Domain/DeleteJob.php +++ b/src/app/Jobs/Domain/DeleteJob.php @@ -18,6 +18,7 @@ // sanity checks if ($domain->isDeleted()) { $this->fail(new \Exception("Domain {$this->domainId} is already marked as deleted.")); + return; } \App\Backends\LDAP::deleteDomain($domain); diff --git a/src/app/Jobs/DomainJob.php b/src/app/Jobs/DomainJob.php --- a/src/app/Jobs/DomainJob.php +++ b/src/app/Jobs/DomainJob.php @@ -2,11 +2,6 @@ namespace App\Jobs; -use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; -use Illuminate\Foundation\Bus\Dispatchable; -use Illuminate\Queue\InteractsWithQueue; - /** * The abstract \App\Jobs\DomainJob implements the logic needed for all dispatchable Jobs related to * \App\Domain objects. @@ -16,12 +11,8 @@ * $job->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. @@ -38,13 +29,6 @@ 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. @@ -63,13 +47,6 @@ } /** - * Execute the job. - * - * @return void - */ - abstract public function handle(); - - /** * Get the \App\Domain entry associated with this job. * * @return \App\Domain|null diff --git a/src/app/Jobs/User/CreateJob.php b/src/app/Jobs/User/CreateJob.php --- a/src/app/Jobs/User/CreateJob.php +++ b/src/app/Jobs/User/CreateJob.php @@ -27,17 +27,24 @@ { $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 @@ -45,10 +52,12 @@ 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()) { diff --git a/src/app/Jobs/User/DeleteJob.php b/src/app/Jobs/User/DeleteJob.php --- a/src/app/Jobs/User/DeleteJob.php +++ b/src/app/Jobs/User/DeleteJob.php @@ -15,9 +15,14 @@ { $user = $this->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); diff --git a/src/app/Jobs/User/VerifyJob.php b/src/app/Jobs/User/VerifyJob.php --- a/src/app/Jobs/User/VerifyJob.php +++ b/src/app/Jobs/User/VerifyJob.php @@ -15,14 +15,20 @@ { $user = $this->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)) { diff --git a/src/app/Jobs/UserJob.php b/src/app/Jobs/UserJob.php --- a/src/app/Jobs/UserJob.php +++ b/src/app/Jobs/UserJob.php @@ -2,11 +2,6 @@ namespace App\Jobs; -use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; -use Illuminate\Foundation\Bus\Dispatchable; -use Illuminate\Queue\InteractsWithQueue; - /** * The abstract \App\Jobs\UserJob implements the logic needed for all dispatchable Jobs related to * \App\User objects. @@ -16,12 +11,8 @@ * $job->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. @@ -38,13 +29,6 @@ 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. @@ -57,17 +41,12 @@ $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 diff --git a/src/tests/Feature/Jobs/UserCreateTest.php b/src/tests/Feature/Jobs/UserCreateTest.php --- a/src/tests/Feature/Jobs/UserCreateTest.php +++ b/src/tests/Feature/Jobs/UserCreateTest.php @@ -2,6 +2,7 @@ namespace Tests\Feature\Jobs; +use App\User; use Tests\TestCase; class UserCreateTest extends TestCase @@ -38,5 +39,40 @@ $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); } }