You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Mike Kienenberger <mk...@gmail.com> on 2007/04/11 21:54:38 UTC
dataTable row state saving / preserveRowStates / keying row states by row data
I'm looking into the behavior of preserveRowStates in order to try to
fix the issues with deleting a row when preserveRowStates=true.
One of the things I notice is that the key to the row state Map is the
clientid of the dataTable, which of course varies based on the row
index. Is there some reason why the key isn't the row index?
What about the possibility of storing the row data in the map using a
key based on the row data itself? That way, changing the row model
(adding/deleting rows) would automatically pick the right row state?
The only issues are that we might be keeping the row state for rows
that no longer in the model, and we might be holding onto a reference
to an object that should be garbage collected (I've never delved into
this, but my understanding is that there are ways around referencing
things that should perhaps be garbage-collected).
However, if it works, it would automaticallly solve all of the row
issues transparently to the end-user. Sorting, inserting, deleting
rows would work transparently.
We might also want to put in a preserveRowStatesConverter so we save a
light-weight key to the row data rather than the row data itself like
f:selectItems does.
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Josué, thanks for contributing.
Unfortunately, the problem we're hitting here doesn't have much to do
with the table id. We're dealing with the problem of input
components in table rows losing their submitted values if an
immediate=true UICommand executes, especially if that action involves
changing the data model backing the dataTable.
On 4/11/07, Josué Alcalde González <jo...@gmail.com> wrote:
> Would not it work the attribute 'forceIdIndexFormula' ?
> I use it to generate rows where the id in the dataTable is the same as
> the real id in my database table.
>
> El mié, 11-04-2007 a las 15:54 -0400, Mike Kienenberger escribió:
> > I'm looking into the behavior of preserveRowStates in order to try to
> > fix the issues with deleting a row when preserveRowStates=true.
> >
> > One of the things I notice is that the key to the row state Map is the
> > clientid of the dataTable, which of course varies based on the row
> > index. Is there some reason why the key isn't the row index?
> >
> > What about the possibility of storing the row data in the map using a
> > key based on the row data itself? That way, changing the row model
> > (adding/deleting rows) would automatically pick the right row state?
> > The only issues are that we might be keeping the row state for rows
> > that no longer in the model, and we might be holding onto a reference
> > to an object that should be garbage collected (I've never delved into
> > this, but my understanding is that there are ways around referencing
> > things that should perhaps be garbage-collected).
> >
> > However, if it works, it would automaticallly solve all of the row
> > issues transparently to the end-user. Sorting, inserting, deleting
> > rows would work transparently.
> >
> > We might also want to put in a preserveRowStatesConverter so we save a
> > light-weight key to the row data rather than the row data itself like
> > f:selectItems does.
>
>
Re: dataTable row state saving / preserveRowStates / keying row
states by row data
Posted by Josué Alcalde González <jo...@gmail.com>.
Would not it work the attribute 'forceIdIndexFormula' ?
I use it to generate rows where the id in the dataTable is the same as
the real id in my database table.
El mié, 11-04-2007 a las 15:54 -0400, Mike Kienenberger escribió:
> I'm looking into the behavior of preserveRowStates in order to try to
> fix the issues with deleting a row when preserveRowStates=true.
>
> One of the things I notice is that the key to the row state Map is the
> clientid of the dataTable, which of course varies based on the row
> index. Is there some reason why the key isn't the row index?
>
> What about the possibility of storing the row data in the map using a
> key based on the row data itself? That way, changing the row model
> (adding/deleting rows) would automatically pick the right row state?
> The only issues are that we might be keeping the row state for rows
> that no longer in the model, and we might be holding onto a reference
> to an object that should be garbage collected (I've never delved into
> this, but my understanding is that there are ways around referencing
> things that should perhaps be garbage-collected).
>
> However, if it works, it would automaticallly solve all of the row
> issues transparently to the end-user. Sorting, inserting, deleting
> rows would work transparently.
>
> We might also want to put in a preserveRowStatesConverter so we save a
> light-weight key to the row data rather than the row data itself like
> f:selectItems does.
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Ok. I've been unable to make this work with the countryTableForm.jsp
page. No idea why as I don't see any reason why it should fail. It
works fine when I create my own nested datatables and test it, so it's
something specific to either jsp or to that example. Or maybe to my
eclipse setup for the examples.
I posted my proposed patch to TOMAHAWK-961. Maybe someone else could
give it a try and see if it works for them. If it works for others,
I'll finish cleaning it up and commit it, probably providing a mode
option to choose between the new and old behavior.
https://issues.apache.org/jira/browse/TOMAHAWK-961
I've accomplished my own personal goal, which was to get it working in
my own project. I'd be happy to pursue it further if someone else
wants to help out, but I can't afford to spend any more time on it
right now (13 hours so far), and I don't want to commit the changes to
svn until the issues with countryTableForm are addressed.
But as far as I can tell, it does work and should work.
On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> Ok. After making these two changes (use rowData as key, record last
> rowData object and only save input component state if the rowData
> object hasn't changed on setRowIndex()), everything seems to be
> working. I'm going to test nested datatables using the
> countryTableForm.jsp page, and that should be it.
>
> The only question is whether I should make the new way of doing things
> the default behavior for preserveRowStates or if I need to create a
> preserveRowStatesMode to determine whether to use the old or new
> behavior.
>
> On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > I've given this more thought overnight. I don't think _rowStates is
> > saved between requests. It only lives for the duration of one
> > Lifecycle iteration. This appears to be true for client-side state
> > saving. I'll need to double-check that it's also true for server-side
> > state saving as I'm pretty ignorant in that area. Thus, there's no
> > reason why we shouldn't be able to use the row data itself as the key
> > instead of a converter String based on the row data. In both cases,
> > we'd still have the issue with the same object appearing in the data
> > model, but again, I'm not sure what the use case would be.
> >
> > Also, the solution for the issue above is probably handled with the
> > following logic -- record the last data row object at the end of
> > setRowIndex(). If the last row object doesn't match the current row
> > object at the start of setRowIndex(), then don't try to save the row
> > state. We'd assume that the last row object is null at the start of
> > the lifecycle (I think rowIndex starts at -1 at the start of the
> > lifecycle as well, so this should be safe).
> >
> > One thing we might want to do is to have an attribute that specifies
> > preserve row state behavior. One option for the methodology above
> > which is save for sort/add/delete, but not safe for multiple copies of
> > the same rowData. One option for clientid (or maybe row number)
> > which is safe for multiple copies but unsafe for changes to the
> > dataModel.
> >
> > On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > the question is if we shouldn't dig deeper - shouldn't for all
> > > row-handling your special converter be used instead of the row-index?
> > > Trinidad has something similar (row-key).
> > >
> > > regards,
> > >
> > > Martin
> > >
> > > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > Ok. Here's why. The data for row 2 is copied to the input
> > > > components, row data 2 is deleted, then the data from the input
> > > > components (still for the original row 2) is copied back to row 2.
> > > >
> > > > ====================
> > > > before invokeAction
> > > > setRowIndex(2)
> > > > copy row 2 data to input components
> > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > > > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > > > got [[Ljava.lang.Object;@650646]
> > > >
> > > > delete row data 2.
> > > >
> > > > setRowIndex(-1)
> > > > copy input component values to row 2
> > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > > > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > > > value=[[Ljava.lang.Object;@1a32ea4]
> > > > ==================================
> > > >
> > > >
> > > > I'm not sure what a good fix for this would be. Perhaps detecting
> > > > that row data has changed in the invoke action phase? Perhaps caching
> > > > the original rows -> rowData associations before invoke action and
> > > > ignoring changes when the row data no longer matches the same row
> > > > instead of saving the components, or always using the cached values
> > > > during this phase?
> > > >
> > > >
> > > >
> > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > Well, this is kinda odd.
> > > > >
> > > > > I've implemented the converter version, and it *almost* works.
> > > > >
> > > > > Without the converter, using the standard clientId key,
> > > > >
> > > > > public String getAsString(FacesContext context, UIComponent
> > > > > component, Object value)
> > > > > {
> > > > > UIData uiData = (UIData)component;
> > > > > return uiData.getClientId(context);
> > > > > }
> > > > >
> > > > > If you have
> > > > >
> > > > > row 1 - 1111
> > > > > row 2 - 2222
> > > > > row 3 - 3333
> > > > > row 4 - 4444
> > > > > row 5 - 5555
> > > > >
> > > > > and you delete row 2, you get:
> > > > >
> > > > > row 1 - 1111
> > > > > row 3 - 2222
> > > > > row 4 - 3333
> > > > > row 5 - 4444
> > > > >
> > > > > Now if I throw in a converter that maps to an object:
> > > > >
> > > > > public String getAsString(FacesContext context, UIComponent
> > > > > component, Object value)
> > > > > {
> > > > > UIData uiData = (UIData)component;
> > > > > if (uiData.isRowAvailable())
> > > > > {
> > > > > Item item = (Item)uiData.getRowData();
> > > > > return "Item" + String.valueOf(item.getId());
> > > > > }
> > > > > return uiData.getClientId(context);
> > > > > }
> > > > >
> > > > > If you have
> > > > >
> > > > > row 1 - 1111
> > > > > row 2 - 2222
> > > > > row 3 - 3333
> > > > > row 4 - 4444
> > > > > row 5 - 5555
> > > > >
> > > > > and you delete row 2, you get:
> > > > >
> > > > > row 1 - 1111
> > > > > row 3 - 2222
> > > > > row 4 - 4444
> > > > > row 5 - 5555
> > > > >
> > > > > Weird. Everything is correct except for row 3 (which picked up the
> > > > > values for row 2), which is a vast improvement over the original, but
> > > > > still not quite right.
> > > > >
> > > > >
> > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > I don't think this will affect the nested datatables since we're not
> > > > > > changing the value, only the key, for each row-state map entry.
> > > > > > However, I might be misunderstanding something.
> > > > > >
> > > > > > A starting point could be
> > > > > >
> > > > > > private Map _rowStates = new WeakHashMap();
> > > > > >
> > > > > > Then, we'd change this:
> > > > > >
> > > > > > _rowStates.put(getClientId(facesContext),
> > > > > > saveDescendantComponentStates(getChildren()
> > > > > > .iterator(), false));
> > > > > >
> > > > > > to
> > > > > >
> > > > > > _rowStates.put(dataModel.getRowData(),
> > > > > > saveDescendantComponentStates(getChildren()
> > > > > > .iterator(), false));
> > > > > >
> > > > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > > > I'm guessing this would require that rowData be serializable.
> > > > > >
> > > > > > A better implementation might be:
> > > > > >
> > > > > > Converter converter = .... [either assigned explicitly or
> > > > > > fetched by RowData type]
> > > > > >
> > > > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > > > saveDescendantComponentStates(getChildren()
> > > > > > .iterator(), false));
> > > > > >
> > > > > > Now we're no longer storing a weak reference to the model object
> > > > > > (good), but we'll now have to be responsible for keeping the
> > > > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > > > issues (good).
> > > > > >
> > > > > > One potential snag might be if the backing data model contains
> > > > > > identical objects. I can't think of a practical use case for this
> > > > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > > > that there isn't one.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > > > implementing preserveRowStates - there were some problems with nested
> > > > > > > data-tables - you'd need to work around those problems specifically.
> > > > > > >
> > > > > > > What would be your idea of a key based on the row-data?
> > > > > > >
> > > > > > > regards,
> > > > > > >
> > > > > > > Martin
> > > > > > >
> > > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > > > >
> > > > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > > > index. Is there some reason why the key isn't the row index?
> > > > > > > >
> > > > > > > > What about the possibility of storing the row data in the map using a
> > > > > > > > key based on the row data itself? That way, changing the row model
> > > > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > > > this, but my understanding is that there are ways around referencing
> > > > > > > > things that should perhaps be garbage-collected).
> > > > > > > >
> > > > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > > > rows would work transparently.
> > > > > > > >
> > > > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > > > f:selectItems does.
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > >
> > > > > > > http://www.irian.at
> > > > > > >
> > > > > > > Your JSF powerhouse -
> > > > > > > JSF Consulting, Development and
> > > > > > > Courses in English and German
> > > > > > >
> > > > > > > Professional Support for Apache MyFaces
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > >
> > > http://www.irian.at
> > >
> > > Your JSF powerhouse -
> > > JSF Consulting, Development and
> > > Courses in English and German
> > >
> > > Professional Support for Apache MyFaces
> > >
> >
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Yes, that should be part of my cleanup process if the code gets to the
point where I feel it's safe to commit it. Thanks for reminding me.
On 4/12/07, Mathias Brökelmann <mb...@googlemail.com> wrote:
> Mike please create a unit test for this (including one for a nested
> table). It will help us tracking this issue if other things are
> changing in this area.
>
> 2007/4/12, Mike Kienenberger <mk...@gmail.com>:
> > Ok. After making these two changes (use rowData as key, record last
> > rowData object and only save input component state if the rowData
> > object hasn't changed on setRowIndex()), everything seems to be
> > working. I'm going to test nested datatables using the
> > countryTableForm.jsp page, and that should be it.
> >
> > The only question is whether I should make the new way of doing things
> > the default behavior for preserveRowStates or if I need to create a
> > preserveRowStatesMode to determine whether to use the old or new
> > behavior.
> >
> > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > I've given this more thought overnight. I don't think _rowStates is
> > > saved between requests. It only lives for the duration of one
> > > Lifecycle iteration. This appears to be true for client-side state
> > > saving. I'll need to double-check that it's also true for server-side
> > > state saving as I'm pretty ignorant in that area. Thus, there's no
> > > reason why we shouldn't be able to use the row data itself as the key
> > > instead of a converter String based on the row data. In both cases,
> > > we'd still have the issue with the same object appearing in the data
> > > model, but again, I'm not sure what the use case would be.
> > >
> > > Also, the solution for the issue above is probably handled with the
> > > following logic -- record the last data row object at the end of
> > > setRowIndex(). If the last row object doesn't match the current row
> > > object at the start of setRowIndex(), then don't try to save the row
> > > state. We'd assume that the last row object is null at the start of
> > > the lifecycle (I think rowIndex starts at -1 at the start of the
> > > lifecycle as well, so this should be safe).
> > >
> > > One thing we might want to do is to have an attribute that specifies
> > > preserve row state behavior. One option for the methodology above
> > > which is save for sort/add/delete, but not safe for multiple copies of
> > > the same rowData. One option for clientid (or maybe row number)
> > > which is safe for multiple copies but unsafe for changes to the
> > > dataModel.
> > >
> > > On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > the question is if we shouldn't dig deeper - shouldn't for all
> > > > row-handling your special converter be used instead of the row-index?
> > > > Trinidad has something similar (row-key).
> > > >
> > > > regards,
> > > >
> > > > Martin
> > > >
> > > > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > Ok. Here's why. The data for row 2 is copied to the input
> > > > > components, row data 2 is deleted, then the data from the input
> > > > > components (still for the original row 2) is copied back to row 2.
> > > > >
> > > > > ====================
> > > > > before invokeAction
> > > > > setRowIndex(2)
> > > > > copy row 2 data to input components
> > > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > > > > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > > > > got [[Ljava.lang.Object;@650646]
> > > > >
> > > > > delete row data 2.
> > > > >
> > > > > setRowIndex(-1)
> > > > > copy input component values to row 2
> > > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > > > > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > > > > value=[[Ljava.lang.Object;@1a32ea4]
> > > > > ==================================
> > > > >
> > > > >
> > > > > I'm not sure what a good fix for this would be. Perhaps detecting
> > > > > that row data has changed in the invoke action phase? Perhaps caching
> > > > > the original rows -> rowData associations before invoke action and
> > > > > ignoring changes when the row data no longer matches the same row
> > > > > instead of saving the components, or always using the cached values
> > > > > during this phase?
> > > > >
> > > > >
> > > > >
> > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > Well, this is kinda odd.
> > > > > >
> > > > > > I've implemented the converter version, and it *almost* works.
> > > > > >
> > > > > > Without the converter, using the standard clientId key,
> > > > > >
> > > > > > public String getAsString(FacesContext context, UIComponent
> > > > > > component, Object value)
> > > > > > {
> > > > > > UIData uiData = (UIData)component;
> > > > > > return uiData.getClientId(context);
> > > > > > }
> > > > > >
> > > > > > If you have
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 2 - 2222
> > > > > > row 3 - 3333
> > > > > > row 4 - 4444
> > > > > > row 5 - 5555
> > > > > >
> > > > > > and you delete row 2, you get:
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 3 - 2222
> > > > > > row 4 - 3333
> > > > > > row 5 - 4444
> > > > > >
> > > > > > Now if I throw in a converter that maps to an object:
> > > > > >
> > > > > > public String getAsString(FacesContext context, UIComponent
> > > > > > component, Object value)
> > > > > > {
> > > > > > UIData uiData = (UIData)component;
> > > > > > if (uiData.isRowAvailable())
> > > > > > {
> > > > > > Item item = (Item)uiData.getRowData();
> > > > > > return "Item" + String.valueOf(item.getId());
> > > > > > }
> > > > > > return uiData.getClientId(context);
> > > > > > }
> > > > > >
> > > > > > If you have
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 2 - 2222
> > > > > > row 3 - 3333
> > > > > > row 4 - 4444
> > > > > > row 5 - 5555
> > > > > >
> > > > > > and you delete row 2, you get:
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 3 - 2222
> > > > > > row 4 - 4444
> > > > > > row 5 - 5555
> > > > > >
> > > > > > Weird. Everything is correct except for row 3 (which picked up the
> > > > > > values for row 2), which is a vast improvement over the original, but
> > > > > > still not quite right.
> > > > > >
> > > > > >
> > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > I don't think this will affect the nested datatables since we're not
> > > > > > > changing the value, only the key, for each row-state map entry.
> > > > > > > However, I might be misunderstanding something.
> > > > > > >
> > > > > > > A starting point could be
> > > > > > >
> > > > > > > private Map _rowStates = new WeakHashMap();
> > > > > > >
> > > > > > > Then, we'd change this:
> > > > > > >
> > > > > > > _rowStates.put(getClientId(facesContext),
> > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > .iterator(), false));
> > > > > > >
> > > > > > > to
> > > > > > >
> > > > > > > _rowStates.put(dataModel.getRowData(),
> > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > .iterator(), false));
> > > > > > >
> > > > > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > > > > I'm guessing this would require that rowData be serializable.
> > > > > > >
> > > > > > > A better implementation might be:
> > > > > > >
> > > > > > > Converter converter = .... [either assigned explicitly or
> > > > > > > fetched by RowData type]
> > > > > > >
> > > > > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > .iterator(), false));
> > > > > > >
> > > > > > > Now we're no longer storing a weak reference to the model object
> > > > > > > (good), but we'll now have to be responsible for keeping the
> > > > > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > > > > issues (good).
> > > > > > >
> > > > > > > One potential snag might be if the backing data model contains
> > > > > > > identical objects. I can't think of a practical use case for this
> > > > > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > > > > that there isn't one.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > > > > implementing preserveRowStates - there were some problems with nested
> > > > > > > > data-tables - you'd need to work around those problems specifically.
> > > > > > > >
> > > > > > > > What would be your idea of a key based on the row-data?
> > > > > > > >
> > > > > > > > regards,
> > > > > > > >
> > > > > > > > Martin
> > > > > > > >
> > > > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > > > > >
> > > > > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > > > > index. Is there some reason why the key isn't the row index?
> > > > > > > > >
> > > > > > > > > What about the possibility of storing the row data in the map using a
> > > > > > > > > key based on the row data itself? That way, changing the row model
> > > > > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > > > > this, but my understanding is that there are ways around referencing
> > > > > > > > > things that should perhaps be garbage-collected).
> > > > > > > > >
> > > > > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > > > > rows would work transparently.
> > > > > > > > >
> > > > > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > > > > f:selectItems does.
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > >
> > > > > > > > http://www.irian.at
> > > > > > > >
> > > > > > > > Your JSF powerhouse -
> > > > > > > > JSF Consulting, Development and
> > > > > > > > Courses in English and German
> > > > > > > >
> > > > > > > > Professional Support for Apache MyFaces
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > http://www.irian.at
> > > >
> > > > Your JSF powerhouse -
> > > > JSF Consulting, Development and
> > > > Courses in English and German
> > > >
> > > > Professional Support for Apache MyFaces
> > > >
> > >
> >
>
>
> --
> Mathias
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mathias Brökelmann <mb...@googlemail.com>.
looks good.
Big thanks for your work Mike!
It would be great if we could have a unit test for each issue along
with the fix for it in myfaces in the future.
While working on myfaces 1.2 I found a lot of code which is really
hard to understand and sometimes impossible to find the reason why it
is coded in a specific way. Although the TCK covers a lot of issues
but running the tck is not quite useful since it takes a lot of
resources and time and it is not integrated in the maven test build.
2007/4/13, Mike Kienenberger <mk...@gmail.com>:
> Mathias, what all should tests cover in this case?
>
> Here's one possibility -- it tests that changing a value works and
> saves the state, then tests that deleting a row properly adjusts the
> saved row state.
>
> /*
> * Test method for
> * 'org.apache.myfaces.component.html.ext.HtmlDataTable
> preserveRowStates=true'
> */
> public void testPreserveRowStatesTrue()
> {
>
> String OUTER_UI_DATA_ID = "outerUIData";
> String INNER_UI_DATA_ID = "innerUIData";
>
> HtmlDataTable outerUIData = new HtmlDataTable();
> outerUIData.setId(OUTER_UI_DATA_ID);
> outerUIData.setPreserveRowStates(true);
>
> List li = new ArrayList();
> li.add(new TestData2("outer1", "outer1"));
> li.add(new TestData2("outer2", "outer2"));
> li.add(new TestData2("outer3", "outer3"));
> li.add(new TestData2("outer4", "outer4"));
> li.add(new TestData2("outer5", "outer5"));
>
> outerUIData.setValue(new ListDataModel(li));
> outerUIData.setVar("testData2");
>
>
> UIColumn column = new UIColumn();
>
> outerUIData.getChildren().add(column);
>
> UIInput input = new UIInput();
> input.setId("input");
> input.setValueBinding("value",
> createValueBinding("#{testData2.description}"));
>
> column.getChildren().add(input);
>
>
> UIColumn column2 = new UIColumn();
>
> outerUIData.getChildren().add(column2);
>
> HtmlDataTable innerUIData = new HtmlDataTable();
> innerUIData.setId(INNER_UI_DATA_ID);
> innerUIData.setPreserveRowStates(true);
>
> innerUIData.setValueBinding("value",
> createValueBinding("#{testData2.testDataList}"));
> innerUIData.setVar("testData");
>
> UIColumn innerInputColumn = new UIColumn();
>
> innerUIData.getChildren().add(innerInputColumn);
>
> UIInput innerInput = new UIInput();
> innerInput.setId("innerInput");
> innerInput.setValueBinding("value",
> createValueBinding("#{testData.description}"));
>
> innerInputColumn.getChildren().add(innerInput);
>
> column.getChildren().add(innerUIData);
>
>
> facesContext.getViewRoot().getChildren().add(outerUIData);
>
> UIComponent comp = outerUIData.findComponent(":" +
> OUTER_UI_DATA_ID + ":1:" + INNER_UI_DATA_ID + ":1:innerInput");
> assertTrue(comp instanceof UIInput);
> UIInput uiInput = (UIInput) comp;
> String originalValue = (String)uiInput.getValue();
> String newValue = "new value";
> assertEquals(originalValue, "outer2-2");
>
> outerUIData.setRowIndex(1);
> innerUIData.setRowIndex(1);
> UIColumn outerColumn1 = ((UIColumn)innerUIData.getChildren().get(0));
> UIInput rawInput = ((UIInput)outerColumn1.getChildren().get(0));
> assertEquals(rawInput.getValue(), originalValue);
> rawInput.setValue(newValue);
> innerUIData.setRowIndex(-1);
> outerUIData.setRowIndex(-1);
>
> comp = outerUIData.findComponent(":" + OUTER_UI_DATA_ID +
> ":1:" + INNER_UI_DATA_ID + ":1:innerInput");
> assertTrue(comp instanceof UIInput);
> assertEquals(((UIInput)comp).getValue(), newValue);
>
> outerUIData.setRowIndex(1);
> innerUIData.setRowIndex(1);
> TestData2 testData2 = (TestData2)outerUIData.getRowData();
> testData2.getTestDataList().remove(innerUIData.getRowData());
> innerUIData.setRowIndex(-1);
> outerUIData.setRowIndex(-1);
>
> comp = outerUIData.findComponent(":" + OUTER_UI_DATA_ID +
> ":1:" + INNER_UI_DATA_ID + ":1:innerInput");
> assertTrue(comp instanceof UIInput);
> assertEquals("outer2-3", ((UIInput)comp).getValue());
> }
>
>
>
>
> On 4/12/07, Mathias Brökelmann <mb...@googlemail.com> wrote:
> > Mike please create a unit test for this (including one for a nested
> > table). It will help us tracking this issue if other things are
> > changing in this area.
> >
> > 2007/4/12, Mike Kienenberger <mk...@gmail.com>:
> > > Ok. After making these two changes (use rowData as key, record last
> > > rowData object and only save input component state if the rowData
> > > object hasn't changed on setRowIndex()), everything seems to be
> > > working. I'm going to test nested datatables using the
> > > countryTableForm.jsp page, and that should be it.
> > >
> > > The only question is whether I should make the new way of doing things
> > > the default behavior for preserveRowStates or if I need to create a
> > > preserveRowStatesMode to determine whether to use the old or new
> > > behavior.
> > >
> > > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > I've given this more thought overnight. I don't think _rowStates is
> > > > saved between requests. It only lives for the duration of one
> > > > Lifecycle iteration. This appears to be true for client-side state
> > > > saving. I'll need to double-check that it's also true for server-side
> > > > state saving as I'm pretty ignorant in that area. Thus, there's no
> > > > reason why we shouldn't be able to use the row data itself as the key
> > > > instead of a converter String based on the row data. In both cases,
> > > > we'd still have the issue with the same object appearing in the data
> > > > model, but again, I'm not sure what the use case would be.
> > > >
> > > > Also, the solution for the issue above is probably handled with the
> > > > following logic -- record the last data row object at the end of
> > > > setRowIndex(). If the last row object doesn't match the current row
> > > > object at the start of setRowIndex(), then don't try to save the row
> > > > state. We'd assume that the last row object is null at the start of
> > > > the lifecycle (I think rowIndex starts at -1 at the start of the
> > > > lifecycle as well, so this should be safe).
> > > >
> > > > One thing we might want to do is to have an attribute that specifies
> > > > preserve row state behavior. One option for the methodology above
> > > > which is save for sort/add/delete, but not safe for multiple copies of
> > > > the same rowData. One option for clientid (or maybe row number)
> > > > which is safe for multiple copies but unsafe for changes to the
> > > > dataModel.
> > > >
> > > > On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > the question is if we shouldn't dig deeper - shouldn't for all
> > > > > row-handling your special converter be used instead of the row-index?
> > > > > Trinidad has something similar (row-key).
> > > > >
> > > > > regards,
> > > > >
> > > > > Martin
> > > > >
> > > > > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > Ok. Here's why. The data for row 2 is copied to the input
> > > > > > components, row data 2 is deleted, then the data from the input
> > > > > > components (still for the original row 2) is copied back to row 2.
> > > > > >
> > > > > > ====================
> > > > > > before invokeAction
> > > > > > setRowIndex(2)
> > > > > > copy row 2 data to input components
> > > > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > > > > > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > > > > > got [[Ljava.lang.Object;@650646]
> > > > > >
> > > > > > delete row data 2.
> > > > > >
> > > > > > setRowIndex(-1)
> > > > > > copy input component values to row 2
> > > > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > > > > > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > > > > > value=[[Ljava.lang.Object;@1a32ea4]
> > > > > > ==================================
> > > > > >
> > > > > >
> > > > > > I'm not sure what a good fix for this would be. Perhaps detecting
> > > > > > that row data has changed in the invoke action phase? Perhaps caching
> > > > > > the original rows -> rowData associations before invoke action and
> > > > > > ignoring changes when the row data no longer matches the same row
> > > > > > instead of saving the components, or always using the cached values
> > > > > > during this phase?
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > Well, this is kinda odd.
> > > > > > >
> > > > > > > I've implemented the converter version, and it *almost* works.
> > > > > > >
> > > > > > > Without the converter, using the standard clientId key,
> > > > > > >
> > > > > > > public String getAsString(FacesContext context, UIComponent
> > > > > > > component, Object value)
> > > > > > > {
> > > > > > > UIData uiData = (UIData)component;
> > > > > > > return uiData.getClientId(context);
> > > > > > > }
> > > > > > >
> > > > > > > If you have
> > > > > > >
> > > > > > > row 1 - 1111
> > > > > > > row 2 - 2222
> > > > > > > row 3 - 3333
> > > > > > > row 4 - 4444
> > > > > > > row 5 - 5555
> > > > > > >
> > > > > > > and you delete row 2, you get:
> > > > > > >
> > > > > > > row 1 - 1111
> > > > > > > row 3 - 2222
> > > > > > > row 4 - 3333
> > > > > > > row 5 - 4444
> > > > > > >
> > > > > > > Now if I throw in a converter that maps to an object:
> > > > > > >
> > > > > > > public String getAsString(FacesContext context, UIComponent
> > > > > > > component, Object value)
> > > > > > > {
> > > > > > > UIData uiData = (UIData)component;
> > > > > > > if (uiData.isRowAvailable())
> > > > > > > {
> > > > > > > Item item = (Item)uiData.getRowData();
> > > > > > > return "Item" + String.valueOf(item.getId());
> > > > > > > }
> > > > > > > return uiData.getClientId(context);
> > > > > > > }
> > > > > > >
> > > > > > > If you have
> > > > > > >
> > > > > > > row 1 - 1111
> > > > > > > row 2 - 2222
> > > > > > > row 3 - 3333
> > > > > > > row 4 - 4444
> > > > > > > row 5 - 5555
> > > > > > >
> > > > > > > and you delete row 2, you get:
> > > > > > >
> > > > > > > row 1 - 1111
> > > > > > > row 3 - 2222
> > > > > > > row 4 - 4444
> > > > > > > row 5 - 5555
> > > > > > >
> > > > > > > Weird. Everything is correct except for row 3 (which picked up the
> > > > > > > values for row 2), which is a vast improvement over the original, but
> > > > > > > still not quite right.
> > > > > > >
> > > > > > >
> > > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > > I don't think this will affect the nested datatables since we're not
> > > > > > > > changing the value, only the key, for each row-state map entry.
> > > > > > > > However, I might be misunderstanding something.
> > > > > > > >
> > > > > > > > A starting point could be
> > > > > > > >
> > > > > > > > private Map _rowStates = new WeakHashMap();
> > > > > > > >
> > > > > > > > Then, we'd change this:
> > > > > > > >
> > > > > > > > _rowStates.put(getClientId(facesContext),
> > > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > > .iterator(), false));
> > > > > > > >
> > > > > > > > to
> > > > > > > >
> > > > > > > > _rowStates.put(dataModel.getRowData(),
> > > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > > .iterator(), false));
> > > > > > > >
> > > > > > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > > > > > I'm guessing this would require that rowData be serializable.
> > > > > > > >
> > > > > > > > A better implementation might be:
> > > > > > > >
> > > > > > > > Converter converter = .... [either assigned explicitly or
> > > > > > > > fetched by RowData type]
> > > > > > > >
> > > > > > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > > .iterator(), false));
> > > > > > > >
> > > > > > > > Now we're no longer storing a weak reference to the model object
> > > > > > > > (good), but we'll now have to be responsible for keeping the
> > > > > > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > > > > > issues (good).
> > > > > > > >
> > > > > > > > One potential snag might be if the backing data model contains
> > > > > > > > identical objects. I can't think of a practical use case for this
> > > > > > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > > > > > that there isn't one.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > > > > > implementing preserveRowStates - there were some problems with nested
> > > > > > > > > data-tables - you'd need to work around those problems specifically.
> > > > > > > > >
> > > > > > > > > What would be your idea of a key based on the row-data?
> > > > > > > > >
> > > > > > > > > regards,
> > > > > > > > >
> > > > > > > > > Martin
> > > > > > > > >
> > > > > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > > > > > >
> > > > > > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > > > > > index. Is there some reason why the key isn't the row index?
> > > > > > > > > >
> > > > > > > > > > What about the possibility of storing the row data in the map using a
> > > > > > > > > > key based on the row data itself? That way, changing the row model
> > > > > > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > > > > > this, but my understanding is that there are ways around referencing
> > > > > > > > > > things that should perhaps be garbage-collected).
> > > > > > > > > >
> > > > > > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > > > > > rows would work transparently.
> > > > > > > > > >
> > > > > > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > > > > > f:selectItems does.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > >
> > > > > > > > > http://www.irian.at
> > > > > > > > >
> > > > > > > > > Your JSF powerhouse -
> > > > > > > > > JSF Consulting, Development and
> > > > > > > > > Courses in English and German
> > > > > > > > >
> > > > > > > > > Professional Support for Apache MyFaces
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > >
> > > > > http://www.irian.at
> > > > >
> > > > > Your JSF powerhouse -
> > > > > JSF Consulting, Development and
> > > > > Courses in English and German
> > > > >
> > > > > Professional Support for Apache MyFaces
> > > > >
> > > >
> > >
> >
> >
> > --
> > Mathias
> >
>
--
Mathias
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Mathias, what all should tests cover in this case?
Here's one possibility -- it tests that changing a value works and
saves the state, then tests that deleting a row properly adjusts the
saved row state.
/*
* Test method for
* 'org.apache.myfaces.component.html.ext.HtmlDataTable
preserveRowStates=true'
*/
public void testPreserveRowStatesTrue()
{
String OUTER_UI_DATA_ID = "outerUIData";
String INNER_UI_DATA_ID = "innerUIData";
HtmlDataTable outerUIData = new HtmlDataTable();
outerUIData.setId(OUTER_UI_DATA_ID);
outerUIData.setPreserveRowStates(true);
List li = new ArrayList();
li.add(new TestData2("outer1", "outer1"));
li.add(new TestData2("outer2", "outer2"));
li.add(new TestData2("outer3", "outer3"));
li.add(new TestData2("outer4", "outer4"));
li.add(new TestData2("outer5", "outer5"));
outerUIData.setValue(new ListDataModel(li));
outerUIData.setVar("testData2");
UIColumn column = new UIColumn();
outerUIData.getChildren().add(column);
UIInput input = new UIInput();
input.setId("input");
input.setValueBinding("value",
createValueBinding("#{testData2.description}"));
column.getChildren().add(input);
UIColumn column2 = new UIColumn();
outerUIData.getChildren().add(column2);
HtmlDataTable innerUIData = new HtmlDataTable();
innerUIData.setId(INNER_UI_DATA_ID);
innerUIData.setPreserveRowStates(true);
innerUIData.setValueBinding("value",
createValueBinding("#{testData2.testDataList}"));
innerUIData.setVar("testData");
UIColumn innerInputColumn = new UIColumn();
innerUIData.getChildren().add(innerInputColumn);
UIInput innerInput = new UIInput();
innerInput.setId("innerInput");
innerInput.setValueBinding("value",
createValueBinding("#{testData.description}"));
innerInputColumn.getChildren().add(innerInput);
column.getChildren().add(innerUIData);
facesContext.getViewRoot().getChildren().add(outerUIData);
UIComponent comp = outerUIData.findComponent(":" +
OUTER_UI_DATA_ID + ":1:" + INNER_UI_DATA_ID + ":1:innerInput");
assertTrue(comp instanceof UIInput);
UIInput uiInput = (UIInput) comp;
String originalValue = (String)uiInput.getValue();
String newValue = "new value";
assertEquals(originalValue, "outer2-2");
outerUIData.setRowIndex(1);
innerUIData.setRowIndex(1);
UIColumn outerColumn1 = ((UIColumn)innerUIData.getChildren().get(0));
UIInput rawInput = ((UIInput)outerColumn1.getChildren().get(0));
assertEquals(rawInput.getValue(), originalValue);
rawInput.setValue(newValue);
innerUIData.setRowIndex(-1);
outerUIData.setRowIndex(-1);
comp = outerUIData.findComponent(":" + OUTER_UI_DATA_ID +
":1:" + INNER_UI_DATA_ID + ":1:innerInput");
assertTrue(comp instanceof UIInput);
assertEquals(((UIInput)comp).getValue(), newValue);
outerUIData.setRowIndex(1);
innerUIData.setRowIndex(1);
TestData2 testData2 = (TestData2)outerUIData.getRowData();
testData2.getTestDataList().remove(innerUIData.getRowData());
innerUIData.setRowIndex(-1);
outerUIData.setRowIndex(-1);
comp = outerUIData.findComponent(":" + OUTER_UI_DATA_ID +
":1:" + INNER_UI_DATA_ID + ":1:innerInput");
assertTrue(comp instanceof UIInput);
assertEquals("outer2-3", ((UIInput)comp).getValue());
}
On 4/12/07, Mathias Brökelmann <mb...@googlemail.com> wrote:
> Mike please create a unit test for this (including one for a nested
> table). It will help us tracking this issue if other things are
> changing in this area.
>
> 2007/4/12, Mike Kienenberger <mk...@gmail.com>:
> > Ok. After making these two changes (use rowData as key, record last
> > rowData object and only save input component state if the rowData
> > object hasn't changed on setRowIndex()), everything seems to be
> > working. I'm going to test nested datatables using the
> > countryTableForm.jsp page, and that should be it.
> >
> > The only question is whether I should make the new way of doing things
> > the default behavior for preserveRowStates or if I need to create a
> > preserveRowStatesMode to determine whether to use the old or new
> > behavior.
> >
> > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > I've given this more thought overnight. I don't think _rowStates is
> > > saved between requests. It only lives for the duration of one
> > > Lifecycle iteration. This appears to be true for client-side state
> > > saving. I'll need to double-check that it's also true for server-side
> > > state saving as I'm pretty ignorant in that area. Thus, there's no
> > > reason why we shouldn't be able to use the row data itself as the key
> > > instead of a converter String based on the row data. In both cases,
> > > we'd still have the issue with the same object appearing in the data
> > > model, but again, I'm not sure what the use case would be.
> > >
> > > Also, the solution for the issue above is probably handled with the
> > > following logic -- record the last data row object at the end of
> > > setRowIndex(). If the last row object doesn't match the current row
> > > object at the start of setRowIndex(), then don't try to save the row
> > > state. We'd assume that the last row object is null at the start of
> > > the lifecycle (I think rowIndex starts at -1 at the start of the
> > > lifecycle as well, so this should be safe).
> > >
> > > One thing we might want to do is to have an attribute that specifies
> > > preserve row state behavior. One option for the methodology above
> > > which is save for sort/add/delete, but not safe for multiple copies of
> > > the same rowData. One option for clientid (or maybe row number)
> > > which is safe for multiple copies but unsafe for changes to the
> > > dataModel.
> > >
> > > On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > the question is if we shouldn't dig deeper - shouldn't for all
> > > > row-handling your special converter be used instead of the row-index?
> > > > Trinidad has something similar (row-key).
> > > >
> > > > regards,
> > > >
> > > > Martin
> > > >
> > > > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > Ok. Here's why. The data for row 2 is copied to the input
> > > > > components, row data 2 is deleted, then the data from the input
> > > > > components (still for the original row 2) is copied back to row 2.
> > > > >
> > > > > ====================
> > > > > before invokeAction
> > > > > setRowIndex(2)
> > > > > copy row 2 data to input components
> > > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > > > > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > > > > got [[Ljava.lang.Object;@650646]
> > > > >
> > > > > delete row data 2.
> > > > >
> > > > > setRowIndex(-1)
> > > > > copy input component values to row 2
> > > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > > > > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > > > > value=[[Ljava.lang.Object;@1a32ea4]
> > > > > ==================================
> > > > >
> > > > >
> > > > > I'm not sure what a good fix for this would be. Perhaps detecting
> > > > > that row data has changed in the invoke action phase? Perhaps caching
> > > > > the original rows -> rowData associations before invoke action and
> > > > > ignoring changes when the row data no longer matches the same row
> > > > > instead of saving the components, or always using the cached values
> > > > > during this phase?
> > > > >
> > > > >
> > > > >
> > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > Well, this is kinda odd.
> > > > > >
> > > > > > I've implemented the converter version, and it *almost* works.
> > > > > >
> > > > > > Without the converter, using the standard clientId key,
> > > > > >
> > > > > > public String getAsString(FacesContext context, UIComponent
> > > > > > component, Object value)
> > > > > > {
> > > > > > UIData uiData = (UIData)component;
> > > > > > return uiData.getClientId(context);
> > > > > > }
> > > > > >
> > > > > > If you have
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 2 - 2222
> > > > > > row 3 - 3333
> > > > > > row 4 - 4444
> > > > > > row 5 - 5555
> > > > > >
> > > > > > and you delete row 2, you get:
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 3 - 2222
> > > > > > row 4 - 3333
> > > > > > row 5 - 4444
> > > > > >
> > > > > > Now if I throw in a converter that maps to an object:
> > > > > >
> > > > > > public String getAsString(FacesContext context, UIComponent
> > > > > > component, Object value)
> > > > > > {
> > > > > > UIData uiData = (UIData)component;
> > > > > > if (uiData.isRowAvailable())
> > > > > > {
> > > > > > Item item = (Item)uiData.getRowData();
> > > > > > return "Item" + String.valueOf(item.getId());
> > > > > > }
> > > > > > return uiData.getClientId(context);
> > > > > > }
> > > > > >
> > > > > > If you have
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 2 - 2222
> > > > > > row 3 - 3333
> > > > > > row 4 - 4444
> > > > > > row 5 - 5555
> > > > > >
> > > > > > and you delete row 2, you get:
> > > > > >
> > > > > > row 1 - 1111
> > > > > > row 3 - 2222
> > > > > > row 4 - 4444
> > > > > > row 5 - 5555
> > > > > >
> > > > > > Weird. Everything is correct except for row 3 (which picked up the
> > > > > > values for row 2), which is a vast improvement over the original, but
> > > > > > still not quite right.
> > > > > >
> > > > > >
> > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > I don't think this will affect the nested datatables since we're not
> > > > > > > changing the value, only the key, for each row-state map entry.
> > > > > > > However, I might be misunderstanding something.
> > > > > > >
> > > > > > > A starting point could be
> > > > > > >
> > > > > > > private Map _rowStates = new WeakHashMap();
> > > > > > >
> > > > > > > Then, we'd change this:
> > > > > > >
> > > > > > > _rowStates.put(getClientId(facesContext),
> > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > .iterator(), false));
> > > > > > >
> > > > > > > to
> > > > > > >
> > > > > > > _rowStates.put(dataModel.getRowData(),
> > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > .iterator(), false));
> > > > > > >
> > > > > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > > > > I'm guessing this would require that rowData be serializable.
> > > > > > >
> > > > > > > A better implementation might be:
> > > > > > >
> > > > > > > Converter converter = .... [either assigned explicitly or
> > > > > > > fetched by RowData type]
> > > > > > >
> > > > > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > > > > saveDescendantComponentStates(getChildren()
> > > > > > > .iterator(), false));
> > > > > > >
> > > > > > > Now we're no longer storing a weak reference to the model object
> > > > > > > (good), but we'll now have to be responsible for keeping the
> > > > > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > > > > issues (good).
> > > > > > >
> > > > > > > One potential snag might be if the backing data model contains
> > > > > > > identical objects. I can't think of a practical use case for this
> > > > > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > > > > that there isn't one.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > > > > implementing preserveRowStates - there were some problems with nested
> > > > > > > > data-tables - you'd need to work around those problems specifically.
> > > > > > > >
> > > > > > > > What would be your idea of a key based on the row-data?
> > > > > > > >
> > > > > > > > regards,
> > > > > > > >
> > > > > > > > Martin
> > > > > > > >
> > > > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > > > > >
> > > > > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > > > > index. Is there some reason why the key isn't the row index?
> > > > > > > > >
> > > > > > > > > What about the possibility of storing the row data in the map using a
> > > > > > > > > key based on the row data itself? That way, changing the row model
> > > > > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > > > > this, but my understanding is that there are ways around referencing
> > > > > > > > > things that should perhaps be garbage-collected).
> > > > > > > > >
> > > > > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > > > > rows would work transparently.
> > > > > > > > >
> > > > > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > > > > f:selectItems does.
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > >
> > > > > > > > http://www.irian.at
> > > > > > > >
> > > > > > > > Your JSF powerhouse -
> > > > > > > > JSF Consulting, Development and
> > > > > > > > Courses in English and German
> > > > > > > >
> > > > > > > > Professional Support for Apache MyFaces
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > http://www.irian.at
> > > >
> > > > Your JSF powerhouse -
> > > > JSF Consulting, Development and
> > > > Courses in English and German
> > > >
> > > > Professional Support for Apache MyFaces
> > > >
> > >
> >
>
>
> --
> Mathias
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mathias Brökelmann <mb...@googlemail.com>.
Mike please create a unit test for this (including one for a nested
table). It will help us tracking this issue if other things are
changing in this area.
2007/4/12, Mike Kienenberger <mk...@gmail.com>:
> Ok. After making these two changes (use rowData as key, record last
> rowData object and only save input component state if the rowData
> object hasn't changed on setRowIndex()), everything seems to be
> working. I'm going to test nested datatables using the
> countryTableForm.jsp page, and that should be it.
>
> The only question is whether I should make the new way of doing things
> the default behavior for preserveRowStates or if I need to create a
> preserveRowStatesMode to determine whether to use the old or new
> behavior.
>
> On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > I've given this more thought overnight. I don't think _rowStates is
> > saved between requests. It only lives for the duration of one
> > Lifecycle iteration. This appears to be true for client-side state
> > saving. I'll need to double-check that it's also true for server-side
> > state saving as I'm pretty ignorant in that area. Thus, there's no
> > reason why we shouldn't be able to use the row data itself as the key
> > instead of a converter String based on the row data. In both cases,
> > we'd still have the issue with the same object appearing in the data
> > model, but again, I'm not sure what the use case would be.
> >
> > Also, the solution for the issue above is probably handled with the
> > following logic -- record the last data row object at the end of
> > setRowIndex(). If the last row object doesn't match the current row
> > object at the start of setRowIndex(), then don't try to save the row
> > state. We'd assume that the last row object is null at the start of
> > the lifecycle (I think rowIndex starts at -1 at the start of the
> > lifecycle as well, so this should be safe).
> >
> > One thing we might want to do is to have an attribute that specifies
> > preserve row state behavior. One option for the methodology above
> > which is save for sort/add/delete, but not safe for multiple copies of
> > the same rowData. One option for clientid (or maybe row number)
> > which is safe for multiple copies but unsafe for changes to the
> > dataModel.
> >
> > On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > the question is if we shouldn't dig deeper - shouldn't for all
> > > row-handling your special converter be used instead of the row-index?
> > > Trinidad has something similar (row-key).
> > >
> > > regards,
> > >
> > > Martin
> > >
> > > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > Ok. Here's why. The data for row 2 is copied to the input
> > > > components, row data 2 is deleted, then the data from the input
> > > > components (still for the original row 2) is copied back to row 2.
> > > >
> > > > ====================
> > > > before invokeAction
> > > > setRowIndex(2)
> > > > copy row 2 data to input components
> > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > > > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > > > got [[Ljava.lang.Object;@650646]
> > > >
> > > > delete row data 2.
> > > >
> > > > setRowIndex(-1)
> > > > copy input component values to row 2
> > > > 22:08:32.578 INFO [SocketListener0-1]
> > > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > > > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > > > value=[[Ljava.lang.Object;@1a32ea4]
> > > > ==================================
> > > >
> > > >
> > > > I'm not sure what a good fix for this would be. Perhaps detecting
> > > > that row data has changed in the invoke action phase? Perhaps caching
> > > > the original rows -> rowData associations before invoke action and
> > > > ignoring changes when the row data no longer matches the same row
> > > > instead of saving the components, or always using the cached values
> > > > during this phase?
> > > >
> > > >
> > > >
> > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > Well, this is kinda odd.
> > > > >
> > > > > I've implemented the converter version, and it *almost* works.
> > > > >
> > > > > Without the converter, using the standard clientId key,
> > > > >
> > > > > public String getAsString(FacesContext context, UIComponent
> > > > > component, Object value)
> > > > > {
> > > > > UIData uiData = (UIData)component;
> > > > > return uiData.getClientId(context);
> > > > > }
> > > > >
> > > > > If you have
> > > > >
> > > > > row 1 - 1111
> > > > > row 2 - 2222
> > > > > row 3 - 3333
> > > > > row 4 - 4444
> > > > > row 5 - 5555
> > > > >
> > > > > and you delete row 2, you get:
> > > > >
> > > > > row 1 - 1111
> > > > > row 3 - 2222
> > > > > row 4 - 3333
> > > > > row 5 - 4444
> > > > >
> > > > > Now if I throw in a converter that maps to an object:
> > > > >
> > > > > public String getAsString(FacesContext context, UIComponent
> > > > > component, Object value)
> > > > > {
> > > > > UIData uiData = (UIData)component;
> > > > > if (uiData.isRowAvailable())
> > > > > {
> > > > > Item item = (Item)uiData.getRowData();
> > > > > return "Item" + String.valueOf(item.getId());
> > > > > }
> > > > > return uiData.getClientId(context);
> > > > > }
> > > > >
> > > > > If you have
> > > > >
> > > > > row 1 - 1111
> > > > > row 2 - 2222
> > > > > row 3 - 3333
> > > > > row 4 - 4444
> > > > > row 5 - 5555
> > > > >
> > > > > and you delete row 2, you get:
> > > > >
> > > > > row 1 - 1111
> > > > > row 3 - 2222
> > > > > row 4 - 4444
> > > > > row 5 - 5555
> > > > >
> > > > > Weird. Everything is correct except for row 3 (which picked up the
> > > > > values for row 2), which is a vast improvement over the original, but
> > > > > still not quite right.
> > > > >
> > > > >
> > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > I don't think this will affect the nested datatables since we're not
> > > > > > changing the value, only the key, for each row-state map entry.
> > > > > > However, I might be misunderstanding something.
> > > > > >
> > > > > > A starting point could be
> > > > > >
> > > > > > private Map _rowStates = new WeakHashMap();
> > > > > >
> > > > > > Then, we'd change this:
> > > > > >
> > > > > > _rowStates.put(getClientId(facesContext),
> > > > > > saveDescendantComponentStates(getChildren()
> > > > > > .iterator(), false));
> > > > > >
> > > > > > to
> > > > > >
> > > > > > _rowStates.put(dataModel.getRowData(),
> > > > > > saveDescendantComponentStates(getChildren()
> > > > > > .iterator(), false));
> > > > > >
> > > > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > > > I'm guessing this would require that rowData be serializable.
> > > > > >
> > > > > > A better implementation might be:
> > > > > >
> > > > > > Converter converter = .... [either assigned explicitly or
> > > > > > fetched by RowData type]
> > > > > >
> > > > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > > > saveDescendantComponentStates(getChildren()
> > > > > > .iterator(), false));
> > > > > >
> > > > > > Now we're no longer storing a weak reference to the model object
> > > > > > (good), but we'll now have to be responsible for keeping the
> > > > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > > > issues (good).
> > > > > >
> > > > > > One potential snag might be if the backing data model contains
> > > > > > identical objects. I can't think of a practical use case for this
> > > > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > > > that there isn't one.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > > > implementing preserveRowStates - there were some problems with nested
> > > > > > > data-tables - you'd need to work around those problems specifically.
> > > > > > >
> > > > > > > What would be your idea of a key based on the row-data?
> > > > > > >
> > > > > > > regards,
> > > > > > >
> > > > > > > Martin
> > > > > > >
> > > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > > > >
> > > > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > > > index. Is there some reason why the key isn't the row index?
> > > > > > > >
> > > > > > > > What about the possibility of storing the row data in the map using a
> > > > > > > > key based on the row data itself? That way, changing the row model
> > > > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > > > this, but my understanding is that there are ways around referencing
> > > > > > > > things that should perhaps be garbage-collected).
> > > > > > > >
> > > > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > > > rows would work transparently.
> > > > > > > >
> > > > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > > > f:selectItems does.
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > >
> > > > > > > http://www.irian.at
> > > > > > >
> > > > > > > Your JSF powerhouse -
> > > > > > > JSF Consulting, Development and
> > > > > > > Courses in English and German
> > > > > > >
> > > > > > > Professional Support for Apache MyFaces
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > >
> > > http://www.irian.at
> > >
> > > Your JSF powerhouse -
> > > JSF Consulting, Development and
> > > Courses in English and German
> > >
> > > Professional Support for Apache MyFaces
> > >
> >
>
--
Mathias
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Ok. After making these two changes (use rowData as key, record last
rowData object and only save input component state if the rowData
object hasn't changed on setRowIndex()), everything seems to be
working. I'm going to test nested datatables using the
countryTableForm.jsp page, and that should be it.
The only question is whether I should make the new way of doing things
the default behavior for preserveRowStates or if I need to create a
preserveRowStatesMode to determine whether to use the old or new
behavior.
On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> I've given this more thought overnight. I don't think _rowStates is
> saved between requests. It only lives for the duration of one
> Lifecycle iteration. This appears to be true for client-side state
> saving. I'll need to double-check that it's also true for server-side
> state saving as I'm pretty ignorant in that area. Thus, there's no
> reason why we shouldn't be able to use the row data itself as the key
> instead of a converter String based on the row data. In both cases,
> we'd still have the issue with the same object appearing in the data
> model, but again, I'm not sure what the use case would be.
>
> Also, the solution for the issue above is probably handled with the
> following logic -- record the last data row object at the end of
> setRowIndex(). If the last row object doesn't match the current row
> object at the start of setRowIndex(), then don't try to save the row
> state. We'd assume that the last row object is null at the start of
> the lifecycle (I think rowIndex starts at -1 at the start of the
> lifecycle as well, so this should be safe).
>
> One thing we might want to do is to have an attribute that specifies
> preserve row state behavior. One option for the methodology above
> which is save for sort/add/delete, but not safe for multiple copies of
> the same rowData. One option for clientid (or maybe row number)
> which is safe for multiple copies but unsafe for changes to the
> dataModel.
>
> On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> > the question is if we shouldn't dig deeper - shouldn't for all
> > row-handling your special converter be used instead of the row-index?
> > Trinidad has something similar (row-key).
> >
> > regards,
> >
> > Martin
> >
> > On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > Ok. Here's why. The data for row 2 is copied to the input
> > > components, row data 2 is deleted, then the data from the input
> > > components (still for the original row 2) is copied back to row 2.
> > >
> > > ====================
> > > before invokeAction
> > > setRowIndex(2)
> > > copy row 2 data to input components
> > > 22:08:32.578 INFO [SocketListener0-1]
> > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > > got [[Ljava.lang.Object;@650646]
> > >
> > > delete row data 2.
> > >
> > > setRowIndex(-1)
> > > copy input component values to row 2
> > > 22:08:32.578 INFO [SocketListener0-1]
> > > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > > value=[[Ljava.lang.Object;@1a32ea4]
> > > ==================================
> > >
> > >
> > > I'm not sure what a good fix for this would be. Perhaps detecting
> > > that row data has changed in the invoke action phase? Perhaps caching
> > > the original rows -> rowData associations before invoke action and
> > > ignoring changes when the row data no longer matches the same row
> > > instead of saving the components, or always using the cached values
> > > during this phase?
> > >
> > >
> > >
> > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > Well, this is kinda odd.
> > > >
> > > > I've implemented the converter version, and it *almost* works.
> > > >
> > > > Without the converter, using the standard clientId key,
> > > >
> > > > public String getAsString(FacesContext context, UIComponent
> > > > component, Object value)
> > > > {
> > > > UIData uiData = (UIData)component;
> > > > return uiData.getClientId(context);
> > > > }
> > > >
> > > > If you have
> > > >
> > > > row 1 - 1111
> > > > row 2 - 2222
> > > > row 3 - 3333
> > > > row 4 - 4444
> > > > row 5 - 5555
> > > >
> > > > and you delete row 2, you get:
> > > >
> > > > row 1 - 1111
> > > > row 3 - 2222
> > > > row 4 - 3333
> > > > row 5 - 4444
> > > >
> > > > Now if I throw in a converter that maps to an object:
> > > >
> > > > public String getAsString(FacesContext context, UIComponent
> > > > component, Object value)
> > > > {
> > > > UIData uiData = (UIData)component;
> > > > if (uiData.isRowAvailable())
> > > > {
> > > > Item item = (Item)uiData.getRowData();
> > > > return "Item" + String.valueOf(item.getId());
> > > > }
> > > > return uiData.getClientId(context);
> > > > }
> > > >
> > > > If you have
> > > >
> > > > row 1 - 1111
> > > > row 2 - 2222
> > > > row 3 - 3333
> > > > row 4 - 4444
> > > > row 5 - 5555
> > > >
> > > > and you delete row 2, you get:
> > > >
> > > > row 1 - 1111
> > > > row 3 - 2222
> > > > row 4 - 4444
> > > > row 5 - 5555
> > > >
> > > > Weird. Everything is correct except for row 3 (which picked up the
> > > > values for row 2), which is a vast improvement over the original, but
> > > > still not quite right.
> > > >
> > > >
> > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > I don't think this will affect the nested datatables since we're not
> > > > > changing the value, only the key, for each row-state map entry.
> > > > > However, I might be misunderstanding something.
> > > > >
> > > > > A starting point could be
> > > > >
> > > > > private Map _rowStates = new WeakHashMap();
> > > > >
> > > > > Then, we'd change this:
> > > > >
> > > > > _rowStates.put(getClientId(facesContext),
> > > > > saveDescendantComponentStates(getChildren()
> > > > > .iterator(), false));
> > > > >
> > > > > to
> > > > >
> > > > > _rowStates.put(dataModel.getRowData(),
> > > > > saveDescendantComponentStates(getChildren()
> > > > > .iterator(), false));
> > > > >
> > > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > > I'm guessing this would require that rowData be serializable.
> > > > >
> > > > > A better implementation might be:
> > > > >
> > > > > Converter converter = .... [either assigned explicitly or
> > > > > fetched by RowData type]
> > > > >
> > > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > > saveDescendantComponentStates(getChildren()
> > > > > .iterator(), false));
> > > > >
> > > > > Now we're no longer storing a weak reference to the model object
> > > > > (good), but we'll now have to be responsible for keeping the
> > > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > > issues (good).
> > > > >
> > > > > One potential snag might be if the backing data model contains
> > > > > identical objects. I can't think of a practical use case for this
> > > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > > that there isn't one.
> > > > >
> > > > >
> > > > >
> > > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > > implementing preserveRowStates - there were some problems with nested
> > > > > > data-tables - you'd need to work around those problems specifically.
> > > > > >
> > > > > > What would be your idea of a key based on the row-data?
> > > > > >
> > > > > > regards,
> > > > > >
> > > > > > Martin
> > > > > >
> > > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > > >
> > > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > > index. Is there some reason why the key isn't the row index?
> > > > > > >
> > > > > > > What about the possibility of storing the row data in the map using a
> > > > > > > key based on the row data itself? That way, changing the row model
> > > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > > this, but my understanding is that there are ways around referencing
> > > > > > > things that should perhaps be garbage-collected).
> > > > > > >
> > > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > > rows would work transparently.
> > > > > > >
> > > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > > f:selectItems does.
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > >
> > > > > > http://www.irian.at
> > > > > >
> > > > > > Your JSF powerhouse -
> > > > > > JSF Consulting, Development and
> > > > > > Courses in English and German
> > > > > >
> > > > > > Professional Support for Apache MyFaces
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> > --
> >
> > http://www.irian.at
> >
> > Your JSF powerhouse -
> > JSF Consulting, Development and
> > Courses in English and German
> >
> > Professional Support for Apache MyFaces
> >
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
I've given this more thought overnight. I don't think _rowStates is
saved between requests. It only lives for the duration of one
Lifecycle iteration. This appears to be true for client-side state
saving. I'll need to double-check that it's also true for server-side
state saving as I'm pretty ignorant in that area. Thus, there's no
reason why we shouldn't be able to use the row data itself as the key
instead of a converter String based on the row data. In both cases,
we'd still have the issue with the same object appearing in the data
model, but again, I'm not sure what the use case would be.
Also, the solution for the issue above is probably handled with the
following logic -- record the last data row object at the end of
setRowIndex(). If the last row object doesn't match the current row
object at the start of setRowIndex(), then don't try to save the row
state. We'd assume that the last row object is null at the start of
the lifecycle (I think rowIndex starts at -1 at the start of the
lifecycle as well, so this should be safe).
One thing we might want to do is to have an attribute that specifies
preserve row state behavior. One option for the methodology above
which is save for sort/add/delete, but not safe for multiple copies of
the same rowData. One option for clientid (or maybe row number)
which is safe for multiple copies but unsafe for changes to the
dataModel.
On 4/12/07, Martin Marinschek <ma...@gmail.com> wrote:
> the question is if we shouldn't dig deeper - shouldn't for all
> row-handling your special converter be used instead of the row-index?
> Trinidad has something similar (row-key).
>
> regards,
>
> Martin
>
> On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > Ok. Here's why. The data for row 2 is copied to the input
> > components, row data 2 is deleted, then the data from the input
> > components (still for the original row 2) is copied back to row 2.
> >
> > ====================
> > before invokeAction
> > setRowIndex(2)
> > copy row 2 data to input components
> > 22:08:32.578 INFO [SocketListener0-1]
> > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> > >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> > got [[Ljava.lang.Object;@650646]
> >
> > delete row data 2.
> >
> > setRowIndex(-1)
> > copy input component values to row 2
> > 22:08:32.578 INFO [SocketListener0-1]
> > org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> > >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> > value=[[Ljava.lang.Object;@1a32ea4]
> > ==================================
> >
> >
> > I'm not sure what a good fix for this would be. Perhaps detecting
> > that row data has changed in the invoke action phase? Perhaps caching
> > the original rows -> rowData associations before invoke action and
> > ignoring changes when the row data no longer matches the same row
> > instead of saving the components, or always using the cached values
> > during this phase?
> >
> >
> >
> > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > Well, this is kinda odd.
> > >
> > > I've implemented the converter version, and it *almost* works.
> > >
> > > Without the converter, using the standard clientId key,
> > >
> > > public String getAsString(FacesContext context, UIComponent
> > > component, Object value)
> > > {
> > > UIData uiData = (UIData)component;
> > > return uiData.getClientId(context);
> > > }
> > >
> > > If you have
> > >
> > > row 1 - 1111
> > > row 2 - 2222
> > > row 3 - 3333
> > > row 4 - 4444
> > > row 5 - 5555
> > >
> > > and you delete row 2, you get:
> > >
> > > row 1 - 1111
> > > row 3 - 2222
> > > row 4 - 3333
> > > row 5 - 4444
> > >
> > > Now if I throw in a converter that maps to an object:
> > >
> > > public String getAsString(FacesContext context, UIComponent
> > > component, Object value)
> > > {
> > > UIData uiData = (UIData)component;
> > > if (uiData.isRowAvailable())
> > > {
> > > Item item = (Item)uiData.getRowData();
> > > return "Item" + String.valueOf(item.getId());
> > > }
> > > return uiData.getClientId(context);
> > > }
> > >
> > > If you have
> > >
> > > row 1 - 1111
> > > row 2 - 2222
> > > row 3 - 3333
> > > row 4 - 4444
> > > row 5 - 5555
> > >
> > > and you delete row 2, you get:
> > >
> > > row 1 - 1111
> > > row 3 - 2222
> > > row 4 - 4444
> > > row 5 - 5555
> > >
> > > Weird. Everything is correct except for row 3 (which picked up the
> > > values for row 2), which is a vast improvement over the original, but
> > > still not quite right.
> > >
> > >
> > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > I don't think this will affect the nested datatables since we're not
> > > > changing the value, only the key, for each row-state map entry.
> > > > However, I might be misunderstanding something.
> > > >
> > > > A starting point could be
> > > >
> > > > private Map _rowStates = new WeakHashMap();
> > > >
> > > > Then, we'd change this:
> > > >
> > > > _rowStates.put(getClientId(facesContext),
> > > > saveDescendantComponentStates(getChildren()
> > > > .iterator(), false));
> > > >
> > > > to
> > > >
> > > > _rowStates.put(dataModel.getRowData(),
> > > > saveDescendantComponentStates(getChildren()
> > > > .iterator(), false));
> > > >
> > > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > > I'm guessing this would require that rowData be serializable.
> > > >
> > > > A better implementation might be:
> > > >
> > > > Converter converter = .... [either assigned explicitly or
> > > > fetched by RowData type]
> > > >
> > > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > > saveDescendantComponentStates(getChildren()
> > > > .iterator(), false));
> > > >
> > > > Now we're no longer storing a weak reference to the model object
> > > > (good), but we'll now have to be responsible for keeping the
> > > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > > issues (good).
> > > >
> > > > One potential snag might be if the backing data model contains
> > > > identical objects. I can't think of a practical use case for this
> > > > (inputs on multiple rows for the same object), but that doesn't mean
> > > > that there isn't one.
> > > >
> > > >
> > > >
> > > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > > Your ideas sound good, one thing that I remember was discussed while
> > > > > implementing preserveRowStates - there were some problems with nested
> > > > > data-tables - you'd need to work around those problems specifically.
> > > > >
> > > > > What would be your idea of a key based on the row-data?
> > > > >
> > > > > regards,
> > > > >
> > > > > Martin
> > > > >
> > > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > > >
> > > > > > One of the things I notice is that the key to the row state Map is the
> > > > > > clientid of the dataTable, which of course varies based on the row
> > > > > > index. Is there some reason why the key isn't the row index?
> > > > > >
> > > > > > What about the possibility of storing the row data in the map using a
> > > > > > key based on the row data itself? That way, changing the row model
> > > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > > The only issues are that we might be keeping the row state for rows
> > > > > > that no longer in the model, and we might be holding onto a reference
> > > > > > to an object that should be garbage collected (I've never delved into
> > > > > > this, but my understanding is that there are ways around referencing
> > > > > > things that should perhaps be garbage-collected).
> > > > > >
> > > > > > However, if it works, it would automaticallly solve all of the row
> > > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > > rows would work transparently.
> > > > > >
> > > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > > light-weight key to the row data rather than the row data itself like
> > > > > > f:selectItems does.
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > >
> > > > > http://www.irian.at
> > > > >
> > > > > Your JSF powerhouse -
> > > > > JSF Consulting, Development and
> > > > > Courses in English and German
> > > > >
> > > > > Professional Support for Apache MyFaces
> > > > >
> > > >
> > >
> >
>
>
> --
>
> http://www.irian.at
>
> Your JSF powerhouse -
> JSF Consulting, Development and
> Courses in English and German
>
> Professional Support for Apache MyFaces
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Martin Marinschek <ma...@gmail.com>.
the question is if we shouldn't dig deeper - shouldn't for all
row-handling your special converter be used instead of the row-index?
Trinidad has something similar (row-key).
regards,
Martin
On 4/12/07, Mike Kienenberger <mk...@gmail.com> wrote:
> Ok. Here's why. The data for row 2 is copied to the input
> components, row data 2 is deleted, then the data from the input
> components (still for the original row 2) is copied back to row 2.
>
> ====================
> before invokeAction
> setRowIndex(2)
> copy row 2 data to input components
> 22:08:32.578 INFO [SocketListener0-1]
> org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
> >35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
> got [[Ljava.lang.Object;@650646]
>
> delete row data 2.
>
> setRowIndex(-1)
> copy input component values to row 2
> 22:08:32.578 INFO [SocketListener0-1]
> org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
> >35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
> value=[[Ljava.lang.Object;@1a32ea4]
> ==================================
>
>
> I'm not sure what a good fix for this would be. Perhaps detecting
> that row data has changed in the invoke action phase? Perhaps caching
> the original rows -> rowData associations before invoke action and
> ignoring changes when the row data no longer matches the same row
> instead of saving the components, or always using the cached values
> during this phase?
>
>
>
> On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > Well, this is kinda odd.
> >
> > I've implemented the converter version, and it *almost* works.
> >
> > Without the converter, using the standard clientId key,
> >
> > public String getAsString(FacesContext context, UIComponent
> > component, Object value)
> > {
> > UIData uiData = (UIData)component;
> > return uiData.getClientId(context);
> > }
> >
> > If you have
> >
> > row 1 - 1111
> > row 2 - 2222
> > row 3 - 3333
> > row 4 - 4444
> > row 5 - 5555
> >
> > and you delete row 2, you get:
> >
> > row 1 - 1111
> > row 3 - 2222
> > row 4 - 3333
> > row 5 - 4444
> >
> > Now if I throw in a converter that maps to an object:
> >
> > public String getAsString(FacesContext context, UIComponent
> > component, Object value)
> > {
> > UIData uiData = (UIData)component;
> > if (uiData.isRowAvailable())
> > {
> > Item item = (Item)uiData.getRowData();
> > return "Item" + String.valueOf(item.getId());
> > }
> > return uiData.getClientId(context);
> > }
> >
> > If you have
> >
> > row 1 - 1111
> > row 2 - 2222
> > row 3 - 3333
> > row 4 - 4444
> > row 5 - 5555
> >
> > and you delete row 2, you get:
> >
> > row 1 - 1111
> > row 3 - 2222
> > row 4 - 4444
> > row 5 - 5555
> >
> > Weird. Everything is correct except for row 3 (which picked up the
> > values for row 2), which is a vast improvement over the original, but
> > still not quite right.
> >
> >
> > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > I don't think this will affect the nested datatables since we're not
> > > changing the value, only the key, for each row-state map entry.
> > > However, I might be misunderstanding something.
> > >
> > > A starting point could be
> > >
> > > private Map _rowStates = new WeakHashMap();
> > >
> > > Then, we'd change this:
> > >
> > > _rowStates.put(getClientId(facesContext),
> > > saveDescendantComponentStates(getChildren()
> > > .iterator(), false));
> > >
> > > to
> > >
> > > _rowStates.put(dataModel.getRowData(),
> > > saveDescendantComponentStates(getChildren()
> > > .iterator(), false));
> > >
> > > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > > I'm guessing this would require that rowData be serializable.
> > >
> > > A better implementation might be:
> > >
> > > Converter converter = .... [either assigned explicitly or
> > > fetched by RowData type]
> > >
> > > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > > saveDescendantComponentStates(getChildren()
> > > .iterator(), false));
> > >
> > > Now we're no longer storing a weak reference to the model object
> > > (good), but we'll now have to be responsible for keeping the
> > > _rowStates map cleaned up (bad). Now we won't have serialization
> > > issues (good).
> > >
> > > One potential snag might be if the backing data model contains
> > > identical objects. I can't think of a practical use case for this
> > > (inputs on multiple rows for the same object), but that doesn't mean
> > > that there isn't one.
> > >
> > >
> > >
> > > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > > Your ideas sound good, one thing that I remember was discussed while
> > > > implementing preserveRowStates - there were some problems with nested
> > > > data-tables - you'd need to work around those problems specifically.
> > > >
> > > > What would be your idea of a key based on the row-data?
> > > >
> > > > regards,
> > > >
> > > > Martin
> > > >
> > > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > > fix the issues with deleting a row when preserveRowStates=true.
> > > > >
> > > > > One of the things I notice is that the key to the row state Map is the
> > > > > clientid of the dataTable, which of course varies based on the row
> > > > > index. Is there some reason why the key isn't the row index?
> > > > >
> > > > > What about the possibility of storing the row data in the map using a
> > > > > key based on the row data itself? That way, changing the row model
> > > > > (adding/deleting rows) would automatically pick the right row state?
> > > > > The only issues are that we might be keeping the row state for rows
> > > > > that no longer in the model, and we might be holding onto a reference
> > > > > to an object that should be garbage collected (I've never delved into
> > > > > this, but my understanding is that there are ways around referencing
> > > > > things that should perhaps be garbage-collected).
> > > > >
> > > > > However, if it works, it would automaticallly solve all of the row
> > > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > > rows would work transparently.
> > > > >
> > > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > > light-weight key to the row data rather than the row data itself like
> > > > > f:selectItems does.
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > http://www.irian.at
> > > >
> > > > Your JSF powerhouse -
> > > > JSF Consulting, Development and
> > > > Courses in English and German
> > > >
> > > > Professional Support for Apache MyFaces
> > > >
> > >
> >
>
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Ok. Here's why. The data for row 2 is copied to the input
components, row data 2 is deleted, then the data from the input
components (still for the original row 2) is copied back to row 2.
====================
before invokeAction
setRowIndex(2)
copy row 2 data to input components
22:08:32.578 INFO [SocketListener0-1]
org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:290)
>35> Fetching rowState for key=Item2, _rowIndex=1, rowData=Item #2,
got [[Ljava.lang.Object;@650646]
delete row data 2.
setRowIndex(-1)
copy input component values to row 2
22:08:32.578 INFO [SocketListener0-1]
org.apache.myfaces.component.html.ext.HtmlDataTableHack.setRowIndex(HtmlDataTableHack.java:245)
>35> Storing rowState for key=Item3, _rowIndex=1, rowData=Item #3,
value=[[Ljava.lang.Object;@1a32ea4]
==================================
I'm not sure what a good fix for this would be. Perhaps detecting
that row data has changed in the invoke action phase? Perhaps caching
the original rows -> rowData associations before invoke action and
ignoring changes when the row data no longer matches the same row
instead of saving the components, or always using the cached values
during this phase?
On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> Well, this is kinda odd.
>
> I've implemented the converter version, and it *almost* works.
>
> Without the converter, using the standard clientId key,
>
> public String getAsString(FacesContext context, UIComponent
> component, Object value)
> {
> UIData uiData = (UIData)component;
> return uiData.getClientId(context);
> }
>
> If you have
>
> row 1 - 1111
> row 2 - 2222
> row 3 - 3333
> row 4 - 4444
> row 5 - 5555
>
> and you delete row 2, you get:
>
> row 1 - 1111
> row 3 - 2222
> row 4 - 3333
> row 5 - 4444
>
> Now if I throw in a converter that maps to an object:
>
> public String getAsString(FacesContext context, UIComponent
> component, Object value)
> {
> UIData uiData = (UIData)component;
> if (uiData.isRowAvailable())
> {
> Item item = (Item)uiData.getRowData();
> return "Item" + String.valueOf(item.getId());
> }
> return uiData.getClientId(context);
> }
>
> If you have
>
> row 1 - 1111
> row 2 - 2222
> row 3 - 3333
> row 4 - 4444
> row 5 - 5555
>
> and you delete row 2, you get:
>
> row 1 - 1111
> row 3 - 2222
> row 4 - 4444
> row 5 - 5555
>
> Weird. Everything is correct except for row 3 (which picked up the
> values for row 2), which is a vast improvement over the original, but
> still not quite right.
>
>
> On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > I don't think this will affect the nested datatables since we're not
> > changing the value, only the key, for each row-state map entry.
> > However, I might be misunderstanding something.
> >
> > A starting point could be
> >
> > private Map _rowStates = new WeakHashMap();
> >
> > Then, we'd change this:
> >
> > _rowStates.put(getClientId(facesContext),
> > saveDescendantComponentStates(getChildren()
> > .iterator(), false));
> >
> > to
> >
> > _rowStates.put(dataModel.getRowData(),
> > saveDescendantComponentStates(getChildren()
> > .iterator(), false));
> >
> > Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> > I'm guessing this would require that rowData be serializable.
> >
> > A better implementation might be:
> >
> > Converter converter = .... [either assigned explicitly or
> > fetched by RowData type]
> >
> > _rowStates.put(converter.getAsString(dataModel.getRowData()),
> > saveDescendantComponentStates(getChildren()
> > .iterator(), false));
> >
> > Now we're no longer storing a weak reference to the model object
> > (good), but we'll now have to be responsible for keeping the
> > _rowStates map cleaned up (bad). Now we won't have serialization
> > issues (good).
> >
> > One potential snag might be if the backing data model contains
> > identical objects. I can't think of a practical use case for this
> > (inputs on multiple rows for the same object), but that doesn't mean
> > that there isn't one.
> >
> >
> >
> > On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > > Your ideas sound good, one thing that I remember was discussed while
> > > implementing preserveRowStates - there were some problems with nested
> > > data-tables - you'd need to work around those problems specifically.
> > >
> > > What would be your idea of a key based on the row-data?
> > >
> > > regards,
> > >
> > > Martin
> > >
> > > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > > I'm looking into the behavior of preserveRowStates in order to try to
> > > > fix the issues with deleting a row when preserveRowStates=true.
> > > >
> > > > One of the things I notice is that the key to the row state Map is the
> > > > clientid of the dataTable, which of course varies based on the row
> > > > index. Is there some reason why the key isn't the row index?
> > > >
> > > > What about the possibility of storing the row data in the map using a
> > > > key based on the row data itself? That way, changing the row model
> > > > (adding/deleting rows) would automatically pick the right row state?
> > > > The only issues are that we might be keeping the row state for rows
> > > > that no longer in the model, and we might be holding onto a reference
> > > > to an object that should be garbage collected (I've never delved into
> > > > this, but my understanding is that there are ways around referencing
> > > > things that should perhaps be garbage-collected).
> > > >
> > > > However, if it works, it would automaticallly solve all of the row
> > > > issues transparently to the end-user. Sorting, inserting, deleting
> > > > rows would work transparently.
> > > >
> > > > We might also want to put in a preserveRowStatesConverter so we save a
> > > > light-weight key to the row data rather than the row data itself like
> > > > f:selectItems does.
> > > >
> > >
> > >
> > > --
> > >
> > > http://www.irian.at
> > >
> > > Your JSF powerhouse -
> > > JSF Consulting, Development and
> > > Courses in English and German
> > >
> > > Professional Support for Apache MyFaces
> > >
> >
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
Well, this is kinda odd.
I've implemented the converter version, and it *almost* works.
Without the converter, using the standard clientId key,
public String getAsString(FacesContext context, UIComponent
component, Object value)
{
UIData uiData = (UIData)component;
return uiData.getClientId(context);
}
If you have
row 1 - 1111
row 2 - 2222
row 3 - 3333
row 4 - 4444
row 5 - 5555
and you delete row 2, you get:
row 1 - 1111
row 3 - 2222
row 4 - 3333
row 5 - 4444
Now if I throw in a converter that maps to an object:
public String getAsString(FacesContext context, UIComponent
component, Object value)
{
UIData uiData = (UIData)component;
if (uiData.isRowAvailable())
{
Item item = (Item)uiData.getRowData();
return "Item" + String.valueOf(item.getId());
}
return uiData.getClientId(context);
}
If you have
row 1 - 1111
row 2 - 2222
row 3 - 3333
row 4 - 4444
row 5 - 5555
and you delete row 2, you get:
row 1 - 1111
row 3 - 2222
row 4 - 4444
row 5 - 5555
Weird. Everything is correct except for row 3 (which picked up the
values for row 2), which is a vast improvement over the original, but
still not quite right.
On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> I don't think this will affect the nested datatables since we're not
> changing the value, only the key, for each row-state map entry.
> However, I might be misunderstanding something.
>
> A starting point could be
>
> private Map _rowStates = new WeakHashMap();
>
> Then, we'd change this:
>
> _rowStates.put(getClientId(facesContext),
> saveDescendantComponentStates(getChildren()
> .iterator(), false));
>
> to
>
> _rowStates.put(dataModel.getRowData(),
> saveDescendantComponentStates(getChildren()
> .iterator(), false));
>
> Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
> I'm guessing this would require that rowData be serializable.
>
> A better implementation might be:
>
> Converter converter = .... [either assigned explicitly or
> fetched by RowData type]
>
> _rowStates.put(converter.getAsString(dataModel.getRowData()),
> saveDescendantComponentStates(getChildren()
> .iterator(), false));
>
> Now we're no longer storing a weak reference to the model object
> (good), but we'll now have to be responsible for keeping the
> _rowStates map cleaned up (bad). Now we won't have serialization
> issues (good).
>
> One potential snag might be if the backing data model contains
> identical objects. I can't think of a practical use case for this
> (inputs on multiple rows for the same object), but that doesn't mean
> that there isn't one.
>
>
>
> On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> > Your ideas sound good, one thing that I remember was discussed while
> > implementing preserveRowStates - there were some problems with nested
> > data-tables - you'd need to work around those problems specifically.
> >
> > What would be your idea of a key based on the row-data?
> >
> > regards,
> >
> > Martin
> >
> > On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > > I'm looking into the behavior of preserveRowStates in order to try to
> > > fix the issues with deleting a row when preserveRowStates=true.
> > >
> > > One of the things I notice is that the key to the row state Map is the
> > > clientid of the dataTable, which of course varies based on the row
> > > index. Is there some reason why the key isn't the row index?
> > >
> > > What about the possibility of storing the row data in the map using a
> > > key based on the row data itself? That way, changing the row model
> > > (adding/deleting rows) would automatically pick the right row state?
> > > The only issues are that we might be keeping the row state for rows
> > > that no longer in the model, and we might be holding onto a reference
> > > to an object that should be garbage collected (I've never delved into
> > > this, but my understanding is that there are ways around referencing
> > > things that should perhaps be garbage-collected).
> > >
> > > However, if it works, it would automaticallly solve all of the row
> > > issues transparently to the end-user. Sorting, inserting, deleting
> > > rows would work transparently.
> > >
> > > We might also want to put in a preserveRowStatesConverter so we save a
> > > light-weight key to the row data rather than the row data itself like
> > > f:selectItems does.
> > >
> >
> >
> > --
> >
> > http://www.irian.at
> >
> > Your JSF powerhouse -
> > JSF Consulting, Development and
> > Courses in English and German
> >
> > Professional Support for Apache MyFaces
> >
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Mike Kienenberger <mk...@gmail.com>.
I don't think this will affect the nested datatables since we're not
changing the value, only the key, for each row-state map entry.
However, I might be misunderstanding something.
A starting point could be
private Map _rowStates = new WeakHashMap();
Then, we'd change this:
_rowStates.put(getClientId(facesContext),
saveDescendantComponentStates(getChildren()
.iterator(), false));
to
_rowStates.put(dataModel.getRowData(),
saveDescendantComponentStates(getChildren()
.iterator(), false));
Note, for describing this, I'm ignoring how we'd handle isRowAvailable=false.
I'm guessing this would require that rowData be serializable.
A better implementation might be:
Converter converter = .... [either assigned explicitly or
fetched by RowData type]
_rowStates.put(converter.getAsString(dataModel.getRowData()),
saveDescendantComponentStates(getChildren()
.iterator(), false));
Now we're no longer storing a weak reference to the model object
(good), but we'll now have to be responsible for keeping the
_rowStates map cleaned up (bad). Now we won't have serialization
issues (good).
One potential snag might be if the backing data model contains
identical objects. I can't think of a practical use case for this
(inputs on multiple rows for the same object), but that doesn't mean
that there isn't one.
On 4/11/07, Martin Marinschek <ma...@gmail.com> wrote:
> Your ideas sound good, one thing that I remember was discussed while
> implementing preserveRowStates - there were some problems with nested
> data-tables - you'd need to work around those problems specifically.
>
> What would be your idea of a key based on the row-data?
>
> regards,
>
> Martin
>
> On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> > I'm looking into the behavior of preserveRowStates in order to try to
> > fix the issues with deleting a row when preserveRowStates=true.
> >
> > One of the things I notice is that the key to the row state Map is the
> > clientid of the dataTable, which of course varies based on the row
> > index. Is there some reason why the key isn't the row index?
> >
> > What about the possibility of storing the row data in the map using a
> > key based on the row data itself? That way, changing the row model
> > (adding/deleting rows) would automatically pick the right row state?
> > The only issues are that we might be keeping the row state for rows
> > that no longer in the model, and we might be holding onto a reference
> > to an object that should be garbage collected (I've never delved into
> > this, but my understanding is that there are ways around referencing
> > things that should perhaps be garbage-collected).
> >
> > However, if it works, it would automaticallly solve all of the row
> > issues transparently to the end-user. Sorting, inserting, deleting
> > rows would work transparently.
> >
> > We might also want to put in a preserveRowStatesConverter so we save a
> > light-weight key to the row data rather than the row data itself like
> > f:selectItems does.
> >
>
>
> --
>
> http://www.irian.at
>
> Your JSF powerhouse -
> JSF Consulting, Development and
> Courses in English and German
>
> Professional Support for Apache MyFaces
>
Re: dataTable row state saving / preserveRowStates / keying row states by row data
Posted by Martin Marinschek <ma...@gmail.com>.
Your ideas sound good, one thing that I remember was discussed while
implementing preserveRowStates - there were some problems with nested
data-tables - you'd need to work around those problems specifically.
What would be your idea of a key based on the row-data?
regards,
Martin
On 4/11/07, Mike Kienenberger <mk...@gmail.com> wrote:
> I'm looking into the behavior of preserveRowStates in order to try to
> fix the issues with deleting a row when preserveRowStates=true.
>
> One of the things I notice is that the key to the row state Map is the
> clientid of the dataTable, which of course varies based on the row
> index. Is there some reason why the key isn't the row index?
>
> What about the possibility of storing the row data in the map using a
> key based on the row data itself? That way, changing the row model
> (adding/deleting rows) would automatically pick the right row state?
> The only issues are that we might be keeping the row state for rows
> that no longer in the model, and we might be holding onto a reference
> to an object that should be garbage collected (I've never delved into
> this, but my understanding is that there are ways around referencing
> things that should perhaps be garbage-collected).
>
> However, if it works, it would automaticallly solve all of the row
> issues transparently to the end-user. Sorting, inserting, deleting
> rows would work transparently.
>
> We might also want to put in a preserveRowStatesConverter so we save a
> light-weight key to the row data rather than the row data itself like
> f:selectItems does.
>
--
http://www.irian.at
Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German
Professional Support for Apache MyFaces