Refactor device status filter management service #384

Open
pramilaniroshan wants to merge 7 commits from pramilaniroshan/device-mgt-core:fix-subtenat-device-status-issue into master
Collaborator
https://roadmap.entgra.net/issues/11060
pramilaniroshan added 1 commit 7 months ago
pahansith requested changes 6 months ago
List<AllowedDeviceStatus> statusList = gson.fromJson(metadata.getMetaValue(), listType);
MetadataManagementService metadataManagementService = new MetadataManagementServiceImpl();
Metadata metadata = metadataManagementService.retrieveMetadata(MetadataConstants.ALLOWED_DEVICE_STATUS_META_KEY);
Gson gson = new Gson();
Owner

Declare the Gson object as a filed to better memory optimization

Declare the Gson object as a filed to better memory optimization
pramilaniroshan marked this conversation as resolved
List<String> allowedStatus = status.getStatus();
return allowedStatus.contains(deviceStatus);
}
MetadataManagementService metadataManagementService = new MetadataManagementServiceImpl();
Owner

Same code block as L224 - L241. Check the possibility of declaring method instead of duplicating the code

Same code block as L224 - L241. Check the possibility of declaring method instead of duplicating the code
Owner

What is the commit of the solution for this? If it is not possible then there should be a comment that explains the reason.

Further check this [1] as well.

[1] - #384/files#issuecomment-15824

What is the commit of the solution for this? If it is not possible then there should be a comment that explains the reason. Further check this [1] as well. [1] - https://repository.entgra.net/community/device-mgt-core/pulls/384/files#issuecomment-15824
Poster
Collaborator

This is the commit eb6186ba28
eb6186ba28

This is the commit eb6186ba280579013c56cf71705bd0b6035cf268 https://repository.entgra.net/community/device-mgt-core/commit/eb6186ba280579013c56cf71705bd0b6035cf268
pramilaniroshan marked this conversation as resolved
pramilaniroshan added 1 commit 6 months ago
eb6186ba28
Extract common metadata retrieval and parsing logic to a separate method
tcdlpds requested changes 5 months ago
}
}
private void addDefaultDeviceStatusFilters(int tenantId) throws MetadataManagementException {
Owner

Add Java Doc comment.

Add Java Doc comment.
pramilaniroshan marked this conversation as resolved
}
}
private void addDefaultDeviceStatusCheck(int tenantId) throws MetadataManagementException {
Owner

Add Java Doc comment.

Add Java Doc comment.
pramilaniroshan marked this conversation as resolved
}
List<AllowedDeviceStatus> statusList = retrieveAndParseMetadata(tenantId);
for (AllowedDeviceStatus status : statusList) {
if (status.getType().equalsIgnoreCase(deviceType)) {
Owner

It is better if we check whether the value of 'deviceType' is empty or not using StringUtils and throw error. Further swap the check and modify it as follows to avoid from getting null point exception.

" deviceType.equalsIgnoreCase(status.getType()) "

It is better if we check whether the value of 'deviceType' is empty or not using StringUtils and throw error. Further swap the check and modify it as follows to avoid from getting null point exception. " deviceType.equalsIgnoreCase(status.getType()) "
pramilaniroshan marked this conversation as resolved
return Collections.emptyList();
}
private List<AllowedDeviceStatus> retrieveAndParseMetadata(int tenantId) throws MetadataManagementException {
Owner

Add Java Doc comment.

Add Java Doc comment.
pramilaniroshan marked this conversation as resolved
}
Owner

Remove unnecessary new line

Remove unnecessary new line
pramilaniroshan marked this conversation as resolved
pramilaniroshan added 1 commit 5 months ago
tcdlpds requested changes 5 months ago
public void updateDefaultDeviceStatusFilters(int tenantId, String deviceType, List<String> deviceStatus) throws MetadataManagementException {
try {
if (StringUtils.isEmpty(deviceType)) {
throw new IllegalArgumentException("Device type must not be empty or null");
Owner

Log and throw the exception.

Log and throw the exception.
pramilaniroshan marked this conversation as resolved
pramilaniroshan added 1 commit 5 months ago
pramilaniroshan added 2 commits 5 months ago
pramilaniroshan added 1 commit 5 months ago
Owner

This PR needs to be improved. Hence until it is improved, it is not possible to merge . Please check the PR and identify mistakes and update the PR.

This PR needs to be improved. Hence until it is improved, it is not possible to merge . Please check the PR and identify mistakes and update the PR.

Reviewers

pahansith requested changes 6 months ago
tcdlpds requested changes 5 months ago
This pull request has changes conflicting with the target branch.
components/tenant-mgt/io.entgra.device.mgt.core.tenant.mgt.core/src/main/java/io/entgra/device/mgt/core/tenant/mgt/core/internal/TenantMgtServiceComponent.java
components/device-mgt/io.entgra.device.mgt.core.device.mgt.api/src/main/java/io/entgra/device/mgt/core/device/mgt/api/jaxrs/service/impl/DeviceStatusFilterServiceImpl.java
components/device-mgt/io.entgra.device.mgt.core.device.mgt.core/src/main/java/io/entgra/device/mgt/core/device/mgt/core/internal/DeviceManagementServiceComponent.java
components/device-mgt/io.entgra.device.mgt.core.device.mgt.core/src/main/java/io/entgra/device/mgt/core/device/mgt/core/metadata/mgt/DeviceStatusManagementServiceImpl.java
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: community/device-mgt-core#384
Loading…
There is no content yet.