Bug 682304 - need to make sure we set pageControl on executeFetch criteria objects
need to make sure we set pageControl on executeFetch criteria objects
Status: CLOSED CURRENTRELEASE
Product: RHQ Project
Classification: Other
Component: Core UI (Show other bugs)
4.0.0.B02
Unspecified Unspecified
high Severity high (vote)
: ---
: ---
Assigned To: John Mazzitelli
Corey Welton
:
Depends On:
Blocks: rhq4
  Show dependency treegraph
 
Reported: 2011-03-04 14:46 EST by John Mazzitelli
Modified: 2011-05-23 21:07 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-05-23 21:07:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description John Mazzitelli 2011-03-04 14:46:32 EST
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 11:25:58 EST
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 13:00:45 EDT
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-18 20:08:03 EDT
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-23 21:07:55 EDT
Bookkeeping - closing bug - fixed in recent release.

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