Bug 1240854 - Group inventory pages (compatible, mixed, all) fail to …

…display

groups due to UI timeouts

Fix generated GROUP BY clauses. On oracle it is not enough to use group by
resourcegroup, but it need also the field name (so it has to look like
'resourcegroup.id'). Added various tests for
findResourceGroupCompositesByCriteria

modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/server/resource/group/test/LargeGroupCriteriaTest.java

@@ -30,14 +30,18 @@
 import org.rhq.core.domain.criteria.ResourceGroupCriteria;
 import org.rhq.core.domain.resource.InventoryStatus;
 import org.rhq.core.domain.resource.Resource;
+import org.rhq.core.domain.resource.group.ResourceGroup;
 import org.rhq.core.domain.resource.group.composite.ResourceGroupComposite;
+import org.rhq.core.domain.util.OrderingField;
 import org.rhq.core.domain.util.PageControl;
 import org.rhq.core.domain.util.PageList;
+import org.rhq.core.domain.util.PageOrdering;
 import org.rhq.enterprise.server.authz.AuthorizationManagerLocal;
 import org.rhq.enterprise.server.resource.ResourceManagerLocal;
 import org.rhq.enterprise.server.resource.group.ResourceGroupManagerLocal;
 import org.rhq.enterprise.server.test.LargeGroupTestBase;
 import org.rhq.enterprise.server.test.TestServerCommunicationsService;
+import org.rhq.enterprise.server.test.TransactionCallback;
 import org.rhq.enterprise.server.util.LookupUtil;
 import org.rhq.enterprise.server.util.SessionTestHelper;
 
@@ -47,6 +51,7 @@ public class LargeGroupCriteriaTest extends LargeGroupTestBase {
     private static final boolean TEST_ENABLED = true;
 
     private ArrayList<LargeGroupEnvironment> env;
+    private List<ResourceGroup> globalGroups;
 
     private class GroupAvailCounts {
         public final int up;
@@ -96,6 +101,13 @@ private void tearDownGroups() throws Exception {
             }
             env = null;
         }
+        if (globalGroups != null) {
+            Iterator<ResourceGroup> iter = globalGroups.iterator();
+            while (iter.hasNext()) {
+                resourceGroupManager.deleteResourceGroup(getOverlord(), iter.next().getId());
+            }
+            globalGroups = null;
+        }
     }
 
     @Test(enabled = TEST_ENABLED)
@@ -197,6 +209,48 @@ public void testLotsOfLargeGroups() throws Exception {
         testGroupQueries(gacs);
     }
 
+    @Test(enabled = TEST_ENABLED)
+    public void testVariousCriteriaQueries() throws Exception {
+
+        globalGroups = new ArrayList<ResourceGroup>();
+        // create 10 empty groups
+        executeInTransaction(false, new TransactionCallback() {
+
+            @Override
+            public void execute() throws Exception {
+                for (int i = 0; i < 10; i++) {
+                    ResourceGroup group = SessionTestHelper.createNewCompatibleGroupForRole(em, null,
+                        "LargeGroupTestCompatGroup", null);
+                    globalGroups.add(group);
+                }
+            }
+        });
+
+        ResourceGroupManagerLocal groupManager = LookupUtil.getResourceGroupManager();
+
+        ResourceGroupCriteria criteria = new ResourceGroupCriteria();
+
+        // test ordering
+        criteria.addSortName(PageOrdering.DESC);
+        PageList<ResourceGroupComposite> result = groupManager.findResourceGroupCompositesByCriteria(getOverlord(),
+            criteria);
+        assert result.size() == 10 : "Expected to return 10 group composites, but was " + result.size();
+
+        // test custom page size
+        criteria = new ResourceGroupCriteria();
+        criteria.setPageControl(new PageControl(0, 2));
+        result = groupManager.findResourceGroupCompositesByCriteria(getOverlord(), criteria);
+        assert result.size() == 2 : "Expected to return 2 group composites when paging is enabled";
+        assert result.getTotalSize() == 10 : "Expected 10 group composites as total pageList size";
+
+        // test custom page size and ordering within pageControl
+        criteria = new ResourceGroupCriteria();
+        criteria.setPageControl(new PageControl(0, 2, new OrderingField("name", PageOrdering.DESC)));
+        result = groupManager.findResourceGroupCompositesByCriteria(getOverlord(), criteria);
+        assert result.size() == 2 : "Expected to return 2 group composites when paging is enabled";
+        assert result.getTotalSize() == 10 : "Expected 10 group composites as total pageList size";
+    }
+
     // test findResourcesByCriteriaBounded here instead of ResourceGroupManagerBeanTest because we want
     // to work with a decent # or resources.
     @Test(enabled = TEST_ENABLED)

modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/resource/group/ResourceGroupManagerBean.java

@@ -1612,7 +1612,9 @@ public PageList<ResourceGroupComposite> findResourceGroupCompositesByCriteria(Su
 
         generator.alterProjection(projection);
         generator.overrideFromClause(from);
-        generator.setGroupByClause(alias);
+        // group by clause must contain alias.id (otherwise this query fails on oracle)
+        // this is safe, because ResourceGroupCriteria always orders at least by id
+        generator.setGroupByClause(alias + ".id");
         // our query overrides FROM clause using several JOINs
         // to be able to order by fields comming from criteria we need to add those fields
         // to GROUP BY clause as well - but not those fields, their aliases. Aliases are generated by
@@ -1629,6 +1631,7 @@ public PageList<ResourceGroupComposite> findResourceGroupCompositesByCriteria(Su
             .replace("%permAlias%", permAlias);
         generator.overrideFromClause(from);
 
+
         // query explicit members
         PageList<ResourceGroupComposite> explicitResults = queryRunner.execute();
 

modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/util/CriteriaQueryGenerator.java

@@ -531,6 +531,7 @@ public String getQueryString(boolean countQuery) {
             if (lastDelimiterIndex == -1) {
                 // does not require joins, just add the ordering field token directly
                 orderingFieldTokens.add(sortFragment + " " + ordering);
+                orderingFieldAliases.add(sortFragment);
                 continue;
             }
 
@@ -539,6 +540,7 @@ public String getQueryString(boolean countQuery) {
                 // only one dot implies its a property/field directly off of the primary alias
                 // thus, also does not require joins, just add the ordering field token directly
                 orderingFieldTokens.add(sortFragment + " " + ordering);
+                orderingFieldAliases.add(sortFragment);
                 continue;
             }
 
@@ -555,7 +557,7 @@ public String getQueryString(boolean countQuery) {
             } else {
                 joinAlias = "orderingField" + expressionRootIndex;
             }
-            orderingFieldAliases.add(joinAlias);
+            orderingFieldAliases.add(joinAlias + "." + expressionLeaf);
             orderingFieldTokens.add(joinAlias + "." + expressionLeaf + " " + ordering);
         }
 
@@ -646,13 +648,15 @@ public String getQueryString(boolean countQuery) {
             // group by clause
             if (groupByClause != null) {
                 results.append(NL).append("GROUP BY ").append(groupByClause);
-
                 if (groupByOrderAliases) {
+                    results.append(", ");
                     for (String field : orderingFieldAliases) {
-                        results.append(" , ").append(field);
+                        if (!field.equals(groupByClause)) { // avoid duplicities in groupby fields
+                            results.append(field).append(", ");
+                        }
                     }
+                    results.deleteCharAt(results.length() - 2); // always delete last comma
                 }
-
             }
 
             // having clause