Page MenuHomePhorge
Files F117871064

D4610.1775328427.diff
No OneTemporary
Actions

Authored By
Unknown
Size
12 KB
Referenced Files
None
Subscribers
None

D4610.1775328427.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
@@ -52,11 +52,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(
@@ -66,12 +66,12 @@
}
);
- Queue::fake();
+ $this->fakeQueueReset();
$user->entitlements()->where('sku_id', $skuMailbox->id)->first()->delete();
//FIXME this sometimes gives 1?
Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 0);
- Queue::fake();
+ $this->fakeQueueReset();
$user->entitlements()->where('sku_id', $skuStorage->id)->first()->delete();
//FIXME this sometimes gives 2?
Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
@@ -134,8 +134,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();
@@ -1136,7 +1136,7 @@
*/
public function testRestore(): void
{
- Queue::fake();
+ $this->fakeQueueReset();
// Test an account with users and domain
$userA = $this->getTestUser('UserAccountA@UserAccount.com', [
@@ -1197,7 +1197,7 @@
\App\Entitlement::withTrashed()->where('wallet_id', $wallet_id)
->update(['updated_at' => $now->subMinutes(10)]);
- Queue::fake();
+ $this->fakeQueueReset();
// Then restore it
$userA->restore();
@@ -1244,7 +1244,7 @@
*/
public function testRestrictAndUnrestrict(): void
{
- Queue::fake();
+ $this->fakeQueueReset();
// Test an account with users, domain
$user = $this->getTestUser('UserAccountA@UserAccount.com');
@@ -1277,7 +1277,7 @@
$userB->restrict();
$this->assertTrue($userB->fresh()->isRestricted());
- Queue::fake(); // reset queue state
+ $this->fakeQueueReset();
$user->refresh();
$user->unrestrict();
@@ -1292,7 +1292,7 @@
}
);
- Queue::fake(); // reset queue state
+ $this->fakeQueueReset();
$user->unrestrict(true);
@@ -1312,8 +1312,7 @@
*/
public function testSetAliases(): void
{
- Queue::fake();
- Queue::assertNothingPushed();
+ $this->fakeQueueReset();
$user = $this->getTestUser('UserAccountA@UserAccount.com');
$domain = $this->getTestDomain('UserAccount.com', [
@@ -1338,10 +1337,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);
@@ -1351,11 +1351,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,
@@ -1371,9 +1372,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());
}
@@ -1426,8 +1428,7 @@
*/
public function testUserSettings(): void
{
- Queue::fake();
- Queue::assertNothingPushed();
+ $this->fakeQueueReset();
$user = $this->getTestUser('UserAccountA@UserAccount.com');
@@ -1455,10 +1456,11 @@
$this->assertSame('Firstname', $user->fresh()->getSetting('first_name'));
// Update a setting
+ $this->fakeQueueReset();
$user->setSetting('first_name', 'Firstname1');
if (\config('app.with_ldap')) {
- 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
@@ -1467,10 +1469,11 @@
$this->assertSame('Firstname1', $user->fresh()->getSetting('first_name'));
// Delete a setting (null)
+ $this->fakeQueueReset();
$user->setSetting('first_name', null);
if (\config('app.with_ldap')) {
- 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
@@ -1479,11 +1482,12 @@
$this->assertSame(null, $user->fresh()->getSetting('first_name'));
// Delete a setting (empty string)
+ $this->fakeQueueReset();
$user->setSetting('first_name', 'Firstname1');
$user->setSetting('first_name', '');
if (\config('app.with_ldap')) {
- 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
@@ -1492,15 +1496,16 @@
$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
+ // Thanks to job locking it creates a single UserUpdate job
if (\config('app.with_ldap')) {
- Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 7);
+ Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1);
}
// Note: We test both current user as well as fresh user object
@@ -1512,18 +1517,21 @@
$this->assertSame(null, $user->getSetting('country'));
$this->assertSame(null, $user->fresh()->getSetting('country'));
- $all_settings = $user->settings()->orderBy('key')->get();
- $this->assertCount(3, $all_settings);
+ $expected = [
+ 'currency' => 'CHF',
+ 'first_name' => 'Firstname2',
+ 'last_name' => 'Lastname2',
+ ];
- // Test getSettings() method
- $this->assertSame(
- [
- 'first_name' => 'Firstname2',
- 'last_name' => 'Lastname2',
- 'unknown' => null,
- ],
- $user->getSettings(['first_name', 'last_name', 'unknown'])
- );
+ $this->assertSame($expected, $user->settings()->orderBy('key')->get()->pluck('value', 'key')->all());
+
+ $expected = [
+ 'first_name' => 'Firstname2',
+ 'last_name' => 'Lastname2',
+ 'unknown' => null,
+ ];
+
+ $this->assertSame($expected, $user->getSettings(['first_name', 'last_name', 'unknown']));
}
/**
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:47 PM (15 h, 40 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18830802
Default Alt Text
D4610.1775328427.diff (12 KB)

Event Timeline