You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Jonathan Tougas <jt...@gmail.com> on 2012/02/15 18:31:41 UTC
AbstractPageableView cachedItemCount
I noticed two count queries go by when using the DataTable component. so I
searched and dug up this jira issue
https://issues.apache.org/jira/browse/WICKET-1766 which is a "won't fix".
Igor states that two queries are required each request, but I see this
differently:
The count is a used as the basis for the paging navigator's isVisible(), so
far so good. The issue is that the count is discarded in onDetach() (as
well as readObject()). It's state and as such should not be discarded when
the request is finished, it's still needed for things like checking if a
link was indeed visible when a click comes in for it. If it's not kept, a
new query to the model will be made, which might return a different result
- consequences ensue. The critical part of that is we are checking if the
link *was* visible, not if it *is* visible.
I think the only time it should be discarded is in the onBeforeRender()
event. This is when we are actually interested in going back to the model
to see if the value has changed. So to me this is indeed a bug. I don't
mind patching something up myself, or reopening the ticket...but I would
like a confirmation that I'm not way out in left field ;)
Cheers!
Jonathan Tougas
Re: AbstractPageableView cachedItemCount
Posted by Dan Retzlaff <dr...@gmail.com>.
I understand your suggestion. But if the page to which the link refers no
longer exists based on the new data content, isn't it a bad idea to go
there?
I feel like I'm drawing this out. Sorry for that. :)
On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas <jt...@gmail.com> wrote:
> The cachedItemCount calculated in onBeforeRender should not be discarded at
> the end of a request (so the clear in onDetach and readObject shouldn't be
> there). This way it would still be around when a request comes in to handle
> a click.
>
> On Wed, Feb 15, 2012 at 5:19 PM, Dan Retzlaff <dr...@gmail.com> wrote:
>
> > Thanks for the clarification. I see your point now: if records are
> deleted
> > from the database, the navigation click is ignored an the page is simply
> > re-rendered. But if the data content has changed such that the navigation
> > no longer makes sense, what behavior would you prefer?
> >
> > On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com>
> > wrote:
> >
> > > The PagingNavigationIncrementLink's linksTo(Page), which calls isLast()
> > > which calls pageable getPageCount() which ends up calling size()
> > > eventually. This is called during Component.canCallListenerInterface
> > > (*you're
> > > right it's not isVisible*) to verify if the link can indeed be clicked.
> > >
> > > And to be clear I am discussing multiple size() calls in one request.
> It
> > > happens when clicking on the navigation links: size() is called first
> as
> > > part of the verifying if the link is enabled (as described above), then
> > the
> > > cached value is discarded just before rendering (in onBeforeRender()).
> > Then
> > > size() is called again as part of the rendering, and again cached. The
> > > cached value is again discarded at the end of the request in
> onDetach().
> > > What I'm saying is the the first size() shouldn't occur because the
> page
> > > count should be cached from the previous rendering (it shouldn't be
> > cleared
> > > in onDetach() nor readObject()).
> > >
> > > On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com>
> > wrote:
> > >
> > > > Hi, Jonathan. Which component are you referring to? I don't see
> > > isVisible()
> > > > overrides in PagingNavigator or its helpers.
> > > >
> > > >
> > > > > It's state and as such should not be discarded when
> > > > > the request is finished, it's still needed for things like checking
> > if
> > > a
> > > > > link was indeed visible when a click comes in for it.
> > > >
> > > >
> > > > How can you receive a click event for a link that was not visible?
> > > > Invisible components aren't rendered.
> > > >
> > > > That JIRA discusses multiple size() calls in a single request. You're
> > > > discussing multiple size() calls with multiple requests. Right?
> > > >
> > > > Dan
> > > >
> > > > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jt...@gmail.com>
> > > > wrote:
> > > >
> > > > > I noticed two count queries go by when using the DataTable
> component.
> > > so
> > > > I
> > > > > searched and dug up this jira issue
> > > > > https://issues.apache.org/jira/browse/WICKET-1766 which is a
> "won't
> > > > fix".
> > > > >
> > > > > Igor states that two queries are required each request, but I see
> > this
> > > > > differently:
> > > > >
> > > > > The count is a used as the basis for the paging navigator's
> > > isVisible(),
> > > > so
> > > > > far so good. The issue is that the count is discarded in onDetach()
> > (as
> > > > > well as readObject()). It's state and as such should not be
> discarded
> > > > when
> > > > > the request is finished, it's still needed for things like checking
> > if
> > > a
> > > > > link was indeed visible when a click comes in for it. If it's not
> > > kept, a
> > > > > new query to the model will be made, which might return a different
> > > > result
> > > > > - consequences ensue. The critical part of that is we are checking
> if
> > > the
> > > > > link *was* visible, not if it *is* visible.
> > > > >
> > > > > I think the only time it should be discarded is in the
> > onBeforeRender()
> > > > > event. This is when we are actually interested in going back to the
> > > model
> > > > > to see if the value has changed. So to me this is indeed a bug. I
> > don't
> > > > > mind patching something up myself, or reopening the ticket...but I
> > > would
> > > > > like a confirmation that I'm not way out in left field ;)
> > > > >
> > > > > Cheers!
> > > > > Jonathan Tougas
> > > > >
> > > >
> > >
> >
>
Re: AbstractPageableView cachedItemCount
Posted by Jonathan Tougas <jt...@gmail.com>.
I see your point, the order the components are rendered is important.
I didn't have any trouble getting this to work though: I have a
UserListPage, which has a SizeLabel (whose getModel() =
datatable.getgetItemCount()), and a UserDataTable. The UserListPage listens
for ajax events to add the SizeLabel to AjaxRequestTarget when appropriate.
The components in the AjaxRequestTarget are ordered, and the datatable is
added first since it is the target of the ajax request, so it's
onBeforeRender is called before the label's. The label then correctly picks
up the new size. This case works, more involves ones might not...
So the deeper reason why the rendering order is important is that the
datatable's model is refreshed as a consequence of rendering. It should
probably be done before the rendering even starts, either as part of the
page load, or a click handler. This way, when the rendering phase comes
around, the data is available, and not dependent on the rendering order of
the components.
If ever I run into the problem where the rendering order is problematic
I'll probably try writing up a datatable that can be explicitly refreshed
like this. For now I'm good :)
Thanks for the help! (and Igor thank you for replying to so many user
questions!)
On Thu, Feb 16, 2012 at 12:26 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> suppose you have a label before the data table that shows how many
> items are in the table. it uses datatable.getitemcount() to do this.
>
> onbeforerender() will be called on the label before it is on the
> datatable so it now uses the stale item count and is out of sync with
> the datatable.
>
> -igor
>
> On Thu, Feb 16, 2012 at 9:08 AM, Jonathan Tougas <jt...@gmail.com>
> wrote:
> > It should be discarded only before rendering.
> >
> > I figured out a way to accomplish this by extending the DataTable class
> > and creating a wrapper for the data provider with a cache of it's own,
> > which bypasses the AbstractPageableView's size cache. This one is cleared
> > by the extension of DataTable before rendering like I'm suggesting:
> >
> > public class SuperTable<T> extends DataTable<T> {
> >
> > private SuperDataProviderWrapper<T> dataProviderWrapper;
> > public SuperTable( String id, List<IColumn<T>> columns,
> > SuperDataProviderWrapper<T> dataProviderWrapper, int rowsPerPage ) {
> > super( id, columns, dataProviderWrapper, rowsPerPage );
> > this.dataProviderWrapper = dataProviderWrapper;
> > setOutputMarkupId( true );
> > setVersioned( false );
> > addTopToolbar( new AjaxNavigationToolbar( this ) );
> > addTopToolbar( new AjaxFallbackHeadersToolbar( this,
> dataProviderWrapper )
> > );
> > addBottomToolbar( new NoRecordsToolbar( this ) );
> > }
> >
> > @Override
> > protected Item<T> newRowItem( final String id, final int index, final
> > IModel<T> model ) {
> > return new OddEvenItem<T>( id, index, model );
> > }
> > @Override
> > protected void onBeforeRender() {
> > // reset size before rendering!
> > dataProviderWrapper.resetSize();
> > super.onBeforeRender();
> > }
> > }
> >
> > public class SuperDataProviderWrapper<T> implements
> > ISortableDataProvider<T> {
> >
> > private ISortableDataProvider<T> delegate;
> > private int size;
> > public SuperDataProviderWrapper( ISortableDataProvider<T> delegate ) {
> > this.delegate = delegate;
> > resetSize();
> > }
> >
> > @Override
> > public int size() {
> > if( size == -1 ) {
> > size = delegate.size();
> > }
> > return size;
> > }
> >
> > public void resetSize() {
> > size = -1;
> > }
> > /*snip delegations...*/
> > }
> >
> > End result is one call to count when the ajax links are clicked instead
> of
> > two.
> >
> > On Wed, Feb 15, 2012 at 7:33 PM, Igor Vaynberg <igor.vaynberg@gmail.com
> >wrote:
> >
> >> so when should it be discarded?
> >>
> >> -igor
> >>
> >> On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas <jt...@gmail.com>
> >> wrote:
> >> > The cachedItemCount calculated in onBeforeRender should not be
> discarded
> >> at
> >> > the end of a request (so the clear in onDetach and readObject
> shouldn't
> >> be
> >> > there). This way it would still be around when a request comes in to
> >> handle
> >> > a click.
> >> >
> >> > On Wed, Feb 15, 2012 at 5:19 PM, Dan Retzlaff <dr...@gmail.com>
> >> wrote:
> >> >
> >> >> Thanks for the clarification. I see your point now: if records are
> >> deleted
> >> >> from the database, the navigation click is ignored an the page is
> simply
> >> >> re-rendered. But if the data content has changed such that the
> >> navigation
> >> >> no longer makes sense, what behavior would you prefer?
> >> >>
> >> >> On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com>
> >> >> wrote:
> >> >>
> >> >> > The PagingNavigationIncrementLink's linksTo(Page), which calls
> >> isLast()
> >> >> > which calls pageable getPageCount() which ends up calling size()
> >> >> > eventually. This is called during
> Component.canCallListenerInterface
> >> >> > (*you're
> >> >> > right it's not isVisible*) to verify if the link can indeed be
> >> clicked.
> >> >> >
> >> >> > And to be clear I am discussing multiple size() calls in one
> request.
> >> It
> >> >> > happens when clicking on the navigation links: size() is called
> first
> >> as
> >> >> > part of the verifying if the link is enabled (as described above),
> >> then
> >> >> the
> >> >> > cached value is discarded just before rendering (in
> onBeforeRender()).
> >> >> Then
> >> >> > size() is called again as part of the rendering, and again cached.
> The
> >> >> > cached value is again discarded at the end of the request in
> >> onDetach().
> >> >> > What I'm saying is the the first size() shouldn't occur because the
> >> page
> >> >> > count should be cached from the previous rendering (it shouldn't be
> >> >> cleared
> >> >> > in onDetach() nor readObject()).
> >> >> >
> >> >> > On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dretzlaff@gmail.com
> >
> >> >> wrote:
> >> >> >
> >> >> > > Hi, Jonathan. Which component are you referring to? I don't see
> >> >> > isVisible()
> >> >> > > overrides in PagingNavigator or its helpers.
> >> >> > >
> >> >> > >
> >> >> > > > It's state and as such should not be discarded when
> >> >> > > > the request is finished, it's still needed for things like
> >> checking
> >> >> if
> >> >> > a
> >> >> > > > link was indeed visible when a click comes in for it.
> >> >> > >
> >> >> > >
> >> >> > > How can you receive a click event for a link that was not
> visible?
> >> >> > > Invisible components aren't rendered.
> >> >> > >
> >> >> > > That JIRA discusses multiple size() calls in a single request.
> >> You're
> >> >> > > discussing multiple size() calls with multiple requests. Right?
> >> >> > >
> >> >> > > Dan
> >> >> > >
> >> >> > > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <
> jtougas@gmail.com
> >> >
> >> >> > > wrote:
> >> >> > >
> >> >> > > > I noticed two count queries go by when using the DataTable
> >> component.
> >> >> > so
> >> >> > > I
> >> >> > > > searched and dug up this jira issue
> >> >> > > > https://issues.apache.org/jira/browse/WICKET-1766 which is a
> >> "won't
> >> >> > > fix".
> >> >> > > >
> >> >> > > > Igor states that two queries are required each request, but I
> see
> >> >> this
> >> >> > > > differently:
> >> >> > > >
> >> >> > > > The count is a used as the basis for the paging navigator's
> >> >> > isVisible(),
> >> >> > > so
> >> >> > > > far so good. The issue is that the count is discarded in
> >> onDetach()
> >> >> (as
> >> >> > > > well as readObject()). It's state and as such should not be
> >> discarded
> >> >> > > when
> >> >> > > > the request is finished, it's still needed for things like
> >> checking
> >> >> if
> >> >> > a
> >> >> > > > link was indeed visible when a click comes in for it. If it's
> not
> >> >> > kept, a
> >> >> > > > new query to the model will be made, which might return a
> >> different
> >> >> > > result
> >> >> > > > - consequences ensue. The critical part of that is we are
> >> checking if
> >> >> > the
> >> >> > > > link *was* visible, not if it *is* visible.
> >> >> > > >
> >> >> > > > I think the only time it should be discarded is in the
> >> >> onBeforeRender()
> >> >> > > > event. This is when we are actually interested in going back to
> >> the
> >> >> > model
> >> >> > > > to see if the value has changed. So to me this is indeed a
> bug. I
> >> >> don't
> >> >> > > > mind patching something up myself, or reopening the
> ticket...but I
> >> >> > would
> >> >> > > > like a confirmation that I'm not way out in left field ;)
> >> >> > > >
> >> >> > > > Cheers!
> >> >> > > > Jonathan Tougas
> >> >> > > >
> >> >> > >
> >> >> >
> >> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> >> For additional commands, e-mail: users-help@wicket.apache.org
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>
Re: AbstractPageableView cachedItemCount
Posted by Igor Vaynberg <ig...@gmail.com>.
suppose you have a label before the data table that shows how many
items are in the table. it uses datatable.getitemcount() to do this.
onbeforerender() will be called on the label before it is on the
datatable so it now uses the stale item count and is out of sync with
the datatable.
-igor
On Thu, Feb 16, 2012 at 9:08 AM, Jonathan Tougas <jt...@gmail.com> wrote:
> It should be discarded only before rendering.
>
> I figured out a way to accomplish this by extending the DataTable class
> and creating a wrapper for the data provider with a cache of it's own,
> which bypasses the AbstractPageableView's size cache. This one is cleared
> by the extension of DataTable before rendering like I'm suggesting:
>
> public class SuperTable<T> extends DataTable<T> {
>
> private SuperDataProviderWrapper<T> dataProviderWrapper;
> public SuperTable( String id, List<IColumn<T>> columns,
> SuperDataProviderWrapper<T> dataProviderWrapper, int rowsPerPage ) {
> super( id, columns, dataProviderWrapper, rowsPerPage );
> this.dataProviderWrapper = dataProviderWrapper;
> setOutputMarkupId( true );
> setVersioned( false );
> addTopToolbar( new AjaxNavigationToolbar( this ) );
> addTopToolbar( new AjaxFallbackHeadersToolbar( this, dataProviderWrapper )
> );
> addBottomToolbar( new NoRecordsToolbar( this ) );
> }
>
> @Override
> protected Item<T> newRowItem( final String id, final int index, final
> IModel<T> model ) {
> return new OddEvenItem<T>( id, index, model );
> }
> @Override
> protected void onBeforeRender() {
> // reset size before rendering!
> dataProviderWrapper.resetSize();
> super.onBeforeRender();
> }
> }
>
> public class SuperDataProviderWrapper<T> implements
> ISortableDataProvider<T> {
>
> private ISortableDataProvider<T> delegate;
> private int size;
> public SuperDataProviderWrapper( ISortableDataProvider<T> delegate ) {
> this.delegate = delegate;
> resetSize();
> }
>
> @Override
> public int size() {
> if( size == -1 ) {
> size = delegate.size();
> }
> return size;
> }
>
> public void resetSize() {
> size = -1;
> }
> /*snip delegations...*/
> }
>
> End result is one call to count when the ajax links are clicked instead of
> two.
>
> On Wed, Feb 15, 2012 at 7:33 PM, Igor Vaynberg <ig...@gmail.com>wrote:
>
>> so when should it be discarded?
>>
>> -igor
>>
>> On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas <jt...@gmail.com>
>> wrote:
>> > The cachedItemCount calculated in onBeforeRender should not be discarded
>> at
>> > the end of a request (so the clear in onDetach and readObject shouldn't
>> be
>> > there). This way it would still be around when a request comes in to
>> handle
>> > a click.
>> >
>> > On Wed, Feb 15, 2012 at 5:19 PM, Dan Retzlaff <dr...@gmail.com>
>> wrote:
>> >
>> >> Thanks for the clarification. I see your point now: if records are
>> deleted
>> >> from the database, the navigation click is ignored an the page is simply
>> >> re-rendered. But if the data content has changed such that the
>> navigation
>> >> no longer makes sense, what behavior would you prefer?
>> >>
>> >> On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com>
>> >> wrote:
>> >>
>> >> > The PagingNavigationIncrementLink's linksTo(Page), which calls
>> isLast()
>> >> > which calls pageable getPageCount() which ends up calling size()
>> >> > eventually. This is called during Component.canCallListenerInterface
>> >> > (*you're
>> >> > right it's not isVisible*) to verify if the link can indeed be
>> clicked.
>> >> >
>> >> > And to be clear I am discussing multiple size() calls in one request.
>> It
>> >> > happens when clicking on the navigation links: size() is called first
>> as
>> >> > part of the verifying if the link is enabled (as described above),
>> then
>> >> the
>> >> > cached value is discarded just before rendering (in onBeforeRender()).
>> >> Then
>> >> > size() is called again as part of the rendering, and again cached. The
>> >> > cached value is again discarded at the end of the request in
>> onDetach().
>> >> > What I'm saying is the the first size() shouldn't occur because the
>> page
>> >> > count should be cached from the previous rendering (it shouldn't be
>> >> cleared
>> >> > in onDetach() nor readObject()).
>> >> >
>> >> > On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com>
>> >> wrote:
>> >> >
>> >> > > Hi, Jonathan. Which component are you referring to? I don't see
>> >> > isVisible()
>> >> > > overrides in PagingNavigator or its helpers.
>> >> > >
>> >> > >
>> >> > > > It's state and as such should not be discarded when
>> >> > > > the request is finished, it's still needed for things like
>> checking
>> >> if
>> >> > a
>> >> > > > link was indeed visible when a click comes in for it.
>> >> > >
>> >> > >
>> >> > > How can you receive a click event for a link that was not visible?
>> >> > > Invisible components aren't rendered.
>> >> > >
>> >> > > That JIRA discusses multiple size() calls in a single request.
>> You're
>> >> > > discussing multiple size() calls with multiple requests. Right?
>> >> > >
>> >> > > Dan
>> >> > >
>> >> > > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jtougas@gmail.com
>> >
>> >> > > wrote:
>> >> > >
>> >> > > > I noticed two count queries go by when using the DataTable
>> component.
>> >> > so
>> >> > > I
>> >> > > > searched and dug up this jira issue
>> >> > > > https://issues.apache.org/jira/browse/WICKET-1766 which is a
>> "won't
>> >> > > fix".
>> >> > > >
>> >> > > > Igor states that two queries are required each request, but I see
>> >> this
>> >> > > > differently:
>> >> > > >
>> >> > > > The count is a used as the basis for the paging navigator's
>> >> > isVisible(),
>> >> > > so
>> >> > > > far so good. The issue is that the count is discarded in
>> onDetach()
>> >> (as
>> >> > > > well as readObject()). It's state and as such should not be
>> discarded
>> >> > > when
>> >> > > > the request is finished, it's still needed for things like
>> checking
>> >> if
>> >> > a
>> >> > > > link was indeed visible when a click comes in for it. If it's not
>> >> > kept, a
>> >> > > > new query to the model will be made, which might return a
>> different
>> >> > > result
>> >> > > > - consequences ensue. The critical part of that is we are
>> checking if
>> >> > the
>> >> > > > link *was* visible, not if it *is* visible.
>> >> > > >
>> >> > > > I think the only time it should be discarded is in the
>> >> onBeforeRender()
>> >> > > > event. This is when we are actually interested in going back to
>> the
>> >> > model
>> >> > > > to see if the value has changed. So to me this is indeed a bug. I
>> >> don't
>> >> > > > mind patching something up myself, or reopening the ticket...but I
>> >> > would
>> >> > > > like a confirmation that I'm not way out in left field ;)
>> >> > > >
>> >> > > > Cheers!
>> >> > > > Jonathan Tougas
>> >> > > >
>> >> > >
>> >> >
>> >>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
>> For additional commands, e-mail: users-help@wicket.apache.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: AbstractPageableView cachedItemCount
Posted by Jonathan Tougas <jt...@gmail.com>.
It should be discarded only before rendering.
I figured out a way to accomplish this by extending the DataTable class
and creating a wrapper for the data provider with a cache of it's own,
which bypasses the AbstractPageableView's size cache. This one is cleared
by the extension of DataTable before rendering like I'm suggesting:
public class SuperTable<T> extends DataTable<T> {
private SuperDataProviderWrapper<T> dataProviderWrapper;
public SuperTable( String id, List<IColumn<T>> columns,
SuperDataProviderWrapper<T> dataProviderWrapper, int rowsPerPage ) {
super( id, columns, dataProviderWrapper, rowsPerPage );
this.dataProviderWrapper = dataProviderWrapper;
setOutputMarkupId( true );
setVersioned( false );
addTopToolbar( new AjaxNavigationToolbar( this ) );
addTopToolbar( new AjaxFallbackHeadersToolbar( this, dataProviderWrapper )
);
addBottomToolbar( new NoRecordsToolbar( this ) );
}
@Override
protected Item<T> newRowItem( final String id, final int index, final
IModel<T> model ) {
return new OddEvenItem<T>( id, index, model );
}
@Override
protected void onBeforeRender() {
// reset size before rendering!
dataProviderWrapper.resetSize();
super.onBeforeRender();
}
}
public class SuperDataProviderWrapper<T> implements
ISortableDataProvider<T> {
private ISortableDataProvider<T> delegate;
private int size;
public SuperDataProviderWrapper( ISortableDataProvider<T> delegate ) {
this.delegate = delegate;
resetSize();
}
@Override
public int size() {
if( size == -1 ) {
size = delegate.size();
}
return size;
}
public void resetSize() {
size = -1;
}
/*snip delegations...*/
}
End result is one call to count when the ajax links are clicked instead of
two.
On Wed, Feb 15, 2012 at 7:33 PM, Igor Vaynberg <ig...@gmail.com>wrote:
> so when should it be discarded?
>
> -igor
>
> On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas <jt...@gmail.com>
> wrote:
> > The cachedItemCount calculated in onBeforeRender should not be discarded
> at
> > the end of a request (so the clear in onDetach and readObject shouldn't
> be
> > there). This way it would still be around when a request comes in to
> handle
> > a click.
> >
> > On Wed, Feb 15, 2012 at 5:19 PM, Dan Retzlaff <dr...@gmail.com>
> wrote:
> >
> >> Thanks for the clarification. I see your point now: if records are
> deleted
> >> from the database, the navigation click is ignored an the page is simply
> >> re-rendered. But if the data content has changed such that the
> navigation
> >> no longer makes sense, what behavior would you prefer?
> >>
> >> On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com>
> >> wrote:
> >>
> >> > The PagingNavigationIncrementLink's linksTo(Page), which calls
> isLast()
> >> > which calls pageable getPageCount() which ends up calling size()
> >> > eventually. This is called during Component.canCallListenerInterface
> >> > (*you're
> >> > right it's not isVisible*) to verify if the link can indeed be
> clicked.
> >> >
> >> > And to be clear I am discussing multiple size() calls in one request.
> It
> >> > happens when clicking on the navigation links: size() is called first
> as
> >> > part of the verifying if the link is enabled (as described above),
> then
> >> the
> >> > cached value is discarded just before rendering (in onBeforeRender()).
> >> Then
> >> > size() is called again as part of the rendering, and again cached. The
> >> > cached value is again discarded at the end of the request in
> onDetach().
> >> > What I'm saying is the the first size() shouldn't occur because the
> page
> >> > count should be cached from the previous rendering (it shouldn't be
> >> cleared
> >> > in onDetach() nor readObject()).
> >> >
> >> > On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com>
> >> wrote:
> >> >
> >> > > Hi, Jonathan. Which component are you referring to? I don't see
> >> > isVisible()
> >> > > overrides in PagingNavigator or its helpers.
> >> > >
> >> > >
> >> > > > It's state and as such should not be discarded when
> >> > > > the request is finished, it's still needed for things like
> checking
> >> if
> >> > a
> >> > > > link was indeed visible when a click comes in for it.
> >> > >
> >> > >
> >> > > How can you receive a click event for a link that was not visible?
> >> > > Invisible components aren't rendered.
> >> > >
> >> > > That JIRA discusses multiple size() calls in a single request.
> You're
> >> > > discussing multiple size() calls with multiple requests. Right?
> >> > >
> >> > > Dan
> >> > >
> >> > > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jtougas@gmail.com
> >
> >> > > wrote:
> >> > >
> >> > > > I noticed two count queries go by when using the DataTable
> component.
> >> > so
> >> > > I
> >> > > > searched and dug up this jira issue
> >> > > > https://issues.apache.org/jira/browse/WICKET-1766 which is a
> "won't
> >> > > fix".
> >> > > >
> >> > > > Igor states that two queries are required each request, but I see
> >> this
> >> > > > differently:
> >> > > >
> >> > > > The count is a used as the basis for the paging navigator's
> >> > isVisible(),
> >> > > so
> >> > > > far so good. The issue is that the count is discarded in
> onDetach()
> >> (as
> >> > > > well as readObject()). It's state and as such should not be
> discarded
> >> > > when
> >> > > > the request is finished, it's still needed for things like
> checking
> >> if
> >> > a
> >> > > > link was indeed visible when a click comes in for it. If it's not
> >> > kept, a
> >> > > > new query to the model will be made, which might return a
> different
> >> > > result
> >> > > > - consequences ensue. The critical part of that is we are
> checking if
> >> > the
> >> > > > link *was* visible, not if it *is* visible.
> >> > > >
> >> > > > I think the only time it should be discarded is in the
> >> onBeforeRender()
> >> > > > event. This is when we are actually interested in going back to
> the
> >> > model
> >> > > > to see if the value has changed. So to me this is indeed a bug. I
> >> don't
> >> > > > mind patching something up myself, or reopening the ticket...but I
> >> > would
> >> > > > like a confirmation that I'm not way out in left field ;)
> >> > > >
> >> > > > Cheers!
> >> > > > Jonathan Tougas
> >> > > >
> >> > >
> >> >
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>
Re: AbstractPageableView cachedItemCount
Posted by Igor Vaynberg <ig...@gmail.com>.
so when should it be discarded?
-igor
On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas <jt...@gmail.com> wrote:
> The cachedItemCount calculated in onBeforeRender should not be discarded at
> the end of a request (so the clear in onDetach and readObject shouldn't be
> there). This way it would still be around when a request comes in to handle
> a click.
>
> On Wed, Feb 15, 2012 at 5:19 PM, Dan Retzlaff <dr...@gmail.com> wrote:
>
>> Thanks for the clarification. I see your point now: if records are deleted
>> from the database, the navigation click is ignored an the page is simply
>> re-rendered. But if the data content has changed such that the navigation
>> no longer makes sense, what behavior would you prefer?
>>
>> On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com>
>> wrote:
>>
>> > The PagingNavigationIncrementLink's linksTo(Page), which calls isLast()
>> > which calls pageable getPageCount() which ends up calling size()
>> > eventually. This is called during Component.canCallListenerInterface
>> > (*you're
>> > right it's not isVisible*) to verify if the link can indeed be clicked.
>> >
>> > And to be clear I am discussing multiple size() calls in one request. It
>> > happens when clicking on the navigation links: size() is called first as
>> > part of the verifying if the link is enabled (as described above), then
>> the
>> > cached value is discarded just before rendering (in onBeforeRender()).
>> Then
>> > size() is called again as part of the rendering, and again cached. The
>> > cached value is again discarded at the end of the request in onDetach().
>> > What I'm saying is the the first size() shouldn't occur because the page
>> > count should be cached from the previous rendering (it shouldn't be
>> cleared
>> > in onDetach() nor readObject()).
>> >
>> > On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com>
>> wrote:
>> >
>> > > Hi, Jonathan. Which component are you referring to? I don't see
>> > isVisible()
>> > > overrides in PagingNavigator or its helpers.
>> > >
>> > >
>> > > > It's state and as such should not be discarded when
>> > > > the request is finished, it's still needed for things like checking
>> if
>> > a
>> > > > link was indeed visible when a click comes in for it.
>> > >
>> > >
>> > > How can you receive a click event for a link that was not visible?
>> > > Invisible components aren't rendered.
>> > >
>> > > That JIRA discusses multiple size() calls in a single request. You're
>> > > discussing multiple size() calls with multiple requests. Right?
>> > >
>> > > Dan
>> > >
>> > > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jt...@gmail.com>
>> > > wrote:
>> > >
>> > > > I noticed two count queries go by when using the DataTable component.
>> > so
>> > > I
>> > > > searched and dug up this jira issue
>> > > > https://issues.apache.org/jira/browse/WICKET-1766 which is a "won't
>> > > fix".
>> > > >
>> > > > Igor states that two queries are required each request, but I see
>> this
>> > > > differently:
>> > > >
>> > > > The count is a used as the basis for the paging navigator's
>> > isVisible(),
>> > > so
>> > > > far so good. The issue is that the count is discarded in onDetach()
>> (as
>> > > > well as readObject()). It's state and as such should not be discarded
>> > > when
>> > > > the request is finished, it's still needed for things like checking
>> if
>> > a
>> > > > link was indeed visible when a click comes in for it. If it's not
>> > kept, a
>> > > > new query to the model will be made, which might return a different
>> > > result
>> > > > - consequences ensue. The critical part of that is we are checking if
>> > the
>> > > > link *was* visible, not if it *is* visible.
>> > > >
>> > > > I think the only time it should be discarded is in the
>> onBeforeRender()
>> > > > event. This is when we are actually interested in going back to the
>> > model
>> > > > to see if the value has changed. So to me this is indeed a bug. I
>> don't
>> > > > mind patching something up myself, or reopening the ticket...but I
>> > would
>> > > > like a confirmation that I'm not way out in left field ;)
>> > > >
>> > > > Cheers!
>> > > > Jonathan Tougas
>> > > >
>> > >
>> >
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: AbstractPageableView cachedItemCount
Posted by Jonathan Tougas <jt...@gmail.com>.
The cachedItemCount calculated in onBeforeRender should not be discarded at
the end of a request (so the clear in onDetach and readObject shouldn't be
there). This way it would still be around when a request comes in to handle
a click.
On Wed, Feb 15, 2012 at 5:19 PM, Dan Retzlaff <dr...@gmail.com> wrote:
> Thanks for the clarification. I see your point now: if records are deleted
> from the database, the navigation click is ignored an the page is simply
> re-rendered. But if the data content has changed such that the navigation
> no longer makes sense, what behavior would you prefer?
>
> On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com>
> wrote:
>
> > The PagingNavigationIncrementLink's linksTo(Page), which calls isLast()
> > which calls pageable getPageCount() which ends up calling size()
> > eventually. This is called during Component.canCallListenerInterface
> > (*you're
> > right it's not isVisible*) to verify if the link can indeed be clicked.
> >
> > And to be clear I am discussing multiple size() calls in one request. It
> > happens when clicking on the navigation links: size() is called first as
> > part of the verifying if the link is enabled (as described above), then
> the
> > cached value is discarded just before rendering (in onBeforeRender()).
> Then
> > size() is called again as part of the rendering, and again cached. The
> > cached value is again discarded at the end of the request in onDetach().
> > What I'm saying is the the first size() shouldn't occur because the page
> > count should be cached from the previous rendering (it shouldn't be
> cleared
> > in onDetach() nor readObject()).
> >
> > On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com>
> wrote:
> >
> > > Hi, Jonathan. Which component are you referring to? I don't see
> > isVisible()
> > > overrides in PagingNavigator or its helpers.
> > >
> > >
> > > > It's state and as such should not be discarded when
> > > > the request is finished, it's still needed for things like checking
> if
> > a
> > > > link was indeed visible when a click comes in for it.
> > >
> > >
> > > How can you receive a click event for a link that was not visible?
> > > Invisible components aren't rendered.
> > >
> > > That JIRA discusses multiple size() calls in a single request. You're
> > > discussing multiple size() calls with multiple requests. Right?
> > >
> > > Dan
> > >
> > > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jt...@gmail.com>
> > > wrote:
> > >
> > > > I noticed two count queries go by when using the DataTable component.
> > so
> > > I
> > > > searched and dug up this jira issue
> > > > https://issues.apache.org/jira/browse/WICKET-1766 which is a "won't
> > > fix".
> > > >
> > > > Igor states that two queries are required each request, but I see
> this
> > > > differently:
> > > >
> > > > The count is a used as the basis for the paging navigator's
> > isVisible(),
> > > so
> > > > far so good. The issue is that the count is discarded in onDetach()
> (as
> > > > well as readObject()). It's state and as such should not be discarded
> > > when
> > > > the request is finished, it's still needed for things like checking
> if
> > a
> > > > link was indeed visible when a click comes in for it. If it's not
> > kept, a
> > > > new query to the model will be made, which might return a different
> > > result
> > > > - consequences ensue. The critical part of that is we are checking if
> > the
> > > > link *was* visible, not if it *is* visible.
> > > >
> > > > I think the only time it should be discarded is in the
> onBeforeRender()
> > > > event. This is when we are actually interested in going back to the
> > model
> > > > to see if the value has changed. So to me this is indeed a bug. I
> don't
> > > > mind patching something up myself, or reopening the ticket...but I
> > would
> > > > like a confirmation that I'm not way out in left field ;)
> > > >
> > > > Cheers!
> > > > Jonathan Tougas
> > > >
> > >
> >
>
Re: AbstractPageableView cachedItemCount
Posted by Dan Retzlaff <dr...@gmail.com>.
Thanks for the clarification. I see your point now: if records are deleted
from the database, the navigation click is ignored an the page is simply
re-rendered. But if the data content has changed such that the navigation
no longer makes sense, what behavior would you prefer?
On Wed, Feb 15, 2012 at 1:37 PM, Jonathan Tougas <jt...@gmail.com> wrote:
> The PagingNavigationIncrementLink's linksTo(Page), which calls isLast()
> which calls pageable getPageCount() which ends up calling size()
> eventually. This is called during Component.canCallListenerInterface
> (*you're
> right it's not isVisible*) to verify if the link can indeed be clicked.
>
> And to be clear I am discussing multiple size() calls in one request. It
> happens when clicking on the navigation links: size() is called first as
> part of the verifying if the link is enabled (as described above), then the
> cached value is discarded just before rendering (in onBeforeRender()). Then
> size() is called again as part of the rendering, and again cached. The
> cached value is again discarded at the end of the request in onDetach().
> What I'm saying is the the first size() shouldn't occur because the page
> count should be cached from the previous rendering (it shouldn't be cleared
> in onDetach() nor readObject()).
>
> On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com> wrote:
>
> > Hi, Jonathan. Which component are you referring to? I don't see
> isVisible()
> > overrides in PagingNavigator or its helpers.
> >
> >
> > > It's state and as such should not be discarded when
> > > the request is finished, it's still needed for things like checking if
> a
> > > link was indeed visible when a click comes in for it.
> >
> >
> > How can you receive a click event for a link that was not visible?
> > Invisible components aren't rendered.
> >
> > That JIRA discusses multiple size() calls in a single request. You're
> > discussing multiple size() calls with multiple requests. Right?
> >
> > Dan
> >
> > On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jt...@gmail.com>
> > wrote:
> >
> > > I noticed two count queries go by when using the DataTable component.
> so
> > I
> > > searched and dug up this jira issue
> > > https://issues.apache.org/jira/browse/WICKET-1766 which is a "won't
> > fix".
> > >
> > > Igor states that two queries are required each request, but I see this
> > > differently:
> > >
> > > The count is a used as the basis for the paging navigator's
> isVisible(),
> > so
> > > far so good. The issue is that the count is discarded in onDetach() (as
> > > well as readObject()). It's state and as such should not be discarded
> > when
> > > the request is finished, it's still needed for things like checking if
> a
> > > link was indeed visible when a click comes in for it. If it's not
> kept, a
> > > new query to the model will be made, which might return a different
> > result
> > > - consequences ensue. The critical part of that is we are checking if
> the
> > > link *was* visible, not if it *is* visible.
> > >
> > > I think the only time it should be discarded is in the onBeforeRender()
> > > event. This is when we are actually interested in going back to the
> model
> > > to see if the value has changed. So to me this is indeed a bug. I don't
> > > mind patching something up myself, or reopening the ticket...but I
> would
> > > like a confirmation that I'm not way out in left field ;)
> > >
> > > Cheers!
> > > Jonathan Tougas
> > >
> >
>
Re: AbstractPageableView cachedItemCount
Posted by Jonathan Tougas <jt...@gmail.com>.
The PagingNavigationIncrementLink's linksTo(Page), which calls isLast()
which calls pageable getPageCount() which ends up calling size()
eventually. This is called during Component.canCallListenerInterface (*you're
right it's not isVisible*) to verify if the link can indeed be clicked.
And to be clear I am discussing multiple size() calls in one request. It
happens when clicking on the navigation links: size() is called first as
part of the verifying if the link is enabled (as described above), then the
cached value is discarded just before rendering (in onBeforeRender()). Then
size() is called again as part of the rendering, and again cached. The
cached value is again discarded at the end of the request in onDetach().
What I'm saying is the the first size() shouldn't occur because the page
count should be cached from the previous rendering (it shouldn't be cleared
in onDetach() nor readObject()).
On Wed, Feb 15, 2012 at 1:09 PM, Dan Retzlaff <dr...@gmail.com> wrote:
> Hi, Jonathan. Which component are you referring to? I don't see isVisible()
> overrides in PagingNavigator or its helpers.
>
>
> > It's state and as such should not be discarded when
> > the request is finished, it's still needed for things like checking if a
> > link was indeed visible when a click comes in for it.
>
>
> How can you receive a click event for a link that was not visible?
> Invisible components aren't rendered.
>
> That JIRA discusses multiple size() calls in a single request. You're
> discussing multiple size() calls with multiple requests. Right?
>
> Dan
>
> On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jt...@gmail.com>
> wrote:
>
> > I noticed two count queries go by when using the DataTable component. so
> I
> > searched and dug up this jira issue
> > https://issues.apache.org/jira/browse/WICKET-1766 which is a "won't
> fix".
> >
> > Igor states that two queries are required each request, but I see this
> > differently:
> >
> > The count is a used as the basis for the paging navigator's isVisible(),
> so
> > far so good. The issue is that the count is discarded in onDetach() (as
> > well as readObject()). It's state and as such should not be discarded
> when
> > the request is finished, it's still needed for things like checking if a
> > link was indeed visible when a click comes in for it. If it's not kept, a
> > new query to the model will be made, which might return a different
> result
> > - consequences ensue. The critical part of that is we are checking if the
> > link *was* visible, not if it *is* visible.
> >
> > I think the only time it should be discarded is in the onBeforeRender()
> > event. This is when we are actually interested in going back to the model
> > to see if the value has changed. So to me this is indeed a bug. I don't
> > mind patching something up myself, or reopening the ticket...but I would
> > like a confirmation that I'm not way out in left field ;)
> >
> > Cheers!
> > Jonathan Tougas
> >
>
Re: AbstractPageableView cachedItemCount
Posted by Dan Retzlaff <dr...@gmail.com>.
Hi, Jonathan. Which component are you referring to? I don't see isVisible()
overrides in PagingNavigator or its helpers.
> It's state and as such should not be discarded when
> the request is finished, it's still needed for things like checking if a
> link was indeed visible when a click comes in for it.
How can you receive a click event for a link that was not visible?
Invisible components aren't rendered.
That JIRA discusses multiple size() calls in a single request. You're
discussing multiple size() calls with multiple requests. Right?
Dan
On Wed, Feb 15, 2012 at 9:31 AM, Jonathan Tougas <jt...@gmail.com> wrote:
> I noticed two count queries go by when using the DataTable component. so I
> searched and dug up this jira issue
> https://issues.apache.org/jira/browse/WICKET-1766 which is a "won't fix".
>
> Igor states that two queries are required each request, but I see this
> differently:
>
> The count is a used as the basis for the paging navigator's isVisible(), so
> far so good. The issue is that the count is discarded in onDetach() (as
> well as readObject()). It's state and as such should not be discarded when
> the request is finished, it's still needed for things like checking if a
> link was indeed visible when a click comes in for it. If it's not kept, a
> new query to the model will be made, which might return a different result
> - consequences ensue. The critical part of that is we are checking if the
> link *was* visible, not if it *is* visible.
>
> I think the only time it should be discarded is in the onBeforeRender()
> event. This is when we are actually interested in going back to the model
> to see if the value has changed. So to me this is indeed a bug. I don't
> mind patching something up myself, or reopening the ticket...but I would
> like a confirmation that I'm not way out in left field ;)
>
> Cheers!
> Jonathan Tougas
>