diff --git a/src/app/Entitlement.php b/src/app/Entitlement.php --- a/src/app/Entitlement.php +++ b/src/app/Entitlement.php @@ -2,7 +2,6 @@ namespace App; -use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\SoftDeletes; use App\Traits\UuidStrKeyTrait; @@ -145,32 +144,6 @@ return $skus; } - /** - * Restore object entitlements. - * - * @param \App\User|\App\Domain|\App\Group $object The user|domain|group object - */ - public static function restoreEntitlementsFor($object): void - { - // We'll restore only these that were deleted last. So, first we get - // the maximum deleted_at timestamp and then use it to select - // entitlements for restore - $deleted_at = $object->entitlements()->withTrashed()->max('deleted_at'); - - if ($deleted_at) { - $threshold = (new \Carbon\Carbon($deleted_at))->subMinute(); - - // Restore object entitlements - $object->entitlements()->withTrashed() - ->where('deleted_at', '>=', $threshold) - ->update(['updated_at' => now(), 'deleted_at' => null]); - - // Note: We're assuming that cost of entitlements was correct - // on deletion, so we don't have to re-calculate it again. - // TODO: We should probably re-calculate the cost - } - } - /** * The SKU concerned. * diff --git a/src/app/Observers/DomainObserver.php b/src/app/Observers/DomainObserver.php --- a/src/app/Observers/DomainObserver.php +++ b/src/app/Observers/DomainObserver.php @@ -35,22 +35,6 @@ \App\Jobs\Domain\CreateJob::dispatch($domain->id); } - /** - * Handle the domain "deleting" event. - * - * @param \App\Domain $domain The domain. - * - * @return void - */ - public function deleting(Domain $domain) - { - // Entitlements do not have referential integrity on the entitled object, so this is our - // way of doing an onDelete('cascade') without the foreign key. - \App\Entitlement::where('entitleable_id', $domain->id) - ->where('entitleable_type', Domain::class) - ->delete(); - } - /** * Handle the domain "deleted" event. * @@ -114,22 +98,7 @@ */ public function restored(Domain $domain) { - // Restore domain entitlements - \App\Entitlement::restoreEntitlementsFor($domain); - // Create the domain in LDAP again \App\Jobs\Domain\CreateJob::dispatch($domain->id); } - - /** - * Handle the domain "force deleted" event. - * - * @param \App\Domain $domain The domain. - * - * @return void - */ - public function forceDeleted(Domain $domain) - { - // - } } diff --git a/src/app/Observers/EntitlementObserver.php b/src/app/Observers/EntitlementObserver.php --- a/src/app/Observers/EntitlementObserver.php +++ b/src/app/Observers/EntitlementObserver.php @@ -79,10 +79,12 @@ $sf->removeFactors(); } - $entitlement->entitleable->updated_at = Carbon::now(); - $entitlement->entitleable->save(); + if ($entitlement->entitleable && !$entitlement->entitleable->trashed()) { + $entitlement->entitleable->updated_at = Carbon::now(); + $entitlement->entitleable->save(); - $entitlement->createTransaction(\App\Transaction::ENTITLEMENT_DELETED); + $entitlement->createTransaction(\App\Transaction::ENTITLEMENT_DELETED); + } } /** diff --git a/src/app/Observers/GroupObserver.php b/src/app/Observers/GroupObserver.php --- a/src/app/Observers/GroupObserver.php +++ b/src/app/Observers/GroupObserver.php @@ -35,22 +35,6 @@ \App\Jobs\Group\CreateJob::dispatch($group->id); } - /** - * Handle the group "deleting" event. - * - * @param \App\Group $group The group - * - * @return void - */ - public function deleting(Group $group) - { - // Entitlements do not have referential integrity on the entitled object, so this is our - // way of doing an onDelete('cascade') without the foreign key. - \App\Entitlement::where('entitleable_id', $group->id) - ->where('entitleable_type', Group::class) - ->delete(); - } - /** * Handle the group "deleted" event. * @@ -113,25 +97,6 @@ */ public function restored(Group $group) { - // Restore group entitlements - \App\Entitlement::restoreEntitlementsFor($group); - \App\Jobs\Group\CreateJob::dispatch($group->id); } - - /** - * Handle the group "force deleting" event. - * - * @param \App\Group $group The group - * - * @return void - */ - public function forceDeleted(Group $group) - { - // A group can be force-deleted separately from the owner - // we have to force-delete entitlements - \App\Entitlement::where('entitleable_id', $group->id) - ->where('entitleable_type', Group::class) - ->forceDelete(); - } } diff --git a/src/app/Observers/ResourceObserver.php b/src/app/Observers/ResourceObserver.php --- a/src/app/Observers/ResourceObserver.php +++ b/src/app/Observers/ResourceObserver.php @@ -65,22 +65,6 @@ \App\Jobs\Resource\CreateJob::withChain($chain)->dispatch($resource->id); } - /** - * Handle the resource "deleting" event. - * - * @param \App\Resource $resource The resource - * - * @return void - */ - public function deleting(Resource $resource) - { - // Entitlements do not have referential integrity on the entitled object, so this is our - // way of doing an onDelete('cascade') without the foreign key. - \App\Entitlement::where('entitleable_id', $resource->id) - ->where('entitleable_type', Resource::class) - ->delete(); - } - /** * Handle the resource "deleted" event. * @@ -117,20 +101,4 @@ $resource->settings()->where('key', 'folder')->update(['value' => $folder]); } } - - /** - * Handle the resource "force deleted" event. - * - * @param \App\Resource $resource The resource - * - * @return void - */ - public function forceDeleted(Resource $resource) - { - // A group can be force-deleted separately from the owner - // we have to force-delete entitlements - \App\Entitlement::where('entitleable_id', $resource->id) - ->where('entitleable_type', Resource::class) - ->forceDelete(); - } } diff --git a/src/app/Observers/SharedFolderObserver.php b/src/app/Observers/SharedFolderObserver.php --- a/src/app/Observers/SharedFolderObserver.php +++ b/src/app/Observers/SharedFolderObserver.php @@ -69,22 +69,6 @@ \App\Jobs\SharedFolder\CreateJob::withChain($chain)->dispatch($folder->id); } - /** - * Handle the shared folder "deleting" event. - * - * @param \App\SharedFolder $folder The folder - * - * @return void - */ - public function deleting(SharedFolder $folder) - { - // Entitlements do not have referential integrity on the entitled object, so this is our - // way of doing an onDelete('cascade') without the foreign key. - \App\Entitlement::where('entitleable_id', $folder->id) - ->where('entitleable_type', SharedFolder::class) - ->delete(); - } - /** * Handle the shared folder "deleted" event. * @@ -121,20 +105,4 @@ $folder->settings()->where('key', 'folder')->update(['value' => $folderName]); } } - - /** - * Handle the shared folder "force deleted" event. - * - * @param \App\SharedFolder $folder The folder - * - * @return void - */ - public function forceDeleted(SharedFolder $folder) - { - // A folder can be force-deleted separately from the owner - // we have to force-delete entitlements - \App\Entitlement::where('entitleable_id', $folder->id) - ->where('entitleable_type', SharedFolder::class) - ->forceDelete(); - } } diff --git a/src/app/Observers/UserObserver.php b/src/app/Observers/UserObserver.php --- a/src/app/Observers/UserObserver.php +++ b/src/app/Observers/UserObserver.php @@ -2,15 +2,8 @@ namespace App\Observers; -use App\Entitlement; -use App\Domain; -use App\Group; -use App\Resource; -use App\SharedFolder; -use App\Transaction; use App\User; use App\Wallet; -use Illuminate\Support\Facades\DB; class UserObserver { @@ -102,18 +95,6 @@ } }); } - - // Debit the reseller's wallet with the user negative balance - $balance = 0; - foreach ($user->wallets as $wallet) { - // Note: here we assume all user wallets are using the same currency. - // It might get changed in the future - $balance += $wallet->balance; - } - - if ($balance < 0 && $user->tenant && ($wallet = $user->tenant->wallet())) { - $wallet->debit($balance * -1, "Deleted user {$user->email}"); - } } /** @@ -125,177 +106,31 @@ */ public function deleting(User $user) { - if ($user->isForceDeleting()) { - $this->forceDeleting($user); - return; - } + // Remove owned users/domains/groups/resources/etc + self::removeRelatedObjects($user, $user->isForceDeleting()); // TODO: Especially in tests we're doing delete() on a already deleted user. // Should we escape here - for performance reasons? - // TODO: I think all of this should use database transactions - - // Entitlements do not have referential integrity on the entitled object, so this is our - // way of doing an onDelete('cascade') without the foreign key. - $entitlements = Entitlement::where('entitleable_id', $user->id) - ->where('entitleable_type', User::class)->get(); - - foreach ($entitlements as $entitlement) { - $entitlement->delete(); - } - - // Remove owned users/domains - $wallets = $user->wallets()->pluck('id')->all(); - $assignments = Entitlement::whereIn('wallet_id', $wallets)->get(); - $users = []; - $domains = []; - $groups = []; - $resources = []; - $folders = []; - $entitlements = []; - - foreach ($assignments as $entitlement) { - if ($entitlement->entitleable_type == Domain::class) { - $domains[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == User::class && $entitlement->entitleable_id != $user->id) { - $users[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == Group::class) { - $groups[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == Resource::class) { - $resources[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == SharedFolder::class) { - $folders[] = $entitlement->entitleable_id; - } else { - $entitlements[] = $entitlement; - } - } - - // Domains/users/entitlements need to be deleted one by one to make sure - // events are fired and observers can do the proper cleanup. - if (!empty($users)) { - foreach (User::whereIn('id', array_unique($users))->get() as $_user) { - $_user->delete(); - } - } - - if (!empty($domains)) { - foreach (Domain::whereIn('id', array_unique($domains))->get() as $_domain) { - $_domain->delete(); - } - } - if (!empty($groups)) { - foreach (Group::whereIn('id', array_unique($groups))->get() as $_group) { - $_group->delete(); - } - } - - if (!empty($resources)) { - foreach (Resource::whereIn('id', array_unique($resources))->get() as $_resource) { - $_resource->delete(); - } - } + if (!$user->isForceDeleting()) { + \App\Jobs\User\DeleteJob::dispatch($user->id); - if (!empty($folders)) { - foreach (SharedFolder::whereIn('id', array_unique($folders))->get() as $_folder) { - $_folder->delete(); + if (\App\Tenant::getConfig($user->tenant_id, 'pgp.enable')) { + \App\Jobs\PGP\KeyDeleteJob::dispatch($user->id, $user->email); } - } - - foreach ($entitlements as $entitlement) { - $entitlement->delete(); - } - - // FIXME: What do we do with user wallets? - - \App\Jobs\User\DeleteJob::dispatch($user->id); - - if (\App\Tenant::getConfig($user->tenant_id, 'pgp.enable')) { - \App\Jobs\PGP\KeyDeleteJob::dispatch($user->id, $user->email); - } - } - - /** - * Handle the "deleting" event on forceDelete() call. - * - * @param User $user The user that is being deleted. - * - * @return void - */ - public function forceDeleting(User $user) - { - // TODO: We assume that at this moment all belongings are already soft-deleted. - - // Remove owned users/domains - $wallets = $user->wallets()->pluck('id')->all(); - $assignments = Entitlement::withTrashed()->whereIn('wallet_id', $wallets)->get(); - $entitlements = []; - $domains = []; - $groups = []; - $resources = []; - $folders = []; - $users = []; - foreach ($assignments as $entitlement) { - $entitlements[] = $entitlement->id; - - if ($entitlement->entitleable_type == Domain::class) { - $domains[] = $entitlement->entitleable_id; - } elseif ( - $entitlement->entitleable_type == User::class - && $entitlement->entitleable_id != $user->id - ) { - $users[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == Group::class) { - $groups[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == Resource::class) { - $resources[] = $entitlement->entitleable_id; - } elseif ($entitlement->entitleable_type == SharedFolder::class) { - $folders[] = $entitlement->entitleable_id; + // Debit the reseller's wallet with the user negative balance + $balance = 0; + foreach ($user->wallets as $wallet) { + // Note: here we assume all user wallets are using the same currency. + // It might get changed in the future + $balance += $wallet->balance; } - } - // Remove the user "direct" entitlements explicitely, if they belong to another - // user's wallet they will not be removed by the wallets foreign key cascade - Entitlement::withTrashed() - ->where('entitleable_id', $user->id) - ->where('entitleable_type', User::class) - ->forceDelete(); - - // Users need to be deleted one by one to make sure observers can do the proper cleanup. - if (!empty($users)) { - foreach (User::withTrashed()->whereIn('id', array_unique($users))->get() as $_user) { - $_user->forceDelete(); + if ($balance < 0 && $user->tenant && ($wallet = $user->tenant->wallet())) { + $wallet->debit($balance * -1, "Deleted user {$user->email}"); } } - - // Domains can be just removed - if (!empty($domains)) { - Domain::withTrashed()->whereIn('id', array_unique($domains))->forceDelete(); - } - - // Groups can be just removed - if (!empty($groups)) { - Group::withTrashed()->whereIn('id', array_unique($groups))->forceDelete(); - } - - // Resources can be just removed - if (!empty($resources)) { - Resource::withTrashed()->whereIn('id', array_unique($resources))->forceDelete(); - } - - // Shared folders can be just removed - if (!empty($folders)) { - SharedFolder::withTrashed()->whereIn('id', array_unique($folders))->forceDelete(); - } - - // Remove transactions, they also have no foreign key constraint - Transaction::where('object_type', Entitlement::class) - ->whereIn('object_id', $entitlements) - ->delete(); - - Transaction::where('object_type', Wallet::class) - ->whereIn('object_id', $wallets) - ->delete(); } /** @@ -335,9 +170,6 @@ */ public function restored(User $user) { - // Restore user entitlements - \App\Entitlement::restoreEntitlementsFor($user); - // We need at least the user domain so it can be created in ldap. // FIXME: What if the domain is owned by someone else? $domain = $user->domain(); @@ -357,28 +189,53 @@ } /** - * Handle the "retrieving" event. - * - * @param User $user The user that is being retrieved. + * Handle the "updating" event. * - * @todo This is useful for audit. + * @param User $user The user that is being updated. * * @return void */ - public function retrieving(User $user) + public function updating(User $user) { - // TODO \App\Jobs\User\ReadJob::dispatch($user->id); + \App\Jobs\User\UpdateJob::dispatch($user->id); } /** - * Handle the "updating" event. - * - * @param User $user The user that is being updated. + * Remove entitleables/transactions related to the user (in user's wallets) * - * @return void + * @param \App\User $user The user + * @param bool $force Force-delete mode */ - public function updating(User $user) + private static function removeRelatedObjects(User $user, $force = false): void { - \App\Jobs\User\UpdateJob::dispatch($user->id); + $wallets = $user->wallets->pluck('id')->all(); + + \App\Entitlement::withTrashed() + ->select('entitleable_id', 'entitleable_type') + ->distinct() + ->whereIn('wallet_id', $wallets) + ->get() + ->each(function ($entitlement) use ($user, $force) { + // Skip the current user (infinite recursion loop) + if ($entitlement->entitleable_type == User::class && $entitlement->entitleable_id == $user->id) { + return; + } + + // Objects need to be deleted one by one to make sure observers can do the proper cleanup + if ($entitlement->entitleable) { + if ($force) { + $entitlement->entitleable->forceDelete(); + } elseif (!$entitlement->entitleable->trashed()) { + $entitlement->entitleable->delete(); + } + } + }); + + if ($force) { + // Remove "wallet" transactions, they have no foreign key constraint + \App\Transaction::where('object_type', Wallet::class) + ->whereIn('object_id', $wallets) + ->delete(); + } } } diff --git a/src/app/Traits/EntitleableTrait.php b/src/app/Traits/EntitleableTrait.php --- a/src/app/Traits/EntitleableTrait.php +++ b/src/app/Traits/EntitleableTrait.php @@ -4,6 +4,45 @@ trait EntitleableTrait { + /** + * Boot function from Laravel. + */ + protected static function bootEntitleableTrait() + { + // Soft-delete and force-delete object's entitlements on object's delete + static::deleting(function ($model) { + $force = $model->isForceDeleting(); + $entitlements = $model->entitlements(); + + if ($force) { + $entitlements = $entitlements->withTrashed(); + } + + $list = $entitlements->get() + ->map(function ($entitlement) use ($force) { + if ($force) { + $entitlement->forceDelete(); + } else { + $entitlement->delete(); + } + return $entitlement->id; + }) + ->all(); + + // Remove transactions, they have no foreign key constraint + if ($force && !empty($list)) { + \App\Transaction::where('object_type', \App\Entitlement::class) + ->whereIn('object_id', $list) + ->delete(); + } + }); + + // Restore object's entitlements on restore + static::restored(function ($model) { + $model->restoreEntitlements(); + }); + } + /** * Entitlements for this object. * @@ -15,6 +54,30 @@ ->where('entitleable_type', self::class); } + /** + * Restore object entitlements. + */ + public function restoreEntitlements(): void + { + // We'll restore only these that were deleted last. So, first we get + // the maximum deleted_at timestamp and then use it to select + // entitlements for restore + $deleted_at = $this->entitlements()->withTrashed()->max('deleted_at'); + + if ($deleted_at) { + $threshold = (new \Carbon\Carbon($deleted_at))->subMinute(); + + // Restore object entitlements + $this->entitlements()->withTrashed() + ->where('deleted_at', '>=', $threshold) + ->update(['updated_at' => now(), 'deleted_at' => null]); + + // Note: We're assuming that cost of entitlements was correct + // on deletion, so we don't have to re-calculate it again. + // TODO: We should probably re-calculate the cost + } + } + /** * Returns the wallet by which the object is controlled * diff --git a/src/app/Transaction.php b/src/app/Transaction.php --- a/src/app/Transaction.php +++ b/src/app/Transaction.php @@ -153,7 +153,7 @@ { $result = [ 'user_email' => $this->user_email, - 'description' => $this->{'description'}, + 'description' => $this->description, ]; $amount = $this->amount * ($this->amount < 0 ? -1 : 1); @@ -165,12 +165,12 @@ $result['entitlement_cost'] = $cost * $discount; $result['object'] = $entitlement->entitleableTitle(); - $result['sku_title'] = $entitlement->sku->{'title'}; + $result['sku_title'] = $entitlement->sku->title; } else { $wallet = $this->wallet(); } - $result['wallet'] = $wallet->{'description'} ?: 'Default wallet'; + $result['wallet'] = $wallet->description ?: 'Default wallet'; $result['amount'] = $wallet->money($amount); return $result; diff --git a/src/tests/Feature/Console/User/ForceDeleteTest.php b/src/tests/Feature/Console/User/ForceDeleteTest.php --- a/src/tests/Feature/Console/User/ForceDeleteTest.php +++ b/src/tests/Feature/Console/User/ForceDeleteTest.php @@ -86,6 +86,7 @@ 0, \App\Entitlement::withTrashed()->where('entitleable_id', $user->id)->get() ); + $this->assertCount( 0, \App\Transaction::whereIn('object_id', $entitlements) 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 @@ -105,32 +105,6 @@ $this->assertTrue($wallet->fresh()->balance < 0); } - /** - * @todo This really should be in User tests file - */ - public function testEntitlementFunctions(): void - { - $user = $this->getTestUser('entitlement-test@kolabnow.com'); - - $package = \App\Package::withEnvTenantContext()->where('title', 'kolab')->first(); - - $user->assignPackage($package); - - $wallet = $user->wallets()->first(); - $this->assertNotNull($wallet); - - $sku = \App\Sku::withEnvTenantContext()->where('title', 'mailbox')->first(); - - $entitlement = Entitlement::where('wallet_id', $wallet->id) - ->where('sku_id', $sku->id)->first(); - - $this->assertNotNull($entitlement); - $this->assertSame($sku->id, $entitlement->sku->id); - $this->assertSame($wallet->id, $entitlement->wallet->id); - $this->assertEquals($user->id, $entitlement->entitleable->id); - $this->assertTrue($entitlement->entitleable instanceof \App\User); - } - /** * Test Entitlement::entitleableTitle() */ 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 @@ -46,7 +46,24 @@ */ public function testAssignPackage(): void { - $this->markTestIncomplete(); + $user = $this->getTestUser('user-test@' . \config('app.domain')); + $wallet = $user->wallets()->first(); + + $package = \App\Package::withEnvTenantContext()->where('title', 'kolab')->first(); + + $user->assignPackage($package); + + $sku = \App\Sku::withEnvTenantContext()->where('title', 'mailbox')->first(); + + $entitlement = \App\Entitlement::where('wallet_id', $wallet->id) + ->where('sku_id', $sku->id)->first(); + + $this->assertNotNull($entitlement); + $this->assertSame($sku->id, $entitlement->sku->id); + $this->assertSame($wallet->id, $entitlement->wallet->id); + $this->assertEquals($user->id, $entitlement->entitleable->id); + $this->assertTrue($entitlement->entitleable instanceof \App\User); + $this->assertCount(7, $user->entitlements()->get()); } /** @@ -496,6 +513,13 @@ $this->assertSame(0, $entitlementsGroup->count()); $this->assertSame(0, $entitlementsResource->count()); $this->assertSame(0, $entitlementsFolder->count()); + $this->assertSame(7, $entitlementsA->withTrashed()->count()); + $this->assertSame(7, $entitlementsB->withTrashed()->count()); + $this->assertSame(7, $entitlementsC->withTrashed()->count()); + $this->assertSame(1, $entitlementsDomain->withTrashed()->count()); + $this->assertSame(1, $entitlementsGroup->withTrashed()->count()); + $this->assertSame(1, $entitlementsResource->withTrashed()->count()); + $this->assertSame(1, $entitlementsFolder->withTrashed()->count()); $this->assertTrue($userA->fresh()->trashed()); $this->assertTrue($userB->fresh()->trashed()); $this->assertTrue($domain->fresh()->trashed()); @@ -512,8 +536,10 @@ $userA->forceDelete(); $all_entitlements = \App\Entitlement::where('wallet_id', $userA->wallets->first()->id); + $transactions = \App\Transaction::where('object_id', $userA->wallets->first()->id); $this->assertSame(0, $all_entitlements->withTrashed()->count()); + $this->assertSame(0, $transactions->count()); $this->assertCount(0, User::withTrashed()->where('id', $userA->id)->get()); $this->assertCount(0, User::withTrashed()->where('id', $userB->id)->get()); $this->assertCount(0, User::withTrashed()->where('id', $userC->id)->get()); diff --git a/src/tests/Unit/EntitlementTest.php b/src/tests/Unit/EntitlementTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Unit/EntitlementTest.php @@ -0,0 +1,28 @@ +cost = 1.1; // @phpstan-ignore-line + $this->assertSame(1, $ent->cost); + + $ent->cost = 1.5; // @phpstan-ignore-line + $this->assertSame(2, $ent->cost); + + $ent->cost = '10'; // @phpstan-ignore-line + $this->assertSame(10, $ent->cost); + } +}