diff --git a/app/Exceptions/Service/ServiceOption/HasChildrenException.php b/app/Exceptions/Service/ServiceOption/HasChildrenException.php new file mode 100644 index 000000000..0857bb887 --- /dev/null +++ b/app/Exceptions/Service/ServiceOption/HasChildrenException.php @@ -0,0 +1,16 @@ +. + * + * This software is licensed under the terms of the MIT license. + * https://opensource.org/licenses/MIT + */ + +namespace Pterodactyl\Exceptions\Service\ServiceOption; + +use Pterodactyl\Exceptions\DisplayException; + +class HasChildrenException extends DisplayException +{ +} diff --git a/app/Services/Services/Options/OptionDeletionService.php b/app/Services/Services/Options/OptionDeletionService.php index 6e9ea7909..27788ca5c 100644 --- a/app/Services/Services/Options/OptionDeletionService.php +++ b/app/Services/Services/Options/OptionDeletionService.php @@ -9,9 +9,11 @@ namespace Pterodactyl\Services\Services\Options; +use Webmozart\Assert\Assert; use Pterodactyl\Exceptions\Service\HasActiveServersException; use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; use Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface; +use Pterodactyl\Exceptions\Service\ServiceOption\HasChildrenException; class OptionDeletionService { @@ -46,17 +48,22 @@ class OptionDeletionService * @return int * * @throws \Pterodactyl\Exceptions\Service\HasActiveServersException + * @throws \Pterodactyl\Exceptions\Service\ServiceOption\HasChildrenException */ public function handle($option) { - $servers = $this->serverRepository->findCountWhere([ - ['option_id', '=', $option], - ]); + Assert::integerish($option, 'First argument passed to handle must be integer, received %s.'); + $servers = $this->serverRepository->findCountWhere([['option_id', '=', $option]]); if ($servers > 0) { throw new HasActiveServersException(trans('exceptions.service.options.delete_has_servers')); } + $children = $this->repository->findCountWhere([['config_from', '=', $option]]); + if ($children > 0) { + throw new HasChildrenException(trans('exceptions.service.options.has_children')); + } + return $this->repository->delete($option); } } diff --git a/resources/lang/en/exceptions.php b/resources/lang/en/exceptions.php index 818ad0cd0..2a8f1e047 100644 --- a/resources/lang/en/exceptions.php +++ b/resources/lang/en/exceptions.php @@ -24,6 +24,7 @@ return [ 'delete_has_servers' => 'A service option with active servers attached to it cannot be deleted from the Panel.', 'invalid_copy_id' => 'The service option selected for copying a script from either does not exist, or is copying a script itself.', 'must_be_child' => 'The "Copy Settings From" directive for this option must be a child option for the selected service.', + 'has_children' => 'This service option is a parent to one or more other options. Please delete those options before deleting this option.', ], 'variables' => [ 'env_not_unique' => 'The environment variable :name must be unique to this service option.', diff --git a/tests/Unit/Services/Services/Options/OptionDeletionServiceTest.php b/tests/Unit/Services/Services/Options/OptionDeletionServiceTest.php index 0c75a53c0..a0425bb3c 100644 --- a/tests/Unit/Services/Services/Options/OptionDeletionServiceTest.php +++ b/tests/Unit/Services/Services/Options/OptionDeletionServiceTest.php @@ -11,20 +11,22 @@ namespace Tests\Unit\Services\Services\Options; use Mockery as m; use Tests\TestCase; +use Pterodactyl\Exceptions\DisplayException; use Pterodactyl\Exceptions\Service\HasActiveServersException; use Pterodactyl\Contracts\Repository\ServerRepositoryInterface; use Pterodactyl\Services\Services\Options\OptionDeletionService; use Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface; +use Pterodactyl\Exceptions\Service\ServiceOption\HasChildrenException; class OptionDeletionServiceTest extends TestCase { /** - * @var \Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface + * @var \Pterodactyl\Contracts\Repository\ServiceOptionRepositoryInterface|\Mockery\Mock */ protected $repository; /** - * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface + * @var \Pterodactyl\Contracts\Repository\ServerRepositoryInterface|\Mockery\Mock */ protected $serverRepository; @@ -33,6 +35,9 @@ class OptionDeletionServiceTest extends TestCase */ protected $service; + /** + * Setup tests. + */ public function setUp() { parent::setUp(); @@ -49,6 +54,7 @@ class OptionDeletionServiceTest extends TestCase public function testOptionIsDeletedIfNoServersAreFound() { $this->serverRepository->shouldReceive('findCountWhere')->with([['option_id', '=', 1]])->once()->andReturn(0); + $this->repository->shouldReceive('findCountWhere')->with([['config_from', '=', 1]])->once()->andReturn(0); $this->repository->shouldReceive('delete')->with(1)->once()->andReturn(1); $this->assertEquals(1, $this->service->handle(1)); @@ -63,9 +69,25 @@ class OptionDeletionServiceTest extends TestCase try { $this->service->handle(1); - } catch (\Exception $exception) { + } catch (DisplayException $exception) { $this->assertInstanceOf(HasActiveServersException::class, $exception); $this->assertEquals(trans('exceptions.service.options.delete_has_servers'), $exception->getMessage()); } } + + /** + * Test that an exception is thrown if children options exist. + */ + public function testExceptionIsThrownIfChildrenArePresent() + { + $this->serverRepository->shouldReceive('findCountWhere')->with([['option_id', '=', 1]])->once()->andReturn(0); + $this->repository->shouldReceive('findCountWhere')->with([['config_from', '=', 1]])->once()->andReturn(1); + + try { + $this->service->handle(1); + } catch (DisplayException $exception) { + $this->assertInstanceOf(HasChildrenException::class, $exception); + $this->assertEquals(trans('exceptions.service.options.has_children'), $exception->getMessage()); + } + } }