From efff1e2cfd058e9219488697964d35d697fa8a91 Mon Sep 17 00:00:00 2001 From: charitha Date: Sun, 3 Nov 2019 23:51:54 +0530 Subject: [PATCH 1/2] Fix device group related issues - Create group - Not working when group properties are not available - Delete group - SQL error - Get devices of group - Database connection management issue --- .../core/impl/SubscriptionManagerImpl.java | 4 +- .../service/api/GroupManagementService.java | 148 ++++++++++++---- .../admin/GroupManagementAdminService.java | 9 +- .../impl/GroupManagementServiceImpl.java | 32 +++- .../GroupManagementAdminServiceImpl.java | 4 +- .../impl/GroupManagementServiceImplTest.java | 36 ++-- .../DeviceAccessAuthorizationServiceImpl.java | 10 +- .../core/dao/impl/AbstractGroupDAOImpl.java | 2 +- .../DeviceManagementProviderServiceImpl.java | 4 +- .../GroupManagementProviderService.java | 43 +++-- .../GroupManagementProviderServiceImpl.java | 159 ++++++++++++------ .../DeviceAccessAuthorizationServiceTest.java | 2 +- ...ManagementProviderServiceNegativeTest.java | 14 +- .../GroupManagementProviderServiceTest.java | 50 +++--- .../core/impl/PolicyInformationPointImpl.java | 2 +- .../mgt/core/mgt/impl/PolicyManagerImpl.java | 2 +- .../mgt/core/BasePolicyManagementDAOTest.java | 2 +- .../core/PolicyManagerServiceImplTest.java | 2 +- .../core/mgt/impl/FeatureManagerImplTest.java | 2 +- .../mgt/impl/MonitoringManagerImplTest.java | 2 +- .../core/mgt/impl/ProfileManagerImplTest.java | 2 +- 21 files changed, 350 insertions(+), 181 deletions(-) diff --git a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java index 892acc28e6..6e0388c843 100644 --- a/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java +++ b/components/application-mgt/org.wso2.carbon.device.application.mgt.core/src/main/java/org/wso2/carbon/device/application/mgt/core/impl/SubscriptionManagerImpl.java @@ -171,7 +171,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager { for (T param : params) { String groupName = (String) param; subscribers.add(groupName); - devices.addAll(groupManagementProviderService.getAllDevicesOfGroup(groupName)); + devices.addAll(groupManagementProviderService.getAllDevicesOfGroup(groupName, true)); } } @@ -401,7 +401,7 @@ public class SubscriptionManagerImpl implements SubscriptionManager { for (T param : params) { String groupName = (String) param; subscribers.add(groupName); - devices.addAll(groupManagementProviderService.getAllDevicesOfGroup(groupName)); + devices.addAll(groupManagementProviderService.getAllDevicesOfGroup(groupName, false)); } } else { String msg = "Found invalid subscription type " + subType+ " to install application release" ; diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java index 9ea2e4b2bb..4880231574 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java @@ -212,29 +212,36 @@ public interface GroupManagementService { message = "Internal Server Error. \n Server error occurred while fetching the groups list.", response = ErrorResponse.class) }) - Response getGroups(@ApiParam( - name = "name", - value = "Name of the group.") - @QueryParam("name") - String name, - @ApiParam( - name = "owner", - value = "Owner of the group.") - @QueryParam("owner") - String owner, - @ApiParam( - name = "offset", - value = "The starting pagination index for the complete list of qualified items.", - defaultValue = "0") - @QueryParam("offset") - int offset, - @ApiParam( - name = "limit", - value = "Provide how many device details you require from the starting pagination " + - "index/offset.", - defaultValue = "5") - @QueryParam("limit") - int limit); + Response getGroups( + @ApiParam( + name = "name", + value = "Name of the group.") + @QueryParam("name") + String name, + @ApiParam( + name = "owner", + value = "Owner of the group.") + @QueryParam("owner") + String owner, + @ApiParam( + name = "offset", + value = "The starting pagination index for the complete list of qualified items.", + defaultValue = "0") + @QueryParam("offset") + int offset, + @ApiParam( + name = "limit", + value = "Provide how many device details you require from the starting pagination " + + "index/offset.", + defaultValue = "5") + @QueryParam("limit") + int limit, + @ApiParam( + name = "requireGroupProps", + value = "Request group properties to include in the response", + defaultValue = "false") + @QueryParam("requireGroupProps") + boolean requireGroupProps); @Path("/count") @GET @@ -394,11 +401,77 @@ public interface GroupManagementService { message = "Internal Server Error. \n Server error occurred while fetching the group details.", response = ErrorResponse.class) }) - Response getGroup(@ApiParam( - name = "groupId", - value = "The ID of the group.", - required = true) - @PathParam("groupId") int groupId); + Response getGroup( + @ApiParam( + name = "groupId", + value = "The ID of the group.", + required = true) + @PathParam("groupId") int groupId, + @ApiParam( + name = "requireGroupProps", + value = "Request group properties to include in the response", + defaultValue = "false") + @QueryParam("requireGroupProps") + boolean requireGroupProps); + + @Path("/name/{groupName}") + @GET + @ApiOperation( + produces = MediaType.APPLICATION_JSON, + httpMethod = HTTPConstants.HEADER_GET, + value = "Getting Details of a Specific Device Group", + notes = "Get the details of a specific device group.", + tags = "Device Group Management", + extensions = { + @Extension(properties = { + @ExtensionProperty(name = Constants.SCOPE, value = "perm:groups:groups-view") + }) + } + ) + @ApiResponses(value = { + @ApiResponse(code = 200, message = "OK. \n Successfully fetched the device group.", + response = DeviceGroup.class, + responseHeaders = { + @ResponseHeader( + name = "Content-Type", + description = "The content type of the body"), + @ResponseHeader( + name = "ETag", + description = "Entity Tag of the response resource.\n" + + "Used by caches, or in conditional requests."), + @ResponseHeader( + name = "Last-Modified", + description = "Date and time the resource has been modified the last time.\n" + + "Used by caches, or in conditional requests."), + }), + @ApiResponse( + code = 304, + message = "Not Modified. \n Empty body because the client has already the latest version of " + + "the requested resource."), + @ApiResponse( + code = 404, + message = "Group found.", + response = ErrorResponse.class), + @ApiResponse( + code = 406, + message = "Not Acceptable.\n The requested media type is not supported."), + @ApiResponse( + code = 500, + message = "Internal Server Error. \n Server error occurred while fetching the group details.", + response = ErrorResponse.class) + }) + Response getGroup( + @ApiParam( + name = "groupName", + value = "Name of the group.", + required = true) + @PathParam("groupName") String groupName, + @ApiParam( + name = "requireGroupProps", + value = "Request group properties to include in the response", + defaultValue = "false") + @QueryParam("requireGroupProps") + boolean requireGroupProps); @Path("/id/{groupId}") @PUT @@ -739,11 +812,12 @@ public interface GroupManagementService { message = "Internal Server Error. \n Server error occurred while fetching device count.", response = ErrorResponse.class) }) - Response getDeviceCountOfGroup(@ApiParam( - name = "groupId", - value = "ID of the group.", - required = true) - @PathParam("groupId") int groupId); + Response getDeviceCountOfGroup( + @ApiParam( + name = "groupId", + value = "ID of the group.", + required = true) + @PathParam("groupId") int groupId); @Path("/id/{groupId}/devices/add") @POST @@ -966,6 +1040,12 @@ public interface GroupManagementService { value = "The type of the device, such as android, ios, or windows.", required = true) @QueryParam("deviceType") - String deviceType); + String deviceType, + @ApiParam( + name = "requireGroupProps", + value = "Request group properties to include in the response", + defaultValue = "false") + @QueryParam("requireGroupProps") + boolean requireGroupProps); } diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/admin/GroupManagementAdminService.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/admin/GroupManagementAdminService.java index 304a00d380..f631b1dc51 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/admin/GroupManagementAdminService.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/admin/GroupManagementAdminService.java @@ -151,12 +151,17 @@ public interface GroupManagementAdminService { defaultValue = "5") @QueryParam("limit") int limit, - @ApiParam( name = "status", value = "status of group to be retrieve.") @QueryParam("status") - String status); + String status, + @ApiParam( + name = "requireGroupProps", + value = "Request group properties to include in the response", + defaultValue = "false") + @QueryParam("requireGroupProps") + boolean requireGroupProps); @Path("/count") @GET 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 2c11397b9a..3b10b8681d 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 @@ -56,7 +56,7 @@ public class GroupManagementServiceImpl implements GroupManagementService { "/permission/device-mgt/user/groups"}; @Override - public Response getGroups(String name, String owner, int offset, int limit) { + public Response getGroups(String name, String owner, int offset, int limit, boolean requireGroupProps) { try { RequestValidationUtil.validatePaginationParameters(offset, limit); String currentUser = PrivilegedCarbonContext.getThreadLocalCarbonContext().getUsername(); @@ -64,7 +64,7 @@ public class GroupManagementServiceImpl implements GroupManagementService { request.setGroupName(name); request.setOwner(owner); PaginationResult deviceGroupsResult = DeviceMgtAPIUtils.getGroupManagementProviderService() - .getGroups(currentUser, request); + .getGroups(currentUser, request, requireGroupProps); DeviceGroupList deviceGroupList = new DeviceGroupList(); if (deviceGroupsResult.getData() != null && deviceGroupsResult.getRecordsTotal() > 0) { deviceGroupList.setList(deviceGroupsResult.getData()); @@ -117,10 +117,27 @@ public class GroupManagementServiceImpl implements GroupManagementService { } @Override - public Response getGroup(int groupId) { + public Response getGroup(int groupId, boolean requireGroupProps) { try { GroupManagementProviderService service = DeviceMgtAPIUtils.getGroupManagementProviderService(); - DeviceGroup deviceGroup = service.getGroup(groupId); + DeviceGroup deviceGroup = service.getGroup(groupId, requireGroupProps); + if (deviceGroup != null) { + return Response.status(Response.Status.OK).entity(deviceGroup).build(); + } else { + return Response.status(Response.Status.NOT_FOUND).build(); + } + } catch (GroupManagementException e) { + String error = "Error occurred while getting the group."; + log.error(error, e); + return Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(error).build(); + } + } + + @Override + public Response getGroup(String groupName, boolean requireGroupProps) { + try { + GroupManagementProviderService service = DeviceMgtAPIUtils.getGroupManagementProviderService(); + DeviceGroup deviceGroup = service.getGroup(groupName, requireGroupProps); if (deviceGroup != null) { return Response.status(Response.Status.OK).entity(deviceGroup).build(); } else { @@ -272,7 +289,7 @@ public class GroupManagementServiceImpl implements GroupManagementService { List deviceIdentifiers = new ArrayList<>(); deviceIdentifiers.add(deviceToGroupsAssignment.getDeviceIdentifier()); GroupManagementProviderService service = DeviceMgtAPIUtils.getGroupManagementProviderService(); - List deviceGroups = service.getGroups(deviceToGroupsAssignment.getDeviceIdentifier()); + List deviceGroups = service.getGroups(deviceToGroupsAssignment.getDeviceIdentifier(), false); for (DeviceGroup group : deviceGroups) { Integer groupId = group.getGroupId(); if (deviceToGroupsAssignment.getDeviceGroupIds().contains(groupId)) { @@ -295,10 +312,11 @@ public class GroupManagementServiceImpl implements GroupManagementService { } @Override - public Response getGroups(String deviceId, String deviceType) { + public Response getGroups(String deviceId, String deviceType, boolean requireGroupProps) { try { DeviceIdentifier deviceIdentifier = new DeviceIdentifier(deviceId, deviceType); - List deviceGroups = DeviceMgtAPIUtils.getGroupManagementProviderService().getGroups(deviceIdentifier); + List deviceGroups = DeviceMgtAPIUtils.getGroupManagementProviderService() + .getGroups(deviceIdentifier, requireGroupProps); return Response.status(Response.Status.OK).entity(deviceGroups).build(); } catch (GroupManagementException e) { String msg = "Error occurred while getting groups of device."; 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 1971890b43..b742a1d8e1 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 @@ -43,7 +43,7 @@ public class GroupManagementAdminServiceImpl implements GroupManagementAdminServ "/permission/device-mgt/user/groups"}; @Override - public Response getGroups(String name, String owner, int offset, int limit, String status) { + public Response getGroups(String name, String owner, int offset, int limit, String status, boolean requireGroupProps) { try { RequestValidationUtil.validatePaginationParameters(offset, limit); GroupPaginationRequest request = new GroupPaginationRequest(offset, limit); @@ -57,7 +57,7 @@ public class GroupManagementAdminServiceImpl implements GroupManagementAdminServ request.setStatus(status.toUpperCase()); } PaginationResult deviceGroupsResult = DeviceMgtAPIUtils.getGroupManagementProviderService() - .getGroups(request); + .getGroups(request, requireGroupProps); DeviceGroupList deviceGroupList = new DeviceGroupList(); if (deviceGroupsResult.getData() != null) { deviceGroupList.setList(deviceGroupsResult.getData()); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/test/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImplTest.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/test/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImplTest.java index 35f3650273..d1bcbb27eb 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/test/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImplTest.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/test/java/org/wso2/carbon/device/mgt/jaxrs/service/impl/GroupManagementServiceImplTest.java @@ -84,8 +84,8 @@ public class GroupManagementServiceImplTest { .toReturn(context); PaginationResult paginationResult = new PaginationResult(); Mockito.doReturn(paginationResult).when(groupManagementProviderService) - .getGroups(Mockito.anyString(), Mockito.any(GroupPaginationRequest.class)); - Response response = groupManagementService.getGroups("test", "admin", 0, 10); + .getGroups(Mockito.anyString(), Mockito.any(GroupPaginationRequest.class), Mockito.anyBoolean()); + Response response = groupManagementService.getGroups("test", "admin", 0, 10, false); Assert.assertEquals(response.getStatus(), Response.Status.OK.getStatusCode(), "GetGroups request failed with valid parameters"); @@ -95,8 +95,8 @@ public class GroupManagementServiceImplTest { paginationResult.setData(deviceGroupList); paginationResult.setRecordsTotal(1); Mockito.doReturn(paginationResult).when(groupManagementProviderService) - .getGroups(Mockito.anyString(), Mockito.any(GroupPaginationRequest.class)); - response = groupManagementService.getGroups("test", "admin", 0, 10); + .getGroups(Mockito.anyString(), Mockito.any(GroupPaginationRequest.class), Mockito.anyBoolean()); + response = groupManagementService.getGroups("test", "admin", 0, 10, false); Assert.assertEquals(response.getStatus(), Response.Status.OK.getStatusCode(), "GetGroups request failed with valid parameters"); } @@ -110,8 +110,8 @@ public class GroupManagementServiceImplTest { .toReturn(context); Mockito.reset(groupManagementProviderService); Mockito.doThrow(new GroupManagementException()).when(groupManagementProviderService) - .getGroups(Mockito.anyString(), Mockito.any(GroupPaginationRequest.class)); - Response response = groupManagementService.getGroups("test", "admin", 0, 10); + .getGroups(Mockito.anyString(), Mockito.any(GroupPaginationRequest.class), Mockito.anyBoolean()); + Response response = groupManagementService.getGroups("test", "admin", 0, 10, false); Assert.assertEquals(response.getStatus(), Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), "GetGroups request succeeded with in-valid parameters"); } @@ -167,16 +167,16 @@ public class GroupManagementServiceImplTest { public void testGetGroup() throws GroupManagementException { PowerMockito.stub(PowerMockito.method(DeviceMgtAPIUtils.class, "getGroupManagementProviderService")) .toReturn(groupManagementProviderService); - Mockito.doReturn(new DeviceGroup()).when(groupManagementProviderService).getGroup(1); - Mockito.doReturn(null).when(groupManagementProviderService).getGroup(2); - Mockito.doThrow(new GroupManagementException()).when(groupManagementProviderService).getGroup(3); - Response response = groupManagementService.getGroup(1); + Mockito.doReturn(new DeviceGroup()).when(groupManagementProviderService).getGroup(1, false); + Mockito.doReturn(null).when(groupManagementProviderService).getGroup(2, false); + Mockito.doThrow(new GroupManagementException()).when(groupManagementProviderService).getGroup(3, false); + Response response = groupManagementService.getGroup(1, false); Assert.assertEquals(response.getStatus(), Response.Status.OK.getStatusCode(), "getGroup request failed for a request with valid parameters"); - response = groupManagementService.getGroup(2); + response = groupManagementService.getGroup(2, false); Assert.assertEquals(response.getStatus(), Response.Status.NOT_FOUND.getStatusCode(), "getGroup request returned a group for a non-existing group"); - response = groupManagementService.getGroup(3); + response = groupManagementService.getGroup(3, false); Assert.assertEquals(response.getStatus(), Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), "getGroup request returned a group for a in-valid request"); } @@ -341,14 +341,14 @@ public class GroupManagementServiceImplTest { PowerMockito.stub(PowerMockito.method(DeviceMgtAPIUtils.class, "getGroupManagementProviderService")) .toReturn(groupManagementProviderService); DeviceIdentifier deviceIdentifier = new DeviceIdentifier("test", "android"); - Mockito.doReturn(new ArrayList()).when(groupManagementProviderService).getGroups(deviceIdentifier); - Response response = groupManagementService.getGroups("test", "android"); + Mockito.doReturn(new ArrayList()).when(groupManagementProviderService).getGroups(deviceIdentifier, true); + Response response = groupManagementService.getGroups("test", "android", false); Assert.assertEquals(response.getStatus(), Response.Status.OK.getStatusCode(), "getGroups request failed with valid parameters"); Mockito.reset(groupManagementProviderService); Mockito.doThrow(new GroupManagementException()).when(groupManagementProviderService) - .getGroups(Mockito.any(DeviceIdentifier.class)); - response = groupManagementService.getGroups("test", "android2"); + .getGroups(Mockito.any(DeviceIdentifier.class), Mockito.anyBoolean()); + response = groupManagementService.getGroups("test", "android2", false); Assert.assertEquals(response.getStatus(), Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), "getGroups request succeeded with in-valid parameters"); } @@ -377,7 +377,7 @@ public class GroupManagementServiceImplTest { deviceGroup.setOwner("test"); deviceGroups.add(deviceGroup); Mockito.doReturn(deviceGroups).when(groupManagementProviderService) - .getGroups(Mockito.any(DeviceIdentifier.class)); + .getGroups(Mockito.any(DeviceIdentifier.class), Mockito.anyBoolean()); Mockito.doNothing().when(groupManagementProviderService).addDevices(Mockito.anyInt(), Mockito.any()); Mockito.doNothing().when(groupManagementProviderService).removeDevice(Mockito.anyInt(), Mockito.any()); Response response = groupManagementService.updateDeviceAssigningToGroups(deviceToGroupsAssignment); @@ -389,7 +389,7 @@ public class GroupManagementServiceImplTest { Assert.assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode(), "updateDeviceAssigningToGroups request succeeded with in-valid parameters"); Mockito.doThrow(new GroupManagementException()).when(groupManagementProviderService) - .getGroups(Mockito.any(DeviceIdentifier.class)); + .getGroups(Mockito.any(DeviceIdentifier.class), Mockito.anyBoolean()); response = groupManagementService.updateDeviceAssigningToGroups(deviceToGroupsAssignment); Assert.assertEquals(response.getStatus(), Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), "updateDeviceAssigningToGroups request succeeded with in-valid parameters"); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceImpl.java index bdc6f54e16..5db998544f 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceImpl.java @@ -79,7 +79,7 @@ public class DeviceAccessAuthorizationServiceImpl implements DeviceAccessAuthori } } return true; - } catch (GroupManagementException | UserStoreException e) { + } catch (GroupManagementException e) { throw new DeviceAccessAuthorizationException("Unable to authorize the access to device : " + deviceIdentifier.getId() + " for the user : " + username, e); @@ -152,7 +152,7 @@ public class DeviceAccessAuthorizationServiceImpl implements DeviceAccessAuthori } else { deviceAuthorizationResult.addUnauthorizedDevice(deviceIdentifier); } - } catch (GroupManagementException | UserStoreException e) { + } catch (GroupManagementException e) { throw new DeviceAccessAuthorizationException("Unable to authorize the access to device : " + deviceIdentifier.getId() + " for the user : " + username, e); @@ -192,13 +192,13 @@ public class DeviceAccessAuthorizationServiceImpl implements DeviceAccessAuthori } private boolean isAuthorizedViaGroup(String username, DeviceIdentifier deviceIdentifier, String groupPermission) - throws GroupManagementException, UserStoreException { + throws GroupManagementException { List authorizedGroups = DeviceManagementDataHolder.getInstance().getGroupManagementProviderService() - .getGroups(username, groupPermission); + .getGroups(username, groupPermission, false); List groupsWithDevice = DeviceManagementDataHolder.getInstance().getGroupManagementProviderService() - .getGroups(deviceIdentifier); + .getGroups(deviceIdentifier, false); for (DeviceGroup group : authorizedGroups) { Iterator groupsWithDeviceIterator = groupsWithDevice.iterator(); while (groupsWithDeviceIterator.hasNext()) { diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/impl/AbstractGroupDAOImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/impl/AbstractGroupDAOImpl.java index d4207c4047..f83d54baab 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/impl/AbstractGroupDAOImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/dao/impl/AbstractGroupDAOImpl.java @@ -216,7 +216,7 @@ public abstract class AbstractGroupDAOImpl implements GroupDAO { try { Connection conn = GroupManagementDAOFactory.getConnection(); stmt = conn.prepareStatement( - "DELETE GROUP_PROPERTIES WHERE GROUP_ID = ? AND TENANT_ID = ?"); + "DELETE FROM GROUP_PROPERTIES WHERE GROUP_ID = ? AND TENANT_ID = ?"); stmt.setInt(1, groupId); stmt.setInt(2, tenantId); stmt.executeUpdate(); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/DeviceManagementProviderServiceImpl.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/DeviceManagementProviderServiceImpl.java index a2f9a9769e..1efdb78a74 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/DeviceManagementProviderServiceImpl.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/main/java/org/wso2/carbon/device/mgt/core/service/DeviceManagementProviderServiceImpl.java @@ -2789,7 +2789,7 @@ public class DeviceManagementProviderServiceImpl implements DeviceManagementProv if (log.isDebugEnabled()) { log.debug("Create default group with name '" + groupName + "'"); } - DeviceGroup defaultGroup = service.getGroup(groupName); + DeviceGroup defaultGroup = service.getGroup(groupName, false); if (defaultGroup == null) { defaultGroup = new DeviceGroup(groupName); // Setting system level user (wso2.system.user) as the owner @@ -2808,7 +2808,7 @@ public class DeviceManagementProviderServiceImpl implements DeviceManagementProv log.error(msg, e); throw new GroupManagementException(msg, e); } - return service.getGroup(groupName); + return service.getGroup(groupName, false); } else { return defaultGroup; } 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 6951862a77..4300ce232b 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 @@ -69,55 +69,63 @@ public interface GroupManagementProviderService { * Get the device group provided the device group id. * * @param groupId of the group. + * @param requireGroupProps to include group properties * @return group with details. * @throws GroupManagementException */ - DeviceGroup getGroup(int groupId) throws GroupManagementException; + DeviceGroup getGroup(int groupId, boolean requireGroupProps) throws GroupManagementException; /** * Get the device group provided the device group name. * * @param groupName of the group. + * @param requireGroupProps to include group properties * @return group with details. * @throws GroupManagementException */ - DeviceGroup getGroup(String groupName) throws GroupManagementException; + DeviceGroup getGroup(String groupName, boolean requireGroupProps) throws GroupManagementException; /** * Get all device groups in tenant. * + * @param requireGroupProps to include group properties * @return list of groups. * @throws GroupManagementException */ - List getGroups() throws GroupManagementException; + List getGroups(boolean requireGroupProps) throws GroupManagementException; /** * Get all device groups for user. * * @param username of the user. + * @param requireGroupProps to include group properties * @return list of groups * @throws GroupManagementException */ - List getGroups(String username) throws GroupManagementException; + List getGroups(String username, boolean requireGroupProps) throws GroupManagementException; /** * Get device groups with pagination. * * @param paginationRequest to filter results + * @param requireGroupProps to include group properties * @return list of groups. * @throws GroupManagementException */ - PaginationResult getGroups(GroupPaginationRequest paginationRequest) throws GroupManagementException; + PaginationResult getGroups(GroupPaginationRequest paginationRequest, boolean requireGroupProps) + throws GroupManagementException; /** * Get device groups belongs to specified user with pagination. * * @param username of the user. * @param paginationRequest to filter results + * @param requireGroupProps to include group properties * @return list of groups. * @throws GroupManagementException */ - PaginationResult getGroups(String username, GroupPaginationRequest paginationRequest) throws GroupManagementException; + PaginationResult getGroups(String username, GroupPaginationRequest paginationRequest, boolean requireGroupProps) + throws GroupManagementException; /** * Get all device group count in tenant @@ -169,12 +177,22 @@ public interface GroupManagementProviderService { * @param groupId of the group * @param startIndex for pagination. * @param rowCount for pagination. + * @param requireDeviceProps to include device properties. * @return list of devices in group. * @throws GroupManagementException */ - List getDevices(int groupId, int startIndex, int rowCount, boolean requireDeviceProps) throws GroupManagementException; + List getDevices(int groupId, int startIndex, int rowCount, boolean requireDeviceProps) + throws GroupManagementException; - List getAllDevicesOfGroup(String groupName) throws GroupManagementException; + /** + * Get all devices in device group as paginated result. + * + * @param groupName of the group. + * @param requireDeviceProps to include device properties. + * @return list of devices in group. + * @throws GroupManagementException + */ + List getAllDevicesOfGroup(String groupName, boolean requireDeviceProps) throws GroupManagementException; /** @@ -203,8 +221,8 @@ public interface GroupManagementProviderService { * @param deviceIdentifiers of devices. * @throws GroupManagementException */ - void removeDevice(int groupId, List deviceIdentifiers) throws GroupManagementException, - DeviceNotFoundException; + void removeDevice(int groupId, List deviceIdentifiers) + throws GroupManagementException, DeviceNotFoundException; /** * Get device groups of user with permission. * @@ -213,7 +231,8 @@ public interface GroupManagementProviderService { * @return group list with specified permissions. * @throws GroupManagementException */ - List getGroups(String username, String permission) throws GroupManagementException; + List getGroups(String username, String permission, boolean requireGroupProps) + throws GroupManagementException; /** * Get groups which contains particular device. @@ -222,7 +241,7 @@ public interface GroupManagementProviderService { * @return groups contain the device. * @throws GroupManagementException */ - List getGroups(DeviceIdentifier deviceIdentifier) throws GroupManagementException; + List getGroups(DeviceIdentifier deviceIdentifier, boolean requireGroupProps) throws GroupManagementException; /** * Checks for the default group existence and create group based on device ownership. 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 457e1217c1..f3c3ab2c82 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 @@ -174,14 +174,15 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid if (log.isDebugEnabled()) { log.debug("Delete group: " + groupId); } - DeviceGroup deviceGroup = getGroup(groupId); + DeviceGroup deviceGroup = getGroup(groupId, false); if (deviceGroup == null) { return false; } + int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); try { GroupManagementDAOFactory.beginTransaction(); - this.groupDAO.deleteGroup(groupId, CarbonContext.getThreadLocalCarbonContext().getTenantId()); - this.groupDAO.deleteAllGroupProperties(groupId, CarbonContext.getThreadLocalCarbonContext().getTenantId()); + this.groupDAO.deleteGroup(groupId, tenantId); + this.groupDAO.deleteAllGroupProperties(groupId, tenantId); GroupManagementDAOFactory.commitTransaction(); if (log.isDebugEnabled()) { log.debug("DeviceGroup " + deviceGroup.getName() + " removed."); @@ -209,18 +210,19 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid * {@inheritDoc} */ @Override - public DeviceGroup getGroup(int groupId) throws GroupManagementException { + public DeviceGroup getGroup(int groupId, boolean requireGroupProps) throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Get group by id: " + groupId); } DeviceGroup deviceGroup; + int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); try { GroupManagementDAOFactory.openConnection(); - deviceGroup = this.groupDAO.getGroup(groupId, CarbonContext.getThreadLocalCarbonContext().getTenantId()); - if(deviceGroup != null && deviceGroup.getGroupId() > 0) { - deviceGroup.setGroupProperties(this.groupDAO.getAllGroupProperties(deviceGroup.getGroupId(), - CarbonContext.getThreadLocalCarbonContext().getTenantId())); + deviceGroup = this.groupDAO.getGroup(groupId, tenantId); + if (requireGroupProps) { + populateGroupProperties(deviceGroup, tenantId); } + return deviceGroup; } catch (GroupManagementDAOException e) { String msg = "Error occurred while obtaining group '" + groupId + "'"; log.error(msg, e); @@ -236,14 +238,13 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } finally { GroupManagementDAOFactory.closeConnection(); } - return deviceGroup; } /** * {@inheritDoc} */ @Override - public DeviceGroup getGroup(String groupName) throws GroupManagementException { + public DeviceGroup getGroup(String groupName, boolean requireGroupProps) throws GroupManagementException { if (groupName == null) { String msg = "Received empty groupName for getGroup"; log.error(msg); @@ -253,13 +254,14 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid log.debug("Get group by name '" + groupName + "'"); } DeviceGroup deviceGroup; + int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); try { GroupManagementDAOFactory.openConnection(); - deviceGroup = this.groupDAO.getGroup(groupName, CarbonContext.getThreadLocalCarbonContext().getTenantId()); - if(deviceGroup != null && deviceGroup.getGroupId() > 0) { - deviceGroup.setGroupProperties(this.groupDAO.getAllGroupProperties(deviceGroup.getGroupId(), - CarbonContext.getThreadLocalCarbonContext().getTenantId())); + deviceGroup = this.groupDAO.getGroup(groupName, tenantId); + if (requireGroupProps) { + populateGroupProperties(deviceGroup, tenantId); } + return deviceGroup; } catch (GroupManagementDAOException e) { String msg = "Error occurred while obtaining group with name: '" + groupName + "'"; log.error(msg, e); @@ -275,11 +277,10 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } finally { GroupManagementDAOFactory.closeConnection(); } - return deviceGroup; } @Override - public List getGroups() throws GroupManagementException { + public List getGroups(boolean requireGroupProps) throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Get groups"); } @@ -288,11 +289,14 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); GroupManagementDAOFactory.openConnection(); deviceGroups = this.groupDAO.getGroups(tenantId); - if(deviceGroups != null && !deviceGroups.isEmpty()) { - for (DeviceGroup group : deviceGroups) { - group.setGroupProperties(this.groupDAO.getAllGroupProperties(group.getGroupId(), tenantId)); + if (requireGroupProps) { + if (deviceGroups != null && !deviceGroups.isEmpty()) { + for (DeviceGroup group : deviceGroups) { + populateGroupProperties(group, tenantId); + } } } + return deviceGroups; } catch (GroupManagementDAOException e) { String msg = "Error occurred while retrieving all groups in tenant"; log.error(msg, e); @@ -308,11 +312,11 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } finally { GroupManagementDAOFactory.closeConnection(); } - return deviceGroups; } @Override - public PaginationResult getGroups(GroupPaginationRequest request) throws GroupManagementException { + public PaginationResult getGroups(GroupPaginationRequest request, boolean requireGroupProps) + throws GroupManagementException { if (request == null) { String msg = "Received incomplete data for getGroup"; log.error(msg); @@ -327,9 +331,11 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); GroupManagementDAOFactory.openConnection(); deviceGroups = this.groupDAO.getGroups(request, tenantId); - if(deviceGroups != null && !deviceGroups.isEmpty()) { - for (DeviceGroup group : deviceGroups) { - group.setGroupProperties(this.groupDAO.getAllGroupProperties(group.getGroupId(), tenantId)); + if (requireGroupProps) { + if (deviceGroups != null && !deviceGroups.isEmpty()) { + for (DeviceGroup group : deviceGroups) { + populateGroupProperties(group, tenantId); + } } } } catch (GroupManagementDAOException e) { @@ -354,7 +360,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } @Override - public List getGroups(String username) throws GroupManagementException { + public List getGroups(String username, boolean requireGroupProps) throws GroupManagementException { if (username == null || username.isEmpty()) { String msg = "Received null user name for getGroups"; log.error(msg); @@ -364,6 +370,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid log.debug("Get groups of owner '" + username + "'"); } Map groups = new HashMap<>(); + List mergedGroups = new ArrayList<>(); UserStoreManager userStoreManager; try { int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); @@ -377,11 +384,16 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } deviceGroups = this.groupDAO.getGroups(roleList, tenantId); for (DeviceGroup deviceGroup : deviceGroups) { - if(deviceGroup != null && deviceGroup.getGroupId() > 0) { - deviceGroup.setGroupProperties(this.groupDAO.getAllGroupProperties(deviceGroup.getGroupId(), tenantId)); - } groups.put(deviceGroup.getGroupId(), deviceGroup); } + if (requireGroupProps) { + for (DeviceGroup deviceGroup : groups.values()) { + populateGroupProperties(deviceGroup, tenantId); + mergedGroups.add(deviceGroup); + } + } else { + mergedGroups.addAll(groups.values()); + } } catch (UserStoreException | SQLException | GroupManagementDAOException e) { String msg = "Error occurred while retrieving all groups accessible to user."; log.error(msg, e); @@ -393,7 +405,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } finally { GroupManagementDAOFactory.closeConnection(); } - return new ArrayList<>(groups.values()); + return mergedGroups; } private List getGroupIds(String username) throws GroupManagementException { @@ -425,7 +437,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } @Override - public PaginationResult getGroups(String currentUser, GroupPaginationRequest request) + public PaginationResult getGroups(String currentUser, GroupPaginationRequest request, boolean requireGroupProps) throws GroupManagementException { if (currentUser == null || request == null) { String msg = "Received incomplete date for getGroups"; @@ -442,9 +454,11 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); GroupManagementDAOFactory.openConnection(); allMatchingGroups = this.groupDAO.getGroups(request, allDeviceGroupIdsOfUser, tenantId); - if(allMatchingGroups != null && !allMatchingGroups.isEmpty()) { - for (DeviceGroup group : allMatchingGroups) { - group.setGroupProperties(this.groupDAO.getAllGroupProperties(group.getGroupId(), tenantId)); + if (requireGroupProps) { + if (allMatchingGroups != null && !allMatchingGroups.isEmpty()) { + for (DeviceGroup group : allMatchingGroups) { + populateGroupProperties(group, tenantId); + } } } } catch (GroupManagementDAOException | SQLException e) { @@ -485,6 +499,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid GroupManagementDAOFactory.closeConnection(); } } + @Override public int getGroupCountByStatus(String status) throws GroupManagementException { if (log.isDebugEnabled()) { @@ -497,7 +512,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid return groupDAO.getGroupCount(tenantId, status); } catch (GroupManagementDAOException | SQLException e) { String msg = "Error occurred while retrieving all groups in tenant " + tenantId - +" by status : " + status; + + " by status : " + status; log.error(msg, e); throw new GroupManagementException(msg, e); } catch (Exception e) { @@ -554,7 +569,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid count = groupDAO.getOwnGroupsCount(username, tenantId); count += groupDAO.getGroupsCount(roleList, tenantId); return count; - } catch (UserStoreException | GroupManagementDAOException | SQLException e) { + } catch (UserStoreException | GroupManagementDAOException | SQLException e) { String msg = "Error occurred while retrieving group count of user '" + username + "'"; log.error(msg, e); throw new GroupManagementException(msg, e); @@ -572,7 +587,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid */ @Override public void manageGroupSharing(int groupId, List newRoles) - throws GroupManagementException, RoleDoesNotExistException { + throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Manage group sharing for group: " + groupId); } @@ -665,8 +680,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid rowCount = DeviceManagerUtil.validateDeviceListPageSize(rowCount); GroupManagementDAOFactory.openConnection(); devices = this.groupDAO.getDevices(groupId, startIndex, rowCount, tenantId); - - if(requireDeviceProps) { + if (requireDeviceProps) { DeviceManagementDAOFactory.openConnection(); for (Device device : devices) { Device retrievedDevice = deviceDAO.getDeviceProps(device.getDeviceIdentifier(), tenantId); @@ -685,15 +699,16 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid throw new GroupManagementException(msg, e); } finally { GroupManagementDAOFactory.closeConnection(); - if(requireDeviceProps){ + if (requireDeviceProps) { DeviceManagementDAOFactory.closeConnection(); } } return devices; } - public List getAllDevicesOfGroup(String groupName) throws GroupManagementException { - + @Override + public List getAllDevicesOfGroup(String groupName, boolean requireDeviceProps) + throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Group devices of group: " + groupName); } @@ -702,6 +717,15 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid try { GroupManagementDAOFactory.openConnection(); devices = this.groupDAO.getAllDevicesOfGroup(groupName, tenantId); + if (requireDeviceProps) { + DeviceManagementDAOFactory.openConnection(); + for (Device device : devices) { + Device retrievedDevice = deviceDAO.getDeviceProps(device.getDeviceIdentifier(), tenantId); + if (retrievedDevice != null && !retrievedDevice.getProperties().isEmpty()) { + device.setProperties(retrievedDevice.getProperties()); + } + } + } } catch (GroupManagementDAOException | SQLException e) { String msg = "Error occurred while getting devices in group."; log.error(msg, e); @@ -712,6 +736,9 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid throw new GroupManagementException(msg, e); } finally { GroupManagementDAOFactory.closeConnection(); + if (requireDeviceProps) { + DeviceManagementDAOFactory.closeConnection(); + } } return devices; } @@ -745,7 +772,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid */ @Override public void addDevices(int groupId, List deviceIdentifiers) - throws GroupManagementException, DeviceNotFoundException { + throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Group devices to the group: " + groupId); } @@ -791,7 +818,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid */ @Override public void removeDevice(int groupId, List deviceIdentifiers) - throws GroupManagementException, DeviceNotFoundException { + throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Remove devices from the group: " + groupId); } @@ -834,11 +861,12 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid * {@inheritDoc} */ @Override - public List getGroups(String username, String permission) throws GroupManagementException { + public List getGroups(String username, String permission, boolean requireGroupProps) + throws GroupManagementException { if (log.isDebugEnabled()) { log.debug("Get groups of user '" + username + "'"); } - List deviceGroups = getGroups(username); + List deviceGroups = getGroups(username, false); Map permittedDeviceGroups = new HashMap<>(); UserRealm userRealm; int tenantId = CarbonContext.getThreadLocalCarbonContext().getTenantId(); @@ -849,6 +877,9 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid for (String roleName : roles) { if (userRealm.getAuthorizationManager(). isRoleAuthorized(roleName, permission, CarbonConstants.UI_PERMISSION_ACTION)) { + if (requireGroupProps) { + populateGroupProperties(deviceGroup, tenantId); + } permittedDeviceGroups.put(deviceGroup.getGroupId(), deviceGroup); } } @@ -866,7 +897,8 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } @Override - public List getGroups(DeviceIdentifier deviceIdentifier) throws GroupManagementException { + public List getGroups(DeviceIdentifier deviceIdentifier, boolean requireGroupProps) + throws GroupManagementException { if (deviceIdentifier == null) { String msg = "Received empty device identifier for getGroups"; log.error(msg); @@ -875,12 +907,20 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid if (log.isDebugEnabled()) { log.debug("Get groups of device " + deviceIdentifier.getId()); } + int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId(); DeviceManagementProviderService managementProviderService = new DeviceManagementProviderServiceImpl(); try { Device device = managementProviderService.getDevice(deviceIdentifier, false); GroupManagementDAOFactory.openConnection(); - return groupDAO.getGroups(device.getId(), - PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId()); + List deviceGroups = groupDAO.getGroups(device.getId(), tenantId); + if (requireGroupProps) { + if (deviceGroups != null && !deviceGroups.isEmpty()) { + for (DeviceGroup group : deviceGroups) { + populateGroupProperties(group, tenantId); + } + } + } + return deviceGroups; } catch (DeviceManagementException | GroupManagementDAOException | SQLException e) { String msg = "Error occurred while retrieving device groups."; log.error(msg, e); @@ -902,7 +942,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid if (log.isDebugEnabled()) { log.debug("Create default group " + groupName); } - DeviceGroup defaultGroup = this.getGroup(groupName); + DeviceGroup defaultGroup = this.getGroup(groupName, false); if (defaultGroup == null) { defaultGroup = new DeviceGroup(groupName); defaultGroup.setStatus(DeviceGroupConstants.GroupStatus.ACTIVE); @@ -921,7 +961,7 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid log.error(msg, e); throw new GroupManagementException(msg, e); } - return this.getGroup(groupName); + return this.getGroup(groupName, false); } else { return defaultGroup; } @@ -940,25 +980,32 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid getDevice(deviceIdentifier, false); if (device == null) { throw new GroupManagementException("Device not found for id '" + deviceIdentifier.getId() + - "' type '" + deviceIdentifier.getType() + "'"); + "' type '" + deviceIdentifier.getType() + "'"); } } catch (DeviceManagementException e) { throw new GroupManagementException("Device management exception occurred when retrieving device. " + - e.getMessage(), e); + e.getMessage(), e); } - try{ + try { GroupManagementDAOFactory.openConnection(); return this.groupDAO.isDeviceMappedToGroup(groupId, device.getId(), tenantId); } catch (GroupManagementDAOException e) { throw new GroupManagementException("Error occurred when checking device, group mapping between device id '" + - deviceIdentifier.getId() + "' and group id '" + groupId + "'", e); + deviceIdentifier.getId() + "' and group id '" + groupId + "'", e); } catch (SQLException e) { throw new GroupManagementException("Error occurred when opening db connection to check device, group " + - "mapping between device id '" + deviceIdentifier.getId() + - "' and group id '" + groupId + "'", e); + "mapping between device id '" + deviceIdentifier.getId() + + "' and group id '" + groupId + "'", e); } finally { GroupManagementDAOFactory.closeConnection(); } } + + private void populateGroupProperties(DeviceGroup deviceGroup, int tenantId) throws GroupManagementDAOException { + if (deviceGroup != null && deviceGroup.getGroupId() > 0) { + deviceGroup.setGroupProperties(this.groupDAO.getAllGroupProperties(deviceGroup.getGroupId(), + tenantId)); + } + } } diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceTest.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceTest.java index e884dc4728..eb8bf8ce0a 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceTest.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/authorization/DeviceAccessAuthorizationServiceTest.java @@ -167,7 +167,7 @@ public class DeviceAccessAuthorizationServiceTest { GroupManagementProviderService groupManagementProviderService = DeviceManagementDataHolder.getInstance() .getGroupManagementProviderService(); groupManagementProviderService.createDefaultGroup(DEFAULT_GROUP); - int groupId = groupManagementProviderService.getGroup(DEFAULT_GROUP).getGroupId(); + int groupId = groupManagementProviderService.getGroup(DEFAULT_GROUP, false).getGroupId(); //Sharing group with admin and non admin roles groupManagementProviderService.manageGroupSharing(groupId, new ArrayList<>(Arrays.asList(ADMIN_ROLE, NON_ADMIN_ROLE))); diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceNegativeTest.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceNegativeTest.java index 719ebe5576..fd34165031 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceNegativeTest.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceNegativeTest.java @@ -68,28 +68,28 @@ public class GroupManagementProviderServiceNegativeTest extends BaseDeviceManage + "negative conditions", expectedExceptions = {GroupManagementException.class}, expectedExceptionsMessageRegExp = "Error occurred while obtaining group.*") public void testGetGroup() throws GroupManagementException { - groupManagementProviderService.getGroup(1); + groupManagementProviderService.getGroup(1, false); } @Test(description = "This method tests the getGroup method of the GroupManagementProviderService under " + "negative conditions", expectedExceptions = {GroupManagementException.class}, expectedExceptionsMessageRegExp = "Error occurred while obtaining group with name.*") public void testGetGroupWithName() throws GroupManagementException { - groupManagementProviderService.getGroup("1"); + groupManagementProviderService.getGroup("1", false); } @Test(description = "This method tests the getGroups method of the GroupManagementProviderService under negative " + "conditions", expectedExceptions = {GroupManagementException.class}, expectedExceptionsMessageRegExp = "Error occurred while retrieving all groups in tenant.*") public void testGetGroups() throws GroupManagementException { - groupManagementProviderService.getGroups(); + groupManagementProviderService.getGroups(true); } @Test(description = "This method tests the getGroups method of the GroupManagementProviderService under negative " + "conditions", expectedExceptions = {GroupManagementException.class}, expectedExceptionsMessageRegExp = "Error occurred while retrieving all groups accessible to user.*") public void testGetGroupsWithUserName() throws GroupManagementException { - groupManagementProviderService.getGroups("test"); + groupManagementProviderService.getGroups("test", false); } @Test(description = "This method tests the getGroupCount method under negative circumstances", expectedExceptions @@ -111,14 +111,14 @@ public class GroupManagementProviderServiceNegativeTest extends BaseDeviceManage + "circumstances", expectedExceptions = {GroupManagementException.class}, expectedExceptionsMessageRegExp = "Error occurred while retrieving all groups in tenant") public void testGetGroupsWithPaginationRequest() throws GroupManagementException { - groupManagementProviderService.getGroups(TestUtils.createPaginationRequest()); + groupManagementProviderService.getGroups(TestUtils.createPaginationRequest(), false); } @Test(description = "This method tests the getGroups method with pagination request and username under negative " + "circumstances", expectedExceptions = {GroupManagementException.class}, expectedExceptionsMessageRegExp = "Error occurred while retrieving all groups accessible to user.") public void testGetGroupsWithPaginationRequestAndUserName() throws GroupManagementException { - groupManagementProviderService.getGroups("test", TestUtils.createPaginationRequest()); + groupManagementProviderService.getGroups("test", TestUtils.createPaginationRequest(), false); } @Test(description = "This method tests the get roles method under negative circumstances", @@ -152,6 +152,6 @@ public class GroupManagementProviderServiceNegativeTest extends BaseDeviceManage expectedExceptionsMessageRegExp = "Received empty device identifier for getGroups", expectedExceptions = {GroupManagementException.class}) public void testGetGroupsWithDeviceIdentifier() throws GroupManagementException { - groupManagementProviderService.getGroups((DeviceIdentifier) null); + groupManagementProviderService.getGroups((DeviceIdentifier) null, false); } } diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceTest.java b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceTest.java index 3f96a8aede..4ffb675fe7 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceTest.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.core/src/test/java/org/wso2/carbon/device/mgt/core/service/GroupManagementProviderServiceTest.java @@ -86,20 +86,20 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest @Test(dependsOnMethods = ("createGroup")) public void updateGroup() throws GroupManagementException, GroupNotExistException { - DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup1().getName()); + DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup1().getName(), false); deviceGroup.setName(deviceGroup.getName() + "_UPDATED"); groupManagementProviderService.updateGroup(deviceGroup, deviceGroup.getGroupId()); } @Test(dependsOnMethods = ("createGroup"), expectedExceptions = {GroupManagementException.class}) public void getGroupNull() throws GroupManagementException, GroupNotExistException { - groupManagementProviderService.getGroup(null); + groupManagementProviderService.getGroup(null, false); } // Rename again to use in different place. @Test(dependsOnMethods = ("updateGroup")) public void updateGroupSecondTime() throws GroupManagementException, GroupNotExistException { - DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup1().getName() + "_UPDATED"); + DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup1().getName() + "_UPDATED", true); deviceGroup.setName(TestUtils.createDeviceGroup1().getName()); groupManagementProviderService.updateGroup(deviceGroup, deviceGroup.getGroupId()); } @@ -111,14 +111,14 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest @Test(dependsOnMethods = ("createGroup"), expectedExceptions = {GroupManagementException.class, GroupNotExistException.class}) public void updateGroupErrorNotExist() throws GroupManagementException, GroupNotExistException { - DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup2().getName()); + DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup2().getName(), false); deviceGroup.setName(deviceGroup.getName() + "_UPDATED"); groupManagementProviderService.updateGroup(deviceGroup, 6); } @Test(dependsOnMethods = ("createGroup")) public void deleteGroup() throws GroupManagementException { - DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup4().getName()); + DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup4().getName(), false); Assert.assertTrue(groupManagementProviderService.deleteGroup(deviceGroup.getGroupId())); } @@ -131,47 +131,47 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest @Test(dependsOnMethods = ("createGroup")) public void getGroup() throws GroupManagementException { - DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup3().getName()); - Assert.assertNotNull(groupManagementProviderService.getGroup(deviceGroup.getGroupId())); + DeviceGroup deviceGroup = groupManagementProviderService.getGroup(TestUtils.createDeviceGroup3().getName(), false); + Assert.assertNotNull(groupManagementProviderService.getGroup(deviceGroup.getGroupId(), false)); } @Test(dependsOnMethods = ("createGroup")) public void getGroupByName() throws GroupManagementException { - Assert.assertNotNull(groupManagementProviderService.getGroup(TestUtils.createDeviceGroup3().getName())); + Assert.assertNotNull(groupManagementProviderService.getGroup(TestUtils.createDeviceGroup3().getName(), false)); } @Test(dependsOnMethods = ("createGroup")) public void getGroups() throws GroupManagementException { - List deviceGroups = groupManagementProviderService.getGroups(); + List deviceGroups = groupManagementProviderService.getGroups(true); Assert.assertNotNull(deviceGroups); } @Test(dependsOnMethods = ("createGroup")) public void getGroupsByUsername() throws GroupManagementException { - List deviceGroups = groupManagementProviderService.getGroups("admin"); + List deviceGroups = groupManagementProviderService.getGroups("admin", true); Assert.assertNotNull(deviceGroups); } @Test(dependsOnMethods = ("createGroup"), expectedExceptions = {GroupManagementException.class}) public void getGroupsByUsernameError() throws GroupManagementException { - groupManagementProviderService.getGroups((String) null); + groupManagementProviderService.getGroups((String) null, false); } @Test(dependsOnMethods = ("createGroup")) public void getGroupsByPagination() throws GroupManagementException { - PaginationResult result = groupManagementProviderService.getGroups(TestUtils.createPaginationRequest()); + PaginationResult result = groupManagementProviderService.getGroups(TestUtils.createPaginationRequest(), true); Assert.assertNotNull(result); } @Test(dependsOnMethods = ("createGroup"), expectedExceptions = {GroupManagementException.class}) public void getGroupsByPaginationError() throws GroupManagementException { - groupManagementProviderService.getGroups((GroupPaginationRequest) null); + groupManagementProviderService.getGroups((GroupPaginationRequest) null, true); } @Test(dependsOnMethods = ("createGroup")) public void getGroupsByUsernameAndPagination() throws GroupManagementException { - PaginationResult result = groupManagementProviderService.getGroups("admin", TestUtils.createPaginationRequest()); + PaginationResult result = groupManagementProviderService.getGroups("admin", TestUtils.createPaginationRequest(), false); Assert.assertNotNull(result); } @@ -179,7 +179,7 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest @Test(dependsOnMethods = ("createGroup"), expectedExceptions = {GroupManagementException.class}) public void getGroupsByUsernameAndPaginationError() throws GroupManagementException { - groupManagementProviderService.getGroups(null, TestUtils.createPaginationRequest()); + groupManagementProviderService.getGroups(null, TestUtils.createPaginationRequest(), true); } @Test(dependsOnMethods = ("createGroup")) @@ -214,7 +214,7 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest userStoreManager.addRole("TEST_ROLE_3", null, permissions); groupManagementProviderService.manageGroupSharing(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup1().getName()).getGroupId(), newRoles); + TestUtils.createDeviceGroup1().getName(), false).getGroupId(), newRoles); } @Test(dependsOnMethods = ("createGroup")) @@ -244,25 +244,25 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest DeviceConfigurationManager.getInstance().getDeviceManagementConfig().setDeviceCacheConfiguration(configuration); List list = TestUtils.getDeviceIdentifiersList(); groupManagementProviderService.addDevices(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup1().getName()).getGroupId(), list); + TestUtils.createDeviceGroup1().getName(), false).getGroupId(), list); groupManagementProviderService.addDevices(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup2().getName()).getGroupId(), list); + TestUtils.createDeviceGroup2().getName(), false).getGroupId(), list); groupManagementProviderService.addDevices(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup3().getName()).getGroupId(), list); + TestUtils.createDeviceGroup3().getName(), false).getGroupId(), list); } @Test(dependsOnMethods = ("addDevices")) public void removeDevice() throws GroupManagementException, DeviceNotFoundException { List list = TestUtils.getDeviceIdentifiersList(); groupManagementProviderService.removeDevice(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup2().getName()).getGroupId(), list); + TestUtils.createDeviceGroup2().getName(), false).getGroupId(), list); groupManagementProviderService.removeDevice(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup3().getName()).getGroupId(), list); + TestUtils.createDeviceGroup3().getName(), false).getGroupId(), list); } @Test(dependsOnMethods = ("createGroup")) public void getGroupsByUsernameAndPermissions() throws GroupManagementException { - List groups = groupManagementProviderService.getGroups("admin", "/permission/device-mgt/admin/groups"); + List groups = groupManagementProviderService.getGroups("admin", "/permission/device-mgt/admin/groups", true); Assert.assertNotNull(groups); } @@ -271,7 +271,7 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest DeviceIdentifier identifier = new DeviceIdentifier(); identifier.setId("12345"); identifier.setType(TestDataHolder.TEST_DEVICE_TYPE); - List groups = groupManagementProviderService.getGroups(identifier); + List groups = groupManagementProviderService.getGroups(identifier, true); Assert.assertNotNull(groups); } @@ -290,7 +290,7 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest List list = TestUtils.getDeviceIdentifiersList(); boolean isMapped = groupManagementProviderService .isDeviceMappedToGroup(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup1().getName()).getGroupId(), list.get(0)); + TestUtils.createDeviceGroup1().getName(), true).getGroupId(), list.get(0)); Assert.assertEquals(isMapped, true); } @@ -306,7 +306,7 @@ public class GroupManagementProviderServiceTest extends BaseDeviceManagementTest @Test(dependsOnMethods = {"createGroup", "updateGroupSecondTime"}, expectedExceptions = {GroupManagementException.class}) public void checkNullDeviceBelongsToGroup() throws GroupManagementException { groupManagementProviderService.isDeviceMappedToGroup(groupManagementProviderService.getGroup( - TestUtils.createDeviceGroup1().getName()).getGroupId(), null); + TestUtils.createDeviceGroup1().getName(), true).getGroupId(), null); } } diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/impl/PolicyInformationPointImpl.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/impl/PolicyInformationPointImpl.java index 20da56c812..498ccc82f4 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/impl/PolicyInformationPointImpl.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/impl/PolicyInformationPointImpl.java @@ -98,7 +98,7 @@ public class PolicyInformationPointImpl implements PolicyInformationPoint { pipDevice.setDeviceIdentifier(deviceIdentifier); pipDevice.setUserId(device.getEnrolmentInfo().getOwner()); pipDevice.setOwnershipType(device.getEnrolmentInfo().getOwnership().toString()); - pipDevice.setDeviceGroups(groupManagementProviderService.getGroups(pipDevice.getDeviceIdentifier())); + pipDevice.setDeviceGroups(groupManagementProviderService.getGroups(pipDevice.getDeviceIdentifier(), false)); } else { throw new PolicyManagementException("Device details cannot be null."); diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/mgt/impl/PolicyManagerImpl.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/mgt/impl/PolicyManagerImpl.java index 4147327197..cb25814f57 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/mgt/impl/PolicyManagerImpl.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/main/java/org/wso2/carbon/policy/mgt/core/mgt/impl/PolicyManagerImpl.java @@ -1160,7 +1160,7 @@ public class PolicyManagerImpl implements PolicyManager { private List getDeviceGroupNames(List groupWrappers) throws GroupManagementException { GroupManagementProviderService groupManagementProviderService = new GroupManagementProviderServiceImpl(); for (DeviceGroupWrapper wrapper : groupWrappers) { - DeviceGroup deviceGroup = groupManagementProviderService.getGroup(wrapper.getId()); + DeviceGroup deviceGroup = groupManagementProviderService.getGroup(wrapper.getId(), false); wrapper.setName(deviceGroup.getName()); wrapper.setOwner(deviceGroup.getOwner()); } diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/BasePolicyManagementDAOTest.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/BasePolicyManagementDAOTest.java index c55155cd7a..d487d35a25 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/BasePolicyManagementDAOTest.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/BasePolicyManagementDAOTest.java @@ -247,7 +247,7 @@ public abstract class BasePolicyManagementDAOTest { List groupDevices = new ArrayList<>(); groupDevices.add(deviceIdentifier); try { - DeviceGroup group = groupMgtService.getGroup(groupName); + DeviceGroup group = groupMgtService.getGroup(groupName, false); groupMgtService.addDevices(group.getGroupId(), groupDevices); } catch (DeviceNotFoundException | GroupManagementException e) { String msg = "Failed to add device " + deviceIdentifier.getId() + " to group " + groupName; diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/PolicyManagerServiceImplTest.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/PolicyManagerServiceImplTest.java index 6dc56ab346..1bd4d6591e 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/PolicyManagerServiceImplTest.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/PolicyManagerServiceImplTest.java @@ -94,7 +94,7 @@ public class PolicyManagerServiceImplTest extends BasePolicyManagementDAOTest { operationManager = new OperationManagerImpl(DEVICE_TYPE_A, deviceManagementService); enrollDevice(DEVICE1, DEVICE_TYPE_A); createDeviceGroup(GROUP1); - DeviceGroup group1 = groupMgtService.getGroup(GROUP1); + DeviceGroup group1 = groupMgtService.getGroup(GROUP1, false); addDeviceToGroup(new DeviceIdentifier(DEVICE1, DEVICE_TYPE_A), GROUP1); Profile profile = new Profile(); diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/FeatureManagerImplTest.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/FeatureManagerImplTest.java index 6d92754b3f..25fe63c9f4 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/FeatureManagerImplTest.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/FeatureManagerImplTest.java @@ -85,7 +85,7 @@ public class FeatureManagerImplTest extends BasePolicyManagementDAOTest { enrollDevice(DEVICE4, DEVICE_TYPE_D); createDeviceGroup(GROUP4); - DeviceGroup group4 = groupMgtService.getGroup(GROUP4); + DeviceGroup group4 = groupMgtService.getGroup(GROUP4, false); addDeviceToGroup(new DeviceIdentifier(DEVICE4, DEVICE_TYPE_D), GROUP4); Profile profile = new Profile(); diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/MonitoringManagerImplTest.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/MonitoringManagerImplTest.java index ee05fddbc3..07dc2dd992 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/MonitoringManagerImplTest.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/MonitoringManagerImplTest.java @@ -97,7 +97,7 @@ public class MonitoringManagerImplTest extends BasePolicyManagementDAOTest{ enrollDevice(DEVICE5, DEVICE_TYPE_E); createDeviceGroup(GROUP5); addDeviceToGroup(new DeviceIdentifier(DEVICE5, DEVICE_TYPE_E), GROUP5); - DeviceGroup group5 = groupMgtService.getGroup(GROUP5); + DeviceGroup group5 = groupMgtService.getGroup(GROUP5, false); device5 = deviceMgtService.getAllDevices().get(0); diff --git a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/ProfileManagerImplTest.java b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/ProfileManagerImplTest.java index ccbce49749..81ac53d18c 100644 --- a/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/ProfileManagerImplTest.java +++ b/components/policy-mgt/org.wso2.carbon.policy.mgt.core/src/test/java/org/wso2/carbon/policy/mgt/core/mgt/impl/ProfileManagerImplTest.java @@ -77,7 +77,7 @@ public class ProfileManagerImplTest extends BasePolicyManagementDAOTest { operationManager = new OperationManagerImpl(DEVICE_TYPE_C, deviceManagementService); enrollDevice(DEVICE3, DEVICE_TYPE_C); createDeviceGroup(GROUP3); - DeviceGroup group1 = groupMgtService.getGroup(GROUP3); + DeviceGroup group1 = groupMgtService.getGroup(GROUP3, false); addDeviceToGroup(new DeviceIdentifier(DEVICE3, DEVICE_TYPE_C), GROUP3); } From e6ea2175cdab7455f1d23e9e6abccce3b9557766 Mon Sep 17 00:00:00 2001 From: charitha Date: Mon, 4 Nov 2019 12:39:15 +0530 Subject: [PATCH 2/2] Add review suggestions --- .../service/api/GroupManagementService.java | 2 +- .../GroupManagementProviderServiceImpl.java | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java index 4880231574..f61d212493 100644 --- a/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java +++ b/components/device-mgt/org.wso2.carbon.device.mgt.api/src/main/java/org/wso2/carbon/device/mgt/jaxrs/service/api/GroupManagementService.java @@ -450,7 +450,7 @@ public interface GroupManagementService { "the requested resource."), @ApiResponse( code = 404, - message = "Group found.", + message = "Group not found.", response = ErrorResponse.class), @ApiResponse( code = 406, 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 f3c3ab2c82..efe71421db 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 @@ -569,12 +569,16 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid count = groupDAO.getOwnGroupsCount(username, tenantId); count += groupDAO.getGroupsCount(roleList, tenantId); return count; - } catch (UserStoreException | GroupManagementDAOException | SQLException e) { - String msg = "Error occurred while retrieving group count of user '" + username + "'"; + } catch (UserStoreException e) { + String msg = "Error occurred while retrieving role list of user '" + username + "'"; log.error(msg, e); throw new GroupManagementException(msg, e); - } catch (Exception e) { - String msg = "Error occurred in getGroupCount for username '" + username + "'"; + } catch (SQLException e) { + String msg = "Error occurred while opening db connection to get group count."; + log.error(msg, e); + throw new GroupManagementException(msg, e); + } catch (GroupManagementDAOException e) { + String msg = "Error occurred while retrieving group count of user '" + username + "'"; log.error(msg, e); throw new GroupManagementException(msg, e); } finally { @@ -1002,6 +1006,13 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid } } + /** + * This is populating group properties in to the group pass as an argument. Call should be manage the transactions. + * + * @param deviceGroup which needs to populate with properties + * @param tenantId of the caller + * @throws GroupManagementDAOException on DAO exceptions + */ private void populateGroupProperties(DeviceGroup deviceGroup, int tenantId) throws GroupManagementDAOException { if (deviceGroup != null && deviceGroup.getGroupId() > 0) { deviceGroup.setGroupProperties(this.groupDAO.getAllGroupProperties(deviceGroup.getGroupId(),