Close cleanup; only try to run power actions against non-suspended & installed servers; closes pterodactyl#2760

DaneEveritt · DaneEveritt · commit 16f49f8dc1be · 2020-11-29T12:50:22.000-08:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,10 @@ This file is a running track of new features and fixes to each version of the pa
 
 This project follows [Semantic Versioning](http://semver.org) guidelines.
 
+## v1.1.3
+### Fixed
+* Server bulk power actions command will no longer attempt to run commands against installing or suspended servers.
+
 ## v1.1.2
 ### Fixed
 * Fixes an exception thrown while trying to validate IP access for the client API.
diff --git a/app/Console/Commands/Server/BulkPowerActionCommand.php b/app/Console/Commands/Server/BulkPowerActionCommand.php
@@ -2,30 +2,15 @@
 
 namespace Pterodactyl\Console\Commands\Server;
 
+use Pterodactyl\Models\Server;
 use Illuminate\Console\Command;
-use GuzzleHttp\Exception\RequestException;
 use Illuminate\Validation\ValidationException;
 use Illuminate\Validation\Factory as ValidatorFactory;
 use Pterodactyl\Repositories\Wings\DaemonPowerRepository;
-use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
+use Pterodactyl\Exceptions\Http\Connection\DaemonConnectionException;
 
 class BulkPowerActionCommand extends Command
 {
-    /**
-     * @var \Pterodactyl\Repositories\Wings\DaemonPowerRepository
-     */
-    private $powerRepository;
-
-    /**
-     * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface
-     */
-    private $repository;
-
-    /**
-     * @var \Illuminate\Validation\Factory
-     */
-    private $validator;
-
     /**
      * @var string
      */
@@ -40,36 +25,19 @@ class BulkPowerActionCommand extends Command
     protected $description = 'Perform bulk power management on large groupings of servers or nodes at once.';
 
     /**
-     * BulkPowerActionCommand constructor.
+     * Handle the bulk power request.
      *
      * @param \Pterodactyl\Repositories\Wings\DaemonPowerRepository $powerRepository
-     * @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $repository
      * @param \Illuminate\Validation\Factory $validator
-     */
-    public function __construct(
-        DaemonPowerRepository $powerRepository,
-        ServerRepositoryInterface $repository,
-        ValidatorFactory $validator
-    ) {
-        parent::__construct();
-
-        $this->repository = $repository;
-        $this->validator = $validator;
-        $this->powerRepository = $powerRepository;
-    }
-
-    /**
-     * Handle the bulk power request.
-     *
      * @throws \Illuminate\Validation\ValidationException
      */
-    public function handle()
+    public function handle(DaemonPowerRepository $powerRepository, ValidatorFactory $validator)
     {
         $action = $this->argument('action');
         $nodes = empty($this->option('nodes')) ? [] : explode(',', $this->option('nodes'));
         $servers = empty($this->option('servers')) ? [] : explode(',', $this->option('servers'));
 
-        $validator = $this->validator->make([
+        $validator = $validator->make([
             'action' => $action,
             'nodes' => $nodes,
             'servers' => $servers,
@@ -89,23 +57,18 @@ public function handle()
             throw new ValidationException($validator);
         }
 
-        $count = $this->repository->getServersForPowerActionCount($servers, $nodes);
+        $count = $this->getQueryBuilder($servers, $nodes)->count();
         if (! $this->confirm(trans('command/messages.server.power.confirm', ['action' => $action, 'count' => $count])) && $this->input->isInteractive()) {
             return;
         }
 
         $bar = $this->output->createProgressBar($count);
-        $servers = $this->repository->getServersForPowerAction($servers, $nodes);
-
-        $servers->each(function ($server) use ($action, &$bar) {
+        $this->getQueryBuilder($servers, $nodes)->each(function (Server $server) use ($action, $powerRepository, &$bar) {
             $bar->clear();
 
             try {
-                $this->powerRepository
-                    ->setNode($server->node)
-                    ->setServer($server)
-                    ->send($action);
-            } catch (RequestException $exception) {
+                $powerRepository->setServer($server)->send($action);
+            } catch (DaemonConnectionException $exception) {
                 $this->output->error(trans('command/messages.server.power.action_failed', [
                     'name' => $server->name,
                     'id' => $server->id,
@@ -120,4 +83,28 @@ public function handle()
 
         $this->line('');
     }
+
+    /**
+     * Returns the query builder instance that will return the servers that should be affected.
+     *
+     * @param array $servers
+     * @param array $nodes
+     * @return \Illuminate\Database\Eloquent\Builder
+     */
+    protected function getQueryBuilder(array $servers, array $nodes)
+    {
+        $instance = Server::query()
+            ->where('suspended', false)
+            ->where('installed', Server::STATUS_INSTALLED);
+
+        if (! empty($nodes) && ! empty($servers)) {
+            $instance->whereIn('id', $servers)->orWhereIn('node_id', $nodes);
+        } else if (empty($nodes) && ! empty($servers)) {
+            $instance->whereIn('id', $servers);
+        } else if (! empty($nodes) && empty($servers)) {
+            $instance->whereIn('node_id', $nodes);
+        }
+
+        return $instance->with('node');
+    }
 }
diff --git a/app/Contracts/Repository/ServerRepositoryInterface.php b/app/Contracts/Repository/ServerRepositoryInterface.php
@@ -95,25 +95,6 @@ public function getDaemonServiceData(Server $server, bool $refresh = false): arr
      */
     public function getByUuid(string $uuid): Server;
 
-    /**
-     * Return all of the servers that should have a power action performed against them.
-     *
-     * @param int[] $servers
-     * @param int[] $nodes
-     * @param bool $returnCount
-     * @return int|\Illuminate\Support\LazyCollection
-     */
-    public function getServersForPowerAction(array $servers = [], array $nodes = [], bool $returnCount = false);
-
-    /**
-     * Return the total number of servers that will be affected by the query.
-     *
-     * @param int[] $servers
-     * @param int[] $nodes
-     * @return int
-     */
-    public function getServersForPowerActionCount(array $servers = [], array $nodes = []): int;
-
     /**
      * Check if a given UUID and UUID-Short string are unique to a server.
      *
diff --git a/app/Repositories/Eloquent/ServerRepository.php b/app/Repositories/Eloquent/ServerRepository.php
@@ -194,45 +194,6 @@ public function getByUuid(string $uuid): Server
         }
     }
 
-    /**
-     * Return all of the servers that should have a power action performed against them.
-     *
-     * @param int[] $servers
-     * @param int[] $nodes
-     * @param bool $returnCount
-     * @return int|\Illuminate\Support\LazyCollection
-     */
-    public function getServersForPowerAction(array $servers = [], array $nodes = [], bool $returnCount = false)
-    {
-        $instance = $this->getBuilder();
-
-        if (! empty($nodes) && ! empty($servers)) {
-            $instance->whereIn('id', $servers)->orWhereIn('node_id', $nodes);
-        } else if (empty($nodes) && ! empty($servers)) {
-            $instance->whereIn('id', $servers);
-        } else if (! empty($nodes) && empty($servers)) {
-            $instance->whereIn('node_id', $nodes);
-        }
-
-        if ($returnCount) {
-            return $instance->count();
-        }
-
-        return $instance->with('node')->cursor();
-    }
-
-    /**
-     * Return the total number of servers that will be affected by the query.
-     *
-     * @param int[] $servers
-     * @param int[] $nodes
-     * @return int
-     */
-    public function getServersForPowerActionCount(array $servers = [], array $nodes = []): int
-    {
-        return $this->getServersForPowerAction($servers, $nodes, true);
-    }
-
     /**
      * Check if a given UUID and UUID-Short string are unique to a server.
      *
diff --git a/tests/Unit/Commands/Server/BulkPowerActionCommandTest.php b/tests/Unit/Commands/Server/BulkPowerActionCommandTest.php
@@ -53,7 +53,7 @@ public function testSendAction()
         $this->repository->expects('getServersForPowerAction')->with([], [])->andReturn($servers);
 
         for ($i = 0; $i < count($servers); $i++) {
-            $this->powerRepository->expects('setNode->setServer->send')->with('kill')->andReturnNull();
+            $this->powerRepository->expects('setServer->send')->with('kill')->andReturnNull();
         }
 
         $display = $this->runCommand($this->getCommand(), ['action' => 'kill'], ['yes']);
@@ -107,7 +107,7 @@ public function testSendWithEmptyOptions()
             ->andReturn(1);
 
         $this->repository->expects('getServersForPowerAction')->with([], [])->andReturn(Collection::make([$server]));
-        $this->powerRepository->expects('setNode->setServer->send')->with('kill')->andReturnNull();
+        $this->powerRepository->expects('setServer->send')->with('kill')->andReturnNull();
 
         $display = $this->runCommand($this->getCommand(), [
             'action' => 'kill',
