Remove suspended & installing fields, replace with single status field

DaneEveritt · DaneEveritt · commit a75a347d6547 · 2021-01-17T15:51:56.000-08:00
diff --git a/app/Http/Controllers/Admin/Servers/ServerViewController.php b/app/Http/Controllers/Admin/Servers/ServerViewController.php
@@ -207,7 +207,7 @@ public function mounts(Request $request, Server $server)
      */
     public function manage(Request $request, Server $server)
     {
-        if ($server->installed > 1) {
+        if ($server->status === Server::STATUS_INSTALL_FAILED) {
             throw new DisplayException(
                 'This server is in a failed install state and cannot be recovered. Please delete and re-create the server.'
             );
diff --git a/app/Http/Controllers/Admin/ServersController.php b/app/Http/Controllers/Admin/ServersController.php
@@ -228,12 +228,12 @@ public function setDetails(Request $request, Server $server)
      */
     public function toggleInstall(Server $server)
     {
-        if ($server->installed > 1) {
+        if ($server->status === Server::STATUS_INSTALL_FAILED) {
             throw new DisplayException(trans('admin/server.exceptions.marked_as_failed'));
         }
 
         $this->repository->update($server->id, [
-            'installed' => ! $server->installed,
+            'status' => $server->isInstalled() ? Server::STATUS_INSTALLING : null,
         ], true, true);
 
         $this->alert->success(trans('admin/server.alerts.install_toggled'))->flash();
diff --git a/app/Http/Controllers/Api/Remote/SftpAuthenticationController.php b/app/Http/Controllers/Api/Remote/SftpAuthenticationController.php
@@ -118,10 +118,8 @@ public function __invoke(SftpAuthenticationFormRequest $request): JsonResponse
 
         // Remember, for security purposes, only reveal the existence of the server to people that
         // have provided valid credentials, and have permissions to know about it.
-        if ($server->installed !== 1 || $server->suspended) {
-            throw new BadRequestHttpException(
-                'Server is not installed or is currently suspended.'
-            );
+        if ($server->isSuspended() || !$server->isInstalled()) {
+            throw new BadRequestHttpException('Server is not installed or is currently suspended.');
         }
 
         return new JsonResponse([
diff --git a/app/Http/Middleware/Admin/Servers/ServerInstalled.php b/app/Http/Middleware/Admin/Servers/ServerInstalled.php
@@ -29,7 +29,7 @@ public function handle(Request $request, Closure $next)
             );
         }
 
-        if ($server->installed !== 1) {
+        if (! $server->isInstalled()) {
             throw new HttpException(
                 Response::HTTP_FORBIDDEN, 'Access to this resource is not allowed due to the current installation state.'
             );
diff --git a/app/Http/Middleware/Api/Client/Server/AuthenticateServerAccess.php b/app/Http/Middleware/Api/Client/Server/AuthenticateServerAccess.php
@@ -64,7 +64,7 @@ public function handle(Request $request, Closure $next)
             }
         }
 
-        if ($server->suspended && ! $request->routeIs('api:client:server.resources')) {
+        if ($server->isSuspended() && ! $request->routeIs('api:client:server.resources')) {
             throw new BadRequestHttpException(
                 'This server is currently suspended and the functionality requested is unavailable.'
             );
diff --git a/app/Http/Middleware/Server/AccessingValidServer.php b/app/Http/Middleware/Server/AccessingValidServer.php
@@ -63,7 +63,7 @@ public function handle(Request $request, Closure $next)
         $isApiRequest = $request->expectsJson() || $request->is(...$this->config->get('pterodactyl.json_routes', []));
         $server = $this->repository->getByUuid($attributes instanceof Server ? $attributes->uuid : $attributes);
 
-        if ($server->suspended) {
+        if ($server->isSuspended()) {
             if ($isApiRequest) {
                 throw new AccessDeniedHttpException('Server is suspended and cannot be accessed.');
             }
@@ -73,7 +73,7 @@ public function handle(Request $request, Closure $next)
 
         // Servers can have install statuses other than 1 or 0, so don't check
         // for a bool-type operator here.
-        if ($server->installed !== 1) {
+        if (! $server->isInstalled()) {
             if ($isApiRequest) {
                 throw new ConflictHttpException('Server is still completing the installation process.');
             }
diff --git a/app/Models/Server.php b/app/Models/Server.php
@@ -15,8 +15,8 @@
  * @property int $node_id
  * @property string $name
  * @property string $description
+ * @property string|null $status
  * @property bool $skip_scripts
- * @property bool $suspended
  * @property int $owner_id
  * @property int $memory
  * @property int $swap
@@ -30,7 +30,6 @@
  * @property int $egg_id
  * @property string $startup
  * @property string $image
- * @property int $installed
  * @property int $allocation_limit
  * @property int $database_limit
  * @property int $backup_limit
@@ -64,9 +63,9 @@ class Server extends Model
      */
     const RESOURCE_NAME = 'server';
 
-    const STATUS_INSTALLING = 0;
-    const STATUS_INSTALLED = 1;
-    const STATUS_INSTALL_FAILED = 2;
+    const STATUS_INSTALLING = 'installing';
+    const STATUS_INSTALL_FAILED = 'install_failed';
+    const STATUS_SUSPENDED = 'suspended';
 
     /**
      * The table associated with the model.
@@ -82,6 +81,7 @@ class Server extends Model
      * @var array
      */
     protected $attributes = [
+        'status' => self::STATUS_INSTALLING,
         'oom_disabled' => true,
     ];
 
@@ -104,7 +104,7 @@ class Server extends Model
      *
      * @var array
      */
-    protected $guarded = ['id', 'installed', self::CREATED_AT, self::UPDATED_AT, 'deleted_at'];
+    protected $guarded = ['id', self::CREATED_AT, self::UPDATED_AT, 'deleted_at'];
 
     /**
      * @var array
@@ -115,6 +115,7 @@ class Server extends Model
         'name' => 'required|string|min:1|max:191',
         'node_id' => 'required|exists:nodes,id',
         'description' => 'string',
+        'status' => 'nullable|string',
         'memory' => 'required|numeric|min:0',
         'swap' => 'required|numeric|min:-1',
         'io' => 'required|numeric|between:10,1000',
@@ -142,7 +143,6 @@ class Server extends Model
     protected $casts = [
         'node_id' => 'integer',
         'skip_scripts' => 'boolean',
-        'suspended' => 'boolean',
         'owner_id' => 'integer',
         'memory' => 'integer',
         'swap' => 'integer',
@@ -153,7 +153,6 @@ class Server extends Model
         'allocation_id' => 'integer',
         'nest_id' => 'integer',
         'egg_id' => 'integer',
-        'installed' => 'integer',
         'database_limit' => 'integer',
         'allocation_limit' => 'integer',
         'backup_limit' => 'integer',
@@ -176,7 +175,12 @@ public function getAllocationMappings(): array
      */
     public function isInstalled(): bool
     {
-        return $this->installed === 1;
+        return $this->status !== self::STATUS_INSTALLING && $this->status !== self::STATUS_INSTALL_FAILED;
+    }
+
+    public function isSuspended(): bool
+    {
+        return $this->status === self::STATUS_SUSPENDED;
     }
 
     /**
diff --git a/app/Services/Servers/ReinstallServerService.php b/app/Services/Servers/ReinstallServerService.php
@@ -44,7 +44,7 @@ public function __construct(
     public function handle(Server $server)
     {
         return $this->connection->transaction(function () use ($server) {
-            $server->forceFill(['installed' => Server::STATUS_INSTALLING])->save();
+            $server->fill(['status' => Server::STATUS_INSTALLING])->save();
 
             $this->daemonServerRepository->setServer($server)->reinstall();
 
diff --git a/app/Services/Servers/ServerConfigurationStructureService.php b/app/Services/Servers/ServerConfigurationStructureService.php
@@ -60,7 +60,7 @@ protected function returnCurrentFormat(Server $server)
     {
         return [
             'uuid' => $server->uuid,
-            'suspended' => $server->suspended,
+            'suspended' => $server->isSuspended(),
             'environment' => $this->environment->handle($server),
             'invocation' => $server->startup,
             'skip_egg_scripts' => $server->skip_scripts,
@@ -137,7 +137,7 @@ protected function returnLegacyFormat(Server $server)
                 'skip_scripts' => $server->skip_scripts,
             ],
             'rebuild' => false,
-            'suspended' => (int)$server->suspended,
+            'suspended' => $server->isSuspended() ? 1 : 0,
         ];
     }
 }
diff --git a/app/Services/Servers/ServerCreationService.php b/app/Services/Servers/ServerCreationService.php
@@ -229,8 +229,8 @@ private function createModel(array $data): Server
             'node_id' => Arr::get($data, 'node_id'),
             'name' => Arr::get($data, 'name'),
             'description' => Arr::get($data, 'description') ?? '',
+            'status' => Server::STATUS_INSTALLING,
             'skip_scripts' => Arr::get($data, 'skip_scripts') ?? isset($data['skip_scripts']),
-            'suspended' => false,
             'owner_id' => Arr::get($data, 'owner_id'),
             'memory' => Arr::get($data, 'memory'),
             'swap' => Arr::get($data, 'swap'),
diff --git a/app/Services/Servers/SuspensionService.php b/app/Services/Servers/SuspensionService.php
@@ -6,7 +6,6 @@
 use Pterodactyl\Models\Server;
 use Illuminate\Database\ConnectionInterface;
 use Pterodactyl\Repositories\Wings\DaemonServerRepository;
-use Symfony\Component\HttpKernel\Exception\ConflictHttpException;
 use Pterodactyl\Exceptions\Http\Server\ServerTransferringException;
 
 class SuspensionService
@@ -54,7 +53,7 @@ public function toggle(Server $server, $action = self::ACTION_SUSPEND)
         // Nothing needs to happen if we're suspending the server and it is already
         // suspended in the database. Additionally, nothing needs to happen if the server
         // is not suspended and we try to un-suspend the instance.
-        if ($isSuspending === $server->suspended) {
+        if ($isSuspending === $server->isSuspended()) {
             return;
         }
 
@@ -63,9 +62,9 @@ public function toggle(Server $server, $action = self::ACTION_SUSPEND)
             throw new ServerTransferringException;
         }
 
-        $this->connection->transaction(function () use ($action, $server) {
+        $this->connection->transaction(function () use ($action, $server, $isSuspending) {
             $server->update([
-                'suspended' => $action === self::ACTION_SUSPEND,
+                'status' => $isSuspending ? Server::STATUS_SUSPENDED : null,
             ]);
 
             // Only send the suspension request to wings if the server is not currently being transferred.
diff --git a/app/Transformers/Api/Application/ServerTransformer.php b/app/Transformers/Api/Application/ServerTransformer.php
@@ -66,7 +66,7 @@ public function transform(Server $server): array
             'identifier' => $server->uuidShort,
             'name' => $server->name,
             'description' => $server->description,
-            'suspended' => (bool) $server->suspended,
+            'suspended' => $server->isSuspended(),
             'limits' => [
                 'memory' => $server->memory,
                 'swap' => $server->swap,
@@ -88,7 +88,7 @@ public function transform(Server $server): array
             'container' => [
                 'startup_command' => $server->startup,
                 'image' => $server->image,
-                'installed' => (int) $server->installed === 1,
+                'installed' => $server->isInstalled() ? 1 : 0,
                 'environment' => $this->environmentService->handle($server),
             ],
             $server->getUpdatedAtColumn() => $this->formatTimestamp($server->updated_at),
diff --git a/app/Transformers/Api/Client/ServerTransformer.php b/app/Transformers/Api/Client/ServerTransformer.php
@@ -70,8 +70,8 @@ public function transform(Server $server): array
                 'allocations' => $server->allocation_limit,
                 'backups' => $server->backup_limit,
             ],
-            'is_suspended' => $server->suspended,
-            'is_installing' => $server->installed !== 1,
+            'is_suspended' => $server->isSuspended(),
+            'is_installing' => ! $server->isInstalled(),
             'is_transferring' => ! is_null($server->transfer),
         ];
     }
diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php
@@ -27,14 +27,13 @@
         'name' => $faker->firstName,
         'description' => implode(' ', $faker->sentences()),
         'skip_scripts' => 0,
-        'suspended' => 0,
+        'status' => null,
         'memory' => 512,
         'swap' => 0,
         'disk' => 512,
         'io' => 500,
         'cpu' => 0,
         'oom_disabled' => 0,
-        'installed' => 1,
         'database_limit' => null,
         'allocation_limit' => null,
         'created_at' => Carbon::now(),
diff --git a/database/migrations/2021_01_17_152623_add_generic_server_status_column.php b/database/migrations/2021_01_17_152623_add_generic_server_status_column.php
@@ -0,0 +1,55 @@
+<?php
+
+use Illuminate\Support\Facades\DB;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class AddGenericServerStatusColumn extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('servers', function (Blueprint $table) {
+            $table->string('status')->nullable()->after('description');
+        });
+
+        DB::transaction(function () {
+            DB::update('UPDATE servers SET `status` = \'suspended\' WHERE `suspended` = 1');
+            DB::update('UPDATE servers SET `status` = \'installing\' WHERE `installed` = 0');
+            DB::update('UPDATE servers SET `status` = \'install_failed\' WHERE `installed` = 2');
+        });
+
+        Schema::table('servers', function (Blueprint $table) {
+            $table->dropColumn('suspended');
+            $table->dropColumn('installed');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('servers', function (Blueprint $table) {
+            $table->unsignedTinyInteger('suspended')->default(0);
+            $table->unsignedTinyInteger('installed')->default(0);
+        });
+
+        DB::transaction(function () {
+            DB::update('UPDATE servers SET `suspended` = 1 WHERE `status` = \'suspended\'');
+            DB::update('UPDATE servers SET `installed` = 1 WHERE `status` IS NULL');
+            DB::update('UPDATE servers SET `installed` = 2 WHERE `status` = \'install_failed\'');
+        });
+
+        Schema::table('servers', function (Blueprint $table) {
+            $table->dropColumn('status');
+        });
+    }
+}
diff --git a/resources/views/admin/servers/index.blade.php b/resources/views/admin/servers/index.blade.php
@@ -57,9 +57,9 @@
                                     <code>{{ $server->allocation->alias }}:{{ $server->allocation->port }}</code>
                                 </td>
                                 <td class="text-center">
-                                    @if($server->suspended)
+                                    @if($server->isSuspended())
                                         <span class="label bg-maroon">Suspended</span>
-                                    @elseif(! $server->installed)
+                                    @elseif(! $server->isInstalled())
                                         <span class="label label-warning">Installing</span>
                                     @else
                                         <span class="label label-success">Active</span>
diff --git a/resources/views/admin/servers/partials/navigation.blade.php b/resources/views/admin/servers/partials/navigation.blade.php
@@ -8,7 +8,7 @@
             <ul class="nav nav-tabs">
                 <li class="{{ $router->currentRouteNamed('admin.servers.view') ? 'active' : '' }}">
                     <a href="{{ route('admin.servers.view', $server->id) }}">About</a></li>
-                @if($server->installed === 1)
+                @if($server->isInstalled())
                     <li class="{{ $router->currentRouteNamed('admin.servers.view.details') ? 'active' : '' }}">
                         <a href="{{ route('admin.servers.view.details', $server->id) }}">Details</a>
                     </li>
diff --git a/resources/views/admin/servers/view/index.blade.php b/resources/views/admin/servers/view/index.blade.php
@@ -133,7 +133,7 @@
         <div class="box box-primary">
             <div class="box-body" style="padding-bottom: 0px;">
                 <div class="row">
-                    @if($server->suspended)
+                    @if($server->isSuspended())
                         <div class="col-sm-12">
                             <div class="small-box bg-yellow">
                                 <div class="inner">
@@ -142,11 +142,11 @@
                             </div>
                         </div>
                     @endif
-                    @if($server->installed !== 1)
+                    @if(!$server->isInstalled())
                         <div class="col-sm-12">
-                            <div class="small-box {{ (! $server->installed) ? 'bg-blue' : 'bg-maroon' }}">
+                            <div class="small-box {{ (! $server->isInstalled()) ? 'bg-blue' : 'bg-maroon' }}">
                                 <div class="inner">
-                                    <h3 class="no-margin">{{ (! $server->installed) ? 'Installing' : 'Install Failed' }}</h3>
+                                    <h3 class="no-margin">{{ (! $server->isInstalled()) ? 'Installing' : 'Install Failed' }}</h3>
                                 </div>
                             </div>
                         </div>
diff --git a/resources/views/admin/servers/view/manage.blade.php b/resources/views/admin/servers/view/manage.blade.php
@@ -31,7 +31,7 @@
                     <p>This will reinstall the server with the assigned service scripts. <strong>Danger!</strong> This could overwrite server data.</p>
                 </div>
                 <div class="box-footer">
-                    @if($server->installed === 1)
+                    @if($server->isInstalled())
                         <form action="{{ route('admin.servers.view.manage.reinstall', $server->id) }}" method="POST">
                             {!! csrf_field() !!}
                             <button type="submit" class="btn btn-danger">Reinstall Server</button>
@@ -59,7 +59,7 @@
             </div>
         </div>
 
-        @if(! $server->suspended)
+        @if(! $server->isSuspended())
             <div class="col-sm-4">
                 <div class="box box-warning">
                     <div class="box-header with-border">
diff --git a/tests/Integration/Api/Client/Server/SettingsControllerTest.php b/tests/Integration/Api/Client/Server/SettingsControllerTest.php
diff --git a/tests/Integration/Services/Servers/ServerCreationServiceTest.php b/tests/Integration/Services/Servers/ServerCreationServiceTest.php
diff --git a/tests/Integration/Services/Servers/StartupModificationServiceTest.php b/tests/Integration/Services/Servers/StartupModificationServiceTest.php
diff --git a/tests/Integration/Services/Servers/SuspensionServiceTest.php b/tests/Integration/Services/Servers/SuspensionServiceTest.php
diff --git a/tests/Unit/Http/Middleware/Server/AccessingValidServerTest.php b/tests/Unit/Http/Middleware/Server/AccessingValidServerTest.php

Original file line number	Diff line number	Diff line change
`@@ -207,7 +207,7 @@ public function mounts(Request $request, Server $server)`
`207`	`207`	`*/`
`208`	`208`	`public function manage(Request $request, Server $server)`
`209`	`209`	`{`
`210`		`- if ($server->installed > 1) {`
	`210`	`+ if ($server->status === Server::STATUS_INSTALL_FAILED) {`
`211`	`211`	`throw new DisplayException(`
`212`	`212`	`'This server is in a failed install state and cannot be recovered. Please delete and re-create the server.'`
`213`	`213`	`);`
Original file line number	Diff line number	Diff line change
`@@ -228,12 +228,12 @@ public function setDetails(Request $request, Server $server)`
`228`	`228`	`*/`
`229`	`229`	`public function toggleInstall(Server $server)`
`230`	`230`	`{`
`231`		`- if ($server->installed > 1) {`
	`231`	`+ if ($server->status === Server::STATUS_INSTALL_FAILED) {`
`232`	`232`	`throw new DisplayException(trans('admin/server.exceptions.marked_as_failed'));`
`233`	`233`	`}`
`234`	`234`
`235`	`235`	`$this->repository->update($server->id, [`
`236`		`- 'installed' => ! $server->installed,`
	`236`	`+ 'status' => $server->isInstalled() ? Server::STATUS_INSTALLING : null,`
`237`	`237`	`], true, true);`
`238`	`238`
`239`	`239`	`$this->alert->success(trans('admin/server.alerts.install_toggled'))->flash();`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public function handle(Request $request, Closure $next)`
`29`	`29`	`);`
`30`	`30`	`}`
`31`	`31`
`32`		`- if ($server->installed !== 1) {`
	`32`	`+ if (! $server->isInstalled()) {`
`33`	`33`	`throw new HttpException(`
`34`	`34`	`Response::HTTP_FORBIDDEN, 'Access to this resource is not allowed due to the current installation state.'`
`35`	`35`	`);`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ public function handle(Request $request, Closure $next)`
`64`	`64`	`}`
`65`	`65`	`}`
`66`	`66`
`67`		`- if ($server->suspended && ! $request->routeIs('api:client:server.resources')) {`
	`67`	`+ if ($server->isSuspended() && ! $request->routeIs('api:client:server.resources')) {`
`68`	`68`	`throw new BadRequestHttpException(`
`69`	`69`	`'This server is currently suspended and the functionality requested is unavailable.'`
`70`	`70`	`);`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ public function __construct(`
`44`	`44`	`public function handle(Server $server)`
`45`	`45`	`{`
`46`	`46`	`return $this->connection->transaction(function () use ($server) {`
`47`		`- $server->forceFill(['installed' => Server::STATUS_INSTALLING])->save();`
	`47`	`+ $server->fill(['status' => Server::STATUS_INSTALLING])->save();`
`48`	`48`
`49`	`49`	`$this->daemonServerRepository->setServer($server)->reinstall();`
`50`	`50`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ protected function returnCurrentFormat(Server $server)`
`60`	`60`	`{`
`61`	`61`	`return [`
`62`	`62`	`'uuid' => $server->uuid,`
`63`		`- 'suspended' => $server->suspended,`
	`63`	`+ 'suspended' => $server->isSuspended(),`
`64`	`64`	`'environment' => $this->environment->handle($server),`
`65`	`65`	`'invocation' => $server->startup,`
`66`	`66`	`'skip_egg_scripts' => $server->skip_scripts,`
`@@ -137,7 +137,7 @@ protected function returnLegacyFormat(Server $server)`
`137`	`137`	`'skip_scripts' => $server->skip_scripts,`
`138`	`138`	`],`
`139`	`139`	`'rebuild' => false,`
`140`		`- 'suspended' => (int)$server->suspended,`
	`140`	`+ 'suspended' => $server->isSuspended() ? 1 : 0,`
`141`	`141`	`];`
`142`	`142`	`}`
`143`	`143`	`}`