[Pkg-owncloud-commits] [php-sabredav] 102/163: Refactoring the propfind system. Work in progress.

David Prévot taffit at moszumanska.debian.org
Tue May 20 18:54:58 UTC 2014


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

taffit pushed a commit to annotated tag upstream/2.0.0_beta1
in repository php-sabredav.

commit 560027c07a5445d36f682e629b452e1aebb1a915
Author: Evert Pot <evert at rooftopsolutions.nl>
Date:   Sat May 3 00:07:10 2014 -0400

    Refactoring the propfind system. Work in progress.
---
 lib/Sabre/DAV/CorePlugin.php                     |  52 ++++++
 lib/Sabre/DAV/Server.php                         | 218 ++++++++++-------------
 tests/Sabre/DAV/Browser/GuessContentTypeTest.php |   2 +-
 tests/Sabre/DAV/Browser/PluginTest.php           |   2 +-
 4 files changed, 151 insertions(+), 123 deletions(-)

diff --git a/lib/Sabre/DAV/CorePlugin.php b/lib/Sabre/DAV/CorePlugin.php
index cdca51c..27e0523 100644
--- a/lib/Sabre/DAV/CorePlugin.php
+++ b/lib/Sabre/DAV/CorePlugin.php
@@ -45,6 +45,7 @@ class CorePlugin extends ServerPlugin {
 
         $server->on('propPatch', [$this, 'propPatchProtectedPropertyCheck'], 90);
         $server->on('propPatch', [$this, 'propPatchNodeUpdate'], 200);
+        $server->on('propFind',  [$this, 'propFind']);
 
     }
 
@@ -765,4 +766,55 @@ class CorePlugin extends ServerPlugin {
 
     }
 
+    /**
+     * This method is called when properties are retrieved.
+     *
+     * Here we add all the default properties.
+     *
+     * @param PropFind $propFind
+     * @param INode $node
+     * @return void
+     */
+    public function propFind(PropFind $propFind, INode $node) {
+
+        $propFind->handle('{DAV:}getlastmodified', function() use ($node) {
+            $lm = $node->getLastModified();
+            if ($lm) {
+                return new Property\GetLastModified($lm);
+            }
+        });
+
+        if ($node instanceof IFile) {
+            $propFind->handle('{DAV:}getcontentlength', [$node, 'getSize']);
+            $propFind->handle('{DAV:}getetag', [$node, 'getETag']);
+            $propFind->handle('{DAV:}getcontenttype', [$node, 'getContentType']);
+        }
+
+        if ($node instanceof IQuota) {
+            $quotaInfo = null;
+            $propFind->handle('{DAV:}quota-used-bytes', function() use (&$quotaInfo) {
+                $quotaInfo = $node->getQuotaInfo();
+                return $quotaInfo[0];
+            });
+            $propFind->handle('{DAV:}quota-available-bytes', function() use (&$quotaInfo) {
+                if (!$quotaInfo) {
+                    $quotaInfo = $node->getQuotaInfo();
+                }
+                return $quotaInfo[1];
+            });
+        }
+
+        $propFind->handle('{DAV:}supported-report-set', function() use ($propFind) {
+            $reports = [];
+            foreach($this->server->plugins as $plugin) {
+                $reports = array_merge($reports, $plugin->getSupportedReportSet($propFind->getPath()));
+            }
+            return new Property\SupportedReportSet($reports);
+        });
+        $propFind->handle('{DAV:}resourcetype', function() use ($node) {
+            return new Property\ResourceType($this->server->getResourceTypeForNode($node));
+        });
+
+    }
+
 }
diff --git a/lib/Sabre/DAV/Server.php b/lib/Sabre/DAV/Server.php
index ad7b5a2..1ab31fc 100644
--- a/lib/Sabre/DAV/Server.php
+++ b/lib/Sabre/DAV/Server.php
@@ -847,11 +847,29 @@ class Server extends EventEmitter {
     /**
      * Small helper to support PROPFIND with DEPTH_INFINITY.
      */
-    private function addPathNodesRecursively(&$nodes, $path) {
+    private function addPathNodesRecursively(&$propFindRequests, PropFind $propFind) {
+
+        $newDepth = $propFind->getDepth();
+        $path = $propFind->getPath();
+
+        if ($newDepth !== self::DEPTH_INFINITY) {
+            $newDepth--;
+        }
+
         foreach($this->tree->getChildren($path) as $childNode) {
-            $nodes[$path . '/' . $childNode->getName()] = $childNode;
-            if ($childNode instanceof ICollection)
-                $this->addPathNodesRecursively($nodes, $path . '/' . $childNode->getName());
+            $subPropFind = clone $propFind;
+            $subPropFind->setDepth($newDepth);
+            $subPropFind->setPath($path . '/' . $childNode->getName());
+
+            $propFindRequests[] = [
+                $subPropFind,
+                $childNode
+            ];
+
+            if ($newDepth===self::DEPTH_INFINITY || $newDepth>=1) {
+                $this->addPathNodesRecursively($propFindRequests, $subPropFind);
+            }
+
         }
     }
 
@@ -875,33 +893,45 @@ class Server extends EventEmitter {
         if (!$this->enablePropfindDepthInfinity && $depth != 0) $depth = 1;
 
         $path = rtrim($path,'/');
+        $propFind = new PropFind(rtrim($path,'/'), $propertyNames, $depth);
 
         // This event allows people to intercept these requests early on in the
         // process.
         //
         // We're not doing anything with the result, but this can be helpful to
         // pre-fetch certain expensive live properties.
-        $this->emit('beforeGetPropertiesForPath', [$path, $propertyNames, $depth]);
-
-        $returnPropertyList = [];
+        $this->emit('beforeGetPropertiesForPath', [$propFind]);
 
         $parentNode = $this->tree->getNodeForPath($path);
         $nodes = [
             $path => $parentNode
         ];
-        if ($depth==1 && $parentNode instanceof ICollection) {
-            foreach($this->tree->getChildren($path) as $childNode)
-                $nodes[$path . '/' . $childNode->getName()] = $childNode;
-        } else if ($depth == self::DEPTH_INFINITY && $parentNode instanceof ICollection) {
-            $this->addPathNodesRecursively($nodes, $path);
-        }
 
+        $propFindRequests = [[
+            $propFind,
+            $parentNode
+        ]];
+
+        if ($depth > 0 || $depth === self::DEPTH_INFINITY) {
+            $this->addPathNodesRecursively($propFindRequests, $propFind);
+        }
 
-        foreach($nodes as $myPath=>$node) {
+        foreach($propFindRequests as $propFindRequest) {
 
-            $r = $this->getPropertiesByNode($myPath, $node, $propertyNames);
+            $r = $this->getPropertiesByNode($propFindRequest[0], $propFindRequest[1]);
             if ($r) {
-                $returnPropertyList[] = $r;
+                $result = $propFindRequest[0]->getResultForMultiStatus();
+                $result['href'] = $propFindRequest[0]->getPath();
+
+                // WebDAV recommends adding a slash to the path, if the path is
+                // a collection.
+                // Furthermore, iCal also demands this to be the case for
+                // principals. This is non-standard, but we support it.
+                $resourceType = $this->getResourceTypeForNode($propFindRequest[1]);
+                if (in_array('{DAV:}collection', $resourceType) || in_array('{DAV:}principal', $resourceType)) {
+                    $result['href'].='/';
+                }
+                $returnPropertyList[] = $result;
             }
 
         }
@@ -952,139 +982,69 @@ class Server extends EventEmitter {
      * target node and simply want to run through the system to get a correct
      * list of properties.
      *
-     * @param string $path The path we're properties for fetching.
+     * @param PropFind $propFind
      * @param INode $node
-     * @param array $propertyNames list of properties to fetch.
-     * @return array
+     * @return bool
      */
-    public function getPropertiesByNode($path, INode $node, array $propertyNames) {
+    public function getPropertiesByNode(PropFind $propFind, INode $node) {
 
         $newProperties = [
             '200' => [],
             '404' => [],
         ];
 
-        // If no properties were supplied, it means this was an 'allprops'
-        // request, and we use a default set of properties.
-        $allProperties = count($propertyNames)===0;
-
-        if ($allProperties) {
-            // Default list of propertyNames, when all properties were requested.
-            $propertyNames = [
-                '{DAV:}getlastmodified',
-                '{DAV:}getcontentlength',
-                '{DAV:}resourcetype',
-                '{DAV:}quota-used-bytes',
-                '{DAV:}quota-available-bytes',
-                '{DAV:}getetag',
-                '{DAV:}getcontenttype',
-            ];
-        }
+        // Note: the beforeGetProperties is deprecated and will be removed from
+        // a future version of sabre/dav.
+        $requestedProperties = $propFind->getRequestedProperties();
+
+        $result = $this->emit('beforeGetProperties', [$propFind->getPath(), $node, &$requestedProperties, &$newProperties]);
 
-        // If the resourceType was not part of the list, we manually add it
-        // and mark it for removal. We need to know the resourcetype in order
-        // to make certain decisions about the entry.
-        // WebDAV dictates we should add a / and the end of href's for collections
-        $removeRT = false;
-        if (!in_array('{DAV:}resourcetype',$propertyNames)) {
-            $propertyNames[] = '{DAV:}resourcetype';
-            $removeRT = true;
+        // If any event handler returned false, it means that the operation
+        // should be stopped and this node should be completely ignored.
+        if ($result===false) {
+            return false;
         }
 
-        $result = $this->emit('beforeGetProperties',[$path, $node, &$propertyNames, &$newProperties]);
-        // If this method explicitly returned false, we must ignore this
-        // node as it is inaccessible.
-        if ($result===false) return;
-
-        if (count($propertyNames) > 0) {
-
-            if ($node instanceof IProperties) {
-                $nodeProperties = $node->getProperties($propertyNames);
-
-                // The getProperties method may give us too much,
-                // properties, in case the implementor was lazy.
-                //
-                // So as we loop through this list, we will only take the
-                // properties that were actually requested and discard the
-                // rest.
-                foreach($propertyNames as $k=>$propertyName) {
-                    if (isset($nodeProperties[$propertyName])) {
-                        unset($propertyNames[$k]);
-                        $newProperties[200][$propertyName] = $nodeProperties[$propertyName];
-                    }
-                }
+        foreach($newProperties as $status=>$propertyList) {
+
+            foreach($propertyList as $propertyName=>$value) {
+
+                $propFind->set($propertyName, $status, $value);
 
             }
 
         }
 
-        foreach($propertyNames as $prop) {
+        $this->emit('propFind', [$propFind, $node]);
 
-            if (isset($newProperties[200][$prop])) continue;
+        if ($node instanceof IProperties && $propertyNames = $propFind->get404Properties()) {
 
-            switch($prop) {
-                case '{DAV:}getlastmodified'       : if ($node->getLastModified()) $newProperties[200][$prop] = new Property\GetLastModified($node->getLastModified()); break;
-                case '{DAV:}getcontentlength'      :
-                    if ($node instanceof IFile) {
-                        $size = $node->getSize();
-                        if (!is_null($size)) {
-                            $newProperties[200][$prop] = (int)$node->getSize();
-                        }
-                    }
-                    break;
-                case '{DAV:}quota-used-bytes'      :
-                    if ($node instanceof IQuota) {
-                        $quotaInfo = $node->getQuotaInfo();
-                        $newProperties[200][$prop] = $quotaInfo[0];
-                    }
-                    break;
-                case '{DAV:}quota-available-bytes' :
-                    if ($node instanceof IQuota) {
-                        $quotaInfo = $node->getQuotaInfo();
-                        $newProperties[200][$prop] = $quotaInfo[1];
-                    }
-                    break;
-                case '{DAV:}getetag'               : if ($node instanceof IFile && $etag = $node->getETag())  $newProperties[200][$prop] = $etag; break;
-                case '{DAV:}getcontenttype'        : if ($node instanceof IFile && $ct = $node->getContentType())  $newProperties[200][$prop] = $ct; break;
-                case '{DAV:}supported-report-set'  :
-                    $reports = [];
-                    foreach($this->plugins as $plugin) {
-                        $reports = array_merge($reports, $plugin->getSupportedReportSet($path));
-                    }
-                    $newProperties[200][$prop] = new Property\SupportedReportSet($reports);
-                    break;
-                case '{DAV:}resourcetype' :
-                    $newProperties[200]['{DAV:}resourcetype'] = new Property\ResourceType();
-                    foreach($this->resourceTypeMapping as $className => $resourceType) {
-                        if ($node instanceof $className) $newProperties[200]['{DAV:}resourcetype']->add($resourceType);
-                    }
-                    break;
+            $nodeProperties = $node->getProperties($propertyNames);
 
+            foreach($nodeProperties as $propertyName=>$value) {
+                $propFind->set($propertyName, 200, $value);
             }
 
-            // If we were unable to find the property, we will list it as 404.
-            if (!$allProperties && !isset($newProperties[200][$prop])) $newProperties[404][$prop] = null;
-
         }
 
-        $this->emit('afterGetProperties',[trim($path,'/'),&$newProperties, $node]);
+        // Note: this event is also deprecated, and will be removed in a future
+        // version.
+        $newProperties = $propFind->getResultForMultiStatus();
+
+        $this->emit('afterGetProperties',[$propFind->getPath(), &$newProperties, $node]);
 
-        $newProperties['href'] = trim($path,'/');
+        // Ew
+        foreach($newProperties as $status=>$propertyList) {
+
+            foreach($propertyList as $propertyName=>$value) {
+
+                $propFind->set($propertyName, $status, $value);
 
-        // Its is a WebDAV recommendation to add a trailing slash to collectionnames.
-        // Apple's iCal also requires a trailing slash for principals (rfc 3744), though this is non-standard.
-        if ($path!='' && isset($newProperties[200]['{DAV:}resourcetype'])) {
-            $rt = $newProperties[200]['{DAV:}resourcetype'];
-            if ($rt->is('{DAV:}collection') || $rt->is('{DAV:}principal')) {
-                $newProperties['href'] .='/';
             }
-        }
 
-        // If the resourcetype property was manually added to the requested property list,
-        // we will remove it again.
-        if ($removeRT) unset($newProperties[200]['{DAV:}resourcetype']);
+        }
 
-        return $newProperties;
+        return true;
 
     }
 
@@ -1676,6 +1636,22 @@ class Server extends EventEmitter {
 
     }
 
+    /**
+     * Returns an array with resourcetypes for a node.
+     *
+     * @param INode $node
+     * @return array
+     */
+    public function getResourceTypeForNode(INode $node) {
+
+        $result = [];
+        foreach($this->resourceTypeMapping as $className => $resourceType) {
+            if ($node instanceof $className) $result[] = $resourceType;
+        }
+        return $result;
+
+    }
+
     // }}}
     // {{{ XML Readers & Writers
 
diff --git a/tests/Sabre/DAV/Browser/GuessContentTypeTest.php b/tests/Sabre/DAV/Browser/GuessContentTypeTest.php
index c88d44c..d3a5c64 100644
--- a/tests/Sabre/DAV/Browser/GuessContentTypeTest.php
+++ b/tests/Sabre/DAV/Browser/GuessContentTypeTest.php
@@ -45,7 +45,7 @@ class GuessContentTypeTest extends DAV\AbstractServer {
         );
         $result = $this->server->getPropertiesForPath('/somefile.jpg',$properties);
         $this->assertArrayHasKey(0,$result);
-        $this->assertArrayHasKey(200,$result[0]);
+        $this->assertArrayHasKey(200,$result[0], 'We received: ' . print_r($result,true));
         $this->assertArrayHasKey('{DAV:}getcontenttype',$result[0][200]);
         $this->assertEquals('image/jpeg',$result[0][200]['{DAV:}getcontenttype']);
 
diff --git a/tests/Sabre/DAV/Browser/PluginTest.php b/tests/Sabre/DAV/Browser/PluginTest.php
index c9da3f7..d5419c0 100644
--- a/tests/Sabre/DAV/Browser/PluginTest.php
+++ b/tests/Sabre/DAV/Browser/PluginTest.php
@@ -27,7 +27,7 @@ class PluginTest extends DAV\AbstractServer{
         $this->server->httpRequest = ($request);
         $this->server->exec();
 
-        $this->assertEquals(200, $this->response->status);
+        $this->assertEquals(200, $this->response->status, "Incorrect status received. Full response body: " . $this->response->getBodyAsString());
         $this->assertEquals(array(
             'Content-Type' => 'text/html; charset=utf-8',
             'Content-Security-Policy' => "img-src 'self'; style-src 'unsafe-inline';"

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