From de923bbb83c8acdfd75c0c5cb1c1e934b091667a Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Mon, 17 Apr 2017 20:16:05 -0400 Subject: [PATCH] Fix server deletion logic, and clean up suspend/unsuspend operations --- CHANGELOG.md | 6 + .../API/Admin/ServerController.php | 2 +- .../Controllers/Admin/ServersController.php | 2 +- app/Models/Server.php | 10 ++ app/Repositories/ServerRepository.php | 117 ++++++------------ 5 files changed, 56 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be1222d59..7d086a9ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. * `[beta.2]` — Fixes exception in tasks when deleting a server. * `[beta.2]` — Fixes bug with Terarria and Voice servers reporting a `TypeError: Service is not a constructor` in the daemon due to a missing service configuration. +### Changed +* Deleting a server safely now continues even if the daemon reports a `HTTP/404` missing server error (requires `Daemon@0.4.0-beta.2.1`) + +### Added +* Server listing and view in Admin CP now shows the SFTP username/Docker container name. + ## v0.6.0-beta.2 (Courageous Carniadactylus) ### Fixed * `[beta.1]` — Fixes task management ststem not running correctly. diff --git a/app/Http/Controllers/API/Admin/ServerController.php b/app/Http/Controllers/API/Admin/ServerController.php index 89dfd8cd0..75bcdc107 100644 --- a/app/Http/Controllers/API/Admin/ServerController.php +++ b/app/Http/Controllers/API/Admin/ServerController.php @@ -318,7 +318,7 @@ class ServerController extends Controller } try { - $repo->$action($id); + $repo->toggleAccess($id, ($action === 'unsuspend')); return response('', 204); } catch (DisplayException $ex) { diff --git a/app/Http/Controllers/Admin/ServersController.php b/app/Http/Controllers/Admin/ServersController.php index 85bc1a4db..1a8a8cfbc 100644 --- a/app/Http/Controllers/Admin/ServersController.php +++ b/app/Http/Controllers/Admin/ServersController.php @@ -367,7 +367,7 @@ class ServersController extends Controller } try { - $repo->$action($id); + $repo->toggleAccess($id, ($action === 'unsuspend')); Alert::success('Server has been ' . $action . 'ed.'); } catch (TransferException $ex) { diff --git a/app/Models/Server.php b/app/Models/Server.php index 921e7ad00..729ae4f0d 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -333,6 +333,16 @@ class Server extends Model return $this->hasMany(Database::class); } + /** + * Gets all downloads associated with a server. + * + * @return \Illuminate\Database\Eloquent\Relations\HasMany + */ + public function downloads() + { + return $this->hasMany(Download::class, 'server', 'id'); + } + /** * Gets the location of the server. * diff --git a/app/Repositories/ServerRepository.php b/app/Repositories/ServerRepository.php index 1ca75984f..67d4468bd 100644 --- a/app/Repositories/ServerRepository.php +++ b/app/Repositories/ServerRepository.php @@ -29,6 +29,7 @@ use Crypt; use Validator; use Pterodactyl\Models; use Pterodactyl\Services\UuidService; +use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\TransferException; use Pterodactyl\Services\DeploymentService; use Pterodactyl\Exceptions\DisplayException; @@ -722,6 +723,16 @@ class ServerRepository 'X-Access-Token' => $server->node->daemonSecret, 'X-Access-Server' => $server->uuid, ])->request('DELETE', '/servers'); + } catch (ClientException $ex) { + // Exception is thrown on 4XX HTTP errors, so catch and determine + // if we should continue, or if there is a permissions error. + // + // Daemon throws a 404 if the server doesn't exist, if that is returned + // continue with deletion, even if not a force deletion. + $response = $ex->getResponse(); + if ($ex->getResponse()->getStatusCode() !== 404 && ! $force) { + throw new DisplayException($ex->getMessage()); + } } catch (TransferException $ex) { if (! $force) { throw new DisplayException($ex->getMessage()); @@ -736,30 +747,26 @@ class ServerRepository $item->save(); }); - $server->variables->each(function ($item) { - $item->delete(); + $server->variables->each->delete(); + + $server->load('subusers.permissions'); + $server->subusers->each(function ($subuser) { + $subuser->permissions->each(function ($permission) { + $perm->delete(); + }); + $subuser->delete(); }); - foreach (Models\Subuser::with('permissions')->where('server_id', $server->id)->get() as &$subuser) { - foreach ($subuser->permissions as &$permission) { - $permission->delete(); - } - $subuser->delete(); - } - - // Remove Downloads - Models\Download::where('server', $server->uuid)->delete(); - - // Clear Tasks - Models\Task::where('server_id', $server->id)->delete(); + $server->downloads->each->delete(); + $server->tasks->each->delete(); // Delete Databases // This is the one un-recoverable point where // transactions will not save us. $repository = new DatabaseRepository; - foreach (Models\Database::select('id')->where('server_id', $server->id)->get() as $database) { - $repository->drop($database->id); - } + $server->databases->each(function ($item) { + $repository->drop($item->id); + }); // Fully delete the server. $server->delete(); @@ -786,71 +793,32 @@ class ServerRepository } /** - * Suspends a server. + * Suspends or unsuspends a server. * * @param int $id - * @param bool $deleted + * @param bool $unsuspend * @return void */ - public function suspend($id, $deleted = false) + public function toggleAccess($id, $unsuspend = true) { $server = Models\Server::with('node')->findOrFail($id); - DB::beginTransaction(); - - try { - - // Already suspended, no need to make more requests. - if ($server->suspended) { + DB::transaction(function () use ($server, $unsuspend) { + if ( + (! $unsuspend && $server->suspended) || + ($unsuspend && ! $server->suspended) + ) { return true; } - $server->suspended = 1; + $server->suspended = ! $unsuspend; $server->save(); $server->node->guzzleClient([ 'X-Access-Token' => $server->node->daemonSecret, 'X-Access-Server' => $server->uuid, - ])->request('POST', '/server/suspend'); - - return DB::commit(); - } catch (\Exception $ex) { - DB::rollBack(); - throw $ex; - } - } - - /** - * Unsuspends a server. - * - * @param int $id - * @return void - */ - public function unsuspend($id) - { - $server = Models\Server::with('node')->findOrFail($id); - - DB::beginTransaction(); - - try { - // Already unsuspended, no need to make more requests. - if ($server->suspended === 0) { - return true; - } - - $server->suspended = 0; - $server->save(); - - $server->node->guzzleClient([ - 'X-Access-Token' => $server->node->daemonSecret, - 'X-Access-Server' => $server->uuid, - ])->request('POST', '/server/unsuspend'); - - return DB::commit(); - } catch (\Exception $ex) { - DB::rollBack(); - throw $ex; - } + ])->request('POST', ($unsuspend) ? '/server/unsuspend' : '/server/suspend'); + }); } /** @@ -874,10 +842,8 @@ class ServerRepository throw new DisplayValidationException(json_encode($validator->errors())); } - DB::beginTransaction(); - $server->sftp_password = Crypt::encrypt($password); - - try { + DB::transaction(function () use ($password, $server) { + $server->sftp_password = Crypt::encrypt($password); $server->save(); $server->node->guzzleClient([ @@ -886,13 +852,6 @@ class ServerRepository ])->request('POST', '/server/password', [ 'json' => ['password' => $password], ]); - - DB::commit(); - - return true; - } catch (\Exception $ex) { - DB::rollBack(); - throw $ex; - } + }); } }