Merge pull request #17 from clue-labs/uri

Build properly escaped URIs via Browser
clue · Sep 4, 2015 · 379b715 · 379b715
2 parents 98d5c14 + 1c39a2e
commit 379b715
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 59 deletions.
diff --git 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
@@ -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
@@ -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;
@@ -43,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)
@@ -62,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
 
@@ -86,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)
@@ -123,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)
@@ -135,7 +147,7 @@ private function fetchXml($url)
 
     private function fetch($url)
     {
-        return $this->browser->get($this->url . 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
@@ -4,18 +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 $url;
-    private $browser;
+    private $uri = 'http://viewvc.example.org/';
+    private $sender;
     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);
+        $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()
@@ -33,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('http://viewvc.example.org/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');
 
@@ -42,17 +47,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->expectRequest($this->uri . '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->expectRequest($this->uri . 'README.md?view=co&pathrev=1.0')->will($this->returnValue(Promise\reject()));
 
         $promise = $this->client->fetchFile('/README.md', '1.0');
 
@@ -61,7 +65,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->expectRequest($this->uri . 'directory/?pathrev=1.0')->will($this->returnValue(Promise\reject()));
 
         $promise = $this->client->fetchDirectory('/directory/', '1.0');
 
@@ -70,7 +74,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->expectRequest($this->uri . 'directory/?hideattic=0')->will($this->returnValue(Promise\reject()));
 
         $promise = $this->client->fetchDirectory('/directory/', null, true);
 
@@ -79,7 +83,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->expectRequest($this->uri . 'directory/?pathrev=1.1&hideattic=0')->will($this->returnValue(Promise\reject()));
 
         $promise = $this->client->fetchDirectory('/directory/', '1.1', true);
 
@@ -88,7 +92,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->expectRequest($this->uri . 'README.md?view=log&pathrev=1.0')->will($this->returnValue(Promise\reject()));
 
         $promise = $this->client->fetchLog('/README.md', '1.0');
 
@@ -97,10 +101,22 @@ 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->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;
+        }));
+    }
 }
diff --git 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
@@ -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()