You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Tobias Gierke <to...@code-sourcery.de> on 2018/08/27 13:00:39 UTC
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<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
Re: Repeaters, dynamic data & detaching models
Posted by Igor Vaynberg <ig...@gmail.com>.
> On Aug 27, 2018, at 6:00 AM, Tobias Gierke <to...@code-sourcery.de> 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<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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: Repeaters, dynamic data & detaching models
Posted by Andrea Del Bene <an...@gmail.com>.
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-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: Repeaters, dynamic data & detaching models
Posted by Tobias Gierke <to...@code-sourcery.de>.
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
Posted by Sven Meier <sv...@meiers.net>.
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<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
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org