D5409.1774831454.diff
No OneTemporary
Actions

Authored By

Unknown

Size

20 KB

Referenced Files

None

Subscribers

None

D5409.1774831454.diff
View Options

	diff --git a/src/app/Jobs/Domain/CreateJob.php b/src/app/Jobs/Domain/CreateJob.php
	--- a/src/app/Jobs/Domain/CreateJob.php
	+++ b/src/app/Jobs/Domain/CreateJob.php
	@@ -19,6 +19,10 @@
	return;
	}

	+ if ($domain->trashed()) {
	+ return;
	+ }
	+
	if (\config('app.with_ldap') && !$domain->isLdapReady()) {
	LDAP::createDomain($domain);

	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
	@@ -19,14 +19,12 @@
	return;
	}

	- // sanity checks
	if (!$domain->trashed()) {
	$this->fail("Domain {$domain->namespace} is not deleted.");
	return;
	}

	if ($domain->isDeleted()) {
	- $this->fail("Domain {$domain->namespace} is already marked as deleted.");
	return;
	}

	diff --git a/src/app/Jobs/Domain/UpdateJob.php b/src/app/Jobs/Domain/UpdateJob.php
	--- a/src/app/Jobs/Domain/UpdateJob.php
	+++ b/src/app/Jobs/Domain/UpdateJob.php
	@@ -18,8 +18,11 @@
	return;
	}

	+ if ($domain->trashed()) {
	+ return;
	+ }
	+
	if (!\config('app.with_ldap') \|\| !$domain->isLdapReady()) {
	- $this->delete();
	return;
	}

	diff --git a/src/app/Jobs/Domain/VerifyJob.php b/src/app/Jobs/Domain/VerifyJob.php
	--- a/src/app/Jobs/Domain/VerifyJob.php
	+++ b/src/app/Jobs/Domain/VerifyJob.php
	@@ -17,6 +17,10 @@
	return;
	}

	+ if ($domain->trashed()) {
	+ return;
	+ }
	+
	$domain->verify();
	}
	}
	diff --git a/src/app/Jobs/Group/CreateJob.php b/src/app/Jobs/Group/CreateJob.php
	--- a/src/app/Jobs/Group/CreateJob.php
	+++ b/src/app/Jobs/Group/CreateJob.php
	@@ -19,6 +19,10 @@
	return;
	}

	+ if ($group->trashed()) {
	+ return;
	+ }
	+
	if (\config('app.with_ldap') && !$group->isLdapReady()) {
	LDAP::createGroup($group);

	diff --git a/src/app/Jobs/Group/DeleteJob.php b/src/app/Jobs/Group/DeleteJob.php
	--- a/src/app/Jobs/Group/DeleteJob.php
	+++ b/src/app/Jobs/Group/DeleteJob.php
	@@ -19,14 +19,12 @@
	return;
	}

	- // sanity checks
	if (!$group->trashed()) {
	- $this->fail("Group {$this->groupId} is not deleted.");
	+ $this->fail("Group {$group->email} is not deleted.");
	return;
	}

	if ($group->isDeleted()) {
	- $this->fail("Group {$this->groupId} is already marked as deleted.");
	return;
	}

	diff --git a/src/app/Jobs/Group/UpdateJob.php b/src/app/Jobs/Group/UpdateJob.php
	--- a/src/app/Jobs/Group/UpdateJob.php
	+++ b/src/app/Jobs/Group/UpdateJob.php
	@@ -19,8 +19,7 @@
	}

	// Cancel the update if the group is deleted or not yet in LDAP
	- if (!\config('app.with_ldap') \|\| !$group->isLdapReady() \|\| $group->isDeleted()) {
	- $this->delete();
	+ if (!\config('app.with_ldap') \|\| !$group->isLdapReady() \|\| $group->trashed()) {
	return;
	}

	diff --git a/src/app/Jobs/PGP/KeyCreateJob.php b/src/app/Jobs/PGP/KeyCreateJob.php
	--- a/src/app/Jobs/PGP/KeyCreateJob.php
	+++ b/src/app/Jobs/PGP/KeyCreateJob.php
	@@ -42,14 +42,8 @@
	return;
	}

	- // sanity checks
	- if ($user->isDeleted()) {
	- $this->fail("User {$this->userId} is marked as deleted.");
	- return;
	- }
	-
	if ($user->trashed()) {
	- $this->fail("User {$this->userId} is actually deleted.");
	+ $this->fail("User {$user->email} is actually deleted.");
	return;
	}

	diff --git a/src/app/Jobs/Resource/CreateJob.php b/src/app/Jobs/Resource/CreateJob.php
	--- a/src/app/Jobs/Resource/CreateJob.php
	+++ b/src/app/Jobs/Resource/CreateJob.php
	@@ -20,14 +20,7 @@
	return;
	}

	- // sanity checks
	- if ($resource->isDeleted()) {
	- $this->fail("Resource {$this->resourceId} is marked as deleted.");
	- return;
	- }
	-
	if ($resource->trashed()) {
	- $this->fail("Resource {$this->resourceId} is actually deleted.");
	return;
	}

	diff --git a/src/app/Jobs/Resource/DeleteJob.php b/src/app/Jobs/Resource/DeleteJob.php
	--- a/src/app/Jobs/Resource/DeleteJob.php
	+++ b/src/app/Jobs/Resource/DeleteJob.php
	@@ -20,14 +20,12 @@
	return;
	}

	- // sanity checks
	if (!$resource->trashed()) {
	$this->fail("Resource {$this->resourceId} is not deleted.");
	return;
	}

	if ($resource->isDeleted()) {
	- $this->fail("Resource {$this->resourceId} is already marked as deleted.");
	return;
	}

	diff --git a/src/app/Jobs/Resource/UpdateJob.php b/src/app/Jobs/Resource/UpdateJob.php
	--- a/src/app/Jobs/Resource/UpdateJob.php
	+++ b/src/app/Jobs/Resource/UpdateJob.php
	@@ -19,9 +19,7 @@
	return;
	}

	- // Cancel the update if the resource is deleted
	- if ($resource->isDeleted()) {
	- $this->delete();
	+ if ($resource->trashed()) {
	return;
	}

	diff --git a/src/app/Jobs/Resource/VerifyJob.php b/src/app/Jobs/Resource/VerifyJob.php
	--- a/src/app/Jobs/Resource/VerifyJob.php
	+++ b/src/app/Jobs/Resource/VerifyJob.php
	@@ -19,8 +19,7 @@
	return;
	}

	- // the resource was already verified
	- if ($resource->isImapReady()) {
	+ if ($resource->trashed() \|\| $resource->isImapReady()) {
	return;
	}

	diff --git a/src/app/Jobs/SharedFolder/CreateJob.php b/src/app/Jobs/SharedFolder/CreateJob.php
	--- a/src/app/Jobs/SharedFolder/CreateJob.php
	+++ b/src/app/Jobs/SharedFolder/CreateJob.php
	@@ -20,14 +20,7 @@
	return;
	}

	- // sanity checks
	- if ($folder->isDeleted()) {
	- $this->fail("Shared folder {$this->folderId} is marked as deleted.");
	- return;
	- }
	-
	if ($folder->trashed()) {
	- $this->fail("Shared folder {$this->folderId} is actually deleted.");
	return;
	}

	diff --git a/src/app/Jobs/SharedFolder/DeleteJob.php b/src/app/Jobs/SharedFolder/DeleteJob.php
	--- a/src/app/Jobs/SharedFolder/DeleteJob.php
	+++ b/src/app/Jobs/SharedFolder/DeleteJob.php
	@@ -20,14 +20,12 @@
	return;
	}

	- // sanity checks
	if (!$folder->trashed()) {
	- $this->fail("Shared folder {$this->folderId} is not deleted.");
	+ $this->fail("Shared folder {$folder->id} is not deleted.");
	return;
	}

	if ($folder->isDeleted()) {
	- $this->fail("Shared folder {$this->folderId} is already marked as deleted.");
	return;
	}

	diff --git a/src/app/Jobs/SharedFolder/UpdateJob.php b/src/app/Jobs/SharedFolder/UpdateJob.php
	--- a/src/app/Jobs/SharedFolder/UpdateJob.php
	+++ b/src/app/Jobs/SharedFolder/UpdateJob.php
	@@ -19,9 +19,7 @@
	return;
	}

	- // Cancel the update if the folder is deleted
	- if ($folder->isDeleted()) {
	- $this->delete();
	+ if ($folder->trashed()) {
	return;
	}

	diff --git a/src/app/Jobs/SharedFolder/VerifyJob.php b/src/app/Jobs/SharedFolder/VerifyJob.php
	--- a/src/app/Jobs/SharedFolder/VerifyJob.php
	+++ b/src/app/Jobs/SharedFolder/VerifyJob.php
	@@ -19,8 +19,7 @@
	return;
	}

	- // the user has a mailbox (or is marked as such)
	- if ($folder->isImapReady()) {
	+ if ($folder->trashed() \|\| $folder->isImapReady()) {
	return;
	}

	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
	@@ -49,14 +49,7 @@
	return;
	}

	- // sanity checks
	- if ($user->isDeleted()) {
	- $this->fail("User {$this->userId} is marked as deleted.");
	- return;
	- }
	-
	if ($user->trashed()) {
	- $this->fail("User {$this->userId} is actually deleted.");
	return;
	}

	@@ -70,12 +63,12 @@
	$domain = $user->domain();

	if (!$domain) {
	- $this->fail("The domain for {$this->userId} does not exist.");
	+ $this->fail("The domain for {$user->email} does not exist.");
	return;
	}

	if ($domain->isDeleted()) {
	- $this->fail("The domain for {$this->userId} is marked as deleted.");
	+ $this->fail("The domain for {$user->email} is marked as deleted.");
	return;
	}

	@@ -105,7 +98,7 @@
	if (!$user->isImapReady()) {
	if (\config('app.with_imap')) {
	if (!IMAP::createUser($user)) {
	- throw new \Exception("Failed to create mailbox for user {$this->userId}.");
	+ throw new \Exception("Failed to create mailbox for user {$user->email}.");
	}
	} else {
	if (!IMAP::verifyAccount($user->email)) {
	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
	@@ -28,13 +28,11 @@
	}

	if (!$user->trashed()) {
	- $this->fail("User {$this->userId} is not deleted.");
	+ $this->fail("User {$user->email} is not deleted.");
	return;
	}

	- // sanity checks
	if ($user->isDeleted()) {
	- $this->fail("User {$this->userId} is already marked as deleted.");
	return;
	}

	@@ -48,7 +46,7 @@
	if ($user->isImapReady()) {
	if (\config('app.with_imap')) {
	if (!IMAP::deleteUser($user)) {
	- throw new \Exception("Failed to delete mailbox for user {$this->userId}.");
	+ throw new \Exception("Failed to delete mailbox for user {$user->email}.");
	}
	}

	diff --git a/src/app/Jobs/User/ResyncJob.php b/src/app/Jobs/User/ResyncJob.php
	--- a/src/app/Jobs/User/ResyncJob.php
	+++ b/src/app/Jobs/User/ResyncJob.php
	@@ -25,7 +25,6 @@
	}

	if ($user->role == User::ROLE_SERVICE) {
	- // Admins/resellers don't reside in LDAP (for now)
	return;
	}

	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
	@@ -35,7 +35,6 @@
	}

	if ($user->trashed()) {
	- $this->delete();
	return;
	}

	@@ -45,7 +44,7 @@

	if (\config('app.with_imap') && $user->isImapReady()) {
	if (!IMAP::updateUser($user)) {
	- throw new \Exception("Failed to update mailbox for user {$this->userId}.");
	+ throw new \Exception("Failed to update mailbox for user {$user->email}.");
	}
	}
	}
	diff --git a/src/tests/Feature/Jobs/Domain/DeleteTest.php b/src/tests/Feature/Jobs/Domain/DeleteTest.php
	--- a/src/tests/Feature/Jobs/Domain/DeleteTest.php
	+++ b/src/tests/Feature/Jobs/Domain/DeleteTest.php
	@@ -60,9 +60,9 @@
	$this->assertFalse($domain->isLdapReady());
	$this->assertTrue($domain->isDeleted());

	- // Test job failure (domain marked as deleted)
	+ // Test domain marked as deleted
	$job = (new DeleteJob($domain->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("Domain {$domain->namespace} is already marked as deleted.");
	+ $job->assertNotFailed();
	}
	}
	diff --git a/src/tests/Feature/Jobs/Group/DeleteTest.php b/src/tests/Feature/Jobs/Group/DeleteTest.php
	--- a/src/tests/Feature/Jobs/Group/DeleteTest.php
	+++ b/src/tests/Feature/Jobs/Group/DeleteTest.php
	@@ -43,7 +43,7 @@
	// Test group that is not deleted yet
	$job = (new DeleteJob($group->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("Group {$group->id} is not deleted.");
	+ $job->assertFailedWith("Group {$group->email} is not deleted.");

	$group->deleted_at = \now();
	$group->saveQuietly();
	diff --git a/src/tests/Feature/Jobs/Group/UpdateTest.php b/src/tests/Feature/Jobs/Group/UpdateTest.php
	--- a/src/tests/Feature/Jobs/Group/UpdateTest.php
	+++ b/src/tests/Feature/Jobs/Group/UpdateTest.php
	@@ -44,7 +44,7 @@
	// Group not LDAP_READY
	$job = (new UpdateJob($group->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertDeleted();
	+ $job->assertNotFailed();

	// Group is LDAP_READY
	$group->status \|= Group::STATUS_LDAP_READY;
	diff --git a/src/tests/Feature/Jobs/Resource/CreateTest.php b/src/tests/Feature/Jobs/Resource/CreateTest.php
	--- a/src/tests/Feature/Jobs/Resource/CreateTest.php
	+++ b/src/tests/Feature/Jobs/Resource/CreateTest.php
	@@ -66,21 +66,12 @@

	// TODO: Test case when IMAP or LDAP method fails

	- // Test job failures
	- $resource->status \|= Resource::STATUS_DELETED;
	- $resource->save();
	-
	- $job = (new CreateJob($resource->id))->withFakeQueueInteractions();
	- $job->handle();
	- $job->assertFailedWith("Resource {$resource->id} is marked as deleted.");
	-
	- $resource->status ^= Resource::STATUS_DELETED;
	- $resource->save();
	+ // Test a resource actually deleted
	$resource->delete();

	$job = (new CreateJob($resource->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("Resource {$resource->id} is actually deleted.");
	+ $job->assertNotFailed();

	// TODO: Test failures on domain sanity checks
	// TODO: Test partial execution, i.e. only IMAP or only LDAP
	diff --git a/src/tests/Feature/Jobs/Resource/DeleteTest.php b/src/tests/Feature/Jobs/Resource/DeleteTest.php
	--- a/src/tests/Feature/Jobs/Resource/DeleteTest.php
	+++ b/src/tests/Feature/Jobs/Resource/DeleteTest.php
	@@ -77,6 +77,6 @@
	// Test deleting already deleted resource
	$job = (new DeleteJob($resource->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("Resource {$resource->id} is already marked as deleted.");
	+ $job->assertNotFailed();
	}
	}
	diff --git a/src/tests/Feature/Jobs/Resource/UpdateTest.php b/src/tests/Feature/Jobs/Resource/UpdateTest.php
	--- a/src/tests/Feature/Jobs/Resource/UpdateTest.php
	+++ b/src/tests/Feature/Jobs/Resource/UpdateTest.php
	@@ -55,13 +55,11 @@
	$job = new UpdateJob($resource->id);
	$job->handle();

	- // Test that the job is being deleted if the resource is not ldap ready or is deleted
	- $resource->refresh();
	- $resource->status \|= Resource::STATUS_DELETED;
	- $resource->save();
	+ // Test deleted resource
	+ $resource->delete();

	$job = (new UpdateJob($resource->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertDeleted();
	+ $job->assertNotFailed();
	}
	}
	diff --git a/src/tests/Feature/Jobs/SharedFolder/CreateTest.php b/src/tests/Feature/Jobs/SharedFolder/CreateTest.php
	--- a/src/tests/Feature/Jobs/SharedFolder/CreateTest.php
	+++ b/src/tests/Feature/Jobs/SharedFolder/CreateTest.php
	@@ -63,21 +63,12 @@
	$this->assertTrue($folder->isImapReady());
	$this->assertTrue($folder->isActive());

	- // Test job failures
	- $folder->status \|= SharedFolder::STATUS_DELETED;
	- $folder->save();
	-
	- $job = (new CreateJob($folder->id))->withFakeQueueInteractions();
	- $job->handle();
	- $job->assertFailedWith("Shared folder {$folder->id} is marked as deleted.");
	-
	- $folder->status ^= SharedFolder::STATUS_DELETED;
	- $folder->save();
	+ // Test folder deleted
	$folder->delete();

	$job = (new CreateJob($folder->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("Shared folder {$folder->id} is actually deleted.");
	+ $job->assertNotFailed();

	// TODO: Test failures on domain sanity checks
	// TODO: Test partial execution, i.e. only IMAP or only LDAP
	diff --git a/src/tests/Feature/Jobs/SharedFolder/DeleteTest.php b/src/tests/Feature/Jobs/SharedFolder/DeleteTest.php
	--- a/src/tests/Feature/Jobs/SharedFolder/DeleteTest.php
	+++ b/src/tests/Feature/Jobs/SharedFolder/DeleteTest.php
	@@ -73,6 +73,6 @@
	// Test deleting already deleted folder
	$job = (new DeleteJob($folder->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("Shared folder {$folder->id} is already marked as deleted.");
	+ $job->assertNotFailed();
	}
	}
	diff --git a/src/tests/Feature/Jobs/SharedFolder/UpdateTest.php b/src/tests/Feature/Jobs/SharedFolder/UpdateTest.php
	--- a/src/tests/Feature/Jobs/SharedFolder/UpdateTest.php
	+++ b/src/tests/Feature/Jobs/SharedFolder/UpdateTest.php
	@@ -55,11 +55,10 @@
	$job->assertNotFailed();

	// Test handling deleted folder
	- $folder->status \|= SharedFolder::STATUS_DELETED;
	- $folder->save();
	+ $folder->delete();

	$job = (new UpdateJob($folder->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertDeleted();
	+ $job->assertNotFailed();
	}
	}
	diff --git a/src/tests/Feature/Jobs/User/CreateTest.php b/src/tests/Feature/Jobs/User/CreateTest.php
	--- a/src/tests/Feature/Jobs/User/CreateTest.php
	+++ b/src/tests/Feature/Jobs/User/CreateTest.php
	@@ -64,22 +64,12 @@
	$this->assertTrue($user->isImapReady());
	$this->assertTrue($user->isActive());

	- // Test job failure (user deleted)
	- $user->status \|= User::STATUS_DELETED;
	- $user->save();
	-
	- $job = (new CreateJob($user->id))->withFakeQueueInteractions();
	- $job->handle();
	- $job->assertFailedWith("User {$user->id} is marked as deleted.");
	-
	- // Test job failure (user removed)
	- $user->status ^= User::STATUS_DELETED;
	- $user->save();
	+ // Test deleted user
	$user->delete();

	$job = (new CreateJob($user->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("User {$user->id} is actually deleted.");
	+ $job->assertNotFailed();

	// Test job failure (user unknown), the job will be released
	$job = (new CreateJob(123))->withFakeQueueInteractions();
	diff --git a/src/tests/Feature/Jobs/User/DeleteTest.php b/src/tests/Feature/Jobs/User/DeleteTest.php
	--- a/src/tests/Feature/Jobs/User/DeleteTest.php
	+++ b/src/tests/Feature/Jobs/User/DeleteTest.php
	@@ -48,16 +48,16 @@
	// Test job failure (user not yet deleted)
	$job = (new DeleteJob($user->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("User {$user->id} is not deleted.");
	+ $job->assertFailedWith("User {$user->email} is not deleted.");

	- // Test job failure (user already deleted)
	+ // Test user already deleted
	$user->status \|= User::STATUS_DELETED;
	$user->deleted_at = \now();
	$user->saveQuietly();

	$job = (new DeleteJob($user->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertFailedWith("User {$user->id} is already marked as deleted.");
	+ $job->assertNotFailed();

	// Test success delete from LDAP, IMAP and Roundcube
	$user->status ^= User::STATUS_DELETED;
	diff --git a/src/tests/Feature/Jobs/User/UpdateTest.php b/src/tests/Feature/Jobs/User/UpdateTest.php
	--- a/src/tests/Feature/Jobs/User/UpdateTest.php
	+++ b/src/tests/Feature/Jobs/User/UpdateTest.php
	@@ -44,14 +44,15 @@
	LDAP::shouldReceive('updateUser')->once()->with($user)->andReturn(true);

	// Test normal update
	- $job = new UpdateJob($user->id);
	+ $job = (new UpdateJob($user->id))->withFakeQueueInteractions();
	$job->handle();
	+ $job->assertNotFailed();

	// Test deleted user
	$user->delete();
	$job = (new UpdateJob($user->id))->withFakeQueueInteractions();
	$job->handle();
	- $job->assertDeleted();
	+ $job->assertNotFailed();

	// Test job failure (user unknown), the job will be released
	$job = (new UpdateJob(123))->withFakeQueueInteractions();

File Metadata

Mime Type: text/plain
Expires: Mon, Mar 30, 12:44 AM (4 d, 7 h ago)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 18784068
Default Alt Text: D5409.1774831454.diff (20 KB)

D5409.1774831454.diffNo OneTemporaryActions

D5409.1774831454.diffView Options

File Metadata

Event Timeline

D5409.1774831454.diff
No OneTemporary
Actions

D5409.1774831454.diff
View Options