mirror of
https://github.com/nextcloud/server.git
synced 2026-03-21 18:11:02 -04:00
fix(pagination): render multistatus to XML before caching
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
This commit is contained in:
Salvatore Martire
parent
4770c2f120
commit
36d515da61
2 changed files with 395 additions and 5 deletions
|
|
@ -11,6 +11,7 @@ namespace OCA\DAV\Paginate;
|
|||
|
||||
use Sabre\DAV\Server;
|
||||
use Sabre\DAV\ServerPlugin;
|
||||
use Sabre\DAV\Xml\Element\Response;
|
||||
use Sabre\HTTP\RequestInterface;
|
||||
use Sabre\HTTP\ResponseInterface;
|
||||
|
||||
|
|
@ -54,8 +55,13 @@ class PaginatePlugin extends ServerPlugin {
|
|||
) {
|
||||
$pageSize = (int)$request->getHeader(self::PAGINATE_COUNT_HEADER) ?: $this->pageSize;
|
||||
$offset = (int)$request->getHeader(self::PAGINATE_OFFSET_HEADER);
|
||||
|
||||
$copyIterator = new LimitedCopyIterator($fileProperties, $pageSize, $offset);
|
||||
['token' => $token, 'count' => $count] = $this->cache->store($url, $copyIterator);
|
||||
// wrap the iterator with another that renders XML, this way we
|
||||
// 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);
|
||||
|
||||
$fileProperties = $copyIterator->getRequestedItems();
|
||||
$this->server->httpResponse->addHeader(self::PAGINATE_HEADER, 'true');
|
||||
|
|
@ -65,6 +71,44 @@ class PaginatePlugin extends ServerPlugin {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a generator that yields rendered XML entries for the provided
|
||||
* $fileProperties, as they would appear in the MultiStatus response.
|
||||
*/
|
||||
private function getXmlRendererGenerator(iterable $fileProperties): \Generator {
|
||||
$writer = $this->server->xml->getWriter();
|
||||
$prefer = $this->server->getHTTPPrefer();
|
||||
$minimal = $prefer['return'] === 'minimal';
|
||||
$writer->contextUri = $this->server->getBaseUri();
|
||||
|
||||
$writer->openMemory();
|
||||
$writer->startDocument();
|
||||
$writer->startElement('{DAV:}multistatus');
|
||||
|
||||
// throw away the beginning of the document
|
||||
$writer->flush();
|
||||
|
||||
foreach ($fileProperties as $entry) {
|
||||
$href = $entry['href'];
|
||||
unset($entry['href']);
|
||||
if ($minimal) {
|
||||
unset($entry[404]);
|
||||
}
|
||||
$response = new Response(
|
||||
ltrim($href, '/'),
|
||||
$entry
|
||||
);
|
||||
$writer->write([
|
||||
'name' => '{DAV:}response',
|
||||
'value' => $response,
|
||||
]);
|
||||
|
||||
// flushing does not remove the > for the previous element
|
||||
// (multistatus)
|
||||
yield ltrim($writer->flush(), '>');
|
||||
}
|
||||
}
|
||||
|
||||
public function onMethod(RequestInterface $request, ResponseInterface $response) {
|
||||
$url = $this->server->httpRequest->getUrl();
|
||||
if (
|
||||
|
|
@ -83,11 +127,20 @@ class PaginatePlugin extends ServerPlugin {
|
|||
$response->setHeader('Content-Type', 'application/xml; charset=utf-8');
|
||||
$response->setHeader('Vary', 'Brief,Prefer');
|
||||
|
||||
$prefer = $this->server->getHTTPPrefer();
|
||||
$minimal = $prefer['return'] === 'minimal';
|
||||
// as we cached strings of XML, rebuild the multistatus response
|
||||
// and output the RAW entries, as stored in the cache
|
||||
$writer = $this->server->xml->getWriter();
|
||||
$writer->contextUri = $this->server->getBaseUri();
|
||||
$writer->openMemory();
|
||||
$writer->startDocument();
|
||||
$writer->startElement('{DAV:}multistatus');
|
||||
foreach ($items as $item) {
|
||||
$writer->writeRaw($item);
|
||||
}
|
||||
$writer->endElement();
|
||||
$writer->endDocument();
|
||||
|
||||
$data = $this->server->generateMultiStatus($items, $minimal);
|
||||
$response->setBody($data);
|
||||
$response->setBody($writer->flush());
|
||||
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
337
apps/dav/tests/unit/Paginate/PaginatePluginTest.php
Normal file
337
apps/dav/tests/unit/Paginate/PaginatePluginTest.php
Normal file
|
|
@ -0,0 +1,337 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
*/
|
||||
|
||||
namespace OCA\DAV\Tests\unit\Paginate;
|
||||
|
||||
use OCA\DAV\Paginate\PaginateCache;
|
||||
use OCA\DAV\Paginate\PaginatePlugin;
|
||||
use PHPUnit\Framework\MockObject\MockObject;
|
||||
use Sabre\DAV\Server;
|
||||
use Sabre\HTTP\RequestInterface;
|
||||
use Sabre\HTTP\ResponseInterface;
|
||||
use Sabre\Xml\Service;
|
||||
use Test\TestCase;
|
||||
|
||||
class PaginatePluginTest extends TestCase {
|
||||
|
||||
private PaginateCache&MockObject $cache;
|
||||
private PaginatePlugin $plugin;
|
||||
private Server&MockObject $server;
|
||||
private RequestInterface&MockObject $request;
|
||||
private ResponseInterface&MockObject $response;
|
||||
|
||||
public function testOnMultiStatusCachesAndUpdatesResponse(): void {
|
||||
$this->initializePlugin();
|
||||
|
||||
$fileProperties = [
|
||||
[
|
||||
'href' => '/file1',
|
||||
200 => [
|
||||
'{DAV:}displayname' => 'File 1',
|
||||
'{DAV:}resourcetype' => null
|
||||
],
|
||||
],
|
||||
[
|
||||
'href' => '/file2',
|
||||
200 => [
|
||||
'{DAV:}displayname' => 'File 2',
|
||||
'{DAV:}resourcetype' => null
|
||||
],
|
||||
],
|
||||
[
|
||||
'href' => '/file3',
|
||||
200 => [
|
||||
'{DAV:}displayname' => 'File 3',
|
||||
'{DAV:}resourcetype' => null
|
||||
],
|
||||
],
|
||||
];
|
||||
|
||||
$this->request->expects(self::exactly(2))
|
||||
->method('hasHeader')
|
||||
->willReturnMap([
|
||||
[PaginatePlugin::PAGINATE_HEADER, true],
|
||||
[PaginatePlugin::PAGINATE_TOKEN_HEADER, false],
|
||||
]);
|
||||
|
||||
$this->request->expects(self::once())
|
||||
->method('getUrl')
|
||||
->willReturn('url');
|
||||
|
||||
$this->request->expects(self::exactly(2))
|
||||
->method('getHeader')
|
||||
->willReturnMap([
|
||||
[PaginatePlugin::PAGINATE_COUNT_HEADER, 2],
|
||||
[PaginatePlugin::PAGINATE_OFFSET_HEADER, 0],
|
||||
]);
|
||||
|
||||
$this->request->expects(self::once())
|
||||
->method('setHeader')
|
||||
->with(PaginatePlugin::PAGINATE_TOKEN_HEADER, 'token');
|
||||
|
||||
$this->cache->expects(self::once())
|
||||
->method('store')
|
||||
->with(
|
||||
'url',
|
||||
$this->callback(function ($generator) {
|
||||
self::assertInstanceOf(\Generator::class, $generator);
|
||||
$items = iterator_to_array($generator);
|
||||
self::assertCount(3, $items);
|
||||
self::assertStringContainsString($this->getResponseXmlForFile('/dav/file1', 'File 1'), $items[0]);
|
||||
self::assertStringContainsString($this->getResponseXmlForFile('/dav/file2', 'File 2'), $items[1]);
|
||||
self::assertStringContainsString($this->getResponseXmlForFile('/dav/file3', 'File 3'), $items[2]);
|
||||
return true;
|
||||
}),
|
||||
)
|
||||
->willReturn([
|
||||
'token' => 'token',
|
||||
'count' => 3,
|
||||
]);
|
||||
|
||||
$this->expectSequentialCalls(
|
||||
$this->response,
|
||||
'addHeader',
|
||||
[
|
||||
[PaginatePlugin::PAGINATE_HEADER, 'true'],
|
||||
[PaginatePlugin::PAGINATE_TOKEN_HEADER, 'token'],
|
||||
[PaginatePlugin::PAGINATE_TOTAL_HEADER, '3'],
|
||||
],
|
||||
);
|
||||
|
||||
$this->plugin->onMultiStatus($fileProperties);
|
||||
|
||||
self::assertInstanceOf(\Iterator::class, $fileProperties);
|
||||
// the iterator should be replaced with one that has the amount of
|
||||
// items for the page
|
||||
$items = iterator_to_array($fileProperties, false);
|
||||
$this->assertCount(2, $items);
|
||||
}
|
||||
|
||||
private function initializePlugin(): void {
|
||||
$this->expectSequentialCalls(
|
||||
$this->server,
|
||||
'on',
|
||||
[
|
||||
['beforeMultiStatus', [$this->plugin, 'onMultiStatus'], 100],
|
||||
['method:SEARCH', [$this->plugin, 'onMethod'], 1],
|
||||
['method:PROPFIND', [$this->plugin, 'onMethod'], 1],
|
||||
['method:REPORT', [$this->plugin, 'onMethod'], 1],
|
||||
],
|
||||
);
|
||||
|
||||
$this->plugin->initialize($this->server);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<int, array<int, mixed>> $expectedCalls
|
||||
*/
|
||||
private function expectSequentialCalls(MockObject $mock, string $method, array $expectedCalls): void {
|
||||
$mock->expects(self::exactly(\count($expectedCalls)))
|
||||
->method($method)
|
||||
->willReturnCallback(function (...$args) use (&$expectedCalls) {
|
||||
$expected = array_shift($expectedCalls);
|
||||
self::assertNotNull($expected);
|
||||
self::assertSame($expected, $args);
|
||||
});
|
||||
}
|
||||
|
||||
private function getResponseXmlForFile(string $fileName, string $displayName): string {
|
||||
return preg_replace('/>\s+</', '><', <<<XML
|
||||
<d:response>
|
||||
<d:href>$fileName</d:href>
|
||||
<d:propstat>
|
||||
<d:prop>
|
||||
<d:displayname>$displayName</d:displayname>
|
||||
<d:resourcetype/>
|
||||
</d:prop>
|
||||
<d:status>HTTP/1.1 200 OK</d:status>
|
||||
</d:propstat>
|
||||
</d:response>
|
||||
XML
|
||||
);
|
||||
}
|
||||
|
||||
public function testOnMultiStatusSkipsWhenHeadersAndCacheExist(): void {
|
||||
$this->initializePlugin();
|
||||
|
||||
$fileProperties = [
|
||||
[
|
||||
'href' => '/file1',
|
||||
],
|
||||
[
|
||||
'href' => '/file2',
|
||||
],
|
||||
];
|
||||
|
||||
$this->request->expects(self::exactly(2))
|
||||
->method('hasHeader')
|
||||
->willReturnMap([
|
||||
[PaginatePlugin::PAGINATE_HEADER, true],
|
||||
[PaginatePlugin::PAGINATE_TOKEN_HEADER, true],
|
||||
]);
|
||||
|
||||
$this->request->expects(self::once())
|
||||
->method('getUrl')
|
||||
->willReturn('');
|
||||
|
||||
$this->request->expects(self::once())
|
||||
->method('getHeader')
|
||||
->with(PaginatePlugin::PAGINATE_TOKEN_HEADER)
|
||||
->willReturn('token');
|
||||
|
||||
$this->cache->expects(self::once())
|
||||
->method('exists')
|
||||
->with('', 'token')
|
||||
->willReturn(true);
|
||||
|
||||
$this->cache->expects(self::never())
|
||||
->method('store');
|
||||
|
||||
$this->plugin->onMultiStatus($fileProperties);
|
||||
|
||||
self::assertInstanceOf(\Iterator::class, $fileProperties);
|
||||
self::assertSame(
|
||||
[
|
||||
['href' => '/file1'],
|
||||
['href' => '/file2'],
|
||||
],
|
||||
iterator_to_array($fileProperties)
|
||||
);
|
||||
}
|
||||
|
||||
public function testOnMethodReturnsCachedResponse(): void {
|
||||
$this->initializePlugin();
|
||||
|
||||
$response = $this->createMock(ResponseInterface::class);
|
||||
|
||||
$this->request->expects(self::exactly(2))
|
||||
->method('hasHeader')
|
||||
->willReturnMap([
|
||||
[PaginatePlugin::PAGINATE_TOKEN_HEADER, true],
|
||||
[PaginatePlugin::PAGINATE_OFFSET_HEADER, true],
|
||||
]);
|
||||
|
||||
$this->request->expects(self::once())
|
||||
->method('getUrl')
|
||||
->willReturn('url');
|
||||
|
||||
$this->request->expects(self::exactly(4))
|
||||
->method('getHeader')
|
||||
->willReturnMap([
|
||||
[PaginatePlugin::PAGINATE_TOKEN_HEADER, 'token'],
|
||||
[PaginatePlugin::PAGINATE_OFFSET_HEADER, '2'],
|
||||
[PaginatePlugin::PAGINATE_COUNT_HEADER, '4'],
|
||||
]);
|
||||
|
||||
$this->cache->expects(self::once())
|
||||
->method('exists')
|
||||
->with('url', 'token')
|
||||
->willReturn(true);
|
||||
|
||||
$this->cache->expects(self::once())
|
||||
->method('get')
|
||||
->with('url', 'token', 2, 4)
|
||||
->willReturn((function (): \Generator {
|
||||
yield $this->getResponseXmlForFile('/file1', 'File 1');
|
||||
yield $this->getResponseXmlForFile('/file2', 'File 2');
|
||||
})());
|
||||
|
||||
$response->expects(self::once())
|
||||
->method('setStatus')
|
||||
->with(207);
|
||||
|
||||
$response->expects(self::once())
|
||||
->method('addHeader')
|
||||
->with(PaginatePlugin::PAGINATE_HEADER, 'true');
|
||||
|
||||
$this->expectSequentialCalls(
|
||||
$response,
|
||||
'setHeader',
|
||||
[
|
||||
['Content-Type', 'application/xml; charset=utf-8'],
|
||||
['Vary', 'Brief,Prefer'],
|
||||
],
|
||||
);
|
||||
|
||||
$response->expects(self::once())
|
||||
->method('setBody')
|
||||
->with($this->callback(function (string $body) {
|
||||
// header of the XML
|
||||
self::assertStringContainsString(<<<XML
|
||||
<?xml version="1.0"?>
|
||||
<d:multistatus xmlns:d="DAV:">
|
||||
XML,
|
||||
$body);
|
||||
self::assertStringContainsString($this->getResponseXmlForFile('/file1', 'File 1'), $body);
|
||||
self::assertStringContainsString($this->getResponseXmlForFile('/file2', 'File 2'), $body);
|
||||
// footer of the XML
|
||||
self::assertStringContainsString('</d:multistatus>', $body);
|
||||
|
||||
return true;
|
||||
}));
|
||||
|
||||
self::assertFalse($this->plugin->onMethod($this->request, $response));
|
||||
}
|
||||
|
||||
public function testOnMultiStatusNoPaginateHeaderShouldSucceed(): void {
|
||||
$this->initializePlugin();
|
||||
|
||||
$this->request->expects(self::once())
|
||||
->method('getUrl')
|
||||
->willReturn('');
|
||||
|
||||
$this->cache->expects(self::never())
|
||||
->method('exists');
|
||||
$this->cache->expects(self::never())
|
||||
->method('store');
|
||||
|
||||
$this->plugin->onMultiStatus($this->request);
|
||||
}
|
||||
|
||||
public function testOnMethodNoTokenHeaderShouldSucceed(): void {
|
||||
$this->initializePlugin();
|
||||
$this->request->expects(self::once())
|
||||
->method('hasHeader')
|
||||
->with(PaginatePlugin::PAGINATE_TOKEN_HEADER)
|
||||
->willReturn(false);
|
||||
|
||||
$this->cache->expects(self::never())
|
||||
->method('exists');
|
||||
$this->cache->expects(self::never())
|
||||
->method('get');
|
||||
|
||||
$this->plugin->onMethod($this->request, $this->response);
|
||||
}
|
||||
|
||||
protected function setUp(): void {
|
||||
parent::setUp();
|
||||
|
||||
$this->cache = $this->createMock(PaginateCache::class);
|
||||
|
||||
$this->server = $this->getMockBuilder(Server::class)
|
||||
->disableOriginalConstructor()
|
||||
->onlyMethods(['on', 'getHTTPPrefer', 'getBaseUri'])
|
||||
->getMock();
|
||||
|
||||
$this->request = $this->createMock(RequestInterface::class);
|
||||
$this->response = $this->createMock(ResponseInterface::class);
|
||||
|
||||
$this->server->httpRequest = $this->request;
|
||||
$this->server->httpResponse = $this->response;
|
||||
$this->server->xml = new Service();
|
||||
$this->server->xml->namespaceMap = [ 'DAV:' => 'd' ];
|
||||
|
||||
$this->server->method('getHTTPPrefer')
|
||||
->willReturn(['return' => null]);
|
||||
$this->server->method('getBaseUri')
|
||||
->willReturn('/dav/');
|
||||
|
||||
$this->plugin = new PaginatePlugin($this->cache, 2);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue