Skip to content

added tests for Jackalope fetch depth option #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 4, 2014
Merged

added tests for Jackalope fetch depth option #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions fixtures/05_Reading/jackalopeFetchDepth.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?xml version="1.0" encoding="UTF-8"?>
<sv:node
xmlns:mix="http://www.jcp.org/jcr/mix/1.0"
xmlns:nt="http://www.jcp.org/jcr/nt/1.0"
xmlns:jcr="http://www.jcp.org/jcr/1.0"
xmlns:sv="http://www.jcp.org/jcr/sv/1.0"

sv:name="tests_read_jackalope_fetch_depth">
<sv:property sv:name="jcr:primaryType" sv:type="Name">
<sv:value>nt:unstructured</sv:value>
</sv:property>

<sv:node sv:name="deepExample">
<sv:property sv:name="jcr:primaryType" sv:type="Name">
<sv:value>nt:unstructured</sv:value>
</sv:property>

<sv:node sv:name="deepExample">
<sv:property sv:name="jcr:primaryType" sv:type="Name">
<sv:value>nt:unstructured</sv:value>
</sv:property>

<sv:node sv:name="deepExample">
<sv:property sv:name="jcr:primaryType" sv:type="Name">
<sv:value>nt:unstructured</sv:value>
</sv:property>

<sv:node sv:name="deepExample">
<sv:property sv:name="jcr:primaryType" sv:type="Name">
<sv:value>nt:unstructured</sv:value>
</sv:property>

<sv:node sv:name="deepExample">
<sv:property sv:name="jcr:primaryType" sv:type="Name">
<sv:value>nt:unstructured</sv:value>
</sv:property>
</sv:node>

</sv:node>
</sv:node>
</sv:node>
</sv:node>
</sv:node>
70 changes: 70 additions & 0 deletions tests/05_Reading/JackalopeFetchDepthTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php
namespace PHPCR\Tests\Reading;

require_once(__DIR__ . '/../../inc/BaseCase.php');

/**
* functional tests for Jackalope fetch depth
*/
class JackalopeFetchDepthTest extends \PHPCR\Test\BaseCase
{

public static function setupBeforeClass($fixtures = '05_Reading/jackalopeFetchDepth')
{
parent::setupBeforeClass($fixtures);
}

public function setUp()
{
parent::setUp();
$this->renewSession();
}

public function testGetNodeWithFetchDepth()
{
if (!$this->session instanceof \Jackalope\Session) {
return;
}

$node = $this->rootNode->getNode('tests_read_jackalope_fetch_depth');

$this->session->setSessionOption(\Jackalope\Session::OPTION_FETCH_DEPTH, 5);
$deepExample = $node->getNode('deepExample');
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());

$deepExample = $deepExample->getNode('deepExample');
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());

$deepExample = $deepExample->getNode('deepExample');
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());

$deepExample = $deepExample->getNode('deepExample');
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());
}

public function testGetNodesWithFetchDepth()
{
if (!$this->session instanceof \Jackalope\Session) {
return;
}

$node = $this->rootNode->getNode('tests_read_jackalope_fetch_depth');

$this->session->setSessionOption(\Jackalope\Session::OPTION_FETCH_DEPTH, 5);
$deepExamples = $node->getNodes('deepExample');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this causes an infinite recursion on Jackalope Doctrine DBAL, note this happens on master as well as with jackalope/jackalope-doctrine-dbal#196

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbu the following change makes the tests pass:

diff --git a/src/Jackalope/ObjectManager.php b/src/Jackalope/ObjectManager.php
index 37db729..7a699aa 100644
--- a/src/Jackalope/ObjectManager.php
+++ b/src/Jackalope/ObjectManager.php
@@ -331,9 +331,9 @@ class ObjectManager
                     $parent = $fetchPath;
                     $relPath = '';
                     while ($parent) {
-                        if (isset($inversePaths[$parent])) {
+//                        if (isset($inversePaths[$parent])) {
                             break;
-                        }
+//                        }
                         if ('/' == $parent) {
                             $parent = false;
                         } else {

not sure yet how they can cause the infinite recursion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah maybe because of the fetch depth?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the following patches fix the issue .. but I feel a bit like I am stabbing in the dark. what is especially strange is that jackrabbit doesn't even pass the while look that I am fixing in jackalope:

diff --git a/src/Jackalope/ObjectManager.php b/src/Jackalope/ObjectManager.php
index 37db729..334474b 100644
--- a/src/Jackalope/ObjectManager.php
+++ b/src/Jackalope/ObjectManager.php
@@ -337,8 +337,8 @@ class ObjectManager
                         if ('/' == $parent) {
                             $parent = false;
                         } else {
-                            $parent = PathHelper::getParentPath($fetchPath);
-                            $relPath = '/' . PathHelper::getNodeName($fetchPath) . $relPath;
+                            $parent = PathHelper::getParentPath($parent);
+                            $relPath = '/' . PathHelper::getNodeName($parent) . $relPath;
                         }
                     }
                     if ($parent) {

diff --git a/src/Jackalope/Transport/DoctrineDBAL/Client.php b/src/Jackalope/Transport/DoctrineDBAL/Client.php
index c8a97ad..78fa1e9 100644
--- a/src/Jackalope/Transport/DoctrineDBAL/Client.php
+++ b/src/Jackalope/Transport/DoctrineDBAL/Client.php
@@ -207,7 +207,9 @@ class Client extends BaseTransport implements QueryTransport, WritingInterface,

             // @TODO: don't know if there are expressions returning more then one row
             if ($list->length > 0) {
-                $type = $list->item(0)->parentNode->attributes->getNamedItem('type')->value;
+                $type = is_object($list->item(0)->parentNode->attributes->getNamedItem('type'))
+                    ? $list->item(0)->parentNode->attributes->getNamedItem('type')->value
+                    : null;
                 $content = $list->item(0)->textContent;

                 switch ($type) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the objectmanager code looks cryptic indeed. the non-saved move operations complicate everything :-(

but i think your change is right, $parent = PathHelper::getParentPath($fetchPath); will work only for one level but then keep doing the same thing, where it is supposed to walk up the tree.

for the transport, not sure what this is about (maybe the TODO is related?) . why would the type ever be null, is that sane?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok .. i will push the jackalope change into a PR
for the type .. let me investigate the case when this happens .. might be an issue in our fixtures.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jackalope/jackalope#215

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the issue is that we are apparently one level too deep?

diff --git a/src/Jackalope/Transport/DoctrineDBAL/Client.php b/src/Jackalope/Transport/DoctrineDBAL/Client.php
index c8a97ad..d92752c 100644
--- a/src/Jackalope/Transport/DoctrineDBAL/Client.php
+++ b/src/Jackalope/Transport/DoctrineDBAL/Client.php
@@ -207,7 +207,9 @@ class Client extends BaseTransport implements QueryTransport, WritingInterface,

             // @TODO: don't know if there are expressions returning more then one row
             if ($list->length > 0) {
-                $type = $list->item(0)->parentNode->attributes->getNamedItem('type')->value;
+                $type = is_object($list->item(0)->parentNode->attributes->getNamedItem('type'))
+                    ? $list->item(0)->parentNode->attributes->getNamedItem('type')->value
+                    : $list->item(0)->parentNode->parentNode->attributes->getNamedItem('type')->value;
                 $content = $list->item(0)->textContent;

                 switch ($type) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see https://github.com/jackalope/jackalope-doctrine-dbal/pull/196/files#diff-a3a0165ed79ca1ba3513ec5ecd59ec56R210

$deepExample = $deepExamples->current();
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());

$deepExamples = $node->getNodes('deepExample');
$deepExample = $deepExamples->current();
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());

$deepExamples = $node->getNodes('deepExample');
$deepExample = $deepExamples->current();
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());

$deepExamples = $node->getNodes('deepExample');
$deepExample = $deepExamples->current();
$this->assertEquals(array('deepExample'), (array) $deepExample->getNodeNames());
}
}