From 94a50584f4920199c755c075200ea8cd1a9aa4e3 Mon Sep 17 00:00:00 2001 From: charitha Date: Thu, 6 Oct 2016 16:01:41 +0530 Subject: [PATCH] Improve group listing with pagination --- .../mgt/jaxrs/beans/DeviceGroupList.java | 8 +- .../impl/GroupManagementServiceImpl.java | 11 +- .../GroupManagementAdminServiceImpl.java | 11 +- .../mgt/core/group/mgt/dao/GroupDAO.java | 12 +- .../mgt/core/group/mgt/dao/GroupDAOImpl.java | 60 +++++++--- .../GroupManagementProviderService.java | 5 +- .../GroupManagementProviderServiceImpl.java | 110 ++++++++++++------ .../mgt/core/dao/GroupPersistTests.java | 27 +---- 8 files changed, 144 insertions(+), 100 deletions(-) diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/beans/DeviceGroupList.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/beans/DeviceGroupList.java index d1e472255a..3e588444da 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/beans/DeviceGroupList.java +++ b/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 deviceGroups = new ArrayList<>(); + private List deviceGroups = new ArrayList<>(); - public List getList() { + public List getList() { return deviceGroups; } - public void setList(List deviceGroups) { + public void setList(List deviceGroups) { this.deviceGroups = deviceGroups; } diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImpl.java index afae7a9495..0d9bea2e8b 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImpl.java +++ b/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 deviceGroups = service.getGroups(currentUser, request); - if (deviceGroups != null && deviceGroups.size() > 0) { + PaginationResult deviceGroupsResult = DeviceMgtAPIUtils.getGroupManagementProviderService() + .getGroups(currentUser, request); + if (deviceGroupsResult.getData() != null && deviceGroupsResult.getRecordsTotal() > 0) { DeviceGroupList deviceGroupList = new DeviceGroupList(); - deviceGroupList.setList(deviceGroups); - deviceGroupList.setCount(service.getGroupCount(currentUser)); + deviceGroupList.setList(deviceGroupsResult.getData()); + deviceGroupList.setCount(deviceGroupsResult.getRecordsTotal()); return Response.status(Response.Status.OK).entity(deviceGroupList).build(); } else { return Response.status(Response.Status.NOT_FOUND).build(); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/admin/GroupManagementAdminServiceImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/admin/GroupManagementAdminServiceImpl.java index 729f32af43..55b67029fa 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/admin/GroupManagementAdminServiceImpl.java +++ b/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 deviceGroups = service.getGroups(request); - if (deviceGroups != null && deviceGroups.size() > 0) { + PaginationResult deviceGroupsResult = DeviceMgtAPIUtils.getGroupManagementProviderService() + .getGroups(request); + if (deviceGroupsResult.getData() != null && deviceGroupsResult.getRecordsTotal() > 0) { DeviceGroupList deviceGroupList = new DeviceGroupList(); - deviceGroupList.setList(deviceGroups); - deviceGroupList.setCount(service.getGroupCount()); + deviceGroupList.setList(deviceGroupsResult.getData()); + deviceGroupList.setCount(deviceGroupsResult.getRecordsTotal()); return Response.status(Response.Status.OK).entity(deviceGroupList).build(); } else { return Response.status(Response.Status.NOT_FOUND).build(); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAO.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAO.java index f9412fcff9..2637f0b0ce 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAO.java +++ b/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 tenantId of user's tenant. - * @return List of DeviceGroup that matches with the given DeviceGroup name. + * @param paginationRequest to filter results. + * @param tenantId of user's tenant. + * @return List of all Device Groups in tenant. * @throws GroupManagementDAOException */ - List findInGroups(String groupName, int tenantId) throws GroupManagementDAOException; + int getGroupCount(GroupPaginationRequest paginationRequest, int tenantId) throws GroupManagementDAOException; /** * Check group already existed with given name. diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAOImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAOImpl.java index 2286f6dbba..0ea0007300 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/group/mgt/dao/GroupDAOImpl.java +++ b/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 findInGroups(String groupName, int tenantId) + public int getGroupCount(GroupPaginationRequest request, int tenantId) throws GroupManagementDAOException { PreparedStatement stmt = null; ResultSet resultSet = null; - List 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 " - + "FROM DM_GROUP WHERE GROUP_NAME LIKE ? AND TENANT_ID = ?"; + String sql = "SELECT COUNT(ID) AS GROUP_COUNT FROM DM_GROUP WHERE 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(2, tenantId); + stmt.setInt(paramIndex++, tenantId); + if (hasGroupName) { + stmt.setString(paramIndex++, groupName + "%"); + } + if (hasOwner) { + stmt.setString(paramIndex, owner + "%"); + } resultSet = stmt.executeQuery(); - while (resultSet.next()) { - deviceGroups.add(GroupManagementDAOUtil.loadGroup(resultSet)); + if (resultSet.next()) { + return resultSet.getInt("GROUP_COUNT"); + } else { + return 0; } } catch (SQLException e) { - throw new GroupManagementDAOException("Error occurred while listing Device Groups by name '" + - groupName + "' in tenant '" + tenantId + "'", e); + throw new GroupManagementDAOException("Error occurred while listing all groups in tenant: " + tenantId, e); } finally { GroupManagementDAOUtil.cleanupResources(stmt, resultSet); } - return deviceGroups; } @Override diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderService.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderService.java index 934e652927..ab84bb4cc5 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderService.java +++ b/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 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 getGroups(String username, GroupPaginationRequest paginationRequest) throws GroupManagementException; + PaginationResult getGroups(String username, GroupPaginationRequest paginationRequest) throws GroupManagementException; /** * Get all device group count in tenant diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceImpl.java index 012bc9b18e..11ea959225 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceImpl.java +++ b/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 getGroups() throws GroupManagementException { List deviceGroups = new ArrayList<>(); @@ -222,9 +214,21 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } @Override - public List getGroups(GroupPaginationRequest request) throws GroupManagementException { - List deviceGroups = new ArrayList<>(); + public PaginationResult getGroups(GroupPaginationRequest request) throws GroupManagementException { request = DeviceManagerUtil.validateGroupListPageSize(request); + List 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 getPlainDeviceGroups(GroupPaginationRequest request) throws GroupManagementException { + List 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 getGroups(String currentUser, GroupPaginationRequest request) throws GroupManagementException { + public PaginationResult getGroups(String currentUser, GroupPaginationRequest request) throws GroupManagementException { request = DeviceManagerUtil.validateGroupListPageSize(request); - Map groups = new HashMap<>(); - UserStoreManager userStoreManager; + int startIndex = request.getStartIndex(); + int count = request.getRowCount(); + int index = 0; + request.setRowCount(0); + List allMatchingGroups = this.getPlainDeviceGroups(request); + List 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 groupIds = new ArrayList<>(); for (String role : roleList) { if (role != null && role.contains("Internal/group-")) { - DeviceGroup deviceGroupBuilder = extractNewGroupFromRole(groups, role); - if (deviceGroupBuilder != null - && request.getStartIndex() <= index++ - && index <= request.getRowCount()) { - groups.put(deviceGroupBuilder.getGroupId(), deviceGroupBuilder); + int groupId = Integer.parseInt(role.split("-")[1]); + if (!groupIds.contains(groupId)) { + groupIds.add(groupId); } } } + 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 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 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); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/dao/GroupPersistTests.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/dao/GroupPersistTests.java index ae6bcc0126..0f58cfb6a1 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/dao/GroupPersistTests.java +++ b/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 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);