diff --git a/src/app/Jobs/CommonJob.php b/src/app/Jobs/CommonJob.php index 6d4602d8..d0c2b001 100644 --- a/src/app/Jobs/CommonJob.php +++ b/src/app/Jobs/CommonJob.php @@ -1,73 +1,107 @@ handle(); * ``` */ abstract class CommonJob implements ShouldQueue { use Dispatchable; use InteractsWithQueue; use Queueable; /** * The failure message. * * @var string */ public $failureMessage; + /** + * The job released state. + * + * @var bool + */ + protected $isReleased = false; + /** * 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; } + + /** + * Release the job back into the queue. + * + * @param int $delay Time in seconds + * @return void + */ + public function release($delay = 0) + { + // We need this for testing purposes + $this->isReleased = true; + + // @phpstan-ignore-next-line + if ($this->job) { + $this->job->release($delay); + } + } + + /** + * Check if the job was released + * + * @return bool + */ + public function isReleased(): bool + { + return $this->isReleased; + } } diff --git a/src/app/Jobs/Domain/CreateJob.php b/src/app/Jobs/Domain/CreateJob.php index 490fee5b..cbe90e97 100644 --- a/src/app/Jobs/Domain/CreateJob.php +++ b/src/app/Jobs/Domain/CreateJob.php @@ -1,27 +1,31 @@ getDomain(); + if (!$domain) { + return; + } + if (!$domain->isLdapReady()) { \App\Backends\LDAP::createDomain($domain); $domain->status |= \App\Domain::STATUS_LDAP_READY; $domain->save(); \App\Jobs\Domain\VerifyJob::dispatch($domain->id); } } } diff --git a/src/app/Jobs/Domain/DeleteJob.php b/src/app/Jobs/Domain/DeleteJob.php index 0b181131..5e070223 100644 --- a/src/app/Jobs/Domain/DeleteJob.php +++ b/src/app/Jobs/Domain/DeleteJob.php @@ -1,34 +1,38 @@ getDomain(); + if (!$domain) { + return; + } + // 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; if ($domain->isLdapReady()) { $domain->status ^= \App\Domain::STATUS_LDAP_READY; } $domain->save(); } } diff --git a/src/app/Jobs/Domain/UpdateJob.php b/src/app/Jobs/Domain/UpdateJob.php index f939dfe4..de84fe08 100644 --- a/src/app/Jobs/Domain/UpdateJob.php +++ b/src/app/Jobs/Domain/UpdateJob.php @@ -1,25 +1,29 @@ getDomain(); + if (!$domain) { + return; + } + if (!$domain->isLdapReady()) { $this->delete(); return; } \App\Backends\LDAP::updateDomain($domain); } } diff --git a/src/app/Jobs/Domain/VerifyJob.php b/src/app/Jobs/Domain/VerifyJob.php index af66ced3..00f1c1f9 100644 --- a/src/app/Jobs/Domain/VerifyJob.php +++ b/src/app/Jobs/Domain/VerifyJob.php @@ -1,20 +1,24 @@ getDomain(); + if (!$domain) { + return; + } + $domain->verify(); } } diff --git a/src/app/Jobs/DomainJob.php b/src/app/Jobs/DomainJob.php index 514d146d..d8cf3d3a 100644 --- a/src/app/Jobs/DomainJob.php +++ b/src/app/Jobs/DomainJob.php @@ -1,66 +1,73 @@ handle(); * ``` */ abstract class DomainJob extends CommonJob { /** * 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; /** * Create a new job instance. * * @param int $domainId The ID for the domain to create. * * @return void */ public function __construct(int $domainId) { $this->domainId = $domainId; $domain = $this->getDomain(); if ($domain) { $this->domainNamespace = $domain->namespace; } } /** * 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) { + // The record might not exist yet in case of a db replication environment + // This will release the job and delay another attempt for 5 seconds + if ($this instanceof Domain\CreateJob) { + $this->release(5); + return null; + } + $this->fail(new \Exception("Domain {$this->domainId} could not be found in the database.")); } return $domain; } } diff --git a/src/app/Jobs/Group/CreateJob.php b/src/app/Jobs/Group/CreateJob.php index ed673d8d..4437a4a2 100644 --- a/src/app/Jobs/Group/CreateJob.php +++ b/src/app/Jobs/Group/CreateJob.php @@ -1,25 +1,29 @@ getGroup(); + if (!$group) { + return; + } + if (!$group->isLdapReady()) { \App\Backends\LDAP::createGroup($group); $group->status |= \App\Group::STATUS_LDAP_READY; $group->save(); } } } diff --git a/src/app/Jobs/Group/DeleteJob.php b/src/app/Jobs/Group/DeleteJob.php index beae2557..2d550693 100644 --- a/src/app/Jobs/Group/DeleteJob.php +++ b/src/app/Jobs/Group/DeleteJob.php @@ -1,34 +1,38 @@ getGroup(); + if (!$group) { + return; + } + // sanity checks if ($group->isDeleted()) { $this->fail(new \Exception("Group {$this->groupId} is already marked as deleted.")); return; } \App\Backends\LDAP::deleteGroup($group); $group->status |= \App\Group::STATUS_DELETED; if ($group->isLdapReady()) { $group->status ^= \App\Group::STATUS_LDAP_READY; } $group->save(); } } diff --git a/src/app/Jobs/Group/UpdateJob.php b/src/app/Jobs/Group/UpdateJob.php index 1154ae10..c6529765 100644 --- a/src/app/Jobs/Group/UpdateJob.php +++ b/src/app/Jobs/Group/UpdateJob.php @@ -1,25 +1,29 @@ getGroup(); + if (!$group) { + return; + } + if (!$group->isLdapReady()) { $this->delete(); return; } \App\Backends\LDAP::updateGroup($group); } } diff --git a/src/app/Jobs/GroupJob.php b/src/app/Jobs/GroupJob.php index 7e63ef1c..80a5db8e 100644 --- a/src/app/Jobs/GroupJob.php +++ b/src/app/Jobs/GroupJob.php @@ -1,66 +1,73 @@ handle(); * ``` */ abstract class GroupJob extends CommonJob { /** * The ID for the \App\Group. This is the shortest globally unique identifier and saves Redis space * compared to a serialized version of the complete \App\Group object. * * @var int */ protected $groupId; /** * The \App\Group email property, for legibility in the queue management. * * @var string */ protected $groupEmail; /** * Create a new job instance. * * @param int $groupId The ID for the group to create. * * @return void */ public function __construct(int $groupId) { $this->groupId = $groupId; $group = $this->getGroup(); if ($group) { $this->groupEmail = $group->email; } } /** * Get the \App\Group entry associated with this job. * * @return \App\Group|null * * @throws \Exception */ protected function getGroup() { $group = \App\Group::withTrashed()->find($this->groupId); if (!$group) { + // The record might not exist yet in case of a db replication environment + // This will release the job and delay another attempt for 5 seconds + if ($this instanceof Group\CreateJob) { + $this->release(5); + return null; + } + $this->fail(new \Exception("Group {$this->groupId} could not be found in the database.")); } return $group; } } diff --git a/src/app/Jobs/User/UpdateJob.php b/src/app/Jobs/User/UpdateJob.php index 2ed454e2..29b0acec 100644 --- a/src/app/Jobs/User/UpdateJob.php +++ b/src/app/Jobs/User/UpdateJob.php @@ -1,25 +1,29 @@ getUser(); + if (!$user) { + return; + } + if (!$user->isLdapReady()) { $this->delete(); return; } \App\Backends\LDAP::updateUser($user); } } diff --git a/src/app/Jobs/UserJob.php b/src/app/Jobs/UserJob.php index 2e1760d3..149540da 100644 --- a/src/app/Jobs/UserJob.php +++ b/src/app/Jobs/UserJob.php @@ -1,66 +1,73 @@ handle(); * ``` */ abstract class UserJob extends CommonJob { /** * 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; /** * 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(); if ($user) { $this->userEmail = $user->email; } } /** * 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) { + // The record might not exist yet in case of a db replication environment + // This will release the job and delay another attempt for 5 seconds + if ($this instanceof User\CreateJob) { + $this->release(5); + return null; + } + $this->fail(new \Exception("User {$this->userId} could not be found in the database.")); } return $user; } } diff --git a/src/tests/Feature/Jobs/DomainCreateTest.php b/src/tests/Feature/Jobs/DomainCreateTest.php index 1fb0eb85..f369362f 100644 --- a/src/tests/Feature/Jobs/DomainCreateTest.php +++ b/src/tests/Feature/Jobs/DomainCreateTest.php @@ -1,67 +1,74 @@ deleteTestDomain('domain-create-test.com'); } public function tearDown(): void { $this->deleteTestDomain('domain-create-test.com'); parent::tearDown(); } /** * Test job handle * * @group ldap */ public function testHandle(): void { $domain = $this->getTestDomain( 'domain-create-test.com', [ 'status' => Domain::STATUS_NEW, 'type' => Domain::TYPE_EXTERNAL, ] ); $this->assertFalse($domain->isLdapReady()); // Fake the queue, assert that no jobs were pushed... Queue::fake(); Queue::assertNothingPushed(); $job = new \App\Jobs\Domain\CreateJob($domain->id); $job->handle(); $this->assertTrue($domain->fresh()->isLdapReady()); Queue::assertPushed(\App\Jobs\Domain\VerifyJob::class, 1); Queue::assertPushed( \App\Jobs\Domain\VerifyJob::class, function ($job) use ($domain) { $domainId = TestCase::getObjectProperty($job, 'domainId'); $domainNamespace = TestCase::getObjectProperty($job, 'domainNamespace'); return $domainId === $domain->id && $domainNamespace === $domain->namespace; } ); + + // Test job releasing on unknown identifier + $job = new \App\Jobs\Domain\CreateJob(123); + $job->handle(); + + $this->assertTrue($job->isReleased()); + $this->assertFalse($job->hasFailed()); } } diff --git a/src/tests/Feature/Jobs/DomainVerifyTest.php b/src/tests/Feature/Jobs/DomainVerifyTest.php index 3da4e606..f0e12d5f 100644 --- a/src/tests/Feature/Jobs/DomainVerifyTest.php +++ b/src/tests/Feature/Jobs/DomainVerifyTest.php @@ -1,74 +1,81 @@ deleteTestDomain('gmail.com'); $this->deleteTestDomain('some-non-existing-domain.fff'); } public function tearDown(): void { $this->deleteTestDomain('gmail.com'); $this->deleteTestDomain('some-non-existing-domain.fff'); parent::tearDown(); } /** * Test job handle (existing domain) * * @group dns */ public function testHandle(): void { $domain = $this->getTestDomain( 'gmail.com', [ 'status' => Domain::STATUS_NEW, 'type' => Domain::TYPE_EXTERNAL, ] ); $this->assertFalse($domain->isVerified()); $job = new \App\Jobs\Domain\VerifyJob($domain->id); $job->handle(); $this->assertTrue($domain->fresh()->isVerified()); + + // Test non-existing domain ID + $job = new \App\Jobs\Domain\VerifyJob(123); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("Domain 123 could not be found in the database.", $job->failureMessage); } /** * Test job handle (non-existing domain) * * @group dns */ public function testHandleNonExisting(): void { $domain = $this->getTestDomain( 'some-non-existing-domain.fff', [ 'status' => Domain::STATUS_NEW, 'type' => Domain::TYPE_EXTERNAL, ] ); $this->assertFalse($domain->isVerified()); $job = new \App\Jobs\Domain\VerifyJob($domain->id); $job->handle(); $this->assertFalse($domain->fresh()->isVerified()); } } diff --git a/src/tests/Feature/Jobs/Group/CreateTest.php b/src/tests/Feature/Jobs/Group/CreateTest.php index 092df4ff..99b87a3b 100644 --- a/src/tests/Feature/Jobs/Group/CreateTest.php +++ b/src/tests/Feature/Jobs/Group/CreateTest.php @@ -1,43 +1,50 @@ deleteTestGroup('group@kolab.org'); } public function tearDown(): void { $this->deleteTestGroup('group@kolab.org'); parent::tearDown(); } /** * Test job handle * * @group ldap */ public function testHandle(): void { $group = $this->getTestGroup('group@kolab.org', ['members' => []]); $this->assertFalse($group->isLdapReady()); $job = new \App\Jobs\Group\CreateJob($group->id); $job->handle(); $this->assertTrue($group->fresh()->isLdapReady()); + + // Test non-existing group ID + $job = new \App\Jobs\Group\CreateJob(123); + $job->handle(); + + $this->assertTrue($job->isReleased()); + $this->assertFalse($job->hasFailed()); } } diff --git a/src/tests/Feature/Jobs/Group/DeleteTest.php b/src/tests/Feature/Jobs/Group/DeleteTest.php index f195a0f9..1a716173 100644 --- a/src/tests/Feature/Jobs/Group/DeleteTest.php +++ b/src/tests/Feature/Jobs/Group/DeleteTest.php @@ -1,53 +1,60 @@ deleteTestGroup('group@kolab.org'); } public function tearDown(): void { $this->deleteTestGroup('group@kolab.org'); parent::tearDown(); } /** * Test job handle * * @group ldap */ public function testHandle(): void { $group = $this->getTestGroup('group@kolab.org', [ 'members' => [], 'status' => Group::STATUS_NEW ]); // create to domain first $job = new \App\Jobs\Group\CreateJob($group->id); $job->handle(); $this->assertTrue($group->fresh()->isLdapReady()); $job = new \App\Jobs\Group\DeleteJob($group->id); $job->handle(); $group->refresh(); $this->assertFalse($group->isLdapReady()); $this->assertTrue($group->isDeleted()); + + // Test non-existing group ID + $job = new \App\Jobs\Group\DeleteJob(123); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("Group 123 could not be found in the database.", $job->failureMessage); } } diff --git a/src/tests/Feature/Jobs/Group/UpdateTest.php b/src/tests/Feature/Jobs/Group/UpdateTest.php index dce3cbd9..723cbb68 100644 --- a/src/tests/Feature/Jobs/Group/UpdateTest.php +++ b/src/tests/Feature/Jobs/Group/UpdateTest.php @@ -1,42 +1,49 @@ deleteTestGroup('group@kolab.org'); } public function tearDown(): void { $this->deleteTestGroup('group@kolab.org'); parent::tearDown(); } /** * Test job handle * * @group ldap */ public function testHandle(): void { $group = $this->getTestGroup('group@kolab.org', ['members' => []]); $job = new \App\Jobs\Group\UpdateJob($group->id); $job->handle(); // TODO: Test if group properties (members) actually changed in LDAP $this->assertTrue(true); + + // Test non-existing group ID + $job = new \App\Jobs\Group\UpdateJob(123); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("Group 123 could not be found in the database.", $job->failureMessage); } } diff --git a/src/tests/Feature/Jobs/UserCreateTest.php b/src/tests/Feature/Jobs/UserCreateTest.php index 3b21d101..0a67c562 100644 --- a/src/tests/Feature/Jobs/UserCreateTest.php +++ b/src/tests/Feature/Jobs/UserCreateTest.php @@ -1,78 +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); + $this->assertTrue($job->isReleased()); + $this->assertFalse($job->hasFailed()); } } diff --git a/src/tests/Feature/Jobs/UserUpdateTest.php b/src/tests/Feature/Jobs/UserUpdateTest.php index ab57a564..27d371a3 100644 --- a/src/tests/Feature/Jobs/UserUpdateTest.php +++ b/src/tests/Feature/Jobs/UserUpdateTest.php @@ -1,87 +1,94 @@ deleteTestUser('new-job-user@' . \config('app.domain')); } /** * {@inheritDoc} */ public function tearDown(): void { $this->deleteTestUser('new-job-user@' . \config('app.domain')); parent::tearDown(); } /** * Test job handle * * @group ldap */ public function testHandle(): void { // Ignore any jobs created here (e.g. on setAliases() use) Queue::fake(); $user = $this->getTestUser('new-job-user@' . \config('app.domain')); // Create the user in LDAP $job = new \App\Jobs\User\CreateJob($user->id); $job->handle(); // Test setting two aliases $aliases = [ 'new-job-user1@' . \config('app.domain'), 'new-job-user2@' . \config('app.domain'), ]; $user->setAliases($aliases); $job = new \App\Jobs\User\UpdateJob($user->id); $job->handle(); $ldap_user = LDAP::getUser('new-job-user@' . \config('app.domain')); $this->assertSame($aliases, $ldap_user['alias']); // Test updating aliases list $aliases = [ 'new-job-user1@' . \config('app.domain'), ]; $user->setAliases($aliases); $job = new \App\Jobs\User\UpdateJob($user->id); $job->handle(); $ldap_user = LDAP::getUser('new-job-user@' . \config('app.domain')); $this->assertSame($aliases, (array) $ldap_user['alias']); // Test unsetting aliases list $aliases = []; $user->setAliases($aliases); $job = new \App\Jobs\User\UpdateJob($user->id); $job->handle(); $ldap_user = LDAP::getUser('new-job-user@' . \config('app.domain')); $this->assertTrue(empty($ldap_user['alias'])); + + // Test non-existing user ID + $job = new \App\Jobs\User\UpdateJob(123); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("User 123 could not be found in the database.", $job->failureMessage); } } diff --git a/src/tests/Feature/Jobs/UserVerifyTest.php b/src/tests/Feature/Jobs/UserVerifyTest.php index 325bcdc7..d2130c18 100644 --- a/src/tests/Feature/Jobs/UserVerifyTest.php +++ b/src/tests/Feature/Jobs/UserVerifyTest.php @@ -1,67 +1,75 @@ getTestUser('ned@kolab.org'); $ned->status |= User::STATUS_IMAP_READY; $ned->save(); } /** * {@inheritDoc} */ public function tearDown(): void { $ned = $this->getTestUser('ned@kolab.org'); $ned->status |= User::STATUS_IMAP_READY; $ned->save(); parent::tearDown(); } /** * Test job handle * * @group imap */ public function testHandle(): void { Queue::fake(); + // Test non-existing user ID + $job = new \App\Jobs\User\VerifyJob(123); + $job->handle(); + + $this->assertTrue($job->hasFailed()); + $this->assertSame("User 123 could not be found in the database.", $job->failureMessage); + + // Test existing user $user = $this->getTestUser('ned@kolab.org'); if ($user->isImapReady()) { $user->status ^= User::STATUS_IMAP_READY; $user->save(); } $this->assertFalse($user->isImapReady()); for ($i = 0; $i < 10; $i++) { $job = new \App\Jobs\User\VerifyJob($user->id); $job->handle(); if ($user->fresh()->isImapReady()) { $this->assertTrue(true); return; } sleep(1); } $this->assertTrue(false, "Unable to verify the IMAP account is set up in time"); } }