diff --git a/app/Controllers/CopyController.php b/app/Controllers/CopyController.php index 90bccb658..b034525d6 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,273 @@ 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 nuova copia fisica 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'] ?? ''; + 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); + } + $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); + } } - $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}"; + $newCopyId = $repo->createWithAllocatedInventoryCode( + $bookId, + $base, + $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); + } - // 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); + if ($newCopyId <= 0) { + throw new \RuntimeException('Unable to create the 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); + $reassignmentService->reassignOnNewCopy($bookId, $newCopyId); + + $reservationManager = new ReservationManager($db); + $reservationManager->setExternalTransaction(true); + $reservationManager->processBookAvailability($bookId); + } + + $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.') + : __('Impossibile aggiungere la copia.'); + 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'] = __('Copia aggiunta con successo.'); + 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(); - } catch (\mysqli_sql_exception $e) { - // 1451 = Cannot delete or update a parent row (vincolo FK) - if ((int) $e->getCode() !== 1451) { - throw $e; + 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); } - $_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); + $stmt = $db->prepare('DELETE FROM copie WHERE id = ?'); + $stmt->bind_param('i', $copyId); + $stmt->execute(); + $deleted = $stmt->affected_rows; + $stmt->close(); + if ($deleted !== 1) { + throw new \RuntimeException('Physical copy delete did not affect exactly one row.'); + } + + 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/LibriController.php b/app/Controllers/LibriController.php index 1987fe55b..4f6c16d5e 100644 --- a/app/Controllers/LibriController.php +++ b/app/Controllers/LibriController.php @@ -842,11 +842,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 +868,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']) @@ -1176,7 +1189,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,31 +1275,6 @@ 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). @@ -1448,53 +1510,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']); @@ -1858,79 +1882,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); 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/ReservationManager.php b/app/Controllers/ReservationManager.php index ed5b1bf88..44b252ed2 100644 --- a/app/Controllers/ReservationManager.php +++ b/app/Controllers/ReservationManager.php @@ -189,11 +189,26 @@ 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 ( + -- Real requested start that has arrived. '1000-01-01' is + -- MySQL's minimum valid DATE: the floor rejects 0000-00-00 + -- dump rows (which would otherwise be promoted back-dated) + -- without embedding the literal '0000-00-00', which a + -- NO_ZERO_DATE server refuses even at prepare time. + (r.data_inizio_richiesta >= '1000-01-01' AND r.data_inizio_richiesta <= ?) + -- Legacy row with no requested start: promote only while the + -- reservation deadline is still valid (today or future). An + -- already-expired deadline must be left for the expiry / + -- cancellation path, never converted into a loan back-dated + -- to the past. + OR (r.data_inizio_richiesta IS NULL + AND r.data_scadenza_prenotazione IS NOT NULL + AND DATE(r.data_scadenza_prenotazione) >= ?) + ) 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 +219,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 +267,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 +382,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 +477,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 +910,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/Models/CopyRepository.php b/app/Models/CopyRepository.php index 1b6205417..6f1d38567 100644 --- a/app/Models/CopyRepository.php +++ b/app/Models/CopyRepository.php @@ -171,29 +171,199 @@ 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']; + } + + /** + * @return array{count: int, ids: list} + */ + private function createManyForBookResult( + int $bookId, + string $base, + int $howMany, + string $stato, + ?string $noteTemplate, + bool $resolveIds + ): 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 = 'pinakes-copy-inventory-allocation'; + $this->acquireInventoryAllocatorLock($lockName); + try { + $inserted = $this->insertAllocatedCopiesWhileLocked( + $bookId, + $base, + $howMany, + $stato, + $noteTemplate, + null + ); + $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. Both book creation and the single-copy form 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 $singleNote + ): 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 = $total === 1 && $singleNote !== null + ? $singleNote + : ($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 = 'pinakes-copy-inventory-allocation'; + + $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. + $existingCount = 0; + $sel = $this->db->prepare( + 'SELECT numero_inventario FROM copie WHERE numero_inventario LIKE ? FOR UPDATE' + ); if ($sel === false) { throw new \RuntimeException('Unable to prepare inventory-code lookup: ' . $this->db->error); } @@ -204,47 +374,147 @@ public function createManyForBook(int $bookId, string $base, int $howMany, strin throw new \RuntimeException('Unable to load inventory codes: ' . $error); } $res = $sel->get_result(); - while ($row = $res->fetch_assoc()) { - $taken[$row['numero_inventario']] = true; + while ($res->fetch_assoc()) { + $existingCount++; } $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; + } } - $stmt->bind_param($types, ...$params); + return $missing; + } + + /** + * @param list $codes + * @return list + */ + private function copyIdsForInventoryCodes(array $codes): array + { + if ($codes === []) { + return []; + } + $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; + } - return $total; + private function acquireInventoryAllocatorLock(string $lockName): void + { + $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..4e61d60cf 100644 --- a/app/Models/LoanRepository.php +++ b/app/Models/LoanRepository.php @@ -277,7 +277,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/DataIntegrity.php b/app/Support/DataIntegrity.php index eeefe5c5f..0b3fcac12 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 diff --git a/app/Support/LoanEligibility.php b/app/Support/LoanEligibility.php index 1f471af92..e584d1431 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 { diff --git a/app/Support/MaintenanceService.php b/app/Support/MaintenanceService.php index f70d2f379..7ea319c48 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,22 @@ 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 ( + -- Real requested start that has arrived. '1000-01-01' is MySQL's + -- minimum valid DATE: the floor rejects 0000-00-00 dump rows + -- (which would otherwise be promoted back-dated) without + -- embedding the literal '0000-00-00', which a NO_ZERO_DATE + -- server refuses even at prepare time. + (p.data_inizio_richiesta >= '1000-01-01' AND p.data_inizio_richiesta <= ?) + -- Legacy row with no requested start: promote only while the + -- reservation deadline is still valid (today or future). An + -- already-expired deadline must be left for the expiry / + -- cancellation path, never converted into a loan back-dated to + -- the past. + OR (p.data_inizio_richiesta IS NULL + AND p.data_scadenza_prenotazione IS NOT NULL + AND DATE(p.data_scadenza_prenotazione) >= ?) + ) ORDER BY p.libro_id, p.queue_position ASC "); @@ -479,7 +496,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 +707,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 +891,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/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..b2b99c67a 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,78 @@ function confirmRenewal(e){

+ + + + +