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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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
> >
>
