Page MenuHomePhorge
Files F117871056

D4610.1775328408.diff
No OneTemporary
Actions

Authored By
Unknown
Size
11 KB
Referenced Files
None
Subscribers
None

D4610.1775328408.diff
View Options

diff --git a/src/app/Jobs/User/UpdateJob.php b/src/app/Jobs/User/UpdateJob.php
--- a/src/app/Jobs/User/UpdateJob.php
+++ b/src/app/Jobs/User/UpdateJob.php
@@ -3,9 +3,13 @@
namespace App\Jobs\User;
use App\Jobs\UserJob;
+use Illuminate\Contracts\Queue\ShouldBeUniqueUntilProcessing;
-class UpdateJob extends UserJob
+class UpdateJob extends UserJob implements ShouldBeUniqueUntilProcessing
{
+ /** @var int The number of seconds after which the job's unique lock will be released. */
+ public $uniqueFor = 60;
+
/**
* Execute the job.
*
@@ -34,4 +38,16 @@
}
}
}
+
+ /**
+ * Get the unique ID for the job.
+ *
+ * This together with ShouldBeUniqueUntilProcessing makes sure there's only one update job
+ * for the same user at the same time. E.g. if you delete 5 storage entitlements in one action,
+ * we'll reach to LDAP/IMAP backend only once (instead of five).
+ */
+ public function uniqueId(): string
+ {
+ return (string) $this->userId;
+ }
}
diff --git a/src/config/queue.php b/src/config/queue.php
--- a/src/config/queue.php
+++ b/src/config/queue.php
@@ -68,7 +68,7 @@
'queue' => env('REDIS_QUEUE', 'default'),
'retry_after' => 90,
'block_for' => null,
- 'after_commit' => false,
+ 'after_commit' => true,
],
],
diff --git a/src/tests/Feature/BillingTest.php b/src/tests/Feature/BillingTest.php
--- a/src/tests/Feature/BillingTest.php
+++ b/src/tests/Feature/BillingTest.php
@@ -3,7 +3,6 @@
namespace Tests\Feature;
use Carbon\Carbon;
-use Illuminate\Support\Facades\Queue;
use Tests\TestCase;
class BillingTest extends TestCase
diff --git a/src/tests/Feature/EntitlementTest.php b/src/tests/Feature/EntitlementTest.php
--- a/src/tests/Feature/EntitlementTest.php
+++ b/src/tests/Feature/EntitlementTest.php
@@ -50,11 +50,11 @@
$wallet = $user->wallets->first();
// Test dispatching update jobs for the user, on quota update
- Queue::fake();
+ $this->fakeQueueReset();
$user->assignSku($skuMailbox, 1, $wallet);
Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 0);
- Queue::fake();
+ $this->fakeQueueReset();
$user->assignSku($skuStorage, 1, $wallet);
Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
Queue::assertPushed(
@@ -64,11 +64,11 @@
}
);
- Queue::fake();
+ $this->fakeQueueReset();
$user->entitlements()->where('sku_id', $skuMailbox->id)->first()->delete();
Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 0);
- Queue::fake();
+ $this->fakeQueueReset();
$user->entitlements()->where('sku_id', $skuStorage->id)->first()->delete();
Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
Queue::assertPushed(
@@ -130,8 +130,6 @@
*/
public function testEntitleableTitle(): void
{
- Queue::fake();
-
$packageDomain = Package::withEnvTenantContext()->where('title', 'domain-hosting')->first();
$packageKolab = Package::withEnvTenantContext()->where('title', 'kolab')->first();
$user = $this->getTestUser('entitled-user@custom-domain.com');
diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php
--- a/src/tests/Feature/UserTest.php
+++ b/src/tests/Feature/UserTest.php
@@ -628,7 +628,7 @@
*/
public function testDegradeAndUndegrade(): void
{
- Queue::fake();
+ $this->fakeQueueReset();
// Test an account with users, domain
$userA = $this->getTestUser('UserAccountA@UserAccount.com');
@@ -660,7 +660,7 @@
$this->assertSame(7, $entitlementsB->whereDate('updated_at', $yesterday->toDateString())->count());
$this->assertSame(0, $wallet->balance);
- Queue::fake(); // reset queue state
+ $this->fakeQueueReset();
// Degrade the account/wallet owner
$userA->degrade();
@@ -696,7 +696,7 @@
$this->backdateEntitlements($entitlementsA->get(), $yesterday, Carbon::now()->subMonthsWithoutOverflow(1));
$this->backdateEntitlements($entitlementsB->get(), $yesterday, Carbon::now()->subMonthsWithoutOverflow(1));
- Queue::fake(); // reset queue state
+ $this->fakeQueueReset();
$userA->undegrade();
@@ -1133,7 +1133,7 @@
*/
public function testRestore(): void
{
- Queue::fake();
+ $this->fakeQueueReset();
// Test an account with users and domain
$userA = $this->getTestUser('UserAccountA@UserAccount.com', [
@@ -1194,7 +1194,7 @@
\App\Entitlement::withTrashed()->where('wallet_id', $wallet_id)
->update(['updated_at' => $now->subMinutes(10)]);
- Queue::fake();
+ $this->fakeQueueReset();
// Then restore it
$userA->restore();
@@ -1241,7 +1241,7 @@
*/
public function testRestrictAndUnrestrict(): void
{
- Queue::fake();
+ $this->fakeQueueReset();
// Test an account with users, domain
$user = $this->getTestUser('UserAccountA@UserAccount.com');
@@ -1274,7 +1274,7 @@
$userB->restrict();
$this->assertTrue($userB->fresh()->isRestricted());
- Queue::fake(); // reset queue state
+ $this->fakeQueueReset();
$user->refresh();
$user->unrestrict();
@@ -1289,7 +1289,7 @@
}
);
- Queue::fake(); // reset queue state
+ $this->fakeQueueReset();
$user->unrestrict(true);
@@ -1309,8 +1309,7 @@
*/
public function testSetAliases(): void
{
- Queue::fake();
- Queue::assertNothingPushed();
+ $this->fakeQueueReset();
$user = $this->getTestUser('UserAccountA@UserAccount.com');
$domain = $this->getTestDomain('UserAccount.com', [
@@ -1335,10 +1334,11 @@
$this->assertSame('useralias1@useraccount.com', $aliases[0]['alias']);
// Add another alias
+ $this->fakeQueueReset();
$user->setAliases(['UserAlias1@UserAccount.com', 'UserAlias2@UserAccount.com']);
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 2);
- Queue::assertPushed(\App\Jobs\PGP\KeyCreateJob::class, 1);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
+ Queue::assertPushed(\App\Jobs\PGP\KeyCreateJob::class, 0);
$aliases = $user->aliases()->orderBy('alias')->get();
$this->assertCount(2, $aliases);
@@ -1348,11 +1348,12 @@
$user->tenant->setSetting('pgp.enable', 1);
// Remove an alias
+ $this->fakeQueueReset();
$user->setAliases(['UserAlias1@UserAccount.com']);
$user->tenant->setSetting('pgp.enable', 0);
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 3);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
Queue::assertPushed(\App\Jobs\PGP\KeyDeleteJob::class, 1);
Queue::assertPushed(
\App\Jobs\PGP\KeyDeleteJob::class,
@@ -1368,9 +1369,10 @@
$this->assertSame('useralias1@useraccount.com', $aliases[0]['alias']);
// Remove all aliases
+ $this->fakeQueueReset();
$user->setAliases([]);
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 4);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
$this->assertCount(0, $user->aliases()->get());
}
@@ -1423,8 +1425,7 @@
*/
public function testUserSettings(): void
{
- Queue::fake();
- Queue::assertNothingPushed();
+ $this->fakeQueueReset();
$user = $this->getTestUser('UserAccountA@UserAccount.com');
@@ -1450,9 +1451,10 @@
$this->assertSame('Firstname', $user->fresh()->getSetting('first_name'));
// Update a setting
+ $this->fakeQueueReset();
$user->setSetting('first_name', 'Firstname1');
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 2);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
// Note: We test both current user as well as fresh user object
// to make sure cache works as expected
@@ -1460,9 +1462,10 @@
$this->assertSame('Firstname1', $user->fresh()->getSetting('first_name'));
// Delete a setting (null)
+ $this->fakeQueueReset();
$user->setSetting('first_name', null);
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 3);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
// Note: We test both current user as well as fresh user object
// to make sure cache works as expected
@@ -1470,10 +1473,11 @@
$this->assertSame(null, $user->fresh()->getSetting('first_name'));
// Delete a setting (empty string)
+ $this->fakeQueueReset();
$user->setSetting('first_name', 'Firstname1');
$user->setSetting('first_name', '');
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 5);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
// Note: We test both current user as well as fresh user object
// to make sure cache works as expected
@@ -1481,14 +1485,15 @@
$this->assertSame(null, $user->fresh()->getSetting('first_name'));
// Set multiple settings at once
+ $this->fakeQueueReset();
$user->setSettings([
'first_name' => 'Firstname2',
'last_name' => 'Lastname2',
'country' => null,
]);
- // TODO: This really should create a single UserUpdate job, not 3
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 7);
+ // Thanks to job locking it creates a single UserUpdate job
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
// Note: We test both current user as well as fresh user object
// to make sure cache works as expected
diff --git a/src/tests/Feature/WalletTest.php b/src/tests/Feature/WalletTest.php
--- a/src/tests/Feature/WalletTest.php
+++ b/src/tests/Feature/WalletTest.php
@@ -113,7 +113,7 @@
$this->assertTrue($user1->isRestricted());
$this->assertTrue($user2->isRestricted());
- Queue::fake();
+ $this->fakeQueueReset();
$wallet->balance = 100;
$wallet->save();
diff --git a/src/tests/TestCaseTrait.php b/src/tests/TestCaseTrait.php
--- a/src/tests/TestCaseTrait.php
+++ b/src/tests/TestCaseTrait.php
@@ -620,6 +620,24 @@
return $method->invokeArgs($object, $parameters);
}
+ /**
+ * Init fake queue. Release unique job locks.
+ */
+ protected function fakeQueueReset()
+ {
+ // Release all locks for ShouldBeUnique jobs. Works only with Redis cache.
+ $db = \cache()->getStore()->lockConnection();
+ $prefix = $db->getOptions()->prefix?->getPrefix();
+
+ foreach ($db->keys('*') as $key) {
+ if (strpos($key, 'laravel_unique_job') !== false) {
+ $db->del($prefix ? substr($key, strlen($prefix)) : $key);
+ }
+ }
+
+ Queue::fake();
+ }
+
/**
* Extract content of an email message.
*

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 4, 6:46 PM (21 h, 3 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18830798
Default Alt Text
D4610.1775328408.diff (11 KB)

Event Timeline