From 144a93c5ede5c87af2cbc89b8f5fe54dd33207ec Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 6 Oct 2026 20:44:33 +0200 Subject: [PATCH 1/7] Give each article an admin page, and wait out a proxy that drops the update (0.7.94) #453, #454: in the admin an article had no page of its own. The quick search, the Articles list and the author page all opened its form, and saving returned to the form, so a save looked as if nothing had happened. /admin/periodicals/articles/{id} is now the article's page, laid out like the admin book page: cover, authors, publication, masthead and issue, genre with its path, keywords and the rest of the record, the PDF and the exports, with Edit and Delete as buttons. The form moves to /{id}/edit, saving returns to the page, and the Articles list gains View and Edit icons. The admin menu now highlights only the most specific entry, so Periodicals no longer lights up next to Articles. #450: behind a reverse proxy (here a NAS's remote access) the install request got a 502 after a minute while PHP finished the update, and the page reported a failure. On a gateway error or a dropped connection the page now polls GET /admin/updates/status (installed version, whether the update lock is held, the latest logged attempt) until the update is done, and reports the outcome the server logged. The install request releases the PHP session before the update runs, so the polls are not held behind it. Emeroteca 1.12.1, no migration. --- CHANGELOG.md | 12 + README.md | 8 +- app/Controllers/UpdateController.php | 36 +++ app/Routes/web.php | 8 + app/Support/Updater.php | 59 ++++ app/Views/admin/updates.php | 124 ++++++-- app/Views/autori/scheda_autore.php | 7 +- app/Views/layout.php | 19 +- locale/da_DK.json | 6 +- locale/de_DE.json | 6 +- locale/en_US.json | 6 +- locale/fr_FR.json | 6 +- locale/it_IT.json | 6 +- storage/plugins/emeroteca/EmerotecaPlugin.php | 2 +- storage/plugins/emeroteca/plugin.json | 4 +- .../Controllers/ContributionController.php | 19 ++ .../emeroteca/src/Views/article-form.php | 6 +- .../emeroteca/src/Views/article-show.php | 288 ++++++++++++++++++ .../plugins/emeroteca/src/Views/articles.php | 15 +- .../emeroteca/src/Views/public/article.php | 2 +- tests/emeroteca-412-upgrade.spec.js | 2 +- tests/emeroteca-412.spec.js | 11 +- tests/emeroteca-analytic-record.spec.js | 16 +- tests/emeroteca-articles-453-455.spec.js | 55 +++- tests/update-proxy-drop-450.spec.js | 141 +++++++++ tests/update-status-450.unit.php | 132 ++++++++ tests/uwe-412-followup.spec.js | 2 +- version.json | 2 +- 28 files changed, 938 insertions(+), 62 deletions(-) create mode 100644 storage/plugins/emeroteca/src/Views/article-show.php create mode 100644 tests/update-proxy-drop-450.spec.js create mode 100644 tests/update-status-450.unit.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 979cb4974..e7c13d9c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ Full version-by-version history for Pinakes. The README shows only the latest release; everything older lives here. +## [0.7.94] + +### Added +- **An admin page for each article** ([#453](https://github.com/fabiodalez-dev/Pinakes/issues/453), [#454](https://github.com/fabiodalez-dev/Pinakes/issues/454)), laid out like the admin book page. It shows the cover, authors, publication, masthead and issue, genre with its path, keywords and the rest of the record, the PDF and the RIS and MARCXML exports, with Edit, Delete and the public page as buttons. `/admin/periodicals/articles/{id}` is now this page and the form moved to `/admin/periodicals/articles/{id}/edit`. The quick search, the Articles list (which gains View and Edit icons) and the Details button on the author page open it, and saving the form returns to it. Before, every link opened the form, so saving looked as if nothing had happened. +- **`GET /admin/updates/status`** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)): the installed version, whether the update lock is held, and the latest update attempt with its status and error. Admin only, never cached, reachable during maintenance. + +### Fixed +- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports the attempt the server logged: success, or the error the update recorded. The install request releases the PHP session before the update runs, so those polls are not held behind it. +- **Only the most specific admin menu entry is highlighted.** On an article page both Periodicals and Articles lit up, because the address of one starts with the other's. + +The Emeroteca plugin goes to 1.12.1. No migration. + ## [0.7.93] ### Added diff --git a/README.md b/README.md index 462ae9446..1b774de02 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,13 @@ 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.93 — latest +### v0.7.94 — latest + +**An article has its own page in the admin** ([#453](https://github.com/fabiodalez-dev/Pinakes/issues/453), [#454](https://github.com/fabiodalez-dev/Pinakes/issues/454)). Like a book, it opens on a page that shows the record (cover, authors, publication, genre, keywords, PDF and exports) with Edit and Delete as buttons, instead of opening straight into its form. The quick search, the Articles list and the author page lead there, and saving the form brings you back to it. + +**An update behind a proxy reports what really happened** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). A reverse proxy, such as a NAS's remote access, can give up on the install request after a minute while the server carries the update to its end. The update page then waits for the server and shows the real outcome, instead of an error for an update that succeeded. No migration. + +### v0.7.93 **Articles are found, listed and edited the way books are** ([#453](https://github.com/fabiodalez-dev/Pinakes/issues/453), [#454](https://github.com/fabiodalez-dev/Pinakes/issues/454), [#455](https://github.com/fabiodalez-dev/Pinakes/issues/455)). The admin menu has an Articles entry, and the quick search at the top of the admin finds articles (opening their form) and periodicals. On an author's page the articles are cards with an image, Details and Edit, like the books. The public article page has an Edit button for staff, shows the genre, and lists the author's other works, books included. In the article form, keywords and genre sit in the advanced description, and the genre puts the article in the catalogue filtered by that genre. The Emeroteca plugin 1.12.0 adds the genre column to articles, applied on its own when the plugin updates. diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index 6241dc067..d842e8184 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -837,6 +837,16 @@ public function installManualUpdate(Request $request, Response $response, mysqli ], 400); } + // Release the session before the long part. PHP holds the session + // file locked for the whole request, so the page's status polls + // (status(), same session) would wait for the install to end: and + // when a proxy has dropped this request (#450) they are how the page + // learns the outcome. The package keys removed above are written now; + // nothing below writes to the session. + if (session_status() === PHP_SESSION_ACTIVE) { + session_write_close(); + } + // Perform the update from uploaded file (use resolved path to prevent TOCTOU) $this->answerJsonOnFatal(); $result = $updater->performUpdateFromFile($realTempPath); @@ -855,6 +865,32 @@ public function installManualUpdate(Request $request, Response $response, mysqli ], 500); } + /** + * API: where an update stands, for the page whose install request a proxy + * cut short (#450). The update itself carries on in PHP after the proxy + * gives up, so the page asks here until it ends: the installed version, + * whether the update lock is still held, and the latest attempt logged. + * Reachable during maintenance (index.php allows /admin/updates) and + * read-only, so AdminAuthMiddleware is enough. + */ + public function status(Request $request, Response $response, mysqli $db): Response + { + try { + $updater = new Updater($db); + } catch (\Throwable $e) { + return $this->jsonResponse($response, [ + 'success' => false, + 'error' => $this->updaterUnavailable($e, 'status'), + ], 503); + } + return $this->jsonResponse($response, [ + 'success' => true, + 'version' => $updater->getCurrentVersion(), + 'running' => $updater->isUpdateRunning(), + 'last' => $updater->lastUpdateAttempt(), + ])->withHeader('Cache-Control', 'no-store'); + } + /** * API: Save GitHub API token */ diff --git a/app/Routes/web.php b/app/Routes/web.php index d9ab53a00..fbfa77518 100644 --- a/app/Routes/web.php +++ b/app/Routes/web.php @@ -3573,6 +3573,14 @@ return $controller->clearMaintenance($request, $response); })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware()); + // Where an update stands: polled by the page when a proxy dropped its + // install request while the server carried on (#450) + $app->get('/admin/updates/status', function ($request, $response) use ($app) { + $db = $app->getContainer()->get('db'); + $controller = new \App\Controllers\UpdateController(); + return $controller->status($request, $response, $db); + })->add(new AdminAuthMiddleware()); + // View updater logs (for debugging) $app->get('/admin/updates/logs', function ($request, $response) { $controller = new \App\Controllers\UpdateController(); diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 2ebf9b345..7cf708218 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -4381,6 +4381,65 @@ private function logUpdateComplete(int $logId, bool $success, ?string $error = n } } + /** + * Whether an update is running right now: some request holds the update + * lock. The page asks when the proxy in front of the site dropped its + * install request (#450) while PHP, with ignore_user_abort, carried on. + * A shared, non-blocking probe: it never waits and never takes the lock + * from the update; a lock it cannot even open counts as not running. + */ + public function isUpdateRunning(): bool + { + $lockFile = $this->rootPath . '/storage/cache/update.lock'; + if (!is_file($lockFile)) { + return false; + } + $handle = @fopen($lockFile, 'r'); + if ($handle === false) { + return false; + } + try { + if (flock($handle, LOCK_SH | LOCK_NB)) { + flock($handle, LOCK_UN); + return false; + } + return true; + } finally { + fclose($handle); + } + } + + /** + * The latest update attempt in update_logs (backups excluded), or null + * when there is none or the table is missing. + * + * @return array{id:int,to_version:string,status:string,error:string}|null + */ + public function lastUpdateAttempt(): ?array + { + try { + $tableCheck = $this->db->query("SHOW TABLES LIKE 'update_logs'"); + if ($tableCheck === false || $tableCheck->num_rows === 0) { + return null; + } + $tableCheck->free(); + $result = $this->db->query("SELECT id, to_version, status, error_message FROM update_logs WHERE to_version <> 'backup' ORDER BY id DESC LIMIT 1"); + $row = $result instanceof \mysqli_result ? $result->fetch_assoc() : null; + if (!is_array($row)) { + return null; + } + return [ + 'id' => (int) $row['id'], + 'to_version' => (string) $row['to_version'], + 'status' => (string) $row['status'], + 'error' => (string) ($row['error_message'] ?? ''), + ]; + } catch (\Throwable $e) { + $this->debugLog('WARNING', 'Lettura ultimo aggiornamento fallita', ['error' => $e->getMessage()]); + return null; + } + } + /** * Get update history * @return array diff --git a/app/Views/admin/updates.php b/app/Views/admin/updates.php index 7fa458b2d..73dfca39c 100644 --- a/app/Views/admin/updates.php +++ b/app/Views/admin/updates.php @@ -786,18 +786,13 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text failedStep = 'backup'; setStepActive('backup'); - const response = await fetch(window.BASE_PATH + '/admin/updates/install-manual', { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded', 'Accept': 'application/json' }, - body: `csrf_token=${encodeURIComponent(csrfToken)}&package=${encodeURIComponent(downloadData.package || '')}` - }); - - // Check for maintenance mode before parsing response - if (response.status === 503) { - throw new Error(); - } - - const data = await readUpdateJson(response); + const data = await runInstallRequest( + `csrf_token=${encodeURIComponent(csrfToken)}&package=${encodeURIComponent(downloadData.package || '')}`, + version, + () => { + document.getElementById('updateMessage').textContent = ; + } + ); if (data.success) { setStepComplete('backup'); @@ -850,6 +845,90 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text throw new Error( + ` (HTTP ${response.status}${excerpt ? ': ' + excerpt : ''})`); } +/** + * The install request (#450). It runs backup, files and migrations in one go, + * and a proxy in front of the site (Apache mod_proxy, a NAS's remote access, + * Cloudflare) may give up on it after a minute with a 502 or 504, or drop the + * connection, while PHP carries on to the end. In those cases the outcome is + * read from /admin/updates/status instead of being reported as a failure. + * `onWait` is told once that the page is now waiting for the server. + */ +async function runInstallRequest(body, targetVersion, onWait) { + const before = await fetchUpdateStatus(); + let response; + try { + response = await fetch(window.BASE_PATH + '/admin/updates/install-manual', { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded', 'Accept': 'application/json' }, + body + }); + } catch (networkError) { + return waitForUpdateOutcome(before, targetVersion, onWait); + } + const contentType = response.headers.get('content-type') || ''; + if (!contentType.includes('application/json') && [502, 503, 504, 520, 522, 524].includes(response.status)) { + return waitForUpdateOutcome(before, targetVersion, onWait); + } + return readUpdateJson(response); +} + +/** The update status, or null when it cannot be read right now. */ +async function fetchUpdateStatus() { + try { + const response = await fetch(window.BASE_PATH + '/admin/updates/status', { + headers: { 'Accept': 'application/json' }, + cache: 'no-store' + }); + const contentType = response.headers.get('content-type') || ''; + if (!response.ok || !contentType.includes('application/json')) { + return null; + } + const data = await response.json(); + return data && data.success ? data : null; + } catch (e) { + return null; + } +} + +/** + * Poll until the server is done with the update, then answer as the install + * request would have: {success, message} or {success: false, error}. + * Done means the update lock is free. The attempt logged after `before` + * decides; without one, the installed version does. + */ +async function waitForUpdateOutcome(before, targetVersion, onWait) { + if (typeof onWait === 'function') { + onWait(); + } + // Without a reading from before the request, an older attempt could pass + // for this one: then only the installed version decides. + const trustLog = before !== null; + const beforeId = before && before.last ? before.last.id : 0; + const deadline = Date.now() + 30 * 60 * 1000; + let idlePolls = 0; + while (Date.now() < deadline) { + await sleep(5000); + const status = await fetchUpdateStatus(); + if (!status || status.running) { + idlePolls = 0; + continue; + } + const last = trustLog && status.last && status.last.id > beforeId ? status.last : null; + if ((last && last.status === 'completed') || (targetVersion && status.version === targetVersion)) { + return { success: true, message: }; + } + if (last && (last.status === 'failed' || last.status === 'rolled_back')) { + return { success: false, error: last.error || }; + } + // Not running and nothing settled: give the update a moment to take + // the lock or write its log, then call it. + if (++idlePolls >= 3) { + return { success: false, error: }; + } + } + return { success: false, error: }; +} + function setStepActive(step) { const el = document.querySelector(`[data-step="${step}"]`); if (el) { @@ -1546,16 +1625,17 @@ function removeUploadedFile() { didOpen: () => Swal.showLoading() }); - const installResponse = await fetch(window.BASE_PATH + '/admin/updates/install-manual', { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - 'Accept': 'application/json', - }, - body: `csrf_token=${encodeURIComponent(csrfToken)}&package=${encodeURIComponent(uploadData.package || '')}` - }); - - const installData = await readUpdateJson(installResponse); + const installData = await runInstallRequest( + `csrf_token=${encodeURIComponent(csrfToken)}&package=${encodeURIComponent(uploadData.package || '')}`, + '', + () => { + Swal.update({ + title: , + html: `

${}

` + }); + Swal.showLoading(); + } + ); if (installData.success) { Swal.fire({ diff --git a/app/Views/autori/scheda_autore.php b/app/Views/autori/scheda_autore.php index c6c892775..54ec089bd 100644 --- a/app/Views/autori/scheda_autore.php +++ b/app/Views/autori/scheda_autore.php @@ -338,9 +338,10 @@ class=" justify-center"> if (empty($isStaff) && empty($articolo['pubblico'])) { continue; } $articleId = (int) $articolo['id']; $articlePublicUrl = url('/emeroteca/articolo/' . $articleId); - $articleEditUrl = url('/admin/periodicals/articles/' . $articleId); - // An unpublished article has no public page: its details are the form. - $articleDetailsUrl = !empty($articolo['pubblico']) ? $articlePublicUrl : $articleEditUrl; + $articleEditUrl = url('/admin/periodicals/articles/' . $articleId . '/edit'); + // Staff read the article on its admin page, as they do a book; + // anyone else only ever sees a published one, on its public page. + $articleDetailsUrl = !empty($isStaff) ? url('/admin/periodicals/articles/' . $articleId) : $articlePublicUrl; $cover = (string) ($articolo['cover_url'] ?? ''); if ($cover !== '' && strncmp($cover, 'uploads/', 8) === 0) { $cover = '/' . $cover; } if ($cover === '') { $cover = '/uploads/copertine/placeholder.jpg'; } diff --git a/app/Views/layout.php b/app/Views/layout.php index 74b88fe3f..8f45a9e9c 100644 --- a/app/Views/layout.php +++ b/app/Views/layout.php @@ -1805,13 +1805,26 @@ function initializeActiveNavigation() { const basePath = window.BASE_PATH || ''; const navLinks = document.querySelectorAll('.nav-link'); + // Only the most specific entry lights up: on /admin/periodicals/articles + // both "Emeroteca" (/admin/periodicals) and "Articoli" match as prefixes, + // and the page belongs to the longer one. Entries sharing that href (the + // same page linked twice) all light up. + let bestLength = 0; + const matches = []; navLinks.forEach(link => { const href = link.getAttribute('href'); - if (currentPath.startsWith(href) && href !== '/' && href !== basePath && href !== basePath + '/') { - link.classList.add('bg-rose-50', 'text-rose-700', 'border-r-2', 'border-rose-700'); - link.classList.remove('text-gray-700'); + if (href && currentPath.startsWith(href) && href !== '/' && href !== basePath && href !== basePath + '/') { + matches.push(link); + bestLength = Math.max(bestLength, href.length); } }); + matches.forEach(link => { + if (link.getAttribute('href').length !== bestLength) { + return; + } + link.classList.add('bg-rose-50', 'text-rose-700', 'border-r-2', 'border-rose-700'); + link.classList.remove('text-gray-700'); + }); } diff --git a/locale/da_DK.json b/locale/da_DK.json index 51eda09b1..0b108d2cb 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -8052,5 +8052,9 @@ "Pubblicazione: %s": "Publikation: %s", "Gli stessi generi dei libri: l’articolo compare nel catalogo filtrato per quel genere.": "De samme genrer som bøgerne: artiklen vises i kataloget filtreret efter den genre.", "Altre opere di %s": "Andre værker af %s", - "Cerca libri, articoli, periodici, autori, editori, utenti...": "Søg i bøger, artikler, tidsskrifter, forfattere, forlag, brugere..." + "Cerca libri, articoli, periodici, autori, editori, utenti...": "Søg i bøger, artikler, tidsskrifter, forfattere, forlag, brugere...", + "File e citazione": "Filer og citation", + "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "Forbindelsen blev afbrudt, men opdateringen fortsætter på serveren. Venter på, at den bliver færdig…", + "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "Opdateringen ser ikke ud til at være fuldført. Se loggene for detaljer.", + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Opdateringen kører stadig på serveren efter 30 minutter. Genindlæs siden senere for at se den installerede version." } diff --git a/locale/de_DE.json b/locale/de_DE.json index 2978c7b42..89089bacc 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -8052,5 +8052,9 @@ "Pubblicazione: %s": "Publikation: %s", "Gli stessi generi dei libri: l’articolo compare nel catalogo filtrato per quel genere.": "Dieselben Genres wie bei den Büchern: Der Artikel erscheint im nach diesem Genre gefilterten Katalog.", "Altre opere di %s": "Weitere Werke von %s", - "Cerca libri, articoli, periodici, autori, editori, utenti...": "Bücher, Artikel, Zeitschriften, Autoren, Verlage, Benutzer suchen..." + "Cerca libri, articoli, periodici, autori, editori, utenti...": "Bücher, Artikel, Zeitschriften, Autoren, Verlage, Benutzer suchen...", + "File e citazione": "Dateien und Zitation", + "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "Die Verbindung wurde unterbrochen, aber das Update läuft auf dem Server weiter. Es wird gewartet, bis es abgeschlossen ist…", + "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "Das Update scheint nicht abgeschlossen zu sein. Prüfen Sie die Protokolle für Details.", + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Das Update läuft nach 30 Minuten noch auf dem Server. Laden Sie die Seite später neu, um die installierte Version zu sehen." } diff --git a/locale/en_US.json b/locale/en_US.json index 8dac22820..b35aad43c 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -8052,5 +8052,9 @@ "Pubblicazione: %s": "Publication: %s", "Gli stessi generi dei libri: l’articolo compare nel catalogo filtrato per quel genere.": "The same genres as the books: the article appears in the catalogue filtered by that genre.", "Altre opere di %s": "Other works by %s", - "Cerca libri, articoli, periodici, autori, editori, utenti...": "Search books, articles, periodicals, authors, publishers, users..." + "Cerca libri, articoli, periodici, autori, editori, utenti...": "Search books, articles, periodicals, authors, publishers, users...", + "File e citazione": "Files and citation", + "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "The connection dropped, but the update is still running on the server. Waiting for it to finish…", + "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "The update does not appear to have completed. Check the logs for details.", + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "The update is still running on the server after 30 minutes. Reload the page later to see the installed version." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index c4820c9fd..55c709d42 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -8052,5 +8052,9 @@ "Pubblicazione: %s": "Publication : %s", "Gli stessi generi dei libri: l’articolo compare nel catalogo filtrato per quel genere.": "Les mêmes genres que pour les livres : l’article apparaît dans le catalogue filtré par ce genre.", "Altre opere di %s": "Autres œuvres de %s", - "Cerca libri, articoli, periodici, autori, editori, utenti...": "Rechercher livres, articles, périodiques, auteurs, éditeurs, utilisateurs..." + "Cerca libri, articoli, periodici, autori, editori, utenti...": "Rechercher livres, articles, périodiques, auteurs, éditeurs, utilisateurs...", + "File e citazione": "Fichiers et citation", + "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "La connexion a été interrompue, mais la mise à jour se poursuit sur le serveur. Attente de la fin…", + "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "La mise à jour ne semble pas terminée. Consultez les journaux pour plus de détails.", + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "La mise à jour est toujours en cours sur le serveur après 30 minutes. Rechargez la page plus tard pour voir la version installée." } diff --git a/locale/it_IT.json b/locale/it_IT.json index cee3df172..5469cbb43 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -8052,5 +8052,9 @@ "Pubblicazione: %s": "Pubblicazione: %s", "Gli stessi generi dei libri: l’articolo compare nel catalogo filtrato per quel genere.": "Gli stessi generi dei libri: l’articolo compare nel catalogo filtrato per quel genere.", "Altre opere di %s": "Altre opere di %s", - "Cerca libri, articoli, periodici, autori, editori, utenti...": "Cerca libri, articoli, periodici, autori, editori, utenti..." + "Cerca libri, articoli, periodici, autori, editori, utenti...": "Cerca libri, articoli, periodici, autori, editori, utenti...", + "File e citazione": "File e citazione", + "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…", + "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "L'aggiornamento non risulta completato. Controlla i log per i dettagli.", + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata." } diff --git a/storage/plugins/emeroteca/EmerotecaPlugin.php b/storage/plugins/emeroteca/EmerotecaPlugin.php index a41b32081..190dd9985 100644 --- a/storage/plugins/emeroteca/EmerotecaPlugin.php +++ b/storage/plugins/emeroteca/EmerotecaPlugin.php @@ -1691,7 +1691,7 @@ public function registerRoutes($app): void $public = 'App\\Plugins\\Emeroteca\\Controllers\\PublicController'; $articles = 'App\\Plugins\\Emeroteca\\Controllers\\ContributionController'; - foreach (['' => 'index', '/create' => 'form', '/{id:[0-9]+}' => 'form', '/import' => 'importForm', '/export' => 'export', '/issues' => 'issueOptions', '/{id:[0-9]+}/pdf' => 'pdf', '/{id:[0-9]+}/citation.ris' => 'ris', '/{id:[0-9]+}/marc.xml' => 'marcXml'] as $path => $method) { + foreach (['' => 'index', '/create' => 'form', '/{id:[0-9]+}' => 'show', '/{id:[0-9]+}/edit' => 'form', '/import' => 'importForm', '/export' => 'export', '/issues' => 'issueOptions', '/{id:[0-9]+}/pdf' => 'pdf', '/{id:[0-9]+}/citation.ris' => 'ris', '/{id:[0-9]+}/marc.xml' => 'marcXml'] as $path => $method) { $app->get('/admin/periodicals/articles' . $path, function ($rq, $rs, $args) use ($plugin, $articles, $method) { return $plugin->dispatch($articles, $method, $rq, $rs, $args); })->add($adminMiddleware); diff --git a/storage/plugins/emeroteca/plugin.json b/storage/plugins/emeroteca/plugin.json index 6e414e114..094fc8b26 100644 --- a/storage/plugins/emeroteca/plugin.json +++ b/storage/plugins/emeroteca/plugin.json @@ -2,8 +2,8 @@ "name": "emeroteca", "display_name": "Emeroteca", "description": "Gestione di riviste, giornali e periodici (emeroteca): testate con dati bibliografici (ISSN, editore, periodicità, storia della testata), annate rilegate o sciolte, fascicoli con stato di possesso e kardex dei fascicoli attesi, spoglio degli articoli con ricerca full-text. Sezione pubblica /emeroteca per la consultazione di testate e fascicoli.", - "version": "1.12.0", - "changelog": "v1.12.0: voce Articoli nel menu di amministrazione; la ricerca rapida del pannello trova articoli (anche non pubblicati, aperti in modifica) e testate; nella scheda autore gli articoli sono schede con immagine, Dettagli e Modifica come i libri; la scheda pubblica dell’articolo ha il pulsante Modifica per lo staff, il genere e «Altre opere» dell’autore, libri compresi; nel modulo parole chiave e genere stanno nella descrizione bibliografica avanzata, e il genere fa trovare l’articolo nel catalogo filtrato per genere. Migrazione additiva idempotente (emeroteca_contributi.genere_id con chiave esterna verso generi). v1.11.0: gli articoli compaiono nei suggerimenti della ricerca mentre si scrive, accanto ai libri; nella scheda dell'articolo la testata associata si sceglie cercandola per titolo, e titolo e ISSN della testata riempiono i campi ancora vuoti; l'export MARCXML riporta il codice paese MARC in 008/15-17, il PDF pubblico in 856 e gli autori in ordine diretto in 245 $c; il RIS nomina la rivista anche in JF. Nessuna migrazione. v1.10.0: gli articoli possono essere capitoli di un'antologia, con curatori, editore, luogo e ISBN del volume (record_type book_chapter nel CSV, genre=bookitem in OpenURL e COinS); classificazione secondo uno schema non in elenco, indicato per nome; finestra Cita con APA, Chicago, MLA e Harvard ed export RIS; codici lingua e paese scelti da un elenco. v1.9.0: pagine pubbliche ridisegnate come il catalogo e la scheda libro — /emeroteca con filtri laterali (tipologia, editore, argomento, iniziale) e paginazione; testata con annate, fascicoli e articoli paginati; fascicolo con gli articoli collegati come schede e il sommario; articolo con percorso completo testata › fascicolo, collocazione, articolo precedente e successivo e articoli correlati. Un articolo senza immagine propria mostra la copertina del fascicolo, altrimenti il logo della testata. Nessuna migrazione. v1.8.0: autori degli articoli collegati alla stessa anagrafica dei libri, con selezione per nome e date, creazione esplicita, gestione di omonimi, rinomina e fusione; identificativo GND condiviso ed export MARCXML 100/700 $0. I crediti preesistenti restano disponibili e richiedono associazione esplicita. v1.7.0: gli articoli autonomi diventano schede analitiche complete — sottotitolo, lingua e paese come codici ISO, classificazione con il suo schema (DK5, DDC, UDC, LCC, RVK), nota di possesso e risorsa elettronica con testo del collegamento e condizioni di accesso (danMARC2/MARC 21 856). Citazione pronta in APA 7 e Harvard con pulsante di copia, e download RIS per EndNote, Mendeley e Zotero. Tutti i campi nuovi sono facoltativi: chi non cataloga articoli non li incontra mai. Migrazione additiva idempotente. v1.6.0: gli articoli pubblici entrano nei risultati della ricerca del catalogo (il suggerimento federato porta i titoli trovati, non più solo un link alla sezione); autore, pubblicazione e parole chiave di un articolo sono link a una ricerca filtrata; ogni articolo può avere una propria immagine, con segnaposto condiviso con il catalogo. Migrazione additiva idempotente. v1.5.0: modalità Semplice e Completa; articoli autonomi con citazione, PDF protetto, associazione successiva alle testate, import/export CSV e API additive. Migrazione idempotente; le installazioni esistenti mantengono la modalità Completa. v1.4.0: schema esteso per la gestione bibliotecaria dei periodici — testate con e-ISSN/ISSN-L, barcode EAN-13 (prefisso 977), direttore responsabile, registrazione al tribunale, prezzo di copertina, canale di acquisizione predefinito e politica di prestito; annate con serie, collocazione, consistenza dichiarata e updated_at (volume ora NOT NULL); fascicoli con barcode, separazione possesso/condizione (nuova colonna condizione; stato esteso con 'reclamato' e 'scartato', 'danneggiato'/'in_restauro' migrati a condizione), acquisizione, prezzo e tracciamento reclami; nuova tabella emeroteca_abbonamenti (fornitore, costo, validità, rinnovo). ATTENZIONE aggiornando da una versione precedente: la nuova colonna testate.prestabile arriva con valore predefinito 'consultazione', quindi TUTTE le testate esistenti risultano di sola consultazione finché non vengono impostate a 'prestabile' — verificare le testate che prima venivano date in prestito. Le pagine pubbliche dell'emeroteca entrano ora nella sitemap e la ricerca del catalogo propone l'emeroteca quando il termine vi trova riscontro; i fascicoli scartati non vengono più pubblicati sul frontend. Migrazione additiva idempotente e auto-riparante. v1.3.0: ponte mobile /api/v1/periodicals (testate, annate, fascicoli, spoglio) sulla superficie bearer-token del plugin mobile-api, con ETag/304, paginazione a cursore e documentazione OpenAPI via hook mobile_api.openapi; nessun campo interno esposto (pdf_path, note, inventario). v1.2.3 (Pinakes 0.7.76): filtro pubblico per le sole tipologie presenti, navigazione pubblica coerente, upload Uppy per logo della testata, copertina e PDF del fascicolo; PDF conservati fuori dalla web root con accesso pubblico opzionale; migrazione additiva auto-riparante; interfaccia backend e frontend armonizzata e responsive, inclusa la tabella amministrativa compatta. Include le correzioni di lifecycle, vincoli, atomicità e ricerca della v1.1.0.", + "version": "1.12.1", + "changelog": "v1.12.1: ogni articolo ha una propria scheda nel pannello di amministrazione, come quella di un libro (copertina, autori, pubblicazione, genere, parole chiave, PDF ed esportazioni, con i pulsanti Modifica ed Elimina); la ricerca rapida, l’elenco degli articoli e la scheda autore aprono la scheda, il modulo è su /edit e dopo il salvataggio si torna alla scheda. Nessuna migrazione. v1.12.0: voce Articoli nel menu di amministrazione; la ricerca rapida del pannello trova articoli (anche non pubblicati, aperti in modifica) e testate; nella scheda autore gli articoli sono schede con immagine, Dettagli e Modifica come i libri; la scheda pubblica dell’articolo ha il pulsante Modifica per lo staff, il genere e «Altre opere» dell’autore, libri compresi; nel modulo parole chiave e genere stanno nella descrizione bibliografica avanzata, e il genere fa trovare l’articolo nel catalogo filtrato per genere. Migrazione additiva idempotente (emeroteca_contributi.genere_id con chiave esterna verso generi). v1.11.0: gli articoli compaiono nei suggerimenti della ricerca mentre si scrive, accanto ai libri; nella scheda dell'articolo la testata associata si sceglie cercandola per titolo, e titolo e ISSN della testata riempiono i campi ancora vuoti; l'export MARCXML riporta il codice paese MARC in 008/15-17, il PDF pubblico in 856 e gli autori in ordine diretto in 245 $c; il RIS nomina la rivista anche in JF. Nessuna migrazione. v1.10.0: gli articoli possono essere capitoli di un'antologia, con curatori, editore, luogo e ISBN del volume (record_type book_chapter nel CSV, genre=bookitem in OpenURL e COinS); classificazione secondo uno schema non in elenco, indicato per nome; finestra Cita con APA, Chicago, MLA e Harvard ed export RIS; codici lingua e paese scelti da un elenco. v1.9.0: pagine pubbliche ridisegnate come il catalogo e la scheda libro — /emeroteca con filtri laterali (tipologia, editore, argomento, iniziale) e paginazione; testata con annate, fascicoli e articoli paginati; fascicolo con gli articoli collegati come schede e il sommario; articolo con percorso completo testata › fascicolo, collocazione, articolo precedente e successivo e articoli correlati. Un articolo senza immagine propria mostra la copertina del fascicolo, altrimenti il logo della testata. Nessuna migrazione. v1.8.0: autori degli articoli collegati alla stessa anagrafica dei libri, con selezione per nome e date, creazione esplicita, gestione di omonimi, rinomina e fusione; identificativo GND condiviso ed export MARCXML 100/700 $0. I crediti preesistenti restano disponibili e richiedono associazione esplicita. v1.7.0: gli articoli autonomi diventano schede analitiche complete — sottotitolo, lingua e paese come codici ISO, classificazione con il suo schema (DK5, DDC, UDC, LCC, RVK), nota di possesso e risorsa elettronica con testo del collegamento e condizioni di accesso (danMARC2/MARC 21 856). Citazione pronta in APA 7 e Harvard con pulsante di copia, e download RIS per EndNote, Mendeley e Zotero. Tutti i campi nuovi sono facoltativi: chi non cataloga articoli non li incontra mai. Migrazione additiva idempotente. v1.6.0: gli articoli pubblici entrano nei risultati della ricerca del catalogo (il suggerimento federato porta i titoli trovati, non più solo un link alla sezione); autore, pubblicazione e parole chiave di un articolo sono link a una ricerca filtrata; ogni articolo può avere una propria immagine, con segnaposto condiviso con il catalogo. Migrazione additiva idempotente. v1.5.0: modalità Semplice e Completa; articoli autonomi con citazione, PDF protetto, associazione successiva alle testate, import/export CSV e API additive. Migrazione idempotente; le installazioni esistenti mantengono la modalità Completa. v1.4.0: schema esteso per la gestione bibliotecaria dei periodici — testate con e-ISSN/ISSN-L, barcode EAN-13 (prefisso 977), direttore responsabile, registrazione al tribunale, prezzo di copertina, canale di acquisizione predefinito e politica di prestito; annate con serie, collocazione, consistenza dichiarata e updated_at (volume ora NOT NULL); fascicoli con barcode, separazione possesso/condizione (nuova colonna condizione; stato esteso con 'reclamato' e 'scartato', 'danneggiato'/'in_restauro' migrati a condizione), acquisizione, prezzo e tracciamento reclami; nuova tabella emeroteca_abbonamenti (fornitore, costo, validità, rinnovo). ATTENZIONE aggiornando da una versione precedente: la nuova colonna testate.prestabile arriva con valore predefinito 'consultazione', quindi TUTTE le testate esistenti risultano di sola consultazione finché non vengono impostate a 'prestabile' — verificare le testate che prima venivano date in prestito. Le pagine pubbliche dell'emeroteca entrano ora nella sitemap e la ricerca del catalogo propone l'emeroteca quando il termine vi trova riscontro; i fascicoli scartati non vengono più pubblicati sul frontend. Migrazione additiva idempotente e auto-riparante. v1.3.0: ponte mobile /api/v1/periodicals (testate, annate, fascicoli, spoglio) sulla superficie bearer-token del plugin mobile-api, con ETag/304, paginazione a cursore e documentazione OpenAPI via hook mobile_api.openapi; nessun campo interno esposto (pdf_path, note, inventario). v1.2.3 (Pinakes 0.7.76): filtro pubblico per le sole tipologie presenti, navigazione pubblica coerente, upload Uppy per logo della testata, copertina e PDF del fascicolo; PDF conservati fuori dalla web root con accesso pubblico opzionale; migrazione additiva auto-riparante; interfaccia backend e frontend armonizzata e responsive, inclusa la tabella amministrativa compatta. Include le correzioni di lifecycle, vincoli, atomicità e ricerca della v1.1.0.", "author": "Fabiodalez", "author_url": "", "plugin_url": "", diff --git a/storage/plugins/emeroteca/src/Controllers/ContributionController.php b/storage/plugins/emeroteca/src/Controllers/ContributionController.php index c9e14bd19..682dafd4f 100644 --- a/storage/plugins/emeroteca/src/Controllers/ContributionController.php +++ b/storage/plugins/emeroteca/src/Controllers/ContributionController.php @@ -83,6 +83,25 @@ private function json(Response $rs, array $payload): Response ->withHeader('Cache-Control', 'no-store') ->withHeader('X-Content-Type-Options', 'nosniff'); } + /** + * The article's own admin page (#453, #454): what the record says, with + * Edit, Delete, the public page and the exports as buttons, as the admin + * book page does for a book. 404 if the article does not exist. + */ + public function show(Request $rq, Response $rs, array $args = []): Response + { + $row = $this->service()->get((int)($args['id'] ?? 0)); + if ($row === null) { + return $rs->withStatus(404); + } + try { + $genreTrail = $this->service()->genreTrail((int)($row['genere_id'] ?? 0)); + } catch (\Throwable $e) { + SecureLogger::error('[Emeroteca] article genre: '.$e->getMessage()); + $genreTrail = []; + } + return $this->renderView($rs, 'article-show', ['row' => $row,'genreTrail' => $genreTrail,'isAdmin' => ($_SESSION['user']['tipo_utente'] ?? '') === 'admin']); + } /** * Render the create/edit form. With an id, 404s if the article does not exist * (soft-deleted or never existed); without one, renders a blank form. diff --git a/storage/plugins/emeroteca/src/Views/article-form.php b/storage/plugins/emeroteca/src/Views/article-form.php index bc95859ff..d87a2a05a 100644 --- a/storage/plugins/emeroteca/src/Views/article-form.php +++ b/storage/plugins/emeroteca/src/Views/article-form.php @@ -37,6 +37,10 @@
    • + +
    • +
    • +
    • @@ -357,7 +361,7 @@ function sync() {
    - +
    diff --git a/storage/plugins/emeroteca/src/Views/article-show.php b/storage/plugins/emeroteca/src/Views/article-show.php new file mode 100644 index 000000000..f703b6c47 --- /dev/null +++ b/storage/plugins/emeroteca/src/Views/article-show.php @@ -0,0 +1,288 @@ + htmlspecialchars(is_scalar($v) ? (string) $v : '', ENT_QUOTES, 'UTF-8'); +$id = (int) ($row['id'] ?? 0); +$value = static fn(string $key): string => trim((string) ($row[$key] ?? '')); + +$btnPrimary = 'inline-flex items-center gap-2 rounded-lg bg-gray-900 px-5 py-2.5 text-sm font-semibold text-white transition hover:bg-gray-700'; +$btnGhost = 'inline-flex items-center gap-2 rounded-lg border-2 border-gray-300 px-5 py-2.5 text-sm font-semibold text-gray-700 transition hover:bg-gray-100'; +$btnDanger = 'inline-flex items-center gap-2 rounded-lg bg-red-600 px-5 py-2.5 text-sm font-semibold text-white transition hover:bg-red-700'; + +$isPublic = !empty($row['pubblico']); +$statusBadgeClass = $isPublic + ? 'inline-flex items-center gap-2 rounded-full px-3 py-1 text-xs font-semibold bg-green-500 text-white' + : 'inline-flex items-center gap-2 rounded-full px-3 py-1 text-xs font-semibold bg-gray-600 text-white'; + +/** Own image, else the issue's cover, else the masthead's logo. */ +$cover = ContributionService::coverUrl($row); +if ($cover !== '' && strncmp($cover, 'uploads/', 8) === 0) { $cover = '/' . $cover; } +if ($cover === '') { $cover = '/uploads/copertine/placeholder.jpg'; } +$cover = preg_match('#^https?://#', $cover) ? $cover : url($cover); + +$authors = ContributionService::authorLinks($row); +$keywords = array_values(array_filter(array_map('trim', explode(',', $value('keywords'))), static fn(string $k): bool => $k !== '')); +$tipi = \EmerotecaPlugin::TIPI_ARTICOLO; +$contenitori = \EmerotecaPlugin::TIPI_CONTENITORE; +$testataId = (int) ($row['testata_id'] ?? 0); +$testataTitle = trim((string) ($row['testata_titolo'] ?? '')); +$fascicoloId = (int) ($row['fascicolo_id'] ?? 0); +$fascicoloNumero = trim((string) ($row['fascicolo_numero'] ?? '')); +$resource = ContributionService::resource($row, false); + +/** A stored ISO code in the reader's language; the code itself without intl. */ +$codeLabel = static function (string $code, bool $region): string { + if ($code === '' || !class_exists(\Locale::class)) { + return $code; + } + $locale = \App\Support\I18n::getLocale(); + $label = $region ? \Locale::getDisplayRegion('und_' . $code, $locale) : \Locale::getDisplayLanguage($code, $locale); + return ($label === '' || $label === $code) ? $code : $label; +}; + +// Label => value, shown only when filled in, in the order of the form. +$details = [ + __('Tipo di contributo') => isset($tipi[$value('tipo_contributo')]) ? __($tipi[$value('tipo_contributo')]) : $value('tipo_contributo'), + __('Titolo della pubblicazione') => $value('contenitore_titolo'), + __('Tipo di pubblicazione') => isset($contenitori[$value('contenitore_tipo')]) ? __($contenitori[$value('contenitore_tipo')]) : $value('contenitore_tipo'), + __('Data di pubblicazione') => $value('data_pubblicazione_testo'), + __('Anno') => (int) ($row['anno_pubblicazione'] ?? 0) > 0 ? (string) (int) $row['anno_pubblicazione'] : '', + __('Volume') => $value('volume'), + __('Numero') => $value('numero'), + __('Pagine') => $value('pagine'), + __('Curatori') => $value('contenitore_curatori'), + __('Editore') => $value('contenitore_editore'), + __('Luogo di pubblicazione') => $value('contenitore_luogo'), + 'ISSN' => $value('issn'), + 'ISBN' => $value('isbn'), + 'DOI' => $value('doi'), + __('Lingua') => $codeLabel($value('lingua'), false), + __('Paese') => $codeLabel($value('paese'), true), + __('Classificazione') => trim($value('classificazione_schema') . ' ' . $value('classificazione')), + __('Collocazione') => $value('collocazione'), + __('Nota di possesso') => $value('nota_possesso'), +]; +$details = array_filter($details, static fn(string $v): bool => $v !== ''); +?> +
    +
    + +
    + + +
    +
    +

    + + + + + +

    + +
    + +
    + +
    +
    + + + + + + + + + + +
    + +
    +
    + + + +
    +
    + +
    +
    +
    + +
    + +
    +
    +
    + <?= $e(sprintf(__('Immagine di «%s»'), $value('titolo'))) ?> +
    +
    +
    + + + + +
    +
    + + +
    + + + + + + + + + + + + +
    +
    +
    + + + + $genre): ?> + 0): ?> → + + + + + +
    + 0 && $testataTitle !== ''): ?> +
    + + : + + 0 && $fascicoloNumero !== ''): ?> + → + + +
    + +
    +
    +
    + + +
    +
    +
    +

    + + +

    +
    +
    +
    + $text): ?> +
    +
    +
    +
    + + +
    +
    +
    + + + +
    +
    + +
    +
    +
    + + +
    +
    +

    + + +

    +
    +
    +
    + + + +
    +
    +

    + + +

    +
    +
    +
    + + +
    +
    +

    + + +

    +
    +
    + + + + + + + + + + + + + + + RIS + + + + MARCXML + +
    +
    +
    +
    +
    +
    diff --git a/storage/plugins/emeroteca/src/Views/articles.php b/storage/plugins/emeroteca/src/Views/articles.php index df811a3f8..52ef9702d 100644 --- a/storage/plugins/emeroteca/src/Views/articles.php +++ b/storage/plugins/emeroteca/src/Views/articles.php @@ -67,6 +67,7 @@ + @@ -76,10 +77,22 @@

    + + +
    + + + + + + +
    + + -

    +

    diff --git a/storage/plugins/emeroteca/src/Views/public/article.php b/storage/plugins/emeroteca/src/Views/public/article.php index f82e01861..45f9da17a 100644 --- a/storage/plugins/emeroteca/src/Views/public/article.php +++ b/storage/plugins/emeroteca/src/Views/public/article.php @@ -150,7 +150,7 @@ - +

    diff --git a/tests/emeroteca-412-upgrade.spec.js b/tests/emeroteca-412-upgrade.spec.js index ad8c43934..0cc712e5c 100644 --- a/tests/emeroteca-412-upgrade.spec.js +++ b/tests/emeroteca-412-upgrade.spec.js @@ -137,7 +137,7 @@ test('Emeroteca 412 real upgrade, phase from E2E_412_UPGRADE_PHASE', async ({ pa await page.goto(BASE + '/admin/periodicals/articles/create'); await page.locator('[name=titolo]').fill('Upgraded412 article'); await page.getByRole('button', { name: 'Salva articolo', exact: true }).click(); - await expect(page.getByRole('heading', { name: 'Modifica articolo' })).toBeVisible(); + await expect(page.locator('section[data-article-id]')).toBeVisible(); expect(db("SELECT COUNT(*) FROM emeroteca_contributi WHERE titolo='Upgraded412 article'")).toBe('1'); const id = db("SELECT id FROM plugins WHERE name='emeroteca'"); await page.goto(BASE + `/admin/plugins/${id}/settings`); diff --git a/tests/emeroteca-412.spec.js b/tests/emeroteca-412.spec.js index d3370eed3..15d7072a2 100644 --- a/tests/emeroteca-412.spec.js +++ b/tests/emeroteca-412.spec.js @@ -111,7 +111,8 @@ test.describe.serial('Emeroteca 412 complete workflow',()=>{ await page.locator('[name=note_private]').fill('SECRET412'); await page.locator('[name=pdf]').setInputFiles({name:'tyll.pdf',mimeType:'application/pdf',buffer:Buffer.from('%PDF-1.4\n1 0 obj\n<< /Type /Catalog >>\nendobj\n%%EOF\n')}); await page.getByRole('button',{name:'Salva articolo',exact:true}).click(); - await expect(page.getByRole('heading',{name:'Modifica articolo'})).toBeVisible(); + // A saved article lands on its own page (#453), with Edit one click away. + await expect(page.locator('section[data-article-id]')).toBeVisible(); articleId=Number(page.url().split('/').pop()); expect(articleId).toBeGreaterThan(0); expect(db(`SELECT CONCAT(COALESCE(testata_id,0),':',COALESCE(fascicolo_id,0)) FROM emeroteca_contributi WHERE id=${articleId}`)).toBe('0:0'); @@ -120,16 +121,18 @@ test.describe.serial('Emeroteca 412 complete workflow',()=>{ await expect(publicPage.getByRole('heading',{name:marker+' Tyll'})).toBeVisible(); await expect(publicPage.locator('body')).not.toContainText('SECRET412'); expect((await publicPage.request.get(BASE+`/emeroteca/articolo/${articleId}/pdf`)).status()).toBe(404); + await page.getByTestId('article-edit').click(); await page.getByText('Descrizione, note e PDF',{exact:true}).click(); await page.locator('[name=pdf_pubblico]').check();await page.getByRole('button',{name:'Salva articolo',exact:true}).click(); const pdf=await publicPage.request.get(BASE+`/emeroteca/articolo/${articleId}/pdf`);expect(pdf.status()).toBe(200);expect(pdf.headers()['cache-control']).toContain('no-store'); const originalPdf=db(`SELECT pdf_path FROM emeroteca_contributi WHERE id=${articleId}`); + await page.goto(BASE+`/admin/periodicals/articles/${articleId}/edit`); await page.getByText('Descrizione, note e PDF',{exact:true}).click(); await page.locator('[name=pdf]').setInputFiles({name:'invalid.pdf',mimeType:'application/pdf',buffer:Buffer.from('not a PDF')}); await page.getByRole('button',{name:'Salva articolo',exact:true}).click(); await expect(page.getByRole('alert')).toContainText('PDF'); expect(db(`SELECT pdf_path FROM emeroteca_contributi WHERE id=${articleId}`)).toBe(originalPdf); - await page.goto(BASE+`/admin/periodicals/articles/${articleId}`); + await page.goto(BASE+`/admin/periodicals/articles/${articleId}/edit`); await page.getByText('Descrizione, note e PDF',{exact:true}).click(); await page.locator('[name=pdf]').setInputFiles({name:'replacement.pdf',mimeType:'application/pdf',buffer:Buffer.from('%PDF-1.4\n1 0 obj\n<< /Type /Catalog >>\nendobj\n%%EOF\n')}); await page.getByRole('button',{name:'Salva articolo',exact:true}).click(); @@ -186,7 +189,7 @@ test.describe.serial('Emeroteca 412 complete workflow',()=>{ expect(Number(db(`SELECT testata_id FROM emeroteca_contributi WHERE id=${articleId}`)),'the masthead survives the detach').toBe(testataId); // The form names the linked masthead, and a rejected save must not turn // that into "not linked": the 422 re-render looks the record up too. - await page.goto(BASE+`/admin/periodicals/articles/${articleId}`); + await page.goto(BASE+`/admin/periodicals/articles/${articleId}/edit`); await expect(page.locator('#article-host-record')).toContainText(marker+' Journal'); await page.getByText('Identificativi e collocazione',{exact:true}).click(); await page.locator('#article-doi').fill('not-a-doi'); @@ -215,7 +218,7 @@ test.describe.serial('Emeroteca 412 complete workflow',()=>{ await page.getByRole('button',{name:'Mostra anteprima'}).click();await expect(page.getByText('Anteprima: destinazione Emeroteca')).toBeVisible(); await page.getByRole('button',{name:'Importa le righe valide'}).click();await expect(page.getByText('Risultato importazione')).toBeVisible(); expect(db(`SELECT pagine FROM emeroteca_contributi WHERE titolo='${marker} Imported'`)).toBe('iv–x'); - await page.goto(BASE+`/admin/periodicals/articles/${articleId}`); + await page.goto(BASE+`/admin/periodicals/articles/${articleId}/edit`); await page.locator('[name=pubblico]').uncheck();await page.getByRole('button',{name:'Salva articolo',exact:true}).click(); expect((await publicPage.request.get(BASE+`/emeroteca/articolo/${articleId}`)).status()).toBe(404); expect((await publicPage.request.get(BASE+`/emeroteca/articolo/${articleId}/pdf`)).status()).toBe(404); diff --git a/tests/emeroteca-analytic-record.spec.js b/tests/emeroteca-analytic-record.spec.js index 3dc2d07bf..4c80262de 100644 --- a/tests/emeroteca-analytic-record.spec.js +++ b/tests/emeroteca-analytic-record.spec.js @@ -166,7 +166,7 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { test('Dewey notation comes from the book form picker; other schemes are named', async ({ page }) => { expect(articleId).toBeGreaterThan(0); await login(page); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); const advanced = page.locator('details', { hasText: /Descrizione bibliografica avanzata|Advanced bibliographic/ }).first(); await openFold(advanced); // A stored code comes back selected, shown by name. @@ -211,7 +211,7 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { expect(db(`SELECT CONCAT_WS('|', classificazione_schema, classificazione) FROM emeroteca_contributi WHERE id=${articleId}`)).toBe('DDC|305.8'); // Reopened, the Dewey picker shows the stored code. - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await expect(page.locator('#dewey_chip_code')).toContainText('305.8'); // A scheme outside the list is named, and stored under that name. @@ -223,13 +223,13 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { await page.locator('button[type=submit]:has-text("Salva")').first().click(); await page.waitForURL(/\/admin\/periodicals\/articles\/\d+(\?|$)/); expect(db(`SELECT CONCAT_WS('|', classificazione_schema, classificazione) FROM emeroteca_contributi WHERE id=${articleId}`)).toBe('SAB|Kbb'); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await expect(page.locator('#article-classificazione_schema')).toHaveValue('__altro'); await expect(page.locator('#article-classificazione_schema_altro')).toHaveValue('SAB'); // A Dewey code deeper than the list comes back in the picker as it is. db(`UPDATE emeroteca_contributi SET classificazione_schema='DDC', classificazione='823.91409' WHERE id=${articleId}`); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await expect(page.locator('#dewey_chip_code')).toContainText('823.91409'); await expect(page.locator('#classificazione_dewey')).toHaveValue('823.91409'); // Removed, it can be typed back: the picker accepts every depth it shows. @@ -253,7 +253,7 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { await page.unroute('**/api/dewey/autocomplete**'); // A stored notation the picker cannot show stays in the text box, not behind an empty picker. db(`UPDATE emeroteca_contributi SET classificazione_schema='DDC', classificazione='823.914 BRO' WHERE id=${articleId}`); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await openFold(advanced); await expect(page.locator('#article-classificazione')).toBeVisible(); await expect(page.locator('#article-classificazione')).toHaveValue('823.914 BRO'); @@ -279,7 +279,7 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { // A stored Dewey notation the picker cannot show survives a round trip // through the scheme select: DDC -> UDC -> DDC, then save. db(`UPDATE emeroteca_contributi SET classificazione_schema='DDC', classificazione='823.914 BRO' WHERE id=${articleId}`); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await openFold(advanced); await page.locator('#article-classificazione_schema').selectOption('UDC'); await page.locator('#article-classificazione_schema').selectOption('DDC'); @@ -290,7 +290,7 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { // Host-volume fields typed under "Anthology" and left behind after // switching to a journal neither block the save nor get stored. - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await page.locator('#article-contenitore_tipo').selectOption('antologia'); await page.locator('#article-isbn').fill('978-0-306-40615-8'); await page.locator('#article-contenitore_editore').fill('Leftover Press'); @@ -304,7 +304,7 @@ test.describe.serial('Emeroteca analytic record (#412)', () => { const many = Array.from({ length: 21 }, (_, i) => `Author ${i + 1}`).join('; '); db(`DELETE FROM emeroteca_contributi_autori WHERE contributo_id=${articleId}`); db(`UPDATE emeroteca_contributi SET autori='${many}' WHERE id=${articleId}`); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await expect(page.locator('#article-authors-text-only')).toBeVisible(); await expect(page.locator('#article-autori')).toBeEnabled(); await expect(page.locator('#article-authors-picker')).toBeHidden(); diff --git a/tests/emeroteca-articles-453-455.spec.js b/tests/emeroteca-articles-453-455.spec.js index 8eb8194ba..1616fd94c 100644 --- a/tests/emeroteca-articles-453-455.spec.js +++ b/tests/emeroteca-articles-453-455.spec.js @@ -113,9 +113,20 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { await entry.click(); await expect(admin).toHaveURL(/\/admin\/periodicals\/articles$/); await expect(admin.getByText(ARTICLE).first()).toBeVisible(); + // Only the Articles entry is highlighted, not Periodicals above it, whose + // address is a prefix of this one. + await expect(entry).toHaveClass(/bg-rose-50/); + await expect(admin.locator('a.nav-link[href$="/admin/periodicals"]')).not.toHaveClass(/bg-rose-50/); + // The title opens the article's page, not its form; Edit is its own icon. + const row = admin.locator('tr', { hasText: ARTICLE }); + await expect(row.locator(`a[href$="/admin/periodicals/articles/${articleId}/edit"]`)).toHaveCount(1); + await row.getByRole('link', { name: ARTICLE }).click(); + await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}$`)); + await expect(admin.locator(`section[data-article-id="${articleId}"]`)).toBeVisible(); + await expect(admin.locator('#article-titolo')).toHaveCount(0); }); - test('2 The admin quick search finds the article and the periodical; the article opens its form (#453)', async () => { + test('2 The admin quick search finds the article and the periodical; the article opens its page (#453)', async () => { await admin.goto(BASE + '/admin/dashboard'); await expect(admin.locator('#global-search')).toHaveAttribute('aria-label', /articoli, periodici/); await admin.locator('#global-search').fill(RUN); @@ -138,7 +149,8 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { expect(periodicalAt, 'after it, the periodical').toBeGreaterThan(articleAt); await article.click(); await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}$`)); - await expect(admin.locator('#article-titolo')).toHaveValue(ARTICLE); + await expect(admin.locator('h1', { hasText: ARTICLE })).toBeVisible(); + await expect(admin.locator('#article-titolo')).toHaveCount(0); }); test('3 The author page shows the article as a card with an image, Details and Edit (#453)', async () => { @@ -148,14 +160,15 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { await expect(card.locator('img')).toHaveCount(1); await expect(card.locator('h3')).toContainText(ARTICLE); await expect(card).toContainText(MASTHEAD); - await expect(card.locator(`a.btn-primary[href$="/emeroteca/articolo/${articleId}"]`)).toBeVisible(); - await expect(card.locator(`a.btn-secondary[href$="/admin/periodicals/articles/${articleId}"]`)).toBeVisible(); + // Details opens the article's admin page, as a book's opens the book's. + await expect(card.locator(`a.btn-primary[href$="/admin/periodicals/articles/${articleId}"]`)).toBeVisible(); + await expect(card.locator(`a.btn-secondary[href$="/admin/periodicals/articles/${articleId}/edit"]`)).toBeVisible(); // The plain list of links it replaces is gone. await expect(admin.locator('ul.space-y-2 a.underline', { hasText: ARTICLE })).toHaveCount(0); }); test('4 The article form has keywords and genre in the advanced section; the genre is saved (#455)', async () => { - await admin.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await admin.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); const advanced = admin.locator('details.article-fold', { hasText: 'Descrizione bibliografica avanzata' }); // Open by itself: the article already has keywords, now kept in here. await expect(advanced).toHaveAttribute('open', ''); @@ -166,13 +179,40 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { expect(label, 'a child genre is listed with its path').toContain('›'); await genre.selectOption(String(childGenre)); await admin.locator('form button[type=submit]', { hasText: /Salva/ }).first().click(); + // Saving brings the operator back to the article's page, which shows it. + await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}$`)); + await expect(admin.getByText('Articolo salvato.')).toBeVisible(); + // The genre with its path, root first, as the book page shows it. + const genreLine = admin.getByTestId('genre-display'); + await expect(genreLine).toContainText(db(`SELECT nome FROM generi WHERE id=${parentGenre}`)); + await expect(genreLine).toContainText(db(`SELECT nome FROM generi WHERE id=${childGenre}`)); await expect.poll(() => db(`SELECT COALESCE(genere_id,0) FROM emeroteca_contributi WHERE id=${articleId}`)).toBe(String(childGenre)); expect(db(`SELECT keywords FROM emeroteca_contributi WHERE id=${articleId}`)).toBe('fagbevægelsen, nazisme'); }); + test('4b The admin article page reads like the book page: the record, then Edit, Delete and the exports (#453, #454)', async () => { + await admin.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + const page = admin.locator(`section[data-article-id="${articleId}"]`); + await expect(page.locator('h1', { hasText: ARTICLE })).toBeVisible(); + await expect(page.locator(`a[href$="/admin/authors/${authorId}"]`)).toContainText(AUTHOR.split(' ')[0]); + await expect(page.locator(`a[href$="/admin/periodicals/${mastheadId}/issues"]`)).toContainText(MASTHEAD); + await expect(page.locator('dd', { hasText: 'fagbevægelsen' })).toBeVisible(); + await expect(page.locator('dd', { hasText: '18-38' })).toBeVisible(); + await expect(page.locator(`a[href$="/emeroteca/articolo/${articleId}"]`, { hasText: 'Vedi pagina pubblica' })).toBeVisible(); + await expect(page.locator(`a[href$="/admin/periodicals/articles/${articleId}/citation.ris"]`)).toBeVisible(); + await expect(page.locator(`form[action$="/admin/periodicals/articles/${articleId}/delete"]`)).toHaveCount(1); + // Nothing on it is an input: the record is changed in the form. + await expect(page.locator('input:not([type=hidden]), textarea, select')).toHaveCount(0); + // Edit and Cancel make the round trip without saving anything. + await page.getByTestId('article-edit').click(); + await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}/edit$`)); + await admin.getByRole('link', { name: 'Annulla' }).click(); + await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}$`)); + }); + test('5 The public article page offers staff an Edit button, shows the genre and the author\'s other works, books included (#453, #455)', async () => { await admin.goto(`${BASE}/emeroteca/articolo/${articleId}`); - const edit = admin.locator(`a[href$="/admin/periodicals/articles/${articleId}"]`, { hasText: 'Modifica' }); + const edit = admin.locator(`a[href$="/admin/periodicals/articles/${articleId}/edit"]`, { hasText: 'Modifica' }); await expect(edit).toBeVisible(); await expect(admin.locator(`.meta-item a[href*="genere_id=${childGenre}"]`)).toBeVisible(); await expect(admin.locator(`.meta-item a[href*="genere_id=${parentGenre}"]`)).toBeVisible(); @@ -181,7 +221,8 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { await expect(others.locator('.book-card', { hasText: BOOK })).toBeVisible(); await expect(others.locator(`a[href$="/${authorId}"]`, { hasText: 'Tutte' })).toBeVisible(); await edit.click(); - await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}$`)); + await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}/edit$`)); + await expect(admin.locator('#article-titolo')).toHaveValue(ARTICLE); }); test('6 A visitor sees no Edit button, and the header search suggests the article (#453, #455)', async ({ browser }) => { diff --git a/tests/update-proxy-drop-450.spec.js b/tests/update-proxy-drop-450.spec.js new file mode 100644 index 000000000..5ee9fcb3b --- /dev/null +++ b/tests/update-proxy-drop-450.spec.js @@ -0,0 +1,141 @@ +// @ts-check +/** + * #450: a proxy in front of the site (here, a NAS's remote access) gave up on + * the install request with a 502 after a minute, while PHP carried the update + * to its end. The page reported an error for an update that had succeeded. + * + * The page now reads the outcome from /admin/updates/status when the install + * request comes back as a gateway error or a dropped connection. The proxy is + * simulated by answering the browser's install request ourselves; the status + * replies play the server finishing (or failing) behind it. No update runs. + */ +const { test, expect } = require('@playwright/test'); +const fs = require('fs'); +const path = require('path'); + +const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081'; +const TARGET = '9.9.9'; + +async function login(page) { + await page.goto(`${BASE}/accedi`); + await page.fill('input[name="email"]', process.env.E2E_ADMIN_EMAIL || ''); + await page.fill('input[name="password"]', process.env.E2E_ADMIN_PASS || ''); + await page.locator('button[type=submit]').click(); + await page.waitForURL(u => !u.pathname.includes('accedi') && !u.pathname.includes('login')); +} + +/** + * Serve the status replies in order, the last one for every later poll. + * @param {import('@playwright/test').Page} page + * @param {object[]} replies + */ +async function routeStatus(page, replies) { + let call = 0; + await page.route('**/admin/updates/status', route => { + const body = replies[Math.min(call, replies.length - 1)]; + call++; + return route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(body) }); + }); +} + +/** The download step succeeds: the package is "on the server". */ +async function routeDownload(page) { + await page.route('**/admin/updates/download', route => route.fulfill({ + status: 200, contentType: 'application/json', body: JSON.stringify({ success: true, package: 'proxy450' }), + })); +} + +/** @param {import('@playwright/test').Page} page */ +async function startUpdate(page) { + await page.evaluate(v => { window.startUpdate(v); }, TARGET); + await page.locator('.swal2-confirm').click(); +} + +const PROXY_502 = { + status: 502, + contentType: 'text/html', + body: '

    Proxy Error

    The proxy server received an invalid response from an upstream server. Reason: Error reading from remote server

    ', +}; + +test.describe.serial('Update survives a proxy that drops the install request (#450)', () => { + test.beforeAll(() => { + if (!process.env.E2E_ADMIN_EMAIL) throw new Error('Run with /tmp/run-e2e.sh'); + }); + + test('1 The status endpoint answers with the installed version and no update running', async ({ page }) => { + await login(page); + const res = await page.request.get(`${BASE}/admin/updates/status`); + expect(res.status()).toBe(200); + expect(res.headers()['cache-control']).toContain('no-store'); + const data = await res.json(); + const installed = JSON.parse(fs.readFileSync(path.join(__dirname, '..', 'version.json'), 'utf8')).version; + expect(data.success).toBe(true); + expect(data.version).toBe(installed); + expect(data.running).toBe(false); + // A visitor gets nothing from it. + const anonymous = await page.context().browser().newContext(); + try { + const anon = await anonymous.request.get(`${BASE}/admin/updates/status`, { maxRedirects: 0 }); + expect(anon.status(), 'not for visitors').not.toBe(200); + } finally { + await anonymous.close(); + } + }); + + test('2 A 502 from the proxy, then the server finishes: the page reports success', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: { id: 40, to_version: '0.7.93', status: 'completed', error: '' } }, + { success: true, version: '0.7.93', running: true, last: { id: 41, to_version: TARGET, status: 'started', error: '' } }, + { success: true, version: TARGET, running: false, last: { id: 41, to_version: TARGET, status: 'completed', error: '' } }, + ]); + await startUpdate(page); + await expect(page.locator('#updateMessage'), 'the page says it is waiting, not that it failed').toContainText('continua sul server', { timeout: 10_000 }); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); + await expect(page.locator('[data-step="migrate"]')).not.toContainText('Errore'); + }); + + test('3 A 502 from the proxy, then the update fails on the server: the page shows its error', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: { id: 50, to_version: '0.7.93', status: 'completed', error: '' } }, + { success: true, version: '0.7.93', running: false, last: { id: 51, to_version: TARGET, status: 'failed', error: 'Errore nella copia del file: probe450' } }, + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); + await expect(page.locator('#updateMessage')).toContainText('probe450'); + }); + + test('4 A dropped connection with no log entry: the installed version decides', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await page.route('**/admin/updates/install-manual', route => route.abort('connectionreset')); + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: null }, + { success: true, version: TARGET, running: false, last: null }, + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); + }); + + test('5 An older completed update in the log is not taken for this one', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + // Nothing new is ever logged and the version never moves. + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: { id: 60, to_version: '0.7.93', status: 'completed', error: '' } }, + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 40_000 }); + await expect(page.locator('#updateMessage')).toContainText('non risulta completato'); + }); +}); diff --git a/tests/update-status-450.unit.php b/tests/update-status-450.unit.php new file mode 100644 index 000000000..a07a6410d --- /dev/null +++ b/tests/update-status-450.unit.php @@ -0,0 +1,132 @@ +set_charset('utf8mb4'); +} catch (Throwable $e) { + fwrite(STDERR, "FAIL: database unreachable — Updater needs a connection: {$e->getMessage()}\n"); + exit(1); +} + +$updater = new Updater($db); +$lockFile = $root . '/storage/cache/update.lock'; + +echo "A. the update lock, as another request holds it\n"; +if (!is_dir(dirname($lockFile))) { + mkdir(dirname($lockFile), 0755, true); +} +$holder = fopen($lockFile, 'c'); +$check($holder !== false, 'the lock file opens'); +$check(flock($holder, LOCK_EX | LOCK_NB), 'this test takes the lock, as an update does'); +$started = microtime(true); +$check($updater->isUpdateRunning() === true, 'a held lock reads as an update running'); +$check(microtime(true) - $started < 1.0, 'the probe does not wait for the lock'); +flock($holder, LOCK_UN); +$check($updater->isUpdateRunning() === false, 'a released lock reads as no update running'); +// The probe never keeps the lock: the update can still take it at once. +$check(flock($holder, LOCK_EX | LOCK_NB), 'after the probe, the lock is free for the update'); +flock($holder, LOCK_UN); +fclose($holder); + +echo "B. the latest attempt in update_logs\n"; +$marker = 'u450-' . bin2hex(random_bytes(3)); +$ids = []; +try { + // The table is created by the first logged update; make sure it is there. + $updater->getUpdateHistory(1); + $db->query("CREATE TABLE IF NOT EXISTS `update_logs` ( + `id` int NOT NULL AUTO_INCREMENT, + `from_version` varchar(20) NOT NULL, + `to_version` varchar(20) NOT NULL, + `status` enum('started','completed','failed','rolled_back') NOT NULL DEFAULT 'started', + `backup_path` varchar(500) DEFAULT NULL, + `error_message` text, + `started_at` datetime DEFAULT CURRENT_TIMESTAMP, + `completed_at` datetime DEFAULT NULL, + `executed_by` int DEFAULT NULL, + PRIMARY KEY (`id`) + ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci"); + $insert = static function (string $to, string $status, ?string $error) use ($db, &$ids): int { + $stmt = $db->prepare("INSERT INTO update_logs (from_version, to_version, status, error_message) VALUES ('0.0.1', ?, ?, ?)"); + $stmt->bind_param('sss', $to, $status, $error); + $stmt->execute(); + $ids[] = $id = (int) $db->insert_id; + $stmt->close(); + return $id; + }; + $failedId = $insert('9.9.' . random_int(10, 99), 'failed', 'copy failed ' . $marker); + $last = $updater->lastUpdateAttempt(); + $check(($last['id'] ?? 0) === $failedId, 'the newest attempt is returned'); + $check(($last['status'] ?? '') === 'failed', 'with its status'); + $check(($last['error'] ?? '') === 'copy failed ' . $marker, 'and its error'); + // A backup is logged in the same table but is not an update attempt. + $insert('backup', 'completed', null); + $check(($updater->lastUpdateAttempt()['id'] ?? 0) === $failedId, 'a later backup row is skipped'); + $completedId = $insert('9.9.' . random_int(10, 99), 'completed', null); + $last = $updater->lastUpdateAttempt(); + $check(($last['id'] ?? 0) === $completedId && $last['status'] === 'completed' && $last['error'] === '', 'a completed attempt reads as completed, with no error'); +} finally { + if ($ids !== []) { + $db->query('DELETE FROM update_logs WHERE id IN (' . implode(',', array_map('intval', $ids)) . ')'); + } +} + +echo "C. the install request lets the status polls through\n"; +// The polls come from the same browser session, and PHP keeps a session +// locked until the request that opened it closes it. +$controller = (string) file_get_contents($root . '/app/Controllers/UpdateController.php'); +$install = substr($controller, (int) strpos($controller, 'public function installManualUpdate')); +$install = substr($install, 0, (int) strpos($install, "\n }\n") + 6); +$closeAt = strpos($install, 'session_write_close()'); +$runAt = strpos($install, '->performUpdateFromFile('); +$check($closeAt !== false && $runAt !== false && $closeAt < $runAt, 'the session is released before the update runs'); +$check(strpos($install, "unset(\$_SESSION['manual_update_path']") < $closeAt, 'after the pending package is cleared, so the clearing is saved'); + +echo "\nPassed: {$passed} Failed: {$failed}\n"; +exit($failed === 0 ? 0 : 1); diff --git a/tests/uwe-412-followup.spec.js b/tests/uwe-412-followup.spec.js index 282278883..0fb27aff7 100644 --- a/tests/uwe-412-followup.spec.js +++ b/tests/uwe-412-followup.spec.js @@ -135,7 +135,7 @@ test.describe.serial('Uwe #412 follow-up', () => { db(`UPDATE emeroteca_testate SET titolo='zzz ${MASTHEAD}' WHERE id=${mastheadId}`); try { await login(page); - await page.goto(`${BASE}/admin/periodicals/articles/${articleId}`); + await page.goto(`${BASE}/admin/periodicals/articles/${articleId}/edit`); await expect(page.locator('#article-testata_id')).toHaveValue(String(mastheadId)); // The save posts to /save and redirects back here, so wait for the POST itself: // reading the row before it is handled would pass whatever the save did. diff --git a/version.json b/version.json index 6aec69dcc..8ca42185d 100644 --- a/version.json +++ b/version.json @@ -1,5 +1,5 @@ { "name": "Pinakes", - "version": "0.7.93", + "version": "0.7.94", "description": "Library Management System - Sistema di Gestione Bibliotecaria" } From b5ddd84f1f56b700908db7b5a0c07352b9647826 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 6 Oct 2026 22:10:08 +0200 Subject: [PATCH 2/7] Report every way an update behind a proxy can end; show an article's local resource The status poll read only update_logs, which gets a row once installUpdate() starts. A run that failed earlier (space, backup, extraction, package checks) left nothing to read, and the page fell back to a generic message. performUpdateFromFile() now writes the run's outcome (success or the error it stopped on, with the installed version) to storage/cache/update-outcome.json while it still holds the lock; the status endpoint returns it and the page prefers it. A logged attempt still 'started' once the lock is free is reported as interrupted on the server. /admin/updates/status is admin only, like the update it reports on: AdminAuthMiddleware also lets staff through, so the role is checked in the controller. The admin article page named a linkable resource only. A local path or identifier is now shown as text, and the access conditions next to it. --- CHANGELOG.md | 4 +- app/Controllers/UpdateController.php | 9 +++- app/Support/Updater.php | 50 +++++++++++++++++++ app/Views/admin/updates.php | 14 ++++++ locale/da_DK.json | 3 +- locale/de_DE.json | 3 +- locale/en_US.json | 3 +- locale/fr_FR.json | 3 +- locale/it_IT.json | 3 +- .../emeroteca/src/Views/article-show.php | 9 ++++ tests/emeroteca-articles-453-455.spec.js | 10 ++++ tests/update-proxy-drop-450.spec.js | 29 +++++++++++ tests/update-status-450.unit.php | 32 ++++++++++++ 13 files changed, 163 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7c13d9c3..d220a4e63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,10 @@ Full version-by-version history for Pinakes. The README shows only the latest re ### Added - **An admin page for each article** ([#453](https://github.com/fabiodalez-dev/Pinakes/issues/453), [#454](https://github.com/fabiodalez-dev/Pinakes/issues/454)), laid out like the admin book page. It shows the cover, authors, publication, masthead and issue, genre with its path, keywords and the rest of the record, the PDF and the RIS and MARCXML exports, with Edit, Delete and the public page as buttons. `/admin/periodicals/articles/{id}` is now this page and the form moved to `/admin/periodicals/articles/{id}/edit`. The quick search, the Articles list (which gains View and Edit icons) and the Details button on the author page open it, and saving the form returns to it. Before, every link opened the form, so saving looked as if nothing had happened. -- **`GET /admin/updates/status`** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)): the installed version, whether the update lock is held, and the latest update attempt with its status and error. Admin only, never cached, reachable during maintenance. +- **`GET /admin/updates/status`** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)): the installed version, whether the update lock is held, and the latest update attempt with its status and error, and the outcome of the latest run. Admin only (staff get 403), never cached, reachable during maintenance. ### Fixed -- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports the attempt the server logged: success, or the error the update recorded. The install request releases the PHP session before the update runs, so those polls are not held behind it. +- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports how the run ended: success, or the error it stopped on. The run writes its outcome before it releases the lock, so a failure before the install step (space, backup, extraction, package checks) is reported with its own message too, and an attempt the server left half-way is reported as interrupted. The install request releases the PHP session before the update runs, so those polls are not held behind it. - **Only the most specific admin menu entry is highlighted.** On an article page both Periodicals and Articles lit up, because the address of one starts with the other's. The Emeroteca plugin goes to 1.12.1. No migration. diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index d842e8184..9d89c030f 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -870,11 +870,15 @@ public function installManualUpdate(Request $request, Response $response, mysqli * cut short (#450). The update itself carries on in PHP after the proxy * gives up, so the page asks here until it ends: the installed version, * whether the update lock is still held, and the latest attempt logged. - * Reachable during maintenance (index.php allows /admin/updates) and - * read-only, so AdminAuthMiddleware is enough. + * Reachable during maintenance (index.php allows /admin/updates). Admin + * only, like the update it reports on: AdminAuthMiddleware also lets staff + * through, so the role is checked here. */ public function status(Request $request, Response $response, mysqli $db): Response { + if (($_SESSION['user']['tipo_utente'] ?? '') !== 'admin') { + return $this->jsonResponse($response, ['error' => __('Operazione riservata agli amministratori')], 403); + } try { $updater = new Updater($db); } catch (\Throwable $e) { @@ -888,6 +892,7 @@ public function status(Request $request, Response $response, mysqli $db): Respon 'version' => $updater->getCurrentVersion(), 'running' => $updater->isUpdateRunning(), 'last' => $updater->lastUpdateAttempt(), + 'outcome' => $updater->lastUpdateOutcome(), ])->withHeader('Cache-Control', 'no-store'); } diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 7cf708218..ee491cdde 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -2141,6 +2141,13 @@ public function performUpdateFromFile(string $uploadTempPath): array } finally { $this->cleanup(); + // The outcome is written while the lock is still held, so a page + // whose request a proxy dropped (#450) reads a final answer as + // soon as it sees the lock free, failures before installUpdate() + // (space, backup, extraction, package checks) included: those + // leave no update_logs row of their own. + $this->recordUpdateOutcome($result ?? ['success' => false, 'error' => null]); + // Normal cleanup is complete while this request still owns the // lock. Disarm the shutdown fallback before releasing it: once // another request acquires the lock, this request must never remove @@ -4409,6 +4416,49 @@ public function isUpdateRunning(): bool } } + /** + * Write how the update this request ran ended, for the status endpoint. + * Atomic (temp file + rename); a write that fails is logged and skipped: + * the status then falls back to update_logs and the installed version. + * + * @param array $result + */ + private function recordUpdateOutcome(array $result): void + { + $file = $this->rootPath . '/storage/cache/update-outcome.json'; + $payload = json_encode([ + 'at' => microtime(true), + 'success' => !empty($result['success']), + 'error' => (string) ($result['error'] ?? ''), + 'version' => $this->getCurrentVersion(), + ], JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); + $tmp = $file . '.' . bin2hex(random_bytes(4)) . '.tmp'; + if ($payload === false || @file_put_contents($tmp, $payload) === false || !@rename($tmp, $file)) { + @unlink($tmp); + $this->debugLog('WARNING', 'Esito aggiornamento non registrato', ['file' => $file]); + } + } + + /** + * How the latest update run ended (see recordUpdateOutcome()), or null. + * + * @return array{at:float,success:bool,error:string,version:string}|null + */ + public function lastUpdateOutcome(): ?array + { + $raw = @file_get_contents($this->rootPath . '/storage/cache/update-outcome.json'); + $data = is_string($raw) ? json_decode($raw, true) : null; + if (!is_array($data) || !isset($data['at'])) { + return null; + } + return [ + 'at' => (float) $data['at'], + 'success' => !empty($data['success']), + 'error' => (string) ($data['error'] ?? ''), + 'version' => (string) ($data['version'] ?? ''), + ]; + } + /** * The latest update attempt in update_logs (backups excluded), or null * when there is none or the table is missing. diff --git a/app/Views/admin/updates.php b/app/Views/admin/updates.php index 73dfca39c..01bd8f7b6 100644 --- a/app/Views/admin/updates.php +++ b/app/Views/admin/updates.php @@ -904,6 +904,7 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text // for this one: then only the installed version decides. const trustLog = before !== null; const beforeId = before && before.last ? before.last.id : 0; + const beforeOutcomeAt = before && before.outcome ? before.outcome.at : 0; const deadline = Date.now() + 30 * 60 * 1000; let idlePolls = 0; while (Date.now() < deadline) { @@ -913,6 +914,14 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text idlePolls = 0; continue; } + // The run's own outcome covers every failure, including those before + // the install step that leave no log row (space, backup, package). + const outcome = trustLog && status.outcome && status.outcome.at > beforeOutcomeAt ? status.outcome : null; + if (outcome) { + return outcome.success + ? { success: true, message: } + : { success: false, error: outcome.error || }; + } const last = trustLog && status.last && status.last.id > beforeId ? status.last : null; if ((last && last.status === 'completed') || (targetVersion && status.version === targetVersion)) { return { success: true, message: }; @@ -920,6 +929,11 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text if (last && (last.status === 'failed' || last.status === 'rolled_back')) { return { success: false, error: last.error || }; } + // Started, never finished, and the lock is free: the server process + // ended mid-update (a fatal error, memory, a killed worker). + if (last && last.status === 'started') { + return { success: false, error: }; + } // Not running and nothing settled: give the update a moment to take // the lock or write its log, then call it. if (++idlePolls >= 3) { diff --git a/locale/da_DK.json b/locale/da_DK.json index 0b108d2cb..9e35e3346 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -8056,5 +8056,6 @@ "File e citazione": "Filer og citation", "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "Forbindelsen blev afbrudt, men opdateringen fortsætter på serveren. Venter på, at den bliver færdig…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "Opdateringen ser ikke ud til at være fuldført. Se loggene for detaljer.", - "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Opdateringen kører stadig på serveren efter 30 minutter. Genindlæs siden senere for at se den installerede version." + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Opdateringen kører stadig på serveren efter 30 minutter. Genindlæs siden senere for at se den installerede version.", + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "Opdateringen stoppede på serveren, før den blev færdig. Se loggene for detaljer." } diff --git a/locale/de_DE.json b/locale/de_DE.json index 89089bacc..a4c8e4304 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -8056,5 +8056,6 @@ "File e citazione": "Dateien und Zitation", "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "Die Verbindung wurde unterbrochen, aber das Update läuft auf dem Server weiter. Es wird gewartet, bis es abgeschlossen ist…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "Das Update scheint nicht abgeschlossen zu sein. Prüfen Sie die Protokolle für Details.", - "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Das Update läuft nach 30 Minuten noch auf dem Server. Laden Sie die Seite später neu, um die installierte Version zu sehen." + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Das Update läuft nach 30 Minuten noch auf dem Server. Laden Sie die Seite später neu, um die installierte Version zu sehen.", + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "Das Update wurde auf dem Server vor dem Abschluss unterbrochen. Prüfen Sie die Protokolle für Details." } diff --git a/locale/en_US.json b/locale/en_US.json index b35aad43c..c302102e6 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -8056,5 +8056,6 @@ "File e citazione": "Files and citation", "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "The connection dropped, but the update is still running on the server. Waiting for it to finish…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "The update does not appear to have completed. Check the logs for details.", - "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "The update is still running on the server after 30 minutes. Reload the page later to see the installed version." + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "The update is still running on the server after 30 minutes. Reload the page later to see the installed version.", + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "The update stopped on the server before it finished. Check the logs for details." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 55c709d42..641821d79 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -8056,5 +8056,6 @@ "File e citazione": "Fichiers et citation", "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "La connexion a été interrompue, mais la mise à jour se poursuit sur le serveur. Attente de la fin…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "La mise à jour ne semble pas terminée. Consultez les journaux pour plus de détails.", - "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "La mise à jour est toujours en cours sur le serveur après 30 minutes. Rechargez la page plus tard pour voir la version installée." + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "La mise à jour est toujours en cours sur le serveur après 30 minutes. Rechargez la page plus tard pour voir la version installée.", + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "La mise à jour s'est interrompue sur le serveur avant la fin. Consultez les journaux pour plus de détails." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 5469cbb43..9518fc9b1 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -8056,5 +8056,6 @@ "File e citazione": "File e citazione", "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "L'aggiornamento non risulta completato. Controlla i log per i dettagli.", - "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata." + "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.", + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli." } diff --git a/storage/plugins/emeroteca/src/Views/article-show.php b/storage/plugins/emeroteca/src/Views/article-show.php index f703b6c47..8f6128211 100644 --- a/storage/plugins/emeroteca/src/Views/article-show.php +++ b/storage/plugins/emeroteca/src/Views/article-show.php @@ -271,6 +271,15 @@ class="inline-flex items-center px-2 py-1 rounded-full text-sm bg-gray-100 text- + + + + + + + + +

    :

    diff --git a/tests/emeroteca-articles-453-455.spec.js b/tests/emeroteca-articles-453-455.spec.js index 1616fd94c..753c8dfc9 100644 --- a/tests/emeroteca-articles-453-455.spec.js +++ b/tests/emeroteca-articles-453-455.spec.js @@ -208,6 +208,16 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}/edit$`)); await admin.getByRole('link', { name: 'Annulla' }).click(); await expect(admin).toHaveURL(new RegExp(`/admin/periodicals/articles/${articleId}$`)); + // A resource that is a local path is named, not linked, with its access terms. + db(`UPDATE emeroteca_contributi SET risorsa_url='arkiv/1988/petersen.pdf', risorsa_accesso='Kun på læsesalen' WHERE id=${articleId}`); + try { + await admin.reload(); + await expect(admin.getByTestId('article-resource')).toContainText('arkiv/1988/petersen.pdf'); + await expect(admin.locator('a[href*="arkiv/1988"]')).toHaveCount(0); + await expect(page).toContainText('Kun på læsesalen'); + } finally { + db(`UPDATE emeroteca_contributi SET risorsa_url=NULL, risorsa_accesso=NULL WHERE id=${articleId}`); + } }); test('5 The public article page offers staff an Edit button, shows the genre and the author\'s other works, books included (#453, #455)', async () => { diff --git a/tests/update-proxy-drop-450.spec.js b/tests/update-proxy-drop-450.spec.js index 5ee9fcb3b..1dc971032 100644 --- a/tests/update-proxy-drop-450.spec.js +++ b/tests/update-proxy-drop-450.spec.js @@ -125,6 +125,35 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); }); + test('6 A failure before the install step, which logs no attempt, still shows its own error', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + // The run's outcome file is the only trace: no new update_logs row. + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 1000, success: true, error: '', version: '0.7.93' } }, + { success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 2000, success: false, error: 'Spazio insufficiente probe450', version: '0.7.93' } }, + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); + await expect(page.locator('#updateMessage')).toContainText('Spazio insufficiente probe450'); + }); + + test('7 An attempt the server left half-way is reported as interrupted', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: { id: 80, to_version: '0.7.93', status: 'completed', error: '' }, outcome: null }, + { success: true, version: '0.7.93', running: false, last: { id: 81, to_version: TARGET, status: 'started', error: '' }, outcome: null }, + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); + await expect(page.locator('#updateMessage')).toContainText('interrotto sul server'); + }); + test('5 An older completed update in the log is not taken for this one', async ({ page }) => { await login(page); await page.goto(`${BASE}/admin/updates`); diff --git a/tests/update-status-450.unit.php b/tests/update-status-450.unit.php index a07a6410d..f25ceac3c 100644 --- a/tests/update-status-450.unit.php +++ b/tests/update-status-450.unit.php @@ -117,6 +117,38 @@ } } +echo "B2. the outcome of the latest run\n"; +$outcomeFile = $root . '/storage/cache/update-outcome.json'; +$saved = is_file($outcomeFile) ? file_get_contents($outcomeFile) : null; +try { + $record = new ReflectionMethod(Updater::class, 'recordUpdateOutcome'); + $record->setAccessible(true); + $record->invoke($updater, ['success' => false, 'error' => 'no space ' . $marker]); + $outcome = $updater->lastUpdateOutcome(); + $check(($outcome['success'] ?? null) === false && ($outcome['error'] ?? '') === 'no space ' . $marker, 'a failed run is read back with its error'); + $first = (float) ($outcome['at'] ?? 0); + usleep(10000); + $record->invoke($updater, ['success' => true, 'error' => null]); + $outcome = $updater->lastUpdateOutcome(); + $check(($outcome['success'] ?? null) === true && (float) $outcome['at'] > $first, 'a later run replaces it, with a later time'); + $check(($outcome['version'] ?? '') === $updater->getCurrentVersion(), 'it names the version installed when it ended'); + $check(glob($outcomeFile . '.*.tmp') === [], 'the write leaves no temp file behind'); + file_put_contents($outcomeFile, 'not json'); + $check($updater->lastUpdateOutcome() === null, 'an unreadable file reads as no outcome'); + // Written before the lock is released, inside performUpdateFromFile(). + $src = (string) file_get_contents($root . '/app/Support/Updater.php'); + $body = substr($src, (int) strpos($src, 'public function performUpdateFromFile')); + $recordAt = strpos($body, '$this->recordUpdateOutcome('); + $unlockAt = strpos($body, 'flock($lockHandle, LOCK_UN)'); + $check($recordAt !== false && $unlockAt !== false && $recordAt < $unlockAt, 'the outcome is written while the lock is still held'); +} finally { + if ($saved !== null) { + file_put_contents($outcomeFile, $saved); + } else { + @unlink($outcomeFile); + } +} + echo "C. the install request lets the status polls through\n"; // The polls come from the same browser session, and PHP keeps a session // locked until the request that opened it closes it. From 12f263958604267ca91132fdebb4748f10dff54d Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 7 Oct 2026 00:29:07 +0200 Subject: [PATCH 3/7] Tell the page's own update outcome from another one by an attempt identifier The update page makes a random identifier before it sends the install request; the server writes it next to the outcome and the page waits for the outcome that carries it. Timestamps could not tell two administrators' runs apart, and a status that could not be read before the request made the page ignore its own successful outcome. The identifier is accepted only as 32 hex characters. The article page's access terms get their own test id, asserted on a locator. --- app/Controllers/UpdateController.php | 4 + app/Support/Updater.php | 19 ++++- app/Views/admin/updates.php | 30 +++++--- .../emeroteca/src/Views/article-show.php | 2 +- tests/emeroteca-articles-453-455.spec.js | 2 +- tests/update-pending-package.unit.php | 2 + tests/update-proxy-drop-450.spec.js | 77 ++++++++++++++++--- tests/update-status-450.unit.php | 9 +++ 8 files changed, 121 insertions(+), 24 deletions(-) diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index 9d89c030f..96ec23c11 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -847,6 +847,10 @@ public function installManualUpdate(Request $request, Response $response, mysqli session_write_close(); } + // The page's own name for this install: it finds the outcome by it + // when a proxy drops this request (#450). Validated by the Updater. + $updater->setAttemptId((string) ($data['attempt'] ?? '')); + // Perform the update from uploaded file (use resolved path to prevent TOCTOU) $this->answerJsonOnFatal(); $result = $updater->performUpdateFromFile($realTempPath); diff --git a/app/Support/Updater.php b/app/Support/Updater.php index ee491cdde..29f72fdee 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -4416,6 +4416,21 @@ public function isUpdateRunning(): bool } } + /** The page's identifier for the install it started (see setAttemptId()), or ''. */ + private string $attemptId = ''; + + /** + * Name the install about to run, so its outcome can be told apart from + * another one. The update page makes the identifier before it sends the + * request, and finds its own outcome by it when a proxy dropped the + * request (#450): timestamps cannot tell two administrators' runs apart. + * Anything but 32 hex characters is ignored. + */ + public function setAttemptId(string $attemptId): void + { + $this->attemptId = preg_match('/^[a-f0-9]{32}$/', $attemptId) === 1 ? $attemptId : ''; + } + /** * Write how the update this request ran ended, for the status endpoint. * Atomic (temp file + rename); a write that fails is logged and skipped: @@ -4428,6 +4443,7 @@ private function recordUpdateOutcome(array $result): void $file = $this->rootPath . '/storage/cache/update-outcome.json'; $payload = json_encode([ 'at' => microtime(true), + 'attempt' => $this->attemptId, 'success' => !empty($result['success']), 'error' => (string) ($result['error'] ?? ''), 'version' => $this->getCurrentVersion(), @@ -4442,7 +4458,7 @@ private function recordUpdateOutcome(array $result): void /** * How the latest update run ended (see recordUpdateOutcome()), or null. * - * @return array{at:float,success:bool,error:string,version:string}|null + * @return array{at:float,attempt:string,success:bool,error:string,version:string}|null */ public function lastUpdateOutcome(): ?array { @@ -4453,6 +4469,7 @@ public function lastUpdateOutcome(): ?array } return [ 'at' => (float) $data['at'], + 'attempt' => (string) ($data['attempt'] ?? ''), 'success' => !empty($data['success']), 'error' => (string) ($data['error'] ?? ''), 'version' => (string) ($data['version'] ?? ''), diff --git a/app/Views/admin/updates.php b/app/Views/admin/updates.php index 01bd8f7b6..4a7e15ea4 100644 --- a/app/Views/admin/updates.php +++ b/app/Views/admin/updates.php @@ -852,26 +852,36 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text * connection, while PHP carries on to the end. In those cases the outcome is * read from /admin/updates/status instead of being reported as a failure. * `onWait` is told once that the page is now waiting for the server. + * The request carries an identifier the server writes next to the outcome, + * so the page recognises its own outcome and not another administrator's. */ async function runInstallRequest(body, targetVersion, onWait) { + const attempt = newAttemptId(); const before = await fetchUpdateStatus(); let response; try { response = await fetch(window.BASE_PATH + '/admin/updates/install-manual', { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', 'Accept': 'application/json' }, - body + body: body + '&attempt=' + attempt }); } catch (networkError) { - return waitForUpdateOutcome(before, targetVersion, onWait); + return waitForUpdateOutcome(before, targetVersion, onWait, attempt); } const contentType = response.headers.get('content-type') || ''; if (!contentType.includes('application/json') && [502, 503, 504, 520, 522, 524].includes(response.status)) { - return waitForUpdateOutcome(before, targetVersion, onWait); + return waitForUpdateOutcome(before, targetVersion, onWait, attempt); } return readUpdateJson(response); } +/** 32 random hex characters naming one install request. */ +function newAttemptId() { + const bytes = new Uint8Array(16); + crypto.getRandomValues(bytes); + return Array.from(bytes, b => b.toString(16).padStart(2, '0')).join(''); +} + /** The update status, or null when it cannot be read right now. */ async function fetchUpdateStatus() { try { @@ -893,18 +903,18 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text /** * Poll until the server is done with the update, then answer as the install * request would have: {success, message} or {success: false, error}. - * Done means the update lock is free. The attempt logged after `before` - * decides; without one, the installed version does. + * Done means the update lock is free. The outcome written for `attempt` + * decides; when there is none (the write failed), the attempt logged after + * `before`, then the installed version. */ -async function waitForUpdateOutcome(before, targetVersion, onWait) { +async function waitForUpdateOutcome(before, targetVersion, onWait, attempt) { if (typeof onWait === 'function') { onWait(); } - // Without a reading from before the request, an older attempt could pass - // for this one: then only the installed version decides. + // The log has no attempt identifier: without a reading from before the + // request, an older row could pass for this one, so it is not trusted. const trustLog = before !== null; const beforeId = before && before.last ? before.last.id : 0; - const beforeOutcomeAt = before && before.outcome ? before.outcome.at : 0; const deadline = Date.now() + 30 * 60 * 1000; let idlePolls = 0; while (Date.now() < deadline) { @@ -916,7 +926,7 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text } // The run's own outcome covers every failure, including those before // the install step that leave no log row (space, backup, package). - const outcome = trustLog && status.outcome && status.outcome.at > beforeOutcomeAt ? status.outcome : null; + const outcome = attempt && status.outcome && status.outcome.attempt === attempt ? status.outcome : null; if (outcome) { return outcome.success ? { success: true, message: } diff --git a/storage/plugins/emeroteca/src/Views/article-show.php b/storage/plugins/emeroteca/src/Views/article-show.php index 8f6128211..8f67ba465 100644 --- a/storage/plugins/emeroteca/src/Views/article-show.php +++ b/storage/plugins/emeroteca/src/Views/article-show.php @@ -279,7 +279,7 @@ class="inline-flex items-center px-2 py-1 rounded-full text-sm bg-gray-100 text- -

    :

    +

    :

    diff --git a/tests/emeroteca-articles-453-455.spec.js b/tests/emeroteca-articles-453-455.spec.js index 753c8dfc9..576d39bac 100644 --- a/tests/emeroteca-articles-453-455.spec.js +++ b/tests/emeroteca-articles-453-455.spec.js @@ -214,7 +214,7 @@ test.describe.serial('Articles like books (#453, #454, #455)', () => { await admin.reload(); await expect(admin.getByTestId('article-resource')).toContainText('arkiv/1988/petersen.pdf'); await expect(admin.locator('a[href*="arkiv/1988"]')).toHaveCount(0); - await expect(page).toContainText('Kun på læsesalen'); + await expect(admin.getByTestId('article-resource-access')).toContainText('Kun på læsesalen'); } finally { db(`UPDATE emeroteca_contributi SET risorsa_url=NULL, risorsa_accesso=NULL WHERE id=${articleId}`); } diff --git a/tests/update-pending-package.unit.php b/tests/update-pending-package.unit.php index b5ec3606e..f9b63893d 100644 --- a/tests/update-pending-package.unit.php +++ b/tests/update-pending-package.unit.php @@ -28,6 +28,8 @@ public function checkRequirements(): array return ['met' => true, 'requirements' => []]; } + public function setAttemptId(string $attemptId): void {} + public function performUpdateFromFile(string $path): array { self::$installed[] = $path; diff --git a/tests/update-proxy-drop-450.spec.js b/tests/update-proxy-drop-450.spec.js index 1dc971032..cf90c994a 100644 --- a/tests/update-proxy-drop-450.spec.js +++ b/tests/update-proxy-drop-450.spec.js @@ -24,16 +24,39 @@ async function login(page) { await page.waitForURL(u => !u.pathname.includes('accedi') && !u.pathname.includes('login')); } +/** The identifier the page sent with its install request, once it has. */ +let sentAttempt = ''; + +/** + * Play the proxy on the install request, keeping the attempt identifier the + * page sent with it (the server would write it next to the outcome). + * @param {import('@playwright/test').Page} page + * @param {'502'|'reset'} how + */ +async function routeInstall(page, how) { + sentAttempt = ''; + await page.route('**/admin/updates/install-manual', route => { + sentAttempt = new URLSearchParams(route.request().postData() || '').get('attempt') || ''; + return how === 'reset' ? route.abort('connectionreset') : route.fulfill(PROXY_502); + }); +} + /** - * Serve the status replies in order, the last one for every later poll. + * Serve the status replies in order, the last one for every later poll. A + * reply may be a function of the attempt identifier the page sent; `null` + * answers 500, a status that cannot be read. * @param {import('@playwright/test').Page} page - * @param {object[]} replies + * @param {(object|null|((attempt: string) => object))[]} replies */ async function routeStatus(page, replies) { let call = 0; await page.route('**/admin/updates/status', route => { - const body = replies[Math.min(call, replies.length - 1)]; + const reply = replies[Math.min(call, replies.length - 1)]; call++; + if (reply === null) { + return route.fulfill({ status: 500, contentType: 'text/html', body: 'Internal Server Error' }); + } + const body = typeof reply === 'function' ? reply(sentAttempt) : reply; return route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(body) }); }); } @@ -86,7 +109,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeInstall(page, '502'); await routeStatus(page, [ { success: true, version: '0.7.93', running: false, last: { id: 40, to_version: '0.7.93', status: 'completed', error: '' } }, { success: true, version: '0.7.93', running: true, last: { id: 41, to_version: TARGET, status: 'started', error: '' } }, @@ -102,7 +125,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeInstall(page, '502'); await routeStatus(page, [ { success: true, version: '0.7.93', running: false, last: { id: 50, to_version: '0.7.93', status: 'completed', error: '' } }, { success: true, version: '0.7.93', running: false, last: { id: 51, to_version: TARGET, status: 'failed', error: 'Errore nella copia del file: probe450' } }, @@ -116,7 +139,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await page.route('**/admin/updates/install-manual', route => route.abort('connectionreset')); + await routeInstall(page, 'reset'); await routeStatus(page, [ { success: true, version: '0.7.93', running: false, last: null }, { success: true, version: TARGET, running: false, last: null }, @@ -129,11 +152,11 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeInstall(page, '502'); // The run's outcome file is the only trace: no new update_logs row. await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 1000, success: true, error: '', version: '0.7.93' } }, - { success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 2000, success: false, error: 'Spazio insufficiente probe450', version: '0.7.93' } }, + { success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 1000, attempt: 'a'.repeat(32), success: true, error: '', version: '0.7.93' } }, + attempt => ({ success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 2000, attempt, success: false, error: 'Spazio insufficiente probe450', version: '0.7.93' } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); @@ -144,7 +167,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeInstall(page, '502'); await routeStatus(page, [ { success: true, version: '0.7.93', running: false, last: { id: 80, to_version: '0.7.93', status: 'completed', error: '' }, outcome: null }, { success: true, version: '0.7.93', running: false, last: { id: 81, to_version: TARGET, status: 'started', error: '' }, outcome: null }, @@ -158,7 +181,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await page.route('**/admin/updates/install-manual', route => route.fulfill(PROXY_502)); + await routeInstall(page, '502'); // Nothing new is ever logged and the version never moves. await routeStatus(page, [ { success: true, version: '0.7.93', running: false, last: { id: 60, to_version: '0.7.93', status: 'completed', error: '' } }, @@ -167,4 +190,36 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 40_000 }); await expect(page.locator('#updateMessage')).toContainText('non risulta completato'); }); + + test('8 Another administrator\'s outcome is not taken for this one, even if newer', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await routeInstall(page, '502'); + const other = 'b'.repeat(32); + await routeStatus(page, [ + { success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: null }, + // A second run ended first, with an error that is not this page's. + { success: true, version: '0.7.93', running: true, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 3000, attempt: other, success: false, error: 'not mine probe450', version: '0.7.93' } }, + attempt => ({ success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 4000, attempt, success: true, error: '', version: '0.7.93' } }), + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); + await expect(page.locator('#updateMessage')).not.toContainText('not mine'); + expect(sentAttempt, 'the install request carried an identifier').toMatch(/^[a-f0-9]{32}$/); + }); + + test('9 A status that cannot be read before the request does not hide this run\'s success', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await routeInstall(page, '502'); + // The version never reaches the target: only the outcome can tell. + await routeStatus(page, [ + null, + attempt => ({ success: true, version: '0.7.93', running: false, last: null, outcome: { at: 5000, attempt, success: true, error: '', version: '0.7.93' } }), + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); + }); }); diff --git a/tests/update-status-450.unit.php b/tests/update-status-450.unit.php index f25ceac3c..f06703177 100644 --- a/tests/update-status-450.unit.php +++ b/tests/update-status-450.unit.php @@ -133,6 +133,15 @@ $check(($outcome['success'] ?? null) === true && (float) $outcome['at'] > $first, 'a later run replaces it, with a later time'); $check(($outcome['version'] ?? '') === $updater->getCurrentVersion(), 'it names the version installed when it ended'); $check(glob($outcomeFile . '.*.tmp') === [], 'the write leaves no temp file behind'); + $check(($outcome['attempt'] ?? null) === '', 'a run started without an identifier records none'); + $attempt = bin2hex(random_bytes(16)); + $updater->setAttemptId($attempt); + $record->invoke($updater, ['success' => true, 'error' => null]); + $check(($updater->lastUpdateOutcome()['attempt'] ?? '') === $attempt, 'the outcome carries the identifier the page sent'); + $updater->setAttemptId('../../etc/passwd'); + $record->invoke($updater, ['success' => true, 'error' => null]); + $check(($updater->lastUpdateOutcome()['attempt'] ?? null) === '', 'an identifier that is not 32 hex characters is dropped'); + $updater->setAttemptId(''); file_put_contents($outcomeFile, 'not json'); $check($updater->lastUpdateOutcome() === null, 'an unreadable file reads as no outcome'); // Written before the lock is released, inside performUpdateFromFile(). From a864c42d8f94ec8b020dcef6f3d4c59a179aef61 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 7 Oct 2026 00:34:36 +0200 Subject: [PATCH 4/7] Note the update attempt identifier in the 0.7.94 changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d220a4e63..b6f9b8583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Full version-by-version history for Pinakes. The README shows only the latest re - **`GET /admin/updates/status`** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)): the installed version, whether the update lock is held, and the latest update attempt with its status and error, and the outcome of the latest run. Admin only (staff get 403), never cached, reachable during maintenance. ### Fixed -- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports how the run ended: success, or the error it stopped on. The run writes its outcome before it releases the lock, so a failure before the install step (space, backup, extraction, package checks) is reported with its own message too, and an attempt the server left half-way is reported as interrupted. The install request releases the PHP session before the update runs, so those polls are not held behind it. +- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports how the run ended: success, or the error it stopped on. The page sends an identifier with its install request and the run writes it next to its outcome, so the page reads its own result and never another administrator's update. The run writes its outcome before it releases the lock, so a failure before the install step (space, backup, extraction, package checks) is reported with its own message too, and an attempt the server left half-way is reported as interrupted. The install request releases the PHP session before the update runs, so those polls are not held behind it. - **Only the most specific admin menu entry is highlighted.** On an article page both Periodicals and Articles lit up, because the address of one starts with the other's. The Emeroteca plugin goes to 1.12.1. No migration. From 2df5be9b0dd991efdcca6a67d4f999e977a9213f Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 7 Oct 2026 00:37:31 +0200 Subject: [PATCH 5/7] Make the foreign-outcome test fail without the attempt check: free the lock on that reply --- tests/update-proxy-drop-450.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/update-proxy-drop-450.spec.js b/tests/update-proxy-drop-450.spec.js index cf90c994a..dafd8dcd5 100644 --- a/tests/update-proxy-drop-450.spec.js +++ b/tests/update-proxy-drop-450.spec.js @@ -199,8 +199,9 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 const other = 'b'.repeat(32); await routeStatus(page, [ { success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: null }, - // A second run ended first, with an error that is not this page's. - { success: true, version: '0.7.93', running: true, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 3000, attempt: other, success: false, error: 'not mine probe450', version: '0.7.93' } }, + // A second run ended first, with an error that is not this page's, and + // the lock is free: a timestamp check would take this outcome. + { success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 3000, attempt: other, success: false, error: 'not mine probe450', version: '0.7.93' } }, attempt => ({ success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 4000, attempt, success: true, error: '', version: '0.7.93' } }), ]); await startUpdate(page); From 90e4ecbde08762eabd82b9913d94df6252fc8f67 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 7 Oct 2026 01:17:30 +0200 Subject: [PATCH 6/7] Decide an update's outcome only on what the server filed under its attempt The server keeps a small per-attempt record (update_logs row and outcome, the 10 most recent) while it holds the update lock, and the status endpoint returns the one for ?attempt=. The page trusts only that: a newer log row or the installed version can belong to another administrator's run, so with nothing filed under its attempt the page now says the outcome is to check, with the installed version, instead of reporting a success. A gateway error whose body is not readable JSON (empty, cut short) starts the wait instead of failing on the parse. --- app/Controllers/UpdateController.php | 2 + app/Support/Updater.php | 107 ++++++++++++++++++++++++++ app/Views/admin/updates.php | 91 +++++++++++++--------- locale/da_DK.json | 4 +- locale/de_DE.json | 4 +- locale/en_US.json | 4 +- locale/fr_FR.json | 4 +- locale/it_IT.json | 4 +- tests/update-proxy-drop-450.spec.js | 110 +++++++++++++++++---------- tests/update-status-450.unit.php | 73 ++++++++++++++++++ 10 files changed, 325 insertions(+), 78 deletions(-) diff --git a/app/Controllers/UpdateController.php b/app/Controllers/UpdateController.php index 96ec23c11..79bd8022a 100644 --- a/app/Controllers/UpdateController.php +++ b/app/Controllers/UpdateController.php @@ -897,6 +897,8 @@ public function status(Request $request, Response $response, mysqli $db): Respon 'running' => $updater->isUpdateRunning(), 'last' => $updater->lastUpdateAttempt(), 'outcome' => $updater->lastUpdateOutcome(), + // This page's own install, by the identifier it sent with it. + 'attempt' => $updater->attemptStatus((string) ($request->getQueryParams()['attempt'] ?? '')), ])->withHeader('Cache-Control', 'no-store'); } diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 29f72fdee..510be78e6 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -4355,6 +4355,12 @@ private function logUpdateStart(string $fromVersion, string $toVersion, ?string $id = $this->db->insert_id; $stmt->close(); + // The page that started this install finds its log row by its + // attempt identifier (#450). Backups share the table, not the role. + if ($toVersion !== 'backup' && $id > 0) { + $this->rememberAttempt(['log_id' => (int) $id]); + } + return $id; } catch (\Throwable $e) { $this->debugLog('WARNING', 'Log update start fallito', ['error' => $e->getMessage()]); @@ -4453,6 +4459,107 @@ private function recordUpdateOutcome(array $result): void @unlink($tmp); $this->debugLog('WARNING', 'Esito aggiornamento non registrato', ['file' => $file]); } + $this->rememberAttempt(['outcome' => [ + 'success' => !empty($result['success']), + 'error' => (string) ($result['error'] ?? ''), + 'version' => $this->getCurrentVersion(), + ]]); + } + + /** Most recent attempts kept in update-attempts.json; older ones drop off. */ + private const ATTEMPTS_KEPT = 10; + + /** + * Merge $fields into this run's entry of storage/cache/update-attempts.json, + * the per-attempt record the update page reads (#450): its update_logs row + * and its outcome, under the identifier the page sent. A single "latest" + * record cannot tell two administrators' runs apart. Called while the + * update lock is held, so writers never overlap. Without an identifier + * there is nothing to file it under; a failed write is logged and skipped. + * + * @param array $fields + */ + private function rememberAttempt(array $fields): void + { + if ($this->attemptId === '') { + return; + } + $file = $this->rootPath . '/storage/cache/update-attempts.json'; + $raw = is_file($file) ? @file_get_contents($file) : ''; + $attempts = is_string($raw) && $raw !== '' ? json_decode($raw, true) : []; + $attempts = is_array($attempts) ? $attempts : []; + $entry = is_array($attempts[$this->attemptId] ?? null) ? $attempts[$this->attemptId] : []; + unset($attempts[$this->attemptId]); + $attempts[$this->attemptId] = array_merge($entry, $fields, ['at' => microtime(true)]); + $attempts = array_slice($attempts, -self::ATTEMPTS_KEPT, null, true); + $payload = json_encode($attempts, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); + $tmp = $file . '.' . bin2hex(random_bytes(4)) . '.tmp'; + if ($payload === false || @file_put_contents($tmp, $payload) === false || !@rename($tmp, $file)) { + @unlink($tmp); + $this->debugLog('WARNING', 'Tentativo di aggiornamento non registrato', ['file' => $file]); + } + } + + /** + * What is known of one install attempt (see rememberAttempt()): its + * update_logs row and its outcome, each null until written. Null when the + * identifier is malformed or unknown. + * + * @return array{log:array{id:int,to_version:string,status:string,error:string}|null,outcome:array{success:bool,error:string,version:string}|null}|null + */ + public function attemptStatus(string $attemptId): ?array + { + if (preg_match('/^[a-f0-9]{32}$/', $attemptId) !== 1) { + return null; + } + $raw = @file_get_contents($this->rootPath . '/storage/cache/update-attempts.json'); + $attempts = is_string($raw) ? json_decode($raw, true) : null; + $entry = is_array($attempts) && is_array($attempts[$attemptId] ?? null) ? $attempts[$attemptId] : null; + if ($entry === null) { + return null; + } + $outcome = is_array($entry['outcome'] ?? null) ? [ + 'success' => !empty($entry['outcome']['success']), + 'error' => (string) ($entry['outcome']['error'] ?? ''), + 'version' => (string) ($entry['outcome']['version'] ?? ''), + ] : null; + $log = isset($entry['log_id']) ? $this->updateLogRow((int) $entry['log_id']) : null; + return ['log' => $log, 'outcome' => $outcome]; + } + + /** + * One update_logs row by id, or null when it is missing or unreadable. + * + * @return array{id:int,to_version:string,status:string,error:string}|null + */ + private function updateLogRow(int $id): ?array + { + if ($id <= 0) { + return null; + } + try { + $stmt = $this->db->prepare('SELECT id, to_version, status, error_message FROM update_logs WHERE id = ?'); + if ($stmt === false) { + return null; + } + $stmt->bind_param('i', $id); + $stmt->execute(); + $result = $stmt->get_result(); + $row = $result instanceof \mysqli_result ? $result->fetch_assoc() : null; + $stmt->close(); + if (!is_array($row)) { + return null; + } + return [ + 'id' => (int) $row['id'], + 'to_version' => (string) $row['to_version'], + 'status' => (string) $row['status'], + 'error' => (string) ($row['error_message'] ?? ''), + ]; + } catch (\Throwable $e) { + $this->debugLog('WARNING', 'Lettura riga aggiornamento fallita', ['error' => $e->getMessage()]); + return null; + } } /** diff --git a/app/Views/admin/updates.php b/app/Views/admin/updates.php index 4a7e15ea4..40a3f4c5e 100644 --- a/app/Views/admin/updates.php +++ b/app/Views/admin/updates.php @@ -788,7 +788,6 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text setStepActive('backup'); const data = await runInstallRequest( `csrf_token=${encodeURIComponent(csrfToken)}&package=${encodeURIComponent(downloadData.package || '')}`, - version, () => { document.getElementById('updateMessage').textContent = ; } @@ -807,6 +806,13 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text document.getElementById('updateIcon').className = 'w-20 h-20 bg-green-100 rounded-full flex items-center justify-center mx-auto mb-4'; document.getElementById('updateTitle').textContent = ; document.getElementById('updateMessage').textContent = ; + } else if (data.indeterminate) { + // The page lost the connection and the server left nothing it can + // tie to this run: say what is known, not that it failed. + document.getElementById('updateIcon').innerHTML = ''; + document.getElementById('updateIcon').className = 'w-20 h-20 bg-amber-100 rounded-full flex items-center justify-center mx-auto mb-4'; + document.getElementById('updateTitle').textContent = ; + document.getElementById('updateMessage').textContent = data.error; } else { setStepFailed('backup'); document.getElementById('updateIcon').innerHTML = ''; @@ -852,12 +858,11 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text * connection, while PHP carries on to the end. In those cases the outcome is * read from /admin/updates/status instead of being reported as a failure. * `onWait` is told once that the page is now waiting for the server. - * The request carries an identifier the server writes next to the outcome, - * so the page recognises its own outcome and not another administrator's. + * The request carries an identifier the server files its log row and outcome + * under, so the page reads its own run and never another administrator's. */ -async function runInstallRequest(body, targetVersion, onWait) { +async function runInstallRequest(body, onWait) { const attempt = newAttemptId(); - const before = await fetchUpdateStatus(); let response; try { response = await fetch(window.BASE_PATH + '/admin/updates/install-manual', { @@ -866,11 +871,20 @@ class="inline-flex items-center justify-center w-full px-6 py-3 bg-gray-800 text body: body + '&attempt=' + attempt }); } catch (networkError) { - return waitForUpdateOutcome(before, targetVersion, onWait, attempt); + return waitForUpdateOutcome(attempt, onWait); } - const contentType = response.headers.get('content-type') || ''; - if (!contentType.includes('application/json') && [502, 503, 504, 520, 522, 524].includes(response.status)) { - return waitForUpdateOutcome(before, targetVersion, onWait, attempt); + if ([502, 503, 504, 520, 522, 524].includes(response.status)) { + // A gateway answer: only a readable JSON body from the app itself + // counts. A proxy's page, or JSON cut short or empty, means the + // request was dropped on the way while the update may carry on. + const text = await response.text(); + let data = null; + try { + data = JSON.parse(text); + } catch (e) { + data = null; + } + return data && typeof data === 'object' && 'success' in data ? data : waitForUpdateOutcome(attempt, onWait); } return readUpdateJson(response); } @@ -882,10 +896,10 @@ function newAttemptId() { return Array.from(bytes, b => b.toString(16).padStart(2, '0')).join(''); } -/** The update status, or null when it cannot be read right now. */ -async function fetchUpdateStatus() { +/** The update status for `attempt`, or null when it cannot be read right now. */ +async function fetchUpdateStatus(attempt) { try { - const response = await fetch(window.BASE_PATH + '/admin/updates/status', { + const response = await fetch(window.BASE_PATH + '/admin/updates/status?attempt=' + encodeURIComponent(attempt), { headers: { 'Accept': 'application/json' }, cache: 'no-store' }); @@ -903,51 +917,53 @@ function newAttemptId() { /** * Poll until the server is done with the update, then answer as the install * request would have: {success, message} or {success: false, error}. - * Done means the update lock is free. The outcome written for `attempt` - * decides; when there is none (the write failed), the attempt logged after - * `before`, then the installed version. + * Done means the update lock is free. Only what the server filed under + * `attempt` decides: the run's outcome, else its update_logs row. With + * neither, the result cannot be told (another administrator's run, or a run + * that never reached its log), and the page says so with the installed + * version instead of guessing: {success: false, indeterminate: true, error}. */ -async function waitForUpdateOutcome(before, targetVersion, onWait, attempt) { +async function waitForUpdateOutcome(attempt, onWait) { if (typeof onWait === 'function') { onWait(); } - // The log has no attempt identifier: without a reading from before the - // request, an older row could pass for this one, so it is not trusted. - const trustLog = before !== null; - const beforeId = before && before.last ? before.last.id : 0; const deadline = Date.now() + 30 * 60 * 1000; let idlePolls = 0; while (Date.now() < deadline) { await sleep(5000); - const status = await fetchUpdateStatus(); + const status = await fetchUpdateStatus(attempt); if (!status || status.running) { idlePolls = 0; continue; } + const mine = status.attempt || null; // The run's own outcome covers every failure, including those before // the install step that leave no log row (space, backup, package). - const outcome = attempt && status.outcome && status.outcome.attempt === attempt ? status.outcome : null; - if (outcome) { - return outcome.success + if (mine && mine.outcome) { + return mine.outcome.success ? { success: true, message: } - : { success: false, error: outcome.error || }; + : { success: false, error: mine.outcome.error || }; } - const last = trustLog && status.last && status.last.id > beforeId ? status.last : null; - if ((last && last.status === 'completed') || (targetVersion && status.version === targetVersion)) { + const log = mine && mine.log ? mine.log : null; + if (log && log.status === 'completed') { return { success: true, message: }; } - if (last && (last.status === 'failed' || last.status === 'rolled_back')) { - return { success: false, error: last.error || }; + if (log && (log.status === 'failed' || log.status === 'rolled_back')) { + return { success: false, error: log.error || }; } // Started, never finished, and the lock is free: the server process // ended mid-update (a fatal error, memory, a killed worker). - if (last && last.status === 'started') { + if (log && log.status === 'started') { return { success: false, error: }; } - // Not running and nothing settled: give the update a moment to take - // the lock or write its log, then call it. + // Not running and nothing filed under this attempt: give the update a + // moment to take the lock or write its record, then say what is known. if (++idlePolls >= 3) { - return { success: false, error: }; + return { + success: false, + indeterminate: true, + error: .replace('%s', status.version || '?') + }; } } return { success: false, error: }; @@ -1651,7 +1667,6 @@ function removeUploadedFile() { const installData = await runInstallRequest( `csrf_token=${encodeURIComponent(csrfToken)}&package=${encodeURIComponent(uploadData.package || '')}`, - '', () => { Swal.update({ title: , @@ -1675,6 +1690,14 @@ function removeUploadedFile() { // Reset uppy uppyManualUpdate.cancelAll(); uploadedFile = null; + } else if (installData.indeterminate) { + Swal.fire({ + icon: 'warning', + title: , + text: installData.error + }); + btnText.textContent = originalText; + submitBtn.disabled = false; } else { throw new Error(installData.error || ); } diff --git a/locale/da_DK.json b/locale/da_DK.json index 9e35e3346..19b542ba9 100644 --- a/locale/da_DK.json +++ b/locale/da_DK.json @@ -8057,5 +8057,7 @@ "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "Forbindelsen blev afbrudt, men opdateringen fortsætter på serveren. Venter på, at den bliver færdig…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "Opdateringen ser ikke ud til at være fuldført. Se loggene for detaljer.", "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Opdateringen kører stadig på serveren efter 30 minutter. Genindlæs siden senere for at se den installerede version.", - "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "Opdateringen stoppede på serveren, før den blev færdig. Se loggene for detaljer." + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "Opdateringen stoppede på serveren, før den blev færdig. Se loggene for detaljer.", + "Esito da verificare": "Resultatet skal kontrolleres", + "Non riesco a stabilire come è finito questo aggiornamento. La versione installata ora è %s: controlla il registro degli aggiornamenti.": "Jeg kan ikke afgøre, hvordan denne opdatering endte. Den installerede version er nu %s: tjek opdateringshistorikken." } diff --git a/locale/de_DE.json b/locale/de_DE.json index a4c8e4304..966f93c73 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -8057,5 +8057,7 @@ "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "Die Verbindung wurde unterbrochen, aber das Update läuft auf dem Server weiter. Es wird gewartet, bis es abgeschlossen ist…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "Das Update scheint nicht abgeschlossen zu sein. Prüfen Sie die Protokolle für Details.", "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "Das Update läuft nach 30 Minuten noch auf dem Server. Laden Sie die Seite später neu, um die installierte Version zu sehen.", - "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "Das Update wurde auf dem Server vor dem Abschluss unterbrochen. Prüfen Sie die Protokolle für Details." + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "Das Update wurde auf dem Server vor dem Abschluss unterbrochen. Prüfen Sie die Protokolle für Details.", + "Esito da verificare": "Ergebnis prüfen", + "Non riesco a stabilire come è finito questo aggiornamento. La versione installata ora è %s: controlla il registro degli aggiornamenti.": "Ich kann nicht feststellen, wie dieses Update geendet hat. Die installierte Version ist jetzt %s: Prüfen Sie den Update-Verlauf." } diff --git a/locale/en_US.json b/locale/en_US.json index c302102e6..88fe61fb1 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -8057,5 +8057,7 @@ "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "The connection dropped, but the update is still running on the server. Waiting for it to finish…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "The update does not appear to have completed. Check the logs for details.", "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "The update is still running on the server after 30 minutes. Reload the page later to see the installed version.", - "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "The update stopped on the server before it finished. Check the logs for details." + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "The update stopped on the server before it finished. Check the logs for details.", + "Esito da verificare": "Outcome to check", + "Non riesco a stabilire come è finito questo aggiornamento. La versione installata ora è %s: controlla il registro degli aggiornamenti.": "I cannot tell how this update ended. The installed version is now %s: check the update history." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 641821d79..6248d862b 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -8057,5 +8057,7 @@ "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "La connexion a été interrompue, mais la mise à jour se poursuit sur le serveur. Attente de la fin…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "La mise à jour ne semble pas terminée. Consultez les journaux pour plus de détails.", "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "La mise à jour est toujours en cours sur le serveur après 30 minutes. Rechargez la page plus tard pour voir la version installée.", - "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "La mise à jour s'est interrompue sur le serveur avant la fin. Consultez les journaux pour plus de détails." + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "La mise à jour s'est interrompue sur le serveur avant la fin. Consultez les journaux pour plus de détails.", + "Esito da verificare": "Résultat à vérifier", + "Non riesco a stabilire come è finito questo aggiornamento. La versione installata ora è %s: controlla il registro degli aggiornamenti.": "Je ne peux pas déterminer comment cette mise à jour s'est terminée. La version installée est maintenant %s : consultez l'historique des mises à jour." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 9518fc9b1..8c650138b 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -8057,5 +8057,7 @@ "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…": "La connessione si è interrotta, ma l'aggiornamento continua sul server. Attendo che finisca…", "L'aggiornamento non risulta completato. Controlla i log per i dettagli.": "L'aggiornamento non risulta completato. Controlla i log per i dettagli.", "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.": "L'aggiornamento è ancora in corso sul server dopo 30 minuti. Ricarica la pagina più tardi per vedere la versione installata.", - "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli." + "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.": "L'aggiornamento si è interrotto sul server prima di finire. Controlla i log per i dettagli.", + "Esito da verificare": "Esito da verificare", + "Non riesco a stabilire come è finito questo aggiornamento. La versione installata ora è %s: controlla il registro degli aggiornamenti.": "Non riesco a stabilire come è finito questo aggiornamento. La versione installata ora è %s: controlla il registro degli aggiornamenti." } diff --git a/tests/update-proxy-drop-450.spec.js b/tests/update-proxy-drop-450.spec.js index dafd8dcd5..e2dfaac08 100644 --- a/tests/update-proxy-drop-450.spec.js +++ b/tests/update-proxy-drop-450.spec.js @@ -5,9 +5,11 @@ * to its end. The page reported an error for an update that had succeeded. * * The page now reads the outcome from /admin/updates/status when the install - * request comes back as a gateway error or a dropped connection. The proxy is - * simulated by answering the browser's install request ourselves; the status - * replies play the server finishing (or failing) behind it. No update runs. + * request comes back as a gateway error or a dropped connection. It sends an + * attempt identifier with the install request and only trusts what the server + * filed under it (`attempt: {log, outcome}`). The proxy is simulated by + * answering the browser's install request ourselves; the status replies play + * the server finishing (or failing) behind it. No update runs. */ const { test, expect } = require('@playwright/test'); const fs = require('fs'); @@ -31,13 +33,31 @@ let sentAttempt = ''; * Play the proxy on the install request, keeping the attempt identifier the * page sent with it (the server would write it next to the outcome). * @param {import('@playwright/test').Page} page - * @param {'502'|'reset'} how + * @param {'502'|'502-empty-json'|'reset'} how */ async function routeInstall(page, how) { sentAttempt = ''; await page.route('**/admin/updates/install-manual', route => { sentAttempt = new URLSearchParams(route.request().postData() || '').get('attempt') || ''; - return how === 'reset' ? route.abort('connectionreset') : route.fulfill(PROXY_502); + if (how === 'reset') return route.abort('connectionreset'); + if (how === '502-empty-json') return route.fulfill({ status: 502, contentType: 'application/json', body: '' }); + return route.fulfill(PROXY_502); + }); +} + +/** + * A status reply, with what the server filed under the page's attempt. + * @param {{version?: string, running?: boolean, last?: object|null, outcome?: object|null, mine?: object|null}} o + */ +function status(o) { + return (/** @type {string} */ attempt) => ({ + success: true, + version: o.version || '0.7.93', + running: !!o.running, + last: o.last === undefined ? null : o.last, + outcome: o.outcome === undefined ? null : o.outcome, + attempt: o.mine ? { log: null, outcome: null, ...o.mine } : null, + _for: attempt, }); } @@ -50,7 +70,7 @@ async function routeInstall(page, how) { */ async function routeStatus(page, replies) { let call = 0; - await page.route('**/admin/updates/status', route => { + await page.route(url => url.pathname.endsWith('/admin/updates/status'), route => { const reply = replies[Math.min(call, replies.length - 1)]; call++; if (reply === null) { @@ -87,7 +107,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 test('1 The status endpoint answers with the installed version and no update running', async ({ page }) => { await login(page); - const res = await page.request.get(`${BASE}/admin/updates/status`); + const res = await page.request.get(`${BASE}/admin/updates/status?attempt=${'c'.repeat(32)}`); expect(res.status()).toBe(200); expect(res.headers()['cache-control']).toContain('no-store'); const data = await res.json(); @@ -95,6 +115,7 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 expect(data.success).toBe(true); expect(data.version).toBe(installed); expect(data.running).toBe(false); + expect(data.attempt, 'an attempt the server never saw has nothing filed').toBeNull(); // A visitor gets nothing from it. const anonymous = await page.context().browser().newContext(); try { @@ -111,14 +132,14 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await routeDownload(page); await routeInstall(page, '502'); await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 40, to_version: '0.7.93', status: 'completed', error: '' } }, - { success: true, version: '0.7.93', running: true, last: { id: 41, to_version: TARGET, status: 'started', error: '' } }, - { success: true, version: TARGET, running: false, last: { id: 41, to_version: TARGET, status: 'completed', error: '' } }, + status({ running: true, mine: { log: { id: 41, to_version: TARGET, status: 'started', error: '' } } }), + status({ version: TARGET, mine: { log: { id: 41, to_version: TARGET, status: 'completed', error: '' } } }), ]); await startUpdate(page); await expect(page.locator('#updateMessage'), 'the page says it is waiting, not that it failed').toContainText('continua sul server', { timeout: 10_000 }); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); await expect(page.locator('[data-step="migrate"]')).not.toContainText('Errore'); + expect(sentAttempt, 'the install request carried an identifier').toMatch(/^[a-f0-9]{32}$/); }); test('3 A 502 from the proxy, then the update fails on the server: the page shows its error', async ({ page }) => { @@ -127,22 +148,21 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await routeDownload(page); await routeInstall(page, '502'); await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 50, to_version: '0.7.93', status: 'completed', error: '' } }, - { success: true, version: '0.7.93', running: false, last: { id: 51, to_version: TARGET, status: 'failed', error: 'Errore nella copia del file: probe450' } }, + status({ mine: { log: { id: 51, to_version: TARGET, status: 'failed', error: 'Errore nella copia del file: probe450' } } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); await expect(page.locator('#updateMessage')).toContainText('probe450'); }); - test('4 A dropped connection with no log entry: the installed version decides', async ({ page }) => { + test('4 A dropped connection: the run\'s own outcome decides', async ({ page }) => { await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); await routeInstall(page, 'reset'); await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: null }, - { success: true, version: TARGET, running: false, last: null }, + status({ running: true }), + status({ version: TARGET, mine: { outcome: { success: true, error: '', version: TARGET } } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); @@ -153,10 +173,9 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await page.goto(`${BASE}/admin/updates`); await routeDownload(page); await routeInstall(page, '502'); - // The run's outcome file is the only trace: no new update_logs row. + // The run's outcome is the only trace: no update_logs row of its own. await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 1000, attempt: 'a'.repeat(32), success: true, error: '', version: '0.7.93' } }, - attempt => ({ success: true, version: '0.7.93', running: false, last: { id: 70, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 2000, attempt, success: false, error: 'Spazio insufficiente probe450', version: '0.7.93' } }), + status({ mine: { log: null, outcome: { success: false, error: 'Spazio insufficiente probe450', version: '0.7.93' } } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); @@ -169,58 +188,71 @@ test.describe.serial('Update survives a proxy that drops the install request (#4 await routeDownload(page); await routeInstall(page, '502'); await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 80, to_version: '0.7.93', status: 'completed', error: '' }, outcome: null }, - { success: true, version: '0.7.93', running: false, last: { id: 81, to_version: TARGET, status: 'started', error: '' }, outcome: null }, + status({ mine: { log: { id: 81, to_version: TARGET, status: 'started', error: '' }, outcome: null } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); await expect(page.locator('#updateMessage')).toContainText('interrotto sul server'); }); - test('5 An older completed update in the log is not taken for this one', async ({ page }) => { + test('8 Another administrator\'s run, newer and finished, is not taken for this one', async ({ page }) => { await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); await routeInstall(page, '502'); - // Nothing new is ever logged and the version never moves. + const foreignLog = { id: 91, to_version: TARGET, status: 'completed', error: '' }; + const foreignOutcome = { at: 3000, attempt: 'b'.repeat(32), success: false, error: 'not mine probe450', version: '0.7.93' }; await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 60, to_version: '0.7.93', status: 'completed', error: '' } }, + // The other run ended first, with the lock free: its log row and its + // outcome are the latest, and neither is filed under this attempt. + status({ last: foreignLog, outcome: foreignOutcome }), + status({ last: foreignLog, outcome: foreignOutcome, running: true }), + status({ last: foreignLog, outcome: foreignOutcome, mine: { outcome: { success: false, error: 'mine probe450', version: '0.7.93' } } }), ]); await startUpdate(page); - await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 40_000 }); - await expect(page.locator('#updateMessage')).toContainText('non risulta completato'); + await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento fallito', { timeout: 30_000 }); + await expect(page.locator('#updateMessage')).toContainText('mine probe450'); + await expect(page.locator('#updateMessage')).not.toContainText('not mine'); }); - test('8 Another administrator\'s outcome is not taken for this one, even if newer', async ({ page }) => { + test('9 A status that cannot be read at first does not hide this run\'s success', async ({ page }) => { await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); await routeInstall(page, '502'); - const other = 'b'.repeat(32); + // The version never reaches the target: only the outcome can tell. await routeStatus(page, [ - { success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: null }, - // A second run ended first, with an error that is not this page's, and - // the lock is free: a timestamp check would take this outcome. - { success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 3000, attempt: other, success: false, error: 'not mine probe450', version: '0.7.93' } }, - attempt => ({ success: true, version: '0.7.93', running: false, last: { id: 90, to_version: '0.7.93', status: 'completed', error: '' }, outcome: { at: 4000, attempt, success: true, error: '', version: '0.7.93' } }), + null, + status({ mine: { outcome: { success: true, error: '', version: '0.7.93' } } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); - await expect(page.locator('#updateMessage')).not.toContainText('not mine'); - expect(sentAttempt, 'the install request carried an identifier').toMatch(/^[a-f0-9]{32}$/); }); - test('9 A status that cannot be read before the request does not hide this run\'s success', async ({ page }) => { + test('10 A gateway error with an empty JSON body is waited out, not reported', async ({ page }) => { await login(page); await page.goto(`${BASE}/admin/updates`); await routeDownload(page); - await routeInstall(page, '502'); - // The version never reaches the target: only the outcome can tell. + await routeInstall(page, '502-empty-json'); await routeStatus(page, [ - null, - attempt => ({ success: true, version: '0.7.93', running: false, last: null, outcome: { at: 5000, attempt, success: true, error: '', version: '0.7.93' } }), + status({ version: TARGET, mine: { outcome: { success: true, error: '', version: TARGET } } }), ]); await startUpdate(page); await expect(page.locator('#updateTitle')).toHaveText('Aggiornamento completato!', { timeout: 30_000 }); }); + + test('5 Nothing filed under this attempt: the page says the outcome is to check, never a success', async ({ page }) => { + await login(page); + await page.goto(`${BASE}/admin/updates`); + await routeDownload(page); + await routeInstall(page, '502'); + // The installed version even reached the target, and the latest log row + // completed: neither is this run's, so neither may pass for its success. + await routeStatus(page, [ + status({ version: TARGET, last: { id: 60, to_version: TARGET, status: 'completed', error: '' } }), + ]); + await startUpdate(page); + await expect(page.locator('#updateTitle')).toHaveText('Esito da verificare', { timeout: 40_000 }); + await expect(page.locator('#updateMessage')).toContainText(TARGET); + }); }); diff --git a/tests/update-status-450.unit.php b/tests/update-status-450.unit.php index f06703177..55bc3d7e4 100644 --- a/tests/update-status-450.unit.php +++ b/tests/update-status-450.unit.php @@ -158,6 +158,77 @@ } } +echo "B3. each install attempt, by the identifier the page sent\n"; +$attemptsFile = $root . '/storage/cache/update-attempts.json'; +$savedAttempts = is_file($attemptsFile) ? file_get_contents($attemptsFile) : null; +$savedOutcome = is_file($outcomeFile) ? file_get_contents($outcomeFile) : null; +$attemptLogIds = []; +try { + @unlink($attemptsFile); + $logStart = new ReflectionMethod(Updater::class, 'logUpdateStart'); + $logStart->setAccessible(true); + $record = new ReflectionMethod(Updater::class, 'recordUpdateOutcome'); + $record->setAccessible(true); + $mine = bin2hex(random_bytes(16)); + $other = bin2hex(random_bytes(16)); + + $updater->setAttemptId($mine); + $attemptLogIds[] = $myLog = (int) $logStart->invoke($updater, '0.0.1', '9.9.9', null); + $status = $updater->attemptStatus($mine); + $check(($status['log']['id'] ?? 0) === $myLog && $status['log']['status'] === 'started' && $status['outcome'] === null, + 'the run\'s log row is filed under its attempt as soon as it opens'); + // A backup shares the table but is not the install the page waits for. + $attemptLogIds[] = (int) $logStart->invoke($updater, '0.0.1', 'backup', '/tmp/x'); + $check(($updater->attemptStatus($mine)['log']['id'] ?? 0) === $myLog, 'a backup row does not replace the run\'s row'); + $record->invoke($updater, ['success' => false, 'error' => 'disk full ' . $marker]); + $status = $updater->attemptStatus($mine); + $check(($status['outcome']['success'] ?? null) === false && ($status['outcome']['error'] ?? '') === 'disk full ' . $marker, + 'the run\'s outcome is filed next to its log row'); + + // Another administrator's run, later: it has its own entry, and the latest + // outcome is now theirs, but this attempt still reads its own. + $updater->setAttemptId($other); + $attemptLogIds[] = (int) $logStart->invoke($updater, '0.0.1', '9.9.9', null); + $record->invoke($updater, ['success' => true, 'error' => null]); + $check(($updater->lastUpdateOutcome()['attempt'] ?? '') === $other, 'the latest outcome is the other run\'s'); + $check(($updater->attemptStatus($mine)['outcome']['error'] ?? '') === 'disk full ' . $marker + && ($updater->attemptStatus($mine)['log']['id'] ?? 0) === $myLog, 'this attempt still reads its own row and outcome'); + $check(($updater->attemptStatus($other)['outcome']['success'] ?? null) === true, 'the other attempt reads its own'); + + $check($updater->attemptStatus(bin2hex(random_bytes(16))) === null, 'an attempt the server never saw has nothing filed'); + $check($updater->attemptStatus('../../etc/passwd') === null, 'a malformed identifier reads nothing'); + + // Only the most recent attempts are kept. + for ($i = 0; $i < 12; $i++) { + $updater->setAttemptId(bin2hex(random_bytes(16))); + $record->invoke($updater, ['success' => true, 'error' => null]); + } + $kept = json_decode((string) file_get_contents($attemptsFile), true); + $check(is_array($kept) && count($kept) === 10, 'the file keeps the 10 most recent attempts'); + $check($updater->attemptStatus($mine) === null, 'older attempts drop off'); + $check(glob($attemptsFile . '.*.tmp') === [], 'the writes leave no temp file behind'); + + // Without an identifier nothing is filed. + $updater->setAttemptId(''); + $before = (string) file_get_contents($attemptsFile); + $record->invoke($updater, ['success' => true, 'error' => null]); + $check((string) file_get_contents($attemptsFile) === $before, 'a run without an identifier files nothing'); +} finally { + $updater->setAttemptId(''); + foreach ($attemptLogIds as $id) { + if ($id > 0) { + $db->query('DELETE FROM update_logs WHERE id = ' . (int) $id); + } + } + foreach ([[$attemptsFile, $savedAttempts], [$outcomeFile, $savedOutcome]] as [$file, $content]) { + if ($content !== null) { + file_put_contents($file, $content); + } else { + @unlink($file); + } + } +} + echo "C. the install request lets the status polls through\n"; // The polls come from the same browser session, and PHP keeps a session // locked until the request that opened it closes it. @@ -167,6 +238,8 @@ $closeAt = strpos($install, 'session_write_close()'); $runAt = strpos($install, '->performUpdateFromFile('); $check($closeAt !== false && $runAt !== false && $closeAt < $runAt, 'the session is released before the update runs'); +$attemptAt = strpos($install, '->setAttemptId('); +$check($attemptAt !== false && $attemptAt < $runAt, 'the page\'s attempt identifier is handed over before the update runs'); $check(strpos($install, "unset(\$_SESSION['manual_update_path']") < $closeAt, 'after the pending package is cleared, so the clearing is saved'); echo "\nPassed: {$passed} Failed: {$failed}\n"; From 40a81a72e28ef130e0dd6e304009406f45a764bb Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 7 Oct 2026 01:22:59 +0200 Subject: [PATCH 7/7] Describe the per-attempt status and the outcome to check in the 0.7.94 changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6f9b8583..b641d087e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,10 @@ Full version-by-version history for Pinakes. The README shows only the latest re ### Added - **An admin page for each article** ([#453](https://github.com/fabiodalez-dev/Pinakes/issues/453), [#454](https://github.com/fabiodalez-dev/Pinakes/issues/454)), laid out like the admin book page. It shows the cover, authors, publication, masthead and issue, genre with its path, keywords and the rest of the record, the PDF and the RIS and MARCXML exports, with Edit, Delete and the public page as buttons. `/admin/periodicals/articles/{id}` is now this page and the form moved to `/admin/periodicals/articles/{id}/edit`. The quick search, the Articles list (which gains View and Edit icons) and the Details button on the author page open it, and saving the form returns to it. Before, every link opened the form, so saving looked as if nothing had happened. -- **`GET /admin/updates/status`** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)): the installed version, whether the update lock is held, and the latest update attempt with its status and error, and the outcome of the latest run. Admin only (staff get 403), never cached, reachable during maintenance. +- **`GET /admin/updates/status`** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)): the installed version, whether the update lock is held, the latest update attempt and the outcome of the latest run, and, with `?attempt=`, the update log row and outcome of that one install attempt. Admin only (staff get 403), never cached, reachable during maintenance. ### Fixed -- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports how the run ended: success, or the error it stopped on. The page sends an identifier with its install request and the run writes it next to its outcome, so the page reads its own result and never another administrator's update. The run writes its outcome before it releases the lock, so a failure before the install step (space, backup, extraction, package checks) is reported with its own message too, and an attempt the server left half-way is reported as interrupted. The install request releases the PHP session before the update runs, so those polls are not held behind it. +- **An update behind a reverse proxy is no longer reported as failed when it succeeded** ([#450](https://github.com/fabiodalez-dev/Pinakes/issues/450)). The install request runs the backup, the files and the migrations in one go. A proxy in front of the site (Apache `mod_proxy`, a NAS's remote access, Cloudflare) can give up on it after a minute with a 502 or 504 while PHP, which ignores the aborted connection, finishes the update. The page treated that as a failure. On a gateway error or a dropped connection it now polls the status endpoint until the update lock is free, and reports how the run ended: success, or the error it stopped on. The page sends an identifier with its install request, and the run files its update log row and its outcome under it, so the page reads its own result and never another administrator's update. When nothing is filed under its attempt, the page says the outcome is to check and shows the installed version, instead of guessing. A gateway error whose body is empty or cut short is waited out like the others. The run writes its outcome before it releases the lock, so a failure before the install step (space, backup, extraction, package checks) is reported with its own message too, and an attempt the server left half-way is reported as interrupted. The install request releases the PHP session before the update runs, so those polls are not held behind it. - **Only the most specific admin menu entry is highlighted.** On an article page both Periodicals and Articles lit up, because the address of one starts with the other's. The Emeroteca plugin goes to 1.12.1. No migration.