Fix for the incorrect device statistics shown in the app store. #508

Merged
tcdlpds merged 6 commits from ruwin/device-mgt-core:statistics into master 2 months ago

@ -39,6 +39,7 @@ import io.entgra.device.mgt.core.application.mgt.core.util.HelperUtil;
import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.SubscriptionManagementHelperUtil; import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.SubscriptionManagementHelperUtil;
import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.service.SubscriptionManagementHelperService; import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.service.SubscriptionManagementHelperService;
import io.entgra.device.mgt.core.device.mgt.common.Device; import io.entgra.device.mgt.core.device.mgt.common.Device;
import io.entgra.device.mgt.core.device.mgt.common.EnrolmentInfo;
import io.entgra.device.mgt.core.device.mgt.common.exceptions.DeviceManagementException; import io.entgra.device.mgt.core.device.mgt.common.exceptions.DeviceManagementException;
import io.entgra.device.mgt.core.device.mgt.common.group.mgt.GroupManagementException; import io.entgra.device.mgt.core.device.mgt.common.group.mgt.GroupManagementException;
import io.entgra.device.mgt.core.device.mgt.core.dto.GroupDetailsDTO; import io.entgra.device.mgt.core.device.mgt.core.dto.GroupDetailsDTO;
@ -48,6 +49,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.context.PrivilegedCarbonContext; import org.wso2.carbon.context.PrivilegedCarbonContext;
import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Objects; import java.util.Objects;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -146,7 +148,6 @@ public class GroupBasedSubscriptionManagementHelperServiceImpl implements Subscr
} finally { } finally {
ConnectionManagerUtil.closeDBConnection(); ConnectionManagerUtil.closeDBConnection();
} }
} }
@Override @Override
@ -193,12 +194,14 @@ public class GroupBasedSubscriptionManagementHelperServiceImpl implements Subscr
log.error(msg); log.error(msg);
throw new NotFoundException(msg); throw new NotFoundException(msg);
} }
List<Device> devices = HelperUtil.getGroupManagementProviderService(). List<String> deviceStatuses = Arrays.asList(EnrolmentInfo.Status.ACTIVE.name(),
getAllDevicesOfGroup(subscriptionInfo.getIdentifier(), false); EnrolmentInfo.Status.INACTIVE.name(), EnrolmentInfo.Status.UNREACHABLE.name());
List<Device> devices = HelperUtil.getGroupManagementProviderService().getAllDevicesOfGroup(subscriptionInfo.getIdentifier(), deviceStatuses, false);
tcdlpds marked this conversation as resolved
Review

In this service method are we considering the device type of the application supported?

In this service method are we considering the device type of the application supported?
Review

Yes, your point is valid, but in the latter part SubscriptionStatisticDTO subscriptionStatisticDTO = subscriptionDAO.getSubscriptionStatistic (deviceIdsOwnByGroup, isUnsubscribe, tenantId, applicationReleaseDTO.getId()); when getting the stats, we're considering the release id. So the device type constraint will be implicitly satisfied.

Yes, your point is valid, but in the latter part `SubscriptionStatisticDTO subscriptionStatisticDTO = subscriptionDAO.getSubscriptionStatistic (deviceIdsOwnByGroup, isUnsubscribe, tenantId, applicationReleaseDTO.getId());` when getting the stats, we're considering the release id. So the device type constraint will be implicitly satisfied.
Review

Yes, otherwise if a scenario like triggering an android application installation to group which have both the windows and android devices assigned to it then those app irrelevant devices (in this case windows) are also being counted for the device count but they are not relevant to that app installation. Here we are getting only the completed, pending and error device count and substract it from the total device count and assign that remainder to the new device count and pass it for the calculation of percentages so, when the device type of the application supported is not considered it will count all the devices relevant to that group so the new device percentage might be wrong.

Yes, otherwise if a scenario like triggering an android application installation to group which have both the windows and android devices assigned to it then those app irrelevant devices (in this case windows) are also being counted for the device count but they are not relevant to that app installation. Here we are getting only the completed, pending and error device count and substract it from the total device count and assign that remainder to the new device count and pass it for the calculation of percentages so, when the device type of the application supported is not considered it will count all the devices relevant to that group so the new device percentage might be wrong.
List<Integer> deviceIdsOwnByGroup = devices.stream().map(Device::getId).collect(Collectors.toList()); List<Integer> deviceIdsOwnByGroup = devices.stream().map(Device::getId).collect(Collectors.toList());
SubscriptionStatisticDTO subscriptionStatisticDTO = subscriptionDAO. SubscriptionStatisticDTO subscriptionStatisticDTO = subscriptionDAO.getSubscriptionStatistic
getSubscriptionStatistic(deviceIdsOwnByGroup, isUnsubscribe, tenantId, applicationReleaseDTO.getId()); (deviceIdsOwnByGroup, isUnsubscribe, tenantId, applicationReleaseDTO.getId());
int allDeviceCount = HelperUtil.getGroupManagementProviderService().getDeviceCount(subscriptionInfo.getIdentifier()); int allDeviceCount = HelperUtil.getGroupManagementProviderService().getDeviceCountWithGroup(subscriptionInfo.getIdentifier(),
applicationDAO.getApplication(applicationReleaseDTO.getUuid(), tenantId).getDeviceTypeId());
return SubscriptionManagementHelperUtil.getSubscriptionStatistics(subscriptionStatisticDTO, allDeviceCount); return SubscriptionManagementHelperUtil.getSubscriptionStatistics(subscriptionStatisticDTO, allDeviceCount);
} catch (ApplicationManagementDAOException e) { } catch (ApplicationManagementDAOException e) {
String msg = "Error encountered while getting subscription statistics for group: " + subscriptionInfo.getIdentifier(); String msg = "Error encountered while getting subscription statistics for group: " + subscriptionInfo.getIdentifier();

@ -40,6 +40,7 @@ import io.entgra.device.mgt.core.application.mgt.core.util.HelperUtil;
import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.SubscriptionManagementHelperUtil; import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.SubscriptionManagementHelperUtil;
import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.service.SubscriptionManagementHelperService; import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.service.SubscriptionManagementHelperService;
import io.entgra.device.mgt.core.device.mgt.common.Device; import io.entgra.device.mgt.core.device.mgt.common.Device;
import io.entgra.device.mgt.core.device.mgt.common.EnrolmentInfo;
import io.entgra.device.mgt.core.device.mgt.common.PaginationRequest; import io.entgra.device.mgt.core.device.mgt.common.PaginationRequest;
import io.entgra.device.mgt.core.device.mgt.common.PaginationResult; import io.entgra.device.mgt.core.device.mgt.common.PaginationResult;
import io.entgra.device.mgt.core.device.mgt.common.exceptions.DeviceManagementException; import io.entgra.device.mgt.core.device.mgt.common.exceptions.DeviceManagementException;
@ -210,7 +211,8 @@ public class RoleBasedSubscriptionManagementHelperServiceImpl implements Subscri
for (String user : usersWithRole) { for (String user : usersWithRole) {
PaginationRequest paginationRequest = new PaginationRequest(-1, -1); PaginationRequest paginationRequest = new PaginationRequest(-1, -1);
paginationRequest.setOwner(user); paginationRequest.setOwner(user);
paginationRequest.setStatusList(Arrays.asList("ACTIVE", "INACTIVE", "UNREACHABLE")); paginationRequest.setStatusList(Arrays.asList(EnrolmentInfo.Status.ACTIVE.name(),
EnrolmentInfo.Status.INACTIVE.name(),EnrolmentInfo.Status.UNREACHABLE.name()));
PaginationResult ownDeviceIds = HelperUtil.getDeviceManagementProviderService(). PaginationResult ownDeviceIds = HelperUtil.getDeviceManagementProviderService().
getAllDevicesIdList(paginationRequest); getAllDevicesIdList(paginationRequest);
if (ownDeviceIds.getData() != null) { if (ownDeviceIds.getData() != null) {

@ -39,6 +39,7 @@ import io.entgra.device.mgt.core.application.mgt.core.util.HelperUtil;
import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.SubscriptionManagementHelperUtil; import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.SubscriptionManagementHelperUtil;
import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.service.SubscriptionManagementHelperService; import io.entgra.device.mgt.core.application.mgt.core.util.subscription.mgt.service.SubscriptionManagementHelperService;
import io.entgra.device.mgt.core.device.mgt.common.Device; import io.entgra.device.mgt.core.device.mgt.common.Device;
import io.entgra.device.mgt.core.device.mgt.common.EnrolmentInfo;
import io.entgra.device.mgt.core.device.mgt.common.PaginationRequest; import io.entgra.device.mgt.core.device.mgt.common.PaginationRequest;
import io.entgra.device.mgt.core.device.mgt.common.PaginationResult; import io.entgra.device.mgt.core.device.mgt.common.PaginationResult;
import io.entgra.device.mgt.core.device.mgt.common.exceptions.DeviceManagementException; import io.entgra.device.mgt.core.device.mgt.common.exceptions.DeviceManagementException;
@ -196,7 +197,8 @@ public class UserBasedSubscriptionManagementHelperServiceImpl implements Subscri
List<Device> deviceListOwnByUser = new ArrayList<>(); List<Device> deviceListOwnByUser = new ArrayList<>();
PaginationRequest paginationRequest = new PaginationRequest(-1, -1); PaginationRequest paginationRequest = new PaginationRequest(-1, -1);
paginationRequest.setOwner(username); paginationRequest.setOwner(username);
paginationRequest.setStatusList(Arrays.asList("ACTIVE", "INACTIVE", "UNREACHABLE")); paginationRequest.setStatusList(Arrays.asList(EnrolmentInfo.Status.ACTIVE.name(),
EnrolmentInfo.Status.INACTIVE.name(),EnrolmentInfo.Status.UNREACHABLE.name()));
PaginationResult ownDeviceIds = HelperUtil.getDeviceManagementProviderService(). PaginationResult ownDeviceIds = HelperUtil.getDeviceManagementProviderService().
getAllDevicesIdList(paginationRequest); getAllDevicesIdList(paginationRequest);
if (ownDeviceIds.getData() != null) { if (ownDeviceIds.getData() != null) {

@ -489,4 +489,15 @@ public interface GroupDAO {
throws GroupManagementDAOException; throws GroupManagementDAOException;
int getDeviceCount(String groupName, int tenantId) throws GroupManagementDAOException; int getDeviceCount(String groupName, int tenantId) throws GroupManagementDAOException;
/**
* Retrieves the count of devices for a specific group, device type, and tenant.
*
* @param groupName the name of the group
* @param deviceTypeId the ID of the device type (e.g., Android, iOS, Windows)
* @param tenantId the ID of the tenant
* @return the count of devices for the given group, device type, and tenant
* @throws GroupManagementDAOException if an error occurs during the retrieval of the device count
*/
int getDeviceCountWithGroup(String groupName, int deviceTypeId, int tenantId) throws GroupManagementDAOException;
} }

@ -1584,4 +1584,37 @@ public abstract class AbstractGroupDAOImpl implements GroupDAO {
throw new GroupManagementDAOException(msg, e); throw new GroupManagementDAOException(msg, e);
} }
} }
@Override
public int getDeviceCountWithGroup(String groupName, int deviceTypeId, int tenantId) throws GroupManagementDAOException {
int deviceCount = 0;
try {
Connection connection = GroupManagementDAOFactory.getConnection();
String sql = "SELECT COUNT(e.ID) AS COUNT " +
"FROM DM_GROUP d " +
"INNER JOIN DM_DEVICE_GROUP_MAP m ON d.ID = m.GROUP_ID " +
"INNER JOIN DM_ENROLMENT e ON m.DEVICE_ID = e.DEVICE_ID " +
"INNER JOIN DM_DEVICE r ON e.DEVICE_ID = r.ID " +
"WHERE d.TENANT_ID = ? " +
"AND d.GROUP_NAME = ? " +
"AND r.DEVICE_TYPE_ID = ? " +
"AND e.STATUS NOT IN ('REMOVED', 'DELETED')";
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
preparedStatement.setInt(1, tenantId);
preparedStatement.setString(2, groupName);
preparedStatement.setInt(3, deviceTypeId);
try (ResultSet resultSet = preparedStatement.executeQuery()) {
if (resultSet.next()) {
deviceCount = resultSet.getInt("COUNT");
}
}
}
return deviceCount;
} catch (SQLException e) {
String msg = "Error occurred while retrieving device count for the group: " + groupName;
log.error(msg, e);
throw new GroupManagementDAOException(msg, e);
}
}
} }

@ -392,4 +392,13 @@ public interface GroupManagementProviderService {
int getDeviceCount(String groupName) throws GroupManagementException; int getDeviceCount(String groupName) throws GroupManagementException;
/**
* Retrieves the count of devices for a specific group and device type.
*
* @param groupName the name of the group
* @param deviceTypeId the ID of the device type (e.g., Android, iOS, Windows)
* @return the count of devices for the given group and device type
* @throws GroupManagementException if an error occurs during the retrieval of the device count
*/
int getDeviceCountWithGroup(String groupName, int deviceTypeId) throws GroupManagementException;
} }

@ -1746,4 +1746,23 @@ public class GroupManagementProviderServiceImpl implements GroupManagementProvid
GroupManagementDAOFactory.closeConnection(); GroupManagementDAOFactory.closeConnection();
} }
} }
@Override
public int getDeviceCountWithGroup(String groupName,int deviceTypeId) throws GroupManagementException {
int tenantId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getTenantId();
try {
GroupManagementDAOFactory.openConnection();
return groupDAO.getDeviceCountWithGroup(groupName,deviceTypeId, tenantId);
ruwin marked this conversation as resolved
Review

Please handle this with two exceptions and add two different error messages, it will be useful when we analyze logs if we get an error with this functionality.

Please handle this with two exceptions and add two different error messages, it will be useful when we analyze logs if we get an error with this functionality.
} catch (SQLException e) {
String msg = "SQL error occurred while retrieving device count.";
log.error(msg, e);
throw new GroupManagementException(msg, e);
} catch (GroupManagementDAOException e) {
String msg = "DAO error occurred while retrieving device count.";
log.error(msg, e);
throw new GroupManagementException(msg, e);
} finally {
GroupManagementDAOFactory.closeConnection();
}
}
} }

Loading…
Cancel
Save