From c8c7922e9716a772ffacc0ee4faaa4c89c1ef099 Mon Sep 17 00:00:00 2001 From: isuri Date: Mon, 2 Oct 2023 09:51:56 +0530 Subject: [PATCH] Modifications to getChildrenOf and getParentsOf methods logic --- .../dao/impl/DeviceOrganizationDAOImpl.java | 39 +++++++++---------- .../spi/DeviceOrganizationService.java | 3 ++ .../device/organization/DAONegativeTest.java | 36 +++-------------- .../device/organization/ServiceTest.java | 4 +- 4 files changed, 28 insertions(+), 54 deletions(-) diff --git a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/dao/impl/DeviceOrganizationDAOImpl.java b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/dao/impl/DeviceOrganizationDAOImpl.java index 13d2dbae96..76e9cf4d18 100644 --- a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/dao/impl/DeviceOrganizationDAOImpl.java +++ b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/dao/impl/DeviceOrganizationDAOImpl.java @@ -58,27 +58,27 @@ public class DeviceOrganizationDAOImpl implements DeviceOrganizationDAO { try { Connection conn = ConnectionManagerUtil.getDBConnection(); - getChildrenRecursive(node, maxDepth, visited, conn, childNodes, includeDevice); - if (!includeDevice) { - childNodes.add(node); + boolean parentAdded = false; // Flag to track whether the parent device has been added + getChildrenRecursive(node, maxDepth, visited, conn, childNodes, includeDevice, parentAdded); + if (!includeDevice && !parentAdded) { + childNodes.add(node); // Add the parent device if it hasn't been added and includeDevice is false. } return childNodes; } catch (DBConnectionException e) { String msg = "Error occurred while obtaining DB connection to retrieve all child devices for " + - "parent device ID " + - node.getDeviceId(); + "parent device ID " + node.getDeviceId(); log.error(msg); throw new DeviceOrganizationMgtDAOException(msg, e); } catch (SQLException e) { String msg = "Error occurred while processing SQL to retrieve all child devices for " + - "parent device ID" + node.getDeviceId(); + "parent device ID " + node.getDeviceId(); log.error(msg); throw new DeviceOrganizationMgtDAOException(msg, e); } } private void getChildrenRecursive(DeviceNode node, int maxDepth, Set visited, Connection conn, - List childNodes, boolean includeDevice) throws SQLException { + List childNodes, boolean includeDevice, boolean parentAdded) throws SQLException { if (maxDepth <= 0 || visited.contains(node.getDeviceId())) { return; } @@ -98,11 +98,12 @@ public class DeviceOrganizationDAOImpl implements DeviceOrganizationDAO { while (rs.next()) { DeviceNode child = getDeviceFromResultSet(rs); node.getChildren().add(child); - if (includeDevice) { - childNodes.add(node); // Add the parent device if includeDevice is true. + if (includeDevice && !parentAdded) { + childNodes.add(node); // Add the parent device only if includeDevice is true and it hasn't been added. + parentAdded = true; // Set the flag to true after adding the parent device. } - getChildrenRecursive(child, maxDepth - 1, visited, conn, childNodes, includeDevice); + getChildrenRecursive(child, maxDepth - 1, visited, conn, childNodes, includeDevice, parentAdded); } } } @@ -118,10 +119,10 @@ public class DeviceOrganizationDAOImpl implements DeviceOrganizationDAO { Set visited = new HashSet<>(); try { Connection conn = ConnectionManagerUtil.getDBConnection(); - getParentsRecursive(node, maxDepth, visited, conn, parentNodes, includeDevice); - if (!includeDevice) { - parentNodes.add(node); + if (includeDevice) { + parentNodes.add(node); // Add the current node to the parent nodes list when includeDevice is true } + getParentsRecursive(node, maxDepth, visited, conn, parentNodes, includeDevice); return parentNodes; } catch (DBConnectionException e) { String msg = "Error occurred while obtaining DB connection to retrieve parent devices for " + @@ -136,7 +137,6 @@ public class DeviceOrganizationDAOImpl implements DeviceOrganizationDAO { } } - private void getParentsRecursive(DeviceNode node, int maxDepth, Set visited, Connection conn, List parentNodes, boolean includeDevice) throws SQLException { if (maxDepth <= 0 || visited.contains(node.getDeviceId())) { @@ -157,15 +157,12 @@ public class DeviceOrganizationDAOImpl implements DeviceOrganizationDAO { try (ResultSet rs = stmt.executeQuery()) { while (rs.next()) { DeviceNode parent = getDeviceFromResultSet(rs); - if (!includeDevice && parent.getDeviceId() == node.getDeviceId()) { - // Skip adding the current node as a parent when includeDevice is false - continue; + if (includeDevice || parent.getDeviceId() != node.getDeviceId()) { + node.getParents().add(parent); } - node.getParents().add(parent); - - if (!parentNodes.contains(parent)) { - parentNodes.add(parent); // Add the parent device if it hasn't been added already. + if (!parentNodes.contains(parent) && (includeDevice || parent.getDeviceId() != node.getDeviceId())) { + parentNodes.add(parent); } getParentsRecursive(parent, maxDepth - 1, visited, conn, parentNodes, includeDevice); diff --git a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/spi/DeviceOrganizationService.java b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/spi/DeviceOrganizationService.java index 4294b46959..b61e3fdd6d 100644 --- a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/spi/DeviceOrganizationService.java +++ b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/main/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/spi/DeviceOrganizationService.java @@ -154,4 +154,7 @@ public interface DeviceOrganizationService { boolean deleteDeviceAssociations(int deviceId) throws DeviceOrganizationMgtPluginException; + + //In case we need to remove the device organization with enrollment removal,we need to implement a callback to + //remove the device organization mapping whenever the device removal happening in enrollment level. } diff --git a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/DAONegativeTest.java b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/DAONegativeTest.java index dac3c88dac..f663b6af37 100644 --- a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/DAONegativeTest.java +++ b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/DAONegativeTest.java @@ -2,10 +2,15 @@ package io.entgra.device.mgt.core.device.mgt.extensions.device.organization; import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.dao.DeviceOrganizationDAO; import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.dao.DeviceOrganizationDAOFactory; +import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.dao.util.ConnectionManagerUtil; +import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.dto.DeviceOrganization; +import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.exception.DeviceOrganizationMgtDAOException; import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.mock.BaseDeviceOrganizationTest; +import io.entgra.device.mgt.core.device.mgt.extensions.device.organization.exception.DBConnectionException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; public class DAONegativeTest extends BaseDeviceOrganizationTest { @@ -19,35 +24,4 @@ public class DAONegativeTest extends BaseDeviceOrganizationTest { log.info("DAO test initialized"); } -// @Test(expectedExceptions = DeviceOrganizationMgtDAOException.class, description = "This method tests the addDeviceOrganization method under negative circumstances with null input") -// public void testAddDeviceOrganizationWithNullInput() throws DeviceOrganizationMgtDAOException { -// DeviceOrganization invalidDeviceOrg = null; -// deviceOrganizationDAO.addDeviceOrganization(invalidDeviceOrg); -// } - -// @Test(description = "Test with invalid input parameters (bad request)") -// public void testGetChildrenOfWithInvalidInput() { -// // Create an invalid input (e.g., null node and negative maxDepth) -// DeviceNode invalidNode = null; -// int invalidMaxDepth = -1; -// boolean includeDevice = true; -// -// try { -// deviceOrganizationDAO.getChildrenOf(invalidNode, invalidMaxDepth, includeDevice); -// assert false : "Expected exception for bad request was not thrown."; -// } catch (DeviceOrganizationMgtDAOException e) { -// log.info("Expected exception for bad request was thrown: " + e.getMessage()); -// } -// } - -// @Test(expectedExceptions = DeviceOrganizationMgtDAOException.class, description = "This method tests the " + -// "getParentsOf method under negative circumstances with invalid input") -// public void testGetParentsOfWithInvalidInput() throws DeviceOrganizationMgtDAOException { -// DeviceNode invalidNode = null; -// int invalidMaxDepth = -1; -// boolean includeDevice = true; -// deviceOrganizationDAO.getParentsOf(invalidNode, invalidMaxDepth, includeDevice); -// } - - } diff --git a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/ServiceTest.java b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/ServiceTest.java index 80e05188d3..57b1129a38 100644 --- a/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/ServiceTest.java +++ b/components/device-mgt-extensions/io.entgra.device.mgt.core.device.mgt.extensions.device.organization/src/test/java/io/entgra/device/mgt/core/device/mgt/extensions/device/organization/ServiceTest.java @@ -32,7 +32,7 @@ public class ServiceTest extends BaseDeviceOrganizationTest { DeviceNode deviceNode = new DeviceNode(); deviceNode.setDeviceId(2); int maxDepth = 2; - boolean includeDevice = false; + boolean includeDevice = true; List childrenList = deviceOrganizationService.getChildrenOf(deviceNode, maxDepth, includeDevice); Assert.assertNotNull(childrenList, "Cannot be null"); @@ -44,7 +44,7 @@ public class ServiceTest extends BaseDeviceOrganizationTest { DeviceNode deviceNode = new DeviceNode(); deviceNode.setDeviceId(4); int maxDepth = 2; - boolean includeDevice = true; + boolean includeDevice = false; List parentList = deviceOrganizationService.getParentsOf(deviceNode, maxDepth, includeDevice); Assert.assertNotNull(parentList, "Cannot be null");