Repeaters, dynamic data & detaching models

[Tobias Gierke]

Hi,

A collegue of mine just came across a rather interesting bug in our
Wicket application.

1. We have a simple page with a repeater (ListView) that displays a
table and on each row, some buttons to perform actions on the item shown
on this row  (edit/delete/etc.)
2. The underlying data source (a database table) gets updated
concurrently by another process running on another machine
3. The table is supposed to always show the latest data at the very top,
so the page uses a LoadableDetachableModel to always hit the database on
every request

The bug:

Users complained that performing actions on the data items would
actually affect an item different from what they clicked.

The explanation:

Since the list model got detached at the end of the previous request,
clicking any of the action buttons would re-populate the data model,
fetching previously unseen rows from the database. Since (according to
my collegue,didn't double-check) the ListView associates the item models
only based on their list index, the action button on the very first row
now all of a sudden referred to a database row the user didn't even know
about.

His fix:

Instead of

view = new ListView<Data>("listView" , dataProvider )
{
   @Override
   protected void populateItem(ListItem<PCAPFile> item)
   {
       add(new AjaxButton( "delete , item.getModel() ) // use model from ListItem (model gets detached after request)
       {
            public void onClick(AjaxRequestTarget target) {
                delete( getModelObject() );
            }
       });
       // ... more stuff
   }
}

he changed the code to read:

view = new ListView<Data>("listView" , dataProvider )
{
   @Override
   protected void populateItem(ListItem<PCAPFile> item)
   {
       add(new AjaxButton( "delete , item.getModelObject() ) // capture model object when constructing the button
       {
            public void onClick(AjaxRequestTarget target) {
                delete( getModelObject() );
            }
       });
       // ... more stuff
   }
}

This obviously is a rather subtle issue and - depending on the size of
your model objects - also comes with a certain performance/memory cost
because of the additional serialization for the model items the repeater
components are now holding onto.

Is this the recommended approach for dealing with dynamically changing
data or is there a better way to do it ?

Thanks,
Tobias

[Sven Meier]

Hi,

your first solution was inefficient anyways, because every click
reloaded all Datas first, before acting on a single one only.

You should use a detachable model instead, see JpaLoadableModel here
https://ci.apache.org/projects/wicket/guide/8.x/single.html#_detachable_models

Have fun
Sven

Post by Tobias Gierke
Hi,
A collegue of mine just came across a rather interesting bug in our
Wicket application.
1. We have a simple page with a repeater (ListView) that displays a
table and on each row, some buttons to perform actions on the item
shown on this row  (edit/delete/etc.)
2. The underlying data source (a database table) gets updated
concurrently by another process running on another machine
3. The table is supposed to always show the latest data at the very
top, so the page uses a LoadableDetachableModel to always hit the
database on every request
Users complained that performing actions on the data items would
actually affect an item different from what they clicked.
Since the list model got detached at the end of the previous request,
clicking any of the action buttons would re-populate the data model,
fetching previously unseen rows from the database. Since (according to
my collegue,didn't double-check) the ListView associates the item
models only based on their list index, the action button on the very
first row now all of a sudden referred to a database row the user
didn't even know about.
Instead of
view = new ListView<Data>("listView" , dataProvider )
{
   protected void populateItem(ListItem<PCAPFile> item)
   {
       add(new AjaxButton( "delete , item.getModel() ) // use model
from ListItem (model gets detached after request)
       {
            public void onClick(AjaxRequestTarget target) {
                delete( getModelObject() );
            }
       });
       // ... more stuff
   }
}
view = new ListView<Data>("listView" , dataProvider )
{
   protected void populateItem(ListItem<PCAPFile> item)
   {
       add(new AjaxButton( "delete , item.getModelObject() ) //
capture model object when constructing the button
       {
            public void onClick(AjaxRequestTarget target) {
                delete( getModelObject() );
            }
       });
       // ... more stuff
   }
}
This obviously is a rather subtle issue and - depending on the size of
your model objects - also comes with a certain performance/memory cost
because of the additional serialization for the model items the
repeater components are now holding onto.
Is this the recommended approach for dealing with dynamically changing
data or is there a better way to do it ?
Thanks,
Tobias

---------------------------------------------------------------------
To unsubscribe, e-mail: users-***@wicket.apache.org
For additional commands, e-mail: users-***@wicket.apache.org

[Tobias Gierke]

I've accidently sent my follow-up mail directly to Sven, here it is (and
his reply) for the sake of completeness, just in case someone else
stumbles across this as well.

Cheers,
Tobias

Hi Tobias,

As we're already using a LoadableDetachableModel for the repeater

itself, you probably meant I should override
ListView#getListItemModel() and return a LoadableDetachableModel
there, right ?
+1 exactly.
Sven
Hi Sven,
Thanks for your reply !

Hi,
your first solution was inefficient anyways, because every click
reloaded all Datas first, before acting on a single one only.

Inefficient in terms of database hits - yes. Inefficient in terms of
serialization cost/memory usage - no. As the table is fairly small
(way less than one million rows) and our application has a very low
number of concurrent users, I'm not too concerned with database query
performance.

You should use a detachable model instead, see JpaLoadableModel here
https://ci.apache.org/projects/wicket/guide/8.x/single.html#_detachable_models

As we're already using a LoadableDetachableModel for the repeater
itself, you probably meant I should override
ListView#getListItemModel() and return a LoadableDetachableModel
there, right ?
Cheers,
Tobi

[Andrea Del Bene]

Hi,

you might consider to keep just the object model id in your button in
order to avoid capturing the entire object in your component.

Post by Tobias Gierke
I've accidently sent my follow-up mail directly to Sven, here it is
(and his reply) for the sake of completeness, just in case someone
else stumbles across this as well.
Cheers,
Tobias

Hi Tobias,

As we're already using a LoadableDetachableModel for the repeater

itself, you probably meant I should override
ListView#getListItemModel() and return a LoadableDetachableModel
there, right ?
+1 exactly.
Sven
Hi Sven,
Thanks for your reply !

Hi,
your first solution was inefficient anyways, because every click
reloaded all Datas first, before acting on a single one only.

Inefficient in terms of database hits - yes. Inefficient in terms of
serialization cost/memory usage - no. As the table is fairly small
(way less than one million rows) and our application has a very low
number of concurrent users, I'm not too concerned with database query
performance.

You should use a detachable model instead, see JpaLoadableModel here
https://ci.apache.org/projects/wicket/guide/8.x/single.html#_detachable_models

As we're already using a LoadableDetachableModel for the repeater
itself, you probably meant I should override
ListView#getListItemModel() and return a LoadableDetachableModel
there, right ?
Cheers,
Tobi

---------------------------------------------------------------------
To unsubscribe, e-mail: users-***@wicket.apache.org
For additional commands, e-mail: users-***@wicket.apache.org

[Igor Vaynberg]

Hi,
A collegue of mine just came across a rather interesting bug in our Wicket application.
1. We have a simple page with a repeater (ListView) that displays a table and on each row, some buttons to perform actions on the item shown on this row (edit/delete/etc.)
2. The underlying data source (a database table) gets updated concurrently by another process running on another machine
3. The table is supposed to always show the latest data at the very top, so the page uses a LoadableDetachableModel to always hit the database on every request
Users complained that performing actions on the data items would actually affect an item different from what they clicked.
Since the list model got detached at the end of the previous request, clicking any of the action buttons would re-populate the data model, fetching previously unseen rows from the database. Since (according to my collegue,didn't double-check) the ListView associates the item models only based on their list index, the action button on the very first row now all of a sudden referred to a database row the user didn't even know about.

This is exactly why ListViews should not be used to work with database data unless you override getListItemModel() to return a model to represent the item itself like Sven mentioned in his reply.

Here are three different ways to fix your problem from worst to best:

1. add(new AjaxButton( "delete , new EntityModel(item.getModelObject()))
Where EntityModel knows how to load the entity from the database - ie the jpa model sven mentioned.

2. New ListView<….> {
getListItemModel(imodel list, int index) { return new EntityModel(list.getobject().get(index)); }
This is the same as above but has the advantage of item.getmodel() returning the better model

3. Use a RefreshingView or a DataView instead.

-Igor

add(new AjaxButton( "delete , item.getModelObject() )
Instead of
view = new ListView<Data>("listView" , dataProvider )
{
@Override
protected void populateItem(ListItem<PCAPFile> item)
{
add(new AjaxButton( "delete , item.getModel() ) // use model from ListItem (model gets detached after request)
{
public void onClick(AjaxRequestTarget target) {
delete( getModelObject() );
}
});
// ... more stuff
}
}
view = new ListView<Data>("listView" , dataProvider )
{
@Override
protected void populateItem(ListItem<PCAPFile> item)
{
add(new AjaxButton( "delete , item.getModelObject() ) // capture model object when constructing the button
{
public void onClick(AjaxRequestTarget target) {
delete( getModelObject() );
}
});
// ... more stuff
}
}
This obviously is a rather subtle issue and - depending on the size of your model objects - also comes with a certain performance/memory cost because of the additional serialization for the model items the repeater components are now holding onto.
Is this the recommended approach for dealing with dynamically changing data or is there a better way to do it ?
Thanks,
Tobias

---------------------------------------------------------------------
To unsubscribe, e-mail: users-***@wicket.apache.org
For additional commands, e-mail: users-***@wicket.apache.org