Re: [PR] JCRVLT-831: For collection of namespace prefixes, avoid iterating over sibling nodes not contained in the filter(s) [jackrabbit-filevault]
reschke merged PR #408: URL: https://github.com/apache/jackrabbit-filevault/pull/408 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] JCRVLT-831: For collection of namespace prefixes, avoid iterating over sibling nodes not contained in the filter(s) [jackrabbit-filevault]
reschke commented on code in PR #408:
URL:
https://github.com/apache/jackrabbit-filevault/pull/408#discussion_r2694373744
##
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java:
##
@@ -645,12 +645,18 @@ private void loadNamespaces(Set prefixes, String
parentPath, Node node)
addNamespace(prefixes, p);
}
}
-for (NodeIterator iter = node.getNodes(); iter.hasNext(); ) {
+boolean hasOrderableChildNodes =
node.getPrimaryNodeType().hasOrderableChildNodes();
Review Comment:
Good question - I did not change anything here. :-)
That said, something we should consider separately.
-> https://issues.apache.org/jira/browse/JCRVLT-835
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] JCRVLT-831: For collection of namespace prefixes, avoid iterating over sibling nodes not contained in the filter(s) [jackrabbit-filevault]
reschke commented on code in PR #408:
URL:
https://github.com/apache/jackrabbit-filevault/pull/408#discussion_r2694373744
##
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java:
##
@@ -645,12 +645,18 @@ private void loadNamespaces(Set prefixes, String
parentPath, Node node)
addNamespace(prefixes, p);
}
}
-for (NodeIterator iter = node.getNodes(); iter.hasNext(); ) {
+boolean hasOrderableChildNodes =
node.getPrimaryNodeType().hasOrderableChildNodes();
Review Comment:
Good question - I did not change anything here. :-)
That said, something we should consider separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] JCRVLT-831: For collection of namespace prefixes, avoid iterating over sibling nodes not contained in the filter(s) [jackrabbit-filevault]
kwin commented on code in PR #408:
URL:
https://github.com/apache/jackrabbit-filevault/pull/408#discussion_r2694297530
##
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java:
##
@@ -645,12 +645,18 @@ private void loadNamespaces(Set prefixes, String
parentPath, Node node)
addNamespace(prefixes, p);
}
}
-for (NodeIterator iter = node.getNodes(); iter.hasNext(); ) {
+boolean hasOrderableChildNodes =
node.getPrimaryNodeType().hasOrderableChildNodes();
Review Comment:
I see that the same logic is used inside
https://github.com/apache/jackrabbit-filevault/blob/e67d0425c6dea23725c7014f5644515d156aaf38/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java#L461
so we should be fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] JCRVLT-831: For collection of namespace prefixes, avoid iterating over sibling nodes not contained in the filter(s) [jackrabbit-filevault]
kwin commented on code in PR #408:
URL:
https://github.com/apache/jackrabbit-filevault/pull/408#discussion_r2694290365
##
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java:
##
@@ -645,12 +645,18 @@ private void loadNamespaces(Set prefixes, String
parentPath, Node node)
addNamespace(prefixes, p);
}
}
-for (NodeIterator iter = node.getNodes(); iter.hasNext(); ) {
+boolean hasOrderableChildNodes =
node.getPrimaryNodeType().hasOrderableChildNodes();
Review Comment:
Is it enough to only check the primary node type? What if any of the mixins
have orderable child nodes. Wouldn't that lead to empty ordering nodes in the
serialization as well?
##
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/AggregateImpl.java:
##
@@ -645,12 +645,18 @@ private void loadNamespaces(Set prefixes, String
parentPath, Node node)
addNamespace(prefixes, p);
}
}
-for (NodeIterator iter = node.getNodes(); iter.hasNext(); ) {
+boolean hasOrderableChildNodes =
node.getPrimaryNodeType().hasOrderableChildNodes();
+// use the node iterator optimized for the workspace filter if and
only if the node is not orderable,
+// in which case we still need to visit all sibling nodes, as their
prefixes wil be needed in the
Review Comment:
```suggestion
// in which case we still need to visit all sibling nodes, as their
prefixes will be needed in the
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] JCRVLT-831: For collection of namespace prefixes, avoid iterating over sibling nodes not contained in the filter(s) [jackrabbit-filevault]
reschke merged PR #401: URL: https://github.com/apache/jackrabbit-filevault/pull/401 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
