refactored methods from ObjectManager to Helper

extracted normalizing from ObjectManager::absolutePath into Helper::normalizePath and moved ::absolutePath to Helper:: changed Node::getNode to throw PHPCR_PathNotFoundException instead of PHPCR_ItemNotFoundException. fixed Property::checkMultiple exception instantiation. verify path absoluteness and well-formedness in Session::nodeExists and Session::propertyExists directly.
jackalope · Aug 18, 2010 · 6fd1e42 · 6fd1e42
1 parent 9e44d2c
commit 6fd1e42
Show file tree

Hide file tree

Showing 8 changed files with 175 additions and 73 deletions.
diff --git a/src/jackalope/Helper.php b/src/jackalope/Helper.php
@@ -14,4 +14,87 @@ public static function getBoolAttribute($node, $attribute) {
             return true;
         }
     }
-}
+
+    /**
+     * Normalizes a path according to JCR's spec (3.4.5)
+     * 
+     * <ul>
+     *   <li>All self segments(.) are removed.</li>
+     *   <li>All redundant parent segments(..) are collapsed.</li>
+     *   <li>If the path is an identifier-based absolute path, it is replaced by a root-based 
+     *       absolute path that picks out the same node in the workspace as the identifier it replaces.</li>
+     * </ul>
+     *
+     * @param   string  $path   The path to normalize
+     * @return  string  The normalized path
+     */
+    public static function normalizePath($path) {
+        // UUDID is HEX_CHAR{8}-HEX_CHAR{4}-HEX_CHAR{4}-HEX_CHAR{4}-HEX_CHAR{12}
+        if (1 === preg_match('/^\[([[:xdigit:]]{8}-[[:xdigit:]]{4}-[[:xdigit:]]{4}-[[:xdigit:]]{4}-[[:xdigit:]]{12})\]$/', $path)) {
+            // TODO: replace by a root-based absolute path of the given item
+            $uuid = $path[1];
+            throw new jackalope_NotImplementedException('Normalizing identifier-based absolute paths not implemented');
+        } else {
+            $finalPath = array();
+            $abs = ($path && $path[0] == '/');
+            $parts = explode('/', $path);
+            foreach ($parts as $pathPart) {
+                switch ($pathPart) {
+                    case '.':
+                    case '':
+                        break;
+                    case '..':
+                        array_pop($finalPath);
+                        break;
+                    default:
+                        array_push($finalPath, $pathPart);
+                        break;
+                }
+            }
+            $finalPath = implode('/', $finalPath);
+            if ($abs) {
+              $finalPath = '/'.$finalPath;
+            }
+            return $finalPath;
+        }
+    }
+
+    /**
+     * Creates an absolute path from a root and a relative path
+     * and then normalizes it
+     *
+     * If root is missing or does not start with a slash, a slash will be prepended
+     *
+     * @param string Root path to append the relative
+     * @param string Relative path
+     * @return string Absolute and normalized path
+     */
+    public static function absolutePath($root, $relPath) {
+
+        $root = trim($root, '/');
+        $concat = $root;
+        if (strlen($root)) {
+            $concat = "/$root/";
+        } else {
+            $concat = '/';
+        }
+        $concat .= ltrim($relPath, '/');
+
+        // TODO: maybe this should be required explicitly and not called from within this method...
+        return self::normalizePath($concat);
+    }
+
+    public static function isAbsolutePath($path) {
+        return $path && $path[0] == '/';
+    }
+
+    /**
+     * Whether the path conforms to the JCR Specs (see paragraph 3.2)
+     * 
+     * TODO: only minimal check performed atm, not full specs
+     */ 
+    public static function isValidPath($path) {
+        return (strpos($path, '//') === false && preg_match('/^[\w{}\/#:^+~*\[\]\.-]*$/i', $path));
+    }
+
+}
diff --git a/src/jackalope/Node.php b/src/jackalope/Node.php
@@ -204,7 +204,13 @@ public function setProperty($name, $value, $type = NULL) {
      * @api
      */
     public function getNode($relPath) {
-        return $this->objectManager->getNodeByPath($this->path . "/$relPath");
+        $node = null;
+        try {
+            $node = $this->objectManager->getNodeByPath(jackalope_Helper::absolutePath($this->path, $relPath));
+        } catch (PHPCR_ItemNotFoundException $e) {
+            throw new PHPCR_PathNotFoundException($e->getMessage(), $e->getCode(), $e);
+        }
+        return $node;
     }
 
     /**

diff --git a/src/jackalope/ObjectManager.php b/src/jackalope/ObjectManager.php
@@ -27,23 +27,26 @@ public function __construct(jackalope_TransportInterface $transport,
      *
      * @param string $path The absolute path of the node to create
      * @return PHPCR_Node
-     * @throws PHPCR_RepositoryException    If the path is not absolute or well-formed
+     * @throws PHPCR_RepositoryException    If the path is not absolute or not well-formed
      */
-    public function getNodeByPath($path) {
-        $this->verifyAbsPath($path);
-        if (empty($this->objectsByPath[$path])) {
-            $this->objectsByPath[$path] = jackalope_Factory::get(
+    public function getNodeByPath($absPath) {
+        $absPath = jackalope_Helper::normalizePath($absPath);
+
+        $this->verifyAbsolutePath($absPath);
+
+        if (empty($this->objectsByPath[$absPath])) {
+            $this->objectsByPath[$absPath] = jackalope_Factory::get(
                 'Node',
                 array(
-                    $this->transport->getItem($path),
-                    $path,
+                    $this->transport->getItem($absPath),
+                    $absPath,
                     $this->session,
                     $this
                 )
             );
         }
         //OPTIMIZE: also save in the uuid array
-        return $this->objectsByPath[$path];
+        return $this->objectsByPath[$absPath];
     }
 
     /**
@@ -52,12 +55,15 @@ public function getNodeByPath($path) {
      *
      * @param string $path The absolute path of the property to create
      * @return PHPCR_Property
-     * @throws PHPCR_RepositoryException    If the path is not absolute or well-formed
+     * @throws PHPCR_RepositoryException    If the path is not absolute or not well-formed
      */
-    public function getPropertyByPath($path) {
-        $this->verifyAbsPath($path);
-        $name = substr($path,strrpos($path,'/')+1); //the property name
-        $nodep = substr($path,0,strrpos($path,'/')); //the node this property should be in
+    public function getPropertyByPath($absPath) {
+        $absPath = jackalope_Helper::normalizePath($absPath);
+
+        $this->verifyAbsolutePath($absPath);
+
+        $name = substr($absPath,strrpos($absPath,'/')+1); //the property name
+        $nodep = substr($absPath,0,strrpos($absPath,'/')+1); //the node this property should be in
         /* OPTIMIZE? instead of fetching the node, we could make Transport provide it with a
          * GET /server/tests/jcr%3aroot/tests_level1_access_base/multiValueProperty/jcr%3auuid
          * (davex getItem uses json, which is not applicable to properties)
@@ -86,7 +92,7 @@ public function getNode($identifier, $root = '/'){
                 return $this->getNodeByPath($this->objectsByUuid[$identifier]);
             }
         } else {
-            $path = $this->absolutePath($root, $identifier);
+            $path = jackalope_Helper::absolutePath($root, $identifier);
             return $this->getNodeByPath($path);
         }
     }
@@ -110,49 +116,21 @@ public function getNodeType($nodeType) {
         return $this->getNodeTypes(array($nodeType));
     }
 
-    /**
-     * Creates an absolute path from a root and an relative path
-     * @param string Root path to append the relative
-     * @param string Relative path
-     * @return string Absolute path with . and .. resolved
-     */
-    protected function absolutePath($root, $relPath) {
-        $finalPath = array();
-        $path = array_merge(explode('/', '/' . $root), explode('/', '/' . $relPath));
-        foreach ($path as $pathPart) {
-            switch ($pathPart) {
-                case '.':
-                case '':
-                    break;
-                case '..':
-                    array_pop($finalPath);
-                    break;
-                default:
-                    array_push($finalPath, $pathPart);
-                    break;
-            }
-        }
-
-        return '/'.implode('/', $finalPath);
-    }
-
     /**
      * Verifies the path to be absolute and well-formed
      *
      * @param string $path the path to verify 
+     * @return  bool    Always true :)
      * @throws PHPCR_RepositoryException    If the path is not absolute or well-formed
      */
-    protected function verifyAbsPath($path) {
-        if ($path[0] != '/') {
+    public function verifyAbsolutePath($path) {
+        if (!jackalope_Helper::isAbsolutePath($path)) {
             throw new PHPCR_RepositoryException('Path is not absolute: ' . $path);
         }
-        // TODO: incomplete pattern, see JCR Specs 3.2
-        if (strpos($path, '//') !== false ||
-            1 != preg_match('/^[\w{}\/#:^+~*\[\]-]*$/i', $path)) {
-
+        if (!jackalope_Helper::isValidPath($path)) {
             throw new PHPCR_RepositoryException('Path is not well-formed (TODO: match against spec): ' . $path);
         }
-        return true;
+        return true; 
     }
 
     /**

diff --git a/src/jackalope/Property.php b/src/jackalope/Property.php
@@ -340,10 +340,11 @@ public function isMultiple() {
 
     /**
      * Throws an exception if the property is multivalued
+     * @throws PHPCR_ValueFormatException
      */
     protected function checkMultiple($isMultiple = true) {
         if ($isMultiple === $this->isMultiple) {
-            throw new PHPCR_ValueFormatException($this);
+            throw new PHPCR_ValueFormatException();
         }
     }
 }

diff --git a/src/jackalope/Session.php b/src/jackalope/Session.php
@@ -213,6 +213,11 @@ public function itemExists($absPath) {
      */
     public function nodeExists($absPath) {
         if ($absPath == '/') return true;
+
+        if (!jackalope_Helper::isAbsolutePath($absPath) || !jackalope_Helper::isValidPath($absPath)) {
+            throw new PHPCR_RepositoryException("Path is invalid: $absPath");
+        }
+
         try {
             //OPTIMIZE: avoid throwing and catching errors would improve performance if many node exists calls are made
             //would need to communicate to the lower layer that we do not want exceptions
@@ -233,6 +238,12 @@ public function nodeExists($absPath) {
      * @api
      */
     public function propertyExists($absPath) {
+        // TODO: what about $absPath == '/' here? if not then ::itemExists is faulty
+
+        if (!jackalope_Helper::isAbsolutePath($absPath) || !jackalope_Helper::isValidPath($absPath)) {
+            throw new PHPCR_RepositoryException("Path is invalid: $absPath");
+        }
+
         try {
             //OPTIMIZE: avoid throwing and catching errors would improve performance if many node exists calls are made
             //would need to communicate to the lower layer that we do not want exceptions

diff --git a/tests/ImplementationHelpers.php b/tests/ImplementationHelpers.php
@@ -2,6 +2,7 @@
 require_once(dirname(__FILE__) . '/inc/baseSuite.php');
 require_once(dirname(__FILE__) . '/transport/DavexClient.php');
 require_once(dirname(__FILE__) . '/ImplementationHelpers/ObjectManager.php');
+require_once(dirname(__FILE__) . '/JackalopeObjects/Helper.php');
 
 /** test suite for implementation specific helper classes that do not implement
  *  PHPCR interfaces
@@ -12,6 +13,7 @@ public static function suite() {
         $suite = new jackalope_tests_ImplementationHelpers('ImplementationHelpers');
         $suite->addTestSuite('jackalope_tests_transport_DavexClient');
         $suite->addTestSuite('jackalope_tests_ObjectManager');
+        $suite->addTestSuite('jackalope_tests_Helper');
         return $suite;
     }
 }

diff --git a/tests/ImplementationHelpers/ObjectManager.php b/tests/ImplementationHelpers/ObjectManager.php
@@ -2,16 +2,13 @@
 require_once(dirname(__FILE__) . '/../inc/JackalopeObjectsCase.php');
 
 class OMT extends jackalope_ObjectManager {
-    public function absolutePath($root, $relativePath) {
-        return parent::absolutePath($root, $relativePath);
-    }
 
     public function isUUID($i) {
         return parent::isUUID($i);
     }
 
-    public function verifyAbsPath($path) {
-        parent::verifyAbsPath($path);
+    public function verifyAbsolutePath($path) {
+        parent::verifyAbsolutePath($path);
     }
 }
 
@@ -51,33 +48,21 @@ public function testIsUUID() {
         $this->assertTrue($om->isUUID('842E61C0-09AB-A42a-87c0-308ccc90e6f4'));
     }
 
-    public function testAbsolutePath() {
-        $om = new OMT($this->getTransportStub('/jcr:root'), $this->getSessionMock());
-        $this->assertEquals('/jcr:root', $om->absolutePath('/', 'jcr:root'));
-        $this->assertEquals('/jcr:root', $om->absolutePath('/', './jcr:root'));
-        $this->assertEquals('/jcr:root', $om->absolutePath('/jcr:root', ''));
-        $this->assertEquals('/jcr:root/foo_/b-a/0^', $om->absolutePath('jcr:root', 'foo_/b-a/0^/'));
-        $this->assertEquals('/jcr:root/foo_/b-a/0^', $om->absolutePath('/jcr:root', '/foo_/b-a/0^/'));
-        $this->assertEquals('/jcr:root/foo_/b-a/0^', $om->absolutePath('jcr:root/', '/foo_/b-a/0^'));
-        $this->assertEquals('/jcr:root/foo/bar', $om->absolutePath('/jcr:root/wrong/', '../foo/bar/'));
-        $this->assertEquals('/jcr:root/foo/bar', $om->absolutePath('/jcr:root/wrong/', '/../foo/bar/'));
-        $this->assertEquals('/jcr:root/foo/bar', $om->absolutePath('/jcr:root/wrong/', '/foo/../../foo/bar/'));
-    }
 
-    public function testVerifyAbsPath() {
+    public function testVerifyAbsolutePath() {
         $om = new OMT($this->getTransportStub('/jcr:root'), $this->getSessionMock());
 
-        $om->verifyAbsPath('/jcr:root');
-        $om->verifyAbsPath('/jcr:foo_/b-a/0^');
+        $om->verifyAbsolutePath('/jcr:root');
+        $om->verifyAbsolutePath('/jcr:foo_/b-a/0^.txt');
 
         $this->setExpectedException('PHPCR_RepositoryException');
-        $om->verifyAbsPath('jcr:root');
+        $om->verifyAbsolutePath('jcr:root');
 
         $this->setExpectedException('PHPCR_RepositoryException');
-        $om->verifyAbsPath('/jcr:root//foo');
+        $om->verifyAbsolutePath('/jcr:root//foo');
 
         $this->setExpectedException('PHPCR_RepositoryException');
-        $om->verifyAbsPath('/jcr:root/foo?');
+        $om->verifyAbsolutePath('/jcr:root/foo?');
     }
 
 }
diff --git a/tests/JackalopeObjects/Helper.php b/tests/JackalopeObjects/Helper.php
@@ -0,0 +1,36 @@
+<?php
+require_once dirname(__FILE__) . '/../../src/jackalope/Helper.php';
+require_once 'PHPUnit/Framework.php';
+
+class jackalope_tests_Helper extends PHPUnit_Framework_TestCase {
+
+    /**
+     * @dataProvider dataproviderAbsolutePath
+     * @covers jackalope_Helper::absolutePath
+     * @covers jackalope_Helper::normalizePath
+     */
+    public function testAbsolutePath($inputRoot, $inputRelPath, $output) {
+        $this->assertEquals($output, jackalope_Helper::absolutePath($inputRoot, $inputRelPath));
+    }
+
+    public static function dataproviderAbsolutePath() {
+        return array(
+            array('/', 'foo', '/foo'),
+            array('/', '/foo', '/foo'),
+            array('', 'foo', '/foo'),
+            array('', '/foo', '/foo'),
+            array('/foo', 'bar', '/foo/bar'),
+            array('/foo', '', '/foo'),
+            array('/foo/', 'bar', '/foo/bar'),
+            array('/foo/', '/bar', '/foo/bar'),
+            array('foo', 'bar', '/foo/bar'),
+
+            // normalization is also part of ::absolutePath
+            array('/', '../foo', '/foo'),
+            array('/', 'foo/../bar', '/bar'),
+            array('/', 'foo/./bar', '/foo/bar'),
+            array('/foo/nope', '../bar', '/foo/bar'),
+            array('/foo/nope', '/../bar', '/foo/bar'),
+        );
+    }
+}