Allow limit queries without random ordering (#12598)

Author: sureshanaparti

engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java

@@ -1348,7 +1348,7 @@ public HostVO findAnyStateHypervisorHostInCluster(long clusterId) {
         SearchCriteria<HostVO> sc = TypeClusterStatusSearch.create();
         sc.setParameters("type", Host.Type.Routing);
         sc.setParameters("cluster", clusterId);
-        List<HostVO> list = listBy(sc, new Filter(1));
+        List<HostVO> list = listBy(sc, new Filter(1, true));
         return list.isEmpty() ? null : list.get(0);
     }
 

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java

@@ -202,7 +202,7 @@ public ImageStoreVO findOneByZoneAndProtocol(long dataCenterId, String protocol)
         sc.setParameters("dataCenterId", dataCenterId);
         sc.setParameters("protocol", protocol);
         sc.setParameters("role", DataStoreRole.Image);
-        Filter filter = new Filter(1);
+        Filter filter = new Filter(1, true);
         List<ImageStoreVO> results =  listBy(sc, filter);
         return results.size() == 0 ? null : results.get(0);
     }

framework/db/src/main/java/com/cloud/utils/db/Filter.java

@@ -57,7 +57,18 @@ public Filter(Class<?> clazz, String field, boolean ascending) {
     }
 
     public Filter(long limit) {
-        _orderBy = " ORDER BY RAND()";
+        this(limit, false);
+    }
+
+    /**
+     * Constructor for creating a filter with random ordering
+     * @param limit the maximum number of results to return
+     * @param randomize if true, orders results randomly
+     */
+    public Filter(long limit, boolean randomize) {
+        if (randomize) {
+            _orderBy = " ORDER BY RAND()" ;
+        }
         _limit = limit;
     }
 

framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java

@@ -347,7 +347,7 @@ public List<T> lockRows(final SearchCriteria<T> sc, final Filter filter, final b
     @Override
     @DB()
     public T lockOneRandomRow(final SearchCriteria<T> sc, final boolean exclusive) {
-        final Filter filter = new Filter(1);
+        final Filter filter = new Filter(1, true);
         final List<T> beans = search(sc, filter, exclusive, true);
         return beans.isEmpty() ? null : beans.get(0);
     }
@@ -927,7 +927,7 @@ public Class<T> getEntityBeanType() {
 
     @DB()
     protected T findOneIncludingRemovedBy(final SearchCriteria<T> sc) {
-        Filter filter = new Filter(1);
+        Filter filter = new Filter(1, true);
         List<T> results = searchIncludingRemoved(sc, filter, null, false);
         assert results.size() <= 1 : "Didn't the limiting worked?";
         return results.size() == 0 ? null : results.get(0);
@@ -1324,7 +1324,7 @@ public int batchExpunge(final SearchCriteria<T> sc, final Long batchSize) {
         Filter filter = null;
         final long batchSizeFinal = ObjectUtils.defaultIfNull(batchSize, 0L);
         if (batchSizeFinal > 0) {
-            filter = new Filter(null, batchSizeFinal);
+            filter = new Filter(batchSizeFinal);
         }
         int expunged = 0;
         int currentExpunged = 0;

framework/db/src/test/java/com/cloud/utils/db/FilterTest.java

@@ -41,4 +41,62 @@ public void testAddOrderBy() {
         Assert.assertTrue(filter.getOrderBy().split(",").length == 3);
         Assert.assertTrue(filter.getOrderBy().split(",")[2].trim().toLowerCase().equals("test.fld_int asc"));
     }
+
+
+    @Test
+    public void testFilterWithLimitOnly() {
+        Filter filter = new Filter(5);
+
+        Assert.assertEquals(Long.valueOf(5), filter.getLimit());
+        Assert.assertNull(filter.getOrderBy());
+        Assert.assertNull(filter.getOffset());
+    }
+
+    @Test
+    public void testFilterWithLimitAndRandomizeFalse() {
+        Filter filter = new Filter(10, false);
+
+        Assert.assertEquals(Long.valueOf(10), filter.getLimit());
+        Assert.assertNull(filter.getOrderBy());
+        Assert.assertNull(filter.getOffset());
+    }
+
+    @Test
+    public void testFilterWithLimitAndRandomizeTrue() {
+        Filter filter = new Filter(3, true);
+
+        Assert.assertNull(filter.getLimit());
+        Assert.assertNotNull(filter.getOrderBy());
+        Assert.assertTrue(filter.getOrderBy().contains("ORDER BY RAND()"));
+        Assert.assertTrue(filter.getOrderBy().contains("LIMIT 3"));
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 3", filter.getOrderBy());
+    }
+
+    @Test
+    public void testFilterRandomizeWithDifferentLimits() {
+        Filter filter1 = new Filter(1, true);
+        Filter filter10 = new Filter(10, true);
+        Filter filter100 = new Filter(100, true);
+
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 1", filter1.getOrderBy());
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 10", filter10.getOrderBy());
+        Assert.assertEquals(" ORDER BY RAND() LIMIT 100", filter100.getOrderBy());
+    }
+
+    @Test
+    public void testFilterConstructorBackwardsCompatibility() {
+        // Test that Filter(long) behaves differently now (no ORDER BY RAND())
+        // compared to Filter(long, true) which preserves old behavior
+        Filter simpleLimitFilter = new Filter(1);
+        Filter randomFilter = new Filter(1, true);
+
+        // Simple limit filter should just set limit
+        Assert.assertEquals(Long.valueOf(1), simpleLimitFilter.getLimit());
+        Assert.assertNull(simpleLimitFilter.getOrderBy());
+
+        // Random filter should set orderBy with RAND()
+        Assert.assertNull(randomFilter.getLimit());
+        Assert.assertNotNull(randomFilter.getOrderBy());
+        Assert.assertTrue(randomFilter.getOrderBy().contains("RAND()"));
+    }
 }

framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java

@@ -20,6 +20,7 @@
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 import org.junit.Assert;
 import org.junit.Before;
@@ -263,4 +264,71 @@ public void multiJoinSameTableTest() {
                 " INNER JOIN tableA tableA2Alias ON tableC.column3=tableA2Alias.column2 " +
                 " INNER JOIN tableA tableA3Alias ON tableD.column4=tableA3Alias.column3 AND tableD.column5=? ", joinString.toString());
     }
+
+
+    @Test
+    public void testLockOneRandomRowUsesRandomFilter() {
+        // Create a mock DAO to test lockOneRandomRow behavior
+        GenericDaoBase<DbTestVO, Long> testDao = Mockito.mock(GenericDaoBase.class);
+
+        // Capture the filter passed to the search method
+        final Filter[] capturedFilter = new Filter[1];
+
+        Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean()))
+                .thenCallRealMethod();
+
+        Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class),
+                        Mockito.anyBoolean(), Mockito.anyBoolean()))
+                .thenAnswer(invocation -> {
+                    capturedFilter[0] = invocation.getArgument(1);
+                    return new ArrayList<DbTestVO>();
+                });
+
+        SearchCriteria<DbTestVO> sc = Mockito.mock(SearchCriteria.class);
+        testDao.lockOneRandomRow(sc, true);
+
+        // Verify that the filter uses random ordering
+        Assert.assertNotNull(capturedFilter[0]);
+        Assert.assertNotNull(capturedFilter[0].getOrderBy());
+        Assert.assertTrue(capturedFilter[0].getOrderBy().contains("ORDER BY RAND()"));
+        Assert.assertTrue(capturedFilter[0].getOrderBy().contains("LIMIT 1"));
+    }
+
+    @Test
+    public void testLockOneRandomRowReturnsNullOnEmptyResult() {
+        GenericDaoBase<DbTestVO, Long> testDao = Mockito.mock(GenericDaoBase.class);
+
+        Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean()))
+                .thenCallRealMethod();
+
+        Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class),
+                        Mockito.anyBoolean(), Mockito.anyBoolean()))
+                .thenReturn(new ArrayList<DbTestVO>());
+
+        SearchCriteria<DbTestVO> sc = Mockito.mock(SearchCriteria.class);
+        DbTestVO result = testDao.lockOneRandomRow(sc, true);
+
+        Assert.assertNull(result);
+    }
+
+    @Test
+    public void testLockOneRandomRowReturnsFirstElement() {
+        GenericDaoBase<DbTestVO, Long> testDao = Mockito.mock(GenericDaoBase.class);
+        DbTestVO expectedResult = new DbTestVO();
+        List<DbTestVO> resultList = new ArrayList<>();
+        resultList.add(expectedResult);
+
+        Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean()))
+                .thenCallRealMethod();
+
+        Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class),
+                        Mockito.anyBoolean(), Mockito.anyBoolean()))
+                .thenReturn(resultList);
+
+        SearchCriteria<DbTestVO> sc = Mockito.mock(SearchCriteria.class);
+        DbTestVO result = testDao.lockOneRandomRow(sc, true);
+
+        Assert.assertNotNull(result);
+        Assert.assertEquals(expectedResult, result);
+    }
 }