diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bbbd84f0..67f54d40c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ This project follows [Semantic Versioning](http://semver.org) guidelines. value when showing an error state. * Mass deleting files now executes properly and doesn't result in a JS console error. * Scrolling on email settings page now works. +* Database host management will now properly display an error message to the user when there is any type of MySQL related +error encountered during creation or update. ### Added * Server listing view now displays the total used disk space for each server. diff --git a/app/Http/Controllers/Admin/DatabaseController.php b/app/Http/Controllers/Admin/DatabaseController.php index affb0c80e..777b27792 100644 --- a/app/Http/Controllers/Admin/DatabaseController.php +++ b/app/Http/Controllers/Admin/DatabaseController.php @@ -2,6 +2,7 @@ namespace Pterodactyl\Http\Controllers\Admin; +use Exception; use PDOException; use Illuminate\View\View; use Pterodactyl\Models\DatabaseHost; @@ -118,17 +119,22 @@ class DatabaseController extends Controller * @param \Pterodactyl\Http\Requests\Admin\DatabaseHostFormRequest $request * @return \Illuminate\Http\RedirectResponse * - * @throws \Pterodactyl\Exceptions\Model\DataValidationException - * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException + * @throws \Throwable */ public function create(DatabaseHostFormRequest $request): RedirectResponse { try { $host = $this->creationService->handle($request->normalize()); - } catch (PDOException $ex) { - $this->alert->danger($ex->getMessage())->flash(); + } catch (Exception $exception) { + if ($exception instanceof PDOException || $exception->getPrevious() instanceof PDOException) { + $this->alert->danger( + sprintf('There was an error while trying to connect to the host or while executing a query: "%s"', $exception->getMessage()) + )->flash(); - return redirect()->route('admin.databases'); + redirect()->route('admin.databases')->withInput($request->validated()); + } else { + throw $exception; + } } $this->alert->success('Successfully created a new database host on the system.')->flash(); @@ -143,8 +149,7 @@ class DatabaseController extends Controller * @param \Pterodactyl\Models\DatabaseHost $host * @return \Illuminate\Http\RedirectResponse * - * @throws \Pterodactyl\Exceptions\Model\DataValidationException - * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException + * @throws \Throwable */ public function update(DatabaseHostFormRequest $request, DatabaseHost $host): RedirectResponse { @@ -153,9 +158,17 @@ class DatabaseController extends Controller try { $this->updateService->handle($host->id, $request->normalize()); $this->alert->success('Database host was updated successfully.')->flash(); - } catch (PDOException $ex) { - $this->alert->danger($ex->getMessage())->flash(); - $redirect->withInput($request->normalize()); + } catch (Exception $exception) { + // Catch any SQL related exceptions and display them back to the user, otherwise just + // throw the exception like normal and move on with it. + if ($exception instanceof PDOException || $exception->getPrevious() instanceof PDOException) { + $this->alert->danger( + sprintf('There was an error while trying to connect to the host or while executing a query: "%s"', $exception->getMessage()) + )->flash(); + $redirect->withInput($request->normalize()); + } else { + throw $exception; + } } return $redirect; diff --git a/app/Services/Databases/Hosts/HostCreationService.php b/app/Services/Databases/Hosts/HostCreationService.php index 15b32ea04..d6ea2f485 100644 --- a/app/Services/Databases/Hosts/HostCreationService.php +++ b/app/Services/Databases/Hosts/HostCreationService.php @@ -65,28 +65,26 @@ class HostCreationService * @param array $data * @return \Pterodactyl\Models\DatabaseHost * - * @throws \Pterodactyl\Exceptions\Model\DataValidationException - * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException + * @throws \Throwable */ public function handle(array $data): DatabaseHost { - $this->connection->beginTransaction(); + return $this->connection->transaction(function () use ($data) { + $host = $this->repository->create([ + 'password' => $this->encrypter->encrypt(array_get($data, 'password')), + 'name' => array_get($data, 'name'), + 'host' => array_get($data, 'host'), + 'port' => array_get($data, 'port'), + 'username' => array_get($data, 'username'), + 'max_databases' => null, + 'node_id' => array_get($data, 'node_id'), + ]); - $host = $this->repository->create([ - 'password' => $this->encrypter->encrypt(array_get($data, 'password')), - 'name' => array_get($data, 'name'), - 'host' => array_get($data, 'host'), - 'port' => array_get($data, 'port'), - 'username' => array_get($data, 'username'), - 'max_databases' => null, - 'node_id' => array_get($data, 'node_id'), - ]); + // Confirm access using the provided credentials before saving data. + $this->dynamic->set('dynamic', $host); + $this->databaseManager->connection('dynamic')->select('SELECT 1 FROM dual'); - // Confirm access using the provided credentials before saving data. - $this->dynamic->set('dynamic', $host); - $this->databaseManager->connection('dynamic')->select('SELECT 1 FROM dual'); - $this->connection->commit(); - - return $host; + return $host; + }); } } diff --git a/app/Services/Databases/Hosts/HostUpdateService.php b/app/Services/Databases/Hosts/HostUpdateService.php index 5f4b19b31..cb6312d21 100644 --- a/app/Services/Databases/Hosts/HostUpdateService.php +++ b/app/Services/Databases/Hosts/HostUpdateService.php @@ -71,10 +71,9 @@ class HostUpdateService * * @param int $hostId * @param array $data - * @return mixed + * @return \Pterodactyl\Models\DatabaseHost * - * @throws \Pterodactyl\Exceptions\Model\DataValidationException - * @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException + * @throws \Throwable */ public function handle(int $hostId, array $data): DatabaseHost { @@ -84,13 +83,12 @@ class HostUpdateService unset($data['password']); } - $this->connection->beginTransaction(); - $host = $this->repository->update($hostId, $data); + return $this->connection->transaction(function () use ($data, $hostId) { + $host = $this->repository->update($hostId, $data); + $this->dynamic->set('dynamic', $host); + $this->databaseManager->connection('dynamic')->select('SELECT 1 FROM dual'); - $this->dynamic->set('dynamic', $host); - $this->databaseManager->connection('dynamic')->select('SELECT 1 FROM dual'); - $this->connection->commit(); - - return $host; + return $host; + }); } } diff --git a/tests/Unit/Services/Databases/Hosts/HostCreationServiceTest.php b/tests/Unit/Services/Databases/Hosts/HostCreationServiceTest.php index 603b871a0..ea28637df 100644 --- a/tests/Unit/Services/Databases/Hosts/HostCreationServiceTest.php +++ b/tests/Unit/Services/Databases/Hosts/HostCreationServiceTest.php @@ -60,18 +60,20 @@ class HostCreationServiceTest extends TestCase { $model = factory(DatabaseHost::class)->make(); - $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); - $this->encrypter->shouldReceive('encrypt')->with('test123')->once()->andReturn('enc123'); - $this->repository->shouldReceive('create')->with(m::subset([ + $this->connection->expects('transaction')->with(m::on(function ($closure) { + return ! is_null($closure()); + }))->andReturn($model); + + $this->encrypter->expects('encrypt')->with('test123')->andReturn('enc123'); + $this->repository->expects('create')->with(m::subset([ 'password' => 'enc123', 'username' => $model->username, 'node_id' => $model->node_id, - ]))->once()->andReturn($model); + ]))->andReturn($model); - $this->dynamic->shouldReceive('set')->with('dynamic', $model)->once()->andReturnNull(); - $this->databaseManager->shouldReceive('connection')->with('dynamic')->once()->andReturnSelf(); - $this->databaseManager->shouldReceive('select')->with('SELECT 1 FROM dual')->once()->andReturnNull(); - $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); + $this->dynamic->expects('set')->with('dynamic', $model)->andReturnNull(); + $this->databaseManager->expects('connection')->with('dynamic')->andReturnSelf(); + $this->databaseManager->expects('select')->with('SELECT 1 FROM dual')->andReturnNull(); $response = $this->getService()->handle([ 'password' => 'test123', diff --git a/tests/Unit/Services/Databases/Hosts/HostUpdateServiceTest.php b/tests/Unit/Services/Databases/Hosts/HostUpdateServiceTest.php index 7e115c000..fa7454ca1 100644 --- a/tests/Unit/Services/Databases/Hosts/HostUpdateServiceTest.php +++ b/tests/Unit/Services/Databases/Hosts/HostUpdateServiceTest.php @@ -60,14 +60,15 @@ class HostUpdateServiceTest extends TestCase { $model = factory(DatabaseHost::class)->make(); - $this->encrypter->shouldReceive('encrypt')->with('test123')->once()->andReturn('enc123'); - $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); - $this->repository->shouldReceive('update')->with(1234, ['password' => 'enc123'])->once()->andReturn($model); + $this->connection->expects('transaction')->with(m::on(function ($closure) { + return ! is_null($closure()); + }))->andReturn($model); - $this->dynamic->shouldReceive('set')->with('dynamic', $model)->once()->andReturnNull(); - $this->databaseManager->shouldReceive('connection')->with('dynamic')->once()->andReturnSelf(); - $this->databaseManager->shouldReceive('select')->with('SELECT 1 FROM dual')->once()->andReturnNull(); - $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); + $this->encrypter->expects('encrypt')->with('test123')->andReturn('enc123'); + $this->repository->expects('update')->with(1234, ['password' => 'enc123'])->andReturn($model); + $this->dynamic->expects('set')->with('dynamic', $model)->andReturnNull(); + $this->databaseManager->expects('connection')->with('dynamic')->andReturnSelf(); + $this->databaseManager->expects('select')->with('SELECT 1 FROM dual')->andReturnNull(); $response = $this->getService()->handle(1234, ['password' => 'test123']); $this->assertNotEmpty($response); @@ -81,13 +82,14 @@ class HostUpdateServiceTest extends TestCase { $model = factory(DatabaseHost::class)->make(); - $this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull(); - $this->repository->shouldReceive('update')->with(1234, ['username' => 'test'])->once()->andReturn($model); + $this->connection->expects('transaction')->with(m::on(function ($closure) { + return ! is_null($closure()); + }))->andReturn($model); - $this->dynamic->shouldReceive('set')->with('dynamic', $model)->once()->andReturnNull(); - $this->databaseManager->shouldReceive('connection')->with('dynamic')->once()->andReturnSelf(); - $this->databaseManager->shouldReceive('select')->with('SELECT 1 FROM dual')->once()->andReturnNull(); - $this->connection->shouldReceive('commit')->withNoArgs()->once()->andReturnNull(); + $this->repository->expects('update')->with(1234, ['username' => 'test'])->andReturn($model); + $this->dynamic->expects('set')->with('dynamic', $model)->andReturnNull(); + $this->databaseManager->expects('connection')->with('dynamic')->andReturnSelf(); + $this->databaseManager->expects('select')->with('SELECT 1 FROM dual')->andReturnNull(); $response = $this->getService()->handle(1234, ['password' => '', 'username' => 'test']); $this->assertNotEmpty($response);