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..14b715666f80 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 @@ -947,6 +947,10 @@ public void setForeignSourceRepository(final ForeignSourceRepository foreignSour m_foreignSourceRepository = foreignSourceRepository; } + public void setCategoriesInCache(final Map categoryCache) { + m_categoryCache.set(categoryCache); + } + /** *

getForeignSourceRepository

* @@ -1033,6 +1037,11 @@ 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())) { + m_categoryCache.set(loadCategoryMap()); + } + // the remainder of requisitioned categories get added for (final String cat : categories) { m_categoriesAdded.add(cat); 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 index 26fcdb5223c7..718394a28614 100644 --- 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; + } } 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..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 @@ -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,45 @@ 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()); + + importFromResource("classpath:/provisioner-testCategories-ThreeCategories-oneDeleted.xml", Boolean.TRUE.toString()); + + m_provisionService.updateNodeAttributes(n); + + assertEquals(2, m_categoryAssociationDao.findAll().size()); + + assertTrue(m_categoryAssociationDao.findByNodeId(n.getId()).stream(). + noneMatch(cat -> + cat.getCategory().getName().equals(Objects.requireNonNull(deleteCategory).getName()))); + } + @Test(timeout = 300000) public void testRequisitionedCategoriesWithUserCategoryThenUpdateRequisitionToRemoveRequisitionedCategory() throws Exception { 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 @@ + + + + + + + + + + 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 @@ + + + + + + + + + + +