Bug 682304
| Summary: | need to make sure we set pageControl on executeFetch criteria objects | ||
|---|---|---|---|
| Product: | [Other] RHQ Project | Reporter: | John Mazzitelli <mazz> |
| Component: | Core UI | Assignee: | John Mazzitelli <mazz> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Corey Welton <cwelton> |
| Severity: | high | Docs Contact: | |
| Priority: | high | ||
| Version: | 4.0.0.B02 | CC: | mfoley |
| Target Milestone: | --- | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2011-05-24 01:07:55 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 585306 | ||
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.. 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. 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 Bookkeeping - closing bug - fixed in recent release. |
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.