Skip to content

Commit

Permalink
Merge pull request #872 from PennyDreadfulMTG/database-exception
Browse files Browse the repository at this point in the history
Refactor database exceptions, fix password change buglet
  • Loading branch information
bakert authored Dec 4, 2024
2 parents 7162756 + aeaa88b commit c54b5c6
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 60 deletions.
57 changes: 28 additions & 29 deletions gatherling/Data/Db.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use function Gatherling\Helpers\config;
use function Gatherling\Helpers\logger;
use function Gatherling\Helpers\marshal;
use function Safe\json_encode;
use function Safe\preg_match;
use function Safe\preg_replace;

Expand Down Expand Up @@ -66,7 +65,7 @@ private function connect(bool $connectToDatabase = false): void
$this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$this->pdo->exec("SET time_zone = 'America/New_York'");
} catch (PDOException $e) {
throw new DatabaseException('Failed to connect to database: ' . $e->getMessage(), 0, $e);
throw new DatabaseException('Failed to connect to database: ' . $e->getMessage(), $e);
}
}

Expand All @@ -80,7 +79,7 @@ private function use(): void
try {
$this->pdo->exec('USE ' . $this->quoteIdentifier($database));
} catch (PDOException $e) {
throw new DatabaseException('Failed to connect to database: ' . $e->getMessage(), 0, $e);
throw new DatabaseException('Failed to connect to database: ' . $e->getMessage(), $e);
}
$this->connected = true;
return;
Expand Down Expand Up @@ -117,7 +116,7 @@ public function insert(string $sql, array $params = []): int
{
$ids = $this->insertMany($sql, $params);
if (count($ids) !== 1) {
throw new DatabaseException("Expected 1 id, got " . count($ids) . " for query: $sql with params " . json_encode($params));
throw new DatabaseException("Expected 1 id, got " . count($ids), null, $sql, $params);
}
return $ids[0];
}
Expand Down Expand Up @@ -166,7 +165,7 @@ public function select(string $sql, string $class, array $params = []): array
try {
$rows = $stmt->fetchAll(PDO::FETCH_CLASS, $class);
} catch (TypeError $e) {
throw new DatabaseException("Failed to fetch class $class for query: $sql with params " . json_encode($params), 0, $e);
throw new DatabaseException("Failed to fetch class $class", $e, $sql, $params);
}
return $rows;
});
Expand All @@ -184,10 +183,10 @@ public function selectOnly(string $sql, string $class, array $params = []): Dto
if (count($result) === 0) {
$type = str_replace('Dto', '', basename(str_replace('\\', '/', $class)));
$ids = array_values(array_filter($params, fn($v) => is_int($v) || is_string($v)));
throw new NotFoundInDatabaseException("Expected 1 row, got 0 for query: $sql with params " . json_encode($params), 0, null, $type, $ids);
throw new NotFoundInDatabaseException('Expected 1 row, got 0', null, $sql, $params, $type, $ids);
}
if (count($result) > 1) {
throw new DatabaseException('Expected 1 row, got ' . count($result) . " for query: $sql with params " . json_encode($params));
throw new DatabaseException('Expected 1 row, got ' . count($result), null, $sql, $params);
}
return $result[0];
}
Expand All @@ -202,7 +201,7 @@ public function selectOnlyOrNull(string $sql, string $class, array $params = [])
{
$result = $this->select($sql, $class, $params);
if (count($result) > 1) {
throw new DatabaseException('Expected 1 row, got ' . count($result) . " for query: $sql with params " . json_encode($params));
throw new DatabaseException('Expected 1 row, got ' . count($result), null, $sql, $params);
}
return $result[0] ?? null;
}
Expand All @@ -213,7 +212,7 @@ public function int(string $sql, array $params = []): int
try {
return marshal($this->value($sql, $params))->int();
} catch (MarshalException $e) {
throw new DatabaseException("Expected int value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected int value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -223,7 +222,7 @@ public function optionalInt(string $sql, array $params = []): ?int
try {
return marshal($this->value($sql, $params))->optionalInt();
} catch (MarshalException $e) {
throw new DatabaseException("Expected int value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected int value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -233,7 +232,7 @@ public function string(string $sql, array $params = []): string
try {
return marshal($this->value($sql, $params))->string();
} catch (MarshalException $e) {
throw new DatabaseException("Expected string value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected string value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -243,7 +242,7 @@ public function optionalString(string $sql, array $params = []): ?string
try {
return marshal($this->value($sql, $params))->optionalString();
} catch (MarshalException $e) {
throw new DatabaseException("Expected string value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected string value, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -253,7 +252,7 @@ public function float(string $sql, array $params = []): float
try {
$v = marshal($this->value($sql, $params))->float();
} catch (MarshalException $e) {
throw new DatabaseException("Expected float value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected float value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
return $v;
}
Expand All @@ -264,7 +263,7 @@ public function optionalFloat(string $sql, array $params = []): ?float
try {
return marshal($this->value($sql, $params))->optionalFloat();
} catch (MarshalException $e) {
throw new DatabaseException("Expected float value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected float value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -273,7 +272,7 @@ public function bool(string $sql, array $params = []): bool
{
$v = $this->optionalInt($sql, $params);
if ($v === null) {
throw new DatabaseException("Expected non-null bool value for query: $sql with params " . json_encode($params));
throw new DatabaseException('Expected non-null bool value for query', null, $sql, $params);
}
return (bool) $v;
}
Expand All @@ -293,7 +292,7 @@ private function value(string $sql, array $params = []): mixed
{
$values = $this->values($sql, $params);
if (count($values) > 1) {
throw new DatabaseException("Expected 1 value, got " . count($values) . " for query: $sql with params " . json_encode($params));
throw new DatabaseException('Expected 1 value, got ' . count($values), null, $sql, $params);
}
if (count($values) === 0) {
return null;
Expand All @@ -310,7 +309,7 @@ public function strings(string $sql, array $params = []): array
try {
return marshal($this->values($sql, $params))->strings();
} catch (MarshalException $e) {
throw new DatabaseException("Expected strings value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected strings value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -323,7 +322,7 @@ public function ints(string $sql, array $params = []): array
try {
return marshal($this->values($sql, $params))->ints();
} catch (MarshalException $e) {
throw new DatabaseException("Expected ints value for query: $sql with params " . json_encode($params) . ", got " . $e->getMessage(), 0, $e);
throw new DatabaseException('Expected ints value for query, got ' . $e->getMessage(), $e, $sql, $params);
}
}

Expand All @@ -334,7 +333,7 @@ public function ints(string $sql, array $params = []): array
private function values(string $sql, array $params = []): array
{
/** @var list<mixed> */
return $this->executeInternal($sql, $params, function ($sql, $params) {
return $this->executeInternal($sql, $params, /** @param array<string, mixed> $params */ function (string $sql, array $params) {
$stmt = $this->pdo->prepare($sql);
$this->bindParams($stmt, $params);
$stmt->execute();
Expand All @@ -353,7 +352,7 @@ public function begin(string $rawName): void
try {
$this->pdo->beginTransaction();
} catch (PDOException $e) {
throw new DatabaseException("Failed to begin transaction $rawName", 0, $e);
throw new DatabaseException("Failed to begin transaction $rawName", $e);
}
} else {
$this->execute("SAVEPOINT $name");
Expand All @@ -373,15 +372,15 @@ public function commit(string $rawName): void
try {
$this->pdo->rollback();
} catch (PDOException $e) {
throw new DatabaseException("Failed to rollback $latestTransaction while handling mismatch", 0, $e);
throw new DatabaseException("Failed to rollback $latestTransaction while handling mismatch", $e);
}
throw new DatabaseException("Asked to commit $name, but $latestTransaction is open. ROLLBACK issued.");
}
if (count($this->transactions) === 1) {
try {
$this->pdo->commit();
} catch (PDOException $e) {
throw new DatabaseException("Failed to commit $name", 0, $e);
throw new DatabaseException("Failed to commit $name", $e);
}
}
array_pop($this->transactions);
Expand All @@ -396,7 +395,7 @@ public function rollback(string $rawName): void
try {
$this->pdo->rollback();
} catch (PDOException $e) {
throw new DatabaseException("Failed to rollback $name while handling faulty rollback call", 0, $e);
throw new DatabaseException("Failed to rollback $name while handling faulty rollback call", $e);
}
throw new DatabaseException("Asked to rollback $name, but no transaction is open. ROLLBACK issued.");
}
Expand All @@ -405,7 +404,7 @@ public function rollback(string $rawName): void
try {
$this->pdo->rollback();
} catch (PDOException $e) {
throw new DatabaseException("Failed to rollback while handling incorrect rollback", 0, $e);
throw new DatabaseException("Failed to rollback while handling incorrect rollback", $e);
}
throw new DatabaseException("Asked to rollback $name, but $latestTransaction is open. ROLLBACK issued.");
}
Expand All @@ -414,7 +413,7 @@ public function rollback(string $rawName): void
try {
$this->pdo->rollback();
} catch (PDOException $e) {
throw new DatabaseException("Failed to rollback $name", 0, $e);
throw new DatabaseException("Failed to rollback $name", $e);
}
} else {
$this->execute("ROLLBACK TO SAVEPOINT $name"); // Rollback to the savepoint
Expand All @@ -441,7 +440,7 @@ public function likeEscape(string $value): string

// Debugging code, don't use for anything else, probably bad.
// See: https://stackoverflow.com/questions/210564/getting-raw-sql-query-string-from-pdo-prepared-statements, from
/** @param array<int|string, mixed> $params */
/** @param array<array-key, mixed> $params */
public function interpolateQuery(string $query, array $params): string
{
$s = chr(2); // Escape sequence for start of placeholder
Expand Down Expand Up @@ -491,7 +490,7 @@ private function bindParams(PDOStatement $stmt, array $params): void
}
}
} catch (PDOException $e) {
throw new DatabaseException("Failed to bind params " . json_encode($params), 0, $e);
throw new DatabaseException("Failed to bind params", $e, '', $params);
}
}

Expand Down Expand Up @@ -556,13 +555,13 @@ private function executeInternal(string $sql, array $params, callable $operation
$stmt = $this->pdo->prepare($sql);
return $stmt->execute($params);
} catch (PDOException $e) {
throw new DatabaseException("Failed to reconnect and execute query: $sql with params " . json_encode($params), 0, $e);
throw new DatabaseException('Failed to reconnect and execute query', $e, $sql, $params);
}
}
$msg = "Failed to execute query: " . $this->interpolateQuery($sql, $params);
logger()->error($msg, $context);

throw new DatabaseException($msg, 0, $e);
throw new DatabaseException($msg, $e, $sql, $params);
}
}

Expand Down
18 changes: 18 additions & 0 deletions gatherling/Exceptions/DatabaseException.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@

namespace Gatherling\Exceptions;

use Throwable;

use function Gatherling\Helpers\db;

class DatabaseException extends GatherlingException
{
/** @param array<array-key, mixed> $params */
public function __construct(
string $baseMessage,
?Throwable $previous = null,
string $sql = '',
array $params = [],
)
{
$msg = $baseMessage;
if ($sql) {
$msg .= ' (' . db()->interpolateQuery($sql, $params) . ')';
}
parent::__construct($msg, 0, $previous);
}
}
10 changes: 7 additions & 3 deletions gatherling/Exceptions/NotFoundInDatabaseException.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@

class NotFoundInDatabaseException extends DatabaseException
{
/** @param list<int|string> $ids */
/**
* @param list<int|string> $ids
* @param array<string, mixed> $params
*/
public function __construct(
string $message,
int $code,
?Throwable $previous,
string $sql,
array $params,
public string $type,
public array $ids
) {
parent::__construct($message, $code, $previous);
parent::__construct($message, $previous, $sql, $params);
}
}
2 changes: 1 addition & 1 deletion gatherling/Helpers/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Request
{
private static ?string $requestId = null;

/** @param array<int|string, mixed> $vars */
/** @param array<array-key, mixed> $vars */
public function __construct(private array $vars)
{
}
Expand Down
18 changes: 9 additions & 9 deletions gatherling/Logger.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public function __construct()
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function log(mixed $level, Stringable|string $message, array $context = []): void
{
Expand Down Expand Up @@ -79,63 +79,63 @@ private function normalizeLogLevel(mixed $level): Level
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function emergency(string|Stringable $message, array $context = []): void
{
$this->log(Level::Emergency, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function alert(string|Stringable $message, array $context = []): void
{
$this->log(Level::Alert, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function critical(string|Stringable $message, array $context = []): void
{
$this->log(Level::Critical, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function error(string|Stringable $message, array $context = []): void
{
$this->log(Level::Error, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function warning(string|Stringable $message, array $context = []): void
{
$this->log(Level::Warning, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function notice(string|Stringable $message, array $context = []): void
{
$this->log(Level::Notice, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function info(string|Stringable $message, array $context = []): void
{
$this->log(Level::Info, $message, $context);
}

/**
* @param array<int|string, mixed> $context
* @param array<array-key, mixed> $context
*/
public function debug(string|Stringable $message, array $context = []): void
{
Expand Down
2 changes: 1 addition & 1 deletion gatherling/Models/Entry.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static function findByEventAndPlayer(int $event_id, string $playername):
$sql = 'SELECT player FROM entries WHERE event_id = :event_id AND player = :player';
$params = ['event_id' => $event_id, 'player' => $playername];
$player = db()->optionalString($sql, $params);
if (!$player) {
if ($player === null) {
return null;
}
return new self($event_id, $playername);
Expand Down
Loading

0 comments on commit c54b5c6

Please sign in to comment.