Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor database exceptions, fix password change buglet #872

Merged
merged 4 commits into from
Dec 4, 2024
Merged

Refactor database exceptions, fix password change buglet #872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18074d3
Make DatabaseException more structured, taking sql and params as argu…
bakert Dec 3, 2024
369325c
Complete conversion of updateCard to new db style
bakert Dec 4, 2024
a748cd7
Fix typo in password change code
bakert Dec 4, 2024
aeaa88b
Update psalm baseline for recent changes
bakert Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 28 additions & 29 deletions gatherling/Data/Db.php
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading