Re: AbstractPageableView cachedItemCount

2012-02-17 Thread Jonathan Tougas
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 igor.vaynb...@gmail.comwrote:

 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 jtou...@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 SuperTableT extends DataTableT {
 
   private SuperDataProviderWrapperT dataProviderWrapper;
   public SuperTable( String id, ListIColumnT columns,
  SuperDataProviderWrapperT 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 ItemT newRowItem( final String id, final int index, final
  IModelT model ) {
   return new OddEvenItemT( id, index, model );
  }
   @Override
  protected void onBeforeRender() {
  // reset size before rendering!
   dataProviderWrapper.resetSize();
  super.onBeforeRender();
  }
  }
 
  public class SuperDataProviderWrapperT implements
  ISortableDataProviderT {
 
   private ISortableDataProviderT delegate;
  private int size;
   public SuperDataProviderWrapper( ISortableDataProviderT 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.vaynb...@gmail.com
 wrote:
 
  so when should it be discarded?
 
  -igor
 
  On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas jtou...@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 dretzl...@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 jtou...@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
   

Re: AbstractPageableView cachedItemCount

2012-02-16 Thread Jonathan Tougas
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 SuperTableT extends DataTableT {

 private SuperDataProviderWrapperT dataProviderWrapper;
 public SuperTable( String id, ListIColumnT columns,
SuperDataProviderWrapperT 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 ItemT newRowItem( final String id, final int index, final
IModelT model ) {
 return new OddEvenItemT( id, index, model );
}
 @Override
protected void onBeforeRender() {
// reset size before rendering!
 dataProviderWrapper.resetSize();
super.onBeforeRender();
}
}

public class SuperDataProviderWrapperT implements
ISortableDataProviderT {

 private ISortableDataProviderT delegate;
private int size;
 public SuperDataProviderWrapper( ISortableDataProviderT 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.vaynb...@gmail.comwrote:

 so when should it be discarded?

 -igor

 On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas jtou...@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 dretzl...@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 jtou...@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 dretzl...@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 jtou...@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
 

Re: AbstractPageableView cachedItemCount

2012-02-16 Thread Igor Vaynberg
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 jtou...@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 SuperTableT extends DataTableT {

  private SuperDataProviderWrapperT dataProviderWrapper;
  public SuperTable( String id, ListIColumnT columns,
 SuperDataProviderWrapperT 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 ItemT newRowItem( final String id, final int index, final
 IModelT model ) {
  return new OddEvenItemT( id, index, model );
 }
  @Override
 protected void onBeforeRender() {
 // reset size before rendering!
  dataProviderWrapper.resetSize();
 super.onBeforeRender();
 }
 }

 public class SuperDataProviderWrapperT implements
 ISortableDataProviderT {

  private ISortableDataProviderT delegate;
 private int size;
  public SuperDataProviderWrapper( ISortableDataProviderT 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.vaynb...@gmail.comwrote:

 so when should it be discarded?

 -igor

 On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas jtou...@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 dretzl...@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 jtou...@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 dretzl...@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 jtou...@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 

AbstractPageableView cachedItemCount

2012-02-15 Thread Jonathan Tougas
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

2012-02-15 Thread Dan Retzlaff
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 jtou...@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

2012-02-15 Thread Jonathan Tougas
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 dretzl...@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 jtou...@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

2012-02-15 Thread Dan Retzlaff
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 jtou...@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 dretzl...@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 jtou...@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

2012-02-15 Thread Jonathan Tougas
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 dretzl...@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 jtou...@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 dretzl...@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 jtou...@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

2012-02-15 Thread Dan Retzlaff
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 jtou...@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 dretzl...@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 jtou...@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 dretzl...@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 jtou...@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

2012-02-15 Thread Igor Vaynberg
so when should it be discarded?

-igor

On Wed, Feb 15, 2012 at 4:25 PM, Jonathan Tougas jtou...@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 dretzl...@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 jtou...@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 dretzl...@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 jtou...@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-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org