Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions apps/dav/lib/Paginate/PaginateCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
use OCP\IDBConnection;
use OCP\Security\ISecureRandom;

/**
* @psalm-type PaginateContext = array<string, string|int|bool|null>
*/
class PaginateCache {
public const TTL = 60 * 60;
private const string CACHE_COUNT_SUFFIX = 'count';
Expand All @@ -32,11 +35,12 @@ public function __construct(
/**
* @param string $uri
* @param \Iterator $items
* @param PaginateContext $context
* @return array{'token': string, 'count': int}
*/
public function store(string $uri, \Iterator $items): array {
public function store(string $uri, \Iterator $items, array $context): array {
$token = $this->random->generate(32);
$cacheKey = $this->buildCacheKey($uri, $token);
$cacheKey = $this->buildCacheKey($uri, $token, $context);

$count = 0;
foreach ($items as $item) {
Expand All @@ -50,10 +54,11 @@ public function store(string $uri, \Iterator $items): array {
}

/**
* @param PaginateContext $context
* @return Generator<mixed>
*/
public function get(string $uri, string $token, int $offset, int $count): Generator {
$cacheKey = $this->buildCacheKey($uri, $token);
public function get(string $uri, string $token, int $offset, int $count, array $context): Generator {
$cacheKey = $this->buildCacheKey($uri, $token, $context);
$nbItems = $this->cache->get($cacheKey . self::CACHE_COUNT_SUFFIX);
if (!$nbItems || $offset > $nbItems) {
return [];
Expand All @@ -65,11 +70,17 @@ public function get(string $uri, string $token, int $offset, int $count): Genera
}
}

public function exists(string $uri, string $token): bool {
return $this->cache->get($this->buildCacheKey($uri, $token) . self::CACHE_COUNT_SUFFIX) > 0;
/**
* @param PaginateContext $context
*/
public function exists(string $uri, string $token, array $context): bool {
return $this->cache->get($this->buildCacheKey($uri, $token, $context) . self::CACHE_COUNT_SUFFIX) > 0;
}

private function buildCacheKey(string $uri, string $token): string {
return $token . '_' . crc32($uri) . '_';
/**
* @param PaginateContext $context
*/
private function buildCacheKey(string $uri, string $token, array $context): string {
return $token . '_' . crc32($uri) . '_' . hash('sha256', json_encode($context, JSON_THROW_ON_ERROR)) . '_';
}
}
28 changes: 22 additions & 6 deletions apps/dav/lib/Paginate/PaginatePlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace OCA\DAV\Paginate;

use Sabre\DAV\Auth\Plugin as AuthPlugin;
use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use Sabre\DAV\Xml\Element\Response;
Expand Down Expand Up @@ -53,7 +54,7 @@ public function onMultiStatus(&$fileProperties): void {
$url = $request->getUrl();
if (
$request->hasHeader(self::PAGINATE_HEADER)
&& (!$request->hasHeader(self::PAGINATE_TOKEN_HEADER) || !$this->cache->exists($url, $request->getHeader(self::PAGINATE_TOKEN_HEADER)))
&& (!$request->hasHeader(self::PAGINATE_TOKEN_HEADER) || !$this->cache->exists($url, $request->getHeader(self::PAGINATE_TOKEN_HEADER), $this->getCacheContext()))
) {
$pageSize = (int)$request->getHeader(self::PAGINATE_COUNT_HEADER) ?: $this->pageSize;
$offset = (int)$request->getHeader(self::PAGINATE_OFFSET_HEADER);
Expand All @@ -65,7 +66,7 @@ public function onMultiStatus(&$fileProperties): void {
// cache XML, but we keep the first $pageSize elements as objects
// to use for the response of the first page.
$rendererGenerator = $this->getXmlRendererGenerator($copyIterator);
['token' => $token, 'count' => $count] = $this->cache->store($url, $rendererGenerator);
['token' => $token, 'count' => $count] = $this->cache->store($url, $rendererGenerator, $this->getCacheContext());

$this->server->httpResponse->addHeader(self::PAGINATE_HEADER, 'true');
$this->server->httpResponse->addHeader(self::PAGINATE_TOKEN_HEADER, $token);
Expand All @@ -83,8 +84,7 @@ public function onMultiStatus(&$fileProperties): void {
*/
private function getXmlRendererGenerator(iterable $fileProperties): \Generator {
$writer = $this->server->xml->getWriter();
$prefer = $this->server->getHTTPPrefer();
$minimal = $prefer['return'] === 'minimal';
$minimal = $this->isMinimal();
$writer->contextUri = $this->server->getBaseUri();

$writer->openMemory();
Expand Down Expand Up @@ -115,18 +115,34 @@ private function getXmlRendererGenerator(iterable $fileProperties): \Generator {
}
}

private function isMinimal(): bool {
return $this->server->getHTTPPrefer()['return'] === 'minimal';
}

/**
* @return array{principal: ?string, minimal: bool}
*/
private function getCacheContext(): array {
$authPlugin = $this->server->getPlugin('auth');
return [
'principal' => $authPlugin instanceof AuthPlugin ? $authPlugin->getCurrentPrincipal() : null,
'minimal' => $this->isMinimal(),
];
}

public function onMethod(RequestInterface $request, ResponseInterface $response) {
$url = $this->server->httpRequest->getUrl();
$context = $this->getCacheContext();
if (
$request->hasHeader(self::PAGINATE_TOKEN_HEADER)
&& $request->hasHeader(self::PAGINATE_OFFSET_HEADER)
&& $this->cache->exists($url, $request->getHeader(self::PAGINATE_TOKEN_HEADER))
&& $this->cache->exists($url, $request->getHeader(self::PAGINATE_TOKEN_HEADER), $context)
) {
$token = $request->getHeader(self::PAGINATE_TOKEN_HEADER);
$offset = (int)$request->getHeader(self::PAGINATE_OFFSET_HEADER);
$count = (int)$request->getHeader(self::PAGINATE_COUNT_HEADER) ?: $this->pageSize;

$items = $this->cache->get($url, $token, $offset, $count);
$items = $this->cache->get($url, $token, $offset, $count, $context);

$response->setStatus(207);
$response->addHeader(self::PAGINATE_HEADER, 'true');
Expand Down
73 changes: 73 additions & 0 deletions apps/dav/tests/unit/Paginate/PaginateCacheTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\DAV\Tests\unit\Paginate;

use OC\Memcache\ArrayCache;
use OCA\DAV\Paginate\PaginateCache;
use OCP\ICacheFactory;
use OCP\IDBConnection;
use OCP\Security\ISecureRandom;
use PHPUnit\Framework\Attributes\DataProvider;
use Test\TestCase;

class PaginateCacheTest extends TestCase {
private const CONTEXT = ['principal' => 'principals/users/alice', 'minimal' => false];

private PaginateCache $paginateCache;

protected function setUp(): void {
parent::setUp();

$random = $this->createMock(ISecureRandom::class);
$random->method('generate')
->willReturn('token');

$cacheFactory = $this->createMock(ICacheFactory::class);
$cacheFactory->method('createDistributed')
->willReturn(new ArrayCache('pagination_'));

$this->paginateCache = new PaginateCache(
$this->createMock(IDBConnection::class),
$random,
$cacheFactory,
);
}

public function testStoreAndGetWithSameContext(): void {
$result = $this->paginateCache->store('url', new \ArrayIterator(['a', 'b', 'c']), self::CONTEXT);

self::assertSame(['token' => 'token', 'count' => 3], $result);
self::assertTrue($this->paginateCache->exists('url', 'token', self::CONTEXT));
self::assertSame(['b', 'c'], iterator_to_array($this->paginateCache->get('url', 'token', 1, 5, self::CONTEXT), false));
}

public static function dataDifferentContextDoesNotUseCache(): array {
return [
'other principal' => [['principal' => 'principals/users/bob', 'minimal' => false]],
'no principal' => [['principal' => null, 'minimal' => false]],
'other preference' => [['principal' => 'principals/users/alice', 'minimal' => true]],
'empty context' => [[]],
];
}

#[DataProvider(methodName: 'dataDifferentContextDoesNotUseCache')]
public function testDifferentContextDoesNotUseCache(array $context): void {
$this->paginateCache->store('url', new \ArrayIterator(['a', 'b']), self::CONTEXT);

self::assertFalse($this->paginateCache->exists('url', 'token', $context));
self::assertSame([], iterator_to_array($this->paginateCache->get('url', 'token', 0, 5, $context), false));
}

public function testDifferentUrlCannotAccess(): void {
$this->paginateCache->store('url', new \ArrayIterator(['a', 'b']), self::CONTEXT);

self::assertFalse($this->paginateCache->exists('other-url', 'token', self::CONTEXT));
}
}
41 changes: 36 additions & 5 deletions apps/dav/tests/unit/Paginate/PaginatePluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,22 @@
use OCA\DAV\Paginate\PaginateCache;
use OCA\DAV\Paginate\PaginatePlugin;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\Auth\Plugin as AuthPlugin;
use Sabre\DAV\Server;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Sabre\Xml\Service;
use Test\TestCase;

class PaginatePluginTest extends TestCase {
private const CONTEXT = ['principal' => 'principals/users/alice', 'minimal' => false];

private PaginateCache&MockObject $cache;
private PaginatePlugin $plugin;
private Server&MockObject $server;
private RequestInterface&MockObject $request;
private ResponseInterface&MockObject $response;
private ?string $preferReturn = null;

public function testOnMultiStatusCachesAndUpdatesResponse(): void {
$this->initializePlugin();
Expand Down Expand Up @@ -88,6 +91,7 @@ public function testOnMultiStatusCachesAndUpdatesResponse(): void {
self::assertStringContainsString($this->getResponseXmlForFile('/dav/file3', 'File 3'), $items[2]);
return true;
}),
self::CONTEXT,
)
->willReturn([
'token' => 'token',
Expand Down Expand Up @@ -187,7 +191,7 @@ public function testOnMultiStatusSkipsWhenHeadersAndCacheExist(): void {

$this->cache->expects(self::once())
->method('exists')
->with('', 'token')
->with('', 'token', self::CONTEXT)
->willReturn(true);

$this->cache->expects(self::never())
Expand Down Expand Up @@ -231,12 +235,12 @@ public function testOnMethodReturnsCachedResponse(): void {

$this->cache->expects(self::once())
->method('exists')
->with('url', 'token')
->with('url', 'token', self::CONTEXT)
->willReturn(true);

$this->cache->expects(self::once())
->method('get')
->with('url', 'token', 2, 4)
->with('url', 'token', 2, 4, self::CONTEXT)
->willReturn((function (): \Generator {
yield $this->getResponseXmlForFile('/file1', 'File 1');
yield $this->getResponseXmlForFile('/file2', 'File 2');
Expand Down Expand Up @@ -279,6 +283,27 @@ public function testOnMethodReturnsCachedResponse(): void {
self::assertFalse($this->plugin->onMethod($this->request, $response));
}

public function testOnMethodPassesMinimalPreferenceInContext(): void {
$this->initializePlugin();
$this->preferReturn = 'minimal';

$this->request->method('hasHeader')
->willReturn(true);
$this->request->method('getUrl')
->willReturn('url');
$this->request->method('getHeader')
->willReturn('token');

$this->cache->expects(self::once())
->method('exists')
->with('url', 'token', ['principal' => 'principals/users/alice', 'minimal' => true])
->willReturn(false);
$this->cache->expects(self::never())
->method('get');

$this->plugin->onMethod($this->request, $this->response);
}

public function testOnMultiStatusNoPaginateHeaderShouldSucceed(): void {
$this->initializePlugin();

Expand Down Expand Up @@ -316,7 +341,7 @@ protected function setUp(): void {

$this->server = $this->getMockBuilder(Server::class)
->disableOriginalConstructor()
->onlyMethods(['on', 'getHTTPPrefer', 'getBaseUri'])
->onlyMethods(['on', 'getHTTPPrefer', 'getBaseUri', 'getPlugin'])
->getMock();

$this->request = $this->createMock(RequestInterface::class);
Expand All @@ -328,7 +353,13 @@ protected function setUp(): void {
$this->server->xml->namespaceMap = [ 'DAV:' => 'd' ];

$this->server->method('getHTTPPrefer')
->willReturn(['return' => null]);
->willReturnCallback(fn () => ['return' => $this->preferReturn]);

$authPlugin = $this->createMock(AuthPlugin::class);
$authPlugin->method('getCurrentPrincipal')
->willReturn('principals/users/alice');
$this->server->method('getPlugin')
->willReturnMap([['auth', $authPlugin]]);
$this->server->method('getBaseUri')
->willReturn('/dav/');

Expand Down
Loading