From 83efb2d7b6f3c1d56b3c5a2d15017ab1f6a61532 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Mon, 5 Oct 2020 21:54:29 -0700 Subject: [PATCH] More fixes for broken unit tests --- .../Controllers/Admin/ServersController.php | 2 +- .../Servers/ServerManagementController.php | 2 +- .../Api/Client/Servers/SettingsController.php | 2 +- app/Services/Servers/EnvironmentService.php | 26 +----- .../Servers/ReinstallServerService.php | 18 +--- .../ServerConfigurationStructureService.php | 23 +----- .../Servers/EnvironmentServiceTest.php | 59 ++++++------- .../Servers/ReinstallServerServiceTest.php | 82 ------------------- ...erverConfigurationStructureServiceTest.php | 9 +- .../Servers/StartupCommandViewServiceTest.php | 46 ++++------- 10 files changed, 56 insertions(+), 213 deletions(-) delete mode 100644 tests/Unit/Services/Servers/ReinstallServerServiceTest.php diff --git a/app/Http/Controllers/Admin/ServersController.php b/app/Http/Controllers/Admin/ServersController.php index 3f050b847..dc2168605 100644 --- a/app/Http/Controllers/Admin/ServersController.php +++ b/app/Http/Controllers/Admin/ServersController.php @@ -252,7 +252,7 @@ class ServersController extends Controller */ public function reinstallServer(Server $server) { - $this->reinstallService->reinstall($server); + $this->reinstallService->handle($server); $this->alert->success(trans('admin/server.alerts.server_reinstalled'))->flash(); return redirect()->route('admin.servers.view.manage', $server->id); diff --git a/app/Http/Controllers/Api/Application/Servers/ServerManagementController.php b/app/Http/Controllers/Api/Application/Servers/ServerManagementController.php index 90372f220..5052c884e 100644 --- a/app/Http/Controllers/Api/Application/Servers/ServerManagementController.php +++ b/app/Http/Controllers/Api/Application/Servers/ServerManagementController.php @@ -82,7 +82,7 @@ class ServerManagementController extends ApplicationApiController */ public function reinstall(ServerWriteRequest $request, Server $server): Response { - $this->reinstallServerService->reinstall($server); + $this->reinstallServerService->handle($server); return $this->returnNoContent(); } diff --git a/app/Http/Controllers/Api/Client/Servers/SettingsController.php b/app/Http/Controllers/Api/Client/Servers/SettingsController.php index 6090906ee..7dfbf7b4f 100644 --- a/app/Http/Controllers/Api/Client/Servers/SettingsController.php +++ b/app/Http/Controllers/Api/Client/Servers/SettingsController.php @@ -69,7 +69,7 @@ class SettingsController extends ClientApiController */ public function reinstall(ReinstallServerRequest $request, Server $server) { - $this->reinstallServerService->reinstall($server); + $this->reinstallServerService->handle($server); return new JsonResponse([], Response::HTTP_ACCEPTED); } diff --git a/app/Services/Servers/EnvironmentService.php b/app/Services/Servers/EnvironmentService.php index 8aab214d1..6f7b590f7 100644 --- a/app/Services/Servers/EnvironmentService.php +++ b/app/Services/Servers/EnvironmentService.php @@ -4,8 +4,6 @@ namespace Pterodactyl\Services\Servers; use Pterodactyl\Models\Server; use Pterodactyl\Models\EggVariable; -use Illuminate\Contracts\Config\Repository as ConfigRepository; -use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; class EnvironmentService { @@ -14,28 +12,6 @@ class EnvironmentService */ private $additional = []; - /** - * @var \Illuminate\Contracts\Config\Repository - */ - private $config; - - /** - * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface - */ - private $repository; - - /** - * EnvironmentService constructor. - * - * @param \Illuminate\Contracts\Config\Repository $config - * @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $repository - */ - public function __construct(ConfigRepository $config, ServerRepositoryInterface $repository) - { - $this->config = $config; - $this->repository = $repository; - } - /** * Dynamically configure additional environment variables to be assigned * with a specific server. @@ -79,7 +55,7 @@ class EnvironmentService } // Process variables set in the configuration file. - foreach ($this->config->get('pterodactyl.environment_variables', []) as $key => $object) { + foreach (config('pterodactyl.environment_variables', []) as $key => $object) { $variables->put( $key, is_callable($object) ? call_user_func($object, $server) : object_get($server, $object) ); diff --git a/app/Services/Servers/ReinstallServerService.php b/app/Services/Servers/ReinstallServerService.php index aff0321ce..6f5b56083 100644 --- a/app/Services/Servers/ReinstallServerService.php +++ b/app/Services/Servers/ReinstallServerService.php @@ -19,26 +19,18 @@ class ReinstallServerService */ private $connection; - /** - * @var \Pterodactyl\Repositories\Eloquent\ServerRepository - */ - private $repository; - /** * ReinstallService constructor. * * @param \Illuminate\Database\ConnectionInterface $connection * @param \Pterodactyl\Repositories\Wings\DaemonServerRepository $daemonServerRepository - * @param \Pterodactyl\Repositories\Eloquent\ServerRepository $repository */ public function __construct( ConnectionInterface $connection, - DaemonServerRepository $daemonServerRepository, - ServerRepository $repository + DaemonServerRepository $daemonServerRepository ) { $this->daemonServerRepository = $daemonServerRepository; $this->connection = $connection; - $this->repository = $repository; } /** @@ -49,16 +41,14 @@ class ReinstallServerService * * @throws \Throwable */ - public function reinstall(Server $server) + public function handle(Server $server) { return $this->connection->transaction(function () use ($server) { - $updated = $this->repository->update($server->id, [ - 'installed' => Server::STATUS_INSTALLING, - ], true, true); + $server->forceFill(['installed' => Server::STATUS_INSTALLING])->save(); $this->daemonServerRepository->setServer($server)->reinstall(); - return $updated; + return $server->refresh(); }); } } diff --git a/app/Services/Servers/ServerConfigurationStructureService.php b/app/Services/Servers/ServerConfigurationStructureService.php index f17405fad..b69a5343d 100644 --- a/app/Services/Servers/ServerConfigurationStructureService.php +++ b/app/Services/Servers/ServerConfigurationStructureService.php @@ -1,17 +1,9 @@ . - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ namespace Pterodactyl\Services\Servers; use Pterodactyl\Models\Mount; use Pterodactyl\Models\Server; -use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; class ServerConfigurationStructureService { @@ -22,22 +14,13 @@ class ServerConfigurationStructureService */ private $environment; - /** - * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface - */ - private $repository; - /** * ServerConfigurationStructureService constructor. * - * @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $repository * @param \Pterodactyl\Services\Servers\EnvironmentService $environment */ - public function __construct( - ServerRepositoryInterface $repository, - EnvironmentService $environment - ) { - $this->repository = $repository; + public function __construct(EnvironmentService $environment) + { $this->environment = $environment; } @@ -112,8 +95,6 @@ class ServerConfigurationStructureService * * @param \Pterodactyl\Models\Server $server * @return array - * - * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException */ protected function returnLegacyFormat(Server $server) { diff --git a/tests/Unit/Services/Servers/EnvironmentServiceTest.php b/tests/Unit/Services/Servers/EnvironmentServiceTest.php index 8a28d8068..73a41c346 100644 --- a/tests/Unit/Services/Servers/EnvironmentServiceTest.php +++ b/tests/Unit/Services/Servers/EnvironmentServiceTest.php @@ -6,19 +6,13 @@ use Mockery as m; use Tests\TestCase; use Pterodactyl\Models\Server; use Pterodactyl\Models\Location; -use Illuminate\Contracts\Config\Repository; +use Illuminate\Support\Collection; +use Pterodactyl\Models\EggVariable; use Pterodactyl\Services\Servers\EnvironmentService; use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; class EnvironmentServiceTest extends TestCase { - const CONFIG_MAPPING = 'pterodactyl.environment_variables'; - - /** - * @var \Illuminate\Contracts\Config\Repository|\Mockery\Mock - */ - private $config; - /** * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface|\Mockery\Mock */ @@ -30,9 +24,7 @@ class EnvironmentServiceTest extends TestCase public function setUp(): void { parent::setUp(); - - $this->config = m::mock(Repository::class); - $this->repository = m::mock(ServerRepositoryInterface::class); + config()->set('pterodactyl.environment_variables', []); } /** @@ -55,15 +47,17 @@ class EnvironmentServiceTest extends TestCase */ public function testProcessShouldReturnDefaultEnvironmentVariablesForAServer() { - $model = $this->getServerModel(); - $this->config->shouldReceive('get')->with(self::CONFIG_MAPPING, [])->once()->andReturn([]); - $this->repository->shouldReceive('getVariablesWithValues')->with($model->id)->once()->andReturn([ - 'TEST_VARIABLE' => 'Test Variable', + $model = $this->getServerModel([ + 'TEST_VARIABLE' => factory(EggVariable::class)->make([ + 'id' => 987, + 'env_variable' => 'TEST_VARIABLE', + 'default_value' => 'Test Variable', + ]), ]); $response = $this->getService()->handle($model); $this->assertNotEmpty($response); - $this->assertEquals(4, count($response)); + $this->assertCount(4, $response); $this->assertArrayHasKey('TEST_VARIABLE', $response); $this->assertSame('Test Variable', $response['TEST_VARIABLE']); } @@ -73,10 +67,7 @@ class EnvironmentServiceTest extends TestCase */ public function testProcessShouldReturnKeySetAtRuntime() { - $model = $this->getServerModel(); - $this->config->shouldReceive('get')->with(self::CONFIG_MAPPING, [])->once()->andReturn([]); - $this->repository->shouldReceive('getVariablesWithValues')->with($model->id)->once()->andReturn([]); - + $model = $this->getServerModel([]); $service = $this->getService(); $service->setEnvironmentKey('TEST_VARIABLE', function ($server) { return $server->uuidShort; @@ -94,12 +85,11 @@ class EnvironmentServiceTest extends TestCase */ public function testProcessShouldAllowOverwritingVariablesWithConfigurationFile() { - $model = $this->getServerModel(); - $this->repository->shouldReceive('getVariablesWithValues')->with($model->id)->once()->andReturn([]); - $this->config->shouldReceive('get')->with(self::CONFIG_MAPPING, [])->once()->andReturn([ + config()->set('pterodactyl.environment_variables', [ 'P_SERVER_UUID' => 'name', ]); + $model = $this->getServerModel([]); $response = $this->getService()->handle($model); $this->assertNotEmpty($response); @@ -113,14 +103,13 @@ class EnvironmentServiceTest extends TestCase */ public function testVariablesSetInConfigurationAllowForClosures() { - $model = $this->getServerModel(); - $this->config->shouldReceive('get')->with(self::CONFIG_MAPPING, [])->once()->andReturn([ + config()->set('pterodactyl.environment_variables', [ 'P_SERVER_UUID' => function ($server) { return $server->id * 2; }, ]); - $this->repository->shouldReceive('getVariablesWithValues')->with($model->id)->once()->andReturn([]); + $model = $this->getServerModel([]); $response = $this->getService()->handle($model); $this->assertNotEmpty($response); @@ -135,12 +124,11 @@ class EnvironmentServiceTest extends TestCase */ public function testProcessShouldAllowOverwritingDefaultVariablesWithRuntimeProvided() { - $model = $this->getServerModel(); - $this->config->shouldReceive('get')->with(self::CONFIG_MAPPING, [])->once()->andReturn([ + config()->set('pterodactyl.environment_variables', [ 'P_SERVER_UUID' => 'overwritten-config', ]); - $this->repository->shouldReceive('getVariablesWithValues')->with($model->id)->once()->andReturn([]); + $model = $this->getServerModel([]); $service = $this->getService(); $service->setEnvironmentKey('P_SERVER_UUID', function ($model) { return 'overwritten'; @@ -161,18 +149,25 @@ class EnvironmentServiceTest extends TestCase */ private function getService(): EnvironmentService { - return new EnvironmentService($this->config, $this->repository); + return new EnvironmentService; } /** * Return a server model with a location relationship to be used in the tests. * + * @param array $variables * @return \Pterodactyl\Models\Server */ - private function getServerModel(): Server + private function getServerModel(array $variables): Server { - return factory(Server::class)->make([ + /** @var \Pterodactyl\Models\Server $server */ + $server = factory(Server::class)->make([ + 'id' => 123, 'location' => factory(Location::class)->make(), ]); + + $server->setRelation('variables', Collection::make($variables)); + + return $server; } } diff --git a/tests/Unit/Services/Servers/ReinstallServerServiceTest.php b/tests/Unit/Services/Servers/ReinstallServerServiceTest.php deleted file mode 100644 index 22cc35199..000000000 --- a/tests/Unit/Services/Servers/ReinstallServerServiceTest.php +++ /dev/null @@ -1,82 +0,0 @@ -. - * - * This software is licensed under the terms of the MIT license. - * https://opensource.org/licenses/MIT - */ - -namespace Tests\Unit\Services\Servers; - -use Mockery as m; -use Tests\TestCase; -use Pterodactyl\Models\Server; -use Illuminate\Database\ConnectionInterface; -use Pterodactyl\Repositories\Eloquent\ServerRepository; -use Pterodactyl\Services\Servers\ReinstallServerService; -use Pterodactyl\Repositories\Wings\DaemonServerRepository; - -class ReinstallServerServiceTest extends TestCase -{ - /** - * @var \Pterodactyl\Repositories\Wings\DaemonServerRepository - */ - private $daemonServerRepository; - - /** - * @var \Illuminate\Database\ConnectionInterface - */ - private $connection; - - /** - * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface - */ - private $repository; - - /** - * Setup tests. - */ - public function setUp(): void - { - parent::setUp(); - - $this->repository = m::mock(ServerRepository::class); - $this->connection = m::mock(ConnectionInterface::class); - $this->daemonServerRepository = m::mock(DaemonServerRepository::class); - } - - /** - * Test that a server is reinstalled when it's model is passed to the function. - */ - public function testServerShouldBeReinstalledWhenModelIsPassed() - { - /** @var \Pterodactyl\Models\Server $server */ - $server = factory(Server::class)->make(['id' => 123]); - $updated = clone $server; - $updated->installed = Server::STATUS_INSTALLING; - - $this->connection->expects('transaction')->with(m::on(function ($closure) use ($updated) { - return $closure() instanceof Server; - }))->andReturn($updated); - - $this->repository->expects('update')->with($server->id, [ - 'installed' => Server::STATUS_INSTALLING, - ])->andReturns($updated); - - $this->daemonServerRepository->expects('setServer')->with($server)->andReturnSelf(); - $this->daemonServerRepository->expects('reinstall')->withNoArgs(); - - $this->assertSame($updated, $this->getService()->reinstall($server)); - } - - /** - * @return \Pterodactyl\Services\Servers\ReinstallServerService - */ - private function getService() - { - return new ReinstallServerService( - $this->connection, $this->daemonServerRepository, $this->repository - ); - } -} diff --git a/tests/Unit/Services/Servers/ServerConfigurationStructureServiceTest.php b/tests/Unit/Services/Servers/ServerConfigurationStructureServiceTest.php index a757432ca..9cf838f79 100644 --- a/tests/Unit/Services/Servers/ServerConfigurationStructureServiceTest.php +++ b/tests/Unit/Services/Servers/ServerConfigurationStructureServiceTest.php @@ -57,8 +57,8 @@ class ServerConfigurationStructureServiceTest extends TestCase $this->assertArrayHasKey('suspended', $response); $this->assertArrayHasKey('environment', $response); $this->assertArrayHasKey('invocation', $response); + $this->assertArrayHasKey('skip_egg_scripts', $response); $this->assertArrayHasKey('build', $response); - $this->assertArrayHasKey('service', $response); $this->assertArrayHasKey('container', $response); $this->assertArrayHasKey('allocations', $response); @@ -79,11 +79,6 @@ class ServerConfigurationStructureServiceTest extends TestCase 'disk_space' => $model->disk, ], $response['build']); - $this->assertSame([ - 'egg' => $model->egg->uuid, - 'skip_scripts' => $model->skip_scripts, - ], $response['service']); - $this->assertSame([ 'image' => $model->image, 'oom_disabled' => $model->oom_disabled, @@ -103,6 +98,6 @@ class ServerConfigurationStructureServiceTest extends TestCase */ private function getService(): ServerConfigurationStructureService { - return new ServerConfigurationStructureService($this->repository, $this->environment); + return new ServerConfigurationStructureService($this->environment); } } diff --git a/tests/Unit/Services/Servers/StartupCommandViewServiceTest.php b/tests/Unit/Services/Servers/StartupCommandViewServiceTest.php index a16eb3865..6444b00dd 100644 --- a/tests/Unit/Services/Servers/StartupCommandViewServiceTest.php +++ b/tests/Unit/Services/Servers/StartupCommandViewServiceTest.php @@ -4,9 +4,7 @@ namespace Tests\Unit\Services\Servers; use Mockery as m; use Tests\TestCase; -use Pterodactyl\Models\Egg; use Pterodactyl\Models\Server; -use Illuminate\Support\Collection; use Pterodactyl\Models\Allocation; use Pterodactyl\Models\EggVariable; use Pterodactyl\Services\Servers\StartupCommandService; @@ -34,43 +32,33 @@ class StartupCommandViewServiceTest extends TestCase */ public function testServiceResponse() { - $allocation = factory(Allocation::class)->make(); - $egg = factory(Egg::class)->make(); + $server = factory(Server::class)->make([ + 'id' => 123, 'startup' => 'example {{SERVER_MEMORY}} {{SERVER_IP}} {{SERVER_PORT}} {{TEST_VARIABLE}} {{TEST_VARIABLE_HIDDEN}} {{UNKNOWN}}', ]); $variables = collect([ - factory(EggVariable::class)->make(['env_variable' => 'TEST_VARIABLE', 'user_viewable' => 1]), - factory(EggVariable::class)->make(['env_variable' => 'TEST_VARIABLE_HIDDEN', 'user_viewable' => 0]), + factory(EggVariable::class)->make([ + 'env_variable' => 'TEST_VARIABLE', + 'server_value' => 'Test Value', + 'user_viewable' => 1, + ]), + factory(EggVariable::class)->make([ + 'env_variable' => 'TEST_VARIABLE_HIDDEN', + 'server_value' => 'Hidden Value', + 'user_viewable' => 0, + ]), ]); - $egg->setRelation('variables', $variables); - $server->setRelation('allocation', $allocation); - $server->setRelation('egg', $egg); - - $this->repository->shouldReceive('getVariablesWithValues')->once()->with($server->id, true)->andReturn((object) [ - 'data' => [ - 'TEST_VARIABLE' => 'Test Value', - 'TEST_VARIABLE_HIDDEN' => 'Hidden Value', - ], - 'server' => $server, - ]); - - $this->repository->shouldReceive('getPrimaryAllocation')->once()->with($server)->andReturn($server); - - $response = $this->getService()->handle($server->id); - $this->assertInstanceOf(Collection::class, $response); + $server->setRelation('variables', $variables); + $server->setRelation('allocation', $allocation = factory(Allocation::class)->make()); + $response = $this->getService()->handle($server); $this->assertSame( sprintf('example %s %s %s %s %s {{UNKNOWN}}', $server->memory, $allocation->ip, $allocation->port, 'Test Value', '[hidden]'), - $response->get('startup') + $response ); - $this->assertEquals($variables->only(0), $response->get('variables')); - $this->assertSame([ - 'TEST_VARIABLE' => 'Test Value', - 'TEST_VARIABLE_HIDDEN' => 'Hidden Value', - ], $response->get('server_values')); } /** @@ -80,6 +68,6 @@ class StartupCommandViewServiceTest extends TestCase */ private function getService(): StartupCommandService { - return new StartupCommandService($this->repository); + return new StartupCommandService; } }