From 1c2987691343505f6afb7bf91296afee8b4eb5d0 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Thu, 21 Sep 2023 14:25:43 +0300 Subject: [PATCH 1/4] Perform parallel Processing of Location Hierarchy --- pom.xml | 2 +- .../model/location/LocationHierarchyTree.java | 4 +- .../smartregister/model/location/Tree.java | 63 +++++++++---------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/pom.xml b/pom.xml index 704f951..7664bc3 100755 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ org.smartregister fhir-common-utils - 0.0.10-SNAPSHOT + 0.0.11-SNAPSHOT diff --git a/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java b/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java index 3bdc36a..49e4032 100755 --- a/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java +++ b/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java @@ -58,9 +58,7 @@ public void addLocation(Location l) { * @param locations */ public void buildTreeFromList(List locations) { - for (Location location : locations) { - addLocation(location); - } + locations.parallelStream().forEach(location -> addLocation(location)); } public Tree getLocationsHierarchy() { diff --git a/src/main/java/org/smartregister/model/location/Tree.java b/src/main/java/org/smartregister/model/location/Tree.java index 0654fd4..1b4830a 100755 --- a/src/main/java/org/smartregister/model/location/Tree.java +++ b/src/main/java/org/smartregister/model/location/Tree.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; import static org.smartregister.utils.Constants.SLASH_UNDERSCORE; @@ -98,18 +99,20 @@ private void addToParentChildRelation(String parent, String id) { StringType parentStringType = new StringType(); parentStringType.setValue(parent); kids.add(idStringType); - Boolean setParentChildMap = false; - for (int i = 0; i < parentChildren.size(); i++) { - if (parentChildren.get(i) != null - && parentChildren.get(i).getIdentifier() != null - && StringUtils.isNotBlank(parentChildren.get(i).getIdentifier().getValue()) - && parentChildren.get(i).getIdentifier().getValue().equals(parent)) { - parentChildren.get(i).setChildIdentifiers(kids); - setParentChildMap = true; - } - } + AtomicReference setParentChildMap = new AtomicReference<>(false); + + List finalKids = kids; + parentChildren.parallelStream().filter(parentChildrenMap -> parentChildrenMap != null + && parentChildrenMap.getIdentifier() != null + && StringUtils.isNotBlank(parentChildrenMap.getIdentifier().getValue()) + && parentChildrenMap.getIdentifier().getValue().equals(parent)).forEach(parentChildrenMap -> { + + parentChildrenMap.setChildIdentifiers(finalKids); + setParentChildMap.set(true); + + }); - if (!setParentChildMap) { + if (!setParentChildMap.get()) { ParentChildrenMap parentChildrenMap = new ParentChildrenMap(); parentChildrenMap.setIdentifier(parentStringType); parentChildrenMap.setChildIdentifiers(kids); @@ -140,43 +143,39 @@ public void addNode(String id, String label, Location node, String parentId) { parentNode.addChild(treeNode); } else { // if no parent exists add it as root node - String idString = (String) id; - if (idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } - SingleTreeNode singleTreeNode = new SingleTreeNode(); - StringType treeNodeId = new StringType(); - treeNodeId.setValue(idString); - singleTreeNode.setTreeNodeId(treeNodeId); - singleTreeNode.setTreeNode(treeNode); + SingleTreeNode singleTreeNode = getSingleTreeNode(id, treeNode); listOfNodes = singleTreeNode; } } else { // if no parent add it as root node - String idString = id; - if (idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } - - SingleTreeNode singleTreeNode = new SingleTreeNode(); - StringType treeNodeId = new StringType(); - treeNodeId.setValue(idString); - singleTreeNode.setTreeNodeId(treeNodeId); - singleTreeNode.setTreeNode(treeNode); + SingleTreeNode singleTreeNode = getSingleTreeNode(id, treeNode); listOfNodes = singleTreeNode; } } + private static SingleTreeNode getSingleTreeNode(String id, TreeNode treeNode) { + String idString = id; + if (idString.contains(SLASH_UNDERSCORE)) { + idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); + } + SingleTreeNode singleTreeNode = new SingleTreeNode(); + StringType treeNodeId = new StringType(); + treeNodeId.setValue(idString); + singleTreeNode.setTreeNodeId(treeNodeId); + singleTreeNode.setTreeNode(treeNode); + return singleTreeNode; + } + private TreeNode makeNode(String id, String label, Location node, String parentId) { TreeNode treenode = getNode(id); if (treenode == null) { treenode = new TreeNode(); StringType nodeId = new StringType(); - String idString = (String) id; + String idString = id; if (idString.contains(SLASH_UNDERSCORE)) { idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); } - nodeId.setValue((String) idString); + nodeId.setValue(idString); treenode.setNodeId(nodeId); StringType labelString = new StringType(); labelString.setValue(label); From 402512d176a066e7114a777e203343c6d4c9cfcd Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Wed, 27 Sep 2023 20:20:35 +0300 Subject: [PATCH 2/4] Refactor location hieranchy processing for optimization --- .../model/location/LocationHierarchyTree.java | 33 +++-- .../smartregister/model/location/Tree.java | 123 +++++++----------- .../model/location/TreeNode.java | 47 ++++--- .../java/org/smartregister/utils/Utils.java | 14 ++ .../location/LocationHierarchyTreeTest.java | 63 +++++---- .../model/location/utils/TestUtils.java | 46 +++++++ 6 files changed, 187 insertions(+), 139 deletions(-) create mode 100644 src/main/java/org/smartregister/utils/Utils.java create mode 100644 src/test/java/org/smartregister/model/location/utils/TestUtils.java diff --git a/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java b/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java index 49e4032..721b596 100755 --- a/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java +++ b/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java @@ -18,17 +18,19 @@ import ca.uhn.fhir.model.api.annotation.Child; import ca.uhn.fhir.model.api.annotation.DatatypeDef; import ca.uhn.fhir.util.ElementUtil; +import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.api.ICompositeType; import org.hl7.fhir.r4.model.Location; import org.hl7.fhir.r4.model.StringType; import org.hl7.fhir.r4.model.Type; -import org.apache.commons.lang3.StringUtils; import java.util.List; +import java.util.logging.Logger; @DatatypeDef(name = "LocationHierarchyTree") public class LocationHierarchyTree extends Type implements ICompositeType { + private static final Logger logger = Logger.getLogger(LocationHierarchyTree.class.getSimpleName()); @Child(name = "locationsHierarchy") private Tree locationsHierarchy; @@ -36,19 +38,17 @@ public LocationHierarchyTree() { this.locationsHierarchy = new Tree(); } - public void addLocation(Location l) { + public void addLocation(Location location) { StringType idString = new StringType(); - idString.setValue(l.getId()); - if (!locationsHierarchy.hasNode(idString.getValue())) { - if (l.getPartOf() == null || StringUtils.isEmpty(l.getPartOf().getReference())) { - locationsHierarchy.addNode(idString.getValue(), l.getName(), l, null); - } else { - // get Parent Location - StringType parentId = new StringType(); - parentId.setValue(l.getPartOf().getReference()); - locationsHierarchy.addNode( - idString.getValue(), l.getName(), l, parentId.getValue()); - } + idString.setValue(location.getId()); + if (location.getPartOf() == null || StringUtils.isEmpty(location.getPartOf().getReference())) { + locationsHierarchy.addNode(idString.getValue(), location.getName(), location, null); + } else { + // get Parent Location + StringType parentId = new StringType(); + parentId.setValue(location.getPartOf().getReference()); + locationsHierarchy.addNode( + idString.getValue(), location.getName(), location, parentId.getValue()); } } @@ -58,7 +58,12 @@ public void addLocation(Location l) { * @param locations */ public void buildTreeFromList(List locations) { - locations.parallelStream().forEach(location -> addLocation(location)); + long start = System.currentTimeMillis(); + for (Location location : locations) { + addLocation(location); + } + logger.info(String.format("LocationHierarchyTree.buildTreeFromList took : %d ms", System.currentTimeMillis() - start)); + } public Tree getLocationsHierarchy() { diff --git a/src/main/java/org/smartregister/model/location/Tree.java b/src/main/java/org/smartregister/model/location/Tree.java index 1b4830a..9628993 100755 --- a/src/main/java/org/smartregister/model/location/Tree.java +++ b/src/main/java/org/smartregister/model/location/Tree.java @@ -23,14 +23,17 @@ import org.hl7.fhir.r4.model.Location; import org.hl7.fhir.r4.model.StringType; import org.hl7.fhir.r4.model.Type; +import org.smartregister.utils.Utils; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Logger; -import static org.smartregister.utils.Constants.SLASH_UNDERSCORE; - @DatatypeDef(name = "Tree") public class Tree extends Type implements ICompositeType { @@ -49,7 +52,7 @@ public class Tree extends Type implements ICompositeType { summary = false) private List parentChildren; - private static Logger logger = Logger.getLogger(Tree.class.getSimpleName()); + private static final Logger logger = Logger.getLogger(Tree.class.getSimpleName()); public SingleTreeNode getTree() { return listOfNodes; @@ -60,44 +63,22 @@ public Tree() { parentChildren = new ArrayList<>(); } - private void addToParentChildRelation(String parent, String id) { + private void addToParentChildRelation(String parentId, String id) { if (parentChildren == null) { - parentChildren = new ArrayList<>(); - } - List kids = null; - if (parentChildren != null) { - for (int i = 0; i < parentChildren.size(); i++) { - kids = - parentChildren.get(i) != null - && parentChildren.get(i).getIdentifier() != null - && StringUtils.isNotBlank( - parentChildren.get(i).getIdentifier().getValue()) - && parentChildren - .get(i) - .getIdentifier() - .getValue() - .equals(parent) - ? parentChildren.get(i).getChildIdentifiers() - : null; - logger.info("Kids are : " + kids); - if (kids != null) { - break; - } - } + parentChildren = new CopyOnWriteArrayList<>(); } + List kids = getChildrenIdsByParentId(parentId); + if (kids == null) { kids = new ArrayList<>(); } StringType idStringType = new StringType(); - String idString = id; - if (idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } + String idString = Utils.cleanIdString(id); idStringType.setValue(idString); StringType parentStringType = new StringType(); - parentStringType.setValue(parent); + parentStringType.setValue(parentId); kids.add(idStringType); AtomicReference setParentChildMap = new AtomicReference<>(false); @@ -105,10 +86,10 @@ private void addToParentChildRelation(String parent, String id) { parentChildren.parallelStream().filter(parentChildrenMap -> parentChildrenMap != null && parentChildrenMap.getIdentifier() != null && StringUtils.isNotBlank(parentChildrenMap.getIdentifier().getValue()) - && parentChildrenMap.getIdentifier().getValue().equals(parent)).forEach(parentChildrenMap -> { + && parentChildrenMap.getIdentifier().getValue().equals(parentId)).forEach(innerParentChildrenMap -> { - parentChildrenMap.setChildIdentifiers(finalKids); - setParentChildMap.set(true); + innerParentChildrenMap.setChildIdentifiers(finalKids); + setParentChildMap.set(true); }); @@ -120,44 +101,55 @@ private void addToParentChildRelation(String parent, String id) { } } + private List getChildrenIdsByParentId(String parentId) { + Optional> kidsOptional = parentChildren.parallelStream().filter(parentChildrenMap -> parentChildrenMap != null + && parentChildrenMap.getIdentifier() != null + && StringUtils.isNotBlank(parentChildrenMap.getIdentifier().getValue()) + && parentChildrenMap + .getIdentifier() + .getValue() + .equals(parentId)).map(ParentChildrenMap::getChildIdentifiers).filter(Objects::nonNull).findFirst(); + return kidsOptional.orElse(null); + } + public void addNode(String id, String label, Location node, String parentId) { if (listOfNodes == null) { listOfNodes = new SingleTreeNode(); } - // if node exists we should break since user should write optimized code and also tree can - // not have duplicates - if (hasNode(id)) { - throw new IllegalArgumentException("Node with ID " + id + " already exists in tree"); - } + // We only add node if it doesn't already exist, else log as an exception + TreeNode treenode = getNode(id); - TreeNode treeNode = makeNode(id, label, node, parentId); + if (treenode == null) { + TreeNode treeNode = makeNode(id, null, label, node, parentId); - if (parentId != null) { - addToParentChildRelation(parentId, id); + if (parentId != null) { - TreeNode parentNode = getNode(parentId); + addToParentChildRelation(parentId, id); + TreeNode parentNode = getNode(parentId); - // if parent exists add to it otherwise add as root for now - if (parentNode != null) { - parentNode.addChild(treeNode); + // if parent exists add to it otherwise add as root for now + if (parentNode != null) { + parentNode.addChild(treeNode); + } else { + // if no parent exists add it as root node + SingleTreeNode singleTreeNode = getSingleTreeNode(id, treeNode); + listOfNodes = singleTreeNode; + } } else { - // if no parent exists add it as root node + // if no parent add it as root node SingleTreeNode singleTreeNode = getSingleTreeNode(id, treeNode); listOfNodes = singleTreeNode; } + } else { - // if no parent add it as root node - SingleTreeNode singleTreeNode = getSingleTreeNode(id, treeNode); - listOfNodes = singleTreeNode; + logger.severe("Node with ID " + id + " already exists in tree"); } } private static SingleTreeNode getSingleTreeNode(String id, TreeNode treeNode) { String idString = id; - if (idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } + idString = Utils.cleanIdString(idString); SingleTreeNode singleTreeNode = new SingleTreeNode(); StringType treeNodeId = new StringType(); treeNodeId.setValue(idString); @@ -166,15 +158,11 @@ private static SingleTreeNode getSingleTreeNode(String id, TreeNode treeNode) { return singleTreeNode; } - private TreeNode makeNode(String id, String label, Location node, String parentId) { - TreeNode treenode = getNode(id); + private TreeNode makeNode(String currentNodeId, TreeNode treenode, String label, Location node, String parentId) { if (treenode == null) { treenode = new TreeNode(); StringType nodeId = new StringType(); - String idString = id; - if (idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } + String idString = Utils.cleanIdString(currentNodeId); nodeId.setValue(idString); treenode.setNodeId(nodeId); StringType labelString = new StringType(); @@ -182,24 +170,17 @@ private TreeNode makeNode(String id, String label, Location node, String parentI treenode.setLabel(labelString); treenode.setNode(node); StringType parentIdString = new StringType(); - String parentIdStringVar = parentId; - - if (parentIdStringVar != null && parentIdStringVar.contains(SLASH_UNDERSCORE)) { - parentIdStringVar = - parentIdStringVar.substring(0, parentIdStringVar.indexOf(SLASH_UNDERSCORE)); - } + String parentIdStringVar = Utils.cleanIdString(parentId); parentIdString.setValue(parentIdStringVar); treenode.setParent(parentIdString); } return treenode; } + @Nullable public TreeNode getNode(String id) { // Check if id is any root node - String idString = id; - if (idString != null && idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } + String idString = Utils.cleanIdString(id); if (listOfNodes.getTreeNodeId() != null && StringUtils.isNotBlank(listOfNodes.getTreeNodeId().getValue()) @@ -214,10 +195,6 @@ public TreeNode getNode(String id) { return null; } - public boolean hasNode(String id) { - return getNode(id) != null; - } - public SingleTreeNode getListOfNodes() { return listOfNodes; } diff --git a/src/main/java/org/smartregister/model/location/TreeNode.java b/src/main/java/org/smartregister/model/location/TreeNode.java index 4bc3aff..2c5ded3 100755 --- a/src/main/java/org/smartregister/model/location/TreeNode.java +++ b/src/main/java/org/smartregister/model/location/TreeNode.java @@ -20,13 +20,14 @@ import ca.uhn.fhir.util.ElementUtil; import org.apache.commons.lang3.StringUtils; import org.hl7.fhir.instance.model.api.ICompositeType; -import org.hl7.fhir.r4.model.*; +import org.hl7.fhir.r4.model.Location; +import org.hl7.fhir.r4.model.StringType; +import org.hl7.fhir.r4.model.Type; +import org.smartregister.utils.Utils; import java.util.ArrayList; import java.util.List; -import static org.smartregister.utils.Constants.SLASH_UNDERSCORE; - @DatatypeDef(name = "TreeNode") public class TreeNode extends Type implements ICompositeType { @@ -185,30 +186,28 @@ public void addChild(TreeNode node) { children.add(childTreeNode); } - public TreeNode findChild(String id) { - String idString = (String) id; - if (idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } - if (children != null && children.size() > 0) { - for (int i = 0; i < children.size(); i++) { - if (children.get(i) != null) { - for (ChildTreeNode child : children) { - if (child != null - && child.getChildren() != null - && child.getChildren().getNodeId() != null - && StringUtils.isNotBlank( - child.getChildren().getNodeId().getValue()) - && child.getChildren().getNodeId().getValue().equals(idString)) { - return child.getChildren(); - } else if (child != null && child != null) { - TreeNode node = child.getChildren().findChild(idString); - if (node != null) return node; - } - } + public TreeNode findChild(String childId) { + + String idString = Utils.cleanIdString(childId); + if (children != null && !children.isEmpty()) { + for (ChildTreeNode child : children) { + if (isChildFound(child, idString)) { + return child.getChildren(); + } else if (child != null && child.getChildren() != null) { + TreeNode node = child.getChildren().findChild(idString); + if (node != null) return node; } } } return null; } + + private static boolean isChildFound(ChildTreeNode child, String idString) { + return child != null + && child.getChildren() != null + && child.getChildren().getNodeId() != null + && StringUtils.isNotBlank( + child.getChildren().getNodeId().getValue()) + && child.getChildren().getNodeId().getValue().equals(idString); + } } diff --git a/src/main/java/org/smartregister/utils/Utils.java b/src/main/java/org/smartregister/utils/Utils.java new file mode 100644 index 0000000..c76d876 --- /dev/null +++ b/src/main/java/org/smartregister/utils/Utils.java @@ -0,0 +1,14 @@ +package org.smartregister.utils; + +import org.apache.commons.lang3.StringUtils; + +import static org.smartregister.utils.Constants.SLASH_UNDERSCORE; + +public class Utils { + public static String cleanIdString(String idString) { + if (StringUtils.isNotBlank(idString) && idString.contains(SLASH_UNDERSCORE)) { + idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); + } + return idString; + } +} diff --git a/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java b/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java index 3cbedb3..4c2e3ad 100644 --- a/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java +++ b/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java @@ -18,6 +18,7 @@ import org.hl7.fhir.r4.model.Location; import org.hl7.fhir.r4.model.Reference; import org.junit.Test; +import org.smartregister.model.location.utils.TestUtils; import java.util.ArrayList; import java.util.List; @@ -51,41 +52,16 @@ public void testAddLocationWithoutChildLocations() { @Test public void testBuildTreeFromList() { - Location location1 = new Location(); - location1.setId("Location/1"); - location1.setName("Test Location"); - Reference partOfReference = new Reference(); - partOfReference.setReference(""); - location1.setPartOf(partOfReference); - - Location location2 = new Location(); - location2.setId("Location/2"); - location2.setName("Test Location 2"); - partOfReference = new Reference(); - partOfReference.setReference("Location/1"); - location2.setPartOf(partOfReference); - - Location location3 = new Location(); - location3.setId("Location/3"); - location3.setName("Test Location 3"); - partOfReference = new Reference(); - partOfReference.setReference("Location/2"); - location3.setPartOf(partOfReference); - + List locationList = TestUtils.getTestLocations(); LocationHierarchyTree locationHierarchyTree = new LocationHierarchyTree(); - - List locationList = new ArrayList<>(); - locationList.add(location1); - locationList.add(location2); - locationList.add(location3); - locationHierarchyTree.buildTreeFromList(locationList); + Tree tree = locationHierarchyTree.getLocationsHierarchy(); assertNotNull(tree); assertNotNull(tree.getTree()); assertEquals("Location/1", tree.getTree().getTreeNodeId().getValue()); assertEquals("Location/1", tree.getTree().getTreeNode().getNodeId().getValue()); - assertEquals("Test Location", tree.getTree().getTreeNode().getLabel().getValue()); + assertEquals("Test Location 1", tree.getTree().getTreeNode().getLabel().getValue()); assertNull(tree.getTree().getTreeNode().getParent().getValue()); assertEquals(1, tree.getTree().getTreeNode().getChildren().size()); @@ -248,4 +224,35 @@ public void testBuildTreeFromList() { .get(0) .getValue()); } + + private static List getLocationList() { + Location location1 = new Location(); + location1.setId("Location/1"); + location1.setName("Test Location 1"); + Reference partOfReference = new Reference(); + partOfReference.setReference(""); + location1.setPartOf(partOfReference); + + Location location2 = new Location(); + location2.setId("Location/2"); + location2.setName("Test Location 2"); + partOfReference = new Reference(); + partOfReference.setReference("Location/1"); + location2.setPartOf(partOfReference); + + Location location3 = new Location(); + location3.setId("Location/3"); + location3.setName("Test Location 3"); + partOfReference = new Reference(); + partOfReference.setReference("Location/2"); + location3.setPartOf(partOfReference); + + List locationList = new ArrayList<>(); + locationList.add(location1); + locationList.add(location2); + locationList.add(location3); + + + return locationList; + } } diff --git a/src/test/java/org/smartregister/model/location/utils/TestUtils.java b/src/test/java/org/smartregister/model/location/utils/TestUtils.java new file mode 100644 index 0000000..d5c03f0 --- /dev/null +++ b/src/test/java/org/smartregister/model/location/utils/TestUtils.java @@ -0,0 +1,46 @@ +package org.smartregister.model.location.utils; + +import org.hl7.fhir.r4.model.Location; +import org.hl7.fhir.r4.model.Reference; + +import java.util.ArrayList; +import java.util.List; + +public class TestUtils { + + public static final int TOTAL_LOCATIONS = 50000;// Generate 50K locations + public static final int TOTAL_CHILD_LOCATIONS_PER_NODE = 5; //Total sub-locations per location + public static List getTestLocations() { + + List locationList = new ArrayList<>(); + + + int parentId = 1; + + for (Integer i = 1; i < TOTAL_LOCATIONS + 1; i++) { + + if(i == 1){ + parentId = 1; + } else if(i % TOTAL_CHILD_LOCATIONS_PER_NODE == 0 ){ + parentId++; + } + + locationList.add(getLocation("Location/" + i, "Test Location " + i, (i == 1 ? null : "Location/" + parentId))); + } + + return locationList; + } + + private static Location getLocation(String id, String name, String reference) { + + Location location = new Location(); + location.setId(id); + location.setName(name); + + Reference partOfReference = new Reference(); + partOfReference.setReference(reference); + location.setPartOf(partOfReference); + + return location; + } +} From fd7c3b8859173bf03f0eb19947e4e6902e06b93c Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Wed, 27 Sep 2023 20:28:23 +0300 Subject: [PATCH 3/4] Optimization PR Clean up --- .../smartregister/model/location/LocationHierarchyTree.java | 3 --- src/main/java/org/smartregister/model/location/Tree.java | 5 +---- .../model/location/LocationHierarchyTreeTest.java | 4 +--- .../org/smartregister/model/location/utils/TestUtils.java | 3 +-- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java b/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java index 721b596..48b8d24 100755 --- a/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java +++ b/src/main/java/org/smartregister/model/location/LocationHierarchyTree.java @@ -58,12 +58,9 @@ public void addLocation(Location location) { * @param locations */ public void buildTreeFromList(List locations) { - long start = System.currentTimeMillis(); for (Location location : locations) { addLocation(location); } - logger.info(String.format("LocationHierarchyTree.buildTreeFromList took : %d ms", System.currentTimeMillis() - start)); - } public Tree getLocationsHierarchy() { diff --git a/src/main/java/org/smartregister/model/location/Tree.java b/src/main/java/org/smartregister/model/location/Tree.java index 9628993..420fdb6 100755 --- a/src/main/java/org/smartregister/model/location/Tree.java +++ b/src/main/java/org/smartregister/model/location/Tree.java @@ -32,7 +32,6 @@ import java.util.Optional; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicReference; -import java.util.logging.Logger; @DatatypeDef(name = "Tree") public class Tree extends Type implements ICompositeType { @@ -52,8 +51,6 @@ public class Tree extends Type implements ICompositeType { summary = false) private List parentChildren; - private static final Logger logger = Logger.getLogger(Tree.class.getSimpleName()); - public SingleTreeNode getTree() { return listOfNodes; } @@ -143,7 +140,7 @@ public void addNode(String id, String label, Location node, String parentId) { } } else { - logger.severe("Node with ID " + id + " already exists in tree"); + throw new IllegalArgumentException("Node with ID " + id + " already exists in tree"); } } diff --git a/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java b/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java index 4c2e3ad..960edf4 100644 --- a/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java +++ b/src/test/java/org/smartregister/model/location/LocationHierarchyTreeTest.java @@ -18,7 +18,6 @@ import org.hl7.fhir.r4.model.Location; import org.hl7.fhir.r4.model.Reference; import org.junit.Test; -import org.smartregister.model.location.utils.TestUtils; import java.util.ArrayList; import java.util.List; @@ -52,10 +51,9 @@ public void testAddLocationWithoutChildLocations() { @Test public void testBuildTreeFromList() { - List locationList = TestUtils.getTestLocations(); + List locationList = getLocationList(); LocationHierarchyTree locationHierarchyTree = new LocationHierarchyTree(); locationHierarchyTree.buildTreeFromList(locationList); - Tree tree = locationHierarchyTree.getLocationsHierarchy(); assertNotNull(tree); assertNotNull(tree.getTree()); diff --git a/src/test/java/org/smartregister/model/location/utils/TestUtils.java b/src/test/java/org/smartregister/model/location/utils/TestUtils.java index d5c03f0..6525a6e 100644 --- a/src/test/java/org/smartregister/model/location/utils/TestUtils.java +++ b/src/test/java/org/smartregister/model/location/utils/TestUtils.java @@ -14,10 +14,9 @@ public static List getTestLocations() { List locationList = new ArrayList<>(); - int parentId = 1; - for (Integer i = 1; i < TOTAL_LOCATIONS + 1; i++) { + for (int i = 1; i < TOTAL_LOCATIONS + 1; i++) { if(i == 1){ parentId = 1; From a00cbfdfb84c2c323ad1998ab12b65edee7ce8a4 Mon Sep 17 00:00:00 2001 From: Martin Ndegwa Date: Thu, 28 Sep 2023 11:24:49 +0300 Subject: [PATCH 4/4] Clean up --- src/main/java/org/smartregister/utils/Utils.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/smartregister/utils/Utils.java b/src/main/java/org/smartregister/utils/Utils.java index c76d876..4c86c4c 100644 --- a/src/main/java/org/smartregister/utils/Utils.java +++ b/src/main/java/org/smartregister/utils/Utils.java @@ -2,13 +2,9 @@ import org.apache.commons.lang3.StringUtils; -import static org.smartregister.utils.Constants.SLASH_UNDERSCORE; - public class Utils { public static String cleanIdString(String idString) { - if (StringUtils.isNotBlank(idString) && idString.contains(SLASH_UNDERSCORE)) { - idString = idString.substring(0, idString.indexOf(SLASH_UNDERSCORE)); - } - return idString; + return StringUtils.isNotBlank(idString) && idString.contains(Constants.SLASH_UNDERSCORE) ? + idString.substring(0, idString.indexOf(Constants.SLASH_UNDERSCORE)) : idString; } }