[Pkg-owncloud-commits] [php-sabredav] 54/80: Better error reporting in Client::propPatch.

David Prévot taffit at moszumanska.debian.org
Thu Jan 7 02:56:33 UTC 2016


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch master
in repository php-sabredav.

commit 364012fa4c0a457543e6b24915f6ac9c092dd06e
Author: Evert Pot <me at evertpot.com>
Date:   Mon Jan 4 22:32:07 2016 -0500

    Better error reporting in Client::propPatch.
    
    This function:
    
    1. Now throws an exception if there's a HTTP error, even if
       `throwExceptions` disabled.
    2. Also inspects the response body when it's a 207 Multi-status and also
       throws the appropriate error.
    3. Returns true if it succeeded.
    
    Fixes #726
---
 CHANGELOG.md                   |  2 ++
 lib/DAV/Client.php             | 33 ++++++++++++++++++--
 tests/Sabre/DAV/ClientTest.php | 68 +++++++++++++++++++++++++++++++++++-------
 3 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c4978b1..25de879 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@ ChangeLog
 * #727: Added another workaround to make CalDAV work for Windows 10 clients.
 * #742: Fixes to make sure that vobject 4 is correctly supported.
 * Subtle browser improvements.
+* #726: Better error reporting in `Client::propPatch`. We're now throwing
+  exceptions.
 
 
 3.1.0-alpha2 (2015-09-05)
diff --git a/lib/DAV/Client.php b/lib/DAV/Client.php
index 741ceee..bcedbd7 100644
--- a/lib/DAV/Client.php
+++ b/lib/DAV/Client.php
@@ -260,7 +260,7 @@ class Client extends HTTP\Client {
      *
      * @param string $url
      * @param array $properties
-     * @return void
+     * @return bool
      */
     function propPatch($url, array $properties) {
 
@@ -275,7 +275,36 @@ class Client extends HTTP\Client {
         $request = new HTTP\Request('PROPPATCH', $url, [
             'Content-Type' => 'application/xml',
         ], $xml);
-        $this->send($request);
+        $response = $this->send($request);
+
+        if ($response->getStatus() >= 400) {
+            throw new \Sabre\HTTP\ClientHttpException($response);
+        }
+
+        if ($response->getStatus() === 207) {
+            // If it's a 207, the request could still have failed, but the
+            // information is hidden in the response body.
+            $result = $this->parseMultiStatus($response->getBodyAsString());
+
+            $errorProperties = [];
+            foreach ($result as $href => $statusList) {
+                foreach ($statusList as $status => $properties) {
+
+                    if ($status >= 400) {
+                        foreach ($properties as $propName => $propValue) {
+                            $errorProperties[] = $propName . ' (' . $status . ')';
+                        }
+                    }
+
+                }
+            }
+            if ($errorProperties) {
+
+                throw new \Sabre\HTTP\ClientException('PROPPATCH failed. The following properties errored: ' . implode(', ', $errorProperties));
+            }
+        }
+        return true;
+
     }
 
     /**
diff --git a/tests/Sabre/DAV/ClientTest.php b/tests/Sabre/DAV/ClientTest.php
index 1184057..fe71511 100644
--- a/tests/Sabre/DAV/ClientTest.php
+++ b/tests/Sabre/DAV/ClientTest.php
@@ -19,9 +19,9 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
 
     function testConstruct() {
 
-        $client = new ClientMock(array(
+        $client = new ClientMock([
             'baseUri' => '/',
-        ));
+        ]);
         $this->assertInstanceOf('Sabre\DAV\ClientMock', $client);
 
     }
@@ -31,14 +31,14 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
      */
     function testConstructNoBaseUri() {
 
-        $client = new ClientMock(array());
+        $client = new ClientMock([]);
 
     }
 
     function testAuth() {
 
         $client = new ClientMock([
-            'baseUri' => '/',
+            'baseUri'  => '/',
             'userName' => 'foo',
             'password' => 'bar',
         ]);
@@ -51,7 +51,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
     function testBasicAuth() {
 
         $client = new ClientMock([
-            'baseUri' => '/',
+            'baseUri'  => '/',
             'userName' => 'foo',
             'password' => 'bar',
             'authType' => Client::AUTH_BASIC
@@ -65,7 +65,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
     function testDigestAuth() {
 
         $client = new ClientMock([
-            'baseUri' => '/',
+            'baseUri'  => '/',
             'userName' => 'foo',
             'password' => 'bar',
             'authType' => Client::AUTH_DIGEST
@@ -79,7 +79,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
     function testNTLMAuth() {
 
         $client = new ClientMock([
-            'baseUri' => '/',
+            'baseUri'  => '/',
             'userName' => 'foo',
             'password' => 'bar',
             'authType' => Client::AUTH_NTLM
@@ -94,7 +94,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
 
         $client = new ClientMock([
             'baseUri' => '/',
-            'proxy' => 'localhost:8888',
+            'proxy'   => 'localhost:8888',
         ]);
 
         $this->assertEquals("localhost:8888", $client->curlSettings[CURLOPT_PROXY]);
@@ -104,7 +104,7 @@ class ClientTest extends \PHPUnit_Framework_TestCase {
     function testEncoding() {
 
         $client = new ClientMock([
-            'baseUri' => '/',
+            'baseUri'  => '/',
             'encoding' => Client::ENCODING_IDENTITY | Client::ENCODING_GZIP | Client::ENCODING_DEFLATE,
         ]);
 
@@ -142,7 +142,7 @@ XML;
         $this->assertEquals('PROPFIND', $request->getMethod());
         $this->assertEquals('/foo', $request->getUrl());
         $this->assertEquals([
-            'Depth' => ['0'],
+            'Depth'        => ['0'],
             'Content-Type' => ['application/xml'],
         ], $request->getHeaders());
 
@@ -196,7 +196,7 @@ XML;
         $this->assertEquals('PROPFIND', $request->getMethod());
         $this->assertEquals('/foo', $request->getUrl());
         $this->assertEquals([
-            'Depth' => ['1'],
+            'Depth'        => ['1'],
             'Content-Type' => ['application/xml'],
         ], $request->getHeaders());
 
@@ -225,6 +225,7 @@ XML;
 
         $client->response = new Response(207, [], $responseBody);
         $result = $client->propPatch('foo', ['{DAV:}displayname' => 'hi', '{urn:zim}gir' => null], 1);
+        $this->assertTrue($result);
         $request = $client->request;
         $this->assertEquals('PROPPATCH', $request->getMethod());
         $this->assertEquals('/foo', $request->getUrl());
@@ -234,6 +235,51 @@ XML;
 
     }
 
+    /**
+     * @depends testPropPatch
+     * @expectedException \Sabre\HTTP\ClientHttpException
+     */
+    function testPropPatchHTTPError() {
+
+        $client = new ClientMock([
+            'baseUri' => '/',
+        ]);
+
+        $client->response = new Response(403, [], '');
+        $client->propPatch('foo', ['{DAV:}displayname' => 'hi', '{urn:zim}gir' => null], 1);
+
+    }
+
+    /**
+     * @depends testPropPatch
+     * @expectedException Sabre\HTTP\ClientException
+     */
+    function testPropPatchMultiStatusError() {
+
+        $client = new ClientMock([
+            'baseUri' => '/',
+        ]);
+
+        $responseBody = <<<XML
+<?xml version="1.0"?>
+<multistatus xmlns="DAV:">
+<response>
+  <href>/foo</href>
+  <propstat>
+    <prop>
+      <displayname />
+    </prop>
+    <status>HTTP/1.1 403 Forbidden</status>
+  </propstat>
+</response>
+</multistatus>
+XML;
+
+        $client->response = new Response(207, [], $responseBody);
+        $client->propPatch('foo', ['{DAV:}displayname' => 'hi', '{urn:zim}gir' => null], 1);
+
+    }
+
     function testOPTIONS() {
 
         $client = new ClientMock([

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/php-sabredav.git



More information about the Pkg-owncloud-commits mailing list