Alright, after having trying to implement the ValueExpression, I
realized that it will not work. This is because the value of a Tree,
Table, Page or NavigationTree is not necessarily a CollectionModel.
UIXCollection has a createCollectionModel method that creates a model
from the supported types of values.
I also tried to set the model in the restoreView, only to realize
later that that won't work for the first time the page is rendered.
The main problem here is that RowKeySet does not own the
CollectionModel, and therefore never should have been allowed to hold
a reference to it. Unfortunately it is too late to remove every API
function that relies on the methods like add, remove and such that use
the getCollectionModel method.
Solution #1:
Overriding the createFacesBean function to provide a faces bean
wrapper. This wrapper could inject the UIXCollection into the
RowKeySet when one is asked for.
For example, take UIXNavigationTree:
protected FacesBean createFacesBean(String rendererType)
{
FacesBean bean = super.createFacesBean(rendererType);
return new FacesBeanWrapper(bean) {
@Override
public getProperty(PropertyKey key)
{
if (key == UIXNavigationTree.this.DISCLOSED_ROW_KEYS_KEY)
{
RowKeySet disclosedRowKeys = (RowKeySet)super.getProperty(key);
if (disclosedRowKeys)
{
disclosedRowKeys.setCollection(UIXNavigationTree.this.getCollectionModel());
}
}
}
@Override
public Object saveState(FacesContext context)
{
RowKeySet disclosedRowKeys = (RowKeySet)super.getProperty(
UIXNavigationTree.this.DISCLOSED_ROW_KEYS_KEY);
if (disclosedRowKeys != null)
{
disclosedRowKeys.setCollectionModel(null);
}
}
};
}
This makes sure that all attempts to get the disclosed row keys via
the getProperty method return a set with the correct collection and
also that the model is not pinned in memory. Obviously this incurs a
bit of overhead.
Solution #2:
The other approach is to make UIXCollection.getCollectionModel()
public and pass a reference to the component to all the row key set
objects and let the RowKeySet call UIXCollection.getCollectionModel().
The same usage of a FacesBeanWrapper would be needed.
Either way, I think we should deprecate all methods that depend on
getCollectionModel(), and come up with alternatives if needed (move
all methods like add and remove onto UIXCollection). Then in the
future we can remove the reference from the RowKeySet to the model
completely.
Opinions, other options?
-Andrew
On Wed, Sep 17, 2008 at 4: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
>
