From af97600709510e2f42dad71e01d26513fc1fdea6 Mon Sep 17 00:00:00 2001 From: Pauljd Date: Thu, 10 Sep 2026 16:19:30 +0100 Subject: [PATCH] further Davidson CI testing & AI refinement --- create-critter.php | 113 +++++++++++++++++++++++++++------------------ db.php | 5 ++ register.php | 7 ++- 3 files changed, 78 insertions(+), 47 deletions(-) diff --git a/create-critter.php b/create-critter.php index 59ae450..8953fff 100644 --- a/create-critter.php +++ b/create-critter.php @@ -102,14 +102,6 @@ function secondsSinceLastCritterByAuthor(PDO $pdo, int $authorId): ?int $dailyLimitReached = countCrittersCreatedTodayByAuthor($pdo, (int) $_SESSION['user_id']) >= DAILY_CRITTER_LIMIT; -$cooldownRemaining = 0; -if ($_SERVER['REQUEST_METHOD'] === 'POST') { - $secondsSinceLast = secondsSinceLastCritterByAuthor($pdo, (int) $_SESSION['user_id']); - if ($secondsSinceLast !== null && $secondsSinceLast < CRITTER_COOLDOWN_SECONDS) { - $cooldownRemaining = CRITTER_COOLDOWN_SECONDS - $secondsSinceLast; - } -} - function isAllowedReplicateUrl(string $url): bool { $parts = parse_url($url); @@ -192,33 +184,69 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && $userCredits > 0 && !$dailyLimitRea } } - if (empty($errors) && countCrittersCreatedTodayByAuthor($pdo, (int) $_SESSION['user_id']) >= DAILY_CRITTER_LIMIT) { - $dailyLimitReached = true; - } - - if (empty($errors) && $cooldownRemaining > 0) { - $errors['cooldown'] = "Please wait {$cooldownRemaining} more second(s) before creating another critter."; - } - $creditReserved = false; + $critterId = null; - if (empty($errors) && !$dailyLimitReached) { - // Atomically reserve one credit in the same statement that checks for it, so - // concurrent requests can't all read "1 credit" before any of them writes the - // deduction. SQLite serializes writes, so only as many of these UPDATEs can - // match "credits > 0" as there are credits actually available; everyone else - // gets 0 rows affected and is turned away before the expensive Replicate call. - $reserveStmt = $pdo->prepare('UPDATE users SET credits = credits - 1 WHERE id = :id AND credits > 0'); - $reserveStmt->execute(['id' => $_SESSION['user_id']]); - $creditReserved = $reserveStmt->rowCount() === 1; + if (empty($errors)) { + // Hold SQLite's single write lock across the daily-limit check, the + // cooldown check, the credit reservation, and the placeholder insert + // so concurrent requests can't all read the same stale count/cooldown + // before any of them commits. The row is inserted (image NULL) here, + // before the slow Replicate call, so it immediately counts toward the + // daily limit and cooldown for any other request racing in. + $pdo->exec('BEGIN IMMEDIATE'); + try { + if (countCrittersCreatedTodayByAuthor($pdo, (int) $_SESSION['user_id']) >= DAILY_CRITTER_LIMIT) { + $dailyLimitReached = true; + } - if (!$creditReserved) { - $userCredits = 0; - $errors['credits'] = 'You do not have enough credits to create a critter.'; + $cooldownRemaining = 0; + $secondsSinceLast = secondsSinceLastCritterByAuthor($pdo, (int) $_SESSION['user_id']); + if ($secondsSinceLast !== null && $secondsSinceLast < CRITTER_COOLDOWN_SECONDS) { + $cooldownRemaining = CRITTER_COOLDOWN_SECONDS - $secondsSinceLast; + } + + if (!$dailyLimitReached && $cooldownRemaining > 0) { + $errors['cooldown'] = "Please wait {$cooldownRemaining} more second(s) before creating another critter."; + } + + if (!$dailyLimitReached && empty($errors)) { + // Atomically reserve one credit in the same statement that checks for it, so + // concurrent requests can't all read "1 credit" before any of them writes the + // deduction. SQLite serializes writes, so only as many of these UPDATEs can + // match "credits > 0" as there are credits actually available; everyone else + // gets 0 rows affected and is turned away before the expensive Replicate call. + $reserveStmt = $pdo->prepare('UPDATE users SET credits = credits - 1 WHERE id = :id AND credits > 0'); + $reserveStmt->execute(['id' => $_SESSION['user_id']]); + $creditReserved = $reserveStmt->rowCount() === 1; + + if (!$creditReserved) { + $userCredits = 0; + $errors['credits'] = 'You do not have enough credits to create a critter.'; + } + } + + if ($creditReserved) { + $insertStmt = $pdo->prepare('INSERT INTO critters (noun_id, adjective_id, verb_id, location_id, author_id, image, datetime) VALUES (:noun_id, :adjective_id, :verb_id, :location_id, :author_id, NULL, :datetime)'); + $insertStmt->execute([ + 'noun_id' => $selected['noun_id'], + 'adjective_id' => $selected['adjective_id'], + 'verb_id' => $selected['verb_id'], + 'location_id' => $selected['location_id'], + 'author_id' => $_SESSION['user_id'], + 'datetime' => gmdate('Y-m-d H:i:s'), + ]); + $critterId = (int) $pdo->lastInsertId(); + } + + $pdo->exec('COMMIT'); + } catch (Throwable $e) { + $pdo->exec('ROLLBACK'); + throw $e; } } - if (empty($errors) && !$dailyLimitReached && $creditReserved) { + if ($creditReserved && $critterId !== null) { $adjectiveValue = getValueById($pdo, 'adjectives', $selected['adjective_id']); $nounValue = getValueById($pdo, 'nouns', $selected['noun_id']); $verbValue = getValueById($pdo, 'verbs', $selected['verb_id']); @@ -307,32 +335,27 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && $userCredits > 0 && !$dailyLimitRea mkdir($imagesDir, 0755, true); } - $imageFilename = bin2hex(random_bytes(16)) . '.webp'; - file_put_contents($imagesDir . '/' . $imageFilename, $imageData); + $candidateFilename = bin2hex(random_bytes(16)) . '.webp'; + $bytesWritten = file_put_contents($imagesDir . '/' . $candidateFilename, $imageData); + if ($bytesWritten !== false) { + $imageFilename = $candidateFilename; + } } } } if ($imageFilename === null) { // Every unsuccessful path above (request failure, failed/canceled prediction, - // disallowed or missing image URL, oversized or non-image download) leaves - // $imageFilename null, so this single check refunds the reserved credit for - // all of them instead of only the initial request-failure case. + // disallowed or missing image URL, oversized or non-image download, or a failed + // write to disk) leaves $imageFilename null, so this single check refunds the + // reserved credit for all of them instead of only the initial request-failure case. $refundStmt = $pdo->prepare('UPDATE users SET credits = credits + 1 WHERE id = :id'); $refundStmt->execute(['id' => $_SESSION['user_id']]); + } else { + $updateStmt = $pdo->prepare('UPDATE critters SET image = :image WHERE id = :id'); + $updateStmt->execute(['image' => $imageFilename, 'id' => $critterId]); } - $stmt = $pdo->prepare('INSERT INTO critters (noun_id, adjective_id, verb_id, location_id, author_id, image, datetime) VALUES (:noun_id, :adjective_id, :verb_id, :location_id, :author_id, :image, :datetime)'); - $stmt->execute([ - 'noun_id' => $selected['noun_id'], - 'adjective_id' => $selected['adjective_id'], - 'verb_id' => $selected['verb_id'], - 'location_id' => $selected['location_id'], - 'author_id' => $_SESSION['user_id'], - 'image' => $imageFilename, - 'datetime' => gmdate('Y-m-d H:i:s'), - ]); - header('Location: /'); exit; } diff --git a/db.php b/db.php index 7bfc9c9..6357995 100644 --- a/db.php +++ b/db.php @@ -11,6 +11,11 @@ function getDbConnection(): PDO $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); $pdo->exec('PRAGMA foreign_keys = ON'); $pdo->exec('PRAGMA busy_timeout = 5000'); + // WAL lets readers proceed without blocking a writer's commit (and vice versa), + // which the default rollback-journal mode doesn't: two concurrent requests each + // holding open, unfetched-to-completion statement handles could otherwise make a + // writer's COMMIT fail with "database is locked" even within the busy_timeout. + $pdo->exec('PRAGMA journal_mode = WAL'); $pdo->exec('CREATE TABLE IF NOT EXISTS users ( id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL UNIQUE, diff --git a/register.php b/register.php index 74e757e..477c941 100644 --- a/register.php +++ b/register.php @@ -97,6 +97,11 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { 'email' => $email, 'password' => $hashedPassword, ]); + // Capture the new user's id before any other insert runs on this connection — + // SQLite's lastInsertId() returns the most recent rowid inserted on the + // connection regardless of table, so recordRegistrationAttempt()'s own insert + // would otherwise clobber it and log the session in as the wrong user. + $newUserId = (int) $pdo->lastInsertId(); recordRegistrationAttempt($pdo, $ipIdentifier); } catch (PDOException $e) { if (str_contains($e->getMessage(), 'UNIQUE constraint failed: users.username')) { @@ -109,8 +114,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { } if (empty($errors)) { - $newUserId = (int) $pdo->lastInsertId(); - start_app_session(); session_regenerate_id(true); $_SESSION['user_id'] = $newUserId;