added tests for Jackalope fetch depth option #132
Conversation
| $node = $this->rootNode->getNode('tests_read_jackalope_fetch_depth'); | ||
|
|
||
| $this->session->setSessionOption(\Jackalope\Session::OPTION_FETCH_DEPTH, 5); | ||
| $deepExamples = $node->getNodes('deepExample'); |
There was a problem hiding this comment.
this causes an infinite recursion on Jackalope Doctrine DBAL, note this happens on master as well as with jackalope/jackalope-doctrine-dbal#196
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
ah maybe because of the fetch depth?
There was a problem hiding this comment.
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) {
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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) {
There was a problem hiding this comment.
added tests for Jackalope fetch depth option
No description provided.