Bug 966665

This bug is for the general criteria api 

Bug 966559 is just one specific incarnation that uncovered the issue.


+++ This bug was initially created as a clone of Bug #966559 +++

Description of problem: I have a test, that iterates through REST /resources and checks what is returned by server. This test discovered, that server returns a very same resource on different pages


Version-Release number of selected component (if applicable):
RHQ 4.8-master

How reproducible:always


Steps to Reproduce:
1. run GET on /rest/resource with same page size but iterate over pages

2013-05-23 15:03:54,088 - INFO: Request resource?ps=3&page=0 returned [10021, 10148, 10146] (GetResourceTest)
2013-05-23 15:03:54,591 - INFO: Request resource?ps=3&page=1 returned [10041, 10124, 10126] (GetResourceTest)
2013-05-23 15:03:55,246 - INFO: Request resource?ps=3&page=2 returned [10024, 10097, 10133] (GetResourceTest)
2013-05-23 15:03:55,741 - INFO: Request resource?ps=3&page=3 returned [10132, 10134, 10011] (GetResourceTest)
2013-05-23 15:03:56,264 - INFO: Request resource?ps=3&page=4 returned [10011, 10016, 10168] (GetResourceTest)
2013-05-23 15:03:56,802 - INFO: Request resource?ps=3&page=5 returned [10169, 10023, 10029] (GetResourceTest)
2013-05-23 15:03:57,366 - INFO: Request resource?ps=3&page=6 returned [10025, 10160, 10070] (GetResourceTest)
2013-05-23 15:03:57,920 - INFO: Request resource?ps=3&page=7 returned [10174, 10150, 10107] (GetResourceTest)
2013-05-23 15:03:58,488 - INFO: Request resource?ps=3&page=8 returned [10159, 10110, 10155] (GetResourceTest)
2013-05-23 15:03:59,017 - INFO: Request resource?ps=3&page=9 returned [10155, 10026, 10015] (GetResourceTest)
2013-05-23 15:03:59,549 - INFO: Request resource?ps=3&page=10 returned [10076, 10099, 10130] (GetResourceTest)
2013-05-23 15:04:00,086 - INFO: Request resource?ps=3&page=11 returned [10098, 10115, 10130] (GetResourceTest)
2013-05-23 15:04:00,669 - INFO: Request resource?ps=3&page=12 returned [10089, 10105, 10073] (GetResourceTest)
2013-05-23 15:04:01,166 - INFO: Request resource?ps=3&page=13 returned [10022, 10170, 10013] (GetResourceTest)
2013-05-23 15:04:01,659 - INFO: Request resource?ps=3&page=14 returned [10173, 10154, 10053] (GetResourceTest)
2013-05-23 15:04:02,166 - INFO: Request resource?ps=3&page=15 returned [10056, 10136, 10042] (GetResourceTest)
2013-05-23 15:04:02,674 - INFO: Request resource?ps=3&page=16 returned [10043, 10044, 10045] (GetResourceTest)
2013-05-23 15:04:03,191 - INFO: Request resource?ps=3&page=17 returned [10030, 10116, 10096] (GetResourceTest)
2013-05-23 15:04:03,758 - INFO: Request resource?ps=3&page=18 returned [10111, 10121, 10012] (GetResourceTest)
2013-05-23 15:04:04,381 - INFO: Request resource?ps=3&page=19 returned [10171, 10104, 10147] (GetResourceTest)



Actual results:
resource?ps=3&page=3 returned [10132, 10134, 10011]
resource?ps=3&page=4 returned [10011, 10016, 10168]

this means resource with id 10011 is present on both pages 3 and 4 - which is incorrect

Expected results:

each resource must appear exactly once among all returned pages


Additional info:

--- Additional comment from Heiko W. Rupp on 2013-05-23 10:39:23 EDT ---

I assumed that criteria stuff defaults to sort by id if no other sort is given, which is obviously not right.

--- Additional comment from Heiko W. Rupp on 2013-05-23 11:58:12 EDT ---

Actually the code in question already does/did use a sort by name, so should not return the same resource twice.
Perhaps the name is not enough as there are multiple resources with name 'snert' in my inventory and the db is then returning them in random order


17:03:22] <pilhuhn>	         ResourceCriteria criteria = new ResourceCriteria();
[17:03:22] <pilhuhn>	         criteria.addSortName(PageOrdering.ASC);
[17:03:33] <pilhuhn>	 ...
[17:03:34] <pilhuhn>	 PageList<Resource> ret = resMgr.findResourcesByCriteria(caller,criteria);
[17:04:11] <pilhuhn>	 when I run the results with paging over the various pages, I see duplicates like lzoubek  did in Bug 966559
[17:04:14] <rhq-bot>	 BZ 966559 [product=RHQ, priority=unspecified, status=ON_DEV] Paging returns same resource on different pages [ https://bugzilla.redhat.com/966559 ]
[17:04:31] <pilhuhn>	 If I add a criteria.addSortId(PageOrdering.ASC)  the dupe goes away
[17:06:44] <pilhuhn>	 See https://git.fedorahosted.org/cgit/rhq/rhq.git/tree/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/rest/ResourceHandlerBean.java#n190

[17:17:01] <jshaughn>	 pilhuhn: I think that that is likely the issue
[17:17:21] <jshaughn>	 it's not really something we had considered in the past, but I don't see why that wouldn't be a problem
[17:17:41] <jshaughn>	 the db vendor should not necessarily guarantee any ordering other than name
[17:17:57] <jshaughn>	 and if you have dup names then you could get them ordered in any way
[17:18:05] <jshaughn>	 sort of dumb for us not to really think about that
[17:18:20] <jshaughn>	 I agree sort or name, id for safety
[17:18:34] <jshaughn>	 of course you know what this means...
[17:18:41] <jshaughn>	 a sweep
[17:18:43] <pilhuhn>	 We will get dups "naturally", as all agents are named rhq-agent for example
[17:19:08] <jshaughn>	 +1, paging on a field with dups is not going to be safe, I think
[17:19:42] <jshaughn>	 especially after Lukas's recent changes.
[17:19:51] <pilhuhn>	 For the rest api, it is an easy change for me -- but  I guess CLI clients may run into the same issue. And perhaps even the UI
[17:20:50] <jshaughn>	 we may have been getting lucky, if we were paging on a query with join fetch we would have been actually fetching everything and then getting  consistent ordering wuith the in-mem handling
[17:21:13] <jshaughn>	 absolutely, CLI, GUI, perhaps even SLSB calls
[17:21:22] <jshaughn>	 we need a sweep now...
[17:21:38] <jshaughn>	 looking for our sort specifiers all over the place
[17:21:51] <jshaughn>	 and adding id as a secondary sort if necessary
[17:22:16] <jshaughn>	 either that or we build it into base Criteria handling
[17:22:36] <jshaughn>	 but I'm not sure I like to be that heavy-handed
[17:25:26] <pilhuhn>	 We would perhaps need to make our criteria fields know if they are unique. the Criteria execution could then look them up and if there is a unique one (like id) it would just use that. Otherwise it would automagically add a sortById as fallback
[17:27:23] <pilhuhn>	 biab
[17:28:30] <jshaughn>	 I think perhaps just making a jdoc note on the appropriate addSort methods may be sufficient, and adding another "tip" to the criteria tips and tricks page.
[17:30:42] <jshaughn>	 another possibility is always adding an id sort when paging is specified *unless* there is an override set: addSortId(false)
[17:31:05] <jshaughn>	 that may be smart
[17:57:40] <pilhuhn>	 Yep, that sounds good