It seems ok to me, although I'm not an expert with that class so I cannot really evaluate the risk of breaking something.
~ Simon On Wed, Sep 17, 2008 at 6:39 PM, Andrew Robinson < [EMAIL PROTECTED]> wrote: > Hi all, I have been looking at TRINIDAD-1086 today, and see that the > problem is due to the model being transient in the implementation of > RowKeySet (RowKeySetTreeImpl). There are two problems with this: > > 1) The model is not restored when the tree, table and navigationTree > components are restored, leaving the model to be null > 2) The model is not cleared if the component is kept in memory (this > happens in MyFaces 1.2.3 for example) at the end of the request and it > prevents the model from being disposed of > > Proposed solution: > > 1) Deprecate RowKeySet.setCollectionModel(CollectionModel model); > 2) Introduce RowKeySet.setCollectionModel(ValueExpression modelExpression); > 3) Change implementation of RowKeySet.getCollectionModel(); > 3a) if the local model is set (deprecated), return it > 3b) if there is a cached copy of the model on the request, use it > 3c) if the ValueExpression is set, evaluate it, cache the value on the > request (for performance as it is faster than ValueExpression > evaluations), and return that > > I would also suggest that this default implementation should be done > in RowKeySet and not have RowKeySet.getCollectionModel() and > RowKeySet.setCollectionModel(ValueExpression) as abstract. > > Please let me know if there are any objections to these changes. > > Thanks, > Andrew >
