From 8b31f8b67ff010088874cad085b5af6643bd4ed9 Mon Sep 17 00:00:00 2001 From: shahid Date: Tue, 8 Oct 2024 21:12:33 +0500 Subject: [PATCH 1/8] NMS-16536, initial commit with fixes --- .../netmgt/provision/service/DefaultProvisionService.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java index a94884e4db13..c21f8d261116 100644 --- a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java +++ b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java @@ -1033,6 +1033,12 @@ private boolean handleCategoryChanges(final OnmsNode dbNode) { } } + // If any categories were removed or new ones are to be added, reset the cache + if (changed || !categories.isEmpty()) { + // attributes changed, resetting cache + m_categoryCache.set(loadCategoryMap()); + } + // the remainder of requisitioned categories get added for (final String cat : categories) { m_categoriesAdded.add(cat); From d7ec2555a9f9a9aa25ac88fa864bf5de8b9c71ed Mon Sep 17 00:00:00 2001 From: shahid Date: Mon, 14 Oct 2024 22:38:53 +0500 Subject: [PATCH 2/8] NMS-16536, additional logs added to trace the issue, condition updated to avoid frequent reloading --- .../service/DefaultProvisionService.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) mode change 100644 => 100755 opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java diff --git a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java old mode 100644 new mode 100755 index c21f8d261116..45100e44ea61 --- a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java +++ b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java @@ -46,6 +46,7 @@ import java.util.TreeSet; import java.util.stream.Collectors; +import com.fasterxml.jackson.databind.ObjectMapper; import org.joda.time.DateTime; import org.joda.time.Duration; import org.opennms.core.spring.BeanUtils; @@ -1033,10 +1034,16 @@ private boolean handleCategoryChanges(final OnmsNode dbNode) { } } - // If any categories were removed or new ones are to be added, reset the cache - if (changed || !categories.isEmpty()) { + // If some categories were removed or there are new to be added, reset the cache. + if (m_categoryCache.get() != null && (changed || !categories.isEmpty())) { + LOG.debug("Categories added: {}", categories); + LOG.debug("Categories removed: {}", m_categoriesDeleted); + LOG.debug("Current categories in cache"); + logCache(m_categoryCache.get()); // attributes changed, resetting cache m_categoryCache.set(loadCategoryMap()); + LOG.debug("Reloaded Categories in cache"); + logCache(m_categoryCache.get()); } // the remainder of requisitioned categories get added @@ -1105,6 +1112,25 @@ protected OnmsNode doInsert() { } + private void logCache(Map categoryMap) { + try { + List> resultList = new ArrayList<>(); + + // Iterate over the map and extract key and m_id + for (Map.Entry entry : categoryMap.entrySet()) { + Map jsonEntry = new HashMap<>(); + jsonEntry.put(entry.getKey(), String.valueOf(entry.getValue().getId())); + resultList.add(jsonEntry); + } + + // Convert the list to JSON + String json = new ObjectMapper().writeValueAsString(resultList); + LOG.debug("Categories: {}", json); + } catch (com.fasterxml.jackson.core.JsonProcessingException e) { + LOG.error("Error serializing category cache to JSON", e); + } + } + public Set getCategoriesForNode(final OnmsNode node) { final TreeSet categories = new TreeSet<>(); for (final OnmsCategory cat : node.getCategories()) { From 76512c200a3aa459e35cf33a839f19db5d8a1115 Mon Sep 17 00:00:00 2001 From: shahid Date: Thu, 17 Oct 2024 01:51:09 +0500 Subject: [PATCH 3/8] NMS-16536, added unit test, removed extra logging, added for bug tracing. --- .../opennms-provisiond/src/main/resources/beanRefContext.xml | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 opennms-provision/opennms-provisiond/src/main/resources/beanRefContext.xml diff --git a/opennms-provision/opennms-provisiond/src/main/resources/beanRefContext.xml b/opennms-provision/opennms-provisiond/src/main/resources/beanRefContext.xml old mode 100644 new mode 100755 From 296bd4c13541264191b9d2ed8a98129bde36dc71 Mon Sep 17 00:00:00 2001 From: shahid Date: Thu, 17 Oct 2024 02:37:25 +0500 Subject: [PATCH 4/8] NMS-16536, added unit test, removed extra logging, added for bug tracing. --- .../service/DefaultProvisionService.java | 32 ++------ .../service/DefaultProvisionServiceTest.java | 76 +++++++++++++++++++ 2 files changed, 81 insertions(+), 27 deletions(-) mode change 100644 => 100755 opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java diff --git a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java index 45100e44ea61..701385f12667 100755 --- a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java +++ b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java @@ -948,6 +948,10 @@ public void setForeignSourceRepository(final ForeignSourceRepository foreignSour m_foreignSourceRepository = foreignSourceRepository; } + public void setCategoriesInCache(final Map categoryCache) { + m_categoryCache.set(categoryCache); + } + /** *

getForeignSourceRepository

* @@ -1036,14 +1040,7 @@ private boolean handleCategoryChanges(final OnmsNode dbNode) { // If some categories were removed or there are new to be added, reset the cache. if (m_categoryCache.get() != null && (changed || !categories.isEmpty())) { - LOG.debug("Categories added: {}", categories); - LOG.debug("Categories removed: {}", m_categoriesDeleted); - LOG.debug("Current categories in cache"); - logCache(m_categoryCache.get()); - // attributes changed, resetting cache - m_categoryCache.set(loadCategoryMap()); - LOG.debug("Reloaded Categories in cache"); - logCache(m_categoryCache.get()); + m_categoryCache.set(loadCategoryMap()); } // the remainder of requisitioned categories get added @@ -1112,25 +1109,6 @@ protected OnmsNode doInsert() { } - private void logCache(Map categoryMap) { - try { - List> resultList = new ArrayList<>(); - - // Iterate over the map and extract key and m_id - for (Map.Entry entry : categoryMap.entrySet()) { - Map jsonEntry = new HashMap<>(); - jsonEntry.put(entry.getKey(), String.valueOf(entry.getValue().getId())); - resultList.add(jsonEntry); - } - - // Convert the list to JSON - String json = new ObjectMapper().writeValueAsString(resultList); - LOG.debug("Categories: {}", json); - } catch (com.fasterxml.jackson.core.JsonProcessingException e) { - LOG.error("Error serializing category cache to JSON", e); - } - } - public Set getCategoriesForNode(final OnmsNode node) { final TreeSet categories = new TreeSet<>(); for (final OnmsCategory cat : node.getCategories()) { diff --git a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java old mode 100644 new mode 100755 index 26fcdb5223c7..718394a28614 --- a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java +++ b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java @@ -29,11 +29,15 @@ package org.opennms.netmgt.provision.service; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.argThat; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.HashSet; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -60,6 +64,7 @@ import org.opennms.netmgt.provision.persist.OnmsNodeRequisition; import org.opennms.netmgt.provision.persist.requisition.RequisitionCategory; import org.opennms.netmgt.provision.persist.requisition.RequisitionNode; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.transaction.PlatformTransactionManager; import com.google.common.collect.Sets; @@ -198,4 +203,75 @@ private Set checkSetsOfCategories(Set reqCats, Set polCa .map(OnmsCategory::getName) .collect(Collectors.toSet()); } + + @Test + public void testNMS16536() { + + final ForeignSourceRepository foreignSourceRepository = mock(ForeignSourceRepository.class); + final RequisitionedCategoryAssociationDao requisitionedCategoryAssociationDao = + mock(RequisitionedCategoryAssociationDao.class); + + m_provisionService.setForeignSourceRepository(foreignSourceRepository); + m_provisionService.setCategoryAssociationDao(requisitionedCategoryAssociationDao); + + final OnmsNode node = new OnmsNode(); + node.setForeignId("foreignId"); + node.setForeignSource("foreignSource"); + + final Set categories = new HashSet<>(); + categories.add(createOnmsCategory(1, "foops")); + categories.add(createOnmsCategory(2, "blarbs")); + categories.add(createOnmsCategory(3, "NMS-16536")); + + m_provisionService.setCategoriesInCache(categories.stream() + .collect(Collectors.toMap(OnmsCategory::getName, category -> category))); + + OnmsCategory deletedCategory = createOnmsCategory(3, "NMS-16536"); + OnmsCategory createdCategory = createOnmsCategory(4, "NMS-16536"); + + node.setCategories(categories); + node.setRequisitionedCategories(categories.stream().map(OnmsCategory::getName).collect(Collectors.toSet())); + node.setId(1); + node.setLabel("myNode"); + + when(m_nodeDao.get(node.getId())).thenReturn(node); + + when(requisitionedCategoryAssociationDao.findByNodeId(node.getId())) + .thenReturn(node.getRequisitionedCategories().stream() + .filter(cat -> !cat.equals(deletedCategory.getName())) + .map(c -> new RequisitionedCategoryAssociation(node, new OnmsCategory(c))) + .collect(Collectors.toList())); + + when(m_categoryDao.findAll()).thenReturn(categories.stream() + .map(category -> { + if (deletedCategory.getName().equals(category.getName())) { + return createdCategory; + } else { + return category; + } + }) + .collect(Collectors.toList())); + + doThrow(new DataIntegrityViolationException("Foreign key violation for categoryId=" + deletedCategory.getId())) + .when(requisitionedCategoryAssociationDao) + .saveOrUpdate(argThat(association -> + association.getCategory() != null + && association.getCategory().getId().equals(deletedCategory.getId()))); + + m_provisionService.updateNodeAttributes(node); + + verify(requisitionedCategoryAssociationDao, times(1)) + .findByNodeId(node.getId()); + verify(requisitionedCategoryAssociationDao, times(1)) + .saveOrUpdate(argThat(association -> + Objects.equals(association.getCategory().getId(), createdCategory.getId()) + )); + } + + private OnmsCategory createOnmsCategory(Integer id, String name) { + final OnmsCategory category = new OnmsCategory(); + category.setId(id); + category.setName(name); + return category; + } } From ad2fc6d7a3d4a52465c23a957704350ae124cb39 Mon Sep 17 00:00:00 2001 From: shahid Date: Thu, 17 Oct 2024 03:11:53 +0500 Subject: [PATCH 5/8] NMS-16536, optimized imports, reverted file permission --- .../netmgt/provision/service/DefaultProvisionService.java | 1 - .../opennms-provisiond/src/main/resources/beanRefContext.xml | 0 2 files changed, 1 deletion(-) mode change 100755 => 100644 opennms-provision/opennms-provisiond/src/main/resources/beanRefContext.xml diff --git a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java index 701385f12667..14b715666f80 100755 --- a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java +++ b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java @@ -46,7 +46,6 @@ import java.util.TreeSet; import java.util.stream.Collectors; -import com.fasterxml.jackson.databind.ObjectMapper; import org.joda.time.DateTime; import org.joda.time.Duration; import org.opennms.core.spring.BeanUtils; diff --git a/opennms-provision/opennms-provisiond/src/main/resources/beanRefContext.xml b/opennms-provision/opennms-provisiond/src/main/resources/beanRefContext.xml old mode 100755 new mode 100644 From adf560185c7252f3eead27e3ce075b65d8e5c070 Mon Sep 17 00:00:00 2001 From: shahid Date: Fri, 18 Oct 2024 23:43:19 +0500 Subject: [PATCH 6/8] NMS-16536, reverted files permissions --- .../opennms/netmgt/provision/service/DefaultProvisionService.java | 0 .../netmgt/provision/service/DefaultProvisionServiceTest.java | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java mode change 100755 => 100644 opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java diff --git a/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java b/opennms-provision/opennms-provisiond/src/main/java/org/opennms/netmgt/provision/service/DefaultProvisionService.java old mode 100755 new mode 100644 diff --git a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/DefaultProvisionServiceTest.java old mode 100755 new mode 100644 From 5b36d37e5836f0b706ccd16eb059000568fd15f3 Mon Sep 17 00:00:00 2001 From: shahid Date: Fri, 25 Oct 2024 22:37:07 +0500 Subject: [PATCH 7/8] NMS-16536, added IT for review --- .../provision/service/ProvisionerIT.java | 52 +++++++++++++++++++ ...isioner-testCategories-ThreeCategories.xml | 11 ++++ 2 files changed, 63 insertions(+) create mode 100644 opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories.xml diff --git a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java index 0679eae809f5..c2295c7ed865 100644 --- a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java +++ b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java @@ -44,6 +44,7 @@ import java.util.Collections; import java.util.Date; import java.util.List; +import java.util.Objects; import java.util.Properties; import java.util.concurrent.ExecutionException; @@ -76,6 +77,7 @@ import org.opennms.netmgt.dao.api.MonitoredServiceDao; import org.opennms.netmgt.dao.api.MonitoringLocationDao; import org.opennms.netmgt.dao.api.NodeDao; +import org.opennms.netmgt.dao.api.RequisitionedCategoryAssociationDao; import org.opennms.netmgt.dao.api.ServiceTypeDao; import org.opennms.netmgt.dao.api.SnmpInterfaceDao; import org.opennms.netmgt.dao.mock.EventAnticipator; @@ -98,6 +100,7 @@ import org.opennms.netmgt.model.OnmsNode.NodeLabelSource; import org.opennms.netmgt.model.OnmsServiceType; import org.opennms.netmgt.model.OnmsSnmpInterface; +import org.opennms.netmgt.model.RequisitionedCategoryAssociation; import org.opennms.netmgt.model.events.EventBuilder; import org.opennms.netmgt.model.events.NodeLabelChangedEventBuilder; import org.opennms.netmgt.model.monitoringLocations.OnmsMonitoringLocation; @@ -185,6 +188,9 @@ public class ProvisionerIT extends ProvisioningITCase implements InitializingBea @Autowired private CategoryDao m_categoryDao; + @Autowired + private RequisitionedCategoryAssociationDao m_categoryAssociationDao; + @Autowired private MonitoringLocationDao m_locationDao; @@ -1797,6 +1803,52 @@ public void testRequisitionedCategoriesThenUpdateRequisitionToRemoveCategory() t assertEquals(0, n.getCategories().size()); } + @Test(timeout = 300000) + public void testRequisitionedCategoriesThenRemoveCategoryByApi() throws Exception { + final int nextNodeId = m_nodeDao.getNextNodeId(); + + importFromResource("classpath:/provisioner-testCategories-ThreeCategories.xml", Boolean.TRUE.toString()); + + OnmsNode n = getNodeDao().get(nextNodeId); + assertEquals(3, n.getCategories().size()); + assertTrue(n.hasCategory("NMS-16536")); + + runPendingScans(); + + assertEquals(3, m_categoryAssociationDao.findAll().size()); + + OnmsCategory deleteCategory = n.getCategories().stream() + .filter(cat -> cat.getName().equals("NMS-16536")).findFirst().orElse(null); + m_categoryDao.delete(deleteCategory); + m_categoryDao.flush(); + + assertEquals(2, m_categoryDao.findAll().size()); + + RequisitionedCategoryAssociation deleteCategoryAssociation = m_categoryAssociationDao.findAll().stream() + .filter(cat -> cat.getCategory().getName().equals("NMS-16536")).findFirst().orElse(null); + m_categoryAssociationDao.delete(deleteCategoryAssociation); + m_categoryAssociationDao.flush(); + + assertEquals(2, m_categoryAssociationDao.findAll().size()); + + m_provisionService.updateNodeAttributes(n); + + assertEquals(3, m_categoryAssociationDao.findAll().size()); + + assertTrue(m_categoryAssociationDao.findByNodeId(n.getId()).stream(). + anyMatch(cat -> cat.getCategory().getName().equals(Objects.requireNonNull(deleteCategory).getName()))); + +// assertEquals(3, m_categoryDao.findAll().size()); + +// assertTrue(m_categoryDao.findAll().stream() +// .anyMatch(cat -> cat.getName().equals(Objects.requireNonNull(deleteCategory).getName()) +// && cat.getId() > deleteCategory.getId())); + +// assertTrue(m_categoryAssociationDao.findAll().stream() +// .anyMatch(catAss -> catAss.getCategory().getName().equals(Objects.requireNonNull(deleteCategory).getName()) +// && catAss.getCategory().getId() > deleteCategory.getId())); + } + @Test(timeout = 300000) public void testRequisitionedCategoriesWithUserCategoryThenUpdateRequisitionToRemoveRequisitionedCategory() throws Exception { diff --git a/opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories.xml b/opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories.xml new file mode 100644 index 000000000000..6f710cf39e88 --- /dev/null +++ b/opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories.xml @@ -0,0 +1,11 @@ + + + + + + + + + + + From 59d32fe63d2bd60fdd64d459df69a5156e6db478 Mon Sep 17 00:00:00 2001 From: shahid Date: Fri, 6 Dec 2024 00:26:35 +0500 Subject: [PATCH 8/8] NMS-16536, updated test case --- .../netmgt/provision/service/ProvisionerIT.java | 17 +++++------------ ...estCategories-ThreeCategories-oneDeleted.xml | 10 ++++++++++ 2 files changed, 15 insertions(+), 12 deletions(-) create mode 100755 opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories-oneDeleted.xml diff --git a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java index c2295c7ed865..905bd0fd935b 100644 --- a/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java +++ b/opennms-provision/opennms-provisiond/src/test/java/org/opennms/netmgt/provision/service/ProvisionerIT.java @@ -1831,22 +1831,15 @@ public void testRequisitionedCategoriesThenRemoveCategoryByApi() throws Exceptio assertEquals(2, m_categoryAssociationDao.findAll().size()); + importFromResource("classpath:/provisioner-testCategories-ThreeCategories-oneDeleted.xml", Boolean.TRUE.toString()); + m_provisionService.updateNodeAttributes(n); - assertEquals(3, m_categoryAssociationDao.findAll().size()); + assertEquals(2, m_categoryAssociationDao.findAll().size()); assertTrue(m_categoryAssociationDao.findByNodeId(n.getId()).stream(). - anyMatch(cat -> cat.getCategory().getName().equals(Objects.requireNonNull(deleteCategory).getName()))); - -// assertEquals(3, m_categoryDao.findAll().size()); - -// assertTrue(m_categoryDao.findAll().stream() -// .anyMatch(cat -> cat.getName().equals(Objects.requireNonNull(deleteCategory).getName()) -// && cat.getId() > deleteCategory.getId())); - -// assertTrue(m_categoryAssociationDao.findAll().stream() -// .anyMatch(catAss -> catAss.getCategory().getName().equals(Objects.requireNonNull(deleteCategory).getName()) -// && catAss.getCategory().getId() > deleteCategory.getId())); + noneMatch(cat -> + cat.getCategory().getName().equals(Objects.requireNonNull(deleteCategory).getName()))); } @Test(timeout = 300000) diff --git a/opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories-oneDeleted.xml b/opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories-oneDeleted.xml new file mode 100755 index 000000000000..113100ef2c75 --- /dev/null +++ b/opennms-provision/opennms-provisiond/src/test/resources/provisioner-testCategories-ThreeCategories-oneDeleted.xml @@ -0,0 +1,10 @@ + + + + + + + + + +