diff --git a/lib/Controller/AdminApiController.php b/lib/Controller/AdminApiController.php index ae1f6a4..37995ec 100644 --- a/lib/Controller/AdminApiController.php +++ b/lib/Controller/AdminApiController.php @@ -179,12 +179,12 @@ final class AdminApiController extends Controller { } } - public function trash(int $targetId, int $offset = 0): JSONResponse { + public function trash(int $targetId, string $after = ''): JSONResponse { try { $delay = $this->keys->deleteDelayDays() * 86400; $repo = $this->targets->repository($this->targets->get((string)$targetId)); // In parts, so that the page can show how far it is (a read per removed restore point). - $page = $repo->trashPage($offset, microtime(true) + self::TRASH_PAGE_SECONDS); + $page = $repo->trashPage($after, microtime(true) + self::TRASH_PAGE_SECONDS); $out = []; foreach ($page['items'] as $id => $info) { $out[] = ['id' => $id, 'label' => $info['label'], 'time' => $info['time'], 'removedAt' => $info['forgottenAt'], diff --git a/lib/Job/BackupRun.php b/lib/Job/BackupRun.php index 5b42579..4a0c379 100644 --- a/lib/Job/BackupRun.php +++ b/lib/Job/BackupRun.php @@ -62,9 +62,9 @@ final class BackupRun { if (!mb_check_encoding($logical, 'UTF-8') || !Repository::isSafePath($logical)) { // A name a snapshot cannot hold (a backslash, a control character, not UTF-8). One // such path must not make a backup fail, or the whole snapshot unreadable later. + // The cursor stays where it is: the state is JSON, which cannot hold such a name. A + // next step meets it again, and it is reported once. self::recordUnreadable($state, $stats, mb_scrub($logical, 'UTF-8')); - $state['cur'] = null; - $state['after'] = $logical; continue; } if (is_dir($path)) { diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 398ae20..a631000 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -818,15 +818,20 @@ final class Repository { /** * Part of trash(), for a page that shows how far it is: each entry is a read of its own on the - * location. Reads on from $offset until $deadline (microtime(true)), at least one entry. + * location. Reads the entries with an id after $after, in order, until $deadline + * (microtime(true)), at least one. The id, not a position: a cleanup that changes the trash + * between two parts then makes no entry come twice or be missed. * * @return array{total: int, items: array>} */ - public function trashPage(int $offset, float $deadline): array { + public function trashPage(string $after, float $deadline): array { $ids = array_map('strval', array_keys($this->catalog->trash)); sort($ids, SORT_STRING); $items = []; - foreach (array_slice($ids, max(0, $offset)) as $id) { + foreach ($ids as $id) { + if (strcmp($id, $after) <= 0) { + continue; + } try { $items[$id] = $this->trashInfo($id); } catch (\Throwable) { diff --git a/src/api.js b/src/api.js index c5ad161..85b391b 100644 --- a/src/api.js +++ b/src/api.js @@ -21,7 +21,7 @@ export default { addSlot: (passphrase, label) => call('post', '/key/slots', { passphrase, label }), replaceSlot: (slot, passphrase, label) => call('put', `/key/slots/${slot}`, { passphrase, label }), removeSlot: (slot) => call('delete', `/key/slots/${slot}`), - trash: (targetId, offset = 0) => call('get', `/targets/${targetId}/trash`, { offset }), + trash: (targetId, after = '') => call('get', `/targets/${targetId}/trash`, { after }), untrash: (targetId, snapshotId) => call('post', `/targets/${targetId}/trash/${snapshotId}`), kitUrl: () => url('/key/kit'), confirmKit: (accepted, phrase, code) => call('post', '/key/confirm', { accepted, phrase, code }), diff --git a/src/components/Bin.vue b/src/components/Bin.vue index 7bdb49b..91d1a40 100644 --- a/src/components/Bin.vue +++ b/src/components/Bin.vue @@ -78,13 +78,13 @@ async function loadTarget(target, load) { reading.value = { name: target.name, total: null, done: 0 } const items = [] for (;;) { - const page = await api.trash(target.id, items.length) + const page = await api.trash(target.id, items.length ? items[items.length - 1].id : '') if (load !== current) { return } items.push(...page.items) reading.value = { name: target.name, total: page.total, done: Math.min(items.length, page.total) } - if (items.length >= page.total || !page.items.length) { + if (!page.items.length || items.length >= page.total) { break } } diff --git a/tests/Unit/Repository/CatalogTest.php b/tests/Unit/Repository/CatalogTest.php index bba3047..8bd2646 100644 --- a/tests/Unit/Repository/CatalogTest.php +++ b/tests/Unit/Repository/CatalogTest.php @@ -58,16 +58,30 @@ class CatalogTest extends TestCase { $again = $this->open(); $seen = []; // A deadline already past: one entry per request, as on a slow location + $after = ''; for ($request = 0; count($seen) < 3 && $request < 5; $request++) { - $page = $again->trashPage(count($seen), 0.0); + $page = $again->trashPage($after, 0.0); $this->assertSame(3, $page['total']); $this->assertCount(1, $page['items']); $seen += $page['items']; + $after = (string)array_key_last($page['items']); } - $this->assertSame(array_keys($again->trash()), array_intersect(array_keys($again->trash()), array_keys($seen))); - $this->assertCount(3, $seen); - $this->assertSame(['total' => 3, 'items' => []], $again->trashPage(3, 0.0)); - $this->assertCount(3, $again->trashPage(0, microtime(true) + 60)['items']); + $this->assertEqualsCanonicalizing(array_keys($again->trash()), array_keys($seen)); + $this->assertSame(['total' => 3, 'items' => []], $again->trashPage($after, 0.0)); + $this->assertCount(3, $again->trashPage('', microtime(true) + 60)['items']); + } + + public function testAnEntryThatLeavesTheTrashBetweenPartsMakesNoneComeTwiceOrBeMissed(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'), $this->anchor); + for ($i = 0; $i < 3; $i++) { + $repo->forget($repo->backupDirectory($this->src)['snapshot']); + } + $reader = $this->open(); + $ids = array_keys($reader->trashPage('', microtime(true) + 60)['items']); + $first = $reader->trashPage('', 0.0)['items']; + $this->assertSame([$ids[0]], array_keys($first)); + $this->open()->untrash($ids[0]); // another administrator, between two parts + $this->assertSame([$ids[1], $ids[2]], array_keys($this->open()->trashPage($ids[0], microtime(true) + 60)['items'])); } public function testRollbackIsDetected(): void { diff --git a/tests/Unit/Repository/EmptyDirsTest.php b/tests/Unit/Repository/EmptyDirsTest.php index b22731e..b318c58 100644 --- a/tests/Unit/Repository/EmptyDirsTest.php +++ b/tests/Unit/Repository/EmptyDirsTest.php @@ -78,10 +78,11 @@ class EmptyDirsTest extends TestCase { file_put_contents("$this->src/data/alice/files/\xff.txt", 'x'); $state = BackupRun::start(['data' => "$this->src/data"]); for ($i = 0; $i < 50 && $state['phase'] !== 'commit'; $i++) { - $state = BackupRun::step($this->repo(), $state, microtime(true) + 30); + // A step per entry, state through JSON: as separate processes would + $state = json_decode(json_encode(BackupRun::step($this->repo(), $state, 0.0), JSON_THROW_ON_ERROR), true, 512, JSON_THROW_ON_ERROR); } $this->assertSame('commit', $state['phase']); - $this->assertSame(4, $state['stats']['unreadable']); + $this->assertSame(4, $state['stats']['unreadable'], 'each reported once, over several steps'); $this->repo()->recordSnapshot($state['snapshot']); $this->assertSame(['data/alice/files/Hollow/Nested', 'data/skipped/inner'], array_column($this->repo()->emptyDirs($state['snapshot']), 0)); diff --git a/tests/integration/roundtrip.sh b/tests/integration/roundtrip.sh index 0dce2d3..38b0560 100755 --- a/tests/integration/roundtrip.sh +++ b/tests/integration/roundtrip.sh @@ -99,7 +99,7 @@ done occ backup:list ci | grep " page " >/dev/null || fail "the snapshot of the ticked backup is not listed" ok "without system cron: warning in backup:status and the API, backup finished by $i ticks of the open page" # The Bin reads in parts, as the page does (what is in the parts: CatalogTest) -api "$API/targets/$TID/trash?offset=0" | php -r '$p = json_decode(stream_get_contents(STDIN), true); exit(is_int($p["total"] ?? null) && is_array($p["items"] ?? null) && count($p["items"]) <= $p["total"] ? 0 : 1);' || fail "the Bin API does not give a total and items" +api "$API/targets/$TID/trash" | php -r '$p = json_decode(stream_get_contents(STDIN), true); exit(is_int($p["total"] ?? null) && is_array($p["items"] ?? null) && count($p["items"]) <= $p["total"] ? 0 : 1);' || fail "the Bin API does not give a total and items" ok "the Bin API reads in parts, with a total" occ background:cron >/dev/null $X php -f /var/www/html/cron.php >/dev/null 2>&1 || true