[MAGNOLIA-5783] Provide replacements for info.magnolia.nodebuilder.task and deprecate the outdated ones Created: 23/May/14 Updated: 04/Jun/14 Resolved: 30/May/14 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 5.3 |
| Type: | Task | Priority: | Neutral |
| Reporter: | Daniel Lipp | Assignee: | Daniel Lipp |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Template: |
|
||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||
| Task DoR: |
Empty
|
||||||||||||
| Date of First Response: | |||||||||||||
| Description |
|
Content API has been deprecated a long time ago. Hence info.magnolia.nodebuilder.task has to be replaced by code not exposing content API as well. |
| Comments |
| Comment by Magnolia International [ 27/May/14 ] |
|
I made some minor Javadoc changes on the branch. Otherwise looks good, except for two things 1. NodeBuilderTask
final Session session = ctx.getJCRSession(workspaceName);
if ("/".equals(rootPath)) {
return session.getRootNode();
}
String tmpRoot = StringUtils.removeStart(rootPath, "/");
return session.getRootNode().getNode(tmpRoot);
instead of simply this
final Session session = ctx.getJCRSession(workspaceName);
return session.getNode(rootPath);
The only difference I can see is that the former allows non-absolute paths, which would still be relative to the root (e.g. modules/core would be accepted)), which is misleading/wrong IMO. Do we have cases where we depend on this behavior ? It sounds like the kind of things we were doing in our Content API and I'd rather not go back there, and rely on the more predictable behavior of the JCR API: mgnl> s=MgnlContext.getJCRSession('config')
mgnl> s.getNode('/')
===> node /
mgnl> s.getRootNode()
===> node /
mgnl> s.getNode('/') == s.getRootNode()
===> true
mgnl> s.getNode('modules/cache')
RepositoryException: Not an absolute path: modules/cache
mgnl> s.getNode('/modules/cache')
===> node /modules/cache
mgnl> s.getNode('/modules/cache/')
===> node /modules/cache
2. Changes in Ops
I think that's it |
| Comment by Daniel Lipp [ 30/May/14 ] |
|
1.) I dropped the unwanted Content-API-backwards compatibility 2.) the idea wasn't to fix all defects in Ops - I simply realized there's a bug in Ops#doExec so I went ahead and fixed it for this ticket. I now created a isolated ticket for it (MAGNOLIA-5797) and will modify the commit message of the Ops changes accordingly before merging to master Glad if you could create a separate ticket listing the further shortcomings of the Ops class. |
| Comment by Magnolia International [ 30/May/14 ] |
|
Re-reviewed, merged and squashed to master. |