From a879b63e32bc475305c855b9aa646c000735fc20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 4 Sep 2015 20:59:30 +0200 Subject: [PATCH 1/2] Use Base URIs via Browser --- README.md | 9 ++++++--- examples/directory.php | 6 +++--- src/Client.php | 6 ++---- tests/ClientTest.php | 26 ++++++++++++-------------- tests/FunctionalApacheClientTest.php | 2 +- tests/FunctionalGentooClientTest.php | 2 +- 6 files changed, 25 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index e426ee7..e01d6bd 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ listing from the given ViewVC URL: ```php $loop = React\EventLoop\Factory::create(); $browser = new Clue\React\Buzz\Browser($loop); -$client = new Client('http://example.com/viewvc/', $browser); +$client = new Client($browser->withBase('http://example.com/viewvc/')); $client->fetchDirectory('/')->then(function ($files) { echo 'Files: ' . implode(', ', $files) . PHP_EOL; @@ -34,9 +34,12 @@ in order to handle async requests: $loop = React\EventLoop\Factory::create(); $browser = new Clue\React\Buzz\Browser($loop); -$client = new Client($url, $browser); +$client = new Client($browser->withBase('http://example.com/viewvc/')); ``` +The `Client` API uses relative URIs to reference files and directories in your +ViewVC installation, so make sure to apply the base URI as depicted above. + If you need custom DNS or proxy settings, you can explicitly pass a custom [`Browser`](https://github.com/clue/php-buzz-react#browser) instance. @@ -95,7 +98,7 @@ use Clue\React\Block; $loop = React\EventLoop\Factory::create(); $browser = new Clue\React\Buzz\Browser($loop); -$client = new Client($url /* change me */, $browser); +$client = new Client($browser->withBase($uri /* change me */)); $promise = $client->fetchFile($path /* change me */); try { diff --git a/examples/directory.php b/examples/directory.php index 861f233..cb52315 100644 --- a/examples/directory.php +++ b/examples/directory.php @@ -26,12 +26,12 @@ $browser = new Browser($loop, $sender); -$client = new Client($url, $browser); +$client = new Client($browser->withBase($url)); if (substr($path, -1) === '/') { - $client->fetchDirectory($path, $revision)->then('var_dump', 'printf'); + $client->fetchDirectory($path, $revision)->then('print_r', 'printf'); } else { - $client->fetchFile($path, $revision)->then('printf', 'printf'); + $client->fetchFile($path, $revision)->then('print_r', 'printf'); } $loop->run(); diff --git a/src/Client.php b/src/Client.php index daf642d..cf2e43c 100644 --- a/src/Client.php +++ b/src/Client.php @@ -14,10 +14,9 @@ class Client { - private $url; private $brower; - public function __construct($url, Browser $browser, Parser $parser = null, Loader $loader = null) + public function __construct(Browser $browser, Parser $parser = null, Loader $loader = null) { if ($parser === null) { $parser = new Parser(); @@ -31,7 +30,6 @@ public function __construct($url, Browser $browser, Parser $parser = null, Loade // 'follow_redirects' => false // )); - $this->url = rtrim($url, '/') . '/'; $this->browser = $browser; $this->parser = $parser; $this->loader = $loader; @@ -135,7 +133,7 @@ private function fetchXml($url) private function fetch($url) { - return $this->browser->get($this->url . ltrim($url, '/'))->then( + return $this->browser->get(ltrim($url, '/'))->then( function (Response $response) { return (string)$response->getBody(); }, diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 33b8720..35e39e9 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -7,15 +7,14 @@ class ClientTest extends TestCase { - private $url; private $browser; private $client; public function setUp() { - $this->url = 'http://viewvc.example.org'; - $this->browser = $this->getMockBuilder('Clue\React\Buzz\Browser')->disableOriginalConstructor()->getMock(); - $this->client = new Client($this->url, $this->browser); + // test case does not take base URI into account + $this->browser = $this->getMockBuilder('Clue\React\Buzz\Browser')->disableOriginalConstructor()->setMethods(array('get'))->getMock(); + $this->client = new Client($this->browser); } public function testInvalidDirectory() @@ -33,7 +32,7 @@ public function testInvalidFile() public function testFetchFile() { $response = new Response('HTTP/1.0', 200, 'OK', array(), new Body('# hello')); - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/README.md?view=co'))->will($this->returnValue(Promise\resolve($response))); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co'))->will($this->returnValue(Promise\resolve($response))); $promise = $this->client->fetchFile('README.md'); @@ -42,17 +41,16 @@ public function testFetchFile() public function testFetchFileExcessiveSlashesAreIgnored() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/README.md?view=co'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co'))->will($this->returnValue(Promise\reject())); - $client = new Client($this->url . '/', $this->browser); - $promise = $client->fetchFile('/README.md'); + $promise = $this->client->fetchFile('/README.md'); $this->expectPromiseReject($promise); } public function testFetchFileRevision() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/README.md?view=co&pathrev=1.0'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co&pathrev=1.0'))->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchFile('/README.md', '1.0'); @@ -61,7 +59,7 @@ public function testFetchFileRevision() public function testFetchDirectoryRevision() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/directory/?pathrev=1.0'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?pathrev=1.0'))->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchDirectory('/directory/', '1.0'); @@ -70,7 +68,7 @@ public function testFetchDirectoryRevision() public function testFetchDirectoryAttic() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/directory/?hideattic=0'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?hideattic=0'))->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchDirectory('/directory/', null, true); @@ -79,7 +77,7 @@ public function testFetchDirectoryAttic() public function testFetchDirectoryRevisionAttic() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/directory/?pathrev=1.1&hideattic=0'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?pathrev=1.1&hideattic=0'))->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchDirectory('/directory/', '1.1', true); @@ -88,7 +86,7 @@ public function testFetchDirectoryRevisionAttic() public function testFetchLogRevision() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/README.md?view=log&pathrev=1.0'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=log&pathrev=1.0'))->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchLog('/README.md', '1.0'); @@ -97,7 +95,7 @@ public function testFetchLogRevision() public function testFetchPatch() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('http://viewvc.example.org/README.md?view=patch&r1=1.0&r2=1.1'))->will($this->returnValue(Promise\reject())); + $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=patch&r1=1.0&r2=1.1'))->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchPatch('/README.md', '1.0', '1.1'); diff --git a/tests/FunctionalApacheClientTest.php b/tests/FunctionalApacheClientTest.php index df5943e..3511209 100644 --- a/tests/FunctionalApacheClientTest.php +++ b/tests/FunctionalApacheClientTest.php @@ -17,7 +17,7 @@ public function setUp() $this->loop = LoopFactory::create(); $browser = new Browser($this->loop); - $this->viewvc = new Client($url, $browser); + $this->viewvc = new Client($browser->withBase($url)); } public function testFetchDirectory() diff --git a/tests/FunctionalGentooClientTest.php b/tests/FunctionalGentooClientTest.php index 772455c..1dccac2 100644 --- a/tests/FunctionalGentooClientTest.php +++ b/tests/FunctionalGentooClientTest.php @@ -36,7 +36,7 @@ public function setUp() $this->markTestSkipped('Unable to reach Gentoo ViewVC'); } - $this->viewvc = new Client($url, $browser); + $this->viewvc = new Client($browser->withBase($url)); } public function testFetchDirectoryAttic() From 1c39a2eb030be27f6fcad3244589b43f50ec36b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20L=C3=BCck?= Date: Fri, 4 Sep 2015 21:44:57 +0200 Subject: [PATCH 2/2] Use URI templates to build properly escaped URIs --- src/Client.php | 80 ++++++++++++++++++++++++++------------------ tests/ClientTest.php | 42 ++++++++++++++++------- 2 files changed, 77 insertions(+), 45 deletions(-) diff --git a/src/Client.php b/src/Client.php index cf2e43c..b16a3b6 100644 --- a/src/Client.php +++ b/src/Client.php @@ -41,17 +41,20 @@ public function fetchFile($path, $revision = null) return Promise\reject(new InvalidArgumentException('File path MUST NOT end with trailing slash')); } - $url = $path . '?view=co'; - if ($revision !== null) { - $url .= '&pathrev=' . $revision; - } - // TODO: fetching a directory redirects to path with trailing slash // TODO: status returns 200 OK, but displays an error message anyways.. // TODO: see not-a-file.html // TODO: reject all paths with trailing slashes - return $this->fetch($url); + return $this->fetch( + $this->browser->resolve( + '/{+path}?view=co{&pathrev}', + array( + 'path' => ltrim($path, '/'), + 'pathrev' => $revision + ) + ) + ); } public function fetchDirectory($path, $revision = null, $showAttic = false) @@ -60,21 +63,19 @@ public function fetchDirectory($path, $revision = null, $showAttic = false) return Promise\reject(new InvalidArgumentException('Directory path MUST end with trailing slash')); } - $url = $path; - - if ($revision !== null) { - $url .= '?pathrev=' . $revision; - } - - if ($showAttic) { - $url .= (strpos($url, '?') === false) ? '?' : '&'; - $url .= 'hideattic=0'; - } - // TODO: path MUST end with trailing slash // TODO: accessing files will redirect to file with relative location URL (not supported by clue/buzz-react) - return $this->fetchXml($url)->then(function (SimpleXMLElement $xml) { + return $this->fetchXml( + $this->browser->resolve( + '/{+path}{?pathrev,hideattic}', + array( + 'path' => ltrim($path, '/'), + 'pathrev' => $revision, + 'hideattic' => $showAttic ? '0' : null + ) + ) + )->then(function (SimpleXMLElement $xml) { // TODO: reject if this is a file, instead of directory => contains "Log of" instead of "Index of" // TODO: see is-a-file.html @@ -84,23 +85,31 @@ public function fetchDirectory($path, $revision = null, $showAttic = false) public function fetchPatch($path, $r1, $r2) { - $url = $path . '?view=patch&r1=' . $r1 . '&r2=' . $r2; - - return $this->fetch($url); + return $this->fetch( + $this->browser->resolve( + '/{+path}?view=patch{&r1,r2}', + array( + 'path' => ltrim($path, '/'), + 'r1' => $r1, + 'r2' => $r2 + ) + ) + ); } public function fetchLog($path, $revision = null) { - $url = $path . '?view=log'; - // TODO: invalid revision shows error page, but HTTP 200 OK - if ($revision !== null) { - $url .= (strpos($url, '?') === false) ? '?' : '&'; - $url .= 'pathrev=' . $revision; - } - - return $this->fetchXml($url)->then(array($this->parser, 'parseLogEntries')); + return $this->fetchXml( + $this->browser->resolve( + '/{+path}?view=log{&pathrev}', + array( + 'path' => ltrim($path, '/'), + 'pathrev' => $revision + ) + ) + )->then(array($this->parser, 'parseLogEntries')); } public function fetchRevisionPrevious($path, $revision) @@ -121,9 +130,14 @@ public function fetchAllPreviousRevisions($path) private function fetchLogXml($path) { - $url = $path . '?view=log'; - - return $this->fetchXml($url); + return $this->fetchXml( + $this->browser->resolve( + '/{+path}?view=log', + array( + 'path' => ltrim($path, '/') + ) + ) + ); } private function fetchXml($url) @@ -133,7 +147,7 @@ private function fetchXml($url) private function fetch($url) { - return $this->browser->get(ltrim($url, '/'))->then( + return $this->browser->get($url)->then( function (Response $response) { return (string)$response->getBody(); }, diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 35e39e9..70c3006 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -4,17 +4,22 @@ use Clue\React\Buzz\Message\Response; use Clue\React\Buzz\Message\Body; use React\Promise; +use Clue\React\Buzz\Browser; +use Clue\React\Buzz\Message\Request; class ClientTest extends TestCase { - private $browser; + private $uri = 'http://viewvc.example.org/'; + private $sender; private $client; public function setUp() { - // test case does not take base URI into account - $this->browser = $this->getMockBuilder('Clue\React\Buzz\Browser')->disableOriginalConstructor()->setMethods(array('get'))->getMock(); - $this->client = new Client($this->browser); + $this->sender = $this->getMockBuilder('Clue\React\Buzz\Io\Sender')->disableOriginalConstructor()->getMock(); + + $browser = new Browser($this->getMock('React\EventLoop\LoopInterface'), $this->sender); + + $this->client = new Client($browser->withBase($this->uri)); } public function testInvalidDirectory() @@ -32,7 +37,8 @@ public function testInvalidFile() public function testFetchFile() { $response = new Response('HTTP/1.0', 200, 'OK', array(), new Body('# hello')); - $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co'))->will($this->returnValue(Promise\resolve($response))); + + $this->expectRequest($this->uri . 'README.md?view=co')->will($this->returnValue(Promise\resolve($response))); $promise = $this->client->fetchFile('README.md'); @@ -41,7 +47,7 @@ public function testFetchFile() public function testFetchFileExcessiveSlashesAreIgnored() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'README.md?view=co')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchFile('/README.md'); @@ -50,7 +56,7 @@ public function testFetchFileExcessiveSlashesAreIgnored() public function testFetchFileRevision() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=co&pathrev=1.0'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'README.md?view=co&pathrev=1.0')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchFile('/README.md', '1.0'); @@ -59,7 +65,7 @@ public function testFetchFileRevision() public function testFetchDirectoryRevision() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?pathrev=1.0'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'directory/?pathrev=1.0')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchDirectory('/directory/', '1.0'); @@ -68,7 +74,7 @@ public function testFetchDirectoryRevision() public function testFetchDirectoryAttic() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?hideattic=0'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'directory/?hideattic=0')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchDirectory('/directory/', null, true); @@ -77,7 +83,7 @@ public function testFetchDirectoryAttic() public function testFetchDirectoryRevisionAttic() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('directory/?pathrev=1.1&hideattic=0'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'directory/?pathrev=1.1&hideattic=0')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchDirectory('/directory/', '1.1', true); @@ -86,7 +92,7 @@ public function testFetchDirectoryRevisionAttic() public function testFetchLogRevision() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=log&pathrev=1.0'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'README.md?view=log&pathrev=1.0')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchLog('/README.md', '1.0'); @@ -95,10 +101,22 @@ public function testFetchLogRevision() public function testFetchPatch() { - $this->browser->expects($this->once())->method('get')->with($this->equalTo('README.md?view=patch&r1=1.0&r2=1.1'))->will($this->returnValue(Promise\reject())); + $this->expectRequest($this->uri . 'README.md?view=patch&r1=1.0&r2=1.1')->will($this->returnValue(Promise\reject())); $promise = $this->client->fetchPatch('/README.md', '1.0', '1.1'); $this->expectPromiseReject($promise); } + + private function expectRequest($uri) + { + $that = $this; + + return $this->sender->expects($this->once())->method('send')->with($this->callback(function (Request $request) use ($that, $uri) { + $that->assertEquals('GET', $request->getMethod()); + $that->assertEquals($uri, $request->getUri()); + + return true; + })); + } }