Affects Version/s: 1.4-M1
Fix Version/s: None
I see the following issues with Wicket's tree implementation that should be solved:
AbstractTree and its subclasses were written with the Swing JTree API in mind. This is not a bad thing per se, but the JTree abstractions are not very well suited for a web application. Matej recently removed some of these dependencies, but there's still a lot of code that uses TreeNode, TreeModelEvent and such.
AbstractTree holds a TreeModel in its model, attaching a listener to it. This is an unusual approach for a Wicket component:
- It implies that changes in the TreeModel are automatically propagated to the user interface. This is not always the case, as in an ajax request the AbstractTree has to be explicitely notified to update itself.
- It requires the AbstractTree to monitor the model for a replaced TreeModel.
Most importantly the TreeModel API is complicated and ambiguous (just see the javadoc of TreeModelEvent and TreeModelListener) which makes life harder especially for those who want to use AbstractTree with their own TreeModel implementation (which is difficult enough). Although not directly visible in the API, an implementor has to privide the parent of a node - see AbstractTree#getParentNode(). Tree listeners and events are an annoyance which doesn't match Wicket's elegance.
Currently many components in the AbstractTree hierarchy hold a reference to real nodes of the TreeModel (e.g. junctionLink). TreeState seems like a foreign concept to Wicket, holding references to nodes too. To support detachment AbstractTree tests wether a node implements IDetachable, effectively duplicating functionality that is normally provider by Component and its model out of the box.
The nodes are currently used to identify state (e.g. the parent) in the tree. To add a node, the nodes have to implement equals() and hashCode() to be correctly identified in the tree state. This might be unacceptable, e.g. if entities (business objects) are used as nodes, probably loaded from a database.
Therefore I propose to refactor AbstractTree and subclasses as follows:
A new interface INodeProvider (similar to IDataProvider) defines a concise API to define a tree of nodes. The method #model(Object) gives an implementor the possibility to wrap nodes in a suitable model, e.g. a detachable one. The provided model can handle #equal() and #hashcode() for identification of nodes in the tree.
References to nodes are always indirect through the model, never to the real node object.
Handling of node parents is completely managed inside AbstractTree, no need for an ExtendedTreeModel.
The model of an AbstractTree is used for node selection, similar to AbstractChoices.
Listeners are replaced with notification methods on AbstractTree. No need for tree paths on changes.
Expansion state is held in the AbstracTree#TreeItems (instead of former ITreeState) - similar to component visibility.
The attached patch tries to achieve all this (or at least show how a solution could look like), additionally:
- AbstractTree now utilizes AbstractRepeater and #setOutputMarkupPlaceholderTag() instead of custom solutions.
- The examples are modified and enhanced with 'add' and 'remove' functionality, org.apache.wicket.examples.nested.Home shows usage of an Swing TreeModel adapter.
- Lots of generics are added (Swing's TreeModel will probably never be generic).
- The code is (hopefully) simplified.
I apologies for putting all this stuff into a single patch, but I wasn't able to split these issues apart - it all depends on the proposed INodeProvider interface.
For users of AbstractTree this solution has the following impacts:
- minor API changes for subclasses
- if a node is collapsed, all state of descendants will be lost, i.e. if the node is expanded once again, all previously expanded children will stay collapsed (similar to how Gnome handles trees)
- optimization for adding multiple new children in one AJAX request is not (yet) supported, this was part of the former custom markup handling.
I will continue working on AbstractTree with tests and documentation. But before doing that, I'd like to get a feedback from wicket committers (especially Matej), if this proposed solution matches their conception.