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

Reply via email to