Re: Repeaters, dynamic data & detaching models
> On Aug 27, 2018, at 6:00 AM, Tobias Gierke
> wrote:
>
> 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.
>
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() )
> His fix:
>
> Instead of
>
> view = new ListView("listView" , dataProvider )
> {
>@Override
>protected void populateItem(ListItem 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("listView" , dataProvider )
> {
>@Override
>protected void populateItem(ListItem 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-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
Re: Repeaters, dynamic data & detaching models
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. On 28/08/2018 10:32, Tobias Gierke wrote: 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 Am 28.08.2018 um 09:44 schrieb Tobias Gierke: 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-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org
Re: Repeaters, dynamic data & detaching models
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 Am 28.08.2018 um 09:44 schrieb Tobias Gierke: 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
Re: Repeaters, dynamic data & detaching models
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
Am 27.08.2018 um 15:00 schrieb 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("listView" , dataProvider )
{
@Override
protected void populateItem(ListItem 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("listView" , dataProvider )
{
@Override
protected void populateItem(ListItem 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-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org
Repeaters, dynamic data & detaching models
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("listView" , dataProvider )
{
@Override
protected void populateItem(ListItem 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("listView" , dataProvider )
{
@Override
protected void populateItem(ListItem 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
