Details
-
Type:
Bug
-
Status: Closed (View Workflow)
-
Priority:
Blocker
-
Resolution: Done
-
Affects Version/s: 1.1.0-alpha-3
-
Fix Version/s: 1.1.0-alpha-5
-
Component/s: spi
-
Labels:None
-
Git Pull Request:
Description
Node.get should only try to match the first level.
@Test
|
public void shouldNotMatchAChildsChildrenOnGet()
|
{
|
// /root/child1/child2
|
Node root = new Node(ROOT_NAME)
|
.createChild(CHILD_1_NAME)
|
.createChild(CHILD_2_NAME).getRoot();
|
|
|
Assert.assertNull(root.get(CHILD_2_NAME));
|
}
|
|
@Test
|
public void shouldNotMatchAChildsChildrenOnGetSingle()
|
{
|
// /root/child1/child2
|
Node root = new Node(ROOT_NAME)
|
.createChild(CHILD_1_NAME)
|
.createChild(CHILD_2_NAME).getRoot();
|
|
|
Assert.assertNull(root.getSingle(CHILD_2_NAME));
|
}
|
|
|
@Test
|
public void shouldNotMatchAChildsChildrenOnGetOrCreate()
|
{
|
// /root/child1/child2
|
Node root = new Node(ROOT_NAME);
|
Node child1 = root.createChild(CHILD_1_NAME);
|
Node child2 = child1.createChild(CHILD_2_NAME);
|
|
|
Node createdChild = root.getOrCreate(CHILD_2_NAME);
|
|
|
Assert.assertNotSame(createdChild, child2);
|
}
|
|
|
@Test
|
public void shouldNotMatchAChildsChildrenOnRemoveChild()
|
{
|
// /root/child1/child2
|
Node root = new Node(ROOT_NAME)
|
.createChild(CHILD_1_NAME)
|
.createChild(CHILD_2_NAME).getRoot();
|
|
|
Assert.assertNull(root.removeChild(CHILD_2_NAME));
|
}
|
|
|
@Test
|
public void shouldNotMatchAChildsChildrenOnRemoveChildren()
|
{
|
// /root/child1/child2
|
Node root = new Node(ROOT_NAME)
|
.createChild(CHILD_1_NAME)
|
.createChild(CHILD_2_NAME).getRoot();
|
|
|
List<Node> removed = root.removeChildren(CHILD_2_NAME);
|
Assert.assertNotNull(removed);
|
Assert.assertEquals(0, removed.size());
|
}
|
Gliffy Diagrams
Issue Links
- blocks
-
ARQ-581
Upgrade to ShrinkWrap Descriptors 1.1.0-beta-1
-
- Closed
-
- is related to
-
SHRINKDESC-87
Refactor Queries to enum-based Singletons
-
- Closed
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Links Hierarchy
If no one looked at it yet I can spend some time over the weekend.
Bartosz Majsak
added a comment - If no one looked at it yet I can spend some time over the weekend. Hi Andy, Aslak,
I worked on this task over the weekend and before making a pull request I just want to get some kind of the confirmation that my understanding was correct.
The patterns passed to in Query
T execute(Node node, Pattern... patterns)
|
Considering following tree example:
root
|
1
|
1.1
|
1.1.1
|
1.2
|
2
|
Query formed from two node names patterns /1/1.1.1 should result with no match?
I introduced two implementations for GetQuery type, one dealing with absolute queries (which is actually desired one for mentioned cases here) and the other for relative paths. However I haven't seen any usage for the latter yet. Also the implementation is quite a naive matching algorithm. If you foresee usage on big trees I could introduce some kind of bottom-up matching algorithm which should perform better.
Looking forward for your feedback.
Bartosz Majsak
added a comment - - edited Hi Andy, Aslak,
I worked on this task over the weekend and before making a pull request I just want to get some kind of the confirmation that my understanding was correct.
The patterns passed to in Query
T execute(Node node, Pattern... patterns)
should form XPath-like query, shouldn't they?
Considering following tree example:
root
1
1.1
1.1.1
1.2
2
Query formed from two node names patterns /1/1.1.1 should result with no match?
I introduced two implementations for GetQuery type, one dealing with absolute queries (which is actually desired one for mentioned cases here) and the other for relative paths. However I haven't seen any usage for the latter yet. Also the implementation is quite a naive matching algorithm. If you foresee usage on big trees I could introduce some kind of bottom-up matching algorithm which should perform better.
Looking forward for your feedback. Specified behaviour of GetQuery:
Given:
web-app
|
servlets
|
servlet
|
servlet-name
|
The following function calls and return values should hold:
Get(servlets,"servlet") > servlet
|
Get(web-app,"servlet") > null
|
Get(servlet,"servlet") > null
|
Get(servlets,"servlet/servlet-name") > servlet-name
|
Get(web-app,"web-app/servlets") > null
|
Get(web-app,"servlets/servlet") > servlet
|
Let's get a simple working algorithm now. The use case is for searching in spec XSDs, so the trees aren't likely to get more than 4 or 5 levels deep; I don't think we need to start engineering anything faster at the moment. Already because we have a slim object model for the XML (instead of carrying around DOM or bothering with XB parsing) we're well ahead of the speed game in design.
Andrew Rubinger
added a comment - Specified behaviour of GetQuery:
Given:
web-app
servlets
servlet
servlet-name
The following function calls and return values should hold:
Get(servlets,"servlet") > servlet
Get(web-app,"servlet") > null
Get(servlet,"servlet") > null
Get(servlets,"servlet/servlet-name") > servlet-name
Get(web-app,"web-app/servlets") > null
Get(web-app,"servlets/servlet") > servlet
Let's get a simple working algorithm now. The use case is for searching in spec XSDs, so the trees aren't likely to get more than 4 or 5 levels deep; I don't think we need to start engineering anything faster at the moment. Already because we have a slim object model for the XML (instead of carrying around DOM or bothering with XB parsing) we're well ahead of the speed game in design. Thanks,
it's more or less what I implemented now as absolute get query (matching given path starting from the root). This is used in Node.get(Patterns). I will write some more tests and try to create pull requests this evening CET 
For relative implementation calling
Get(web-app,"servlet") > servlet
|
Bartosz Majsak
added a comment - Thanks,
it's more or less what I implemented now as absolute get query (matching given path starting from the root). This is used in Node.get(Patterns) . I will write some more tests and try to create pull requests this evening CET
For relative implementation calling
Get(web-app,"servlet") > servlet
since I treat it in the same way as XPath version - " //servlet ". My proposal is available on Github.
However I found at least two shortcomings in the current Pattern/Query approach.
Since attributes are stored as plain String->String map and node name is required in Pattern, there is no possibility to perform queries such as:
- All nodes with given attribute(s) -> //[@id]
- All nodes with given pair (attribute,value) -> //[@id=value]
If you see such case useful please create separated task and I'll move on with the implementation. For this issue I simply didn't want to do more speculative programming
Bartosz Majsak
added a comment - - edited My proposal is available on Github.
However I found at least two shortcomings in the current Pattern/Query approach.
Since attributes are stored as plain String->String map and node name is required in Pattern, there is no possibility to perform queries such as:
All nodes with given attribute(s) -> // [@id]
All nodes with given pair (attribute,value) -> // [@id=value]
If you see such case useful please create separated task and I'll move on with the implementation. For this issue I simply didn't want to do more speculative programming Andrew Rubinger
added a comment - Upstream:
https://github.com/shrinkwrap/descriptors/commit/77be81a477c69d3685656e1d6e00731d60ac2b85
https://github.com/shrinkwrap/descriptors/commit/8ed82c896468eab53c3b0a6543ff24ff0e906efb Based on our discussion in this commit I created two alternative implementation for Query API:
Bartosz Majsak
added a comment - Based on our discussion in this commit I created two alternative implementation for Query API:
based on previously used enum singleton approach
based on immutable objects created by factories
Let's also add tests to ensure that the following methods are not acting in recursive fashion:
getSingle
getOrCreate
removeChildren
removeChild