Bug 682304 - need to make sure we set pageControl on executeFetch criteria objects
Summary: need to make sure we set pageControl on executeFetch criteria objects
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: RHQ Project
Classification: Other
Component: Core UI
Version: 4.0.0.B02
Hardware: Unspecified
OS: Unspecified
high
high vote
Target Milestone: ---
: ---
Assignee: John Mazzitelli
QA Contact: Corey Welton
URL:
Whiteboard:
Depends On:
Blocks: rhq4
TreeView+ depends on / blocked
 
Reported: 2011-03-04 19:46 UTC by John Mazzitelli
Modified: 2011-05-24 01:07 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-05-24 01:07:55 UTC


Attachments (Terms of Use)

Description John Mazzitelli 2011-03-04 19:46:32 UTC
I just noticed something yesterday and I wanted to bring it up because I 
think there may problem(s) for large datasets.

I noticed in some DataSource implementations of ours, in our 
executeFetch() methods, we aren't setting the page control in the 
criteria... like this:

    criteria.setPageControl(getPageControl(request));

without setting that, I think we are always going to only get back to 
top 200 rows (since that's the default page control). Which means if we 
are paging or sorting, if I read this code right, it won't actually get 
us the right data.

So, we need to scour the source code, look at all executeFetch impls and 
make sure we are setting the page control properly. I think if we don't 
do this, its not gonna work right for datasets larger than 200.

Comment 1 Charles Crouch 2011-03-07 16:25:58 UTC
On 03/04/2011 04:26 PM, Ian Springer wrote:
> Nice catch. We should see if there's a way to move this up into
> RPCDataSource.transformRequest() so it's take care of by the base class,
> and subclasses don't have to remember to do it. Perhaps add:
>
>      protected abstract C getFetchCriteria(final DSRequest request)
>
> to RPCDataSource and then transformRequest() could do:
>
>      C criteria = getFetchCriteria(request);
>      executeFetch(request, response, criteria);   // note, the signature
> change
>
> or something along those lines..

Comment 2 John Mazzitelli 2011-03-22 17:00:45 UTC
commit 76890a0

refactored/changed 74 files to support this.

RPCDataSource how has a second generic type associated with it: <C extends Criteria>

It now has this abstract method:

  protected abstract C getFetchCriteria(final DSRequest request)

It will call that to obtain a criteria, then it will set the page control on it if the returned criteria is not null and the criteria does not already have a page control override already set.

The executeFetch has been altered to now have a third parameter: C criteria.

This needs lots of testing. Essentially, I changed all data sources - so any list grid/table that uses data sources (which is about 95% of them in our app) needs smoke testing. As devs work in the app, they will implicitly test this - anytime they view a page with tables/listgrids will verify that particular page is ok. Not sure what formal testing QE will want to do.

Comment 3 Mike Foley 2011-04-19 00:08:03 UTC
this bug required broad test coverage to verify.  i am going to offer as evidence the testing done for the 4/29/2011 release ... the testing is documented here and covered the entire ui.  http://www.rhq-project.org/display/RHQ/RHQ+4.0+Release+Plan

Comment 4 Corey Welton 2011-05-24 01:07:55 UTC
Bookkeeping - closing bug - fixed in recent release.


Note You need to log in before you can comment on or make changes to this bug.