[Pkg-owncloud-commits] [php-sabredav] 122/148: Refactoring MKCOL to behave more like PROPPATCH

David Prévot taffit at moszumanska.debian.org
Wed Apr 15 01:37:29 UTC 2015


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

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

commit 88c49167dd17b075d19666eda055298eb80c45f0
Author: Evert Pot <me at evertpot.com>
Date:   Sat Apr 11 18:35:50 2015 -0400

    Refactoring MKCOL to behave more like PROPPATCH
    
    The changes we made earlier for PROPPATCH can also be applied to MKCOL.
    This patch makes MKCOL and PROPPATCH more consistent with each other,
    and also allows the PropertyStorage plugin to hook into MKCOL correctly.
    
    Work in progress.
---
 lib/CalDAV/CalendarHome.php                        | 15 ++--
 .../{UserAddressBooks.php => AddressBookHome.php}  | 23 +++---
 lib/CardDAV/AddressBookRoot.php                    |  2 +-
 lib/DAV/CorePlugin.php                             |  4 +-
 lib/DAV/IExtendedCollection.php                    | 24 +++++-
 lib/DAV/MkCol.php                                  | 73 ++++++++++++++++++
 lib/DAV/PropPatch.php                              | 24 +++++-
 lib/DAV/Server.php                                 | 88 +++++++---------------
 .../Sabre/CalDAV/CalendarHomeSubscriptionsTest.php | 10 ++-
 ...ddressBooksTest.php => AddressBookHomeTest.php} |  6 +-
 10 files changed, 178 insertions(+), 91 deletions(-)

diff --git a/lib/CalDAV/CalendarHome.php b/lib/CalDAV/CalendarHome.php
index 66874b1..af3bd83 100644
--- a/lib/CalDAV/CalendarHome.php
+++ b/lib/CalDAV/CalendarHome.php
@@ -5,6 +5,7 @@ namespace Sabre\CalDAV;
 use
     Sabre\DAV,
     Sabre\DAV\Exception\NotFound,
+    Sabre\DAV\MkCol,
     Sabre\DAVACL,
     Sabre\HTTP\URLUtil;
 
@@ -230,18 +231,18 @@ class CalendarHome implements DAV\IExtendedCollection, DAVACL\IACL {
     }
 
     /**
-     * Creates a new calendar
+     * Creates a new calendar or subscription.
      *
      * @param string $name
-     * @param array $resourceType
-     * @param array $properties
+     * @param MkCol $mkCol
+     * @throws DAV\Exception\InvalidResourceType
      * @return void
      */
-    function createExtendedCollection($name, array $resourceType, array $properties) {
+    function createExtendedCollection($name, MkCol $mkCol) {
 
         $isCalendar = false;
         $isSubscription = false;
-        foreach($resourceType as $rt) {
+        foreach($mkCol->getResourceType() as $rt) {
             switch ($rt) {
                 case '{DAV:}collection' :
                 case '{http://calendarserver.org/ns/}shared-owner' :
@@ -257,6 +258,10 @@ class CalendarHome implements DAV\IExtendedCollection, DAVACL\IACL {
                     throw new DAV\Exception\InvalidResourceType('Unknown resourceType: ' . $rt);
             }
         }
+
+        $properties = $mkCol->getRemainingValues();
+        $mkCol->setRemainingResultCode(201);
+
         if ($isSubscription) {
             if (!$this->caldavBackend instanceof Backend\SubscriptionSupport) {
                 throw new DAV\Exception\InvalidResourceType('This backend does not support subscriptions');
diff --git a/lib/CardDAV/UserAddressBooks.php b/lib/CardDAV/AddressBookHome.php
similarity index 89%
rename from lib/CardDAV/UserAddressBooks.php
rename to lib/CardDAV/AddressBookHome.php
index 82994e3..17b0172 100644
--- a/lib/CardDAV/UserAddressBooks.php
+++ b/lib/CardDAV/AddressBookHome.php
@@ -4,19 +4,20 @@ namespace Sabre\CardDAV;
 
 use
     Sabre\DAV,
+    Sabre\DAV\MkCol,
     Sabre\DAVACL,
-    Sabre\HTTP\URLUtil;
+    Sabre\Uri;
 
 /**
- * UserAddressBooks class
+ * AddressBook Home class
  *
- * The UserAddressBooks collection contains a list of addressbooks associated with a user
+ * This collection contains a list of addressbooks associated with one user.
  *
  * @copyright Copyright (C) 2007-2015 fruux GmbH (https://fruux.com/).
  * @author Evert Pot (http://evertpot.com/)
  * @license http://sabre.io/license/ Modified BSD License
  */
-class UserAddressBooks extends DAV\Collection implements DAV\IExtendedCollection, DAVACL\IACL {
+class AddressBookHome extends DAV\Collection implements DAV\IExtendedCollection, DAVACL\IACL {
 
     /**
      * Principal uri
@@ -52,7 +53,7 @@ class UserAddressBooks extends DAV\Collection implements DAV\IExtendedCollection
      */
     function getName() {
 
-        list(,$name) = URLUtil::splitPath($this->principalUri);
+        list(,$name) = Uri\split($this->principalUri);
         return $name;
 
     }
@@ -155,18 +156,20 @@ class UserAddressBooks extends DAV\Collection implements DAV\IExtendedCollection
     }
 
     /**
-     * Creates a new addressbook
+     * Creates a new calendar or subscription.
      *
      * @param string $name
-     * @param array $resourceType
-     * @param array $properties
+     * @param MkCol $mkCol
+     * @throws DAV\Exception\InvalidResourceType
      * @return void
      */
-    function createExtendedCollection($name, array $resourceType, array $properties) {
+    function createExtendedCollection($name, MkCol $mkCol) {
 
-        if (!in_array('{'.Plugin::NS_CARDDAV.'}addressbook',$resourceType) || count($resourceType)!==2) {
+        if (!$mkCol->hasResourceType('{'.Plugin::NS_CARDDAV.'}addressbook')) {
             throw new DAV\Exception\InvalidResourceType('Unknown resourceType for this collection');
         }
+        $properties = $mkCol->getRemainingValues();
+        $mkCol->setRemainingStatus(201);
         $this->carddavBackend->createAddressBook($this->principalUri, $name, $properties);
 
     }
diff --git a/lib/CardDAV/AddressBookRoot.php b/lib/CardDAV/AddressBookRoot.php
index 20d6e20..c1c5cee 100644
--- a/lib/CardDAV/AddressBookRoot.php
+++ b/lib/CardDAV/AddressBookRoot.php
@@ -73,7 +73,7 @@ class AddressBookRoot extends DAVACL\AbstractPrincipalCollection {
      */
     function getChildForPrincipal(array $principal) {
 
-        return new UserAddressBooks($this->carddavBackend, $principal['uri']);
+        return new AddressBookHome($this->carddavBackend, $principal['uri']);
 
     }
 
diff --git a/lib/DAV/CorePlugin.php b/lib/DAV/CorePlugin.php
index dd163b1..72d0c92 100644
--- a/lib/DAV/CorePlugin.php
+++ b/lib/DAV/CorePlugin.php
@@ -586,7 +586,9 @@ class CorePlugin extends ServerPlugin {
 
         }
 
-        $result = $this->server->createCollection($path, $resourceType, $properties);
+        $mkcol = new MkCol($resourceType, $properties);
+
+        $result = $this->server->createCollection($path, $mkcol);
 
         if (is_array($result)) {
             $response->setStatus(207);
diff --git a/lib/DAV/IExtendedCollection.php b/lib/DAV/IExtendedCollection.php
index 1ce3ea6..e398d5e 100644
--- a/lib/DAV/IExtendedCollection.php
+++ b/lib/DAV/IExtendedCollection.php
@@ -15,14 +15,30 @@ namespace Sabre\DAV;
 interface IExtendedCollection extends ICollection {
 
     /**
-     * Creates a new collection
+     * Creates a new collection.
+     *
+     * This method will receive MkCol object with all the information about the
+     * new collection that's being created.
+     *
+     * The MkCol object contains information about the resourceType of the new
+     * collection. If you don't support the specified resourceType, you should
+     * throw Exception\InvalidResourceType.
+     *
+     * The object also contains a list of WebDAV properties for the new
+     * collection.
+     *
+     * You should call the handle() method on this object to specify exactly
+     * which properties you are storing. This allows the system to figure out
+     * exactly which properties you didn't store, which in turn allows other
+     * plugins (such as the propertystorage plugin) to handle storing the
+     * property for you.
      *
      * @param string $name
-     * @param array $resourceType
-     * @param array $properties
+     * @param MkCol $mkCol
+     * @throws Exception\InvalidResourceType
      * @return void
      */
-    function createExtendedCollection($name, array $resourceType, array $properties);
+    function createExtendedCollection($name, MkCol $mkCol);
 
 }
 
diff --git a/lib/DAV/MkCol.php b/lib/DAV/MkCol.php
new file mode 100644
index 0000000..b874a3d
--- /dev/null
+++ b/lib/DAV/MkCol.php
@@ -0,0 +1,73 @@
+<?php
+
+namespace Sabre\DAV;
+
+use UnexpectedValueException;
+
+/**
+ * This class represents a MKCOL operation.
+ *
+ * MKCOL creates a new collection. MKCOL comes in two flavours:
+ *
+ * 1. MKCOL with no body, signifies the creation of a simple collection.
+ * 2. MKCOL with a request body. This can create a collection with a specific
+ *    resource type, and a set of properties that should be set on the new
+ *    collection. This can be used to create caldav calendars, carddav address
+ *    books, etc.
+ *
+ * Property updates must always be atomic. This means that a property update
+ * must either completely succeed, or completely fail.
+ *
+ * @copyright Copyright (C) 2007-2015 fruux GmbH (https://fruux.com/).
+ * @author Evert Pot (http://evertpot.com/)
+ * @license http://sabre.io/license/ Modified BSD License
+ */
+class MkCol extends PropPatch {
+
+    /**
+     * A list of resource-types in clark-notation.
+     * 
+     * @var array 
+     */
+    protected $resourceType;
+
+    /**
+     * Creates the MKCOL object.
+     * 
+     * @param string[] $resourceType List of resourcetype values. 
+     * @param array $mutations List of new properties values.
+     */
+    function __construct(array $resourceType, array $mutations) {
+
+        $this->resourceType = $resourceType;
+        parent::__construct($mutations);
+
+    }
+
+    /**
+     * Returns the resourcetype of the new collection.
+     *
+     * @return string[]
+     */
+    function getResourceType() {
+
+        return $this->resourceType;
+
+    }
+
+    /**
+     * Returns true or false if the MKCOL operation has at least the specified
+     * resource type.
+     *
+     * If the resourcetype is specified as an array, all resourcetypes are
+     * checked.
+     *
+     * @param string|string[] $resourceType
+     */
+    function hasResourceType($resourceType) {
+
+        return count(array_diff((array)$resourceType, $this->resourceType)) === 0;
+
+    }
+
+}
diff --git a/lib/DAV/PropPatch.php b/lib/DAV/PropPatch.php
index 4ac8791..d14f8e4 100644
--- a/lib/DAV/PropPatch.php
+++ b/lib/DAV/PropPatch.php
@@ -170,7 +170,9 @@ class PropPatch {
     /**
      * Returns the list of properties that don't have a result code yet.
      *
-     * @return array
+     * This method returns a list of property names, but not its values.
+     *
+     * @return string[]
      */
     function getRemainingMutations() {
 
@@ -186,6 +188,26 @@ class PropPatch {
     }
 
     /**
+     * Returns the list of properties that don't have a result code yet.
+     *
+     * This method returns list of properties and their values.
+     *
+     * @return array
+     */
+    function getRemainingValues() {
+
+        $remaining = [];
+        foreach($this->mutations as $propertyName => $propValue) {
+            if (!isset($this->result[$propertyName])) {
+                $remaining[$propertyName] = $propValue;
+            }
+        }
+
+        return $remaining;
+
+    }
+
+    /**
      * Performs the actual update, and calls all callbacks.
      *
      * This method returns true or false depending on if the operation was
diff --git a/lib/DAV/Server.php b/lib/DAV/Server.php
index ecd8f5d..ee50f64 100644
--- a/lib/DAV/Server.php
+++ b/lib/DAV/Server.php
@@ -1095,40 +1095,32 @@ class Server extends EventEmitter {
      */
     function createDirectory($uri) {
 
-        $this->createCollection($uri,['{DAV:}collection'], []);
+        $this->createCollection($uri,new MkCol(['{DAV:}collection'], []));
 
     }
 
     /**
      * Use this method to create a new collection
      *
-     * The {DAV:}resourcetype is specified using the resourceType array.
-     * At the very least it must contain {DAV:}collection.
-     *
-     * The properties array can contain a list of additional properties.
-     *
      * @param string $uri The new uri
-     * @param array $resourceType The resourceType(s)
-     * @param array $properties A list of properties
+     * @param MkCol $mkCol
      * @return array|null
      */
-    function createCollection($uri, array $resourceType, array $properties) {
+    function createCollection($uri, MkCol $mkCol) {
 
         list($parentUri,$newName) = URLUtil::splitPath($uri);
 
         // Making sure {DAV:}collection was specified as resourceType
-        if (!in_array('{DAV:}collection', $resourceType)) {
+        if (!$mkCol->hasResourceType('{DAV:}collection')) {
             throw new Exception\InvalidResourceType('The resourceType for this collection must at least include {DAV:}collection');
         }
 
 
         // Making sure the parent exists
         try {
-
             $parent = $this->tree->getNodeForPath($parentUri);
 
         } catch (Exception\NotFound $e) {
-
             throw new Exception\Conflict('Parent node does not exist');
 
         }
@@ -1138,8 +1130,6 @@ class Server extends EventEmitter {
             throw new Exception\Conflict('Parent node is not a collection');
         }
 
-
-
         // Making sure the child does not already exist
         try {
             $parent->getChild($newName);
@@ -1148,72 +1138,46 @@ class Server extends EventEmitter {
             throw new Exception\MethodNotAllowed('The resource you tried to create already exists');
 
         } catch (Exception\NotFound $e) {
-            // This is correct
+            // NotFound is the expected behavior.
         }
 
 
         if (!$this->emit('beforeBind',[$uri])) return;
 
-        // There are 2 modes of operation. The standard collection
-        // creates the directory, and then updates properties
-        // the extended collection can create it directly.
         if ($parent instanceof IExtendedCollection) {
 
-            $parent->createExtendedCollection($newName, $resourceType, $properties);
+            /**
+             * If the parent is an instanced of IExtendedCollection, it means that
+             * we can pass the MkCol object directly as it may be able to store
+             * properties immediately.
+             */
+            $parent->createExtendedCollection($newName, $mkCol);
 
         } else {
 
-            // No special resourcetypes are supported
-            if (count($resourceType)>1) {
+            /**
+             * If the parent is a standard ICollection, it means only
+             * 'standard' collections can be created, so we should fail any
+             * MKCOL operation that carry extra resourcetypes.
+             */
+            if (count($mkCol->getResourceType())>1) {
                 throw new Exception\InvalidResourceType('The {DAV:}resourcetype you specified is not supported here.');
             }
 
             $parent->createDirectory($newName);
-            $rollBack = false;
-            $exception = null;
-            $errorResult = null;
-
-            if (count($properties)>0) {
-
-                try {
-
-                    $errorResult = $this->updateProperties($uri, $properties);
-                    if (!isset($errorResult[200])) {
-                        $rollBack = true;
-                    }
-
-                } catch (Exception $e) {
-
-                    $rollBack = true;
-                    $exception = $e;
 
-                }
-
-            }
-
-            if ($rollBack) {
-                if (!$this->emit('beforeUnbind',[$uri])) return;
-                $this->tree->delete($uri);
+        }
 
-                // Re-throwing exception
-                if ($exception) throw $exception;
+        // If there are any properties that have not been handled/stored,
+        // we ask the 'propPatch' event to handle them. This will allow for
+        // example the propertyStorage system to store properties upon MKCOL.
+        if ($mkCol->getRemainingMutations()) {
+            $this->emit('propPatch', [$uri, $mkCol]);
+        }
+        $success = $mkCol->commit();
 
-                // Re-arranging the result so it makes sense for
-                // generateMultiStatus.
-                $newResult = [
-                    'href' => $uri,
-                ];
-                foreach($errorResult as $property=>$code) {
-                    if (!isset($newResult[$code])) {
-                        $newResult[$code] = [$property => null];
-                    } else {
-                        $newResult[$code][$property] = null;
-                    }
-                }
-                return $newResult;
-            }
+        if (!$success) return $mkCol->getResult();
 
-        }
         $this->tree->markDirty($parentUri);
         $this->emit('afterBind',[$uri]);
 
diff --git a/tests/Sabre/CalDAV/CalendarHomeSubscriptionsTest.php b/tests/Sabre/CalDAV/CalendarHomeSubscriptionsTest.php
index 7ebc331..9610ed3 100644
--- a/tests/Sabre/CalDAV/CalendarHomeSubscriptionsTest.php
+++ b/tests/Sabre/CalDAV/CalendarHomeSubscriptionsTest.php
@@ -2,7 +2,9 @@
 
 namespace Sabre\CalDAV;
 
-use Sabre\DAVACL;
+use
+    Sabre\DAV\MkCol,
+    Sabre\DAVACL;
 
 class CalendarHomeSubscriptionsTest extends \PHPUnit_Framework_TestCase {
 
@@ -54,7 +56,7 @@ class CalendarHomeSubscriptionsTest extends \PHPUnit_Framework_TestCase {
             '{DAV:}displayname' => 'baz',
             '{http://calendarserver.org/ns/}source' => new \Sabre\DAV\Xml\Property\Href('http://example.org/test2.ics'),
         ];
-        $instance->createExtendedCollection('sub2', $rt, $props);
+        $instance->createExtendedCollection('sub2', new MkCol($rt, $props));
 
         $children = $instance->getChildren();
         $this->assertEquals(2, count($children));
@@ -64,7 +66,7 @@ class CalendarHomeSubscriptionsTest extends \PHPUnit_Framework_TestCase {
     /**
      * @expectedException \Sabre\DAV\Exception\InvalidResourceType
      */
-    public function testNoSubscriptionSupport() {
+    function testNoSubscriptionSupport() {
 
         $principal = [
             'uri' => 'principals/user1'
@@ -78,7 +80,7 @@ class CalendarHomeSubscriptionsTest extends \PHPUnit_Framework_TestCase {
             '{DAV:}displayname' => 'baz',
             '{http://calendarserver.org/ns/}source' => new \Sabre\DAV\Xml\Property\Href('http://example.org/test2.ics'),
         ];
-        $uC->createExtendedCollection('sub2', $rt, $props);
+        $uC->createExtendedCollection('sub2', new MkCol($rt, $props));
 
     }
 
diff --git a/tests/Sabre/CardDAV/UserAddressBooksTest.php b/tests/Sabre/CardDAV/AddressBookHomeTest.php
similarity index 95%
rename from tests/Sabre/CardDAV/UserAddressBooksTest.php
rename to tests/Sabre/CardDAV/AddressBookHomeTest.php
index a6ecf3e..cdf88ca 100644
--- a/tests/Sabre/CardDAV/UserAddressBooksTest.php
+++ b/tests/Sabre/CardDAV/AddressBookHomeTest.php
@@ -2,10 +2,10 @@
 
 namespace Sabre\CardDAV;
 
-class UserAddressBooksTest extends \PHPUnit_Framework_TestCase {
+class AddressBookHomeTest extends \PHPUnit_Framework_TestCase {
 
     /**
-     * @var Sabre\CardDAV\UserAddressBooks
+     * @var Sabre\CardDAV\AddressBookHome
      */
     protected $s;
     protected $backend;
@@ -13,7 +13,7 @@ class UserAddressBooksTest extends \PHPUnit_Framework_TestCase {
     function setUp() {
 
         $this->backend = new Backend\Mock();
-        $this->s = new UserAddressBooks(
+        $this->s = new AddressBookHome(
             $this->backend,
             'principals/user1'
         );

-- 
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