Add some additional test coverage and clean up modification service and suspension service

This commit is contained in:
Dane Everitt 2020-10-07 21:56:44 -07:00
parent 83efb2d7b6
commit d087bebc93
No known key found for this signature in database
GPG key ID: EEA66103B3D71F53
12 changed files with 214 additions and 503 deletions

View file

@ -7,6 +7,7 @@ use Illuminate\Validation\Rule;
use Illuminate\Container\Container;
use Illuminate\Contracts\Validation\Factory;
use Illuminate\Database\Eloquent\Model as IlluminateModel;
use Pterodactyl\Exceptions\Model\DataValidationException;
abstract class Model extends IlluminateModel
{
@ -55,7 +56,11 @@ abstract class Model extends IlluminateModel
static::$validatorFactory = Container::getInstance()->make(Factory::class);
static::saving(function (Model $model) {
return $model->validate();
if (! $model->validate()) {
throw new DataValidationException($model->getValidator());
}
return true;
});
}
@ -147,9 +152,9 @@ abstract class Model extends IlluminateModel
}
return $this->getValidator()->setData(
// Trying to do self::toArray() here will leave out keys based on the whitelist/blacklist
// for that model. Doing this will return all of the attributes in a format that can
// properly be validated.
// Trying to do self::toArray() here will leave out keys based on the whitelist/blacklist
// for that model. Doing this will return all of the attributes in a format that can
// properly be validated.
$this->addCastAttributesToArray(
$this->getAttributes(), $this->getMutatedAttributes()
)

View file

@ -15,7 +15,7 @@ use Znck\Eloquent\Traits\BelongsToThrough;
* @property string $name
* @property string $description
* @property bool $skip_scripts
* @property int $suspended
* @property bool $suspended
* @property int $owner_id
* @property int $memory
* @property int $swap
@ -133,7 +133,7 @@ class Server extends Model
protected $casts = [
'node_id' => 'integer',
'skip_scripts' => 'boolean',
'suspended' => 'integer',
'suspended' => 'boolean',
'owner_id' => 'integer',
'memory' => 'integer',
'swap' => 'integer',

View file

@ -55,7 +55,7 @@ class ServerConfigurationStructureService
{
return [
'uuid' => $server->uuid,
'suspended' => (bool) $server->suspended,
'suspended' => $server->suspended,
'environment' => $this->environment->handle($server),
'invocation' => $server->startup,
'skip_egg_scripts' => $server->skip_scripts,

View file

@ -2,13 +2,13 @@
namespace Pterodactyl\Services\Servers;
use Illuminate\Support\Arr;
use Pterodactyl\Models\Egg;
use Pterodactyl\Models\User;
use Pterodactyl\Models\Server;
use Pterodactyl\Models\ServerVariable;
use Illuminate\Database\ConnectionInterface;
use Pterodactyl\Traits\Services\HasUserLevels;
use Pterodactyl\Contracts\Repository\EggRepositoryInterface;
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
use Pterodactyl\Contracts\Repository\ServerVariableRepositoryInterface;
class StartupModificationService
{
@ -19,63 +19,21 @@ class StartupModificationService
*/
private $connection;
/**
* @var \Pterodactyl\Contracts\Repository\EggRepositoryInterface
*/
private $eggRepository;
/**
* @var \Pterodactyl\Services\Servers\EnvironmentService
*/
private $environmentService;
/**
* @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface
*/
private $repository;
/**
* @var \Pterodactyl\Contracts\Repository\ServerVariableRepositoryInterface
*/
private $serverVariableRepository;
/**
* @var \Pterodactyl\Services\Servers\VariableValidatorService
*/
private $validatorService;
/**
* @var \Pterodactyl\Services\Servers\ServerConfigurationStructureService
*/
private $structureService;
/**
* StartupModificationService constructor.
*
* @param \Illuminate\Database\ConnectionInterface $connection
* @param \Pterodactyl\Contracts\Repository\EggRepositoryInterface $eggRepository
* @param \Pterodactyl\Services\Servers\EnvironmentService $environmentService
* @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $repository
* @param \Pterodactyl\Services\Servers\ServerConfigurationStructureService $structureService
* @param \Pterodactyl\Contracts\Repository\ServerVariableRepositoryInterface $serverVariableRepository
* @param \Pterodactyl\Services\Servers\VariableValidatorService $validatorService
*/
public function __construct(
ConnectionInterface $connection,
EggRepositoryInterface $eggRepository,
EnvironmentService $environmentService,
ServerRepositoryInterface $repository,
ServerConfigurationStructureService $structureService,
ServerVariableRepositoryInterface $serverVariableRepository,
VariableValidatorService $validatorService
) {
public function __construct(ConnectionInterface $connection, VariableValidatorService $validatorService)
{
$this->connection = $connection;
$this->eggRepository = $eggRepository;
$this->environmentService = $environmentService;
$this->repository = $repository;
$this->serverVariableRepository = $serverVariableRepository;
$this->validatorService = $validatorService;
$this->structureService = $structureService;
}
/**
@ -85,34 +43,42 @@ class StartupModificationService
* @param array $data
* @return \Pterodactyl\Models\Server
*
* @throws \Illuminate\Validation\ValidationException
* @throws \Pterodactyl\Exceptions\Model\DataValidationException
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
* @throws \Throwable
*/
public function handle(Server $server, array $data): Server
{
$this->connection->beginTransaction();
if (! is_null(array_get($data, 'environment'))) {
$this->validatorService->setUserLevel($this->getUserLevel());
$results = $this->validatorService->handle(array_get($data, 'egg_id', $server->egg_id), array_get($data, 'environment', []));
return $this->connection->transaction(function () use ($server, $data) {
if (! empty($data['environment'])) {
$egg = $this->isUserLevel(User::USER_LEVEL_ADMIN) ? ($data['egg_id'] ?? $server->egg_id) : $server->egg_id;
$results->each(function ($result) use ($server) {
$this->serverVariableRepository->withoutFreshModel()->updateOrCreate([
'server_id' => $server->id,
'variable_id' => $result->id,
], [
'variable_value' => $result->value ?? '',
]);
});
}
$results = $this->validatorService
->setUserLevel($this->getUserLevel())
->handle($egg, $data['environment']);
if ($this->isUserLevel(User::USER_LEVEL_ADMIN)) {
$this->updateAdministrativeSettings($data, $server);
}
foreach ($results as $result) {
ServerVariable::query()->updateOrCreate(
[
'server_id' => $server->id,
'variable_id' => $result->id,
],
['variable_value' => $result->value ?? '']
);
}
}
$this->connection->commit();
if ($this->isUserLevel(User::USER_LEVEL_ADMIN)) {
$this->updateAdministrativeSettings($data, $server);
}
return $server;
// Calling ->refresh() rather than ->fresh() here causes it to return the
// variables as triplicates for some reason? Not entirely sure, should dig
// in more to figure it out, but luckily we have a test case covering this
// specific call so we can be assured we're not breaking it _here_ at least.
//
// TODO(dane): this seems like a red-flag for the code powering the relationship
// that should be looked into more.
return $server->fresh();
});
}
/**
@ -120,28 +86,27 @@ class StartupModificationService
*
* @param array $data
* @param \Pterodactyl\Models\Server $server
*
* @throws \Pterodactyl\Exceptions\Model\DataValidationException
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
*/
private function updateAdministrativeSettings(array $data, Server &$server)
protected function updateAdministrativeSettings(array $data, Server &$server)
{
if (
is_digit(array_get($data, 'egg_id'))
is_digit(Arr::get($data, 'egg_id'))
&& $data['egg_id'] != $server->egg_id
&& is_null(array_get($data, 'nest_id'))
&& is_null(Arr::get($data, 'nest_id'))
) {
$egg = $this->eggRepository->setColumns(['id', 'nest_id'])->find($data['egg_id']);
/** @var \Pterodactyl\Models\Egg $egg */
$egg = Egg::query()->select(['nest_id'])->findOrFail($data['egg_id']);
$data['nest_id'] = $egg->nest_id;
}
$server = $this->repository->update($server->id, [
$server->forceFill([
'installed' => 0,
'startup' => array_get($data, 'startup', $server->startup),
'nest_id' => array_get($data, 'nest_id', $server->nest_id),
'egg_id' => array_get($data, 'egg_id', $server->egg_id),
'skip_scripts' => array_get($data, 'skip_scripts') ?? isset($data['skip_scripts']),
'image' => array_get($data, 'docker_image', $server->image),
]);
'startup' => $data['startup'] ?? $server->startup,
'nest_id' => $data['nest_id'] ?? $server->nest_id,
'egg_id' => $data['egg_id'] ?? $server->egg_id,
'skip_scripts' => $data['skip_scripts'] ?? isset($data['skip_scripts']),
'image' => $data['docker_image'] ?? $server->image,
])->save();
}
}

View file

@ -2,12 +2,10 @@
namespace Pterodactyl\Services\Servers;
use Psr\Log\LoggerInterface;
use Webmozart\Assert\Assert;
use Pterodactyl\Models\Server;
use Illuminate\Database\ConnectionInterface;
use Pterodactyl\Repositories\Wings\DaemonServerRepository;
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
class SuspensionService
{
@ -19,16 +17,6 @@ class SuspensionService
*/
private $connection;
/**
* @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface
*/
private $repository;
/**
* @var \Psr\Log\LoggerInterface
*/
private $writer;
/**
* @var \Pterodactyl\Repositories\Wings\DaemonServerRepository
*/
@ -39,25 +27,19 @@ class SuspensionService
*
* @param \Illuminate\Database\ConnectionInterface $connection
* @param \Pterodactyl\Repositories\Wings\DaemonServerRepository $daemonServerRepository
* @param \Pterodactyl\Contracts\Repository\ServerRepositoryInterface $repository
* @param \Psr\Log\LoggerInterface $writer
*/
public function __construct(
ConnectionInterface $connection,
DaemonServerRepository $daemonServerRepository,
ServerRepositoryInterface $repository,
LoggerInterface $writer
DaemonServerRepository $daemonServerRepository
) {
$this->connection = $connection;
$this->repository = $repository;
$this->writer = $writer;
$this->daemonServerRepository = $daemonServerRepository;
}
/**
* Suspends a server on the system.
*
* @param int|\Pterodactyl\Models\Server $server
* @param \Pterodactyl\Models\Server $server
* @param string $action
*
* @throws \Throwable
@ -66,15 +48,16 @@ class SuspensionService
{
Assert::oneOf($action, [self::ACTION_SUSPEND, self::ACTION_UNSUSPEND]);
if (
$action === self::ACTION_SUSPEND && $server->suspended ||
$action === self::ACTION_UNSUSPEND && ! $server->suspended
) {
$isSuspending = $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) {
return;
}
$this->connection->transaction(function () use ($action, $server) {
$this->repository->withoutFreshModel()->update($server->id, [
$server->update([
'suspended' => $action === self::ACTION_SUSPEND,
]);

View file

@ -67,7 +67,7 @@ class ServerTransformer extends BaseClientTransformer
'allocations' => $server->allocation_limit,
'backups' => $server->backup_limit,
],
'is_suspended' => $server->suspended !== 0,
'is_suspended' => $server->suspended,
'is_installing' => $server->installed !== 1,
];
}