Improve group listing with pagination

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/beans/DeviceGroupList.java

@@ -20,8 +20,6 @@ package org.wso2.carbon.device.mgt.jaxrs.beans;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import io.swagger.annotations.ApiModelProperty;
-import org.wso2.carbon.device.mgt.common.Device;
-import org.wso2.carbon.device.mgt.common.group.mgt.DeviceGroup;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -30,13 +28,13 @@ public class DeviceGroupList extends BasePaginatedResult {
 
     @ApiModelProperty(value = "List of device groups returned")
     @JsonProperty("groups")
-    private List<DeviceGroup> deviceGroups = new ArrayList<>();
+    private List<?> deviceGroups = new ArrayList<>();
 
-    public List<DeviceGroup> getList() {
+    public List<?> getList() {
         return deviceGroups;
     }
 
-    public void setList(List<DeviceGroup> deviceGroups) {
+    public void setList(List<?> deviceGroups) {
         this.deviceGroups = deviceGroups;
     }
 

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImpl.java

@@ -27,6 +27,7 @@ import org.wso2.carbon.device.mgt.common.Device;
 import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
 import org.wso2.carbon.device.mgt.common.DeviceNotFoundException;
 import org.wso2.carbon.device.mgt.common.GroupPaginationRequest;
+import org.wso2.carbon.device.mgt.common.PaginationResult;
 import org.wso2.carbon.device.mgt.common.group.mgt.DeviceGroup;
 import org.wso2.carbon.device.mgt.common.group.mgt.GroupAlreadyExistException;
 import org.wso2.carbon.device.mgt.common.group.mgt.GroupManagementException;
@@ -58,16 +59,16 @@ public class GroupManagementServiceImpl implements GroupManagementService {
     public Response getGroups(String name, String owner, int offset, int limit) {
         try {
             RequestValidationUtil.validatePaginationParameters(offset, limit);
-            GroupManagementProviderService service = DeviceMgtAPIUtils.getGroupManagementProviderService();
             String currentUser = CarbonContext.getThreadLocalCarbonContext().getUsername();
             GroupPaginationRequest request = new GroupPaginationRequest(offset, limit);
             request.setGroupName(name);
             request.setOwner(owner);
-            List<DeviceGroup> deviceGroups = service.getGroups(currentUser, request);
+            PaginationResult deviceGroupsResult = DeviceMgtAPIUtils.getGroupManagementProviderService()
-            if (deviceGroups != null && deviceGroups.size() > 0) {
+                    .getGroups(currentUser, request);
+            if (deviceGroupsResult.getData() != null && deviceGroupsResult.getRecordsTotal() > 0) {
                 DeviceGroupList deviceGroupList = new DeviceGroupList();
-                deviceGroupList.setList(deviceGroups);
+                deviceGroupList.setList(deviceGroupsResult.getData());
-                deviceGroupList.setCount(service.getGroupCount(currentUser));
+                deviceGroupList.setCount(deviceGroupsResult.getRecordsTotal());
                 return Response.status(Response.Status.OK).entity(deviceGroupList).build();
             } else {
                 return Response.status(Response.Status.NOT_FOUND).build();

components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/admin/GroupManagementAdminServiceImpl.java

@@ -22,6 +22,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.wso2.carbon.device.mgt.common.GroupPaginationRequest;
 import org.wso2.carbon.device.mgt.common.PaginationRequest;
+import org.wso2.carbon.device.mgt.common.PaginationResult;
 import org.wso2.carbon.device.mgt.common.group.mgt.DeviceGroup;
 import org.wso2.carbon.device.mgt.common.group.mgt.GroupManagementException;
 import org.wso2.carbon.device.mgt.core.service.GroupManagementProviderService;
@@ -41,15 +42,15 @@ public class GroupManagementAdminServiceImpl implements GroupManagementAdminServ
     public Response getGroups(String name, String owner, int offset, int limit) {
         try {
             RequestValidationUtil.validatePaginationParameters(offset, limit);
-            GroupManagementProviderService service = DeviceMgtAPIUtils.getGroupManagementProviderService();
             GroupPaginationRequest request = new GroupPaginationRequest(offset, limit);
             request.setGroupName(name);
             request.setOwner(owner);
-            List<DeviceGroup> deviceGroups = service.getGroups(request);
+            PaginationResult deviceGroupsResult = DeviceMgtAPIUtils.getGroupManagementProviderService()
-            if (deviceGroups != null && deviceGroups.size() > 0) {
+                    .getGroups(request);
+            if (deviceGroupsResult.getData() != null && deviceGroupsResult.getRecordsTotal() > 0) {
                 DeviceGroupList deviceGroupList = new DeviceGroupList();
-                deviceGroupList.setList(deviceGroups);
+                deviceGroupList.setList(deviceGroupsResult.getData());
-                deviceGroupList.setCount(service.getGroupCount());
+                deviceGroupList.setCount(deviceGroupsResult.getRecordsTotal());
                 return Response.status(Response.Status.OK).entity(deviceGroupList).build();
             } else {
                 return Response.status(Response.Status.NOT_FOUND).build();

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAO.java

@@ -79,7 +79,7 @@ public interface GroupDAO {
     /**
      * Get paginated list of Device Groups in tenant.
      *
-     * @param paginationRequest to filter results
+     * @param paginationRequest to filter results.
      * @param tenantId of user's tenant.
      * @return List of all Device Groups in tenant.
      * @throws GroupManagementDAOException
@@ -105,14 +105,14 @@ public interface GroupDAO {
     int getGroupCount(int tenantId) throws GroupManagementDAOException;
 
     /**
-     * Get the list of Groups that matches with the given DeviceGroup name.
+     * Get paginated count of Device Groups in tenant.
      *
-     * @param groupName of the Device Group.
+     * @param paginationRequest to filter results.
-     * @param tenantId  of user's tenant.
+     * @param tenantId of user's tenant.
-     * @return List of DeviceGroup that matches with the given DeviceGroup name.
+     * @return List of all Device Groups in tenant.
      * @throws GroupManagementDAOException
      */
-    List<DeviceGroup> findInGroups(String groupName, int tenantId) throws GroupManagementDAOException;
+    int getGroupCount(GroupPaginationRequest paginationRequest, int tenantId) throws GroupManagementDAOException;
 
     /**
      * Check group already existed with given name.

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAOImpl.java

@@ -182,32 +182,37 @@ public class GroupDAOImpl implements GroupDAO {
         boolean hasGroupName = false;
         String owner = request.getOwner();
         boolean hasOwner = false;
+        boolean hasLimit = request.getRowCount() != 0;
 
         try {
             Connection conn = GroupManagementDAOFactory.getConnection();
             String sql = "SELECT ID, DESCRIPTION, GROUP_NAME, DATE_OF_CREATE, DATE_OF_LAST_UPDATE, OWNER "
                          + "FROM DM_GROUP WHERE TENANT_ID = ?";
             if (groupName != null && !groupName.isEmpty()) {
-                sql += " GROUP_NAME LIKE ?";
+                sql += " AND GROUP_NAME LIKE ?";
                 hasGroupName = true;
             }
             if (owner != null && !owner.isEmpty()) {
-                sql += " OWNER LIKE ?";
+                sql += " AND OWNER LIKE ?";
                 hasOwner = true;
             }
-            sql += " LIMIT ?, ?";
+            if (hasLimit) {
+                sql += " LIMIT ?, ?";
+            }
 
             int paramIndex = 1;
             stmt = conn.prepareStatement(sql);
             stmt.setInt(paramIndex++, tenantId);
             if (hasGroupName) {
-                stmt.setString(paramIndex++, groupName);
+                stmt.setString(paramIndex++, groupName +  "%");
             }
             if (hasOwner) {
-                stmt.setString(paramIndex++, owner);
+                stmt.setString(paramIndex++, owner + "%");
+            }
+            if (hasLimit) {
+                stmt.setInt(paramIndex++, request.getStartIndex());
+                stmt.setInt(paramIndex, request.getRowCount());
             }
-            stmt.setInt(paramIndex++, request.getStartIndex());
-            stmt.setInt(paramIndex, request.getRowCount());
             resultSet = stmt.executeQuery();
             deviceGroupList = new ArrayList<>();
             while (resultSet.next()) {
@@ -268,29 +273,48 @@ public class GroupDAOImpl implements GroupDAO {
     }
 
     @Override
-    public List<DeviceGroup> findInGroups(String groupName, int tenantId)
+    public int getGroupCount(GroupPaginationRequest request, int tenantId)
             throws GroupManagementDAOException {
         PreparedStatement stmt = null;
         ResultSet resultSet = null;
-        List<DeviceGroup> deviceGroups = new ArrayList<>();
+
+        String groupName = request.getGroupName();
+        boolean hasGroupName = false;
+        String owner = request.getOwner();
+        boolean hasOwner = false;
+
         try {
             Connection conn = GroupManagementDAOFactory.getConnection();
-            String sql = "SELECT ID, DESCRIPTION, GROUP_NAME, DATE_OF_CREATE, DATE_OF_LAST_UPDATE, OWNER "
+            String sql = "SELECT COUNT(ID) AS GROUP_COUNT FROM DM_GROUP WHERE TENANT_ID = ?";
-                         + "FROM DM_GROUP WHERE GROUP_NAME LIKE ? AND TENANT_ID = ?";
+            if (groupName != null && !groupName.isEmpty()) {
+                sql += " AND GROUP_NAME LIKE ?";
+                hasGroupName = true;
+            }
+            if (owner != null && !owner.isEmpty()) {
+                sql += " AND OWNER LIKE ?";
+                hasOwner = true;
+            }
+
+            int paramIndex = 1;
             stmt = conn.prepareStatement(sql);
-            stmt.setString(1, "%" + groupName + "%");
+            stmt.setInt(paramIndex++, tenantId);
-            stmt.setInt(2, tenantId);
+            if (hasGroupName) {
+                stmt.setString(paramIndex++, groupName + "%");
+            }
+            if (hasOwner) {
+                stmt.setString(paramIndex, owner + "%");
+            }
             resultSet = stmt.executeQuery();
-            while (resultSet.next()) {
+            if (resultSet.next()) {
-                deviceGroups.add(GroupManagementDAOUtil.loadGroup(resultSet));
+                return resultSet.getInt("GROUP_COUNT");
+            } else {
+                return 0;
             }
         } catch (SQLException e) {
-            throw new GroupManagementDAOException("Error occurred while listing Device Groups by name '" +
+            throw new GroupManagementDAOException("Error occurred while listing all groups in tenant: " + tenantId, e);
-                    groupName + "' in tenant '" + tenantId + "'", e);
         } finally {
             GroupManagementDAOUtil.cleanupResources(stmt, resultSet);
         }
-        return deviceGroups;
     }
 
     @Override

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderService.java

@@ -23,6 +23,7 @@ import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
 import org.wso2.carbon.device.mgt.common.DeviceNotFoundException;
 import org.wso2.carbon.device.mgt.common.GroupPaginationRequest;
 import org.wso2.carbon.device.mgt.common.PaginationRequest;
+import org.wso2.carbon.device.mgt.common.PaginationResult;
 import org.wso2.carbon.device.mgt.common.group.mgt.DeviceGroup;
 import org.wso2.carbon.device.mgt.common.group.mgt.GroupAlreadyExistException;
 import org.wso2.carbon.device.mgt.common.group.mgt.GroupManagementException;
@@ -99,7 +100,7 @@ public interface GroupManagementProviderService {
      * @return list of groups.
      * @throws GroupManagementException
      */
-    List<DeviceGroup> getGroups(GroupPaginationRequest paginationRequest) throws GroupManagementException;
+    PaginationResult getGroups(GroupPaginationRequest paginationRequest) throws GroupManagementException;
 
     /**
      * Get device groups with pagination.
@@ -109,7 +110,7 @@ public interface GroupManagementProviderService {
      * @return list of groups.
      * @throws GroupManagementException
      */
-    List<DeviceGroup> getGroups(String username, GroupPaginationRequest paginationRequest) throws GroupManagementException;
+    PaginationResult getGroups(String username, GroupPaginationRequest paginationRequest) throws GroupManagementException;
 
     /**
      * Get all device group count in tenant

components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceImpl.java

@@ -28,6 +28,7 @@ import org.wso2.carbon.device.mgt.common.DeviceIdentifier;
 import org.wso2.carbon.device.mgt.common.DeviceManagementException;
 import org.wso2.carbon.device.mgt.common.DeviceNotFoundException;
 import org.wso2.carbon.device.mgt.common.GroupPaginationRequest;
+import org.wso2.carbon.device.mgt.common.PaginationResult;
 import org.wso2.carbon.device.mgt.common.TransactionManagementException;
 import org.wso2.carbon.device.mgt.common.group.mgt.DeviceGroup;
 import org.wso2.carbon.device.mgt.common.group.mgt.GroupAlreadyExistException;
@@ -139,7 +140,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
     @Override
     public boolean deleteGroup(int groupId) throws GroupManagementException {
         String roleName;
-        DeviceGroup deviceGroup = buildDeviceGroup(groupId);
+        DeviceGroup deviceGroup = getGroup(groupId);
         if (deviceGroup == null) {
             return false;
         }
@@ -168,7 +169,11 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
         }
     }
 
-    private DeviceGroup buildDeviceGroup(int groupId) throws GroupManagementException {
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public DeviceGroup getGroup(int groupId) throws GroupManagementException {
         DeviceGroup deviceGroup;
         try {
             GroupManagementDAOFactory.openConnection();
@@ -187,19 +192,6 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
         return deviceGroup;
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public DeviceGroup getGroup(int groupId) throws GroupManagementException {
-        DeviceGroup deviceGroup = this.buildDeviceGroup(groupId);
-        if (deviceGroup != null) {
-            deviceGroup.setUsers(this.getUsers(groupId));
-            deviceGroup.setRoles(this.getRoles(groupId));
-        }
-        return deviceGroup;
-    }
-
     @Override
     public List<DeviceGroup> getGroups() throws GroupManagementException {
         List<DeviceGroup> deviceGroups = new ArrayList<>();
@@ -222,9 +214,21 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
     }
 
     @Override
-    public List<DeviceGroup> getGroups(GroupPaginationRequest request) throws GroupManagementException {
+    public PaginationResult getGroups(GroupPaginationRequest request) throws GroupManagementException {
-        List<DeviceGroup> deviceGroups = new ArrayList<>();
         request = DeviceManagerUtil.validateGroupListPageSize(request);
+        List<DeviceGroup> deviceGroups = getPlainDeviceGroups(request);
+        for (DeviceGroup group : deviceGroups) {
+            group.setUsers(this.getUsers(group.getGroupId()));
+            group.setRoles(this.getRoles(group.getGroupId()));
+        }
+        PaginationResult groupResult = new PaginationResult();
+        groupResult.setData(deviceGroups);
+        groupResult.setRecordsTotal(getGroupCount(request));
+        return groupResult;
+    }
+
+    private List<DeviceGroup> getPlainDeviceGroups(GroupPaginationRequest request) throws GroupManagementException {
+        List<DeviceGroup> deviceGroups = new ArrayList<>();
         try {
             int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId();
             GroupManagementDAOFactory.openConnection();
@@ -236,10 +240,6 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
         } finally {
             GroupManagementDAOFactory.closeConnection();
         }
-        for (DeviceGroup group : deviceGroups) {
-            group.setUsers(this.getUsers(group.getGroupId()));
-            group.setRoles(this.getRoles(group.getGroupId()));
-        }
         return deviceGroups;
     }
 
@@ -254,7 +254,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
             String[] roleList = userStoreManager.getRoleListOfUser(username);
             for (String role : roleList) {
                 if (role != null && role.contains("Internal/group-")) {
-                    DeviceGroup deviceGroup = extractNewGroupFromRole(groups, role);
+                    DeviceGroup deviceGroup = checkAndExtractNonExistingGroup(groups, role);
                     if (deviceGroup != null) {
                         groups.put(deviceGroup.getGroupId(), deviceGroup);
                     }
@@ -266,30 +266,46 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
         return new ArrayList<>(groups.values());
     }
 
-    public List<DeviceGroup> getGroups(String currentUser, GroupPaginationRequest request) throws GroupManagementException {
+    public PaginationResult getGroups(String currentUser, GroupPaginationRequest request) throws GroupManagementException {
         request = DeviceManagerUtil.validateGroupListPageSize(request);
-        Map<Integer, DeviceGroup> groups = new HashMap<>();
+        int startIndex = request.getStartIndex();
-        UserStoreManager userStoreManager;
+        int count = request.getRowCount();
+        int index = 0;
+        request.setRowCount(0);
+        List<DeviceGroup> allMatchingGroups = this.getPlainDeviceGroups(request);
+        List<DeviceGroup> deviceGroups = new ArrayList<>();
         try {
             int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId();
-            userStoreManager = DeviceManagementDataHolder.getInstance().getRealmService().getTenantUserRealm(tenantId)
+            UserStoreManager userStoreManager = DeviceManagementDataHolder.getInstance().getRealmService().getTenantUserRealm(tenantId)
                     .getUserStoreManager();
             String[] roleList = userStoreManager.getRoleListOfUser(currentUser);
-            int index = 0;
+            List<Integer> groupIds = new ArrayList<>();
             for (String role : roleList) {
                 if (role != null && role.contains("Internal/group-")) {
-                    DeviceGroup deviceGroupBuilder = extractNewGroupFromRole(groups, role);
+                    int groupId = Integer.parseInt(role.split("-")[1]);
-                    if (deviceGroupBuilder != null
+                    if (!groupIds.contains(groupId)) {
-                        && request.getStartIndex() <= index++
+                        groupIds.add(groupId);
-                        && index <= request.getRowCount()) {
-                        groups.put(deviceGroupBuilder.getGroupId(), deviceGroupBuilder);
                     }
                 }
             }
+            for (DeviceGroup group : allMatchingGroups) {
+                int groupId = group.getGroupId();
+                if (groupIds.contains(groupId)) {
+                    if (startIndex <= index && index < count) {
+                        group.setUsers(this.getUsers(groupId));
+                        group.setRoles(this.getRoles(groupId));
+                        deviceGroups.add(group);
+                    }
+                    index++;
+                }
+            }
         } catch (UserStoreException e) {
             throw new GroupManagementException("Error occurred while getting user store manager.", e);
         }
-        return new ArrayList<>(groups.values());
+        PaginationResult groupResult = new PaginationResult();
+        groupResult.setData(deviceGroups);
+        groupResult.setRecordsTotal(index);
+        return groupResult;
     }
 
     @Override
@@ -307,6 +323,20 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
         }
     }
 
+    private int getGroupCount(GroupPaginationRequest request) throws GroupManagementException {
+        try {
+            int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId();
+            GroupManagementDAOFactory.openConnection();
+            return groupDAO.getGroupCount(request, tenantId);
+        } catch (GroupManagementDAOException e) {
+            throw new GroupManagementException("Error occurred while retrieving all groups in tenant", e);
+        } catch (SQLException e) {
+            throw new GroupManagementException("Error occurred while opening a connection to the data source.", e);
+        } finally {
+            GroupManagementDAOFactory.closeConnection();
+        }
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -677,7 +707,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
             for (String role : roles) {
                 if (role != null && role.contains("Internal/group-") && userRealm.getAuthorizationManager()
                         .isRoleAuthorized(role, permission, CarbonConstants.UI_PERMISSION_ACTION)) {
-                    DeviceGroup group = extractNewGroupFromRole(groups, role);
+                    DeviceGroup group = checkAndExtractNonExistingGroup(groups, role);
                     if (group != null) {
                         groups.put(group.getGroupId(), group);
                     }
@@ -708,12 +738,20 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
         }
     }
 
-    private DeviceGroup extractNewGroupFromRole(Map<Integer, DeviceGroup> groups, String role)
+    /**
+     * This method returns group belongs to particular role, if it is not existed in groups map.
+     *
+     * @param groups existing groups map.
+     * @param role   group related role which needs to evaluate.
+     * @return device group if it is not existing in the groups map.
+     * @throws GroupManagementException
+     */
+    private DeviceGroup checkAndExtractNonExistingGroup(Map<Integer, DeviceGroup> groups, String role)
             throws GroupManagementException {
         try {
             int groupId = Integer.parseInt(role.split("-")[1]);
             if (!groups.containsKey(groupId)) {
-                return buildDeviceGroup(groupId);
+                return getGroup(groupId);
             }
         } catch (NumberFormatException e) {
             log.error("Unable to extract groupId from role " + role, e);

components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/dao/GroupPersistTests.java

@@ -40,7 +40,7 @@ import java.util.List;
 public class GroupPersistTests extends BaseDeviceManagementTest {
 
     private static final Log log = LogFactory.getLog(GroupPersistTests.class);
-    int groupId = -1;
+    private int groupId = -1;
     private GroupDAO groupDAO;
 
     @BeforeClass
@@ -76,27 +76,6 @@ public class GroupPersistTests extends BaseDeviceManagementTest {
         log.debug("Group name: " + group.getName());
     }
 
-    @Test(dependsOnMethods = {"testAddGroupTest"})
-    public void findGroupTest() {
-        try {
-            GroupManagementDAOFactory.openConnection();
-            List<DeviceGroup> groups = groupDAO.findInGroups("Test", TestDataHolder.SUPER_TENANT_ID);
-            Assert.assertNotEquals(groups.size(), 0, "No groups found");
-            Assert.assertNotNull(groups.get(0), "Group is null");
-            log.debug("Group found: " + groups.get(0).getName());
-        } catch (GroupManagementDAOException e) {
-            String msg = "Error occurred while find group by name.";
-            log.error(msg, e);
-            Assert.fail(msg, e);
-        } catch (SQLException e) {
-            String msg = "Error occurred while opening a connection to the data source.";
-            log.error(msg, e);
-            Assert.fail(msg, e);
-        } finally {
-            GroupManagementDAOFactory.closeConnection();
-        }
-    }
-
     @Test(dependsOnMethods = {"testAddGroupTest"})
     public void getGroupTest() {
         try {
@@ -125,6 +104,7 @@ public class GroupPersistTests extends BaseDeviceManagementTest {
     public void addDeviceToGroupTest() {
         Device initialTestDevice = TestDataHolder.initialTestDevice;
         DeviceGroup deviceGroup = getGroupById(groupId);
+        Assert.assertNotNull(deviceGroup, "Group is null");
         try {
             GroupManagementDAOFactory.beginTransaction();
             groupDAO.addDevice(deviceGroup.getGroupId(), initialTestDevice.getId(), TestDataHolder.SUPER_TENANT_ID);
@@ -166,6 +146,7 @@ public class GroupPersistTests extends BaseDeviceManagementTest {
     public void removeDeviceFromGroupTest() {
         Device initialTestDevice = TestDataHolder.initialTestDevice;
         DeviceGroup deviceGroup = getGroupById(groupId);
+        Assert.assertNotNull(deviceGroup, "Group is null");
         try {
             GroupManagementDAOFactory.beginTransaction();
             groupDAO.removeDevice(deviceGroup.getGroupId(), initialTestDevice.getId(), TestDataHolder.SUPER_TENANT_ID);
@@ -245,7 +226,7 @@ public class GroupPersistTests extends BaseDeviceManagementTest {
         Assert.assertNull(group, "Group is not deleted");
     }
 
-    public DeviceGroup getGroupById(int groupId) {
+    private DeviceGroup getGroupById(int groupId) {
         try {
             GroupManagementDAOFactory.openConnection();
             return groupDAO.getGroup(groupId, TestDataHolder.SUPER_TENANT_ID);