Skip to content

Commit f6b8554

Browse files
authored
Merge pull request #63119 from nextcloud/backport/62365/stable34
[stable34] Don't fill the pagination cache if there are no extra items
2 parents bcb4625 + d03adb0 commit f6b8554

3 files changed

Lines changed: 59 additions & 13 deletions

File tree

apps/dav/lib/Paginate/LimitedCopyIterator.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
class LimitedCopyIterator extends \AppendIterator {
2121
private array $skipped = [];
2222
private array $copy = [];
23+
private readonly bool $hasOthers;
2324

2425
public function __construct(\Traversable $iterator, int $count, int $offset = 0) {
2526
parent::__construct();
@@ -29,17 +30,17 @@ public function __construct(\Traversable $iterator, int $count, int $offset = 0)
2930
}
3031
$iterator = new \NoRewindIterator($iterator);
3132

32-
$i = 0;
33-
while ($iterator->valid() && ++$i <= $offset) {
34-
$this->skipped[] = $iterator->current();
33+
while ($iterator->valid() && count($this->skipped) < $offset) {
34+
$this->skipped[$iterator->key()] = $iterator->current();
3535
$iterator->next();
3636
}
3737

3838
while ($iterator->valid() && count($this->copy) < $count) {
39-
$this->copy[] = $iterator->current();
39+
$this->copy[$iterator->key()] = $iterator->current();
4040
$iterator->next();
4141
}
4242

43+
$this->hasOthers = $iterator->valid() || count($this->skipped) > 0;
4344
$this->append(new \ArrayIterator($this->skipped));
4445
$this->append($this->getRequestedItems());
4546
$this->append($iterator);
@@ -48,4 +49,11 @@ public function __construct(\Traversable $iterator, int $count, int $offset = 0)
4849
public function getRequestedItems(): \Iterator {
4950
return new \ArrayIterator($this->copy);
5051
}
52+
53+
/**
54+
* Are there any other items in the iterator aside from the requested ones
55+
*/
56+
public function hasOthers(): bool {
57+
return $this->hasOthers;
58+
}
5159
}

apps/dav/lib/Paginate/PaginatePlugin.php

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,17 +59,21 @@ public function onMultiStatus(&$fileProperties): void {
5959
$offset = (int)$request->getHeader(self::PAGINATE_OFFSET_HEADER);
6060

6161
$copyIterator = new LimitedCopyIterator($fileProperties, $pageSize, $offset);
62-
// wrap the iterator with another that renders XML, this way we
63-
// cache XML, but we keep the first $pageSize elements as objects
64-
// to use for the response of the first page.
65-
$rendererGenerator = $this->getXmlRendererGenerator($copyIterator);
66-
['token' => $token, 'count' => $count] = $this->cache->store($url, $rendererGenerator);
62+
63+
if ($copyIterator->hasOthers()) {
64+
// wrap the iterator with another that renders XML, this way we
65+
// cache XML, but we keep the first $pageSize elements as objects
66+
// to use for the response of the first page.
67+
$rendererGenerator = $this->getXmlRendererGenerator($copyIterator);
68+
['token' => $token, 'count' => $count] = $this->cache->store($url, $rendererGenerator);
69+
70+
$this->server->httpResponse->addHeader(self::PAGINATE_HEADER, 'true');
71+
$this->server->httpResponse->addHeader(self::PAGINATE_TOKEN_HEADER, $token);
72+
$this->server->httpResponse->addHeader(self::PAGINATE_TOTAL_HEADER, (string)$count);
73+
$request->setHeader(self::PAGINATE_TOKEN_HEADER, $token);
74+
}
6775

6876
$fileProperties = $copyIterator->getRequestedItems();
69-
$this->server->httpResponse->addHeader(self::PAGINATE_HEADER, 'true');
70-
$this->server->httpResponse->addHeader(self::PAGINATE_TOKEN_HEADER, $token);
71-
$this->server->httpResponse->addHeader(self::PAGINATE_TOTAL_HEADER, (string)$count);
72-
$request->setHeader(self::PAGINATE_TOKEN_HEADER, $token);
7377
}
7478
}
7579

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\DAV\Tests\unit\Paginate;
10+
11+
use OCA\DAV\Paginate\LimitedCopyIterator;
12+
use Test\TestCase;
13+
14+
class LimitedCopyIteratorTest extends TestCase {
15+
public function testBasic() {
16+
$data = [1, 2, 3, 4, 5, 6, 7];
17+
18+
$copy = new LimitedCopyIterator(new \ArrayIterator($data), 5, 0);
19+
$this->assertEquals([1, 2, 3, 4, 5], iterator_to_array($copy->getRequestedItems()));
20+
$this->assertTrue($copy->hasOthers());
21+
$this->assertEquals($data, iterator_to_array($copy));
22+
23+
$copy = new LimitedCopyIterator(new \ArrayIterator($data), 15, 0);
24+
$this->assertEquals([1, 2, 3, 4, 5, 6, 7], iterator_to_array($copy->getRequestedItems()));
25+
$this->assertFalse($copy->hasOthers());
26+
$this->assertEquals($data, iterator_to_array($copy));
27+
28+
$copy = new LimitedCopyIterator(new \ArrayIterator($data), 15, 1);
29+
$this->assertEquals([1 => 2, 3, 4, 5, 6, 7], iterator_to_array($copy->getRequestedItems()));
30+
$this->assertTrue($copy->hasOthers());
31+
$this->assertEquals($data, iterator_to_array($copy));
32+
}
33+
34+
}

0 commit comments

Comments
 (0)