diff --git a/CHANGELOG.md b/CHANGELOG.md index ca1bf53a1..941a10514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,65 @@ Full version-by-version history for Pinakes. The README shows only the latest release; everything older lives here. +## [0.7.61-rc.3] + +Release candidate for 0.7.61 — see the [0.7.61] notes below. Cut for +verification on the reference install before the stable release. + +## [0.7.61] + +Physical-copy management from the book summary, with the whole holding and +circulation lifecycle made atomic and derived from the copies. + +### Features + +- The book page (`/admin/books/{id}`) now shows a **Copie Fisiche** section for + every book — even one with no copies — with an "Aggiungi copia" modal, per-copy + status editing, and per-copy delete. Copy status covers the physical states + (available, maintenance, under restoration, in transfer, lost, damaged); an + out-of-circulation copy lowers the derived total on its own. +- A book can be created with zero physical copies and have them added later from + the summary. On the edit form the copy count is read-only and delegates to the + per-copy management, so availability is always derived from the copies. + +### Fixes + +- Book creation is now atomic: the book row and its initial copies are committed + together, so a copy-creation failure can no longer leave an orphan book with no + holdings. The bulk `increase-copies` endpoint is likewise transactional, + allocates collision-free inventory codes, promotes the wait-list, and validates + its input. +- Adding an available copy repairs blocked reservations and promotes the next + wait-list entry into a physical-copy-linked loan, mirroring the loan engine. +- Copies under restoration or in transfer can now be deleted from the UI; the + loan/reservation system keeps exclusive ownership of the `prestato`/`prenotato` + states. +- Legacy reservations with a missing or past start date are no longer promoted + into back-dated loans. +- Admin copy routes stay fixed English literals (not routed through the i18n + system), inventory-code allocation escapes LIKE metacharacters, and the copy + note is sanitised and length-capped like the inventory number. +- All new copy-management strings are translated across the five locales. + +### Upgrade notes + +- **Legacy availability is migrated automatically.** Books that predate copy + tracking (only the old counters, no per-copy rows) are backfilled into real + copies *before* availability is recalculated, so availability carries over + from the old counter model to the new copy-derived one without being zeroed. + Active loans and reservations are preserved, and copies already marked + lost/damaged/maintenance/under-restoration/in-transfer are left untouched. +- A book whose only record of unavailability was the legacy counter (marked + unavailable with no active loan) becomes available again after the upgrade: + the new model derives availability from physical copies, and a physically + missing book with no loan leaves no machine-readable trace to preserve. + Re-mark those copies from the book page after upgrading. +- If you upgraded through an intermediate version that had already zeroed a + legacy book's counters before this release, the backfill cannot reconstruct + the lost count. Restore `libri.copie_totali` from the automatic pre-upgrade + backup under `storage/backups/`, then re-run the availability recalculation + from Maintenance, or re-add the copies from the book page. + ## [0.7.60] Maintenance release: UPC barcode support, a read-only availability field, and a diff --git a/README.md b/README.md index 0ba0d152b..6314b763d 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,25 @@ Pinakes is a self-hosted, full-featured ILS for schools, municipalities, and pri Highlights of the latest release are below. The full version-by-version history (v0.7.59 → v0.6.x) lives in **[CHANGELOG.md](CHANGELOG.md)**. -### v0.7.60 — latest +### v0.7.61 — latest + +Physical-copy management from the book page, with availability derived from the copies — plus a per-user interface language and a safe upgrade path for existing catalogues. + +### New + +- **Manage physical copies from the book page** — every book now has a *Copie Fisiche* section (even one with no copies) to add copies, edit each copy's status (available, maintenance, under restoration, in transfer, lost, damaged), and delete copies. Availability is derived from the copies; an out-of-circulation copy lowers the total on its own. +- **Per-user interface language** — each account keeps its own language, set from the profile/language switcher or by an admin from the *Edit user* form, instead of everyone being forced to Italian regardless of the install language ([#238](https://github.com/fabiodalez-dev/Pinakes/discussions/238)). + +### Fixes + +- **Book creation and copy changes are atomic** — a book and its initial copies are committed together, and the bulk *increase copies* endpoint allocates collision-free inventory codes, promotes the wait-list, and validates its input. +- **New users inherit the installation language** instead of the old hard-coded Italian default; an admin can set any user's language from the *Edit user* form (#238). + +### Upgrade Notes + +- **Legacy availability is migrated automatically** (`migrate_0.7.61-rc.1.sql`). Books that predate copy tracking are backfilled into real copies *before* availability is recalculated, so availability carries over from the old counters to the new copy-derived model without being zeroed. Active loans and reservations are preserved, and out-of-circulation copies are left untouched. See **[CHANGELOG.md](CHANGELOG.md)** for the full notes, including how to recover a catalogue already zeroed by an intermediate version. + +### v0.7.60 A maintenance release: UPC barcode support, a read-only availability field, and a PHP 8.5 scraping fix. diff --git a/app/Controllers/Admin/LanguagesController.php b/app/Controllers/Admin/LanguagesController.php index f0e3be443..abf213563 100644 --- a/app/Controllers/Admin/LanguagesController.php +++ b/app/Controllers/Admin/LanguagesController.php @@ -703,20 +703,17 @@ private function synchronizeGlobalLocale(\mysqli $db, string $code): void $this->updateEnvLocale($normalized); - I18n::setLocale($normalized); - $_SESSION['locale'] = $normalized; - - // Propagate the new default to every user account so that - // AuthController and RememberMeMiddleware pick it up on the - // next login/token refresh. - try { - $stmt = $db->prepare("UPDATE utenti SET locale = ?"); - $stmt->bind_param('s', $normalized); - $stmt->execute(); - $stmt->close(); - } catch (\Throwable $e) { - SecureLogger::error('LanguagesController: Unable to propagate locale to users: ' . $e->getMessage()); - } + // The installation default governs NEW accounts (created with the + // current default) and anonymous rendering only. Deliberately it does + // NOT: + // - touch existing `utenti.locale`: there is no inherited-vs-explicit + // flag, so any propagation (even scoped by the previous default) + // would silently overwrite a user who deliberately chose that + // language. Existing accounts keep their own preference and change + // it via the switcher/profile (#238, Option B); + // - change the current admin's session locale: the admin keeps the + // language stored on their own account (forcing it here would + // diverge from `utenti.locale` and silently revert on next login). } private function updateEnvLocale(string $locale): void diff --git a/app/Controllers/AuthController.php b/app/Controllers/AuthController.php index 8c4377781..033bdb779 100644 --- a/app/Controllers/AuthController.php +++ b/app/Controllers/AuthController.php @@ -146,13 +146,13 @@ public function login(Request $request, Response $response, mysqli $db): Respons 'name' => trim(\App\Support\HtmlHelper::decode((string) ($row['nome'] ?? '')) . ' ' . \App\Support\HtmlHelper::decode((string) ($row['cognome'] ?? ''))), ]; - // Load and apply user's preferred locale (only persist if setLocale succeeds) - if (!empty($row['locale'])) { - $requestedLocale = (string) $row['locale']; - if (\App\Support\I18n::setLocale($requestedLocale)) { - $_SESSION['locale'] = $requestedLocale; - } - } + // Apply the per-user preference on every login. NULL, empty or + // obsolete values inherit the current installation default, + // preventing a previous anonymous/user session locale leaking in. + $requestedLocale = \App\Support\I18n::resolveUserLocale($row['locale'] ?? null); + \App\Support\I18n::setLocale($requestedLocale); + $_SESSION['locale'] = $requestedLocale; + $_SESSION['user']['locale'] = $requestedLocale; // Handle "Remember Me" functionality with database-backed tokens if ($remember) { diff --git a/app/Controllers/CollaneController.php b/app/Controllers/CollaneController.php index e098168b3..71bd1d51e 100644 --- a/app/Controllers/CollaneController.php +++ b/app/Controllers/CollaneController.php @@ -574,69 +574,77 @@ public function createParentWork(Request $request, Response $response, mysqli $d } } - // Create the parent book - $stmt = $db->prepare("INSERT INTO libri (titolo, collana, copie_totali, copie_disponibili, created_at, updated_at) VALUES (?, ?, 0, 0, NOW(), NOW())"); - if (!$stmt) { - $_SESSION['error_message'] = __('Errore database'); - return $response->withHeader('Location', url('/admin/series'))->withStatus(302); - } - $stmt->bind_param('ss', $parentTitle, $collana); - $stmt->execute(); - $parentId = (int) $db->insert_id; - $stmt->close(); - - // SEC1-1 (review): bail BEFORE calling assignPrimarySeries on a - // failed insert (parentId == 0). Pre-fix the check was after the - // mutation and the early-return was dead code. - if ($parentId <= 0) { - $_SESSION['error_message'] = __('Errore nella creazione dell\'opera'); - return $response->withHeader('Location', url('/admin/series'))->withStatus(302); - } + // The zero-copy parent row, its series membership, volume links and + // canonical availability projection are one logical write. + $parentId = 0; + $linkedCount = 0; + $db->begin_transaction(); + try { + $stmt = $db->prepare("INSERT INTO libri (titolo, collana, copie_totali, copie_disponibili, created_at, updated_at) VALUES (?, ?, 0, 0, NOW(), NOW())"); + if (!$stmt) { + throw new \RuntimeException('Unable to prepare parent-work insert'); + } + $stmt->bind_param('ss', $parentTitle, $collana); + $stmt->execute(); + $parentId = (int) $db->insert_id; + $stmt->close(); - $seriesRepo = new SeriesRepository($db); - $seriesRepo->assignPrimarySeries($parentId, $collana); + if ($parentId <= 0) { + throw new \RuntimeException('Unable to create parent work'); + } - // Link all books in the collana as volumes - $linkedCount = 0; - $rows = array_values(array_filter( - $seriesRepo->getBooksForSeries($collana), - static fn(array $row): bool => (int) ($row['id'] ?? 0) !== $parentId - )); - if ($rows !== []) { - // Build set of used numero_serie values - $usedNumbers = []; - foreach ($rows as $row) { - if (!empty($row['numero_serie'])) { - $usedNumbers[(int) $row['numero_serie']] = true; + $seriesRepo = new SeriesRepository($db); + $seriesRepo->assignPrimarySeries($parentId, $collana); + + // Link all books in the collana as volumes. + $rows = array_values(array_filter( + $seriesRepo->getBooksForSeries($collana), + static fn(array $row): bool => (int) ($row['id'] ?? 0) !== $parentId + )); + if ($rows !== []) { + $usedNumbers = []; + foreach ($rows as $row) { + if (!empty($row['numero_serie'])) { + $usedNumbers[(int) $row['numero_serie']] = true; + } } - } - $stmtInsert = $db->prepare("INSERT IGNORE INTO volumi (opera_id, volume_id, numero_volume) VALUES (?, ?, ?)"); - $nextFree = 1; - foreach ($rows as $row) { - $bookId = (int) $row['id']; - if (!empty($row['numero_serie'])) { - $num = (int) $row['numero_serie']; - } else { - // Find next free number not already used - while (isset($usedNumbers[$nextFree])) { + $stmtInsert = $db->prepare("INSERT IGNORE INTO volumi (opera_id, volume_id, numero_volume) VALUES (?, ?, ?)"); + if (!$stmtInsert) { + throw new \RuntimeException('Unable to prepare parent-work volume links'); + } + $nextFree = 1; + foreach ($rows as $row) { + $bookId = (int) $row['id']; + if (!empty($row['numero_serie'])) { + $num = (int) $row['numero_serie']; + } else { + while (isset($usedNumbers[$nextFree])) { + $nextFree++; + } + $num = $nextFree; + $usedNumbers[$nextFree] = true; $nextFree++; } - $num = $nextFree; - $usedNumbers[$nextFree] = true; - $nextFree++; - } - if ($stmtInsert) { $stmtInsert->bind_param('iii', $parentId, $bookId, $num); $stmtInsert->execute(); if ($stmtInsert->affected_rows > 0) { $linkedCount++; } } - } - if ($stmtInsert) { $stmtInsert->close(); } + + if (!(new \App\Support\DataIntegrity($db))->recalculateBookAvailability($parentId, insideTransaction: true)) { + throw new \RuntimeException('Unable to derive parent-work availability'); + } + + $db->commit(); + } catch (\Throwable $e) { + $db->rollback(); + \App\Support\SecureLogger::error('CollaneController::createParentWork failed', ['error' => $e->getMessage()]); + $_SESSION['error_message'] = __('Errore nella creazione dell\'opera'); + return $response->withHeader('Location', url('/admin/series'))->withStatus(302); } // Build the new parent book's denormalized search_index — otherwise it diff --git a/app/Controllers/CopyController.php b/app/Controllers/CopyController.php index 90bccb658..42309de2e 100644 --- a/app/Controllers/CopyController.php +++ b/app/Controllers/CopyController.php @@ -5,6 +5,10 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use App\Controllers\ReservationManager; +use App\Models\CopyRepository; +use App\Services\ReservationReassignmentService; +use App\Support\DataIntegrity; use App\Support\SecureLogger; use mysqli; @@ -13,12 +17,23 @@ class CopyController /** * SECURITY: Validate and sanitize HTTP_REFERER to prevent open redirect */ - private function safeReferer(string $default = '/admin/books'): string + private function safeReferer(?string $default = null): string { // Delegate to the single audited implementation. localPath() uses only // the referer's path (never its scheme/host), which is strictly safer // than the previous same-host comparison and port-agnostic. - return \App\Support\RefererGuard::localPath((string) ($_SERVER['HTTP_REFERER'] ?? ''), $default); + return \App\Support\RefererGuard::localPath( + (string) ($_SERVER['HTTP_REFERER'] ?? ''), + // Admin routes are fixed English literals — never routed through the + // i18n system (CLAUDE.md rule #4 / decision #145). + $default ?? '/admin/books' + ); + } + + private function adminBookPath(int $bookId): string + { + // Fixed admin literal, not an i18n route (CLAUDE.md rule #4). + return '/admin/books/' . $bookId; } /** @@ -56,7 +71,8 @@ private function isCopyHeld(\mysqli $db, int $copyId): bool public function byCode(Request $request, Response $response, mysqli $db): Response { $params = $request->getQueryParams(); - $code = trim((string) ($params['code'] ?? '')); + $rawCode = $params['code'] ?? ''; + $code = is_string($rawCode) ? trim($rawCode) : ''; if ($code === '') { $response->getBody()->write((string) json_encode(['found' => false])); @@ -107,14 +123,24 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int $data = (array) $request->getParsedBody(); // CSRF validated by CsrfMiddleware - $stato = $data['stato'] ?? 'disponibile'; - $note = $data['note'] ?? ''; + $statoInput = $data['stato'] ?? 'disponibile'; + $noteInput = $data['note'] ?? ''; + if (!is_string($statoInput) || !is_string($noteInput)) { + $_SESSION['error_message'] = __('Impossibile aggiornare la copia senza lasciare dati incoerenti. Nessuna modifica è stata salvata.'); + return $response->withHeader('Location', $this->safeReferer())->withStatus(302); + } + $stato = $statoInput; + $note = $this->sanitizeNote($noteInput); // Validazione stato (deve corrispondere all'enum in copie.stato) + // 'prestato'/'prenotato' are owned by the loan/reservation system. They + // remain valid only so an existing loan-owned state can be preserved + // while staff update the copy note; transitions into them are rejected + // explicitly after loading the current row. $statiValidi = ['disponibile', 'prestato', 'prenotato', 'manutenzione', 'in_restauro', 'perso', 'danneggiato', 'in_trasferimento']; - if (!in_array($stato, $statiValidi)) { + if (!in_array($stato, $statiValidi, true)) { $_SESSION['error_message'] = __('Stato non valido.'); - return $response->withHeader('Location', $this->safeReferer('/admin/books'))->withStatus(302); + return $response->withHeader('Location', $this->safeReferer())->withStatus(302); } // Recupera la copia per ottenere il libro_id @@ -127,12 +153,21 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int if (!$copy) { $_SESSION['error_message'] = __('Copia non trovata.'); - return $response->withHeader('Location', $this->safeReferer('/admin/books'))->withStatus(302); + return $response->withHeader('Location', $this->safeReferer())->withStatus(302); } $libroId = (int) $copy['libro_id']; $statoCorrente = $copy['stato']; + if ($stato === 'prenotato' && $statoCorrente !== 'prenotato') { + $_SESSION['error_message'] = __('Per prenotare una copia, utilizza il sistema Prestiti. Non è possibile impostare manualmente lo stato "Prenotato".'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } + if ($statoCorrente === 'prenotato' && $stato === 'disponibile') { + $_SESSION['error_message'] = __('Per annullare o spostare una prenotazione, utilizza il sistema Prestiti.'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } + // Prestito "in carico" su questa copia (in_corso/in_ritardo): usato per la // chiusura automatica quando la copia torna 'disponibile'. $stmt = $db->prepare(" @@ -155,7 +190,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int // Non permettere cambio diretto a "prestato", deve usare il sistema prestiti if ($stato === 'prestato' && $statoCorrente !== 'prestato') { $_SESSION['error_message'] = __('Per prestare una copia, utilizza il sistema Prestiti dalla sezione dedicata. Non è possibile impostare manualmente lo stato "Prestato".'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } // GESTIONE CAMBIO STATO DA "PRESTATO" A "DISPONIBILE" @@ -173,7 +208,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int $delegated = $request->withParsedBody([ 'stato' => 'restituito', 'note' => $returnNote, - 'redirect_to' => "/admin/books/{$libroId}", + 'redirect_to' => $this->adminBookPath($libroId), 'csrf_token' => $data['csrf_token'] ?? '', ]); return (new PrestitiController())->processReturn($delegated, $response, $db, (int) $prestito['id']); @@ -184,7 +219,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int // copy still in the library may instead be reassigned atomically below. if ($copyHeld && $prestito) { $_SESSION['error_message'] = __('La copia è fisicamente in prestito: registra prima la restituzione o l’esito perso/danneggiato dal sistema Prestiti.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } // L'aggiornamento avviene sotto lock del libro (ordine di lock canonico, @@ -203,7 +238,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int if (!$bookLocked) { $db->rollback(); $_SESSION['error_message'] = __('Libro non trovato o non più disponibile.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } // Recheck only physical possession after the book lock. Scheduled @@ -216,7 +251,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int if ($physicallyOut) { $db->rollback(); $_SESSION['error_message'] = __('La copia è fisicamente in prestito: usa il flusso di restituzione.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } $stmt = $db->prepare("UPDATE copie SET stato = ?, note = ?, updated_at = NOW() WHERE id = ?"); @@ -271,7 +306,7 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int 'error' => $e->getMessage() ]); $_SESSION['error_message'] = __('Impossibile aggiornare la copia senza lasciare dati incoerenti. Nessuna modifica è stata salvata.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } try { @@ -287,84 +322,295 @@ public function updateCopy(Request $request, Response $response, mysqli $db, int if (!isset($_SESSION['success_message'])) { $_SESSION['success_message'] = __('Stato della copia aggiornato con successo.'); } - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } /** - * Elimina una singola copia + * Crea una o più copie fisiche per un libro, direttamente dalla scheda. + * + * A copy is never *created* in a loan state here — 'prestato'/'prenotato' + * belong to the Prestiti system — but creating an available copy may promote a + * waiting reservation, which sets that new copy to 'prenotato' before commit. + * A copy created out of circulation ('perso'/'danneggiato'/'manutenzione'/ + * 'in_restauro'/'in_trasferimento') is excluded from copie_totali by the + * availability recalculation, so marking a copy lost reduces the book's total. */ - public function deleteCopy(Request $request, Response $response, mysqli $db, int $copyId): Response + public function createCopy(Request $request, Response $response, mysqli $db, int $bookId): Response { + $data = (array) $request->getParsedBody(); // CSRF validated by CsrfMiddleware - // Recupera la copia per ottenere il libro_id e verificare lo stato - $stmt = $db->prepare("SELECT libro_id, stato FROM copie WHERE id = ?"); - $stmt->bind_param('i', $copyId); - $stmt->execute(); - $result = $stmt->get_result(); - $copy = $result->fetch_assoc(); - $stmt->close(); - - if (!$copy) { - $_SESSION['error_message'] = __('Copia non trovata.'); - return $response->withHeader('Location', $this->safeReferer('/admin/books'))->withStatus(302); + // Only physical statuses a copy can be created in — loan states are + // managed by the Prestiti system, never set here. + $statoInput = $data['stato'] ?? 'disponibile'; + $noteInput = $data['note'] ?? ''; + $numeroInput = $data['numero_inventario'] ?? ''; + $quantitaInput = $data['quantita'] ?? '1'; + if (!is_string($statoInput) || !is_string($noteInput) || !is_string($numeroInput)) { + $_SESSION['error_message'] = __('Impossibile aggiungere la copia.'); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); + } + $quantita = is_int($quantitaInput) + ? $quantitaInput + : (is_string($quantitaInput) && preg_match('/^[1-9]\d*$/D', $quantitaInput) === 1 + ? (int) $quantitaInput + : 0); + if ($quantita < 1 || $quantita > 100) { + $_SESSION['error_message'] = __('Numero di copie non valido.'); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); + } + $stato = $statoInput; + $statiValidi = ['disponibile', 'manutenzione', 'in_restauro', 'perso', 'danneggiato', 'in_trasferimento']; + if (!in_array($stato, $statiValidi, true)) { + $_SESSION['error_message'] = __('Stato non valido.'); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); + } + $note = $this->sanitizeNote($noteInput); + + // Inventory code: honour an explicit value (must be unique), otherwise + // auto-allocate the next collision-free "{base}-C{N}" like book creation. + $numero = trim($numeroInput); + if ($numero !== '') { + $numero = trim((string) preg_replace('/[\x00-\x1F]/', '', $numero)); + if (mb_strlen($numero) > 100) { + $numero = mb_substr($numero, 0, 100); + } + } + if ($quantita > 1 && $numero !== '') { + $_SESSION['error_message'] = __('Per aggiungere più copie, lascia vuoto il numero di inventario: verrà assegnato automaticamente a ogni copia.'); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); } - $libroId = (int) $copy['libro_id']; - $stato = $copy['stato']; + $repo = new CopyRepository($db); + $reassignmentService = null; + $reservationManager = null; + $transactionStarted = false; + + try { + // Keep the same canonical lock order used by circulation writes: + // book first, then copies/loans. Copy creation, queue processing and + // derived counters must become visible as one atomic change. + if (!$db->begin_transaction()) { + throw new \RuntimeException('Unable to begin the physical-copy creation transaction.'); + } + $transactionStarted = true; + + $stmt = $db->prepare("SELECT id, numero_inventario FROM libri WHERE id = ? AND deleted_at IS NULL FOR UPDATE"); + $stmt->bind_param('i', $bookId); + $stmt->execute(); + $book = $stmt->get_result()->fetch_assoc(); + $stmt->close(); + if (!$book) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Libro non trovato.'); + return $response->withHeader('Location', $this->safeReferer())->withStatus(302); + } + + // Re-evaluate after sanitisation: an explicit value made only of + // control characters must fall back to automatic allocation. + if ($numero === '') { + $base = !empty($book['numero_inventario']) ? (string) $book['numero_inventario'] : "LIB-{$bookId}"; + $newCopyIds = $repo->createManyForBookWithIdsAndNote( + $bookId, + $base, + $quantita, + $stato, + $note !== '' ? $note : null + ); + } elseif ($repo->inventoryCodeExists($numero)) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Esiste già una copia con questo numero di inventario.'); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); + } else { + $newCopyId = $repo->create($bookId, $numero, $stato, $note !== '' ? $note : null); + $newCopyIds = $newCopyId > 0 ? [$newCopyId] : []; + } + + if (count($newCopyIds) !== $quantita) { + throw new \RuntimeException('Unable to create every requested physical copy.'); + } + + // A newly available copy is new circulation capacity. Mirror the + // existing book-edit path: first repair blocked copy assignments, + // then promote the next eligible wait-list entry. + if ($stato === 'disponibile') { + $reassignmentService = new ReservationReassignmentService($db); + $reassignmentService->setExternalTransaction(true); + foreach ($newCopyIds as $newCopyId) { + $reassignmentService->reassignOnNewCopy($bookId, $newCopyId); + } - // Verifica se la copia è trattenuta da QUALSIASI impegno HOLDING (prestito - // attivo o pendente-con-copia, incluse prenotazioni future e ritiri in attesa). - $hasPrestito = $this->isCopyHeld($db, $copyId); + $reservationManager = new ReservationManager($db); + $reservationManager->setExternalTransaction(true); + for ($guard = 0; $guard < 1000 && $reservationManager->processBookAvailability($bookId); $guard++) { + // Promote every date-eligible reservation allowed by the new capacity. + } + } + + $integrity = new DataIntegrity($db); + if (!$integrity->recalculateBookAvailability($bookId, insideTransaction: true)) { + throw new \RuntimeException('Unable to recalculate book availability.'); + } + + if (!$db->commit()) { + throw new \RuntimeException('Unable to commit physical-copy creation.'); + } + $transactionStarted = false; + } catch (\Throwable $e) { + if ($transactionStarted) { + $db->rollback(); + } + SecureLogger::error('[CopyController] createCopy failed', ['book' => $bookId, 'error' => $e->getMessage()]); + $_SESSION['error_message'] = (int) $e->getCode() === 1062 + ? __('Esiste già una copia con questo numero di inventario.') + : ($quantita === 1 ? __('Impossibile aggiungere la copia.') : __('Impossibile aggiungere le copie.')); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); + } - if ($hasPrestito) { - $_SESSION['error_message'] = __('Impossibile eliminare una copia attualmente impegnata in un prestito o una prenotazione.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + // Notifications are deliberately emitted only after the transaction that + // made the new assignment/promotion durable. + try { + $reassignmentService?->flushDeferredNotifications(); + $reservationManager?->flushDeferredNotifications(); + } catch (\Throwable $e) { + SecureLogger::warning(__('Invio notifica nuova copia fallito'), ['error' => $e->getMessage()]); } - // Permetti eliminazione solo per copie perse, danneggiate o in manutenzione - if (!in_array($stato, ['perso', 'danneggiato', 'manutenzione'])) { - $_SESSION['error_message'] = __('Puoi eliminare solo copie perse, danneggiate o in manutenzione. Prima modifica lo stato della copia.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + $_SESSION['success_message'] = $quantita === 1 + ? __('Copia aggiunta con successo.') + : sprintf(__('%d copie aggiunte con successo.'), $quantita); + return $response->withHeader('Location', url($this->adminBookPath($bookId)))->withStatus(302); + } + + /** + * Normalize an administrator-entered copy note consistently in create/edit. + */ + private function sanitizeNote(string $note): string + { + $note = trim($note); + if ($note === '') { + return ''; } - // Anche i prestiti CHIUSI referenziano copia_id e il FK fk_prestiti_copia - // è ON DELETE RESTRICT: senza questo check la DELETE esplode con - // mysqli_sql_exception (500). Una copia con storico non si elimina, si - // mette fuori circolazione cambiandone lo stato. - $stmt = $db->prepare("SELECT 1 FROM prestiti WHERE copia_id = ? LIMIT 1"); + // Keep tab/newline for multi-line notes, drop other control characters. + $note = (string) preg_replace('/[\x00-\x08\x0B\x0C\x0E-\x1F]/', '', $note); + return mb_strlen($note) > 500 ? mb_substr($note, 0, 500) : $note; + } + + /** + * Elimina una singola copia + */ + public function deleteCopy(Request $request, Response $response, mysqli $db, int $copyId): Response + { + // CSRF validated by CsrfMiddleware + + // Resolve the parent first; the transaction below then follows the + // canonical circulation lock order (book -> copy -> loans). + $stmt = $db->prepare('SELECT libro_id FROM copie WHERE id = ?'); $stmt->bind_param('i', $copyId); $stmt->execute(); - $hasHistory = (bool) $stmt->get_result()->fetch_row(); + $copy = $stmt->get_result()->fetch_assoc(); $stmt->close(); - - if ($hasHistory) { - $_SESSION['error_message'] = __('Impossibile eliminare la copia: ha uno storico prestiti. Puoi metterla fuori circolazione cambiandone lo stato.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + if (!$copy) { + $_SESSION['error_message'] = __('Copia non trovata.'); + return $response->withHeader('Location', $this->safeReferer())->withStatus(302); } - // Elimina la copia. Difesa in profondità: un prestito creato tra il check - // e la DELETE fa comunque scattare il FK — intercetta e degrada a errore - // gestito invece di propagare un 500. + $libroId = (int) $copy['libro_id']; + $transactionStarted = false; try { - $stmt = $db->prepare("DELETE FROM copie WHERE id = ?"); + if (!$db->begin_transaction()) { + throw new \RuntimeException('Unable to begin copy-delete transaction.'); + } + $transactionStarted = true; + + $lockBook = $db->prepare('SELECT id FROM libri WHERE id = ? AND deleted_at IS NULL FOR UPDATE'); + $lockBook->bind_param('i', $libroId); + $lockBook->execute(); + $bookExists = (bool) $lockBook->get_result()->fetch_row(); + $lockBook->close(); + if (!$bookExists) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Libro non trovato o non più disponibile.'); + return $response->withHeader('Location', $this->safeReferer())->withStatus(302); + } + + // Re-read under lock: state and commitments may have changed since + // the initial parent lookup. + $stmt = $db->prepare('SELECT stato FROM copie WHERE id = ? AND libro_id = ? FOR UPDATE'); + $stmt->bind_param('ii', $copyId, $libroId); + $stmt->execute(); + $lockedCopy = $stmt->get_result()->fetch_assoc(); + $stmt->close(); + if (!$lockedCopy) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Copia non trovata.'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } + + // A copy with any current/future commitment or historical loan is + // retained permanently; operators can only move it out of circulation. + if ($this->isCopyHeld($db, $copyId)) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Impossibile eliminare una copia attualmente impegnata in un prestito o una prenotazione.'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } + if (!in_array($lockedCopy['stato'], ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento'], true)) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Puoi eliminare solo copie fuori circolazione (perse, danneggiate, in manutenzione, in restauro o in trasferimento). Prima modifica lo stato della copia.'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } + + $stmt = $db->prepare('SELECT 1 FROM prestiti WHERE copia_id = ? LIMIT 1 FOR UPDATE'); + $stmt->bind_param('i', $copyId); + $stmt->execute(); + $hasHistory = (bool) $stmt->get_result()->fetch_row(); + $stmt->close(); + if ($hasHistory) { + $db->rollback(); + $transactionStarted = false; + $_SESSION['error_message'] = __('Impossibile eliminare la copia: ha uno storico prestiti. Puoi metterla fuori circolazione cambiandone lo stato.'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } + + $stmt = $db->prepare('DELETE FROM copie WHERE id = ?'); $stmt->bind_param('i', $copyId); $stmt->execute(); + $deleted = $stmt->affected_rows; $stmt->close(); - } catch (\mysqli_sql_exception $e) { - // 1451 = Cannot delete or update a parent row (vincolo FK) - if ((int) $e->getCode() !== 1451) { - throw $e; + if ($deleted !== 1) { + throw new \RuntimeException('Physical copy delete did not affect exactly one row.'); } - $_SESSION['error_message'] = __('Impossibile eliminare la copia: ha uno storico prestiti. Puoi metterla fuori circolazione cambiandone lo stato.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); - } - // Ricalcola disponibilità del libro - $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($libroId); + if (!(new DataIntegrity($db))->recalculateBookAvailability($libroId, insideTransaction: true)) { + throw new \RuntimeException('Unable to recalculate availability after copy delete.'); + } + if (!$db->commit()) { + throw new \RuntimeException('Unable to commit copy-delete transaction.'); + } + $transactionStarted = false; + } catch (\Throwable $e) { + if ($transactionStarted) { + try { + $db->rollback(); + } catch (\Throwable $rollbackError) { + // best-effort + } + } + SecureLogger::error('[CopyController] deleteCopy failed', ['copy' => $copyId, 'error' => $e->getMessage()]); + $_SESSION['error_message'] = (int) $e->getCode() === 1451 + ? __('Impossibile eliminare la copia: ha uno storico prestiti. Puoi metterla fuori circolazione cambiandone lo stato.') + : __('Impossibile eliminare la copia.'); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); + } $_SESSION['success_message'] = __('Copia eliminata con successo.'); - return $response->withHeader('Location', url("/admin/books/{$libroId}"))->withStatus(302); + return $response->withHeader('Location', url($this->adminBookPath($libroId)))->withStatus(302); } } diff --git a/app/Controllers/LanguageController.php b/app/Controllers/LanguageController.php index 79f142298..8cc80178e 100644 --- a/app/Controllers/LanguageController.php +++ b/app/Controllers/LanguageController.php @@ -40,6 +40,7 @@ public function switchLanguage(Request $request, Response $response, mysqli $db, $stmt->bind_param('si', $locale, $userId); $stmt->execute(); $stmt->close(); + $_SESSION['user']['locale'] = $locale; } } diff --git a/app/Controllers/LibriApiController.php b/app/Controllers/LibriApiController.php index 025b90b46..7e2e5a1b9 100644 --- a/app/Controllers/LibriApiController.php +++ b/app/Controllers/LibriApiController.php @@ -488,105 +488,22 @@ public function byGenre(Request $request, Response $response, mysqli $db): Respo } /** - * Bulk update status for multiple books + * Compatibility endpoint retained for older admin clients. + * + * Book status is derived from physical copies, loans and reservations. It + * must never be accepted as a user-authored field: fabricating "prestato" or + * "prenotato" would create no matching commitment, while marking a title + * "perso"/"danneggiato" would leave its actual copies lendable. */ public function bulkStatus(Request $request, Response $response, mysqli $db): Response { - $body = $request->getParsedBody(); - if (!$body) { - $body = json_decode((string) $request->getBody(), true); - } - // CSRF validated by CsrfMiddleware - - $ids = $body['ids'] ?? []; - $stato = trim((string) ($body['stato'] ?? '')); - - // Validate input - if (empty($ids) || !is_array($ids)) { - $response->getBody()->write(json_encode([ - 'success' => false, - 'error' => __('Nessun libro selezionato') - ], JSON_UNESCAPED_UNICODE)); - return $response->withStatus(400)->withHeader('Content-Type', 'application/json'); - } - - // Map frontend labels to database ENUM values - // Database ENUM: 'disponibile', 'prestato', 'prenotato', 'perso', 'danneggiato' - $stateMap = [ - 'disponibile' => 'disponibile', - 'prestato' => 'prestato', - 'in_prestito' => 'prestato', - 'prenotato' => 'prenotato', - 'riservato' => 'prenotato', - 'perso' => 'perso', - 'smarrito' => 'perso', - 'danneggiato' => 'danneggiato', - 'in_manutenzione' => 'danneggiato', - 'non disponibile' => 'prestato' - ]; - $statoLower = strtolower($stato); - if (!isset($stateMap[$statoLower])) { - $response->getBody()->write(json_encode([ - 'success' => false, - 'error' => __('Stato non valido') - ], JSON_UNESCAPED_UNICODE)); - return $response->withStatus(400)->withHeader('Content-Type', 'application/json'); - } - - // Filter and sanitize IDs - $cleanIds = array_filter(array_map('intval', $ids), fn($id) => $id > 0); - if (empty($cleanIds)) { - $response->getBody()->write(json_encode([ - 'success' => false, - 'error' => __('ID libri non validi') - ], JSON_UNESCAPED_UNICODE)); - return $response->withStatus(400)->withHeader('Content-Type', 'application/json'); - } - - // Build placeholders for IN clause - $placeholders = implode(',', array_fill(0, count($cleanIds), '?')); - $types = str_repeat('i', count($cleanIds)); - - $sql = "UPDATE libri SET stato = ? WHERE id IN ($placeholders) AND deleted_at IS NULL"; - $stmt = $db->prepare($sql); - if (!$stmt) { - AppLog::error('libri.bulk_status.prepare_failed', ['error' => $db->error]); - $response->getBody()->write(json_encode([ - 'success' => false, - 'error' => __('Errore interno del database') - ], JSON_UNESCAPED_UNICODE)); - return $response->withStatus(500)->withHeader('Content-Type', 'application/json'); - } - - // Bind normalized stato + all IDs - $normalizedStato = $stateMap[$statoLower]; - $params = array_merge([$normalizedStato], $cleanIds); - $stmt->bind_param('s' . $types, ...$params); - if (!$stmt->execute()) { - AppLog::error('libri.bulk_status.execute_failed', ['error' => $stmt->error]); - $stmt->close(); - $response->getBody()->write(json_encode([ - 'success' => false, - 'error' => __('Errore durante l\'aggiornamento dello stato') - ], JSON_UNESCAPED_UNICODE)); - return $response->withStatus(500)->withHeader('Content-Type', 'application/json'); - } - $affected = $stmt->affected_rows; - $stmt->close(); - - if ($affected > 0) { - ContentCache::booksChanged(); - } - - AppLog::info('libri.bulk_status', ['ids' => $cleanIds, 'stato' => $normalizedStato, 'affected' => $affected]); - $response->getBody()->write(json_encode([ - 'success' => true, - 'affected' => $affected, - 'message' => sprintf(__('Stato aggiornato per %d libri'), $affected) + 'success' => false, + 'code' => 'derived_book_state', + 'error' => __('Lo stato del libro è calcolato automaticamente. Modifica lo stato delle singole copie dalla scheda del libro.') ], JSON_UNESCAPED_UNICODE)); - return $response->withHeader('Content-Type', 'application/json'); + return $response->withStatus(409)->withHeader('Content-Type', 'application/json'); } /** diff --git a/app/Controllers/LibriController.php b/app/Controllers/LibriController.php index 1987fe55b..d1a4347a9 100644 --- a/app/Controllers/LibriController.php +++ b/app/Controllers/LibriController.php @@ -685,6 +685,103 @@ private function resolveContributorIds(mysqli $db, array $data, string $roleKey) return array_values(array_unique(array_filter($ids, static fn($i) => $i > 0))); } + /** + * Build one bounded advisory-lock name per identifier value. Field names are + * intentionally irrelevant: ISBN-13 and EAN are cross-compared by the + * duplicate query, so the same value must map to the same lock whichever + * field carried it. Sorting makes multi-lock acquisition deadlock-safe. + * + * @param array $codes + * @return list + */ + private static function bookIdentifierLockNames(string $databaseName, array $codes): array + { + if ($databaseName === '') { + throw new \RuntimeException('Unable to resolve the current database for identifier locking.'); + } + + $values = []; + foreach ($codes as $value) { + // Identifier input is ASCII by definition. Normalise the ISBN-10 + // check digit as well: the database collation compares x/X as the + // same value, so their advisory-lock identity must do the same. + $normalized = strtoupper(trim((string) $value)); + if ($normalized !== '') { + $values[$normalized] = true; + } + } + $values = array_keys($values); + sort($values, SORT_STRING); + + return array_map( + static fn(string $value): string => 'pinakes-book-id:' . md5($databaseName . "\0" . $value), + $values + ); + } + + /** @param array $codes @return list */ + private function acquireBookIdentifierLocks(mysqli $db, array $codes): array + { + $databaseName = ''; + $databaseResult = $db->query('SELECT DATABASE()'); + if ($databaseResult instanceof \mysqli_result) { + $databaseName = (string) ($databaseResult->fetch_row()[0] ?? ''); + $databaseResult->free(); + } + + $lockNames = self::bookIdentifierLockNames($databaseName, $codes); + $acquired = []; + try { + foreach ($lockNames as $lockName) { + $stmt = $db->prepare('SELECT GET_LOCK(?, 10)'); + if ($stmt === false) { + throw new \RuntimeException('Unable to prepare book-identifier lock.'); + } + $stmt->bind_param('s', $lockName); + try { + if (!$stmt->execute()) { + throw new \RuntimeException('Unable to acquire book-identifier lock: ' . $stmt->error); + } + $result = $stmt->get_result(); + $locked = $result instanceof \mysqli_result + && (int) ($result->fetch_row()[0] ?? 0) === 1; + } finally { + $stmt->close(); + } + if (!$locked) { + throw new \RuntimeException('Timed out while waiting for a book-identifier lock.'); + } + $acquired[] = $lockName; + } + return $acquired; + } catch (\Throwable $e) { + $this->releaseBookIdentifierLocks($db, $acquired); + throw $e; + } + } + + /** @param list $lockNames */ + private function releaseBookIdentifierLocks(mysqli $db, array $lockNames): void + { + foreach (array_reverse($lockNames) as $lockName) { + try { + $stmt = $db->prepare('SELECT RELEASE_LOCK(?)'); + if ($stmt === false) { + continue; + } + $stmt->bind_param('s', $lockName); + try { + $stmt->execute(); + } finally { + $stmt->close(); + } + } catch (\Throwable $e) { + // Advisory locks are connection-scoped and a broken connection + // releases them server-side. Never mask the original save error. + } + } + } + /** * Create a book from the admin form: validates input, resolves authors and * publishers (multi-publisher, issue #143), handles cover + scraping data, @@ -842,11 +939,15 @@ public function store(Request $request, Response $response, mysqli $db): Respons $fields['editore_id'] = empty($fields['editore_id']) || $fields['editore_id'] == 0 ? null : (int) $fields['editore_id']; $fields['genere_id'] = empty($fields['genere_id']) || $fields['genere_id'] == 0 ? null : (int) $fields['genere_id']; $fields['sottogenere_id'] = empty($fields['sottogenere_id']) || $fields['sottogenere_id'] == 0 ? null : (int) $fields['sottogenere_id']; - $fields['copie_totali'] = (int) $fields['copie_totali']; - // Add bounds checking to prevent integer overflow - if ($fields['copie_totali'] < 1) { - $fields['copie_totali'] = 1; - } elseif ($fields['copie_totali'] > 9999) { + // Reject non-scalar or non-integer input BEFORE casting: (int) "abc" is 0 + // and (int) (non-empty array) is 1, both of which would slip past the + // 0..9999 bounds as a silent, wrong copy count. Only a genuine integer + // string is honoured; anything else falls back to zero copies. + $rawCopie = $fields['copie_totali'] ?? 0; + $fields['copie_totali'] = (is_scalar($rawCopie) && preg_match('/^\d+$/', trim((string) $rawCopie)) === 1) + ? (int) trim((string) $rawCopie) + : 0; + if ($fields['copie_totali'] > 9999) { $fields['copie_totali'] = 9999; } // In creazione, copie_disponibili = copie_totali (le copie sono tutte nuove e disponibili) @@ -864,12 +965,21 @@ public function store(Request $request, Response $response, mysqli $db): Respons // scraped_cover_url; this flag lets the scraped-cover branch skip the // redundant second download that would orphan a file on disk (#F009). $scrapedCoverAlreadySaved = false; + // A localised external cover is written before the DB transaction. Keep + // its path so an atomic book/copies rollback can remove that request's + // file as well as its database rows. + $coverCreatedBeforeAtomicInsert = ''; if ($fields['copertina_url'] === '' || $fields['copertina_url'] === null) { $fields['copertina_url'] = null; } else { // Auto-download external cover URLs $originalCoverUrl = (string) $fields['copertina_url']; $fields['copertina_url'] = $this->downloadExternalCover($fields['copertina_url']); + if (preg_match('#^https?://#i', $originalCoverUrl) === 1 + && is_string($fields['copertina_url']) + && strpos($fields['copertina_url'], '/uploads/copertine/') === 0) { + $coverCreatedBeforeAtomicInsert = $fields['copertina_url']; + } if (is_string($fields['copertina_url']) && strpos($fields['copertina_url'], '/uploads/copertine/') === 0 && isset($data['scraped_cover_url']) @@ -928,33 +1038,23 @@ public function store(Request $request, Response $response, mysqli $db): Respons } } - // Acquire advisory lock to make duplicate check + insert atomic - $lockKey = null; + // Acquire one schema-scoped advisory lock per normalized identifier. + // Create and update deliberately share this protocol: locking the whole + // submitted tuple would not serialize partially-overlapping sets (for + // example isbn13=X versus ean=X), while different create/update prefixes + // would not serialize at all. + $lockKeys = []; if (!empty($codes)) { - // Create unique lock key from identifiers - $lockKey = 'book_create_' . md5(implode('|', array_values($codes))); - $lockStmt = $db->prepare("SELECT GET_LOCK(?, 10)"); - if (!$lockStmt) { + try { + $lockKeys = $this->acquireBookIdentifierLocks($db, $codes); + } catch (\Throwable $e) { + SecureLogger::error('LibriController::store identifier lock failed: ' . $e->getMessage()); $response->getBody()->write(json_encode([ 'error' => 'lock_error', 'message' => __('Errore interno durante acquisizione lock.') ], JSON_UNESCAPED_UNICODE)); return $response->withStatus(503)->withHeader('Content-Type', 'application/json'); } - $lockStmt->bind_param('s', $lockKey); - $lockStmt->execute(); - $lockResult = $lockStmt->get_result(); - $locked = $lockResult ? (int) $lockResult->fetch_row()[0] : 0; - $lockStmt->close(); - - if (!$locked) { - // Failed to acquire lock (timeout or error) - $response->getBody()->write(json_encode([ - 'error' => 'lock_timeout', - 'message' => __('Impossibile acquisire il lock. Riprova tra qualche secondo.') - ], JSON_UNESCAPED_UNICODE)); - return $response->withStatus(503)->withHeader('Content-Type', 'application/json'); - } } try { @@ -984,10 +1084,6 @@ public function store(Request $request, Response $response, mysqli $db): Respons $stmt->execute(); $dup = $stmt->get_result()->fetch_assoc(); if ($dup) { - // Release lock before returning - if ($rlStmt = $db->prepare("SELECT RELEASE_LOCK(?)")) { $rlStmt->bind_param('s', $lockKey); $rlStmt->execute(); $rlStmt->close(); } - - // Build location string $location = ''; if (!empty($dup['scaffale_codice']) && !empty($dup['mensola_livello']) && !empty($dup['posizione_progressiva'])) { @@ -1176,7 +1272,81 @@ public function store(Request $request, Response $response, mysqli $db): Respons // Plugin hook: Before book save \App\Support\Hooks::do('book.save.before', [$fields, null]); - $id = $repo->createBasic($fields); + // Atomic create: the book row and its initial physical copies must + // persist together or not at all — a copy-creation failure used to + // leave an orphan book with no/partial holdings. The transaction + // wraps ONLY createBasic() + createManyForBook() + the SQL-only + // availability reconciliation. createBasic detects the open + // transaction via its savepoint probe and nests with SAVEPOINT; + // DataIntegrity is explicitly told that this transaction belongs + // to the caller. No plugin hook fires inside it — book.save.after + // handlers (e.g. book-club) can open their own transaction, which + // under mysqli would implicitly commit the enclosing transaction + // and silently destroy this atomicity. Hooks and series/LT metadata + // therefore run strictly after the commit. + if (!$db->begin_transaction()) { + throw new \RuntimeException('Database error: unable to begin book create transaction'); + } + try { + $id = $repo->createBasic($fields); + + // Genera copie fisiche del libro + $copyRepo = new \App\Models\CopyRepository($db); + $copieTotali = (int) $fields['copie_totali']; + $baseInventario = !empty($fields['numero_inventario']) + ? $fields['numero_inventario'] + : "LIB-{$id}"; + + // Create the requested holding set with one atomic multi-row INSERT: + // a request for three copies must never leave a partial 1/3 or 2/3 + // result if one inventory code fails. Codes stay uniform (-C1, -C2, + // ...) and collision-free through the repository allocator. + $createdCopies = $copyRepo->createManyForBook( + $id, + $baseInventario, + $copieTotali, + 'disponibile', + __('Copia %d di %d') + ); + if ($createdCopies !== $copieTotali) { + throw new \RuntimeException('Unable to create the requested physical copies.'); + } + + // Counters and canonical state are part of the same invariant as + // the book and its holdings. If reconciliation fails, rolling + // back here prevents a committed book whose summary/API fields + // disagree with the physical copies just created. + $availabilityUpdated = (new \App\Support\DataIntegrity($db)) + ->recalculateBookAvailability($id, true); + if (!$availabilityUpdated) { + throw new \RuntimeException('Unable to recalculate availability for the new book.'); + } + + if (!$db->commit()) { + throw new \RuntimeException('Database error: unable to commit book create transaction'); + } + } catch (\Throwable $atomicCreateError) { + try { + $db->rollback(); + } catch (\Throwable $rollbackError) { + // best-effort — a dropped connection must not mask the original error + } + // The cover download is filesystem I/O and cannot participate in + // mysqli's transaction. Delete only the file localised by this + // request; raw external URLs and pre-existing local files no-op. + try { + $this->deleteLocalCoverFile($coverCreatedBeforeAtomicInsert); + } catch (\Throwable $coverCleanupError) { + \App\Support\SecureLogger::warning('Unable to clean up cover after atomic book rollback', [ + 'error' => $coverCleanupError->getMessage(), + ]); + } + \App\Support\SecureLogger::error('LibriController::store atomic book+copies create failed', [ + 'error' => $atomicCreateError->getMessage(), + ]); + throw $atomicCreateError; + } + $this->syncSeriesMetadataFromBookForm($db, $id, $fields, $data); // Handle LibraryThing fields visibility preferences @@ -1188,37 +1358,17 @@ public function store(Request $request, Response $response, mysqli $db): Respons // Plugin hook: After book save \App\Support\Hooks::do('book.save.after', [$id, $fields]); - // Genera copie fisiche del libro - $copyRepo = new \App\Models\CopyRepository($db); - $copieTotali = (int) $fields['copie_totali']; - $baseInventario = !empty($fields['numero_inventario']) - ? $fields['numero_inventario'] - : "LIB-{$id}"; - - // Uniform "-C{N}" codes for every copy (#238): even a single copy is - // "{base}-C1", so later adding a 2nd copy yields a consistent C1/C2 pair - // instead of a bare base plus a "-C2". allocateInventoryCodes guarantees - // no collision with any existing numero_inventario. - $codes = $copyRepo->allocateInventoryCodes($baseInventario, $copieTotali); - foreach ($codes as $i => $numeroInventario) { - $note = "Copia " . ($i + 1) . " di {$copieTotali}"; - $copyRepo->create($id, $numeroInventario, 'disponibile', $note); - } - - // Ricalcola disponibilità dopo aver generato le copie, come fa il - // percorso di update. Senza questo, copie_disponibili/copie_totali e - // lo stato canonico non vengono derivati dalle copie appena create: - // ogni superficie OPAC calcola la disponibilità da copie_disponibili, - // quindi un nuovo libro resterebbe con i contatori a zero (o con lo - // stato grezzo scritto dal form) finché non passa un altro salvataggio. - (new \App\Support\DataIntegrity($db))->recalculateBookAvailability($id); - // Persist all fields first, then apply an explicitly chosen cover // (file upload or scraped URL) on top so it isn't reverted by the // field update (mirrors update(); see #165). // Optionals (numero_pagine, ean, data_pubblicazione, traduttore) // Merge normalized $fields over $data so NULL isbn/ean values are preserved (new \App\Models\BookRepository($db))->updateOptionals($id, array_merge($data, $fields)); + // Every identifier write is now committed and visible. Release before + // cover I/O/search-index work so a duplicate request can fail fast + // instead of waiting behind unrelated post-save processing. + $this->releaseBookIdentifierLocks($db, $lockKeys); + $lockKeys = []; // The cover persisted just above may be a local file that // downloadExternalCover() saved during this submit; capture it so // it can be cleaned up if the branches below replace it (#F002). @@ -1264,10 +1414,7 @@ public function store(Request $request, Response $response, mysqli $db): Respons return $response->withHeader('Location', url('/admin/books/' . $id))->withStatus(302); } finally { - // Release advisory lock - if ($lockKey) { - if ($rlStmt = $db->prepare("SELECT RELEASE_LOCK(?)")) { $rlStmt->bind_param('s', $lockKey); $rlStmt->execute(); $rlStmt->close(); } - } + $this->releaseBookIdentifierLocks($db, $lockKeys); } } @@ -1448,53 +1595,15 @@ public function update(Request $request, Response $response, mysqli $db, int $id $fields['editore_id'] = empty($fields['editore_id']) || $fields['editore_id'] == 0 ? null : (int) $fields['editore_id']; $fields['genere_id'] = empty($fields['genere_id']) || $fields['genere_id'] == 0 ? null : (int) $fields['genere_id']; $fields['sottogenere_id'] = empty($fields['sottogenere_id']) || $fields['sottogenere_id'] == 0 ? null : (int) $fields['sottogenere_id']; - // Clamp to the same 1..9999 range as store(): an unbounded value would build - // a huge allocation loop / copy set. (#252 CodeRabbit) - $fields['copie_totali'] = (int) $fields['copie_totali']; - if ($fields['copie_totali'] < 1) { - $fields['copie_totali'] = 1; - } elseif ($fields['copie_totali'] > 9999) { - $fields['copie_totali'] = 9999; - } - - // Validazione copie: verifica che sia possibile ridurre il numero di copie. - // Usa lo stesso conteggio "in circolazione" della gestione copie più sotto - // (e di libri.copie_totali via DataIntegrity): con il conteggio grezzo, - // un libro con copie fuori circolazione divergerebbe dalla logica reale e - // bloccherebbe il salvataggio con un messaggio errato. - $copyRepo = new \App\Models\CopyRepository($db); - $currentCopieCount = $copyRepo->countInCirculationByBookId($id); - $newCopieCount = $fields['copie_totali']; - - if ($newCopieCount < $currentCopieCount) { - // Conta quante copie sono disponibili per la rimozione - $copie = $copyRepo->getByBookId($id); - $removableCopies = 0; - - foreach ($copie as $copia) { - if ($copia['stato'] === 'disponibile' && empty($copia['prestito_id'])) { - $removableCopies++; - } - } - - $requiredReduction = $currentCopieCount - $newCopieCount; - - if ($requiredReduction > $removableCopies) { - // The floor is the in-circulation count minus what we can remove. - // Deriving it from $currentCopieCount (which already excludes - // out-of-circulation copies) keeps the message consistent with the - // canonical libri.copie_totali, instead of counting perso/danneggiato - // copies that aren't part of that total at all. - $minimumCopies = $currentCopieCount - $removableCopies; - $_SESSION['error_message'] = sprintf( - __('Impossibile ridurre le copie a %d. Ci sono %d copie non disponibili (in prestito, perse o danneggiate). Il numero minimo di copie totali è %d.'), - $newCopieCount, - $minimumCopies, - $minimumCopies - ); - return $response->withHeader('Location', url('/admin/books/edit/' . $id))->withStatus(302); - } - } + // Copies are managed individually from the book summary (#physical-copies); + // the edit form's "Copie in circolazione" field is read-only. Ignore any + // submitted copie_totali — derive it from the copie table — so a crafted + // POST that bypasses the client-side readonly cannot drive the copy + // reconciliation below to silently add or delete copies. The derived value + // matches libri.copie_totali (same out-of-circulation exclusion as + // DataIntegrity), so the add/remove branches become guaranteed no-ops. + $copyRepoCount = new \App\Models\CopyRepository($db); + $fields['copie_totali'] = $copyRepoCount->countInCirculationByBookId($id); // Non aggiorniamo disponibilità/stato dall'utente: sono derivati dalle copie. unset($fields['copie_disponibili']); @@ -1604,27 +1713,17 @@ public function update(Request $request, Response $response, mysqli $db, int $id } } - // Acquire advisory lock to make duplicate check + update atomic - $lockKey = null; + // Same per-identifier protocol as store(): create/update and partially + // overlapping ISBN/EAN sets must contend on the same lock names. + $lockKeys = []; if (!empty($codes)) { - // Create unique lock key from identifiers - $lockKey = 'book_update_' . md5(implode('|', array_values($codes))); - $lockStmt = $db->prepare("SELECT GET_LOCK(?, 10)"); - if (!$lockStmt) { + try { + $lockKeys = $this->acquireBookIdentifierLocks($db, $codes); + } catch (\Throwable $e) { + SecureLogger::error('LibriController::update identifier lock failed: ' . $e->getMessage()); $_SESSION['error_message'] = __('Errore del server. Riprova.'); return $response->withHeader('Location', url('/admin/books/edit/' . $id))->withStatus(302); } - $lockStmt->bind_param('s', $lockKey); - $lockStmt->execute(); - $lockResult = $lockStmt->get_result(); - $locked = $lockResult ? (int) $lockResult->fetch_row()[0] : 0; - $lockStmt->close(); - - if (!$locked) { - // Failed to acquire lock (timeout or error) - $_SESSION['error_message'] = __('Impossibile acquisire il lock. Riprova tra qualche secondo.'); - return $response->withHeader('Location', url('/admin/books/edit/' . $id))->withStatus(302); - } } try { @@ -1655,10 +1754,6 @@ public function update(Request $request, Response $response, mysqli $db, int $id $stmt->execute(); $dup = $stmt->get_result()->fetch_assoc(); if ($dup) { - // Release lock before returning - if ($rlStmt = $db->prepare("SELECT RELEASE_LOCK(?)")) { $rlStmt->bind_param('s', $lockKey); $rlStmt->execute(); $rlStmt->close(); } - - // Build location string $location = ''; if (!empty($dup['scaffale_codice']) && !empty($dup['mensola_livello']) && !empty($dup['posizione_progressiva'])) { @@ -1858,79 +1953,6 @@ public function update(Request $request, Response $response, mysqli $db, int $id // Plugin hook: After book save (update) \App\Support\Hooks::do('book.save.after', [$id, $fields]); - // Gestione copie: aggiorna il numero di copie se cambiato. - // Il conteggio di riferimento DEVE escludere le copie fuori - // circolazione (perso/danneggiato/manutenzione/in_restauro/ - // in_trasferimento), perché `$fields['copie_totali']` arriva dal - // form pre-compilato con `libri.copie_totali`, che DataIntegrity - // calcola con la stessa esclusione. Con il conteggio grezzo - // (countByBookId) un libro con una copia fuori circolazione avrebbe - // current > form a ogni salvataggio e cancellerebbe una copia buona. - $copyRepo = new \App\Models\CopyRepository($db); - $currentCopieCount = $copyRepo->countInCirculationByBookId($id); - $newCopieCount = (int) $fields['copie_totali']; - - if ($newCopieCount > $currentCopieCount) { - // Aggiungi nuove copie. #238: generate gap-filling, collision-free - // "-C{N}" codes instead of "-C{count+1}" (which duplicated an existing - // code after a copy had been removed). allocateInventoryCodes checks - // every candidate against the whole `copie` table. - $baseInventario = !empty($fields['numero_inventario']) - ? $fields['numero_inventario'] - : "LIB-{$id}"; - - $howMany = $newCopieCount - $currentCopieCount; - $codes = $copyRepo->allocateInventoryCodes($baseInventario, $howMany); - foreach ($codes as $numeroInventario) { - $note = "Copia {$numeroInventario}"; - $newCopyId = $copyRepo->create($id, $numeroInventario, 'disponibile', $note); - - // Case 1: Reassign pending reservations to this new copy - try { - $reassignmentService = new \App\Services\ReservationReassignmentService($db); - $reassignmentService->reassignOnNewCopy($id, $newCopyId); - } catch (\Throwable $e) { - SecureLogger::error(__('Riassegnazione prenotazione nuova copia fallita') . ': ' . $e->getMessage(), [ - 'copia_id' => $newCopyId, - ]); - } - - // Also process waitlist (prenotazioni -> prestiti) as we have more capacity now - try { - $reservationManager = new \App\Controllers\ReservationManager($db); - $reservationManager->processBookAvailability($id); - } catch (\Throwable $e) { - SecureLogger::error(__('Elaborazione lista attesa fallita') . ': ' . $e->getMessage(), [ - 'libro_id' => $id, - ]); - } - } - } elseif ($newCopieCount < $currentCopieCount) { - // Rimuovi copie in eccesso DALLA CODA (le ultime aggiunte), solo quelle - // disponibili e senza impegni (#238: prima si rimuoveva la PRIMA della - // lista ASC, lasciando codici col suffisso più alto → collisioni dopo). - $removable = $copyRepo->getRemovableCopiesNewestFirst($id); - $toRemove = $currentCopieCount - $newCopieCount; - $removed = 0; - - foreach ($removable as $copia) { - if ($removed >= $toRemove) { - break; - } - // Conditional delete: only counts if the copy is still removable - // (a loan may have claimed it since the SELECT). Miscounting is - // thus impossible; the FK RESTRICT is the hard backstop. - if ($copyRepo->deleteIfRemovable($copia['id'])) { - $removed++; - } - } - - // Se non riusciamo a rimuovere abbastanza copie, avvisa l'utente - if ($removed < $toRemove) { - $_SESSION['warning_message'] = __("Attenzione: Non è stato possibile rimuovere tutte le copie richieste. Alcune copie sono attualmente in prestito."); - } - } - // Ricalcola disponibilità dopo aver modificato le copie $integrity = new \App\Support\DataIntegrity($db); $integrity->recalculateBookAvailability($id); @@ -1942,6 +1964,8 @@ public function update(Request $request, Response $response, mysqli $db, int $id // dance to swap an auto-imported cover (#165). // Merge normalized $fields over $data so NULL isbn/ean values are preserved (new \App\Models\BookRepository($db))->updateOptionals($id, array_merge($data, $fields)); + $this->releaseBookIdentifierLocks($db, $lockKeys); + $lockKeys = []; // The cover persisted just above may be a local file that // downloadExternalCover() saved during this submit; capture it so // the cleanup below can also remove it when a file upload or a @@ -1992,10 +2016,7 @@ public function update(Request $request, Response $response, mysqli $db, int $id return $response->withHeader('Location', url('/admin/books/' . $id))->withStatus(302); } finally { - // Release advisory lock - if ($lockKey) { - if ($rlStmt = $db->prepare("SELECT RELEASE_LOCK(?)")) { $rlStmt->bind_param('s', $lockKey); $rlStmt->execute(); $rlStmt->close(); } - } + $this->releaseBookIdentifierLocks($db, $lockKeys); } } diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index a0a9408ff..a3a668403 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -1317,7 +1317,9 @@ public function processReturn(Request $request, Response $response, mysqli $db, // delle prenotazioni: solo così copie_disponibili e libri.stato riflettono lo // stato finale e un libro restituito torna correttamente prestabile (TXN-002, // TXN-005, A2). insideTransaction:true mantiene l'atomicità della transazione. - $integrity->recalculateBookAvailability($libro_id, insideTransaction: true); + if (!$integrity->recalculateBookAvailability($libro_id, insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after loan return.'); + } $db->commit(); diff --git a/app/Controllers/ProfileController.php b/app/Controllers/ProfileController.php index 177853c9c..4ae91fde3 100644 --- a/app/Controllers/ProfileController.php +++ b/app/Controllers/ProfileController.php @@ -156,10 +156,17 @@ public function update(Request $request, Response $response, mysqli $db): Respon $sesso = null; // Invalid value, set to null } - // Validate locale - only allow known locales + // Validate locale. An empty value explicitly means "site default"; + // an unknown non-empty value is ignored instead of resetting the user. if ($localeProvided) { $availableLocales = \App\Support\I18n::getAvailableLocales(); - $locale = ($locale !== '' && isset($availableLocales[$locale])) ? $locale : null; + $locale = \App\Support\I18n::normalizeLocaleCode((string) $locale); + if ($locale === '') { + $locale = null; + } elseif (!isset($availableLocales[$locale])) { + $localeProvided = false; + $locale = null; + } } // Validate required fields. Surname/phone/address follow the admin @@ -245,14 +252,10 @@ public function update(Request $request, Response $response, mysqli $db): Respon $_SESSION['user']['name'] = trim($nome . ' ' . $cognome); // Apply locale change immediately (only when locale was in the form) if ($localeProvided) { - if ($locale !== null) { - \App\Support\I18n::setLocale($locale); - $_SESSION['locale'] = $locale; - } else { - // Reset runtime locale to site default so flash renders correctly - \App\Support\I18n::setLocale(\App\Support\I18n::getInstallationLocale()); - unset($_SESSION['locale']); - } + $runtimeLocale = \App\Support\I18n::resolveUserLocale($locale); + \App\Support\I18n::setLocale($runtimeLocale); + $_SESSION['locale'] = $runtimeLocale; + $_SESSION['user']['locale'] = $locale; } // Flash message AFTER locale switch so it renders in the new language $_SESSION['success_message'] = __('Profilo aggiornato con successo.'); diff --git a/app/Controllers/RegistrationController.php b/app/Controllers/RegistrationController.php index 6750152cd..754f17dc4 100644 --- a/app/Controllers/RegistrationController.php +++ b/app/Controllers/RegistrationController.php @@ -146,6 +146,15 @@ public function register(Request $request, Response $response, mysqli $db): Resp // Default stato: sospeso (richiede approvazione admin). Email da verificare $stato = 'sospeso'; $ruolo = 'standard'; + // The application language is selected for the whole installation. + // Persist it explicitly instead of relying on the historical it_IT + // schema default, which is wrong on installations using another locale. + $locale = \App\Support\I18n::normalizeLocaleCode( + \App\Support\I18n::getInstallationLocale() + ); + if (!\App\Support\I18n::isValidLocaleCode($locale)) { + $locale = 'it_IT'; + } // Ensure timezone consistency BEFORE generating dates $db->query("SET SESSION time_zone = '+00:00'"); @@ -161,9 +170,9 @@ public function register(Request $request, Response $response, mysqli $db): Resp // cognome stays in the base list: the column is NOT NULL by design (70+ // display paths CONCAT nome+cognome and a NULL would blank the whole // name), so an optional surname is stored as an empty string. - $columns = 'nome, cognome, email, password, codice_tessera, stato, tipo_utente, email_verificata, token_verifica_email, data_token_verifica, data_scadenza_tessera, privacy_accettata, data_accettazione_privacy, privacy_policy_version'; - $placeholders = '?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, 1, ?, ?'; - $types = 'ssssssssssss'; + $columns = 'nome, cognome, email, password, codice_tessera, stato, tipo_utente, locale, email_verificata, token_verifica_email, data_token_verifica, data_scadenza_tessera, privacy_accettata, data_accettazione_privacy, privacy_policy_version'; + $placeholders = '?, ?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, 1, ?, ?'; + $types = 'sssssssssssss'; $values = [ $nome, $cognome, @@ -172,6 +181,7 @@ public function register(Request $request, Response $response, mysqli $db): Resp $codice_tessera, $stato, $ruolo, + $locale, $token, $data_scadenza_token, $data_scadenza_tessera, diff --git a/app/Controllers/ReservationManager.php b/app/Controllers/ReservationManager.php index ed5b1bf88..d3b5dba4f 100644 --- a/app/Controllers/ReservationManager.php +++ b/app/Controllers/ReservationManager.php @@ -189,11 +189,11 @@ public function processBookAvailability($bookId) FROM prenotazioni r JOIN utenti u ON r.utente_id = u.id WHERE r.libro_id = ? AND r.stato = 'attiva' - AND r.data_inizio_richiesta <= ? + AND " . \App\Support\LoanEligibility::promotableReservationWhere('r') . " ORDER BY r.queue_position ASC LIMIT 1 "); - $stmt->bind_param('is', $bookId, $today); + $stmt->bind_param('iss', $bookId, $today, $today); $stmt->execute(); $result = $stmt->get_result(); $nextReservation = $result->fetch_assoc(); @@ -204,13 +204,19 @@ public function processBookAvailability($bookId) // R_END once: a legacy prenotazione may have data_fine_richiesta NULL but // data_scadenza_prenotazione set — passing the raw NULL would make // isDateRangeAvailable() return false and the row would never promote. - $startDate = $nextReservation['data_inizio_richiesta']; + $startDate = $nextReservation['data_inizio_richiesta'] + ?: (!empty($nextReservation['data_scadenza_prenotazione']) + ? substr((string) $nextReservation['data_scadenza_prenotazione'], 0, 10) + : null); $endDate = $nextReservation['data_fine_richiesta'] ?: (!empty($nextReservation['data_scadenza_prenotazione']) ? substr((string) $nextReservation['data_scadenza_prenotazione'], 0, 10) : $startDate); - // Feed the resolved end to createLoanFromReservation() too (it reads - // $reservation['data_fine_richiesta'] for the loan period). + // Feed both resolved bounds to createLoanFromReservation() too. + // Legacy rows can have a NULL requested start but a valid legacy + // deadline; selecting them without normalising the start would + // still make isDateRangeAvailable() reject them forever. + $nextReservation['data_inizio_richiesta'] = $startDate; $nextReservation['data_fine_richiesta'] = $endDate; // #157: pass the promoted reservation's queue_position so the @@ -246,7 +252,9 @@ public function processBookAvailability($bookId) // counted. Recalc again now that the reservation is 'completata', so the // commitment is counted exactly once. $integrity = new \App\Support\DataIntegrity($this->db); - $integrity->recalculateBookAvailability($bookId, true); + if (!$integrity->recalculateBookAvailability($bookId, true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation promotion.'); + } // Update queue positions for remaining reservations. // Pass the completed reservation's position: the converted @@ -359,8 +367,10 @@ private function createLoanFromReservation($reservation) $ownTransaction = $this->beginTransactionIfNeeded(); try { - // Find an available copy for this date range (no overlapping loans) - // Consider 'disponibile' and 'prenotato' copies (exclude perso/danneggiato/manutenzione) + // Find an available copy for this date range (no overlapping loans). + // Promotion happens only when the requested start has arrived, so a + // physically-out ('prestato') copy is intentionally excluded here; + // it becomes eligible through the return/reassignment path. // The NOT EXISTS clause ensures no overlapping loans for the requested dates // Note: 'da_ritirare' copies are still 'disponibile' but have a loan reservation $copyStmt = $this->db->prepare(" @@ -452,7 +462,9 @@ private function createLoanFromReservation($reservation) // Update book availability (inside transaction) $integrity = new \App\Support\DataIntegrity($this->db); - $integrity->recalculateBookAvailability($bookId, true); + if (!$integrity->recalculateBookAvailability($bookId, true)) { + throw new \RuntimeException('Failed to recalculate availability for the promoted reservation.'); + } $this->commitIfOwned($ownTransaction); return $loanId; @@ -883,7 +895,9 @@ public function cancelExpiredReservations(): int $integrity = new \App\Support\DataIntegrity($this->db); foreach ($affectedBooks as $bookId) { $this->reorderQueuePositions($bookId); - $integrity->recalculateBookAvailability($bookId, true); + if (!$integrity->recalculateBookAvailability($bookId, true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation expiry.'); + } } $this->commitIfOwned($ownTransaction); diff --git a/app/Controllers/ReservationsAdminController.php b/app/Controllers/ReservationsAdminController.php index b7ca10050..2204d1451 100644 --- a/app/Controllers/ReservationsAdminController.php +++ b/app/Controllers/ReservationsAdminController.php @@ -280,7 +280,9 @@ public function update(Request $request, Response $response, mysqli $db, int $id } $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($libroId, insideTransaction: true); + if (!$integrity->recalculateBookAvailability($libroId, insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation update.'); + } // Cancelling/completing an active reservation frees a slot: promote the next // queued reservation(s) right away, exactly like every other release path @@ -535,7 +537,9 @@ public function store(Request $request, Response $response, mysqli $db): Respons $stmt->close(); $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($libroId, insideTransaction: true); + if (!$integrity->recalculateBookAvailability($libroId, insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation creation.'); + } $db->commit(); return $response->withHeader('Location', url('/admin/reservations') . '?created=1')->withStatus(302); diff --git a/app/Controllers/ReservationsController.php b/app/Controllers/ReservationsController.php index 45c09350c..4bf4c472f 100644 --- a/app/Controllers/ReservationsController.php +++ b/app/Controllers/ReservationsController.php @@ -434,17 +434,27 @@ public function createReservation($request, $response, $args) } $dupReservationStmt->close(); + // The pre-transaction eligibility check is only a fast fail. Lock + // and re-check the patron before creating the durable request so a + // concurrent suspension/card expiry cannot slip through the gap. + $userLockStmt = $this->db->prepare("SELECT id FROM utenti WHERE id = ? FOR UPDATE"); + $userLockStmt->bind_param('i', $userId); + $userLockStmt->execute(); + $userLockStmt->get_result(); + $userLockStmt->close(); + $eligibilityError = \App\Support\LoanEligibility::checkUser($this->db, $userId); + if ($eligibilityError !== null) { + $this->db->rollback(); + $response->getBody()->write(json_encode([ + 'success' => false, + 'message' => \App\Support\LoanEligibility::errorMessage($eligibilityError), + ])); + return $response->withHeader('Content-Type', 'application/json')->withStatus(403); + } + // Enforce max active loans per user (admin setting; 0 = no limit) $maxLoans = (int) ((new \App\Models\SettingsRepository($this->db))->get('loans', 'max_active_loans_per_user', '0') ?? 0); if ($maxLoans > 0) { - // Serialize concurrent same-user requests on different books: the - // per-book libri lock taken earlier does not mutually-exclude them, - // so without this both could pass the limit check and both commit. - $userLockStmt = $this->db->prepare("SELECT id FROM utenti WHERE id = ? FOR UPDATE"); - $userLockStmt->bind_param('i', $userId); - $userLockStmt->execute(); - $userLockStmt->close(); - $cntStmt = $this->db->prepare("SELECT COUNT(*) FROM prestiti WHERE utente_id = ? AND attivo = 1 AND stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')"); $cntStmt->bind_param('i', $userId); $cntStmt->execute(); diff --git a/app/Controllers/UserActionsController.php b/app/Controllers/UserActionsController.php index cc9f57ae6..197fdd344 100644 --- a/app/Controllers/UserActionsController.php +++ b/app/Controllers/UserActionsController.php @@ -232,7 +232,9 @@ public function cancelLoan(Request $request, Response $response, mysqli $db): Re // siamo dentro la transazione aperta in questo metodo, evita il // commit implicito di una begin_transaction() annidata) $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability((int) $loan['libro_id'], insideTransaction: true); + if (!$integrity->recalculateBookAvailability((int) $loan['libro_id'], insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after loan cancellation.'); + } $db->commit(); @@ -347,12 +349,28 @@ public function cancelReservation(Request $request, Response $response, mysqli $ $updatePos->close(); $reorderStmt->close(); - // Recalculate book availability + // Cancelling a queue reservation frees a promised capacity unit. + // Promote every now-eligible row in the same transaction, matching + // the admin cancellation and physical-copy release paths. + $reservationManager = new \App\Controllers\ReservationManager($db); + $reservationManager->setExternalTransaction(true); + for ($promoGuard = 0; $promoGuard < 1000 && $reservationManager->processBookAvailability($libroId); $promoGuard++) { + // promote until the newly-freed capacity is exhausted + } + $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($libroId, insideTransaction: true); + if (!$integrity->recalculateBookAvailability($libroId, insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation cancellation.'); + } $db->commit(); + try { + $reservationManager->flushDeferredNotifications(); + } catch (\Throwable $e) { + SecureLogger::warning('Failed to flush reservation notifications after user cancellation', ['error' => $e->getMessage()]); + } + return $response->withHeader('Location', RouteTranslator::route('reservations') . '?canceled=1')->withStatus(302); } catch (\Throwable $e) { @@ -452,7 +470,9 @@ public function changeReservationDate(Request $request, Response $response, mysq // Recalculate book availability $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($libroId, insideTransaction: true); + if (!$integrity->recalculateBookAvailability($libroId, insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation date change.'); + } $db->commit(); @@ -568,19 +588,22 @@ public function loan(Request $request, Response $response, mysqli $db): Response } $dupReservationStmt->close(); + // Revalidate eligibility while holding the user row. The fast check + // before the transaction improves feedback, but an administrator can + // suspend the patron between that check and this INSERT. + $userLockStmt = $db->prepare("SELECT id FROM utenti WHERE id = ? FOR UPDATE"); + $userLockStmt->bind_param('i', $utenteId); + $userLockStmt->execute(); + $userLockStmt->get_result(); + $userLockStmt->close(); + if (\App\Support\LoanEligibility::checkUser($db, $utenteId) !== null) { + $db->rollback(); + return $this->back($response, ['loan_error' => 'not_eligible']); + } + // Enforce max active loans per user (admin setting; 0 = no limit) $maxLoans = (int) ((new \App\Models\SettingsRepository($db))->get('loans', 'max_active_loans_per_user', '0') ?? 0); if ($maxLoans > 0) { - // Serialize concurrent loan requests by the SAME user: the per-book - // libri lock taken earlier does not mutually exclude two requests - // for *different* books, so without this both could read the same - // activeCount below the limit and both insert, exceeding it. - // Locking the user row forces them to run one at a time. - $userLockStmt = $db->prepare("SELECT id FROM utenti WHERE id = ? FOR UPDATE"); - $userLockStmt->bind_param('i', $utenteId); - $userLockStmt->execute(); - $userLockStmt->close(); - $cntStmt = $db->prepare("SELECT COUNT(*) FROM prestiti WHERE utente_id = ? AND attivo = 1 AND stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')"); $cntStmt->bind_param('i', $utenteId); $cntStmt->execute(); @@ -729,6 +752,18 @@ public function reserve(Request $request, Response $response, mysqli $db): Respo } $dupLoanStmt->close(); + // Revalidate under the user lock so a concurrent suspension/card + // expiry cannot race the pre-transaction eligibility check. + $userLockStmt = $db->prepare("SELECT id FROM utenti WHERE id = ? FOR UPDATE"); + $userLockStmt->bind_param('i', $utenteId); + $userLockStmt->execute(); + $userLockStmt->get_result(); + $userLockStmt->close(); + if (\App\Support\LoanEligibility::checkUser($db, $utenteId) !== null) { + $db->rollback(); + return $this->back($response, ['reserve_error' => 'not_eligible']); + } + // Canonical peak-capacity decision (same service as admin create, // approval, renew and audit), excluding this user defensively. $capacity = new \App\Services\CapacityService($db); @@ -759,7 +794,9 @@ public function reserve(Request $request, Response $response, mysqli $db): Respo // Recalculate book availability after reservation $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($libroId, insideTransaction: true); + if (!$integrity->recalculateBookAvailability($libroId, insideTransaction: true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation creation.'); + } $db->commit(); $params = ['reserve_success' => 1]; diff --git a/app/Controllers/UsersController.php b/app/Controllers/UsersController.php index 0390a9058..a701d510c 100644 --- a/app/Controllers/UsersController.php +++ b/app/Controllers/UsersController.php @@ -51,6 +51,8 @@ public function createForm(Request $request, Response $response): Response if ($guard = $this->guardAdminStaff($response)) { return $guard; } + $availableLocales = \App\Support\I18n::getAvailableLocales(); + $selectedLocale = $this->installationLocale(); ob_start(); require __DIR__ . '/../Views/utenti/crea_utente.php'; $content = ob_get_clean(); @@ -148,6 +150,9 @@ public function store(Request $request, Response $response, mysqli $db): Respons $note = trim(strip_tags((string) ($data['note_utente'] ?? ''))); $note = $note !== '' ? $note : null; + // New accounts start from the installation language unless the admin + // explicitly selects another active interface language. + $locale = $this->localeFromInput($data['locale'] ?? null, $this->installationLocale()); $codiceTesseraInput = trim((string) ($data['codice_tessera'] ?? '')); $dataScadenzaInput = trim((string) ($data['data_scadenza_tessera'] ?? '')); @@ -184,7 +189,7 @@ public function store(Request $request, Response $response, mysqli $db): Respons $telefono = $telefono !== '' ? $telefono : null; $emailVerificata = $isAdmin ? 1 : 1; // l'admin crea utenti già verificati - $types = str_repeat('s', 14) . 'i' . str_repeat('s', 2); + $types = str_repeat('s', 15) . 'i' . str_repeat('s', 2); // Exception mode: a failed INSERT throws. prepare()/bind_param() can throw too under // MYSQLI_REPORT_STRICT, so they're inside the try — map a UNIQUE violation (1062) to @@ -194,9 +199,9 @@ public function store(Request $request, Response $response, mysqli $db): Respons try { $stmt = $db->prepare("INSERT INTO utenti ( nome, cognome, email, telefono, password, indirizzo, cod_fiscale, data_nascita, sesso, - codice_tessera, data_scadenza_tessera, stato, tipo_utente, note_utente, + codice_tessera, data_scadenza_tessera, stato, tipo_utente, note_utente, locale, email_verificata, token_reset_password, data_token_reset - ) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"); + ) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"); $stmt->bind_param( $types, $nome, @@ -213,6 +218,7 @@ public function store(Request $request, Response $response, mysqli $db): Respons $stato, $role, $note, + $locale, $emailVerificata, $tokenReset, $dataTokenReset @@ -275,6 +281,12 @@ public function editForm(Request $request, Response $response, mysqli $db, int $ $utente = $result->fetch_assoc(); $stmt->close(); + $availableLocales = \App\Support\I18n::getAvailableLocales(); + $selectedLocale = $this->localeFromInput( + $utente['locale'] ?? null, + $this->installationLocale() + ); + // Custom registration field values (issue #255) — shown read-only so the // admin can see community handles (e.g. Telegram) alongside the account. $customFieldValues = \App\Support\RegistrationFields::labelledValuesForUser($db, $id); @@ -413,6 +425,9 @@ public function update(Request $request, Response $response, mysqli $db, int $id $note = trim(strip_tags((string) ($data['note_utente'] ?? ''))); $note = $note !== '' ? $note : null; + // Locale is a per-user preference. Invalid or omitted input preserves + // the user's current value instead of resetting it to the site default. + $locale = $this->localeFromInput($data['locale'] ?? null, (string) ($original['locale'] ?? '')); $telefono = $telefono !== '' ? $telefono : null; @@ -451,7 +466,7 @@ public function update(Request $request, Response $response, mysqli $db, int $id $dataTokenReset = null; } - $types = str_repeat('s', 14) . 'i' . str_repeat('s', 2) . 'i'; + $types = str_repeat('s', 15) . 'i' . str_repeat('s', 2) . 'i'; // Exception mode: a failed UPDATE throws. prepare()/bind_param() can throw too under // MYSQLI_REPORT_STRICT, so they're inside the try — map a UNIQUE violation (1062) to the @@ -461,7 +476,7 @@ public function update(Request $request, Response $response, mysqli $db, int $id $stmt = $db->prepare("UPDATE utenti SET nome = ?, cognome = ?, email = ?, telefono = ?, password = ?, indirizzo = ?, cod_fiscale = ?, data_nascita = ?, sesso = ?, codice_tessera = ?, data_scadenza_tessera = ?, - stato = ?, tipo_utente = ?, note_utente = ?, email_verificata = ?, + stato = ?, tipo_utente = ?, note_utente = ?, locale = ?, email_verificata = ?, token_reset_password = ?, data_token_reset = ? WHERE id = ?"); $stmt->bind_param( @@ -480,6 +495,7 @@ public function update(Request $request, Response $response, mysqli $db, int $id $stato, $role, $note, + $locale, $emailVerificata, $tokenReset, $dataTokenReset, @@ -499,6 +515,14 @@ public function update(Request $request, Response $response, mysqli $db, int $id } $stmt->close(); + // Keep the current browser session coherent when an administrator edits + // their own account; other users pick up the persisted locale at login. + if ($currentUserId === $id) { + $_SESSION['locale'] = $locale; + $_SESSION['user']['locale'] = $locale; + \App\Support\I18n::setLocale($locale); + } + // Hardening: when an admin sets a non-active state, invalidate any // pending email-verification token so a stale verification link cannot // silently re-activate a suspended/expired account (companion to the @@ -697,6 +721,31 @@ private function generateAdminCode(mysqli $db): string return $code; } + private function installationLocale(): string + { + $locale = \App\Support\I18n::normalizeLocaleCode( + \App\Support\I18n::getInstallationLocale() + ); + + return \App\Support\I18n::isValidLocaleCode($locale) ? $locale : 'it_IT'; + } + + private function localeFromInput(mixed $input, string $fallback): string + { + $availableLocales = \App\Support\I18n::getAvailableLocales(); + $requested = \App\Support\I18n::normalizeLocaleCode(is_string($input) ? trim($input) : ''); + if ($requested !== '' && isset($availableLocales[$requested])) { + return $requested; + } + + $current = \App\Support\I18n::normalizeLocaleCode($fallback); + if ($current !== '' && isset($availableLocales[$current])) { + return $current; + } + + return $this->installationLocale(); + } + private function generateTessera(mysqli $db): string { do { diff --git a/app/Middleware/RememberMeMiddleware.php b/app/Middleware/RememberMeMiddleware.php index af22edd25..91917b206 100644 --- a/app/Middleware/RememberMeMiddleware.php +++ b/app/Middleware/RememberMeMiddleware.php @@ -104,15 +104,13 @@ private function attemptAutoLogin(): void // the misleading "session expired" screen even though they are logged in. \App\Support\Csrf::ensureToken(); - // Load and apply user's preferred locale (only persist if setLocale succeeds) - if (!empty($row['locale'])) { - $locale = (string) $row['locale']; - // Ensure language cache is loaded (middleware may run before bootstrap i18n) - \App\Support\I18n::loadFromDatabase($this->db); - if (\App\Support\I18n::setLocale($locale)) { - $_SESSION['locale'] = $locale; - } - } + // Ensure language cache is loaded (middleware may run before bootstrap i18n), + // then apply the per-user preference or the installation default. + \App\Support\I18n::loadFromDatabase($this->db); + $locale = \App\Support\I18n::resolveUserLocale($row['locale'] ?? null); + \App\Support\I18n::setLocale($locale); + $_SESSION['locale'] = $locale; + $_SESSION['user']['locale'] = $locale; // Log auto-login for security auditing (no PII logged per GDPR) \App\Support\Log::security('login.remember_me', [ diff --git a/app/Models/BookRepository.php b/app/Models/BookRepository.php index b88d15536..d43858f1d 100644 --- a/app/Models/BookRepository.php +++ b/app/Models/BookRepository.php @@ -362,13 +362,16 @@ public function createBasic(array $data): int $sottogenere_id_val = $data['sottogenere_id'] ?? null; $editore_id_val = $data['editore_id'] ?? null; - $copie_totali = isset($data['copie_totali']) ? (int) $data['copie_totali'] : 1; - $copie_disponibili = isset($data['copie_disponibili']) ? (int) $data['copie_disponibili'] : 1; + // On create, copie_totali is a requested physical-holding count, not a + // writable availability cache. New copies start available; the caller + // must materialise them and run DataIntegrity in the same transaction. + // Never trust a submitted copie_disponibili/stato value. + $copie_totali = max(0, isset($data['copie_totali']) ? (int) $data['copie_totali'] : 1); + $copie_disponibili = $copie_totali; $tipo_acquisizione = $this->sanitizeAcquisitionType($data['tipo_acquisizione'] ?? null); - $stato = array_key_exists('stato', $data) - ? $this->normalizeEnumValue($data['stato'], 'stato', 'disponibile') - : null; + $initialState = $copie_totali > 0 ? 'disponibile' : 'non_disponibile'; + $stato = $this->normalizeEnumValue($initialState, 'stato', 'disponibile'); $fields = []; $placeholders = []; @@ -615,6 +618,10 @@ public function createBasic(array $data): int 'editore_id' => $editore_id_val, 'tipo_acquisizione' => $tipo_acquisizione, 'stato' => $stato, + 'derived_fields_ignored' => array_values(array_intersect( + ['stato', 'copie_disponibili'], + array_keys($data) + )), 'posizione_id' => $posizione_id_val, 'scaffale_id' => $scaffale_id_val, 'mensola_id' => $mensola_id_val, @@ -723,11 +730,7 @@ public function updateBasic(int $id, array $data): bool $sottogenere_id_val = $data['sottogenere_id'] ?? null; $editore_id_val = $data['editore_id'] ?? null; - $copie_totali = isset($data['copie_totali']) ? (int) $data['copie_totali'] : 1; - $copie_disponibili = isset($data['copie_disponibili']) ? (int) $data['copie_disponibili'] : 1; - $tipo_acquisizione = $this->sanitizeAcquisitionType($data['tipo_acquisizione'] ?? null); - $stato = $this->normalizeEnumValue($data['stato'] ?? null, 'stato', 'disponibile'); $setParts = []; $typeParts = []; @@ -792,12 +795,9 @@ public function updateBasic(int $id, array $data): bool if ($this->hasColumn('posizione_progressiva')) { $addSet('posizione_progressiva', 'i', $posizione_progressiva_val); } - if ($this->hasColumn('copie_totali')) { - $addSet('copie_totali', 'i', $copie_totali); - } - if ($this->hasColumn('copie_disponibili')) { - $addSet('copie_disponibili', 'i', $copie_disponibili); - } + // Availability fields are a read model derived atomically by + // DataIntegrity from copie/prestiti/prenotazioni. Never accept them from + // a form, plugin or direct repository caller. if ($this->hasColumn('numero_inventario')) { $addSet('numero_inventario', 's', $data['numero_inventario'] ?? null); } @@ -825,9 +825,6 @@ public function updateBasic(int $id, array $data): bool if ($this->hasColumn('posizione_id')) { $addSet('posizione_id', 'i', $posizione_id_val); } - if ($this->hasColumn('stato') && array_key_exists('stato', $data)) { - $addSet('stato', 's', $stato); - } if ($this->hasColumn('lingua')) { $addSet('lingua', 's', $data['lingua'] ?? null); } @@ -964,7 +961,10 @@ public function updateBasic(int $id, array $data): bool 'preview' => [ 'titolo' => $data['titolo'] ?? null, 'tipo_acquisizione' => $tipo_acquisizione, - 'stato' => $stato, + 'derived_fields_ignored' => array_values(array_intersect( + ['stato', 'copie_totali', 'copie_disponibili'], + array_keys($data) + )), 'posizione_id' => $posizione_id_val, 'scaffale_id' => $scaffale_id_val, 'mensola_id' => $mensola_id_val, diff --git a/app/Models/CopyRepository.php b/app/Models/CopyRepository.php index 1b6205417..f263cd625 100644 --- a/app/Models/CopyRepository.php +++ b/app/Models/CopyRepository.php @@ -171,29 +171,233 @@ public function create(int $bookId, string $numeroInventario, string $stato = 'd /** * Create $howMany copies for a book in a single round-trip: pre-load the - * existing codes of this base's family ONCE, generate collision-free + * existing codes of this base's family, generate collision-free * "{base}-C{N}" codes in memory, then batch-insert every row with one - * prepared statement. Replaces the per-copy inventoryCodeExists()+create() - * pair that turned a large copie_totali import into thousands of queries - * inside the per-row transaction (holding locks on `copie`). + * prepared statement. A connection-level advisory lock serializes inventory + * allocation even when a prefix's unique-index range is still empty (where + * gap locks alone can deadlock). Candidate membership is evaluated by SQL, + * using numero_inventario's case/accent-insensitive collation rather than a + * case-sensitive PHP array. The locking current-read then sees rows committed + * after the transaction snapshot. A bounded duplicate-key retry remains as a + * final guard for external writers. A failed multi-row INSERT never leaves a + * partial batch. Replaces the per-copy + * inventoryCodeExists()+create() pair that turned a large copie_totali + * import into thousands of queries. * * @param string|null $noteTemplate already-translated sprintf template with * two %d (index, total); null = no note. * @return int number of copies inserted */ public function createManyForBook(int $bookId, string $base, int $howMany, string $stato = 'disponibile', ?string $noteTemplate = null): int + { + return $this->createManyForBookResult($bookId, $base, $howMany, $stato, $noteTemplate, false)['count']; + } + + /** + * Create a batch and return every generated copy id. Circulation callers use + * this variant so each new physical copy can repair a copy-less HOLDING row + * before the reservation queue consumes the remaining capacity. + * + * @return list + */ + public function createManyForBookWithIds(int $bookId, string $base, int $howMany, string $stato = 'disponibile', ?string $noteTemplate = null): array + { + $result = $this->createManyForBookResult($bookId, $base, $howMany, $stato, $noteTemplate, true); + return $result['ids']; + } + + /** + * Create a batch whose copies all share the same administrator-entered + * note. This is deliberately separate from the sprintf-based import helper: + * a literal '%' in an operator note must never be interpreted as a format + * placeholder. + * + * @return list + */ + public function createManyForBookWithIdsAndNote( + int $bookId, + string $base, + int $howMany, + string $stato = 'disponibile', + ?string $note = null + ): array { + $result = $this->createManyForBookResult( + $bookId, + $base, + $howMany, + $stato, + null, + true, + $note + ); + return $result['ids']; + } + + /** + * @return array{count: int, ids: list} + */ + private function createManyForBookResult( + int $bookId, + string $base, + int $howMany, + string $stato, + ?string $noteTemplate, + bool $resolveIds, + ?string $sharedNote = null + ): array { $howMany = max(0, $howMany); if ($howMany === 0) { - return 0; + return ['count' => 0, 'ids' => []]; } // numero_inventario is VARCHAR(100); leave room for the "-C{N}" suffix. $base = mb_substr($base, 0, 90); - // Pre-load, once, every existing code that could collide with this family. - $taken = []; - $likeParam = $base . '%'; - $sel = $this->db->prepare("SELECT numero_inventario FROM copie WHERE numero_inventario LIKE ?"); + // GET_LOCK() is server-wide and connection-scoped. It does not start, + // commit or roll back the caller's transaction and is released after the + // INSERT and id lookup, never held for hooks or external services. + $lockName = $this->inventoryAllocatorLockName(); + $this->acquireInventoryAllocatorLock($lockName); + try { + $inserted = $this->insertAllocatedCopiesWhileLocked( + $bookId, + $base, + $howMany, + $stato, + $noteTemplate, + $sharedNote + ); + $ids = []; + if ($resolveIds) { + $ids = $this->copyIdsForInventoryCodes($inserted['codes']); + if (count($ids) !== $inserted['count']) { + throw new \RuntimeException('Unable to resolve every inserted physical-copy id.'); + } + } + return ['count' => $inserted['count'], 'ids' => $ids]; + } finally { + $this->releaseInventoryAllocatorLock($lockName); + } + } + + /** + * Insert allocated copies while the caller holds the inventory allocator + * lock. Book creation and both single/batch copy forms use this path, so + * code allocation, duplicate retries and INSERT behavior cannot drift apart. + * + * @return array{count: int, first_id: int, codes: list} + */ + private function insertAllocatedCopiesWhileLocked( + int $bookId, + string $base, + int $howMany, + string $stato, + ?string $noteTemplate, + ?string $sharedNote + ): array { + // Escape LIKE metacharacters in the base before appending the wildcard. + // A base ending in a backslash would otherwise escape the trailing '%', + // making the taken-set miss the whole -C{N} family and loop into 1062. + $likeParam = addcslashes($base, '%_\\') . '%'; + $maxAttempts = 5; + + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + $codes = $this->allocateInventoryCodesWithCurrentRead($base, $likeParam, $howMany); + + // Single multi-row INSERT. MySQL/MariaDB roll back the entire + // statement if any unique inventory code collides. + $total = count($codes); + $placeholders = implode(',', array_fill(0, $total, '(?, ?, ?, ?)')); + $stmt = $this->db->prepare("INSERT INTO copie (libro_id, numero_inventario, stato, note) VALUES {$placeholders}"); + if ($stmt === false) { + throw new \RuntimeException('Unable to prepare copy batch insert: ' . $this->db->error); + } + $types = ''; + $params = []; + foreach ($codes as $i => $code) { + $note = $sharedNote !== null + ? $sharedNote + : ($noteTemplate !== null ? sprintf($noteTemplate, $i + 1, $total) : null); + $types .= 'isss'; + $params[] = $bookId; + $params[] = $code; + $params[] = $stato; + $params[] = $note; + } + $stmt->bind_param($types, ...$params); + + try { + $inserted = $stmt->execute(); + $errno = $stmt->errno; + $error = $stmt->error; + } catch (\mysqli_sql_exception $e) { + $stmt->close(); + if ($e->getCode() === 1062 && $attempt < $maxAttempts) { + continue; + } + throw $e; + } + $firstInsertId = (int) $this->db->insert_id; + $stmt->close(); + + if ($inserted) { + return ['count' => $total, 'first_id' => $firstInsertId, 'codes' => $codes]; + } + if ($errno === 1062 && $attempt < $maxAttempts) { + continue; + } + throw new \RuntimeException('Unable to insert copy batch: ' . $error); + } + + throw new \RuntimeException('Unable to allocate unique inventory codes after concurrent retries.'); + } + + /** + * Create one copy with an automatically allocated inventory code and return + * its id. Allocation and INSERT share the same advisory-lock critical + * section, unlike allocateInventoryCodes() which is intentionally read-only. + */ + public function createWithAllocatedInventoryCode(int $bookId, string $base, string $stato = 'disponibile', ?string $note = null): int + { + $base = mb_substr($base, 0, 90); + $lockName = $this->inventoryAllocatorLockName(); + + $this->acquireInventoryAllocatorLock($lockName); + try { + $result = $this->insertAllocatedCopiesWhileLocked( + $bookId, + $base, + 1, + $stato, + null, + $note + ); + if ($result['count'] === 1 && $result['first_id'] > 0) { + return $result['first_id']; + } + throw new \RuntimeException('Allocated copy insert did not affect exactly one row.'); + } finally { + $this->releaseInventoryAllocatorLock($lockName); + } + } + + /** + * @return list + */ + private function allocateInventoryCodesWithCurrentRead(string $base, string $likeParam, int $howMany): array + { + // A locking current-read sees rows committed after this transaction's + // snapshot and locks matching unique-index rows/ranges until commit. + // Only the COUNT feeds the pigeonhole bound below, so COUNT(*) avoids + // materialising every matching row; FOR UPDATE keeps the locking + // current-read that gap-locks the base family until commit. The count + // stays GLOBAL (no libro_id filter) because numero_inventario is globally + // unique — scoping it to one book would undercount a base shared across + // books and break the "at least howMany free among existingCount+howMany" + // guarantee. + $sel = $this->db->prepare( + 'SELECT COUNT(*) AS n FROM copie WHERE numero_inventario LIKE ? FOR UPDATE' + ); if ($sel === false) { throw new \RuntimeException('Unable to prepare inventory-code lookup: ' . $this->db->error); } @@ -203,48 +407,171 @@ public function createManyForBook(int $bookId, string $base, int $howMany, strin $sel->close(); throw new \RuntimeException('Unable to load inventory codes: ' . $error); } - $res = $sel->get_result(); - while ($row = $res->fetch_assoc()) { - $taken[$row['numero_inventario']] = true; - } + $existingCount = (int) ($sel->get_result()->fetch_assoc()['n'] ?? 0); $sel->close(); - // Generate collision-free codes in memory (walk up, filling gaps). $codes = []; - for ($index = 1; count($codes) < $howMany; $index++) { - $candidate = "{$base}-C{$index}"; - if (!isset($taken[$candidate])) { - $taken[$candidate] = true; + $nextIndex = 1; + // Among existingCount + howMany candidates there must be at least + // howMany free values. Check them in bounded chunks so even a 9,999-copy + // import does not create an enormous UNION or one query per candidate. + $lastIndex = $existingCount + $howMany; + while (count($codes) < $howMany && $nextIndex <= $lastIndex) { + $candidates = []; + while (count($candidates) < 250 && $nextIndex <= $lastIndex) { + $candidates[] = "{$base}-C{$nextIndex}"; + $nextIndex++; + } + foreach ($this->inventoryCodesMissingUnderDatabaseCollation($candidates) as $candidate) { $codes[] = $candidate; + if (count($codes) === $howMany) { + break; + } } } + if (count($codes) !== $howMany) { + throw new \RuntimeException('Unable to allocate the requested inventory-code batch.'); + } + return $codes; + } - // Single multi-row INSERT. - $total = count($codes); - $placeholders = implode(',', array_fill(0, $total, '(?, ?, ?, ?)')); - $stmt = $this->db->prepare("INSERT INTO copie (libro_id, numero_inventario, stato, note) VALUES {$placeholders}"); + /** + * Return candidates that do not compare equal to an existing inventory code. + * The JOIN deliberately delegates equality to the database column collation. + * + * @param list $candidates + * @return list + */ + private function inventoryCodesMissingUnderDatabaseCollation(array $candidates): array + { + if ($candidates === []) { + return []; + } + $rows = []; + foreach (array_keys($candidates) as $ordinal) { + $rows[] = 'SELECT ? AS inventory_code, ' . (int) $ordinal . ' AS ordinal'; + } + $sql = 'SELECT candidates.ordinal FROM (' . implode(' UNION ALL ', $rows) . ') candidates ' + // FOR UPDATE is essential here: createBasic() may already have + // established an older transaction snapshot. A plain JOIN could + // miss a competing batch committed while GET_LOCK() was awaited + // and select C1 again; a locking read sees the latest committed row. + . 'JOIN copie c ON c.numero_inventario = candidates.inventory_code FOR UPDATE'; + $stmt = $this->db->prepare($sql); if ($stmt === false) { - throw new \RuntimeException('Unable to prepare copy batch insert: ' . $this->db->error); + throw new \RuntimeException('Unable to prepare collation-aware inventory lookup: ' . $this->db->error); } - $types = ''; - $params = []; - foreach ($codes as $i => $code) { - $note = ($noteTemplate !== null && $total > 1) ? sprintf($noteTemplate, $i + 1, $total) : null; - $types .= 'isss'; - $params[] = $bookId; - $params[] = $code; - $params[] = $stato; - $params[] = $note; + $stmt->bind_param(str_repeat('s', count($candidates)), ...$candidates); + if (!$stmt->execute()) { + $error = $stmt->error; + $stmt->close(); + throw new \RuntimeException('Unable to compare inventory codes: ' . $error); + } + $taken = []; + foreach ($stmt->get_result()->fetch_all(MYSQLI_NUM) as $row) { + $taken[(int) $row[0]] = true; + } + $stmt->close(); + + $missing = []; + foreach ($candidates as $ordinal => $candidate) { + if (!isset($taken[$ordinal])) { + $missing[] = $candidate; + } + } + return $missing; + } + + /** + * @param list $codes + * @return list + */ + private function copyIdsForInventoryCodes(array $codes): array + { + if ($codes === []) { + return []; } - $stmt->bind_param($types, ...$params); + $placeholders = implode(',', array_fill(0, count($codes), '?')); + $stmt = $this->db->prepare( + "SELECT id FROM copie WHERE numero_inventario IN ({$placeholders}) ORDER BY id" + ); + if ($stmt === false) { + throw new \RuntimeException('Unable to prepare inserted-copy lookup: ' . $this->db->error); + } + $stmt->bind_param(str_repeat('s', count($codes)), ...$codes); if (!$stmt->execute()) { $error = $stmt->error; $stmt->close(); - throw new \RuntimeException('Unable to insert copy batch: ' . $error); + throw new \RuntimeException('Unable to resolve inserted-copy ids: ' . $error); } + $ids = array_map( + static fn (array $row): int => (int) $row[0], + $stmt->get_result()->fetch_all(MYSQLI_NUM) + ); $stmt->close(); + return $ids; + } + + /** + * Advisory-lock name for inventory-code allocation, scoped to the current + * schema so two Pinakes installs on one MySQL server don't serialise each + * other's copy allocation. GET_LOCK() is server-wide and its name is capped + * at 64 chars (since MySQL 5.7.5), and a database name can itself be up to + * 64 chars — so the schema is folded through a PHP md5 (fixed 32 hex): + * 'pinakes-copy:' + 32 = 45 chars, always bounded, and independent of any + * server-side hash function. + */ + private function inventoryAllocatorLockName(): string + { + $dbName = ''; + $res = $this->db->query('SELECT DATABASE()'); + if ($res instanceof \mysqli_result) { + $row = $res->fetch_row(); + $dbName = (string) ($row[0] ?? ''); + $res->free(); + } + return 'pinakes-copy:' . md5($dbName); + } - return $total; + private function acquireInventoryAllocatorLock(string $lockName): void + { + // $lockName is already schema-scoped and length-bounded in PHP + // (see inventoryAllocatorLockName) — bind it verbatim. Hashing in PHP, + // not via a SQL MD5(DATABASE()), keeps this working on servers whose + // MySQL build does not expose MD5() and stays under MySQL's 64-char + // GET_LOCK limit regardless of the database name length. + $stmt = $this->db->prepare('SELECT GET_LOCK(?, 10)'); + if ($stmt === false) { + throw new \RuntimeException('Unable to prepare inventory allocator lock: ' . $this->db->error); + } + $stmt->bind_param('s', $lockName); + try { + if (!$stmt->execute()) { + throw new \RuntimeException('Unable to acquire inventory allocator lock: ' . $stmt->error); + } + $acquired = (int) ($stmt->get_result()->fetch_row()[0] ?? 0); + } finally { + $stmt->close(); + } + if ($acquired !== 1) { + throw new \RuntimeException('Timed out while waiting to allocate inventory codes.'); + } + } + + private function releaseInventoryAllocatorLock(string $lockName): void + { + try { + $stmt = $this->db->prepare('SELECT RELEASE_LOCK(?)'); + if ($stmt === false) { + return; + } + $stmt->bind_param('s', $lockName); + $stmt->execute(); + $stmt->close(); + } catch (\Throwable $e) { + // A broken connection releases its advisory locks server-side. Do + // not hide the original copy-allocation error from the caller. + } } /** diff --git a/app/Models/LoanRepository.php b/app/Models/LoanRepository.php index ab16783b5..c3720c15f 100644 --- a/app/Models/LoanRepository.php +++ b/app/Models/LoanRepository.php @@ -230,20 +230,6 @@ public function close(int $id): bool return false; } - // Determina se il libro ha altri prestiti attivi (include 'prenotato' for scheduled future loans) - $activeCount = 0; - $countStmt = $this->db->prepare("SELECT COUNT(*) AS c FROM prestiti WHERE libro_id=? AND attivo=1 AND stato IN ('in_corso','in_ritardo','da_ritirare','prenotato')"); - $countStmt->bind_param('i', $bookId); - $countStmt->execute(); - $activeCount = (int)($countStmt->get_result()->fetch_assoc()['c'] ?? 0); - $countStmt->close(); - - $newStatus = $activeCount > 0 ? 'prestato' : 'disponibile'; - $updateBookStmt = $this->db->prepare('UPDATE libri SET stato = ? WHERE id = ?'); - $updateBookStmt->bind_param('si', $newStatus, $bookId); - $updateBookStmt->execute(); - $updateBookStmt->close(); - // Recalculate availability and process reservations INSIDE the transaction // This ensures FOR UPDATE locks in processBookAvailability are effective $integrity = new DataIntegrity($this->db); @@ -277,7 +263,9 @@ public function close(int $id): bool // recalculated successfully above in this same transaction, so it cannot // have vanished (recalculateBookAvailability only returns false when the // book row is missing). - $integrity->recalculateBookAvailability($bookId, true); + if (!$integrity->recalculateBookAvailability($bookId, true)) { + throw new \RuntimeException('Unable to recalculate final book availability.'); + } $this->db->commit(); diff --git a/app/Routes/web.php b/app/Routes/web.php index 6c04f3a55..5a8a9395b 100644 --- a/app/Routes/web.php +++ b/app/Routes/web.php @@ -1494,6 +1494,12 @@ return $controller->deleteCopy($request, $response, $db, (int) $args['id']); })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware()); + $app->post('/admin/books/{id:\d+}/copies/create', function ($request, $response, $args) use ($app) { + $controller = new \App\Controllers\CopyController(); + $db = $app->getContainer()->get('db'); + return $controller->createCopy($request, $response, $db, (int) $args['id']); + })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware()); + // Series (Series) management $app->get('/admin/series', function ($request, $response) use ($app) { $controller = new \App\Controllers\CollaneController(); @@ -2386,13 +2392,20 @@ // Get request body $body = $request->getParsedBody(); - if (!$body) { - $body = json_decode((string) $request->getBody(), true); + if (!is_array($body) || $body === []) { + $decodedBody = json_decode((string) $request->getBody(), true); + $body = is_array($decodedBody) ? $decodedBody : []; } - $copiesToAdd = (int) ($body['copies'] ?? 0); + // Keep the server-side contract aligned with the admin prompt. Avoid + // PHP's surprising casts ((int) ['anything'] === 1) and cap the batch so + // one request cannot build an unbounded multi-row INSERT. + $rawCopies = $body['copies'] ?? null; + $copiesToAdd = is_int($rawCopies) + ? $rawCopies + : (is_string($rawCopies) && preg_match('/^[1-9]\d*$/D', $rawCopies) === 1 ? (int) $rawCopies : 0); - if ($copiesToAdd < 1) { + if ($copiesToAdd < 1 || $copiesToAdd > 100) { $response->getBody()->write(json_encode([ 'error' => true, 'message' => __('Numero di copie non valido.') @@ -2416,39 +2429,100 @@ return $response->withStatus(404)->withHeader('Content-Type', 'application/json'); } - // Calculate new total - $currentCopieTotali = (int) $book['copie_totali']; - $newCopieTotali = $currentCopieTotali + $copiesToAdd; - - // Update copie_totali counter in libri table - $stmt = $db->prepare('UPDATE libri SET copie_totali = ? WHERE id = ?'); - $stmt->bind_param('ii', $newCopieTotali, $bookId); - $stmt->execute(); - $stmt->close(); - - // Create physical copies in copie table + // Create the copies atomically and let DataIntegrity derive the counters, + // mirroring CopyController::createCopy(). The allocator produces + // collision-free "{base}-C{N}" codes: the previous "-C{copie_totali+i}" + // scheme collided with an existing code as soon as a copy was out of + // circulation (copie_totali excludes those), raising a 1062 that left the + // manually-inflated copie_totali and a partial copy set behind. No manual + // UPDATE, one transaction, rolled back on any failure. $copyRepo = new \App\Models\CopyRepository($db); $baseInventario = !empty($book['numero_inventario']) ? $book['numero_inventario'] : "LIB-{$bookId}"; - // Start from current total + 1 for new copies - for ($i = 1; $i <= $copiesToAdd; $i++) { - $copyNumber = $currentCopieTotali + $i; - $numeroInventario = $newCopieTotali > 1 - ? "{$baseInventario}-C{$copyNumber}" - : $baseInventario; + $reassignmentService = null; + $reservationManager = null; + $transactionStarted = false; + try { + if (!$db->begin_transaction()) { + throw new \RuntimeException('Unable to begin the increase-copies transaction.'); + } + $transactionStarted = true; + + // Canonical lock order: book row first, then copies. + $lock = $db->prepare('SELECT id FROM libri WHERE id = ? AND deleted_at IS NULL FOR UPDATE'); + $lock->bind_param('i', $bookId); + $lock->execute(); + $stillExists = (bool) $lock->get_result()->fetch_assoc(); + $lock->close(); + if (!$stillExists) { + throw new \RuntimeException('Book not found.'); + } + + $createdCopyIds = $copyRepo->createManyForBookWithIds($bookId, $baseInventario, $copiesToAdd, 'disponibile', __('Copia %d di %d')); + if (count($createdCopyIds) !== $copiesToAdd) { + throw new \RuntimeException('Unable to create the requested physical copies.'); + } + + // First repair copy-less/blocked HOLDING assignments, exactly like + // CopyController::createCopy(). Only the capacity left after those + // repairs may promote wait-list rows from prenotazioni. + $reassignmentService = new \App\Services\ReservationReassignmentService($db); + $reassignmentService->setExternalTransaction(true); + foreach ($createdCopyIds as $createdCopyId) { + $reassignmentService->reassignOnNewCopy($bookId, $createdCopyId); + } - $note = "Copia {$copyNumber} di {$newCopieTotali}"; - $copyRepo->create($bookId, $numeroInventario, 'disponibile', $note); + $reservationManager = new \App\Controllers\ReservationManager($db); + $reservationManager->setExternalTransaction(true); + for ($guard = 0; $guard < 1000 && $reservationManager->processBookAvailability($bookId); $guard++) { + // promote the next eligible reservation into a pending loan + } + + $integrity = new \App\Support\DataIntegrity($db); + if (!$integrity->recalculateBookAvailability($bookId, true)) { + throw new \RuntimeException('Unable to recalculate book availability.'); + } + + if (!$db->commit()) { + throw new \RuntimeException('Unable to commit the increase-copies transaction.'); + } + $transactionStarted = false; + } catch (\Throwable $e) { + if ($transactionStarted) { + try { + $db->rollback(); + } catch (\Throwable $rollbackError) { + // best-effort — preserve the original failure + } + } + \App\Support\SecureLogger::error('[increase-copies] failed to add copies', [ + 'book' => $bookId, + 'error' => $e->getMessage(), + ]); + $response->getBody()->write(json_encode([ + 'error' => true, + 'message' => __('Impossibile aggiungere le copie.') + ], JSON_UNESCAPED_UNICODE)); + return $response->withStatus(500)->withHeader('Content-Type', 'application/json'); } - // Recalculate availability using DataIntegrity - $integrity = new \App\Support\DataIntegrity($db); - $integrity->recalculateBookAvailability($bookId); + // Both services defer I/O while the transaction is open. Flush only + // after the assignment/promotion has become durable; notification errors + // must not turn a committed copy batch into an apparent API failure. + try { + $reassignmentService->flushDeferredNotifications(); + $reservationManager->flushDeferredNotifications(); + } catch (\Throwable $e) { + \App\Support\SecureLogger::warning('[increase-copies] deferred notification failed', [ + 'book' => $bookId, + 'error' => $e->getMessage(), + ]); + } - // Get updated availability - $stmt = $db->prepare('SELECT copie_disponibili FROM libri WHERE id = ? AND deleted_at IS NULL'); + // Read the derived counters (post-commit) for the response. + $stmt = $db->prepare('SELECT copie_totali, copie_disponibili FROM libri WHERE id = ? AND deleted_at IS NULL'); $stmt->bind_param('i', $bookId); $stmt->execute(); $result = $stmt->get_result(); @@ -2457,8 +2531,8 @@ $response->getBody()->write(json_encode([ 'success' => true, - 'copie_totali' => $newCopieTotali, - 'copie_disponibili' => (int) $updatedBook['copie_disponibili'], + 'copie_totali' => (int) ($updatedBook['copie_totali'] ?? 0), + 'copie_disponibili' => (int) ($updatedBook['copie_disponibili'] ?? 0), 'added' => $copiesToAdd ], JSON_UNESCAPED_UNICODE)); diff --git a/app/Services/ReservationReassignmentService.php b/app/Services/ReservationReassignmentService.php index 3b61547ad..c9dc21b6a 100644 --- a/app/Services/ReservationReassignmentService.php +++ b/app/Services/ReservationReassignmentService.php @@ -231,10 +231,12 @@ public function reassignOnNewCopy(int $libroId, int $newCopiaId): void $stmt->execute(); $stmt->close(); - // Block the copy for the reserved loan period - $copyRepo = new \App\Models\CopyRepository($this->db); - if (!$copyRepo->updateStatus($newCopiaId, 'prenotato')) { - throw new \RuntimeException("Failed to update copy status for copia_id={$newCopiaId}"); + // Derive the physical-copy state from every commitment instead of + // forcing 'prenotato'. This matters when a copy has another, + // non-overlapping current loan: 'prestato' has priority until that + // loan is returned, while the future hold remains linked correctly. + if (!(new \App\Support\DataIntegrity($this->db))->recalculateBookAvailability($libroId, true)) { + throw new \RuntimeException("Failed to recalculate availability for libro_id={$libroId}"); } // Se la prenotazione aveva una vecchia copia assegnata, dobbiamo verificare @@ -304,11 +306,21 @@ public function reassignOnCopyLost(int $copiaId): void $resStart = (string) $reservation['data_prestito']; $resEnd = (string) $reservation['data_scadenza']; $excludedCopies = [$copiaId]; // Copie da escludere dalla ricerca - $maxRetries = 5; // Limite tentativi per evitare loop infiniti + // The allocator pre-filters overlaps, so retries are only needed when a + // concurrent transaction claims a candidate between lookup and lock. + // A fixed limit of five used to give up even when a sixth physical copy + // was free for the requested period. + $maxRetries = 1000; for ($attempt = 0; $attempt < $maxRetries; $attempt++) { // Cerca un'altra copia disponibile per questo libro - $nextCopyId = $this->findAvailableCopyExcluding($libroId, $excludedCopies); + $nextCopyId = $this->findAvailableCopyExcluding( + $libroId, + $excludedCopies, + $reservationId, + $resStart, + $resEnd + ); if (!$nextCopyId) { // Nessuna copia disponibile @@ -351,8 +363,9 @@ public function reassignOnCopyLost(int $copiaId): void $copyStatus = $stmt->get_result()->fetch_assoc(); $stmt->close(); - // Verifica che la copia sia ancora disponibile (potrebbe essere cambiata) - if (!$copyStatus || !in_array($copyStatus['stato'], ['disponibile', 'prenotato'], true)) { + // A copy currently 'prestato' is a valid target for a disjoint + // future hold. Operationally unavailable states never are. + if (!$copyStatus || in_array($copyStatus['stato'], ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento'], true)) { $this->rollbackIfOwned($ownTransaction); // Aggiungi questa copia alle escluse e riprova $excludedCopies[] = $nextCopyId; @@ -386,10 +399,11 @@ public function reassignOnCopyLost(int $copiaId): void $stmt->execute(); $stmt->close(); - // Block the copy for the reserved loan period - $copyRepo = new \App\Models\CopyRepository($this->db); - if (!$copyRepo->updateStatus($nextCopyId, 'prenotato')) { - throw new \RuntimeException("Failed to update copy status for copia_id={$nextCopyId}"); + // Recompute instead of forcing 'prenotato': if the replacement + // is physically out on a non-overlapping current loan it must + // remain 'prestato' until return. + if (!(new \App\Support\DataIntegrity($this->db))->recalculateBookAvailability($libroId, true)) { + throw new \RuntimeException("Failed to recalculate availability for libro_id={$libroId}"); } $this->commitIfOwned($ownTransaction); @@ -522,27 +536,45 @@ public function reassignOnReturn(int $copiaId): void * @param int $libroId ID del libro * @param array $excludeCopiaIds Array di ID copie da escludere */ - private function findAvailableCopyExcluding(int $libroId, array $excludeCopiaIds): ?int + private function findAvailableCopyExcluding( + int $libroId, + array $excludeCopiaIds, + int $reservationId, + string $startDate, + string $endDate + ): ?int { $sql = " - SELECT id - FROM copie - WHERE libro_id = ? - AND stato IN ('disponibile', 'prenotato') + SELECT c.id + FROM copie c + WHERE c.libro_id = ? + AND c.stato NOT IN ('perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento') + AND NOT EXISTS ( + SELECT 1 + FROM prestiti p + WHERE p.copia_id = c.id + AND p.id <> ? + AND p.data_prestito <= ? + AND (p.stato = 'in_ritardo' OR p.data_scadenza >= ?) + AND ( + (p.attivo = 1 AND p.stato IN ('prenotato','da_ritirare','in_corso','in_ritardo')) + OR (p.attivo = 0 AND p.stato = 'pendente' AND p.copia_id IS NOT NULL) + ) + ) "; - $params = [$libroId]; - $types = "i"; + $params = [$libroId, $reservationId, $endDate, $startDate]; + $types = 'iiss'; if (!empty($excludeCopiaIds)) { $placeholders = implode(',', array_fill(0, count($excludeCopiaIds), '?')); - $sql .= " AND id NOT IN ($placeholders)"; + $sql .= " AND c.id NOT IN ($placeholders)"; foreach ($excludeCopiaIds as $id) { $params[] = $id; $types .= "i"; } } - $sql .= " LIMIT 1"; + $sql .= " ORDER BY c.id ASC LIMIT 1"; $stmt = $this->db->prepare($sql); $stmt->bind_param($types, ...$params); diff --git a/app/Support/ContributorBackfill.php b/app/Support/ContributorBackfill.php index 95152a9a1..e3c2691ec 100644 --- a/app/Support/ContributorBackfill.php +++ b/app/Support/ContributorBackfill.php @@ -38,15 +38,37 @@ final class ContributorBackfill public static function run(mysqli $db): bool { $lockAcquired = false; + $lockName = null; try { + // Schema-scoped advisory-lock name, hashed in PHP so it stays within + // MySQL's 64-char GET_LOCK limit for ANY database name (a raw + // CONCAT('pinakes-contributor-backfill:', DATABASE()) overflows past + // a ~35-char schema name) and does not depend on server-side MD5(). + // Keep this lookup inside the best-effort boundary: run() promises + // to return false, never throw, when the database is unavailable. + $dbNameRes = $db->query('SELECT DATABASE()'); + if (!($dbNameRes instanceof \mysqli_result)) { + throw new \RuntimeException('Unable to resolve the current database for contributor backfill locking'); + } + $dbName = (string) ($dbNameRes->fetch_row()[0] ?? ''); + $dbNameRes->free(); + if ($dbName === '') { + throw new \RuntimeException('Contributor backfill requires a selected database'); + } + $lockName = 'pinakes-cb:' . md5($dbName); + // The updater and maintenance recovery can run concurrently. The // marker alone is not a lock: both processes could resolve the same // new name before either writes it (autori.nome is not unique). - $lockResult = $db->query( - "SELECT GET_LOCK(CONCAT('pinakes-contributor-backfill:', DATABASE()), 30)" - ); - $lockAcquired = $lockResult instanceof \mysqli_result - && (int) ($lockResult->fetch_row()[0] ?? 0) === 1; + $lockStmt = $db->prepare('SELECT GET_LOCK(?, 30)'); + if ($lockStmt !== false) { + $lockStmt->bind_param('s', $lockName); + $lockStmt->execute(); + $lockRow = $lockStmt->get_result(); + $lockAcquired = $lockRow instanceof \mysqli_result + && (int) ($lockRow->fetch_row()[0] ?? 0) === 1; + $lockStmt->close(); + } if (!$lockAcquired) { throw new \RuntimeException('Unable to acquire contributor backfill lock'); } @@ -77,8 +99,28 @@ public static function run(mysqli $db): bool SecureLogger::warning('ContributorBackfill failed: ' . $e->getMessage()); return false; } finally { - if ($lockAcquired) { - $db->query("SELECT RELEASE_LOCK(CONCAT('pinakes-contributor-backfill:', DATABASE()))"); + if ($lockAcquired && $lockName !== null) { + $rel = null; + try { + $rel = $db->prepare('SELECT RELEASE_LOCK(?)'); + if ($rel !== false) { + $rel->bind_param('s', $lockName); + $rel->execute(); + } + } catch (\Throwable $releaseError) { + // A dropped connection releases the lock server-side. This + // cleanup must not turn a best-effort false result into an + // exception or mask a prior backfill error. + SecureLogger::warning('ContributorBackfill lock release failed: ' . $releaseError->getMessage()); + } finally { + if ($rel instanceof \mysqli_stmt) { + try { + $rel->close(); + } catch (\Throwable $closeError) { + // best-effort cleanup on a possibly broken connection + } + } + } } } } diff --git a/app/Support/DataIntegrity.php b/app/Support/DataIntegrity.php index eeefe5c5f..725f54331 100644 --- a/app/Support/DataIntegrity.php +++ b/app/Support/DataIntegrity.php @@ -262,6 +262,8 @@ public function recalculateAllBookAvailabilityBatched(int $chunkSize = 500, ?cal /** * Ricalcola le copie disponibili per un singolo libro * Supports being called inside or outside a transaction + * + * @phpstan-impure Mutates and re-reads circulation state in the database. */ public function recalculateBookAvailability(int $bookId, bool $insideTransaction = false, bool $skipCacheInvalidation = false): bool { // App-timezone "today" (see recalculateAllBookAvailability) — interpolated in place of @@ -1004,25 +1006,15 @@ public function fixDataInconsistencies(): array { try { $this->db->begin_transaction(); - // 1. Ricalcola tutte le copie disponibili - $availabilityResult = $this->recalculateAllBookAvailability(insideTransaction: true); - $results['fixed'] += $availabilityResult['updated']; - $results['errors'] = array_merge($results['errors'], $availabilityResult['errors']); - - // 2. Correggi stati libri attivi basandosi sulle copie correnti - $stmt = $this->db->prepare(" - UPDATE libri SET stato = CASE - WHEN copie_disponibili > 0 THEN 'disponibile' - WHEN EXISTS (SELECT 1 FROM copie c WHERE c.libro_id = libri.id AND c.stato = 'prestato') THEN 'prestato' - WHEN EXISTS (SELECT 1 FROM copie c WHERE c.libro_id = libri.id) THEN 'non_disponibile' - ELSE 'non_disponibile' - END - WHERE stato IN ('disponibile', 'prestato', 'non_disponibile') - AND deleted_at IS NULL - "); - $stmt->execute(); - $results['fixed'] += $this->db->affected_rows; - $stmt->close(); + // Circulation writes use the canonical libri -> prestiti/copie lock + // order. Maintenance changes loans and reservations below, so lock + // every book first and keep those locks until the final canonical + // recalculation. This avoids crossing concurrent web requests. + // CI-SOFT-DELETE-EXEMPT: global maintenance locks restorable rows too. + $lockBooks = $this->db->prepare('SELECT id FROM libri ORDER BY id FOR UPDATE'); + $lockBooks->execute(); + $lockBooks->get_result()->fetch_all(MYSQLI_NUM); + $lockBooks->close(); // 3. Aggiorna prestiti in ritardo $stmt = $this->db->prepare(" @@ -1123,6 +1115,14 @@ public function fixDataInconsistencies(): array { $upd->close(); } + // Recalculate only after every loan/reservation repair. This is the + // sole writer of libri.stato/copie_* and guarantees the committed + // read model describes this transaction's final circulation rows, + // including active reservation occupancy. + $availabilityResult = $this->recalculateAllBookAvailability(insideTransaction: true); + $results['fixed'] += $availabilityResult['updated']; + $results['errors'] = array_merge($results['errors'], $availabilityResult['errors']); + $this->db->commit(); } catch (\Throwable $e) { diff --git a/app/Support/I18n.php b/app/Support/I18n.php index a52108cd1..a73915926 100644 --- a/app/Support/I18n.php +++ b/app/Support/I18n.php @@ -344,6 +344,28 @@ public static function getAvailableLocales(): array return self::$availableLocales; } + /** + * Resolve a stored user preference to an active locale. + * + * NULL, empty and obsolete locale codes inherit the installation default. + */ + public static function resolveUserLocale(mixed $locale): string + { + $availableLocales = self::getAvailableLocales(); + $requested = self::normalizeLocaleCode(is_string($locale) ? trim($locale) : ''); + if ($requested !== '' && isset($availableLocales[$requested])) { + return $requested; + } + + $installationLocale = self::normalizeLocaleCode(self::getInstallationLocale()); + if ($installationLocale !== '' && isset($availableLocales[$installationLocale])) { + return $installationLocale; + } + + $firstAvailable = array_key_first($availableLocales); + return is_string($firstAvailable) ? $firstAvailable : 'it_IT'; + } + /** * Normalize locale codes to canonical format (xx_YY) */ diff --git a/app/Support/LoanEligibility.php b/app/Support/LoanEligibility.php index 1f471af92..15e5714c6 100644 --- a/app/Support/LoanEligibility.php +++ b/app/Support/LoanEligibility.php @@ -32,6 +32,7 @@ class LoanEligibility * * @return string|null Codice errore ('user_not_found', 'user_suspended', * 'card_expired') oppure null se l'utente è idoneo. + * @phpstan-impure Reads mutable database state; repeated calls can differ. */ public static function checkUser(\mysqli $db, int $userId): ?string { @@ -80,4 +81,29 @@ public static function errorMessage(string $code): string return __('Utente non idoneo al prestito'); } } + + /** + * Shared WHERE fragment selecting reservations eligible for promotion, used + * by both the single-book path (ReservationManager::processBookAvailability) + * and the batch path (MaintenanceService::processScheduledReservations) so a + * future edge-case fix cannot drift between them. + * + * Matches a real requested start that has arrived — with a '1000-01-01' + * floor (MySQL's minimum valid DATE) that rejects 0000-00-00 dump rows + * without embedding the literal a NO_ZERO_DATE server refuses at prepare + * time — OR a legacy no-start row while its deadline is still today or in + * the future. + * + * Requires TWO positional placeholders bound in order (today, today). + * $alias is the `prenotazioni` table alias: a fixed literal, never user input. + */ + public static function promotableReservationWhere(string $alias): string + { + return "( + ({$alias}.data_inizio_richiesta >= '1000-01-01' AND {$alias}.data_inizio_richiesta <= ?) + OR ({$alias}.data_inizio_richiesta IS NULL + AND {$alias}.data_scadenza_prenotazione IS NOT NULL + AND DATE({$alias}.data_scadenza_prenotazione) >= ?) + )"; + } } diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index f70d2f379..a9564eb38 100644 --- a/app/Support/MaintenanceService.php +++ b/app/Support/MaintenanceService.php @@ -419,7 +419,9 @@ public function activateScheduledLoans(): int // Recalculate book availability using DataIntegrity for consistency // (da_ritirare counts as "slot occupied" even if copy is available) - $integrity->recalculateBookAvailability((int)$loan['libro_id'], true); + if (!$integrity->recalculateBookAvailability((int) $loan['libro_id'], true)) { + throw new \RuntimeException('Failed to recalculate availability while activating a scheduled loan.'); + } $this->db->commit(); $activatedCount++; @@ -471,7 +473,7 @@ public function processScheduledReservations(): int FROM prenotazioni p JOIN utenti u ON p.utente_id = u.id WHERE p.stato = 'attiva' - AND p.data_inizio_richiesta <= ? + AND " . \App\Support\LoanEligibility::promotableReservationWhere('p') . " ORDER BY p.libro_id, p.queue_position ASC "); @@ -479,7 +481,7 @@ public function processScheduledReservations(): int throw new \RuntimeException('Failed to prepare scheduled reservations query'); } - $stmt->bind_param('s', $today); + $stmt->bind_param('ss', $today, $today); $stmt->execute(); $result = $stmt->get_result(); $reservations = $result ? $result->fetch_all(MYSQLI_ASSOC) : []; @@ -690,7 +692,9 @@ public function checkExpiredReservations(): int } // Recalculate book availability (inside transaction) - $integrity->recalculateBookAvailability($libroId, true); + if (!$integrity->recalculateBookAvailability($libroId, true)) { + throw new \RuntimeException('Failed to recalculate availability after reservation expiry.'); + } $this->db->commit(); $expiredCount++; @@ -872,7 +876,9 @@ public function checkExpiredPickups(): int } // Recalculate book availability (inside transaction) - $integrity->recalculateBookAvailability($libroId, true); + if (!$integrity->recalculateBookAvailability($libroId, true)) { + throw new \RuntimeException('Failed to recalculate availability after pickup expiry.'); + } $this->db->commit(); $expiredCount++; diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 165bee7a4..9fb825f62 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -3349,6 +3349,15 @@ public function runMigrations(string $fromVersion, string $toVersion): array ]; } catch (\Throwable $e) { + // A data-only migration may explicitly START TRANSACTION so all of + // its statements remain atomic. If any statement fails before its + // COMMIT, close that transaction here instead of returning the + // connection to the updater with partial uncommitted state. + try { + $this->db->rollback(); + } catch (\Throwable $rollbackError) { + // Best effort: preserve the migration error that caused the abort. + } $this->debugLog('ERROR', 'Errore durante migrazioni', [ 'error' => $e->getMessage(), 'executed_so_far' => $executed diff --git a/app/Views/libri/partials/book_form.php b/app/Views/libri/partials/book_form.php index 6e7f4f7b6..17ac850d4 100644 --- a/app/Views/libri/partials/book_form.php +++ b/app/Views/libri/partials/book_form.php @@ -319,10 +319,8 @@ - - -

+

@@ -488,13 +486,25 @@
- - - + + /> +

- Puoi ridurre le copie solo se non sono in prestito, perse o danneggiate. + + + +

+ +

@@ -511,8 +521,13 @@
- + +

+ +

@@ -3451,7 +3466,7 @@ function setupEnhancedAutocomplete(inputId, suggestId, fetchUrl, onSelect, onEmp title: __('Copie Aggiunte!'), html: `

${__('Hai aggiunto %s copie a "%s"').replace('%s', copiesToAdd).replace('%s', escapeHtml(book.title))}

-

${__('Copie totali:')}: ${data.copie_totali}

+

${__('Copie in circolazione')}: ${data.copie_totali}

${__('Copie disponibili:')}: ${data.copie_disponibili}

`, confirmButtonText: __('OK') diff --git a/app/Views/libri/scheda_libro.php b/app/Views/libri/scheda_libro.php index 421014f15..385cc8980 100644 --- a/app/Views/libri/scheda_libro.php +++ b/app/Views/libri/scheda_libro.php @@ -908,20 +908,20 @@ class="inline-flex items-center gap-1 px-3 py-2 text-xs font-medium rounded-lg b
- - 0): ?> -
+ + +

- ( 1 ? __("copie") : __("copia") ?>) + ( ) • - : + : • @@ -930,11 +930,19 @@ class="inline-flex items-center gap-1 px-3 py-2 text-xs font-medium rounded-lg b

- - - +
+ + + + + + +
@@ -951,6 +959,15 @@ class="inline-flex items-center gap-2 px-3 py-1.5 bg-gray-800 text-white text-sm + + + + + + + + + @@ -998,16 +1015,20 @@ class="inline-flex items-center gap-2 px-3 py-1.5 bg-gray-800 text-white text-sm 'prestato' => __('Prestato'), 'prenotato' => __('Prenotato'), 'manutenzione' => __('In manutenzione'), + 'in_restauro' => __('In restauro'), 'perso' => __('Perso'), 'danneggiato' => __('Danneggiato'), + 'in_trasferimento' => __('In trasferimento'), ]; $copiaStatusClasses = [ 'disponibile' => 'bg-green-100 text-green-800', 'prestato' => 'bg-red-100 text-red-800', 'prenotato' => 'bg-purple-100 text-purple-800', 'manutenzione' => 'bg-yellow-100 text-yellow-800', + 'in_restauro' => 'bg-indigo-100 text-indigo-800', 'perso' => 'bg-gray-100 text-gray-800', 'danneggiato' => 'bg-orange-100 text-orange-800', + 'in_trasferimento' => 'bg-blue-100 text-blue-800', ]; $effectiveLabel = $copiaStatusLabels[$effectiveStatus] ?? ucfirst($effectiveStatus); $effectiveClass = $copiaStatusClasses[$effectiveStatus] ?? 'bg-gray-100 text-gray-800'; @@ -1078,7 +1099,7 @@ class="inline-flex items-center gap-2 px-3 py-1.5 bg-gray-800 text-white text-sm $loanStatusVal = $copia['prestito_stato'] ?? null; $bookAtLibrary = in_array($loanStatusVal, ['da_ritirare', 'prenotato'], true); $canEdit = empty($copia['prestito_id']) || $bookAtLibrary; - $canDelete = $canEdit && in_array($rawCopiaStatus, ['perso', 'danneggiato', 'manutenzione']); + $canDelete = $canEdit && in_array($rawCopiaStatus, ['perso', 'danneggiato', 'manutenzione', 'in_restauro', 'in_trasferimento'], true); ?>
- @@ -1844,6 +1864,8 @@ function confirmRenewal(e){ + +

@@ -1866,6 +1888,130 @@ function confirmRenewal(e){

+ + + + +